diff --git a/docs/sidecar-configuration-format.md b/docs/sidecar-configuration-format.md index 17a1e00..32faef3 100644 --- a/docs/sidecar-configuration-format.md +++ b/docs/sidecar-configuration-format.md @@ -38,6 +38,13 @@ name: "test:v1.2" # NOTE: this is relative to the current file, and does not allow for absolute pathing! inherits: "some-sidecar.yaml" +# prependContainers modifies the behaviour of the injector so that containers +# are injected at the top of the list of normal containers, rather than the +# bottom. Defaults to `false`. This primarily allows exploitation of this workaround for +# ensuring sidecars finish starting before the other containers in the pod are launched: +# https://medium.com/@marko.luksa/delaying-application-start-until-sidecar-is-ready-2ec2d21a7b74 +prependContainers: false + containers: # we inject a nginx container - name: sidecar-nginx diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index dc2d2ef..2fa243d 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -43,6 +43,7 @@ type InjectionConfig struct { HostPID bool `json:"hostPID"` InitContainers []corev1.Container `json:"initContainers"` ServiceAccountName string `json:"serviceAccountName"` + PrependContainers bool `json:"prependContainers"` version string } diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index c097c5c..08f5664 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -194,6 +194,18 @@ var ( HostNetwork: true, HostPID: true, }, + "prepend-containers": testhelper.ConfigExpectation{ + Name: "prepend-containers", + Version: "latest", + Path: fixtureSidecarsDir + "/prepend-containers.yaml", + EnvCount: 0, + ContainerCount: 2, + VolumeCount: 0, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, + PrependContainers: true, + }, } ) diff --git a/internal/pkg/config/watcher/loader_test.go b/internal/pkg/config/watcher/loader_test.go index edce1a5..0d165ac 100644 --- a/internal/pkg/config/watcher/loader_test.go +++ b/internal/pkg/config/watcher/loader_test.go @@ -133,6 +133,21 @@ var ( InitContainerCount: 1, }, }, + + "configmap-prepend-containers": []testhelper.ConfigExpectation{ + testhelper.ConfigExpectation{ + Name: "prepend-containers", + Version: "latest", + Path: fixtureSidecarsDir + "/prepend-containers.yaml", + VolumeCount: 0, + EnvCount: 0, + ContainerCount: 2, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, + PrependContainers: true, + }, + }, } ) diff --git a/internal/pkg/testing/config.go b/internal/pkg/testing/config.go index 5655b0e..fefa78f 100644 --- a/internal/pkg/testing/config.go +++ b/internal/pkg/testing/config.go @@ -22,6 +22,7 @@ type ConfigExpectation struct { HostPID bool InitContainerCount int ServiceAccount string + PrependContainers bool // LoadError is an error, if any, that is expected during load LoadError error diff --git a/pkg/server/webhook.go b/pkg/server/webhook.go index 2142ae7..f0903a7 100644 --- a/pkg/server/webhook.go +++ b/pkg/server/webhook.go @@ -210,7 +210,7 @@ func setEnvironment(target []corev1.Container, addedEnv []corev1.EnvVar, basePat return patch } -func addContainers(target, added []corev1.Container, basePath string) (patch []patchOperation) { +func appendContainers(target, added []corev1.Container, basePath string) (patch []patchOperation) { first := len(target) == 0 var value interface{} for _, add := range added { @@ -231,6 +231,28 @@ func addContainers(target, added []corev1.Container, basePath string) (patch []p return patch } +func prependContainers(target, added []corev1.Container, basePath string) (patch []patchOperation) { + first := len(target) == 0 + var value interface{} + for i := len(added) - 1; i >= 0; i-- { + add := added[i] + value = add + path := basePath + if first { + first = false + value = []corev1.Container{add} + } else { + path = path + "/0" + } + patch = append(patch, patchOperation{ + Op: "add", + Path: path, + Value: value, + }) + } + return patch +} + func setHostNetwork(target bool, addedHostNetwork bool, basePath string) (patch []patchOperation) { if addedHostNetwork == true { patch = append(patch, patchOperation{ @@ -464,7 +486,7 @@ func createPatch(pod *corev1.Pod, inj *config.InjectionConfig, annotations map[s // this mutates inj.InitContainers with our environment vars mutatedInjectedInitContainers := mergeEnvVars(inj.Environment, inj.InitContainers) mutatedInjectedInitContainers = mergeVolumeMounts(inj.VolumeMounts, mutatedInjectedInitContainers) - patch = append(patch, addContainers(pod.Spec.InitContainers, mutatedInjectedInitContainers, "/spec/initContainers")...) + patch = append(patch, appendContainers(pod.Spec.InitContainers, mutatedInjectedInitContainers, "/spec/initContainers")...) } { // container injections @@ -475,7 +497,12 @@ func createPatch(pod *corev1.Pod, inj *config.InjectionConfig, annotations map[s // this mutates inj.Containers with our environment vars mutatedInjectedContainers := mergeEnvVars(inj.Environment, inj.Containers) mutatedInjectedContainers = mergeVolumeMounts(inj.VolumeMounts, mutatedInjectedContainers) - patch = append(patch, addContainers(pod.Spec.Containers, mutatedInjectedContainers, "/spec/containers")...) + // then, add containers to the patch + if inj.PrependContainers { + patch = append(patch, prependContainers(pod.Spec.Containers, mutatedInjectedContainers, "/spec/containers")...) + } else { + patch = append(patch, appendContainers(pod.Spec.Containers, mutatedInjectedContainers, "/spec/containers")...) + } } { // pod level mutations diff --git a/pkg/server/webhook_test.go b/pkg/server/webhook_test.go index 4f0d58c..e13ed31 100644 --- a/pkg/server/webhook_test.go +++ b/pkg/server/webhook_test.go @@ -33,6 +33,7 @@ var ( obj7 = "test/fixtures/k8s/object7.yaml" obj7v2 = "test/fixtures/k8s/object7-v2.yaml" obj7v3 = "test/fixtures/k8s/object7-badrequestformat.yaml" + obj8 = "test/fixtures/k8s/object8.yaml" ignoredNamespace = "test/fixtures/k8s/ignored-namespace-pod.yaml" badSidecar = "test/fixtures/k8s/bad-sidecar.yaml" @@ -51,6 +52,7 @@ var ( {configuration: obj7, expectedSidecar: "init-containers:latest"}, {configuration: obj7v2, expectedSidecar: "init-containers:v2"}, {configuration: obj7v3, expectedSidecar: "", expectedError: ErrRequestedSidecarNotFound}, + {configuration: obj8, expectedSidecar: "prepend-containers:latest"}, {configuration: ignoredNamespace, expectedSidecar: "", expectedError: ErrSkipIgnoredNamespace}, {configuration: badSidecar, expectedSidecar: "", expectedError: ErrRequestedSidecarNotFound}, } @@ -60,6 +62,7 @@ var ( {name: "missing-sidecar-config", allowed: true, patchExpected: false}, {name: "sidecar-test-1", allowed: true, patchExpected: true}, {name: "env-override", allowed: true, patchExpected: true}, + {name: "prepend-containers", allowed: true, patchExpected: true}, {name: "service-account", allowed: true, patchExpected: true}, {name: "service-account-already-set", allowed: true, patchExpected: true}, {name: "service-account-set-default", allowed: true, patchExpected: true}, diff --git a/test/fixtures/k8s/admissioncontrol/patch/prepend-containers.json b/test/fixtures/k8s/admissioncontrol/patch/prepend-containers.json new file mode 100644 index 0000000..da6bebe --- /dev/null +++ b/test/fixtures/k8s/admissioncontrol/patch/prepend-containers.json @@ -0,0 +1,38 @@ +[ + { + "op": "add", + "path": "/spec/containers", + "value": [ + { + "image": "foo:69", + "name": "sidecar-existing-vm", + "ports": [ + { + "containerPort": 420 + } + ], + "resources": {} + } + ] + }, + { + "op": "add", + "path": "/spec/containers/0", + "value": { + "image": "nginx:1.12.2", + "imagePullPolicy": "IfNotPresent", + "name": "sidecar-add-vm", + "ports": [ + { + "containerPort": 80 + } + ], + "resources": {} + } + }, + { + "op": "add", + "path": "/metadata/annotations/injector.unittest.com~1status", + "value": "injected" + } +] diff --git a/test/fixtures/k8s/admissioncontrol/request/prepend-containers.yaml b/test/fixtures/k8s/admissioncontrol/request/prepend-containers.yaml new file mode 100644 index 0000000..db20215 --- /dev/null +++ b/test/fixtures/k8s/admissioncontrol/request/prepend-containers.yaml @@ -0,0 +1,9 @@ +--- +# this is an AdmissionRequest object +# https://godoc.org/k8s.io/api/admission/v1beta1#AdmissionRequest +object: + metadata: + annotations: + injector.unittest.com/request: "prepend-containers" + spec: + containers: [] diff --git a/test/fixtures/k8s/configmap-prepend-containers.yaml b/test/fixtures/k8s/configmap-prepend-containers.yaml new file mode 100644 index 0000000..fbc9a55 --- /dev/null +++ b/test/fixtures/k8s/configmap-prepend-containers.yaml @@ -0,0 +1,20 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-prepend-containers + namespace: default +data: + test-tumblr1: | + name: prepend-containers + containers: + - name: sidecar-add-vm + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + ports: + - containerPort: 80 + - name: sidecar-existing-vm + image: foo:69 + ports: + - containerPort: 420 + prependContainers: true diff --git a/test/fixtures/k8s/object8.yaml b/test/fixtures/k8s/object8.yaml new file mode 100644 index 0000000..84e6219 --- /dev/null +++ b/test/fixtures/k8s/object8.yaml @@ -0,0 +1,4 @@ +name: object8 +namespace: unittest +annotations: + "injector.unittest.com/request": "prepend-containers" diff --git a/test/fixtures/sidecars/prepend-containers.yaml b/test/fixtures/sidecars/prepend-containers.yaml new file mode 100644 index 0000000..751811f --- /dev/null +++ b/test/fixtures/sidecars/prepend-containers.yaml @@ -0,0 +1,12 @@ +name: prepend-containers +containers: + - name: sidecar-add-vm + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + ports: + - containerPort: 80 + - name: sidecar-existing-vm + image: foo:69 + ports: + - containerPort: 420 +prependContainers: true