Skip to content

fix(helm): stack-owned keycloak-internal Service so nginx survives Keycloak pod rolls - #1677

Open
doppelc wants to merge 1 commit into
agentic-community:mainfrom
doppelc:fix/helm-keycloak-internal-service
Open

fix(helm): stack-owned keycloak-internal Service so nginx survives Keycloak pod rolls#1677
doppelc wants to merge 1 commit into
agentic-community:mainfrom
doppelc:fix/helm-keycloak-internal-service

Conversation

@doppelc

@doppelc doppelc commented Aug 21, 2026

Copy link
Copy Markdown
Contributor

The charts set KEYCLOAK_URL to the Bitnami Keycloak headless Service. The registry pod's nginx interpolates it into a literal proxy_pass and resolves the hostname once at startup — and a headless Service's A record is the Keycloak pod IP itself. When the pod is replaced (upgrade, node roll, OOM), nginx keeps proxying to the dead IP: every /realms/* request (login, OIDC discovery, token mint) returns 503 until the registry pod is manually restarted. KEYCLOAK_URL is consumed via envFrom, snapshotted at pod start, so even a corrected Secret needs a pod roll to take effect.

Reproduction (defaults, bundled Keycloak)

  1. helm install the stack chart with defaults.
  2. kubectl delete pod <release>-keycloak-0; wait for the replacement to become ready.
  3. curl <registry>/realms/<realm>/.well-known/openid-configuration → 503, indefinitely.
  4. kubectl rollout restart the registry deployment → 200 again.

Fix

A stack-owned ClusterIP Service (<release>-kc-int, port 8080) in front of the Keycloak pods — a stable virtual IP that kube-proxy retargets to the current pod on replacement. All four keycloakInternalUrl helpers (stack, registry, auth-server, keycloak-configure) render that same release-scoped constant, and a checksum/keycloak-internal-url pod annotation on registry and auth-server rolls exactly the pods that consume the URL via envFrom when it changes.

Design notes

  • Stack-owned rather than derived from the Bitnami Service: the Bitnami Service name follows nameOverride/fullnameOverride and its port list is replaceable user values, so a derived URL can be silently broken by overrides or helm upgrade --reuse-values. Owning the Service fixes name and port where no values override can reach.
  • Port 8080 stays explicit: HTTP clients omit default ports from the Host header, Keycloak derives issuer/discovery URLs from Host, and a portless issuer breaks auth-server's exact-match issuer allowlist and discovery rewrite.
  • No truncation: Helm caps release names at 53 chars and 53 + 7 fits the 63-char DNS label, so the name is unique per release by construction. Max-length tests pin this, including two 53-char names sharing a prefix.
  • Fail closed: configurations that would leave the Service with no endpoints (keycloak.podLabels/commonLabels overriding a selector label, keycloak.namespaceOverride) are rejected at render time.
  • Verified on a live EKS cluster: deleted the bundled Keycloak pod; its IP changed when the StatefulSet replaced it, and the same registry pod — no restart, nginx untouched — served /realms/<realm>/.well-known/openid-configuration with 200 as soon as the replacement was ready.

@aarora79
aarora79 requested a review from omrishiv August 25, 2026 01:29
@aarora79 aarora79 added this to the 1.31.0 - Registry Copilot milestone Aug 25, 2026
@omrishiv

Copy link
Copy Markdown
Contributor

@doppelc, thanks for opening this. Is there a reason we can't switch to the non headless service? I see in the design choices you've called out that we don't own the release name. That's true, but when deploying the stack, we know can figure out what it will be called and we can override the KEYCLOAK_URL. additionally, we are preparing to move off of the bitnami stack.

My hesitation with the way proposed is that it will cause the registry to restart on keycloak restarts, which I understand fixes the issue you're experiencing, but I believe we have a cleaner way to do this. what do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants