-
Notifications
You must be signed in to change notification settings - Fork 14.6k
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
docs: extend VAP docs with exempt resources #49520
docs: extend VAP docs with exempt resources #49520
Conversation
Signed-off-by: Thomas Gosteli <thomas.gosteli@protonmail.ch>
Welcome @ghouscht! |
✅ Pull request preview available for checkingBuilt without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Thanks for this proposed change @ghouscht
We'd like this PR revised to be more aligned with our style guide, but I really like the intent here.
Since #48646 has merged, we should also make sure to make this change - I think almost the exact same change - to https://kubernetes.io/docs/reference/access-authn-authz/mutating-admission-policy/
/sig api-machinery
content/en/docs/reference/access-authn-authz/validating-admission-policy.md
Outdated
Show resolved
Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md
Outdated
Show resolved
Hide resolved
content/en/docs/reference/access-authn-authz/validating-admission-policy.md
Outdated
Show resolved
Hide resolved
* `validatingadmissionpolicies` (API group: `admissionregistration.k8s.io`) | ||
* `validatingadmissionpolicybindings` (API group: `admissionregistration.k8s.io`) | ||
* `mutatingadmissionpolicies` (API group: `admissionregistration.k8s.io`) | ||
* `mutatingadmissionpolicybindings` (API group: `admissionregistration.k8s.io`) | ||
* `selfsubjectreviews` (API group: `authentication.k8s.io`) | ||
* `tokenreviews` (API group: `authentication.k8s.io`) | ||
* `localsubjectaccessreviews` (API group: `authentication.k8s.io`) | ||
* `selfsubjectaccessreviews` (API group: `authentication.k8s.io`) | ||
* `subjectaccessreviews` (API group: `authentication.k8s.io`) |
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.
Please use the API kind names (in UpperCamelCase, eg TokenReviews). No need to mention the API group IMO
Extra credit for making each one a hyperlink to the relevant API reference page, eg https://kubernetes.io/docs/reference/kubernetes-api/authentication-resources/token-review-v1/
You can use the relref
shortcode - but you don't have to - if you want to ensure the links are valid.
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.
Will have a look later today 👍🏻
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.
applied the suggested change and pluralized the API kind names. I hope that this is ok. There is no reference for MutatingAdmissionPolicies
and MutatingAdmissionPolicyBindings
yet as far as I have seen, because of that they are missing the links.
…ion-policy.md Co-authored-by: Tim Bannister <tim@scalefactory.com>
…ion-policy.md Co-authored-by: Tim Bannister <tim@scalefactory.com>
…ion-policy.md Co-authored-by: Tim Bannister <tim@scalefactory.com>
Thank you for the review, I really appreciate it. I'll have a look later today and will also take care of the mutating admission policy page. |
I added more or less the same note to the mutating admission policy page. However I still need to verify in the code that the same resources are exempt. I'll do this at a later point in time. Thinking out loud: Not sure I should raise an issue/PR to add some logging to the k8s apiserver so that users could see something in the apiserver logs if they try to target an exempt resource with admission policies. As of now you have no chance to figure this out except for following the apiserver code. Maybe some of the sig-apimachinery people that were tagged here can give a hint if this would be something they are willing to add to the apiserver. |
This all looks good to me. |
LGTM label has been added. Git tree hash: 12852425282f47573ddbd2674dad3feb54a108f9
|
Three places we could surface that:
@ghouscht if you've the time to, please make that suggestion either on Slack or in a new k/k issue |
/assign @rayandas |
Thanks! |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rayandas The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Certain resources are (by design) exempt from being targeted by
ValidatingAdmissionPolicy
andMutatingAdmissionPolicy
. Unfortunately there is no clear documentation in the KEP-3488 nor in the official Kubernetes docs which resources are affected by this exemption.This MR extends the documentation to mention these resources in the official docs. I think this helps other users in the future to not waste a lot of time (like I did) to figure out which resources are exempt.
Im not sure if this is the right approach but I think it is better than no documentation at all. See also these issues/PRs for reference:
I appreciate your feedback on this. Thank you ❤️
Issue
-