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.

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

Merged
merged 4 commits into from
Jan 22, 2020

Conversation

jeffa5
Copy link
Contributor

@jeffa5 jeffa5 commented Jan 20, 2020

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 :)

@jeffa5
Copy link
Contributor Author

jeffa5 commented Jan 20, 2020

Should fix #417

pkg/target/regolib/src.rego Outdated Show resolved Hide resolved
@jeffa5 jeffa5 force-pushed the add-excluded-namespaces branch 2 times, most recently from c89ae83 to d5758dc Compare January 21, 2020 09:14
Copy link
Contributor

@maxsmythe maxsmythe left a 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:

{
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:

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).

pkg/target/regolib/src.rego Outdated Show resolved Hide resolved
pkg/target/regolib/src.rego Outdated Show resolved Hide resolved
pkg/target/target_template_source.go Outdated Show resolved Hide resolved
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>
Signed-off-by: Jeffas <dev@jeffas.io>
@jeffa5
Copy link
Contributor Author

jeffa5 commented Jan 21, 2020

Thanks for the comments, I've added the tests and renamed the things 😄

Copy link
Contributor

@maxsmythe maxsmythe left a 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!

@maxsmythe maxsmythe merged commit 1cb4c9c into open-policy-agent:master Jan 22, 2020
@jeffa5 jeffa5 deleted the add-excluded-namespaces branch January 22, 2020 09:05
@jeffa5
Copy link
Contributor Author

jeffa5 commented Jan 22, 2020

Thanks!

Is it planned to have a new release soon that would include this or is it possible to cut one?

@maxsmythe
Copy link
Contributor

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 make docker-build REGISTRY=<YOUR DOCKER REGISTRY> and make docker-push-release REGISTRY=<YOUR DOCKER REGISTRY>

Then you can either update the gatekeeper.yaml manifest to point to the correct image or run:

make deploy REGISTRY=<YOUR DOCKER REGISTRY> while your kubernetes context is set to the cluster you want to install to.

Running make deploy should be equivalent to running kubectl apply -f manifest_staging/deploy/gatekeeper.yaml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants