M18 (dump re-validation perf) @ f8afe5c and M19 (deriveStackName misattribution) @ 6bab68b implemented trunk-based on controller main with regression tests + deployed. Notes retained for provenance.
3.6 KiB
STATUS: FIXED in controller v0.62.0 @
6bab68b(2026-06-14) — implemented trunk-based onmainper the plan below, with a regression test. This note is retained for provenance.
fix/m19-stackname-crossref — NOTES (pending review, NOT deployed)
Verdict: LIVE @ controller/internal/appbackup/dbdump.go:536-551, used at :115 (commit 6953899).
Class: correctness edge — low real-world incidence with the current catalog. Escape-hatch branch
— the clean fix injects the deployed-stack list into appbackup (cross-package), not forced unattended.
The bug (verified mechanism)
deriveStackName(containerName) pure-suffix-strips: it splits on - and, if the last segment is in
{postgres,db,mariadb,mysql,database,redis,cache}, returns the join of the remaining parts. It never
cross-references actual deployed stack names. DiscoverDatabases assigns StackName: deriveStackName(name)
directly (line 115).
So a stack whose real name ends in one of those tokens is misattributed:
- a stack literally named
my-cache→ its DB containermy-cache(ormy-cache-postgres) derives tomy/my-cache, attributing the dump to the wrong (or a non-existent) stack. - worse,
a-dbandacould collide.
Impact
A DB dump is filed under the wrong stack name → that stack's backup/restore accounting is wrong, and the
restore-by-stack path could miss or cross-wire the dump. Incidence is effectively zero in the current
felhom catalog (stack slugs are romm, nextcloud, paperless-ngx, immich, adventurelog,
actualbudget, mealie, vikunja, … — none ends in a DB-role token; DB containers are <stack>-postgres
etc., which strip correctly). It becomes real only if a future app slug ends in a role token.
Fix plan (implementable)
- Thread the set of known deployed stack names into discovery:
DiscoverDatabases(ctx, logger, debug, knownStacks []string)andderiveStackName(containerName string, known map[string]bool). Source the list from the caller inbackup.go(it holds theStackDataProvider— expose/known stack names via a lookup func to avoid importingstacksintoappbackupand creating a cycle). - New
deriveStackNamelogic:- candidate := current suffix-strip result.
- if
known[candidate]→ use it (the suffix was a real DB-role suffix of a real stack). - else if
known[containerName]→ the container name IS the stack (don't strip). - else → longest
knownstack name that is a prefix ofcontainerName(handles<stack>_postgres,<stack>-1, compose-suffixed names); tie-break to the longest match. - else → fall back to the current suffix-strip (preserve today's behaviour when the stack list is unavailable/empty, so nothing regresses).
- Keep a nil/empty-
knownfast path = today's behaviour (back-compat for other callers/tests).
Test plan (regression)
- Table test for
deriveStackNamewithknown = {romm, my-cache}:romm-postgres→romm(suffix is a role;rommis known).my-cache→my-cache(known as-is; must NOT strip tomy). ← fails on pre-fix code.my-cache-postgres→my-cache(strip role, result known).unknown-dbwith no matching known → falls back tounknown(today's behaviour).
Why not tonight
Requires plumbing the deployed-stack list across the backup→appbackup boundary (cycle-avoidance via a
lookup func) and touching DiscoverDatabases's signature + caller. Low-incidence, so not worth a risky
unattended change. The plan + tests above are complete for the supervised session.