Conversation
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis pull request refactors Open Banking signing configuration by renaming the signing alias parameter from "external" to "internal" and introducing explicit key ID fields. Changes span Helm charts, Terraform infrastructure code, configuration files, and documentation, with consistent updates to parameter names and descriptive text throughout. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 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 |
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 (3)
docs/openbanking/install-cn.md (3)
50-57:⚠️ Potential issue | 🟠 MajorFix YAML snippet: missing colon after
global.The snippet is invalid YAML as written.
✅ Suggested fix
- global + global:🤖 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 50 - 57, The YAML snippet is invalid because the top-level key `global` is missing a colon and proper mapping; fix it by adding a colon after `global` and ensure `auth-server` and its `ingress` children (including `authServerProtectedToken` and `authServerProtectedRegister`) are nested as a mapping under `global` with correct indentation so the keys become valid YAML mappings.
231-240:⚠️ Potential issue | 🟠 MajorFix indentation for
volumeMountssnippet.
mountPath/subPathare misindented, which makes the YAML invalid when copied.✅ Suggested fix
- - name: custom-scopes - mountPath: "/app/templates/scopes.ob.ldif" - subPath: scopes.ob.ldif + - name: custom-scopes + mountPath: "/app/templates/scopes.ob.ldif" + subPath: scopes.ob.ldif🤖 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 231 - 240, The YAML snippet under persistence has incorrect indentation for the volumeMounts block causing invalid YAML; update the indentation so that volumeMounts is aligned as a sibling of volumes (same level as volumes), and ensure the nested entries (name: custom-scopes, mountPath: "/app/templates/scopes.ob.ldif", subPath: scopes.ob.ldif) are properly indented under volumeMounts; verify the keys volumeMounts, mountPath and subPath are indented consistently with the volumes -> - name: custom-scopes block so the snippet parses correctly.
1-3:⚠️ Potential issue | 🟡 MinorUse H2 as the top-level heading.
Please change
# Cloud-Nativeto## Cloud-Nativeto follow the repo’s docs convention.
Based on learnings: In the GluuFederation/flex repository, Markdown documentation should use H2 headings (##) as the top-level heading convention instead of H1 (#).🤖 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 1 - 3, Change the top-level Markdown heading from H1 to H2 by replacing the line that reads "# Cloud-Native" with "## Cloud-Native" in the docs file; ensure the rest of the file keeps existing heading levels relative to this change so the document follows the repo convention of using H2 as the top-level heading.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@automation/rancher-partner-charts/questions.yaml`:
- Around line 50-56: The label for variable cnObInternalSigningAlias is
misleadingly marked "external"; update the user-facing label for
cnObInternalSigningAlias to indicate it is the internal alias (e.g., "Open
banking internal signing AS Alias") and add a trailing period to the description
text so the description ends with a period; ensure you modify the label and
description entries associated with cnObInternalSigningAlias.
- Around line 57-61: The default for global.cnObStaticSigningKeyKid is
inconsistent with the internal alias described; update the chart so the kid
default matches the internal alias (or replace both defaults with the same
placeholder) to avoid misconfiguration. Locate the variables
global.cnObStaticSigningKeyKid and the corresponding internal alias variable
(e.g., global.cnObStaticSigningKeyInternalAlias) in the questions.yaml and set
their default values to the same string (or a single shared placeholder) and
keep the description indicating they must be identical.
In `@charts/gluu/README.md`:
- Around line 452-457: The README is generated by helm-docs; do not edit it
directly—apply your changes to the chart source instead: update the
corresponding keys/descriptions for global.cnObExtSigningJwksCrt,
global.cnObExtSigningJwksKey, global.cnObExtSigningJwksKeyPassPhrase,
global.cnObExtSigningJwksUri, global.cnObInternalSigningAlias, and
global.cnObStaticSigningKeyKid in values.yaml and/or Chart.yaml annotations or
template comments, then re-run helm-docs to regenerate README.md so the changes
persist.
In `@charts/gluu/values.yaml`:
- Around line 1119-1129: Replace the hard-coded-looking signing ID values with
an obvious placeholder and ensure both keys remain identical: update
cnObStaticSigningKeyKid and cnObInternalSigningAlias to a clearly non-secret
placeholder (e.g. REPLACE_WITH_SIGNING_KID or <REPLACE_WITH_SIGNING_KID>) so
secret scanners won't flag it and users are prompted to provide their own;
ensure the two values match exactly after the change.
In `@docs/reference/kubernetes/helm-chart.md`:
- Line 442: The description for global.cnObInternalSigningAlias is incorrect (it
refers to an external signing kid); update the source helm chart values.yaml
comment for global.cnObInternalSigningAlias to describe it as the internal JKS
alias used for internal signing (internal-alias semantics), then re-run
helm-docs to regenerate docs so docs/reference/kubernetes/helm-chart.md is
updated; ensure you only change the values.yaml comment (not the generated
markdown) and confirm the regenerated entry for global.cnObInternalSigningAlias
reflects internal JKS alias semantics.
---
Outside diff comments:
In `@docs/openbanking/install-cn.md`:
- Around line 50-57: The YAML snippet is invalid because the top-level key
`global` is missing a colon and proper mapping; fix it by adding a colon after
`global` and ensure `auth-server` and its `ingress` children (including
`authServerProtectedToken` and `authServerProtectedRegister`) are nested as a
mapping under `global` with correct indentation so the keys become valid YAML
mappings.
- Around line 231-240: The YAML snippet under persistence has incorrect
indentation for the volumeMounts block causing invalid YAML; update the
indentation so that volumeMounts is aligned as a sibling of volumes (same level
as volumes), and ensure the nested entries (name: custom-scopes, mountPath:
"/app/templates/scopes.ob.ldif", subPath: scopes.ob.ldif) are properly indented
under volumeMounts; verify the keys volumeMounts, mountPath and subPath are
indented consistently with the volumes -> - name: custom-scopes block so the
snippet parses correctly.
- Around line 1-3: Change the top-level Markdown heading from H1 to H2 by
replacing the line that reads "# Cloud-Native" with "## Cloud-Native" in the
docs file; ensure the rest of the file keeps existing heading levels relative to
this change so the document follows the repo convention of using H2 as the
top-level heading.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
admin-ui/plugins/admin/__tests__/components/CustomScripts/script.test.jsautomation/janssen_helm_chart/non_janssen.yamlautomation/rancher-partner-charts/questions.yamlcharts/gluu/README.mdcharts/gluu/charts/config/templates/configmaps.yamlcharts/gluu/openbanking-values.yamlcharts/gluu/values.schema.jsoncharts/gluu/values.yamldocs/openbanking/install-cn.mddocs/reference/kubernetes/helm-chart.md
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
docs/reference/kubernetes/helm-chart.md (1)
442-447: Auto-generated file: verify changes were produced by helm-docs, not direct edits.The description at line 442 uses
"cnObStaticSigningKeyKid"in double quotes, which mirrors the same quoting style as the syntax error identified invariables.tfline 493. If the sourcecharts/gluu/values.yamlcomment was edited with the same unescaped-quotes pattern and this file was then regenerated, the helm-docs output would faithfully reproduce it. Please confirm:
- The source
values.yamlcomment forcnObInternalSigningAliasuses backticks (not double quotes) around the code identifier.- This file was regenerated via
helm-docsrather than edited directly.Based on learnings, the file
docs/reference/kubernetes/helm-chart.mdin the GluuFederation/flex repository is auto-generated using the helm-docs tool and should not be manually edited. Any documentation corrections must be made in the source helm chart files (values.yaml comments or templates) and then regenerated.#!/bin/bash # Description: Check whether values.yaml contains the updated description for cnObInternalSigningAlias # and whether it uses backticks or double-quotes around the identifier. # Expected: Description should use backticks around cnObStaticSigningKeyKid, not raw double quotes. fd -e yaml -p 'charts/gluu' -x grep -n 'cnObInternalSigningAlias' {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/kubernetes/helm-chart.md` around lines 442 - 447, The docs show cnObInternalSigningAlias referencing "cnObStaticSigningKeyKid" with raw double quotes; verify and fix the source chart comment for the variable cnObInternalSigningAlias so the identifier cnObStaticSigningKeyKid is wrapped in backticks (not double quotes) in the charts' values comment, then regenerate the markdown with helm-docs (do not edit docs/reference/kubernetes/helm-chart.md by hand) and commit the regenerated output; ensure the change is made in the source comment and that helm-docs was used to produce the updated docs.automation/rancher-partner-charts/questions.yaml (1)
50-56:⚠️ Potential issue | 🟡 MinorDescription still missing trailing period and backtick formatting around
cnObStaticSigningKeyKid.Both issues were flagged in the previous review round but remain unaddressed. The description on line 54 ends without a period, and the code identifier
cnObStaticSigningKeyKidis not wrapped in backticks (Rancher UI typically renders Markdown in descriptions).✏️ Proposed fix
- description: "Internal Java Keystore (JKS) alias used to locate the Open Banking private signing key. To ensure correct internal mapping, this string must identically match your cnObStaticSigningKeyKid" + description: "Internal Java Keystore (JKS) alias used to locate the Open Banking private signing key. To ensure correct internal mapping, this string must identically match your `cnObStaticSigningKeyKid`."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@automation/rancher-partner-charts/questions.yaml` around lines 50 - 56, Update the description string for variable global.cnObInternalSigningAlias so it ends with a period and wraps the identifier cnObStaticSigningKeyKid in backticks; specifically modify the description text in the questions.yaml entry for global.cnObInternalSigningAlias to read something like: "...must identically match your `cnObStaticSigningKeyKid`." ensuring the final character is a period and the code identifier is backticked.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@demos/terraform-gluu-flex-eks-fargate/variables.tf`:
- Around line 490-494: The description string for variable
"ob_internal_signing_alias" contains unescaped double quotes around
cnObStaticSigningKeyKid which breaks HCL parsing; update the description in the
variable block for ob_internal_signing_alias to avoid unescaped double
quotes—either replace the inner double quotes with backticks or escape them
(e.g., use `cnObStaticSigningKeyKid` or \"cnObStaticSigningKeyKid\") so the
description becomes a valid HCL string.
---
Duplicate comments:
In `@automation/rancher-partner-charts/questions.yaml`:
- Around line 50-56: Update the description string for variable
global.cnObInternalSigningAlias so it ends with a period and wraps the
identifier cnObStaticSigningKeyKid in backticks; specifically modify the
description text in the questions.yaml entry for global.cnObInternalSigningAlias
to read something like: "...must identically match your
`cnObStaticSigningKeyKid`." ensuring the final character is a period and the
code identifier is backticked.
In `@docs/reference/kubernetes/helm-chart.md`:
- Around line 442-447: The docs show cnObInternalSigningAlias referencing
"cnObStaticSigningKeyKid" with raw double quotes; verify and fix the source
chart comment for the variable cnObInternalSigningAlias so the identifier
cnObStaticSigningKeyKid is wrapped in backticks (not double quotes) in the
charts' values comment, then regenerate the markdown with helm-docs (do not edit
docs/reference/kubernetes/helm-chart.md by hand) and commit the regenerated
output; ensure the change is made in the source comment and that helm-docs was
used to produce the updated docs.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
automation/rancher-partner-charts/questions.yamldemos/terraform-gluu-flex-eks-fargate/variables.tfdocs/reference/kubernetes/helm-chart.md
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
charts/gluu/README.md (2)
355-355: Avoid editing helm-docs generated README.
Please update the chart sources (values.yaml comments/Chart.yaml/templates) and re-run helm-docs so this change persists.
Based on learnings: README.md files in Helm chart directories are auto-generated by helm-docs and should not be manually edited.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gluu/README.md` at line 355, The README.md change is auto-generated by helm-docs and should not be edited directly; instead modify the chart sources (update values.yaml comments, Chart.yaml, or templates such as the globals/values blocks shown in the diff) to include the desired documentation text/values, then re-run helm-docs to regenerate README.md so the change persists (ensure you update the entries referenced like the global parameters block and any service-specific sections such as auth-server, casa, config-api, etc., before regenerating).
452-457: Avoid editing helm-docs generated README.
Please update the chart sources (values.yaml comments/Chart.yaml/templates) and re-run helm-docs so these entries are regenerated.
Based on learnings: README.md files in Helm chart directories are auto-generated by helm-docs and should not be manually edited.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gluu/README.md` around lines 452 - 457, The README changes were made directly but this file is generated by helm-docs; revert manual edits and update the chart source comments instead: edit the values.yaml comments (and Chart.yaml/templates if needed) for the keys global.cnObExtSigningJwksCrt, global.cnObExtSigningJwksKey, global.cnObExtSigningJwksKeyPassPhrase, global.cnObExtSigningJwksUri, global.cnObInternalSigningAlias and global.cnObStaticSigningKeyKid to contain the correct descriptions/encoding notes, then re-run helm-docs to regenerate README.md so the entries are updated automatically.charts/gluu/openbanking-values.yaml (1)
380-390: Use explicit placeholders for the signing KID/alias defaults.
These values look like real identifiers; prefer a clear placeholder and keep both values identical.🧩 Suggested fix
- cnObStaticSigningKeyKid: "XkwIzWy44xWSlcWnMiEc8iq9s2G" - cnObInternalSigningAlias: "XkwIzWy44xWSlcWnMiEc8iq9s2G" + cnObStaticSigningKeyKid: "<CHANGE_ME_KID>" + cnObInternalSigningAlias: "<CHANGE_ME_KID>"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gluu/openbanking-values.yaml` around lines 380 - 390, The chart defaults use a real-looking KID/alias value for cnObStaticSigningKeyKid and cnObInternalSigningAlias; replace both with an explicit clear placeholder (e.g., YOUR_SIGNING_KEY_KID) and make sure the two values remain identical so internal alias matches the external KID; update only the default values for cnObStaticSigningKeyKid and cnObInternalSigningAlias in the values YAML.charts/gluu/values.yaml (1)
1119-1129: Use explicit placeholders for the signing KID/alias defaults.
The current values look like real identifiers and trigger secret scanners; use an obvious placeholder and keep both values identical.🧩 Suggested fix
- cnObStaticSigningKeyKid: "XkwIzWy44xWSlcWnMiEc8iq9s2G" - cnObInternalSigningAlias: "XkwIzWy44xWSlcWnMiEc8iq9s2G" + cnObStaticSigningKeyKid: "<CHANGE_ME_KID>" + cnObInternalSigningAlias: "<CHANGE_ME_KID>"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/gluu/values.yaml` around lines 1119 - 1129, Replace the real-looking default values for cnObStaticSigningKeyKid and cnObInternalSigningAlias with an obvious placeholder (e.g., "REPLACE_WITH_SIGNING_KID") and ensure both keys remain identical; update the values of cnObStaticSigningKeyKid and cnObInternalSigningAlias to the placeholder so secret scanners won’t flag them and operators will know to set real values during deployment.docs/reference/kubernetes/helm-chart.md (2)
442-447: Avoid editing helm-docs generated docs.
Update the source chart files and re-run helm-docs to regenerate these entries.
Based on learnings: The filedocs/reference/kubernetes/helm-chart.mdin the GluuFederation/flex repository is auto-generated using the helm-docs tool and should not be manually edited.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/kubernetes/helm-chart.md` around lines 442 - 447, The helm-chart.md entries are generated by helm-docs and should not be edited directly; instead update the source Helm chart values or annotations that define the fields (e.g., values for keys like global.cnObInternalSigningAlias, global.cnObExtSigningJwksCrt, global.cnObExtSigningJwksKey, global.cnObExtSigningJwksKeyPassPhrase, global.cnObExtSigningJwksUri, global.cnObStaticSigningKeyKid) in the chart YAML/templates, then re-run helm-docs to regenerate docs so the changes are reflected in docs/reference/kubernetes/helm-chart.md.
346-346: Avoid editing helm-docs generated docs.
Update the source chart files and re-run helm-docs to regenerate this documentation.
Based on learnings: The filedocs/reference/kubernetes/helm-chart.mdin the GluuFederation/flex repository is auto-generated using the helm-docs tool and should not be manually edited.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/reference/kubernetes/helm-chart.md` at line 346, The generated helm docs file (helm-chart.md) was edited directly; revert any manual changes there, update the source Helm chart values/templates (the chart values containing the global object and service entries like auth-server, casa, config-api, etc.), then re-run the helm-docs generation tool to regenerate the documentation so the file reflects the canonical chart sources rather than manual edits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@demos/terraform-gluu-flex-eks-fargate/variables.tf`:
- Line 499: The description string "External Key ID (kid) stamped onto the
header of outgoing JWTs. This tells receiving parties which public key to fetch
from your JWKS URI to verify the signature" is missing a terminal period; update
the description value in the variable block (the description field containing
that exact text) to end with a period so it matches the other variable
descriptions.
- Around line 490-494: The rename from ob_ext_signing_alias to
ob_internal_signing_alias is incomplete and will break Terraform; update any
references to the old symbol so they point to the new one (e.g., in
app-config.tf change the mapping that sets CN_OB_EXT_SIGNING_ALIAS to use
var.ob_internal_signing_alias, and in example/example-ob.tf rename the
assignment ob_ext_signing_alias = "" to ob_internal_signing_alias = ""); also
fix the description punctuation for the ob_static_signing_key_kid variable in
variables.tf by adding a terminal period to match file style.
---
Duplicate comments:
In `@charts/gluu/openbanking-values.yaml`:
- Around line 380-390: The chart defaults use a real-looking KID/alias value for
cnObStaticSigningKeyKid and cnObInternalSigningAlias; replace both with an
explicit clear placeholder (e.g., YOUR_SIGNING_KEY_KID) and make sure the two
values remain identical so internal alias matches the external KID; update only
the default values for cnObStaticSigningKeyKid and cnObInternalSigningAlias in
the values YAML.
In `@charts/gluu/README.md`:
- Line 355: The README.md change is auto-generated by helm-docs and should not
be edited directly; instead modify the chart sources (update values.yaml
comments, Chart.yaml, or templates such as the globals/values blocks shown in
the diff) to include the desired documentation text/values, then re-run
helm-docs to regenerate README.md so the change persists (ensure you update the
entries referenced like the global parameters block and any service-specific
sections such as auth-server, casa, config-api, etc., before regenerating).
- Around line 452-457: The README changes were made directly but this file is
generated by helm-docs; revert manual edits and update the chart source comments
instead: edit the values.yaml comments (and Chart.yaml/templates if needed) for
the keys global.cnObExtSigningJwksCrt, global.cnObExtSigningJwksKey,
global.cnObExtSigningJwksKeyPassPhrase, global.cnObExtSigningJwksUri,
global.cnObInternalSigningAlias and global.cnObStaticSigningKeyKid to contain
the correct descriptions/encoding notes, then re-run helm-docs to regenerate
README.md so the entries are updated automatically.
In `@charts/gluu/values.yaml`:
- Around line 1119-1129: Replace the real-looking default values for
cnObStaticSigningKeyKid and cnObInternalSigningAlias with an obvious placeholder
(e.g., "REPLACE_WITH_SIGNING_KID") and ensure both keys remain identical; update
the values of cnObStaticSigningKeyKid and cnObInternalSigningAlias to the
placeholder so secret scanners won’t flag them and operators will know to set
real values during deployment.
In `@docs/reference/kubernetes/helm-chart.md`:
- Around line 442-447: The helm-chart.md entries are generated by helm-docs and
should not be edited directly; instead update the source Helm chart values or
annotations that define the fields (e.g., values for keys like
global.cnObInternalSigningAlias, global.cnObExtSigningJwksCrt,
global.cnObExtSigningJwksKey, global.cnObExtSigningJwksKeyPassPhrase,
global.cnObExtSigningJwksUri, global.cnObStaticSigningKeyKid) in the chart
YAML/templates, then re-run helm-docs to regenerate docs so the changes are
reflected in docs/reference/kubernetes/helm-chart.md.
- Line 346: The generated helm docs file (helm-chart.md) was edited directly;
revert any manual changes there, update the source Helm chart values/templates
(the chart values containing the global object and service entries like
auth-server, casa, config-api, etc.), then re-run the helm-docs generation tool
to regenerate the documentation so the file reflects the canonical chart sources
rather than manual edits.
ℹ️ Review info
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (6)
admin-ui/plugins/admin/__tests__/components/CustomScripts/script.test.jscharts/gluu/README.mdcharts/gluu/openbanking-values.yamlcharts/gluu/values.yamldemos/terraform-gluu-flex-eks-fargate/variables.tfdocs/reference/kubernetes/helm-chart.md
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
|
| @@ -377,17 +377,17 @@ global: | |||
| # -- Persistence backend to run Gluu with hybrid|sql. | |||
| cnPersistenceType: sql | |||
| # -- Open banking external signing jwks uri. Used in SSA Validation. | |||



closes #2669
Summary by CodeRabbit
New Features
Documentation
Chores