diff --git a/pkg/agent/docker_secrets_postrenderer.go b/pkg/agent/docker_secrets_postrenderer.go index 3f1c5a4ca8f..d192e1fed43 100644 --- 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 index 3bac39e1d36..add57c99e84 100644 --- 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)) } })