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 PSP constraints and CTs to library #203

Merged
merged 1 commit into from
Aug 14, 2019

Conversation

ritazh
Copy link
Member

@ritazh ritazh commented Aug 7, 2019

Signed-off-by: Rita Zhang rita.z.zhang@gmail.com

Adding the PSP constraints and constraint templates to the library folder until we have the gatekeeper-library repo available per community call on August 6th.

Thank you @sozercan for your contribution!

cc @timothyhinrichs @teq0

@ritazh ritazh requested a review from maxsmythe August 7, 2019 23:26
Copy link
Member

@timothyhinrichs timothyhinrichs left a comment

Choose a reason for hiding this comment

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

Very cool Rita! It'll take a long while to go through these in detail. I wonder if we should just get them into the repo so we've got something to build on. Would you say these are all ready to go live, or are they more experimental in terms of say test-coverage?

A couple highlevel thoughts...

  • I was going to ask whether we want to commit the ConstraintTemplates into the repo, but we need that metadata somewhere, and we don't have the tooling yet to generate them from Rego + Metadata. So I think it makes sense to commit them now and refactor later, if needed.

  • The directory name could be something like just psp or pod-security-policy. Obviously a nit that could be adjusted later.

@ritazh
Copy link
Member Author

ritazh commented Aug 8, 2019

Would you say these are all ready to go live, or are they more experimental in terms of say test-coverage?

We have tested these with opa test --coverage as well as in a k8s cluster. The lowest coverage right now is at 94.45, most are at 100. You can see the test coverage by running make test in /library/pod-security-policy/. Please LMK if there are other forms of tests I need to run to validate them.

I wonder if we should just get them into the repo so we've got something to build on

I would really love some review on these regos if possible. I'm afraid once these are merged, we might not have time to come back to them. WDYT? We have 10 policies now, maybe each reviewer can take 3? between @maxsmythe @teq0 @timothyhinrichs Would that work?

Copy link
Member

@timothyhinrichs timothyhinrichs left a comment

Choose a reason for hiding this comment

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

I looked at the first 3: allow-privilege-escalation, flexvolume-drivers, fsgroup

Copy link
Member

@timothyhinrichs timothyhinrichs left a comment

Choose a reason for hiding this comment

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

Looked at a couple more.

Copy link
Member

@timothyhinrichs timothyhinrichs left a comment

Choose a reason for hiding this comment

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

Finished review of the whole thing

Copy link
Member

@timothyhinrichs timothyhinrichs left a comment

Choose a reason for hiding this comment

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

Just nits. Overall, consistent and clean rego code. Nice work!

Signed-off-by: Rita Zhang <rita.z.zhang@gmail.com>
@ritazh
Copy link
Member Author

ritazh commented Aug 14, 2019

Thanks for the review @timothyhinrichs! Merging per this week's GK weekly call.

@ritazh ritazh merged commit 834f3ee into open-policy-agent:master Aug 14, 2019
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.

2 participants