Add annotation to change init container order (hashicorp#91)

* Add annotation to change init container order * Fix comment * Fix init containers not getting secret mounts * Resolve suggestions * Guard against misconfiguration of agent first
RemcoBuddelmeijer · Mar 3, 2020 · 1aad0fe · 1aad0fe
1 parent 3bedd55
commit 1aad0fe
Show file tree

Hide file tree

Showing 4 changed files with 156 additions and 8 deletions.
diff --git a/agent-inject/agent/agent.go b/agent-inject/agent/agent.go
@@ -33,6 +33,9 @@ type Agent struct {
 	// in a pod request.
 	Inject bool
 
+	// InitFirst controls whether an init container is first to run.
+	InitFirst bool
+
 	// LimitsCPU is the upper CPU limit the sidecar container is allowed to consume.
 	LimitsCPU string
 
@@ -206,6 +209,11 @@ func New(pod *corev1.Pod, patches []*jsonpatch.JsonPatchOperation) (*Agent, erro
 		return agent, err
 	}
 
+	agent.InitFirst, err = agent.initFirst()
+	if err != nil {
+		return agent, err
+	}
+
 	agent.PrePopulate, err = agent.prePopulate()
 	if err != nil {
 		return agent, err
@@ -309,10 +317,44 @@ func (a *Agent) Patch() ([]byte, error) {
 		if err != nil {
 			return patches, err
 		}
-		a.Patches = append(a.Patches, addContainers(
-			a.Pod.Spec.InitContainers,
-			[]corev1.Container{container},
-			"/spec/initContainers")...)
+
+		containers := a.Pod.Spec.InitContainers
+
+		// Init Containers run sequentially in Kubernetes and sometimes the order in
+		// which they run matters.  This reorders the init containers to put the agent first.
+		// For example, if an init container needed Vault secrets to work, the agent would need
+		// to run first.
+		if a.InitFirst {
+
+			// Remove all init containers from the document so we can re-add them after the agent.
+			if len(a.Pod.Spec.InitContainers) != 0 {
+				a.Patches = append(a.Patches, removeContainers("/spec/initContainers")...)
+			}
+
+			containers = []corev1.Container{container}
+			containers = append(containers, a.Pod.Spec.InitContainers...)
+
+			a.Patches = append(a.Patches, addContainers(
+				[]corev1.Container{},
+				containers,
+				"/spec/initContainers")...)
+		} else {
+			a.Patches = append(a.Patches, addContainers(
+				a.Pod.Spec.InitContainers,
+				[]corev1.Container{container},
+				"/spec/initContainers")...)
+		}
+
+		//Add Volume Mounts
+		for i, container := range containers {
+			if container.Name == "vault-agent-init" {
+				continue
+			}
+			a.Patches = append(a.Patches, addVolumeMounts(
+				container.VolumeMounts,
+				a.ContainerVolumeMounts(),
+				fmt.Sprintf("/spec/initContainers/%d/volumeMounts", i))...)
+		}
 	}
 
 	// Sidecar Container

diff --git a/agent-inject/agent/annotations.go b/agent-inject/agent/annotations.go
@@ -53,6 +53,10 @@ const (
 	// originated from.
 	AnnotationAgentRequestNamespace = "vault.hashicorp.com/agent-request-namespace"
 
+	// AnnotationAgentInitFirst makes the initialization container the first container
+	// to run when a pod starts. Default is last.
+	AnnotationAgentInitFirst = "vault.hashicorp.com/agent-init-first"
+
 	// AnnotationAgentPrePopulate controls whether an init container is included
 	// to pre-populate the shared memory volume with secrets prior to the application
 	// starting.
@@ -290,6 +294,15 @@ func (a *Agent) inject() (bool, error) {
 	return strconv.ParseBool(raw)
 }
 
+func (a *Agent) initFirst() (bool, error) {
+	raw, ok := a.Annotations[AnnotationAgentInitFirst]
+	if !ok {
+		return false, nil
+	}
+
+	return strconv.ParseBool(raw)
+}
+
 func (a *Agent) prePopulate() (bool, error) {
 	raw, ok := a.Annotations[AnnotationAgentPrePopulate]
 	if !ok {

diff --git a/agent-inject/agent/patch.go b/agent-inject/agent/patch.go
@@ -55,6 +55,15 @@ func addVolumeMounts(target, mounts []corev1.VolumeMount, base string) []*jsonpa
 	return result
 }
 
+func removeContainers(path string) []*jsonpatch.JsonPatchOperation {
+	var result []*jsonpatch.JsonPatchOperation
+
+	return append(result, &jsonpatch.JsonPatchOperation{
+		Operation: "remove",
+		Path:      path,
+	})
+}
+
 func addContainers(target, containers []corev1.Container, base string) []*jsonpatch.JsonPatchOperation {
 	var result []*jsonpatch.JsonPatchOperation
 	first := len(target) == 0

diff --git a/agent-inject/handler_test.go b/agent-inject/handler_test.go
@@ -18,6 +18,17 @@ import (
 
 func TestHandlerHandle(t *testing.T) {
 	basicSpec := corev1.PodSpec{
+		InitContainers: []corev1.Container{
+			{
+				Name: "web-init",
+				VolumeMounts: []corev1.VolumeMount{
+					{
+						Name:      "foobar",
+						MountPath: "serviceaccount/somewhere",
+					},
+				},
+			},
+		},
 		Containers: []corev1.Container{
 			{
 				Name: "web",
@@ -131,10 +142,67 @@ func TestHandlerHandle(t *testing.T) {
 					Operation: "add",
 					Path:      "/spec/containers/0/volumeMounts/-",
 				},
+				{
+					Operation: "add",
+					Path:      "/spec/initContainers/-",
+				},
+				{
+					Operation: "add",
+					Path:      "/spec/initContainers/0/volumeMounts/-",
+				},
+				{
+					Operation: "add",
+					Path:      "/spec/containers/-",
+				},
+				{
+					Operation: "add",
+					Path:      "/metadata/annotations/" + agent.EscapeJSONPointer(agent.AnnotationAgentStatus),
+				},
+			},
+		},
+
+		{
+			"init first ",
+			Handler{VaultAddress: "https://vault:8200", VaultAuthPath: "kubernetes", ImageVault: "vault", Log: hclog.Default().Named("handler")},
+			v1beta1.AdmissionRequest{
+				Namespace: "test",
+				Object: encodeRaw(t, &corev1.Pod{
+					ObjectMeta: metav1.ObjectMeta{
+						Annotations: map[string]string{
+							agent.AnnotationAgentInject: "true",
+							agent.AnnotationVaultRole:   "demo",
+							agent.AnnotationAgentInitFirst: "true",
+						},
+					},
+					Spec: basicSpec,
+				}),
+			},
+			"",
+			[]jsonpatch.JsonPatchOperation{
+				{
+					Operation: "add",
+					Path:      "/spec/volumes",
+				},
+				{
+					Operation: "add",
+					Path:      "/spec/containers/0/volumeMounts/-",
+				},
+				{
+					Operation: "remove",
+					Path:      "/spec/initContainers",
+				},
 				{
 					Operation: "add",
 					Path:      "/spec/initContainers",
 				},
+				{
+					Operation: "add",
+					Path:      "/spec/initContainers/-",
+				},
+				{
+					Operation: "add",
+					Path:      "/spec/initContainers/1/volumeMounts/-",
+				},
 				{
 					Operation: "add",
 					Path:      "/spec/containers/-",
@@ -177,7 +245,11 @@ func TestHandlerHandle(t *testing.T) {
 				},
 				{
 					Operation: "add",
-					Path:      "/spec/initContainers",
+					Path:      "/spec/initContainers/-",
+				},
+				{
+					Operation: "add",
+					Path:      "/spec/initContainers/0/volumeMounts/-",
 				},
 				{
 					Operation: "add",
@@ -226,7 +298,11 @@ func TestHandlerHandle(t *testing.T) {
 				},
 				{
 					Operation: "add",
-					Path:      "/spec/initContainers",
+					Path:      "/spec/initContainers/-",
+				},
+				{
+					Operation: "add",
+					Path:      "/spec/initContainers/0/volumeMounts/-",
 				},
 				{
 					Operation: "add",
@@ -271,7 +347,11 @@ func TestHandlerHandle(t *testing.T) {
 				},
 				{
 					Operation: "add",
-					Path:      "/spec/initContainers",
+					Path:      "/spec/initContainers/-",
+				},
+				{
+					Operation: "add",
+					Path:      "/spec/initContainers/0/volumeMounts/-",
 				},
 				{
 					Operation: "add",
@@ -359,7 +439,11 @@ func TestHandlerHandle(t *testing.T) {
 				},
 				{
 					Operation: "add",
-					Path:      "/spec/initContainers",
+					Path:      "/spec/initContainers/-",
+				},
+				{
+					Operation: "add",
+					Path:      "/spec/initContainers/0/volumeMounts/-",
 				},
 				{
 					Operation: "add",