fix(admin-ui): Enable admin-ui ingress by default #2461
Conversation
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
📝 WalkthroughWalkthroughEnabled the Admin UI ingress by default in Helm values and added conditional security warning blocks to two Helm NOTES templates that emit a notice when the Admin UI ingress is enabled. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mo-auto
added
comp-charts-flex
misba7
changed the title
| ingress: | ||
| # -- Enable Admin UI endpoints in either istio or nginx ingress depending on users choice | ||
| adminUiEnabled: false | ||
| adminUiEnabled: true |
There was a problem hiding this comment.
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 helm install / helm upgrade is run.
There was a problem hiding this comment.
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 templates/NOTES.txt in both charts with the following message
{{- $adminUiEnabled := false -}}
{{- if and .Values.global (index .Values.global "admin-ui") -}}
{{- if (index .Values.global "admin-ui" "ingress" "adminUiEnabled") -}}
{{- $adminUiEnabled = true -}}
{{- end -}}
{{- end -}}
{{- if $adminUiEnabled }}
********************************************************************************
*** SECURITY CONFIGURATION WARNING ***
********************************************************************************
The flag `global.admin-ui.ingress.adminUiEnabled` is set to TRUE.
This exposes the Admin UI at: /admin-ui
RECOMMENDATION:
1. Use this setting ONLY for demo or internal development environments.
2. For production, ensure this endpoint is restricted via NetworkPolicies,
IP whitelisting, or an OAuth2 proxy. This endpoint is normally not internet facing.
********************************************************************************
{{- end }}
|
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
charts/gluu-all-in-one/templates/NOTES.txt(1 hunks)charts/gluu/templates/NOTES.txt(1 hunks)
🧰 Additional context used
🪛 LanguageTool
charts/gluu-all-in-one/templates/NOTES.txt
[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)
charts/gluu/templates/NOTES.txt
[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)
| {{ 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 }} No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find values.yaml files in the charts directory
fd -t f "values.yaml" charts/Repository: GluuFederation/flex
Length of output: 740
🏁 Script executed:
# Read the values.yaml file
cat -n charts/gluu-all-in-one/values.yaml | grep -A 10 -B 2 "admin-ui"Repository: GluuFederation/flex
Length of output: 2718
Perfect! I've verified the configuration. The charts/gluu-all-in-one/values.yaml file at line 384 confirms that admin-ui.ingress.adminUiEnabled is set to true, 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 to true in the values.yaml file. However, apply this diff to fix the grammar:
- IP whitelisting, or an OAuth2 proxy. This endpoint is normally not internet facing.
+ IP whitelisting, or an OAuth2 proxy. This endpoint is normally not internet-facing.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{ 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 }} | |
| {{ 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 }} |
🧰 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
In charts/gluu-all-in-one/templates/NOTES.txt around lines 1 to 15, fix the
hyphenation on line 11 by replacing any occurrence of "Admin-UI" with "Admin UI"
(remove the hyphen) so the phrase reads consistently as "Admin UI" in the
notice.
|
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Signed-off-by: Mohammad Abudayyeh <47318409+moabu@users.noreply.github.com>
3 participants
closes #2460
Summary by CodeRabbit
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.