-
Notifications
You must be signed in to change notification settings - Fork 253
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
feat: add pod security standards (restricted) in CEL expressions - Part 3 #776
feat: add pod security standards (restricted) in CEL expressions - Part 3 #776
Conversation
655029c
to
98411e6
Compare
The |
Per Slack discussion, if |
be23f9f
to
2162ee4
Compare
Regarding the failed test
The main issue is in the second CEL expression. When I tried to remove it from the policy, the
|
So is the second expression wrong? And is there one or two bugs here? |
The 2nd expression is required because it basically checks that either of For example, the following pod should fail:
The 1st expression will be evaluated to true, whereas the 2nd one will be evaluated to false so it is required. The |
Have you tried the CEL playground to make sure this is being evaluated correctly? |
Were you able to get the last policy written? |
@MariamFahmy98 |
b83bfbb
to
99f229b
Compare
For testing purposes, I extracted the expression that causes failures into a separate policy and tested it against three bad pods.
Upon investigation, here is what I have discovered so far:
Conclusion:
I started a discussion thread in #sig-api-machinery-cel to see why VAP fails to block the creation of the pod. |
@chipzoller - I think it would be better to remove this policy temporarily to get this PR into the main, and I will create a separate issue for this failed policy for more discussion. |
b02b933
to
ab614f7
Compare
Are we close to getting this in? |
Ping @MariamFahmy98 |
81b5e22
to
69c3b67
Compare
@chipzoller - any additional comments or feedback? |
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.
The value of the policies.kyverno.io/category
annotation should be Pod Security Standards (Restricted) in CEL
to be consistent with the CEL Baseline policies
ae4cfd0
to
a5da65d
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.
Apologies for missing this earlier and requesting more changes, but I also see the annotation policies.kyverno.io/minversion: 1.11.0
is missing from these while it is present on the other CEL policies for PSS.
5683283
to
26727eb
Compare
Signed-off-by: Mariam Fahmy <[email protected]>
Signed-off-by: Mariam Fahmy <[email protected]>
Signed-off-by: Mariam Fahmy <[email protected]>
Signed-off-by: Mariam Fahmy <[email protected]>
Signed-off-by: Mariam Fahmy <[email protected]>
Signed-off-by: Mariam Fahmy <[email protected]>
Signed-off-by: Mariam Fahmy <[email protected]>
Signed-off-by: Mariam Fahmy <[email protected]>
Signed-off-by: Mariam Fahmy <[email protected]>
26727eb
to
3d9886e
Compare
@chipzoller - anything else required to merge this? |
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. Nice job, @MariamFahmy98!
…rt 3 (kyverno#776) * feat: add pod security standards (restricted) in CEL expressions Signed-off-by: Mariam Fahmy <[email protected]> * fix: use cel.expression.message instead of validate.message Signed-off-by: Mariam Fahmy <[email protected]> * chore: update artifacthub-pkg.yaml Signed-off-by: Mariam Fahmy <[email protected]> * remove the failed policy Signed-off-by: Mariam Fahmy <[email protected]> * fix the name field in artifacthub-pkg.yaml Signed-off-by: Mariam Fahmy <[email protected]> * fix chainsaw tests Signed-off-by: Mariam Fahmy <[email protected]> * fix a lint issue Signed-off-by: Mariam Fahmy <[email protected]> * fix the value of policies.kyverno.io/category Signed-off-by: Mariam Fahmy <[email protected]> * fix: add the minversion annotation Signed-off-by: Mariam Fahmy <[email protected]> --------- Signed-off-by: Mariam Fahmy <[email protected]>
Related Issue(s)
Closes #773
Description
This PR adds Kyverno policies written in CEL expressions for pod security standards (restricted).
Checklist