diff --git a/kyaml/openapi/openapi.go b/kyaml/openapi/openapi.go index 0c80233af..08d161d04 100644 --- a/kyaml/openapi/openapi.go +++ b/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 - return "", "" + // no mergeKey -- may be a primitive associative list (e.g. finalizers) + mk = "" } return ps.(string), mk.(string) } diff --git a/kyaml/yaml/merge2/element_test.go b/kyaml/yaml/merge2/element_test.go index 6994b1b2f..6a5442b04 100644 --- a/kyaml/yaml/merge2/element_test.go +++ b/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 +`, + }, } diff --git a/kyaml/yaml/walk/associative_sequence.go b/kyaml/yaml/walk/associative_sequence.go index 72e8e02e7..8d64e7a49 100644 --- a/kyaml/yaml/walk/associative_sequence.go +++ b/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