Merge pull request #3673 from natasha41575/PanicDuplicateKeys

Return error instead of panicking for duplicate keys

api/builtins/NamespaceTransformer.go

@@ -30,12 +30,16 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error {
 		return nil
 	}
 	for _, r := range m.Resources() {
-		if r.IsEmpty() {
+		empty, err := r.IsEmpty()
+		if err != nil {
+			return err
+		}
+		if empty {
 			// Don't mutate empty objects?
 			continue
 		}
 		r.StorePreviousId()
-		err := r.ApplyFilter(namespace.Filter{
+		err = r.ApplyFilter(namespace.Filter{
 			Namespace: p.Namespace,
 			FsSlice:   p.FieldSpecs,
 		})

api/ifc/ifc.go

@@ -76,7 +76,7 @@ type Kunstructured interface {
 	GetString(string) (string, error)
 
 	// Several uses.
-	Map() map[string]interface{}
+	Map() (map[string]interface{}, error)
 
 	// Used by Resource.AsYAML and Resource.String
 	MarshalJSON() ([]byte, error)

api/internal/accumulator/resaccumulator_test.go

@@ -399,7 +399,8 @@ func find(name string, resMap resmap.ResMap) *resource.Resource {
 func getCommand(r *resource.Resource) string {
 	var m map[string]interface{}
 	var c []interface{}
-	m, _ = r.Map()["spec"].(map[string]interface{})
+	resourceMap, _ := r.Map()
+	m, _ = resourceMap["spec"].(map[string]interface{})
 	m, _ = m["template"].(map[string]interface{})
 	m, _ = m["spec"].(map[string]interface{})
 	c, _ = m["containers"].([]interface{})

api/internal/target/multitransformer.go

@@ -43,7 +43,11 @@ func (o *multiTransformer) transform(m resmap.ResMap) error {
 		}
 	}
 	for _, r := range m.Resources() {
-		if r.IsEmpty() {
+		empty, err := r.IsEmpty()
+		if err != nil {
+			return err
+		}
+		if empty {
 			err := m.Remove(r.CurId())
 			if err != nil {
 				return err

api/internal/wrappy/factory_test.go

@@ -306,13 +306,16 @@ items:
 			assert.False(t, tc.exp.isErr)
 			assert.Equal(t, len(tc.exp.out), len(rs))
 			for i := range rs {
+				rsMap, err := rs[i].Map()
+				assert.NoError(t, err)
 				assert.Equal(
-					t, fmt.Sprintf("%v", tc.exp.out[i]), fmt.Sprintf("%v", rs[i].Map()))
+					t, fmt.Sprintf("%v", tc.exp.out[i]), fmt.Sprintf("%v", rsMap))
 				if n != "listWithAnchors" {
 					// https://github.com/kubernetes-sigs/kustomize/issues/3271
-					if !reflect.DeepEqual(tc.exp.out[i], rs[i].Map()) {
+					m, _ := rs[i].Map()
+					if !reflect.DeepEqual(tc.exp.out[i], m) {
 						t.Fatalf("%s:\nexpected: %v\n  actual: %v",
-							n, tc.exp.out[i], rs[i].Map())
+							n, tc.exp.out[i], m)
 					}
 				}
 			}

api/internal/wrappy/wnode.go

@@ -220,7 +220,7 @@ func (wn *WNode) GetString(path string) (string, error) {
 }
 
 // Map implements ifc.Kunstructured.
-func (wn *WNode) Map() map[string]interface{} {
+func (wn *WNode) Map() (map[string]interface{}, error) {
 	return wn.node.Map()
 }
 

api/internal/wrappy/wnode_test.go

@@ -559,7 +559,9 @@ func TestGetSlice(t *testing.T) {
 }
 
 func TestMapEmpty(t *testing.T) {
-	assert.Equal(t, 0, len(NewWNode().Map()))
+	newNodeMap, err := NewWNode().Map()
+	assert.NoError(t, err)
+	assert.Equal(t, 0, len(newNodeMap))
 }
 
 func TestMap(t *testing.T) {
@@ -577,7 +579,8 @@ func TestMap(t *testing.T) {
 		},
 	}
 
-	actual := wn.Map()
+	actual, err := wn.Map()
+	assert.NoError(t, err)
 	if diff := cmp.Diff(expected, actual); diff != "" {
 		t.Fatalf("actual map does not deep equal expected map:\n%v", diff)
 	}

api/krusty/duplicatekeys_test.go

@@ -0,0 +1,39 @@
+package krusty_test
+
+import (
+	"testing"
+
+	kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
+)
+
+func TestDuplicateKeys(t *testing.T) {
+	th := kusttest_test.MakeHarness(t)
+	th.WriteK(".", `
+resources:
+- resources.yaml
+`)
+	th.WriteF("resources.yaml", `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: podinfo
+spec:
+  selector:
+    matchLabels:
+      app: podinfo
+  template:
+    spec:
+      containers:
+      - name: podinfod
+        image: ghcr.io/stefanprodan/podinfo:5.0.3 
+        command:
+        - ./podinfo
+        env:
+        - name: PODINFO_UI_COLOR
+          value: "#34577c"
+        env:
+          - name: PODINFO_UI_COLOR
+            value: "#34577c"
+`)
+	th.RunWithErr(".", th.MakeDefaultOptions())
+}

api/resmap/merginator.go

@@ -90,9 +90,14 @@ func (m *merginator) makeError(cd resource.ConflictDetector, index int) error {
 	if conflict == nil {
 		return fmt.Errorf("expected conflict for %s", m.incoming[index].OrgId())
 	}
+	conflictMap, _ := conflict.Map()
+	incomingIndexMap, _ := m.incoming[index].Map()
 	return fmt.Errorf(
 		"conflict between %#v at index %d and %#v",
-		m.incoming[index].Map(), index, conflict.Map())
+		incomingIndexMap,
+		index,
+		conflictMap,
+	)
 }
 
 // findConflict looks for a conflict in a resource slice.

api/resmap/reswrangler.go

@@ -119,7 +119,11 @@ func (m *resWrangler) Debug(title string) {
 			fmt.Println("---")
 		}
 		fmt.Printf("# %d  %s\n", i, r.OrgId())
-		blob, err := yaml.Marshal(r.Map())
+		m, err := r.Map()
+		if err != nil {
+			panic(err)
+		}
+		blob, err := yaml.Marshal(m)
 		if err != nil {
 			panic(err)
 		}
@@ -272,7 +276,8 @@ func (m *resWrangler) AsYaml() ([]byte, error) {
 	for _, res := range m.Resources() {
 		out, err := res.AsYAML()
 		if err != nil {
-			return nil, errors.Wrapf(err, "%#v", res.Map())
+			m, _ := res.Map()
+			return nil, errors.Wrapf(err, "%#v", m)
 		}
 		if firstObj {
 			firstObj = false
@@ -602,14 +607,23 @@ func (m *resWrangler) ApplySmPatch(
 				// Some unknown error, let it through.
 				return err
 			}
-			if !res.IsEmpty() {
+			empty, err := res.IsEmpty()
+			if err != nil {
+				return err
+			}
+			if !empty {
+				m, _ := res.Map()
 				return errors.Wrapf(
 					err, "with unexpectedly non-empty object map of size %d",
-					len(res.Map()))
+					len(m))
 			}
 			// Fall through to handle deleted object.
 		}
-		if !res.IsEmpty() {
+		empty, err := res.IsEmpty()
+		if err != nil {
+			return err
+		}
+		if !empty {
 			// IsEmpty means all fields have been removed from the object.
 			// This can happen if a patch required deletion of the
 			// entire resource (not just a part of it).  This means

api/resource/factory.go

@@ -114,7 +114,11 @@ func (rf *Factory) SliceFromBytes(in []byte) ([]*Resource, error) {
 		u := kunStructs[0]
 		kunStructs = kunStructs[1:]
 		if strings.HasSuffix(u.GetKind(), "List") {
-			items := u.Map()["items"]
+			m, err := u.Map()
+			if err != nil {
+				return nil, err
+			}
+			items := m["items"]
 			itemsSlice, ok := items.([]interface{})
 			if !ok {
 				if items == nil {

api/resource/resource.go

@@ -98,11 +98,12 @@ func (r *Resource) GetString(p string) (string, error) {
 	return r.kunStr.GetString(p)
 }
 
-func (r *Resource) IsEmpty() bool {
+func (r *Resource) IsEmpty() (bool, error) {
-	return len(r.kunStr.Map()) == 0
+	m, err := r.kunStr.Map()
+	return len(m) == 0, err
 }
 
-func (r *Resource) Map() map[string]interface{} {
+func (r *Resource) Map() (map[string]interface{}, error) {
 	return r.kunStr.Map()
 }
 
@@ -488,7 +489,11 @@ func (r *Resource) ApplySmPatch(patch *Resource) error {
 	if err != nil {
 		return err
 	}
-	if !r.IsEmpty() {
+	empty, err := r.IsEmpty()
+	if err != nil {
+		return err
+	}
+	if !empty {
 		r.SetName(n)
 		r.SetNamespace(ns)
 	}

kustomize/commands/build/writer.go

@@ -46,7 +46,11 @@ func (w Writer) WriteIndividualFiles(dirPath string, m resmap.ResMap) error {
 }
 
 func (w Writer) write(path, fName string, res *resource.Resource) error {
-	yml, err := yaml.Marshal(res.Map())
+	m, err := res.Map()
+	if err != nil {
+		return err
+	}
+	yml, err := yaml.Marshal(m)
 	if err != nil {
 		return err
 	}

kyaml/yaml/rnode.go

@@ -816,17 +816,18 @@ func FromMap(m map[string]interface{}) (*RNode, error) {
 	return Parse(string(c))
 }
 
-func (rn *RNode) Map() map[string]interface{} {
+func (rn *RNode) Map() (map[string]interface{}, error) {
 	if rn == nil || rn.value == nil {
-		return make(map[string]interface{})
+		return make(map[string]interface{}), nil
 	}
 	var result map[string]interface{}
 	if err := rn.value.Decode(&result); err != nil {
 		// Should not be able to create an RNode that cannot be decoded;
 		// this is an unrecoverable error.
-		log.Fatalf("failed to decode ynode: %v", err)
+		str, _ := rn.String()
+		return nil, fmt.Errorf("received error %w for the following resource:\n%s", err, str)
 	}
-	return result
+	return result, nil
 }
 
 // ConvertJSONToYamlNode parses input json string and returns equivalent yaml node

kyaml/yaml/rnode_test.go

@@ -386,7 +386,9 @@ func TestRNodeGetValidatedMetadata(t *testing.T) {
 }
 
 func TestRNodeMapEmpty(t *testing.T) {
-	assert.Equal(t, 0, len(NewRNode(nil).Map()))
+	newRNodeMap, err := NewRNode(nil).Map()
+	assert.NoError(t, err)
+	assert.Equal(t, 0, len(newRNodeMap))
 }
 
 func TestRNodeMap(t *testing.T) {
@@ -411,7 +413,8 @@ func TestRNodeMap(t *testing.T) {
 		},
 	}
 
-	actual := wn.Map()
+	actual, err := wn.Map()
+	assert.NoError(t, err)
 	if diff := cmp.Diff(expected, actual); diff != "" {
 		t.Fatalf("actual map does not deep equal expected map:\n%v", diff)
 	}

plugin/builtin/namespacetransformer/NamespaceTransformer.go

@@ -34,12 +34,16 @@ func (p *plugin) Transform(m resmap.ResMap) error {
 		return nil
 	}
 	for _, r := range m.Resources() {
-		if r.IsEmpty() {
+		empty, err := r.IsEmpty()
+		if err != nil {
+			return err
+		}
+		if empty {
 			// Don't mutate empty objects?
 			continue
 		}
 		r.StorePreviousId()
-		err := r.ApplyFilter(namespace.Filter{
+		err = r.ApplyFilter(namespace.Filter{
 			Namespace: p.Namespace,
 			FsSlice:   p.FieldSpecs,
 		})