feat(openbao): reconcile the OIDC client after ZITADEL rotates it - #2078
Merged
Merged
Conversation
…2045) Option B + E, approved by the owner on 2026-09-21: the ZITADEL sync rotates the three fields that change on every rebuild, oidc.tf ignores them, and a stage5 check halts the deploy on a mismatch.
scripts/lib/openbao-api.sh centralises the curl -K root-token pattern openbao-adopt-jwt-mount.sh inlined, for reconcile_openbao_oidc (#2045) to reuse rather than copy a third time. The new contract-guard suite pins two facts the rotation depends on: the CONSUMERS "openbao" store key matches variables.tfvars' uncommented openbao_oidc_secret_id (guards the #2011 regression shape), and oidc.tf's UI callback path is /ui/vault/auth/oidc/oidc/callback. Task 2 extends the same file with the reconcile itself.
The callback guard used grep -qF, a plain substring match satisfied by the literal sitting after a "#" -- so commenting out oidc.tf's whole redirect_uris entry still reported "pins the UI callback path". Anchored it to an active list element (leading quote after indentation, trailing quote plus optional comma at end of line), the same shape the tfvars guard next to it already required for #2011. Also asserts the CONSUMERS side: the header claims this guards an agreement between oidc.tf and the "openbao" CONSUMERS entry, but nothing read the CONSUMERS callback before now -- only its store key.
Every rebuild creates a new ZITADEL app for OpenBao, while auth/oidc keeps the old client, so SSO breaks (#2045). Two functions in zitadel-oidc-clients.sh, not yet called from cmd_sync (Task 3): - openbao_oidc_config_payload: the current config minus `status`, plus the new id and secret, all on stdin. It refuses a non-empty provider_config and an empty id or secret. - reconcile_openbao_oidc: design section 2, steps 1-10. A no-op without a URL, a store key or an oidc/ mount. It waits for the store to agree with ZITADEL, then writes the config and the role independently and only when they differ. A dry run only reads. The config write is retried on the discovery error only, and when it fails the role is not written. It reads both back. The EXIT trap is private to a subshell. openbao-api.sh, from Task 1's review: `|| return 1` on both assignments, `local -; set +x` in openbao_token_config_write, and --fail-with-body in openbao_req, because -f discards the error body that tells a discovery failure from any other. The suite now stubs curl, jq and sleep on PATH. The curl stub is a small fake OpenBao. jq's argv is logged, which proves the secret never reaches it. New guards: printf is only ever the builtin, and the POST body reaches curl's stdin.
The mount check read an empty 200 body, `null` or `{}` as "no oidc/ mount"
and skipped with exit 0. That reports a stale client as a first bootstrap.
Now only a real `.data` map without oidc/ skips; anything else is [FAILED ].
Review minors, folded in:
- Under --apply, an empty client id or store key is [FAILED ], a wiring
bug; a dry run still skips.
- When the config was written but the role write failed, the message says
the config has already moved and that re-running sync --apply finishes
the job. The payload's refusals carry the [FAILED ] marker.
- Four role fields reset when a write omits them (role_type,
bound_claims_type, callback_mode, oidc_disable_confirmation). The comment
and design fact 7 now say so. role_type is sent and oidc.tf sets none of
the other three, so behaviour is unchanged.
- Suite: the printf-builtin guard also covers the lifted reconcile. The fake
resets those four fields on a role write, BAO_CONFIG carries
override_allowed_server_names, and the discovery error is the server's
exact message.
Adds --openbao-url/--openbao-root-token-secret/--openbao-ca-file (a URL requires the other two, and the CA file must exist), and calls reconcile_openbao_oidc once after the consumer loop with the "openbao" consumer's store key and client id, captured on both the existing-app and the create path. A reconcile failure exits 1 after the run summary prints.
Terraform creates the oidc/ mount, but oidc_client_id, oidc_client_secret and bound_audiences churn on every ZITADEL rebuild and the vault provider never refreshes the secret. Without ignore_changes, the next management apply replays OpenBao's discovery check while ZITADEL is still down mid-rebuild and fails (design facts 6, 8). zitadel-oidc-clients.sh's reconcile_openbao_oidc now owns rotating those three fields after Terraform's initial create, following the workforce-identity precedent.
Adds scripts/openbao-oidc-check.sh (design E, #2045): a post-deploy check that verifies OpenBao's auth/oidc config and role agree with the "openbao-oidc" secret store entry, then probes ZITADEL's authorize endpoint directly to catch a stale client id that happens to match a stale store copy (design risk R6). A later task wires it as a stage5 deploy job that halts on failure. Exit 0 means consistent or not bootstrapped; exit 1 is a definite, named problem; exit 2 means it cannot tell (OpenBao unreachable, the root token unreadable, or a first-hop HTTP code other than 302/400). T0, the live spike that would confirm 302/400 against a real ZITADEL, never ran, so those two codes are unverified until the first live run. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xuNsQcGpQPcEH8m1h3rDD
…nches Review of 80c9d2b found the auth_url POST and the authorize URL's first hop each have a "cannot reach" branch (openbao-oidc-check.sh:209-212, :225-228) that no test case exercised, even though the curl stub already had AUTHORIZE_CURL_FAIL and AUTH_URL_ERROR wired for exactly this. Mutating either exit 2 to exit 0 left the suite green. Adds one test per branch, plus a body-content assertion on the auth_url POST (role and redirect_uri) and two new cases for the auth/oidc/config and auth/oidc/role/default read-failure branches. Test-only; the production script is unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xuNsQcGpQPcEH8m1h3rDD
…eploy stage4-oidc-clients now carries the cloud gate, fetches OpenBao's CA into a temp dir, and hands --openbao-url/--openbao-root-token-secret/--openbao-ca-file to aws-0's own sync only. The gcp-0 consumer call and gcp/gke/init pass none. A failed CA fetch warns and registers without reconciling. A new stage5-verify-openbao-oidc job runs scripts/openbao-oidc-check.sh after stage4. It does not swallow the exit code: exit 1 (a definite mismatch) and exit 2 (cannot tell) both fail the job and halt the run, so every later stack (gcp/gke/init, aws/eks/configure, ...) is skipped. That is the owner's decision in the design, section 7. Contract guards in test-zitadel-oidc-clients-openbao.sh pin who passes the flags, that stage5 follows stage4 in script "deploy", and that the check's exit code reaches Terramate. Refs #2045
Terraform creates the oidc/ mount once and now ignores the three fields zitadel-oidc-clients.sh rotates on every deploy; operators need to know that a re-apply no longer fixes a stale client, and what does.
The old guard only rejected a literal `||` on the check call's own line, which an `if ! check; then echo; fi` wrapper or a `set +e` plus a trailing command both dodge without ever writing one. Anchor on the call being the heredoc's last statement instead, and add a guard for stage5's primary_cloud skip, each proved against a mutated temp copy of the real file. Also reword two comments that named an incidental stack run order (gcp/gke/init now runs after aws/eks/init, not before) instead of the missing `after` edge that actually decides it, move stage5's context echo above its CA fetch, and soften a stage4 comment that overclaimed what stage5 reports on a CA-fetch failure.
check_call_last() proved the call was the heredoc's last statement but only matched a PREFIX of that line, so stage4's own swallow style — `check ... || echo "[warn] ..."` appended after the call's final argument, on the same line — still matched and passed. Require the whole last logical line to carry none of `||`, `&&`, `;` or `|`.
OpenBao v2.6.2 answers auth_url with 200 and an EMPTY auth_url when it cannot fetch ZITADEL's discovery document -- ZITADEL still starting mid-rebuild -- and gives the same answer for a redirect_uri the role does not allow. The check mapped that to a definite exit 1, and any transient first-hop error to exit 2, with no retry; stage5 halts the deploy on both. The auth_url POST and the first hop now get 5 attempts, OPENBAO_CHECK_RETRY_SLEEP (default 10s) apart. A 302 or a 400 is definite and ends the loop at once, and the id/audience mismatch before it is never retried. An empty auth_url that persists is exit 2, naming both causes, since the check cannot tell them apart.
A 400 on the authorize URL is ZITADEL's answer for an unknown client AND
for a known client asked for a redirect_uri it never registered, so the
message now names both. A 302 is not always acceptance: ZITADEL refuses a
request for a known client by redirecting straight back to the
redirect_uri with ?error=. The check now reads %{redirect_url} and treats
that as exit 1, printing only the error parameter -- the rest of the query
carries OpenBao's state.
zitadel-oidc-clients.sh refuses to start without IDP_URL and PRIVATE_DOMAIN, and a wrong PRIVATE_DOMAIN rewrites every app's redirect URIs -- yet openbao.md's recovery command set neither, nor --region, and the check printed a partial `sync --apply` hint twice. The docs command now carries every value, checked against config.tm.hcl and stage4's own derivation, and the check's two fix lines point at that section instead of restating it. The suite pins that the section still exists. sso.md's step 1 had the same gap on aws-0: no --openbao-* flags, so a re-run after a restore left OpenBao on a client ZITADEL had forgotten. An OPENBAO variable carries them (empty on gcp-0), and IDP_URL / PRIVATE_DOMAIN are now exported -- as plain assignments they never reached the script in steps 1 and 4.
The management stack runs before ZITADEL has issued OpenBao's OIDC client, so on a brand-new platform it cannot create the oidc/ mount and stage5 halts the first deploy -- by design (ruling R3), but the getting-started page still promised "one command". Both it and the rotation section now say the red run is expected, give the one management apply that creates the mount, and how to resume: re-run the deploy, or `TM_CLOUD=gcp terramate -C opentofu/gcp/gke/init script run deploy` for the GCP stacks the halt skipped. TM_CLOUD=gcp is required there: without it every GCP job gates itself to [skip].
…OIDC secret ignore_changes only stops Terraform planning oidc_client_id and oidc_client_secret. terraform-provider-vault's jwtAuthBackendUpdate still re-sends the whole config, secret from state, on any other in-place update to the mount -- quietly undoing the reconcile's rotation. The lifecycle comment now says so, names the recovery (re-run the sync with --openbao-*) and oidc_client_secret_wo as the follow-up fix.
… read The guard asserting gcp/gke/init passes no --openbao-* flag is an absence check: pointed at a missing file (GCP_GKE_INIT_WORKFLOWS=/nonexistent), it read nothing and passed. It now requires the file to exist and to still hold at least one zitadel-oidc-clients.sh sync.
The suite pinned only stage5's --redirect-uri. openbao-oidc-check.sh exits 2 on an unknown argument and stage5 halts on 2, so a misspelt flag would break every deploy and show only on a live run. Every --flag in stage5's rendered check call must now be a case label in the check's argument parser; an in-suite mutant (--root-token-secret) proves the guard fails.
`( set -o errexit ...; cmd_sync ) || true` never honoured errexit: a subshell on the left of `||` runs with it ignored, whatever it sets. That hid a 127 -- reconcile_workforce_audience was never stubbed, and the test passed only because the failed call fell through. The stub is now explicit, and each run's exit status is captured and asserted.
The client-rotation note was a bare bullet under Negative consequences, which reads as part of the original decision. It is now an `> **Amendment (2026-09-22)**` block in ADR-0002's style, naming who owns the client since #2045: oidc.tf's ignore_changes, the sync that rotates it, and the stage5 job that halts the deploy on disagreement. lastVerified moves to 2026-09-22.
The section still said "three jobs" and called stage3 "a final" job; stage4-oidc-clients and stage5-verify-openbao-oidc run after it. The section is now a table of all five, in order, one line each.
Contributor
🔍 Rendered manifest diff — this PR vs
|
1 task
Smana
added a commit
that referenced
this pull request
Sep 22, 2026
…2077) * docs(superpowers): implementation plan for PR 3 of the scripts restructure provision/: three risk-grouped moves, each simulated end to end (refs 88/0, paths 0 failed, suites 23/1/0). The openbao-snapshot symlink moves with openbao-config.sh, which runs it as a sibling at apply time. * refactor(scripts): move the terramate and tofu plumbing to scripts/provision/ * refactor(scripts): move OpenBao configuration and the secret store to scripts/provision/ * refactor(scripts): move ZITADEL provisioning to scripts/provision/ * feat(task): index the human-run provision scripts * docs(scripts): say who actually runs the provision scripts * refactor(scripts): move openbao-oidc-check.sh to scripts/provision/ #2078 landed first and added the stage5 check at scripts/ root. It runs at deploy time, so it belongs with the other provision scripts. Also points #2078's new references (stage4's CA fetch, stage5, both suites, the lib sources and docs) at their provision/ paths.
Open
3 tasks
This was referenced Sep 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔍 feat
📝 Summary
Closes #2045. Every rebuild gives ZITADEL a new OpenBao OIDC client. OpenBao's
auth/oidckept the previous one, so SSO failed withApp.NotFounduntil someone re-applied the management stack by hand. The deploy now rotates the client itself (option B), and a newstage5job halts the run if OpenBao, the secret store and ZITADEL still disagree (option E).Important
Do not merge before both owner boxes below are ticked. Only a live platform proves this. Every gate here is offline.
📋 Design
Design:
2026-09-21-openbao-oidc-client-rotation-design.mdPlan:
2026-09-21-openbao-oidc-client-rotation-plan.mdThe whole design: B (the script rotates the client) plus E (the stage5 check), halting on mismatch. The ownership change is recorded as an amendment to ADR-0034.
🎯 Changes
-Kfile, never argv📊 Flow
flowchart LR zitadel["ZITADEL (new client each rebuild)"] -->|"stage4: register"| sync["zitadel-oidc-clients.sh sync"]:::modified sync -->|"store client"| sm["Secrets Manager openbao-oidc"] sync -->|"reconcile id, secret, audience"| bao["OpenBao auth/oidc"]:::modified tf["management stack (oidc.tf)"]:::modified -->|"creates mount, ignores 3 fields"| bao check["stage5 openbao-oidc-check.sh"]:::new -->|"compare"| sm check -->|"compare + auth_url probe"| bao check -->|"first hop: 302 known, 400 unknown"| zitadel classDef new fill:#1e3a8a,stroke:#3b82f6,stroke-width:3px,color:#fff classDef modified fill:#c2410c,stroke:#f97316,stroke-width:3px,color:#fff🗂️ Files
scripts/zitadel-oidc-clients.shreconcile_openbao_oidc,--openbao-*flags, wired intocmd_syncscripts/lib/openbao-api.shopenbao_req(--fail-with-body, never-k)scripts/openbao-oidc-check.shopentofu/aws/openbao/management/oidc.tfignore_changeson client id, secret andbound_audiences, with the hazard noted belowopentofu/aws/eks/init/workflows.tm.hcl--openbao-*on aws-0's sync only; newstage5-verify-openbao-oidcopentofu/aws/openbao/management/variables.tfvarsscripts/ci/tests/test-zitadel-oidc-clients-openbao.sh,test-openbao-oidc-check.shscripts/ci/tests/test-zitadel-oidc-clients-{convergence,redirects}.shwebsite/content/docs/…(openbao, sso, get-started/aws, commands, ADR-0034)⚖️ Rulings for the owner
You decided Q1 (B), Q2 (halt) and Q7 (after #2061). I ruled on the rest from the planner's recommendations. Overturn any of them here:
[action]line is a 3-line changeopenbao-adopt-jwt-mount.shalready does. Minting a narrow token needs root anywayinvalid_clienthazard stays until thenterramate script run -j. stage5 depends on stage4 having runErrInvalidRequestRedirectURI) and a known one gets a 302. A 302 that redirects back to theredirect_uriwith?error=counts as a failure, and any other code is exit 2, which still fails the job_wofix is a follow-up (below)ignore_changesstops Terraform from planning a secret change, but terraform-provider-vault's update ofvault_jwt_auth_backendre-sends the full config, withoidc_client_secrettaken from state. (Checked against v5.12.0, the version this stack pins:jwtAuthBackendUpdatesends every config field for whichGetOkExists || HasChangeis true.) So any future in-place change to that resource, such as a tune setting or a scope, restores the creation-time secret. SSO then fails withinvalid_client, and stage5's first-hop probe can't see that. Recovery: re-run the sync with--openbao-*(openbao.md § OIDC client rotation). The real fix isoidc_client_secret_wowith a static version, which v5.12.0 already ships. It's a follow-up because it needs a live apply to validate.🧪 Evidence (offline)
Branch rebased on
origin/main(0 behind). All results atcf07ef2e:task ci:test(task 3.53.1)vectornot installed), 0 failedtest-openbao-oidc-check.sh/test-zitadel-oidc-clients-openbao.shtest-no-secret-argv.shtest-terramate-script-refs.shtest-script-paths.shtask ci:links/ci:doc-paths/ci:doc-claimsterramate fmt --check,tofu fmt -check,shellcheck -x -S warningThe fix waves ran mutation checks, and the suites caught every mutant (16 in the last wave). Examples: an unknown flag in stage5's call, and
GCP_GKE_INIT_WORKFLOWSpointing at a missing file. Never run live: the 302/400 codes, the 5 × 10s probe retry, and the redirect-back-with-error verdict. T11 covers them.✅ Merge gate (owner)
syncprints[ok]or a diff;scripts/openbao-oidc-check.shexits 0 (orsync --applywith the--openbao-*flags heals it);tofu plan -detailed-exitcodeonopentofu/aws/openbao/managementexits 0.secrets-admin. RecordBEFORE(bao read -field=oidc_client_id auth/oidc/config), then runterramate script run --reverse destroy, verify against the cloud, and deploy with no manual re-apply. Proof:BAO == AUD == SM == NEW, whereNEWis the id on stage4's[created] openbaoline and differs fromBEFORE.bao login -method=oidcworks, and the managementtofu planexits 0.T11 and T12 can share a cluster session with #2077's preview.
🔀 Sequencing with #2077
Whichever PR merges second rebases. If #2077 (
scripts/provision/) lands first:openbao-oidc-check.shmoves toprovision/;../../provision/;lib/openbao-api.shis sourced as../lib/;zitadel-oidc-clients.shedits carry across the rename.Both path gates (
test-script-paths.sh,test-terramate-script-refs.sh) fail on a missed spot.📌 Follow-ups (not in this PR)
oidc_client_secret_woplus a static_wo_versiononvault_jwt_auth_backend(I3).store_existstreats a read error as "absent" (M4). The check can then print "will DESTROY the mount" on a transient describe failure, and the reconcile reports[skip]instead of[FAILED]for a key it just wrote.openbao_reqor on the first hop (M5). An OpenBao #3411-style deadlock would hang stage4 and stage5. This matches the repo-wide convention today; the fix is--max-time 30.🏷️ Labels
security, infrastructure