diff --git a/api/builtins/PatchStrategicMergeTransformer.go b/api/builtins/PatchStrategicMergeTransformer.go index fe35533ca..7b9bbb171 100644 --- a/api/builtins/PatchStrategicMergeTransformer.go +++ b/api/builtins/PatchStrategicMergeTransformer.go @@ -5,14 +5,10 @@ package builtins import ( "fmt" - "strings" - "github.com/pkg/errors" - "sigs.k8s.io/kustomize/api/filters/patchstrategicmerge" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" - "sigs.k8s.io/kustomize/kyaml/filtersutil" "sigs.k8s.io/yaml" ) @@ -73,44 +69,10 @@ func (p *PatchStrategicMergeTransformerPlugin) Transform(m resmap.ResMap) error if err != nil { return err } - patchCopy := patch.DeepCopy() - patchCopy.SetName(target.GetName()) - patchCopy.SetNamespace(target.GetNamespace()) - patchCopy.SetGvk(target.GetGvk()) - node, err := filtersutil.GetRNode(patchCopy) - if err != nil { + if err = m.ApplySmPatch( + resource.MakeIdSet([]*resource.Resource{target}), patch); err != nil { return err } - err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ - Patch: node, - }, target) - if err != nil { - // Check for an error string from UnmarshalJSON that's indicative - // of an object that's missing basic KRM fields, and thus may have been - // entirely deleted (an acceptable outcome). This error handling should - // be deleted along with use of ResMap and apimachinery functions like - // UnmarshalJSON. - if !strings.Contains(err.Error(), "Object 'Kind' is missing") { - // Some unknown error, let it through. - return err - } - if !target.IsEmpty() { - return errors.Wrapf( - err, "with unexpectedly non-empty object map of size %d", - len(target.Map())) - } - // Fall through to handle deleted object. - } - if target.IsEmpty() { - // This means all fields have been removed from the object. - // This can happen if a patch required deletion of the - // entire resource (not just a part of it). This means - // the overall resmap must shrink by one. - err = m.Remove(target.CurId()) - if err != nil { - return err - } - } } return nil } diff --git a/api/builtins/PatchTransformer.go b/api/builtins/PatchTransformer.go index 5d6f03e68..62f1f4196 100644 --- a/api/builtins/PatchTransformer.go +++ b/api/builtins/PatchTransformer.go @@ -8,9 +8,7 @@ import ( "strings" jsonpatch "github.com/evanphx/json-patch" - "github.com/pkg/errors" "sigs.k8s.io/kustomize/api/filters/patchjson6902" - "sigs.k8s.io/kustomize/api/filters/patchstrategicmerge" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" @@ -41,7 +39,6 @@ func (p *PatchTransformerPlugin) Config( return fmt.Errorf( "patch and path can't be set at the same time\n%s", string(c)) } - if p.Path != "" { loaded, loadErr := h.Loader().Load(p.Path) if loadErr != nil { @@ -88,66 +85,13 @@ func (p *PatchTransformerPlugin) transformStrategicMerge(m resmap.ResMap, patch if err != nil { return err } - return p.applySMPatch(target, patch) + return target.ApplySmPatch(patch) } - - resources, err := m.Select(*p.Target) + selected, err := m.Select(*p.Target) if err != nil { return err } - for _, res := range resources { - patchCopy := patch.DeepCopy() - patchCopy.SetName(res.GetName()) - patchCopy.SetNamespace(res.GetNamespace()) - patchCopy.SetGvk(res.GetGvk()) - err := p.applySMPatch(res, patchCopy) - if err != nil { - // Check for an error string from UnmarshalJSON that's indicative - // of an object that's missing basic KRM fields, and thus may have been - // entirely deleted (an acceptable outcome). This error handling should - // be deleted along with use of ResMap and apimachinery functions like - // UnmarshalJSON. - if !strings.Contains(err.Error(), "Object 'Kind' is missing") { - // Some unknown error, let it through. - return err - } - if !res.IsEmpty() { - return errors.Wrapf( - err, "with unexpectedly non-empty object map of size %d", - len(res.Map())) - } - // Fall through to handle deleted object. - } - if res.IsEmpty() { - // This means all fields have been removed from the object. - // This can happen if a patch required deletion of the - // entire resource (not just a part of it). This means - // the overall resmap must shrink by one. - err = m.Remove(res.CurId()) - if err != nil { - return err - } - } - } - return nil -} - -// applySMPatch applies the provided strategic merge patch to the -// given resource. -func (p *PatchTransformerPlugin) applySMPatch(resource, patch *resource.Resource) error { - node, err := filtersutil.GetRNode(patch) - if err != nil { - return err - } - n, ns := resource.GetName(), resource.GetNamespace() - err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ - Patch: node, - }, resource) - if !resource.IsEmpty() { - resource.SetName(n) - resource.SetNamespace(ns) - } - return err + return m.ApplySmPatch(resource.MakeIdSet(selected), patch) } // transformJson6902 applies the provided json6902 patch diff --git a/api/internal/target/kusttarget_configplugin.go b/api/internal/target/kusttarget_configplugin.go index a46b2e6ba..640cc742a 100644 --- a/api/internal/target/kusttarget_configplugin.go +++ b/api/internal/target/kusttarget_configplugin.go @@ -183,8 +183,7 @@ var transformerConfigurators = map[builtinhelpers.BuiltinPluginType]func( return } var c struct { - Paths []types.PatchStrategicMerge `json:"paths,omitempty" yaml:"paths,omitempty"` - Patches string `json:"patches,omitempty" yaml:"patches,omitempty"` + Paths []types.PatchStrategicMerge `json:"paths,omitempty" yaml:"paths,omitempty"` } c.Paths = kt.kustomization.PatchesStrategicMerge p := f() diff --git a/api/resmap/resmap.go b/api/resmap/resmap.go index 83fa75461..3d3559ea1 100644 --- a/api/resmap/resmap.go +++ b/api/resmap/resmap.go @@ -240,4 +240,9 @@ type ResMap interface { // ToRNodeSlice converts the resources in the resmp // to a list of RNodes ToRNodeSlice() ([]*yaml.RNode, error) + + // ApplySmPatch applies a strategic-merge patch to the + // selected set of resources. + ApplySmPatch( + selectedSet *resource.IdSet, patch *resource.Resource) error } diff --git a/api/resmap/reswrangler.go b/api/resmap/reswrangler.go index f2eaf9783..c6c894141 100644 --- a/api/resmap/reswrangler.go +++ b/api/resmap/reswrangler.go @@ -6,6 +6,7 @@ package resmap import ( "bytes" "fmt" + "strings" "github.com/pkg/errors" "sigs.k8s.io/kustomize/api/resid" @@ -578,3 +579,46 @@ func (m *resWrangler) ToRNodeSlice() ([]*kyaml_yaml.RNode, error) { } return rnodes, nil } + +func (m *resWrangler) ApplySmPatch( + selectedSet *resource.IdSet, patch *resource.Resource) error { + newRm := New() + for _, res := range m.Resources() { + if !selectedSet.Contains(res.CurId()) { + newRm.Append(res) + continue + } + patchCopy := patch.DeepCopy() + patchCopy.SetName(res.GetName()) + patchCopy.SetNamespace(res.GetNamespace()) + patchCopy.SetGvk(res.GetGvk()) + err := res.ApplySmPatch(patchCopy) + if err != nil { + // Check for an error string from UnmarshalJSON that's indicative + // of an object that's missing basic KRM fields, and thus may have been + // entirely deleted (an acceptable outcome). This error handling should + // be deleted along with use of ResMap and apimachinery functions like + // UnmarshalJSON. + if !strings.Contains(err.Error(), "Object 'Kind' is missing") { + // Some unknown error, let it through. + return err + } + if !res.IsEmpty() { + return errors.Wrapf( + err, "with unexpectedly non-empty object map of size %d", + len(res.Map())) + } + // Fall through to handle deleted object. + } + if !res.IsEmpty() { + // IsEmpty means all fields have been removed from the object. + // This can happen if a patch required deletion of the + // entire resource (not just a part of it). This means + // the overall resmap must shrink by one. + newRm.Append(res) + } + } + m.Clear() + m.AppendAll(newRm) + return nil +} diff --git a/api/resource/resource.go b/api/resource/resource.go index a317c744a..38e5d7c9b 100644 --- a/api/resource/resource.go +++ b/api/resource/resource.go @@ -9,9 +9,11 @@ import ( "reflect" "strings" + "sigs.k8s.io/kustomize/api/filters/patchstrategicmerge" "sigs.k8s.io/kustomize/api/ifc" "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/types" + "sigs.k8s.io/kustomize/kyaml/filtersutil" "sigs.k8s.io/yaml" ) @@ -396,6 +398,23 @@ func (r *Resource) AppendRefVarName(variable types.Var) { r.refVarNames = append(r.refVarNames, variable.Name) } +// ApplySmPatch applies the provided strategic merge patch. +func (r *Resource) ApplySmPatch(patch *Resource) error { + node, err := filtersutil.GetRNode(patch) + if err != nil { + return err + } + n, ns := r.GetName(), r.GetNamespace() + err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ + Patch: node, + }, r) + if !r.IsEmpty() { + r.SetName(n) + r.SetNamespace(ns) + } + return err +} + // TODO: Add BinaryData once we sync to new k8s.io/api func mergeConfigmap( mergedTo map[string]interface{}, diff --git a/api/resource/resource_test.go b/api/resource/resource_test.go index a5a67792b..ba147e9cc 100644 --- a/api/resource/resource_test.go +++ b/api/resource/resource_test.go @@ -4,9 +4,11 @@ package resource_test import ( + "fmt" "reflect" "testing" + "github.com/stretchr/testify/assert" "sigs.k8s.io/kustomize/api/provider" "sigs.k8s.io/kustomize/api/resid" . "sigs.k8s.io/kustomize/api/resource" @@ -123,3 +125,575 @@ func TestDeepCopy(t *testing.T) { t.Errorf("expected %v\nbut got%v", r, cr) } } + +func TestApplySmPatch_1(t *testing.T) { + resource, err := factory.FromBytes([]byte(` +apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + baseAnno: This is a base annotation + labels: + app: mungebot + foo: bar + name: bingo +spec: + replicas: 1 + selector: + matchLabels: + foo: bar + template: + metadata: + labels: + app: mungebot + spec: + containers: + - env: + - name: foo + value: bar + image: nginx + name: nginx + ports: + - containerPort: 80 +`)) + assert.NoError(t, err) + patch, err := factory.FromBytes([]byte(` +apiVersion: apps/v1 +kind: Deployment +metadata: + name: baseprefix-mungebot +spec: + template: + spec: + containers: + - image: nginx + name: nginx + ports: + - containerPort: 777 +`)) + assert.NoError(t, err) + + assert.NoError(t, resource.ApplySmPatch(patch)) + bytes, err := resource.AsYAML() + assert.NoError(t, err) + assert.Equal(t, `apiVersion: apps/v1 +kind: Deployment +metadata: + annotations: + baseAnno: This is a base annotation + labels: + app: mungebot + foo: bar + name: bingo +spec: + replicas: 1 + selector: + matchLabels: + foo: bar + template: + metadata: + labels: + app: mungebot + spec: + containers: + - env: + - name: foo + value: bar + image: nginx + name: nginx + ports: + - containerPort: 777 + - containerPort: 80 +`, string(bytes)) +} + +func TestApplySmPatch(t *testing.T) { + const ( + myDeployment = "Deployment" + myCRD = "myCRD" + ) + + tests := map[string]struct { + base string + patch []string + expected string + errorExpected bool + errorMsg string + }{ + "withschema-label-image-container": { + base: baseResource(myDeployment), + patch: []string{ + addLabelAndEnvPatch(myDeployment), + changeImagePatch(myDeployment, "nginx:latest"), + addContainerAndEnvPatch(myDeployment), + }, + errorExpected: false, + expected: expectedResultMultiPatch(myDeployment, false), + }, + "withschema-image-container-label": { + base: baseResource(myDeployment), + patch: []string{ + changeImagePatch(myDeployment, "nginx:latest"), + addContainerAndEnvPatch(myDeployment), + addLabelAndEnvPatch(myDeployment), + }, + errorExpected: false, + expected: expectedResultMultiPatch(myDeployment, true), + }, + "withschema-container-label-image": { + base: baseResource(myDeployment), + patch: []string{ + addContainerAndEnvPatch(myDeployment), + addLabelAndEnvPatch(myDeployment), + changeImagePatch(myDeployment, "nginx:latest"), + }, + errorExpected: false, + expected: expectedResultMultiPatch(myDeployment, true), + }, + "noschema-label-image-container": { + base: baseResource(myCRD), + patch: []string{ + addLabelAndEnvPatch(myCRD), + changeImagePatch(myCRD, "nginx:latest"), + addContainerAndEnvPatch(myCRD), + }, + // Might be better if this complained about patch conflict. + // See plugin/builtin/patchstrategicmergetransformer/psmt_test.go + expected: `apiVersion: apps/v1 +kind: myCRD +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - env: + - name: ANOTHERENV + value: ANOTHERVALUE + name: nginx + - image: anotherimage + name: anothercontainer +`, + }, + "noschema-image-container-label": { + base: baseResource(myCRD), + patch: []string{ + changeImagePatch(myCRD, "nginx:latest"), + addContainerAndEnvPatch(myCRD), + addLabelAndEnvPatch(myCRD), + }, + // Might be better if this complained about patch conflict. + expected: `apiVersion: apps/v1 +kind: myCRD +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - env: + - name: SOMEENV + value: SOMEVALUE + name: nginx +`, + }, + "noschema-container-label-image": { + base: baseResource(myCRD), + patch: []string{ + addContainerAndEnvPatch(myCRD), + addLabelAndEnvPatch(myCRD), + changeImagePatch(myCRD, "nginx:latest"), + }, + // Might be better if this complained about patch conflict. + expected: `apiVersion: apps/v1 +kind: myCRD +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - image: nginx:latest + name: nginx +`, + }, + + "withschema-label-latest-someV-01": { + base: baseResource(myDeployment), + patch: []string{ + addLabelAndEnvPatch(myDeployment), + changeImagePatch(myDeployment, "nginx:latest"), + changeImagePatch(myDeployment, "nginx:1.7.9"), + }, + expected: `apiVersion: apps/v1 +kind: Deployment +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - env: + - name: SOMEENV + value: SOMEVALUE + image: nginx:1.7.9 + name: nginx +`, + }, + "withschema-latest-label-someV-02": { + base: baseResource(myDeployment), + patch: []string{ + changeImagePatch(myDeployment, "nginx:latest"), + addLabelAndEnvPatch(myDeployment), + changeImagePatch(myDeployment, "nginx:1.7.9"), + }, + expected: `apiVersion: apps/v1 +kind: Deployment +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - env: + - name: SOMEENV + value: SOMEVALUE + image: nginx:1.7.9 + name: nginx +`, + }, + "withschema-latest-label-someV-03": { + base: baseResource(myDeployment), + patch: []string{ + changeImagePatch(myDeployment, "nginx:1.7.9"), + addLabelAndEnvPatch(myDeployment), + changeImagePatch(myDeployment, "nginx:latest"), + }, + expected: `apiVersion: apps/v1 +kind: Deployment +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - env: + - name: SOMEENV + value: SOMEVALUE + image: nginx:latest + name: nginx +`, + }, + "withschema-latest-label-someV-04": { + base: baseResource(myDeployment), + patch: []string{ + changeImagePatch(myDeployment, "nginx:1.7.9"), + changeImagePatch(myDeployment, "nginx:latest"), + addLabelAndEnvPatch(myDeployment), + changeImagePatch(myDeployment, "nginx:nginx"), + }, + expected: `apiVersion: apps/v1 +kind: Deployment +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - env: + - name: SOMEENV + value: SOMEVALUE + image: nginx:nginx + name: nginx +`, + }, + "noschema-latest-label-someV-01": { + base: baseResource(myCRD), + patch: []string{ + addLabelAndEnvPatch(myCRD), + changeImagePatch(myCRD, "nginx:latest"), + changeImagePatch(myCRD, "nginx:1.7.9"), + }, + expected: `apiVersion: apps/v1 +kind: myCRD +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - image: nginx:1.7.9 + name: nginx +`, + }, + "noschema-latest-label-someV-02": { + base: baseResource(myCRD), + patch: []string{ + changeImagePatch(myCRD, "nginx:latest"), + addLabelAndEnvPatch(myCRD), + changeImagePatch(myCRD, "nginx:1.7.9"), + }, + expected: expectedResultJMP("nginx:1.7.9"), + }, + "noschema-latest-label-someV-03": { + base: baseResource(myCRD), + patch: []string{ + changeImagePatch(myCRD, "nginx:1.7.9"), + addLabelAndEnvPatch(myCRD), + changeImagePatch(myCRD, "nginx:latest"), + }, + expected: `apiVersion: apps/v1 +kind: myCRD +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - image: nginx:latest + name: nginx +`, + }, + "noschema-latest-label-someV-04": { + base: baseResource(myCRD), + patch: []string{ + changeImagePatch(myCRD, "nginx:1.7.9"), + changeImagePatch(myCRD, "nginx:latest"), + addLabelAndEnvPatch(myCRD), + changeImagePatch(myCRD, "nginx:nginx"), + }, + expected: `apiVersion: apps/v1 +kind: myCRD +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - image: nginx:nginx + name: nginx +`, + }, + } + + for name, test := range tests { + resource, err := factory.FromBytes([]byte(test.base)) + assert.NoError(t, err) + for _, p := range test.patch { + patch, err := factory.FromBytes([]byte(p)) + assert.NoError(t, err, name) + assert.NoError(t, resource.ApplySmPatch(patch), name) + } + bytes, err := resource.AsYAML() + if test.errorExpected { + assert.Error(t, err, name) + } else { + assert.NoError(t, err, name) + assert.Equal(t, test.expected, string(bytes), name) + } + } +} + +// baseResource produces a base object which used to test +// patch transformation +// Also the structure is matching the Deployment syntax +// the kind can be replaced to allow testing using CRD +// without access to the schema +func baseResource(kind string) string { + res := ` +apiVersion: apps/v1 +kind: %s +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + spec: + containers: + - name: nginx + image: nginx` + return fmt.Sprintf(res, kind) +} + +// addContainerAndEnvPatch produces a patch object which adds +// an entry in the env slice of the first/nginx container +// as well as adding a label in the metadata +// Note that for SMP/WithSchema merge, the name:nginx entry +// is mandatory +func addLabelAndEnvPatch(kind string) string { + return fmt.Sprintf(` +apiVersion: apps/v1 +kind: %s +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + some-label: some-value + spec: + containers: + - name: nginx + env: + - name: SOMEENV + value: SOMEVALUE`, kind) +} + +// addContainerAndEnvPatch produces a patch object which adds +// an entry in the env slice of the first/nginx container +// as well as adding a second container in the container list +// Note that for SMP/WithSchema merge, the name:nginx entry +// is mandatory +func addContainerAndEnvPatch(kind string) string { + return fmt.Sprintf(` +apiVersion: apps/v1 +kind: %s +metadata: + name: deploy1 +spec: + template: + spec: + containers: + - name: nginx + env: + - name: ANOTHERENV + value: ANOTHERVALUE + - name: anothercontainer + image: anotherimage`, kind) +} + +// addContainerAndEnvPatch produces a patch object which replaces +// the value of the image field in the first/nginx container +// Note that for SMP/WithSchema merge, the name:nginx entry +// is mandatory +func changeImagePatch(kind string, newImage string) string { + return fmt.Sprintf(` +apiVersion: apps/v1 +kind: %s +metadata: + name: deploy1 +spec: + template: + spec: + containers: + - name: nginx + image: %s`, kind, newImage) +} + +// utility method to build the expected result of a multipatch +// the order of the patches still have influence especially +// in the insertion location within arrays. +func expectedResultMultiPatch(kind string, reversed bool) string { + pattern := `apiVersion: apps/v1 +kind: %s +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - env: + %s + image: nginx:latest + name: nginx + - image: anotherimage + name: anothercontainer +` + if reversed { + return fmt.Sprintf(pattern, kind, `- name: SOMEENV + value: SOMEVALUE + - name: ANOTHERENV + value: ANOTHERVALUE`) + } + return fmt.Sprintf(pattern, kind, `- name: ANOTHERENV + value: ANOTHERVALUE + - name: SOMEENV + value: SOMEVALUE`) +} + +// utility method building the expected output of a JMP. +// imagename parameter allows to build a result consistent +// with the JMP behavior which basically overrides the +// entire "containers" list. +func expectedResultJMP(imagename string) string { + if imagename == "" { + return `apiVersion: apps/v1 +kind: myCRD +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - env: + - name: SOMEENV + value: SOMEVALUE + name: nginx +` + } + return fmt.Sprintf(`apiVersion: apps/v1 +kind: myCRD +metadata: + name: deploy1 +spec: + template: + metadata: + labels: + old-label: old-value + some-label: some-value + spec: + containers: + - image: %s + name: nginx +`, imagename) +} diff --git a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go index 100af7f3f..5d7b4f738 100644 --- a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go +++ b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go @@ -6,14 +6,9 @@ package main import ( "fmt" - "strings" - - "github.com/pkg/errors" - "sigs.k8s.io/kustomize/api/filters/patchstrategicmerge" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" - "sigs.k8s.io/kustomize/kyaml/filtersutil" "sigs.k8s.io/yaml" ) @@ -77,44 +72,10 @@ func (p *plugin) Transform(m resmap.ResMap) error { if err != nil { return err } - patchCopy := patch.DeepCopy() - patchCopy.SetName(target.GetName()) - patchCopy.SetNamespace(target.GetNamespace()) - patchCopy.SetGvk(target.GetGvk()) - node, err := filtersutil.GetRNode(patchCopy) - if err != nil { + if err = m.ApplySmPatch( + resource.MakeIdSet([]*resource.Resource{target}), patch); err != nil { return err } - err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ - Patch: node, - }, target) - if err != nil { - // Check for an error string from UnmarshalJSON that's indicative - // of an object that's missing basic KRM fields, and thus may have been - // entirely deleted (an acceptable outcome). This error handling should - // be deleted along with use of ResMap and apimachinery functions like - // UnmarshalJSON. - if !strings.Contains(err.Error(), "Object 'Kind' is missing") { - // Some unknown error, let it through. - return err - } - if !target.IsEmpty() { - return errors.Wrapf( - err, "with unexpectedly non-empty object map of size %d", - len(target.Map())) - } - // Fall through to handle deleted object. - } - if target.IsEmpty() { - // This means all fields have been removed from the object. - // This can happen if a patch required deletion of the - // entire resource (not just a part of it). This means - // the overall resmap must shrink by one. - err = m.Remove(target.CurId()) - if err != nil { - return err - } - } } return nil } diff --git a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go index 069782391..acd8764c2 100644 --- a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go +++ b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer_test.go @@ -1107,7 +1107,7 @@ func TestMultiplePatchesWithConflict(t *testing.T) { errorMsg: "conflict", }, { - name: "withschema-latest-label-1.7.9", + name: "withschema-latest-label-1.7.9-difforder", base: baseResource(Deployment), patch: []string{ changeImagePatch(Deployment, "nginx:latest"), diff --git a/plugin/builtin/patchstrategicmergetransformer/go.mod b/plugin/builtin/patchstrategicmergetransformer/go.mod index 4590ebebd..e399cf47f 100644 --- a/plugin/builtin/patchstrategicmergetransformer/go.mod +++ b/plugin/builtin/patchstrategicmergetransformer/go.mod @@ -3,9 +3,7 @@ module sigs.k8s.io/kustomize/plugin/builtin/patchstrategicmergetransformer go 1.15 require ( - github.com/pkg/errors v0.8.1 sigs.k8s.io/kustomize/api v0.6.5 - sigs.k8s.io/kustomize/kyaml v0.9.4 sigs.k8s.io/yaml v1.2.0 ) diff --git a/plugin/builtin/patchtransformer/PatchTransformer.go b/plugin/builtin/patchtransformer/PatchTransformer.go index 172c92732..a92758dae 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer.go +++ b/plugin/builtin/patchtransformer/PatchTransformer.go @@ -9,9 +9,7 @@ import ( "strings" jsonpatch "github.com/evanphx/json-patch" - "github.com/pkg/errors" "sigs.k8s.io/kustomize/api/filters/patchjson6902" - "sigs.k8s.io/kustomize/api/filters/patchstrategicmerge" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" "sigs.k8s.io/kustomize/api/types" @@ -45,7 +43,6 @@ func (p *plugin) Config( return fmt.Errorf( "patch and path can't be set at the same time\n%s", string(c)) } - if p.Path != "" { loaded, loadErr := h.Loader().Load(p.Path) if loadErr != nil { @@ -92,66 +89,13 @@ func (p *plugin) transformStrategicMerge(m resmap.ResMap, patch *resource.Resour if err != nil { return err } - return p.applySMPatch(target, patch) + return target.ApplySmPatch(patch) } - - resources, err := m.Select(*p.Target) + selected, err := m.Select(*p.Target) if err != nil { return err } - for _, res := range resources { - patchCopy := patch.DeepCopy() - patchCopy.SetName(res.GetName()) - patchCopy.SetNamespace(res.GetNamespace()) - patchCopy.SetGvk(res.GetGvk()) - err := p.applySMPatch(res, patchCopy) - if err != nil { - // Check for an error string from UnmarshalJSON that's indicative - // of an object that's missing basic KRM fields, and thus may have been - // entirely deleted (an acceptable outcome). This error handling should - // be deleted along with use of ResMap and apimachinery functions like - // UnmarshalJSON. - if !strings.Contains(err.Error(), "Object 'Kind' is missing") { - // Some unknown error, let it through. - return err - } - if !res.IsEmpty() { - return errors.Wrapf( - err, "with unexpectedly non-empty object map of size %d", - len(res.Map())) - } - // Fall through to handle deleted object. - } - if res.IsEmpty() { - // This means all fields have been removed from the object. - // This can happen if a patch required deletion of the - // entire resource (not just a part of it). This means - // the overall resmap must shrink by one. - err = m.Remove(res.CurId()) - if err != nil { - return err - } - } - } - return nil -} - -// applySMPatch applies the provided strategic merge patch to the -// given resource. -func (p *plugin) applySMPatch(resource, patch *resource.Resource) error { - node, err := filtersutil.GetRNode(patch) - if err != nil { - return err - } - n, ns := resource.GetName(), resource.GetNamespace() - err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ - Patch: node, - }, resource) - if !resource.IsEmpty() { - resource.SetName(n) - resource.SetNamespace(ns) - } - return err + return m.ApplySmPatch(resource.MakeIdSet(selected), patch) } // transformJson6902 applies the provided json6902 patch diff --git a/plugin/builtin/patchtransformer/go.mod b/plugin/builtin/patchtransformer/go.mod index 3092ab58d..ce15ea508 100644 --- a/plugin/builtin/patchtransformer/go.mod +++ b/plugin/builtin/patchtransformer/go.mod @@ -4,7 +4,6 @@ go 1.15 require ( github.com/evanphx/json-patch v4.5.0+incompatible - github.com/pkg/errors v0.8.1 sigs.k8s.io/kustomize/api v0.6.5 sigs.k8s.io/kustomize/kyaml v0.9.4 sigs.k8s.io/yaml v1.2.0