Skip to content

[Backport release-1.5] fix(seaweedfs): make naming audit fail closed on kubectl and payload errors - #3878

Merged
myasnikovdaniil merged 2 commits into
release-1.5from
backport-3436-to-release-1.5
Aug 19, 2026
Merged

[Backport release-1.5] fix(seaweedfs): make naming audit fail closed on kubectl and payload errors#3878
myasnikovdaniil merged 2 commits into
release-1.5from
backport-3436-to-release-1.5

Conversation

@github-actions

Copy link
Copy Markdown

Description

Backport of #3436 to release-1.5.

…errors

hack/seaweedfs-naming-audit.sh was fail-open: any kubectl failure
produced an empty table indistinguishable from an honestly clean fleet
(namespace LIST failure = zero namespaces walked, secret/pvc/sts LIST
failures = zero findings, all under 2>/dev/null with no error handling).
The runbook uses this output as the gate before PVC deletion, as
post-deletion verification, and as an upgrade precondition, so a
transient API error could green-light destroying data (#3431).

Route every kubectl call through a run_kubectl helper: non-zero exit
prints a FATAL line naming the failed query and propagates the code up
the whole chain (enumerations restructured to capture-then-check, since
an exit inside $(...) dies with the subshell). By-name GETs distinguish
legitimate absence from real errors via --ignore-not-found on a
separate existence check; a Secret that exists but has no decodable
release payload, or one that decodes without a chart name, is corrupt
state and fails loudly instead of silently dropping the tenant.
Incomplete PV-age evidence now marks the generation incomplete and
falls to the safe no-direction branch, so a failed GET can no longer
flip OVERLAP into a wrong Step-3 deletion candidate. A successful run
with zero findings still prints the same bytes as before.

Tests: 14 new cozytest cases — failure injection for every LIST and
by-name GET site, corrupt-payload variants, and full-output golden
diffs for the success paths (verified byte-identical to the pre-change
script). 25/25 green.

Fixes #3431

Assisted-By: Claude <noreply@anthropic.com>
Assisted-By: GPT-5 <noreply@openai.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit 3dc6cbd)
…odelled calls

Review findings from #3436, plus what an adversarial re-check of the first
attempt at them turned up.

The chart-name extraction was a greedy sed, i.e. LAST match. Helm marshals
"config" (the release`s values) after "chart", so a values subtree that spells
chart.metadata.name shadowed the real chart name: the release then read as
non-SeaweedFS, the tenant silently vanished from the report, and the script
exited 0 -- the precise false clean this PR exists to prevent, reachable
without any corruption at all. It now takes the FIRST match. The path stays
adjacent on purpose: a looser "any name after metadata" matches
chart.templates[].name, which Helm emits immediately after metadata on every
healthy release, so the review`s suggested relaxation would have returned a
template path for every tenant. Over-strictness fails loudly and recoverably;
over-looseness deletes data quietly.

Newlines are now folded before matching, so a pretty-printed payload parses at
all rather than aborting, and whitespace around the punctuation is tolerated.
first_deployed got the same treatment -- it had the identical greedy, single-
line-only shape, and a spaced payload silently dropped the PV-vintage row from
the report.

The FATAL message names the path it read and the shape it expected, so a future
Helm format change is diagnosable instead of looking like real corruption.

Tests: three new payload shapes (spaced, pretty multi-line, and a values decoy)
each byte-compared against the same golden as the compact payload, so the shape
must make no difference to the report. Mutation-checked -- restoring last-match
extraction fails the decoy test on its own, restoring the single-line matcher
fails the pretty test.

The test fake answered any unmodelled kubectl invocation with exit 0 and empty
stdout, the fail-open shape this script was rewritten to reject, and enough to
let a newly added query pass the goldens unnoticed. Unmodelled calls now exit 97
naming the invocation. Verified inert first by instrumenting the fake: 88
invocations across the suite, none unmodelled. Also added the missing test for
the by-name PVC GET fatal path, whose FAIL mode existed with nothing behind it.

The runbook now tells the operator to read the exit code: a non-zero exit means
the table is incomplete and no step may be taken on it. That contract lived only
in the script header, while the runbook is what the operator follows.

29/29 tests green; sh -n clean.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Myasnikov Daniil <myasnikovdaniil2001@gmail.com>
(cherry picked from commit e02adad)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant