-
Notifications
You must be signed in to change notification settings - Fork 1.6k
KEP-5474: Introduce WritableCgroups option #5475
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
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Divya063 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @Divya063! |
Hi @Divya063. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
018f069
to
bb09551
Compare
bb09551
to
dcd59fa
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.
Hi! Thank you for the work on this KEP. I'm requesting changes before this can be approved, primarily to address a security and resource isolation concern.
I strongly support the proposed direction of adding a field to the Pod SecurityContext
and CRI API. This is the right approach for providing fine-grained control and is much more secure than relying on a broad, runtime-level configuration.
My primary concern, which I believe is a blocker for approval, is the risk of intra-pod resource starvation. Please see my comments under "Risks and Mitigations" for details.
I also left a couple of comments with suggestions to improve clarify, primarily around removing any potentially confusing references to the Runtime
config field to make it unambiguous that the CRI-based API field is the sole proposed approach.
I'm happy to discuss these points further. Thank you!
/assign @chrishenzie |
Thank you @chrishenzie for providing the initial feedback! I'm changing the PR to draft state while I work on the doc. Once it's ready, I'll let you know. |
Thanks for the update and taking my feedback into consideration. I look forward to the next version of the document. As you might have gathered from my review, this is a feature I'm interested in and have spent a good deal of time researching independently. It seems we're aligned on the importance of this capability for the community and the need to get the API and security model right. Given our mutual interest, I wanted to offer to partner with you on this KEP if you'd be open to it. I'd be happy to contribute my research and help however I can to move this forward. If you're interested, please feel free to DM me on the Kubernetes Slack (@chrishenzie) or reach out via email so we can connect. Regardless, I appreciate you driving this important work. Let me know if there is any way I can help. |
60f61fc
to
bd0bdae
Compare
Thanks @chrishenzie for offering to partner on this KEP! I will be happy to collaborate. I have addressed your comments and would like to discuss a few things.
Let me know what you think. |
This commit introduces a KEP that proposes adding a `CgroupWritable` field to the container SecurityContext in Kubernetes to allow unprivileged containers to have writable access to cgroup interfaces on cgroup v2 systems
bd0bdae
to
ca08278
Compare
Uh oh!
There was an error while loading. Please reload this page.