Skip to content
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

Fix reject needing both current and previous ids #5263

Merged
merged 2 commits into from
Aug 14, 2023
Merged

Fix reject needing both current and previous ids #5263

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9756d92
test: demonstrate reject not working on targets with changed id
bugoverdose Aug 11, 2023
9f1d5ac
fix: check both prev and current ids for filtering reject targets
bugoverdose Aug 11, 2023
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
15 changes: 10 additions & 5 deletions api/filters/replacement/replacement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targetSelectors []
}

// filter targets by matching resource IDs
for i, id := range ids {
if id.IsSelectedBy(selector.Select.ResId) && !rejectId(selector.Reject, &ids[i]) {
for _, id := range ids {
if id.IsSelectedBy(selector.Select.ResId) && !containsRejectId(selector.Reject, ids) {
err := copyValueToTarget(possibleTarget, value, selector)
if err != nil {
return nil, err
Expand Down Expand Up @@ -168,10 +168,15 @@ func matchesAnnoAndLabelSelector(n *yaml.RNode, selector *types.Selector) (bool,
return annoMatch && labelMatch, nil
}

func rejectId(rejects []*types.Selector, id *resid.ResId) bool {
func containsRejectId(rejects []*types.Selector, ids []resid.ResId) bool {
for _, r := range rejects {
if !r.ResId.IsEmpty() && id.IsSelectedBy(r.ResId) {
return true
if r.ResId.IsEmpty() {
continue
}
for _, id := range ids {
if id.IsSelectedBy(r.ResId) {
return true
}
}
}
return false
Expand Down
68 changes: 68 additions & 0 deletions api/krusty/replacementtransformer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -546,3 +546,71 @@ metadata:
name: red-dc6gc5btkc
`)
}

func TestReplacementTransformerWithSuffixTransformerAndReject(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Test looks good! Thanks for adding this.

th := kusttest_test.MakeEnhancedHarness(t)
defer th.Reset()

th.WriteF("base/app.yaml", `
apiVersion: apps/v1
kind: Deployment
metadata:
name: original-name
spec:
template:
spec:
containers:
- image: app1:1.0
name: app
`)
th.WriteK("base", `
resources:
- app.yaml
`)
th.WriteK("overlay", `
apiVersion: kustomize.config.k8s.io/v1beta1
kind: Kustomization

nameSuffix: -dev
resources:
- ../base

configMapGenerator:
- name: app-config
literals:
- name=something-else

replacements:
- source:
kind: ConfigMap
name: app-config
fieldPath: data.name
targets:
- fieldPaths:
- spec.template.spec.containers.0.name
select:
kind: Deployment
reject:
- name: original-name
`)
m := th.Run("overlay", th.MakeDefaultOptions())
th.AssertActualEqualsExpected(m, `
apiVersion: apps/v1
kind: Deployment
metadata:
name: original-name-dev
spec:
template:
spec:
containers:
- image: app1:1.0
name: app
---
apiVersion: v1
data:
name: something-else
kind: ConfigMap
metadata:
name: app-config-dev-97544dk6t8
`)
}