diff --git a/hack/seaweedfs-naming-audit.bats b/hack/seaweedfs-naming-audit.bats index eb6356e02..424de20a3 100644 --- a/hack/seaweedfs-naming-audit.bats +++ b/hack/seaweedfs-naming-audit.bats @@ -138,3 +138,372 @@ EOF done < "$chart/rendered" rm -rf "$chart" } + +# ----------------------------------------------------------------------------- +# Fail-closed tests (issue #3431). +# +# The classification tests above mock nothing below the classifier. These drive +# the KUBECTL layer, where the fail-open bug lived: a kubectl call that failed +# used to return empty stdout, byte-identical to a genuinely clean fleet, so the +# audit printed an empty table and exited 0 -- the exact false "nothing to do" the +# operator is told to trust before deleting PVCs. They shim `kubectl` on PATH with +# a fake and assert the audit fails LOUDLY (non-zero exit + a FATAL naming the +# query) on any real error, while still treating a genuinely-absent object as +# clean. Two families: +# +# * enumerating LISTs (get ns / secret / pvc / sts) -- a failure is always fatal; +# * by-NAME GETs (a release secret in system_releases, a PVC/PV in pv_epoch) -- +# `--ignore-not-found` splits a real error (fatal) from a legitimate absence: +# an absent release revision is a clean skip; an absent PV degrades to the +# safe "cannot establish direction" note and must NOT become a deletion +# candidate (the range-narrowing flip Codex reproduced). +# +# The two golden tests guard the reverse: on a healthy cluster the output stays +# byte-for-byte what it was on origin/main, including the MIXED path that exercises +# every pv_epoch GET. +# +# The fake is a real executable on PATH, so it exercises the actual exit-status +# handling in run_kubectl and the propagation up through every caller -- a for +# loop over `$(...)` swallows the status, so this is where a regression would hide. + +# _release_blob [chart-name] -- the value kubectl returns for a Helm release +# secret's `.data.release`: base64(base64(gzip(json))). release_json base64-decodes +# twice and gunzips it, and system_releases reads the chart name from +# chart.metadata.name (as real Helm payloads carry it). Defaults to a cozy-seaweedfs +# release so the audit confirms the tenant; pass another chart name for a non- +# SeaweedFS release, or the literal EMPTY for a chartless {} payload. +_release_blob() { + _rb_name=${1:-cozy-seaweedfs} + if [ "$_rb_name" = EMPTY ]; then + _rb_json='{}' + else + _rb_json='{"name":"seaweedfs-system","chart":{"metadata":{"name":"'"$_rb_name"'"}}}' + fi + printf '%s' "$_rb_json" | gzip | base64 | base64 | tr -d '\n' +} + +# _write_fake_kubectl -- drop a fake kubectl +# into . +# one of ns | secretlist | secretget | secretget_absent | +# secret_missing_field | secret_bad_payload | pvclist | pvcget | +# sts | pvget | "" -- the single call to make behave badly. Real- +# error targets exit non-zero with a stderr message; secretget_absent +# mirrors an absent Secret (--ignore-not-found: exit 0, empty -o +# name); secret_missing_field / secret_bad_payload keep the Secret +# present but return an empty / undecodable .data.release payload. +# a PV name (pv-legacy|pv-legacy2|pv-renamed) to report as absent +# (exit 0, empty), exercising the "bound claim, PV gone" path. +# newline-separated `get pvc -o name` LIST output. +# It otherwise walks one confirmed seaweedfs-system tenant. The by-name PVC->PV and +# PV->timestamp maps are fixed here; timestamps are chosen so the two legacy PVs +# straddle the single renamed PV (true answer OVERLAP), which is what makes the +# range-narrowing flip observable. Kept POSIX and free of a column-0 `}` so +# cozytest.sh's awk converter passes the heredoc through untouched. +_write_fake_kubectl() { + # Grouped redirect (one open of the file). The closing brace is indented, so + # cozytest.sh's awk -- which only rewrites a `}` in column 0 -- leaves it and the + # heredoc alone. + { + printf '#!/bin/sh\n' + printf "FAIL='%s'\n" "$2" + printf "ABSENT_PV='%s'\n" "$3" + printf "BLOB='%s'\n" "$4" + printf "PVCS='%s'\n" "$5" + cat <<'FAKE' +verb=${1:-}; res=${2:-}; args="$*" +fail() { echo "fake kubectl: $1 (real error)" >&2; exit 1; } +if [ "$verb $res" = "get ns" ]; then + [ "$FAIL" = ns ] && fail "get ns" + printf 'namespace/tenant-test\n'; exit 0 +fi +if [ "$verb $res" = "get secret" ]; then + case "$args" in + *sh.helm.release.v1*) + # release_json now asks two questions: existence (-o name) then payload + # (jsonpath .data.release). Mirror that split so absence, real error, and + # corrupt-payload are all reachable independently. + case "$args" in + *"-o name"*) + [ "$FAIL" = secretget ] && fail "release secret existence GET" + [ "$FAIL" = secretget_absent ] && exit 0 + printf 'secret/sh.helm.release.v1.seaweedfs-system.v1\n'; exit 0 ;; + *) + [ "$FAIL" = secret_missing_field ] && exit 0 + if [ "$FAIL" = secret_bad_payload ]; then printf '@@@not-base64@@@\n'; exit 0; fi + printf '%s\n' "$BLOB"; exit 0 ;; + esac ;; + *owner=helm*) printf '1\n'; exit 0 ;; + *) [ "$FAIL" = secretlist ] && fail "namespace secret LIST" + printf 'seaweedfs-system\n'; exit 0 ;; + esac +fi +if [ "$verb $res" = "get pvc" ]; then + case "$args" in + *"-o name"*) + [ "$FAIL" = pvclist ] && fail "pvc LIST" + [ -n "$PVCS" ] && printf '%s\n' "$PVCS" + exit 0 ;; + *data1-seaweedfs-system-volume-0*) [ "$FAIL" = pvcget ] && fail "pvc GET"; printf 'pv-renamed\n'; exit 0 ;; + *data1-seaweedfs-volume-1*) [ "$FAIL" = pvcget ] && fail "pvc GET"; printf 'pv-legacy2\n'; exit 0 ;; + *data1-seaweedfs-volume-0*) [ "$FAIL" = pvcget ] && fail "pvc GET"; printf 'pv-legacy\n'; exit 0 ;; + esac + exit 0 +fi +if [ "$verb $res" = "get sts" ]; then + [ "$FAIL" = sts ] && fail "sts LIST" + exit 0 +fi +if [ "$verb $res" = "get pv" ]; then + [ "$FAIL" = pvget ] && fail "get pv" + case "$args" in + *pv-legacy2*) [ "$ABSENT_PV" = pv-legacy2 ] && exit 0; printf '2099-01-01T00:00:00Z\n'; exit 0 ;; + *pv-legacy*) [ "$ABSENT_PV" = pv-legacy ] && exit 0; printf '2020-01-01T00:00:00Z\n'; exit 0 ;; + *pv-renamed*) [ "$ABSENT_PV" = pv-renamed ] && exit 0; printf '2020-06-01T00:00:00Z\n'; exit 0 ;; + esac + exit 0 +fi +exit 0 +FAKE + } > "$1/kubectl" + chmod +x "$1/kubectl" +} + +# _pvcs_mixed -- the `get pvc -o name` LIST for a MIXED tenant: two legacy claims +# and one release-named claim, whose PV vintages truly overlap. +_pvcs_mixed() { + printf '%s\n' \ + persistentvolumeclaim/data1-seaweedfs-volume-0 \ + persistentvolumeclaim/data1-seaweedfs-volume-1 \ + persistentvolumeclaim/data1-seaweedfs-system-volume-0 +} + +# _expected_L / _expected_mixed_overlap -- golden output built with the SAME printf +# contract the script uses, so a drift in row count, text, or padding fails the diff. +_expected_L() { + printf '%-24s %-14s %-8s %s\n' NAMESPACE RELEASE CLASS NOTE + printf '%-24s %-14s %-8s %s\n' --------- ------- ----- ---- + printf '%-24s %-14s %-8s %s\n' tenant-test seaweedfs-system L 'chart-named only; the upgrade adopts it, nothing to do' +} +_expected_mixed_overlap() { + printf '%-24s %-14s %-8s %s\n' NAMESPACE RELEASE CLASS NOTE + printf '%-24s %-14s %-8s %s\n' --------- ------- ----- ---- + printf '%-24s %-14s %-8s %s\n' tenant-test seaweedfs-system MIXED 'both generations; the chart REFUSES until one is removed' + printf '%-24s %-14s %-8s %s\n' '' '' '' 'PV vintages OVERLAP => no candidate. An interrupted Step 2 re-bind looks exactly like this (both generations on original PVs). Finish Step 2 if one is in progress; otherwise escalate. Do NOT run Step 2a.' + printf '%-24s %-14s %-8s %s\n' '' '' '' 'CANDIDATE ONLY: "original" does not mean the other set is EMPTY. A duplicate that' + printf '%-24s %-14s %-8s %s\n' '' '' '' 'served writes and later crashed looks identical here. Verify emptiness before deleting.' +} + +@test "fails closed when 'kubectl get ns' fails (whole-cluster mode)" { + # No namespace args -> main enumerates namespaces itself. If that LIST fails, + # the OLD code audited zero namespaces and still printed an empty clean table. + d=$(mktemp -d) + _write_fake_kubectl "$d" ns "" "$(_release_blob)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -ne 0 ] + printf '%s\n' "$out" | grep -q 'FATAL' + printf '%s\n' "$out" | grep -q 'list namespaces' +} + +@test "fails closed when the namespace-wide secret LIST fails" { + # system_releases enumerates Helm releases with a namespace-wide secret LIST -- + # the exact call that timed out in the field and reported a false clean. + d=$(mktemp -d) + _write_fake_kubectl "$d" secretlist "" "$(_release_blob)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -ne 0 ] + printf '%s\n' "$out" | grep -q 'FATAL' + printf '%s\n' "$out" | grep -q 'list Helm release secrets' +} + +@test "fails closed when the by-name release-secret existence GET hits a real error" { + # Codex Finding 1: system_releases uses release_json to decide whether a release + # IS SeaweedFS. Both LISTs succeed, then a transient/forbidden by-name Secret GET + # used to leave `chart` empty -> the tenant was silently skipped -> exit 0, empty + # table. A real error here must now be fatal, not a false clean. + d=$(mktemp -d) + _write_fake_kubectl "$d" secretget "" "$(_release_blob)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -ne 0 ] + printf '%s\n' "$out" | grep -q 'FATAL' + printf '%s\n' "$out" | grep -q 'Helm release secret' +} + +@test "an absent release-secret revision is a clean skip, not a failure" { + # The one legitimately-empty case for release_json: the revision secret does not + # exist (pruned by Helm history limit). The existence check (--ignore-not-found -o + # name) returns empty + exit 0, so the release is simply not confirmed as + # SeaweedFS. No error, no crash -- the audit completes and reports nothing here. + d=$(mktemp -d) + _write_fake_kubectl "$d" secretget_absent "" "$(_release_blob)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -eq 0 ] + [ "$(printf '%s\n' "$out" | grep -c 'FATAL')" -eq 0 ] +} + +@test "fails closed when a present release secret has no .data.release payload" { + # Codex re-review round 2: the existence check passes (Secret EXISTS), but its + # .data.release field is empty/missing. That is CORRUPT state, not an absence -- + # and because the earlier fix used --ignore-not-found -o jsonpath, which returns + # empty+0 for BOTH, it used to read as a clean skip. It must be a loud stop. + d=$(mktemp -d) + _write_fake_kubectl "$d" secret_missing_field "" "$(_release_blob)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -ne 0 ] + printf '%s\n' "$out" | grep -q 'FATAL' + printf '%s\n' "$out" | grep -q 'no .data.release payload' +} + +@test "fails closed when a present release secret has an undecodable payload" { + # Existence passes, .data.release is non-empty but is not base64(base64(gzip(...))), + # so every decode fails. The old `|| return 0` converted that decode failure into + # a clean 0 return -> silent skip. A corrupt payload must be fatal. + d=$(mktemp -d) + _write_fake_kubectl "$d" secret_bad_payload "" "$(_release_blob)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -ne 0 ] + printf '%s\n' "$out" | grep -q 'FATAL' + printf '%s\n' "$out" | grep -q 'not decodable' +} + +@test "fails closed when a present release secret decodes to a chartless payload" { + # Codex round 3: the payload fetches and DECODES cleanly (valid JSON {}), so the + # decode guards all pass -- but it carries no chart name. system_releases then + # extracted chart='' and silently skipped the tenant -> exit 0, clean table. A + # decoded helm.sh/release.v1 release without a chart name is corrupt: fatal. + d=$(mktemp -d) + _write_fake_kubectl "$d" "" "" "$(_release_blob EMPTY)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -ne 0 ] + printf '%s\n' "$out" | grep -q 'FATAL' + printf '%s\n' "$out" | grep -q 'no chart name' +} + +@test "a present release secret for a non-SeaweedFS chart is a silent legitimate skip" { + # The counterpart the guard must NOT break: a real, well-formed release whose + # chart is simply not cozy-seaweedfs. It has a chart name (so it is not corrupt), + # but the wrong one, so it is filtered out exactly as before -- no row, no error. + d=$(mktemp -d) + _write_fake_kubectl "$d" "" "" "$(_release_blob cozy-postgres)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -eq 0 ] + [ "$(printf '%s\n' "$out" | grep -c 'FATAL')" -eq 0 ] + # Not confirmed as SeaweedFS -> no classification row for the release. + [ "$(printf '%s\n' "$out" | grep -c 'seaweedfs-system')" -eq 0 ] +} + +@test "fails closed when 'kubectl get pvc' LIST fails" { + # The secret enumeration succeeds and confirms a seaweedfs-system tenant, so the + # audit reaches the per-namespace PVC LIST; a failure there must abort, not read + # as "this tenant has no claims". + d=$(mktemp -d) + _write_fake_kubectl "$d" pvclist "" "$(_release_blob)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -ne 0 ] + printf '%s\n' "$out" | grep -q 'FATAL' + printf '%s\n' "$out" | grep -q 'list PVCs' +} + +@test "fails closed when 'kubectl get sts' LIST fails" { + # PVC LIST succeeds (empty), so the audit reaches the StatefulSet LIST; a + # failure there must abort rather than read as "no StatefulSets". + d=$(mktemp -d) + _write_fake_kubectl "$d" sts "" "$(_release_blob)" "" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -ne 0 ] + printf '%s\n' "$out" | grep -q 'FATAL' + printf '%s\n' "$out" | grep -q 'StatefulSets' +} + +@test "fails closed when the by-name PV GET hits a real error" { + # Codex Finding 2, error half: pv_epoch reads each bound PV's age by name. A real + # error (RBAC/timeout) must abort, not silently drop that PV from the range. + d=$(mktemp -d) + _write_fake_kubectl "$d" pvget "" "$(_release_blob)" "$(_pvcs_mixed)" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -ne 0 ] + printf '%s\n' "$out" | grep -q 'FATAL' + printf '%s\n' "$out" | grep -q "get PV '" +} + +@test "an absent PV degrades to the safe fallback, never a wrong candidate" { + # Codex Finding 2, absence half. True vintages: legacy PVs 2020 + 2099 straddle + # the single renamed PV 2020-06 => OVERLAP. Report the newest legacy PV (2099) as + # ABSENT: the observed legacy range collapses to {2020} and, unguarded, "every + # release-named PV is strictly newer" would fire -- naming the release-named set a + # deletion candidate on incomplete evidence. The generation must instead read as + # incomplete and fall to "cannot establish direction". + d=$(mktemp -d) + _write_fake_kubectl "$d" "" pv-legacy2 "$(_release_blob)" "$(_pvcs_mixed)" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + rm -rf "$d" + echo "rc=$rc"; echo "$out" + [ "$rc" -eq 0 ] + [ "$(printf '%s\n' "$out" | grep -c 'FATAL')" -eq 0 ] + printf '%s\n' "$out" | grep -q 'direction cannot be established from PV ages' + # The whole point: incomplete evidence must NOT be reported as a deletion candidate. + [ "$(printf '%s\n' "$out" | grep -c 'candidate duplicate')" -eq 0 ] +} + +@test "success path (L) is byte-identical to the expected fixture" { + # A single chart-named claim, no release-named one, no StatefulSets => L, adopt in + # place. Full-output compare, so an extra row / warning / duplicate line fails. + d=$(mktemp -d) + _write_fake_kubectl "$d" "" "" "$(_release_blob)" "persistentvolumeclaim/data1-seaweedfs-volume-0" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + printf '%s\n' "$out" > "$d/got" + _expected_L > "$d/want" + echo "rc=$rc"; echo "--- got ---"; cat "$d/got"; echo "--- want ---"; cat "$d/want" + [ "$rc" -eq 0 ] + diff "$d/want" "$d/got" + rm -rf "$d" +} + +@test "success path (MIXED/overlap) is byte-identical and exercises pv_epoch" { + # Both generations present with all PVs readable => the MIXED path runs every + # pv_epoch GET and lands on OVERLAP. Full-output compare against the golden. + d=$(mktemp -d) + _write_fake_kubectl "$d" "" "" "$(_release_blob)" "$(_pvcs_mixed)" + rc=0 + out=$(PATH="$d:$PATH" SEAWEEDFS_AUDIT_LIB=0 sh "$PWD/hack/seaweedfs-naming-audit.sh" tenant-test 2>&1) || rc=$? + printf '%s\n' "$out" > "$d/got" + _expected_mixed_overlap > "$d/want" + echo "rc=$rc"; echo "--- got ---"; cat "$d/got"; echo "--- want ---"; cat "$d/want" + [ "$rc" -eq 0 ] + diff "$d/want" "$d/got" + rm -rf "$d" +} diff --git a/hack/seaweedfs-naming-audit.sh b/hack/seaweedfs-naming-audit.sh index a1e28c4c4..b01138a27 100755 --- a/hack/seaweedfs-naming-audit.sh +++ b/hack/seaweedfs-naming-audit.sh @@ -44,7 +44,15 @@ # # Usage: hack/seaweedfs-naming-audit.sh [namespace...] # KUBECONFIG= hack/seaweedfs-naming-audit.sh -# Exit: 0 always (audit only). Nothing is mutated. +# Exit: 0 the audit RAN TO COMPLETION; the table is the whole answer +# (an empty table then means a genuinely clean fleet). +# non-zero a kubectl query FAILED, so the audit is INCOMPLETE and the +# table must NOT be trusted -- a message naming the failed query +# is on stderr. This is fail-CLOSED by design (see run_kubectl): +# the output gates a destructive runbook step, so an unreachable +# API aborts loudly rather than printing an empty table that +# reads identically to "nothing to do". +# Nothing is ever mutated on either path -- every kubectl call is read-only. # # POSIX sh, deliberately. hack/seaweedfs-naming-audit.bats sources this file, and # cozytest.sh sources the converted test into its own /bin/sh — which is dash on @@ -55,6 +63,48 @@ # particular is not POSIX and dash 0.5.12 (Ubuntu) rejects it outright. set -u +# run_kubectl -- run ONE read-only kubectl query +# fail-CLOSED and print its stdout. This audit is the documented gate in front of +# a destructive runbook step (deleting a tenant's PVCs), so a query that FAILS +# must never be mistaken for a query that found nothing: an enumerating LIST +# returns empty stdout on a timeout or an RBAC denial, byte-identical to the same +# LIST succeeding on a genuinely clean fleet, and the old `2>/dev/null` on each +# call turned an unreachable API into a silent false "nothing to do". Every +# enumeration is routed through here so any failure is loud and terminal instead. +# +# On success prints kubectl's stdout verbatim -- which may legitimately be empty, +# because a LIST that matched nothing is honestly clean; keeping that case +# distinct from failure is the whole point. On a non-zero exit prints the failed +# query to stderr and returns that exit code; kubectl's own stderr stays on fd 2 +# and reaches the operator unfiltered. Callers MUST propagate the non-zero status +# (`|| return`/`|| exit`) -- a bare `for x in $(run_kubectl ...)` would swallow +# it, because a for-loop ignores the exit status of its word-list command, and an +# `exit` inside the `$(...)` subshell would only leave that subshell. +run_kubectl() { + _rk_what="$1"; shift + if _rk_out=$(kubectl "$@"); then + printf '%s' "$_rk_out" + return 0 + else + _rk_rc=$? + printf 'seaweedfs-naming-audit: FATAL: %s failed (kubectl %s -> exit %s). Refusing to report a clean fleet on an unreachable API.\n' \ + "$_rk_what" "$*" "$_rk_rc" >&2 + return "$_rk_rc" + fi +} + +# audit_fatal -- the counterpart to run_kubectl for corruption the API +# call itself did NOT report: the query succeeded, but what it returned is +# impossible for a healthy object (a helm.sh/release.v1 Secret with no decodable +# release payload). Same fail-CLOSED contract -- print a FATAL line naming the +# object to stderr and return non-zero; callers MUST propagate it. A silent skip +# here would be the identical false-clean this whole script guards against, just +# one field deeper than an unreachable API. +audit_fatal() { + printf 'seaweedfs-naming-audit: FATAL: %s Refusing to report a clean fleet on corrupt state.\n' "$1" >&2 + return 1 +} + # renamed_volume_prefix -- reconstruct the name 4.31 gives the volume # component of . Mirrors seaweedfs.fullname + seaweedfs.componentName # (see packages/system/seaweedfs/templates/_naming.tpl, which the chart's guard @@ -71,18 +121,51 @@ renamed_volume_prefix() { printf '%s-volume' "$(printf '%s' "$full" | cut -c1-56 | sed 's/-$//')" } -# release_json [rev] -- decode a Helm release secret. -# Helm stores base64(gzip(json)) in Secret.data.release, and Kubernetes base64s -# the data value again, hence the two decodes. +# release_json [rev] -- decode a Helm release secret's payload, or +# "" when that revision's secret does not exist. Helm stores base64(gzip(json)) in +# Secret.data.release, and Kubernetes base64s the data value again, hence the two +# decodes. +# +# This is a by-NAME GET, but it is NOT best-effort: system_releases decides whether +# a release IS SeaweedFS from the result, so a swallowed failure silently drops a +# real tenant -- the false-clean this script exists to prevent. EXISTENCE and +# EXTRACTION are therefore two separate questions, because `--ignore-not-found +# -o jsonpath` cannot tell them apart (it returns empty + exit 0 both for an absent +# Secret AND for a present Secret whose .data.release is missing): +# +# 1. Existence, via `--ignore-not-found -o name`. Empty + exit 0 = the revision +# was pruned by Helm's history limit -- a legitimate absence; return "" and +# let rev1_scheme / first_deployed treat it as "pruned". A real error (RBAC, +# timeout, apiserver down) stays non-zero and run_kubectl makes it fatal. +# 2. The Secret EXISTS, so its payload MUST decode. A helm.sh/release.v1 Secret +# with no .data.release, a payload that is not base64(base64(gzip(...))), or an +# empty JSON after decoding is CORRUPT state, not an absence -- and here that +# is safety-critical, so every such anomaly is audit_fatal, never a silent skip +# (contrast pv_epoch, where an empty field is a benign "no evidence"). +# +# The extraction GET drops --ignore-not-found deliberately: the Secret existed a +# moment ago, so a NotFound now is a mid-audit deletion race, and failing closed on +# it is correct. release_json() { - kubectl get secret -n "$1" "sh.helm.release.v1.$2.v${3:-1}" -o jsonpath='{.data.release}' 2>/dev/null \ - | base64 -d 2>/dev/null | base64 -d 2>/dev/null | gunzip 2>/dev/null + _rj_secret="sh.helm.release.v1.$2.v${3:-1}" + _rj_exists=$(run_kubectl "check Helm release secret '$_rj_secret' in namespace '$1'" \ + get secret -n "$1" "$_rj_secret" --ignore-not-found -o name) || return $? + [ -n "$_rj_exists" ] || return 0 + _rj_field=$(run_kubectl "read .data.release of secret '$_rj_secret' in namespace '$1'" \ + get secret -n "$1" "$_rj_secret" -o jsonpath='{.data.release}') || return $? + [ -n "$_rj_field" ] || { audit_fatal "secret '$_rj_secret' in namespace '$1' exists but has no .data.release payload (corrupt Helm release)."; return 1; } + _rj_json=$(printf '%s' "$_rj_field" | base64 -d 2>/dev/null | base64 -d 2>/dev/null | gunzip 2>/dev/null) \ + || { audit_fatal "secret '$_rj_secret' in namespace '$1': .data.release is not decodable base64(base64(gzip(json))) (corrupt Helm release)."; return 1; } + [ -n "$_rj_json" ] || { audit_fatal "secret '$_rj_secret' in namespace '$1': release payload decoded to nothing (corrupt Helm release)."; return 1; } + printf '%s' "$_rj_json" } # revisions -- retained revision numbers, oldest first. revisions() { - kubectl get secret -n "$1" -l "name=$2,owner=helm" \ - -o jsonpath='{range .items[*]}{.metadata.labels.version}{"\n"}{end}' 2>/dev/null | sort -n + _rev_out=$(run_kubectl "list Helm revision secrets for '$2' in namespace '$1'" \ + get secret -n "$1" -l "name=$2,owner=helm" \ + -o jsonpath='{range .items[*]}{.metadata.labels.version}{"\n"}{end}') || return $? + printf '%s\n' "$_rev_out" | sort -n } # system_releases -- the SeaweedFS -system Helm releases in a namespace. @@ -92,12 +175,29 @@ revisions() { # unrelated release in a namespace that happens to run SeaweedFS would be reported # as a SeaweedFS tenant. Confirm the chart. system_releases() { - for rel in $(kubectl get secret -n "$1" \ - -o jsonpath='{range .items[?(@.type=="helm.sh/release.v1")]}{.metadata.labels.name}{"\n"}{end}' 2>/dev/null \ - | grep -E -- '-system$' | sort -u); do - for rev in $(revisions "$1" "$rel"); do - chart=$(release_json "$1" "$rel" "$rev" | sed -n 's/.*"name":"\(cozy-seaweedfs\)".*/\1/p' | head -1) - if [ -n "$chart" ]; then printf '%s\n' "$rel"; fi + _sr_secrets=$(run_kubectl "list Helm release secrets in namespace '$1'" \ + get secret -n "$1" \ + -o jsonpath='{range .items[?(@.type=="helm.sh/release.v1")]}{.metadata.labels.name}{"\n"}{end}') || return $? + for rel in $(printf '%s\n' "$_sr_secrets" | grep -E -- '-system$' | sort -u); do + _sr_revs=$(revisions "$1" "$rel") || return $? + for rev in $_sr_revs; do + _sr_json=$(release_json "$1" "$rel" "$rev") || return $? + # Empty "" here means the revision secret was pruned (release_json's absence + # path) -- a legitimate skip. A NON-empty payload, however, was fetched and + # decoded successfully, so it MUST name its chart: a helm.sh/release.v1 release + # always carries chart.metadata.name. A payload without one ({} , or any valid + # JSON lacking it) is corrupt/unexpected, and since "no chart name" is + # indistinguishable downstream from "not SeaweedFS", silently skipping it is + # the very false-clean this guard exists to stop -- so it is FATAL. A present + # but different chart name is a real, non-SeaweedFS release and stays a + # legitimate skip, filtered exactly as before by the cozy-seaweedfs test. + # ("chart" is the release struct's only such key -- vendored subcharts nest + # under "dependencies" -- so this match is unambiguous.) + if [ -n "$_sr_json" ]; then + _sr_chart=$(printf '%s' "$_sr_json" | sed -n 's/.*"chart":{"metadata":{"name":"\([^"]*\)".*/\1/p' | head -1) + [ -n "$_sr_chart" ] || { audit_fatal "secret 'sh.helm.release.v1.$rel.v$rev' in namespace '$1' decoded to a payload with no chart name (corrupt Helm release)."; return 1; } + if [ "$_sr_chart" = cozy-seaweedfs ]; then printf '%s\n' "$rel"; fi + fi break done done @@ -106,28 +206,52 @@ system_releases() { # first_deployed -- epoch seconds of the release's first install, # from any retained revision (the field is identical on all of them). first_deployed() { - for rev in $(revisions "$1" "$2"); do - ts=$(release_json "$1" "$2" "$rev" | sed -n 's/.*"first_deployed":"\([^"]*\)".*/\1/p' | head -1) + _fd_revs=$(revisions "$1" "$2") || return $? + for rev in $_fd_revs; do + _fd_json=$(release_json "$1" "$2" "$rev") || return $? + ts=$(printf '%s' "$_fd_json" | sed -n 's/.*"first_deployed":"\([^"]*\)".*/\1/p' | head -1) if [ -n "$ts" ]; then date -u -d "$(printf '%s' "$ts" | cut -c1-19)" +%s 2>/dev/null; return; fi done } # rev1_scheme -- "legacy" | "renamed" | "" (revision 1 pruned). rev1_scheme() { - m=$(release_json "$1" "$2" 1) + m=$(release_json "$1" "$2" 1) || return $? [ -n "$m" ] || return 0 if printf '%s' "$m" | grep -q 'name: seaweedfs-master'; then printf 'legacy' elif printf '%s' "$m" | grep -qE "name: $2(-seaweedfs)?-master"; then printf 'renamed' fi } -# pv_epoch -- creation epoch of the PV the claim is BOUND to. +# pv_epoch -- creation epoch of the PV the claim is BOUND to, or "" when +# there is no usable PV age. Two by-NAME GETs. +# +# A real API error must abort (an epoch silently missing from a range can invert +# the strict-newer comparison and name the WRONG deletion candidate), so both GETs +# go through run_kubectl. But UNLIKE release_json, an EMPTY field here is NOT +# corruption and is NOT fatal -- because the consequence differs. When release_json +# returns empty for a present Secret the tenant is dropped from the report: a +# false-clean. When pv_epoch returns "" the claim merely contributes no epoch, +# which marks its whole generation INCOMPLETE (see audit_ns) and forces the safe +# "direction cannot be established -> classify by hand" branch. That degradation is +# conservative by construction: it never yields a false-clean and never names a +# candidate. So the two empty-field cases are deliberately kept benign: +# * PVC .spec.volumeName empty -- a Pending / unbound claim, a routine state; +# * PV .metadata.creationTimestamp empty -- shouldn't happen (the apiserver always +# stamps it), but if it ever did the only effect is one missing epoch, i.e. the +# same safe incomplete fallback, so a hard stop is not warranted here. +# `--ignore-not-found` keeps a genuinely-absent PVC/PV (NotFound) on that same +# benign path rather than turning it into a run_kubectl failure, and the final +# `|| return 0` keeps an unparseable timestamp there too; only a real run_kubectl +# error propagates. pv_epoch() { - pv=$(kubectl get pvc -n "$1" "$2" -o jsonpath='{.spec.volumeName}' 2>/dev/null) + pv=$(run_kubectl "get PVC '$2' in namespace '$1'" \ + get pvc -n "$1" "$2" --ignore-not-found -o jsonpath='{.spec.volumeName}') || return $? [ -n "$pv" ] || return 0 - t=$(kubectl get pv "$pv" -o jsonpath='{.metadata.creationTimestamp}' 2>/dev/null) + t=$(run_kubectl "get PV '$pv' (bound by PVC '$2' in namespace '$1')" \ + get pv "$pv" --ignore-not-found -o jsonpath='{.metadata.creationTimestamp}') || return $? [ -n "$t" ] || return 0 - date -u -d "$(printf '%s' "$t" | cut -c1-19)" +%s 2>/dev/null + date -u -d "$(printf '%s' "$t" | cut -c1-19)" +%s 2>/dev/null || return 0 } # classify_mixed_direction -- direction of a MIXED @@ -153,17 +277,21 @@ classify_mixed_direction() { audit_ns() { ns="$1" - for rel in $(system_releases "$ns"); do + _rels=$(system_releases "$ns") || return $? + for rel in $_rels; do prefix=$(renamed_volume_prefix "$rel") legacy_pvcs=""; renamed_pvcs="" - for pvc in $(kubectl get pvc -n "$ns" -o name 2>/dev/null | sed 's|persistentvolumeclaim/||'); do + _pvcs=$(run_kubectl "list PVCs in namespace '$ns'" get pvc -n "$ns" -o name) || return $? + for pvc in $(printf '%s\n' "$_pvcs" | sed 's|persistentvolumeclaim/||'); do case "$pvc" in "data1-${prefix}"*) renamed_pvcs="$renamed_pvcs $pvc" ;; data1-seaweedfs-volume*) legacy_pvcs="$legacy_pvcs $pvc" ;; esac done legacy_sts=""; renamed_sts="" - for sts in $(kubectl get sts -n "$ns" -l app.kubernetes.io/name=seaweedfs -o name 2>/dev/null | sed 's|statefulset.apps/||'); do + _sts_list=$(run_kubectl "list SeaweedFS StatefulSets in namespace '$ns'" \ + get sts -n "$ns" -l app.kubernetes.io/name=seaweedfs -o name) || return $? + for sts in $(printf '%s\n' "$_sts_list" | sed 's|statefulset.apps/||'); do case "$sts" in "${prefix}"*) renamed_sts="$renamed_sts $sts" ;; seaweedfs-volume*) legacy_sts="$legacy_sts $sts" ;; @@ -184,21 +312,30 @@ audit_ns() { fi # Both generations. Report which is ORIGINAL from durable evidence. - fd=$(first_deployed "$ns" "$rel") - scheme=$(rev1_scheme "$ns" "$rel") + fd=$(first_deployed "$ns" "$rel") || return $? + scheme=$(rev1_scheme "$ns" "$rel") || return $? printf '%-24s %-14s %-8s %s\n' "$ns" "$rel" "MIXED" "both generations; the chart REFUSES until one is removed" if [ -n "$scheme" ]; then printf '%-24s %-14s %-8s revision 1 was installed with the %s names => the %s generation is ORIGINAL\n' \ "" "" "" "$scheme" "$scheme" fi - lmin=""; lmax=""; rmin=""; rmax="" + # The direction rule is "EVERY PV of one generation strictly newer than every + # PV of the other". That holds only if we saw EVERY bound PV: a claim whose PV + # age we could not read (unbound, or PV gone) leaves the range unbounded on one + # side, and a silently-narrowed range can flip OVERLAP into a confident (wrong) + # candidate. So a generation with any unreadable claim is INCOMPLETE + # and forces the safe "cannot establish" branch -- distinct from a real API + # error, which pv_epoch has already turned into a hard failure above. + lmin=""; lmax=""; rmin=""; rmax=""; l_incomplete=0; r_incomplete=0 for p in $legacy_pvcs; do - e=$(pv_epoch "$ns" "$p"); [ -n "$e" ] || continue + e=$(pv_epoch "$ns" "$p") || return $? + if [ -z "$e" ]; then l_incomplete=1; continue; fi { [ -z "$lmin" ] || [ "$e" -lt "$lmin" ]; } && lmin=$e { [ -z "$lmax" ] || [ "$e" -gt "$lmax" ]; } && lmax=$e done for p in $renamed_pvcs; do - e=$(pv_epoch "$ns" "$p"); [ -n "$e" ] || continue + e=$(pv_epoch "$ns" "$p") || return $? + if [ -z "$e" ]; then r_incomplete=1; continue; fi { [ -z "$rmin" ] || [ "$e" -lt "$rmin" ]; } && rmin=$e { [ -z "$rmax" ] || [ "$e" -gt "$rmax" ]; } && rmax=$e done @@ -206,7 +343,7 @@ audit_ns() { printf '%-24s %-14s %-8s oldest PV vs first_deployed: chart-named +%ss, release-named +%ss (context only, not the rule)\n' \ "" "" "" "$((lmin - fd))" "$((rmin - fd))" fi - if [ -n "$lmin" ] && [ -n "$rmin" ]; then + if [ -n "$lmin" ] && [ -n "$rmin" ] && [ "$l_incomplete" = 0 ] && [ "$r_incomplete" = 0 ]; then case $(classify_mixed_direction "$lmin" "$lmax" "$rmin" "$rmax") in legacy-original) printf '%-24s %-14s %-8s every release-named PV is strictly newer => chart-named is ORIGINAL, the release-named set is the candidate duplicate (Step 3)\n' "" "" "" ;; @@ -216,7 +353,7 @@ audit_ns() { printf '%-24s %-14s %-8s PV vintages OVERLAP => no candidate. An interrupted Step 2 re-bind looks exactly like this (both generations on original PVs). Finish Step 2 if one is in progress; otherwise escalate. Do NOT run Step 2a.\n' "" "" "" ;; esac else - printf '%-24s %-14s %-8s a generation has no bound PVs (Pending claims, or StatefulSets only) => direction cannot be established from PV ages. Resolve the Pending claims or classify by hand.\n' "" "" "" + printf '%-24s %-14s %-8s direction cannot be established from PV ages: a generation has no bound PVs (Pending/unbound claims, or StatefulSets only) or a bound PV age could not be read. Resolve those claims or classify by hand. Do NOT run Step 2a.\n' "" "" "" fi printf '%-24s %-14s %-8s CANDIDATE ONLY: "original" does not mean the other set is EMPTY. A duplicate that\n' "" "" "" printf '%-24s %-14s %-8s served writes and later crashed looks identical here. Verify emptiness before deleting.\n' "" "" "" @@ -227,9 +364,10 @@ main() { printf '%-24s %-14s %-8s %s\n' NAMESPACE RELEASE CLASS NOTE printf '%-24s %-14s %-8s %s\n' --------- ------- ----- ---- if [ "$#" -gt 0 ]; then - for ns in "$@"; do audit_ns "$ns"; done + for ns in "$@"; do audit_ns "$ns" || exit $?; done else - for ns in $(kubectl get ns -o name 2>/dev/null | sed 's|namespace/||'); do audit_ns "$ns"; done + _ns_list=$(run_kubectl 'list namespaces (whole-cluster mode)' get ns -o name) || exit $? + for ns in $(printf '%s\n' "$_ns_list" | sed 's|namespace/||'); do audit_ns "$ns" || exit $?; done fi }