Merge pull request #3433 from monopole/anotherTowards3412

Improve handling of empty resource maps.
kubernetes-sigs · Jan 9, 2021 · cf0bb49 · cf0bb49
2 parents 4f468fc + c2fbb70
commit cf0bb49
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 19 deletions.
diff --git a/api/filters/patchstrategicmerge/patchstrategicmerge.go b/api/filters/patchstrategicmerge/patchstrategicmerge.go
@@ -15,6 +15,7 @@ type Filter struct {
 
 var _ kio.Filter = Filter{}
 
+// Filter does a strategic merge patch, which can delete nodes.
 func (pf Filter) Filter(nodes []*yaml.RNode) ([]*yaml.RNode, error) {
 	var result []*yaml.RNode
 	for i := range nodes {

diff --git a/api/internal/wrappy/wnode.go b/api/internal/wrappy/wnode.go
@@ -163,12 +163,7 @@ func (wn *WNode) GetString(path string) (string, error) {
 
 // Map implements ifc.Kunstructured.
 func (wn *WNode) Map() map[string]interface{} {
-	var result map[string]interface{}
-	if err := wn.node.YNode().Decode(&result); err != nil {
-		// Log and die since interface doesn't allow error.
-		log.Fatalf("failed to decode ynode: %v", err)
-	}
-	return result
+	return wn.node.Map()
 }
 
 // MarshalJSON implements ifc.Kunstructured.

diff --git a/api/internal/wrappy/wnode_test.go b/api/internal/wrappy/wnode_test.go
@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/stretchr/testify/assert"
 	"gopkg.in/yaml.v3"
 	"sigs.k8s.io/kustomize/api/resid"
 	kyaml "sigs.k8s.io/kustomize/kyaml/yaml"
@@ -480,6 +481,10 @@ func TestGetSlice(t *testing.T) {
 	}
 }
 
+func TestMapEmpty(t *testing.T) {
+	assert.Equal(t, 0, len(NewWNode().Map()))
+}
+
 func TestMap(t *testing.T) {
 	wn := NewWNode()
 	if err := wn.UnmarshalJSON([]byte(deploymentLittleJson)); err != nil {

diff --git a/api/resmap/reswrangler_test.go b/api/resmap/reswrangler_test.go
@@ -1100,18 +1100,21 @@ $patch: delete
 			finalMapSize: 0,
 		},
 	}
-	for name, test := range tests {
-		m, err := rmF.NewResMapFromBytes([]byte(target))
-		assert.NoError(t, err, name)
-		idSet := resource.MakeIdSet(m.Resources())
-		assert.Equal(t, 1, idSet.Size(), name)
-		p, err := rf.FromBytes([]byte(test.patch))
-		assert.NoError(t, err, name)
-		assert.NoError(t, m.ApplySmPatch(idSet, p), name)
-		assert.Equal(t, test.finalMapSize, m.Size(), name)
-		yml, err := m.AsYaml()
-		assert.NoError(t, err, name)
-		assert.Equal(t, test.expected, string(yml), name)
+	for name := range tests {
+		tc := tests[name]
+		t.Run(name, func(t *testing.T) {
+			m, err := rmF.NewResMapFromBytes([]byte(target))
+			assert.NoError(t, err, name)
+			idSet := resource.MakeIdSet(m.Resources())
+			assert.Equal(t, 1, idSet.Size(), name)
+			p, err := rf.FromBytes([]byte(tc.patch))
+			assert.NoError(t, err, name)
+			assert.NoError(t, m.ApplySmPatch(idSet, p), name)
+			assert.Equal(t, tc.finalMapSize, m.Size(), name)
+			yml, err := m.AsYaml()
+			assert.NoError(t, err, name)
+			assert.Equal(t, tc.expected, string(yml), name)
+		})
 	}
 }
 

diff --git a/api/resource/resource.go b/api/resource/resource.go
@@ -423,9 +423,12 @@ func (r *Resource) ApplySmPatch(patch *Resource) error {
 		return err
 	}
 	n, ns := r.GetName(), r.GetNamespace()
-	r.ApplyFilter(patchstrategicmerge.Filter{
+	err = r.ApplyFilter(patchstrategicmerge.Filter{
 		Patch: node,
 	})
+	if err != nil {
+		return err
+	}
 	if !r.IsEmpty() {
 		r.SetName(n)
 		r.SetNamespace(ns)

diff --git a/kyaml/go.mod b/kyaml/go.mod
@@ -8,6 +8,7 @@ require (
 	github.com/go-openapi/spec v0.19.5
 	github.com/go-openapi/strfmt v0.19.5
 	github.com/go-openapi/validate v0.19.8
+	github.com/google/go-cmp v0.3.0
 	github.com/markbates/pkger v0.17.1
 	github.com/monochromegane/go-gitignore v0.0.0-20200626010858-205db1a8cc00
 	github.com/qri-io/starlib v0.4.2-0.20200213133954-ff2e8cd5ef8d

diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go
@@ -785,6 +785,19 @@ func FromMap(m map[string]interface{}) (*RNode, error) {
 	return Parse(string(c))
 }
 
+func (rn *RNode) Map() map[string]interface{} {
+	if rn == nil || rn.value == nil {
+		return make(map[string]interface{})
+	}
+	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)
+	}
+	return result
+}
+
 // ConvertJSONToYamlNode parses input json string and returns equivalent yaml node
 func ConvertJSONToYamlNode(jsonStr string) (*RNode, error) {
 	var body map[string]interface{}

diff --git a/kyaml/yaml/rnode_test.go b/kyaml/yaml/rnode_test.go
@@ -8,6 +8,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/stretchr/testify/assert"
 )
 
@@ -384,6 +385,38 @@ func TestRNodeGetValidatedMetadata(t *testing.T) {
 	}
 }
 
+func TestRNodeMapEmpty(t *testing.T) {
+	assert.Equal(t, 0, len(NewRNode(nil).Map()))
+}
+
+func TestRNodeMap(t *testing.T) {
+	wn := NewRNode(nil)
+	if err := wn.UnmarshalJSON([]byte(`{
+  "apiVersion": "apps/v1",
+  "kind": "Deployment",
+  "metadata": {
+    "name": "homer",
+    "namespace": "simpsons"
+  }
+}`)); err != nil {
+		t.Fatalf("unexpected unmarshaljson err: %v", err)
+	}
+
+	expected := map[string]interface{}{
+		"apiVersion": "apps/v1",
+		"kind":       "Deployment",
+		"metadata": map[string]interface{}{
+			"name":      "homer",
+			"namespace": "simpsons",
+		},
+	}
+
+	actual := wn.Map()
+	if diff := cmp.Diff(expected, actual); diff != "" {
+		t.Fatalf("actual map does not deep equal expected map:\n%v", diff)
+	}
+}
+
 func TestRNodeFromMap(t *testing.T) {
 	testConfigMap := map[string]interface{}{
 		"apiVersion": "v1",