Add prependedContainers

Author: ribbybibby

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

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 (c *InjectionConfig) Merge(child *InjectionConfig) error {
 		}
 	}
 
+	// merge prepended containers
+	for _, cv := range child.PrependedContainers {
+		contains := false
+
+		for bi, bv := range c.PrependedContainers {
+			if bv.Name == cv.Name {
+				contains = true
+				c.PrependedContainers[bi] = cv
+			}
+		}
+
+		if !contains {
+			c.PrependedContainers = append(c.PrependedContainers, cv)
+		}
+	}
+
 	// merge serviceAccount settings to the left
 	if child.ServiceAccountName != "" {
 		c.ServiceAccountName = child.ServiceAccountName

internal/pkg/config/config_test.go

@@ -112,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",
@@ -194,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,
+		},
 	}
 )
 

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,
+			},
+		},
 	}
 )
 

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

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

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},

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"
+  }
+]

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: []

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

test/fixtures/k8s/object8.yaml

@@ -0,0 +1,4 @@
+name: object8
+namespace: unittest
+annotations:
+  "injector.unittest.com/request": "prepended-containers"