From 8f9b2e4270c80bc93adeb9755793c1d1144f84f9 Mon Sep 17 00:00:00 2001 From: Keith Jackson Date: Sat, 5 Sep 2026 13:24:26 -0500 Subject: [PATCH] =?UTF-8?q?feat(backup):=20s3check=20proves=20a=20real=20r?= =?UTF-8?q?ound=20trip=20=E2=80=94=20and=20revives=20a=20guard=20I=20had?= =?UTF-8?q?=20killed?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit *** THE BUG, AND IT WAS MINE. *** The placeholder guards were `case "$s3" in ""|TBD-*)` — a PREFIX glob. Two commits earlier (32d6766) I changed the placeholder from `TBD-PROD-S3-BACKUP-ENDPOINT` to `https://TBD-R2-ACCOUNT-ID.r2.cloudflarestorage.com`, which does NOT start with `TBD-`. Verified: fnmatch('https://TBD-R2-...', 'TBD-*') is False. All three guards silently stopped matching, so **step_resume would have resumed velero on prod straight onto a nonexistent hostname** — velero installs, reports healthy, and no backup exists. That is precisely the failure the guard was written to prevent, disarmed by a value change in a DIFFERENT file with nothing tying the two together. Replaced with substring-matching s3_is_placeholder in preflight, s3check and resume. s3check rewritten from a reachability ping into a real proof. Per credential, from a throwaway in-cluster pod (pod egress is the only egress that matters — the workstation sits on a different network side and a host-side reading has already misled this project once): 1. the credential LOADS AND AUTHENTICATES in the exact shape its consumer reads — Velero's INI mounted from the `cloud` key, CNPG's discrete env keys; 2. the bucket EXISTS and is LISTABLE under this environment's own prefix, via ListObjectsV2 — the op Velero's BSL sync and Barman's WAL scan actually use. ListBuckets deliberately avoided: a bucket-scoped R2 token may not grant it; 3. PUT -> GET -> sha256+length compare -> DELETE -> confirm-gone. A HEAD would pass with a read-only token and with every write being rejected; 4. the R2 checksum trap probed ON PURPOSE by repeating the PUT with checksums enabled, and named either way. Buckets and prefixes are DERIVED (velero's from the authored HelmRelease via yq, cnpg's parsed from CNPG_BACKUP_PATH), not restated. Fails closed, with no "unknown, carrying on" arm left: no pod, unfinished pod, empty log, missing/duplicate/malformed verdict, any failed assertion, a claimed PASS with failures>0, an assertion count below the caller-computed minimum, an OK-line count disagreeing with the claimed count, or a PASS from a pod that ended Failed. Failures are classified by cause — no egress, NoSuchBucket, InvalidAccessKeyId, AccessDenied (token not scoped to BOTH buckets), InvalidArgument -> the checksum trap. `amazon/aws-cli` pinned rather than `minio/mc`: mc lacks sed/awk (which already broke one probe) AND cannot send checksum headers, so it cannot test for the trap. TIER_ORDER now includes platform-config-cdi, between platform-config and the leaf tiers. Without it `resume` returned success without ever waiting on the tier that guarantees the CDI StorageProfiles — and apps-vms depends on them: unifi provisions on longhorn-vm, where an unapplied profile means CDI infers RWO instead of the authored RWX, permanently non-live-migratable. CREDENTIALS: one key, four committed Secrets, two shapes — and NO manifest change is needed, only encrypted values. platform/backup/set-r2-credentials.sh writes all four with `sops set`, which encrypts IN PLACE, so no decrypted Secret ever exists in the working tree to be committed by accident — strictly better than staging a plaintext template. Prompts with `read -rs`, rejects anything outside the S3 credential charset (a paste that swallowed a newline authenticates nowhere), never prints a value, and verifies by decrypting back and comparing fingerprints. `--check` is read-only. The MinIO key is RETAINED — it is what reads the pre-migration backups. THE ACCOUNT ID IS ALREADY IN THE REPO: infra/tofu/backend.r2.hcl commits https://7ce452a1b3d6c86daf2c020edcb82d37.r2.cloudflarestorage.com for the tofu state bucket. If the backup buckets go in the same account, only the two token values are needed. The placeholder is KEPT anyway — fail-closed beats convenience, and the guard above just demonstrated why. Corroborating the checksum fix independently: infra/tofu/backend.tf already carries `region = "auto"` and `skip_s3_checksum = true # R2 rejects AWS trailing (streaming) checksums`. This repo had learned both R2 facts for tofu state already. VERSION FINDINGS, documented not measured against R2: plugin v1.14.2 documents checksumAlgorithm defaulting to CRC32, so the authored "" is load-bearing; Velero's kopia backend has NO checksum knob, so BSL metadata and PVC-data compatibility are SEPARATE questions; and in the live CNPG 16.4 image, boto3/botocore 1.35.60 is below the 1.36 line that turned on default request checksums — so CNPG should be clear today, but barman-cloud exposes no escape hatch, making a CNPG image bump an R2 re-verification event. VERIFIED BY ME, independently of the agent: the dead-guard glob (fnmatch False); the placeholder now refused on prod before any pod is created; a full round trip against dev's MinIO — 7/7 assertions on BOTH credential paths, byte-identical, pod Succeeded, nothing left behind, checksum trap correctly reported absent; all four Secrets and the committed account id present. Full CI gate PASS. STILL UNPROVEN AND SAID SO IN THE DOC: Velero-to-R2 (metadata or kopia data), CNPG-barman-to-R2, and any restore from R2. R-STATE-03 is PROVEN MinIO-ONLY and its ledger row predates this move. infra/BACKUP-R2.md §5 is the re-proof sequence and it ends in a RESTORE, not an upload. Refs R-STATE-03, R-IDP-05, R-ENV-02. Co-Authored-By: Claude Opus 5 --- bootstrap/standup.sh | 553 ++++++++++++++++-- infra/BACKUP-R2.md | 315 ++++++++++ .../minio-credentials.sops.yaml.example | 14 + platform/backup/set-r2-credentials.sh | 195 ++++++ platform/velero/README.md | 52 +- platform/velero/kustomization.yaml | 9 +- platform/velero/values.yaml | 13 +- 7 files changed, 1088 insertions(+), 63 deletions(-) create mode 100644 infra/BACKUP-R2.md create mode 100755 platform/backup/set-r2-credentials.sh diff --git a/bootstrap/standup.sh b/bootstrap/standup.sh index b7f9e3e..366849a 100755 --- a/bootstrap/standup.sh +++ b/bootstrap/standup.sh @@ -26,6 +26,11 @@ # # --confirm-destructive is required by `unwedge` only. Everything else is additive or reversible. # +# --s3-endpoint makes `s3check` probe a CANDIDATE backup endpoint instead of the live +# cluster-vars value, so a proposed target can be PROVEN before it is authored into cluster-vars. +# `s3check` writes and then deletes ONE throwaway object under `/.s3check/`; it changes +# nothing else and applies nothing. See infra/BACKUP-R2.md. +# set -euo pipefail # ---------------------------------------------------------------------------- args @@ -85,10 +90,18 @@ say "target confirmed: ${live_server}" # Dependency order. Flux enforces the edges itself via dependsOn; resuming in this order simply means # each wait is meaningful instead of blocking on a dependency that is still suspended. -# flux-system -> cilium -> platform -> platform-config -> {leaf tiers, apps-wave-0} -> wave-1 -> wave-2 -TIER_ORDER=(platform platform-config oauth2-proxy headlamp homepage crafty apps-wave-0 apps-wave-1 apps-wave-2 apps-vms) -# velero is NOT in that list on purpose — see step `resume`: it consumes S3_BACKUP_ENDPOINT, which on -# prod is an intentionally-invalid TBD placeholder pending an operator decision. +# flux-system -> cilium -> platform -> platform-config -> platform-config-cdi +# -> {leaf tiers, apps-wave-0} -> wave-1 -> wave-2 -> apps-vms +# +# `platform-config-cdi` MUST stay in this list and MUST stay before apps-vms. It carries the CDI +# StorageProfiles, which were split out of platform-config because StorageProfile's CRD does not exist +# until cdi-operator has reconciled the CDI CR (clusters/base/platform-config-cdi.yaml). apps-vms +# dependsOn it: apps/vms/unifi provisions on `longhorn-vm`, and without the profile applied CDI infers +# ReadWriteOnce instead of the authored ReadWriteMany — permanently non-live-migratable for that disk. +# Omitting it here would make `resume` return success without ever waiting on the tier that guarantees it. +TIER_ORDER=(platform platform-config platform-config-cdi oauth2-proxy headlamp homepage crafty apps-wave-0 apps-wave-1 apps-wave-2 apps-vms) +# velero is NOT in that list on purpose — see step `resume`: it consumes S3_BACKUP_ENDPOINT, which is +# an intentionally-invalid placeholder until the operator supplies the real R2 endpoint. cvar() { kubectl -n flux-system get cm cluster-vars -o jsonpath="{.data.$1}" 2>/dev/null || true; } @@ -150,6 +163,354 @@ dns_diff() { # — returns non-zero if anything changed return 1 } +# ======================================================================= s3check plumbing +# WHY THIS IS THIS BIG. The previous version of `s3check` curl'd /minio/health/live and reported +# REACHABLE. That probe could not pass against Cloudflare R2 (no such path), and reachability is not +# the property anyone cares about: a reachable endpoint with a wrong key, a missing bucket, or R2's +# checksum rejection still yields a velero tier that reports Ready while NO backup exists. What must +# be proven is the write path Velero and CNPG-barman actually use, with the credentials they actually +# hold, from the network position they actually occupy. + +# Pinned. The AWS CLI is a deliberate choice over `minio/mc`: +# - it prints the S3 ERROR CODE (NoSuchBucket / AccessDenied / InvalidAccessKeyId / InvalidArgument), +# so a failure can be NAMED and turned into a fix instead of guessed at from an HTTP status; +# - it exposes the FLEXIBLE-CHECKSUM behaviour that is the R2 trap +# (AWS_REQUEST_CHECKSUM_CALCULATION), so the trap can be reproduced on purpose here instead of +# discovered by a silently-failing backup at 03:00; +# - its image carries bash + coreutils. `minio/mc` ships no sed/awk/diff — that minimalism has +# already broken one probe in this repo — and it cannot be made to send the checksum headers, so +# it cannot test for the trap at all. +S3CHECK_IMAGE="amazon/aws-cli:2.36.40" +S3CHECK_RUN_ID="$(date +%s)" + +# Throwaway probe pods, tracked so they are removed on EVERY exit path (including `die`). A probe +# must never leave a pod behind on a live cluster. +S3CHECK_PODS=() +s3_cleanup_pods() { + local p + for p in ${S3CHECK_PODS[@]+"${S3CHECK_PODS[@]}"}; do + kubectl -n "${p%%/*}" delete pod "${p#*/}" --ignore-not-found --wait=false >/dev/null 2>&1 || true + done + S3CHECK_PODS=() +} +trap s3_cleanup_pods EXIT + +# TRUE when the endpoint is not a usable target. `TBD-*` alone was WRONG once the placeholder became +# `https://TBD-R2-ACCOUNT-ID.r2.cloudflarestorage.com`: that does not START with TBD-, so the old +# prefix pattern matched nothing and `resume` would have resumed velero straight onto a placeholder +# hostname — the exact silent-no-backups failure the guard exists to prevent. Match anywhere. +s3_is_placeholder() { # + case "$1" in + ""|*TBD*|*tbd-*|*REPLACE-ME*|*replace-me*|*CHANGE-ME*|*change-me*) return 0 ;; + esac + return 1 +} + +# Refuse any value that is not a plain URL/name fragment. This is BOTH a safety guard — these values +# are interpolated into the probe pod's JSON overrides AND into a shell script that runs inside it, so +# a quote, a backslash or a newline could rewrite either — and a correctness guard: a cluster-vars +# value with a trailing space or a stray quote is a defect, not something to paper over. +s3_safe_token() { #