From 1fc5c6196da369d758a717772644b3a0b9fd8edc Mon Sep 17 00:00:00 2001 From: Smana Date: Tue, 29 Sep 2026 11:52:06 +0200 Subject: [PATCH 1/9] refactor(secrets): one managed-store to OpenBao map, shared --- scripts/ci/tests/test-bao-map.sh | 20 +++++++++++++ scripts/lib/bao-map.sh | 50 +++++++++++++++++++++++++++++++ scripts/provision/secret-store.sh | 49 ++---------------------------- 3 files changed, 73 insertions(+), 46 deletions(-) create mode 100755 scripts/ci/tests/test-bao-map.sh create mode 100644 scripts/lib/bao-map.sh diff --git a/scripts/ci/tests/test-bao-map.sh b/scripts/ci/tests/test-bao-map.sh new file mode 100755 index 000000000..003479dc2 --- /dev/null +++ b/scripts/ci/tests/test-bao-map.sh @@ -0,0 +1,20 @@ +#!/usr/bin/env bash +# +# The managed-store -> OpenBao map lives in one file (GCP parity GP-5): migrate +# copies through it, and the OIDC sync mirrors through it. Two copies would drift +# and put a client secret where no ExternalSecret reads it. +set -uo pipefail +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" +fails=0 +check() { if [ "$2" = "$3" ]; then echo " ok $1"; else echo " FAIL $1: want '$2' got '$3'"; fails=1; fi; } +# shellcheck source=scripts/lib/bao-map.sh +. "$REPO_ROOT/scripts/lib/bao-map.sh" \ + || { echo "FAIL cannot source scripts/lib/bao-map.sh"; exit 1; } +check "grafana-envvars" "platform/victoria-metrics/grafana-envvars" "$(bao_target_for observability-victoria-metrics-k8s-stack-grafana-envvars)" +check "harbor-oidc" "platform/harbor/oidc" "$(bao_target_for harbor-oidc)" +check "app-wizard llm" "apps/app-wizard/llm" "$(bao_target_for apps-app-wizard-llm)" +bao_target_for openbao-oidc >/dev/null; check "openbao-oidc is unmapped" "1" "$?" +grep -q '^bao_target_for()' "$REPO_ROOT/scripts/provision/secret-store.sh" && { echo " FAIL secret-store.sh still defines its own copy"; fails=1; } +grep -q 'lib/bao-map.sh' "$REPO_ROOT/scripts/provision/secret-store.sh" || { echo " FAIL secret-store.sh does not source the map"; fails=1; } +[ "$fails" -eq 0 ] && echo "all checks passed" +exit "$fails" diff --git a/scripts/lib/bao-map.sh b/scripts/lib/bao-map.sh new file mode 100644 index 000000000..8f65d17aa --- /dev/null +++ b/scripts/lib/bao-map.sh @@ -0,0 +1,50 @@ +#!/usr/bin/env bash +# Managed-store key -> OpenBao path, the one copy (GCP parity GP-5). Read by +# secret-store.sh's `migrate` and by zitadel-oidc-clients.sh's --mirror-openbao. +# Why the map is explicit and what must stay out of it: see the comment kept +# above the function below. + +# Managed-store key -> OpenBao path, for `migrate`. +# +# Explicit rather than derived, because the managed store's names are not +# uniform: ADR-0023 introduced the dash form, slash-form names predate it, and +# the cnpg/* ones are generated by secret-store.sh's `seed` rather than curated by a human. +# +# Anything not listed is SKIPPED and reported, never guessed. A wrong guess here +# writes a platform secret into an app's prefix, which is a privilege boundary +# rather than a cosmetic mistake. +# +# Four keys are deliberately absent and must stay so: +# certificates/*/ca-chain bootstrap tier -- read before OpenBao is up, +# and the OpenBao-backed store needs the Secret +# it produces, so repointing it is circular +# cnpg/* (three of them) the SQLInstance composition hardcodes +# `secretStoreRef.name: clustersecretstore` and +# its XRD exposes no store field, so the claim +# cannot be repointed -- the same blocker App +# had before crossplane-configuration v0.6.1. +# NOT because a machine writes them: `seed` +# creates them (they are in GENERATABLE) +# and it is operator-run. Nothing in the +# cluster writes to any secret store. +bao_target_for() { + case "$1" in + harbor-admin-password) printf 'platform/harbor/admin-password' ;; + harbor-oidc) printf 'platform/harbor/oidc' ;; + harbor-valkey-password) printf 'platform/harbor/valkey-password' ;; + headlamp-envvars) printf 'platform/headlamp/envvars' ;; + zitadel-envvars) printf 'platform/zitadel/envvars' ;; + runlore-credentials) printf 'platform/runlore/credentials' ;; + runlore-slack-app) printf 'platform/runlore/slack-app' ;; + runlore-webhook) printf 'platform/runlore/webhook' ;; + security-flux-ui-oidc) printf 'platform/flux/ui-oidc' ;; + observability-flux-slack-app) printf 'platform/flux/slack-app' ;; + observability-victoria-metrics-k8s-stack-grafana-envvars) printf 'platform/victoria-metrics/grafana-envvars' ;; + observability-victoria-metrics-k8s-stack-alertmanager-slack-app) printf 'platform/victoria-metrics/alertmanager-slack-app' ;; + tailscale-k8s-operator-oauth-client) printf 'platform/tailscale/operator-oauth-client' ;; + apps-app-wizard-llm) printf 'apps/app-wizard/llm' ;; + apps-app-wizard-oauth) printf 'apps/app-wizard/oauth' ;; + apps/image-gallery/config) printf 'apps/image-gallery/config' ;; + *) return 1 ;; + esac +} diff --git a/scripts/provision/secret-store.sh b/scripts/provision/secret-store.sh index 55f17fe91..869249274 100755 --- a/scripts/provision/secret-store.sh +++ b/scripts/provision/secret-store.sh @@ -57,7 +57,7 @@ # existing destination is left alone and the source is never deleted, so # it is safe to re-run and safe to stop half-way. # -# The key mapping is explicit rather than derived -- see bao_target_for. +# The key mapping is explicit rather than derived -- see scripts/lib/bao-map.sh. # Anything unmapped is reported and skipped, never guessed: a wrong guess # writes a platform secret into an app's prefix, which is a privilege # boundary rather than a cosmetic mistake. @@ -87,6 +87,8 @@ set -o pipefail # gcloud must run as the identity OpenTofu uses, not the CLI account. # shellcheck source=scripts/lib/gcloud-adc.sh . "$(dirname "$0")/../lib/gcloud-adc.sh" +# shellcheck source=scripts/lib/bao-map.sh +. "$(dirname "$0")/../lib/bao-map.sh" CLOUD="" CONTEXT="" @@ -419,51 +421,6 @@ OLD_NAMES=( "zitadel/envvars" ) -# Managed-store key -> OpenBao path, for `migrate`. -# -# Explicit rather than derived, because the managed store's names are not -# uniform: ADR-0023 introduced the dash form, slash-form names predate it, and -# the cnpg/* ones are generated by `seed` below rather than curated by a human. -# -# Anything not listed is SKIPPED and reported, never guessed. A wrong guess here -# writes a platform secret into an app's prefix, which is a privilege boundary -# rather than a cosmetic mistake. -# -# Four keys are deliberately absent and must stay so: -# certificates/*/ca-chain bootstrap tier -- read before OpenBao is up, -# and the OpenBao-backed store needs the Secret -# it produces, so repointing it is circular -# cnpg/* (three of them) the SQLInstance composition hardcodes -# `secretStoreRef.name: clustersecretstore` and -# its XRD exposes no store field, so the claim -# cannot be repointed -- the same blocker App -# had before crossplane-configuration v0.6.1. -# NOT because a machine writes them: `seed` -# below creates them (they are in GENERATABLE) -# and it is operator-run. Nothing in the -# cluster writes to any secret store. -bao_target_for() { - case "$1" in - harbor-admin-password) printf 'platform/harbor/admin-password' ;; - harbor-oidc) printf 'platform/harbor/oidc' ;; - harbor-valkey-password) printf 'platform/harbor/valkey-password' ;; - headlamp-envvars) printf 'platform/headlamp/envvars' ;; - zitadel-envvars) printf 'platform/zitadel/envvars' ;; - runlore-credentials) printf 'platform/runlore/credentials' ;; - runlore-slack-app) printf 'platform/runlore/slack-app' ;; - runlore-webhook) printf 'platform/runlore/webhook' ;; - security-flux-ui-oidc) printf 'platform/flux/ui-oidc' ;; - observability-flux-slack-app) printf 'platform/flux/slack-app' ;; - observability-victoria-metrics-k8s-stack-grafana-envvars) printf 'platform/victoria-metrics/grafana-envvars' ;; - observability-victoria-metrics-k8s-stack-alertmanager-slack-app) printf 'platform/victoria-metrics/alertmanager-slack-app' ;; - tailscale-k8s-operator-oauth-client) printf 'platform/tailscale/operator-oauth-client' ;; - apps-app-wizard-llm) printf 'apps/app-wizard/llm' ;; - apps-app-wizard-oauth) printf 'apps/app-wizard/oauth' ;; - apps/image-gallery/config) printf 'apps/image-gallery/config' ;; - *) return 1 ;; - esac -} - # The keys to consider: every key the cluster's ExternalSecrets ask for. Same # resolution `check` uses, so the two commands can never disagree about what # the cluster actually consumes. From b91e295cdce9f6762f1ccf23fa284d730bfdc06f Mon Sep 17 00:00:00 2001 From: Smana Date: Tue, 29 Sep 2026 11:57:06 +0200 Subject: [PATCH 2/9] feat(zitadel): mirror consumer secrets into the OpenBao path they are read from --- .../test-zitadel-oidc-clients-convergence.sh | 4 + .../tests/test-zitadel-oidc-clients-mirror.sh | 76 +++++++++++++++++++ .../test-zitadel-oidc-clients-openbao.sh | 1 + scripts/provision/zitadel-oidc-clients.sh | 68 ++++++++++++++++- 4 files changed, 147 insertions(+), 2 deletions(-) create mode 100755 scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh b/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh index ca381c78b..abb7b40f8 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh @@ -115,6 +115,10 @@ store_probe() { } store_read() { cat "$STORE_DIR/$1" 2>/dev/null || true; } store_write() { cat > "$STORE_DIR/$1"; } +# cmd_sync writes through the real wrapper; the OpenBao mirror behind it has its +# own suite (test-zitadel-oidc-clients-mirror.sh). +load_function store_write_and_mirror "$SRC" +mirror_to_openbao() { cat >/dev/null; } # ── globals cmd_sync / converge_secret read ───────────────────────────────── CLUSTER="aws-0" diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh b/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh new file mode 100755 index 000000000..b2f8410c1 --- /dev/null +++ b/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh @@ -0,0 +1,76 @@ +#!/usr/bin/env bash +# shellcheck disable=SC2034 +# (MIRROR_OPENBAO and OPENBAO_ROOT_TOKEN_SECRET are read by the eval'd body of +# mirror_to_openbao(), which static analysis cannot see.) +# requires: jq +# +# --mirror-openbao (GCP parity GP-5): a fresh directory re-registers every client +# each build, and gcp-0's consumers read OpenBao. The mirror merges into the mapped +# path (admin credentials survive), skips unmapped keys, and does nothing unset. +# mirror_to_openbao() is lifted out of the script, so this tests the code that ships. +set -uo pipefail +REPO_ROOT="$(cd "$(dirname "$0")/../../.." && pwd)" +cd "$REPO_ROOT" || exit 1 +S=scripts/provision/zitadel-oidc-clients.sh +fail=0 +ok() { printf ' ok %s\n' "$1"; } +bad() { printf ' FAIL %s\n' "$1"; fail=1; } + +body="$(sed -n '/^mirror_to_openbao() (/,/^)/p' "$S")" +[ -n "$body" ] || { echo "could not extract mirror_to_openbao() from $S" >&2; exit 1; } +# shellcheck source=scripts/lib/bao-map.sh +. "$REPO_ROOT/scripts/lib/bao-map.sh" +eval "$body" + +T="$(mktemp -d)"; trap 'rm -rf "$T"' EXIT +openbao_token_config_write() { : >"$1"; } +GET_BODY='{"data":{"data":{"GF_SECURITY_ADMIN_USER":"admin","GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"old"}}}' +GET_CODE=200 +openbao_req() { + printf '%s %s\n' "$1" "$2" >>"$T/calls" + local method="$1" out=/dev/null + shift 2 + while [ $# -gt 0 ]; do case "$1" in -o) out="$2"; shift 2 ;; *) shift ;; esac; done + case "$method" in + GET) printf '%s' "$GET_BODY" >"$out"; printf '%s' "$GET_CODE" ;; + POST) cat >"$T/body" ;; + esac +} + +MIRROR_OPENBAO=true +OPENBAO_ROOT_TOKEN_SECRET=fixture +printf '%s' '{"GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"new"}' \ + | mirror_to_openbao observability-victoria-metrics-k8s-stack-grafana-envvars >/dev/null +grep -qx 'POST platform/data/victoria-metrics/grafana-envvars' "$T/calls" \ + && ok "grafana-envvars is written to its mapped path" || bad "grafana-envvars path: $(cat "$T/calls")" +[ "$(jq -c '.data' "$T/body")" = '{"GF_SECURITY_ADMIN_USER":"admin","GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"new"}' ] \ + && ok "merged: the admin user kept, the client id replaced" || bad "merge: $(cat "$T/body")" + +: >"$T/calls"; rm -f "$T/body" +GET_CODE=404 GET_BODY='{"errors":[]}' +printf '%s' '{"client_id":"new"}' | mirror_to_openbao harbor-oidc >/dev/null +[ "$(jq -c '.data' "$T/body" 2>/dev/null)" = '{"client_id":"new"}' ] \ + && ok "absent path (404): written fresh" || bad "404 case: $(cat "$T/body" 2>/dev/null)" + +: >"$T/calls"; rm -f "$T/body" +GET_CODE=403 +printf '%s' '{"client_id":"new"}' | mirror_to_openbao harbor-oidc >/dev/null 2>&1; rc=$? +[ "$rc" -ne 0 ] && [ ! -e "$T/body" ] \ + && ok "unreadable path (403): fails and overwrites nothing" || bad "403 case: rc=$rc, wrote $(cat "$T/body" 2>/dev/null)" +GET_CODE=200 + +: >"$T/calls" +printf '%s' '{"client_id":"x"}' | mirror_to_openbao openbao-oidc +[ ! -s "$T/calls" ] && ok "openbao-oidc (unmapped) never reaches OpenBao" || bad "an unmapped key reached OpenBao" + +: >"$T/calls" +MIRROR_OPENBAO=false +printf '%s' '{"a":"b"}' | mirror_to_openbao harbor-oidc +[ ! -s "$T/calls" ] && ok "without --mirror-openbao nothing is written" || bad "wrote without the flag" + +grep -q -- '--mirror-openbao) MIRROR_OPENBAO="true"; shift ;;' "$S" && ok "--mirror-openbao is parsed" || bad "--mirror-openbao is not parsed" +[ "$(grep -c 'store_write_and_mirror "\$key"' "$S")" -eq 2 ] \ + && ok "both consumer writes go through store_write_and_mirror" || bad "a consumer write bypasses the mirror" + +[ "$fail" -eq 0 ] && echo "all checks passed" +exit "$fail" diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh index edb9e043d..664d0d190 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh @@ -1054,6 +1054,7 @@ store_exists() { return 0; } store_probe() { return 0; } store_read() { echo '{}'; } store_write() { cat >/dev/null; } +store_write_and_mirror() { cat >/dev/null; } RECONCILE_LOG="$WORK/reconcile-openbao-calls.log" RECONCILE_RC=0 diff --git a/scripts/provision/zitadel-oidc-clients.sh b/scripts/provision/zitadel-oidc-clients.sh index b09c5b6db..cb068d756 100755 --- a/scripts/provision/zitadel-oidc-clients.sh +++ b/scripts/provision/zitadel-oidc-clients.sh @@ -39,6 +39,7 @@ # Usage: # # a cluster that HOSTS its own identity provider # zitadel-oidc-clients.sh sync --cluster gcp-0 --cloud gcp [--project ID] [--apply] +# [--openbao-url U --openbao-root-token-secret S --openbao-ca-file F [--mirror-openbao]] # zitadel-oidc-clients.sh sync --cluster aws-0 --cloud aws [--region R] [--apply] # # # a SECONDARY cluster consuming the primary cloud's identity provider: @@ -64,6 +65,8 @@ set -o pipefail . "$(dirname "$0")/../lib/zitadel-pat.sh" # shellcheck source=scripts/lib/openbao-api.sh . "$(dirname "$0")/../lib/openbao-api.sh" +# shellcheck source=scripts/lib/bao-map.sh +. "$(dirname "$0")/../lib/bao-map.sh" COMMAND="${1:-}" [ $# -gt 0 ] && shift @@ -114,6 +117,10 @@ GRANT_ADMIN="" OPENBAO_URL="" OPENBAO_ROOT_TOKEN_SECRET="" OPENBAO_CA_FILE="" +# --mirror-openbao: also write each consumer secret to the OpenBao path its +# ExternalSecret reads. Only gcp-0's own sync sets it (GCP parity GP-5): a fresh +# directory re-registers every client on every build, and gcp-0 reads OpenBao. +MIRROR_OPENBAO="false" while [ $# -gt 0 ]; do case "$1" in @@ -128,6 +135,7 @@ while [ $# -gt 0 ]; do --openbao-url) OPENBAO_URL="$2"; shift 2 ;; --openbao-root-token-secret) OPENBAO_ROOT_TOKEN_SECRET="$2"; shift 2 ;; --openbao-ca-file) OPENBAO_CA_FILE="$2"; shift 2 ;; + --mirror-openbao) MIRROR_OPENBAO="true"; shift ;; *) echo "unknown argument: $1" >&2; exit 2 ;; esac done @@ -144,6 +152,9 @@ if [ -n "$OPENBAO_URL" ]; then [ -n "$OPENBAO_CA_FILE" ] || { echo "--openbao-url requires --openbao-ca-file" >&2; exit 2; } [ -f "$OPENBAO_CA_FILE" ] || { echo "--openbao-ca-file ${OPENBAO_CA_FILE} not found" >&2; exit 2; } fi +if [ "$MIRROR_OPENBAO" = "true" ] && [ -z "$OPENBAO_URL" ]; then + echo "--mirror-openbao requires --openbao-url" >&2; exit 2 +fi # Which cloud's secret store holds the ZITADEL ADMIN PAT, as opposed to which # one receives the client secrets this script writes. They are the same cloud @@ -844,6 +855,59 @@ openbao_oidc_config_payload() { end' || return 1 } +# Mirror one consumer secret into the OpenBao path its ExternalSecret reads +# (--mirror-openbao). Merges into what is there -- grafana-envvars also carries +# the generated admin credentials -- and the payload wins on a shared key. +# Unmapped keys (openbao-oidc, headlamp-oauth2-proxy) are read from the managed +# store, so they are left alone. A subshell, like reconcile_openbao_oidc, so its +# token file and trap stay local; the secret goes through stdin, never argv. +mirror_to_openbao() ( + # xtrace would print the payload, and the client secret with it. + set +x + key="$1" + [ "${MIRROR_OPENBAO:-false}" = "true" ] || exit 0 + target="$(bao_target_for "$key")" || exit 0 + mount="${target%%/*}" + path="${target#*/}" + payload="$(cat)" + OPENBAO_TOKEN_CONFIG="$(umask 077 && mktemp -t openbao-mirror-curl.XXXXXX)" || exit 1 + body="$(umask 077 && mktemp -t openbao-mirror-read.XXXXXX)" || exit 1 + trap 'rm -f "$OPENBAO_TOKEN_CONFIG" "$body"' EXIT + if ! openbao_token_config_write "$OPENBAO_TOKEN_CONFIG" "${OPENBAO_ROOT_TOKEN_SECRET:-}"; then + echo "[FAILED ] ${key} -- no OpenBao root token readable from ${OPENBAO_ROOT_TOKEN_SECRET:-}" >&2 + exit 1 + fi + # Only a 404 means "nothing there yet". Any other failure (403, timeout) + # must not become an empty merge base, or the POST below would drop every + # key the payload does not carry. + code="$(openbao_req GET "${mount}/data/${path}" -o "$body" -w '%{http_code}' 2>/dev/null)" || true + case "$code" in + 200) current="$(jq -c '.data.data // {}' "$body")" \ + || { echo "[FAILED ] ${key} -- ${target} is not readable JSON; not overwriting it" >&2; exit 1; } ;; + 404) current='{}' ;; + *) echo "[FAILED ] ${key} -- reading ${target} returned HTTP ${code:-none}; not overwriting it" >&2 + exit 1 ;; + esac + if ! printf '%s\n%s\n' "$current" "$payload" \ + | jq -c -s '{data: (.[0] * .[1])}' \ + | openbao_req POST "${mount}/data/${path}" --data-binary @- >/dev/null; then + echo "[FAILED ] ${key} -- not mirrored to ${target}" >&2 + exit 1 + fi + echo "[mirrored] ${key} -> ${target}" +) + +# The managed store first, then the mirror, with one payload. +store_write_and_mirror() { + # The payload sits in a variable here, so xtrace would print it. + local - + set +x + local key="$1" payload + payload="$(cat)" + printf '%s' "$payload" | store_write "$key" || return 1 + printf '%s' "$payload" | mirror_to_openbao "$key" +} + # Point OpenBao's auth/oidc at the client ZITADEL issued. Two resources carry # it: the config (id and secret) and the default role's bound_audiences. Moving # only the config leaves every login failing on audience (design fact 5). @@ -1195,7 +1259,7 @@ cmd_sync() { echo "[dry-run] ${name} -- would converge non-secret fields in ${key}" converged=$((converged + 1)) else - printf '%s' "$desired" | store_write "$key" + printf '%s' "$desired" | store_write_and_mirror "$key" echo "[converged] ${name} -> ${key} (client id ${client_id}, secret untouched)" converged=$((converged + 1)) fi @@ -1225,7 +1289,7 @@ cmd_sync() { openbao_client_id="$client_id" fi - merge_secret "$key" "$consumer" "$client_id" "$client_secret" | store_write "$key" + merge_secret "$key" "$consumer" "$client_id" "$client_secret" | store_write_and_mirror "$key" echo "[created] ${name} -> ${key} (client ${client_id})" created=$((created + 1)) done From ef915a1b399d9e75f1ef17230edf6e847a499459 Mon Sep 17 00:00:00 2001 From: Smana Date: Tue, 29 Sep 2026 12:23:25 +0200 Subject: [PATCH 3/9] fix(zitadel): the OpenBao mirror repairs itself, writes only what it owns, and fails loudly --- .../tests/test-zitadel-oidc-clients-mirror.sh | 111 +++++++++++++--- .../test-zitadel-oidc-clients-openbao.sh | 54 +++++++- scripts/provision/zitadel-oidc-clients.sh | 120 ++++++++++++++---- 3 files changed, 240 insertions(+), 45 deletions(-) diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh b/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh index b2f8410c1..4470ea588 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh @@ -1,17 +1,20 @@ #!/usr/bin/env bash # shellcheck disable=SC2034 -# (MIRROR_OPENBAO and OPENBAO_ROOT_TOKEN_SECRET are read by the eval'd body of -# mirror_to_openbao(), which static analysis cannot see.) -# requires: jq +# (MIRROR_OPENBAO, OPENBAO_ROOT_TOKEN_SECRET, IDP_URL and HEADLAMP_OIDC_SCOPES +# are read by function bodies eval'd out of the script, which static analysis +# cannot see.) +# requires: jq openssl # # --mirror-openbao (GCP parity GP-5): a fresh directory re-registers every client -# each build, and gcp-0's consumers read OpenBao. The mirror merges into the mapped -# path (admin credentials survive), skips unmapped keys, and does nothing unset. -# mirror_to_openbao() is lifted out of the script, so this tests the code that ships. +# each build, and gcp-0's consumers read OpenBao. The mirror copies only the +# fields the sync owns onto the mapped path, writes only on a change, says when +# it skips an unmapped key, and does nothing unset. The functions are lifted out +# of the script, so this tests the code that ships. set -uo pipefail REPO_ROOT="$(cd "$(dirname "$0")/../../.." && pwd)" cd "$REPO_ROOT" || exit 1 -S=scripts/provision/zitadel-oidc-clients.sh +# Overridable so a mutant copy can be pointed at (see the fix-round proofs). +S="${ZITADEL_OIDC_CLIENTS_SCRIPT:-scripts/provision/zitadel-oidc-clients.sh}" fail=0 ok() { printf ' ok %s\n' "$1"; } bad() { printf ' FAIL %s\n' "$1"; fail=1; } @@ -21,11 +24,15 @@ body="$(sed -n '/^mirror_to_openbao() (/,/^)/p' "$S")" # shellcheck source=scripts/lib/bao-map.sh . "$REPO_ROOT/scripts/lib/bao-map.sh" eval "$body" +eval "$(sed -n '/^MIRRORED_FIELDS=(/,/^)/p' "$S")" +eval "$(sed -n '/^store_write_and_mirror() {/,/^}/p' "$S")" +eval "$(sed -n '/^merge_secret() {/,/^}/p' "$S")" T="$(mktemp -d)"; trap 'rm -rf "$T"' EXIT openbao_token_config_write() { : >"$1"; } GET_BODY='{"data":{"data":{"GF_SECURITY_ADMIN_USER":"admin","GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"old"}}}' GET_CODE=200 +POST_RC=0 openbao_req() { printf '%s %s\n' "$1" "$2" >>"$T/calls" local method="$1" out=/dev/null @@ -33,12 +40,14 @@ openbao_req() { while [ $# -gt 0 ]; do case "$1" in -o) out="$2"; shift 2 ;; *) shift ;; esac; done case "$method" in GET) printf '%s' "$GET_BODY" >"$out"; printf '%s' "$GET_CODE" ;; - POST) cat >"$T/body" ;; + POST) cat >"$T/body"; return "$POST_RC" ;; esac } +reset() { : >"$T/calls"; rm -f "$T/body"; } MIRROR_OPENBAO=true OPENBAO_ROOT_TOKEN_SECRET=fixture +reset printf '%s' '{"GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"new"}' \ | mirror_to_openbao observability-victoria-metrics-k8s-stack-grafana-envvars >/dev/null grep -qx 'POST platform/data/victoria-metrics/grafana-envvars' "$T/calls" \ @@ -46,31 +55,103 @@ grep -qx 'POST platform/data/victoria-metrics/grafana-envvars' "$T/calls" \ [ "$(jq -c '.data' "$T/body")" = '{"GF_SECURITY_ADMIN_USER":"admin","GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"new"}' ] \ && ok "merged: the admin user kept, the client id replaced" || bad "merge: $(cat "$T/body")" -: >"$T/calls"; rm -f "$T/body" +# I-1: a field the sync does not own keeps OpenBao's value, even when the +# store's blob carries an older one (seed's admin password, rotated in OpenBao). +reset +GET_BODY='{"data":{"data":{"GF_SECURITY_ADMIN_PASSWORD":"rotated-in-bao","GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"old"}}}' # pragma: allowlist secret +store_blob='{"GF_SECURITY_ADMIN_PASSWORD":"stale-in-store","GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"new","GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET":"fake"}' # pragma: allowlist secret +want='{"GF_SECURITY_ADMIN_PASSWORD":"rotated-in-bao","GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"new","GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET":"fake"}' # pragma: allowlist secret +printf '%s' "$store_blob" | mirror_to_openbao observability-victoria-metrics-k8s-stack-grafana-envvars >/dev/null +[ "$(jq -c '.data' "$T/body" 2>/dev/null)" = "$want" ] \ + && ok "a field the sync does not own keeps OpenBao's value" || bad "unowned field: $(cat "$T/body" 2>/dev/null)" + +# No KV version churn: nothing written when OpenBao already holds the fields. +reset +GET_BODY='{"data":{"data":{"GF_SECURITY_ADMIN_USER":"admin","GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"same"}}}' +out="$(printf '%s' '{"GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"same"}' \ + | mirror_to_openbao observability-victoria-metrics-k8s-stack-grafana-envvars)"; rc=$? +[ "$rc" -eq 0 ] && ! grep -q '^POST' "$T/calls" && [ ! -e "$T/body" ] \ + && ok "unchanged: no write, so no new KV version" || bad "unchanged: rc=$rc, calls $(cat "$T/calls")" + +reset GET_CODE=404 GET_BODY='{"errors":[]}' -printf '%s' '{"client_id":"new"}' | mirror_to_openbao harbor-oidc >/dev/null +printf '%s' '{"client_id":"new","not_ours":"x"}' | mirror_to_openbao harbor-oidc >/dev/null [ "$(jq -c '.data' "$T/body" 2>/dev/null)" = '{"client_id":"new"}' ] \ - && ok "absent path (404): written fresh" || bad "404 case: $(cat "$T/body" 2>/dev/null)" + && ok "absent path (404): written fresh, owned fields only" || bad "404 case: $(cat "$T/body" 2>/dev/null)" -: >"$T/calls"; rm -f "$T/body" +reset GET_CODE=403 printf '%s' '{"client_id":"new"}' | mirror_to_openbao harbor-oidc >/dev/null 2>&1; rc=$? [ "$rc" -ne 0 ] && [ ! -e "$T/body" ] \ && ok "unreadable path (403): fails and overwrites nothing" || bad "403 case: rc=$rc, wrote $(cat "$T/body" 2>/dev/null)" GET_CODE=200 -: >"$T/calls" -printf '%s' '{"client_id":"x"}' | mirror_to_openbao openbao-oidc +# I-3 (M1): OpenBao refusing the write is a failure, not a success. +reset +GET_BODY='{"data":{"data":{}}}' +POST_RC=22 +printf '%s' '{"client_id":"new"}' | mirror_to_openbao harbor-oidc >/dev/null 2>&1; rc=$? +[ "$rc" -ne 0 ] && ok "a refused write (POST fails): mirror_to_openbao fails" || bad "a refused write returned 0" +POST_RC=0 + +# I-2: an unmapped key is skipped out loud, and still succeeds. +reset +err="$(printf '%s' '{"client_id":"x"}' | mirror_to_openbao openbao-oidc 2>&1 >/dev/null)"; rc=$? [ ! -s "$T/calls" ] && ok "openbao-oidc (unmapped) never reaches OpenBao" || bad "an unmapped key reached OpenBao" +[ "$rc" -eq 0 ] && [ "$err" = "[skip ] openbao-oidc: no OpenBao path in scripts/lib/bao-map.sh" ] \ + && ok "an unmapped key says so on stderr, exit 0" || bad "unmapped skip: rc=$rc stderr='$err'" -: >"$T/calls" +reset MIRROR_OPENBAO=false printf '%s' '{"a":"b"}' | mirror_to_openbao harbor-oidc [ ! -s "$T/calls" ] && ok "without --mirror-openbao nothing is written" || bad "wrote without the flag" +MIRROR_OPENBAO=true + +# I-3 (M2): a failed store write stops the mirror; a failed mirror is told apart. +reset +store_write() { cat >/dev/null; return 1; } +printf '%s' '{"client_id":"new"}' | store_write_and_mirror harbor-oidc >/dev/null 2>&1; rc=$? +[ "$rc" -eq 1 ] && [ ! -s "$T/calls" ] \ + && ok "a failed store write: returns 1, OpenBao never called" || bad "store write failed: rc=$rc, calls $(cat "$T/calls")" +reset +store_write() { cat >/dev/null; } +POST_RC=22 +printf '%s' '{"client_id":"new"}' | store_write_and_mirror harbor-oidc >/dev/null 2>&1; rc=$? +[ "$rc" -eq 2 ] && ok "a failed mirror after a good store write: returns 2" || bad "mirror failure: rc=$rc, want 2" +POST_RC=0 + +# MIRRORED_FIELDS is the one list of what the sync owns: every key merge_secret +# writes must be in it, or that field never reaches OpenBao. +store_exists() { return 1; } +store_read() { :; } +IDP_URL="https://auth.example.invalid" +HEADLAMP_OIDC_SCOPES="profile,email,groups" +missing="" +for consumer in grafana headlamp flux-ui harbor openbao headlamp-proxy; do + keys="$(merge_secret k "$consumer" id-fixture fake-secret | jq -r 'keys[]')" # pragma: allowlist secret + [ -n "$keys" ] || missing="${missing} ${consumer}:" + while IFS= read -r k; do + [ -z "$k" ] && continue + printf '%s\n' "${MIRRORED_FIELDS[@]}" | grep -qxF -- "$k" || missing="${missing} ${consumer}:${k}" + done <<< "$keys" +done +[ -z "$missing" ] && ok "every field merge_secret writes is in MIRRORED_FIELDS" || bad "not in MIRRORED_FIELDS:${missing}" grep -q -- '--mirror-openbao) MIRROR_OPENBAO="true"; shift ;;' "$S" && ok "--mirror-openbao is parsed" || bad "--mirror-openbao is not parsed" [ "$(grep -c 'store_write_and_mirror "\$key"' "$S")" -eq 2 ] \ && ok "both consumer writes go through store_write_and_mirror" || bad "a consumer write bypasses the mirror" +# --mirror-openbao without --openbao-url is refused before any ZITADEL or cloud +# call: every CLI the script could reach is a stub that records being called. +mkdir -p "$T/bin" +for cli in curl gcloud aws kubectl bao; do + printf '#!/usr/bin/env bash\necho %s >> "%s/cli-calls"\nexit 1\n' "$cli" "$T" > "$T/bin/$cli" + chmod +x "$T/bin/$cli" +done +out="$(PATH="$T/bin:$PATH" timeout 10 bash "$S" sync --cluster gcp-0 --cloud gcp --mirror-openbao 2>&1)"; rc=$? +[ "$rc" -eq 2 ] && grep -qF -- '--mirror-openbao requires --openbao-url' <<< "$out" && [ ! -e "$T/cli-calls" ] \ + && ok "--mirror-openbao without --openbao-url: exit 2, no CLI called" \ + || bad "flag guard: rc=$rc, called $(cat "$T/cli-calls" 2>/dev/null | tr '\n' ' '), out: $(head -3 <<< "$out")" + [ "$fail" -eq 0 ] && echo "all checks passed" exit "$fail" diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh index 664d0d190..9b290f6e2 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh @@ -1034,7 +1034,7 @@ contains "$out" "[FAILED ]" "writes that do not stick: says [FAILED echo echo "== cmd_sync wiring: the reconcile runs once, after the loop (Task 3) ==" -for f in cmd_sync oidc_config_payload; do +for f in cmd_sync oidc_config_payload store_write_and_mirror; do body="$(sed -n "/^${f}() {/,/^}/p" "$CONSUMERS_SRC")" [ -n "$body" ] || { echo " FAIL could not extract ${f}() from $CONSUMERS_SRC" >&2; fail=1; } eval "$body" @@ -1053,8 +1053,17 @@ converge_secret() { echo '{}'; } store_exists() { return 0; } store_probe() { return 0; } store_read() { echo '{}'; } -store_write() { cat >/dev/null; } -store_write_and_mirror() { cat >/dev/null; } +STORE_WRITE_RC=0 +store_write() { cat >/dev/null; return "$STORE_WRITE_RC"; } +# The mirror's own behaviour is test-zitadel-oidc-clients-mirror.sh; here only +# whether cmd_sync calls it, and what it does with a failure. +MIRROR_LOG="$WORK/mirror-calls.log" +MIRROR_RC=0 +mirror_to_openbao() { + cat >/dev/null + printf 'MIRROR %s\n' "$1" >> "$MIRROR_LOG" + return "$MIRROR_RC" +} RECONCILE_LOG="$WORK/reconcile-openbao-calls.log" RECONCILE_RC=0 @@ -1076,7 +1085,7 @@ run_cmd_sync() { } echo "-- the existing-app path --" -: > "$RECONCILE_LOG" +: > "$RECONCILE_LOG"; : > "$MIRROR_LOG" CONSUMERS=("openbao|${WIRE_CB}|openbao-oidc") app_id_by_name() { echo "app-1"; } app_get() { jq -n --arg r "$WIRE_CB" --arg cid "existing-openbao-id" \ @@ -1087,10 +1096,20 @@ check "existing-app path: cmd_sync succeeds" "0" "$rc" check "existing-app path: reconcile called once" "1" "$(wc -l < "$RECONCILE_LOG")" check "existing-app path: called with (key, id)" "CALL openbao-oidc existing-openbao-id" \ "$(cat "$RECONCILE_LOG")" +# The store is already converged (converge_secret == store_read), so only the +# retry in that branch can repair a mirror an earlier run failed. +check "already converged, --apply: the mirror is retried" "MIRROR openbao-oidc" "$(cat "$MIRROR_LOG")" + +: > "$RECONCILE_LOG"; : > "$MIRROR_LOG" +APPLY=false +run_cmd_sync +check "already converged, dry run: cmd_sync succeeds" "0" "$rc" +check "already converged, dry run: OpenBao is never written" "" "$(cat "$MIRROR_LOG")" +APPLY=true echo echo "-- the create path --" -: > "$RECONCILE_LOG" +: > "$RECONCILE_LOG"; : > "$MIRROR_LOG" app_id_by_name() { echo ""; } api_or_fail() { printf '{"clientId":"created-openbao-id","clientSecret":"created-secret"}'; } # pragma: allowlist secret APPLY=true @@ -1099,6 +1118,31 @@ check "create path: cmd_sync succeeds" "0" "$rc" check "create path: reconcile called once" "1" "$(wc -l < "$RECONCILE_LOG")" check "create path: called with (key, id)" "CALL openbao-oidc created-openbao-id" \ "$(cat "$RECONCILE_LOG")" +check "create path: the new secret is mirrored" "MIRROR openbao-oidc" "$(cat "$MIRROR_LOG")" + +echo +echo "-- a failed mirror: the loop, both reconciles and the summary still run, then exit 1 --" +: > "$RECONCILE_LOG"; : > "$MIRROR_LOG" +CONSUMERS=("harbor|https://harbor.priv.aws.ogenki.io/c/oidc/callback|harbor-oidc" + "openbao|${WIRE_CB}|openbao-oidc") +MIRROR_RC=1 +run_cmd_sync +check "mirror fails: cmd_sync exits 1" "1" "$rc" +check "mirror fails: the next consumer is still processed" "2" "$(wc -l < "$MIRROR_LOG")" +check "mirror fails: reconcile still called once" "1" "$(wc -l < "$RECONCILE_LOG")" +contains "$out" "created: 2" "mirror fails: the summary line still prints" +contains "$out" "not mirrored to OpenBao: harbor-oidc openbao-oidc" "mirror fails: the summary names each failed mirror" +MIRROR_RC=0 + +echo +echo "-- a failed store write: stops at once, OpenBao untouched --" +: > "$RECONCILE_LOG"; : > "$MIRROR_LOG" +STORE_WRITE_RC=1 +run_cmd_sync +check "store write fails: cmd_sync exits 1" "1" "$rc" +check "store write fails: no mirror is attempted" "" "$(cat "$MIRROR_LOG")" +STORE_WRITE_RC=0 +CONSUMERS=("openbao|${WIRE_CB}|openbao-oidc") echo echo "-- a reconcile failure exits 1, after the summary --" diff --git a/scripts/provision/zitadel-oidc-clients.sh b/scripts/provision/zitadel-oidc-clients.sh index cb068d756..9644dba44 100755 --- a/scripts/provision/zitadel-oidc-clients.sh +++ b/scripts/provision/zitadel-oidc-clients.sh @@ -606,8 +606,10 @@ merge_secret() { # It measures the STRING, not the decoded bytes, so the base64 has to # be truncated to the cipher length rather than sized to decode into # it. `head -c 32` is the recipe the chart's own values.yaml gives. - cookie_secret="$(jq -r '."cookie-secret" // empty' <<< "$existing")" - [ -n "$cookie_secret" ] || cookie_secret="$(openssl rand -base64 32 | head -c 32)" + # `|| return 1` on both: cmd_sync calls this in $( ), where errexit is + # not inherited, and an empty cookie secret would be written as is. + cookie_secret="$(jq -r '."cookie-secret" // empty' <<< "$existing")" || return 1 + [ -n "$cookie_secret" ] || cookie_secret="$(openssl rand -base64 32 | head -c 32)" || return 1 fi { @@ -855,49 +857,87 @@ openbao_oidc_config_payload() { end' || return 1 } +# The fields this script owns in a consumer secret: every key merge_secret and +# converge_secret write. The mirror copies only these. The rest of a blob +# belongs to another writer -- seed's GF_SECURITY_ADMIN_PASSWORD in +# grafana-envvars -- and OpenBao's copy of it may be newer than the store's (a +# rotation made in OpenBao), so the mirror never overwrites it. +# test-zitadel-oidc-clients-mirror.sh fails when merge_secret writes a key +# missing here. +MIRRORED_FIELDS=( + GF_AUTH_GENERIC_OAUTH_CLIENT_ID GF_AUTH_GENERIC_OAUTH_CLIENT_SECRET + OIDC_CLIENT_ID OIDC_CLIENT_SECRET OIDC_ISSUER_URL OIDC_SCOPES + OIDC_VALIDATOR_CLIENT_ID OIDC_VALIDATOR_ISSUER_URL + clientID clientSecret + client_id client_secret endpoint + client-id client-secret cookie-secret +) + # Mirror one consumer secret into the OpenBao path its ExternalSecret reads -# (--mirror-openbao). Merges into what is there -- grafana-envvars also carries -# the generated admin credentials -- and the payload wins on a shared key. -# Unmapped keys (openbao-oidc, headlamp-oauth2-proxy) are read from the managed -# store, so they are left alone. A subshell, like reconcile_openbao_oidc, so its -# token file and trap stay local; the secret goes through stdin, never argv. +# (--mirror-openbao): MIRRORED_FIELDS from the store's blob, onto what OpenBao +# holds. Writes only when that changes OpenBao's value, so a re-run adds no KV +# version. Unmapped keys (openbao-oidc, headlamp-oauth2-proxy) are read from the +# managed store; they are skipped, and said so. A subshell, like +# reconcile_openbao_oidc, so its temp files and trap stay local. Secrets move +# through stdin and 0700-directory files, never argv. mirror_to_openbao() ( # xtrace would print the payload, and the client secret with it. set +x key="$1" [ "${MIRROR_OPENBAO:-false}" = "true" ] || exit 0 - target="$(bao_target_for "$key")" || exit 0 + if ! target="$(bao_target_for "$key")"; then + echo "[skip ] ${key}: no OpenBao path in scripts/lib/bao-map.sh" >&2 + exit 0 + fi mount="${target%%/*}" path="${target#*/}" - payload="$(cat)" - OPENBAO_TOKEN_CONFIG="$(umask 077 && mktemp -t openbao-mirror-curl.XXXXXX)" || exit 1 - body="$(umask 077 && mktemp -t openbao-mirror-read.XXXXXX)" || exit 1 - trap 'rm -f "$OPENBAO_TOKEN_CONFIG" "$body"' EXIT + # One directory, trapped the moment it exists, so no second mktemp can + # fail and strand the first. The path is baked in at trap-set time. + tmp="$(umask 077 && mktemp -d -t openbao-mirror.XXXXXX)" || exit 1 + # shellcheck disable=SC2064 + trap "rm -rf '$tmp'" EXIT + cat > "$tmp/payload" || exit 1 + OPENBAO_TOKEN_CONFIG="$tmp/token" if ! openbao_token_config_write "$OPENBAO_TOKEN_CONFIG" "${OPENBAO_ROOT_TOKEN_SECRET:-}"; then echo "[FAILED ] ${key} -- no OpenBao root token readable from ${OPENBAO_ROOT_TOKEN_SECRET:-}" >&2 exit 1 fi - # Only a 404 means "nothing there yet". Any other failure (403, timeout) - # must not become an empty merge base, or the POST below would drop every - # key the payload does not carry. - code="$(openbao_req GET "${mount}/data/${path}" -o "$body" -w '%{http_code}' 2>/dev/null)" || true + # Only a 404 means "nothing there yet". Any other answer (403, a TLS or CA + # error, a timeout) must not become an empty merge base. curl's stderr is + # shown for those: it holds curl's own error line only -- the token is in a + # -K file and a body goes to -o -- and a 404's "(22)" line is expected. + code="$(openbao_req GET "${mount}/data/${path}" -o "$tmp/read" -w '%{http_code}' 2>"$tmp/err")" || true case "$code" in - 200) current="$(jq -c '.data.data // {}' "$body")" \ + 200) jq -ce '.data.data // {} | objects' "$tmp/read" > "$tmp/current" \ || { echo "[FAILED ] ${key} -- ${target} is not readable JSON; not overwriting it" >&2; exit 1; } ;; - 404) current='{}' ;; + 404) printf '{}' > "$tmp/current" ;; *) echo "[FAILED ] ${key} -- reading ${target} returned HTTP ${code:-none}; not overwriting it" >&2 + cat "$tmp/err" >&2 exit 1 ;; esac - if ! printf '%s\n%s\n' "$current" "$payload" \ - | jq -c -s '{data: (.[0] * .[1])}' \ - | openbao_req POST "${mount}/data/${path}" --data-binary @- >/dev/null; then + # Field NAMES go in as --args (none is secret); both values on stdin. + if ! cat "$tmp/current" "$tmp/payload" | jq -cn ' + input as $cur | input as $p | + ($cur + ($p | with_entries(select(.key | IN($ARGS.positional[]))))) as $new | + if $new == $cur then empty else {data: $new} end' \ + --args "${MIRRORED_FIELDS[@]}" > "$tmp/write"; then + echo "[FAILED ] ${key} -- could not merge into ${target}; not overwriting it" >&2 + exit 1 + fi + if [ ! -s "$tmp/write" ]; then + echo "[ok ] ${key} -- ${target} already holds these fields" + exit 0 + fi + if ! openbao_req POST "${mount}/data/${path}" --data-binary @- < "$tmp/write" >/dev/null; then echo "[FAILED ] ${key} -- not mirrored to ${target}" >&2 exit 1 fi echo "[mirrored] ${key} -> ${target}" ) -# The managed store first, then the mirror, with one payload. +# The managed store first, then the mirror, with one payload. Returns 1 when +# the store write fails and 2 when only the mirror does: cmd_sync stops on the +# first and carries on past the second (failed_mirrors there). store_write_and_mirror() { # The payload sits in a variable here, so xtrace would print it. local - @@ -905,7 +945,7 @@ store_write_and_mirror() { local key="$1" payload payload="$(cat)" printf '%s' "$payload" | store_write "$key" || return 1 - printf '%s' "$payload" | mirror_to_openbao "$key" + printf '%s' "$payload" | mirror_to_openbao "$key" || return 2 } # Point OpenBao's auth/oidc at the client ZITADEL issued. Two resources carry @@ -1104,6 +1144,9 @@ cmd_sync() { grant_admin_role "$GRANT_ADMIN" "$project_id" local created=0 skipped=0 updated=0 converged=0 + # A failed mirror does not stop the loop, like openbao_failed below: the + # other consumers and both reconciles still run, then the sync exits 1. + local failed_mirrors="" wrc=0 # Fed to reconcile_openbao_oidc after the loop -- see the consumer's own # branches below for where each is set. Empty stays empty on a dry run # (reconcile_openbao_oidc treats that as its own skip) and on any topology @@ -1255,11 +1298,24 @@ cmd_sync() { desired="$(converge_secret "$consumer" "$client_id" "$existing_secret")" if [ "$desired" = "$existing_secret" ]; then echo "[ok ] ${name} -- ${key} already converged" + # A mirror that failed after its store write leaves the store + # converged, so nothing else here would ever retry it. Never on + # a dry run; a no-op without --mirror-openbao. + if [ "$APPLY" = "true" ]; then + printf '%s' "$existing_secret" | mirror_to_openbao "$key" \ + || failed_mirrors="${failed_mirrors}${failed_mirrors:+ }${key}" + fi elif [ "$APPLY" != "true" ]; then echo "[dry-run] ${name} -- would converge non-secret fields in ${key}" converged=$((converged + 1)) else - printf '%s' "$desired" | store_write_and_mirror "$key" + wrc=0 + printf '%s' "$desired" | store_write_and_mirror "$key" || wrc=$? + case "$wrc" in + 0) ;; + 2) failed_mirrors="${failed_mirrors}${failed_mirrors:+ }${key}" ;; + *) exit 1 ;; + esac echo "[converged] ${name} -> ${key} (client id ${client_id}, secret untouched)" converged=$((converged + 1)) fi @@ -1289,7 +1345,17 @@ cmd_sync() { openbao_client_id="$client_id" fi - merge_secret "$key" "$consumer" "$client_id" "$client_secret" | store_write_and_mirror "$key" + # Built first, so a failed merge stops here instead of feeding the + # store an empty payload. + local merged + merged="$(merge_secret "$key" "$consumer" "$client_id" "$client_secret")" || exit 1 + wrc=0 + printf '%s' "$merged" | store_write_and_mirror "$key" || wrc=$? + case "$wrc" in + 0) ;; + 2) failed_mirrors="${failed_mirrors}${failed_mirrors:+ }${key}" ;; + *) exit 1 ;; + esac echo "[created] ${name} -> ${key} (client ${client_id})" created=$((created + 1)) done @@ -1314,8 +1380,12 @@ cmd_sync() { echo echo "This was a DRY RUN. Nothing was created and nothing was written." fi + if [ -n "$failed_mirrors" ]; then + echo "[FAILED ] not mirrored to OpenBao: ${failed_mirrors} -- the managed store has them; re-run sync --apply" >&2 + fi [ "$openbao_failed" -eq 0 ] || exit 1 + [ -z "$failed_mirrors" ] || exit 1 } case "$COMMAND" in From 6ba74878263fb32f426c59518151e87a339ed1cb Mon Sep 17 00:00:00 2001 From: Smana Date: Tue, 29 Sep 2026 12:29:34 +0200 Subject: [PATCH 4/9] fix(secrets): store_write refuses a short payload write Called under ||, store_write runs with errexit off. A cat cut short by a full disk, or a failed mktemp or jq, fell through to versions add / put-secret-value, which stored a truncated secret and returned 0. Every step before the cloud call now exits 1 explicitly, on both clouds. --- scripts/ci/tests/test-cloud-secret-store.sh | 37 +++++++++++++++++++++ scripts/lib/cloud-secret-store.sh | 17 ++++++---- 2 files changed, 48 insertions(+), 6 deletions(-) diff --git a/scripts/ci/tests/test-cloud-secret-store.sh b/scripts/ci/tests/test-cloud-secret-store.sh index fcc48962c..a16dd5f66 100755 --- a/scripts/ci/tests/test-cloud-secret-store.sh +++ b/scripts/ci/tests/test-cloud-secret-store.sh @@ -221,4 +221,41 @@ rm -rf "$FAILTMP2" rm -rf "$FAILSTUB" "$FAILTMP" +# --- store_write: a failed payload write never reaches the cloud ------------ +# +# Called under `||`, as every mirror and PAT caller does, store_write runs with +# errexit off. A `cat > "$payload"` cut short (a full disk) then fell through to +# `versions add` / `put-secret-value`, which stored the truncated secret and +# returned 0. The failing `cat` is a function so it can land half the payload +# first, the way a full disk does. +short_write() { # cloud label -> " " + : > "$STUB_LOG" + local rc=0 + PATH="$STUB:$PATH" bash -c ' + set -o errexit -o nounset -o pipefail + # shellcheck source=scripts/lib/cloud-secret-store.sh + . "'"$HERE"'/../../lib/cloud-secret-store.sh" + cat() { command cat >/dev/null; printf "%s" "{\"pat\":\"trunc"; return 1; } + CLOUD="$1" REGION=eu-west-3 GCP_PROJECT=proj + store_write existing-secret <<< "{\"pat\":\"fake-full-token\"}" || exit $? + ' _ "$1" || rc=$? + echo "$rc $(grep -cE 'versions add|put-secret-value|create-secret' "$STUB_LOG")" +} +check "aws short payload write: non-zero, no put-secret-value" "1 0" "$(short_write aws)" +check "gcp short payload write: non-zero, no versions add" "1 0" "$(short_write gcp)" + +# mktemp failing (TMPDIR gone) is the same fall-through with an empty path. +no_tmp() { + : > "$STUB_LOG" + local rc=0 + PATH="$STUB:$PATH" TMPDIR=/nonexistent/cloud-secret-store-test bash -c ' + . "'"$HERE"'/../../lib/cloud-secret-store.sh" + CLOUD="$1" REGION=eu-west-3 GCP_PROJECT=proj + store_write existing-secret <<< "{\"pat\":\"fake-full-token\"}" || exit $? + ' _ "$1" 2>/dev/null || rc=$? + echo "$rc $(grep -cE 'versions add|put-secret-value|create-secret' "$STUB_LOG")" +} +check "aws mktemp failure: non-zero, no cloud write" "1 0" "$(no_tmp aws)" +check "gcp mktemp failure: non-zero, no cloud write" "1 0" "$(no_tmp gcp)" + exit $fail diff --git a/scripts/lib/cloud-secret-store.sh b/scripts/lib/cloud-secret-store.sh index 37e82f94b..dbcc67986 100644 --- a/scripts/lib/cloud-secret-store.sh +++ b/scripts/lib/cloud-secret-store.sh @@ -77,12 +77,16 @@ store_read() { # is private, so it can't step on the caller's. # * AWS goes through jq into --cli-input-json rather than --secret-string, # which is what lets the payload be an arbitrary JSON document. +# * every step before the cloud call ends in `|| exit 1`. A caller that +# writes `store_write … || …` turns errexit off for the whole body, and a +# `cat` cut short by a full disk then went on to store the truncated +# secret and return 0. store_write() { local name="$1" ( local payload body - payload=$(umask 077 && mktemp -t cloud-secret.XXXXXX) - body=$(umask 077 && mktemp -t cloud-secret-body.XXXXXX) + payload=$(umask 077 && mktemp -t cloud-secret.XXXXXX) || exit 1 + body=$(umask 077 && mktemp -t cloud-secret-body.XXXXXX) || { rm -f "$payload"; exit 1; } # Paths are baked into the trap string at trap-SET time, not expanded when # it fires. Two earlier attempts used '...$payload...': on an errexit abort # bash pops the function's locals before running the EXIT trap, so under the @@ -91,19 +95,19 @@ store_write() { # leaving the plaintext payload on disk. mktemp paths never contain quotes. # shellcheck disable=SC2064 trap "shred -u '$payload' '$body' 2>/dev/null || rm -f '$payload' '$body'" EXIT - cat > "$payload" + cat > "$payload" || exit 1 case "$CLOUD" in aws) if store_exists "$name"; then jq --arg id "$name" '{SecretId: $id, SecretString: (. | tostring)}' \ - < "$payload" > "$body" + < "$payload" > "$body" || exit 1 aws secretsmanager put-secret-value ${REGION:+--region "$REGION"} \ --cli-input-json "file://${body}" >/dev/null else jq --arg n "$name" --arg d "${STORE_WRITE_DESCRIPTION:-Written by cloud-secret-store.sh}" \ '{Name: $n, Description: $d, SecretString: (. | tostring)}' \ - < "$payload" > "$body" + < "$payload" > "$body" || exit 1 aws secretsmanager create-secret ${REGION:+--region "$REGION"} \ --cli-input-json "file://${body}" >/dev/null fi @@ -112,7 +116,8 @@ store_write() { store_exists "$name" || gcp_gcloud secrets create "$name" \ ${GCP_PROJECT:+--project "$GCP_PROJECT"} \ --replication-policy=automatic \ - --labels=managed-by="${STORE_WRITE_LABEL:-cloud-secret-store}" >/dev/null + --labels=managed-by="${STORE_WRITE_LABEL:-cloud-secret-store}" >/dev/null \ + || exit 1 gcp_gcloud secrets versions add "$name" \ ${GCP_PROJECT:+--project "$GCP_PROJECT"} \ --data-file="$payload" >/dev/null From 5917a4b6793fad30c6c49991f9b0178a46827ec3 Mon Sep 17 00:00:00 2001 From: Smana Date: Tue, 29 Sep 2026 12:31:20 +0200 Subject: [PATCH 5/9] fix(gcp): a hosted ZITADEL's project id reaches gke/configure through Secret Manager --- opentofu/gcp/gke/configure/data.tf | 17 +++++++++++++++++ opentofu/gcp/gke/configure/kubernetes.tf | 3 ++- opentofu/gcp/gke/configure/locals.tf | 8 ++++++++ scripts/ci/tests/test-gcp-zitadel-project-id.sh | 17 +++++++++++++++++ scripts/provision/zitadel-oidc-clients.sh | 10 ++++++++++ 5 files changed, 54 insertions(+), 1 deletion(-) create mode 100755 scripts/ci/tests/test-gcp-zitadel-project-id.sh diff --git a/opentofu/gcp/gke/configure/data.tf b/opentofu/gcp/gke/configure/data.tf index e0551958b..2301d420f 100644 --- a/opentofu/gcp/gke/configure/data.tf +++ b/opentofu/gcp/gke/configure/data.tf @@ -39,3 +39,20 @@ data "google_secret_manager_secret_version" "openbao_root_token" { secret = var.openbao_root_token_secret_name project = var.project_id } + +# This directory's ZITADEL project id when the cluster HOSTS it (GCP parity GP-4). +# A fresh directory gets a new id every build; zitadel-oidc-clients.sh publishes +# it here in stage 3. Read at plan time so this stack's later apply agrees with +# the value the sync patched into the ConfigMap. Listed before it is read: absent +# is "first build", and var.zitadel_project_id stands in until stage 3. +data "google_secret_manager_secrets" "zitadel_project" { + count = var.deploy_identity_provider ? 1 : 0 + project = var.project_id + filter = "name:zitadel-project-id" +} + +data "google_secret_manager_secret_version" "zitadel_project" { + count = local.zitadel_project_secret_present ? 1 : 0 + secret = "zitadel-project-id" # pragma: allowlist secret + project = var.project_id +} diff --git a/opentofu/gcp/gke/configure/kubernetes.tf b/opentofu/gcp/gke/configure/kubernetes.tf index edb717d90..ba518b8ea 100644 --- a/opentofu/gcp/gke/configure/kubernetes.tf +++ b/opentofu/gcp/gke/configure/kubernetes.tf @@ -135,7 +135,8 @@ resource "kubectl_manifest" "flux_cluster_vars" { # scope has to name it. Substituting it means a rename is caught by # check-substitution.py instead of surfacing as a bare `invalid_grant` # with oauth2-proxy, the exchange proxy and Headlamp all reporting healthy. - zitadel_project_id = var.zitadel_project_id + # Live-reconciled when this cluster hosts ZITADEL: see local.zitadel_project_id. + zitadel_project_id = local.zitadel_project_id } }) server_side_apply = true diff --git a/opentofu/gcp/gke/configure/locals.tf b/opentofu/gcp/gke/configure/locals.tf index 2ade56434..f07056dcc 100644 --- a/opentofu/gcp/gke/configure/locals.tf +++ b/opentofu/gcp/gke/configure/locals.tf @@ -31,4 +31,12 @@ locals { # hosts the IdP" become unanswerable from configuration in the first place. # See ADR-0024 and the two-gate note on var.deploy_identity_provider. identity_provider_url = var.deploy_identity_provider ? "https://auth.${var.public_domain_name}" : var.identity_provider_url + + # The list filter is a substring match; contains() makes it exact. + zitadel_project_secret_present = var.deploy_identity_provider && contains( + [for s in try(data.google_secret_manager_secrets.zitadel_project[0].secrets, []) : s.secret_id], + "zitadel-project-id" + ) + # Not secret: nonsensitive() keeps the whole ConfigMap out of (sensitive) diffs. + zitadel_project_id = local.zitadel_project_secret_present ? nonsensitive(jsondecode(data.google_secret_manager_secret_version.zitadel_project[0].secret_data)["project_id"]) : var.zitadel_project_id } diff --git a/scripts/ci/tests/test-gcp-zitadel-project-id.sh b/scripts/ci/tests/test-gcp-zitadel-project-id.sh new file mode 100755 index 000000000..32d1434f3 --- /dev/null +++ b/scripts/ci/tests/test-gcp-zitadel-project-id.sh @@ -0,0 +1,17 @@ +#!/usr/bin/env bash +# +# GCP parity GP-4: a fresh directory has a new project id every build. The sync +# publishes it, and gke/configure reads it, so the standalone configure apply +# that runs after stage 3 cannot put the committed id back. +set -uo pipefail +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/../../.." && pwd)" +C="$ROOT/opentofu/gcp/gke/configure" +fails=0 +f() { echo "FAIL $*"; fails=$((fails + 1)); } +grep -Eq '^[[:space:]]*zitadel_project_id[[:space:]]*=[[:space:]]*local\.zitadel_project_id[[:space:]]*$' "$C/kubernetes.tf" || f "the ConfigMap does not publish local.zitadel_project_id" +grep -q 'data "google_secret_manager_secrets" "zitadel_project"' "$C/data.tf" || f "configure does not list zitadel-project-id before reading it" +grep -q 'data "google_secret_manager_secret_version" "zitadel_project"' "$C/data.tf" || f "configure does not read zitadel-project-id" +grep -Eq 'zitadel_project_id[[:space:]]*=.*var\.zitadel_project_id' "$C/locals.tf" || f "no fallback to the committed id" +grep -q 'store_write zitadel-project-id' "$ROOT/scripts/provision/zitadel-oidc-clients.sh" || f "the sync never publishes zitadel-project-id" +[ "$fails" -eq 0 ] || exit 1 +echo PASS diff --git a/scripts/provision/zitadel-oidc-clients.sh b/scripts/provision/zitadel-oidc-clients.sh index 9644dba44..b21c859f4 100755 --- a/scripts/provision/zitadel-oidc-clients.sh +++ b/scripts/provision/zitadel-oidc-clients.sh @@ -1366,6 +1366,16 @@ cmd_sync() { # is an app client id rather than the project id. reconcile_workforce_audience "$project_id" + # GCP hosting: publish this directory's project id for gke/configure, which + # reads it at plan time (GCP parity GP-4). A fresh directory gets a new id + # every build, and the committed one would otherwise come back on the next + # configure apply. + if [ "$CLOUD" = "gcp" ] && [ "$IDP_CLOUD" = "$CLOUD" ] && [ "$APPLY" = "true" ] \ + && [ "$project_id" != "DRYRUN-PROJECT" ]; then + printf '%s' "$project_id" | jq -Rc '{project_id: .}' | store_write zitadel-project-id + echo "project: published to zitadel-project-id" + fi + # Same reasoning as the workforce provider above: OpenBao's client id is # only known once the "openbao" consumer's app has been found or created. # `|| openbao_failed=1` rather than exit here so the summary below still From d8d39323bb9687852c45e9d518282ef3a0ea38b8 Mon Sep 17 00:00:00 2001 From: Smana Date: Tue, 29 Sep 2026 12:32:48 +0200 Subject: [PATCH 6/9] fix(zitadel): the chart's fresh admin PAT wins over a stored one --- scripts/ci/tests/test-zitadel-pat.sh | 26 +++++-- scripts/lib/zitadel-pat.sh | 87 ++++++++++------------- scripts/provision/zitadel-oidc-clients.sh | 6 +- 3 files changed, 60 insertions(+), 59 deletions(-) diff --git a/scripts/ci/tests/test-zitadel-pat.sh b/scripts/ci/tests/test-zitadel-pat.sh index 952ba1bf1..cc8aae3e2 100755 --- a/scripts/ci/tests/test-zitadel-pat.sh +++ b/scripts/ci/tests/test-zitadel-pat.sh @@ -15,13 +15,29 @@ CLOUD=gcp check "gcp secret name" "zitadel-iam-admin-pat" "$(zitadel_pat_secret_name)" CLOUD=aws -# 1. Store has it -> used, and the cluster is never consulted. The store holds -# a JSON object ({"pat": ...}), matching what store_write actually accepts -# on its AWS branch (bare strings fail to parse as JSON there). +# 1. GCP parity GP-20 (owner, 2026-09-29): the cluster's PAT wins over a stored +# one. A fresh directory every build makes a stored PAT belong to a directory +# that no longer exists, and every call made with it gets a 401. +persisted="" store_exists() { return 0; } +store_read() { printf '%s' '{"pat":"stale-token"}'; } +store_write() { persisted="$(cat)"; } +kubectl() { printf '%s' "dG9rZW4tZnJvbS1jbHVzdGVy"; } # base64 of token-from-cluster +check "cluster wins over a stale store" "token-from-cluster" "$(resolve_zitadel_pat 2>/dev/null)" +resolve_zitadel_pat >/dev/null 2>&1 +check "the stale store is overwritten" "token-from-cluster" "$(printf '%s' "$persisted" | jq -r .pat)" + +# 1a. Same token in both: no rewrite (one Secret Manager version per change). +store_write_called=0 +store_read() { printf '%s' '{"pat":"token-from-cluster"}'; } +store_write() { store_write_called=1; cat >/dev/null; } +resolve_zitadel_pat >/dev/null 2>&1 +check "an equal store is not rewritten" "0" "$store_write_called" + +# 1b. No cluster Secret (a directory restored from a seed): the store is used. store_read() { printf '%s' '{"pat":"token-from-store"}'; } -kubectl() { echo "KUBECTL MUST NOT BE CALLED" >&2; return 1; } -check "store wins" "token-from-store" "$(resolve_zitadel_pat)" +kubectl() { return 1; } +check "the store serves a restored directory" "token-from-store" "$(resolve_zitadel_pat 2>/dev/null)" # 2. Store empty, cluster has it -> used AND persisted. persisted="" diff --git a/scripts/lib/zitadel-pat.sh b/scripts/lib/zitadel-pat.sh index cee41cdb2..5b3e999d5 100644 --- a/scripts/lib/zitadel-pat.sh +++ b/scripts/lib/zitadel-pat.sh @@ -16,10 +16,11 @@ # # THE FIX # -# Persist the token to the cloud secret store the first time it exists, and read -# it from there afterwards. This works because the restored database keeps the -# machine user AND its token hash, so a token captured at first bootstrap stays -# valid against the restored instance. +# The chart's Secret when it exists, overwriting the stored copy; the store only +# when the cluster has none (a restore). GCP parity GP-20, owner 2026-09-29. The +# restore case works because the restored database keeps the machine user AND +# its token hash, so a token captured at first bootstrap stays valid against +# the restored instance. # # No ExternalSecret: nothing in the cluster consumes this credential, only # operator scripts, and an ExternalSecret would contend with the chart for @@ -58,60 +59,44 @@ zitadel_pat_secret_name() { # run must still capture the PAT into the store on its first sight of one, # same as before this existed. resolve_zitadel_pat() { - local name stored token b64 + local name stored="" current="" token b64 local dry_run="${ZITADEL_PAT_DRY_RUN:-false}" name="$(zitadel_pat_secret_name)" - - # 1. The store, which is the only source that survives a restore. Stored as - # a JSON object {"pat": ...} -- store_write's AWS branch parses stdin as - # JSON (it round-trips the payload through `jq '. | tostring'`), so a - # bare token string fails there with a parse error. Wrapping it is what - # lets both clouds share one store_write/store_read. - if store_exists "$name" && stored="$(store_read "$name")" \ - && token="$(printf '%s' "$stored" | jq -r '.pat // empty' 2>/dev/null)" \ - && [ -n "$token" ]; then - printf '%s' "$token" - return 0 - fi - - # 2. The cluster, where the chart writes it on a fresh bootstrap only. This - # read is also the one chance to capture it. + # 1. The cluster. The chart writes this Secret on FirstInstance, so when it + # exists it belongs to the directory that is running. A stored copy may + # belong to one a fresh build replaced (GCP parity GP-20), so it never + # wins over this. b64="$(kubectl get secret "$ZITADEL_PAT_K8S_SECRET" \ -n "$ZITADEL_PAT_K8S_NAMESPACE" -o jsonpath='{.data.pat}' 2>/dev/null || true)" - if [ -n "$b64" ]; then - token="$(printf '%s' "$b64" | base64 -d 2>/dev/null || true)" - if [ -n "$token" ]; then + token="" + [ -n "$b64" ] && token="$(printf '%s' "$b64" | base64 -d 2>/dev/null || true)" + if [ -n "$token" ]; then + if store_exists "$name" && stored="$(store_read "$name")"; then + current="$(printf '%s' "$stored" | jq -r '.pat // empty' 2>/dev/null || true)" + fi + if [ "$current" != "$token" ]; then if [ "$dry_run" = "true" ]; then - # The header promise ("Dry-run unless --apply") is a promise - # about every write this script makes, and persisting the PAT - # into the cloud secret store is one -- confirmed live: - # zitadel/iam-admin-pat got created in Secrets Manager on a - # plain sync with no --apply. Nothing is lost by not writing - # it here: the token stays exactly where it already was, in - # the Kubernetes Secret this branch just read it from, and - # will be captured on the next --apply run the normal way. - echo "[dry-run] would capture the admin PAT into ${name} so it survives a restore" >&2 - printf '%s' "$token" - return 0 + echo "[dry-run] would write the cluster's admin PAT to ${name}" >&2 + else + echo "[persist] writing the cluster's admin PAT to ${name}" >&2 + # Own our provenance: a caller's STORE_WRITE_* globals are for its own secrets. + local STORE_WRITE_DESCRIPTION="ZITADEL iam-admin PAT for ${CLUSTER:-this cluster}. Captured by zitadel-pat.sh." + local STORE_WRITE_LABEL="zitadel-pat" + # jq -Rs: the token reaches jq on stdin, never in argv. + store_write "$name" <<< "$(printf '%s' "$token" | jq -Rs '{pat: .}')" fi - echo "[persist] capturing the admin PAT into ${name} so it survives a restore" >&2 - # Own our provenance rather than trusting whatever the caller set. - # store_write reads these as plain globals, and a caller that sets - # them for ITS OWN secrets (e.g. zitadel-oidc-clients.sh, for the - # OIDC client secrets it writes) leaves them set in the shell that - # eventually calls us -- a `local` here shadows that for this call - # only, so the PAT gets ITS OWN description/label and the caller's - # values are unchanged once we return. - local STORE_WRITE_DESCRIPTION="ZITADEL iam-admin PAT for ${CLUSTER:-this cluster}. Captured by zitadel-pat.sh." - local STORE_WRITE_LABEL="zitadel-pat" - # jq -Rs reads stdin as one raw string rather than parsing it as - # JSON -- unlike `jq --arg pat "$token"`, the token never appears - # in an external process's argv (readable via /proc//cmdline - # for the life of that call), only on stdin. - store_write "$name" <<< "$(printf '%s' "$token" | jq -Rs '{pat: .}')" - printf '%s' "$token" - return 0 fi + printf '%s' "$token" + return 0 + fi + # 2. The store: the only source after a restore from a seed, where + # FirstInstance never ran and the chart wrote no Secret. Stored as + # {"pat": ...}, the one shape store_write's AWS branch accepts. + if store_exists "$name" && stored="$(store_read "$name")" \ + && token="$(printf '%s' "$stored" | jq -r '.pat // empty' 2>/dev/null)" \ + && [ -n "$token" ]; then + printf '%s' "$token" + return 0 fi echo "ERROR: no ZITADEL admin PAT available." >&2 diff --git a/scripts/provision/zitadel-oidc-clients.sh b/scripts/provision/zitadel-oidc-clients.sh index b21c859f4..8c2a4906c 100755 --- a/scripts/provision/zitadel-oidc-clients.sh +++ b/scripts/provision/zitadel-oidc-clients.sh @@ -222,9 +222,9 @@ ZITADEL_PAT_DRY_RUN="true" # both can be supplied at once. # # When --idp-cloud differs from --cloud, point kubectl at the cluster that HOSTS -# the identity provider. resolve_zitadel_pat falls back to reading the PAT from -# the current context's Kubernetes Secret when the store has none, and on a -# fresh primary that fallback is the only place the token exists yet. +# the identity provider. resolve_zitadel_pat reads the PAT from the current +# context's Kubernetes Secret first (GP-20), and on a fresh primary that Secret +# is the only place the token exists yet. _target_cloud="$CLOUD" CLOUD="$IDP_CLOUD" PAT="$(resolve_zitadel_pat)" || exit 1 From 21c2788e9b42c5270c2e235c0e3b63b41430a647 Mon Sep 17 00:00:00 2001 From: Smana Date: Tue, 29 Sep 2026 12:47:06 +0200 Subject: [PATCH 7/9] fix(zitadel): a consumer never overwrites the IdP cloud's PAT, and the project id is written only on change resolve_zitadel_pat takes an explicit hosting|consuming role. A consuming sync's kube context is its own cluster, so a chart Secret left there from when it hosted is never read and never overwrites the IdP cloud's only copy. A failed overwrite now warns and still returns the token. IDP_URL is checked before the resolve, which can write. zitadel-project-id is compared before it is written, and a failed publish accumulates like a failed mirror: the OpenBao reconcile and the summary still run, then the sync exits 1. data.tf documents the version-less-secret recovery. --- opentofu/gcp/gke/configure/data.tf | 7 ++ scripts/ci/tests/test-cloud-secret-store.sh | 21 +++++ .../ci/tests/test-gcp-zitadel-project-id.sh | 35 +++++++- .../test-zitadel-oidc-clients-convergence.sh | 1 + .../test-zitadel-oidc-clients-openbao.sh | 13 ++- scripts/ci/tests/test-zitadel-pat.sh | 83 +++++++++++++++---- scripts/lib/zitadel-pat.sh | 26 ++++-- scripts/provision/zitadel-idp.sh | 3 +- scripts/provision/zitadel-oidc-clients.sh | 58 ++++++++----- 9 files changed, 203 insertions(+), 44 deletions(-) diff --git a/opentofu/gcp/gke/configure/data.tf b/opentofu/gcp/gke/configure/data.tf index 2301d420f..1e2eb159f 100644 --- a/opentofu/gcp/gke/configure/data.tf +++ b/opentofu/gcp/gke/configure/data.tf @@ -45,6 +45,13 @@ data "google_secret_manager_secret_version" "openbao_root_token" { # it here in stage 3. Read at plan time so this stack's later apply agrees with # the value the sync patched into the ConfigMap. Listed before it is read: absent # is "first build", and var.zitadel_project_id stands in until stage 3. +# +# The gate checks that the SECRET exists, not that it has a VERSION -- the +# provider has no version-listing data source. store_write creates the secret +# then adds a version, so a failed add leaves a version-less secret and this plan +# then fails on "latest" not found. Recovery: re-run zitadel-oidc-clients.sh +# sync --apply, or delete the empty secret +# (gcloud secrets delete zitadel-project-id --project ), then re-apply. data "google_secret_manager_secrets" "zitadel_project" { count = var.deploy_identity_provider ? 1 : 0 project = var.project_id diff --git a/scripts/ci/tests/test-cloud-secret-store.sh b/scripts/ci/tests/test-cloud-secret-store.sh index a16dd5f66..8c8455635 100755 --- a/scripts/ci/tests/test-cloud-secret-store.sh +++ b/scripts/ci/tests/test-cloud-secret-store.sh @@ -258,4 +258,25 @@ no_tmp() { check "aws mktemp failure: non-zero, no cloud write" "1 0" "$(no_tmp aws)" check "gcp mktemp failure: non-zero, no cloud write" "1 0" "$(no_tmp gcp)" +# A failed `secrets create` stops before `versions add`, which would only fail +# again on a secret that does not exist. +CREATEFAIL="$(mktemp -d)" +cat > "$CREATEFAIL/gcloud" <<'EOF' +#!/usr/bin/env bash +printf 'CALL:' >> "$STUB_LOG"; printf ' %q' "$@" >> "$STUB_LOG"; printf '\n' >> "$STUB_LOG" +for a in "$@"; do case "$a" in describe|create) exit 1 ;; esac; done +exit 0 +EOF +chmod +x "$CREATEFAIL/gcloud" +: > "$STUB_LOG" +rc=0 +PATH="$CREATEFAIL:$PATH" bash -c ' + . "'"$HERE"'/../../lib/cloud-secret-store.sh" + CLOUD=gcp REGION="" GCP_PROJECT=proj + store_write notfound-secret <<< "{\"pat\":\"fake-full-token\"}" || exit $? +' || rc=$? +check "gcp create failure: non-zero" "1" "$rc" +check_log_absent "gcp create failure: no versions add" 'versions add' "$STUB_LOG" +rm -rf "$CREATEFAIL" + exit $fail diff --git a/scripts/ci/tests/test-gcp-zitadel-project-id.sh b/scripts/ci/tests/test-gcp-zitadel-project-id.sh index 32d1434f3..1fc5104af 100755 --- a/scripts/ci/tests/test-gcp-zitadel-project-id.sh +++ b/scripts/ci/tests/test-gcp-zitadel-project-id.sh @@ -12,6 +12,39 @@ grep -Eq '^[[:space:]]*zitadel_project_id[[:space:]]*=[[:space:]]*local\.zitadel grep -q 'data "google_secret_manager_secrets" "zitadel_project"' "$C/data.tf" || f "configure does not list zitadel-project-id before reading it" grep -q 'data "google_secret_manager_secret_version" "zitadel_project"' "$C/data.tf" || f "configure does not read zitadel-project-id" grep -Eq 'zitadel_project_id[[:space:]]*=.*var\.zitadel_project_id' "$C/locals.tf" || f "no fallback to the committed id" -grep -q 'store_write zitadel-project-id' "$ROOT/scripts/provision/zitadel-oidc-clients.sh" || f "the sync never publishes zitadel-project-id" +S="$ROOT/scripts/provision/zitadel-oidc-clients.sh" +grep -q 'store_write zitadel-project-id' "$S" || f "the sync never publishes zitadel-project-id" + +# The gate and the compare-before-write, on the function that ships (review M-1, +# M-5). A failed publish accumulating in cmd_sync (M-3) is tested in +# test-zitadel-oidc-clients-openbao.sh, which runs cmd_sync. +body="$(sed -n '/^publish_project_id() {/,/^}/p' "$S")" +[ -n "$body" ] || { f "could not extract publish_project_id()"; exit 1; } +eval "$body" +T="$(mktemp -d)"; trap 'rm -rf "$T"' EXIT +STORED="" WRITE_RC=0 +store_read() { [ -n "$STORED" ] || return 1; printf '%s' "$STORED"; } +store_write() { printf '%s %s\n' "$1" "$(cat)" >> "$T/writes"; return "$WRITE_RC"; } +# cloud idp apply project_id stored -> " " +run() { + : > "$T/writes" + local rc=0 + # shellcheck disable=SC2034 # read by the eval'd publish_project_id + CLOUD="$1" IDP_CLOUD="$2" APPLY="$3" STORED="$5" + publish_project_id "$4" >/dev/null 2>&1 || rc=$? + echo "$rc $(wc -l < "$T/writes")" +} +want() { [ "$2" = "$3" ] || f "$1: expected '$2' got '$3'"; } +want "hosting, changed: one write" "0 1" "$(run gcp gcp true new-id '{"project_id":"old-id"}')" +grep -qx 'zitadel-project-id {"project_id":"new-id"}' "$T/writes" || f "payload: $(cat "$T/writes")" +want "hosting, absent: one write" "0 1" "$(run gcp gcp true new-id '')" +want "hosting, unchanged: no write" "0 0" "$(run gcp gcp true same-id '{"project_id":"same-id"}')" +want "dry run: no write" "0 0" "$(run gcp gcp false new-id '')" +want "consuming (idp aws): no write" "0 0" "$(run gcp aws true new-id '')" +want "aws hosting: no write" "0 0" "$(run aws aws true new-id '')" +want "DRYRUN-PROJECT sentinel: no write" "0 0" "$(run gcp gcp true DRYRUN-PROJECT '')" +WRITE_RC=1 +want "failed write: non-zero" "1 1" "$(run gcp gcp true new-id '')" + [ "$fails" -eq 0 ] || exit 1 echo PASS diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh b/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh index abb7b40f8..5747b7330 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh @@ -118,6 +118,7 @@ store_write() { cat > "$STORE_DIR/$1"; } # cmd_sync writes through the real wrapper; the OpenBao mirror behind it has its # own suite (test-zitadel-oidc-clients-mirror.sh). load_function store_write_and_mirror "$SRC" +load_function publish_project_id "$SRC" # a no-op here: CLOUD=aws mirror_to_openbao() { cat >/dev/null; } # ── globals cmd_sync / converge_secret read ───────────────────────────────── diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh index 9b290f6e2..75c27fe41 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh @@ -1034,7 +1034,7 @@ contains "$out" "[FAILED ]" "writes that do not stick: says [FAILED echo echo "== cmd_sync wiring: the reconcile runs once, after the loop (Task 3) ==" -for f in cmd_sync oidc_config_payload store_write_and_mirror; do +for f in cmd_sync oidc_config_payload store_write_and_mirror publish_project_id; do body="$(sed -n "/^${f}() {/,/^}/p" "$CONSUMERS_SRC")" [ -n "$body" ] || { echo " FAIL could not extract ${f}() from $CONSUMERS_SRC" >&2; fail=1; } eval "$body" @@ -1154,6 +1154,17 @@ check "reconcile fails: cmd_sync exits 1" "1" "$rc" contains "$out" "created: " "reconcile fails: the summary line still prints" RECONCILE_RC=0 +echo +echo "-- a failed project-id publish: the reconcile and the summary still run, then exit 1 --" +: > "$RECONCILE_LOG" +publish_project_id() { return 1; } +run_cmd_sync +check "publish fails: cmd_sync exits 1" "1" "$rc" +check "publish fails: reconcile still called once" "1" "$(wc -l < "$RECONCILE_LOG")" +contains "$out" "created: " "publish fails: the summary line still prints" +contains "$out" "zitadel-project-id not published" "publish fails: the summary names it" +eval "$(sed -n '/^publish_project_id() {/,/^}/p' "$CONSUMERS_SRC")" + echo echo "== flags: --openbao-url requires --openbao-root-token-secret and --openbao-ca-file ==" REAL_CA="$WORK/real-ca.pem" diff --git a/scripts/ci/tests/test-zitadel-pat.sh b/scripts/ci/tests/test-zitadel-pat.sh index cc8aae3e2..3bdcd4029 100755 --- a/scripts/ci/tests/test-zitadel-pat.sh +++ b/scripts/ci/tests/test-zitadel-pat.sh @@ -23,21 +23,21 @@ store_exists() { return 0; } store_read() { printf '%s' '{"pat":"stale-token"}'; } store_write() { persisted="$(cat)"; } kubectl() { printf '%s' "dG9rZW4tZnJvbS1jbHVzdGVy"; } # base64 of token-from-cluster -check "cluster wins over a stale store" "token-from-cluster" "$(resolve_zitadel_pat 2>/dev/null)" -resolve_zitadel_pat >/dev/null 2>&1 +check "cluster wins over a stale store" "token-from-cluster" "$(resolve_zitadel_pat hosting 2>/dev/null)" +resolve_zitadel_pat hosting >/dev/null 2>&1 check "the stale store is overwritten" "token-from-cluster" "$(printf '%s' "$persisted" | jq -r .pat)" # 1a. Same token in both: no rewrite (one Secret Manager version per change). store_write_called=0 store_read() { printf '%s' '{"pat":"token-from-cluster"}'; } store_write() { store_write_called=1; cat >/dev/null; } -resolve_zitadel_pat >/dev/null 2>&1 +resolve_zitadel_pat hosting >/dev/null 2>&1 check "an equal store is not rewritten" "0" "$store_write_called" # 1b. No cluster Secret (a directory restored from a seed): the store is used. store_read() { printf '%s' '{"pat":"token-from-store"}'; } kubectl() { return 1; } -check "the store serves a restored directory" "token-from-store" "$(resolve_zitadel_pat 2>/dev/null)" +check "the store serves a restored directory" "token-from-store" "$(resolve_zitadel_pat hosting 2>/dev/null)" # 2. Store empty, cluster has it -> used AND persisted. persisted="" @@ -48,8 +48,8 @@ store_read() { return 1; } # vanish, which is also why the library uses `store_write ... <<< "$json"`. store_write() { persisted="$(cat)"; } kubectl() { printf '%s' "dG9rZW4tZnJvbS1jbHVzdGVy"; } # base64 of token-from-cluster -check "cluster seeds" "token-from-cluster" "$(resolve_zitadel_pat 2>/dev/null)" -resolve_zitadel_pat >/dev/null 2>&1 +check "cluster seeds" "token-from-cluster" "$(resolve_zitadel_pat hosting 2>/dev/null)" +resolve_zitadel_pat hosting >/dev/null 2>&1 check "persisted" "token-from-cluster" "$(printf '%s' "$persisted" | jq -r .pat)" # 2a. DEFECT 2: store empty, cluster has it, caller is in a DRY RUN @@ -72,10 +72,10 @@ store_read() { return 1; } store_write() { store_write_called=1; cat >/dev/null; } kubectl() { printf '%s' "dG9rZW4tZnJvbS1jbHVzdGVy"; } # base64 of token-from-cluster ZITADEL_PAT_DRY_RUN=true -check "dry-run: token still returned" "token-from-cluster" "$(resolve_zitadel_pat 2>/dev/null)" -resolve_zitadel_pat >/dev/null 2>&1 +check "dry-run: token still returned" "token-from-cluster" "$(resolve_zitadel_pat hosting 2>/dev/null)" +resolve_zitadel_pat hosting >/dev/null 2>&1 check "dry-run: store_write NOT called" "0" "$store_write_called" -err="$(resolve_zitadel_pat 2>&1 >/dev/null)" +err="$(resolve_zitadel_pat hosting 2>&1 >/dev/null)" case "$err" in *'[dry-run]'*) printf ' ok dry-run: prints [dry-run], not [persist]\n' ;; *) printf ' FAIL dry-run: did not print a [dry-run] line\n'; fail=1 ;; esac case "$err" in *'[persist]'*) printf ' FAIL dry-run: also printed [persist]\n'; fail=1 ;; @@ -91,8 +91,8 @@ store_exists() { return 1; } store_read() { return 1; } store_write() { store_write_called=1; cat >/dev/null; } kubectl() { printf '%s' "dG9rZW4tZnJvbS1jbHVzdGVy"; } -check "unset ZITADEL_PAT_DRY_RUN: token returned" "token-from-cluster" "$(resolve_zitadel_pat 2>/dev/null)" -resolve_zitadel_pat >/dev/null 2>&1 +check "unset ZITADEL_PAT_DRY_RUN: token returned" "token-from-cluster" "$(resolve_zitadel_pat hosting 2>/dev/null)" +resolve_zitadel_pat hosting >/dev/null 2>&1 check "unset ZITADEL_PAT_DRY_RUN: still persists (default false)" "1" "$store_write_called" # 2b. A caller sets STORE_WRITE_DESCRIPTION/LABEL for its OWN secrets (this is @@ -108,7 +108,7 @@ store_exists() { return 1; } store_read() { return 1; } store_write() { seen_desc="$STORE_WRITE_DESCRIPTION"; seen_label="$STORE_WRITE_LABEL"; cat >/dev/null; } kubectl() { printf '%s' "dG9rZW4tZnJvbS1jbHVzdGVy"; } # base64 of token-from-cluster -resolve_zitadel_pat >/dev/null 2>&1 +resolve_zitadel_pat hosting >/dev/null 2>&1 check "PAT write ignores caller's Description" \ "ZITADEL iam-admin PAT for aws-0. Captured by zitadel-pat.sh." "$seen_desc" check "PAT write ignores caller's Label" "zitadel-pat" "$seen_label" @@ -126,26 +126,75 @@ store_exists() { return 1; } store_read() { return 1; } store_write() { seeded="$(cat)"; } kubectl() { printf '%s' "$awkward" | base64 -w0; } -check "awkward token seeds" "$awkward" "$(resolve_zitadel_pat 2>/dev/null)" +check "awkward token seeds" "$awkward" "$(resolve_zitadel_pat hosting 2>/dev/null)" # A second, unwrapped call so store_write's assignment to $seeded (visible only # because the library uses a herestring, not a pipe -- see above) isn't lost # inside the command substitution's own subshell the check above just ran in. -resolve_zitadel_pat >/dev/null 2>&1 +resolve_zitadel_pat hosting >/dev/null 2>&1 store_exists() { return 0; } store_read() { printf '%s' "$seeded"; } kubectl() { echo "KUBECTL MUST NOT BE CALLED" >&2; return 1; } -check "awkward token reads back" "$awkward" "$(resolve_zitadel_pat 2>/dev/null)" +check "awkward token reads back" "$awkward" "$(resolve_zitadel_pat hosting 2>/dev/null)" # 4. Neither -> fail, with a diagnosis, and no token on stdout. store_exists() { return 1; } store_read() { return 1; } kubectl() { return 1; } -out="$(resolve_zitadel_pat 2>/dev/null)"; rc=$? +out="$(resolve_zitadel_pat hosting 2>/dev/null)"; rc=$? check "fails" "1" "$rc" check "silent stdout" "" "$out" -err="$(resolve_zitadel_pat 2>&1 >/dev/null)" +err="$(resolve_zitadel_pat hosting 2>&1 >/dev/null)" case "$err" in *FIRSTINSTANCE*) printf ' ok explains FirstInstance\n' ;; *) printf ' FAIL error does not explain the cause\n'; fail=1 ;; esac +# 5. Review I-1: a CONSUMING cluster (--idp-cloud differs) has kubectl on its +# own cluster. A chart Secret left there from when it hosted belongs to a +# dead directory; trusting it would overwrite the IdP cloud's only PAT and +# 401 every call there. The store answers, and nothing is written. +store_write_called=0 +KUBECTL_MARK="$(mktemp)"; rm -f "$KUBECTL_MARK" # kubectl runs inside $(...): a variable would not survive +store_exists() { return 0; } +store_read() { printf '%s' '{"pat":"token-from-idp-store"}'; } +store_write() { store_write_called=1; cat >/dev/null; } +kubectl() { : > "$KUBECTL_MARK"; printf '%s' "dG9rZW4tZnJvbS1jbHVzdGVy"; } # leftover Secret +check "consuming: the IdP store's PAT, not the local Secret" "token-from-idp-store" "$(resolve_zitadel_pat consuming 2>/dev/null)" +resolve_zitadel_pat consuming >/dev/null 2>&1 +check "consuming: the IdP store is never written" "0" "$store_write_called" +check "consuming: the local Secret is never read" "no" "$([ -e "$KUBECTL_MARK" ] && echo yes || echo no)" +rm -f "$KUBECTL_MARK" + +# 5a. Consuming with an empty store: fail, still without trusting the local Secret. +store_exists() { return 1; } +store_read() { return 1; } +out="$(resolve_zitadel_pat consuming 2>/dev/null)"; rc=$? +check "consuming, empty store: fails" "1" "$rc" +check "consuming, empty store: no local token leaks out" "" "$out" + +# 5b. No role is a caller bug, not a guess. +resolve_zitadel_pat >/dev/null 2>&1; rc=$? +check "no role: refused" "2" "$rc" + +# 6. Review M-2: a failed overwrite warns and still returns the cluster token. +store_exists() { return 0; } +store_read() { printf '%s' '{"pat":"stale-token"}'; } +store_write() { cat >/dev/null; return 1; } +kubectl() { printf '%s' "dG9rZW4tZnJvbS1jbHVzdGVy"; } +out="$(resolve_zitadel_pat hosting 2>/dev/null)"; rc=$? +check "failed overwrite: token still returned" "token-from-cluster" "$out" +check "failed overwrite: rc 0" "0" "$rc" +err="$(resolve_zitadel_pat hosting 2>&1 >/dev/null)" +case "$err" in *"WARN: zitadel/iam-admin-pat still holds the old PAT; the next --apply retries"*) + printf ' ok failed overwrite: warns on stderr\n' ;; + *) printf ' FAIL failed overwrite: no WARN line\n'; fail=1 ;; esac + +# 7. Review M-5: a dry run over a stale store returns the cluster token and writes nothing. +store_write_called=0 +store_write() { store_write_called=1; cat >/dev/null; } +ZITADEL_PAT_DRY_RUN=true +check "dry-run, stale store: cluster token returned" "token-from-cluster" "$(resolve_zitadel_pat hosting 2>/dev/null)" +resolve_zitadel_pat hosting >/dev/null 2>&1 +check "dry-run, stale store: nothing written" "0" "$store_write_called" +unset ZITADEL_PAT_DRY_RUN + exit $fail diff --git a/scripts/lib/zitadel-pat.sh b/scripts/lib/zitadel-pat.sh index 5b3e999d5..839a4e330 100644 --- a/scripts/lib/zitadel-pat.sh +++ b/scripts/lib/zitadel-pat.sh @@ -58,17 +58,27 @@ zitadel_pat_secret_name() { # Defaulting to "false" also matters for the persist path below: a --apply # run must still capture the PAT into the store on its first sight of one, # same as before this existed. +# +# $1 is required: `hosting` when the current kube context runs this directory's +# ZITADEL, `consuming` when the caller only registers clients in another cloud's. resolve_zitadel_pat() { - local name stored="" current="" token b64 + local role="${1:-}" name stored="" current="" token="" b64="" local dry_run="${ZITADEL_PAT_DRY_RUN:-false}" + case "$role" in + hosting|consuming) ;; + *) echo "resolve_zitadel_pat: pass hosting or consuming, got '${role}'" >&2; return 2 ;; + esac name="$(zitadel_pat_secret_name)" # 1. The cluster. The chart writes this Secret on FirstInstance, so when it # exists it belongs to the directory that is running. A stored copy may # belong to one a fresh build replaced (GCP parity GP-20), so it never # wins over this. - b64="$(kubectl get secret "$ZITADEL_PAT_K8S_SECRET" \ - -n "$ZITADEL_PAT_K8S_NAMESPACE" -o jsonpath='{.data.pat}' 2>/dev/null || true)" - token="" + # A consumer's kube context is its own cluster: a Secret left there from when + # it hosted is a dead directory's, and would overwrite the IdP cloud's only copy. + if [ "$role" = "hosting" ]; then + b64="$(kubectl get secret "$ZITADEL_PAT_K8S_SECRET" \ + -n "$ZITADEL_PAT_K8S_NAMESPACE" -o jsonpath='{.data.pat}' 2>/dev/null || true)" + fi [ -n "$b64" ] && token="$(printf '%s' "$b64" | base64 -d 2>/dev/null || true)" if [ -n "$token" ]; then if store_exists "$name" && stored="$(store_read "$name")"; then @@ -83,7 +93,8 @@ resolve_zitadel_pat() { local STORE_WRITE_DESCRIPTION="ZITADEL iam-admin PAT for ${CLUSTER:-this cluster}. Captured by zitadel-pat.sh." local STORE_WRITE_LABEL="zitadel-pat" # jq -Rs: the token reaches jq on stdin, never in argv. - store_write "$name" <<< "$(printf '%s' "$token" | jq -Rs '{pat: .}')" + store_write "$name" <<< "$(printf '%s' "$token" | jq -Rs '{pat: .}')" \ + || echo "WARN: ${name} still holds the old PAT; the next --apply retries" >&2 fi fi printf '%s' "$token" @@ -101,6 +112,11 @@ resolve_zitadel_pat() { echo "ERROR: no ZITADEL admin PAT available." >&2 echo " looked in: ${name} (cloud secret store)" >&2 + if [ "$role" = "consuming" ]; then + echo "A consuming cluster reads only the IdP cloud's store. Run the hosting" >&2 + echo "cluster's sync --apply first; it stores the PAT there." >&2 + return 1 + fi echo " ${ZITADEL_PAT_K8S_NAMESPACE}/${ZITADEL_PAT_K8S_SECRET} (cluster)" >&2 echo >&2 echo "The chart writes that Secret during FIRSTINSTANCE, which runs only" >&2 diff --git a/scripts/provision/zitadel-idp.sh b/scripts/provision/zitadel-idp.sh index 145b71e8f..c1151fc76 100755 --- a/scripts/provision/zitadel-idp.sh +++ b/scripts/provision/zitadel-idp.sh @@ -116,7 +116,8 @@ case "$CLOUD" in aws|gcp) ;; *) echo "--cloud must be aws or gcp" >&2; exit 2 ;; ZITADEL_PAT_DRY_RUN="true" [ "$APPLY" = "true" ] && ZITADEL_PAT_DRY_RUN="false" -PAT="$(resolve_zitadel_pat)" || exit 1 +# This script configures the directory it runs against, so that cluster hosts it. +PAT="$(resolve_zitadel_pat hosting)" || exit 1 : "${IDP_URL:?set IDP_URL to the ZITADEL base URL, e.g. https://auth.gcp.cloud.ogenki.io}" diff --git a/scripts/provision/zitadel-oidc-clients.sh b/scripts/provision/zitadel-oidc-clients.sh index 8c2a4906c..1012ed377 100755 --- a/scripts/provision/zitadel-oidc-clients.sh +++ b/scripts/provision/zitadel-oidc-clients.sh @@ -221,20 +221,22 @@ ZITADEL_PAT_DRY_RUN="true" # $GCP_PROJECT need no swap: each is only read by its own cloud's branch, so # both can be supplied at once. # -# When --idp-cloud differs from --cloud, point kubectl at the cluster that HOSTS -# the identity provider. resolve_zitadel_pat reads the PAT from the current -# context's Kubernetes Secret first (GP-20), and on a fresh primary that Secret -# is the only place the token exists yet. -_target_cloud="$CLOUD" -CLOUD="$IDP_CLOUD" -PAT="$(resolve_zitadel_pat)" || exit 1 -CLOUD="$_target_cloud" - +# A hosting sync reads the PAT from its own cluster's chart Secret first (GP-20). +# A consuming one (--idp-cloud differs) reads only the IdP cloud's store: its +# kube context is its own cluster, not the one that runs ZITADEL. +# # The IdP base URL. Derived the same way the platform derives it, so a mismatch -# here is a mismatch everywhere. +# here is a mismatch everywhere. Checked before the PAT resolve, which can write. : "${IDP_URL:?set IDP_URL to the ZITADEL base URL, e.g. https://auth.gcp.cloud.ogenki.io}" : "${PRIVATE_DOMAIN:?set PRIVATE_DOMAIN, e.g. priv.gcp.ogenki.io}" +_pat_role="hosting" +[ "$IDP_CLOUD" = "$CLOUD" ] || _pat_role="consuming" +_target_cloud="$CLOUD" +CLOUD="$IDP_CLOUD" +PAT="$(resolve_zitadel_pat "$_pat_role")" || exit 1 +CLOUD="$_target_cloud" + # Optional escape hatch for split-DNS workstations. The IdP hostname is public, # but a machine on the tailnet may resolve *.ogenki.io through a resolver that # has not picked up a freshly created record -- 8.8.8.8 answers while the system @@ -948,6 +950,26 @@ store_write_and_mirror() { printf '%s' "$payload" | mirror_to_openbao "$key" || return 2 } +# GCP hosting: publish this directory's project id for gke/configure, which +# reads it at plan time (GCP parity GP-4). A fresh directory gets a new id +# every build, and the committed one would otherwise come back on the next +# configure apply. Written only on a change: one Secret Manager version per +# directory, not per sync. Called under `||`, so every step is checked. +publish_project_id() { + local project_id="$1" current="" + if [ "$CLOUD" != "gcp" ] || [ "$IDP_CLOUD" != "$CLOUD" ] || [ "$APPLY" != "true" ] \ + || [ "$project_id" = "DRYRUN-PROJECT" ]; then + return 0 + fi + current="$(store_read zitadel-project-id 2>/dev/null | jq -r '.project_id // empty' 2>/dev/null || true)" + if [ "$current" = "$project_id" ]; then + echo "project: zitadel-project-id unchanged" + return 0 + fi + printf '%s' "$project_id" | jq -Rc '{project_id: .}' | store_write zitadel-project-id || return 1 + echo "project: published to zitadel-project-id" +} + # Point OpenBao's auth/oidc at the client ZITADEL issued. Two resources carry # it: the config (id and secret) and the default role's bound_audiences. Moving # only the config leaves every login failing on audience (design fact 5). @@ -1366,15 +1388,9 @@ cmd_sync() { # is an app client id rather than the project id. reconcile_workforce_audience "$project_id" - # GCP hosting: publish this directory's project id for gke/configure, which - # reads it at plan time (GCP parity GP-4). A fresh directory gets a new id - # every build, and the committed one would otherwise come back on the next - # configure apply. - if [ "$CLOUD" = "gcp" ] && [ "$IDP_CLOUD" = "$CLOUD" ] && [ "$APPLY" = "true" ] \ - && [ "$project_id" != "DRYRUN-PROJECT" ]; then - printf '%s' "$project_id" | jq -Rc '{project_id: .}' | store_write zitadel-project-id - echo "project: published to zitadel-project-id" - fi + # Accumulated like failed_mirrors: the reconcile and the summary still run. + local publish_failed=0 + publish_project_id "$project_id" || publish_failed=1 # Same reasoning as the workforce provider above: OpenBao's client id is # only known once the "openbao" consumer's app has been found or created. @@ -1393,9 +1409,13 @@ cmd_sync() { if [ -n "$failed_mirrors" ]; then echo "[FAILED ] not mirrored to OpenBao: ${failed_mirrors} -- the managed store has them; re-run sync --apply" >&2 fi + if [ "$publish_failed" -ne 0 ]; then + echo "[FAILED ] zitadel-project-id not published -- gke/configure keeps the committed id; re-run sync --apply" >&2 + fi [ "$openbao_failed" -eq 0 ] || exit 1 [ -z "$failed_mirrors" ] || exit 1 + [ "$publish_failed" -eq 0 ] || exit 1 } case "$COMMAND" in From 95fdd6649794035a48ff1894c8406616cf8495b1 Mon Sep 17 00:00:00 2001 From: Smana Date: Tue, 29 Sep 2026 12:57:47 +0200 Subject: [PATCH 8/9] fix(gcp): a hosting stage 3 registers the IdP, then mirrors every client into OpenBao --- opentofu/gcp/gke/init/workflows.tm.hcl | 16 +++++++++++++++- scripts/ci/tests/test-gcp-gke-init-workflow.py | 7 +++++++ .../tests/test-zitadel-oidc-clients-openbao.sh | 17 ++++++++++++----- scripts/provision/zitadel-idp.sh | 4 ++++ scripts/provision/zitadel-oidc-clients.sh | 9 ++++----- 5 files changed, 42 insertions(+), 11 deletions(-) diff --git a/opentofu/gcp/gke/init/workflows.tm.hcl b/opentofu/gcp/gke/init/workflows.tm.hcl index 11b352b2d..11ef9b219 100644 --- a/opentofu/gcp/gke/init/workflows.tm.hcl +++ b/opentofu/gcp/gke/init/workflows.tm.hcl @@ -302,11 +302,25 @@ script "deploy" { # cluster fails as a bare `invalid_grant` with everything looking healthy. # Empty (no such stack / no such key) simply skips that reconciliation. WORKFORCE_POOL="$(awk -F'=' '/^[[:space:]]*workforce_pool_id/{gsub(/[[:space:]"]/,"",$2); print $2}' "$${ROOT}/opentofu/gcp/workforce-identity/variables.tfvars" 2>/dev/null || true)" + # A fresh directory (GCP parity GP-3) has neither the Google IdP nor the + # groups Action; both must exist before the clients, or no token carries + # a groups claim. + echo "== registering the Google IdP and the groups Action" + IDP_URL="https://auth.$${PUBLIC_DOMAIN}" \ + bash "$${ROOT}/scripts/provision/zitadel-idp.sh" sync \ + --cluster "$${NAME}" --cloud gcp --project "$${PROJECT}" --apply || \ + echo "[warn] IdP registration failed; re-run it by hand" + echo "== registering the OIDC clients" IDP_URL="https://auth.$${PUBLIC_DOMAIN}" PRIVATE_DOMAIN="$${PRIVATE_DOMAIN}" \ bash "$${ROOT}/scripts/provision/zitadel-oidc-clients.sh" sync \ --cluster "$${NAME}" --cloud gcp --project "$${PROJECT}" \ - --workforce-pool "$${WORKFORCE_POOL}" --apply || \ + --workforce-pool "$${WORKFORCE_POOL}" \ + --openbao-url "https://bao.$${PRIVATE_DOMAIN}:8200" \ + --openbao-root-token-secret openbao-priv-gcp-root-token \ + --openbao-ca-file "$${ROOT}/opentofu/gcp/gke/configure/.tls/ca.pem" \ + --mirror-openbao \ + --apply || \ echo "[warn] OIDC registration failed; re-run it by hand" echo "== granting access to the secrets it just created" diff --git a/scripts/ci/tests/test-gcp-gke-init-workflow.py b/scripts/ci/tests/test-gcp-gke-init-workflow.py index 37a712b14..6305a1589 100644 --- a/scripts/ci/tests/test-gcp-gke-init-workflow.py +++ b/scripts/ci/tests/test-gcp-gke-init-workflow.py @@ -60,6 +60,13 @@ def before(body, first, second, what): if not any("state rm -lock-timeout=" in l for l in state_lines): fails.append("the destroy's custom-role state rm waits for the state lock") +s3 = job_body(TEXT, "stage3-secrets-and-oidc") +hosting = s3[s3.find("== registering the OIDC clients"):] if "== registering the OIDC clients" in s3 else "" +before(s3, 'zitadel-idp.sh" sync', "== registering the OIDC clients", "a hosting stage 3 configures the IdP and Action before its clients") +for flag in ("--openbao-url", "--openbao-root-token-secret openbao-priv-gcp-root-token", "--openbao-ca-file", "--mirror-openbao"): + if flag not in hosting: + fails.append(f"the hosting clients sync passes {flag}") + for f in fails: print("FAIL", f) if fails: diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh index 75c27fe41..bcfd015e8 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh @@ -17,8 +17,9 @@ # registers in ZITADEL. # * the reconcile is only as good as its invocation: the unit tests below # pass whether or not any deploy ever hands the script --openbao-url. The -# Terramate guards pin who passes it (aws-0's own sync, nobody else) and -# that stage5 runs the check without swallowing its exit code. +# Terramate guards pin who passes it (each cloud's hosting sync, never a +# consumer's) and that stage5 runs the check without swallowing its exit +# code. # # WHY PATH STUBS. curl, jq and sleep are executables on a stub PATH, not shell # functions: a function is bypassed by `command curl` or `env curl`, and a jq @@ -135,7 +136,7 @@ in_job() { # body, needle, label } echo -echo "== contract: only aws-0's own sync passes the --openbao-* flags (design §2) ==" +echo "== contract: only a hosting sync passes the --openbao-* flags (design §2, GP-5) ==" aws_syncs="$(logical_lines "$AWS_WORKFLOWS_SRC" | grep -F 'zitadel-oidc-clients.sh" sync' || true)" own_sync="$(grep -F -- '--cluster "${global.eks_cluster_name}"' <<< "$aws_syncs" || true)" consumer_sync="$(grep -F -- '--idp-cloud aws' <<< "$aws_syncs" || true)" @@ -157,8 +158,14 @@ contains "$openbao_args" '--openbao-ca-file ' "OPENBAO_ARGS carries the CA file" check "gcp/gke/init's workflows file exists" "yes" "$([ -f "$GCP_WORKFLOWS_SRC" ] && echo yes || echo no)" gcp_syncs="$(logical_lines "$GCP_WORKFLOWS_SRC" 2>/dev/null | grep -cF 'zitadel-oidc-clients.sh" sync')" check "gcp/gke/init runs at least one sync" "yes" "$([ "${gcp_syncs:-0}" -ge 1 ] && echo yes || echo no)" -gcp_openbao="$(logical_lines "$GCP_WORKFLOWS_SRC" | grep -E -- '--openbao-|OPENBAO_ARGS' || true)" -check "gcp/gke/init passes no --openbao-* flag" "" "$gcp_openbao" +# GCP parity GP-5: gcp-0's HOSTING sync reconciles its own OpenBao and mirrors +# into it; the CONSUMER sync (registering in another cloud's directory) must not. +gcp_hosting="$(logical_lines "$GCP_WORKFLOWS_SRC" | grep -F 'zitadel-oidc-clients.sh" sync' | grep -vF -- '--idp-cloud' || true)" +gcp_consumer="$(logical_lines "$GCP_WORKFLOWS_SRC" | grep -F 'zitadel-oidc-clients.sh" sync' | grep -F -- '--idp-cloud' || true)" +contains "$gcp_consumer" '--idp-cloud' "gcp/gke/init's consumer sync is found" +contains "$gcp_hosting" '--openbao-url' "gcp/gke/init's hosting sync passes --openbao-url" +contains "$gcp_hosting" '--mirror-openbao' "gcp/gke/init's hosting sync mirrors into OpenBao" +check "gcp/gke/init's consumer sync passes no --openbao-* flag" "" "$(grep -E -- '--openbao-|--mirror-openbao' <<<"$gcp_consumer" || true)" echo echo "== contract: stage5 runs the check after stage4, and its failure halts the deploy ==" diff --git a/scripts/provision/zitadel-idp.sh b/scripts/provision/zitadel-idp.sh index c1151fc76..a1d5b7ced 100755 --- a/scripts/provision/zitadel-idp.sh +++ b/scripts/provision/zitadel-idp.sh @@ -36,6 +36,10 @@ # # Dry-run unless --apply. The client secret is never printed. # +# It resolves the admin PAT as the HOSTING cloud, so run it with kubectl +# pointed at the cluster that hosts the IdP: a leftover security/iam-admin-pat +# in any other cluster would overwrite that cloud's stored PAT (GP-20). +# # THE ONE THING TO DO BY HAND, ONCE PER CLUSTER # # Google OAuth clients accept MANY authorized redirect URIs -- unlike a GitHub diff --git a/scripts/provision/zitadel-oidc-clients.sh b/scripts/provision/zitadel-oidc-clients.sh index 1012ed377..37fc38b9f 100755 --- a/scripts/provision/zitadel-oidc-clients.sh +++ b/scripts/provision/zitadel-oidc-clients.sh @@ -29,8 +29,7 @@ # it (--openbao-url and friends; #2045). A rebuild restores ZITADEL from # a seed that predates the app, so its id is new every time and # Terraform, which created the mount, ignores this field afterwards. -# No-op when --openbao-url is empty, which is every call except aws-0's -# own sync. +# No-op when --openbao-url is empty, which is every consumer call. # # Step 4 MERGES rather than overwrites where a secret holds more than OIDC: # grafana-envvars also carries the generated admin credentials, and clobbering @@ -111,9 +110,9 @@ ZITADEL_PROJECT_ROLES=(admin backend frontend data) # nobody can reproduce, which is how gcp-0 ended up with no groups claim at all. GRANT_ADMIN="" -# Empty means reconcile_openbao_oidc (#2045) is a no-op -- correct on GCP and -# for the gcp-0 consumer call, since OpenBao OIDC exists only on AWS (design -# fact 1). Only aws-0's own sync passes these. +# Empty means reconcile_openbao_oidc is a no-op: the consumer call on a +# secondary cluster. A cluster hosting its own directory (aws-0's stage 4, +# gcp-0's hosting stage 3) passes all three. OPENBAO_URL="" OPENBAO_ROOT_TOKEN_SECRET="" OPENBAO_CA_FILE="" From ac96645b8d64fe7d55cae5d3bc0b2b1b3039bcd3 Mon Sep 17 00:00:00 2001 From: Smana Date: Tue, 29 Sep 2026 13:20:42 +0200 Subject: [PATCH 9/9] fix(gcp): mirror an absent OpenBao path in full, print a complete recovery, and halt on OIDC drift - The mirror writes the whole store payload when OpenBao has nothing at the path. migrate skips an existing path, so an owned-fields-only write left grafana-envvars without its admin credentials for good. The ownership filter still protects an existing value. - Stage 3's flags live in IDP_SYNC_ARGS and CLIENT_SYNC_ARGS, expanded by both the real calls and the ZITADEL-not-ready recovery, which now prints the IdP sync and the clients sync with every OpenBao flag. - gcp/gke/init gains stage5-verify-openbao-oidc: on a hosting gcp-0 it runs openbao-oidc-check.sh as its last statement, so exit 1 and exit 2 halt the deploy. A consuming gcp-0 skips it and says why. - After a mirror, the ExternalSecrets reading the mirrored paths are force-synced (warn-only, --apply only). - store_write adds the version when secrets create reports ALREADY_EXISTS. - sso.md gives gcp-0's hosting and consuming cases, and states GP-20. --- opentofu/gcp/gke/init/workflows.tm.hcl | 86 ++++++++++++++----- scripts/ci/tests/test-cloud-secret-store.sh | 21 +++++ .../ci/tests/test-gcp-gke-init-workflow.py | 58 ++++++++++++- .../test-zitadel-oidc-clients-convergence.sh | 1 + .../tests/test-zitadel-oidc-clients-mirror.sh | 64 ++++++++++++-- .../test-zitadel-oidc-clients-openbao.sh | 14 ++- scripts/lib/cloud-secret-store.sh | 12 ++- scripts/provision/zitadel-oidc-clients.sh | 81 +++++++++++++---- website/content/docs/get-started/sso.md | 34 ++++++-- 9 files changed, 308 insertions(+), 63 deletions(-) diff --git a/opentofu/gcp/gke/init/workflows.tm.hcl b/opentofu/gcp/gke/init/workflows.tm.hcl index 11ef9b219..ba1a3e3e5 100644 --- a/opentofu/gcp/gke/init/workflows.tm.hcl +++ b/opentofu/gcp/gke/init/workflows.tm.hcl @@ -6,10 +6,15 @@ # Stage 1 (this stack): GKE Standard cluster, static spot node pool, Workload # Identity, Crossplane WIF bootstrap # Stage 2 (configure): Gateway API CRDs -> Cilium -> Flux Operator -> Flux Instance +# Stage 3: External Secrets grants, then the OIDC clients; when +# gcp-0 hosts the IdP, first the Google IdP and groups +# Action, and the clients mirrored into OpenBao +# Stage 5: a hosting gcp-0's OpenBao OIDC check, which halts the +# deploy on drift # -# There is no stage 3. The EKS equivalent recycles bootstrap nodes whose ENIs -# predate Cilium, which is specific to ENI prefix delegation and has no GCP -# counterpart -- ipam.mode=kubernetes takes pod CIDRs from the node object. +# Stage 5 keeps aws-0's job name, so one contract suite guards both. EKS's node +# recycle has no GCP counterpart: it exists for ENI prefix delegation, while +# ipam.mode=kubernetes takes pod CIDRs from the node object. # # The control-plane endpoint is PRIVATE, so stage 2 must run from a machine on the # tailnet. @@ -260,6 +265,25 @@ script "deploy" { exit 0 fi + # The workforce provider's audience is the ZITADEL PROJECT id, which does + # not exist until the sync below creates the project. Passing the pool + # lets the script reconcile it; without this, per-user RBAC on this + # cluster fails as a bare `invalid_grant` with everything looking healthy. + # Empty (no such stack / no such key) simply skips that reconciliation. + WORKFORCE_POOL="$(awk -F'=' '/^[[:space:]]*workforce_pool_id/{gsub(/[[:space:]"]/,"",$2); print $2}' "$${ROOT}/opentofu/gcp/workforce-identity/variables.tfvars" 2>/dev/null || true)" + # One flag list per sync, expanded by the real call AND the recovery + # printed when ZITADEL is late: a hand re-run missing the OpenBao flags + # leaves every ExternalSecret on the dead directory's clients. + IDP_SYNC_ARGS=(--cluster "$${NAME}" --cloud gcp --project "$${PROJECT}") + CLIENT_SYNC_ARGS=( + --cluster "$${NAME}" --cloud gcp --project "$${PROJECT}" + --workforce-pool "$${WORKFORCE_POOL}" + --openbao-url "https://bao.$${PRIVATE_DOMAIN}:8200" + --openbao-root-token-secret openbao-priv-gcp-root-token + --openbao-ca-file "$${ROOT}/opentofu/gcp/gke/configure/.tls/ca.pem" + --mirror-openbao + ) + # A BUDGET FOR A COLD BUILD, NOT A REBUILD. # # ZITADEL is last in a long chain on a fresh cluster -- @@ -290,37 +314,23 @@ script "deploy" { if [ "$(kubectl get deploy zitadel -n security -o jsonpath='{.status.readyReplicas}' 2>/dev/null || echo 0)" -lt 1 ] 2>/dev/null; then echo "[warn] ZITADEL not ready in $(( ZITADEL_WAIT_SECONDS / 60 ))m; skipping OIDC client registration." - echo " Re-run by hand once it is up:" - echo " IDP_URL=https://auth.$${PUBLIC_DOMAIN} PRIVATE_DOMAIN=$${PRIVATE_DOMAIN} \\" - echo " scripts/provision/zitadel-oidc-clients.sh sync --cluster $${NAME} --cloud gcp --project $${PROJECT} --apply" + echo " Re-run by hand once it is up, kubectl pointed at $${NAME}:" + echo " IDP_URL=https://auth.$${PUBLIC_DOMAIN} scripts/provision/zitadel-idp.sh sync$(printf ' %q' "$${IDP_SYNC_ARGS[@]}") --apply" + echo " IDP_URL=https://auth.$${PUBLIC_DOMAIN} PRIVATE_DOMAIN=$${PRIVATE_DOMAIN} scripts/provision/zitadel-oidc-clients.sh sync$(printf ' %q' "$${CLIENT_SYNC_ARGS[@]}") --apply" exit 0 fi - # The workforce provider's audience is the ZITADEL PROJECT id, which does - # not exist until the sync below creates the project. Passing the pool - # lets the script reconcile it; without this, per-user RBAC on this - # cluster fails as a bare `invalid_grant` with everything looking healthy. - # Empty (no such stack / no such key) simply skips that reconciliation. - WORKFORCE_POOL="$(awk -F'=' '/^[[:space:]]*workforce_pool_id/{gsub(/[[:space:]"]/,"",$2); print $2}' "$${ROOT}/opentofu/gcp/workforce-identity/variables.tfvars" 2>/dev/null || true)" # A fresh directory (GCP parity GP-3) has neither the Google IdP nor the # groups Action; both must exist before the clients, or no token carries # a groups claim. echo "== registering the Google IdP and the groups Action" IDP_URL="https://auth.$${PUBLIC_DOMAIN}" \ - bash "$${ROOT}/scripts/provision/zitadel-idp.sh" sync \ - --cluster "$${NAME}" --cloud gcp --project "$${PROJECT}" --apply || \ + bash "$${ROOT}/scripts/provision/zitadel-idp.sh" sync "$${IDP_SYNC_ARGS[@]}" --apply || \ echo "[warn] IdP registration failed; re-run it by hand" echo "== registering the OIDC clients" IDP_URL="https://auth.$${PUBLIC_DOMAIN}" PRIVATE_DOMAIN="$${PRIVATE_DOMAIN}" \ - bash "$${ROOT}/scripts/provision/zitadel-oidc-clients.sh" sync \ - --cluster "$${NAME}" --cloud gcp --project "$${PROJECT}" \ - --workforce-pool "$${WORKFORCE_POOL}" \ - --openbao-url "https://bao.$${PRIVATE_DOMAIN}:8200" \ - --openbao-root-token-secret openbao-priv-gcp-root-token \ - --openbao-ca-file "$${ROOT}/opentofu/gcp/gke/configure/.tls/ca.pem" \ - --mirror-openbao \ - --apply || \ + bash "$${ROOT}/scripts/provision/zitadel-oidc-clients.sh" sync "$${CLIENT_SYNC_ARGS[@]}" --apply || \ echo "[warn] OIDC registration failed; re-run it by hand" echo "== granting access to the secrets it just created" @@ -329,6 +339,38 @@ script "deploy" { ], ] } + + job { + name = "stage5-verify-openbao-oidc" + description = "Fail the deploy when a hosting gcp-0's OpenBao OIDC client disagrees with the store or ZITADEL no longer knows it" + commands = [ + ["bash", "-c", <<-BASH + ${global.cloud_gate} + set -euo pipefail + ROOT="${terramate.root.path.fs.absolute}" + + # Stage 3's warn-and-continue lets a failed rotation through; this is + # what stops it. The check is the last statement, so exit 1 (drift) and + # exit 2 (cannot tell) both halt, as on aws-0. + DEPLOY_IDP="$${TF_VAR_deploy_identity_provider:-${global.deploy_identity_provider_gcp}}" + if [ "$${DEPLOY_IDP}" != "true" ]; then + echo "== skipping: this cluster consumes ${global.primary_cloud}'s directory, whose own deploy verifies OpenBao's OIDC client" + exit 0 + fi + + PROJECT="$(${global.provisioner} output -raw project_id)" + OPENBAO_URL="https://bao.$(${global.provisioner} output -raw private_domain_name):8200" + echo "== verifying OpenBao's OIDC client" + bash "$${ROOT}/scripts/provision/openbao-oidc-check.sh" \ + --url "$${OPENBAO_URL}" \ + --root-token-secret-name openbao-priv-gcp-root-token \ + --ca-file "$${ROOT}/opentofu/gcp/gke/configure/.tls/ca.pem" \ + --cloud gcp --project "$${PROJECT}" \ + --redirect-uri "$${OPENBAO_URL}/ui/vault/auth/oidc/oidc/callback" + BASH + ], + ] + } } script "deploy-stage1" { diff --git a/scripts/ci/tests/test-cloud-secret-store.sh b/scripts/ci/tests/test-cloud-secret-store.sh index 8c8455635..08655e10c 100755 --- a/scripts/ci/tests/test-cloud-secret-store.sh +++ b/scripts/ci/tests/test-cloud-secret-store.sh @@ -277,6 +277,27 @@ PATH="$CREATEFAIL:$PATH" bash -c ' ' || rc=$? check "gcp create failure: non-zero" "1" "$rc" check_log_absent "gcp create failure: no versions add" 'versions add' "$STUB_LOG" + +# A describe that failed transiently sends an existing secret down the create +# path; ALREADY_EXISTS then proves it exists, so the version is still added. +cat > "$CREATEFAIL/gcloud" <<'EOF' +#!/usr/bin/env bash +printf 'CALL:' >> "$STUB_LOG"; printf ' %q' "$@" >> "$STUB_LOG"; printf '\n' >> "$STUB_LOG" +for a in "$@"; do case "$a" in + describe) exit 1 ;; + create) echo "ERROR: (gcloud.secrets.create) ALREADY_EXISTS: Secret [projects/proj/secrets/raced-secret] already exists." >&2; exit 1 ;; +esac; done +exit 0 +EOF +: > "$STUB_LOG" +rc=0 +PATH="$CREATEFAIL:$PATH" bash -c ' + . "'"$HERE"'/../../lib/cloud-secret-store.sh" + CLOUD=gcp REGION="" GCP_PROJECT=proj + store_write raced-secret <<< "{\"pat\":\"fake-full-token\"}" || exit $? +' 2>/dev/null || rc=$? +check "gcp create ALREADY_EXISTS: succeeds" "0" "$rc" +check_log "gcp create ALREADY_EXISTS: versions add still runs" 'versions add' "$STUB_LOG" rm -rf "$CREATEFAIL" exit $fail diff --git a/scripts/ci/tests/test-gcp-gke-init-workflow.py b/scripts/ci/tests/test-gcp-gke-init-workflow.py index 6305a1589..e4ec0d82b 100644 --- a/scripts/ci/tests/test-gcp-gke-init-workflow.py +++ b/scripts/ci/tests/test-gcp-gke-init-workflow.py @@ -61,11 +61,61 @@ def before(body, first, second, what): fails.append("the destroy's custom-role state rm waits for the state lock") s3 = job_body(TEXT, "stage3-secrets-and-oidc") -hosting = s3[s3.find("== registering the OIDC clients"):] if "== registering the OIDC clients" in s3 else "" before(s3, 'zitadel-idp.sh" sync', "== registering the OIDC clients", "a hosting stage 3 configures the IdP and Action before its clients") -for flag in ("--openbao-url", "--openbao-root-token-secret openbao-priv-gcp-root-token", "--openbao-ca-file", "--mirror-openbao"): - if flag not in hosting: - fails.append(f"the hosting clients sync passes {flag}") +# One flag list per sync, expanded by both the real call and the printed +# recovery, so the recovery cannot drift from what the deploy runs. +client_args = re.search(r"CLIENT_SYNC_ARGS=\((.*?)\n\s*\)", s3, re.S) +client_args = client_args.group(1) if client_args else "" +for flag in ("--openbao-url", "--openbao-root-token-secret openbao-priv-gcp-root-token", "--openbao-ca-file", "--mirror-openbao", + "--workforce-pool", "--cluster", "--project"): + if flag not in client_args: + fails.append(f"CLIENT_SYNC_ARGS carries {flag}") +if "--cluster" not in (re.search(r"IDP_SYNC_ARGS=\((.*?)\)", s3, re.S) or [None, ""])[1]: + fails.append("IDP_SYNC_ARGS carries --cluster") + + +def call_line(text, script): + """The logical line (continuations joined) that runs `script" sync`.""" + joined = re.sub(r"\\\n\s*", " ", text) + return next((l for l in joined.splitlines() if f'{script}" sync' in l and not l.lstrip().startswith("echo")), "") + + +real = s3[s3.find("== registering the Google IdP"):] if "== registering the Google IdP" in s3 else "" +if "IDP_SYNC_ARGS[@]" not in call_line(real, "zitadel-idp.sh"): + fails.append("the real IdP sync expands IDP_SYNC_ARGS") +if "CLIENT_SYNC_ARGS[@]" not in call_line(real, "zitadel-oidc-clients.sh"): + fails.append("the real clients sync expands CLIENT_SYNC_ARGS") +m = re.search(r"ZITADEL not ready.*?\n\s*exit 0", s3, re.S) +recovery = m.group(0) if m else "" +before(recovery, "zitadel-idp.sh sync", "zitadel-oidc-clients.sh sync", "the not-ready recovery prints the IdP sync, then the clients sync") +for line_has, arr in (("zitadel-idp.sh sync", "IDP_SYNC_ARGS[@]"), ("zitadel-oidc-clients.sh sync", "CLIENT_SYNC_ARGS[@]")): + line = next((l for l in recovery.splitlines() if line_has in l), "") + if arr not in line or "--apply" not in line: + fails.append(f"the not-ready recovery's {line_has} expands {arr} and passes --apply") +before(s3, "CLIENT_SYNC_ARGS=(", "ZITADEL not ready", "the flag lists exist before the recovery prints them") + +# Stage 5 (item 3): the same check aws-0's deploy halts on, for a hosting gcp-0. +deploy = re.search(r'\nscript "deploy" \{(.*?)(?=\nscript "|\Z)', TEXT, re.S) +deploy = deploy.group(1) if deploy else "" +s5 = job_body(deploy, "stage5-verify-openbao-oidc") +if not s5: + fails.append("script deploy has a stage5-verify-openbao-oidc job") +before(deploy, '"stage3-secrets-and-oidc"', '"stage5-verify-openbao-oidc"', "stage 5 runs after stage 3") +if "set -euo pipefail" not in s5: + fails.append("stage 5 runs under errexit") +body5 = s5.split("<<-BASH", 1)[-1].split("\n BASH", 1)[0] +logical5 = [l.strip() for l in re.sub(r"\\\n\s*", " ", body5).splitlines() if l.strip()] +last5 = logical5[-1] if logical5 else "" +if not last5.startswith('bash "$${ROOT}/scripts/provision/openbao-oidc-check.sh"') or re.search(r"\|\||&&|;|\|", last5): + fails.append("stage 5's check is its last statement, so exit 1 and exit 2 both halt the deploy") +for arg in ("--cloud gcp", "--project", "--root-token-secret-name openbao-priv-gcp-root-token", + "/opentofu/gcp/gke/configure/.tls/ca.pem", "/ui/vault/auth/oidc/oidc/callback"): + if arg not in last5: + fails.append(f"stage 5's check passes {arg}") +before(s5, 'DEPLOY_IDP}" != "true" ]', "openbao-oidc-check.sh", "stage 5 skips a consuming cluster before the check") +skip = s5[s5.find('DEPLOY_IDP}" != "true" ]'):s5.find("openbao-oidc-check.sh")] +if "exit 0" not in skip or "echo" not in skip: + fails.append("stage 5's consumer skip says why and exits 0") for f in fails: print("FAIL", f) diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh b/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh index 5747b7330..bc9c6ab7c 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-convergence.sh @@ -120,6 +120,7 @@ store_write() { cat > "$STORE_DIR/$1"; } load_function store_write_and_mirror "$SRC" load_function publish_project_id "$SRC" # a no-op here: CLOUD=aws mirror_to_openbao() { cat >/dev/null; } +force_sync_mirrored() { :; } # its own suite; no kubectl here # ── globals cmd_sync / converge_secret read ───────────────────────────────── CLUSTER="aws-0" diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh b/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh index 4470ea588..6054c5444 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-mirror.sh @@ -7,9 +7,10 @@ # # --mirror-openbao (GCP parity GP-5): a fresh directory re-registers every client # each build, and gcp-0's consumers read OpenBao. The mirror copies only the -# fields the sync owns onto the mapped path, writes only on a change, says when -# it skips an unmapped key, and does nothing unset. The functions are lifted out -# of the script, so this tests the code that ships. +# fields the sync owns onto an existing value (the whole payload onto an absent +# one), writes only on a change, says when it skips an unmapped key, and does +# nothing unset. The functions are lifted out of the script, so this tests the +# code that ships. set -uo pipefail REPO_ROOT="$(cd "$(dirname "$0")/../../.." && pwd)" cd "$REPO_ROOT" || exit 1 @@ -73,11 +74,16 @@ out="$(printf '%s' '{"GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"same"}' \ [ "$rc" -eq 0 ] && ! grep -q '^POST' "$T/calls" && [ ! -e "$T/body" ] \ && ok "unchanged: no write, so no new KV version" || bad "unchanged: rc=$rc, calls $(cat "$T/calls")" +# A 404 means OpenBao holds nothing the ownership filter could protect, and +# `migrate` is additive per path: it skips a path that exists. An owned-fields +# write here would leave grafana-envvars without its admin credentials for good. reset GET_CODE=404 GET_BODY='{"errors":[]}' -printf '%s' '{"client_id":"new","not_ours":"x"}' | mirror_to_openbao harbor-oidc >/dev/null -[ "$(jq -c '.data' "$T/body" 2>/dev/null)" = '{"client_id":"new"}' ] \ - && ok "absent path (404): written fresh, owned fields only" || bad "404 case: $(cat "$T/body" 2>/dev/null)" +store_blob='{"GF_SECURITY_ADMIN_USER":"admin","GF_SECURITY_ADMIN_PASSWORD":"fixture-pw","GF_AUTH_GENERIC_OAUTH_CLIENT_ID":"new"}' # pragma: allowlist secret +printf '%s' "$store_blob" | mirror_to_openbao observability-victoria-metrics-k8s-stack-grafana-envvars >/dev/null +[ "$(jq -c '.data' "$T/body" 2>/dev/null)" = "$store_blob" ] \ + && ok "absent path (404): written fresh with the full store payload" \ + || bad "404 case: wrote keys $(jq -c '.data | keys' "$T/body" 2>/dev/null)" reset GET_CODE=403 @@ -137,6 +143,52 @@ for consumer in grafana headlamp flux-ui harbor openbao headlamp-proxy; do done [ -z "$missing" ] && ok "every field merge_secret writes is in MIRRORED_FIELDS" || bad "not in MIRRORED_FIELDS:${missing}" +# M-4: the ExternalSecrets reading a mirrored path are force-synced, so they do +# not wait out their refreshInterval on the dead directory's clients. Matched on +# store (openbao-) AND key, through both data and dataFrom. +fs_body="$(sed -n '/^force_sync_mirrored() {/,/^}/p' "$S")" +if [ -z "$fs_body" ]; then + bad "could not extract force_sync_mirrored() from $S" +else + eval "$fs_body" + ES_JSON='{"items":[ + {"metadata":{"namespace":"observability","name":"grafana-es"},"spec":{"secretStoreRef":{"name":"openbao-platform"},"dataFrom":[{"extract":{"key":"victoria-metrics/grafana-envvars"}}]}}, + {"metadata":{"namespace":"tooling","name":"harbor-es"},"spec":{"secretStoreRef":{"name":"openbao-platform"},"data":[{"secretKey":"x","remoteRef":{"key":"harbor/oidc"}}]}}, + {"metadata":{"namespace":"apps","name":"wrong-mount"},"spec":{"secretStoreRef":{"name":"openbao-apps"},"dataFrom":[{"extract":{"key":"harbor/oidc"}}]}}, + {"metadata":{"namespace":"security","name":"gsm-es"},"spec":{"secretStoreRef":{"name":"clustersecretstore"},"dataFrom":[{"extract":{"key":"harbor-oidc"}}]}}, + {"metadata":{"namespace":"tooling","name":"find-only"},"spec":{"secretStoreRef":{"name":"openbao-platform"},"dataFrom":[{"find":{"path":"harbor"}}]}}]}' + KGET_RC=0 + kubectl() { + printf '%s\n' "$*" >>"$T/kubectl" + case "$1" in + get) printf '%s' "$ES_JSON"; return "$KGET_RC" ;; + annotate) return 0 ;; + esac + } + APPLY=true MIRROR_OPENBAO=true + : >"$T/kubectl" + force_sync_mirrored observability-victoria-metrics-k8s-stack-grafana-envvars harbor-oidc openbao-oidc >/dev/null 2>&1; rc=$? + annotated="$(grep '^annotate' "$T/kubectl" | awk '{print $5"/"$3}' | sort | tr '\n' ' ')" + [ "$rc" -eq 0 ] && [ "$annotated" = "observability/grafana-es tooling/harbor-es " ] \ + && ok "force-sync: exactly the ExternalSecrets reading a mirrored path" || bad "force-sync annotated: '$annotated' rc=$rc" + grep '^annotate' "$T/kubectl" | grep -qv -- '--overwrite' \ + && bad "force-sync annotate without --overwrite" || ok "force-sync annotates with --overwrite" + : >"$T/kubectl" + APPLY=false force_sync_mirrored harbor-oidc >/dev/null 2>&1 + [ ! -s "$T/kubectl" ] && ok "force-sync: nothing on a dry run" || bad "force-sync ran on a dry run" + : >"$T/kubectl" + MIRROR_OPENBAO=false force_sync_mirrored harbor-oidc >/dev/null 2>&1 + [ ! -s "$T/kubectl" ] && ok "force-sync: nothing without --mirror-openbao" || bad "force-sync ran without the flag" + KGET_RC=1 + err="$(force_sync_mirrored harbor-oidc 2>&1 >/dev/null)"; rc=$? + [ "$rc" -eq 0 ] && grep -q 'WARN' <<< "$err" \ + && ok "force-sync: a failed listing warns and returns 0" || bad "force-sync listing failure: rc=$rc err='$err'" + KGET_RC=0 + unset -f kubectl +fi +grep -q '^ force_sync_mirrored "\${mirrored_keys\[@\]}"' "$S" \ + && ok "cmd_sync force-syncs the mirrored keys" || bad "cmd_sync never calls force_sync_mirrored" + grep -q -- '--mirror-openbao) MIRROR_OPENBAO="true"; shift ;;' "$S" && ok "--mirror-openbao is parsed" || bad "--mirror-openbao is not parsed" [ "$(grep -c 'store_write_and_mirror "\$key"' "$S")" -eq 2 ] \ && ok "both consumer writes go through store_write_and_mirror" || bad "a consumer write bypasses the mirror" diff --git a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh index bcfd015e8..39d3fac67 100755 --- a/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh +++ b/scripts/ci/tests/test-zitadel-oidc-clients-openbao.sh @@ -163,8 +163,12 @@ check "gcp/gke/init runs at least one sync" "yes" "$([ "${gcp_syncs:-0}" -ge 1 ] gcp_hosting="$(logical_lines "$GCP_WORKFLOWS_SRC" | grep -F 'zitadel-oidc-clients.sh" sync' | grep -vF -- '--idp-cloud' || true)" gcp_consumer="$(logical_lines "$GCP_WORKFLOWS_SRC" | grep -F 'zitadel-oidc-clients.sh" sync' | grep -F -- '--idp-cloud' || true)" contains "$gcp_consumer" '--idp-cloud' "gcp/gke/init's consumer sync is found" -contains "$gcp_hosting" '--openbao-url' "gcp/gke/init's hosting sync passes --openbao-url" -contains "$gcp_hosting" '--mirror-openbao' "gcp/gke/init's hosting sync mirrors into OpenBao" +# The hosting sync takes its flags from CLIENT_SYNC_ARGS, one multi-line array +# shared with the recovery the job prints; comment lines inside it dropped. +gcp_client_args="$(awk '/^[[:space:]]*CLIENT_SYNC_ARGS=\(/ { on = 1 } on && !/^[[:space:]]*#/ { print } on && /^[[:space:]]*\)[[:space:]]*$/ { exit }' "$GCP_WORKFLOWS_SRC")" +contains "$gcp_hosting" '"$${CLIENT_SYNC_ARGS[@]}"' "gcp/gke/init's hosting sync expands CLIENT_SYNC_ARGS" +contains "$gcp_client_args" '--openbao-url' "gcp/gke/init's hosting sync passes --openbao-url" +contains "$gcp_client_args" '--mirror-openbao' "gcp/gke/init's hosting sync mirrors into OpenBao" check "gcp/gke/init's consumer sync passes no --openbao-* flag" "" "$(grep -E -- '--openbao-|--mirror-openbao' <<<"$gcp_consumer" || true)" echo @@ -210,6 +214,8 @@ check_flags_known() { # workflows file, job name -> "yes", or "no: yes/no: the check call is the heredoc's ONLY last } check "the committed file: the check call is the heredoc's only last statement" \ "yes" "$(check_call_last "$AWS_WORKFLOWS_SRC")" +check "gcp/gke/init: the check call is the heredoc's only last statement" \ + "yes" "$(check_call_last "$GCP_WORKFLOWS_SRC")" # Both mutants are built from the COMMITTED file, not a hand-written fixture, # so a future reformat of the real heredoc cannot make this proof stale @@ -1046,6 +1054,8 @@ for f in cmd_sync oidc_config_payload store_write_and_mirror publish_project_id; [ -n "$body" ] || { echo " FAIL could not extract ${f}() from $CONSUMERS_SRC" >&2; fail=1; } eval "$body" done +# Tested in test-zitadel-oidc-clients-mirror.sh; stubbed so no kubectl runs. +force_sync_mirrored() { :; } # Every OTHER thing cmd_sync calls, stubbed: this section is about the ONE new # call, not a restatement of the redirect/convergence suites' own coverage. diff --git a/scripts/lib/cloud-secret-store.sh b/scripts/lib/cloud-secret-store.sh index dbcc67986..2de4e40a9 100644 --- a/scripts/lib/cloud-secret-store.sh +++ b/scripts/lib/cloud-secret-store.sh @@ -94,7 +94,7 @@ store_write() { # "payload: unbound variable" -- taking the `|| rm -f` fallback with it and # leaving the plaintext payload on disk. mktemp paths never contain quotes. # shellcheck disable=SC2064 - trap "shred -u '$payload' '$body' 2>/dev/null || rm -f '$payload' '$body'" EXIT + trap "shred -u '$payload' '$body' 2>/dev/null || rm -f '$payload' '$body'; rm -f '$body.err'" EXIT cat > "$payload" || exit 1 case "$CLOUD" in @@ -113,11 +113,15 @@ store_write() { fi ;; gcp) - store_exists "$name" || gcp_gcloud secrets create "$name" \ + # A describe that failed transiently lands an existing secret + # here; ALREADY_EXISTS proves it exists, so add the version. + if ! store_exists "$name" && ! gcp_gcloud secrets create "$name" \ ${GCP_PROJECT:+--project "$GCP_PROJECT"} \ --replication-policy=automatic \ - --labels=managed-by="${STORE_WRITE_LABEL:-cloud-secret-store}" >/dev/null \ - || exit 1 + --labels=managed-by="${STORE_WRITE_LABEL:-cloud-secret-store}" \ + >/dev/null 2>"${body}.err"; then + grep -q 'ALREADY_EXISTS' "${body}.err" || { cat "${body}.err" >&2; exit 1; } + fi gcp_gcloud secrets versions add "$name" \ ${GCP_PROJECT:+--project "$GCP_PROJECT"} \ --data-file="$payload" >/dev/null diff --git a/scripts/provision/zitadel-oidc-clients.sh b/scripts/provision/zitadel-oidc-clients.sh index 37fc38b9f..fe3ce7a78 100755 --- a/scripts/provision/zitadel-oidc-clients.sh +++ b/scripts/provision/zitadel-oidc-clients.sh @@ -42,11 +42,16 @@ # zitadel-oidc-clients.sh sync --cluster aws-0 --cloud aws [--region R] [--apply] # # # a SECONDARY cluster consuming the primary cloud's identity provider: -# # admin PAT from AWS, client secrets into GCP, kubectl pointed at aws-0. +# # admin PAT read from AWS's store (never kubectl), client secrets into GCP, +# # kubectl pointed at gcp-0 for its vars ConfigMap's audience scope. # IDP_URL=https://auth.cloud.ogenki.io PRIVATE_DOMAIN=priv.gcp.ogenki.io \ # zitadel-oidc-clients.sh sync --cluster gcp-0 \ # --cloud gcp --project ID --idp-cloud aws --region eu-west-3 --apply # +# On the HOSTING cloud the PAT is resolved from kubectl's cluster and overwrites +# the stored one (GP-20), so kubectl must point at the hosting cluster: a +# leftover security/iam-admin-pat anywhere else would replace that cloud's PAT. +# # Dry-run unless --apply. Client secrets are never printed: ZITADEL returns a # client secret exactly once, at creation, so it goes straight from the API # response into the secret store. @@ -859,10 +864,12 @@ openbao_oidc_config_payload() { } # The fields this script owns in a consumer secret: every key merge_secret and -# converge_secret write. The mirror copies only these. The rest of a blob -# belongs to another writer -- seed's GF_SECURITY_ADMIN_PASSWORD in -# grafana-envvars -- and OpenBao's copy of it may be newer than the store's (a -# rotation made in OpenBao), so the mirror never overwrites it. +# converge_secret write. Onto an EXISTING OpenBao value the mirror copies only +# these. The rest of a blob belongs to another writer -- seed's +# GF_SECURITY_ADMIN_PASSWORD in grafana-envvars -- and OpenBao's copy of it may +# be newer than the store's (a rotation made in OpenBao), so the mirror never +# overwrites it. An ABSENT path gets the whole payload: `migrate` skips a path +# that exists, so nothing else would ever fill in the rest. # test-zitadel-oidc-clients-mirror.sh fails when merge_secret writes a key # missing here. MIRRORED_FIELDS=( @@ -876,9 +883,10 @@ MIRRORED_FIELDS=( # Mirror one consumer secret into the OpenBao path its ExternalSecret reads # (--mirror-openbao): MIRRORED_FIELDS from the store's blob, onto what OpenBao -# holds. Writes only when that changes OpenBao's value, so a re-run adds no KV -# version. Unmapped keys (openbao-oidc, headlamp-oauth2-proxy) are read from the -# managed store; they are skipped, and said so. A subshell, like +# holds, or the whole blob when OpenBao holds nothing. Writes only when that +# changes OpenBao's value, so a re-run adds no KV version. Unmapped keys +# (openbao-oidc, headlamp-oauth2-proxy) are read from the managed store; they +# are skipped, and said so. A subshell, like # reconcile_openbao_oidc, so its temp files and trap stay local. Secrets move # through stdin and 0700-directory files, never argv. mirror_to_openbao() ( @@ -907,19 +915,21 @@ mirror_to_openbao() ( # error, a timeout) must not become an empty merge base. curl's stderr is # shown for those: it holds curl's own error line only -- the token is in a # -K file and a body goes to -o -- and a 404's "(22)" line is expected. + absent=false code="$(openbao_req GET "${mount}/data/${path}" -o "$tmp/read" -w '%{http_code}' 2>"$tmp/err")" || true case "$code" in 200) jq -ce '.data.data // {} | objects' "$tmp/read" > "$tmp/current" \ || { echo "[FAILED ] ${key} -- ${target} is not readable JSON; not overwriting it" >&2; exit 1; } ;; - 404) printf '{}' > "$tmp/current" ;; + 404) printf '{}' > "$tmp/current"; absent=true ;; *) echo "[FAILED ] ${key} -- reading ${target} returned HTTP ${code:-none}; not overwriting it" >&2 cat "$tmp/err" >&2 exit 1 ;; esac # Field NAMES go in as --args (none is secret); both values on stdin. - if ! cat "$tmp/current" "$tmp/payload" | jq -cn ' + if ! cat "$tmp/current" "$tmp/payload" | jq -cn --argjson absent "$absent" ' input as $cur | input as $p | - ($cur + ($p | with_entries(select(.key | IN($ARGS.positional[]))))) as $new | + (if $absent then $p + else $cur + ($p | with_entries(select(.key | IN($ARGS.positional[])))) end) as $new | if $new == $cur then empty else {data: $new} end' \ --args "${MIRRORED_FIELDS[@]}" > "$tmp/write"; then echo "[FAILED ] ${key} -- could not merge into ${target}; not overwriting it" >&2 @@ -949,6 +959,39 @@ store_write_and_mirror() { printf '%s' "$payload" | mirror_to_openbao "$key" || return 2 } +# Force-sync every ExternalSecret that reads a mirrored path. Left alone, each +# waits out its refreshInterval (up to 1h) serving the dead directory's client. +# Matched on store (openbao-) and key. Warn-only: the mirror already +# converged OpenBao, and the next refresh picks it up regardless. +force_sync_mirrored() { + [ "$APPLY" = "true" ] && [ "${MIRROR_OPENBAO:-false}" = "true" ] || return 0 + local key target es_json ns name now targets=() + for key in "$@"; do + target="$(bao_target_for "$key")" && targets+=("$target") + done + [ "${#targets[@]}" -gt 0 ] || return 0 + if ! es_json="$(kubectl get externalsecrets -A -o json 2>/dev/null)"; then + echo "WARN: could not list ExternalSecrets; mirrored ones refresh on their own interval" >&2 + return 0 + fi + now="$(date +%s)" + jq -r '.items[] + | (.spec.secretStoreRef.name // "") as $store + | select($store | startswith("openbao-")) + | ($store | ltrimstr("openbao-")) as $mount + | select([(.spec.data // [])[].remoteRef.key?, (.spec.dataFrom // [])[].extract.key?] + | map(select(. != null) | $mount + "/" + .) + | any(IN($ARGS.positional[]))) + | "\(.metadata.namespace) \(.metadata.name)"' --args "${targets[@]}" <<< "$es_json" \ + | while read -r ns name; do + if kubectl annotate externalsecret "$name" -n "$ns" force-sync="$now" --overwrite >/dev/null; then + echo "[synced ] externalsecret ${ns}/${name}" + else + echo "WARN: could not force-sync externalsecret ${ns}/${name}" >&2 + fi + done || echo "WARN: could not match ExternalSecrets to the mirrored paths" >&2 +} + # GCP hosting: publish this directory's project id for gke/configure, which # reads it at plan time (GCP parity GP-4). A fresh directory gets a new id # every build, and the committed one would otherwise come back on the next @@ -1168,6 +1211,8 @@ cmd_sync() { # A failed mirror does not stop the loop, like openbao_failed below: the # other consumers and both reconciles still run, then the sync exits 1. local failed_mirrors="" wrc=0 + # Keys mirrored without error, force-synced after the loop. + local mirrored_keys=() # Fed to reconcile_openbao_oidc after the loop -- see the consumer's own # branches below for where each is set. Empty stays empty on a dry run # (reconcile_openbao_oidc treats that as its own skip) and on any topology @@ -1323,8 +1368,11 @@ cmd_sync() { # converged, so nothing else here would ever retry it. Never on # a dry run; a no-op without --mirror-openbao. if [ "$APPLY" = "true" ]; then - printf '%s' "$existing_secret" | mirror_to_openbao "$key" \ - || failed_mirrors="${failed_mirrors}${failed_mirrors:+ }${key}" + if printf '%s' "$existing_secret" | mirror_to_openbao "$key"; then + mirrored_keys+=("$key") + else + failed_mirrors="${failed_mirrors}${failed_mirrors:+ }${key}" + fi fi elif [ "$APPLY" != "true" ]; then echo "[dry-run] ${name} -- would converge non-secret fields in ${key}" @@ -1333,7 +1381,7 @@ cmd_sync() { wrc=0 printf '%s' "$desired" | store_write_and_mirror "$key" || wrc=$? case "$wrc" in - 0) ;; + 0) mirrored_keys+=("$key") ;; 2) failed_mirrors="${failed_mirrors}${failed_mirrors:+ }${key}" ;; *) exit 1 ;; esac @@ -1373,7 +1421,7 @@ cmd_sync() { wrc=0 printf '%s' "$merged" | store_write_and_mirror "$key" || wrc=$? case "$wrc" in - 0) ;; + 0) mirrored_keys+=("$key") ;; 2) failed_mirrors="${failed_mirrors}${failed_mirrors:+ }${key}" ;; *) exit 1 ;; esac @@ -1398,6 +1446,7 @@ cmd_sync() { # reconcile failure should not hide it. local openbao_failed=0 reconcile_openbao_oidc "$openbao_key" "$openbao_client_id" || openbao_failed=1 + force_sync_mirrored "${mirrored_keys[@]}" echo echo "created: ${created}, updated: ${updated}, unchanged: ${skipped}, converged: ${converged}" @@ -1409,7 +1458,7 @@ cmd_sync() { echo "[FAILED ] not mirrored to OpenBao: ${failed_mirrors} -- the managed store has them; re-run sync --apply" >&2 fi if [ "$publish_failed" -ne 0 ]; then - echo "[FAILED ] zitadel-project-id not published -- gke/configure keeps the committed id; re-run sync --apply" >&2 + echo "[FAILED ] zitadel-project-id not published -- gke/configure keeps publishing the id already there (the previous build's, or the committed one on a first build); re-run sync --apply" >&2 fi [ "$openbao_failed" -eq 0 ] || exit 1 diff --git a/website/content/docs/get-started/sso.md b/website/content/docs/get-started/sso.md index 8e2378a5b..3b621d543 100644 --- a/website/content/docs/get-started/sso.md +++ b/website/content/docs/get-started/sso.md @@ -26,7 +26,7 @@ Everything below is the same on both clouds except these: | `IDP_URL` | `https://auth.cloud.ogenki.io` | `https://auth.gcp.cloud.ogenki.io` | | `PRIVATE_DOMAIN` | `priv.aws.ogenki.io` | `priv.gcp.ogenki.io` | | step 2 | **not needed** — External Secrets uses EKS Pod Identity, granted by OpenTofu | **required** | -| `OPENBAO` | the `--openbao-*` flags: step 1 re-points OpenBao's OIDC client too ([#2045](https://github.com/Smana/cloud-native-ref/issues/2045)) | empty — OpenBao OIDC exists only on `aws-0` | +| `OPENBAO` | the `--openbao-*` flags: step 1 re-points OpenBao's OIDC client too ([#2045](https://github.com/Smana/cloud-native-ref/issues/2045)) | hosting its own directory: the `--openbao-*` flags plus `--mirror-openbao`. Consuming `aws-0`'s: empty | Set them once and the rest of the page copies straight into a shell: @@ -39,15 +39,27 @@ export IDP_URL=https://auth.cloud.ogenki.io export PRIVATE_DOMAIN=priv.aws.ogenki.io ``` -On `gcp-0`: +On `gcp-0`, when it hosts its own directory: ```bash CL="--cluster gcp-0 --cloud gcp --project ogenki-435905" -OPENBAO="" +# gcp-0's ExternalSecrets read OpenBao, so step 1 also mirrors each client there. +OPENBAO="--openbao-url https://bao.priv.gcp.ogenki.io:8200 --openbao-root-token-secret openbao-priv-gcp-root-token --openbao-ca-file opentofu/gcp/gke/configure/.tls/ca.pem --mirror-openbao" export IDP_URL=https://auth.gcp.cloud.ogenki.io export PRIVATE_DOMAIN=priv.gcp.ogenki.io ``` +On `gcp-0`, when it consumes `aws-0`'s directory, `aws-0`'s own run rotates +OpenBao OIDC. Step 1 only registers `gcp-0`'s clients there, and step 3 is not +run: + +```bash +CL="--cluster gcp-0 --cloud gcp --project ogenki-435905 --idp-cloud aws --region eu-west-3" +OPENBAO="" +export IDP_URL=https://auth.cloud.ogenki.io +export PRIVATE_DOMAIN=priv.gcp.ogenki.io +``` + ## The steps The order is load-bearing: each one creates what the next reads. Running only the @@ -60,8 +72,8 @@ Every step is idempotent — re-running prints `[skip …]` and changes nothing. # projectRoleAssertion. That last one is not optional: with it off ZITADEL # puts NO roles in any token AND leaves ctx.v1.user.grants empty inside the # groups action, so every consumer authenticates and then has no groups. -# On aws-0, $OPENBAO points OpenBao at whatever client ZITADEL now holds; -# after a restore, leaving it out strands OpenBao on a client ZITADEL forgot. +# On a hosting cluster, $OPENBAO points OpenBao at whatever client ZITADEL +# now holds; leaving it out strands OpenBao on a client ZITADEL forgot. ./scripts/provision/zitadel-oidc-clients.sh sync $CL $OPENBAO --apply ``` @@ -132,10 +144,14 @@ converges from whatever step 1 last wrote to the `harbor-oidc` store entry. See {{< callout type="info" >}} **The admin PAT every script above needs is automatic, not a prerequisite.** The chart writes it once, into the `iam-admin-pat` Secret in the `security` -namespace, at `FirstInstance`. The first script run above reads that Secret and -captures the token into the cloud secret store; every later run — on any -cluster, restored or not — reads it back from there. Nothing here needs a token -minted by hand. +namespace, at `FirstInstance`. On the cloud that hosts the directory, every run +reads that Secret first, and it wins: it overwrites the cloud store's copy when +they differ, because a fresh `FirstInstance` minted a new token. The store is +read only when the Secret is gone, after a restore. A cluster consuming another +cloud's directory reads that cloud's store and never writes it. Run hosting +syncs with `kubectl` pointed at the hosting cluster, since a leftover Secret +elsewhere would overwrite the stored token. Nothing here needs a token minted +by hand. {{< /callout >}} {{< callout type="warning" >}}