fix(platform): route OIDC backend calls through the in-cluster Keycloak by default - #3289
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe platform now defaults OIDC backend requests to the in-cluster Keycloak URL. Dashboard and Linstor GUI gatekeepers support default, explicit, and empty-string URL modes, with expanded rendering tests and stronger end-to-end rollout validation. ChangesOIDC internal Keycloak routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AuthProxy
participant KeycloakService
participant ExternalIssuer
AuthProxy->>KeycloakService: Backend token and discovery-related requests
AuthProxy->>ExternalIssuer: Browser-facing redirects and issuer URLs
AuthProxy->>KeycloakService: Use internal URL with discovery skipped
Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
…ak by default authentication.oidc.keycloakInternalUrl defaulted to an empty string, which left oauth2-proxy performing OIDC discovery against the external Keycloak hostname. That request resolves through public DNS and lands on the root ingress controller, which does not serve yet on a fresh install: the root tenant's IngressClass only exists once tenant-root is ready, and tenant-root is still reconciling while the dashboard starts. Discovery has nothing to talk to, the gatekeeper fails, and the dashboard never comes up. The bundled Keycloak is the only supported provider — the external hostname is templated as keycloak.<host> and the realm is fixed — so backend-to-backend calls have no reason to leave the cluster. Default the value to http://keycloak-http.cozy-keycloak.svc:8080/realms/cozy, the same Service address keycloak-configure already uses. The dashboard and linstor-gui gatekeepers then run with --skip-oidc-discovery and send token, jwks, userinfo and logout requests to the Service, while browser redirects stay on the external host. Those calls travel as plaintext HTTP between pods, as keycloak-configure's admin calls to the same address already do. The value remains overridable for a Keycloak that does not live at the standard in-cluster address, and setting it back to an empty string restores external discovery. Existing OIDC clusters that never set it get a gatekeeper rollout onto the internal path on upgrade. Enabling OIDC swaps the dashboard's token-proxy container for oauth2-proxy, so the E2E install now drives that swap to completion: it waits for the auth-proxy container to appear, then gates on the gatekeeper rollout. Readiness alone proves nothing there — the HelmRelease is still Ready from the token-proxy install, and the Deployment keeps the old ReplicaSet Available while a new pod crashloops, so only the rollout catches a regression back to external discovery. Assisted-By: Claude <noreply@anthropic.com> Signed-off-by: Aleksei Sviridkin <f@lex.la>
5f520d0 to
f24b563
Compare
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request optimizes OIDC authentication by routing backend-to-backend calls through the internal Keycloak Service by default. This change improves system stability during initial cluster deployment by decoupling the dashboard's readiness from the external ingress controller. The configuration remains fully backward compatible and overridable for custom setups. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on Gemini (@gemini-code-assist) comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request updates the default value of keycloakInternalUrl in the platform package to point to the in-cluster Keycloak Service address. This change allows backend-to-backend OIDC requests to bypass external DNS and root ingress, preventing dashboard startup failures on fresh installations. The PR also adds extensive unit tests for the platform, dashboard, and linstor-gui packages to verify this behavior, and updates the E2E installation tests to validate the rollout of the gatekeeper with the new default. The feedback suggests regenerating the values.schema.json file by running make generate to keep it in sync with the updated default value in values.yaml.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| # | ||
| # Override only for a KeyCloak that does not live at the standard in-cluster address. Setting it | ||
| # to an empty string restores full OIDC discovery against the external issuer. | ||
| keycloakInternalUrl: "http://keycloak-http.cozy-keycloak.svc:8080/realms/cozy" |
There was a problem hiding this comment.
Since the default value of keycloakInternalUrl has been updated in values.yaml, the corresponding values.schema.json file needs to be regenerated to stay in sync. Please run make generate to update the schema file.
myasnikovdaniil
left a comment
There was a problem hiding this comment.
LGTM.
Minimal, reversible fix — it flips exactly one default (keycloakInternalUrl → the in-cluster Keycloak Service) and rides template branches that already existed. Verified against the tree: value wiring (apps.yaml:92 → _cluster → both gatekeepers read index ... "keycloak-internal-url" | default "" and gate the in-cluster args in one {{- if }} block, "" escape hatch intact); the issuer stays external (KC_HOSTNAME pins the external host so the token iss still matches the unchanged --oidc-issuer-url); and the plaintext backend leg is already precedented by keycloak-configure hitting the same keycloak-http...svc:8080. helm-unittest plus the reworked e2e (which gates on rollout status, not a vacuous condition=ready) cover it. One nit inline; the website OIDC docs are the tracked follow-up.
| # | ||
| # Override only for a KeyCloak that does not live at the standard in-cluster address. Setting it | ||
| # to an empty string restores full OIDC discovery against the external issuer. | ||
| keycloakInternalUrl: "http://keycloak-http.cozy-keycloak.svc:8080/realms/cozy" |
There was a problem hiding this comment.
Nit: a one-line upgrade note here would make values.yaml self-documenting — flipping this default silently rolls existing OIDC gatekeepers once on upgrade; set it to "" to keep the previous external-discovery behaviour. (Already captured in the PR body / release note.)
What this PR does
Defaults
authentication.oidc.keycloakInternalUrltohttp://keycloak-http.cozy-keycloak.svc:8080/realms/cozyinstead of an empty string.With the empty default, oauth2-proxy performed OIDC discovery against the external Keycloak hostname. That request resolves through public DNS and lands on the root ingress controller, which does not serve yet on a fresh install: the root tenant's IngressClass only exists once
tenant-rootis ready, andtenant-rootis still reconciling while the dashboard starts. Discovery has nothing to talk to, the gatekeeper fails, and the dashboard never comes up — with no obvious diagnosis for a first-time installer.The bundled Keycloak is the only supported provider (the external hostname is templated as
keycloak.<host>and the realm is fixed), so backend-to-backend calls have no reason to leave the cluster. The new default is the same Service addresskeycloak-configurealready uses for its own backend calls. The dashboard and linstor-gui gatekeepers then run with--skip-oidc-discoveryand send token, jwks, userinfo and logout requests straight to the Service, while browser-facing redirects stay on the external host.Both gatekeeper templates already branch on the value being non-empty, so neither needed a change. The value stays overridable for a Keycloak that does not live at the standard in-cluster address, and setting it back to
""restores the previous external-discovery behaviour.Token validation is unaffected: Keycloak pins
KC_HOSTNAMEto the external URL, so tokens redeemed over the internal Service still carry the externalissand match the unchanged--oidc-issuer-url.Upgrade behaviour
Existing OIDC clusters that never set
keycloakInternalUrlpick up the new default on upgrade: their gatekeepers roll once and move to the internal path. Clusters that set the value explicitly — including to""— keep exactly what they configured.Security posture
The backend leg (token redemption, which carries the gatekeeper's Keycloak client secret, plus jwks/userinfo/logout) now travels as plaintext HTTP between pods rather than HTTPS to the ingress. This is stated in the
values.yamlcomment rather than left implicit. Thekeycloak-httpService has no TLS listener, so in-cluster HTTPS is not an available alternative, andkeycloak-configurealready sends Keycloak admin credentials over that identical address — anyone positioned to read this traffic can already reach a strictly more powerful secret. The externally reachable front door (browser to oauth2-proxy) is unchanged.Testing
helm-unittest coverage across all three affected charts: the default landing on the
_clusterchannel, both gatekeepers rendering the in-cluster backend URLs while keeping browser URLs external, an explicit override winning, the empty-string escape hatch still rendering the external-discovery shape, and OIDC-disabled clusters (now the common case of "OIDC off, internal URL set") ignoring the value entirely.The E2E OIDC case previously applied this exact URL as a manual workaround. It now sets only
oidc.enabledand asserts the consequence: it waits for theauth-proxycontainer to appear in the gatekeeper Deployment, then gates onkubectl rollout status. Readiness alone would prove nothing there — the HelmRelease is stillReadyfrom the token-proxy install, and withreplicas: 1andmaxUnavailable: 25%(which rounds down to 0) the Deployment keeps the old ReplicaSetAvailablewhile a new pod crashloops. Only the rollout catches a regression back to external discovery.Follow-up: the OIDC documentation in the website repo still frames
keycloakInternalUrlas an optional tweak "for self-signed certificates or restricted external access". It is now the default path and needs a separate docs PR.Closes #3288
Screenshots
No UI changes.
Release note
Summary by CodeRabbit
New Features
Bug Fixes