Fix keycloak secret sync#123
Merged
Merged
Conversation
📝 WalkthroughWalkthroughIntroduces a normalized Keycloak HTTP relative path helper and updates templates to use it instead of hardcoded Changes
Sequence DiagramsequenceDiagram
participant Job as Keycloak Secret Sync Job
participant Keycloak as Keycloak Admin API
participant Kube as Kubernetes API
participant Deploy as Deployment Controller
Job->>Keycloak: Wait for readiness ({{relativePath}}/health/ready)
Keycloak-->>Job: Ready
Job->>Keycloak: Request admin token (master realm)
Keycloak-->>Job: Access token
Job->>Kube: Read `<release>-keycloak-client-secrets` Secret
Kube-->>Job: Secret data
loop For each client (hub-adapter, node-ui, service1)
Job->>Keycloak: Query client UUID & fetch client-secret
Keycloak-->>Job: Client secret
Job->>Job: Compare with Kubernetes Secret field
alt Mismatch detected
Job->>Kube: Patch Secret (merge patch)
Kube-->>Job: Patched
Job->>Job: Mark deployment restart needed
end
end
alt Restarts needed
Job->>Deploy: Patch deployments' pod-template annotation (restartedAt)
Deploy-->>Job: Rolling restarts triggered
else No restarts needed
Job-->>Job: Exit success
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
charts/flame-node/templates/ui/deployment.yaml (1)
85-86: Don't re-encode an explicitui.nuxtAuthSecret.The new value is documented as a caller-supplied secret, but this pipeline base64-encodes it again before injecting
NUXT_AUTH_SECRET. A value copied fromopenssl rand -base64 32therefore won't be used verbatim.Suggested change
- name: NUXT_AUTH_SECRET - value: {{ .Values.ui.nuxtAuthSecret | default (randAlphaNum 32) | b64enc }} + value: {{ default (randAlphaNum 32 | b64enc) .Values.ui.nuxtAuthSecret | quote }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/flame-node/templates/ui/deployment.yaml` around lines 85 - 86, The template currently always base64-encodes .Values.ui.nuxtAuthSecret which double-encodes caller-supplied base64 secrets; change the NUXT_AUTH_SECRET expression to use the provided .Values.ui.nuxtAuthSecret as-is when set, and only generate-and-b64encode the secret when no value is supplied. Replace the single-line expression using .Values.ui.nuxtAuthSecret, randAlphaNum 32 and b64enc with a conditional: if .Values.ui.nuxtAuthSecret then output .Values.ui.nuxtAuthSecret directly, else output randAlphaNum 32 piped to b64enc; keep references to NUXT_AUTH_SECRET, .Values.ui.nuxtAuthSecret and randAlphaNum to locate the change.
🤖 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/flame-node/templates/_helpers.tpl`:
- Around line 70-71: The helper "keycloak.base.endpoint" currently concatenates
.Values.keycloakx.http.relativePath directly which can produce double slashes
when relativePath is "/" or has trailing slashes; create a shared template
helper (e.g., "keycloak.normalize.relativePath") that normalizes
.Values.keycloakx.http.relativePath by converting "/" to empty and trimming any
leading/trailing slashes, then update "keycloak.base.endpoint" and the other
affected templates (including readiness URL helpers referenced elsewhere) to
call this normalizer and append the normalized path so joins never emit "//".
In `@charts/flame-node/templates/keycloak-secret-sync-job.yaml`:
- Around line 204-205: The sync job currently hardcodes client names
("hub-adapter" and "node-ui") when calling sync_client (refer to sync_client,
hubAdapterClientSecret, nodeUiClientSecret), which will mismatch installs that
override client IDs; update those calls to use the chart values for client IDs
(use .Values.hubAdapter.idp.clientId for the hub adapter and
.Values.ui.idp.clientId for the UI) so the job queries and patches the actual
clients into the {{ .Release.Name }}-keycloak-client-secrets; ensure you
reference the values in the same template expression style used elsewhere in the
chart and preserve the existing secret key names (hubAdapterClientSecret and
nodeUiClientSecret).
- Around line 34-39: The pod-orchestrator Deployment "{{ .Release.Name }}-po"
isn't included in the Role's resourceNames nor in the restart loop, so updates
to podOrcClientSecret won't trigger a rollout; update the Role (resourceNames
array that currently lists "{{ .Release.Name }}-hub-adapter-deployment" and "{{
.Release.Name }}-node-ui-deployment") to also include "{{ .Release.Name }}-po",
and extend the restart loop that currently patches the hub-adapter and node-ui
Deployments to also patch the "{{ .Release.Name }}-po" Deployment (i.e., apply
the same restart annotation/patch logic used for the other two deployments after
syncing podOrcClientSecret).
---
Nitpick comments:
In `@charts/flame-node/templates/ui/deployment.yaml`:
- Around line 85-86: The template currently always base64-encodes
.Values.ui.nuxtAuthSecret which double-encodes caller-supplied base64 secrets;
change the NUXT_AUTH_SECRET expression to use the provided
.Values.ui.nuxtAuthSecret as-is when set, and only generate-and-b64encode the
secret when no value is supplied. Replace the single-line expression using
.Values.ui.nuxtAuthSecret, randAlphaNum 32 and b64enc with a conditional: if
.Values.ui.nuxtAuthSecret then output .Values.ui.nuxtAuthSecret directly, else
output randAlphaNum 32 piped to b64enc; keep references to NUXT_AUTH_SECRET,
.Values.ui.nuxtAuthSecret and randAlphaNum to locate the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c059b8c9-7146-48dc-b9f9-40c01e803dfa
📒 Files selected for processing (6)
charts/flame-node/templates/_helpers.tplcharts/flame-node/templates/hub-adapter/deployment.yamlcharts/flame-node/templates/keycloak-secret-sync-job.yamlcharts/flame-node/templates/secret.yamlcharts/flame-node/templates/ui/deployment.yamlcharts/flame-node/values.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
charts/flame-node/templates/keycloak-secret-sync-job.yaml (1)
86-89: Pin the container image to a specific version.Using the
latesttag can lead to unpredictable behavior when the upstream image is updated. Pin to a specific digest or version tag for reproducibility and security.♻️ Suggested fix
- name: sync - image: curlimages/curl:latest + image: curlimages/curl:8.7.1 imagePullPolicy: IfNotPresent🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/flame-node/templates/keycloak-secret-sync-job.yaml` around lines 86 - 89, The container image for the "sync" container is pinned to the floating tag "latest"; update the image field in the containers spec for the container named "sync" to a specific version tag or digest (e.g., use curlimages/curl:<version> or curlimages/curl@sha256:<digest>) so the Job is reproducible and secure; replace the current image value in the containers -> - name: sync block with a fixed tag or digest and ensure imagePullPolicy remains appropriate for the chosen pin.charts/flame-node/templates/hub-adapter/deployment.yaml (1)
46-48: Pin the initContainer image to a specific version.Same as the sync job, using
latestcan lead to unpredictable behavior and should be pinned for reproducibility.♻️ Suggested fix
initContainers: - name: wait-for-keycloak - image: curlimages/curl:latest + image: curlimages/curl:8.7.1 imagePullPolicy: IfNotPresent🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@charts/flame-node/templates/hub-adapter/deployment.yaml` around lines 46 - 48, The initContainer "wait-for-keycloak" is using the mutable image tag "curlimages/curl:latest"; change the image field for that initContainer to a fixed, versioned tag or digest (for example a specific semver like curlimages/curl:8.1.2 or an SHA256 digest) to ensure reproducible deployments and avoid breaking changes from the latest tag; update the image value for the initContainer named wait-for-keycloak accordingly.
🤖 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/flame-node/templates/keycloak-secret-sync-job.yaml`:
- Around line 205-207: The keycloak secret-sync job currently uses a hardcoded
client ID "service1" in the sync_client call; add a configurable clientId for
the pod-orchestrator to follow the same pattern as hub-adapter and ui. Update
values.yaml to add podOrchestrator.idp.clientId with a sensible default, and
change the sync_client invocation in keycloak-secret-sync-job.yaml to reference
{{ .Values.podOrchestrator.idp.clientId | default "service1" | quote }} (replace
the hardcoded "service1"), ensuring the template uses the new
podOrchestrator.idp key consistently across charts.
---
Nitpick comments:
In `@charts/flame-node/templates/hub-adapter/deployment.yaml`:
- Around line 46-48: The initContainer "wait-for-keycloak" is using the mutable
image tag "curlimages/curl:latest"; change the image field for that
initContainer to a fixed, versioned tag or digest (for example a specific semver
like curlimages/curl:8.1.2 or an SHA256 digest) to ensure reproducible
deployments and avoid breaking changes from the latest tag; update the image
value for the initContainer named wait-for-keycloak accordingly.
In `@charts/flame-node/templates/keycloak-secret-sync-job.yaml`:
- Around line 86-89: The container image for the "sync" container is pinned to
the floating tag "latest"; update the image field in the containers spec for the
container named "sync" to a specific version tag or digest (e.g., use
curlimages/curl:<version> or curlimages/curl@sha256:<digest>) so the Job is
reproducible and secure; replace the current image value in the containers -> -
name: sync block with a fixed tag or digest and ensure imagePullPolicy remains
appropriate for the chosen pin.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0f68d94f-a53b-4ffc-b329-b1d1075b4ff8
📒 Files selected for processing (3)
charts/flame-node/templates/_helpers.tplcharts/flame-node/templates/hub-adapter/deployment.yamlcharts/flame-node/templates/keycloak-secret-sync-job.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/flame-node/templates/_helpers.tpl
Comment on lines
+205
to
+207
| sync_client {{ .Values.hubAdapter.idp.clientId | default "hub-adapter" | quote }} "hubAdapterClientSecret" | ||
| sync_client {{ .Values.ui.idp.clientId | default "node-ui" | quote }} "nodeUiClientSecret" | ||
| sync_client "service1" "podOrcClientSecret" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there's a configurable client ID for pod-orchestrator in values
echo "=== Checking for pod-orchestrator client ID config ==="
rg -n "podOrchestrator|pod-orchestrator|service1" charts/flame-node/values.yaml
echo ""
echo "=== Checking pod-orchestrator templates for client ID usage ==="
rg -n "clientId|service1" charts/flame-node/templates/pod-orchestrator/Repository: PrivateAIM/helm
Length of output: 295
🏁 Script executed:
# Check the values.yaml structure for all three components
echo "=== hubAdapter configuration ==="
rg -n -A 10 "hubAdapter:" charts/flame-node/values.yaml | head -30
echo ""
echo "=== ui configuration ==="
rg -n -A 10 "^ui:" charts/flame-node/values.yaml | head -30
echo ""
echo "=== podOrchestrator configuration ==="
rg -n -A 10 "podOrchestrator:" charts/flame-node/values.yaml | head -40Repository: PrivateAIM/helm
Length of output: 1456
🏁 Script executed:
# Get more detailed idp configurations
echo "=== hubAdapter full idp section ==="
rg -n -A 20 "^hubAdapter:" charts/flame-node/values.yaml | grep -A 20 "idp:"
echo ""
echo "=== ui full idp section ==="
rg -n -A 30 "^ui:" charts/flame-node/values.yaml | grep -A 30 "idp:"
echo ""
echo "=== podOrchestrator full idp section ==="
rg -n -A 20 "^podOrchestrator:" charts/flame-node/values.yaml | grep -A 20 "idp:"Repository: PrivateAIM/helm
Length of output: 2849
Add configurable client ID for pod-orchestrator to match hub-adapter and ui patterns.
Hub-adapter and ui define configurable idp.clientId values in their respective values sections (lines 390 and 696), but pod-orchestrator's idp section only contains debug: false without a clientId field. Line 207 in keycloak-secret-sync-job.yaml consequently uses a hardcoded "service1". Either add clientId to podOrchestrator.idp in values.yaml to match the configurable pattern, or document why it must remain hardcoded.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@charts/flame-node/templates/keycloak-secret-sync-job.yaml` around lines 205 -
207, The keycloak secret-sync job currently uses a hardcoded client ID
"service1" in the sync_client call; add a configurable clientId for the
pod-orchestrator to follow the same pattern as hub-adapter and ui. Update
values.yaml to add podOrchestrator.idp.clientId with a sensible default, and
change the sync_client invocation in keycloak-secret-sync-job.yaml to reference
{{ .Values.podOrchestrator.idp.clientId | default "service1" | quote }} (replace
the hardcoded "service1"), ensuring the template uses the new
podOrchestrator.idp key consistently across charts.
This was referenced May 22, 2026
Merged
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
1 participant
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.
Summary by CodeRabbit