@@ -138,3 +138,372 @@ EOF
138138 done < " $chart /rendered"
139139 rm -rf " $chart "
140140}
141+
142+ # -----------------------------------------------------------------------------
143+ # Fail-closed tests (issue #3431).
144+ #
145+ # The classification tests above mock nothing below the classifier. These drive
146+ # the KUBECTL layer, where the fail-open bug lived: a kubectl call that failed
147+ # used to return empty stdout, byte-identical to a genuinely clean fleet, so the
148+ # audit printed an empty table and exited 0 -- the exact false "nothing to do" the
149+ # operator is told to trust before deleting PVCs. They shim `kubectl` on PATH with
150+ # a fake and assert the audit fails LOUDLY (non-zero exit + a FATAL naming the
151+ # query) on any real error, while still treating a genuinely-absent object as
152+ # clean. Two families:
153+ #
154+ # * enumerating LISTs (get ns / secret / pvc / sts) -- a failure is always fatal;
155+ # * by-NAME GETs (a release secret in system_releases, a PVC/PV in pv_epoch) --
156+ # `--ignore-not-found` splits a real error (fatal) from a legitimate absence:
157+ # an absent release revision is a clean skip; an absent PV degrades to the
158+ # safe "cannot establish direction" note and must NOT become a deletion
159+ # candidate (the range-narrowing flip Codex reproduced).
160+ #
161+ # The two golden tests guard the reverse: on a healthy cluster the output stays
162+ # byte-for-byte what it was on origin/main, including the MIXED path that exercises
163+ # every pv_epoch GET.
164+ #
165+ # The fake is a real executable on PATH, so it exercises the actual exit-status
166+ # handling in run_kubectl and the propagation up through every caller -- a for
167+ # loop over `$(...)` swallows the status, so this is where a regression would hide.
168+
169+ # _release_blob [chart-name] -- the value kubectl returns for a Helm release
170+ # secret's `.data.release`: base64(base64(gzip(json))). release_json base64-decodes
171+ # twice and gunzips it, and system_releases reads the chart name from
172+ # chart.metadata.name (as real Helm payloads carry it). Defaults to a cozy-seaweedfs
173+ # release so the audit confirms the tenant; pass another chart name for a non-
174+ # SeaweedFS release, or the literal EMPTY for a chartless {} payload.
175+ _release_blob () {
176+ _rb_name=${1:- cozy-seaweedfs}
177+ if [ " $_rb_name " = EMPTY ]; then
178+ _rb_json=' {}'
179+ else
180+ _rb_json=' {"name":"seaweedfs-system","chart":{"metadata":{"name":"' " $_rb_name " ' "}}}'
181+ fi
182+ printf ' %s' " $_rb_json " | gzip | base64 | base64 | tr -d ' \n'
183+ }
184+
185+ # _write_fake_kubectl <dir> <fail> <absent-pv> <blob> <pvcs> -- drop a fake kubectl
186+ # into <dir>.
187+ # <fail> one of ns | secretlist | secretget | secretget_absent |
188+ # secret_missing_field | secret_bad_payload | pvclist | pvcget |
189+ # sts | pvget | "" -- the single call to make behave badly. Real-
190+ # error targets exit non-zero with a stderr message; secretget_absent
191+ # mirrors an absent Secret (--ignore-not-found: exit 0, empty -o
192+ # name); secret_missing_field / secret_bad_payload keep the Secret
193+ # present but return an empty / undecodable .data.release payload.
194+ # <absent-pv> a PV name (pv-legacy|pv-legacy2|pv-renamed) to report as absent
195+ # (exit 0, empty), exercising the "bound claim, PV gone" path.
196+ # <pvcs> newline-separated `get pvc -o name` LIST output.
197+ # It otherwise walks one confirmed seaweedfs-system tenant. The by-name PVC->PV and
198+ # PV->timestamp maps are fixed here; timestamps are chosen so the two legacy PVs
199+ # straddle the single renamed PV (true answer OVERLAP), which is what makes the
200+ # range-narrowing flip observable. Kept POSIX and free of a column-0 `}` so
201+ # cozytest.sh's awk converter passes the heredoc through untouched.
202+ _write_fake_kubectl () {
203+ # Grouped redirect (one open of the file). The closing brace is indented, so
204+ # cozytest.sh's awk -- which only rewrites a `}` in column 0 -- leaves it and the
205+ # heredoc alone.
206+ {
207+ printf ' #!/bin/sh\n'
208+ printf " FAIL='%s'\n" " $2 "
209+ printf " ABSENT_PV='%s'\n" " $3 "
210+ printf " BLOB='%s'\n" " $4 "
211+ printf " PVCS='%s'\n" " $5 "
212+ cat << 'FAKE '
213+ verb=${1:-}; res=${2:-}; args="$*"
214+ fail() { echo "fake kubectl: $1 (real error)" >&2; exit 1; }
215+ if [ "$verb $res" = "get ns" ]; then
216+ [ "$FAIL" = ns ] && fail "get ns"
217+ printf 'namespace/tenant-test\n'; exit 0
218+ fi
219+ if [ "$verb $res" = "get secret" ]; then
220+ case "$args" in
221+ *sh.helm.release.v1*)
222+ # release_json now asks two questions: existence (-o name) then payload
223+ # (jsonpath .data.release). Mirror that split so absence, real error, and
224+ # corrupt-payload are all reachable independently.
225+ case "$args" in
226+ *"-o name"*)
227+ [ "$FAIL" = secretget ] && fail "release secret existence GET"
228+ [ "$FAIL" = secretget_absent ] && exit 0
229+ printf 'secret/sh.helm.release.v1.seaweedfs-system.v1\n'; exit 0 ;;
230+ *)
231+ [ "$FAIL" = secret_missing_field ] && exit 0
232+ if [ "$FAIL" = secret_bad_payload ]; then printf '@@@not-base64@@@\n'; exit 0; fi
233+ printf '%s\n' "$BLOB"; exit 0 ;;
234+ esac ;;
235+ *owner=helm*) printf '1\n'; exit 0 ;;
236+ *) [ "$FAIL" = secretlist ] && fail "namespace secret LIST"
237+ printf 'seaweedfs-system\n'; exit 0 ;;
238+ esac
239+ fi
240+ if [ "$verb $res" = "get pvc" ]; then
241+ case "$args" in
242+ *"-o name"*)
243+ [ "$FAIL" = pvclist ] && fail "pvc LIST"
244+ [ -n "$PVCS" ] && printf '%s\n' "$PVCS"
245+ exit 0 ;;
246+ *data1-seaweedfs-system-volume-0*) [ "$FAIL" = pvcget ] && fail "pvc GET"; printf 'pv-renamed\n'; exit 0 ;;
247+ *data1-seaweedfs-volume-1*) [ "$FAIL" = pvcget ] && fail "pvc GET"; printf 'pv-legacy2\n'; exit 0 ;;
248+ *data1-seaweedfs-volume-0*) [ "$FAIL" = pvcget ] && fail "pvc GET"; printf 'pv-legacy\n'; exit 0 ;;
249+ esac
250+ exit 0
251+ fi
252+ if [ "$verb $res" = "get sts" ]; then
253+ [ "$FAIL" = sts ] && fail "sts LIST"
254+ exit 0
255+ fi
256+ if [ "$verb $res" = "get pv" ]; then
257+ [ "$FAIL" = pvget ] && fail "get pv"
258+ case "$args" in
259+ *pv-legacy2*) [ "$ABSENT_PV" = pv-legacy2 ] && exit 0; printf '2099-01-01T00:00:00Z\n'; exit 0 ;;
260+ *pv-legacy*) [ "$ABSENT_PV" = pv-legacy ] && exit 0; printf '2020-01-01T00:00:00Z\n'; exit 0 ;;
261+ *pv-renamed*) [ "$ABSENT_PV" = pv-renamed ] && exit 0; printf '2020-06-01T00:00:00Z\n'; exit 0 ;;
262+ esac
263+ exit 0
264+ fi
265+ exit 0
266+ FAKE
267+ } > " $1 /kubectl"
268+ chmod +x " $1 /kubectl"
269+ }
270+
271+ # _pvcs_mixed -- the `get pvc -o name` LIST for a MIXED tenant: two legacy claims
272+ # and one release-named claim, whose PV vintages truly overlap.
273+ _pvcs_mixed () {
274+ printf ' %s\n'
275+ persistentvolumeclaim/data1-seaweedfs-volume-0 \
276+ persistentvolumeclaim/data1-seaweedfs-volume-1 \
277+ persistentvolumeclaim/data1-seaweedfs-system-volume-0
278+ }
279+
280+ # _expected_L / _expected_mixed_overlap -- golden output built with the SAME printf
281+ # contract the script uses, so a drift in row count, text, or padding fails the diff.
282+ _expected_L () {
283+ printf ' %-24s %-14s %-8s %s\n'
284+ printf ' %-24s %-14s %-8s %s\n'
285+ printf ' %-24s %-14s %-8s %s\n' ' chart-named only; the upgrade adopts it, nothing to do'
286+ }
287+ _expected_mixed_overlap () {
288+ printf ' %-24s %-14s %-8s %s\n'
289+ printf ' %-24s %-14s %-8s %s\n'
290+ printf ' %-24s %-14s %-8s %s\n' ' both generations; the chart REFUSES until one is removed'
291+ 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.'
292+ printf ' %-24s %-14s %-8s %s\n' ' ' ' ' ' ' ' CANDIDATE ONLY: "original" does not mean the other set is EMPTY. A duplicate that'
293+ printf ' %-24s %-14s %-8s %s\n' ' ' ' ' ' ' ' served writes and later crashed looks identical here. Verify emptiness before deleting.'
294+ }
295+
296+ @test " fails closed when 'kubectl get ns' fails (whole-cluster mode)"
297+ # No namespace args -> main enumerates namespaces itself. If that LIST fails,
298+ # the OLD code audited zero namespaces and still printed an empty clean table.
299+ d=$( mktemp -d)
300+ _write_fake_kubectl " $d " " " " $( _release_blob) " " "
301+ rc=0
302+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
303+ rm -rf " $d "
304+ echo " rc=$rc " ; echo " $out "
305+ [ " $rc " -ne 0 ]
306+ printf ' %s\n' " $out " | grep -q ' FATAL'
307+ printf ' %s\n' " $out " | grep -q ' list namespaces'
308+ }
309+
310+ @test " fails closed when the namespace-wide secret LIST fails"
311+ # system_releases enumerates Helm releases with a namespace-wide secret LIST --
312+ # the exact call that timed out in the field and reported a false clean.
313+ d=$( mktemp -d)
314+ _write_fake_kubectl " $d " " " " $( _release_blob) " " "
315+ rc=0
316+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
317+ rm -rf " $d "
318+ echo " rc=$rc " ; echo " $out "
319+ [ " $rc " -ne 0 ]
320+ printf ' %s\n' " $out " | grep -q ' FATAL'
321+ printf ' %s\n' " $out " | grep -q ' list Helm release secrets'
322+ }
323+
324+ @test " fails closed when the by-name release-secret existence GET hits a real error"
325+ # Codex Finding 1: system_releases uses release_json to decide whether a release
326+ # IS SeaweedFS. Both LISTs succeed, then a transient/forbidden by-name Secret GET
327+ # used to leave `chart` empty -> the tenant was silently skipped -> exit 0, empty
328+ # table. A real error here must now be fatal, not a false clean.
329+ d=$( mktemp -d)
330+ _write_fake_kubectl " $d " " " " $( _release_blob) " " "
331+ rc=0
332+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
333+ rm -rf " $d "
334+ echo " rc=$rc " ; echo " $out "
335+ [ " $rc " -ne 0 ]
336+ printf ' %s\n' " $out " | grep -q ' FATAL'
337+ printf ' %s\n' " $out " | grep -q ' Helm release secret'
338+ }
339+
340+ @test " an absent release-secret revision is a clean skip, not a failure"
341+ # The one legitimately-empty case for release_json: the revision secret does not
342+ # exist (pruned by Helm history limit). The existence check (--ignore-not-found -o
343+ # name) returns empty + exit 0, so the release is simply not confirmed as
344+ # SeaweedFS. No error, no crash -- the audit completes and reports nothing here.
345+ d=$( mktemp -d)
346+ _write_fake_kubectl " $d " " " " $( _release_blob) " " "
347+ rc=0
348+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
349+ rm -rf " $d "
350+ echo " rc=$rc " ; echo " $out "
351+ [ " $rc " -eq 0 ]
352+ [ " $( printf ' %s\n' " $out " | grep -c ' FATAL' ) " -eq 0 ]
353+ }
354+
355+ @test " fails closed when a present release secret has no .data.release payload"
356+ # Codex re-review round 2: the existence check passes (Secret EXISTS), but its
357+ # .data.release field is empty/missing. That is CORRUPT state, not an absence --
358+ # and because the earlier fix used --ignore-not-found -o jsonpath, which returns
359+ # empty+0 for BOTH, it used to read as a clean skip. It must be a loud stop.
360+ d=$( mktemp -d)
361+ _write_fake_kubectl " $d " " " " $( _release_blob) " " "
362+ rc=0
363+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
364+ rm -rf " $d "
365+ echo " rc=$rc " ; echo " $out "
366+ [ " $rc " -ne 0 ]
367+ printf ' %s\n' " $out " | grep -q ' FATAL'
368+ printf ' %s\n' " $out " | grep -q ' no .data.release payload'
369+ }
370+
371+ @test " fails closed when a present release secret has an undecodable payload"
372+ # Existence passes, .data.release is non-empty but is not base64(base64(gzip(...))),
373+ # so every decode fails. The old `|| return 0` converted that decode failure into
374+ # a clean 0 return -> silent skip. A corrupt payload must be fatal.
375+ d=$( mktemp -d)
376+ _write_fake_kubectl " $d " " " " $( _release_blob) " " "
377+ rc=0
378+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
379+ rm -rf " $d "
380+ echo " rc=$rc " ; echo " $out "
381+ [ " $rc " -ne 0 ]
382+ printf ' %s\n' " $out " | grep -q ' FATAL'
383+ printf ' %s\n' " $out " | grep -q ' not decodable'
384+ }
385+
386+ @test " fails closed when a present release secret decodes to a chartless payload"
387+ # Codex round 3: the payload fetches and DECODES cleanly (valid JSON {}), so the
388+ # decode guards all pass -- but it carries no chart name. system_releases then
389+ # extracted chart='' and silently skipped the tenant -> exit 0, clean table. A
390+ # decoded helm.sh/release.v1 release without a chart name is corrupt: fatal.
391+ d=$( mktemp -d)
392+ _write_fake_kubectl " $d " " " " " " $( _release_blob EMPTY) " " "
393+ rc=0
394+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
395+ rm -rf " $d "
396+ echo " rc=$rc " ; echo " $out "
397+ [ " $rc " -ne 0 ]
398+ printf ' %s\n' " $out " | grep -q ' FATAL'
399+ printf ' %s\n' " $out " | grep -q ' no chart name'
400+ }
401+
402+ @test " a present release secret for a non-SeaweedFS chart is a silent legitimate skip"
403+ # The counterpart the guard must NOT break: a real, well-formed release whose
404+ # chart is simply not cozy-seaweedfs. It has a chart name (so it is not corrupt),
405+ # but the wrong one, so it is filtered out exactly as before -- no row, no error.
406+ d=$( mktemp -d)
407+ _write_fake_kubectl " $d " " " " " " $( _release_blob cozy-postgres) " " "
408+ rc=0
409+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
410+ rm -rf " $d "
411+ echo " rc=$rc " ; echo " $out "
412+ [ " $rc " -eq 0 ]
413+ [ " $( printf ' %s\n' " $out " | grep -c ' FATAL' ) " -eq 0 ]
414+ # Not confirmed as SeaweedFS -> no classification row for the release.
415+ [ " $( printf ' %s\n' " $out " | grep -c ' seaweedfs-system' ) " -eq 0 ]
416+ }
417+
418+ @test " fails closed when 'kubectl get pvc' LIST fails"
419+ # The secret enumeration succeeds and confirms a seaweedfs-system tenant, so the
420+ # audit reaches the per-namespace PVC LIST; a failure there must abort, not read
421+ # as "this tenant has no claims".
422+ d=$( mktemp -d)
423+ _write_fake_kubectl " $d " " " " $( _release_blob) " " "
424+ rc=0
425+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
426+ rm -rf " $d "
427+ echo " rc=$rc " ; echo " $out "
428+ [ " $rc " -ne 0 ]
429+ printf ' %s\n' " $out " | grep -q ' FATAL'
430+ printf ' %s\n' " $out " | grep -q ' list PVCs'
431+ }
432+
433+ @test " fails closed when 'kubectl get sts' LIST fails"
434+ # PVC LIST succeeds (empty), so the audit reaches the StatefulSet LIST; a
435+ # failure there must abort rather than read as "no StatefulSets".
436+ d=$( mktemp -d)
437+ _write_fake_kubectl " $d " " " " $( _release_blob) " " "
438+ rc=0
439+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
440+ rm -rf " $d "
441+ echo " rc=$rc " ; echo " $out "
442+ [ " $rc " -ne 0 ]
443+ printf ' %s\n' " $out " | grep -q ' FATAL'
444+ printf ' %s\n' " $out " | grep -q ' StatefulSets'
445+ }
446+
447+ @test " fails closed when the by-name PV GET hits a real error"
448+ # Codex Finding 2, error half: pv_epoch reads each bound PV's age by name. A real
449+ # error (RBAC/timeout) must abort, not silently drop that PV from the range.
450+ d=$( mktemp -d)
451+ _write_fake_kubectl " $d " " " " $( _release_blob) " " $( _pvcs_mixed) "
452+ rc=0
453+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
454+ rm -rf " $d "
455+ echo " rc=$rc " ; echo " $out "
456+ [ " $rc " -ne 0 ]
457+ printf ' %s\n' " $out " | grep -q ' FATAL'
458+ printf ' %s\n' " $out " | grep -q " get PV '"
459+ }
460+
461+ @test " an absent PV degrades to the safe fallback, never a wrong candidate"
462+ # Codex Finding 2, absence half. True vintages: legacy PVs 2020 + 2099 straddle
463+ # the single renamed PV 2020-06 => OVERLAP. Report the newest legacy PV (2099) as
464+ # ABSENT: the observed legacy range collapses to {2020} and, unguarded, "every
465+ # release-named PV is strictly newer" would fire -- naming the release-named set a
466+ # deletion candidate on incomplete evidence. The generation must instead read as
467+ # incomplete and fall to "cannot establish direction".
468+ d=$( mktemp -d)
469+ _write_fake_kubectl " $d " " " " $( _release_blob) " " $( _pvcs_mixed) "
470+ rc=0
471+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
472+ rm -rf " $d "
473+ echo " rc=$rc " ; echo " $out "
474+ [ " $rc " -eq 0 ]
475+ [ " $( printf ' %s\n' " $out " | grep -c ' FATAL' ) " -eq 0 ]
476+ printf ' %s\n' " $out " | grep -q ' direction cannot be established from PV ages'
477+ # The whole point: incomplete evidence must NOT be reported as a deletion candidate.
478+ [ " $( printf ' %s\n' " $out " | grep -c ' candidate duplicate' ) " -eq 0 ]
479+ }
480+
481+ @test " success path (L) is byte-identical to the expected fixture"
482+ # A single chart-named claim, no release-named one, no StatefulSets => L, adopt in
483+ # place. Full-output compare, so an extra row / warning / duplicate line fails.
484+ d=$( mktemp -d)
485+ _write_fake_kubectl " $d " " " " " " $( _release_blob) " " persistentvolumeclaim/data1-seaweedfs-volume-0"
486+ rc=0
487+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
488+ printf ' %s\n' " $out " > " $d /got"
489+ _expected_L > " $d /want"
490+ echo " rc=$rc " ; echo " --- got ---" ; cat " $d /got" ; echo " --- want ---" ; cat " $d /want"
491+ [ " $rc " -eq 0 ]
492+ diff " $d /want" " $d /got"
493+ rm -rf " $d "
494+ }
495+
496+ @test " success path (MIXED/overlap) is byte-identical and exercises pv_epoch"
497+ # Both generations present with all PVs readable => the MIXED path runs every
498+ # pv_epoch GET and lands on OVERLAP. Full-output compare against the golden.
499+ d=$( mktemp -d)
500+ _write_fake_kubectl " $d " " " " " " $( _release_blob) " " $( _pvcs_mixed) "
501+ rc=0
502+ out=$( PATH=" $d :$PATH " " $PWD /hack/seaweedfs-naming-audit.sh" 2>&1 ) || rc=$?
503+ printf ' %s\n' " $out " > " $d /got"
504+ _expected_mixed_overlap > " $d /want"
505+ echo " rc=$rc " ; echo " --- got ---" ; cat " $d /got" ; echo " --- want ---" ; cat " $d /want"
506+ [ " $rc " -eq 0 ]
507+ diff " $d /want" " $d /got"
508+ rm -rf " $d "
509+ }
0 commit comments