diff --git a/api/builtins/PatchTransformer.go b/api/builtins/PatchTransformer.go index a5a074877..a8421316a 100644 --- a/api/builtins/PatchTransformer.go +++ b/api/builtins/PatchTransformer.go @@ -5,6 +5,7 @@ package builtins import ( "fmt" + "strings" jsonpatch "github.com/evanphx/json-patch" "github.com/pkg/errors" @@ -28,20 +29,19 @@ type PatchTransformerPlugin struct { } func (p *PatchTransformerPlugin) Config( - h *resmap.PluginHelpers, c []byte) (err error) { - err = yaml.Unmarshal(c, p) + h *resmap.PluginHelpers, c []byte) error { + err := yaml.Unmarshal(c, p) if err != nil { return err } + p.Patch = strings.TrimSpace(p.Patch) if p.Patch == "" && p.Path == "" { - err = fmt.Errorf( + return fmt.Errorf( "must specify one of patch and path in\n%s", string(c)) - return } if p.Patch != "" && p.Path != "" { - err = fmt.Errorf( + return fmt.Errorf( "patch and path can't be set at the same time\n%s", string(c)) - return } if p.Path != "" { @@ -54,22 +54,21 @@ func (p *PatchTransformerPlugin) Config( patchSM, errSM := h.ResmapFactory().RF().FromBytes([]byte(p.Patch)) patchJson, errJson := jsonPatchFromBytes([]byte(p.Patch)) + if (errSM == nil && errJson == nil) || + (patchSM != nil && patchJson != nil) { + return fmt.Errorf( + "illegally qualifies as both an SM and JSON patch: [%v]", + p.Patch) + } if errSM != nil && errJson != nil { - err = fmt.Errorf( - "unable to get either a Strategic Merge Patch or JSON patch 6902 from %s", p.Patch) - return + return fmt.Errorf( + "unable to parse SM or JSON patch from [%v]", p.Patch) } - if errSM == nil && errJson != nil { + if errSM == nil { p.loadedPatch = patchSM - } - if errJson == nil && errSM != nil { + } else { p.decodedPatch = patchJson } - if patchSM != nil && patchJson != nil { - err = fmt.Errorf( - "a patch can't be both a Strategic Merge Patch and JSON patch 6902 %s", p.Patch) - } - return nil } diff --git a/plugin/builtin/patchtransformer/PatchTransformer.go b/plugin/builtin/patchtransformer/PatchTransformer.go index 55337e548..daa4ecc7d 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer.go +++ b/plugin/builtin/patchtransformer/PatchTransformer.go @@ -6,6 +6,7 @@ package main import ( "fmt" + "strings" jsonpatch "github.com/evanphx/json-patch" "github.com/pkg/errors" @@ -32,20 +33,19 @@ type plugin struct { var KustomizePlugin plugin func (p *plugin) Config( - h *resmap.PluginHelpers, c []byte) (err error) { - err = yaml.Unmarshal(c, p) + h *resmap.PluginHelpers, c []byte) error { + err := yaml.Unmarshal(c, p) if err != nil { return err } + p.Patch = strings.TrimSpace(p.Patch) if p.Patch == "" && p.Path == "" { - err = fmt.Errorf( + return fmt.Errorf( "must specify one of patch and path in\n%s", string(c)) - return } if p.Patch != "" && p.Path != "" { - err = fmt.Errorf( + return fmt.Errorf( "patch and path can't be set at the same time\n%s", string(c)) - return } if p.Path != "" { @@ -58,22 +58,21 @@ func (p *plugin) Config( patchSM, errSM := h.ResmapFactory().RF().FromBytes([]byte(p.Patch)) patchJson, errJson := jsonPatchFromBytes([]byte(p.Patch)) + if (errSM == nil && errJson == nil) || + (patchSM != nil && patchJson != nil) { + return fmt.Errorf( + "illegally qualifies as both an SM and JSON patch: [%v]", + p.Patch) + } if errSM != nil && errJson != nil { - err = fmt.Errorf( - "unable to get either a Strategic Merge Patch or JSON patch 6902 from %s", p.Patch) - return + return fmt.Errorf( + "unable to parse SM or JSON patch from [%v]", p.Patch) } - if errSM == nil && errJson != nil { + if errSM == nil { p.loadedPatch = patchSM - } - if errJson == nil && errSM != nil { + } else { p.decodedPatch = patchJson } - if patchSM != nil && patchJson != nil { - err = fmt.Errorf( - "a patch can't be both a Strategic Merge Patch and JSON patch 6902 %s", p.Patch) - } - return nil } diff --git a/plugin/builtin/patchtransformer/PatchTransformer_test.go b/plugin/builtin/patchtransformer/PatchTransformer_test.go index 2c24e32f7..173a50881 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer_test.go +++ b/plugin/builtin/patchtransformer/PatchTransformer_test.go @@ -12,7 +12,7 @@ import ( ) const ( - target = ` + someDeploymentResources = ` apiVersion: apps/v1 metadata: name: myDeploy @@ -76,7 +76,7 @@ kind: PatchTransformer metadata: name: notImportantHere path: patch.yaml -`, target, func(t *testing.T, err error) { +`, someDeploymentResources, func(t *testing.T, err error) { if err == nil { t.Fatalf("expected error") } @@ -98,12 +98,13 @@ kind: PatchTransformer metadata: name: notImportantHere patch: "thisIsNotAPatch" -`, target, func(t *testing.T, err error) { +`, someDeploymentResources, func(t *testing.T, err error) { if err == nil { t.Fatalf("expected error") } + fmt.Print(err) if !strings.Contains(err.Error(), - "unable to get either a Strategic Merge Patch or JSON patch 6902 from") { + "unable to parse SM or JSON patch from ") { t.Fatalf("unexpected err: %v", err) } }) @@ -120,7 +121,7 @@ kind: PatchTransformer metadata: name: notImportantHere patch: '[{"op": "add", "path": "/spec/template/spec/dnsPolicy", "value": "ClusterFirst"}]' -`, target, func(t *testing.T, err error) { +`, someDeploymentResources, func(t *testing.T, err error) { if err == nil { t.Fatalf("expected error") } @@ -131,6 +132,30 @@ patch: '[{"op": "add", "path": "/spec/template/spec/dnsPolicy", "value": "Cluste }) } +func TestPatchTransformerBlankPatch(t *testing.T) { + th := kusttest_test.MakeEnhancedHarness(t). + PrepBuiltin("PatchTransformer") + defer th.Reset() + + th.RunTransformerAndCheckError(` +apiVersion: builtin +kind: PatchTransformer +metadata: + name: notImportantHere +patch: " " +target: + name: .*Deploy +`, someDeploymentResources, func(t *testing.T, err error) { + if err == nil { + t.Fatalf("expected error") + } + if !strings.Contains(err.Error(), + "must specify one of patch and path in") { + t.Fatalf("unexpected err: %v", err) + } + }) +} + func TestPatchTransformerBothEmptyPathAndPatch(t *testing.T) { th := kusttest_test.MakeEnhancedHarness(t). PrepBuiltin("PatchTransformer") @@ -141,11 +166,12 @@ apiVersion: builtin kind: PatchTransformer metadata: name: notImportantHere -`, target, func(t *testing.T, err error) { +`, someDeploymentResources, func(t *testing.T, err error) { if err == nil { t.Fatalf("expected error") } - if !strings.Contains(err.Error(), "must specify one of patch and path in") { + if !strings.Contains(err.Error(), + "must specify one of patch and path in") { t.Fatalf("unexpected err: %v", err) } }) @@ -163,11 +189,12 @@ metadata: name: notImportantHere Path: patch.yaml Patch: "something" -`, target, func(t *testing.T, err error) { +`, someDeploymentResources, func(t *testing.T, err error) { if err == nil { t.Fatalf("expected error") } - if !strings.Contains(err.Error(), "patch and path can't be set at the same time") { + if !strings.Contains(err.Error(), + "patch and path can't be set at the same time") { t.Fatalf("unexpected err: %v", err) } }) @@ -196,7 +223,7 @@ path: patch.yaml target: name: .*Deploy `, - target, + someDeploymentResources, ` apiVersion: apps/v1 kind: Deployment @@ -426,7 +453,7 @@ spec: th.WriteF("patch.yaml", patch) c := fmt.Sprintf(config, tc.yamlSupport) - rm := th.LoadAndRunTransformer(c, target) + rm := th.LoadAndRunTransformer(c, someDeploymentResources) th.AssertActualEqualsExpected(rm, tc.expectedOutput) }) } @@ -446,7 +473,7 @@ patch: '[{"op": "replace", "path": "/spec/template/spec/containers/0/image", "va target: name: .*Deploy kind: Deployment -`, target, +`, someDeploymentResources, ` apiVersion: apps/v1 kind: Deployment @@ -520,7 +547,7 @@ patch: |- kind: Deployment spec: replica: 77 -`, target, ` +`, someDeploymentResources, ` apiVersion: apps/v1 kind: Deployment metadata: @@ -573,7 +600,7 @@ spec: `) } -const ingressTarget = `apiVersion: networking.k8s.io/v1beta1 +const anIngressResource = `apiVersion: networking.k8s.io/v1beta1 kind: Ingress metadata: name: my-ingress @@ -623,7 +650,7 @@ metadata: path: patch.json target: kind: Ingress -`, ingressTarget, ` +`, anIngressResource, ` apiVersion: networking.k8s.io/v1beta1 kind: Ingress metadata: @@ -673,7 +700,7 @@ metadata: path: patch.yaml target: kind: Ingress -`, ingressTarget, ` +`, anIngressResource, ` apiVersion: networking.k8s.io/v1beta1 kind: Ingress metadata: