fix: simplify openbanking-values.yaml and fix docs#2692
Merged
Conversation
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
Contributor
📝 WalkthroughWalkthroughRemoved many nested defaults from Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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
area-documentation
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/openbanking/install-cn.md (1)
84-91:⚠️ Potential issue | 🟠 MajorAdd the missing
:afterglobal.This block is still invalid YAML, so users cannot enable
authServerProtectedTokenorauthServerProtectedRegisterby copying it from the install guide.Corrected example
global: auth-server: ingress: authServerProtectedToken: true authServerProtectedRegister: true🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openbanking/install-cn.md` around lines 84 - 91, The YAML sample is invalid because the top-level key "global" is missing its trailing colon; update the snippet so "global" becomes "global:" and ensure the nested keys "auth-server", "ingress", and the boolean flags "authServerProtectedToken" and "authServerProtectedRegister" remain correctly indented under it to produce valid YAML.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@charts/gluu/openbanking-values.yaml`:
- Around line 45-47: Move the adminUiEnabled flag out of the global.admin-ui
scope and keep it with the service-specific ingress configuration: add
adminUiEnabled under the service's ingress block (the existing ingress:
adminUiEnabled key) and remove any duplicate under global.admin-ui; follow the
chart pattern where global.<service>.enabled holds only the top-level enable
boolean and all other service-specific settings (like ingress.adminUiEnabled)
live under the service block so look for the ingress section and the
global.admin-ui entry to update accordingly.
- Around line 12-16: The two flags authServerProtectedToken and
authServerProtectedRegister are currently at the top-level ingress key but the
templates (auth-server-protected-ingress.yaml and
auth-server-protected-virtual-services.yaml) read them from
global.auth-server.ingress.*, so move these two keys under
global.auth-server.ingress (i.e., add them as children of global -> auth-server
-> ingress with correct YAML indentation) and remove the old top-level ingress
entries so the templates pick up the true values.
In `@docs/openbanking/install-cn.md`:
- Around line 126-149: The config job's secret mount is using wrong key names
and incorrect YAML nesting; update the config block (volumes and volumeMounts)
to define a single secret volume (e.g., name: web-https) with secret: ->
secretName: tls-ob-ca-certificates and items: listing key: tls.crt -> path:
web_https.crt and key: tls.key -> path: web_https.key, then reference that same
volume name in two volumeMounts with mountPath and subPath set to
/etc/certs/web_https.crt (subPath: web_https.crt) and /etc/certs/web_https.key
(subPath: web_https.key) so the cert and key are actually mounted; ensure
secretName, items, mountPath and subPath are properly nested under their parent
keys.
- Around line 67-79: The example YAML nests the hosts as a sibling of tls
instead of under the tls entry, so it won't produce
nginx-ingress.ingress.tls[0].hosts; fix the indentation so the tls block
contains secretName and its hosts list (i.e., move the second hosts key to be a
child of the tls array item alongside secretName) so the resulting shape matches
nginx-ingress.ingress.tls and nginx-ingress.ingress.hosts expectations.
- Around line 242-250: The docs currently use curl -k (in the TOKEN assignment
and the PATCH request) which disables server certificate validation; update both
examples (the TOKEN curl that sets TOKEN and the PATCH curl that updates
jans-auth-server config) to stop using -k and instead either remove the flag to
use the system trust store or replace it with --cacert /path/to/ca-bundle.pem
(and mention the placeholder CA path) so the examples validate the server cert
against the provisioned CA bundle.
---
Outside diff comments:
In `@docs/openbanking/install-cn.md`:
- Around line 84-91: The YAML sample is invalid because the top-level key
"global" is missing its trailing colon; update the snippet so "global" becomes
"global:" and ensure the nested keys "auth-server", "ingress", and the boolean
flags "authServerProtectedToken" and "authServerProtectedRegister" remain
correctly indented under it to produce valid YAML.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 927fd4e6-1bb6-4c14-9c6e-34a14038dabf
📒 Files selected for processing (2)
charts/gluu/openbanking-values.yamldocs/openbanking/install-cn.md
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/openbanking/install-cn.md (1)
78-83:⚠️ Potential issue | 🟡 MinorFix YAML syntax error: missing colon after
global.Line 78 is missing the required colon after
global, which will cause a YAML parse error if users copy this example.📝 Proposed fix
- global + global: auth-server:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openbanking/install-cn.md` around lines 78 - 83, YAML snippet has a syntax error: the mapping key "global" is missing its colon; update the snippet so "global" is a proper map key (add the ":" after global) and ensure the nested keys "auth-server", "ingress", "authServerProtectedToken", and "authServerProtectedRegister" remain indented under "global" so the YAML parses correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/openbanking/install-cn.md`:
- Line 118: Remove the trailing whitespace at the end of the line containing the
text "Inject OBIE certificates, keys and URI:" in the
docs/openbanking/install-cn.md file; edit that line to delete the extra space
character so the line ends exactly after the colon, then save the file to clear
the static analysis warning.
---
Outside diff comments:
In `@docs/openbanking/install-cn.md`:
- Around line 78-83: YAML snippet has a syntax error: the mapping key "global"
is missing its colon; update the snippet so "global" is a proper map key (add
the ":" after global) and ensure the nested keys "auth-server", "ingress",
"authServerProtectedToken", and "authServerProtectedRegister" remain indented
under "global" so the YAML parses correctly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dd9ee075-9f14-408f-9e15-4027f64a1c6c
📒 Files selected for processing (2)
charts/gluu/openbanking-values.yamldocs/openbanking/install-cn.md
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (3)
docs/openbanking/install-cn.md (3)
219-219:⚠️ Potential issue | 🟠 MajorReplace
-kwith proper certificate validation.The
-kflag disables server certificate validation. Use--cacert ca.crtinstead to properly validate the server certificate against the provisioned CA bundle.🔒 Proposed fix
-curl -k -X PATCH "https://demoexample.gluu.org/jans-config-api/api/v1/jans-auth-server/config" \ +curl --cacert ca.crt -X PATCH "https://demoexample.gluu.org/jans-config-api/api/v1/jans-auth-server/config" \🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openbanking/install-cn.md` at line 219, In the curl invocation string 'curl -k -X PATCH "https://demoexample.gluu.org/jans-config-api/api/v1/jans-auth-server/config"', remove the insecure '-k' option and replace it with a CA bundle option such as '--cacert ca.crt' (or '--cacert /path/to/provisioned-ca.pem') so the request performs proper server certificate validation against the provisioned CA; ensure '-k' is deleted and the new '--cacert' flag is added to the same curl command.
211-211:⚠️ Potential issue | 🟠 MajorReplace
-kwith proper certificate validation.The
-kflag disables server certificate validation, which undermines the security of the mTLS setup. The guide already instructs users to provision a CA bundle (line 107), so use--cacert ca.crtinstead to validate the server certificate.🔒 Proposed fix
-TOKEN=$(curl -s -k -u $TESTCLIENT:$TESTCLIENTSECRET https://demoexample.gluu.org/jans-auth/restv1/token -d "grant_type=client_credentials&scope=https://jans.io/oauth/jans-auth-server/config/properties.write" --cert obtransport.pem --key obtransport.key | grep -o '"access_token":"[^"]*' | cut -d'"' -f4) +TOKEN=$(curl -s --cacert ca.crt -u $TESTCLIENT:$TESTCLIENTSECRET https://demoexample.gluu.org/jans-auth/restv1/token -d "grant_type=client_credentials&scope=https://jans.io/oauth/jans-auth-server/config/properties.write" --cert obtransport.pem --key obtransport.key | grep -o '"access_token":"[^"]*' | cut -d'"' -f4)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openbanking/install-cn.md` at line 211, The curl invocation that sets TOKEN currently uses the insecure -k option; remove -k and use the CA bundle provisioned earlier by adding --cacert ca.crt to the curl command (keep the existing --cert obtransport.pem and --key obtransport.key and the same URL/parameters) so the server certificate is properly validated instead of being skipped.
120-120:⚠️ Potential issue | 🟡 MinorRemove trailing whitespace.
Static analysis flagged a trailing space at the end of this line.
📝 Proposed fix
-1. Inject OBIE certificates, keys and URI: +1. Inject OBIE certificates, keys and URI:🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/openbanking/install-cn.md` at line 120, Remove the trailing whitespace at the end of the line containing "Inject OBIE certificates, keys and URI:" so the line ends immediately after the colon; update the Markdown to remove that extra space and re-save (or run your editor/formatter to trim trailing whitespace) and optionally scan the rest of docs/openbanking/install-cn.md for any other trailing spaces.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@docs/openbanking/install-cn.md`:
- Line 219: In the curl invocation string 'curl -k -X PATCH
"https://demoexample.gluu.org/jans-config-api/api/v1/jans-auth-server/config"',
remove the insecure '-k' option and replace it with a CA bundle option such as
'--cacert ca.crt' (or '--cacert /path/to/provisioned-ca.pem') so the request
performs proper server certificate validation against the provisioned CA; ensure
'-k' is deleted and the new '--cacert' flag is added to the same curl command.
- Line 211: The curl invocation that sets TOKEN currently uses the insecure -k
option; remove -k and use the CA bundle provisioned earlier by adding --cacert
ca.crt to the curl command (keep the existing --cert obtransport.pem and --key
obtransport.key and the same URL/parameters) so the server certificate is
properly validated instead of being skipped.
- Line 120: Remove the trailing whitespace at the end of the line containing
"Inject OBIE certificates, keys and URI:" so the line ends immediately after the
colon; update the Markdown to remove that extra space and re-save (or run your
editor/formatter to trim trailing whitespace) and optionally scan the rest of
docs/openbanking/install-cn.md for any other trailing spaces.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e6496f59-cd18-40f3-8adb-b6ef02f7237c
📒 Files selected for processing (1)
docs/openbanking/install-cn.md
moabu
approved these changes
Mar 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
closes #2691
Summary by CodeRabbit
New Features
Documentation
Chores