Skip to content

Commit

Permalink
Add prependedContainers
Browse files Browse the repository at this point in the history
  • Loading branch information
ribbybibby committed Sep 8, 2020
1 parent 85bf83c commit c0d4b4e
Show file tree
Hide file tree
Showing 14 changed files with 274 additions and 45 deletions.
22 changes: 22 additions & 0 deletions docs/sidecar-configuration-format.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 29 additions & 12 deletions internal/pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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),
Expand Down Expand Up @@ -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
Expand Down
51 changes: 32 additions & 19 deletions internal/pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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,
},
}
)

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

Expand Down
21 changes: 11 additions & 10 deletions internal/pkg/testing/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 24 additions & 4 deletions pkg/server/webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
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: "prepended-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: "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},
Expand Down
67 changes: 67 additions & 0 deletions test/fixtures/k8s/admissioncontrol/patch/prepended-containers.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
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: "prepended-containers"
spec:
containers: []
29 changes: 29 additions & 0 deletions test/fixtures/k8s/configmap-prepended-containers.yaml
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit c0d4b4e

Please sign in to comment.