Postrenderer support resourcelists (#1680)

* Support CronJob and simplify * Handle resource lists recursively. * Just test for items key to determine list type.
vmware-tanzu · Apr 22, 2020 · 132318f · 132318f
1 parent 0d2315b
commit 132318f
Show file tree

Hide file tree

Showing 2 changed files with 170 additions and 65 deletions.
diff --git a/pkg/agent/docker_secrets_postrenderer.go b/pkg/agent/docker_secrets_postrenderer.go
@@ -52,6 +52,40 @@ func NewDockerSecretsPostRenderer(secrets map[string]string) (*DockerSecretsPost
 	return r, nil
 }
 
+func (r *DockerSecretsPostRenderer) processResourceList(resourceList []interface{}) {
+	for _, resourceItem := range resourceList {
+		resource, ok := resourceItem.(map[interface{}]interface{})
+		if !ok {
+			continue
+		}
+		kindValue, ok := resource["kind"]
+		if !ok {
+			log.Errorf("invalid resource: no kind. %+v", resource)
+			continue
+		}
+
+		kind, ok := kindValue.(string)
+		if !ok {
+			log.Errorf("invalid resource: non-string resource kind. %+v", resource)
+			continue
+		}
+		if items, ok := resource["items"]; ok {
+			if itemsSlice, ok := items.([]interface{}); ok {
+				r.processResourceList(itemsSlice)
+			} else {
+				log.Errorf("Items of list type did not contain a slice: %+v", resource)
+			}
+			continue
+		}
+
+		podSpec := getResourcePodSpec(kind, resource)
+		if podSpec == nil {
+			continue
+		}
+		r.updatePodSpecWithPullSecrets(podSpec)
+	}
+}
+
 // Run returns the rendered yaml including any additions of the post-renderer.
 // An error is only returned if the manifests cannot be parsed or re-rendered.
 func (r *DockerSecretsPostRenderer) Run(renderedManifests *bytes.Buffer) (modifiedManifests *bytes.Buffer, err error) {
@@ -77,17 +111,7 @@ func (r *DockerSecretsPostRenderer) Run(renderedManifests *bytes.Buffer) (modifi
 	// could instead find the correct byte position and insert the image pull
 	// secret into the byte stream at the relevant points, but this will be
 	// more complex.
-	for _, resourceItem := range resourceList {
-		resource, ok := resourceItem.(map[interface{}]interface{})
-		if !ok {
-			continue
-		}
-		podSpec := getResourcePodSpec(resource)
-		if podSpec == nil {
-			continue
-		}
-		r.updatePodSpecWithPullSecrets(podSpec)
-	}
+	r.processResourceList(resourceList)
 
 	modifiedManifests = bytes.NewBuffer([]byte{})
 	encoder := yaml.NewEncoder(modifiedManifests)
@@ -183,47 +207,34 @@ func (r *DockerSecretsPostRenderer) updatePodSpecWithPullSecrets(podSpec map[int
 // invalid docs ignored and left for the API server to respond accordingly:
 // - A resource doc is a map with a "kind" key with a string value
 // - A pod resource doc has a "spec" key containing a map
-func getResourcePodSpec(resource map[interface{}]interface{}) map[interface{}]interface{} {
-	kindValue, ok := resource["kind"]
-	if !ok {
-		log.Errorf("invalid resource: no kind. %+v", resource)
-		return nil
-	}
-
-	kind, ok := kindValue.(string)
-	if !ok {
-		log.Errorf("invalid resource: non-string resource kind. %+v", resource)
-		return nil
-	}
-
+func getResourcePodSpec(kind string, resource map[interface{}]interface{}) map[interface{}]interface{} {
 	switch kind {
 	case "Pod":
-		podSpec, ok := resource["spec"].(map[interface{}]interface{})
-		if !ok {
-			log.Errorf("invalid resource: non-map pod spec. %+v", resource)
-			return nil
-		}
-		return podSpec
-	case "DaemonSet", "Deployment", "Job", "PodTemplate", "ReplicaSet", "ReplicationController", "StatefulSet":
+		return getMapForKeys([]string{"spec"}, resource)
+	case "DaemonSet", "Deployment", "Job", "ReplicaSet", "ReplicationController", "StatefulSet":
 		// These resources all include a spec.template.spec PodSpec.
 		// https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#podtemplatespec-v1-core
-		spec, ok := resource["spec"].(map[interface{}]interface{})
-		if !ok {
-			log.Errorf("invalid resource: non-map spec. %+v", resource)
-			return nil
-		}
-		template, ok := spec["template"].(map[interface{}]interface{})
-		if !ok {
-			log.Errorf("invalid resource: non-map spec.template. %+v", resource)
-			return nil
-		}
-		podSpec, ok := template["spec"].(map[interface{}]interface{})
+		return getMapForKeys([]string{"spec", "template", "spec"}, resource)
+	case "PodTemplate":
+		return getMapForKeys([]string{"template", "spec"}, resource)
+	case "CronJob":
+		// A CronJob spec contains a jobTemplate:
+		// https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.18/#cronjobspec-v1beta1-batch
+		return getMapForKeys([]string{"spec", "jobTemplate", "spec", "template", "spec"}, resource)
+	}
+
+	return nil
+}
+
+func getMapForKeys(keys []string, m map[interface{}]interface{}) map[interface{}]interface{} {
+	current := m
+	var ok bool
+	for _, k := range keys {
+		current, ok = current[k].(map[interface{}]interface{})
 		if !ok {
-			log.Errorf("invalid resource: non-map spec.template.spec. %+v", resource)
+			log.Errorf("invalid resource: non-map %q, in %+v", k, m)
 			return nil
 		}
-		return podSpec
 	}
-
-	return nil
+	return current
 }
diff --git a/pkg/agent/docker_secrets_postrenderer_test.go b/pkg/agent/docker_secrets_postrenderer_test.go
@@ -168,6 +168,92 @@ spec:
 ---
 kind: Unknown
 other: doc
+`)),
+			secrets: map[string]string{"example.com": "secret-1"},
+		},
+		{
+			name: "it appends relevant image pull secret for nested lists of resources",
+			input: bytes.NewBuffer([]byte(`apiVersion: v1
+kind: PodTemplateList
+metadata:
+  annotations:
+    annotation-1: some-annotation
+  name: image-secret-test
+items:
+- kind: PodTemplate
+  template:
+    spec:
+      containers:
+      - command:
+        - sh
+        - -c
+        - echo 'foo'
+        env:
+        - name: SOME_ENV
+          value: env_value
+        image: example.com/bitnami/nginx:1.16.1-debian-10-r42
+        name: container-name
+      restartPolicy: Never
+- kind: PodTemplate
+  template:
+    spec:
+      containers:
+      - command:
+        - sh
+        - -c
+        - echo 'bar'
+        env:
+        - name: SOME_ENV
+          value: env_value
+        image: example.com/bitnami/nginx:1.16.1-debian-10-r42
+        name: container-name
+      restartPolicy: Never
+---
+kind: Unknown
+other: doc
+`)),
+			output: bytes.NewBuffer([]byte(`apiVersion: v1
+items:
+- kind: PodTemplate
+  template:
+    spec:
+      containers:
+      - command:
+        - sh
+        - -c
+        - echo 'foo'
+        env:
+        - name: SOME_ENV
+          value: env_value
+        image: example.com/bitnami/nginx:1.16.1-debian-10-r42
+        name: container-name
+      imagePullSecrets:
+      - name: secret-1
+      restartPolicy: Never
+- kind: PodTemplate
+  template:
+    spec:
+      containers:
+      - command:
+        - sh
+        - -c
+        - echo 'bar'
+        env:
+        - name: SOME_ENV
+          value: env_value
+        image: example.com/bitnami/nginx:1.16.1-debian-10-r42
+        name: container-name
+      imagePullSecrets:
+      - name: secret-1
+      restartPolicy: Never
+kind: PodTemplateList
+metadata:
+  annotations:
+    annotation-1: some-annotation
+  name: image-secret-test
+---
+kind: Unknown
+other: doc
 `)),
 			secrets: map[string]string{"example.com": "secret-1"},
 		},
@@ -411,37 +497,22 @@ func TestUpdatePodSpecWithPullSecrets(t *testing.T) {
 func TestGetResourcePodSpec(t *testing.T) {
 	testCases := []struct {
 		name     string
+		kind     string
 		resource map[interface{}]interface{}
 		result   map[interface{}]interface{}
 	}{
-		{
-			name: "it ignores an invalid doc without a kind",
-			resource: map[interface{}]interface{}{
-				"notkind": "Pod",
-				"spec":    map[string]interface{}{"some": "spec"},
-			},
-			result: nil,
-		},
-		{
-			name: "it ignores an invalid doc with a non-string kind",
-			resource: map[interface{}]interface{}{
-				"kind": map[string]interface{}{"not": "string"},
-				"spec": map[interface{}]interface{}{"some": "spec"},
-			},
-			result: nil,
-		},
 		{
 			name: "it ignores an invalid doc with a non-map spec",
+			kind: "Pod",
 			resource: map[interface{}]interface{}{
-				"kind": "Pod",
 				"spec": "not a map",
 			},
 			result: nil,
 		},
 		{
 			name: "it returns the pod spec from a pod",
+			kind: "Pod",
 			resource: map[interface{}]interface{}{
-				"kind": "Pod",
 				"spec": map[interface{}]interface{}{"some": "spec"},
 			},
 			result: map[interface{}]interface{}{
@@ -450,6 +521,7 @@ func TestGetResourcePodSpec(t *testing.T) {
 		},
 		{
 			name: "it returns the pod spec from a daemon set",
+			kind: "DaemonSet",
 			resource: map[interface{}]interface{}{
 				"kind": "DaemonSet",
 				"spec": map[interface{}]interface{}{
@@ -464,6 +536,7 @@ func TestGetResourcePodSpec(t *testing.T) {
 		},
 		{
 			name: "it returns the pod spec from a deployment",
+			kind: "Deployment",
 			resource: map[interface{}]interface{}{
 				"kind": "Deployment",
 				"spec": map[interface{}]interface{}{
@@ -476,11 +549,32 @@ func TestGetResourcePodSpec(t *testing.T) {
 				"some": "spec",
 			},
 		},
+		{
+			name: "it returns the pod spec from a CronJob",
+			kind: "CronJob",
+			resource: map[interface{}]interface{}{
+				"kind": "CronJob",
+				"spec": map[interface{}]interface{}{
+					"jobTemplate": map[interface{}]interface{}{
+						"spec": map[interface{}]interface{}{
+							"template": map[interface{}]interface{}{
+								"spec": map[interface{}]interface{}{
+									"some": "spec",
+								},
+							},
+						},
+					},
+				},
+			},
+			result: map[interface{}]interface{}{
+				"some": "spec",
+			},
+		},
 	}
 
 	for _, tc := range testCases {
 		t.Run(tc.name, func(t *testing.T) {
-			if got, want := getResourcePodSpec(tc.resource), tc.result; !cmp.Equal(got, want) {
+			if got, want := getResourcePodSpec(tc.kind, tc.resource), tc.result; !cmp.Equal(got, want) {
 				t.Errorf("mismatch (-want +got):\n%s", cmp.Diff(want, got))
 			}
 		})