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.

Sign up for GitHub

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

Postrenderer support resourcelists #1680

Merged
merged 3 commits into from
Apr 22, 2020
Merged

Postrenderer support resourcelists #1680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ca729d
Support CronJob and simplify
absoludity Apr 21, 2020
adf0737
Handle resource lists recursively.
absoludity Apr 21, 2020
29f37c3
Just test for items key to determine list type.
absoludity Apr 22, 2020
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
103 changes: 57 additions & 46 deletions pkg/agent/docker_secrets_postrenderer.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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)
Expand Down Expand Up @@ -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
}
132 changes: 113 additions & 19 deletions pkg/agent/docker_secrets_postrenderer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"},
},
Expand Down Expand Up @@ -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{}{
Expand All @@ -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{}{
Expand All @@ -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{}{
Expand All @@ -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))
}
})
Expand Down