fix(keycloak): readiness could not fail on the database, and the measured budget

Two corrections found while verifying the first CI run, not from the plan.

THE DEFAULT THAT MADE THE READINESS PROBE TOOTHLESS. Keycloak's DATABASE health
check is the one row of upstream's "Available Checks" table whose "Requires
Metrics" column says yes; Cluster, Graceful shutdown and Keycloak Initialized
say no. So with metrics-enabled=false - this chart's default until now -
/health/ready answers 200 while the database is gone, and the pod keeps taking
logins it cannot serve. The probe path was right and the aggregate behind it
contained nothing external, which is the urlaubsverwaltung defect (#218) in a
new hiding place: not a health group this time, but an option named for
observability, where nobody looks when reviewing a probe.

keycloak.metrics.enabled therefore defaults to TRUE, and its comment argues
about readiness rather than about scraping. The price is stated at the value:
/metrics then answers on the management port, unauthenticated, carrying realm
names, client ids and login counters - on a port that is ClusterIP-only and in
no Ingress. Switching it off is now documented as a decision about whether
readiness can fail at all. The CI fixture keeps it on and says why, so a green
run really does touch the database.

THE STARTUP BUDGET NOW CARRIES ITS MEASUREMENT. From the install-test of PR #87
on the Gitea docker runner (k3s v1.35.5, two replicas, empty database, image
pulled during the run): 71.8 seconds from `helm install` to BOTH pods Ready -
first at ~70.8s, second one second later, 0 restarts. The 600s budget stays,
with the reason for the headroom written down: everything in that run was the
cheap case, and a first install against a real database serializes the second
replica behind the first's Liquibase migration lock instead of letting it
arrive a second later.

AND AN OVERCLAIM OF MY OWN, REMOVED. ci/keycloak/test-values.yaml said a green
run proves jdbc-ping found the second replica. It does not: readiness reports
one node's own health, so a node that never found its peer is Ready, healthy
and alone - and two such nodes are two islands that each believe they are the
cluster. What the test does rule out is the louder half, a replica that cannot
start or cannot share the database. The Infinispan view has to be read out of
the log, which is what step 2 of NOTES.txt tells the operator to do, now with
the note that no probe and no install-test replaces that step. Measurement
point 4 of the plan therefore stays open.

Part of #83

README.md

@@ -22,7 +22,7 @@ Every chart has a wiki page of its own — `https://git.jk-dev.de/jlab-cluster/h
 | `garage` | Garage (S3-compatible object store) | **One** node, `replication_factor = 1` — a deliberate trade with its price named (Gitea #81): the volume layer underneath already replicates and encrypts, so Garage replication on top would be nine times the raw data across two layers that know nothing about each other. What that costs is that the factor applies to the **metadata** too, where a corrupted database is upstream-documented as "generally not recoverable" and a replicated volume replicates the corruption. Hence three settings that are mandatory rather than optional: `metadata_fsync = true` (**off** in Garage's defaults), the automatic metadata snapshots, and those snapshots on a **second PVC** — a file-system snapshot of a live metadata database is not a restore point, so the volume backup covers the blocks and not the index. The first PVC carries `metadata_dir` **and** `data_dir` as two subdirectories, because volume snapshots are atomic per volume: that is what keeps index and blocks consistent with each other. Two further properties shape the whole chart: the image is **`FROM scratch`** and contains exactly one file, `/garage` — no shell, so **exec probes are impossible here** and an init container would need a second image — and the RPC secret has to arrive as an **environment variable**, because `rpc_secret_file` rejects any file with a group bit set while the kubelet sets one (the reasoning sits at the value). Started with `garage server --single-node`, which creates the cluster layout in-process before any listener opens and is idempotent on the layout version, and with `--default-access-key`/`--default-bucket`, which **import** key and bucket from the 1Password item — so the vault is the source of truth instead of a copy of what the server generated. Readiness is the admin API's `/health`, liveness and startup deliberately are **not**: `/health` answers 503 until a layout exists, so a node waiting for one would be restarted forever |
 | `gotenberg` | Gotenberg | Deployment + Service only (no ingress) |
 | `homeassistant` | Home Assistant | StatefulSet; the only chart carrying Helm tests (`templates/tests/`) |
-| `keycloak` | Keycloak (OpenID Connect / SAML identity provider) | **Two** replicas with `cache=ispn`, clustered through Keycloak's default `jdbc-ping` stack — discovery runs over the PostgreSQL that is there anyway, so no headless Service and no JGroups wiring. Deployment, **no PVC** (Keycloak 26 keeps everything in the database, user sessions included). **The chart has two init containers and needs both** (Gitea #83): `db`, `health-enabled` and `metrics-enabled` are Keycloak *build* options, the published image is built with their defaults, and a runtime change makes Keycloak rewrite `/opt/keycloak/lib/quarkus` at start — which `readOnlyRootFilesystem: true` forbids and which upstream describes as a start that "creates an image that is mutable". An empty `emptyDir` over that path does not work either, because `QuarkusEntryPoint` loads `quarkus/quarkus-application.dat` before it dispatches any command, `build` included. So one init container **seeds** the directory into a volume, the next runs `kc.sh build` into it, and the serving container runs `start --optimized` — every container keeping the full hardening baseline. Build options therefore exist in exactly **one** place and a build option in `keycloak.env` fails the render. First chart with a `PodDisruptionBudget` (§11) and first `quay.io` image (hence the entry in `ci/trivy/data/ksv0125.yaml`). Two Services (8080, and the management port 9000 **never** published — it serves `/health/*` and optionally `/metrics` unauthenticated) and two Ingresses. **The admin hostname is not an access control**: `hostname-admin` only decides which URLs the server generates, and upstream states that it "does not prevent accessing the Administration REST API endpoints via the frontend URL" — restricting that API is reverse-proxy work, and the chart renders no `/admin` deny rule. `ingress.domain` is read as `KC_HOSTNAME` (as a full URL, which upstream requires alongside `hostname-admin`), so it stays required with the Ingress off. Liveness is `/health/live` and deliberately not `/health` or `/health/ready` — those include the database, and a liveness probe on them restarts the whole cluster for as long as the database is away. Two traps at the values: upstream's `db-pool-max-size` default of 100 would ask a default PostgreSQL for 200 connections across two replicas (the chart ships 20), and two releases sharing one database **merge into one Infinispan cluster** unless the cluster names differ |
+| `keycloak` | Keycloak (OpenID Connect / SAML identity provider) | **Two** replicas with `cache=ispn`, clustered through Keycloak's default `jdbc-ping` stack — discovery runs over the PostgreSQL that is there anyway, so no headless Service and no JGroups wiring. Deployment, **no PVC** (Keycloak 26 keeps everything in the database, user sessions included). **The chart has two init containers and needs both** (Gitea #83): `db`, `health-enabled` and `metrics-enabled` are Keycloak *build* options, the published image is built with their defaults, and a runtime change makes Keycloak rewrite `/opt/keycloak/lib/quarkus` at start — which `readOnlyRootFilesystem: true` forbids and which upstream describes as a start that "creates an image that is mutable". An empty `emptyDir` over that path does not work either, because `QuarkusEntryPoint` loads `quarkus/quarkus-application.dat` before it dispatches any command, `build` included. So one init container **seeds** the directory into a volume, the next runs `kc.sh build` into it, and the serving container runs `start --optimized` — every container keeping the full hardening baseline. Build options therefore exist in exactly **one** place and a build option in `keycloak.env` fails the render. First chart with a `PodDisruptionBudget` (§11) and first `quay.io` image (hence the entry in `ci/trivy/data/ksv0125.yaml`). Two Services (8080, and the management port 9000 **never** published — it serves `/health/*` and optionally `/metrics` unauthenticated) and two Ingresses. **The admin hostname is not an access control**: `hostname-admin` only decides which URLs the server generates, and upstream states that it "does not prevent accessing the Administration REST API endpoints via the frontend URL" — restricting that API is reverse-proxy work, and the chart renders no `/admin` deny rule. `ingress.domain` is read as `KC_HOSTNAME` (as a full URL, which upstream requires alongside `hostname-admin`), so it stays required with the Ingress off. Liveness is `/health/live` and deliberately not `/health` or `/health/ready` — those include the database, and a liveness probe on them restarts the whole cluster for as long as the database is away. **`keycloak.metrics.enabled` defaults to `true` for the readiness probe, not for the metrics**: upstream's table of health checks carries a "Requires Metrics" column and the *database* check is the row that says yes, so with it off `/health/ready` answers 200 while the database is gone — the `urlaubsverwaltung` shape (#218) with the dependency hidden behind an option whose name says observability. Two traps at the values: upstream's `db-pool-max-size` default of 100 would ask a default PostgreSQL for 200 connections across two replicas (the chart ships 20), and two releases sharing one database **merge into one Infinispan cluster** unless the cluster names differ |
 | `matrix-synapse` | Matrix Synapse | StatefulSet plus an nginx delegation Deployment/Service |
 | `minecraft-router` | itzg/mc-router | Deployment + routing ConfigMap + NodePort Service (TCP only, no ingress); `mappings` is a required value |
 | `minecraft-server` | itzg/minecraft-server | StatefulSet + PVC + Service + init-script ConfigMap (no ingress); service type is a value (`ClusterIP` by default, routed via `minecraft-router`). **Only the newest Java line is maintained** (chart 5.0.0, #158): the directory carries one line, older ones stay published as frozen artifacts (java8/java17 at `4.0.0`, java21 at `4.1.0`) and receive no further chart changes — publishing once per line out of one directory is what made the lines drift. To run an older JVM with the current chart, set `image.tagOverride`; the chart then checks that `minecraft.version` fits the Java line of the **effective** tag and fails the render with both values named. Two cases pass unchecked on purpose: a tag without a `java<N>` part and a `minecraft.version` that is not a plain numeric `x[.y[.z]]`. `image.tagOverride` is deliberately outside Renovate's reach — `renovate.json5` tracks `appVersion` only. Since chart 2.0.0 the tag is an **immutable dated upstream release** (`appVersion: <release>-java8` / `-java17` / `-java21` / `-java25`, e.g. `2026.8.2-java8`) instead of the continuously rebuilt rolling line tag. Pilot for the strict `values.schema.json` (#68), which is now binding for every chart |

ci/keycloak/test-values.yaml

@@ -24,20 +24,32 @@
 #      re-augment. A disagreement between the build options and the serving
 #      container would show up here as a crash loop rather than as a slow
 #      start, because the path it would want to rewrite is read-only.
-#   4. jdbc-ping FINDS THE SECOND REPLICA from cache=ispn and the database
+#   4. TWO REPLICAS BOTH COME UP AND STAY UP against one database - the
-#      configuration alone - which is why this test runs TWO replicas (see
+#      rollout is complete only when BOTH pods are Ready, so a second replica
-#      below) and why that is the single most valuable line in this file.
+#      that crashed, deadlocked on the schema migration lock or fought the
+#      first over the database would fail the run. Measured in Gitea PR #87:
+#      71.8s from `helm install` to both Ready, 0 restarts.
 #   Plus the mechanical half: the Secret key names match what the Deployment
 #   references, /health/started, /health/live and /health/ready all answer on
 #   the management port (so health-enabled really was baked in by the init
 #   container), the resource quantities are valid, and the schema accepts a
 #   realistic non-default values file.
 #
-# WHAT IT DOES NOT PROVE, and no install-test here could:
+# WHAT IT DOES NOT PROVE, and the first entry is the one not to misread:
+#   - THAT jdbc-ping ACTUALLY FOUND THE PEER. Two replicas both reaching Ready
+#     is NOT that proof, and believing it was is the mistake this paragraph
+#     exists to prevent: readiness reports this node's own health, so a node
+#     that never found its peer is Ready, healthy and alone - and two such
+#     nodes are two islands that each believe they are the cluster. The
+#     Infinispan view has to be read out of the log, which is what step 2 of
+#     NOTES.txt tells the operator to do. So measurement point 4 of the plan
+#     stays open after this test; what the test does rule out is the louder
+#     half (a second replica that cannot start or cannot share the database).
 #   - that hostname-admin serves the console cleanly on a second hostname. No
-#     ingress controller answers in CI and nothing drives a browser, so the
+#     ingress controller answers in CI and nothing drives a browser, so
-#     chart's own measurement point 5 stays open. What IS checked is that both
+#     measurement point 5 stays open too. What IS checked is that both Ingress
-#     Ingress objects are accepted and that the Service port they name exists.
+#     objects are accepted, that the Service port they name exists, and that
+#     Keycloak accepts both hostname values as full URLs at start.
 #   - anything about an upgrade. The rolling-update restriction between minor
 #     versions is a property of two versions meeting, and this test installs
 #     one.
@@ -75,12 +87,16 @@ keycloak:
       minSize: 1
       maxSize: 5
 
-  # ON against the chart's default of off, because it is a BUILD option: with
+  # The chart default, spelled out because it is load-bearing for what this
-  # this true the init container has to build an installation that differs from
+  # test means: metrics-enabled is what puts the DATABASE check into
-  # the published image in TWO options rather than one, and /metrics has to
+  # /health/ready (upstream's check table, "Requires Metrics: Yes"). With it
-  # appear on the management port. A green run therefore proves the build
+  # off, readiness would pass without ever touching the database, and a green
-  # arguments really reach `kc.sh build` - with metrics off, a build that
+  # run would say nothing about the connection at all.
-  # silently did nothing would look identical.
+  #
+  # It is also a BUILD option, so with it true the init container has to build
+  # an installation differing from the published image in three options rather
+  # than two - which is what makes a build that silently did nothing fail the
+  # run instead of looking identical.
   metrics:
     enabled: true
 

keycloak/templates/NOTES.txt

@@ -52,11 +52,14 @@ FIRST INSTALL - AND STEP 4 IS MANDATORY, NOT ADVICE
      creates the entire schema through Liquibase, and with {{ $replicas }} replica(s)
      the second waits on the first's migration lock.
 
-  2. Check that the cluster really formed - this is what the second replica is
+  2. Check that the cluster really formed. THIS STEP IS NOT OPTIONAL AND NO
-     for, and a silent failure here leaves two independent islands:
+     PROBE REPLACES IT: a node that never found its peer is Ready, healthy and
+     alone, and two such nodes are two islands that each believe they are the
+     cluster. The install-test cannot catch it either.
        kubectl -n {{ .Release.Namespace }} logs deployment/{{ $deploy }} | grep -i "ISPN000094\|view accepted\|members"
      Look for a view with {{ $replicas }} members. One member per pod means
-     jdbc-ping did not find the peer.
+     jdbc-ping did not find the peer - check that both pods can reach each
+     other on port {{ $settings.ports.jgroups | int64 }} and that they really share one database.
 
   3. Log in at {{ if $adminHostname }}{{ include "keycloak.adminUrl" . }}/admin{{ else if .Values.ingress.enabled }}{{ include "keycloak.frontendUrl" . }}/admin{{ else }}the in-cluster address, port-forwarded{{ end }}
      with the bootstrap credentials from the vault item above.
@@ -140,6 +143,14 @@ TRAPS WORTH KNOWING BEFORE YOU NEED THEM
     unset. Read its comment in values.yaml before setting it: upstream also
     warns about cache invalidation between differently-named clusters.{{ end }}
 
+  - READINESS ONLY CHECKS THE DATABASE WHILE metrics-enabled IS TRUE. Upstream's
+    table of health checks has a "Requires Metrics" column and the DATABASE
+    check is the row that says yes. So with keycloak.metrics.enabled false,
+    /health/ready answers 200 with the database gone and the pod keeps taking
+    logins it cannot serve. This release has it {{ if $settings.metrics.enabled }}ON, which is the chart default
+    and the reason the default is what it is.{{ else }}OFF, so the readiness probe
+    cannot currently detect a database outage.{{ end }}
+
   - THERE IS NO ConfigMap IN THIS CHART, so no checksum/config annotation. That
     is not an omission: Keycloak is configured entirely through options and
     KC_* variables, so there is no config file whose change could fail to

keycloak/templates/_helpers.tpl

@@ -180,7 +180,7 @@ Usage: {{ $settings := fromYaml (include "keycloak.settings" .) }}
         "database" "keycloak"
         "pool" (dict "initialSize" 2 "minSize" 2 "maxSize" 20))
       "bootstrapAdmin" (dict "enabled" true)
-      "metrics" (dict "enabled" false)
+      "metrics" (dict "enabled" true)
       "features" (list)
       "cache" (dict "clusterName" nil)
       "ports" (dict "http" 8080 "management" 9000 "jgroups" 7800)
@@ -392,11 +392,25 @@ away, which is also why the wrong path would look entirely plausible in a diff.
 {{- end -}}
 
 {{/*
-READINESS: /health/ready - which DOES include the database, and must. A node
+READINESS: /health/ready - which includes the database, and must. A node that
-that cannot reach its database answers every login with an error, so no Service
+cannot reach its database answers every login with an error, so no Service
 should route to it. This is the other half of guideline 12.3: a dependency-free
 liveness path is only correct when readiness really does hang on the
 dependency.
+
+AND THE PATH ALONE DOES NOT GUARANTEE THAT, which is the part worth reading
+before anybody "cleans up" keycloak.metrics.enabled. Upstream's table of
+available checks carries a "Requires Metrics" column, and the DATABASE check -
+"Returns the status of the database connection pool" - is the row that says
+YES; Cluster, Graceful shutdown and Keycloak Initialized say no. So with
+metrics-enabled=false this endpoint still answers 200 while the database is
+gone, and the pod keeps taking logins it cannot serve.
+
+That is exactly the urlaubsverwaltung shape (#218): the probe path was right
+and the aggregate behind it contained nothing external. Here the dependency
+hides behind an option whose name says observability, which is why
+keycloak.metrics.enabled defaults to TRUE and why its comment in values.yaml
+argues about readiness rather than about scraping.
 */}}
 {{- define "keycloak.readinessProbe" -}}
 {{- $defaults := dict

keycloak/values.schema.json

@@ -76,7 +76,7 @@
           }
         },
         "metrics": {
-          "description": "A Keycloak BUILD option (metrics-enabled). Off, because the management port is unauthenticated in this chart's configuration and metrics add realm and client names to that surface.",
+          "description": "A Keycloak BUILD option (metrics-enabled). ON, and not for the metrics: upstream's table of health checks has a 'Requires Metrics' column, and the DATABASE check is the row that says yes. With this false, /health/ready still answers 200 while the database is gone - the urlaubsverwaltung shape (#218), with the dependency hidden behind an option whose name says observability. The price is that /metrics then answers on the management port, unauthenticated, carrying realm and client names - that port is ClusterIP-only and in no Ingress. Switching this off is a decision about whether readiness can fail.",
           "type": ["object", "null"],
           "additionalProperties": false,
           "properties": {

keycloak/values.yaml

@@ -200,17 +200,33 @@ keycloak:
     # a credential the pod no longer needs.
     enabled: true
 
-  # Expose /metrics on the management port (metrics-enabled, a BUILD option -
+  # metrics-enabled, a BUILD option - it goes to `kc.sh build`, so changing it
-  # it goes to `kc.sh build`, so changing it re-runs the init container).
+  # re-runs the init containers.
   #
-  # OFF, and the reason is the port rather than the data: the management
+  # ON, AND NOT FOR THE METRICS. This value is misnamed from our point of view:
-  # interface is UNAUTHENTICATED in this chart's configuration, and switching
+  # it also decides whether /health/ready CONTAINS A DATABASE CHECK AT ALL.
-  # metrics on adds Keycloak's full metric set - realm names, client ids, login
+  # Upstream's table of available checks has a "Requires Metrics" column, and
-  # and token counters - to a surface nothing authenticates. Switch it on
+  # the Database check - "Returns the status of the database connection pool" -
-  # together with whatever scrapes it, and read
+  # is the one row that says YES. The Cluster, Graceful shutdown and Keycloak
-  # keycloak.ports.management below first.
+  # Initialized checks say no.
+  #
+  # So with metrics-enabled=false, a pod whose database is gone still reports
+  # Ready and keeps serving errors to every login. That is precisely the shape
+  # urlaubsverwaltung cost this repo two years of false confidence (#218, and
+  # README guideline 12.3): the probe path was right and the thing behind it
+  # contained nothing external. Here the dependency hides behind an option
+  # whose name says "observability".
+  #
+  # The price, which is real and is why the value still exists: /metrics then
+  # answers on the management port, UNAUTHENTICATED, carrying realm names,
+  # client ids and login counters. That port is ClusterIP-only and in no
+  # Ingress (see keycloak.ports.management), so the exposure is in-cluster.
+  #
+  # SWITCHING THIS OFF IS A DECISION ABOUT WHETHER READINESS CAN FAIL, not
+  # about scraping. If you do it, the readiness probe stops being able to
+  # detect a database outage, and nothing else in this chart replaces it.
   metrics:
-    enabled: false
+    enabled: true
 
   # Additional Keycloak feature flags (--features), e.g. ["organization",
   # "declarative-ui"]. A BUILD option: entries here change what the init
@@ -349,9 +365,14 @@ keycloak:
     timeoutSeconds: 5
     failureThreshold: 3
 
-  # READINESS: /health/ready - which DOES include the database, and must. A
+  # READINESS: /health/ready - which includes the database, and must. A node
-  # node that cannot reach its database serves errors to every login, so no
+  # that cannot reach its database serves errors to every login, so no Service
-  # Service should route to it.
+  # should route to it.
+  #
+  # AND THAT "INCLUDES THE DATABASE" IS CONDITIONAL, which is the trap worth
+  # knowing before anybody tidies up keycloak.metrics.enabled: the database
+  # check is only part of the report while metrics-enabled is true (upstream's
+  # check table, "Requires Metrics: Yes"). The path alone guarantees nothing.
   readinessProbe:
     httpGet:
       path: /health/ready
@@ -364,13 +385,24 @@ keycloak:
   # endpoint. It gates liveness and readiness, which is why neither of those
   # carries an initialDelaySeconds.
   #
-  # BUDGET = periodSeconds x failureThreshold = 10 x 60 = 600s, and it is
+  # BUDGET = periodSeconds x failureThreshold = 10 x 60 = 600s, and the gap
-  # generous on purpose. What has to fit inside it is not only the JVM and
+  # between that and the MEASURED time is deliberate rather than lazy.
-  # Infinispan: on a FIRST install Keycloak creates its entire schema through
+  #
-  # Liquibase, and with two replicas starting at once the second waits on the
+  # MEASURED, in the install-test of Gitea PR #87 on the Gitea docker runner
-  # first's migration lock. A budget that is too short there produces a
+  # (k3s v1.35.5, two replicas, empty database, image pulled during the run):
-  # CrashLoopBackOff that looks like an application fault and is really a
+  # from `helm install` to BOTH pods Ready in 71.8 seconds - the first at
-  # stopwatch.
+  # ~70.8s, the second one second later, 0 restarts. That figure already
+  # contains the image pull, both init containers and the schema creation.
+  #
+  # Why the budget is still eight times that: everything in the measured run
+  # was the cheap case. A first install against a real database creates the
+  # whole schema through Liquibase, and with two replicas the second waits on
+  # the first's migration lock instead of arriving a second later; a loaded
+  # node, a slower disk or a bigger existing schema all push in the same
+  # direction. A budget that is too short there produces a CrashLoopBackOff
+  # that looks like an application fault and is really a stopwatch - and the
+  # cost of the headroom is nothing, because a pod that IS healthy becomes
+  # Ready on the first probe either way.
   #
   # The Quarkus augmentation is NOT in this budget - it runs in the init
   # container, which has no probe and no deadline of its own. That is the