diff --git a/api/internal/builtins/PatchJson6902Transformer.go b/api/internal/builtins/PatchJson6902Transformer.go index 04625e5109..a5c0fbad8f 100644 --- a/api/internal/builtins/PatchJson6902Transformer.go +++ b/api/internal/builtins/PatchJson6902Transformer.go @@ -78,6 +78,11 @@ func (p *PatchJson6902TransformerPlugin) Transform(m resmap.ResMap) error { if err != nil { return err } + + if len(resources) == 0 { + return fmt.Errorf("patchesJson6902 target not found for %s", p.Target.ResId) + } + for _, res := range resources { internalAnnotations := kioutil.GetInternalAnnotations(&res.RNode) diff --git a/api/internal/builtins/PatchTransformer.go b/api/internal/builtins/PatchTransformer.go index 6161ada864..08481273b8 100644 --- a/api/internal/builtins/PatchTransformer.go +++ b/api/internal/builtins/PatchTransformer.go @@ -5,6 +5,7 @@ package builtins import ( "fmt" + "os" "strings" jsonpatch "gopkg.in/evanphx/json-patch.v4" @@ -69,18 +70,7 @@ func (p *PatchTransformerPlugin) Config(h *resmap.PluginHelpers, c []byte) error } if errSM == nil { p.smPatches = patchesSM - for _, loadedPatch := range p.smPatches { - if p.Options == nil { - continue - } - - if p.Options.AllowNameChange { - loadedPatch.AllowNameChange() - } - if p.Options.AllowKindChange { - loadedPatch.AllowKindChange() - } - } + p.loadPatchOptions(p.smPatches) } else { p.jsonPatches = patchesJson } @@ -110,6 +100,9 @@ func (p *PatchTransformerPlugin) transformStrategicMerge(m resmap.ResMap) error if err != nil { return fmt.Errorf("unable to find patch target %q in `resources`: %w", p.Target, err) } + if err := p.allowNoTargetMatch(selected); err != nil { + return err + } return errors.Wrap(m.ApplySmPatch(resource.MakeIdSet(selected), patch)) } @@ -134,6 +127,11 @@ func (p *PatchTransformerPlugin) transformJson6902(m resmap.ResMap) error { if err != nil { return err } + + if err := p.allowNoTargetMatch(resources); err != nil { + return err + } + for _, res := range resources { res.StorePreviousId() internalAnnotations := kioutil.GetInternalAnnotations(&res.RNode) @@ -173,6 +171,36 @@ func jsonPatchFromBytes(in []byte) (jsonpatch.Patch, error) { return jsonpatch.DecodePatch([]byte(ops)) } +// allowNoTargetMatch checks whether no target match is allowed and return an error in case the patch violates that. +func (p *PatchTransformerPlugin) allowNoTargetMatch(resources []*resource.Resource) error { + if len(resources) > 0 { + return nil + } + + if p.Options == nil || !p.Options.AllowNoTargetMatch { + return fmt.Errorf("patches target not found for %s", p.Target.ResId) + } + + fmt.Fprintf(os.Stderr, "# %v %s\n", "Warning: patches target not found for Target", p.Target.ResId) + return nil +} + +// loadPatchOptions, given a list of resources, enables the available patch options for each resource. +func (p *PatchTransformerPlugin) loadPatchOptions(patches []*resource.Resource) { + if p.Options == nil { + return + } + + for _, patch := range patches { + if p.Options.AllowNameChange { + patch.AllowNameChange() + } + if p.Options.AllowKindChange { + patch.AllowKindChange() + } + } +} + func NewPatchTransformerPlugin() resmap.TransformerPlugin { return &PatchTransformerPlugin{} } diff --git a/api/krusty/extendedpatch_test.go b/api/krusty/extendedpatch_test.go index 955082f617..d3b99d3140 100644 --- a/api/krusty/extendedpatch_test.go +++ b/api/krusty/extendedpatch_test.go @@ -6,6 +6,7 @@ package krusty_test import ( "testing" + "github.com/stretchr/testify/assert" kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest" ) @@ -816,6 +817,32 @@ patches: th.WriteF("base/patch.yaml", ` apiVersion: apps/v1 kind: Deployment +metadata: + name: busybox + annotations: + new-key: new-value +`) + err := th.RunWithErr("base", th.MakeDefaultOptions()) + assert.Contains(t, err.Error(), "patches target not found for [noKind].[noVer].[noGrp]/no-match.[noNs]") +} + +func TestExtendedPatchAllowNoMatch(t *testing.T) { + th := kusttest_test.MakeHarness(t) + makeCommonFileForExtendedPatchTest(th) + th.WriteK("base", ` +resources: +- deployment.yaml +- service.yaml +patches: +- path: patch.yaml + target: + name: no-match + options: + allowNoTargetMatch: true +`) + th.WriteF("base/patch.yaml", ` +apiVersion: apps/v1 +kind: Deployment metadata: name: busybox annotations: @@ -1016,6 +1043,38 @@ patches: th.WriteF("base/patch.yaml", ` apiVersion: apps/v1 kind: Deployment +metadata: + name: busybox + annotations: + new-key: new-value +`) + err := th.RunWithErr("base", th.MakeDefaultOptions()) + assert.Contains(t, err.Error(), "patches target not found for [noKind].[noVer].[noGrp]/no-match.[noNs]") +} + +func TestExtendedPatchAllowNoMatchMultiplePatch(t *testing.T) { + th := kusttest_test.MakeHarness(t) + makeCommonFileForExtendedPatchTest(th) + th.WriteK("base", ` +resources: +- deployment.yaml +- service.yaml +patches: +- path: patch.yaml + target: + name: no-match + options: + allowNoTargetMatch: true +- path: patch.yaml + target: + name: busybox + kind: Job + options: + allowNoTargetMatch: true +`) + th.WriteF("base/patch.yaml", ` +apiVersion: apps/v1 +kind: Deployment metadata: name: busybox annotations: @@ -1213,3 +1272,95 @@ spec: app: busybox `) } + +func TestTargetMissingPatchJson6902Error(t *testing.T) { + th := kusttest_test.MakeHarness(t) + makeCommonFileForExtendedPatchTest(th) + th.WriteK("base", ` +resources: +- service.yaml +patchesJson6902: +- target: + kind: Service + name: busybox + version: v2 + path: patch.yaml +`) + th.WriteF("base/patch.yaml", ` +- op: add + path: /metadata/labels/release + value: this-label-will-not-go-through +`) + err := th.RunWithErr("base", th.MakeDefaultOptions()) + assert.Contains(t, err.Error(), "patchesJson6902 target not found for Service.v2.[noGrp]/busybox.[noNs]") +} + +func TestTargetMissingJsonPatchError(t *testing.T) { + th := kusttest_test.MakeHarness(t) + makeCommonFileForExtendedPatchTest(th) + th.WriteK("base", ` +resources: +- service.yaml +patches: +- target: + kind: Service + name: busybox + version: v2 + path: patch.yaml +`) + th.WriteF("base/patch.yaml", ` +- op: add + path: /metadata/labels/release + value: this-label-will-not-go-through +`) + err := th.RunWithErr("base", th.MakeDefaultOptions()) + assert.Contains(t, err.Error(), "patches target not found for Service.v2.[noGrp]/busybox.[noNs]") +} + +func TestAllowNoTargetMatchPatchTransformerOptions(t *testing.T) { + th := kusttest_test.MakeHarness(t) + makeCommonFileForExtendedPatchTest(th) + th.WriteK("base", ` +resources: +- service.yaml +patches: +- target: + kind: Service + name: busybox + version: v2 + path: patch.yaml + options: + allowNoTargetMatch: true +`) + th.WriteF("base/patch.yaml", ` +- op: add + path: /metadata/labels/release + value: this-label-will-not-go-through +`) + m := th.Run("base", th.MakeDefaultOptions()) + th.AssertActualEqualsExpected(m, ` +apiVersion: v1 +kind: Service +metadata: + labels: + app: nginx + name: nginx +spec: + ports: + - port: 80 + selector: + app: nginx +--- +apiVersion: v1 +kind: Service +metadata: + labels: + app: busybox + name: busybox +spec: + ports: + - port: 8080 + selector: + app: busybox +`) +} diff --git a/api/types/patch_test.go b/api/types/patch_test.go index fbf2af0423..b2ab1adae6 100644 --- a/api/types/patch_test.go +++ b/api/types/patch_test.go @@ -108,8 +108,9 @@ func TestPatchEquals(t *testing.T) { Patch: "bar", Target: &selector, Options: &PatchArgs{ - AllowNameChange: true, - AllowKindChange: true, + AllowNameChange: true, + AllowKindChange: true, + AllowNoTargetMatch: false, }, }, patch2: Patch{ @@ -117,8 +118,9 @@ func TestPatchEquals(t *testing.T) { Patch: "bar", Target: &selector, Options: &PatchArgs{ - AllowNameChange: true, - AllowKindChange: true, + AllowNameChange: true, + AllowKindChange: true, + AllowNoTargetMatch: false, }, }, expect: true, @@ -153,7 +155,6 @@ func TestPatchEquals(t *testing.T) { Patch: "bar", Target: &selector, Options: &PatchArgs{ - AllowNameChange: false, AllowKindChange: true, }, }, @@ -162,8 +163,8 @@ func TestPatchEquals(t *testing.T) { Patch: "bar", Target: &selector, Options: &PatchArgs{ - AllowNameChange: true, - AllowKindChange: true, + AllowKindChange: true, + AllowNoTargetMatch: true, }, }, expect: false, diff --git a/api/types/patchargs.go b/api/types/patchargs.go index 453849fa6b..126d2cb3a3 100644 --- a/api/types/patchargs.go +++ b/api/types/patchargs.go @@ -10,4 +10,7 @@ type PatchArgs struct { // AllowKindChange allows kind changes to the resource. AllowKindChange bool `json:"allowKindChange,omitempty" yaml:"allowKindChange,omitempty"` + + // AllowNoTargetMatch allows files rendering in case of no target (`api/types/selector`) match. + AllowNoTargetMatch bool `json:"allowNoTargetMatch,omitempty" yaml:"allowNoTargetMatch,omitempty"` } diff --git a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go index 285f1796c1..c6c6e4943f 100644 --- a/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go +++ b/plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go @@ -81,6 +81,11 @@ func (p *plugin) Transform(m resmap.ResMap) error { if err != nil { return err } + + if len(resources) == 0 { + return fmt.Errorf("patchesJson6902 target not found for %s", p.Target.ResId) + } + for _, res := range resources { internalAnnotations := kioutil.GetInternalAnnotations(&res.RNode) diff --git a/plugin/builtin/patchtransformer/PatchTransformer.go b/plugin/builtin/patchtransformer/PatchTransformer.go index 77e5439b09..28e9338eea 100644 --- a/plugin/builtin/patchtransformer/PatchTransformer.go +++ b/plugin/builtin/patchtransformer/PatchTransformer.go @@ -6,6 +6,7 @@ package main import ( "fmt" + "os" "strings" jsonpatch "gopkg.in/evanphx/json-patch.v4" @@ -72,18 +73,7 @@ func (p *plugin) Config(h *resmap.PluginHelpers, c []byte) error { } if errSM == nil { p.smPatches = patchesSM - for _, loadedPatch := range p.smPatches { - if p.Options == nil { - continue - } - - if p.Options.AllowNameChange { - loadedPatch.AllowNameChange() - } - if p.Options.AllowKindChange { - loadedPatch.AllowKindChange() - } - } + p.loadPatchOptions(p.smPatches) } else { p.jsonPatches = patchesJson } @@ -113,6 +103,9 @@ func (p *plugin) transformStrategicMerge(m resmap.ResMap) error { if err != nil { return fmt.Errorf("unable to find patch target %q in `resources`: %w", p.Target, err) } + if err := p.allowNoTargetMatch(selected); err != nil { + return err + } return errors.Wrap(m.ApplySmPatch(resource.MakeIdSet(selected), patch)) } @@ -137,6 +130,11 @@ func (p *plugin) transformJson6902(m resmap.ResMap) error { if err != nil { return err } + + if err := p.allowNoTargetMatch(resources); err != nil { + return err + } + for _, res := range resources { res.StorePreviousId() internalAnnotations := kioutil.GetInternalAnnotations(&res.RNode) @@ -175,3 +173,33 @@ func jsonPatchFromBytes(in []byte) (jsonpatch.Patch, error) { } return jsonpatch.DecodePatch([]byte(ops)) } + +// allowNoTargetMatch checks whether no target match is allowed and return an error in case the patch violates that. +func (p *plugin) allowNoTargetMatch(resources []*resource.Resource) error { + if len(resources) > 0 { + return nil + } + + if p.Options == nil || !p.Options.AllowNoTargetMatch { + return fmt.Errorf("patches target not found for %s", p.Target.ResId) + } + + fmt.Fprintf(os.Stderr, "# %v %s\n", "Warning: patches target not found for Target", p.Target.ResId) + return nil +} + +// loadPatchOptions, given a list of resources, enables the available patch options for each resource. +func (p *plugin) loadPatchOptions(patches []*resource.Resource) { + if p.Options == nil { + return + } + + for _, patch := range patches { + if p.Options.AllowNameChange { + patch.AllowNameChange() + } + if p.Options.AllowKindChange { + patch.AllowKindChange() + } + } +}