From 94b8efc01f8f2324c52b6ee02c123c4644d681c4 Mon Sep 17 00:00:00 2001 From: Thorsten Klein Date: Fri, 25 Jan 2019 14:49:14 +0100 Subject: [PATCH 1/8] enhance example to include volumemount and correct env --- .../kubernetes/configmap-sidecar-test.yaml | 24 ++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/examples/kubernetes/configmap-sidecar-test.yaml b/examples/kubernetes/configmap-sidecar-test.yaml index 082a49e..d8a5c59 100644 --- a/examples/kubernetes/configmap-sidecar-test.yaml +++ b/examples/kubernetes/configmap-sidecar-test.yaml @@ -10,12 +10,34 @@ metadata: data: test1: | name: test1 - environment: + env: - name: HELLO value: world + - name: TEST + value: test_that + volumeMounts: + - name: test-vol + mountPath: /tmp/test + volumes: + - name: test-vol + configMap: + name: test-config containers: - name: sidecar-nginx image: nginx:1.12.2 imagePullPolicy: IfNotPresent ports: - containerPort: 80 + env: + - name: ENV_IN_SIDECAR + value: test-in-sidecar +--- +# configmap to test sharing a volume between sidecar and existing container +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-config + namespace: default +data: + test.txt: | + this is some test message shared between containers From ed0148077207e84db24fecd1e9161f25a98941da Mon Sep 17 00:00:00 2001 From: Thorsten Klein Date: Fri, 25 Jan 2019 14:51:49 +0100 Subject: [PATCH 2/8] add funcitonality to inject volumeMounts --- internal/pkg/config/config.go | 11 +++--- pkg/server/webhook.go | 64 +++++++++++++++++++++++++++++++++-- 2 files changed, 67 insertions(+), 8 deletions(-) diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index aa1c4c0..fd3f52f 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -29,10 +29,11 @@ var ( // InjectionConfig is a specific instance of a injected config, for a given annotation type InjectionConfig struct { - Name string `json:"name"` - Containers []corev1.Container `json:"containers"` - Volumes []corev1.Volume `json:"volumes"` - Environment []corev1.EnvVar `json:"env"` + Name string `json:"name"` + Containers []corev1.Container `json:"containers"` + Volumes []corev1.Volume `json:"volumes"` + Environment []corev1.EnvVar `json:"env"` + VolumeMounts []corev1.VolumeMount `json:"volumeMounts"` } // Config is a struct indicating how a given injection should be configured @@ -44,7 +45,7 @@ type Config struct { // String returns a string representation of the config func (c *InjectionConfig) String() string { - return fmt.Sprintf("%s: %d containers, %d volumes, %d environment vars", c.Name, len(c.Containers), len(c.Volumes), len(c.Environment)) + return fmt.Sprintf("%s: %d containers, %d volumes, %d environment vars, %d volume mounts", c.Name, len(c.Containers), len(c.Volumes), len(c.Environment), len(c.VolumeMounts)) } // ReplaceInjectionConfigs will take a list of new InjectionConfigs, and replace the current configuration with them. diff --git a/pkg/server/webhook.go b/pkg/server/webhook.go index 268a7d5..662cde9 100644 --- a/pkg/server/webhook.go +++ b/pkg/server/webhook.go @@ -227,6 +227,41 @@ func addVolumes(target, added []corev1.Volume, basePath string) (patch []patchOp return patch } +func addVolumeMounts(target []corev1.Container, addedVolumeMounts []corev1.VolumeMount) (patch []patchOperation) { + var value interface{} + for containerIndex, container := range target { + // for each container in the spec, determine if we want to patch with any volume mounts + first := len(container.VolumeMounts) == 0 + for _, add := range addedVolumeMounts { + path := fmt.Sprintf("/spec/containers/%d/volumeMounts", containerIndex) + hasKey := false + // make sure we dont override any existing volume mounts; we only add, dont replace + for _, origVolumeMount := range container.VolumeMounts { + if origVolumeMount.Name == add.Name { + hasKey = true + break + } + } + if !hasKey { + // make a patch + value = add + if first { + first = false + value = []corev1.VolumeMount{add} + } else { + path = path + "/-" + } + patch = append(patch, patchOperation{ + Op: "add", + Path: path, + Value: value, + }) + } + } + } + return patch +} + // for containers, add any env vars that are not already defined in the Env list. // this does _not_ return patches; this is intended to be used only on containers defined // in the injection config, so the resources do not exist yet in the k8s api (thus no patch needed) @@ -252,6 +287,28 @@ func mergeEnvVars(envs []corev1.EnvVar, containers []corev1.Container) []corev1. return mutatedContainers } +func mergeVolumeMounts(volumeMounts []corev1.VolumeMount, containers []corev1.Container) []corev1.Container { + mutatedContainers := []corev1.Container{} + for _, c := range containers { + for _, newVolumeMount := range volumeMounts { + // check each container for each volume mount by name. + // if the container has a matching name, dont override! + skip := false + for _, origVolumeMount := range c.VolumeDevices { + if origVolumeMount.Name == newVolumeMount.Name { + skip = true + break + } + } + if !skip { + c.VolumeMounts = append(c.VolumeMounts, newVolumeMount) + } + } + mutatedContainers = append(mutatedContainers, c) + } + return mutatedContainers +} + func updateAnnotations(target map[string]string, added map[string]string) (patch []patchOperation) { for key, value := range added { if target == nil || target[key] == "" { @@ -278,17 +335,18 @@ func updateAnnotations(target map[string]string, added map[string]string) (patch func createPatch(pod *corev1.Pod, inj *config.InjectionConfig, annotations map[string]string) ([]byte, error) { var patch []patchOperation - // first, make sure any injected containers in our config get the EnvVars injected + // first, make sure any injected containers in our config get the EnvVars and VolumeMounts injected // this mutates inj.Containers with our environment vars mutatedInjectedContainers := mergeEnvVars(inj.Environment, inj.Containers) + mutatedInjectedContainers = mergeVolumeMounts(inj.VolumeMounts, mutatedInjectedContainers) // next, patch containers with our injected containers patch = append(patch, addContainers(pod.Spec.Containers, mutatedInjectedContainers, "/spec/containers")...) - // now, patch all existing containers with the env vars + // now, patch all existing containers with the env vars and volume mounts patch = append(patch, setEnvironment(pod.Spec.Containers, inj.Environment)...) + patch = append(patch, addVolumeMounts(pod.Spec.Containers, inj.VolumeMounts)...) // now, add volumes and set annotations patch = append(patch, addVolumes(pod.Spec.Volumes, inj.Volumes, "/spec/volumes")...) patch = append(patch, updateAnnotations(pod.Annotations, annotations)...) - return json.Marshal(patch) } From bfd60ffa37b4eacfd8d47ab9c133df7d27592ce2 Mon Sep 17 00:00:00 2001 From: Thorsten Klein Date: Mon, 28 Jan 2019 11:45:49 +0100 Subject: [PATCH 3/8] add tests --- internal/pkg/config/config.go | 2 +- internal/pkg/config/config_test.go | 43 ++++++++++++++++++- internal/pkg/config/watcher/loader_test.go | 31 +++++++++++++ pkg/server/webhook_test.go | 4 +- .../fixtures/k8s/configmap-volume-mounts.yaml | 43 +++++++++++++++++++ test/fixtures/k8s/object5.yaml | 4 ++ test/fixtures/sidecars/volume-mounts.yaml | 35 +++++++++++++++ 7 files changed, 159 insertions(+), 3 deletions(-) create mode 100644 test/fixtures/k8s/configmap-volume-mounts.yaml create mode 100644 test/fixtures/k8s/object5.yaml create mode 100644 test/fixtures/sidecars/volume-mounts.yaml diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index fd3f52f..962202c 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -45,7 +45,7 @@ type Config struct { // String returns a string representation of the config func (c *InjectionConfig) String() string { - return fmt.Sprintf("%s: %d containers, %d volumes, %d environment vars, %d volume mounts", c.Name, len(c.Containers), len(c.Volumes), len(c.Environment), len(c.VolumeMounts)) + return fmt.Sprintf("%s: %d containers, %d volumes, %d environment vars", c.Name, len(c.Containers), len(c.Volumes), len(c.Environment)) } // ReplaceInjectionConfigs will take a list of new InjectionConfigs, and replace the current configuration with them. diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index 2112fdc..8ac3f92 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -11,10 +11,11 @@ var ( cfg1 = sidecars + "/sidecar-test.yaml" complicatedConfig = sidecars + "/complex-sidecar.yaml" env1 = sidecars + "/env1.yaml" + volumeMounts = sidecars + "/volume-mounts.yaml" ) func TestLoadConfig(t *testing.T) { - expectedNumInjectionsConfig := 3 + expectedNumInjectionsConfig := 4 c, err := LoadConfigDirectory(sidecars) if err != nil { t.Fatal(err) @@ -103,6 +104,46 @@ func TestLoadComplexConfig(t *testing.T) { } } +func TestLoadVolumeMountsConfig(t *testing.T) { + cfg := volumeMounts + c, err := LoadInjectionConfigFromFilePath(cfg) + if err != nil { + t.Fatal(err) + } + expectedName := "volume-mounts" + nExpectedContainers := 2 + nExpectedVolumes := 2 + nExpectedEnvironmentVars := 2 + expectedVolumeMounts := []string{"test-vol"} + + if c.Name != expectedName { + t.Fatalf("expected %s Name loaded from %s but got %s", expectedName, cfg, c.Name) + } + if len(c.Environment) != nExpectedEnvironmentVars { + t.Fatalf("expected %d EnvVars loaded from %s but got %d", nExpectedEnvironmentVars, cfg, len(c.Environment)) + } + if len(c.Containers) != nExpectedContainers { + t.Fatalf("expected %d Containers loaded from %s but got %d", nExpectedContainers, cfg, len(c.Containers)) + } + if len(c.Volumes) != nExpectedVolumes { + t.Fatalf("expected %d Volumes loaded from %s but got %d", nExpectedVolumes, cfg, len(c.Volumes)) + } + for _, expectedVolumeMount := range expectedVolumeMounts { + for _, container := range c.Containers { + volumeMountExists := false + for _, volumeMount := range container.VolumeMounts { + if volumeMount.Name == expectedVolumeMount { + volumeMountExists = true + break + } + } + if !volumeMountExists { + t.Fatalf("did not find expected VolumeMount '%s' in container '%s' loaded from %s", expectedVolumeMount, container.Name, cfg) + } + } + } +} + func TestHasInjectionConfig(t *testing.T) { c, err := LoadConfigDirectory(sidecars) if err != nil { diff --git a/internal/pkg/config/watcher/loader_test.go b/internal/pkg/config/watcher/loader_test.go index f40003d..fa4556c 100644 --- a/internal/pkg/config/watcher/loader_test.go +++ b/internal/pkg/config/watcher/loader_test.go @@ -19,6 +19,7 @@ type injectionConfigExpectation struct { volumeCount int envCount int containerCount int + volumeMounts []string } var ( @@ -30,6 +31,7 @@ var ( volumeCount: 0, envCount: 3, containerCount: 0, + volumeMounts: []string{}, }, }, "configmap-sidecar-test": []injectionConfigExpectation{ @@ -38,6 +40,7 @@ var ( volumeCount: 1, envCount: 2, containerCount: 2, + volumeMounts: []string{}, }, }, "configmap-complex-sidecar": []injectionConfigExpectation{ @@ -46,6 +49,7 @@ var ( volumeCount: 1, envCount: 0, containerCount: 4, + volumeMounts: []string{}, }, }, "configmap-multiple1": []injectionConfigExpectation{ @@ -54,12 +58,23 @@ var ( volumeCount: 0, envCount: 3, containerCount: 0, + volumeMounts: []string{}, }, injectionConfigExpectation{ name: "sidecar-test", volumeCount: 1, envCount: 2, containerCount: 2, + volumeMounts: []string{}, + }, + }, + "configmap-volume-mounts": []injectionConfigExpectation{ + injectionConfigExpectation{ + name: "volume-mounts", + volumeCount: 2, + envCount: 2, + containerCount: 2, + volumeMounts: []string{"test-vol"}, }, }, } @@ -128,6 +143,22 @@ func TestLoadFromConfigMap(t *testing.T) { if len(ic.Volumes) != expectedICF.volumeCount { t.Fatalf("expected %d volumes in %s, but found %d", expectedICF.volumeCount, expectedICF.name, len(ic.Volumes)) } + + for _, expectedVolumeMount := range expectedICF.volumeMounts { + for _, container := range ic.Containers { + volumeMountExists := false + for _, volumeMount := range container.VolumeMounts { + if volumeMount.Name == expectedVolumeMount { + volumeMountExists = true + break + } + } + if !volumeMountExists { + t.Fatalf("did not find expected VolumeMount '%s' in container '%s'", expectedVolumeMount, container.Name) + } + } + } + for _, actualIC := range ics { if ic.Name == actualIC.Name { if ic.String() != actualIC.String() { diff --git a/pkg/server/webhook_test.go b/pkg/server/webhook_test.go index ee19d1e..e65fccd 100644 --- a/pkg/server/webhook_test.go +++ b/pkg/server/webhook_test.go @@ -18,10 +18,11 @@ var ( env1 = "test/fixtures/k8s/env1.yaml" obj3Missing = "test/fixtures/k8s/object3-missing.yaml" obj4 = "test/fixtures/k8s/object4.yaml" + obj5 = "test/fixtures/k8s/object5.yaml" ) func TestLoadConfig(t *testing.T) { - expectedNumInjectionConfigs := 3 + expectedNumInjectionConfigs := 4 c, err := config.LoadConfigDirectory(sidecars) if err != nil { t.Error(err) @@ -51,6 +52,7 @@ func TestLoadConfig(t *testing.T) { env1: "env1", obj3Missing: "", // this one is missing any annotations :) obj4: "", // this one is already injected, so it should not get injected again + obj5: "volume-mounts", } for f, k := range objects { data, err := ioutil.ReadFile(f) diff --git a/test/fixtures/k8s/configmap-volume-mounts.yaml b/test/fixtures/k8s/configmap-volume-mounts.yaml new file mode 100644 index 0000000..633f8df --- /dev/null +++ b/test/fixtures/k8s/configmap-volume-mounts.yaml @@ -0,0 +1,43 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-injectionconfig-vms + namespace: default +data: + test-tumblr1: | + name: volume-mounts + volumeMounts: + - name: test-vol + mountPath: /tmp/test + env: + - name: DATACENTER + value: foo + - name: FROM_INJECTOR + value: bar + containers: + - name: sidecar-nginx + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + env: + - name: DATACENTER + value: bf2 + ports: + - containerPort: 80 + volumeMounts: + - name: nginx-conf + mountPath: /etc/nginx + - name: another-sidecar + image: foo:69 + ports: + - containerPort: 420 + volumeMounts: + - name: test-vol + mountPath: /tmp/another-dir + volumes: + - name: nginx-conf + configMap: + name: nginx-configmap + - name: test-vol + configMap: + name: test-config diff --git a/test/fixtures/k8s/object5.yaml b/test/fixtures/k8s/object5.yaml new file mode 100644 index 0000000..2bc0e26 --- /dev/null +++ b/test/fixtures/k8s/object5.yaml @@ -0,0 +1,4 @@ +name: object2 +namespace: unittest +annotations: + "injector.unittest.com/request": "volume-mounts" diff --git a/test/fixtures/sidecars/volume-mounts.yaml b/test/fixtures/sidecars/volume-mounts.yaml new file mode 100644 index 0000000..18fba8b --- /dev/null +++ b/test/fixtures/sidecars/volume-mounts.yaml @@ -0,0 +1,35 @@ +--- +name: volume-mounts +env: + - name: DATACENTER + value: foo + - name: FROM_INJECTOR + value: bar +containers: +- name: sidecar-nginx + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + env: + - name: DATACENTER + value: bf2 + ports: + - containerPort: 80 + volumeMounts: + - name: nginx-conf + mountPath: /etc/nginx + - name: test-vol + mountPath: /tmp/test +- name: another-sidecar + image: foo:69 + ports: + - containerPort: 420 + volumeMounts: + - name: test-vol + mountPath: /tmp/another-dir +volumes: +- name: nginx-conf + configMap: + name: nginx-configmap +- name: test-vol + configMap: + name: test-config From 9a57a1af21e9ac7ee4d257f55e8086392f8dc57b Mon Sep 17 00:00:00 2001 From: Thorsten Klein Date: Tue, 29 Jan 2019 08:53:47 +0100 Subject: [PATCH 4/8] fix test fixtures and adapt test to counter method --- internal/pkg/config/config.go | 2 +- internal/pkg/config/config_test.go | 33 ++++--- internal/pkg/config/watcher/loader_test.go | 99 ++++++++++--------- .../fixtures/k8s/configmap-volume-mounts.yaml | 11 ++- test/fixtures/sidecars/volume-mounts.yaml | 17 ++-- 5 files changed, 89 insertions(+), 73 deletions(-) diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 962202c..fd3f52f 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -45,7 +45,7 @@ type Config struct { // String returns a string representation of the config func (c *InjectionConfig) String() string { - return fmt.Sprintf("%s: %d containers, %d volumes, %d environment vars", c.Name, len(c.Containers), len(c.Volumes), len(c.Environment)) + return fmt.Sprintf("%s: %d containers, %d volumes, %d environment vars, %d volume mounts", c.Name, len(c.Containers), len(c.Volumes), len(c.Environment), len(c.VolumeMounts)) } // ReplaceInjectionConfigs will take a list of new InjectionConfigs, and replace the current configuration with them. diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index 8ac3f92..a7845b3 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -40,6 +40,7 @@ func TestLoadEnvironmentInjectionConfig(t *testing.T) { expectedEnvVarCount := 3 expectedContainerCount := 0 expectedVolumeCount := 0 + nExpectedVolumeMounts := 0 if c.Name != expectedName { t.Errorf("expected %s Name loaded from %s but got %s", expectedName, cfg, c.Name) t.Fail() @@ -56,6 +57,9 @@ func TestLoadEnvironmentInjectionConfig(t *testing.T) { t.Errorf("expected %d Volumes loaded from %s but got %d", expectedVolumeCount, cfg, len(c.Volumes)) t.Fail() } + if len(c.VolumeMounts) != nExpectedVolumeMounts { + t.Fatalf("expected %d VolumeMounts loaded from %s but got %d", nExpectedVolumeMounts, cfg, len(c.VolumeMounts)) + } } func TestLoadInjectionConfig1(t *testing.T) { @@ -77,6 +81,9 @@ func TestLoadInjectionConfig1(t *testing.T) { if len(c.Volumes) != 1 { t.Fatalf("expected %d Volumes loaded from %s but got %d", 1, cfg, len(c.Volumes)) } + if len(c.VolumeMounts) != 0 { + t.Fatalf("expected %d VolumeMounts loaded from %s but got %d", 0, cfg, len(c.VolumeMounts)) + } } // load a more complicated test config with LOTS of configuration @@ -90,6 +97,7 @@ func TestLoadComplexConfig(t *testing.T) { nExpectedContainers := 4 nExpectedVolumes := 1 nExpectedEnvironmentVars := 0 + nExpectedVolumeMounts := 0 if c.Name != expectedName { t.Fatalf("expected %s Name loaded from %s but got %s", expectedName, cfg, c.Name) } @@ -102,6 +110,9 @@ func TestLoadComplexConfig(t *testing.T) { if len(c.Volumes) != nExpectedVolumes { t.Fatalf("expected %d Volumes loaded from %s but got %d", nExpectedVolumes, cfg, len(c.Volumes)) } + if len(c.VolumeMounts) != nExpectedVolumeMounts { + t.Fatalf("expected %d VolumeMounts loaded from %s but got %d", nExpectedVolumeMounts, cfg, len(c.VolumeMounts)) + } } func TestLoadVolumeMountsConfig(t *testing.T) { @@ -111,10 +122,10 @@ func TestLoadVolumeMountsConfig(t *testing.T) { t.Fatal(err) } expectedName := "volume-mounts" - nExpectedContainers := 2 + nExpectedContainers := 3 nExpectedVolumes := 2 nExpectedEnvironmentVars := 2 - expectedVolumeMounts := []string{"test-vol"} + nExpectedVolumeMounts := 1 if c.Name != expectedName { t.Fatalf("expected %s Name loaded from %s but got %s", expectedName, cfg, c.Name) @@ -128,19 +139,8 @@ func TestLoadVolumeMountsConfig(t *testing.T) { if len(c.Volumes) != nExpectedVolumes { t.Fatalf("expected %d Volumes loaded from %s but got %d", nExpectedVolumes, cfg, len(c.Volumes)) } - for _, expectedVolumeMount := range expectedVolumeMounts { - for _, container := range c.Containers { - volumeMountExists := false - for _, volumeMount := range container.VolumeMounts { - if volumeMount.Name == expectedVolumeMount { - volumeMountExists = true - break - } - } - if !volumeMountExists { - t.Fatalf("did not find expected VolumeMount '%s' in container '%s' loaded from %s", expectedVolumeMount, container.Name, cfg) - } - } + if len(c.VolumeMounts) != nExpectedVolumeMounts { + t.Fatalf("expected %d VolumeMounts loaded from %s but got %d", nExpectedVolumeMounts, cfg, len(c.VolumeMounts)) } } @@ -185,4 +185,7 @@ func TestGetInjectionConfig(t *testing.T) { if len(i.Volumes) != 1 { t.Fatalf("expected 1 volume, but got %d", len(i.Volumes)) } + if len(i.VolumeMounts) != 0 { + t.Fatalf("expected %d VolumeMounts, but got %d", 0, len(i.VolumeMounts)) + } } diff --git a/internal/pkg/config/watcher/loader_test.go b/internal/pkg/config/watcher/loader_test.go index fa4556c..a279b47 100644 --- a/internal/pkg/config/watcher/loader_test.go +++ b/internal/pkg/config/watcher/loader_test.go @@ -15,11 +15,11 @@ import ( ) type injectionConfigExpectation struct { - name string - volumeCount int - envCount int - containerCount int - volumeMounts []string + name string + volumeCount int + envCount int + containerCount int + volumeMountCount int } var ( @@ -27,54 +27,54 @@ var ( ExpectedInjectionConfigFixtures = map[string][]injectionConfigExpectation{ "configmap-env1": []injectionConfigExpectation{ injectionConfigExpectation{ - name: "env1", - volumeCount: 0, - envCount: 3, - containerCount: 0, - volumeMounts: []string{}, + name: "env1", + volumeCount: 0, + envCount: 3, + containerCount: 0, + volumeMountCount: 0, }, }, "configmap-sidecar-test": []injectionConfigExpectation{ injectionConfigExpectation{ - name: "sidecar-test", - volumeCount: 1, - envCount: 2, - containerCount: 2, - volumeMounts: []string{}, + name: "sidecar-test", + volumeCount: 1, + envCount: 2, + containerCount: 2, + volumeMountCount: 0, }, }, "configmap-complex-sidecar": []injectionConfigExpectation{ injectionConfigExpectation{ - name: "complex-sidecar", - volumeCount: 1, - envCount: 0, - containerCount: 4, - volumeMounts: []string{}, + name: "complex-sidecar", + volumeCount: 1, + envCount: 0, + containerCount: 4, + volumeMountCount: 0, }, }, "configmap-multiple1": []injectionConfigExpectation{ injectionConfigExpectation{ - name: "env1", - volumeCount: 0, - envCount: 3, - containerCount: 0, - volumeMounts: []string{}, + name: "env1", + volumeCount: 0, + envCount: 3, + containerCount: 0, + volumeMountCount: 0, }, injectionConfigExpectation{ - name: "sidecar-test", - volumeCount: 1, - envCount: 2, - containerCount: 2, - volumeMounts: []string{}, + name: "sidecar-test", + volumeCount: 1, + envCount: 2, + containerCount: 2, + volumeMountCount: 0, }, }, "configmap-volume-mounts": []injectionConfigExpectation{ injectionConfigExpectation{ - name: "volume-mounts", - volumeCount: 2, - envCount: 2, - containerCount: 2, - volumeMounts: []string{"test-vol"}, + name: "volume-mounts", + volumeCount: 2, + envCount: 2, + containerCount: 3, + volumeMountCount: 1, }, }, } @@ -143,22 +143,25 @@ func TestLoadFromConfigMap(t *testing.T) { if len(ic.Volumes) != expectedICF.volumeCount { t.Fatalf("expected %d volumes in %s, but found %d", expectedICF.volumeCount, expectedICF.name, len(ic.Volumes)) } - - for _, expectedVolumeMount := range expectedICF.volumeMounts { - for _, container := range ic.Containers { - volumeMountExists := false - for _, volumeMount := range container.VolumeMounts { - if volumeMount.Name == expectedVolumeMount { - volumeMountExists = true - break + if len(ic.VolumeMounts) != expectedICF.volumeMountCount { + t.Fatalf("expected %d volume mounts in %s, but found %d", expectedICF.volumeMountCount, expectedICF.name, len(ic.VolumeMounts)) + } + /* + for _, expectedVolumeMount := range expectedICF.volumeMounts { + for _, container := range ic.Containers { + volumeMountExists := false + for _, volumeMount := range container.VolumeMounts { + if volumeMount.Name == expectedVolumeMount { + volumeMountExists = true + break + } + } + if !volumeMountExists { + t.Fatalf("did not find expected VolumeMount '%s' in container '%s'", expectedVolumeMount, container.Name) } - } - if !volumeMountExists { - t.Fatalf("did not find expected VolumeMount '%s' in container '%s'", expectedVolumeMount, container.Name) } } - } - + */ for _, actualIC := range ics { if ic.Name == actualIC.Name { if ic.String() != actualIC.String() { diff --git a/test/fixtures/k8s/configmap-volume-mounts.yaml b/test/fixtures/k8s/configmap-volume-mounts.yaml index 633f8df..fe4942c 100644 --- a/test/fixtures/k8s/configmap-volume-mounts.yaml +++ b/test/fixtures/k8s/configmap-volume-mounts.yaml @@ -2,7 +2,7 @@ apiVersion: v1 kind: ConfigMap metadata: - name: test-injectionconfig-vms + name: test-volume-mounts namespace: default data: test-tumblr1: | @@ -16,7 +16,7 @@ data: - name: FROM_INJECTOR value: bar containers: - - name: sidecar-nginx + - name: sidecar-add-vm image: nginx:1.12.2 imagePullPolicy: IfNotPresent env: @@ -27,13 +27,18 @@ data: volumeMounts: - name: nginx-conf mountPath: /etc/nginx - - name: another-sidecar + - name: sidecar-existing-vm image: foo:69 ports: - containerPort: 420 volumeMounts: - name: test-vol mountPath: /tmp/another-dir + - name: sidecar-first-vm + image: bar:42 + imagePullPolicy: always + ports: + - containerPort: 43 volumes: - name: nginx-conf configMap: diff --git a/test/fixtures/sidecars/volume-mounts.yaml b/test/fixtures/sidecars/volume-mounts.yaml index 18fba8b..0a93f2b 100644 --- a/test/fixtures/sidecars/volume-mounts.yaml +++ b/test/fixtures/sidecars/volume-mounts.yaml @@ -1,12 +1,14 @@ ---- name: volume-mounts +volumeMounts: + - name: test-vol + mountPath: /tmp/test env: - name: DATACENTER value: foo - name: FROM_INJECTOR value: bar containers: -- name: sidecar-nginx +- name: sidecar-add-vm image: nginx:1.12.2 imagePullPolicy: IfNotPresent env: @@ -17,19 +19,22 @@ containers: volumeMounts: - name: nginx-conf mountPath: /etc/nginx - - name: test-vol - mountPath: /tmp/test -- name: another-sidecar +- name: sidecar-existing-vm image: foo:69 ports: - containerPort: 420 volumeMounts: - name: test-vol mountPath: /tmp/another-dir +- name: sidecar-first-vm + image: bar:42 + imagePullPolicy: always + ports: + - containerPort: 43 volumes: - name: nginx-conf configMap: name: nginx-configmap - name: test-vol configMap: - name: test-config + name: test-config \ No newline at end of file From 071bad1830670b749f6ffe63a7adda76ddf23eba Mon Sep 17 00:00:00 2001 From: Thorsten Klein Date: Thu, 31 Jan 2019 12:47:09 +0100 Subject: [PATCH 5/8] dashes instead of camelCase --- internal/pkg/config/config.go | 2 +- internal/pkg/config/watcher/loader_test.go | 16 ---------------- test/fixtures/k8s/configmap-volume-mounts.yaml | 2 +- test/fixtures/sidecars/volume-mounts.yaml | 2 +- 4 files changed, 3 insertions(+), 19 deletions(-) diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index fd3f52f..11f44d3 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -33,7 +33,7 @@ type InjectionConfig struct { Containers []corev1.Container `json:"containers"` Volumes []corev1.Volume `json:"volumes"` Environment []corev1.EnvVar `json:"env"` - VolumeMounts []corev1.VolumeMount `json:"volumeMounts"` + VolumeMounts []corev1.VolumeMount `json:"volume-mounts"` } // Config is a struct indicating how a given injection should be configured diff --git a/internal/pkg/config/watcher/loader_test.go b/internal/pkg/config/watcher/loader_test.go index a279b47..7978436 100644 --- a/internal/pkg/config/watcher/loader_test.go +++ b/internal/pkg/config/watcher/loader_test.go @@ -146,22 +146,6 @@ func TestLoadFromConfigMap(t *testing.T) { if len(ic.VolumeMounts) != expectedICF.volumeMountCount { t.Fatalf("expected %d volume mounts in %s, but found %d", expectedICF.volumeMountCount, expectedICF.name, len(ic.VolumeMounts)) } - /* - for _, expectedVolumeMount := range expectedICF.volumeMounts { - for _, container := range ic.Containers { - volumeMountExists := false - for _, volumeMount := range container.VolumeMounts { - if volumeMount.Name == expectedVolumeMount { - volumeMountExists = true - break - } - } - if !volumeMountExists { - t.Fatalf("did not find expected VolumeMount '%s' in container '%s'", expectedVolumeMount, container.Name) - } - } - } - */ for _, actualIC := range ics { if ic.Name == actualIC.Name { if ic.String() != actualIC.String() { diff --git a/test/fixtures/k8s/configmap-volume-mounts.yaml b/test/fixtures/k8s/configmap-volume-mounts.yaml index fe4942c..6ca8562 100644 --- a/test/fixtures/k8s/configmap-volume-mounts.yaml +++ b/test/fixtures/k8s/configmap-volume-mounts.yaml @@ -7,7 +7,7 @@ metadata: data: test-tumblr1: | name: volume-mounts - volumeMounts: + volume-mounts: - name: test-vol mountPath: /tmp/test env: diff --git a/test/fixtures/sidecars/volume-mounts.yaml b/test/fixtures/sidecars/volume-mounts.yaml index 0a93f2b..2f169d9 100644 --- a/test/fixtures/sidecars/volume-mounts.yaml +++ b/test/fixtures/sidecars/volume-mounts.yaml @@ -1,5 +1,5 @@ name: volume-mounts -volumeMounts: +volume-mounts: - name: test-vol mountPath: /tmp/test env: From 4311f028ec91e600084325067a915eafde6e825f Mon Sep 17 00:00:00 2001 From: Thorsten Klein Date: Sat, 2 Feb 2019 16:59:04 +0100 Subject: [PATCH 6/8] Revert "dashes instead of camelCase" This reverts commit 071bad1830670b749f6ffe63a7adda76ddf23eba. --- internal/pkg/config/config.go | 2 +- internal/pkg/config/watcher/loader_test.go | 16 ++++++++++++++++ test/fixtures/k8s/configmap-volume-mounts.yaml | 2 +- test/fixtures/sidecars/volume-mounts.yaml | 2 +- 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 11f44d3..fd3f52f 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -33,7 +33,7 @@ type InjectionConfig struct { Containers []corev1.Container `json:"containers"` Volumes []corev1.Volume `json:"volumes"` Environment []corev1.EnvVar `json:"env"` - VolumeMounts []corev1.VolumeMount `json:"volume-mounts"` + VolumeMounts []corev1.VolumeMount `json:"volumeMounts"` } // Config is a struct indicating how a given injection should be configured diff --git a/internal/pkg/config/watcher/loader_test.go b/internal/pkg/config/watcher/loader_test.go index 7978436..a279b47 100644 --- a/internal/pkg/config/watcher/loader_test.go +++ b/internal/pkg/config/watcher/loader_test.go @@ -146,6 +146,22 @@ func TestLoadFromConfigMap(t *testing.T) { if len(ic.VolumeMounts) != expectedICF.volumeMountCount { t.Fatalf("expected %d volume mounts in %s, but found %d", expectedICF.volumeMountCount, expectedICF.name, len(ic.VolumeMounts)) } + /* + for _, expectedVolumeMount := range expectedICF.volumeMounts { + for _, container := range ic.Containers { + volumeMountExists := false + for _, volumeMount := range container.VolumeMounts { + if volumeMount.Name == expectedVolumeMount { + volumeMountExists = true + break + } + } + if !volumeMountExists { + t.Fatalf("did not find expected VolumeMount '%s' in container '%s'", expectedVolumeMount, container.Name) + } + } + } + */ for _, actualIC := range ics { if ic.Name == actualIC.Name { if ic.String() != actualIC.String() { diff --git a/test/fixtures/k8s/configmap-volume-mounts.yaml b/test/fixtures/k8s/configmap-volume-mounts.yaml index 6ca8562..fe4942c 100644 --- a/test/fixtures/k8s/configmap-volume-mounts.yaml +++ b/test/fixtures/k8s/configmap-volume-mounts.yaml @@ -7,7 +7,7 @@ metadata: data: test-tumblr1: | name: volume-mounts - volume-mounts: + volumeMounts: - name: test-vol mountPath: /tmp/test env: diff --git a/test/fixtures/sidecars/volume-mounts.yaml b/test/fixtures/sidecars/volume-mounts.yaml index 2f169d9..0a93f2b 100644 --- a/test/fixtures/sidecars/volume-mounts.yaml +++ b/test/fixtures/sidecars/volume-mounts.yaml @@ -1,5 +1,5 @@ name: volume-mounts -volume-mounts: +volumeMounts: - name: test-vol mountPath: /tmp/test env: From 25c8a154888fb89ebb225ef296c51ee95266d2f0 Mon Sep 17 00:00:00 2001 From: Thorsten Klein Date: Sat, 2 Feb 2019 16:59:29 +0100 Subject: [PATCH 7/8] remove commented out code --- internal/pkg/config/watcher/loader_test.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/internal/pkg/config/watcher/loader_test.go b/internal/pkg/config/watcher/loader_test.go index a279b47..7978436 100644 --- a/internal/pkg/config/watcher/loader_test.go +++ b/internal/pkg/config/watcher/loader_test.go @@ -146,22 +146,6 @@ func TestLoadFromConfigMap(t *testing.T) { if len(ic.VolumeMounts) != expectedICF.volumeMountCount { t.Fatalf("expected %d volume mounts in %s, but found %d", expectedICF.volumeMountCount, expectedICF.name, len(ic.VolumeMounts)) } - /* - for _, expectedVolumeMount := range expectedICF.volumeMounts { - for _, container := range ic.Containers { - volumeMountExists := false - for _, volumeMount := range container.VolumeMounts { - if volumeMount.Name == expectedVolumeMount { - volumeMountExists = true - break - } - } - if !volumeMountExists { - t.Fatalf("did not find expected VolumeMount '%s' in container '%s'", expectedVolumeMount, container.Name) - } - } - } - */ for _, actualIC := range ics { if ic.Name == actualIC.Name { if ic.String() != actualIC.String() { From e87746e48ba85e410107590d0481852f5c9cad4e Mon Sep 17 00:00:00 2001 From: Thorsten Klein Date: Mon, 4 Feb 2019 15:53:17 +0100 Subject: [PATCH 8/8] newline at EOF --- test/fixtures/sidecars/volume-mounts.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/fixtures/sidecars/volume-mounts.yaml b/test/fixtures/sidecars/volume-mounts.yaml index 0a93f2b..1df7de8 100644 --- a/test/fixtures/sidecars/volume-mounts.yaml +++ b/test/fixtures/sidecars/volume-mounts.yaml @@ -37,4 +37,4 @@ volumes: name: nginx-configmap - name: test-vol configMap: - name: test-config \ No newline at end of file + name: test-config