From c732006d263a25744eca11a11cdeef343b1c95af Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 31 Aug 2026 11:16:02 +0200 Subject: [PATCH] R-102 phase 1.1: split the recovery-unit path helpers, zero behaviour change Every unit path helper took (nsRoot, stackName) and joined backups/primary//... . That hard-coded 'primary' is the mechanism of R-102: Tier-2 mirrors the whole unit directory to /backups/secondary//recovery-unit/ every night, and because no reader could NAME a unit outside backups/primary/, that mirror has been captured for months and read by nothing. Adds four unit-directory-relative primitives - UnitComposeDir, UnitManifestFile, UnitDBDumpDir, UnitVolumeDumpDir - each taking the recovery-unit DIRECTORY itself. The four existing (nsRoot, stackName) helpers become thin wrappers over them and keep their exact signatures and their exact return values; every current caller compiles untouched. ONE implementation, two callers - the rule restoreDockerVolumesFrom already follows in this repo. TestR102_PathWrappersAreByteIdenticalToToday pins the wrappers against hand-written literals (not re-derived from the helpers under test). Red-proof: UnitComposeDir join changed to 'compose2' -> the test fails on all three fixtures. --- controller/internal/appbackup/paths.go | 43 ++++++++++- .../appbackup/r102_paths_split_test.go | 73 +++++++++++++++++++ .../internal/backup/appbackup_bridge.go | 12 +++ 3 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 controller/internal/appbackup/r102_paths_split_test.go diff --git a/controller/internal/appbackup/paths.go b/controller/internal/appbackup/paths.go index 5dbbcd1..28c64f3 100644 --- a/controller/internal/appbackup/paths.go +++ b/controller/internal/appbackup/paths.go @@ -77,25 +77,60 @@ func RecoveryUnitPath(nsRoot, stackName string) string { return filepath.Join(nsRoot, "backups", "primary", stackName) } +// UNIT-DIRECTORY-RELATIVE HELPERS (R-102). The four below take THE RECOVERY-UNIT DIRECTORY ITSELF +// and know only the unit's internal layout. The `(nsRoot, stackName)` helpers that follow are thin +// wrappers over them, and every existing caller keeps its exact signature and its exact result. +// +// They exist because the `(nsRoot, stackName)` form resolves through RecoveryUnitPath, which joins +// `backups/primary/` — a hard-coded `primary` that was the MECHANISM of R-102. Tier-2 mirrors +// the whole unit directory to `/backups/secondary//recovery-unit/`, and because every +// reader of a unit could only name a `primary` path, that mirror was captured nightly for months and +// read by nothing. The unit's INTERNAL layout is identical wherever the directory sits, so the fix is +// to let a reader name the directory rather than re-derive it. +// +// ONE implementation, two callers — the same rule as backup.restoreDockerVolumesFrom. Do not add a +// second copy of a join: a duplicated layout constant is how the two sides drift apart. + +// UnitComposeDir returns the compose/config capture dir within a recovery-unit DIRECTORY +// (docker-compose.yml + .felhom.yml + app.yaml carrying the portable secret class). +func UnitComposeDir(unitDir string) string { + return filepath.Join(unitDir, "compose") +} + +// UnitManifestFile returns the manifest.json path within a recovery-unit DIRECTORY. +func UnitManifestFile(unitDir string) string { + return filepath.Join(unitDir, "manifest.json") +} + +// UnitDBDumpDir returns the DB dump directory within a recovery-unit DIRECTORY. +func UnitDBDumpDir(unitDir string) string { + return filepath.Join(unitDir, "db-dumps") +} + +// UnitVolumeDumpDir returns the Docker-volume dump-tar directory within a recovery-unit DIRECTORY. +func UnitVolumeDumpDir(unitDir string) string { + return filepath.Join(unitDir, "volume-dumps") +} + // RecoveryUnitComposePath returns the compose/config capture dir within an app's recovery unit // (docker-compose.yml + .felhom.yml + secret-stripped app.yaml). func RecoveryUnitComposePath(nsRoot, stackName string) string { - return filepath.Join(RecoveryUnitPath(nsRoot, stackName), "compose") + return UnitComposeDir(RecoveryUnitPath(nsRoot, stackName)) } // RecoveryUnitManifestPath returns the manifest.json path within an app's recovery unit. func RecoveryUnitManifestPath(nsRoot, stackName string) string { - return filepath.Join(RecoveryUnitPath(nsRoot, stackName), "manifest.json") + return UnitManifestFile(RecoveryUnitPath(nsRoot, stackName)) } // AppDBDumpPath returns the DB dump directory for an app under a felhom-data namespace root. func AppDBDumpPath(nsRoot, stackName string) string { - return filepath.Join(RecoveryUnitPath(nsRoot, stackName), "db-dumps") + return UnitDBDumpDir(RecoveryUnitPath(nsRoot, stackName)) } // AppVolumeDumpPath returns the Docker-volume dump-tar directory for an app under a namespace root. func AppVolumeDumpPath(nsRoot, stackName string) string { - return filepath.Join(RecoveryUnitPath(nsRoot, stackName), "volume-dumps") + return UnitVolumeDumpDir(RecoveryUnitPath(nsRoot, stackName)) } // AppDataDir returns the app data directory under a felhom-data namespace root. The final segment diff --git a/controller/internal/appbackup/r102_paths_split_test.go b/controller/internal/appbackup/r102_paths_split_test.go new file mode 100644 index 0000000..c82f1c8 --- /dev/null +++ b/controller/internal/appbackup/r102_paths_split_test.go @@ -0,0 +1,73 @@ +package appbackup + +import ( + "path/filepath" + "testing" +) + +// A1 — TestR102_PathWrappersAreByteIdenticalToToday. +// +// R-102 Part 1.1 split each unit path helper into a unit-directory-relative primitive plus a thin +// `(nsRoot, stackName)` wrapper. The refactor's whole claim is ZERO behaviour change, and the only +// honest way to pin that is to compare each wrapper against the literal string it produced before the +// split — written out here by hand, not re-derived from the helpers under test (a test that called +// UnitComposeDir to compute its own expectation would pass for any consistent pair of wrong joins). +// +// Red-proof (recorded in REPORT.md): change one wrapper's join — e.g. UnitComposeDir to "compose2" — +// and this test fails. +func TestR102_PathWrappersAreByteIdenticalToToday(t *testing.T) { + cases := []struct { + nsRoot string + stack string + }{ + {"/mnt/hdd1", "docmost"}, + {"/mnt/sys_drive/felhom-data", "immich"}, + {"/mnt/felhom-usb", "paperless-ngx"}, + } + for _, c := range cases { + unit := c.nsRoot + "/backups/primary/" + c.stack + + if got, want := RecoveryUnitPath(c.nsRoot, c.stack), unit; got != want { + t.Errorf("RecoveryUnitPath(%q,%q) = %q, want %q", c.nsRoot, c.stack, got, want) + } + if got, want := RecoveryUnitComposePath(c.nsRoot, c.stack), unit+"/compose"; got != want { + t.Errorf("RecoveryUnitComposePath(%q,%q) = %q, want %q", c.nsRoot, c.stack, got, want) + } + if got, want := RecoveryUnitManifestPath(c.nsRoot, c.stack), unit+"/manifest.json"; got != want { + t.Errorf("RecoveryUnitManifestPath(%q,%q) = %q, want %q", c.nsRoot, c.stack, got, want) + } + if got, want := AppDBDumpPath(c.nsRoot, c.stack), unit+"/db-dumps"; got != want { + t.Errorf("AppDBDumpPath(%q,%q) = %q, want %q", c.nsRoot, c.stack, got, want) + } + if got, want := AppVolumeDumpPath(c.nsRoot, c.stack), unit+"/volume-dumps"; got != want { + t.Errorf("AppVolumeDumpPath(%q,%q) = %q, want %q", c.nsRoot, c.stack, got, want) + } + } +} + +// TestR102_UnitHelpersAreDirectoryRelative pins the OTHER half of the contract: the four primitives +// join onto whatever directory they are handed, with no `primary` segment reintroduced. The secondary +// mirror path below is the exact shape Tier-2 writes (tier2.go: `/backups/secondary// +// recovery-unit`), so a regression that re-derived a namespace root would show up here as a `primary` +// appearing in a secondary path. +func TestR102_UnitHelpersAreDirectoryRelative(t *testing.T) { + unit := "/mnt/hdd2/backups/secondary/docmost/recovery-unit" + checks := []struct { + name string + got string + want string + }{ + {"UnitComposeDir", UnitComposeDir(unit), unit + "/compose"}, + {"UnitManifestFile", UnitManifestFile(unit), unit + "/manifest.json"}, + {"UnitDBDumpDir", UnitDBDumpDir(unit), unit + "/db-dumps"}, + {"UnitVolumeDumpDir", UnitVolumeDumpDir(unit), unit + "/volume-dumps"}, + } + for _, c := range checks { + if c.got != c.want { + t.Errorf("%s(%q) = %q, want %q", c.name, unit, c.got, c.want) + } + if filepath.Base(filepath.Dir(c.got)) == "primary" { + t.Errorf("%s reintroduced a primary segment: %q", c.name, c.got) + } + } +} diff --git a/controller/internal/backup/appbackup_bridge.go b/controller/internal/backup/appbackup_bridge.go index 5dcdacf..ae8dcb2 100644 --- a/controller/internal/backup/appbackup_bridge.go +++ b/controller/internal/backup/appbackup_bridge.go @@ -156,6 +156,18 @@ func RecoveryUnitManifestPath(nsRoot, stackName string) string { return appbackup.RecoveryUnitManifestPath(nsRoot, stackName) } +// The four UNIT-DIRECTORY-RELATIVE helpers (R-102). They take the recovery-unit DIRECTORY itself, so +// a reader can name a unit that is NOT under backups/primary/ — the Tier-2 mirror at +// /backups/secondary//recovery-unit/ being the whole point. + +func UnitComposeDir(unitDir string) string { return appbackup.UnitComposeDir(unitDir) } + +func UnitManifestFile(unitDir string) string { return appbackup.UnitManifestFile(unitDir) } + +func UnitDBDumpDir(unitDir string) string { return appbackup.UnitDBDumpDir(unitDir) } + +func UnitVolumeDumpDir(unitDir string) string { return appbackup.UnitVolumeDumpDir(unitDir) } + func AppDataDir(nsRoot, stackName string) string { return appbackup.AppDataDir(nsRoot, stackName) }