chore: adding cel for psp/volume and psp/selinux #530
Conversation
Signed-off-by: Jaydip Gabani <[email protected]>
Signed-off-by: Jaydip Gabani <[email protected]>
JaydipGabani
changed the title
Signed-off-by: Jaydip Gabani <[email protected]>
Signed-off-by: Jaydip Gabani <[email protected]>
Signed-off-by: Jaydip Gabani <[email protected]>
Signed-off-by: Jaydip Gabani <[email protected]>
|
This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions. |
| variables: | ||
| - name: usingAllowedSELinuxOptions | ||
| expression: | | ||
| has(variables.anyObject.spec.securityContext) && has(variables.anyObject.spec.securityContext.seLinuxOptions) ? |
There was a problem hiding this comment.
This test looks wrong. Shouldn't we be assuming an option is disallowed unless specifically allowed by parameters?
As-written, it looks like undefined parameters => allow everything, which would seem to drift from the rego.
Adding gator tests for this would validate the drift.
There was a problem hiding this comment.
Good catch! I fixed it, it should be in parity with rego now.
|
@sozercan @maxsmythe @ritazh as of now the rego written for policy will deny a pod when disallowed |
Signed-off-by: Jaydip Gabani <[email protected]>
| has(variables.anyObject.spec.securityContext) && has(variables.anyObject.spec.securityContext.seLinuxOptions) ? | ||
| (has(variables.params.allowedSELinuxOptions) ? | ||
| ( | ||
| size(variables.params.allowedSELinuxOptions) > 0 && variables.params.allowedSELinuxOptions.all(opts, |
There was a problem hiding this comment.
I don't think .all() is correct here, the way the rego reads, at least one provided allowedSELinuxOptions must be satisfied.
As always, verify with testing.
| ( | ||
| size(variables.params.allowedSELinuxOptions) > 0 && variables.params.allowedSELinuxOptions.all(opts, | ||
| (has(opts.level) ? has(variables.anyObject.spec.securityContext.seLinuxOptions.level) && (variables.anyObject.spec.securityContext.seLinuxOptions.level == opts.level) : !has(variables.anyObject.spec.securityContext.seLinuxOptions.level)) && | ||
| (has(opts.role) ? has(variables.anyObject.spec.securityContext.seLinuxOptions.role) && (variables.anyObject.spec.securityContext.seLinuxOptions.role == opts.role) : !has(variables.anyObject.spec.securityContext.seLinuxOptions.role)) && |
There was a problem hiding this comment.
As mentioned in previous comment, this seems incorrect. It looks like you are disabling the check if the params side is undefined, as opposed to skipping if securityContext side is undefined.
Previous comment:
This test looks wrong. Shouldn't we be assuming an option is disallowed unless specifically allowed by parameters?
As-written, it looks like undefined parameters => allow everything, which would seem to drift from the rego.
Adding gator tests for this would validate the drift.
Please add the gator tests for this scenario, relying on reviewers to catch this drift consistently is brittle.
| ) : | ||
| true) | ||
| : true | ||
| - name: containers |
There was a problem hiding this comment.
I have not looked at containers code, but I'm guessing it will need to be modified to comply with pod level code.
|
This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions. |
|
This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions. |
|
Not stale |
|
This issue/PR has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions. |
|
not stale |
What this PR does / why we need it:
Which issue(s) does this PR fix (optional, using
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when the PR gets merged):Fixes #541
Special notes for your reviewer: