-
Notifications
You must be signed in to change notification settings - Fork 306
disable namespace-lister's group support in prod #7925
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: main
Are you sure you want to change the base?
disable namespace-lister's group support in prod #7925
Conversation
We don't want to return the list of all namespaces shared with the system:authenticated group (the only group supported nowadays). We just want to show the list of namespaces directly shared with the user. Signed-off-by: Francesco Ilario <[email protected]>
We don't want to return the list of all namespaces shared with the system:authenticated group (the only group supported nowadays). We just want to show the list of namespaces directly shared with the user. Signed-off-by: Francesco Ilario <[email protected]>
Skipping CI for Draft Pull Request. |
Code Review by GeminiThe changes disable the There are no bugs or issues with this change. The approach of commenting out the patch is a valid way to disable it in Kustomize, and the added comments provide good context for future maintainers. --- a/components/namespace-lister/base/kustomization.yaml
+++ b/components/namespace-lister/base/kustomization.yaml
@@ -19,16 +19,19 @@ patches:
group: apps
kind: Deployment
name: namespace-lister
namespace: namespace-lister
- path: ./patches/with_header_auth_user.yaml
target:
group: apps
kind: Deployment
name: namespace-lister
namespace: namespace-lister
-- path: ./patches/with_header_auth_groups.yaml
- target:
- group: apps
- kind: Deployment
- name: namespace-lister
- namespace: namespace-lister
+# uncomment this line to add support for groups forwarded through
+# a custom header
+#
+# - path: ./patches/with_header_auth_groups.yaml
+# target:
+# group: apps
+# kind: Deployment
+# name: namespace-lister
+# namespace: namespace-lister --- a/components/namespace-lister/production/base/kustomization.yaml
+++ b/components/namespace-lister/production/base/kustomization.yaml
@@ -19,16 +19,19 @@ patches:
group: apps
kind: Deployment
name: namespace-lister
namespace: namespace-lister
- path: ./patches/with_header_auth_user.yaml
target:
group: apps
kind: Deployment
name: namespace-lister
namespace: namespace-lister
-- path: ./patches/with_header_auth_groups.yaml
- target:
- group: apps
- kind: Deployment
- name: namespace-lister
- namespace: namespace-lister
+# uncomment this line to add support for groups forwarded through
+# a custom header
+#
+# - path: ./patches/with_header_auth_groups.yaml
+# target:
+# group: apps
+# kind: Deployment
+# name: namespace-lister
+# namespace: namespace-lister |
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
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: filariow, hugares 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 |
New changes are detected. LGTM label has been removed. |
Code Review by GeminiThe changes correctly disable the No bugs or issues were found. The added comment provides clear instructions for future maintainers if the feature needs to be re-enabled. --- a/components/namespace-lister/production/base/kustomization.yaml
+++ b/components/namespace-lister/production/base/kustomization.yaml
@@ -19,16 +19,19 @@ patches:
group: apps
kind: Deployment
name: namespace-lister
namespace: namespace-lister
- path: ./patches/with_header_auth_user.yaml
target:
group: apps
kind: Deployment
name: namespace-lister
namespace: namespace-lister
-- path: ./patches/with_header_auth_groups.yaml
- target:
- group: apps
- kind: Deployment
- name: namespace-lister
- namespace: namespace-lister
+# uncomment this line to add support for groups forwarded through
+# a custom header
+#
+# - path: ./patches/with_header_auth_groups.yaml
+# target:
+# group: apps
+# kind: Deployment
+# name: namespace-lister
+# namespace: namespace-lister
|
/hold |
Requires:
We don't want to return the list of all namespaces shared with the
system:authenticated
group (the only group supported nowadays).We just want to show the list of namespaces directly shared with the user.