From 41d374f2e4b9d101ffd2eadd568c5059c900a20a Mon Sep 17 00:00:00 2001 From: Nikolai Emil Damm Date: Thu, 8 Oct 2026 03:13:49 +0200 Subject: [PATCH] fix(openbao): let a lost production server be recreated Production OpenBao has run on two of three servers since openbao-0 was lost: the partitioned rolling update makes the StatefulSet controller build every ordinal below the partition from the stored current revision, which predates the baseline-worker pins, and the storage-placement policy refuses that Pod. Return production to the chart's OnDelete strategy. A missing Pod is then created from the newest revision, which the canary has run unsealed, and no running server is replaced. Part of #4391 Co-Authored-By: Claude Opus 5.5 --- docs/operations/arc-runners.md | 4 +-- .../controllers/kustomization.yaml | 2 +- .../openbao/patches/standby-oidc-canary.yaml | 17 +++++++----- .../patches/enable-arc-transport.yaml | 8 +++--- scripts/tests/arc-staging/transport_test.go | 6 ++--- scripts/tests/test-openbao-standby-canary.sh | 26 ++++++++++--------- .../approved-surface-history.md | 18 +++++++++++++ .../approved-surface.txt | 2 +- 8 files changed, 53 insertions(+), 30 deletions(-) diff --git a/docs/operations/arc-runners.md b/docs/operations/arc-runners.md index ef28fd5819..836a91b126 100644 --- a/docs/operations/arc-runners.md +++ b/docs/operations/arc-runners.md @@ -52,8 +52,8 @@ and timestamps would fail on a group-writable Kubernetes volume. Production prepares credential transport separately from runner activation. The dedicated native TLS listener is served by the already repaired highest -Raft ordinal; the held partition is unchanged. Standby requests use OpenBao's -authenticated cluster connection to reach the leader. The existing API listener, +Raft ordinal, and enabling it replaces no running server. Standby requests use +OpenBao's authenticated cluster connection to reach the leader. The existing API listener, Raft storage, audit configuration and unseal hook remain in the base configuration. An additional listener file is appended through the pinned chart. diff --git a/k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml b/k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml index 6e397ab07d..86b086574b 100644 --- a/k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml +++ b/k8s/providers/hetzner/infrastructure/controllers/kustomization.yaml @@ -131,7 +131,7 @@ patches: # Prod-only: HARD-pin OpenBao to the static baseline workers so its hcloud # block volumes never ride a node the autoscaler can reclaim (#2363) — see file. - path: openbao/patches/keep-off-autoscale-nodes.yaml - # Stage only one OpenBao replica; lower ordinals wait for healthy readback. + # Replace an OpenBao server only when its Pod is deleted or lost — see file. - path: openbao/patches/standby-oidc-canary.yaml - path: openbao/transport/patches/enable-arc-transport.yaml # Umami's Namespace moved into the preceding controllers layer with its diff --git a/k8s/providers/hetzner/infrastructure/controllers/openbao/patches/standby-oidc-canary.yaml b/k8s/providers/hetzner/infrastructure/controllers/openbao/patches/standby-oidc-canary.yaml index 718dbf727b..c6c30b7fa5 100644 --- a/k8s/providers/hetzner/infrastructure/controllers/openbao/patches/standby-oidc-canary.yaml +++ b/k8s/providers/hetzner/infrastructure/controllers/openbao/patches/standby-oidc-canary.yaml @@ -1,7 +1,11 @@ -# Deliver the already-selected standby OIDC repair to one production replica. -# Keep the lower ordinals on their current revision until the canary is unsealed -# and healthy. Advancing this partition requires a separate reviewed change and -# deployment readback (#4391); the base/local chart retains OnDelete. +# Production replaces an OpenBao server only when its Pod is deleted or lost. +# OnDelete creates a missing Pod from the newest StatefulSet revision. A rolling +# update with a partition instead builds every ordinal below the partition from +# the stored current revision, which production kept from before the pins that +# keep these volumes on the baseline workers: when openbao-0 was lost, admission +# refused its replacement and the cluster ran on two of three servers (#4391). +# The earlier one-replica canary (openbao-2) has run the repaired release +# unsealed since; nothing here replaces a running server. apiVersion: helm.toolkit.fluxcd.io/v2 kind: HelmRelease metadata: @@ -25,6 +29,5 @@ spec: namespace: openbao spec: updateStrategy: - type: RollingUpdate - rollingUpdate: - partition: 2 + type: OnDelete + rollingUpdate: null diff --git a/k8s/providers/hetzner/infrastructure/controllers/openbao/transport/patches/enable-arc-transport.yaml b/k8s/providers/hetzner/infrastructure/controllers/openbao/transport/patches/enable-arc-transport.yaml index 9316ebfe8b..72282572a0 100644 --- a/k8s/providers/hetzner/infrastructure/controllers/openbao/transport/patches/enable-arc-transport.yaml +++ b/k8s/providers/hetzner/infrastructure/controllers/openbao/transport/patches/enable-arc-transport.yaml @@ -1,5 +1,6 @@ # Additional -config listeners append; Raft, audit, unseal and the legacy -# listener stay in the base. The existing partition still replaces only #2. +# listener stay in the base. This patch replaces the earlier post-renderer list, +# so it repeats the OnDelete strategy from patches/standby-oidc-canary.yaml. apiVersion: helm.toolkit.fluxcd.io/v2 kind: HelmRelease metadata: @@ -26,9 +27,8 @@ spec: namespace: openbao spec: updateStrategy: - type: RollingUpdate - rollingUpdate: - partition: 2 + type: OnDelete + rollingUpdate: null template: metadata: labels: diff --git a/scripts/tests/arc-staging/transport_test.go b/scripts/tests/arc-staging/transport_test.go index 0e92ffde72..1dbda85003 100644 --- a/scripts/tests/arc-staging/transport_test.go +++ b/scripts/tests/arc-staging/transport_test.go @@ -162,13 +162,13 @@ func TestCredentialAuthenticationStagingDoesNotReadAppKeys(t *testing.T) { equal(t, resources[1], "secret-store.yaml") } -func TestCredentialTLSDoesNotAdvanceTheHeldRaftRollout(t *testing.T) { +func TestCredentialTLSDoesNotReplaceRunningRaftServers(t *testing.T) { canary := readYAML(t, "k8s/providers/hetzner/infrastructure/controllers/openbao/patches/standby-oidc-canary.yaml") patches := field(t, canary, "spec", "postRenderers").([]any) patch := field(t, patches[0], "kustomize", "patches").([]any)[0] text := field(t, patch, "patch").(string) - if !strings.Contains(text, "partition: 2") { - t.Fatal("TLS must not advance the held lower Raft ordinals") + if !strings.Contains(text, "type: OnDelete") || strings.Contains(text, "RollingUpdate") { + t.Fatal("TLS must not replace a running Raft server") } service := readYAML(t, transportPath+"service.yaml") equal(t, field(t, service, "spec", "selector", "statefulset.kubernetes.io/pod-name"), "openbao-2") diff --git a/scripts/tests/test-openbao-standby-canary.sh b/scripts/tests/test-openbao-standby-canary.sh index 1ab71ee272..b0e1d132d3 100755 --- a/scripts/tests/test-openbao-standby-canary.sh +++ b/scripts/tests/test-openbao-standby-canary.sh @@ -16,9 +16,7 @@ checked_postrenderer_changes() { jq -en --slurpfile before "$1" --slurpfile after "$2" ' ($before | length) == 1 and ($after | length) == 1 and $after[0].spec.replicas == 3 and - $after[0].spec.updateStrategy == { - type: "RollingUpdate", rollingUpdate: {partition: 2} - } and + $after[0].spec.updateStrategy == {type: "OnDelete"} and $after[0].spec.template.spec.automountServiceAccountToken == false and $before[0].spec.template.metadata.labels["platform.devantler.tech/arc-transport"] == null and $after[0].spec.template.metadata.labels["platform.devantler.tech/arc-transport"] == "tls" and @@ -102,7 +100,7 @@ yq -o=json '.spec.postRenderers' "${release}" | jq -e ' "group": "apps", "version": "v1", "kind": "StatefulSet", "name": "openbao", "namespace": "openbao" } -' >/dev/null || fail 'production OpenBao must permit only the highest-ordinal canary replacement' +' >/dev/null || fail 'production OpenBao must carry exactly one reviewed StatefulSet post-renderer patch' yq '.spec.postRenderers[0].kustomize.patches // [] | {"apiVersion": "kustomize.config.k8s.io/v1beta1", "kind": "Kustomization", "resources": ["rendered.yaml"], "patches": .}' "${release}" >"${scratch}/kustomization.yaml" @@ -110,13 +108,16 @@ kubectl kustomize "${scratch}" | yq ea 'select(.kind == "StatefulSet" and .metadata.name == "openbao")' - >"${scratch}/statefulset.yaml" readonly statefulset="${scratch}/statefulset.yaml" -yq -e '.spec.updateStrategy.type == "RollingUpdate" and - .spec.updateStrategy.rollingUpdate.partition == 2 and +# A partitioned rolling update recreates a lost lower ordinal from the stored +# current revision, which admission refuses in production (#4391). OnDelete +# recreates it from the newest revision and replaces no running server. +yq -e '.spec.updateStrategy.type == "OnDelete" and + (.spec.updateStrategy | length) == 1 and .spec.replicas == 3' "${statefulset}" >/dev/null || - fail 'production OpenBao must permit only the highest-ordinal canary replacement' + fail 'production OpenBao must recreate a lost server from the newest revision and replace no running one' yq -e '.spec.template.spec.containers[] | select(.name == "openbao") | .image == "quay.io/openbao/openbao:2.6.3"' "${statefulset}" >/dev/null || - fail 'the canary must use the released standby OIDC repair' + fail 'a recreated server must use the released standby OIDC repair' yq -e '.spec.template.spec.automountServiceAccountToken == false' "${statefulset}" >/dev/null || fail 'the certificate reload helper must not receive an injected API token' @@ -125,7 +126,7 @@ yq ea -o=json -I=0 'select(.kind == "StatefulSet" and .metadata.name == "openbao "${scratch}/rendered.yaml" >"${scratch}/before.json" yq -o=json -I=0 '.' "${statefulset}" >"${scratch}/after.json" checked_postrenderer_changes "${scratch}/before.json" "${scratch}/after.json" || - fail 'the canary post-renderer must change only partition, token injection and the exact TLS identity label' + fail 'the canary post-renderer must change only token injection and the exact TLS identity label' # Exercise rejected mutations against the actual chart and Flux-rendered Pod. # Allowing one retained identity label must not permit other metadata or spec edits. @@ -137,8 +138,9 @@ for mutation in \ '.spec.template.metadata.labels["app.kubernetes.io/name"] = "other"' \ '.spec.template.spec.serviceAccountName = "other"' \ '.spec.template.spec.automountServiceAccountToken = true' \ - '.spec.updateStrategy.rollingUpdate.partition = 0' \ - '.spec.updateStrategy.rollingUpdate.maxUnavailable = 1' \ + '.spec.updateStrategy = {"type": "RollingUpdate"}' \ + '.spec.updateStrategy = {"type": "RollingUpdate", "rollingUpdate": {"partition": 2}}' \ + '.spec.updateStrategy.rollingUpdate = {"partition": 0}' \ '.spec.replicas = 1'; do jq "$mutation" "${scratch}/after.json" >"${scratch}/mutated.json" if checked_postrenderer_changes "${scratch}/before.json" "${scratch}/mutated.json"; then @@ -178,4 +180,4 @@ grep -Fq "'scripts/tests/test-openbao-standby-canary.sh'" "${root_dir}/.github/w grep -Fq 'bash scripts/tests/test-openbao-standby-canary.sh' "${root_dir}/.github/workflows/ci.yaml" || fail 'CI must execute the OpenBao canary regression' -printf 'PASS: OpenBao 2.6.3 rollout is confined to the production canary\n' +printf 'PASS: production OpenBao recreates a lost server at 2.6.3 and replaces no running one\n' diff --git a/scripts/validate-eks-ci-role-policy/approved-surface-history.md b/scripts/validate-eks-ci-role-policy/approved-surface-history.md index 2e5631b777..8ce8e015c9 100644 --- a/scripts/validate-eks-ci-role-policy/approved-surface-history.md +++ b/scripts/validate-eks-ci-role-policy/approved-surface-history.md @@ -3322,3 +3322,21 @@ devantler-tech/ksail could list usage but not the limit. These values were computed locally with kubectl v1.37.1 / Kustomize v5.8.1, which is not the approved renderer; the required CI job re-renders with the approved kubectl v1.36.2 / Kustomize v5.8.1 and is the authority. + +## 2026-10-08 — OpenBao replaces a server only when its Pod is lost + +The renderer measured exactly one changed authorization entry for #4391 +against base `6de1a8b9`: HelmRelease openbao/openbao. Nothing was added or +removed from the selected authorization surface. Its ledger entry moves from +`5f42ca30331162fc4ee98a0487fa3651140f0542ff49b573a99a46a343582616` to +`83c3232f567e9320bf9561380834fb64ba0e34f974c4dfd026bef228bf59ecd3`. + +The source delta is the StatefulSet update strategy in the release's single +post-renderer patch: `RollingUpdate` with `partition: 2` becomes `OnDelete`, +the chart's own default. No value, identity, mount, listener, policy or +network rule changes. The partition made the controller recreate a lost +lower ordinal from a stored revision that predates the baseline-worker +pins, which admission refuses, so a lost server could not return. + +These values were computed locally with the approved kubectl v1.36.2 / +Kustomize v5.8.1 renderer. diff --git a/scripts/validate-eks-ci-role-policy/approved-surface.txt b/scripts/validate-eks-ci-role-policy/approved-surface.txt index 6cd6295a5e..3bd7de6f71 100644 --- a/scripts/validate-eks-ci-role-policy/approved-surface.txt +++ b/scripts/validate-eks-ci-role-policy/approved-surface.txt @@ -88,7 +88,7 @@ helm.toolkit.fluxcd.io/v2|HelmRelease|observability|coroot-operator a6c5d6dfdec7 helm.toolkit.fluxcd.io/v2|HelmRelease|open-feature-operator-system|open-feature-operator da38c30c19aea0c3e099bb86cda46d89ab6301ff5898b4fad8f020812f8ad800 -helm.toolkit.fluxcd.io/v2|HelmRelease|openbao|openbao 5f42ca30331162fc4ee98a0487fa3651140f0542ff49b573a99a46a343582616 +helm.toolkit.fluxcd.io/v2|HelmRelease|openbao|openbao 83c3232f567e9320bf9561380834fb64ba0e34f974c4dfd026bef228bf59ecd3 helm.toolkit.fluxcd.io/v2|HelmRelease|policy-reporter|policy-reporter f92d67d39b5638fa931d19f4015ee490d6c98043e00a01319f4ef1a5fc64bc29