fix(helm): infer Keycloak DB driver from external database URL - #42076
Conversation
With postgresql.enabled: false and a user-supplied APPSMITH_KEYCLOAK_DB_URL, the ConfigMap resolved APPSMITH_KEYCLOAK_DB_DRIVER to h2, so Keycloak silently persisted SSO data to a local H2 file inside the pod while Temporal and the AI DB did use the external PostgreSQL — a partial, inconsistent externalization. Verified on a live deployment: Keycloak boots with jdbc-h2 and KC_DB=dev-file, and creates no tables in the external database. Treat a non-empty APPSMITH_KEYCLOAK_DB_URL as PostgreSQL regardless of the subchart flag. The h2 fallback remains for deployments with no PostgreSQL at all, and an explicitly set driver is honored unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
WalkthroughThe Helm chart now emits the Keycloak PostgreSQL driver only when PostgreSQL configuration is detected. Helm tests cover default, omitted, and explicit driver cases. The chart version increases to ChangesKeycloak driver configuration
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When postgresql.enabled is false and no APPSMITH_KEYCLOAK_DB_URL is present in applicationConfig, stop rendering APPSMITH_KEYCLOAK_DB_DRIVER (previously "h2") and leave the decision to the container entrypoint. The URL may reach the pod through secretName/secrets/externalSecrets, which this template cannot inspect; the entrypoint sees the final resolved environment and can infer the driver from the URL, the same way Temporal and RTS already do. With an image that predates the runtime inference, an absent driver falls back to dev-file — the same behavior as the previous explicit h2 — so no chart/image ordering is required. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
deploy/helm/tests/keycloak_db_driver_test.yaml (1)
38-48: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse a non-default explicit driver in this test.
postgresqlis also the inferred value. This test passes if the template overwrites an explicit driver with the default. SetAPPSMITH_KEYCLOAK_DB_DRIVERand the expected value todev-fileto verify that explicit values remain unchanged.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@deploy/helm/tests/keycloak_db_driver_test.yaml` around lines 38 - 48, Update the test case “explicitly set driver is honored unchanged” to set APPSMITH_KEYCLOAK_DB_DRIVER to dev-file and assert that data.APPSMITH_KEYCLOAK_DB_DRIVER equals dev-file, while preserving the disabled PostgreSQL configuration and external database URL.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@deploy/helm/tests/keycloak_db_driver_test.yaml`:
- Around line 38-48: Update the test case “explicitly set driver is honored
unchanged” to set APPSMITH_KEYCLOAK_DB_DRIVER to dev-file and assert that
data.APPSMITH_KEYCLOAK_DB_DRIVER equals dev-file, while preserving the disabled
PostgreSQL configuration and external database URL.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d1fa85da-b33f-4605-9837-4fb9f2d60ed6
⛔ Files ignored due to path filters (1)
deploy/helm/tests/__snapshot__/defaults_snapshot_test.yaml.snapis excluded by!**/*.snap
📒 Files selected for processing (3)
deploy/helm/Chart.yamldeploy/helm/templates/configMap.yamldeploy/helm/tests/keycloak_db_driver_test.yaml
Description
Fixes https://linear.app/appsmith/issue/APP-15798
With
postgresql.enabled: falseand a user-suppliedapplicationConfig.APPSMITH_KEYCLOAK_DB_URL, the ConfigMap resolvedAPPSMITH_KEYCLOAK_DB_DRIVERtoh2— the ternary keys off the subchart flag, not the URL. The container entrypoint only reads the Keycloak DB URL when the driver ispostgresql, so Keycloak silently persisted all SSO data (realms, users, OIDC/SAML clients, sessions) to a local H2 file inside the pod, while Temporal and the AI DB did use the external PostgreSQL. The deployment looks healthy but the externalization is partial: SSO data is lost on pod restart, and each HA replica would have its own H2 file.This is the path of the careful user — following the external PostgreSQL guide and correctly disabling the bundled subchart is exactly what triggers it.
Fix
Treat a non-empty
APPSMITH_KEYCLOAK_DB_URLas PostgreSQL regardless of the subchart flag:postgresql.enabledtruepostgresqlpostgresql(unchanged)truepostgresqlpostgresql(unchanged)falseh2postgresql✅falseh2An explicitly set
APPSMITH_KEYCLOAK_DB_DRIVERis honored unchanged in all cases.The last row covers URLs supplied through
secretName/secrets/externalSecrets, which this template cannot inspect: instead of pinningh2, the chart leaves the key unset so the entrypoint can infer the driver from the final resolved environment (the same presence-based pattern Temporal and RTS already use with this variable). A companion EE entrypoint PR adds that inference; with an older image, an absent driver falls back todev-file— identical to the previous explicith2— so no chart/image ordering is required.Upgrade note
Deployments in the fixed cohort (
postgresql.enabled: false, external URL set, driver unset) switch Keycloak from pod-local H2 to the external PostgreSQL on upgrade. Keycloak bootstraps a fresh schema there — state accumulated in the local H2 file (realms, users, clients) does not migrate. That state was already ephemeral (lost on every pod reschedule), which is what this fix addresses. Two related behavior changes:APPSMITH_KEYCLOAK_DB_DRIVERexplicitly bypasses the inference entirely, and is the escape hatch for anyone who wants the old behavior.Validation
helm unittest: 96 tests / 20 suites / 11 snapshots green, including a newtests/keycloak_db_driver_test.yamlcovering the four rows of the matrix above; chart-version snapshots regenerated for 3.9.1.v2.2image): before the fix, Keycloak boots withjdbc-h2/KC_DB=dev-fileand creates zero tables in the external database; with driverpostgresql, Keycloak creates its ~94 tables there. Temporal/AI DB used the external URL in both cases.helm lintpasses.Automation
/ok-to-test tags="@tag.Sanity"
🔍 Cypress test results
Tip
🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/31744829943
Commit: c90ebf2
Cypress dashboard.
Tags:
@tag.SanitySpec:
Thu, 13 Aug 2026 21:47:38 UTC
Communication
Should the DevRel and Marketing teams inform users about this change?
🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Chores