-
Notifications
You must be signed in to change notification settings - Fork 21
fix(admin-ui): Enable admin-ui ingress by default #2461
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
Changes from all commits
8e08fe8
770ec3f
15cc62f
e637f90
415e7e5
149ce5e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| {{ if index .Values "admin-ui" "ingress" "adminUiEnabled" -}} | ||
| ******************************************************************************** | ||
| *** SECURITY WARNING: ADMIN-UI EXPOSED *** | ||
| ******************************************************************************** | ||
| The flag `admin-ui.ingress.adminUiEnabled` is set to TRUE. | ||
|
|
||
| This publicly exposes the Admin UI at "/admin" | ||
|
|
||
| RECOMMENDATION: | ||
| 1. For production, ensure this endpoint is restricted via NetworkPolicies, | ||
| IP whitelisting, or an OAuth2 proxy. This endpoint is normally not internet facing. | ||
| 2. This can be left public in demo or internal development environments only. | ||
|
|
||
| ******************************************************************************** | ||
| {{- end }} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -381,7 +381,7 @@ admin-ui: | |
| enabled: true | ||
| ingress: | ||
| # -- Enable Admin UI endpoints in either istio or nginx ingress depending on users choice | ||
| adminUiEnabled: false | ||
| adminUiEnabled: true | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we are enabling this by default we need to post a message that this is enabled and that the user should protect the /admin endpoint when
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. This comment applies to the gluu microservices chart as well. I think we need to post a warning. What I’m suggesting is that we introduce |
||
| # -- Admin UI ingress resource labels. key app is taken. | ||
| adminUiLabels: {} | ||
| # -- Admin UI ingress resource additional annotations. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| {{ if index .Values "global" "admin-ui" "ingress" "adminUiEnabled" -}} | ||
| ******************************************************************************** | ||
| *** SECURITY WARNING: ADMIN-UI EXPOSED *** | ||
| ******************************************************************************** | ||
| The flag `global.admin-ui.ingress.adminUiEnabled` is set to TRUE. | ||
|
|
||
| This publicly exposes the Admin UI at "/admin" | ||
|
|
||
| RECOMMENDATION: | ||
| 1. For production, ensure this endpoint is restricted via NetworkPolicies, | ||
| IP whitelisting, or an OAuth2 proxy. This endpoint is normally not internet-facing. | ||
| 2. This can be left public in demo or internal development environments only. | ||
|
|
||
| ******************************************************************************** | ||
| {{- end }} | ||
moabu marked this conversation as resolved.
Show resolved
Hide resolved
|
||
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.
🧩 Analysis chain
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 740
🏁 Script executed:
Repository: GluuFederation/flex
Length of output: 2718
Perfect! I've verified the configuration. The
charts/gluu-all-in-one/values.yamlfile at line 384 confirms thatadmin-ui.ingress.adminUiEnabledis set totrue, which aligns with the template conditional and the PR objectives.Fix the hyphenation on line 11; values.yaml configuration verified as correct.
The Helm conditional template syntax is correct and properly references
admin-ui.ingress.adminUiEnabled, which is confirmed to be set totruein the values.yaml file. However, apply this diff to fix the grammar:📝 Committable suggestion
🧰 Tools
🪛 LanguageTool
[grammar] ~11-~11: Use a hyphen to join words.
Context: .... This endpoint is normally not internet facing. 2. This can be left public in de...
(QB_NEW_EN_HYPHEN)
🤖 Prompt for AI Agents