Support merging primitive lists

kyaml/openapi/openapi.go

@@ -285,14 +285,14 @@ func (rs *ResourceSchema) Field(field string) *ResourceSchema {
 func (rs *ResourceSchema) PatchStrategyAndKey() (string, string) {
 	ps, found := rs.Schema.Extensions[kubernetesPatchStrategyExtensionKey]
 	if !found {
-		// merge key and patch strategy must appear together
+		// empty patch strategy
 		return "", ""
 	}
 
 	mk, found := rs.Schema.Extensions[kubernetesMergeKeyExtensionKey]
 	if !found {
-		// merge key and patch strategy must appear together
+		// no mergeKey -- may be a primitive associative list (e.g. finalizers)
-		return "", ""
+		mk = ""
 	}
 	return ps.(string), mk.(string)
 }

kyaml/setters2/delete.go

@@ -132,12 +132,12 @@ func (dd DeleterDefinition) Filter(object *yaml.RNode) (*yaml.RNode, error) {
 		return nil, errors.Errorf("setter is used in substitution %s, please delete the substitution first", subst)
 	}
 
-	_, err = definitions.Pipe(yaml.FieldClearer{Name:key})
+	_, err = definitions.Pipe(yaml.FieldClearer{Name: key})
 	if err != nil {
 		return nil, err
 	}
 	// remove definitions if it's empty
-	_, err = object.Pipe(yaml.Lookup(openapi.SupplementaryOpenAPIFieldName), yaml.FieldClearer{Name:"definitions", IfEmpty: true})
+	_, err = object.Pipe(yaml.Lookup(openapi.SupplementaryOpenAPIFieldName), yaml.FieldClearer{Name: "definitions", IfEmpty: true})
 	if err != nil {
 		return nil, err
 	}

kyaml/yaml/merge2/element_test.go

@@ -461,4 +461,57 @@ containers: # {"items":{"$ref": "#/definitions/io.k8s.api.core.v1.Container"},"t
 `,
 		infer: false,
 	},
+
+	{description: `merge_primitive_finalizers`,
+		source: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  finalizers:
+  - a
+  - b 
+`,
+		dest: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  finalizers:
+  - b
+  - c
+`,
+		expected: `
+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  finalizers:
+  - b
+  - c
+  - a
+`,
+	},
+
+	{description: `merge_primitive_items`,
+		source: `
+apiVersion: apps/v1
+kind: Deployment
+items: # {"type":"array", "x-kubernetes-patch-strategy": "merge"}
+- a
+- b
+`,
+		dest: `
+apiVersion: apps/v1
+kind: Deployment
+items:
+- b
+- c
+`,
+		expected: `
+apiVersion: apps/v1
+kind: Deployment
+items:
+- b
+- c
+- a
+`,
+	},
 }

kyaml/yaml/walk/associative_sequence.go

@@ -19,11 +19,11 @@ func (l *Walker) walkAssociativeSequence() (*yaml.RNode, error) {
 		return nil, err
 	}
 
-	var key string
+	var key, strategy string
 	if l.Schema != nil {
-		_, key = l.Schema.PatchStrategyAndKey()
+		strategy, key = l.Schema.PatchStrategyAndKey()
 	}
-	if key == "" { // no key from the schema, try to infer one
+	if strategy == "" && key == "" { // neither strategy nor not present in the schema -- infer the key
 		// find the list of elements we need to recursively walk
 		key, err = l.elementKey()
 		if err != nil {
@@ -31,9 +31,57 @@ func (l *Walker) walkAssociativeSequence() (*yaml.RNode, error) {
 		}
 	}
 
-	values := l.elementValues(key)
+	// non-primitive associative list -- merge the elements
+	if key != "" {
+		values := l.elementValues(key)
 
-	// recursively set the elements in the list
+		// recursively set the elements in the list
+		var s *openapi.ResourceSchema
+		if l.Schema != nil {
+			s = l.Schema.Elements()
+		}
+		for _, value := range values {
+			val, err := Walker{
+				VisitKeysAsScalars:    l.VisitKeysAsScalars,
+				InferAssociativeLists: l.InferAssociativeLists,
+				Visitor:               l,
+				Schema:                s,
+				Sources:               l.elementValue(key, value),
+			}.Walk()
+			if err != nil {
+				return nil, err
+			}
+			if yaml.IsEmpty(val) {
+				_, err = dest.Pipe(yaml.ElementSetter{Key: key, Value: value})
+				if err != nil {
+					return nil, err
+				}
+				continue
+			}
+
+			if val.Field(key) == nil {
+				// make sure the key is set on the field
+				_, err = val.Pipe(yaml.SetField(key, yaml.NewScalarRNode(value)))
+				if err != nil {
+					return nil, err
+				}
+			}
+
+			// this handles empty and non-empty values
+			_, err = dest.Pipe(yaml.ElementSetter{Element: val.YNode(), Key: key, Value: value})
+			if err != nil {
+				return nil, err
+			}
+		}
+		// field is empty
+		if yaml.IsEmpty(dest) {
+			return nil, nil
+		}
+		return dest, nil
+	}
+
+	// primitive associative list -- merge the values
+	values := l.elementPrimitiveValues()
 	var s *openapi.ResourceSchema
 	if l.Schema != nil {
 		s = l.Schema.Elements()
@@ -44,7 +92,7 @@ func (l *Walker) walkAssociativeSequence() (*yaml.RNode, error) {
 			InferAssociativeLists: l.InferAssociativeLists,
 			Visitor:               l,
 			Schema:                s,
-			Sources:               l.elementValue(key, value),
+			Sources:               l.elementValue(key /*empty key implies primitive*/, value),
 		}.Walk()
 		if err != nil {
 			return nil, err
@@ -71,6 +119,7 @@ func (l *Walker) walkAssociativeSequence() (*yaml.RNode, error) {
 			return nil, err
 		}
 	}
+
 	// field is empty
 	if yaml.IsEmpty(dest) {
 		return nil, nil
@@ -127,6 +176,32 @@ func (l Walker) elementValues(key string) []string {
 	return returnValues
 }
 
+// elementPrimitiveValues returns the primitive values in an associative list -- eg. finalizers
+// TODO: figure out the right order -- currently the order is deterministic but may be improved
+// upon.
+func (l Walker) elementPrimitiveValues() []string {
+	// use slice to to keep elements in the original order
+	// dest node must be first
+	var returnValues []string
+	seen := sets.String{}
+	for i := range l.Sources {
+		if l.Sources[i] == nil {
+			continue
+		}
+
+		// add the value of the field for each element
+		// don't check error, we know this is a list node
+		for _, item := range l.Sources[i].YNode().Content {
+			if seen.Has(item.Value) {
+				continue
+			}
+			returnValues = append(returnValues, item.Value)
+			seen.Insert(item.Value)
+		}
+	}
+	return returnValues
+}
+
 // fieldValue returns a slice containing each source's value for fieldName
 func (l Walker) elementValue(key, value string) []*yaml.RNode {
 	var fields []*yaml.RNode