diff --git a/api/builtins/NamespaceTransformer.go b/api/builtins/NamespaceTransformer.go index 22937055c..8f7fddec8 100644 --- a/api/builtins/NamespaceTransformer.go +++ b/api/builtins/NamespaceTransformer.go @@ -5,13 +5,11 @@ package builtins import ( "fmt" - "strings" "sigs.k8s.io/kustomize/api/filters/namespace" "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" - "sigs.k8s.io/kustomize/api/transform" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/filtersutil" "sigs.k8s.io/yaml" @@ -21,22 +19,12 @@ import ( type NamespaceTransformerPlugin struct { types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` FieldSpecs []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` - - // YAMLSupport can be set to true to use the kyaml filter instead of the - // kunstruct transformer. - // TODO: change the default to use kyaml when it is stable - YAMLSupport bool `json:"yamlSupport,omitempty" yaml:"yamlSupport,omitempty"` } func (p *NamespaceTransformerPlugin) Config( _ *resmap.PluginHelpers, c []byte) (err error) { p.Namespace = "" p.FieldSpecs = nil - if !strings.Contains(string(c), "yamlSupport") { - // If not explicitly denied, - // activate kyaml-based transformation. - p.YAMLSupport = true - } return yaml.Unmarshal(c, p) } @@ -49,31 +37,13 @@ func (p *NamespaceTransformerPlugin) Transform(m resmap.ResMap) error { // Don't mutate empty objects? continue } - - id := r.OrgId() - - if p.YAMLSupport { - // use the new style transform - err := filtersutil.ApplyToJSON(namespace.Filter{ - Namespace: p.Namespace, - FsSlice: p.FieldSpecs, - }, r) - if err != nil { - return err - } - } else { - // use the old style transform - applicableFs := p.applicableFieldSpecs(id) - for _, fs := range applicableFs { - err := transform.MutateField( - r.Map(), fs.PathSlice(), fs.CreateIfNotPresent, - p.changeNamespace(r)) - if err != nil { - return err - } - } + err := filtersutil.ApplyToJSON(namespace.Filter{ + Namespace: p.Namespace, + FsSlice: p.FieldSpecs, + }, r) + if err != nil { + return err } - matches := m.GetMatchingResourcesByCurrentId(r.CurId().Equals) if len(matches) != 1 { return fmt.Errorf( diff --git a/api/builtins/PatchJson6902Transformer.go b/api/builtins/PatchJson6902Transformer.go index 7a4535410..7d56a15f3 100644 --- a/api/builtins/PatchJson6902Transformer.go +++ b/api/builtins/PatchJson6902Transformer.go @@ -5,7 +5,6 @@ package builtins import ( "fmt" - "strings" jsonpatch "github.com/evanphx/json-patch" "github.com/pkg/errors" @@ -24,8 +23,6 @@ type PatchJson6902TransformerPlugin struct { Target types.PatchTarget `json:"target,omitempty" yaml:"target,omitempty"` Path string `json:"path,omitempty" yaml:"path,omitempty"` JsonOp string `json:"jsonOp,omitempty" yaml:"jsonOp,omitempty"` - - YAMLSupport bool `json:"yamlSupport,omitempty" yaml:"yamlSupport,omitempty"` } func (p *PatchJson6902TransformerPlugin) Config( @@ -35,11 +32,6 @@ func (p *PatchJson6902TransformerPlugin) Config( if err != nil { return err } - if !strings.Contains(string(c), "yamlSupport") { - // If not explicitly denied, - // activate kyaml-based transformation. - p.YAMLSupport = true - } if p.Target.Name == "" { return fmt.Errorf("must specify the target name") } @@ -93,22 +85,9 @@ func (p *PatchJson6902TransformerPlugin) Transform(m resmap.ResMap) error { if err != nil { return err } - if !p.YAMLSupport { - rawObj, err := obj.MarshalJSON() - if err != nil { - return err - } - modifiedObj, err := p.decodedPatch.Apply(rawObj) - if err != nil { - return errors.Wrapf( - err, "failed to apply json patch '%s'", p.JsonOp) - } - return obj.UnmarshalJSON(modifiedObj) - } else { - return filtersutil.ApplyToJSON(patchjson6902.Filter{ - Patch: p.JsonOp, - }, obj) - } + return filtersutil.ApplyToJSON(patchjson6902.Filter{ + Patch: p.JsonOp, + }, obj) } func NewPatchJson6902TransformerPlugin() resmap.TransformerPlugin { diff --git a/api/builtins/PatchStrategicMergeTransformer.go b/api/builtins/PatchStrategicMergeTransformer.go index 0ad21cb88..e7636a71e 100644 --- a/api/builtins/PatchStrategicMergeTransformer.go +++ b/api/builtins/PatchStrategicMergeTransformer.go @@ -7,6 +7,7 @@ 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" @@ -20,8 +21,6 @@ type PatchStrategicMergeTransformerPlugin struct { loadedPatches []*resource.Resource Paths []types.PatchStrategicMerge `json:"paths,omitempty" yaml:"paths,omitempty"` Patches string `json:"patches,omitempty" yaml:"patches,omitempty"` - - YAMLSupport bool `json:"yamlSupport,omitempty" yaml:"yamlSupport,omitempty"` } func (p *PatchStrategicMergeTransformerPlugin) Config( @@ -31,11 +30,6 @@ func (p *PatchStrategicMergeTransformerPlugin) Config( if err != nil { return err } - if !strings.Contains(string(c), "yamlSupport") { - // If not explicitly denied, - // activate kyaml-based transformation. - p.YAMLSupport = true - } if len(p.Paths) == 0 && p.Patches == "" { return fmt.Errorf("empty file path and empty patch content") } @@ -79,24 +73,34 @@ func (p *PatchStrategicMergeTransformerPlugin) Transform(m resmap.ResMap) error if err != nil { return err } - if !p.YAMLSupport { - err = target.Patch(patch.Copy()) - } else { - patchCopy := patch.DeepCopy() - patchCopy.SetName(target.GetName()) - patchCopy.SetNamespace(target.GetNamespace()) - patchCopy.SetGvk(target.GetGvk()) - node, err := filtersutil.GetRNode(patchCopy) - if err != nil { - return err - } - err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ - Patch: node, - }, target) - } + patchCopy := patch.DeepCopy() + patchCopy.SetName(target.GetName()) + patchCopy.SetNamespace(target.GetNamespace()) + patchCopy.SetGvk(target.GetGvk()) + node, err := filtersutil.GetRNode(patchCopy) if 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 len(target.Map()) != 0 { + return errors.Wrapf( + err, "with unexpectedly non-empty object map of size %d", + len(target.Map())) + } + // Fall through to handle deleted object. + } if len(target.Map()) == 0 { // This means all fields have been removed from the object. // This can happen if a patch required deletion of the diff --git a/api/builtins/PatchTransformer.go b/api/builtins/PatchTransformer.go index fd4651a9b..cb7a83c01 100644 --- a/api/builtins/PatchTransformer.go +++ b/api/builtins/PatchTransformer.go @@ -8,7 +8,6 @@ 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" @@ -24,8 +23,6 @@ type PatchTransformerPlugin struct { Path string `json:"path,omitempty" yaml:"path,omitempty"` Patch string `json:"patch,omitempty" yaml:"patch,omitempty"` Target *types.Selector `json:"target,omitempty" yaml:"target,omitempty"` - - YAMLSupport bool `json:"yamlSupport,omitempty" yaml:"yamlSupport,omitempty"` } func (p *PatchTransformerPlugin) Config( @@ -34,11 +31,6 @@ func (p *PatchTransformerPlugin) Config( if err != nil { return err } - if !strings.Contains(string(c), "yamlSupport") { - // If not explicitly denied, - // activate kyaml-based transformation. - p.YAMLSupport = true - } p.Patch = strings.TrimSpace(p.Patch) if p.Patch == "" && p.Path == "" { return fmt.Errorf( @@ -78,11 +70,11 @@ func (p *PatchTransformerPlugin) Config( } func (p *PatchTransformerPlugin) Transform(m resmap.ResMap) error { - if p.loadedPatch != nil { + if p.loadedPatch == nil { + return p.transformJson6902(m, p.decodedPatch) + } else { // The patch was a strategic merge patch return p.transformStrategicMerge(m, p.loadedPatch) - } else { - return p.transformJson6902(m, p.decodedPatch) } } @@ -116,20 +108,15 @@ func (p *PatchTransformerPlugin) transformStrategicMerge(m resmap.ResMap, patch } // applySMPatch applies the provided strategic merge patch to the -// given resource. Depending on the value of YAMLSupport, it will either -// use the legacy implementation or the kyaml-based solution. +// given resource. func (p *PatchTransformerPlugin) applySMPatch(resource, patch *resource.Resource) error { - if !p.YAMLSupport { - return resource.Patch(patch.Copy()) - } else { - node, err := filtersutil.GetRNode(patch) - if err != nil { - return err - } - return filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ - Patch: node, - }, resource) + node, err := filtersutil.GetRNode(patch) + if err != nil { + return err } + return filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ + Patch: node, + }, resource) } // transformJson6902 applies the provided json6902 patch @@ -138,13 +125,14 @@ func (p *PatchTransformerPlugin) transformJson6902(m resmap.ResMap, patch jsonpa if p.Target == nil { return fmt.Errorf("must specify a target for patch %s", p.Patch) } - resources, err := m.Select(*p.Target) if err != nil { return err } for _, res := range resources { - err = p.applyJson6902Patch(res, patch) + err = filtersutil.ApplyToJSON(patchjson6902.Filter{ + Patch: p.Patch, + }, res) if err != nil { return err } @@ -152,28 +140,6 @@ func (p *PatchTransformerPlugin) transformJson6902(m resmap.ResMap, patch jsonpa return nil } -// applyJson6902Patch applies the provided patch to the given resource. -// Depending on the value of YAMLSupport, it will either -// use the legacy implementation or the kyaml-based solution. -func (p *PatchTransformerPlugin) applyJson6902Patch(resource *resource.Resource, patch jsonpatch.Patch) error { - if !p.YAMLSupport { - rawObj, err := resource.MarshalJSON() - if err != nil { - return err - } - modifiedObj, err := patch.Apply(rawObj) - if err != nil { - return errors.Wrapf( - err, "failed to apply json patch '%s'", p.Patch) - } - return resource.UnmarshalJSON(modifiedObj) - } else { - return filtersutil.ApplyToJSON(patchjson6902.Filter{ - Patch: p.Patch, - }, resource) - } -} - // jsonPatchFromBytes loads a Json 6902 patch from // a bytes input func jsonPatchFromBytes( diff --git a/api/testutils/kusttest/harnessenhanced.go b/api/testutils/kusttest/harnessenhanced.go index 6ed84af14..f40320b0f 100644 --- a/api/testutils/kusttest/harnessenhanced.go +++ b/api/testutils/kusttest/harnessenhanced.go @@ -4,9 +4,6 @@ package kusttest_test import ( - "bytes" - "fmt" - "strconv" "testing" "sigs.k8s.io/kustomize/api/filesys" @@ -20,8 +17,6 @@ import ( "sigs.k8s.io/kustomize/api/resource" valtest_test "sigs.k8s.io/kustomize/api/testutils/valtest" "sigs.k8s.io/kustomize/api/types" - "sigs.k8s.io/kustomize/kyaml/kio" - "sigs.k8s.io/kustomize/kyaml/yaml" ) // HarnessEnhanced manages a full plugin environment for tests. @@ -130,42 +125,8 @@ func (th *HarnessEnhanced) LoadAndRunTransformer( func (th *HarnessEnhanced) RunTransformerAndCheckResult( config, input, expected string) { - for _, b := range []bool{true, false} { - th.t.Run(fmt.Sprintf("yaml-%v", b), func(t *testing.T) { - c, err := toggleYamlSupportField(config, b) - if err != nil { - th.t.Fatalf("Err: %v", err) - } - resMap, err := th.RunTransformer(c, input) - if err != nil { - th.t.Fatalf("Err: %v", err) - } - th.AssertActualEqualsExpected(resMap, expected) - }) - } -} - -func toggleYamlSupportField(config string, yamlSupport bool) (string, error) { - var out bytes.Buffer - rw := kio.ByteReadWriter{ - Reader: bytes.NewBufferString(config), - Writer: &out, - } - err := kio.Pipeline{ - Inputs: []kio.Reader{&rw}, - Filters: []kio.Filter{ - kio.FilterAll(yaml.FilterFunc( - func(node *yaml.RNode) (*yaml.RNode, error) { - return node.Pipe(yaml.FieldSetter{ - Name: "yamlSupport", - StringValue: strconv.FormatBool(yamlSupport), - }) - }), - ), - }, - Outputs: []kio.Writer{&rw}, - }.Execute() - return out.String(), err + resMap := th.LoadAndRunTransformer(config, input) + th.AssertActualEqualsExpected(resMap, expected) } func (th *HarnessEnhanced) ErrorFromLoadAndRunTransformer( @@ -178,16 +139,8 @@ type AssertFunc func(t *testing.T, err error) func (th *HarnessEnhanced) RunTransformerAndCheckError( config, input string, assertFn AssertFunc) { - for _, b := range []bool{true, false} { - th.t.Run(fmt.Sprintf("yaml-%v", b), func(t *testing.T) { - c, err := toggleYamlSupportField(config, b) - if err != nil { - th.t.Fatalf("Err: %v", err) - } - _, err = th.RunTransformer(c, input) - assertFn(t, err) - }) - } + _, err := th.RunTransformer(config, input) + assertFn(th.t, err) } func (th *HarnessEnhanced) RunTransformer( @@ -203,6 +156,7 @@ func (th *HarnessEnhanced) RunTransformerFromResMap( config string, resMap resmap.ResMap) (resmap.ResMap, error) { transConfig, err := th.rf.RF().FromBytes([]byte(config)) if err != nil { + th.t.Logf("config: '%s'", config) th.t.Fatalf("Err: %v", err) } g, err := th.pl.LoadTransformer( diff --git a/plugin/builtin/namespacetransformer/NamespaceTransformer.go b/plugin/builtin/namespacetransformer/NamespaceTransformer.go index 32d9c05a3..b027f50a4 100644 --- a/plugin/builtin/namespacetransformer/NamespaceTransformer.go +++ b/plugin/builtin/namespacetransformer/NamespaceTransformer.go @@ -6,13 +6,11 @@ package main import ( "fmt" - "strings" "sigs.k8s.io/kustomize/api/filters/namespace" "sigs.k8s.io/kustomize/api/resid" "sigs.k8s.io/kustomize/api/resmap" "sigs.k8s.io/kustomize/api/resource" - "sigs.k8s.io/kustomize/api/transform" "sigs.k8s.io/kustomize/api/types" "sigs.k8s.io/kustomize/kyaml/filtersutil" "sigs.k8s.io/yaml" @@ -22,11 +20,6 @@ import ( type plugin struct { types.ObjectMeta `json:"metadata,omitempty" yaml:"metadata,omitempty" protobuf:"bytes,1,opt,name=metadata"` FieldSpecs []types.FieldSpec `json:"fieldSpecs,omitempty" yaml:"fieldSpecs,omitempty"` - - // YAMLSupport can be set to true to use the kyaml filter instead of the - // kunstruct transformer. - // TODO: change the default to use kyaml when it is stable - YAMLSupport bool `json:"yamlSupport,omitempty" yaml:"yamlSupport,omitempty"` } //noinspection GoUnusedGlobalVariable @@ -36,11 +29,6 @@ func (p *plugin) Config( _ *resmap.PluginHelpers, c []byte) (err error) { p.Namespace = "" p.FieldSpecs = nil - if !strings.Contains(string(c), "yamlSupport") { - // If not explicitly denied, - // activate kyaml-based transformation. - p.YAMLSupport = true - } return yaml.Unmarshal(c, p) } @@ -53,31 +41,13 @@ func (p *plugin) Transform(m resmap.ResMap) error { // Don't mutate empty objects? continue } - - id := r.OrgId() - - if p.YAMLSupport { - // use the new style transform - err := filtersutil.ApplyToJSON(namespace.Filter{ - Namespace: p.Namespace, - FsSlice: p.FieldSpecs, - }, r) - if err != nil { - return err - } - } else { - // use the old style transform - applicableFs := p.applicableFieldSpecs(id) - for _, fs := range applicableFs { - err := transform.MutateField( - r.Map(), fs.PathSlice(), fs.CreateIfNotPresent, - p.changeNamespace(r)) - if err != nil { - return err - } - } + err := filtersutil.ApplyToJSON(namespace.Filter{ + Namespace: p.Namespace, + FsSlice: p.FieldSpecs, + }, r) + if err != nil { + return err } - matches := m.GetMatchingResourcesByCurrentId(r.CurId().Equals) if len(matches) != 1 { return fmt.Errorf( diff --git a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go index 838a52e33..f07226fc9 100644 --- a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go +++ b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go @@ -6,7 +6,6 @@ package main import ( "fmt" - "strings" jsonpatch "github.com/evanphx/json-patch" "github.com/pkg/errors" @@ -25,8 +24,6 @@ type plugin struct { Target types.PatchTarget `json:"target,omitempty" yaml:"target,omitempty"` Path string `json:"path,omitempty" yaml:"path,omitempty"` JsonOp string `json:"jsonOp,omitempty" yaml:"jsonOp,omitempty"` - - YAMLSupport bool `json:"yamlSupport,omitempty" yaml:"yamlSupport,omitempty"` } //noinspection GoUnusedGlobalVariable @@ -39,11 +36,6 @@ func (p *plugin) Config( if err != nil { return err } - if !strings.Contains(string(c), "yamlSupport") { - // If not explicitly denied, - // activate kyaml-based transformation. - p.YAMLSupport = true - } if p.Target.Name == "" { return fmt.Errorf("must specify the target name") } @@ -97,20 +89,7 @@ func (p *plugin) Transform(m resmap.ResMap) error { if err != nil { return err } - if !p.YAMLSupport { - rawObj, err := obj.MarshalJSON() - if err != nil { - return err - } - modifiedObj, err := p.decodedPatch.Apply(rawObj) - if err != nil { - return errors.Wrapf( - err, "failed to apply json patch '%s'", p.JsonOp) - } - return obj.UnmarshalJSON(modifiedObj) - } else { - return filtersutil.ApplyToJSON(patchjson6902.Filter{ - Patch: p.JsonOp, - }, obj) - } + return filtersutil.ApplyToJSON(patchjson6902.Filter{ + Patch: p.JsonOp, + }, obj) } diff --git a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go index c6b487ad0..4f61f031d 100644 --- a/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go +++ b/plugin/builtin/patchstrategicmergetransformer/PatchStrategicMergeTransformer.go @@ -8,6 +8,7 @@ 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" @@ -21,8 +22,6 @@ type plugin struct { loadedPatches []*resource.Resource Paths []types.PatchStrategicMerge `json:"paths,omitempty" yaml:"paths,omitempty"` Patches string `json:"patches,omitempty" yaml:"patches,omitempty"` - - YAMLSupport bool `json:"yamlSupport,omitempty" yaml:"yamlSupport,omitempty"` } //noinspection GoUnusedGlobalVariable @@ -35,11 +34,6 @@ func (p *plugin) Config( if err != nil { return err } - if !strings.Contains(string(c), "yamlSupport") { - // If not explicitly denied, - // activate kyaml-based transformation. - p.YAMLSupport = true - } if len(p.Paths) == 0 && p.Patches == "" { return fmt.Errorf("empty file path and empty patch content") } @@ -83,24 +77,34 @@ func (p *plugin) Transform(m resmap.ResMap) error { if err != nil { return err } - if !p.YAMLSupport { - err = target.Patch(patch.Copy()) - } else { - patchCopy := patch.DeepCopy() - patchCopy.SetName(target.GetName()) - patchCopy.SetNamespace(target.GetNamespace()) - patchCopy.SetGvk(target.GetGvk()) - node, err := filtersutil.GetRNode(patchCopy) - if err != nil { - return err - } - err = filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ - Patch: node, - }, target) - } + patchCopy := patch.DeepCopy() + patchCopy.SetName(target.GetName()) + patchCopy.SetNamespace(target.GetNamespace()) + patchCopy.SetGvk(target.GetGvk()) + node, err := filtersutil.GetRNode(patchCopy) if 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 len(target.Map()) != 0 { + return errors.Wrapf( + err, "with unexpectedly non-empty object map of size %d", + len(target.Map())) + } + // Fall through to handle deleted object. + } if len(target.Map()) == 0 { // This means all fields have been removed from the object. // This can happen if a patch required deletion of the diff --git a/plugin/builtin/patchstrategicmergetransformer/go.mod b/plugin/builtin/patchstrategicmergetransformer/go.mod index 0ebc81b05..b9c7ec3b2 100644 --- a/plugin/builtin/patchstrategicmergetransformer/go.mod +++ b/plugin/builtin/patchstrategicmergetransformer/go.mod @@ -3,6 +3,7 @@ module sigs.k8s.io/kustomize/plugin/builtin/patchstrategicmergetransformer go 1.14 require ( + github.com/pkg/errors v0.8.1 sigs.k8s.io/kustomize/api v0.5.0 sigs.k8s.io/kustomize/kyaml v0.3.4 sigs.k8s.io/yaml v1.2.0 diff --git a/plugin/builtin/patchtransformer/PatchTransformer.go b/plugin/builtin/patchtransformer/PatchTransformer.go index 3fb225e68..c2e917492 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer.go +++ b/plugin/builtin/patchtransformer/PatchTransformer.go @@ -9,7 +9,6 @@ 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" @@ -25,8 +24,6 @@ type plugin struct { Path string `json:"path,omitempty" yaml:"path,omitempty"` Patch string `json:"patch,omitempty" yaml:"patch,omitempty"` Target *types.Selector `json:"target,omitempty" yaml:"target,omitempty"` - - YAMLSupport bool `json:"yamlSupport,omitempty" yaml:"yamlSupport,omitempty"` } //noinspection GoUnusedGlobalVariable @@ -38,11 +35,6 @@ func (p *plugin) Config( if err != nil { return err } - if !strings.Contains(string(c), "yamlSupport") { - // If not explicitly denied, - // activate kyaml-based transformation. - p.YAMLSupport = true - } p.Patch = strings.TrimSpace(p.Patch) if p.Patch == "" && p.Path == "" { return fmt.Errorf( @@ -82,11 +74,11 @@ func (p *plugin) Config( } func (p *plugin) Transform(m resmap.ResMap) error { - if p.loadedPatch != nil { + if p.loadedPatch == nil { + return p.transformJson6902(m, p.decodedPatch) + } else { // The patch was a strategic merge patch return p.transformStrategicMerge(m, p.loadedPatch) - } else { - return p.transformJson6902(m, p.decodedPatch) } } @@ -120,20 +112,15 @@ func (p *plugin) transformStrategicMerge(m resmap.ResMap, patch *resource.Resour } // applySMPatch applies the provided strategic merge patch to the -// given resource. Depending on the value of YAMLSupport, it will either -// use the legacy implementation or the kyaml-based solution. +// given resource. func (p *plugin) applySMPatch(resource, patch *resource.Resource) error { - if !p.YAMLSupport { - return resource.Patch(patch.Copy()) - } else { - node, err := filtersutil.GetRNode(patch) - if err != nil { - return err - } - return filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ - Patch: node, - }, resource) + node, err := filtersutil.GetRNode(patch) + if err != nil { + return err } + return filtersutil.ApplyToJSON(patchstrategicmerge.Filter{ + Patch: node, + }, resource) } // transformJson6902 applies the provided json6902 patch @@ -142,13 +129,14 @@ func (p *plugin) transformJson6902(m resmap.ResMap, patch jsonpatch.Patch) error if p.Target == nil { return fmt.Errorf("must specify a target for patch %s", p.Patch) } - resources, err := m.Select(*p.Target) if err != nil { return err } for _, res := range resources { - err = p.applyJson6902Patch(res, patch) + err = filtersutil.ApplyToJSON(patchjson6902.Filter{ + Patch: p.Patch, + }, res) if err != nil { return err } @@ -156,28 +144,6 @@ func (p *plugin) transformJson6902(m resmap.ResMap, patch jsonpatch.Patch) error return nil } -// applyJson6902Patch applies the provided patch to the given resource. -// Depending on the value of YAMLSupport, it will either -// use the legacy implementation or the kyaml-based solution. -func (p *plugin) applyJson6902Patch(resource *resource.Resource, patch jsonpatch.Patch) error { - if !p.YAMLSupport { - rawObj, err := resource.MarshalJSON() - if err != nil { - return err - } - modifiedObj, err := patch.Apply(rawObj) - if err != nil { - return errors.Wrapf( - err, "failed to apply json patch '%s'", p.Patch) - } - return resource.UnmarshalJSON(modifiedObj) - } else { - return filtersutil.ApplyToJSON(patchjson6902.Filter{ - Patch: p.Patch, - }, resource) - } -} - // jsonPatchFromBytes loads a Json 6902 patch from // a bytes input func jsonPatchFromBytes( diff --git a/plugin/builtin/patchtransformer/PatchTransformer_test.go b/plugin/builtin/patchtransformer/PatchTransformer_test.go index 7373f6437..eb287a991 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer_test.go +++ b/plugin/builtin/patchtransformer/PatchTransformer_test.go @@ -4,7 +4,6 @@ package main_test import ( - "fmt" "strings" "testing" @@ -278,7 +277,11 @@ spec: } func TestPatchTransformerSmpSidecars(t *testing.T) { - patch := ` + th := kusttest_test.MakeEnhancedHarness(t). + PrepBuiltin("PatchTransformer") + defer th.Reset() + + th.WriteF("patch.yaml", ` apiVersion: apps/v1 kind: Deployment metadata: @@ -287,100 +290,23 @@ spec: template: spec: containers: - - name: istio-proxy - image: docker.io/istio/proxyv2 - args: - - proxy - - sidecar -` + - name: istio-proxy + image: docker.io/istio/proxyv2 + args: + - proxy + - sidecar +`) - config := ` + rm := th.LoadAndRunTransformer(` apiVersion: builtin kind: PatchTransformer metadata: name: notImportantHere -yamlSupport: %t path: patch.yaml target: name: myDeploy -` - - // The expected results with and without yamlSupport is - // slightly different for this test. This is because - // the two different implementations order the results - // differently. - testCases := []struct { - testName string - yamlSupport bool - expectedOutput string - }{ - { - testName: "yaml=false", - yamlSupport: false, - expectedOutput: ` -apiVersion: apps/v1 -kind: Deployment -metadata: - labels: - old-label: old-value - name: myDeploy -spec: - replica: 2 - template: - metadata: - labels: - old-label: old-value - spec: - containers: - - args: - - proxy - - sidecar - image: docker.io/istio/proxyv2 - name: istio-proxy - - image: nginx - name: nginx ---- -apiVersion: apps/v1 -kind: Deployment -metadata: - labels: - new-label: new-value - name: yourDeploy -spec: - replica: 1 - 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: - template: - metadata: - labels: - old-label: old-value - spec: - containers: - - args: - - proxy - - sidecar - image: docker.io/istio/proxyv2 - name: istio-proxy -`, - }, - { - testName: "yaml=true", - yamlSupport: true, - expectedOutput: ` +`, someDeploymentResources) + th.AssertActualEqualsExpected(rm, ` apiVersion: apps/v1 kind: Deployment metadata: @@ -438,24 +364,7 @@ spec: - sidecar image: docker.io/istio/proxyv2 name: istio-proxy -`, - }, - } - - for i := range testCases { - tc := testCases[i] - t.Run(tc.testName, func(t *testing.T) { - th := kusttest_test.MakeEnhancedHarness(t). - PrepBuiltin("PatchTransformer") - defer th.Reset() - - th.WriteF("patch.yaml", patch) - - c := fmt.Sprintf(config, tc.yamlSupport) - rm := th.LoadAndRunTransformer(c, someDeploymentResources) - th.AssertActualEqualsExpected(rm, tc.expectedOutput) - }) - } +`) } func TestPatchTransformerWithInlineJson(t *testing.T) { diff --git a/plugin/builtin/patchtransformer/go.mod b/plugin/builtin/patchtransformer/go.mod index 4cf93d86d..4d8fb37bb 100644 --- a/plugin/builtin/patchtransformer/go.mod +++ b/plugin/builtin/patchtransformer/go.mod @@ -4,7 +4,6 @@ go 1.14 require ( github.com/evanphx/json-patch v4.5.0+incompatible - github.com/pkg/errors v0.8.1 sigs.k8s.io/kustomize/api v0.5.0 sigs.k8s.io/kustomize/kyaml v0.3.4 sigs.k8s.io/yaml v1.2.0