diff --git a/docs/sidecar-configuration-format.md b/docs/sidecar-configuration-format.md index 17a1e00..74ba502 100644 --- a/docs/sidecar-configuration-format.md +++ b/docs/sidecar-configuration-format.md @@ -97,6 +97,28 @@ initContainers: - name: some-initcontainer image: init:1.12.2 imagePullPolicy: IfNotPresent + +# prependedContainers will be prepended to the top of the list of normal +# containers. 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 +prependedContainers: +- name: prepended-nginx-container + image: nginx:1.12.2 + imagePullPolicy: IfNotPresent + ports: + - containerPort: 80 + volumeMounts: + - name: nginx-conf + mountPath: /etc/nginx + lifecycle: + postStart: + exec: + command: + - /bin/sh + - -c + - | + while ! nc -w 1 127.0.0.1 80; do sleep 1; done ``` ## Configuring new sidecars diff --git a/internal/pkg/config/config.go b/internal/pkg/config/config.go index 2737ea3..1055881 100644 --- a/internal/pkg/config/config.go +++ b/internal/pkg/config/config.go @@ -32,17 +32,18 @@ var ( // InjectionConfig is a specific instance of a injected config, for a given annotation type InjectionConfig struct { - Name string `json:"name"` - Inherits string `json:"inherits"` - Containers []corev1.Container `json:"containers"` - Volumes []corev1.Volume `json:"volumes"` - Environment []corev1.EnvVar `json:"env"` - VolumeMounts []corev1.VolumeMount `json:"volumeMounts"` - HostAliases []corev1.HostAlias `json:"hostAliases"` - HostNetwork bool `json:"hostNetwork"` - HostPID bool `json:"hostPID"` - InitContainers []corev1.Container `json:"initContainers"` - ServiceAccountName string `json:"serviceAccountName"` + Name string `json:"name"` + Inherits string `json:"inherits"` + Containers []corev1.Container `json:"containers"` + Volumes []corev1.Volume `json:"volumes"` + Environment []corev1.EnvVar `json:"env"` + VolumeMounts []corev1.VolumeMount `json:"volumeMounts"` + HostAliases []corev1.HostAlias `json:"hostAliases"` + HostNetwork bool `json:"hostNetwork"` + HostPID bool `json:"hostPID"` + InitContainers []corev1.Container `json:"initContainers"` + ServiceAccountName string `json:"serviceAccountName"` + PrependedContainers []corev1.Container `json:"prependedContainers"` version string } @@ -68,7 +69,7 @@ func (c *InjectionConfig) String() string { return fmt.Sprintf("%s%s: %d containers, %d init containers, %d volumes, %d environment vars, %d volume mounts, %d host aliases%s", c.FullName(), inheritsString, - len(c.Containers), + len(c.Containers)+len(c.PrependedContainers), len(c.InitContainers), len(c.Volumes), len(c.Environment), @@ -272,6 +273,22 @@ func (base *InjectionConfig) Merge(child *InjectionConfig) error { } } + // merge prepended containers + for _, cv := range child.PrependedContainers { + contains := false + + for bi, bv := range base.PrependedContainers { + if bv.Name == cv.Name { + contains = true + base.PrependedContainers[bi] = cv + } + } + + if !contains { + base.PrependedContainers = append(base.PrependedContainers, cv) + } + } + // merge serviceAccount settings to the left if child.ServiceAccountName != "" { base.ServiceAccountName = child.ServiceAccountName diff --git a/internal/pkg/config/config_test.go b/internal/pkg/config/config_test.go index b1ae880..2cacc83 100644 --- a/internal/pkg/config/config_test.go +++ b/internal/pkg/config/config_test.go @@ -7,7 +7,6 @@ import ( testhelper "github.com/tumblr/k8s-sidecar-injector/internal/pkg/testing" ) - var ( // location of the fixture sidecar files fixtureSidecarsDir = "test/fixtures/sidecars" @@ -113,27 +112,29 @@ var ( }, // test simple inheritance "simple inheritance from complex-sidecar": testhelper.ConfigExpectation{ - Name: "inheritance-complex", - Version: "v1", - Path: fixtureSidecarsDir + "/inheritance-1.yaml", - EnvCount: 2, - ContainerCount: 5, - VolumeCount: 2, - VolumeMountCount: 0, - HostAliasCount: 1, - InitContainerCount: 1, + Name: "inheritance-complex", + Version: "v1", + Path: fixtureSidecarsDir + "/inheritance-1.yaml", + EnvCount: 2, + ContainerCount: 5, + VolumeCount: 2, + VolumeMountCount: 0, + HostAliasCount: 1, + InitContainerCount: 1, + PrependedContainerCount: 1, }, // test deep inheritance "deep inheritance from inheritance-complex": testhelper.ConfigExpectation{ - Name: "inheritance-deep", - Version: "v2", - Path: fixtureSidecarsDir + "/inheritance-deep-2.yaml", - EnvCount: 3, - ContainerCount: 6, - VolumeCount: 3, - VolumeMountCount: 0, - HostAliasCount: 3, - InitContainerCount: 2, + Name: "inheritance-deep", + Version: "v2", + Path: fixtureSidecarsDir + "/inheritance-deep-2.yaml", + EnvCount: 3, + ContainerCount: 6, + VolumeCount: 3, + VolumeMountCount: 0, + HostAliasCount: 3, + InitContainerCount: 2, + PrependedContainerCount: 2, }, "service-account": testhelper.ConfigExpectation{ Name: "service-account", @@ -195,6 +196,18 @@ var ( HostNetwork: true, HostPID: true, }, + "prepended-containers": testhelper.ConfigExpectation{ + Name: "prepended-containers", + Version: "latest", + Path: fixtureSidecarsDir + "/prepended-containers.yaml", + EnvCount: 0, + ContainerCount: 2, + VolumeCount: 0, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, + PrependedContainerCount: 2, + }, } ) diff --git a/internal/pkg/config/watcher/loader_test.go b/internal/pkg/config/watcher/loader_test.go index edce1a5..47e900f 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-prepended-containers": []testhelper.ConfigExpectation{ + testhelper.ConfigExpectation{ + Name: "prepended-containers", + Version: "latest", + Path: fixtureSidecarsDir + "/prepended-containers.yaml", + VolumeCount: 0, + EnvCount: 0, + ContainerCount: 2, + VolumeMountCount: 0, + HostAliasCount: 0, + InitContainerCount: 0, + PrependedContainerCount: 2, + }, + }, } ) diff --git a/internal/pkg/testing/config.go b/internal/pkg/testing/config.go index c0ed924..fe60d1d 100644 --- a/internal/pkg/testing/config.go +++ b/internal/pkg/testing/config.go @@ -12,16 +12,17 @@ type ConfigExpectation struct { // version is the parsed version string, or "latest" if omitted Version string // Path is the path to the YAML to load the sidecar yaml from - Path string - EnvCount int - ContainerCount int - VolumeCount int - VolumeMountCount int - HostAliasCount int - HostNetwork bool - HostPID bool - InitContainerCount int - ServiceAccount string + Path string + EnvCount int + ContainerCount int + VolumeCount int + VolumeMountCount int + HostAliasCount int + HostNetwork bool + HostPID bool + InitContainerCount int + ServiceAccount string + PrependedContainerCount int // 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..8b0809e 100644 --- a/pkg/server/webhook.go +++ b/pkg/server/webhook.go @@ -210,10 +210,27 @@ func setEnvironment(target []corev1.Container, addedEnv []corev1.EnvVar, basePat return patch } -func addContainers(target, added []corev1.Container, basePath string) (patch []patchOperation) { +func addContainers(target, prepended, appended []corev1.Container, basePath string) (patch []patchOperation) { first := len(target) == 0 var value interface{} - for _, add := range added { + for i := len(prepended) - 1; i >= 0; i-- { + add := prepended[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, + }) + } + + for _, add := range appended { value = add path := basePath if first { @@ -464,7 +481,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, addContainers(pod.Spec.InitContainers, []corev1.Container{}, mutatedInjectedInitContainers, "/spec/initContainers")...) } { // container injections @@ -475,7 +492,10 @@ 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")...) + mutatedInjectedPrependedContainers := mergeEnvVars(inj.Environment, inj.PrependedContainers) + mutatedInjectedPrependedContainers = mergeVolumeMounts(inj.VolumeMounts, mutatedInjectedPrependedContainers) + // then, add containers to the patch + patch = append(patch, addContainers(pod.Spec.Containers, mutatedInjectedPrependedContainers, mutatedInjectedContainers, "/spec/containers")...) } { // pod level mutations diff --git a/pkg/server/webhook_test.go b/pkg/server/webhook_test.go index 4f0d58c..e3be74e 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: "prepended-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: "prepended-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/prepended-containers.json b/test/fixtures/k8s/admissioncontrol/patch/prepended-containers.json new file mode 100644 index 0000000..8243409 --- /dev/null +++ b/test/fixtures/k8s/admissioncontrol/patch/prepended-containers.json @@ -0,0 +1,67 @@ +[ + { + "op": "add", + "path": "/spec/containers", + "value": [ + { + "image": "alpine:latest", + "name": "prepended-container-2", + "ports": [ + { + "containerPort": 1234 + } + ], + "resources": {} + } + ] + }, + { + "op": "add", + "path": "/spec/containers/0", + "value": { + "image": "foobar:2020", + "imagePullPolicy": "IfNotPresent", + "name": "prepended-container-1", + "ports": [ + { + "containerPort": 666 + } + ], + "resources": {} + } + }, + { + "op": "add", + "path": "/spec/containers/-", + "value": { + "image": "nginx:1.12.2", + "imagePullPolicy": "IfNotPresent", + "name": "sidecar-add-vm", + "ports": [ + { + "containerPort": 80 + } + ], + "resources": {} + } + }, + { + "op": "add", + "path": "/spec/containers/-", + "value": { + "image": "foo:69", + "name": "sidecar-existing-vm", + "ports": [ + { + "containerPort": 420 + } + ], + "resources": {} + } + }, + { + "op": "add", + "path": "/metadata/annotations/injector.unittest.com~1status", + "value": "injected" + } +] diff --git a/test/fixtures/k8s/admissioncontrol/request/prepended-containers.yaml b/test/fixtures/k8s/admissioncontrol/request/prepended-containers.yaml new file mode 100644 index 0000000..bd2386a --- /dev/null +++ b/test/fixtures/k8s/admissioncontrol/request/prepended-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: "prepended-containers" + spec: + containers: [] diff --git a/test/fixtures/k8s/configmap-prepended-containers.yaml b/test/fixtures/k8s/configmap-prepended-containers.yaml new file mode 100644 index 0000000..63c5306 --- /dev/null +++ b/test/fixtures/k8s/configmap-prepended-containers.yaml @@ -0,0 +1,29 @@ +--- +apiVersion: v1 +kind: ConfigMap +metadata: + name: test-prepended-containers + namespace: default +data: + test-tumblr1: | + name: prepended-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 + prependedContainers: + - name: prepended-container-1 + image: foobar:2020 + imagePullPolicy: IfNotPresent + ports: + - containerPort: 666 + - name: prepended-container-2 + image: alpine:latest + ports: + - containerPort: 1234 diff --git a/test/fixtures/k8s/object8.yaml b/test/fixtures/k8s/object8.yaml new file mode 100644 index 0000000..4365839 --- /dev/null +++ b/test/fixtures/k8s/object8.yaml @@ -0,0 +1,4 @@ +name: object8 +namespace: unittest +annotations: + "injector.unittest.com/request": "prepended-containers" diff --git a/test/fixtures/sidecars/inheritance-1.yaml b/test/fixtures/sidecars/inheritance-1.yaml index 97beacf..13352cd 100644 --- a/test/fixtures/sidecars/inheritance-1.yaml +++ b/test/fixtures/sidecars/inheritance-1.yaml @@ -40,3 +40,7 @@ containers: initContainers: - name: a-new-image image: some-value + +prependedContainers: + - name: a-new-image + image: some-value diff --git a/test/fixtures/sidecars/inheritance-deep-2.yaml b/test/fixtures/sidecars/inheritance-deep-2.yaml index 87f9640..c39c6d3 100644 --- a/test/fixtures/sidecars/inheritance-deep-2.yaml +++ b/test/fixtures/sidecars/inheritance-deep-2.yaml @@ -43,3 +43,7 @@ containers: initContainers: - name: a-new-image-2 image: some-value + +prependedContainers: + - name: a-new-image-2 + image: some-value diff --git a/test/fixtures/sidecars/prepended-containers.yaml b/test/fixtures/sidecars/prepended-containers.yaml new file mode 100644 index 0000000..76c104a --- /dev/null +++ b/test/fixtures/sidecars/prepended-containers.yaml @@ -0,0 +1,21 @@ +name: prepended-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 +prependedContainers: + - name: prepended-container-1 + image: foobar:2020 + imagePullPolicy: IfNotPresent + ports: + - containerPort: 666 + - name: prepended-container-2 + image: alpine:latest + ports: + - containerPort: 1234