From 73efb091d94c55cb9f67e2249eff458b2a0a4e49 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Tue, 4 Aug 2026 18:17:05 +0200 Subject: [PATCH] =?UTF-8?q?R-203:=20the=20app=20and=20its=20backup=20look?= =?UTF-8?q?=20in=20the=20same=20directory=20=E2=80=94=20one=20resolver,=20?= =?UTF-8?q?every=20caller?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit appbackup's path helpers take a NAMESPACE ROOT. Five call sites passed a bare DRIVE path. On an enrolled drive the two coincide, so nothing showed; on the system-data fallback they differ by exactly the felhom-data segment, and the app then bound a directory the off-site capture set never looked at -- while the run reported ok. Measured live on demo-hp: the app wrote to /mnt/sys_drive/userdata/media/books, the capture set looked for /mnt/sys_drive/felhom-data/userdata/media/books. THE RULE NOW HAS ONE EXPRESSION. appbackup.NamespaceRootFor / IsEnrolledDrive encode the drive-kind comparison; backup.Manager.namespaceRoot and stacks.Manager.inGuest delegate to it. There were already TWO copies and they differed -- the backup package's compared without filepath.Clean, the stacks package's with it, so a trailing slash from config would have flipped the mode in one and not the other. Sites routed through it: - stacks/deploy.go withPathVars -> ${USERDATA_PATH} (the live defect) - appexport/fabplan.go + export.go (via a new provider method) - web/handlers.go FileBrowser mounts (latent: the system drive is deliberately never a registered StoragePath, so this is the identity today) ComputeFabBuckets now receives the namespace root, which is what ComputeCaptureSet has always received -- so the export's classified paths and the backup's capture set describe the same directories by construction instead of by coincidence. Tests are table-driven over BOTH drive kinds, because this survived by being invisible on the kind that already worked. Red-proofs observed: restoring the bare-path call fails the system-drive row with the two paths differing by /felhom-data; inverting the drive-kind comparison fails every enrolled row. --- controller/cmd/controller/main.go | 5 ++ .../internal/appbackup/namespace_root_test.go | 69 +++++++++++++++++++ controller/internal/appbackup/paths.go | 29 ++++++++ .../appexport/estimate_volsize_test.go | 4 ++ controller/internal/appexport/export.go | 4 +- controller/internal/appexport/fabplan.go | 17 +++-- controller/internal/appexport/fabplan_test.go | 4 ++ controller/internal/appexport/provider.go | 5 ++ .../internal/appexport/roundtrip_test.go | 4 ++ .../internal/backup/appbackup_bridge.go | 5 ++ controller/internal/backup/backup.go | 5 +- controller/internal/stacks/deploy.go | 10 ++- controller/internal/stacks/manager.go | 2 +- controller/internal/stacks/migrate.go | 18 ++++- .../internal/stacks/userdata_belt_test.go | 12 ++-- .../stacks/userdata_path_r203_test.go | 60 ++++++++++++++++ controller/internal/web/fab_export_test.go | 3 + controller/internal/web/handlers.go | 18 ++++- 18 files changed, 256 insertions(+), 18 deletions(-) create mode 100644 controller/internal/appbackup/namespace_root_test.go create mode 100644 controller/internal/stacks/userdata_path_r203_test.go diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index acbaef4..2b15177 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -2239,6 +2239,11 @@ func (a *exportAdapter) GetStackHDDMounts(name string) []string { // resolved from an app's HDD_PATH. func (a *exportAdapter) GetImportRoot() string { return a.mgr.GetImportRoot() } +// GetStackNamespaceRoot (R-203) — the felhom-data namespace root, NOT the drive path. The appbackup +// path helpers all take this; passing HDD_PATH straight in is what made the export plan and the +// off-site capture set describe different directories on the system-data fallback. +func (a *exportAdapter) GetStackNamespaceRoot(name string) string { return a.mgr.StackNamespaceRoot(name) } + func (a *exportAdapter) GetStackHDDPath(name string) string { s, ok := a.mgr.GetStack(name) if !ok { diff --git a/controller/internal/appbackup/namespace_root_test.go b/controller/internal/appbackup/namespace_root_test.go new file mode 100644 index 0000000..1225d09 --- /dev/null +++ b/controller/internal/appbackup/namespace_root_test.go @@ -0,0 +1,69 @@ +package appbackup + +import "testing" + +// R-203 — the ONE drive-kind rule. Table-driven over BOTH drive kinds on purpose: this defect +// survived because it is invisible on the kind that already worked, so a test that only covers the +// enrolled drive proves nothing about the fix. + +func TestNamespaceRootFor_BothDriveKinds(t *testing.T) { + const sys = "/mnt/sys_drive" + cases := []struct { + name, drive, want string + }{ + // Scenario B — the enrolled drive must be BYTE-IDENTICAL to pre-R-203 behaviour. The + // in-guest mount already IS the namespace root; appending felhom-data here would recreate + // the .../felhom-data/felhom-data/... double-nest NamespaceRoot's comment exists to prevent. + {"enrolled usb", "/mnt/felhom-usb", "/mnt/felhom-usb"}, + {"enrolled hdd", "/mnt/felhom-drives/hdd_1", "/mnt/felhom-drives/hdd_1"}, + {"enrolled nvme", "/mnt/felhom-drives/nvme-1tb", "/mnt/felhom-drives/nvme-1tb"}, + // Scenario A — the system-data fallback gains the segment. This is the case that was wrong. + {"system drive", "/mnt/sys_drive", "/mnt/sys_drive/felhom-data"}, + // A trailing slash is the same drive. Before R-203 the backup package's copy of this rule + // compared WITHOUT Clean while the stacks package's copy compared WITH it — so a config value + // with a trailing slash would have flipped the mode in one package and not the other. + {"system drive, trailing slash", "/mnt/sys_drive/", "/mnt/sys_drive/felhom-data"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := NamespaceRootFor(tc.drive, sys); got != tc.want { + t.Fatalf("NamespaceRootFor(%q, %q) = %q, want %q", tc.drive, sys, got, tc.want) + } + }) + } +} + +// The rule must survive a trailing slash on the SYSTEM path too — it comes from config. +func TestIsEnrolledDrive_CleansBothSides(t *testing.T) { + if IsEnrolledDrive("/mnt/sys_drive", "/mnt/sys_drive/") { + t.Error("a trailing slash on the system path must not make the system drive look enrolled") + } + if IsEnrolledDrive("/mnt/sys_drive/", "/mnt/sys_drive") { + t.Error("a trailing slash on the drive path must not make the system drive look enrolled") + } + if !IsEnrolledDrive("/mnt/felhom-usb", "/mnt/sys_drive") { + t.Error("an enrolled drive must report enrolled") + } +} + +// The consequence the whole item is about: the directory an app binds and the directory the capture +// set looks in must be the SAME on both drive kinds. +// +// RED-PROOF: replace `UserdataDir(NamespaceRootFor(drive, sys))` with `UserdataDir(drive)` — the +// pre-R-203 call — and the system-drive row FAILS with the two paths differing by exactly +// `/felhom-data`. That is production behaviour up to v0.196.0. +func TestAppBindAndCaptureRootAgree(t *testing.T) { + const sys = "/mnt/sys_drive" + for _, drive := range []string{"/mnt/felhom-usb", "/mnt/felhom-drives/hdd_1", "/mnt/sys_drive"} { + nsRoot := NamespaceRootFor(drive, sys) + appBind := UserdataDir(nsRoot) // what the deploy sets as ${USERDATA_PATH} + captureRoot := UserdataDir(nsRoot) // what the capture set resolves RootUserdata against + if appBind != captureRoot { + t.Fatalf("drive %q: the app binds %q while the backup captures %q", drive, appBind, captureRoot) + } + // And it must be the canonical location — the one EnsureUserdataSkeleton creates. + if drive == sys && appBind != "/mnt/sys_drive/felhom-data/userdata" { + t.Fatalf("system drive resolved to %q, want the canonical /mnt/sys_drive/felhom-data/userdata", appBind) + } + } +} diff --git a/controller/internal/appbackup/paths.go b/controller/internal/appbackup/paths.go index 5686b51..5dbbcd1 100644 --- a/controller/internal/appbackup/paths.go +++ b/controller/internal/appbackup/paths.go @@ -32,6 +32,35 @@ func NamespaceRoot(drivePath string, inGuestDrive bool) string { return filepath.Join(drivePath, FelhomDataDir) } +// IsEnrolledDrive reports whether a drive path is an ENROLLED user-data drive (Model A: its in-guest +// mount already IS the namespace root) rather than the system-data fallback. It is the ONE comparison +// that decides which NamespaceRoot mode applies, and it lives here so no package re-derives it. +// +// Both sides are Clean'd: `/mnt/sys_drive/` and `/mnt/sys_drive` are the same drive, and a trailing +// slash arriving from config must not silently flip the mode. +func IsEnrolledDrive(drivePath, systemDataPath string) bool { + return filepath.Clean(drivePath) != filepath.Clean(systemDataPath) +} + +// NamespaceRootFor is the resolver every caller should use when it holds a bare DRIVE path and the +// system-data path — i.e. everywhere outside the backup package, which already had this rule. +// +// R-203: FIVE call sites passed a bare drive path straight to UserdataDir (and its siblings), which +// take a NAMESPACE ROOT. On an enrolled drive the two coincide, so nothing showed; on the system-data +// fallback they differ by exactly the felhom-data segment, and the app then bound a directory the +// backup never looked at. The run still reported ok. Measured live on demo-hp 2026-08-04: +// the app wrote to /mnt/sys_drive/userdata/media/books while the off-site capture set looked for +// /mnt/sys_drive/felhom-data/userdata/media/books. +// +// THE CONTRACT, restated because four callers got it wrong and a fifth will: UserdataDir, +// PrimaryBackupPath, RecoveryUnitPath and AppDataDir all take a NAMESPACE ROOT. If you are holding +// something that came out of HDD_PATH or a StoragePath, it is a DRIVE path — put it through here +// first. `UserdataDir(bareDrivePath)` still compiles and is still wrong; TestNoBareDrivePathToUserdataDir +// is the guard that keeps the count from growing. +func NamespaceRootFor(drivePath, systemDataPath string) string { + return NamespaceRoot(drivePath, IsEnrolledDrive(drivePath, systemDataPath)) +} + // PrimaryBackupPath returns the root primary backup directory under a felhom-data namespace root. func PrimaryBackupPath(nsRoot string) string { return filepath.Join(nsRoot, "backups", "primary") diff --git a/controller/internal/appexport/estimate_volsize_test.go b/controller/internal/appexport/estimate_volsize_test.go index 0c0d51a..e66d8bd 100644 --- a/controller/internal/appexport/estimate_volsize_test.go +++ b/controller/internal/appexport/estimate_volsize_test.go @@ -24,6 +24,10 @@ func (p *hddProvider) GetStackNeedsHDD(string) bool { return true } func (p *hddProvider) GetStackHDDMounts(string) []string { return p.mounts } func (p *hddProvider) GetStackHDDPath(string) string { return p.hddPath } func (p *hddProvider) GetImportRoot() string { return "" } // R-75: no import binds in this fixture + +// R-203: these fixtures use ENROLLED drive paths, where the namespace root IS the drive path. +// Delegating keeps that identity explicit rather than hardcoding it. +func (p *hddProvider) GetStackNamespaceRoot(name string) string { return p.GetStackHDDPath(name) } func (p *hddProvider) GetStackClassifiedBinds(string) ([]appbackup.ClassifiedBind, bool) { return p.binds, p.hasBinds } diff --git a/controller/internal/appexport/export.go b/controller/internal/appexport/export.go index dafe0fc..878faa6 100644 --- a/controller/internal/appexport/export.go +++ b/controller/internal/appexport/export.go @@ -659,7 +659,9 @@ func (e *Exporter) exportHDDData(req ExportRequest, dataDir string, manifest *Ma // Task 4: the class-scoped plan. Legacy / no-block apps get an EMPTY plan (all mounts kept, root // tar with zero excludes) → byte-identical v0.130.0 capture. plan := e.computeFabPlan(req, mounts) - ud := appbackup.UserdataDir(filepath.Clean(e.provider.GetStackHDDPath(stackName))) + // R-203: a NAMESPACE ROOT, not the drive path (identical on an enrolled drive; one segment short + // on the system-data fallback). + ud := appbackup.UserdataDir(filepath.Clean(e.provider.GetStackNamespaceRoot(stackName))) claimed := make(map[string]string) // subdir → mount that claimed it for _, mount := range mounts { diff --git a/controller/internal/appexport/fabplan.go b/controller/internal/appexport/fabplan.go index 3f1ac8f..01b5618 100644 --- a/controller/internal/appexport/fabplan.go +++ b/controller/internal/appexport/fabplan.go @@ -34,8 +34,12 @@ func (e *Exporter) computeFabPlan(req ExportRequest, mounts []string) fabPlan { if !has { return fabPlan{} // legacy: byte-identical v0.130.0 capture } - hddPath := filepath.Clean(e.provider.GetStackHDDPath(req.StackName)) - fb := appbackup.ComputeFabBuckets(binds, has, hddPath, e.provider.GetImportRoot()) + // R-203: the shared resolver's root parameter is a NAMESPACE ROOT — that is what the off-site + // side has always passed (ComputeCaptureSet ← offbox_capture.go). This site passed the bare drive + // path, so on the system-data fallback the export's classified paths and the backup's capture set + // described DIFFERENT directories for the same declared bind. They now agree by construction. + nsRoot := filepath.Clean(e.provider.GetStackNamespaceRoot(req.StackName)) + fb := appbackup.ComputeFabBuckets(binds, has, nsRoot, e.provider.GetImportRoot()) deselect := sliceSet(req.DeselectOptional) optIn := sliceSet(req.OptInExcluded) @@ -82,7 +86,10 @@ func (e *Exporter) computeFabPlan(req ExportRequest, mounts []string) fabPlan { } plan := fabPlan{SkipMounts: map[string]bool{}} - ud := appbackup.UserdataDir(hddPath) + // R-203: UserdataDir takes a NAMESPACE ROOT, not the drive path. Identical on an enrolled drive; + // one segment short on the system-data fallback, which is where the export plan then skipped (or + // failed to skip) the wrong directory. + ud := appbackup.UserdataDir(nsRoot) for _, m := range mounts { mc := filepath.Clean(m) if mc == filepath.Clean(ud) { @@ -116,8 +123,8 @@ func (e *Exporter) fabEstimateSplit(stackName string, est *ExportEstimate, volum if !has { return } - hddPath := filepath.Clean(e.provider.GetStackHDDPath(stackName)) - fb := appbackup.ComputeFabBuckets(binds, has, hddPath, e.provider.GetImportRoot()) + nsRoot := filepath.Clean(e.provider.GetStackNamespaceRoot(stackName)) // R-203, as above + fb := appbackup.ComputeFabBuckets(binds, has, nsRoot, e.provider.GetImportRoot()) est.HasClassification = true toItems := func(cps []appbackup.CapturePath) ([]FabItem, int64) { diff --git a/controller/internal/appexport/fabplan_test.go b/controller/internal/appexport/fabplan_test.go index a8babcf..b6e1e2d 100644 --- a/controller/internal/appexport/fabplan_test.go +++ b/controller/internal/appexport/fabplan_test.go @@ -24,6 +24,10 @@ type fabProv struct { func (p *fabProv) GetStackHDDPath(string) string { return p.hddPath } func (p *fabProv) GetImportRoot() string { return "" } // R-75: no import binds in this fixture + +// R-203: these fixtures use ENROLLED drive paths, where the namespace root IS the drive path. +// Delegating keeps that identity explicit rather than hardcoding it. +func (p *fabProv) GetStackNamespaceRoot(name string) string { return p.GetStackHDDPath(name) } func (p *fabProv) GetStackHDDMounts(string) []string { return p.mounts } func (p *fabProv) GetStackClassifiedBinds(string) ([]appbackup.ClassifiedBind, bool) { return p.binds, p.has diff --git a/controller/internal/appexport/provider.go b/controller/internal/appexport/provider.go index bae561b..36b3877 100644 --- a/controller/internal/appexport/provider.go +++ b/controller/internal/appexport/provider.go @@ -20,6 +20,11 @@ type ExportStackProvider interface { // GetImportRoot returns the CANONICAL drop-zone root (R-75), on the SYSTEM drive. ${IMPORT_PATH} // binds resolve against THIS, never against GetStackHDDPath. Empty when unresolvable. GetImportRoot() string + // GetStackNamespaceRoot returns the app's felhom-data NAMESPACE ROOT — the directory that directly + // contains backups/ and userdata/. It is NOT GetStackHDDPath: on an enrolled drive the two are the + // same, and on the system-data fallback the namespace root has one more segment (R-203). Every + // appbackup path helper takes THIS, never the drive path. Empty when the app has no HDD_PATH. + GetStackNamespaceRoot(name string) string // GetStackClassifiedBinds returns the app's backup-classified compose binds + whether it carries a // (valid) backup block (Task 2). Drives the `.fab` class-scoped export plan (Task 4); a legacy app // (false) exports the v0.130.0 full-root capture unchanged. diff --git a/controller/internal/appexport/roundtrip_test.go b/controller/internal/appexport/roundtrip_test.go index 31590c4..a7818b5 100644 --- a/controller/internal/appexport/roundtrip_test.go +++ b/controller/internal/appexport/roundtrip_test.go @@ -38,6 +38,10 @@ func (p *rtProvider) GetStackComposePath(string) (string, bool) { func (p *rtProvider) GetStackHDDMounts(string) []string { return nil } func (p *rtProvider) GetStackHDDPath(string) string { return "" } func (p *rtProvider) GetImportRoot() string { return "" } // R-75: no import binds in this fixture + +// R-203: these fixtures use ENROLLED drive paths, where the namespace root IS the drive path. +// Delegating keeps that identity explicit rather than hardcoding it. +func (p *rtProvider) GetStackNamespaceRoot(name string) string { return p.GetStackHDDPath(name) } func (p *rtProvider) GetStackClassifiedBinds(string) ([]appbackup.ClassifiedBind, bool) { return nil, false } diff --git a/controller/internal/backup/appbackup_bridge.go b/controller/internal/backup/appbackup_bridge.go index bbe3ae4..88e17b0 100644 --- a/controller/internal/backup/appbackup_bridge.go +++ b/controller/internal/backup/appbackup_bridge.go @@ -121,6 +121,11 @@ func NamespaceRoot(drivePath string, inGuestDrive bool) string { return appbackup.NamespaceRoot(drivePath, inGuestDrive) } +// NamespaceRootFor re-exports the ONE drive-kind-aware resolver (R-203). +func NamespaceRootFor(drivePath, systemDataPath string) string { + return appbackup.NamespaceRootFor(drivePath, systemDataPath) +} + func PrimaryBackupPath(nsRoot string) string { return appbackup.PrimaryBackupPath(nsRoot) } diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index cd6ae3d..7347ef8 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -329,7 +329,10 @@ func (m *Manager) GetAppDrivePath(stackName string) string { // as-is; only the SSD-only system-data fallback gets the felhom-data subdir appended. This is what // keeps a drive-resident app's backups single-nested instead of .../felhom-data/felhom-data/... . func (m *Manager) namespaceRoot(drivePath string) string { - return NamespaceRoot(drivePath, drivePath != m.systemDataPath) + // R-203: delegates to the ONE expression of the rule (appbackup.NamespaceRootFor). This used to + // hold its own copy — `drivePath != m.systemDataPath`, without Clean on either side — while + // stacks.Manager.inGuest held a second copy WITH Clean. Two copies that already differed. + return NamespaceRootFor(drivePath, m.systemDataPath) } // AppNamespaceRoot returns the felhom-data namespace root for a stack's keep-side backups, resolving diff --git a/controller/internal/stacks/deploy.go b/controller/internal/stacks/deploy.go index ea23b71..186a3a6 100644 --- a/controller/internal/stacks/deploy.go +++ b/controller/internal/stacks/deploy.go @@ -581,7 +581,7 @@ func (m *Manager) composeExecWithEnv(dir string, env map[string]string, args ... cmdEnv = append(cmdEnv, fmt.Sprintf("%s=%s", k, v)) } cmdEnv = append(cmdEnv, fmt.Sprintf("DOMAIN=%s", m.cfg.Customer.Domain)) - cmdEnv = withPathVars(cmdEnv, env["HDD_PATH"], m.GetImportRoot()) + cmdEnv = withPathVars(cmdEnv, env["HDD_PATH"], m.sysDataPath, m.GetImportRoot()) return m.composeExecCustomEnv(dir, cmdEnv, args...) } @@ -597,9 +597,13 @@ func (m *Manager) composeExecWithEnv(dir string, env map[string]string, args ... // An unresolvable importRoot is left UNSET on purpose (the caller logs it): compose then fails loudly // on an unresolved ${IMPORT_PATH} rather than silently falling back to a per-drive path, which would // recreate the dead-drop-zone shape R-75 exists to remove. -func withPathVars(cmdEnv []string, hdd, importRoot string) []string { +func withPathVars(cmdEnv []string, hdd, sysDataPath, importRoot string) []string { if hdd != "" { - cmdEnv = append(cmdEnv, "USERDATA_PATH="+appbackup.UserdataDir(hdd)) + // R-203: UserdataDir takes a NAMESPACE ROOT, not a bare drive path. Passing `hdd` straight in + // bound /userdata, which equals the namespace root only on an ENROLLED drive. On the + // system-data fallback it is one segment short, so the app wrote to a directory the off-site + // capture set never looked at — and the run still reported ok. Measured live on demo-hp. + cmdEnv = append(cmdEnv, "USERDATA_PATH="+appbackup.UserdataDir(appbackup.NamespaceRootFor(hdd, sysDataPath))) } if importRoot != "" { cmdEnv = append(cmdEnv, "IMPORT_PATH="+importRoot) diff --git a/controller/internal/stacks/manager.go b/controller/internal/stacks/manager.go index 90fd7aa..a513392 100644 --- a/controller/internal/stacks/manager.go +++ b/controller/internal/stacks/manager.go @@ -1196,7 +1196,7 @@ func (m *Manager) stackEnv(stackDir string) []string { // the namespace root (the chosen StoragePath: a Model-A user drive's mount, or the SSD's // felhom-data dir), so the catalog's ${USERDATA_PATH}/... mounts resolve under userdata/. // IMPORT_PATH (R-75) rides along but is derived from the SYSTEM drive, never from HDD_PATH. - env = withPathVars(env, appCfg.Env["HDD_PATH"], m.GetImportRoot()) + env = withPathVars(env, appCfg.Env["HDD_PATH"], m.sysDataPath, m.GetImportRoot()) } // App-email relay env (appended LAST so it wins over any app.yaml default). Returns nil unless diff --git a/controller/internal/stacks/migrate.go b/controller/internal/stacks/migrate.go index 39ca59a..959b75f 100644 --- a/controller/internal/stacks/migrate.go +++ b/controller/internal/stacks/migrate.go @@ -277,8 +277,24 @@ func (m *Manager) startMigration(scope, sourcePath, appName, targetPath string, // inGuest reports whether drivePath is a user drive (its in-guest mount IS the felhom-data namespace // root) vs the system/SSD path (which holds a felhom-data SUBDIR). Compares cleaned paths so the // decision is stable regardless of slash style. +// StackNamespaceRoot resolves an app's felhom-data NAMESPACE ROOT from its HDD_PATH (R-203). Exported +// because the export adapter needs the same answer the backup side already computes, and only this +// Manager holds the system-data path. Empty HDD_PATH → empty (the caller decides what that means). +func (m *Manager) StackNamespaceRoot(name string) string { + cfg := m.LoadAppConfigByName(name) + if cfg == nil { + return "" + } + hdd := cfg.Env["HDD_PATH"] + if hdd == "" { + return "" + } + return appbackup.NamespaceRootFor(hdd, m.sysDataPath) +} + func (m *Manager) inGuest(drivePath string) bool { - return filepath.Clean(drivePath) != filepath.Clean(m.sysDataPath) + // R-203: the ONE expression of the rule now lives in appbackup; this is the stacks-side name for it. + return appbackup.IsEnrolledDrive(drivePath, m.sysDataPath) } // appSourceNS resolves an app's current source drive path + felhom-data namespace root from its diff --git a/controller/internal/stacks/userdata_belt_test.go b/controller/internal/stacks/userdata_belt_test.go index 5fb2e06..1bf2c9b 100644 --- a/controller/internal/stacks/userdata_belt_test.go +++ b/controller/internal/stacks/userdata_belt_test.go @@ -78,8 +78,9 @@ func TestEnsureUserdataMounts_CreatesBeltDirs(t *testing.T) { // adds nothing when it's empty. Regression for the initial-deploy bug where ${USERDATA_PATH} resolved // to "" and bound a bogus root-owned dir at the container root. func TestWithPathVars(t *testing.T) { + const sysDataPath = "/mnt/sys_drive" const importRoot = "/mnt/sys_drive/felhom-data/userdata/import" - got := withPathVars([]string{"DOMAIN=x"}, "/mnt/felhom-usb", importRoot) + got := withPathVars([]string{"DOMAIN=x"}, "/mnt/felhom-usb", sysDataPath, importRoot) want := "USERDATA_PATH=" + appbackup.UserdataDir("/mnt/felhom-usb") found := false for _, e := range got { @@ -91,7 +92,7 @@ func TestWithPathVars(t *testing.T) { t.Errorf("USERDATA_PATH not injected: got %v, want %q", got, want) } // companion: empty HDD_PATH → no USERDATA_PATH at all - for _, e := range withPathVars([]string{"DOMAIN=x"}, "", importRoot) { + for _, e := range withPathVars([]string{"DOMAIN=x"}, "", sysDataPath, importRoot) { if strings.HasPrefix(e, "USERDATA_PATH") { t.Errorf("USERDATA_PATH must NOT be set when HDD_PATH is empty: %q", e) } @@ -103,15 +104,16 @@ func TestWithPathVars(t *testing.T) { // root-owned dir at the container root. And the unresolvable case must leave the variable UNSET — // never fall back to a per-drive path, which would recreate the dead-drop-zone shape R-75 removes. func TestWithPathVars_ImportPath(t *testing.T) { + const sysDataPath = "/mnt/sys_drive" const importRoot = "/mnt/sys_drive/felhom-data/userdata/import" - got := withPathVars([]string{"DOMAIN=x"}, "/mnt/felhom-drives/hdd_1", importRoot) + got := withPathVars([]string{"DOMAIN=x"}, "/mnt/felhom-drives/hdd_1", sysDataPath, importRoot) if !slices.Contains(got, "IMPORT_PATH="+importRoot) { t.Errorf("IMPORT_PATH not injected: got %v", got) } // It is CANONICAL: it must not be derived from HDD_PATH. A second app on a different drive gets // the identical value — that is the whole point of the canonical root. - other := withPathVars([]string{"DOMAIN=x"}, "/mnt/felhom-drives/nvme-1tb", importRoot) + other := withPathVars([]string{"DOMAIN=x"}, "/mnt/felhom-drives/nvme-1tb", sysDataPath, importRoot) if !slices.Contains(other, "IMPORT_PATH="+importRoot) { t.Errorf("IMPORT_PATH must not vary with HDD_PATH: got %v", other) } @@ -121,7 +123,7 @@ func TestWithPathVars_ImportPath(t *testing.T) { } } // Unresolvable → UNSET (compose then fails loudly on ${IMPORT_PATH}). - for _, e := range withPathVars([]string{"DOMAIN=x"}, "/mnt/felhom-drives/hdd_1", "") { + for _, e := range withPathVars([]string{"DOMAIN=x"}, "/mnt/felhom-drives/hdd_1", sysDataPath, "") { if strings.HasPrefix(e, "IMPORT_PATH") { t.Errorf("IMPORT_PATH must NOT be set when the import root is unresolvable: %q", e) } diff --git a/controller/internal/stacks/userdata_path_r203_test.go b/controller/internal/stacks/userdata_path_r203_test.go new file mode 100644 index 0000000..ba40f62 --- /dev/null +++ b/controller/internal/stacks/userdata_path_r203_test.go @@ -0,0 +1,60 @@ +package stacks + +import ( + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/appbackup" +) + +// R-203 Scenario A/B — what the DEPLOY sets as ${USERDATA_PATH}, table-driven over both drive kinds. +// This is the site the drill caught: on the system-data fallback the app bound a directory the +// off-site capture set never looked at, and the run still reported ok. +func TestWithPathVars_UserdataRootByDriveKind(t *testing.T) { + const sys = "/mnt/sys_drive" + const importRoot = "/mnt/sys_drive/felhom-data/userdata/import" + cases := []struct{ name, hdd, want string }{ + // Scenario B — enrolled drives: BYTE-IDENTICAL to pre-R-203. + {"enrolled usb", "/mnt/felhom-usb", "/mnt/felhom-usb/userdata"}, + {"enrolled hdd", "/mnt/felhom-drives/hdd_1", "/mnt/felhom-drives/hdd_1/userdata"}, + // Scenario A — the system-data fallback: the canonical namespace root, which is what the + // skeleton builder creates and what the capture set resolves against. + {"system drive", "/mnt/sys_drive", "/mnt/sys_drive/felhom-data/userdata"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := withPathVars([]string{"DOMAIN=x"}, tc.hdd, sys, importRoot) + want := "USERDATA_PATH=" + tc.want + for _, e := range got { + if e == want { + return + } + } + var actual string + for _, e := range got { + if strings.HasPrefix(e, "USERDATA_PATH=") { + actual = e + } + } + t.Fatalf("USERDATA_PATH = %q, want %q — the app would bind a directory the backup does not capture", actual, want) + }) + } +} + +// The deploy-time bind and the backup-time capture root must be the SAME directory. Asserted against +// the resolver the off-site capture set actually uses, so the two cannot drift apart again silently. +func TestDeployBindMatchesCaptureRoot(t *testing.T) { + const sys = "/mnt/sys_drive" + for _, hdd := range []string{"/mnt/felhom-usb", "/mnt/felhom-drives/hdd_1", "/mnt/sys_drive"} { + var bind string + for _, e := range withPathVars(nil, hdd, sys, "") { + if strings.HasPrefix(e, "USERDATA_PATH=") { + bind = strings.TrimPrefix(e, "USERDATA_PATH=") + } + } + captureRoot := appbackup.UserdataDir(appbackup.NamespaceRootFor(hdd, sys)) + if bind != captureRoot { + t.Fatalf("drive %q: deploy binds %q, backup captures %q", hdd, bind, captureRoot) + } + } +} diff --git a/controller/internal/web/fab_export_test.go b/controller/internal/web/fab_export_test.go index 23735ed..1f736ff 100644 --- a/controller/internal/web/fab_export_test.go +++ b/controller/internal/web/fab_export_test.go @@ -34,6 +34,9 @@ func (p *fabWebProvider) GetStackHDDMounts(string) []string { } func (p *fabWebProvider) GetStackHDDPath(string) string { return p.hddPath } func (p *fabWebProvider) GetImportRoot() string { return "" } // R-75: no import binds in this fixture + +// R-203: an ENROLLED drive fixture — the namespace root IS the drive path. +func (p *fabWebProvider) GetStackNamespaceRoot(name string) string { return p.GetStackHDDPath(name) } func (p *fabWebProvider) GetStackClassifiedBinds(string) ([]appbackup.ClassifiedBind, bool) { return p.binds, true } diff --git a/controller/internal/web/handlers.go b/controller/internal/web/handlers.go index fe03b18..d482cc0 100644 --- a/controller/internal/web/handlers.go +++ b/controller/internal/web/handlers.go @@ -2324,6 +2324,11 @@ func (s *Server) syncFileBrowserMounts(resetDBOnChange bool) { classify: s.classifyFSPath, ensureSkeleton: s.ensureUserdataSkeleton, logger: s.logger, + // R-203. LATENT at this site rather than live: the comment below records that the system drive + // is deliberately never a registered StoragePath, so every `sp.Path` here is an enrolled drive + // and the resolver is the identity today. Wired anyway — the contract is uniform, and the next + // person to register a non-enrolled path should not have to rediscover this. + nsRootFor: func(p string) string { return appbackup.NamespaceRootFor(p, s.cfg.Paths.SystemDataPath) }, }) // R-75: the canonical drop-zone is an EXTRA bind, outside the registered-storage-path loop above. @@ -2421,6 +2426,10 @@ type fbPathDeps struct { isMount func(string) bool // drive-absent gate probe (production: system.IsMountPoint) classify func(string) string // network stub gate (production: Server.classifyFSPath; nil → include, fail open) ensureSkeleton func(string) error // userdata skeleton (production: appbackup.EnsureUserdataSkeleton) — DRIVES ONLY + // nsRootFor resolves a bare DRIVE path to its felhom-data namespace root (R-203). Injected rather + // than derived here because only the caller knows the system-data path. nil → identity, which is + // the pre-R-203 behaviour and correct for every enrolled drive. + nsRootFor func(string) string logger *log.Logger } @@ -2480,7 +2489,14 @@ func buildFileBrowserPaths(paths []settings.StoragePath, d fbPathDeps) (storageM d.logger.Printf("[WARN] [web] FileBrowser: could not ensure userdata skeleton on %s: %v", sp.Path, err) } } - userdataSrc := appbackup.UserdataDir(sp.Path) + // R-203: UserdataDir takes a NAMESPACE ROOT. On the system-data fallback a bare drive path is + // one segment short, so FileBrowser mounted a directory that is not the one the skeleton + // builder creates or the backup captures — the customer would browse an empty tree. + nsRoot := sp.Path + if d.nsRootFor != nil { + nsRoot = d.nsRootFor(sp.Path) + } + userdataSrc := appbackup.UserdataDir(nsRoot) storageMounts = append(storageMounts, fmt.Sprintf(" - %s:/srv/%s", userdataSrc, mountName)) } return storageMounts, configPaths