Whitelist recursively transformable kinds.

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"},
 		}, {

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)

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",