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

Annotation and label selection in replacement targets #4229

Merged
merged 2 commits into from Nov 4, 2021
Merged

Annotation and label selection in replacement targets #4229

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
605239a
test to demonstrate broken annotation selection
natasha41575 Oct 7, 2021
ef5f1d3
support label and annotation selection in replacement targets
natasha41575 Oct 7, 2021
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
42 changes: 41 additions & 1 deletion api/filters/replacement/replacement.go
View file
Open in desktop
Expand Up @@ -8,6 +8,7 @@ import (
"strings"

"sigs.k8s.io/kustomize/api/internal/utils"
"sigs.k8s.io/kustomize/api/resource"
"sigs.k8s.io/kustomize/api/types"
"sigs.k8s.io/kustomize/kyaml/resid"
"sigs.k8s.io/kustomize/kyaml/yaml"
Expand Down Expand Up @@ -48,6 +49,17 @@ func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targets []*types.T
if err != nil {
return nil, err
}

// filter targets by label and annotation selectors
selectByAnnoAndLabel, err := selectByAnnoAndLabel(n, t)
if err != nil {
return nil, err
}
if !selectByAnnoAndLabel {
continue
}

// filter targets by matching resource IDs
for _, id := range ids {
if id.IsSelectedBy(t.Select.ResId) && !rejectId(t.Reject, &id) {
err := applyToNode(n, value, t)
Expand All @@ -62,9 +74,37 @@ func applyReplacement(nodes []*yaml.RNode, value *yaml.RNode, targets []*types.T
return nodes, nil
}

func selectByAnnoAndLabel(n *yaml.RNode, t *types.TargetSelector) (bool, error) {
if matchesSelect, err := matchesAnnoAndLabelSelector(n, t.Select); !matchesSelect || err != nil {
return false, err
}
for _, reject := range t.Reject {
if reject.AnnotationSelector == "" && reject.LabelSelector == "" {
continue
}
if m, err := matchesAnnoAndLabelSelector(n, reject); m || err != nil {
return false, err
}
}
return true, nil
}

func matchesAnnoAndLabelSelector(n *yaml.RNode, selector *types.Selector) (bool, error) {
r := resource.Resource{RNode: *n}
annoMatch, err := r.MatchesAnnotationSelector(selector.AnnotationSelector)
if err != nil {
return false, err
}
labelMatch, err := r.MatchesLabelSelector(selector.LabelSelector)
if err != nil {
return false, err
}
return annoMatch && labelMatch, nil
}

func rejectId(rejects []*types.Selector, id *resid.ResId) bool {
for _, r := range rejects {
if id.IsSelectedBy(r.ResId) {
if !r.ResId.IsEmpty() && id.IsSelectedBy(r.ResId) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this reason this can be empty now that the rejection may have been defined by selector (handled elsewhere) instead of by GVK? Does one of the existing tests cover the regression if we were to not do this check?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I added this condition specifically because the reject via labelSelector test was failing without it.

return true
}
}
Expand Down
224 changes: 224 additions & 0 deletions api/filters/replacement/replacement_test.go
View file
Open in desktop
Expand Up @@ -1588,6 +1588,230 @@ data:
`,
expectedErr: "fieldPath `data.httpPort` is missing for replacement source ~G_~V_ConfigMap|~X|ports-from:data.httpPort",
},
"annotationSelector": {
input: `apiVersion: v1
kind: Deployment
metadata:
name: deploy-1
annotations:
foo: bar-1
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: postgres:1.8.0
name: postgresdb
---
apiVersion: v1
kind: Deployment
metadata:
name: deploy-2
annotations:
foo: bar-2
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: postgres:1.8.0
name: postgresdb

`,
replacements: `replacements:
- source:
kind: Deployment
name: deploy-1
fieldPath: spec.template.spec.containers.0.image
targets:
- select:
annotationSelector: foo=bar-1
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
kind: Deployment
metadata:
name: deploy-1
annotations:
foo: bar-1
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: nginx:1.7.9
name: postgresdb
---
apiVersion: v1
kind: Deployment
metadata:
name: deploy-2
annotations:
foo: bar-2
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: postgres:1.8.0
name: postgresdb
`,
},
"labelSelector": {
input: `apiVersion: v1
kind: Deployment
metadata:
name: deploy-1
labels:
foo: bar-1
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: postgres:1.8.0
name: postgresdb
---
apiVersion: v1
kind: Deployment
metadata:
name: deploy-2
labels:
foo: bar-2
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: postgres:1.8.0
name: postgresdb

`,
replacements: `replacements:
- source:
kind: Deployment
name: deploy-1
fieldPath: spec.template.spec.containers.0.image
targets:
- select:
labelSelector: foo=bar-1
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
kind: Deployment
metadata:
name: deploy-1
labels:
foo: bar-1
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: nginx:1.7.9
name: postgresdb
---
apiVersion: v1
kind: Deployment
metadata:
name: deploy-2
labels:
foo: bar-2
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: postgres:1.8.0
name: postgresdb
`,
},
"reject via labelSelector": {
input: `apiVersion: v1
kind: Deployment
metadata:
name: deploy-1
labels:
foo: bar-1
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: postgres:1.8.0
name: postgresdb
---
apiVersion: v1
kind: Deployment
metadata:
name: deploy-2
labels:
foo: bar-2
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: postgres:1.8.0
name: postgresdb

`,
replacements: `replacements:
- source:
kind: Deployment
name: deploy-1
fieldPath: spec.template.spec.containers.0.image
targets:
- select:
kind: Deployment
reject:
- labelSelector: foo=bar-2
fieldPaths:
- spec.template.spec.containers.1.image
`,
expected: `apiVersion: v1
kind: Deployment
metadata:
name: deploy-1
labels:
foo: bar-1
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: nginx:1.7.9
name: postgresdb
---
apiVersion: v1
kind: Deployment
metadata:
name: deploy-2
labels:
foo: bar-2
spec:
template:
spec:
containers:
- image: nginx:1.7.9
name: nginx-tagged
- image: postgres:1.8.0
name: postgresdb
`,
},
}

for tn, tc := range testCases {
Expand Down
7 changes: 7 additions & 0 deletions kyaml/resid/resid.go
View file
Open in desktop
Expand Up @@ -4,6 +4,7 @@
package resid

import (
"reflect"
"strings"
)

Expand Down Expand Up @@ -129,3 +130,9 @@ func (id ResId) EffectiveNamespace() string {
}
return id.Namespace
}

// IsEmpty returns true of all of the id's fields are
// empty strings
func (id ResId) IsEmpty() bool {
natasha41575 marked this conversation as resolved.
Show resolved Hide resolved
return reflect.DeepEqual(id, ResId{})
}