From 1a002005c1aed623f8adc83933a8e9f0e30c57c7 Mon Sep 17 00:00:00 2001 From: monopole Date: Sat, 9 Jan 2021 06:57:01 -0800 Subject: [PATCH 1/2] Add RNode.Map method and test to help decoding. --- kyaml/go.mod | 1 + kyaml/yaml/rnode.go | 13 +++++++++++++ kyaml/yaml/rnode_test.go | 33 +++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+) diff --git a/kyaml/go.mod b/kyaml/go.mod index fc5657db7..0efc5d9f7 100644 --- 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 index 3165d723d..63e028b06 100644 --- 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 index b6b6cf580..57f0276ed 100644 --- 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", From c2fbb709dab497e91011da8e72956b670d892d94 Mon Sep 17 00:00:00 2001 From: monopole Date: Sat, 9 Jan 2021 06:57:29 -0800 Subject: [PATCH 2/2] Don't swallow error in SM patch and use new RNode Map method. --- .../patchstrategicmerge.go | 1 + api/internal/wrappy/wnode.go | 7 +---- api/internal/wrappy/wnode_test.go | 5 ++++ api/resmap/reswrangler_test.go | 27 ++++++++++--------- api/resource/resource.go | 5 +++- 5 files changed, 26 insertions(+), 19 deletions(-) diff --git a/api/filters/patchstrategicmerge/patchstrategicmerge.go b/api/filters/patchstrategicmerge/patchstrategicmerge.go index 987ea93f1..a993e1e4c 100644 --- 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 index 26b94117d..207ff3a3f 100644 --- 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 index c3c7c29f1..7330a3d35 100644 --- 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 index 0eec321d7..bc8a20c96 100644 --- 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 index aa881ae16..0fff83527 100644 --- 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)