-
Notifications
You must be signed in to change notification settings - Fork 777
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
fix: Added .Values.validatingWebhookExemptNamespacesLabels
to the /v1/admitlabel
webhook
#3147
fix: Added .Values.validatingWebhookExemptNamespacesLabels
to the /v1/admitlabel
webhook
#3147
Conversation
2a0c7cd
to
72b2e38
Compare
Thanks for the PR! Unfortunately, this change would get clobbered by our auto-gen pipeline as-is. Please see https://open-policy-agent.github.io/gatekeeper/website/docs/help/#contributing-to-helm-chart for more details. |
0552b92
to
b5eeeff
Compare
@maxsmythe like this then? |
@Gaardsholt you need to modify this and add this there as well. Then run |
Thank you so much for pointing me the the right direction @JaydipGabani ! |
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
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3147 +/- ##
==========================================
+ Coverage 53.68% 53.78% +0.10%
==========================================
Files 136 136
Lines 12198 12198
==========================================
+ Hits 6548 6561 +13
+ Misses 5145 5136 -9
+ Partials 505 501 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
16f1f2e
to
1f5a5a7
Compare
@Gaardsholt please sign the commit to resolve DCO |
Signed-off-by: Lasse Gaardsholt <[email protected]>
…ebhook" This reverts commit 72b2e38. Signed-off-by: Lasse Gaardsholt <[email protected]>
Signed-off-by: Lasse Gaardsholt <[email protected]>
Signed-off-by: Lasse Gaardsholt <[email protected]>
1f5a5a7
to
29de8fd
Compare
@JaydipGabani DCO has been fixed |
There is a security hole in modifying the admitLabel webhook like this in the form of a privilege escalation. The purpose of If we were to merge this as-is, the effect would be to nullify the purpose of the webhook, since any user with permissions to edit namespaces would be able to modify that namespace's label, bypassing the webhook. At that point, it would be clearer (and more stable), to just remove the admitLabel entirely. Some users may consider the ability to add/remove/edit namespaces to be a privileged operation, and therefore would be fine with relying on RBAC alone as the security mechanism here. For these users, disabling TL;DR we probably just want a switch that makes the |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 14 days if no further activity occurs. Thank you for your contributions. |
What this PR does / why we need it: When running on GKE we get the following error in the console: