From 236ae29e9a2e6abb31920be4d7bbea51b9a25b10 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Fri, 10 Jul 2020 16:54:07 -0700 Subject: [PATCH 1/5] Don't consider empty array as "empty" --- kustomize/go.mod | 5 +++++ kyaml/yaml/types.go | 10 +++++----- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/kustomize/go.mod b/kustomize/go.mod index 39328093c..32113e082 100644 --- a/kustomize/go.mod +++ b/kustomize/go.mod @@ -16,3 +16,8 @@ exclude ( github.com/russross/blackfriday v2.0.0+incompatible sigs.k8s.io/kustomize/api v0.2.0 ) + +replace ( + sigs.k8s.io/kustomize/api => ../api + sigs.k8s.io/kustomize/kyaml => ../kyaml +) diff --git a/kyaml/yaml/types.go b/kyaml/yaml/types.go index 862a0ecc8..46112eeda 100644 --- a/kyaml/yaml/types.go +++ b/kyaml/yaml/types.go @@ -35,18 +35,18 @@ func IsMissingOrNull(node *RNode) bool { } // IsEmpty returns true if the RNode is MissingOrNull, or is either a MappingNode with -// no fields, or a SequenceNode with no elements. +// no fields. func IsEmpty(node *RNode) bool { - if node == nil || node.YNode() == nil || node.YNode().Tag == NullNodeTag { + if IsMissingOrNull(node) { return true } + // Empty sequence is a special case and temporarily not considered as empty here. + // Some users may want to keep empty sequence for compatibility reason. + // For example, use JSON 6902 patch. if node.YNode().Kind == yaml.MappingNode && len(node.YNode().Content) == 0 { return true } - if node.YNode().Kind == yaml.SequenceNode && len(node.YNode().Content) == 0 { - return true - } return false } From 67cdd2e27ea7648c4639ec09d1a050478e9a66c6 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Mon, 13 Jul 2020 11:50:01 -0700 Subject: [PATCH 2/5] Add test for keeping empty array --- api/krusty/keepemptyarray_test.go | 48 +++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 api/krusty/keepemptyarray_test.go diff --git a/api/krusty/keepemptyarray_test.go b/api/krusty/keepemptyarray_test.go new file mode 100644 index 000000000..924e32f1c --- /dev/null +++ b/api/krusty/keepemptyarray_test.go @@ -0,0 +1,48 @@ +package krusty_test + +import ( + "testing" + + kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" +) + +func TestKeepEmptyArray(t *testing.T) { + th := kusttest_test.MakeHarness(t) + th.WriteF("/app/resources.yaml", ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: testing123 +spec: + replicas: 1 + selector: null + template: + spec: + containers: + - name: event + image: testing123 + imagePullPolicy: IfNotPresent + imagePullSecrets: []`) + + th.WriteK("/app", ` +resources: +- resources.yaml`) + + m := th.Run("/app", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: testing123 +spec: + replicas: 1 + selector: null + template: + spec: + containers: + - image: testing123 + imagePullPolicy: IfNotPresent + name: event + imagePullSecrets: [] +`) +} From c6b6dec91fb160e13c68b55408f81051d78b6097 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Mon, 13 Jul 2020 14:15:55 -0700 Subject: [PATCH 3/5] Add tests for IsEmpty and IsMissingorNull --- kyaml/yaml/types_test.go | 80 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/kyaml/yaml/types_test.go b/kyaml/yaml/types_test.go index b963d5b28..68f26c001 100644 --- a/kyaml/yaml/types_test.go +++ b/kyaml/yaml/types_test.go @@ -8,6 +8,7 @@ import ( "testing" "github.com/stretchr/testify/assert" + "gopkg.in/yaml.v3" ) // Test that non-UTF8 characters in comments don't cause failures @@ -166,3 +167,82 @@ type: string } assert.Equal(t, expected, actual) } + +func TestIsMissingOrNull(t *testing.T) { + if IsMissingOrNull(nil) != true { + t.Fatalf("input: nil") + } + // missing value or null value + node := &RNode{value: nil} + if IsMissingOrNull(node) != true { + t.Fatalf("input: nil value") + } + + node.value = &yaml.Node{} + if IsMissingOrNull(node) != false { + t.Fatalf("input: valid node") + } + // node with NullNodeTag + node.value.Tag = NullNodeTag + if IsMissingOrNull(node) != true { + t.Fatalf("input: with NullNodeTag") + } + + node.value = &yaml.Node{} + if IsMissingOrNull(node) != false { + t.Fatalf("input: valid node") + } +} + +func TestIsEmpty(t *testing.T) { + if IsEmpty(nil) != true { + t.Fatalf("input: nil") + } + + // missing value or null value + node := &RNode{value: nil} + if IsEmpty(node) != true { + t.Fatalf("input: nil value") + } + // not array or map + node.value = &yaml.Node{} + if IsEmpty(node) != false { + t.Fatalf("input: not array or map") + } + + // === array tests === + node.value.Kind = yaml.SequenceNode + // empty array. empty array is not expected as empty + if IsEmpty(node) != false { + t.Fatalf("input: empty array") + } + // array with 1 item + node.value.Content = append(node.value.Content, &yaml.Node{}) + if IsEmpty(node) != false { + t.Fatalf("input: array with 1 item") + } + // delete the item in array + node.value.Content = node.value.Content[:len(node.value.Content)-1] + if IsEmpty(node) != false { + t.Fatalf("input: empty array") + } + + // === map tests === + node.value = &yaml.Node{ + Kind: yaml.MappingNode, + } + // empty map + if IsEmpty(node) != true { + t.Fatalf("input: empty map") + } + // map with 1 item + node.value.Content = append(node.value.Content, &yaml.Node{}) + if IsEmpty(node) != false { + t.Fatalf("input: map with 1 item") + } + // delete the item in map + node.value.Content = node.value.Content[:len(node.value.Content)-1] + if IsEmpty(node) != true { + t.Fatalf("input: empty map") + } +} From 23bd4390d35722bd2a7c6ef74a237606edba95d1 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Mon, 13 Jul 2020 16:40:34 -0700 Subject: [PATCH 4/5] code review --- kyaml/yaml/types.go | 11 ++--------- kyaml/yaml/types_test.go | 42 ++++++++++++++++++++++------------------ 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/kyaml/yaml/types.go b/kyaml/yaml/types.go index 46112eeda..36522920c 100644 --- a/kyaml/yaml/types.go +++ b/kyaml/yaml/types.go @@ -28,10 +28,7 @@ func NullNode() *RNode { // IsMissingOrNull returns true if the RNode is nil or contains and explicitly null value. func IsMissingOrNull(node *RNode) bool { - if node == nil || node.YNode() == nil || node.YNode().Tag == NullNodeTag { - return true - } - return false + return node == nil || node.YNode() == nil || node.YNode().Tag == NullNodeTag } // IsEmpty returns true if the RNode is MissingOrNull, or is either a MappingNode with @@ -44,11 +41,7 @@ func IsEmpty(node *RNode) bool { // Empty sequence is a special case and temporarily not considered as empty here. // Some users may want to keep empty sequence for compatibility reason. // For example, use JSON 6902 patch. - if node.YNode().Kind == yaml.MappingNode && len(node.YNode().Content) == 0 { - return true - } - - return false + return node.YNode().Kind == yaml.MappingNode && len(node.YNode().Content) == 0 } func IsNull(node *RNode) bool { diff --git a/kyaml/yaml/types_test.go b/kyaml/yaml/types_test.go index 68f26c001..b281a45aa 100644 --- a/kyaml/yaml/types_test.go +++ b/kyaml/yaml/types_test.go @@ -169,80 +169,84 @@ type: string } func TestIsMissingOrNull(t *testing.T) { - if IsMissingOrNull(nil) != true { + if !IsMissingOrNull(nil) { t.Fatalf("input: nil") } // missing value or null value node := &RNode{value: nil} - if IsMissingOrNull(node) != true { + if !IsMissingOrNull(node) { t.Fatalf("input: nil value") } node.value = &yaml.Node{} - if IsMissingOrNull(node) != false { + if IsMissingOrNull(node) { t.Fatalf("input: valid node") } // node with NullNodeTag node.value.Tag = NullNodeTag - if IsMissingOrNull(node) != true { + if !IsMissingOrNull(node) { t.Fatalf("input: with NullNodeTag") } node.value = &yaml.Node{} - if IsMissingOrNull(node) != false { + if IsMissingOrNull(node) { t.Fatalf("input: valid node") } } func TestIsEmpty(t *testing.T) { - if IsEmpty(nil) != true { + if !IsEmpty(nil) { t.Fatalf("input: nil") } // missing value or null value node := &RNode{value: nil} - if IsEmpty(node) != true { + if !IsEmpty(node) { t.Fatalf("input: nil value") } // not array or map node.value = &yaml.Node{} - if IsEmpty(node) != false { + if IsEmpty(node) { t.Fatalf("input: not array or map") } +} - // === array tests === - node.value.Kind = yaml.SequenceNode +func TestIsEmpty_Arrays(t *testing.T) { + node := &RNode{value: &yaml.Node{ + Kind: yaml.SequenceNode, + }} // empty array. empty array is not expected as empty - if IsEmpty(node) != false { + if IsEmpty(node) { t.Fatalf("input: empty array") } // array with 1 item node.value.Content = append(node.value.Content, &yaml.Node{}) - if IsEmpty(node) != false { + if IsEmpty(node) { t.Fatalf("input: array with 1 item") } // delete the item in array node.value.Content = node.value.Content[:len(node.value.Content)-1] - if IsEmpty(node) != false { + if IsEmpty(node) { t.Fatalf("input: empty array") } +} - // === map tests === - node.value = &yaml.Node{ +func TestIsEmpty_Maps(t *testing.T) { + node := &RNode{value: &yaml.Node{ Kind: yaml.MappingNode, - } + }} // empty map - if IsEmpty(node) != true { + if !IsEmpty(node) { t.Fatalf("input: empty map") } // map with 1 item node.value.Content = append(node.value.Content, &yaml.Node{}) - if IsEmpty(node) != false { + if IsEmpty(node) { t.Fatalf("input: map with 1 item") } // delete the item in map node.value.Content = node.value.Content[:len(node.value.Content)-1] - if IsEmpty(node) != true { + if !IsEmpty(node) { t.Fatalf("input: empty map") } } From 3a828941fa9b5a4bcc0cf8b45d6957f233cff0b7 Mon Sep 17 00:00:00 2001 From: Donny Xia Date: Tue, 14 Jul 2020 11:36:49 -0700 Subject: [PATCH 5/5] Improve tests --- kyaml/yaml/types.go | 22 ++++++++++++++++++++++ kyaml/yaml/types_test.go | 39 +++++++++++++-------------------------- 2 files changed, 35 insertions(+), 26 deletions(-) diff --git a/kyaml/yaml/types.go b/kyaml/yaml/types.go index 36522920c..98b4e2cce 100644 --- a/kyaml/yaml/types.go +++ b/kyaml/yaml/types.go @@ -182,6 +182,28 @@ func NewListRNode(values ...string) *RNode { return seq } +// NewMapRNode returns a new Map *RNode containing the provided values +func NewMapRNode(values *map[string]string) *RNode { + m := &RNode{value: &yaml.Node{ + Kind: yaml.MappingNode, + }} + if values == nil { + return m + } + + for k, v := range *values { + m.value.Content = append(m.value.Content, &yaml.Node{ + Kind: yaml.ScalarNode, + Value: k, + }, &yaml.Node{ + Kind: yaml.ScalarNode, + Value: v, + }) + } + + return m +} + // NewRNode returns a new RNode pointer containing the provided Node. func NewRNode(value *yaml.Node) *RNode { return &RNode{value: value} diff --git a/kyaml/yaml/types_test.go b/kyaml/yaml/types_test.go index b281a45aa..56fd9a0a2 100644 --- a/kyaml/yaml/types_test.go +++ b/kyaml/yaml/types_test.go @@ -8,7 +8,6 @@ import ( "testing" "github.com/stretchr/testify/assert" - "gopkg.in/yaml.v3" ) // Test that non-UTF8 characters in comments don't cause failures @@ -173,25 +172,17 @@ func TestIsMissingOrNull(t *testing.T) { t.Fatalf("input: nil") } // missing value or null value - node := &RNode{value: nil} - if !IsMissingOrNull(node) { + if !IsMissingOrNull(NewRNode(nil)) { t.Fatalf("input: nil value") } - node.value = &yaml.Node{} - if IsMissingOrNull(node) { + if IsMissingOrNull(NewScalarRNode("foo")) { t.Fatalf("input: valid node") } // node with NullNodeTag - node.value.Tag = NullNodeTag - if !IsMissingOrNull(node) { + if !IsMissingOrNull(NullNode()) { t.Fatalf("input: with NullNodeTag") } - - node.value = &yaml.Node{} - if IsMissingOrNull(node) { - t.Fatalf("input: valid node") - } } func TestIsEmpty(t *testing.T) { @@ -200,52 +191,48 @@ func TestIsEmpty(t *testing.T) { } // missing value or null value - node := &RNode{value: nil} - if !IsEmpty(node) { + if !IsEmpty(NewRNode(nil)) { t.Fatalf("input: nil value") } // not array or map - node.value = &yaml.Node{} - if IsEmpty(node) { + if IsEmpty(NewScalarRNode("foo")) { t.Fatalf("input: not array or map") } } func TestIsEmpty_Arrays(t *testing.T) { - node := &RNode{value: &yaml.Node{ - Kind: yaml.SequenceNode, - }} + node := NewListRNode() // empty array. empty array is not expected as empty if IsEmpty(node) { t.Fatalf("input: empty array") } // array with 1 item - node.value.Content = append(node.value.Content, &yaml.Node{}) + node = NewListRNode("foo") if IsEmpty(node) { t.Fatalf("input: array with 1 item") } // delete the item in array - node.value.Content = node.value.Content[:len(node.value.Content)-1] + node.value.Content = nil if IsEmpty(node) { t.Fatalf("input: empty array") } } func TestIsEmpty_Maps(t *testing.T) { - node := &RNode{value: &yaml.Node{ - Kind: yaml.MappingNode, - }} + node := NewMapRNode(nil) // empty map if !IsEmpty(node) { t.Fatalf("input: empty map") } // map with 1 item - node.value.Content = append(node.value.Content, &yaml.Node{}) + node = NewMapRNode(&map[string]string{ + "foo": "bar", + }) if IsEmpty(node) { t.Fatalf("input: map with 1 item") } // delete the item in map - node.value.Content = node.value.Content[:len(node.value.Content)-1] + node.value.Content = nil if !IsEmpty(node) { t.Fatalf("input: empty map") }