-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
🌱 Implement privileged namespace security policy update for tilt-prepare #10178
🌱 Implement privileged namespace security policy update for tilt-prepare #10178
Conversation
@maxrantil: The label(s) In response to this:
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. |
Hi @maxrantil. Thanks for your PR. I'm waiting for a kubernetes-sigs 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. |
345a10d
to
6d65534
Compare
/ok-to-test |
/remove-area provider/infrastructure-in-memory |
/area devtools |
6d65534
to
aa7fc10
Compare
I just wanted to clarify this a bit. Tilt-prepare already removes the security context from the deployments, but this is not enough if the namespace has enforced pod security standards. The issue is not visible for CAPI/CAPD since CAPD anyway uses the privileged policy in order to mount the docker socket. Other providers will likely be able to run with a restricted policy for the namespace, except when using Tilt. This is why we want to make tilt-prepare change the label on the namespace. |
100% agree, this gets helpful when PSA is enforced in a clusters and we want to run a provider or the core CAPI providers using tilt. Edit: it comes down to that the reason for wanting this in tilt-prepare is this line in tilt-prepare:
|
aa7fc10
to
a8e4f53
Compare
Good point! So the reason it is currently working for CAPI is that there are no restrictions on the namespace. Probably the same for many other providers. |
a8e4f53
to
cdbdeb2
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.
/lgtm
LGTM label has been added. Git tree hash: 7cd7cabbf76b59c96494ba7f5bad36da33982cb3
|
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.
One last nit, let's add a comment on why we have to do this to provide future context.
c17656c
to
03335e9
Compare
/lgtm |
LGTM label has been added. Git tree hash: 92cf5b3fa87d43dd1aec4c273f8892485401cc79
|
28d9938
to
abf180d
Compare
This commit updates the updateNamespaceSecurityStandard function to set the pod-security.kubernetes.io/enforce label to 'privileged' for Namespace objects. Signed-off-by: Max Rantil <max.rantil@est.tech> Co-authored-by: Christian Schlotter <chrischdi@users.noreply.github.com>
abf180d
to
8be5d93
Compare
/cc @killianmuldoon @JoelSpeed would any of you please be willing to do a review? |
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.
Readding LGTM after all comments so far have been resolved.
/lgtm
LGTM label has been added. Git tree hash: dc11a973b8c001a38361c1b5656ab20d9dbd884b
|
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
/approve
Thanks for this!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: killianmuldoon 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 this PR does / why we need it:
This PR creates a
updateNamespacePodSecurityStandard
function to ensure that thepod-security.kubernetes.io/enforce
label is set to"privileged"
for Namespace objects. This change is essential for compatibility with CAPIs tilt-prepare and Tiltfile when using CAPM3 (and other providers?), where accessing securityContext is required. Without this update, the existing pod-security policy restricts the usage of securityContext, hindering necessary operations. This aligns the behavior with CAPD, where the privileged policy is already set for Namespace objects./area provider/infrastructure-in-memory