Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add prependedContainers #51

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/sidecar-configuration-format.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions internal/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
12 changes: 12 additions & 0 deletions internal/pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
)

Expand Down
15 changes: 15 additions & 0 deletions internal/pkg/config/watcher/loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
},
}
)

Expand Down
1 change: 1 addition & 0 deletions internal/pkg/testing/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
29 changes: 28 additions & 1 deletion pkg/server/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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, addContainers(pod.Spec.Containers, mutatedInjectedContainers, "/spec/containers")...)
ribbybibby marked this conversation as resolved.
Show resolved Hide resolved
}
}

{ // pod level mutations
Expand Down
3 changes: 3 additions & 0 deletions pkg/server/webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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},
}
Expand All @@ -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},
Expand Down
38 changes: 38 additions & 0 deletions test/fixtures/k8s/admissioncontrol/patch/prepend-containers.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
Original file line number Diff line number Diff line change
@@ -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: []
20 changes: 20 additions & 0 deletions test/fixtures/k8s/configmap-prepend-containers.yaml
Original file line number Diff line number Diff line change
@@ -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
4 changes: 4 additions & 0 deletions test/fixtures/k8s/object8.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
name: object8
namespace: unittest
annotations:
"injector.unittest.com/request": "prepend-containers"
12 changes: 12 additions & 0 deletions test/fixtures/sidecars/prepend-containers.yaml
Original file line number Diff line number Diff line change
@@ -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