From 6bab68b1326ab3620c4b867c18d3b9403d99956a Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Sun, 14 Jun 2026 14:08:56 +0200 Subject: [PATCH] M19: deriveStackName cross-references deployed stacks (fix DB-container misattribution) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit deriveStackName pure-suffix-stripped on '-' (postgres/db/mariadb/.../cache), so a stack whose slug ENDS in a role token (e.g. 'my-cache') was misattributed (stripped to 'my') — filing its DB dump under the wrong/nonexistent stack. Now threads the set of deployed stack names (m.knownStackNames() <- ListDeployedStacks) into DiscoverDatabases and cross-references: candidate suffix-strip if known, else the container name if it IS a known stack, else longest known stack that is a prefix (handles _postgres / -1), else legacy strip. nil/empty known = legacy behaviour (appexport passes nil). Table test incl. the my-cache case (fails pre-fix). --- controller/internal/appbackup/dbdump.go | 60 +++++++++++++++-- .../appbackup/derivestackname_test.go | 65 +++++++++++++++++++ controller/internal/appexport/export.go | 2 +- .../internal/backup/appbackup_bridge.go | 4 +- controller/internal/backup/backup.go | 20 +++++- controller/internal/backup/restore_db.go | 2 +- 6 files changed, 142 insertions(+), 11 deletions(-) create mode 100644 controller/internal/appbackup/derivestackname_test.go diff --git a/controller/internal/appbackup/dbdump.go b/controller/internal/appbackup/dbdump.go index c2ad46a..015a513 100644 --- a/controller/internal/appbackup/dbdump.go +++ b/controller/internal/appbackup/dbdump.go @@ -64,7 +64,16 @@ type DumpFileInfo struct { } // DiscoverDatabases finds running database containers via docker ps. -func DiscoverDatabases(ctx context.Context, logger *log.Logger, debug bool) ([]DiscoveredDB, error) { +// +// knownStacks is the set of actually-deployed stack names; it is used to attribute each DB container to +// the correct stack (M19). Pass nil/empty for the legacy suffix-strip behaviour. +func DiscoverDatabases(ctx context.Context, logger *log.Logger, debug bool, knownStacks []string) ([]DiscoveredDB, error) { + known := make(map[string]bool, len(knownStacks)) + for _, s := range knownStacks { + if s != "" { + known[s] = true + } + } if debug { logger.Printf("[DEBUG] DiscoverDatabases: running docker ps to find database containers") } @@ -113,7 +122,7 @@ func DiscoverDatabases(ctx context.Context, logger *log.Logger, debug bool) ([]D ContainerID: id, ContainerName: name, DBType: dbType, - StackName: deriveStackName(name), + StackName: deriveStackName(name, known), } // Get env vars from container @@ -640,8 +649,51 @@ func getMariaDBPassword(ctx context.Context, containerID string) string { return "" } -// deriveStackName strips known DB suffixes from container name. -func deriveStackName(containerName string) string { +// deriveStackName maps a DB container name to its owning stack name. +// +// M19: the old logic pure-suffix-stripped on `-` (postgres/db/mariadb/mysql/database/redis/cache), +// which misattributes a stack whose real slug ENDS in a role token (e.g. a stack literally named +// `my-cache` → stripped to `my`). When the set of actually-deployed stack names is known, cross-reference +// it so the result is a real stack: +// - candidate := suffix-strip result. +// - known[candidate] → candidate (a real DB-role suffix of a real stack, e.g. romm-postgres→romm). +// - else known[containerName] → containerName (the container name IS the stack — don't strip, e.g. my-cache). +// - else longest known prefix → handles _postgres / -1 / compose-suffixed names. +// - else → candidate (fall back to today's suffix-strip; preserves behaviour when +// the stack list is empty/unavailable, so nothing regresses). +// A nil/empty `known` map = the legacy fast path (pure suffix-strip). +func deriveStackName(containerName string, known map[string]bool) string { + candidate := suffixStripStackName(containerName) + + if len(known) == 0 { + return candidate + } + if known[candidate] { + return candidate + } + if known[containerName] { + return containerName + } + // Longest known stack name that is a prefix of the container name (tie-break: longest wins). + best := "" + for name := range known { + if name == "" || len(name) >= len(containerName) { + continue + } + // boundary char so "rom" doesn't match "romm-..."; compose/role separators are - or _. + sep := containerName[len(name)] + if strings.HasPrefix(containerName, name) && (sep == '-' || sep == '_') && len(name) > len(best) { + best = name + } + } + if best != "" { + return best + } + return candidate +} + +// suffixStripStackName is the legacy pure suffix-strip (the M19 fallback when no stack list is known). +func suffixStripStackName(containerName string) string { knownSuffixes := []string{"postgres", "db", "mariadb", "mysql", "database", "redis", "cache"} parts := strings.Split(containerName, "-") diff --git a/controller/internal/appbackup/derivestackname_test.go b/controller/internal/appbackup/derivestackname_test.go new file mode 100644 index 0000000..0553092 --- /dev/null +++ b/controller/internal/appbackup/derivestackname_test.go @@ -0,0 +1,65 @@ +package appbackup + +import "testing" + +// TestDeriveStackName_KnownCrossRef asserts M19: deriveStackName cross-references the deployed-stack set +// so a stack whose slug ends in a DB-role token (e.g. `my-cache`) is NOT misattributed by pure +// suffix-stripping. The `my-cache` case fails on the pre-fix code (which stripped it to `my`). +func TestDeriveStackName_KnownCrossRef(t *testing.T) { + known := map[string]bool{"romm": true, "my-cache": true, "paperless-ngx": true} + + cases := []struct { + container string + want string + note string + }{ + {"romm-postgres", "romm", "role suffix of a real stack → strip"}, + {"my-cache", "my-cache", "container name IS a real stack → do NOT strip to 'my'"}, // pre-fix: "my" + {"my-cache-postgres", "my-cache", "strip role, result is a known stack"}, // pre-fix: "my-cache" (ok) — but via prefix here + {"paperless-ngx-postgres", "paperless-ngx", "multi-hyphen stack, role suffix"}, + {"romm_postgres", "romm", "underscore-separated compose name → longest known prefix"}, + {"romm-1", "romm", "compose numeric suffix → longest known prefix"}, + } + for _, c := range cases { + if got := deriveStackName(c.container, known); got != c.want { + t.Errorf("deriveStackName(%q, known) = %q, want %q (%s)", c.container, got, c.want, c.note) + } + } +} + +// TestDeriveStackName_LegacyFallback asserts the nil/empty-known fast path preserves the old behaviour +// (pure suffix-strip), so callers without a stack list (e.g. appexport) don't regress. +func TestDeriveStackName_LegacyFallback(t *testing.T) { + cases := []struct { + container string + want string + }{ + {"romm-postgres", "romm"}, + {"paperless-ngx-postgres", "paperless-ngx"}, + {"my-cache", "my"}, // legacy strips the role-token suffix + {"unknown-db", "unknown"}, // legacy strip + {"standalone", "standalone"}, + } + for _, c := range cases { + if got := deriveStackName(c.container, nil); got != c.want { + t.Errorf("deriveStackName(%q, nil) = %q, want %q (legacy)", c.container, got, c.want) + } + // empty (non-nil) map must behave identically to nil + if got := deriveStackName(c.container, map[string]bool{}); got != c.want { + t.Errorf("deriveStackName(%q, {}) = %q, want %q (legacy)", c.container, got, c.want) + } + } +} + +// TestDeriveStackName_UnknownContainerFallsBack asserts that when the container matches no known stack at +// all, the result falls back to the suffix-strip candidate (no spurious prefix match). +func TestDeriveStackName_UnknownContainerFallsBack(t *testing.T) { + known := map[string]bool{"romm": true} + if got := deriveStackName("grafana-db", known); got != "grafana" { + t.Fatalf("deriveStackName(grafana-db, {romm}) = %q, want grafana (fallback strip)", got) + } + // must NOT match "romm" as a prefix of an unrelated name + if got := deriveStackName("rommother-db", known); got != "rommother" { + t.Fatalf("deriveStackName(rommother-db) = %q, want rommother (no false prefix match)", got) + } +} diff --git a/controller/internal/appexport/export.go b/controller/internal/appexport/export.go index 481f011..e3b6da6 100644 --- a/controller/internal/appexport/export.go +++ b/controller/internal/appexport/export.go @@ -510,7 +510,7 @@ func (e *Exporter) dumpDatabase(stackName, dbDir string, manifest *Manifest) boo defer cancel() e.debugf("discovering databases (looking for stack %s)...", stackName) - dbs, err := appbackup.DiscoverDatabases(ctx, e.logger, e.debug) + dbs, err := appbackup.DiscoverDatabases(ctx, e.logger, e.debug, nil) if err != nil { e.logger.Printf("[WARN] Export: DB discovery error: %v", err) return false diff --git a/controller/internal/backup/appbackup_bridge.go b/controller/internal/backup/appbackup_bridge.go index a919b83..45bc35f 100644 --- a/controller/internal/backup/appbackup_bridge.go +++ b/controller/internal/backup/appbackup_bridge.go @@ -47,8 +47,8 @@ const FelhomDataDir = appbackup.FelhomDataDir // --- function forwarders (dbdump) --- -func DiscoverDatabases(ctx context.Context, logger *log.Logger, debug bool) ([]DiscoveredDB, error) { - return appbackup.DiscoverDatabases(ctx, logger, debug) +func DiscoverDatabases(ctx context.Context, logger *log.Logger, debug bool, knownStacks []string) ([]DiscoveredDB, error) { + return appbackup.DiscoverDatabases(ctx, logger, debug, knownStacks) } func DumpAll(ctx context.Context, dbs []DiscoveredDB, dumpDir string, logger *log.Logger, debug bool) []DumpResult { diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index a491918..d266818 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -122,6 +122,20 @@ func (m *Manager) AppNamespaceRoot(stackName string) string { return m.namespaceRoot(drivePath) } +// knownStackNames returns the names of all deployed stacks, for M19 DB-container→stack attribution. +// Empty when no provider is wired (DiscoverDatabases then falls back to legacy suffix-strip). +func (m *Manager) knownStackNames() []string { + if m.stackProvider == nil { + return nil + } + stacks := m.stackProvider.ListDeployedStacks() + names := make([]string, 0, len(stacks)) + for _, s := range stacks { + names = append(names, s.Name) + } + return names +} + // groupStacksByDrive groups deployed stacks by their home drive path. func (m *Manager) groupStacksByDrive() map[string][]StackSummary { if m.stackProvider == nil { @@ -158,7 +172,7 @@ func (m *Manager) runDBDumpsInternal(ctx context.Context) error { start := time.Now() m.logger.Printf("[INFO] [backup] Starting database dump run") - dbs, err := DiscoverDatabases(ctx, m.logger, m.isDebug()) + dbs, err := DiscoverDatabases(ctx, m.logger, m.isDebug(), m.knownStackNames()) if err != nil { m.logger.Printf("[ERROR] [backup] Database discovery failed: %v", err) return err @@ -415,7 +429,7 @@ func (m *Manager) GetStackHDDMounts(name string) []string { // DumpStackDB runs a database dump for containers belonging to a specific stack. // Dumps to the stack's home drive: /backups/primary//db-dumps/. func (m *Manager) DumpStackDB(ctx context.Context, stackName string) error { - dbs, err := DiscoverDatabases(ctx, m.logger, m.isDebug()) + dbs, err := DiscoverDatabases(ctx, m.logger, m.isDebug(), m.knownStackNames()) if err != nil { return fmt.Errorf("database discovery failed: %w", err) } @@ -492,7 +506,7 @@ func (m *Manager) RefreshCache(nextDBDump time.Time) { ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) defer cancel() - if dbs, err := DiscoverDatabases(ctx, m.logger, m.isDebug()); err == nil { + if dbs, err := DiscoverDatabases(ctx, m.logger, m.isDebug(), m.knownStackNames()); err == nil { status.DiscoveredDBs = dbs } diff --git a/controller/internal/backup/restore_db.go b/controller/internal/backup/restore_db.go index 6d8ad06..7e3473d 100644 --- a/controller/internal/backup/restore_db.go +++ b/controller/internal/backup/restore_db.go @@ -41,7 +41,7 @@ func (m *Manager) reimportDBDumps(ctx context.Context, stackName, nsRoot string) discover := m.discoverDBs if discover == nil { discover = func(ctx context.Context) ([]DiscoveredDB, error) { - return DiscoverDatabases(ctx, m.logger, m.isDebug()) + return DiscoverDatabases(ctx, m.logger, m.isDebug(), m.knownStackNames()) } } imp := m.importDBDump