Skip to content

feat: Detect no matching resources #5917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

feat: Detect no matching resources #5917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4d61b8
rebase
laxmikantbpandhare Dec 22, 2022
2f127dd
updated error message according to new discussion
laxmikantbpandhare Jul 23, 2023
f329f91
feat: allowNoTargetMatch feature for PatchTransformer and Path6902Tra…
adoramshoval Aug 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions api/internal/builtins/PatchJson6902Transformer.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

52 changes: 40 additions & 12 deletions api/internal/builtins/PatchTransformer.go
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

151 changes: 151 additions & 0 deletions api/krusty/extendedpatch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package krusty_test
import (
"testing"

"github.com/stretchr/testify/assert"
kusttest_test "sigs.k8s.io/kustomize/api/testutils/kusttest"
)

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
`)
}
15 changes: 8 additions & 7 deletions api/types/patch_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,19 @@ func TestPatchEquals(t *testing.T) {
Patch: "bar",
Target: &selector,
Options: &PatchArgs{
AllowNameChange: true,
AllowKindChange: true,
AllowNameChange: true,
AllowKindChange: true,
AllowNoTargetMatch: false,
},
},
patch2: Patch{
Path: "foo",
Patch: "bar",
Target: &selector,
Options: &PatchArgs{
AllowNameChange: true,
AllowKindChange: true,
AllowNameChange: true,
AllowKindChange: true,
AllowNoTargetMatch: false,
},
},
expect: true,
Expand Down Expand Up @@ -153,7 +155,6 @@ func TestPatchEquals(t *testing.T) {
Patch: "bar",
Target: &selector,
Options: &PatchArgs{
AllowNameChange: false,
AllowKindChange: true,
},
},
Expand All @@ -162,8 +163,8 @@ func TestPatchEquals(t *testing.T) {
Patch: "bar",
Target: &selector,
Options: &PatchArgs{
AllowNameChange: true,
AllowKindChange: true,
AllowKindChange: true,
AllowNoTargetMatch: true,
Copy link
Member

@stormqueen1990 stormqueen1990 Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is changing an existing test. Perhaps it would be better to keep it as-is, making only the necessary changes to pass with the current API changes, and add a new test to cover the new feature?

I'm also a bit confused about the removal of line 156.

Copy link
Contributor Author

@adoramshoval adoramshoval Aug 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh my bad, I thought it wouldn't make a difference if I change a bit the test.
I'll revert this change.

},
},
expect: false,
Expand Down
3 changes: 3 additions & 0 deletions api/types/patchargs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"`
}
5 changes: 5 additions & 0 deletions plugin/builtin/patchjson6902transformer/PatchJson6902Transformer.go
View file
Open in desktop
Copy link
Member

@stormqueen1990 stormqueen1990 Aug 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we include some tests for this change?

Copy link
Contributor Author

@adoramshoval adoramshoval Aug 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added a test for the change here: https://github.com/kubernetes-sigs/kustomize/pull/5917/files#diff-08aa260c0029be90fe08448a134735f879c3aec100f88c5b7275090c0a84228dR1276

Should it be somewhere else?

Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
Loading
Loading