diff --git a/pkg/skaffold/deploy/kubectl/namespace_test.go b/pkg/skaffold/deploy/kubectl/namespace_test.go index 3315fa36428..c4d35018e38 100644 --- a/pkg/skaffold/deploy/kubectl/namespace_test.go +++ b/pkg/skaffold/deploy/kubectl/namespace_test.go @@ -29,7 +29,7 @@ func TestCollectNamespaces(t *testing.T) { expected []string }{ { - description: "single manifest in the list", + description: "single Pod manifest in the list", manifests: ManifestList{[]byte(` apiVersion: v1 kind: Pod @@ -40,6 +40,24 @@ spec: containers: - image: gcr.io/k8s-skaffold/example name: example +`)}, + expected: []string{"test"}, + }, { + description: "single Service manifest in the list", + manifests: ManifestList{[]byte(` +apiVersion: v1 +kind: Service +metadata: + name: getting-started + namespace: test +spec: + type: ClusterIP + ports: + - port: 443 + targetPort: 8443 + protocol: TCP + selector: + app: getting-started `)}, expected: []string{"test"}, }, { diff --git a/pkg/skaffold/deploy/kubectl/visitor.go b/pkg/skaffold/deploy/kubectl/visitor.go index 61a9c2097a5..87965c72d95 100644 --- a/pkg/skaffold/deploy/kubectl/visitor.go +++ b/pkg/skaffold/deploy/kubectl/visitor.go @@ -21,6 +21,17 @@ import ( "gopkg.in/yaml.v2" ) +// recursivelyTransformableKinds whitelists kinds that can be transformed recursively. +var recursivelyTransformableKinds = map[string]bool{ + "Pod": true, + "ReplicaSet": true, + "StatefulSet": true, + "Deployment": true, + "DaemonSet": true, + "Job": true, + "CronJob": true, +} + // FieldVisitor represents the aggregation/transformation that should be performed on each traversed field. type FieldVisitor interface { // Visit is called for each transformable key contained in the object and may apply transformations/aggregations on it. @@ -58,7 +69,7 @@ func (l *ManifestList) Visit(visitor FieldVisitor) (ManifestList, error) { // traverseManifest traverses all transformable fields contained within the manifest. func traverseManifestFields(manifest map[interface{}]interface{}, visitor FieldVisitor) { kind := manifest["kind"] - if kind != "CustomResourceDefinition" { + if k, ok := kind.(string); ok && recursivelyTransformableKinds[k] { visitor = &recursiveVisitorDecorator{visitor} } visitFields(manifest, visitor) diff --git a/pkg/skaffold/deploy/kubectl/visitor_test.go b/pkg/skaffold/deploy/kubectl/visitor_test.go index 0ec259f3749..ff2b5ccfcdc 100644 --- a/pkg/skaffold/deploy/kubectl/visitor_test.go +++ b/pkg/skaffold/deploy/kubectl/visitor_test.go @@ -69,47 +69,88 @@ func TestVisit(t *testing.T) { expected: []string{"test=bar"}, }, { - description: "nested map", + description: "skip nested map", manifests: ManifestList{[]byte(`nested: prop: x test: foo`)}, - expected: []string{"test=foo", "nested=map[...", "prop=x"}, + expected: []string{"test=foo", "nested=map[..."}, + }, + { + description: "skip nested map in Role", + manifests: ManifestList{[]byte(`apiVersion: rbac.authorization.k8s.io/v1 +kind: Role +metadata: + name: myrole +rules: +- apiGroups: + - "" + resources: + - configmaps + verbs: + - list + - get`)}, + expected: []string{"apiVersion=rbac...", "kind=Role", "metadata=map[...", "rules=[map..."}, + }, + { + description: "nested map in Pod", + manifests: ManifestList{[]byte(`kind: Pod +metadata: + name: mpod +spec: + restartPolicy: Always`)}, + expected: []string{"kind=Pod", "metadata=map[...", "name=mpod", "spec=map[...", "restartPolicy=Alwa..."}, }, { description: "skip recursion at key", - pivotKey: "nested", - manifests: ManifestList{[]byte(`nested: - prop: x -test: foo`)}, - expected: []string{"test=foo", "nested=map[..."}, + pivotKey: "metadata", + manifests: ManifestList{[]byte(`kind: Pod +metadata: + name: mpod +spec: + restartPolicy: Always`)}, + expected: []string{"kind=Pod", "metadata=map[...", "spec=map[...", "restartPolicy=Alwa..."}, }, { - description: "nested array and map", - manifests: ManifestList{[]byte(`items: -- a -- 3 -- name: item - value: data -- c -test: foo`)}, - expected: []string{"test=foo", "items=[a 3...", "name=item", "value=data"}, + description: "nested array and map in Pod", + manifests: ManifestList{[]byte(`kind: Pod +metadata: + name: mpod +spec: + containers: + - env: + name: k + value: v + name: c1 + - name: c2 + restartPolicy: Always`)}, + expected: []string{"kind=Pod", "metadata=map[...", "name=mpod", + "spec=map[...", "containers=[map...", + "name=c1", "env=map[...", "name=k", "value=v", + "name=c2", "restartPolicy=Alwa...", + }, }, { description: "replace key", - pivotKey: "test", + pivotKey: "name", replaceWith: "repl", - manifests: ManifestList{[]byte(`nested: - name: item - test: - sub: - name: asdf -test: foo`), []byte(`test: bar`)}, - expectedManifests: ManifestList{[]byte(` -nested: - name: item - test: repl -test: repl`), []byte(`test: repl`)}, - expected: []string{"nested=map[...", "name=item", "test=map[...", "test=foo", "test=bar"}, + manifests: ManifestList{[]byte(`kind: Deployment +metadata: + labels: + name: x + name: app +spec: + replicas: 0`), []byte(`name: foo`)}, + // This behaviour is questionable but implemented like this for simplicity. + // In practice this is not a problem (currently) since only the fields + // "metadata" and "image" are matched in known kinds without ambiguous field names. + expectedManifests: ManifestList{[]byte(`kind: Deployment +metadata: + labels: + name: repl + name: repl +spec: + replicas: 0`), []byte(`name: repl`)}, + expected: []string{"kind=Depl...", "metadata=map[...", "name=app", "labels=map[...", "name=x", "spec=map[...", "replicas=0", "name=foo"}, }, { description: "invalid input",