-
Notifications
You must be signed in to change notification settings - Fork 38.7k
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 CAP_NET_RAW to netadmin debugging profile #118647
Add CAP_NET_RAW to netadmin debugging profile #118647
Conversation
Hi @mochizuki875. 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/test-infra repository. |
/sig cli |
@verb @ardaguclu |
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.
I think, we first need to update KEP as stated in here #118962 (comment) to reflect the NET_RAW capability is needed for network admins. After it is updated, we can return back to this PR. What do you think?.
/ok-to-test |
@ardaguclu |
9b58c76
to
fbad1f3
Compare
/retest |
I understand that the content of the KEP update has been discussed(kubernetes/enhancements#1441 (comment) , #118962) and there are no restrictions on who can submit PR to KEP. Although I can't directly update the KEP, I think it will be merged based on member's reviews and approval. |
/retest |
772473e
to
fcb1453
Compare
/retest |
1 similar comment
/retest |
a4a3cf6
to
0ecb8f4
Compare
0ecb8f4
to
e22f8ed
Compare
/retest |
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.
I renew my approval!
I would maybe split the commit in two: one to add the capability and another to remove privileged (order does not really matter).
@ardaguclu @verb |
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
LGTM label has been added. Git tree hash: 893cc89c8083104cb65772bb4ebf060b259b9d8e
|
@ardaguclu |
Thanks /triage accepted |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ardaguclu, eiffel-fl, mochizuki875, verb 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 |
What type of PR is this?
/kind bug
What this PR does / why we need it:
netadmin profile adds only
CAP_NET_ADMIN
toEphemeralContainer
andDebuggingContainer
.(In case of debugging Node, debugging Pod will be added
privileged
andCAP_NET_ADMIN
.)As disscussed here, I add
CAP_NET_RAW
tonetadmin
profile and remove privileged when debug Node in this PR.Which issue(s) this PR fixes:
Fixes: #118962 kubernetes/enhancements#1441 (comment)
Related: #115712 kubernetes/kubectl#1108
KEP Update: kubernetes/enhancements#4160
Special notes for your reviewer:
In this PR:
I add
CAP_NET_RAW
tonetadmin
profile in each case of EphemeralContainer, Pod Copy and Node.Especially in case of Node, since
privileged
is added, it seems that theCAP_NET_ADMIN
currently added andCAP_NET_RAW
to be newly added this time overlap.(Shouldprivileged
be addednetadmin
? It added #115712)Does this PR introduce a user-facing change?
Additional documentation e.g., KEPs (Kubernetes Enhancement Proposals), usage docs, etc.: