-
Notifications
You must be signed in to change notification settings - Fork 743
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.
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
Add excludedNamespaces match type #433
Add excludedNamespaces match type #433
Conversation
8b8e4f6
to
998f631
Compare
Should fix #417 |
c89ae83
to
d5758dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a test to make sure things are working properly. It looks like we've been relying on integration tests to validate the namespace name code.
The basic tests for by-name namespace matching are here:
gatekeeper/pkg/target/target_integration_test.go
Lines 132 to 145 in 0e6879a
{ | |
name: "match namespace", | |
obj: makeResource("some", "Thing"), | |
ns: makeNamespace("my-ns"), | |
constraint: makeConstraint(setNamespaceName("my-ns")), | |
allowed: false, | |
}, | |
{ | |
name: "no match namespace", | |
obj: makeResource("some", "Thing"), | |
ns: makeNamespace("my-ns"), | |
constraint: makeConstraint(setNamespaceName("not-my-ns")), | |
allowed: true, | |
}, |
This function was used to set the namespace-name match criteria on the test constraint, a similar one can be used to set excludedNamespaces:
gatekeeper/pkg/target/target_integration_test.go
Lines 81 to 87 in 0e6879a
func setNamespaceName(name string) buildArg { | |
return func(obj *unstructured.Unstructured) { | |
if err := unstructured.SetNestedSlice(obj.Object, []interface{}{name}, "spec", "match", "namespaces"); err != nil { | |
panic(err) | |
} | |
} | |
} |
Adding two new tests that use the newly written function should be sufficient for this PR.
For the project generally, we should add tests for the special object-under-test-is-a-namespace case for all the match criteria, given all the special handling it requires (this last point is not feedback for this PR).
Signed-off-by: Andrew Jeffery <andrew.jeffery@thehutgroup.com>
Signed-off-by: Andrew Jeffery <andrew.jeffery@thehutgroup.com>
Signed-off-by: Andrew Jeffery <andrew.jeffery@thehutgroup.com>
d5758dc
to
f7d3003
Compare
Signed-off-by: Jeffas <dev@jeffas.io>
f7d3003
to
9ef4d5e
Compare
Thanks for the comments, I've added the tests and renamed the things 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This is awesome! Thanks!
Thanks! Is it planned to have a new release soon that would include this or is it possible to cut one? |
I don't think we have a set release cadence yet (something we could work on), but I think we can cut a release this week or next. It's also possible to build from head using Then you can either update the gatekeeper.yaml manifest to point to the correct image or run:
Running |
This adds excludedNamespaces to the MatchSchema and provides the logic for checking it.
If there is anything that needs to be added or changed let me know :)