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 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/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index 2112fdc..a7845b3 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) @@ -39,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() @@ -55,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) { @@ -76,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 @@ -89,6 +97,36 @@ 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) + } + 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)) + } + 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) { + cfg := volumeMounts + c, err := LoadInjectionConfigFromFilePath(cfg) + if err != nil { + t.Fatal(err) + } + expectedName := "volume-mounts" + nExpectedContainers := 3 + nExpectedVolumes := 2 + nExpectedEnvironmentVars := 2 + nExpectedVolumeMounts := 1 + if c.Name != expectedName { t.Fatalf("expected %s Name loaded from %s but got %s", expectedName, cfg, c.Name) } @@ -101,6 +139,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 TestHasInjectionConfig(t *testing.T) { @@ -144,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 f40003d..7978436 100644 --- a/internal/pkg/config/watcher/loader_test.go +++ b/internal/pkg/config/watcher/loader_test.go @@ -15,10 +15,11 @@ import ( ) type injectionConfigExpectation struct { - name string - volumeCount int - envCount int - containerCount int + name string + volumeCount int + envCount int + containerCount int + volumeMountCount int } var ( @@ -26,40 +27,54 @@ var ( ExpectedInjectionConfigFixtures = map[string][]injectionConfigExpectation{ "configmap-env1": []injectionConfigExpectation{ injectionConfigExpectation{ - name: "env1", - volumeCount: 0, - envCount: 3, - containerCount: 0, + name: "env1", + volumeCount: 0, + envCount: 3, + containerCount: 0, + volumeMountCount: 0, }, }, "configmap-sidecar-test": []injectionConfigExpectation{ injectionConfigExpectation{ - name: "sidecar-test", - volumeCount: 1, - envCount: 2, - containerCount: 2, + 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, + name: "complex-sidecar", + volumeCount: 1, + envCount: 0, + containerCount: 4, + volumeMountCount: 0, }, }, "configmap-multiple1": []injectionConfigExpectation{ injectionConfigExpectation{ - name: "env1", - volumeCount: 0, - envCount: 3, - containerCount: 0, + name: "env1", + volumeCount: 0, + envCount: 3, + containerCount: 0, + volumeMountCount: 0, }, injectionConfigExpectation{ - name: "sidecar-test", - volumeCount: 1, - envCount: 2, - containerCount: 2, + name: "sidecar-test", + volumeCount: 1, + envCount: 2, + containerCount: 2, + volumeMountCount: 0, + }, + }, + "configmap-volume-mounts": []injectionConfigExpectation{ + injectionConfigExpectation{ + name: "volume-mounts", + volumeCount: 2, + envCount: 2, + containerCount: 3, + volumeMountCount: 1, }, }, } @@ -128,6 +143,9 @@ 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)) } + 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 _, actualIC := range ics { if ic.Name == actualIC.Name { if ic.String() != actualIC.String() { 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) } 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..fe4942c --- /dev/null +++ b/test/fixtures/k8s/configmap-volume-mounts.yaml @@ -0,0 +1,48 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-volume-mounts + 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-add-vm + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + env: + - name: DATACENTER + value: bf2 + ports: + - containerPort: 80 + volumeMounts: + - name: nginx-conf + mountPath: /etc/nginx + - 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 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..1df7de8 --- /dev/null +++ b/test/fixtures/sidecars/volume-mounts.yaml @@ -0,0 +1,40 @@ +name: volume-mounts +volumeMounts: + - name: test-vol + mountPath: /tmp/test +env: + - name: DATACENTER + value: foo + - name: FROM_INJECTOR + value: bar +containers: +- name: sidecar-add-vm + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + env: + - name: DATACENTER + value: bf2 + ports: + - containerPort: 80 + volumeMounts: + - name: nginx-conf + mountPath: /etc/nginx +- 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