Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve handling of empty resource maps. #3433

Merged
merged 2 commits into from
Jan 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/filters/patchstrategicmerge/patchstrategicmerge.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
7 changes: 1 addition & 6 deletions api/internal/wrappy/wnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
5 changes: 5 additions & 0 deletions api/internal/wrappy/wnode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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 {
Expand Down
27 changes: 15 additions & 12 deletions api/resmap/reswrangler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

Expand Down
5 changes: 4 additions & 1 deletion api/resource/resource.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions kyaml/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
13 changes: 13 additions & 0 deletions kyaml/yaml/rnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{}
Expand Down
33 changes: 33 additions & 0 deletions kyaml/yaml/rnode_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"strings"
"testing"

"github.com/google/go-cmp/cmp"
"github.com/stretchr/testify/assert"
)

Expand Down Expand Up @@ -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",
Expand Down