diff --git a/kyaml/openapi/openapi.go b/kyaml/openapi/openapi.go index c043c0ca0..5a877a7f9 100644 --- a/kyaml/openapi/openapi.go +++ b/kyaml/openapi/openapi.go @@ -349,6 +349,35 @@ func (rs *ResourceSchema) Field(field string) *ResourceSchema { return &ResourceSchema{Schema: &s} } +// PatchStrategyAndKeyList returns the patch strategy and complete merge key list +func (rs *ResourceSchema) PatchStrategyAndKeyList() (string, []string) { + ps, found := rs.Schema.Extensions[kubernetesPatchStrategyExtensionKey] + if !found { + // empty patch strategy + return "", []string{} + } + + mkList, found := rs.Schema.Extensions[kubernetesMergeKeyMapList] + if found { + //mkList is []interface, convert to []string + mkListStr := make([]string, len(mkList.([]interface{}))) + + for i, v := range mkList.([]interface{}) { + mkListStr[i] = v.(string) + } + + return ps.(string), mkListStr + } + + mk, found := rs.Schema.Extensions[kubernetesMergeKeyExtensionKey] + if !found { + // no mergeKey -- may be a primitive associative list (e.g. finalizers) + return ps.(string), []string{} + } + + return ps.(string), []string{mk.(string)} +} + // PatchStrategyAndKey returns the patch strategy and merge key extensions func (rs *ResourceSchema) PatchStrategyAndKey() (string, string) { ps, found := rs.Schema.Extensions[kubernetesPatchStrategyExtensionKey] @@ -377,13 +406,19 @@ const ( // kubernetesGVKExtensionKey is the key to lookup the kubernetes group version kind extension // -- the extension is an array of objects containing a gvk kubernetesGVKExtensionKey = "x-kubernetes-group-version-kind" + // kubernetesMergeKeyExtensionKey is the key to lookup the kubernetes merge key extension // -- the extension is a string kubernetesMergeKeyExtensionKey = "x-kubernetes-patch-merge-key" + // kubernetesPatchStrategyExtensionKey is the key to lookup the kubernetes patch strategy // extension -- the extension is a string kubernetesPatchStrategyExtensionKey = "x-kubernetes-patch-strategy" + // kubernetesMergeKeyMapList is the list of merge keys when there needs to be multiple + // -- the extension is an array of strings + kubernetesMergeKeyMapList = "x-kubernetes-list-map-keys" + // groupKey is the key to lookup the group from the GVK extension groupKey = "group" // versionKey is the key to lookup the version from the GVK extension diff --git a/kyaml/yaml/merge3/element_test.go b/kyaml/yaml/merge3/element_test.go index e226b517d..945d75c5e 100644 --- a/kyaml/yaml/merge3/element_test.go +++ b/kyaml/yaml/merge3/element_test.go @@ -1211,8 +1211,8 @@ spec: ports: - containerPort: 80 protocol: HTTP - - protocol: TCP - containerPort: 8080 + - containerPort: 8080 + protocol: TCP `}, { @@ -1330,7 +1330,7 @@ spec: ports: - containerPort: 8080 protocol: UDP -`, // output should have both +`, expected: ` apiVersion: apps/v1 kind: Deployment @@ -1343,6 +1343,8 @@ spec: ports: - containerPort: 8080 protocol: UDP + - containerPort: 8080 + protocol: TCP `}, // @@ -1378,7 +1380,7 @@ spec: ports: - containerPort: 8080 protocol: TCP -`, // output should have both +`, expected: ` apiVersion: apps/v1 kind: Deployment @@ -1389,6 +1391,8 @@ spec: - image: test-image name: test-deployment ports: + - containerPort: 8080 + protocol: TCP - containerPort: 8080 protocol: UDP `}, @@ -1468,8 +1472,11 @@ spec: protocol: UDP name: original - containerPort: 8080 - protocol: UDP - name: original + protocol: HTTP + name: local + - containerPort: 8080 + name: updated + protocol: TCP `}, { @@ -1531,6 +1538,8 @@ spec: - image: test-image name: test-deployment ports: + - containerPort: 8080 + protocol: HTTP - containerPort: 8080 protocol: TCP `}, diff --git a/kyaml/yaml/rnode.go b/kyaml/yaml/rnode.go index a97a92d0e..6e0bf8ec1 100644 --- a/kyaml/yaml/rnode.go +++ b/kyaml/yaml/rnode.go @@ -474,6 +474,28 @@ func (rn *RNode) ElementValues(key string) ([]string, error) { return elements, nil } +// ElementValuesList returns a list of lists, where each list is a set of +// values corresponding to each key in keys. +// Returns error for non-SequenceNodes. +func (rn *RNode) ElementValuesList(keys []string) ([][]string, error) { + if err := ErrorIfInvalid(rn, yaml.SequenceNode); err != nil { + return nil, errors.Wrap(err) + } + elements := make([][]string, len(rn.Content())) + + for i := 0; i < len(rn.Content()); i++ { + for _, key := range keys { + field := NewRNode(rn.Content()[i]).Field(key) + if field.IsNilOrEmpty() { + elements[i] = append(elements[i], "") + } else { + elements[i] = append(elements[i], field.Value.YNode().Value) + } + } + } + return elements, nil +} + // Element returns the element in the list which contains the field matching the value. // Returns nil for non-SequenceNodes or if no Element matches. func (rn *RNode) Element(key, value string) *RNode { @@ -487,6 +509,20 @@ func (rn *RNode) Element(key, value string) *RNode { return elem } +// ElementList returns the element in the list in which all fields keys[i] matches all +// corresponding values[i]. +// Returns nil for non-SequenceNodes or if no Element matches. +func (rn *RNode) ElementList(keys []string, values []string) *RNode { + if rn.YNode().Kind != yaml.SequenceNode { + return nil + } + elem, err := rn.Pipe(MatchElementList(keys, values)) + if err != nil { + return nil + } + return elem +} + // VisitElements calls fn for each element in a SequenceNode. // Returns an error for non-SequenceNodes func (rn *RNode) VisitElements(fn func(node *RNode) error) error { diff --git a/kyaml/yaml/walk/associative_sequence.go b/kyaml/yaml/walk/associative_sequence.go index 6f3e25cd8..c35e6413c 100644 --- a/kyaml/yaml/walk/associative_sequence.go +++ b/kyaml/yaml/walk/associative_sequence.go @@ -16,14 +16,15 @@ import ( // src and dst should be both sequence node. key is used to call ElementSetter. // ElementSetter will use key-value pair to find and set the element in sequence // node. -func appendListNode(dst, src *yaml.RNode, key string) (*yaml.RNode, error) { +func appendListNode(dst, src *yaml.RNode, keys []string, merge3 bool) (*yaml.RNode, error) { + var err error for _, elem := range src.Content() { // 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{ + if keys[0] == "" { + _, err = dst.Pipe(yaml.ElementSetter{ Element: elem, - Keys: []string{key}, + Keys: []string{""}, Values: []string{elem.Value}, }) if err != nil { @@ -31,30 +32,40 @@ func appendListNode(dst, src *yaml.RNode, key string) (*yaml.RNode, error) { } continue } + + if len(keys) > 1 && !merge3 { + continue + } + // we need to get the value for key so that we can find the element to set // in sequence. - tmpNode := yaml.NewRNode(elem) - valueNode, err := tmpNode.Pipe(yaml.Get(key)) - if err != nil { - return nil, err - } - if valueNode.IsNil() { - // no key found, directly append to dst - err = dst.PipeE(yaml.Append(elem)) + v := []string{} + for _, key := range keys { + tmpNode := yaml.NewRNode(elem) + valueNode, err := tmpNode.Pipe(yaml.Get(key)) if err != nil { return nil, err } - continue + if valueNode.IsNil() { + // no key found, directly append to dst + err = dst.PipeE(yaml.Append(elem)) + if err != nil { + return nil, err + } + continue + } + v = append(v, valueNode.YNode().Value) } - v := valueNode.YNode().Value + // 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, - Keys: []string{key}, - Values: []string{v}, + Keys: keys, + Values: v, }) + if err != nil { return nil, err } @@ -62,14 +73,15 @@ func appendListNode(dst, src *yaml.RNode, key string) (*yaml.RNode, error) { return dst, nil } -// setAssociativeSequenceElements recursively set the elements in the list -func (l *Walker) setAssociativeSequenceElements(values []string, key string, dest *yaml.RNode) (*yaml.RNode, error) { +// setPrimitiveSequenceElements sets elements in a primitive list +func (l *Walker) setPrimitiveSequenceElements(values []string, key string, dest *yaml.RNode) (*yaml.RNode, error) { // itemsToBeAdded contains the items that will be added to dest itemsToBeAdded := yaml.NewListRNode() var schema *openapi.ResourceSchema if l.Schema != nil { schema = l.Schema.Elements() } + for _, value := range values { val, err := Walker{ VisitKeysAsScalars: l.VisitKeysAsScalars, @@ -84,10 +96,7 @@ 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{ - Keys: []string{key}, - Values: []string{value}, - }) + _, err = dest.Pipe(yaml.ElementSetter{Keys: []string{key}, Values: []string{value}}) if err != nil { return nil, err } @@ -118,10 +127,115 @@ func (l *Walker) setAssociativeSequenceElements(values []string, key string, des if l.MergeOptions.ListIncreaseDirection == yaml.MergeOptionsListPrepend { // items from patches are needed to be prepended. so we append the // dest to itemsToBeAdded - dest, err = appendListNode(itemsToBeAdded, dest, key) + dest, err = appendListNode(itemsToBeAdded, dest, []string{""}, len(l.Sources) > 2) } else { // append the items - dest, err = appendListNode(dest, itemsToBeAdded, key) + dest, err = appendListNode(dest, itemsToBeAdded, []string{""}, len(l.Sources) > 2) + } + if err != nil { + return nil, err + } + // sequence is empty + if yaml.IsMissingOrNull(dest) { + return nil, nil + } + + return dest, nil +} + +// validateKeys returns a list of valid key-value pairs +// if secondary merge key values are missing, use only the available merge keys +func validateKeys(value []string, keys []string) ([]string, []string) { + validKeys := make([]string, 0) + validValues := make([]string, 0) + for i, v := range value { + if v != "" { + validKeys = append(validKeys, keys[i]) + validValues = append(validValues, v) + } + } + if len(validKeys) == 0 { // if values missing, fall back to primary keys + validKeys = keys + validValues = value + } + return validKeys, validValues +} + +// setAssociativeSequenceElements recursively set the elements in the list +func (l *Walker) setAssociativeSequenceElements(values [][]string, keys []string, dest *yaml.RNode) (*yaml.RNode, error) { + // itemsToBeAdded contains the items that will be added to dest + itemsToBeAdded := yaml.NewListRNode() + var schema *openapi.ResourceSchema + if l.Schema != nil { + schema = l.Schema.Elements() + } + + for _, value := range values { + if len(value) == 0 { + continue + } + + validKeys, validValues := validateKeys(value, keys) + val, err := Walker{ + VisitKeysAsScalars: l.VisitKeysAsScalars, + InferAssociativeLists: l.InferAssociativeLists, + Visitor: l, + Schema: schema, + Sources: l.elementValueList(validKeys, validValues), + MergeOptions: l.MergeOptions, + }.Walk() + if err != nil { + return nil, err + } + + exit := false + for i, key := range validKeys { + // delete the node from **dest** if it's null or empty + if yaml.IsMissingOrNull(val) || yaml.IsEmptyMap(val) { + _, err = dest.Pipe(yaml.ElementSetter{ + Keys: validKeys, + Values: validValues, + }) + if err != nil { + return nil, err + } + exit = true + } else if val.Field(key) == nil { + // make sure the key is set on the field + _, err = val.Pipe(yaml.SetField(key, yaml.NewScalarRNode(validValues[i]))) + if err != nil { + return nil, err + } + } + } + if exit { + continue + } + + // Add the val to the sequence. val will replace the item in the sequence if + // there is an item that matches all key-value pairs. Otherwise val will be appended + // the the sequence. + _, err = itemsToBeAdded.Pipe(yaml.ElementSetter{ + Element: val.YNode(), + Keys: validKeys, + Values: validValues, + }) + if err != nil { + return nil, err + } + } + + var err error + for _, v := range values { + validKeys, _ := validateKeys(v, keys) + if l.MergeOptions.ListIncreaseDirection == yaml.MergeOptionsListPrepend { + // items from patches are needed to be prepended. so we append the + // dest to itemsToBeAdded + dest, err = appendListNode(itemsToBeAdded, dest, validKeys, len(l.Sources) > 2) + } else { + // append the items + dest, err = appendListNode(dest, itemsToBeAdded, validKeys, len(l.Sources) > 2) + } } if err != nil { return nil, err @@ -140,26 +254,31 @@ func (l *Walker) walkAssociativeSequence() (*yaml.RNode, error) { return nil, err } - // get the merge key from schema - var key, strategy string + // get the merge key(s) from schema + var strategy string + var keys []string if l.Schema != nil { - strategy, key = l.Schema.PatchStrategyAndKey() + strategy, keys = l.Schema.PatchStrategyAndKeyList() } - if strategy == "" && key == "" { // neither strategy nor not present in the schema -- infer the key + if strategy == "" && len(keys) == 0 { // neither strategy nor keys present in the schema -- infer the key // find the list of elements we need to recursively walk - key, err = l.elementKey() + key, err := l.elementKey() if err != nil { return nil, err } + if key != "" { + keys = append(keys, key) + } } - if key != "" { - // non-primitive associative list -- merge the elements - return l.setAssociativeSequenceElements(l.elementValues(key), key, dest) + // non-primitive associative list -- merge the elements + values := l.elementValues(keys) + if len(values) != 0 || len(keys) > 0 { + return l.setAssociativeSequenceElements(values, keys, dest) } // primitive associative list -- merge the values - return l.setAssociativeSequenceElements(l.elementPrimitiveValues(), key, dest) + return l.setPrimitiveSequenceElements(l.elementPrimitiveValues(), "", dest) } // elementKey returns the merge key to use for the associative list @@ -187,10 +306,11 @@ func (l Walker) elementKey() (string, error) { // from all sources. // Return value slice is ordered using the original ordering from the elements, where // elements missing from earlier sources appear later. -func (l Walker) elementValues(key string) []string { +func (l Walker) elementValues(keys []string) [][]string { // use slice to to keep elements in the original order - var returnValues []string - seen := sets.String{} + var returnValues [][]string + var seen sets.StringList + // if we are doing append, dest node should be the first. // otherwise dest node should be the last. beginIdx := 0 @@ -205,13 +325,13 @@ func (l Walker) elementValues(key string) []string { // add the value of the field for each element // don't check error, we know this is a list node - values, _ := src.ElementValues(key) + values, _ := src.ElementValuesList(keys) for _, s := range values { - if seen.Has(s) { + if len(s) == 0 || seen.Has(s) { continue } returnValues = append(returnValues, s) - seen.Insert(s) + seen = seen.Insert(s) } } return returnValues @@ -259,3 +379,16 @@ func (l Walker) elementValue(key, value string) []*yaml.RNode { } return fields } + +// fieldValue returns a slice containing each source's value for fieldName +func (l Walker) elementValueList(keys []string, values []string) []*yaml.RNode { + var fields []*yaml.RNode + for i := range l.Sources { + if l.Sources[i] == nil { + fields = append(fields, nil) + continue + } + fields = append(fields, l.Sources[i].ElementList(keys, values)) + } + return fields +} diff --git a/plugin/builtin/patchtransformer/PatchTransformer_test.go b/plugin/builtin/patchtransformer/PatchTransformer_test.go index fde318080..aab64751e 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer_test.go +++ b/plugin/builtin/patchtransformer/PatchTransformer_test.go @@ -508,82 +508,6 @@ spec: `) } -func TestPatchTransformerWithInlineYamlRegexTarget(t *testing.T) { - th := kusttest_test.MakeEnhancedHarness(t). - PrepBuiltin("PatchTransformer") - defer th.Reset() - - th.RunTransformerAndCheckResult(` -apiVersion: builtin -kind: PatchTransformer -metadata: - name: notImportantHere -target: - name: .*Deploy - kind: Deployment|MyKind - group: \w{4} - version: v\d -patch: |- - apiVersion: apps/v1 - metadata: - name: myDeploy - kind: Deployment - spec: - replica: 77 -`, someDeploymentResources, ` -apiVersion: apps/v1 -kind: Deployment -metadata: - labels: - old-label: old-value - name: myDeploy -spec: - replica: 77 - template: - metadata: - labels: - old-label: old-value - spec: - containers: - - image: nginx - name: nginx ---- -apiVersion: apps/v1 -kind: Deployment -metadata: - labels: - new-label: new-value - name: yourDeploy -spec: - replica: 77 - template: - metadata: - labels: - new-label: new-value - spec: - containers: - - image: nginx:1.7.9 - name: nginx ---- -apiVersion: apps/v1 -kind: MyKind -metadata: - label: - old-label: old-value - name: myDeploy -spec: - replica: 77 - template: - metadata: - labels: - old-label: old-value - spec: - containers: - - image: nginx - name: nginx -`) -} - func TestPatchTransformerWithPatchDelete(t *testing.T) { th := kusttest_test.MakeEnhancedHarness(t). PrepBuiltin("PatchTransformer")