Conversation
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
Contributor
📝 WalkthroughWalkthroughExpanded OpenBanking Helm values with many new configuration keys (tolerations, topologySpreadConstraints, pdb, lifecycle, customScripts, gatewayApi, auth-server-key-rotation, migration, SSL/DB SSL fields, persistence/cleanup, annotations, resource adjustments). Installation docs updated with revised resource table and added job services. Changes
Sequence Diagram(s)(Skipped — changes are configuration-heavy and do not introduce new multi-component control flow requiring a sequence diagram.) Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
charts/gluu/openbanking-values.yaml (2)
440-450:⚠️ Potential issue | 🟡 Minorconfig-api memory bumped to 1200Mi — verify the docs match.
The config-api memory limit and request are both set to
1200Mihere, but the documentation table indocs/openbanking/install-cn.md(Line 16) lists config-api RAM as1GB. These should be consistent.🤖 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 440 - 450, The config-api memory in the helm values (resources.limits.memory and resources.requests.memory under the config-api block) is set to 1200Mi but the docs table still lists it as 1GB; make them consistent by updating the docs entry for config-api RAM from "1GB" to "1200Mi" (or alternatively change both resources.limits.memory and resources.requests.memory to "1Gi" if you prefer using Gi units), and ensure the doc includes the note that Java memory uses Mi units if that constraint remains.
460-467:⚠️ Potential issue | 🔴 CriticalFix: Missing leading
/in config-api readiness probe path.Line 463 uses
path: jans-config-api/api/v1/health/readybut the liveness probe on line 455 correctly usespath: /jans-config-api/api/v1/health/live. The missing leading slash will cause the readiness probe to target an incorrect path, potentially leaving the pod in a non-ready state.Proposed fix
readinessProbe: # -- http readiness probe endpoint httpGet: - path: jans-config-api/api/v1/health/ready + path: /jans-config-api/api/v1/health/ready port: 8074🤖 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 460 - 467, The readinessProbe configuration (readinessProbe.httpGet.path) uses "jans-config-api/api/v1/health/ready" without a leading slash which makes the probe target the wrong endpoint; update the readinessProbe.httpGet.path to include the leading slash (i.e., "/jans-config-api/api/v1/health/ready") to match the liveness probe and ensure proper readiness checks for the jans-config-api container.
🤖 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 102-104: The file mixes empty-collection formatting (e.g.,
additionalLabels: { } vs additionalAnnotations: {}, pullSecrets: [ ] vs
customScripts: []); standardize to a single style across the file (choose either
"{}" and "[]" or "{ }" and "[ ]") and replace all occurrences accordingly —
update additionalLabels, additionalAnnotations, pullSecrets, customScripts and
the other flagged blocks (around the regions corresponding to lines 372-374,
481-483, 963-965) so the entire YAML uses the chosen empty-collection format
consistently.
- Around line 3-19: The comments under topologySpreadConstraints currently look
like children of an empty map (topologySpreadConstraints: {}), which is
misleading and triggers linters; update the YAML by either changing
topologySpreadConstraints to the block form (topologySpreadConstraints: | or
better: topologySpreadConstraints: # with no inline {}) and then place the
example/commented entries directly below as intended, or move the explanatory
comment block above the topologySpreadConstraints key; apply the same change for
the config-api section that repeats the pattern, referencing the
topologySpreadConstraints and config-api keys so reviewers can locate the spots
to fix.
In `@docs/openbanking/install-cn.md`:
- Around line 8-16: The table row for the "config-api" service is inconsistent
with the deployment values: update the RAM column for the config-api entry in
the markdown table from "1GB" to "1.2GB" to match openbanking-values.yaml (which
sets memory request/limit to 1200Mi), or alternatively change
openbanking-values.yaml to use 1000Mi if you intend the table to be
authoritative; ensure the "config-api" row and openbanking-values.yaml memory
settings are aligned.
---
Outside diff comments:
In `@charts/gluu/openbanking-values.yaml`:
- Around line 440-450: The config-api memory in the helm values
(resources.limits.memory and resources.requests.memory under the config-api
block) is set to 1200Mi but the docs table still lists it as 1GB; make them
consistent by updating the docs entry for config-api RAM from "1GB" to "1200Mi"
(or alternatively change both resources.limits.memory and
resources.requests.memory to "1Gi" if you prefer using Gi units), and ensure the
doc includes the note that Java memory uses Mi units if that constraint remains.
- Around line 460-467: The readinessProbe configuration
(readinessProbe.httpGet.path) uses "jans-config-api/api/v1/health/ready" without
a leading slash which makes the probe target the wrong endpoint; update the
readinessProbe.httpGet.path to include the leading slash (i.e.,
"/jans-config-api/api/v1/health/ready") to match the liveness probe and ensure
proper readiness checks for the jans-config-api container.
Signed-off-by: Amro Misbah <amromisba7@gmail.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
docs/openbanking/install-cn.md (1)
16-16:⚠️ Potential issue | 🟡 Minorconfig-api RAM in the table still doesn't match the values file.
Line 16 lists config-api RAM as
1GB, butopenbanking-values.yamlsets memory limit and request to1200Mi(~1.2 GB). This discrepancy was flagged in a prior review and remains unresolved.✏️ Suggested fix (if values file is authoritative)
-| config-api | 1.2 | 1GB | N/A | 64 Bit | No | +| config-api | 1.2 | 1.2GB | N/A | 64 Bit | No |🤖 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 16, Update the docs table to match the authoritative memory settings in openbanking-values.yaml: change the config-api RAM entry from "1GB" to "1200Mi" (or an equivalent "1.2GB" label if you prefer human-readable units) so the table entry for "config-api" aligns with the memory request/limit defined in openbanking-values.yaml; ensure the same unit format is used across the table for consistency.
🤖 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`:
- Around line 11-14: Normalize the service-name formatting by removing the
spaces around the dash so all rows use the same convention as auth-key-rotation;
update the entries "config - job", "persistence - job", and "cleanup - job" to
"config-job", "persistence-job", and "cleanup-job" respectively so they match
"auth-key-rotation".
- Line 13: Update the service name in the table entry from the shortened alias
auth-key-rotation to the canonical name auth-server-key-rotation so it matches
the Helm chart and values files; locate the table row containing
"auth-key-rotation" and replace that token with "auth-server-key-rotation" to
ensure consistency with the rest of the docs and chart references.
---
Duplicate comments:
In `@docs/openbanking/install-cn.md`:
- Line 16: Update the docs table to match the authoritative memory settings in
openbanking-values.yaml: change the config-api RAM entry from "1GB" to "1200Mi"
(or an equivalent "1.2GB" label if you prefer human-readable units) so the table
entry for "config-api" aligns with the memory request/limit defined in
openbanking-values.yaml; ensure the same unit format is used across the table
for consistency.
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)
charts/gluu/openbanking-values.yaml (1)
463-467:⚠️ Potential issue | 🟡 MinorMissing leading slash in
readinessProbepath.The
readinessProbe.httpGet.pathis missing a leading/. Compare with Line 455 (livenessProbe) which correctly uses/jans-config-api/api/v1/health/live. This could cause the readiness check to fail.🔧 Proposed fix
readinessProbe: # -- http readiness probe endpoint httpGet: - path: jans-config-api/api/v1/health/ready + path: /jans-config-api/api/v1/health/ready port: 8074🤖 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 463 - 467, The readinessProbe.httpGet.path is missing a leading slash which can break the probe; update the readinessProbe (the httpGet.path value currently "jans-config-api/api/v1/health/ready") to include the leading slash "/jans-config-api/api/v1/health/ready" so it matches the livenessProbe path format and functions 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 16: The config-api entry in the markdown table is inconsistent with
openbanking-values.yaml: update the config-api row (the table cell showing CPU
and RAM) to match the values file by changing CPU from "1.2" to "1.0" (or "1.0
CPU"/"1000m" style used elsewhere) and change RAM from "1GB" to "1.2GB" (or
"1200Mi") so the config-api table entry matches the CPU limit/request `1000m`
and memory `1200Mi` defined in the values file.
---
Outside diff comments:
In `@charts/gluu/openbanking-values.yaml`:
- Around line 463-467: The readinessProbe.httpGet.path is missing a leading
slash which can break the probe; update the readinessProbe (the httpGet.path
value currently "jans-config-api/api/v1/health/ready") to include the leading
slash "/jans-config-api/api/v1/health/ready" so it matches the livenessProbe
path format and functions correctly.
| | auth-server-key-rotation - job | 0.3 | 0.3GB | N/A | 64 Bit | No [Strongly recommended] | | ||
| | cleanup - job | 0.3 | 0.3GB | N/A | 64 Bit | No [Strongly recommended] | | ||
| | nginx | 1 | 1GB | N/A | 64 Bit | No | | ||
| | config-api | 1.2 | 1GB | N/A | 64 Bit | No | |
Contributor
There was a problem hiding this comment.
config-api CPU and RAM values in table don't match the values file.
Line 16 lists config-api as 1.2 CPU and 1GB RAM, but openbanking-values.yaml (Lines 443-450) sets:
- CPU limit/request:
1000m(1.0 CPU, not 1.2) - Memory limit/request:
1200Mi(~1.2GB, not 1GB)
Update the table to reflect the actual values file settings.
✏️ Suggested fix
-| config-api | 1.2 | 1GB | N/A | 64 Bit | No |
+| config-api | 1 | 1.2GB | N/A | 64 Bit | No |🤖 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 16, The config-api entry in the
markdown table is inconsistent with openbanking-values.yaml: update the
config-api row (the table cell showing CPU and RAM) to match the values file by
changing CPU from "1.2" to "1.0" (or "1.0 CPU"/"1000m" style used elsewhere) and
change RAM from "1GB" to "1.2GB" (or "1200Mi") so the config-api table entry
matches the CPU limit/request `1000m` and memory `1200Mi` defined in the values
file.
moabu
approved these changes
Mar 2, 2026
|
This was referenced Mar 3, 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 #2673
Summary by CodeRabbit
New Features
Documentation
Chores