From 1fef312102fd320e5aa818147c43f8c067a1f523 Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Tue, 31 Oct 2017 14:31:38 -0700 Subject: [PATCH 1/3] Small refactorings for kubectl/apply merge packages - move strings into constants - remove unnecessary interface - fix documentation - improve error messaging --- pkg/kubectl/apply/element.go | 8 ++--- pkg/kubectl/apply/parse/factory.go | 6 ++-- pkg/kubectl/apply/parse/list_element.go | 4 +-- pkg/kubectl/apply/parse/visitor.go | 2 +- pkg/kubectl/apply/strategy/merge_visitor.go | 36 +++++++++++-------- .../apply/strategy/strategic_visitor.go | 16 +++++---- pkg/kubectl/apply/visitor.go | 15 +++++--- 7 files changed, 49 insertions(+), 38 deletions(-) diff --git a/pkg/kubectl/apply/element.go b/pkg/kubectl/apply/element.go index 44b90d85aa..0b55b109b5 100644 --- a/pkg/kubectl/apply/element.go +++ b/pkg/kubectl/apply/element.go @@ -78,16 +78,14 @@ type FieldMeta interface { // FieldMetaImpl implements FieldMeta type FieldMetaImpl struct { - // The type of merge strategy to use for this field + // MergeType is the type of merge strategy to use for this field // maybe "merge", "replace" or "retainkeys" - // TODO: There maybe multiple strategies, so this may need to be a slice, map, or struct - // Address this in a follow up in the PR to introduce retainkeys strategy MergeType string - // The merge key to use when the MergeType is "merge" and underlying type is a list + // MergeKeys are the merge keys to use when the MergeType is "merge" and underlying type is a list MergeKeys MergeKeys - // The openapi type of the field - "list", "primitive", "map" + // Type is the openapi type of the field - "list", "primitive", "map" Type string // Name contains of the field diff --git a/pkg/kubectl/apply/parse/factory.go b/pkg/kubectl/apply/parse/factory.go index c8907da4d8..c9cc8fb2e2 100644 --- a/pkg/kubectl/apply/parse/factory.go +++ b/pkg/kubectl/apply/parse/factory.go @@ -78,13 +78,13 @@ func (v *ElementBuildingVisitor) getItem(s proto.Schema, name string, data apply reflect.String: p, err := getPrimitive(s) if err != nil { - return nil, fmt.Errorf("expected openapi Primitive, was %T for %v", s, kind) + return nil, fmt.Errorf("expected openapi Primitive, was %T for %v (%v)", s, kind, err) } return &primitiveItem{name, p, data}, nil case reflect.Array, reflect.Slice: a, err := getArray(s) if err != nil { - return nil, fmt.Errorf("expected openapi Array, was %T for %v", s, kind) + return nil, fmt.Errorf("expected openapi Array, was %T for %v (%v)", s, kind, err) } return &listItem{ Name: name, @@ -106,7 +106,7 @@ func (v *ElementBuildingVisitor) getItem(s proto.Schema, name string, data apply // If it looks like a map, and no openapi type is found, default to mapItem m, err := getMap(s) if err != nil { - return nil, fmt.Errorf("expected openapi Kind or Map, was %T for %v", s, kind) + return nil, fmt.Errorf("expected openapi Kind or Map, was %T for %v (%v)", s, kind, err) } return &mapItem{ Name: name, diff --git a/pkg/kubectl/apply/parse/list_element.go b/pkg/kubectl/apply/parse/list_element.go index 3a16318fb0..487a5f19ff 100644 --- a/pkg/kubectl/apply/parse/list_element.go +++ b/pkg/kubectl/apply/parse/list_element.go @@ -45,7 +45,7 @@ func (v ElementBuildingVisitor) mergeListElement(meta apply.FieldMetaImpl, item func (v ElementBuildingVisitor) doPrimitiveList(meta apply.FieldMetaImpl, item *listItem) (*apply.ListElement, error) { result := &apply.ListElement{ FieldMetaImpl: apply.FieldMetaImpl{ - MergeType: "merge", + MergeType: apply.MergeStrategy, Name: item.Name, }, ListElementData: item.ListElementData, @@ -101,7 +101,7 @@ func (v ElementBuildingVisitor) doMapList(meta apply.FieldMetaImpl, item *listIt key := meta.GetFieldMergeKeys() result := &apply.ListElement{ FieldMetaImpl: apply.FieldMetaImpl{ - MergeType: "merge", + MergeType: apply.MergeStrategy, MergeKeys: key, Name: item.Name, }, diff --git a/pkg/kubectl/apply/parse/visitor.go b/pkg/kubectl/apply/parse/visitor.go index 5c01d26c16..d67ba6a79b 100644 --- a/pkg/kubectl/apply/parse/visitor.go +++ b/pkg/kubectl/apply/parse/visitor.go @@ -54,7 +54,7 @@ func (v ElementBuildingVisitor) CreateListElement(item *listItem) (apply.Element if err != nil { return nil, err } - if meta.GetFieldMergeType() == "merge" { + if meta.GetFieldMergeType() == apply.MergeStrategy { return v.mergeListElement(meta, item) } return v.replaceListElement(meta, item) diff --git a/pkg/kubectl/apply/strategy/merge_visitor.go b/pkg/kubectl/apply/strategy/merge_visitor.go index 3669cf0b55..94a9d701f1 100644 --- a/pkg/kubectl/apply/strategy/merge_visitor.go +++ b/pkg/kubectl/apply/strategy/merge_visitor.go @@ -29,13 +29,13 @@ func createMergeStrategy(options Options, strategic *delegatingStrategy) mergeSt } } -// mergeStrategy creates a patch to merge a local file value into a remote field value +// mergeStrategy merges the values in an Element into a single Result type mergeStrategy struct { strategic *delegatingStrategy options Options } -// MergeList creates a patch to merge a local list field value into a remote list field value +// MergeList merges the lists in a ListElement into a single Result func (v mergeStrategy) MergeList(e apply.ListElement) (apply.Result, error) { // No merge logic if adding or deleting a field if result, done := v.doAddOrDelete(e); done { @@ -70,26 +70,32 @@ func (v mergeStrategy) MergeList(e apply.ListElement) (apply.Result, error) { return apply.Result{Operation: apply.SET, MergedResult: merged}, nil } -// MergeMap creates a patch to merge a local map field into a remote map field +// MergeMap merges the maps in a MapElement into a single Result func (v mergeStrategy) MergeMap(e apply.MapElement) (apply.Result, error) { - return v.doMergeMap(e) -} - -// MergeType creates a patch to merge a local map field into a remote map field -func (v mergeStrategy) MergeType(e apply.TypeElement) (apply.Result, error) { - return v.doMergeMap(e) -} - -// do merges a recorded, local and remote map into a new object -func (v mergeStrategy) doMergeMap(e apply.MapValuesElement) (apply.Result, error) { // No merge logic if adding or deleting a field if result, done := v.doAddOrDelete(e); done { return result, nil } + return v.doMergeMap(e.GetValues()) +} + +// MergeMap merges the type instances in a TypeElement into a single Result +func (v mergeStrategy) MergeType(e apply.TypeElement) (apply.Result, error) { + // No merge logic if adding or deleting a field + if result, done := v.doAddOrDelete(e); done { + return result, nil + } + + return v.doMergeMap(e.GetValues()) +} + +// do merges a recorded, local and remote map into a new object +func (v mergeStrategy) doMergeMap(e map[string]apply.Element) (apply.Result, error) { + // Merge each item in the list merged := map[string]interface{}{} - for key, value := range e.GetValues() { + for key, value := range e { // Recursively merge the map element before adding the value to the map result, err := value.Merge(v.strategic) if err != nil { @@ -134,7 +140,7 @@ func (v mergeStrategy) MergePrimitive(diff apply.PrimitiveElement) (apply.Result return apply.Result{}, fmt.Errorf("Cannot merge primitive element %v", diff.Name) } -// MergeEmpty +// MergeEmpty returns an empty result func (v mergeStrategy) MergeEmpty(diff apply.EmptyElement) (apply.Result, error) { return apply.Result{Operation: apply.SET}, nil } diff --git a/pkg/kubectl/apply/strategy/strategic_visitor.go b/pkg/kubectl/apply/strategy/strategic_visitor.go index 3992271729..bd198e7e9f 100644 --- a/pkg/kubectl/apply/strategy/strategic_visitor.go +++ b/pkg/kubectl/apply/strategy/strategic_visitor.go @@ -16,7 +16,9 @@ limitations under the License. package strategy -import "k8s.io/kubernetes/pkg/kubectl/apply" +import ( + "k8s.io/kubernetes/pkg/kubectl/apply" +) // delegatingStrategy delegates merging fields to other visitor implementations // based on the merge strategy preferred by the field. @@ -41,9 +43,9 @@ func createDelegatingStrategy(options Options) *delegatingStrategy { func (v delegatingStrategy) MergeList(diff apply.ListElement) (apply.Result, error) { // TODO: Support retainkeys switch diff.GetFieldMergeType() { - case "merge": + case apply.MergeStrategy: return v.merge.MergeList(diff) - case "replace": + case apply.ReplaceStrategy: return v.replace.MergeList(diff) default: return v.replace.MergeList(diff) @@ -55,9 +57,9 @@ func (v delegatingStrategy) MergeList(diff apply.ListElement) (apply.Result, err func (v delegatingStrategy) MergeMap(diff apply.MapElement) (apply.Result, error) { // TODO: Support retainkeys switch diff.GetFieldMergeType() { - case "merge": + case apply.MergeStrategy: return v.merge.MergeMap(diff) - case "replace": + case apply.ReplaceStrategy: return v.replace.MergeMap(diff) default: return v.merge.MergeMap(diff) @@ -69,9 +71,9 @@ func (v delegatingStrategy) MergeMap(diff apply.MapElement) (apply.Result, error func (v delegatingStrategy) MergeType(diff apply.TypeElement) (apply.Result, error) { // TODO: Support retainkeys switch diff.GetFieldMergeType() { - case "merge": + case apply.MergeStrategy: return v.merge.MergeType(diff) - case "replace": + case apply.ReplaceStrategy: return v.replace.MergeType(diff) default: return v.merge.MergeType(diff) diff --git a/pkg/kubectl/apply/visitor.go b/pkg/kubectl/apply/visitor.go index 0cf895718b..d63d973cd7 100644 --- a/pkg/kubectl/apply/visitor.go +++ b/pkg/kubectl/apply/visitor.go @@ -56,8 +56,13 @@ type Result struct { MergedResult interface{} } -// MapValuesElement exposes how to get the field / key - value pairs out of a Map or Type Element -type MapValuesElement interface { - Element - GetValues() map[string]Element -} +const ( + // MergeStrategy is the strategy to merge the local and remote values + MergeStrategy = "merge" + + // RetainKeysStrategy is the strategy to merge the local and remote values, but drop any fields not defined locally + RetainKeysStrategy = "retainKeys" + + // ReplaceStrategy is the strategy to replace the remote value with the local value + ReplaceStrategy = "replace" +) From 1b7118d9653bccb2f13637e73129050f4d99b47b Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Tue, 31 Oct 2017 14:34:50 -0700 Subject: [PATCH 2/3] kubectl apply parse libraries copy extensions to references and list elements when getting field / type metadata from openapi, copy the openapi extensions from - references to the underlying type - lists to the subtype --- pkg/kubectl/apply/parse/openapi.go | 53 ++++++++++++++++++++++++++++++ pkg/kubectl/apply/parse/util.go | 15 ++++++--- 2 files changed, 64 insertions(+), 4 deletions(-) diff --git a/pkg/kubectl/apply/parse/openapi.go b/pkg/kubectl/apply/parse/openapi.go index 73a7e7289f..a55594de4b 100644 --- a/pkg/kubectl/apply/parse/openapi.go +++ b/pkg/kubectl/apply/parse/openapi.go @@ -18,6 +18,7 @@ package parse import ( "fmt" + "strings" "k8s.io/kube-openapi/pkg/util/proto" ) @@ -123,6 +124,22 @@ func (v *kindSchemaVisitor) VisitKind(result *proto.Kind) { // VisitReference implements openapi func (v *kindSchemaVisitor) VisitReference(reference proto.Reference) { reference.SubSchema().Accept(v) + if v.Err == nil { + v.Err = copyExtensions(reference.GetPath().String(), reference.GetExtensions(), v.Result.Extensions) + } +} + +func copyExtensions(field string, from, to map[string]interface{}) error { + // Copy extensions from field to type for references + for key, val := range from { + if curr, found := to[key]; found { + // Don't allow the same extension to be defined both on the field and on the type + return fmt.Errorf("Cannot override value for extension %s on field %s from %v to %v", + key, field, curr, val) + } + to[key] = val + } + return nil } type mapSchemaVisitor struct { @@ -139,6 +156,9 @@ func (v *mapSchemaVisitor) VisitMap(result *proto.Map) { // VisitReference implements openapi func (v *mapSchemaVisitor) VisitReference(reference proto.Reference) { reference.SubSchema().Accept(v) + if v.Err == nil { + v.Err = copyExtensions(reference.GetPath().String(), reference.GetExtensions(), v.Result.Extensions) + } } type arraySchemaVisitor struct { @@ -150,11 +170,41 @@ type arraySchemaVisitor struct { func (v *arraySchemaVisitor) VisitArray(result *proto.Array) { v.Result = result v.Kind = "array" + v.Err = copySubElementPatchStrategy(result.Path.String(), result.GetExtensions(), result.SubType.GetExtensions()) +} + +// copyPatchStrategy copies the strategies to subelements to the subtype +// e.g. PodTemplate.Volumes is a []Volume with "x-kubernetes-patch-strategy": "merge,retainKeys" +// the "retainKeys" strategy applies to merging Volumes, and must be copied to the sub element +func copySubElementPatchStrategy(field string, from, to map[string]interface{}) error { + // Check if the parent has a patch strategy extension + if ext, found := from["x-kubernetes-patch-strategy"]; found { + strategy, ok := ext.(string) + if !ok { + return fmt.Errorf("Expected string value for x-kubernetes-patch-strategy on %s, was %T", + field, ext) + } + // Check of the parent patch strategy has a sub patch strategy, and if so copy to the sub type + if strings.Contains(strategy, ",") { + strategies := strings.Split(strategy, ",") + if len(strategies) != 2 { + // Only 1 sub strategy is supported + return fmt.Errorf( + "Expected between 0 and 2 elements for x-kubernetes-patch-merge-strategy by got %v", + strategies) + } + to["x-kubernetes-patch-strategy"] = strategies[1] + } + } + return nil } // MergePrimitive implements openapi func (v *arraySchemaVisitor) VisitReference(reference proto.Reference) { reference.SubSchema().Accept(v) + if v.Err == nil { + v.Err = copyExtensions(reference.GetPath().String(), reference.GetExtensions(), v.Result.Extensions) + } } type primitiveSchemaVisitor struct { @@ -171,4 +221,7 @@ func (v *primitiveSchemaVisitor) VisitPrimitive(result *proto.Primitive) { // VisitReference implements openapi func (v *primitiveSchemaVisitor) VisitReference(reference proto.Reference) { reference.SubSchema().Accept(v) + if v.Err == nil { + v.Err = copyExtensions(reference.GetPath().String(), reference.GetExtensions(), v.Result.Extensions) + } } diff --git a/pkg/kubectl/apply/parse/util.go b/pkg/kubectl/apply/parse/util.go index 1743b7db62..ce7adddab8 100644 --- a/pkg/kubectl/apply/parse/util.go +++ b/pkg/kubectl/apply/parse/util.go @@ -101,13 +101,20 @@ func getFieldMeta(s proto.Schema, name string) (apply.FieldMetaImpl, error) { m := apply.FieldMetaImpl{} if s != nil { ext := s.GetExtensions() - if s, found := ext["x-kubernetes-patch-strategy"]; found { - strategy, ok := s.(string) + if e, found := ext["x-kubernetes-patch-strategy"]; found { + strategy, ok := e.(string) if !ok { return apply.FieldMetaImpl{}, fmt.Errorf("Expected string for x-kubernetes-patch-strategy by got %T", s) } - // TODO: Support multi-strategy - m.MergeType = strategy + + // Take the first strategy if there are substrategies. + // Sub strategies are copied to sub types in openapi.go + strategies := strings.Split(strategy, ",") + if len(strategies) > 2 { + return apply.FieldMetaImpl{}, fmt.Errorf("Expected between 0 and 2 elements for x-kubernetes-patch-merge-strategy by got %v", strategies) + } + // For lists, choose the strategy for this type, not the subtype + m.MergeType = strategies[0] } if k, found := ext["x-kubernetes-patch-merge-key"]; found { key, ok := k.(string) From 51d1da1e945c691eb8b41d274e145b356f29f7ac Mon Sep 17 00:00:00 2001 From: Phillip Wittrock Date: Tue, 31 Oct 2017 14:37:37 -0700 Subject: [PATCH 3/3] Support retainkeys strategy in new apply merge code --- pkg/kubectl/apply/strategy/BUILD | 1 + .../apply/strategy/retain_keys_test.go | 195 ++++++++++++++++++ .../apply/strategy/retain_keys_visitor.go | 60 +++++- .../apply/strategy/strategic_visitor.go | 17 +- 4 files changed, 266 insertions(+), 7 deletions(-) create mode 100644 pkg/kubectl/apply/strategy/retain_keys_test.go diff --git a/pkg/kubectl/apply/strategy/BUILD b/pkg/kubectl/apply/strategy/BUILD index b489a95451..e1810a7862 100644 --- a/pkg/kubectl/apply/strategy/BUILD +++ b/pkg/kubectl/apply/strategy/BUILD @@ -25,6 +25,7 @@ go_test( "replace_map_list_test.go", "replace_map_test.go", "replace_primitive_list_test.go", + "retain_keys_test.go", "suite_test.go", "utils_test.go", ], diff --git a/pkg/kubectl/apply/strategy/retain_keys_test.go b/pkg/kubectl/apply/strategy/retain_keys_test.go new file mode 100644 index 0000000000..481da91a09 --- /dev/null +++ b/pkg/kubectl/apply/strategy/retain_keys_test.go @@ -0,0 +1,195 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package strategy_test + +import ( + . "github.com/onsi/ginkgo" + + "k8s.io/kubernetes/pkg/kubectl/apply/strategy" +) + +var _ = Describe("Merging fields with the retainkeys strategy", func() { + Context("where some fields are only defined remotely", func() { + It("should drop those fields ", func() { + recorded := create(` +apiVersion: extensions/v1beta1 +kind: Deployment +spec: + strategy: +`) + local := create(` +apiVersion: extensions/v1beta1 +kind: Deployment +spec: + strategy: + type: Recreate +`) + remote := create(` +apiVersion: extensions/v1beta1 +kind: Deployment +spec: + strategy: + type: RollingUpdate + rollingUpdate: + maxUnavailable: 1 + maxSurge: 1 +`) + expected := create(` +apiVersion: extensions/v1beta1 +kind: Deployment +spec: + strategy: + type: Recreate +`) + run(strategy.Create(strategy.Options{}), recorded, local, remote, expected) + }) + }) + + Context("where some fields are defined both locally and remotely", func() { + It("should merge those fields", func() { + recorded := create(` +apiVersion: extensions/v1beta1 +kind: Deployment +spec: + strategy: +`) + local := create(` +apiVersion: extensions/v1beta1 +kind: Deployment +spec: + strategy: + type: RollingUpdate + rollingUpdate: + maxUnavailable: 2 +`) + remote := create(` +apiVersion: extensions/v1beta1 +kind: Deployment +spec: + strategy: + type: RollingUpdate + rollingUpdate: + maxSurge: 1 +`) + expected := create(` +apiVersion: extensions/v1beta1 +kind: Deployment +spec: + strategy: + type: RollingUpdate + rollingUpdate: + maxUnavailable: 2 + maxSurge: 1 +`) + run(strategy.Create(strategy.Options{}), recorded, local, remote, expected) + }) + }) + + Context("where the elements are in a list and some fields are only defined remotely", func() { + It("should drop those fields ", func() { + recorded := create(` +apiVersion: apps/v1beta1 +kind: Deployment +spec: + template: + spec: +`) + local := create(` +apiVersion: apps/v1beta1 +kind: Deployment +spec: + template: + spec: + volumes: + - name: cache-volume + emptyDir: +`) + remote := create(` +apiVersion: apps/v1beta1 +kind: Deployment +spec: + template: + spec: + volumes: + - name: cache-volume + hostPath: + path: /tmp/cache-volume +`) + expected := create(` +apiVersion: apps/v1beta1 +kind: Deployment +spec: + template: + spec: + volumes: + - name: cache-volume + emptyDir: +`) + run(strategy.Create(strategy.Options{}), recorded, local, remote, expected) + }) + }) + + Context("where the elements are in a list", func() { + It("the fields defined both locally and remotely should be merged", func() { + recorded := create(` +apiVersion: apps/v1beta1 +kind: Deployment +spec: + template: + spec: +`) + local := create(` +apiVersion: apps/v1beta1 +kind: Deployment +spec: + template: + spec: + volumes: + - name: cache-volume + hostPath: + path: /tmp/cache-volume + emptyDir: +`) + remote := create(` +apiVersion: apps/v1beta1 +kind: Deployment +spec: + template: + spec: + volumes: + - name: cache-volume + hostPath: + path: /tmp/cache-volume + type: Directory +`) + expected := create(` +apiVersion: apps/v1beta1 +kind: Deployment +spec: + template: + spec: + volumes: + - name: cache-volume + hostPath: + path: /tmp/cache-volume + type: Directory + emptyDir: +`) + run(strategy.Create(strategy.Options{}), recorded, local, remote, expected) + }) + }) +}) diff --git a/pkg/kubectl/apply/strategy/retain_keys_visitor.go b/pkg/kubectl/apply/strategy/retain_keys_visitor.go index 2483af0fab..b901285be1 100644 --- a/pkg/kubectl/apply/strategy/retain_keys_visitor.go +++ b/pkg/kubectl/apply/strategy/retain_keys_visitor.go @@ -16,4 +16,62 @@ limitations under the License. package strategy -// TODO: Write this +import ( + "fmt" + "k8s.io/kubernetes/pkg/kubectl/apply" +) + +func createRetainKeysStrategy(options Options, strategic *delegatingStrategy) retainKeysStrategy { + return retainKeysStrategy{ + &mergeStrategy{strategic, options}, + strategic, + options, + } +} + +// retainKeysStrategy merges the values in an Element into a single Result, +// dropping any fields omitted from the local copy. (but merging values when +// defined locally and remotely) +type retainKeysStrategy struct { + merge *mergeStrategy + strategic *delegatingStrategy + options Options +} + +// MergeMap merges the type instances in a TypeElement into a single Result +// keeping only the fields defined locally, but merging their values with +// the remote values. +func (v retainKeysStrategy) MergeType(e apply.TypeElement) (apply.Result, error) { + // No merge logic if adding or deleting a field + if result, done := v.merge.doAddOrDelete(&e); done { + return result, nil + } + + elem := map[string]apply.Element{} + for key := range e.GetLocalMap() { + elem[key] = e.GetValues()[key] + } + return v.merge.doMergeMap(elem) +} + +// MergeMap returns an error. Only TypeElements can have retainKeys. +func (v retainKeysStrategy) MergeMap(e apply.MapElement) (apply.Result, error) { + return apply.Result{}, fmt.Errorf("Cannot use retainkeys with map element %v", e.Name) +} + +// MergeList returns an error. Only TypeElements can have retainKeys. +func (v retainKeysStrategy) MergeList(e apply.ListElement) (apply.Result, error) { + return apply.Result{}, fmt.Errorf("Cannot use retainkeys with list element %v", e.Name) +} + +// MergePrimitive returns an error. Only TypeElements can have retainKeys. +func (v retainKeysStrategy) MergePrimitive(diff apply.PrimitiveElement) (apply.Result, error) { + return apply.Result{}, fmt.Errorf("Cannot use retainkeys with primitive element %v", diff.Name) +} + +// MergeEmpty returns an empty result +func (v retainKeysStrategy) MergeEmpty(diff apply.EmptyElement) (apply.Result, error) { + return v.merge.MergeEmpty(diff) +} + +var _ apply.Strategy = &retainKeysStrategy{} diff --git a/pkg/kubectl/apply/strategy/strategic_visitor.go b/pkg/kubectl/apply/strategy/strategic_visitor.go index bd198e7e9f..eae1f9230a 100644 --- a/pkg/kubectl/apply/strategy/strategic_visitor.go +++ b/pkg/kubectl/apply/strategy/strategic_visitor.go @@ -23,9 +23,10 @@ import ( // delegatingStrategy delegates merging fields to other visitor implementations // based on the merge strategy preferred by the field. type delegatingStrategy struct { - options Options - merge mergeStrategy - replace replaceStrategy + options Options + merge mergeStrategy + replace replaceStrategy + retainKeys retainKeysStrategy } // createDelegatingStrategy returns a new delegatingStrategy @@ -35,18 +36,20 @@ func createDelegatingStrategy(options Options) *delegatingStrategy { } v.replace = createReplaceStrategy(options, v) v.merge = createMergeStrategy(options, v) + v.retainKeys = createRetainKeysStrategy(options, v) return v } // MergeList delegates visiting a list based on the field patch strategy. // Defaults to "replace" func (v delegatingStrategy) MergeList(diff apply.ListElement) (apply.Result, error) { - // TODO: Support retainkeys switch diff.GetFieldMergeType() { case apply.MergeStrategy: return v.merge.MergeList(diff) case apply.ReplaceStrategy: return v.replace.MergeList(diff) + case apply.RetainKeysStrategy: + return v.retainKeys.MergeList(diff) default: return v.replace.MergeList(diff) } @@ -55,12 +58,13 @@ func (v delegatingStrategy) MergeList(diff apply.ListElement) (apply.Result, err // MergeMap delegates visiting a map based on the field patch strategy. // Defaults to "merge" func (v delegatingStrategy) MergeMap(diff apply.MapElement) (apply.Result, error) { - // TODO: Support retainkeys switch diff.GetFieldMergeType() { case apply.MergeStrategy: return v.merge.MergeMap(diff) case apply.ReplaceStrategy: return v.replace.MergeMap(diff) + case apply.RetainKeysStrategy: + return v.retainKeys.MergeMap(diff) default: return v.merge.MergeMap(diff) } @@ -69,12 +73,13 @@ func (v delegatingStrategy) MergeMap(diff apply.MapElement) (apply.Result, error // MergeType delegates visiting a map based on the field patch strategy. // Defaults to "merge" func (v delegatingStrategy) MergeType(diff apply.TypeElement) (apply.Result, error) { - // TODO: Support retainkeys switch diff.GetFieldMergeType() { case apply.MergeStrategy: return v.merge.MergeType(diff) case apply.ReplaceStrategy: return v.replace.MergeType(diff) + case apply.RetainKeysStrategy: + return v.retainKeys.MergeType(diff) default: return v.merge.MergeType(diff) }