From 1f806b0aa2d2fe64d59c96a1afc53f48d737a9cc Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Tue, 3 Nov 2020 15:48:17 -0800 Subject: [PATCH 1/9] add elementsetterlist and elementmatcherlist to fns.go --- kyaml/yaml/fns.go | 124 +++++++++++++++++++++++++----- kyaml/yaml/fns_test.go | 169 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 272 insertions(+), 21 deletions(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index bec46eeb0..1f75eedb6 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -63,17 +63,48 @@ type ElementSetter struct { Value string `yaml:"value,omitempty"` } +func (e ElementSetter) Filter(rn *RNode) (*RNode, error) { + return rn.Pipe(ElementSetterList{ + Element: e.Element, + Keys: []string{e.Key}, + Values: []string{e.Value}, + }) +} + +// ElementSetterList sets the value for an Element in an associative list. +// It behaves identically to ElementSetter, except that it uses multiple +// key-value pairs (in the form of a list of keys and a corresponding list +// of values) in order to find a matching element, whereas ElementSetter +// uses only one key-value pair. +type ElementSetterList struct { + Kind string `yaml:"kind,omitempty"` + + // Element is the new value to set -- remove the existing element if nil + Element *Node + + // Key is a list of fields on the elements. It is used to find matching elements to + // update / delete + Keys []string + + // Value is a list of field values on the elements corresponding to the keys. It is + // used to find matching elements to update / delete. + Values []string +} + // isMappingNode returns whether node is a mapping node -func (e ElementSetter) isMappingNode(node *RNode) bool { +func (e ElementSetterList) isMappingNode(node *RNode) bool { return ErrorIfInvalid(node, yaml.MappingNode) == nil } // isMappingSetter returns is this setter intended to set a mapping node -func (e ElementSetter) isMappingSetter() bool { - return e.Key != "" && e.Value != "" +func (e ElementSetterList) isMappingSetter() bool { + return len(e.Keys) > 0 && e.Keys[0] != "" && + len(e.Values) > 0 && e.Values[0] != "" } -func (e ElementSetter) Filter(rn *RNode) (*RNode, error) { +// the main difference between this Filter and the ElementSetter Filter +// is that here we must iterate through all the key-value pairs +func (e ElementSetterList) Filter(rn *RNode) (*RNode, error) { if err := ErrorIfInvalid(rn, SequenceNode); err != nil { return nil, err } @@ -96,11 +127,18 @@ func (e ElementSetter) Filter(rn *RNode) (*RNode, error) { } // check if this is the element we are matching - val, err := newNode.Pipe(FieldMatcher{Name: e.Key, StringValue: e.Value}) - if err != nil { - return nil, err + found := true + for j, key := range e.Keys { + val, err := newNode.Pipe(FieldMatcher{Name: key, StringValue: e.Values[j]}) + if err != nil { + return nil, err + } + if val == nil { + found = false + break + } } - if val == nil { + if !found { // not the element we are looking for, keep it in the Content newContent = append(newContent, elem) continue @@ -242,18 +280,54 @@ type ElementMatcher struct { } func (e ElementMatcher) Filter(rn *RNode) (*RNode, error) { + return rn.Pipe(ElementMatcherList{ + Kind: e.Kind, + Keys: []string{e.FieldName}, + Values: []string{e.FieldValue}, + Create: e.Create, + MatchAnyValue: e.MatchAnyValue, + }) +} + +func MatchElementList(keys []string, values []string) ElementMatcherList { + return ElementMatcherList{Keys: keys, Values: values} +} + +// ElementMatcherList returns the first element from a Sequence matching all +// specified key, value pairs (as opposed to ElementMatcher, which matches +// a single key, value pair). If there's no match, and no configuration error, +// the matcher returns nil, nil. +type ElementMatcherList struct { + Kind string `yaml:"kind,omitempty"` + + // Keys are the list of fields upon which to match this element. + Keys []string + + // Values are the list of values upon which to match this element. + Values []string + + // Create will create the Element if it is not found + Create *RNode `yaml:"create,omitempty"` + + // MatchAnyValue indicates that matcher should only consider the key and ignore + // the actual value in the list. FieldValue must be empty when NoValue is + // set to true. + MatchAnyValue bool `yaml:"noValue,omitempty"` +} + +func (e ElementMatcherList) Filter(rn *RNode) (*RNode, error) { if err := ErrorIfInvalid(rn, yaml.SequenceNode); err != nil { return nil, err } - if e.MatchAnyValue && e.FieldValue != "" { + if e.MatchAnyValue && len(e.Values) != 0 && e.Values[0] != "" { return nil, fmt.Errorf("FieldValue must be empty when NoValue is set to true") } // SequenceNode Content is a slice of ScalarNodes. Each ScalarNode has a // YNode containing the primitive data. - if len(e.FieldName) == 0 { + if len(e.Keys) == 0 || len(e.Keys[0]) == 0 { for i := range rn.Content() { - if rn.Content()[i].Value == e.FieldValue { + if rn.Content()[i].Value == e.Values[0] { return &RNode{value: rn.Content()[i]}, nil } } @@ -268,20 +342,28 @@ func (e ElementMatcher) Filter(rn *RNode) (*RNode, error) { for i := range rn.Content() { // cast the entry to a RNode so we can operate on it elem := NewRNode(rn.Content()[i]) + var field *RNode + var err error // only check mapping node - if err := ErrorIfInvalid(elem, yaml.MappingNode); err != nil { + if err = ErrorIfInvalid(elem, yaml.MappingNode); err != nil { continue } - var field *RNode - var err error - if e.MatchAnyValue { - field, err = elem.Pipe(Get(e.FieldName)) - } else { - field, err = elem.Pipe(MatchField(e.FieldName, e.FieldValue)) + matchesElement := true + for i, key := range e.Keys { + if e.MatchAnyValue { + field, err = elem.Pipe(Get(key)) + } else { + field, err = elem.Pipe(MatchField(key, e.Values[i])) + } + if !IsFoundOrError(field, err) { + // this is not the element we are looking for + matchesElement = false + break + } } - if IsFoundOrError(field, err) { + if matchesElement { return elem, err } } @@ -351,7 +433,7 @@ func (f FieldMatcher) Filter(rn *RNode) (*RNode, error) { return rn, nil } return nil, nil - case rn.value.Value == f.Value.YNode().Value: + case f.Value.YNode() != nil && rn.value.Value == f.Value.YNode().Value: return rn, nil default: return nil, nil @@ -404,7 +486,7 @@ type PathGetter struct { // See FieldMatcher for more on Fields and Map Keys. // // List Entries can be specified as map entry to match [fieldName=fieldValue] - // or a postional index like 0 to get the element. - (unquoted hyphen) is + // or a positional index like 0 to get the element. - (unquoted hyphen) is // special and means the last element. // // See Elem for more on List Entries. diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index 1b8628c97..4a060233a 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -217,6 +217,133 @@ func TestElementSetter(t *testing.T) { `, assertNoErrorString(t)(node.String())) } +func TestElementSetterList(t *testing.T) { + orig := MustParse(` +- a: b + c: d +- scalarValue +- e: f +# null will be removed +- null +`) + + // ElementSetter will update node, so make a copy + node := orig.Copy() + // Remove an element using one key-value pair, + // because ElementSetter.Element is left nil. + rn, err := node.Pipe(ElementSetterList{ + Keys: []string{"a"}, + Values: []string{"b"}, + }) + assert.NoError(t, err) + assert.Nil(t, rn) + assert.Equal(t, `- scalarValue +- e: f +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Remove an element using multiple key-value pairs, + // because ElementSetter.Element is left nil. + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"a", "c"}, + Values: []string{"b", "d"}, + }) + assert.NoError(t, err) + assert.Nil(t, rn) + assert.Equal(t, `- scalarValue +- e: f +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Should do nothing, because Element is nil + // and there is no element which matches all + // give key-value pairs + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"a", "c"}, + Values: []string{"b", "wrong value"}, + }) + assert.NoError(t, err) + assert.Nil(t, rn) + assert.Equal(t, `- a: b + c: d +- scalarValue +- e: f +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Set an element, with a single key-value pair + // replacing 'a: b, c: d' with 'g: h' + newElement := NewMapRNode(&map[string]string{ + "g": "h", + }) + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"a"}, + Values: []string{"b"}, + Element: newElement.YNode(), + }) + assert.NoError(t, err) + assert.Equal(t, rn, newElement) + assert.Equal(t, `- g: h +- scalarValue +- e: f +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Set an element, with multiple key-value pairs + // replacing 'a: b, c: d' with 'g: h' + newElement = NewMapRNode(&map[string]string{ + "g": "h", + }) + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"a", "c"}, + Values: []string{"b", "d"}, + Element: newElement.YNode(), + }) + assert.NoError(t, err) + assert.Equal(t, rn, newElement) + assert.Equal(t, `- g: h +- scalarValue +- e: f +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Set an element scalar, + // {'a: b, c: d'} to "foo" + newElement = NewScalarRNode("foo") + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"a", "c"}, + Values: []string{"b", "d"}, + Element: newElement.YNode(), + }) + assert.NoError(t, err) + assert.Equal(t, rn, newElement) + assert.Equal(t, `- foo +- scalarValue +- e: f +`, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Append an element + // There is no element which matches all given + // key-value pairs, so the element will be appended. + newElement = NewMapRNode(&map[string]string{ + "g": "h", + }) + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"a", "c"}, + Values: []string{"b", "wrong value"}, + Element: newElement.YNode(), + }) + assert.NoError(t, err) + assert.Equal(t, rn, newElement) + assert.Equal(t, `- a: b + c: d +- scalarValue +- e: f +- g: h +`, assertNoErrorString(t)(node.String())) +} + func TestElementMatcherWithNoValue(t *testing.T) { node, err := Parse(` - a: c @@ -239,6 +366,48 @@ func TestElementMatcherWithNoValue(t *testing.T) { _, err = node.Pipe(ElementMatcher{FieldName: "a", FieldValue: "c", MatchAnyValue: true}) assert.Errorf(t, err, "FieldValue must be empty when NoValue is set to true") } + +func TestElementMatcherList(t *testing.T) { + node, err := Parse(` +- a: b + c: d +- e: f +`) + assert.NoError(t, err) + + // matches all key-value pairs + rn, err := node.Pipe(MatchElementList( + []string{"a", "c"}, // keys + []string{"b", "d"}, // values + )) + assert.NoError(t, err) + assert.NotEmpty(t, rn) + + // matches one key value pair but not the other + rn, err = node.Pipe(MatchElementList( + []string{"a", "c"}, // keys + []string{"b", "f"}, // values + )) + assert.NoError(t, err) + assert.Nil(t, rn) + + // matches single given key value pair + rn, err = node.Pipe(MatchElementList( + []string{"e"}, // keys + []string{"f"}, // values + )) + assert.NoError(t, err) + assert.NotEmpty(t, rn) + + // matching key, but value doesn't match + rn, err = node.Pipe(MatchElementList( + []string{"e"}, // keys + []string{"g"}, // values + )) + assert.NoError(t, err) + assert.Nil(t, rn) +} + func TestClearField_Fn(t *testing.T) { node, err := Parse(NodeSampleData) assert.NoError(t, err) From 99aaa80e1dac3dc04807c5508460a69f12df435c Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Thu, 5 Nov 2020 11:39:50 -0800 Subject: [PATCH 2/9] updated stale comments --- kyaml/yaml/fns.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 1f75eedb6..053f6d623 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -274,7 +274,7 @@ type ElementMatcher struct { Create *RNode `yaml:"create,omitempty"` // MatchAnyValue indicates that matcher should only consider the key and ignore - // the actual value in the list. FieldValue must be empty when NoValue is + // the actual value in the list. FieldValue must be empty when MatchAnyValue is // set to true. MatchAnyValue bool `yaml:"noValue,omitempty"` } @@ -310,7 +310,7 @@ type ElementMatcherList struct { Create *RNode `yaml:"create,omitempty"` // MatchAnyValue indicates that matcher should only consider the key and ignore - // the actual value in the list. FieldValue must be empty when NoValue is + // the actual value in the list. Values must be empty when MatchAnyValue is // set to true. MatchAnyValue bool `yaml:"noValue,omitempty"` } @@ -320,7 +320,7 @@ func (e ElementMatcherList) Filter(rn *RNode) (*RNode, error) { return nil, err } if e.MatchAnyValue && len(e.Values) != 0 && e.Values[0] != "" { - return nil, fmt.Errorf("FieldValue must be empty when NoValue is set to true") + return nil, fmt.Errorf("Values must be empty when MatchAnyValue is set to true") } // SequenceNode Content is a slice of ScalarNodes. Each ScalarNode has a From 03e2fed9255a7ba133ac40ae1bee5577acdd4789 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Thu, 5 Nov 2020 12:16:17 -0800 Subject: [PATCH 3/9] checked array length --- kyaml/yaml/fns.go | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 053f6d623..382204050 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -126,10 +126,15 @@ func (e ElementSetterList) Filter(rn *RNode) (*RNode, error) { continue } + // make sure keys and values are the same length + if len(e.Keys) != len(e.Values) { + return nil, fmt.Errorf("length of keys and values must be the same") + } + // check if this is the element we are matching found := true - for j, key := range e.Keys { - val, err := newNode.Pipe(FieldMatcher{Name: key, StringValue: e.Values[j]}) + for j := range e.Keys { + val, err := newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: e.Values[j]}) if err != nil { return nil, err } @@ -350,12 +355,16 @@ func (e ElementMatcherList) Filter(rn *RNode) (*RNode, error) { continue } + if !e.MatchAnyValue && len(e.Keys) != len(e.Values) { + return nil, fmt.Errorf("length of keys must equal length of values when MatchAnyValue is false") + } + matchesElement := true - for i, key := range e.Keys { + for i := range e.Keys { if e.MatchAnyValue { - field, err = elem.Pipe(Get(key)) + field, err = elem.Pipe(Get(e.Keys[i])) } else { - field, err = elem.Pipe(MatchField(key, e.Values[i])) + field, err = elem.Pipe(MatchField(e.Keys[i], e.Values[i])) } if !IsFoundOrError(field, err) { // this is not the element we are looking for From a04e3a575cd4d21a1cdded86e948c6cc1b71a9fe Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Thu, 5 Nov 2020 12:19:54 -0800 Subject: [PATCH 4/9] added test case for different length keys/values --- kyaml/yaml/fns_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index 4a060233a..30bc8f794 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -342,6 +342,20 @@ func TestElementSetterList(t *testing.T) { - e: f - g: h `, assertNoErrorString(t)(node.String())) + + node = orig.Copy() + // Should return an error + // keys and values are not the same length + newElement = NewMapRNode(&map[string]string{ + "g": "h", + }) + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"a", "c"}, + Values: []string{"b"}, + Element: newElement.YNode(), + }) + assert.Error(t, err) + assert.Nil(t, rn) } func TestElementMatcherWithNoValue(t *testing.T) { From 8f80a898b69d109de306a2d962fd24faa8cf9686 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Thu, 5 Nov 2020 16:50:18 -0800 Subject: [PATCH 5/9] removed elementsetterlist and updated elementsetter --- kyaml/yaml/example_test.go | 10 +++--- kyaml/yaml/fns.go | 74 ++++++++++++++------------------------ kyaml/yaml/fns_test.go | 44 +++++++++++------------ 3 files changed, 55 insertions(+), 73 deletions(-) diff --git a/kyaml/yaml/example_test.go b/kyaml/yaml/example_test.go index baa8e8ed9..89da82731 100644 --- a/kyaml/yaml/example_test.go +++ b/kyaml/yaml/example_test.go @@ -228,7 +228,7 @@ func ExampleElementMatcher_Filter() { log.Fatal(err) } elem, err := obj.Pipe(ElementMatcher{ - FieldValue: "c", Create: NewScalarRNode("c"), + Values: []string{"c"}, Create: NewScalarRNode("c"), }) if err != nil { log.Fatal(err) @@ -255,7 +255,9 @@ func ExampleElementMatcher_Filter_primitiveFound() { log.Fatal(err) } elem, err := obj.Pipe(ElementMatcher{ - FieldValue: "c", Create: NewScalarRNode("c"), + Keys: []string{""}, + Values: []string{"c"}, + Create: NewScalarRNode("c"), }) if err != nil { log.Fatal(err) @@ -287,7 +289,7 @@ image: nginx log.Fatal(err) } elem, err := obj.Pipe(ElementMatcher{ - FieldName: "name", FieldValue: "baz", Create: append}) + Keys: []string{"name"}, Values: []string{"baz"}, Create: append}) if err != nil { log.Fatal(err) } @@ -321,7 +323,7 @@ image: nginx log.Fatal(err) } elem, err := obj.Pipe(ElementMatcher{ - FieldName: "name", FieldValue: "baz", Create: append}) + Keys: []string{"name"}, Values: []string{"baz"}, Create: append}) if err != nil { log.Fatal(err) } diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 382204050..72e44c6b6 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -65,6 +65,7 @@ type ElementSetter struct { func (e ElementSetter) Filter(rn *RNode) (*RNode, error) { return rn.Pipe(ElementSetterList{ + Kind: e.Kind, Element: e.Element, Keys: []string{e.Key}, Values: []string{e.Value}, @@ -105,6 +106,13 @@ func (e ElementSetterList) isMappingSetter() bool { // the main difference between this Filter and the ElementSetter Filter // is that here we must iterate through all the key-value pairs func (e ElementSetterList) Filter(rn *RNode) (*RNode, error) { + if len(e.Keys) == 0 { + e.Keys = append(e.Keys, "") + } + if len(e.Values) == 0 { + e.Values = append(e.Values, "") + } + if err := ErrorIfInvalid(rn, SequenceNode); err != nil { return nil, err } @@ -126,15 +134,20 @@ func (e ElementSetterList) Filter(rn *RNode) (*RNode, error) { continue } - // make sure keys and values are the same length - if len(e.Keys) != len(e.Values) { - return nil, fmt.Errorf("length of keys and values must be the same") + if len(e.Keys) > len(e.Values) { + return nil, fmt.Errorf("cannot have more keys than values") } // check if this is the element we are matching + var val *RNode + var err error found := true for j := range e.Keys { - val, err := newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: e.Values[j]}) + if j >= len(e.Values) { + val, err = newNode.Pipe(Get(e.Keys[j])) + } else { + val, err = newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: e.Values[j]}) + } if err != nil { return nil, err } @@ -254,57 +267,24 @@ func (c FieldClearer) Filter(rn *RNode) (*RNode, error) { } func MatchElement(field, value string) ElementMatcher { - return ElementMatcher{FieldName: field, FieldValue: value} + return ElementMatcher{Keys: []string{field}, Values: []string{value}} +} + +func MatchElementList(keys []string, values []string) ElementMatcher { + return ElementMatcher{Keys: keys, Values: values} } func GetElementByKey(key string) ElementMatcher { - return ElementMatcher{FieldName: key, MatchAnyValue: true} + return ElementMatcher{Keys: []string{key}, MatchAnyValue: true} } // ElementMatcher returns the first element from a Sequence matching the -// specified field's value. If there's no match, and no configuration error, +// specified key-value pairs. If there's no match, and no configuration error, // the matcher returns nil, nil. + type ElementMatcher struct { Kind string `yaml:"kind,omitempty"` - // FieldName will attempt to match this field in each list element. - // Optional. Leave empty for lists of primitives (ScalarNode). - FieldName string `yaml:"name,omitempty"` - - // FieldValue will attempt to match each element field to this value. - // For lists of primitives, this will be used to match the primitive value. - FieldValue string `yaml:"value,omitempty"` - - // Create will create the Element if it is not found - Create *RNode `yaml:"create,omitempty"` - - // MatchAnyValue indicates that matcher should only consider the key and ignore - // the actual value in the list. FieldValue must be empty when MatchAnyValue is - // set to true. - MatchAnyValue bool `yaml:"noValue,omitempty"` -} - -func (e ElementMatcher) Filter(rn *RNode) (*RNode, error) { - return rn.Pipe(ElementMatcherList{ - Kind: e.Kind, - Keys: []string{e.FieldName}, - Values: []string{e.FieldValue}, - Create: e.Create, - MatchAnyValue: e.MatchAnyValue, - }) -} - -func MatchElementList(keys []string, values []string) ElementMatcherList { - return ElementMatcherList{Keys: keys, Values: values} -} - -// ElementMatcherList returns the first element from a Sequence matching all -// specified key, value pairs (as opposed to ElementMatcher, which matches -// a single key, value pair). If there's no match, and no configuration error, -// the matcher returns nil, nil. -type ElementMatcherList struct { - Kind string `yaml:"kind,omitempty"` - // Keys are the list of fields upon which to match this element. Keys []string @@ -320,7 +300,7 @@ type ElementMatcherList struct { MatchAnyValue bool `yaml:"noValue,omitempty"` } -func (e ElementMatcherList) Filter(rn *RNode) (*RNode, error) { +func (e ElementMatcher) Filter(rn *RNode) (*RNode, error) { if err := ErrorIfInvalid(rn, yaml.SequenceNode); err != nil { return nil, err } @@ -595,7 +575,7 @@ func (l PathGetter) elemFilter(part string) (Filter, error) { }) } // Append the Node - return ElementMatcher{FieldName: name, FieldValue: value, Create: elem}, nil + return ElementMatcher{Keys: []string{name}, Values: []string{value}, Create: elem}, nil } func (l PathGetter) fieldFilter( diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index 30bc8f794..febdd9923 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -109,7 +109,7 @@ func TestElementSetter(t *testing.T) { // ElementSetter will update node, so make a copy node := orig.Copy() // Remove an element, because ElementSetter.Element is left nil. - rn, err := node.Pipe(ElementSetter{Key: "a", Value: "b"}) + rn, err := node.Pipe(ElementSetterList{Keys: []string{"a"}, Values: []string{"b"}}) assert.NoError(t, err) assert.Nil(t, rn) assert.Equal(t, `- scalarValue @@ -118,7 +118,7 @@ func TestElementSetter(t *testing.T) { node = orig.Copy() // Nothing happens because no element is matched - rn, err = node.Pipe(ElementSetter{Key: "a", Value: "zebra"}) + rn, err = node.Pipe(ElementSetterList{Keys: []string{"a"}, Values: []string{"zebra"}}) assert.NoError(t, err) assert.Nil(t, rn) assert.Equal(t, `- a: b @@ -129,7 +129,7 @@ func TestElementSetter(t *testing.T) { node = orig.Copy() // Return error because ElementSetter doesn't support a single key // when there is a scalar value in the list - _, err = node.Pipe(ElementSetter{Key: "a"}) + _, err = node.Pipe(ElementSetterList{Keys: []string{"a"}}) assert.EqualError(t, err, "wrong Node Kind for expected: MappingNode was ScalarNode: value: {scalarValue}") node = MustParse(` @@ -138,7 +138,7 @@ func TestElementSetter(t *testing.T) { `) // {a: b} is removed since the value is omitted and only key is used // to match and no Element specified. - rn, err = node.Pipe(ElementSetter{Key: "a"}) + rn, err = node.Pipe(ElementSetterList{Keys: []string{"a"}}) assert.NoError(t, err) assert.Nil(t, rn) assert.Equal(t, `- c: d @@ -150,7 +150,7 @@ func TestElementSetter(t *testing.T) { `) // Return error because ElementSetter will assume all elements are scalar when // there is only value provided. - _, err = node.Pipe(ElementSetter{Value: "b"}) + _, err = node.Pipe(ElementSetterList{Values: []string{"b"}}) assert.EqualError(t, err, "wrong Node Kind for expected: ScalarNode was MappingNode: value: {a: b}") node = MustParse(` @@ -159,7 +159,7 @@ func TestElementSetter(t *testing.T) { `) // b is removed since ElementSetter use the value "b" to match the // scalar values. - rn, err = node.Pipe(ElementSetter{Value: "b"}) + rn, err = node.Pipe(ElementSetterList{Values: []string{"b"}}) assert.NoError(t, err) assert.Nil(t, rn) assert.Equal(t, `- a @@ -170,9 +170,9 @@ func TestElementSetter(t *testing.T) { newElement := NewMapRNode(&map[string]string{ "e": "f", }) - rn, err = node.Pipe(ElementSetter{ - Key: "a", - Value: "b", + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"a"}, + Values: []string{"b"}, Element: newElement.YNode(), }) assert.NoError(t, err) @@ -185,9 +185,9 @@ func TestElementSetter(t *testing.T) { node = orig.Copy() // Set an element with scalar, {"a": "b"} to "foo" newElement = NewScalarRNode("foo") - rn, err = node.Pipe(ElementSetter{ - Key: "a", - Value: "b", + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"a"}, + Values: []string{"b"}, Element: newElement.YNode(), }) assert.NoError(t, err) @@ -203,9 +203,9 @@ func TestElementSetter(t *testing.T) { newElement = NewMapRNode(&map[string]string{ "e": "f", }) - rn, err = node.Pipe(ElementSetter{ - Key: "x", - Value: "y", + rn, err = node.Pipe(ElementSetterList{ + Keys: []string{"x"}, + Values: []string{"y"}, Element: newElement.YNode(), }) assert.NoError(t, err) @@ -217,7 +217,7 @@ func TestElementSetter(t *testing.T) { `, assertNoErrorString(t)(node.String())) } -func TestElementSetterList(t *testing.T) { +func TestElementSetterMultipleKeys(t *testing.T) { orig := MustParse(` - a: b c: d @@ -365,23 +365,23 @@ func TestElementMatcherWithNoValue(t *testing.T) { `) assert.NoError(t, err) - rn, err := node.Pipe(ElementMatcher{FieldName: "b"}) + rn, err := node.Pipe(ElementMatcher{Keys: []string{"b"}}) assert.NoError(t, err) assert.Equal(t, "b: \"\"\n", assertNoErrorString(t)(rn.String())) - rn, err = node.Pipe(ElementMatcher{FieldName: "a"}) - assert.NoError(t, err) + rn, err = node.Pipe(ElementMatcher{Keys: []string{"a"}}) + assert.Error(t, err) assert.Nil(t, rn) - rn, err = node.Pipe(ElementMatcher{FieldName: "a", MatchAnyValue: true}) + rn, err = node.Pipe(ElementMatcher{Keys: []string{"a"}, MatchAnyValue: true}) assert.NoError(t, err) assert.Equal(t, "a: c\n", assertNoErrorString(t)(rn.String())) - _, err = node.Pipe(ElementMatcher{FieldName: "a", FieldValue: "c", MatchAnyValue: true}) + _, err = node.Pipe(ElementMatcher{Keys: []string{"a"}, Values: []string{"c"}, MatchAnyValue: true}) assert.Errorf(t, err, "FieldValue must be empty when NoValue is set to true") } -func TestElementMatcherList(t *testing.T) { +func TestElementMatcherMultipleKeys(t *testing.T) { node, err := Parse(` - a: b c: d From e785bab47493fbd33d9877cf410d360843355386 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Thu, 5 Nov 2020 16:52:36 -0800 Subject: [PATCH 6/9] updated matchelementlist --- kyaml/yaml/fns.go | 8 +++++++- kyaml/yaml/fns_test.go | 16 ++++++++-------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 72e44c6b6..fa1ad609b 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -281,7 +281,6 @@ func GetElementByKey(key string) ElementMatcher { // ElementMatcher returns the first element from a Sequence matching the // specified key-value pairs. If there's no match, and no configuration error, // the matcher returns nil, nil. - type ElementMatcher struct { Kind string `yaml:"kind,omitempty"` @@ -301,6 +300,13 @@ type ElementMatcher struct { } func (e ElementMatcher) Filter(rn *RNode) (*RNode, error) { + if len(e.Keys) == 0 { + e.Keys = append(e.Keys, "") + } + if len(e.Values) == 0 { + e.Values = append(e.Values, "") + } + if err := ErrorIfInvalid(rn, yaml.SequenceNode); err != nil { return nil, err } diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index febdd9923..9f976883f 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -171,8 +171,8 @@ func TestElementSetter(t *testing.T) { "e": "f", }) rn, err = node.Pipe(ElementSetterList{ - Keys: []string{"a"}, - Values: []string{"b"}, + Keys: []string{"a"}, + Values: []string{"b"}, Element: newElement.YNode(), }) assert.NoError(t, err) @@ -186,8 +186,8 @@ func TestElementSetter(t *testing.T) { // Set an element with scalar, {"a": "b"} to "foo" newElement = NewScalarRNode("foo") rn, err = node.Pipe(ElementSetterList{ - Keys: []string{"a"}, - Values: []string{"b"}, + Keys: []string{"a"}, + Values: []string{"b"}, Element: newElement.YNode(), }) assert.NoError(t, err) @@ -204,8 +204,8 @@ func TestElementSetter(t *testing.T) { "e": "f", }) rn, err = node.Pipe(ElementSetterList{ - Keys: []string{"x"}, - Values: []string{"y"}, + Keys: []string{"x"}, + Values: []string{"y"}, Element: newElement.YNode(), }) assert.NoError(t, err) @@ -370,7 +370,7 @@ func TestElementMatcherWithNoValue(t *testing.T) { assert.Equal(t, "b: \"\"\n", assertNoErrorString(t)(rn.String())) rn, err = node.Pipe(ElementMatcher{Keys: []string{"a"}}) - assert.Error(t, err) + assert.NoError(t, err) assert.Nil(t, rn) rn, err = node.Pipe(ElementMatcher{Keys: []string{"a"}, MatchAnyValue: true}) @@ -378,7 +378,7 @@ func TestElementMatcherWithNoValue(t *testing.T) { assert.Equal(t, "a: c\n", assertNoErrorString(t)(rn.String())) _, err = node.Pipe(ElementMatcher{Keys: []string{"a"}, Values: []string{"c"}, MatchAnyValue: true}) - assert.Errorf(t, err, "FieldValue must be empty when NoValue is set to true") + assert.Errorf(t, err, "Values must be empty when MatchAnyValue is set to true") } func TestElementMatcherMultipleKeys(t *testing.T) { From 9f06376ab2103f33f08d8dc8411f46ca501cd33a Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Thu, 5 Nov 2020 17:11:37 -0800 Subject: [PATCH 7/9] updated associative sequence --- kyaml/yaml/fns.go | 43 +++---------------------- kyaml/yaml/fns_test.go | 34 +++++++++---------- kyaml/yaml/merge2/smpdirective.go | 4 +-- kyaml/yaml/walk/associative_sequence.go | 23 ++++++++++--- 4 files changed, 42 insertions(+), 62 deletions(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index fa1ad609b..940db9ba0 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -54,35 +54,6 @@ type ElementSetter struct { // Element is the new value to set -- remove the existing element if nil Element *Node - // Key is a field on the elements. It is used to find the matching element to - // update / delete. - Key string `yaml:"key,omitempty"` - - // Value is a field value on the elements. It is used to find matching elements to - // update / delete. - Value string `yaml:"value,omitempty"` -} - -func (e ElementSetter) Filter(rn *RNode) (*RNode, error) { - return rn.Pipe(ElementSetterList{ - Kind: e.Kind, - Element: e.Element, - Keys: []string{e.Key}, - Values: []string{e.Value}, - }) -} - -// ElementSetterList sets the value for an Element in an associative list. -// It behaves identically to ElementSetter, except that it uses multiple -// key-value pairs (in the form of a list of keys and a corresponding list -// of values) in order to find a matching element, whereas ElementSetter -// uses only one key-value pair. -type ElementSetterList struct { - Kind string `yaml:"kind,omitempty"` - - // Element is the new value to set -- remove the existing element if nil - Element *Node - // Key is a list of fields on the elements. It is used to find matching elements to // update / delete Keys []string @@ -93,19 +64,17 @@ type ElementSetterList struct { } // isMappingNode returns whether node is a mapping node -func (e ElementSetterList) isMappingNode(node *RNode) bool { +func (e ElementSetter) isMappingNode(node *RNode) bool { return ErrorIfInvalid(node, yaml.MappingNode) == nil } // isMappingSetter returns is this setter intended to set a mapping node -func (e ElementSetterList) isMappingSetter() bool { +func (e ElementSetter) isMappingSetter() bool { return len(e.Keys) > 0 && e.Keys[0] != "" && len(e.Values) > 0 && e.Values[0] != "" } -// the main difference between this Filter and the ElementSetter Filter -// is that here we must iterate through all the key-value pairs -func (e ElementSetterList) Filter(rn *RNode) (*RNode, error) { +func (e ElementSetter) Filter(rn *RNode) (*RNode, error) { if len(e.Keys) == 0 { e.Keys = append(e.Keys, "") } @@ -143,11 +112,7 @@ func (e ElementSetterList) Filter(rn *RNode) (*RNode, error) { var err error found := true for j := range e.Keys { - if j >= len(e.Values) { - val, err = newNode.Pipe(Get(e.Keys[j])) - } else { - val, err = newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: e.Values[j]}) - } + val, err = newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: e.Values[j]}) if err != nil { return nil, err } diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index 9f976883f..f9dc606bd 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -109,7 +109,7 @@ func TestElementSetter(t *testing.T) { // ElementSetter will update node, so make a copy node := orig.Copy() // Remove an element, because ElementSetter.Element is left nil. - rn, err := node.Pipe(ElementSetterList{Keys: []string{"a"}, Values: []string{"b"}}) + rn, err := node.Pipe(ElementSetter{Keys: []string{"a"}, Values: []string{"b"}}) assert.NoError(t, err) assert.Nil(t, rn) assert.Equal(t, `- scalarValue @@ -118,7 +118,7 @@ func TestElementSetter(t *testing.T) { node = orig.Copy() // Nothing happens because no element is matched - rn, err = node.Pipe(ElementSetterList{Keys: []string{"a"}, Values: []string{"zebra"}}) + rn, err = node.Pipe(ElementSetter{Keys: []string{"a"}, Values: []string{"zebra"}}) assert.NoError(t, err) assert.Nil(t, rn) assert.Equal(t, `- a: b @@ -129,7 +129,7 @@ func TestElementSetter(t *testing.T) { node = orig.Copy() // Return error because ElementSetter doesn't support a single key // when there is a scalar value in the list - _, err = node.Pipe(ElementSetterList{Keys: []string{"a"}}) + _, err = node.Pipe(ElementSetter{Keys: []string{"a"}}) assert.EqualError(t, err, "wrong Node Kind for expected: MappingNode was ScalarNode: value: {scalarValue}") node = MustParse(` @@ -138,7 +138,7 @@ func TestElementSetter(t *testing.T) { `) // {a: b} is removed since the value is omitted and only key is used // to match and no Element specified. - rn, err = node.Pipe(ElementSetterList{Keys: []string{"a"}}) + rn, err = node.Pipe(ElementSetter{Keys: []string{"a"}}) assert.NoError(t, err) assert.Nil(t, rn) assert.Equal(t, `- c: d @@ -150,7 +150,7 @@ func TestElementSetter(t *testing.T) { `) // Return error because ElementSetter will assume all elements are scalar when // there is only value provided. - _, err = node.Pipe(ElementSetterList{Values: []string{"b"}}) + _, err = node.Pipe(ElementSetter{Values: []string{"b"}}) assert.EqualError(t, err, "wrong Node Kind for expected: ScalarNode was MappingNode: value: {a: b}") node = MustParse(` @@ -159,7 +159,7 @@ func TestElementSetter(t *testing.T) { `) // b is removed since ElementSetter use the value "b" to match the // scalar values. - rn, err = node.Pipe(ElementSetterList{Values: []string{"b"}}) + rn, err = node.Pipe(ElementSetter{Values: []string{"b"}}) assert.NoError(t, err) assert.Nil(t, rn) assert.Equal(t, `- a @@ -170,7 +170,7 @@ func TestElementSetter(t *testing.T) { newElement := NewMapRNode(&map[string]string{ "e": "f", }) - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"a"}, Values: []string{"b"}, Element: newElement.YNode(), @@ -185,7 +185,7 @@ func TestElementSetter(t *testing.T) { node = orig.Copy() // Set an element with scalar, {"a": "b"} to "foo" newElement = NewScalarRNode("foo") - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"a"}, Values: []string{"b"}, Element: newElement.YNode(), @@ -203,7 +203,7 @@ func TestElementSetter(t *testing.T) { newElement = NewMapRNode(&map[string]string{ "e": "f", }) - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"x"}, Values: []string{"y"}, Element: newElement.YNode(), @@ -231,7 +231,7 @@ func TestElementSetterMultipleKeys(t *testing.T) { node := orig.Copy() // Remove an element using one key-value pair, // because ElementSetter.Element is left nil. - rn, err := node.Pipe(ElementSetterList{ + rn, err := node.Pipe(ElementSetter{ Keys: []string{"a"}, Values: []string{"b"}, }) @@ -244,7 +244,7 @@ func TestElementSetterMultipleKeys(t *testing.T) { node = orig.Copy() // Remove an element using multiple key-value pairs, // because ElementSetter.Element is left nil. - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"a", "c"}, Values: []string{"b", "d"}, }) @@ -258,7 +258,7 @@ func TestElementSetterMultipleKeys(t *testing.T) { // Should do nothing, because Element is nil // and there is no element which matches all // give key-value pairs - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"a", "c"}, Values: []string{"b", "wrong value"}, }) @@ -276,7 +276,7 @@ func TestElementSetterMultipleKeys(t *testing.T) { newElement := NewMapRNode(&map[string]string{ "g": "h", }) - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"a"}, Values: []string{"b"}, Element: newElement.YNode(), @@ -294,7 +294,7 @@ func TestElementSetterMultipleKeys(t *testing.T) { newElement = NewMapRNode(&map[string]string{ "g": "h", }) - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"a", "c"}, Values: []string{"b", "d"}, Element: newElement.YNode(), @@ -310,7 +310,7 @@ func TestElementSetterMultipleKeys(t *testing.T) { // Set an element scalar, // {'a: b, c: d'} to "foo" newElement = NewScalarRNode("foo") - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"a", "c"}, Values: []string{"b", "d"}, Element: newElement.YNode(), @@ -329,7 +329,7 @@ func TestElementSetterMultipleKeys(t *testing.T) { newElement = NewMapRNode(&map[string]string{ "g": "h", }) - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"a", "c"}, Values: []string{"b", "wrong value"}, Element: newElement.YNode(), @@ -349,7 +349,7 @@ func TestElementSetterMultipleKeys(t *testing.T) { newElement = NewMapRNode(&map[string]string{ "g": "h", }) - rn, err = node.Pipe(ElementSetterList{ + rn, err = node.Pipe(ElementSetter{ Keys: []string{"a", "c"}, Values: []string{"b"}, Element: newElement.YNode(), diff --git a/kyaml/yaml/merge2/smpdirective.go b/kyaml/yaml/merge2/smpdirective.go index b5b2fbb7e..f38b188ea 100644 --- a/kyaml/yaml/merge2/smpdirective.go +++ b/kyaml/yaml/merge2/smpdirective.go @@ -95,7 +95,7 @@ func elideMappingPatchDirective(patch *yaml.RNode) error { func elideSequencePatchDirective(patch *yaml.RNode, value string) error { return patch.PipeE(yaml.ElementSetter{ Element: nil, - Key: strategicMergePatchDirectiveKey, - Value: value, + Keys: []string{strategicMergePatchDirectiveKey}, + Values: []string{value}, }) } diff --git a/kyaml/yaml/walk/associative_sequence.go b/kyaml/yaml/walk/associative_sequence.go index 66ead3dcc..6f3e25cd8 100644 --- a/kyaml/yaml/walk/associative_sequence.go +++ b/kyaml/yaml/walk/associative_sequence.go @@ -21,7 +21,11 @@ func appendListNode(dst, src *yaml.RNode, key string) (*yaml.RNode, error) { // If key is empty, we know this is a scalar value and we can directly set the // node if key == "" { - _, err := dst.Pipe(yaml.ElementSetter{Element: elem, Key: key, Value: elem.Value}) + _, err := dst.Pipe(yaml.ElementSetter{ + Element: elem, + Keys: []string{key}, + Values: []string{elem.Value}, + }) if err != nil { return nil, err } @@ -46,7 +50,11 @@ func appendListNode(dst, src *yaml.RNode, key string) (*yaml.RNode, error) { // We use the key and value from elem to find the corresponding element in dst. // Then we will use ElementSetter to replace the element with elem. If we cannot // find the item, the element will be appended. - _, err = dst.Pipe(yaml.ElementSetter{Element: elem, Key: key, Value: v}) + _, err = dst.Pipe(yaml.ElementSetter{ + Element: elem, + Keys: []string{key}, + Values: []string{v}, + }) if err != nil { return nil, err } @@ -76,7 +84,10 @@ func (l *Walker) setAssociativeSequenceElements(values []string, key string, des } // delete the node from **dest** if it's null or empty if yaml.IsMissingOrNull(val) || yaml.IsEmptyMap(val) { - _, err = dest.Pipe(yaml.ElementSetter{Key: key, Value: value}) + _, err = dest.Pipe(yaml.ElementSetter{ + Keys: []string{key}, + Values: []string{value}, + }) if err != nil { return nil, err } @@ -94,7 +105,11 @@ func (l *Walker) setAssociativeSequenceElements(values []string, key string, des // Add the val to the sequence. val will replace the item in the sequence if // there is an item that matches the key-value pair. Otherwise val will be appended // the the sequence. - _, err = itemsToBeAdded.Pipe(yaml.ElementSetter{Element: val.YNode(), Key: key, Value: value}) + _, err = itemsToBeAdded.Pipe(yaml.ElementSetter{ + Element: val.YNode(), + Keys: []string{key}, + Values: []string{value}, + }) if err != nil { return nil, err } From 73d91dda6e817d117e3d22ea45b3573e57ca32d0 Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Fri, 6 Nov 2020 11:42:17 -0800 Subject: [PATCH 8/9] changed handling of empty values --- kyaml/yaml/fns.go | 18 +++++++++--------- kyaml/yaml/fns_test.go | 30 ------------------------------ 2 files changed, 9 insertions(+), 39 deletions(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 940db9ba0..9c4cb6402 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -48,6 +48,7 @@ func (a ElementAppender) Filter(rn *RNode) (*RNode, error) { // not designed for this purpose. To append an element, please use ElementAppender. // To replace, set the key-value pair and a non-nil Element. // To delete, set the key-value pair and leave the Element as nil. +// Every key must have a corresponding value. type ElementSetter struct { Kind string `yaml:"kind,omitempty"` @@ -78,9 +79,6 @@ func (e ElementSetter) Filter(rn *RNode) (*RNode, error) { if len(e.Keys) == 0 { e.Keys = append(e.Keys, "") } - if len(e.Values) == 0 { - e.Values = append(e.Values, "") - } if err := ErrorIfInvalid(rn, SequenceNode); err != nil { return nil, err @@ -103,16 +101,16 @@ func (e ElementSetter) Filter(rn *RNode) (*RNode, error) { continue } - if len(e.Keys) > len(e.Values) { - return nil, fmt.Errorf("cannot have more keys than values") - } - // check if this is the element we are matching var val *RNode var err error found := true for j := range e.Keys { - val, err = newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: e.Values[j]}) + if j >= len(e.Values) { + val, err = newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: ""}) + } else { + val, err = newNode.Pipe(FieldMatcher{Name: e.Keys[j], StringValue: e.Values[j]}) + } if err != nil { return nil, err } @@ -123,7 +121,9 @@ func (e ElementSetter) Filter(rn *RNode) (*RNode, error) { } if !found { // not the element we are looking for, keep it in the Content - newContent = append(newContent, elem) + if len(e.Values) > 0 { + newContent = append(newContent, elem) + } continue } matchingElementFound = true diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index f9dc606bd..30080e851 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -132,22 +132,6 @@ func TestElementSetter(t *testing.T) { _, err = node.Pipe(ElementSetter{Keys: []string{"a"}}) assert.EqualError(t, err, "wrong Node Kind for expected: MappingNode was ScalarNode: value: {scalarValue}") - node = MustParse(` -- a: b -- c: d -`) - // {a: b} is removed since the value is omitted and only key is used - // to match and no Element specified. - rn, err = node.Pipe(ElementSetter{Keys: []string{"a"}}) - assert.NoError(t, err) - assert.Nil(t, rn) - assert.Equal(t, `- c: d -`, assertNoErrorString(t)(node.String())) - - node = MustParse(` -- a: b -- c: d -`) // Return error because ElementSetter will assume all elements are scalar when // there is only value provided. _, err = node.Pipe(ElementSetter{Values: []string{"b"}}) @@ -342,20 +326,6 @@ func TestElementSetterMultipleKeys(t *testing.T) { - e: f - g: h `, assertNoErrorString(t)(node.String())) - - node = orig.Copy() - // Should return an error - // keys and values are not the same length - newElement = NewMapRNode(&map[string]string{ - "g": "h", - }) - rn, err = node.Pipe(ElementSetter{ - Keys: []string{"a", "c"}, - Values: []string{"b"}, - Element: newElement.YNode(), - }) - assert.Error(t, err) - assert.Nil(t, rn) } func TestElementMatcherWithNoValue(t *testing.T) { From 886f73aa0f042015310d9874d0e74a653a3d5e5f Mon Sep 17 00:00:00 2001 From: Natasha Sarkar Date: Fri, 6 Nov 2020 13:10:18 -0800 Subject: [PATCH 9/9] added test case for no values --- kyaml/yaml/fns.go | 2 +- kyaml/yaml/fns_test.go | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/kyaml/yaml/fns.go b/kyaml/yaml/fns.go index 9c4cb6402..e28b79e24 100644 --- a/kyaml/yaml/fns.go +++ b/kyaml/yaml/fns.go @@ -393,7 +393,7 @@ func (f FieldMatcher) Filter(rn *RNode) (*RNode, error) { return rn, nil } return nil, nil - case f.Value.YNode() != nil && rn.value.Value == f.Value.YNode().Value: + case GetValue(rn) == GetValue(f.Value): return rn, nil default: return nil, nil diff --git a/kyaml/yaml/fns_test.go b/kyaml/yaml/fns_test.go index 30080e851..3331a1fb9 100644 --- a/kyaml/yaml/fns_test.go +++ b/kyaml/yaml/fns_test.go @@ -137,6 +137,27 @@ func TestElementSetter(t *testing.T) { _, err = node.Pipe(ElementSetter{Values: []string{"b"}}) assert.EqualError(t, err, "wrong Node Kind for expected: ScalarNode was MappingNode: value: {a: b}") + node = MustParse(` +- a: b +- c: d +`) + // If given a key and no values, ElementSetter will + // change node to be an empty list + rn, err = node.Pipe(ElementSetter{Keys: []string{"a"}}) + assert.NoError(t, err) + assert.Nil(t, rn) + assert.Equal(t, `[] +`, assertNoErrorString(t)(node.String())) + + node = MustParse(` +- a: b +- c: d +`) + // Return error because ElementSetter will assume all elements are scalar when + // there is only value provided. + _, err = node.Pipe(ElementSetter{Values: []string{"b"}}) + assert.EqualError(t, err, "wrong Node Kind for expected: ScalarNode was MappingNode: value: {a: b}") + node = MustParse(` - a - b