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

Jump to bottom

Allow setting every array element in replacements #4424

Merged
merged 4 commits into from Feb 10, 2022
Merged

Allow setting every array element in replacements #4424

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
26b9af0
Allow setting every array element in replacements
koba1t Jan 26, 2022
b79d77a
add replacements every element match test
koba1t Jan 30, 2022
22f9daa
replacements allow to replace multi values
koba1t Feb 2, 2022
5ed96a3
rename IsMatchEveryIndex to IsWildcard
koba1t Feb 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
183 changes: 156 additions & 27 deletions api/filters/replacement/replacement_test.go
View file
Open in desktop
Expand Up @@ -42,7 +42,7 @@ spec:
- select:
kind: Deployment
name: deploy
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -95,7 +95,7 @@ spec:
targets:
- select:
kind: Deployment
fieldPaths:
fieldPaths:
- spec.template.spec.containers
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -328,7 +328,7 @@ spec:
- select:
kind: Deployment
name: deploy1
fieldPaths:
fieldPaths:
- spec.template.spec.containers.[name=postgresdb].image
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -405,7 +405,7 @@ spec:
targets:
- select:
version: v3
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: my-group-1/v1
Expand Down Expand Up @@ -492,7 +492,7 @@ spec:
targets:
- select:
name: my-name-2
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `spec:
Expand Down Expand Up @@ -582,7 +582,7 @@ spec:
reject:
- name: deploy2
- name: deploy3
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -662,7 +662,7 @@ spec:
reject:
- kind: Deployment
name: my-name
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -731,7 +731,7 @@ spec:
reject:
- kind: Deployment
- name: my-name
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -799,7 +799,7 @@ spec:
- select:
kind: Deployment
name: deploy1
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
options:
delimiter: ':'
Expand Down Expand Up @@ -872,7 +872,7 @@ spec:
- select:
kind: Pod
name: pod2
fieldPaths:
fieldPaths:
- spec.volumes.0.projected.sources.0.configMap.items.0.path
options:
delimiter: '/'
Expand Down Expand Up @@ -948,7 +948,7 @@ spec:
- select:
kind: Pod
name: pod1
fieldPaths:
fieldPaths:
- spec.volumes.0.projected.sources.0.configMap.items.0.path
options:
delimiter: '/'
Expand Down Expand Up @@ -1024,7 +1024,7 @@ spec:
- select:
kind: Pod
name: pod1
fieldPaths:
fieldPaths:
- spec.volumes.0.projected.sources.0.configMap.items.0.path
options:
delimiter: '/'
Expand Down Expand Up @@ -1100,7 +1100,7 @@ spec:
- select:
kind: Pod
name: pod1
fieldPaths:
fieldPaths:
- spec.volumes.0.projected.sources.0.configMap.items.0.path
options:
delimiter: '/'
Expand Down Expand Up @@ -1176,7 +1176,7 @@ spec:
- select:
kind: Pod
name: pod1
fieldPaths:
fieldPaths:
- spec.volumes.0.projected.sources.0.configMap.items.0.path
options:
delimiter: '/'
Expand Down Expand Up @@ -1212,7 +1212,7 @@ metadata:
targets:
- select:
name: deploy1
fieldPaths:
fieldPaths:
- spec.template.spec.containers
options:
create: true
Expand All @@ -1223,7 +1223,7 @@ metadata:
targets:
- select:
name: deploy2
fieldPaths:
fieldPaths:
- spec.template.spec.containers
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -1285,12 +1285,12 @@ spec:
kind: Pod
name: pod
fieldPath: spec.containers
options:
options:
delimiter: "/"
targets:
- select:
kind: Deployment
fieldPaths:
fieldPaths:
- spec.template.spec.containers
`,
expectedErr: "delimiter option can only be used with scalar nodes",
Expand Down Expand Up @@ -1331,9 +1331,9 @@ spec:
targets:
- select:
kind: Deployment
fieldPaths:
fieldPaths:
- spec.template.spec.containers
options:
options:
delimiter: "/"
`,
expectedErr: "delimiter option can only be used with scalar nodes",
Expand All @@ -1354,7 +1354,7 @@ metadata:
targets:
- select:
name: custom
fieldPaths:
fieldPaths:
- metadata.annotations.[f.g.h/i-j]
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -1431,6 +1431,136 @@ spec:
name: second
version: latest
property: second`,
},
"index contains '*' character": {
input: `apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: sample-deploy
name: sample-deploy
spec:
replicas: 1
selector:
matchLabels:
app: sample-deploy
template:
metadata:
labels:
app: sample-deploy
spec:
containers:
- image: nginx
name: main
env:
- name: deployment-name
value: XXXXX
`,
replacements: `replacements:
- source:
kind: Deployment
name: sample-deploy
fieldPath: metadata.name
targets:
- select:
kind: Deployment
fieldPaths:
- spec.template.spec.containers.*.env.[name=deployment-name].value
`,
expected: `apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: sample-deploy
name: sample-deploy
spec:
replicas: 1
selector:
matchLabels:
app: sample-deploy
template:
metadata:
labels:
app: sample-deploy
spec:
containers:
- image: nginx
name: main
env:
- name: deployment-name
value: sample-deploy`,
},
"list index contains '*' character": {
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the difference between this test and the above test?

Also, the test seems not to be passing.

Copy link
Member Author

@koba1t koba1t Feb 2, 2022 •
edited

Choose a reason for hiding this comment

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

It tests to replace multi values in a single '*' character.

input: `apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: sample-deploy
name: sample-deploy
spec:
replicas: 1
selector:
matchLabels:
app: sample-deploy
template:
metadata:
labels:
app: sample-deploy
spec:
containers:
- image: nginx
name: main
env:
- name: deployment-name
value: XXXXX
- name: foo
value: bar
- image: nginx
name: sidecar
env:
- name: deployment-name
value: YYYYY
`,
replacements: `replacements:
- source:
kind: Deployment
name: sample-deploy
fieldPath: metadata.name
targets:
- select:
kind: Deployment
fieldPaths:
- spec.template.spec.containers.*.env.[name=deployment-name].value
`,
expected: `apiVersion: apps/v1
kind: Deployment
metadata:
labels:
app: sample-deploy
name: sample-deploy
spec:
replicas: 1
selector:
matchLabels:
app: sample-deploy
template:
metadata:
labels:
app: sample-deploy
spec:
containers:
- image: nginx
name: main
env:
- name: deployment-name
value: sample-deploy
- name: foo
value: bar
- image: nginx
name: sidecar
env:
- name: deployment-name
value: sample-deploy`,
},
"multiple field paths in target": {
input: `apiVersion: v1
Expand Down Expand Up @@ -1513,7 +1643,7 @@ spec:
kind: Deployment
metadata:
name: pre-deploy
annotations:
annotations:
internal.config.kubernetes.io/previousNames: deploy,deploy
internal.config.kubernetes.io/previousKinds: CronJob,Deployment
internal.config.kubernetes.io/previousNamespaces: default,default
Expand All @@ -1535,7 +1665,7 @@ spec:
- select:
kind: Deployment
name: deploy
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
Expand All @@ -1556,7 +1686,6 @@ spec:
name: postgresdb
`,
},

"replacement source.fieldPath does not exist": {
input: `apiVersion: v1
kind: ConfigMap
Expand Down Expand Up @@ -1628,7 +1757,7 @@ spec:
targets:
- select:
annotationSelector: foo=bar-1
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -1702,7 +1831,7 @@ spec:
targets:
- select:
labelSelector: foo=bar-1
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
Expand Down Expand Up @@ -1778,7 +1907,7 @@ spec:
kind: Deployment
reject:
- labelSelector: foo=bar-2
fieldPaths:
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
Expand Down
6 changes: 6 additions & 0 deletions kyaml/yaml/fns.go
View file
Open in desktop
Expand Up @@ -796,6 +796,12 @@ func SplitIndexNameValue(p string) (string, string, error) {
return parts[0], parts[1], nil
}

// IsMatchEveryIndex returns true if p is matching every elements.
// e.g. "*"
func IsMatchEveryIndex(p string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest changing the name here to IsWildcard and moving this up to line 791 to keep the IsXXXX functions together

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree with you. I fix it.

return p == "*"
}

// IncrementFieldIndex increments i to point to the next field name element in
// a slice of Contents.
func IncrementFieldIndex(i int) int {
Expand Down