diff --git a/controller/internal/backup/backup.go b/controller/internal/backup/backup.go index f892496..0dce7f6 100644 --- a/controller/internal/backup/backup.go +++ b/controller/internal/backup/backup.go @@ -13,6 +13,7 @@ import ( "gitea.dooplex.hu/admin/felhom-controller/internal/config" "gitea.dooplex.hu/admin/felhom-controller/internal/settings" + "gitea.dooplex.hu/admin/felhom-controller/internal/system" ) // Manager orchestrates app-data backups: database dumps and Docker-volume tars. @@ -44,6 +45,16 @@ type Manager struct { // disconnected) can be unit-tested without Docker. Nil → the real DumpAppVolumesSafe. dumpVolumesSafe func(stackName string) error + // F7 tar seam — the ONE docker exec inside DumpAppVolumes, overridable so the atomic-write + // behaviour (tmp+fsync+rename; the last good `.tar` survives a mid-write failure) is unit-testable + // without Docker. It must write the tar to `/.tar.tmp` and return the combined + // output + error. Nil → the real `docker run … alpine tar cf …tar.tmp`. + tarVolume func(volName, dumpDir string) ([]byte, error) + + // F6 per-app tier-2 seam — overridable so RunAllTier2's app SELECTION (now incl. volume-only apps) + // is unit-testable without rsync/du. Nil → the real RunTier2. + perAppTier2 func(stackName string) error + // generateSecret (O4), if set, produces a replacement value for a RESETTABLE secret that could // not be recovered during restore-from-unit (wired to stacks.Manager.GenerateSecretForField in // main.go). Nil / ok=false → the secret stays absent and the restore proceeds with a loud WARN. @@ -95,8 +106,21 @@ type FullBackupStatus struct { // Flash messages (set by handlers, passed through redirect) FlashSuccess string FlashError string + + // SingleCopyWarning (F6, CAMPAIGN-3) is a non-empty honest Hungarian notice when the box has NO + // off-drive target at all — tier-1 is then the ONLY local copy and 3-2-1 needs a 2nd drive or + // offsite. Empty when an off-drive (tier-2) target exists. Never a fake 3-2-1 guarantee. + SingleCopyWarning string } +// systemDriveLabel is the human label for the internal SSD / system drive (F6 — a sys_drive app's +// backup used to render with a blank drive label). Matches the tier-2 UI wording. +const systemDriveLabel = "Belső SSD (rendszer)" + +// singleCopyNotice is the honest single-drive signal (F6): shown when no off-drive tier-2 target +// exists, instead of silently implying a 3-2-1 guarantee the box cannot provide. +const singleCopyNotice = "Csak egy másolat készül (nincs második meghajtó) — a 3-2-1 mentéshez csatlakoztasson egy második meghajtót vagy offsite tárolót." + // DBDumpStatus holds the last DB dump result. type DBDumpStatus struct { LastRun time.Time @@ -336,6 +360,11 @@ func (m *Manager) runDBDumpsInternal(ctx context.Context) error { // Phase 2: refresh each deployed app's self-contained recovery unit (compose + manifest). m.captureAllRecoveryUnits() + // F5 (CAMPAIGN-3): after the units are fresh on the CURRENT drives, prune any orphaned + // backups/primary/ dir an app left on an OLD drive when its HDD_PATH moved — pure disk + // residue, invisible in the snapshot list. Guarded (deployed + different current drive only). + m.pruneStalePrimaryDirs() + // No silent partials: a DB-dump or volume-dump failure fails the whole run. if !allOK { return fmt.Errorf("some backup steps failed: %s", strings.Join(failedSummaryLines(summary), "; ")) @@ -436,22 +465,35 @@ func (m *Manager) DumpAppVolumes(stackName string) error { var dumpErrors []string for _, volName := range volumes { tarPath := filepath.Join(dumpDir, volName+".tar") + // F7 (CAMPAIGN-3, HIGH): write the tar to a `.tar.tmp` sibling and only atomically rename it + // over the restore point on success — the same crash-safe pattern the DB-dump path uses + // (appbackup/dbdump.go DumpOne). Before this, tar wrote the `.tar` IN PLACE, so a mid-write NFS + // cut left a 0-byte tar REPLACING the last good dump (tier-1 restore is replace-semantics → an + // empty volume). Now a failed/interrupted write only ever touches the `.tmp`; the last good + // `.tar` is untouched. The `.tmp` name (ends `.tmp`, not `.tar`) is invisible to the + // restore-point/stale scans, so it is never mistaken for a restore point. + tmpPath := tarPath + ".tmp" if m.isDebug() { m.logger.Printf("[DEBUG] [backup] Dumping volume %s for %s", volName, stackName) } - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) - cmd := exec.CommandContext(ctx, "docker", "run", "--rm", - "-v", volName+":/vol:ro", - "-v", dumpDir+":/out", - "alpine", "tar", "cf", "/out/"+volName+".tar", "-C", "/vol", ".") - out, err := cmd.CombinedOutput() - cancel() + out, err := m.tarVolumeOrDefault(volName, dumpDir) if err != nil { - m.logger.Printf("[WARN] [backup] Volume dump failed for %s/%s: %s — %v", + // Any tar error or context timeout (incl. a dead NFS target → EIO): remove ONLY the tmp, + // leave the existing `.tar` restore point byte-untouched, WARN, continue. + m.logger.Printf("[WARN] [backup] Volume dump failed for %s/%s (last good dump preserved): %s — %v", stackName, volName, strings.TrimSpace(string(out)), err) - os.Remove(tarPath) + os.Remove(tmpPath) + dumpErrors = append(dumpErrors, volName) + continue + } + + // fsync the tmp file (flush the tar to disk) then atomically rename over the restore point. + if err := atomicPromoteTar(tmpPath, tarPath); err != nil { + m.logger.Printf("[WARN] [backup] Volume dump promote failed for %s/%s (last good dump preserved): %v", + stackName, volName, err) + os.Remove(tmpPath) dumpErrors = append(dumpErrors, volName) continue } @@ -461,17 +503,24 @@ func (m *Manager) DumpAppVolumes(stackName string) error { } } - // Clean up tars for volumes that no longer exist + // Clean up tars (and any orphan `.tar.tmp` from a killed run) for volumes that no longer exist. entries, _ := os.ReadDir(dumpDir) activeVols := make(map[string]bool) for _, v := range volumes { activeVols[v+".tar"] = true } for _, e := range entries { - if !activeVols[e.Name()] && strings.HasSuffix(e.Name(), ".tar") { - os.Remove(filepath.Join(dumpDir, e.Name())) + name := e.Name() + // A leftover `.tar.tmp` is never a restore point — always safe to remove (its `.tar` sibling, + // if any, is the real restore point and is handled by the `.tar` branch). + if strings.HasSuffix(name, ".tar.tmp") { + os.Remove(filepath.Join(dumpDir, name)) + continue + } + if !activeVols[name] && strings.HasSuffix(name, ".tar") { + os.Remove(filepath.Join(dumpDir, name)) if m.isDebug() { - m.logger.Printf("[DEBUG] [backup] Removed stale volume dump: %s/%s", stackName, e.Name()) + m.logger.Printf("[DEBUG] [backup] Removed stale volume dump: %s/%s", stackName, name) } } } @@ -482,6 +531,51 @@ func (m *Manager) DumpAppVolumes(stackName string) error { return nil } +// tarVolumeOrDefault runs the F7 tar seam (m.tarVolume) or, when unset, the real docker tar into the +// `.tar.tmp` sibling under dumpDir. The 10-minute bound matches the original. +func (m *Manager) tarVolumeOrDefault(volName, dumpDir string) ([]byte, error) { + if m.tarVolume != nil { + return m.tarVolume(volName, dumpDir) + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + cmd := exec.CommandContext(ctx, "docker", "run", "--rm", + "-v", volName+":/vol:ro", + "-v", dumpDir+":/out", + "alpine", "tar", "cf", "/out/"+volName+".tar.tmp", "-C", "/vol", ".") + return cmd.CombinedOutput() +} + +// atomicPromoteTar fsyncs a completed `.tar.tmp` then atomically renames it over the final `.tar` +// (same-dir rename = atomic on the target fs). It mirrors DumpOne's crash-safety (F7) and EXCEEDS it +// by also best-effort fsync'ing the directory entry, so the rename itself survives a power loss — the +// DB-dump path fsyncs the file but not the dir; a follow-up could add the dir fsync there too. On any +// error the tmp is left for the caller to remove; the final `.tar` is never touched here except by a +// successful rename. +func atomicPromoteTar(tmpPath, finalPath string) error { + f, err := os.Open(tmpPath) + if err != nil { + return fmt.Errorf("opening tmp dump: %w", err) + } + if err := f.Sync(); err != nil { + f.Close() + return fmt.Errorf("syncing tmp dump: %w", err) + } + if err := f.Close(); err != nil { + return fmt.Errorf("closing tmp dump: %w", err) + } + if err := os.Rename(tmpPath, finalPath); err != nil { + return fmt.Errorf("renaming tmp dump: %w", err) + } + // Best-effort: fsync the directory so the rename is durable (ignore errors — the rename already + // made the new content visible; this only hardens against a power loss immediately after). + if dir, derr := os.Open(filepath.Dir(finalPath)); derr == nil { + _ = dir.Sync() + _ = dir.Close() + } + return nil +} + // DumpAppVolumesSafe stops the stack before dumping volumes and restarts after. // Prevents inconsistent tars of live database volumes (e.g. PostgreSQL). // Protected stacks that reject StopStack will return an error — callers handle as warning. @@ -749,6 +843,7 @@ func (m *Manager) GetFullStatus(nextDBDump time.Time) *FullBackupStatus { // Update dynamic fields that don't need subprocess calls status.Running = m.running status.NextDBDump = nextDBDump + status.SingleCopyWarning = m.singleCopyWarning() // F6: honest single-drive signal // Deep-copy lastDBDump so callers cannot mutate shared state. if m.lastDBDump != nil { copyDump := *m.lastDBDump @@ -786,10 +881,11 @@ func (m *Manager) GetFullStatus(nextDBDump time.Time) *FullBackupStatus { // No cache yet — return a minimal status (first page load before cache is populated) status := &FullBackupStatus{ - Enabled: m.cfg.Backup.Enabled, - Running: m.running, - DBDumpSchedule: m.cfg.Backup.DBDumpSchedule, - NextDBDump: nextDBDump, + Enabled: m.cfg.Backup.Enabled, + Running: m.running, + DBDumpSchedule: m.cfg.Backup.DBDumpSchedule, + NextDBDump: nextDBDump, + SingleCopyWarning: m.singleCopyWarning(), // F6 } if m.lastDBDump != nil { copyDump := *m.lastDBDump @@ -802,6 +898,114 @@ func (m *Manager) GetFullStatus(nextDBDump time.Time) *FullBackupStatus { return status } +// hasOffDriveTarget reports whether any registered, schedulable storage path lives on a physical disk +// OTHER than the system drive — i.e. whether a genuine off-drive (tier-2) copy is possible at all. +// When false the box is single-drive: tier-1 is the ONLY local copy and 3-2-1 needs a 2nd drive or +// offsite (F6 — surfaced honestly via SingleCopyWarning, never a faked guarantee). +func (m *Manager) hasOffDriveTarget() bool { + if m.settings == nil || m.systemDataPath == "" { + return false + } + for _, sp := range m.settings.GetSchedulableStoragePaths() { + if sp.Path == m.systemDataPath || system.SamePhysicalDevice(m.systemDataPath, sp.Path) { + continue + } + return true + } + return false +} + +// singleCopyWarning returns the honest single-drive notice, or "" when an off-drive target exists. +func (m *Manager) singleCopyWarning() string { + if m.hasOffDriveTarget() { + return "" + } + return singleCopyNotice +} + +// sysDriveLabelFor returns the drive label for a stack's tier-1 restore point — the clear +// system-drive label for a sys_drive (volume-only) app (F6: never blank), else the enrolled drive's +// storage label. +func (m *Manager) sysDriveLabelFor(stackName string) string { + drive := m.GetAppDrivePath(stackName) + if drive == "" { + return "" + } + if drive == m.systemDataPath { + return systemDriveLabel + } + if m.settings != nil { + return m.settings.GetStorageLabel(drive) + } + return "" +} + +// pruneStalePrimaryDirs removes orphaned `backups/primary/` dirs left on a drive after an app's +// HDD_PATH moved to another drive (F5, CAMPAIGN-3 — pure disk residue, invisible in the snapshot +// list). LOAD-BEARING GUARDS: a dir is removed ONLY when is currently deployed AND its current +// namespace root differs from this dir's drive. It NEVER removes the dir on the app's CURRENT drive +// (that IS the live restore point), and NEVER removes a dir for an app NOT in the deployed set (an +// undeployed app's last backup is still its restore point — orphaned-app cleanup is a separate, +// user-driven concern). Only operates strictly under a `backups/primary/` prefix. +func (m *Manager) pruneStalePrimaryDirs() { + if m.stackProvider == nil { + return + } + // Current namespace root per DEPLOYED app. + current := map[string]string{} + for _, s := range m.stackProvider.ListDeployedStacks() { + if drive := m.GetAppDrivePath(s.Name); drive != "" { + current[s.Name] = filepath.Clean(m.namespaceRoot(drive)) + } + } + // Candidate drives to scan: the system drive + every registered storage path. + var nsRoots []string + if m.systemDataPath != "" { + nsRoots = append(nsRoots, filepath.Clean(m.namespaceRoot(m.systemDataPath))) + } + if m.settings != nil { + for _, sp := range m.settings.GetStoragePaths() { + nsRoots = append(nsRoots, filepath.Clean(NamespaceRoot(sp.Path, true))) + } + } + seen := map[string]bool{} + for _, nsRoot := range nsRoots { + if seen[nsRoot] { + continue + } + seen[nsRoot] = true + primaryDir := PrimaryBackupPath(nsRoot) + entries, err := os.ReadDir(primaryDir) + if err != nil { + continue // absent/unreadable (e.g. a disconnected drive) — nothing to prune here + } + for _, e := range entries { + if !e.IsDir() { + continue + } + app := e.Name() + cur, deployed := current[app] + if !deployed { + continue // GUARD: an undeployed app's last backup is still its restore point + } + if cur == nsRoot { + continue // GUARD: this IS the app's current drive — the live restore point + } + stalePath := RecoveryUnitPath(nsRoot, app) + // Prefix safety: only ever remove strictly inside `backups/primary/` (no surprise user data). + if !strings.HasPrefix(filepath.Clean(stalePath)+string(filepath.Separator), + filepath.Clean(primaryDir)+string(filepath.Separator)) { + continue + } + if err := os.RemoveAll(stalePath); err != nil { + m.logger.Printf("[WARN] [backup] F5: could not remove stale primary dir for %s on old drive: %v", app, err) + } else { + m.logger.Printf("[INFO] [backup] F5: removed stale primary backup dir for %s on an old drive (app now on %s)", app, cur) + } + } + } +} + // isDebug returns true if logging level is "debug". func (m *Manager) isDebug() bool { return m.cfg != nil && m.cfg.Logging.Level == "debug" diff --git a/controller/internal/backup/f5_stale_primary_test.go b/controller/internal/backup/f5_stale_primary_test.go new file mode 100644 index 0000000..8906a1f --- /dev/null +++ b/controller/internal/backup/f5_stale_primary_test.go @@ -0,0 +1,107 @@ +package backup + +import ( + "io" + "log" + "os" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// f5Manager wires a Manager with a real settings store holding the given enrolled drives, plus a +// sys drive, and a fake provider mapping each deployed app to its CURRENT HDD path. +func f5Manager(t *testing.T, sysDrive string, enrolled []string, deployed map[string]string) *Manager { + t.Helper() + sett, err := settings.Load(filepath.Join(t.TempDir(), "settings.json"), log.New(io.Discard, "", 0)) + if err != nil { + t.Fatal(err) + } + for _, d := range enrolled { + if err := sett.AddStoragePath(settings.StoragePath{Path: d, Label: filepath.Base(d), Schedulable: true}); err != nil { + t.Fatal(err) + } + } + var stacks []StackSummary + for name := range deployed { + stacks = append(stacks, StackSummary{Name: name}) + } + cfg := &config.Config{} + cfg.Paths.SystemDataPath = sysDrive + fake := &volDumpFakeProvider{stacks: stacks, hdd: deployed} + return &Manager{cfg: cfg, settings: sett, logger: log.New(io.Discard, "", 0), + systemDataPath: sysDrive, stackProvider: fake} +} + +// seedPrimaryDir creates a backups/primary/ dir (with a marker file) under a drive's namespace. +func seedPrimaryDir(t *testing.T, driveNsRoot, app string) string { + t.Helper() + dir := RecoveryUnitPath(driveNsRoot, app) + if err := os.MkdirAll(filepath.Join(dir, "volume-dumps"), 0755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(dir, "manifest.json"), []byte("{}"), 0644); err != nil { + t.Fatal(err) + } + return dir +} + +// F5: an app redeployed from drive A to drive B → its stale primary dir on A is removed, its live +// dir on B is kept. Guard red-proof lives in the two "kept" assertions. +func TestPruneStalePrimaryDirs_RemovesRedeployedResidue(t *testing.T) { + driveA := t.TempDir() + driveB := t.TempDir() + sys := filepath.Join(t.TempDir(), "sys") + // nextcloud currently on B; a stale unit remains on A. + m := f5Manager(t, sys, []string{driveA, driveB}, map[string]string{"nextcloud": driveB}) + + nsA := NamespaceRoot(driveA, true) + nsB := NamespaceRoot(driveB, true) + staleA := seedPrimaryDir(t, nsA, "nextcloud") + liveB := seedPrimaryDir(t, nsB, "nextcloud") + + m.pruneStalePrimaryDirs() + + if _, err := os.Stat(staleA); !os.IsNotExist(err) { + t.Errorf("stale primary dir on the OLD drive was not removed (F5)") + } + if _, err := os.Stat(liveB); err != nil { + t.Errorf("the LIVE primary dir on the current drive was wrongly removed: %v", err) + } +} + +// GUARD: an UNDEPLOYED app's primary dir is a restore point — it must be KEPT (companion: drop the +// `!deployed { continue }` guard → this dir is deleted → fail). +func TestPruneStalePrimaryDirs_KeepsUndeployedRestorePoint(t *testing.T) { + driveA := t.TempDir() + sys := filepath.Join(t.TempDir(), "sys") + // Only "nextcloud" is deployed (on A); "removed-app" has a leftover unit but is NOT deployed. + m := f5Manager(t, sys, []string{driveA}, map[string]string{"nextcloud": driveA}) + nsA := NamespaceRoot(driveA, true) + seedPrimaryDir(t, nsA, "nextcloud") + orphan := seedPrimaryDir(t, nsA, "removed-app") + + m.pruneStalePrimaryDirs() + + if _, err := os.Stat(orphan); err != nil { + t.Errorf("an UNDEPLOYED app's restore point was wrongly deleted (guard failure): %v", err) + } +} + +// GUARD: the dir on an app's CURRENT drive is never touched (already covered above, but assert it +// directly with a single-drive app so no cross-drive move is involved). +func TestPruneStalePrimaryDirs_KeepsCurrentDriveDir(t *testing.T) { + driveA := t.TempDir() + sys := filepath.Join(t.TempDir(), "sys") + m := f5Manager(t, sys, []string{driveA}, map[string]string{"nextcloud": driveA}) + nsA := NamespaceRoot(driveA, true) + live := seedPrimaryDir(t, nsA, "nextcloud") + + m.pruneStalePrimaryDirs() + + if _, err := os.Stat(live); err != nil { + t.Errorf("the current-drive primary dir was wrongly removed: %v", err) + } +} diff --git a/controller/internal/backup/f6_single_copy_test.go b/controller/internal/backup/f6_single_copy_test.go new file mode 100644 index 0000000..a150378 --- /dev/null +++ b/controller/internal/backup/f6_single_copy_test.go @@ -0,0 +1,76 @@ +package backup + +import ( + "io" + "log" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" + "gitea.dooplex.hu/admin/felhom-controller/internal/settings" +) + +// F6: a volume-only app (no HDD_PATH → data on sys_drive) must now flow through the tier-2 run — it +// used to be skipped, leaving a single controller-level copy. COMPANION red-proof: restore the +// `GetStackHDDPath == "" { continue }` skip in RunAllTier2 → "actualbudget" is absent → this fails. +func TestRunAllTier2_IncludesVolumeOnlyApps(t *testing.T) { + cfg := &config.Config{} + cfg.Paths.SystemDataPath = filepath.Join(t.TempDir(), "sys") + fake := &volDumpFakeProvider{ + stacks: []StackSummary{{Name: "nextcloud"}, {Name: "actualbudget"}}, + hdd: map[string]string{"nextcloud": filepath.Join(t.TempDir(), "usb")}, // actualbudget = volume-only + } + m := &Manager{cfg: cfg, logger: log.New(io.Discard, "", 0), + systemDataPath: cfg.Paths.SystemDataPath, stackProvider: fake} + + var processed []string + m.perAppTier2 = func(name string) error { processed = append(processed, name); return nil } + m.RunAllTier2() + + var sawVolumeOnly bool + for _, p := range processed { + if p == "actualbudget" { + sawVolumeOnly = true + } + } + if !sawVolumeOnly { + t.Errorf("volume-only app 'actualbudget' was NOT included in the tier-2 run (F6): processed=%v", processed) + } +} + +// F6: a single-drive box (no off-drive target) surfaces the honest single-copy notice; a box with an +// off-drive target does not (never a faked 3-2-1 guarantee). +func TestSingleCopyWarning_HonestOnSingleDrive(t *testing.T) { + // Single-drive: only the system drive, no schedulable off-drive paths. + m1, _ := newTestManager(t, "/srv/sys") + if got := m1.singleCopyWarning(); got != singleCopyNotice { + t.Errorf("single-drive box must surface the honest notice, got %q", got) + } + + // Multi-drive: an enrolled off-drive path → no warning. + m2, sett := newTestManager(t, "/srv/sys") + if err := sett.AddStoragePath(settings.StoragePath{Path: "/mnt/usb", Label: "USB", Schedulable: true}); err != nil { + t.Fatal(err) + } + if got := m2.singleCopyWarning(); got != "" { + t.Errorf("a box with an off-drive target must NOT show the single-copy notice, got %q", got) + } +} + +// F6: a sys_drive (volume-only) app's restore point carries a clear drive label, never blank. +func TestRestorePoint_SysDriveLabelNotBlank(t *testing.T) { + cfg := &config.Config{} + sysDrive := t.TempDir() + cfg.Paths.SystemDataPath = sysDrive + fake := &volDumpFakeProvider{ + stacks: []StackSummary{{Name: "actualbudget"}}, + volumes: map[string][]string{"actualbudget": {"actualbudget_data"}}, + } + m := &Manager{cfg: cfg, logger: log.New(io.Discard, "", 0), + systemDataPath: sysDrive, stackProvider: fake} + + label := m.sysDriveLabelFor("actualbudget") + if label != systemDriveLabel { + t.Errorf("sys_drive app drive label = %q, want %q (never blank — F6)", label, systemDriveLabel) + } +} diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index a277359..5beb089 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -47,8 +47,10 @@ func defaultOffboxRunner(ctx context.Context, env []string, args ...string) ([]b } // SetOffboxRunner overrides the restic exec (tests). SetOffboxNotify wires the failure→operator alert. -func (m *Manager) SetOffboxRunner(r offboxRunner) { m.offboxRunner = r } -func (m *Manager) SetOffboxNotify(fn func(dur time.Duration, snapshots int, err error)) { m.offboxNotify = fn } +func (m *Manager) SetOffboxRunner(r offboxRunner) { m.offboxRunner = r } +func (m *Manager) SetOffboxNotify(fn func(dur time.Duration, snapshots int, err error)) { + m.offboxNotify = fn +} func (m *Manager) runner() offboxRunner { if m.offboxRunner != nil { @@ -57,10 +59,10 @@ func (m *Manager) runner() offboxRunner { return defaultOffboxRunner } -func (m *Manager) offboxDir() string { return filepath.Join(m.cfg.Paths.DataDir, "offbox") } -func (m *Manager) offboxKeyPath() string { return filepath.Join(m.offboxDir(), "ssh_key") } -func (m *Manager) offboxPwPath() string { return filepath.Join(m.offboxDir(), "repo_password") } -func (m *Manager) offboxKnownHosts() string { return filepath.Join(m.offboxDir(), "known_hosts") } +func (m *Manager) offboxDir() string { return filepath.Join(m.cfg.Paths.DataDir, "offbox") } +func (m *Manager) offboxKeyPath() string { return filepath.Join(m.offboxDir(), "ssh_key") } +func (m *Manager) offboxPwPath() string { return filepath.Join(m.offboxDir(), "repo_password") } +func (m *Manager) offboxKnownHosts() string { return filepath.Join(m.offboxDir(), "known_hosts") } // WriteOffboxSecrets persists the SSH private key + (auto-generated if empty) repo password + the pinned // known-host line as 0600/0644 files in the data dir. The key is provided out-of-band by the operator diff --git a/controller/internal/backup/offbox_test.go b/controller/internal/backup/offbox_test.go index 7e5e235..1a38576 100644 --- a/controller/internal/backup/offbox_test.go +++ b/controller/internal/backup/offbox_test.go @@ -93,7 +93,7 @@ func TestOffbox_BaseArgsCarryConnectTimeout(t *testing.T) { base, env := m.offboxBaseArgs(sett.GetOffboxTarget()) joined := strings.Join(base, " ") for _, want := range []string{ - "-oConnectTimeout=10", // THE spike Q8 fail-fast knob + "-oConnectTimeout=10", // THE spike Q8 fail-fast knob "-oStrictHostKeyChecking=yes", // no blind TOFU "-oUserKnownHostsFile=", // pinned host key "-oBatchMode=yes", // no interactive hang @@ -466,8 +466,8 @@ const resticLockErr = "unable to create lock in backend: repository is already l // lockRunner records calls and returns the lock error for `backup` the first `lockTimes` times it's called. type lockRunner struct { backups, prunes, unlockStale, unlockRemoveAll int - lockTimes int - seq []string + lockTimes int + seq []string } func (r *lockRunner) run(_ context.Context, _ []string, args ...string) ([]byte, error) { @@ -629,10 +629,10 @@ func TestOffbox_ValidateRejectsInjection(t *testing.T) { {Host: "-oProxyCommand=touch /tmp/pwn", User: "felhom", RepoPath: "/srv/repo"}, // ssh option injection {Host: "nas;rm -rf /", User: "felhom", RepoPath: "/srv/repo"}, // metacharacters {Host: "nas.local", User: "-oProxyCommand=x", RepoPath: "/srv/repo"}, // user option injection - {Host: "nas.local", User: "felhom", RepoPath: "/srv/repo; evil"}, // path metacharacters - {Host: "nas.local", User: "felhom", RepoPath: "/srv/../etc"}, // traversal - {Host: "nas local", User: "felhom", RepoPath: "/srv/repo"}, // space - {Host: "nas.local", User: "felhom", RepoPath: "relative/path"}, // non-absolute + {Host: "nas.local", User: "felhom", RepoPath: "/srv/repo; evil"}, // path metacharacters + {Host: "nas.local", User: "felhom", RepoPath: "/srv/../etc"}, // traversal + {Host: "nas local", User: "felhom", RepoPath: "/srv/repo"}, // space + {Host: "nas.local", User: "felhom", RepoPath: "relative/path"}, // non-absolute } for i, b := range bad { bb := b diff --git a/controller/internal/backup/opstatus.go b/controller/internal/backup/opstatus.go index 3ca1ed8..a8ab2fb 100644 --- a/controller/internal/backup/opstatus.go +++ b/controller/internal/backup/opstatus.go @@ -11,7 +11,7 @@ import "time" // RestoreOpResult is the terminal record of the most recent restore op. type RestoreOpResult struct { - Op string `json:"op"` // "restore" | "tier2-restore" | "offbox-restore" + Op string `json:"op"` // "restore" | "tier2-restore" | "offbox-restore" Stack string `json:"stack"` OK bool `json:"ok"` Message string `json:"message"` diff --git a/controller/internal/backup/recovery_unit.go b/controller/internal/backup/recovery_unit.go index 286fde0..e044bfa 100644 --- a/controller/internal/backup/recovery_unit.go +++ b/controller/internal/backup/recovery_unit.go @@ -34,9 +34,9 @@ type RecoveryManifest struct { DisplayName string `json:"display_name"` ControllerVer string `json:"controller_version"` CreatedAt string `json:"created_at"` - Drive string `json:"drive"` // HDD_PATH (in-guest mount) - NamespaceRoot string `json:"namespace_root"` // resolved felhom-data namespace root - ImagePins []string `json:"image_pins"` // image NOT stored — re-pulled on restore + Drive string `json:"drive"` // HDD_PATH (in-guest mount) + NamespaceRoot string `json:"namespace_root"` // resolved felhom-data namespace root + ImagePins []string `json:"image_pins"` // image NOT stored — re-pulled on restore SecretEnvVars []string `json:"secret_env_vars"` // NAMES only — recovered from guest/PBS DataKeyEnvVars []string `json:"data_key_env_vars"` // fail-closed gate on restore SecretSource string `json:"secret_source"` // human note: where secrets come from diff --git a/controller/internal/backup/recovery_unit_test.go b/controller/internal/backup/recovery_unit_test.go index 6e3f0b3..dc3f492 100644 --- a/controller/internal/backup/recovery_unit_test.go +++ b/controller/internal/backup/recovery_unit_test.go @@ -24,13 +24,13 @@ type fakeRecoveryProvider struct { func (f *fakeRecoveryProvider) GetStackComposePath(string) (string, bool) { return filepath.Join(f.info.StackDir, "docker-compose.yml"), true } -func (f *fakeRecoveryProvider) ListDeployedStacks() []StackSummary { return nil } -func (f *fakeRecoveryProvider) GetStackHDDMounts(string) []string { return nil } -func (f *fakeRecoveryProvider) GetStackHDDPath(string) string { return f.hdd } -func (f *fakeRecoveryProvider) GetDockerVolumes(string) []string { return nil } -func (f *fakeRecoveryProvider) StopStack(string) error { f.stopped = true; return nil } -func (f *fakeRecoveryProvider) StartStack(string) error { return nil } -func (f *fakeRecoveryProvider) RefreshAndIsRunning(string) bool { return f.running } +func (f *fakeRecoveryProvider) ListDeployedStacks() []StackSummary { return nil } +func (f *fakeRecoveryProvider) GetStackHDDMounts(string) []string { return nil } +func (f *fakeRecoveryProvider) GetStackHDDPath(string) string { return f.hdd } +func (f *fakeRecoveryProvider) GetDockerVolumes(string) []string { return nil } +func (f *fakeRecoveryProvider) StopStack(string) error { f.stopped = true; return nil } +func (f *fakeRecoveryProvider) StartStack(string) error { return nil } +func (f *fakeRecoveryProvider) RefreshAndIsRunning(string) bool { return f.running } func (f *fakeRecoveryProvider) GetStackRecoveryInfo(string) (RecoveryInfo, bool) { return f.info, true } diff --git a/controller/internal/backup/restore_points.go b/controller/internal/backup/restore_points.go index a5b9735..b8b9338 100644 --- a/controller/internal/backup/restore_points.go +++ b/controller/internal/backup/restore_points.go @@ -58,16 +58,11 @@ func (m *Manager) ListRestorePoints(stackName string) (points []RestorePoint, fo newest = newestArtifact(AppDBDumpPath(nsRoot, stackName), ".sql", newest) newest = newestArtifact(AppVolumeDumpPath(nsRoot, stackName), ".tar", newest) - driveLabel := "" - if drive := m.GetAppDrivePath(stackName); drive != "" && drive != m.systemDataPath && m.settings != nil { - driveLabel = m.settings.GetStorageLabel(drive) - } - return []RestorePoint{{ Time: newest.UTC().Format(time.RFC3339), ShortID: restorePointShortID, Tier: 1, - DriveLabel: driveLabel, + DriveLabel: m.sysDriveLabelFor(stackName), }}, true } diff --git a/controller/internal/backup/tier2.go b/controller/internal/backup/tier2.go index 941150c..beac6b1 100644 --- a/controller/internal/backup/tier2.go +++ b/controller/internal/backup/tier2.go @@ -206,19 +206,25 @@ func (m *Manager) RunAllTier2() { } var n int for _, stack := range m.stackProvider.ListDeployedStacks() { - if m.stackProvider.GetStackHDDPath(stack.Name) == "" { - continue // not an HDD app — its data is on the rootfs, covered by PBS - } + // F6 (CAMPAIGN-3): volume-only apps (no HDD_PATH, backups on sys_drive) previously got NO + // tier-2 copy — a single controller-level copy on one device. They now flow through too: their + // recovery unit (which holds the db/volume dumps) gets a cross-drive second copy like any HDD + // app. Apps with no recovery unit yet (infra/never-backed-up) are a cheap no-op inside RunTier2 + // (the `os.Stat(unitDir)` guard), so this doesn't spuriously copy protected stacks. if m.settings != nil && (m.settings.IsDisconnected(m.GetAppDrivePath(stack.Name)) || m.settings.IsDecommissioned(m.GetAppDrivePath(stack.Name))) { continue } - if err := m.RunTier2(stack.Name); err != nil { + runOne := m.perAppTier2 + if runOne == nil { + runOne = m.RunTier2 + } + if err := runOne(stack.Name); err != nil { m.logger.Printf("[WARN] [backup] Tier 2 failed for %s: %v", stack.Name, err) } n++ } - m.logger.Printf("[INFO] [backup] Tier 2 run complete: %d HDD app(s) processed", n) + m.logger.Printf("[INFO] [backup] Tier 2 run complete: %d app(s) processed (incl. volume-only — F6)", n) } // --- per-app config-panel view (drives the Tier-2 "Beállítás" page) --- diff --git a/controller/internal/backup/volume_atomic_test.go b/controller/internal/backup/volume_atomic_test.go new file mode 100644 index 0000000..66009ef --- /dev/null +++ b/controller/internal/backup/volume_atomic_test.go @@ -0,0 +1,119 @@ +package backup + +import ( + "io" + "log" + "os" + "path/filepath" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" +) + +// atomicTestManager wires a Manager whose GetAppDrivePath resolves to a single drive dir, with the +// F7 tar seam injected. Returns the manager + the volume-dump dir where `.tar`/`.tar.tmp` land. +func atomicTestManager(t *testing.T, stack, volName string, tar func(volName, dumpDir string) ([]byte, error)) (*Manager, string) { + t.Helper() + drive := t.TempDir() + cfg := &config.Config{} + cfg.Paths.SystemDataPath = drive + fake := &volDumpFakeProvider{ + stacks: []StackSummary{{Name: stack}}, + volumes: map[string][]string{stack: {volName}}, + } + m := &Manager{cfg: cfg, logger: log.New(io.Discard, "", 0), + systemDataPath: drive, stackProvider: fake, tarVolume: tar} + dumpDir := AppVolumeDumpPath(m.namespaceRoot(drive), stack) + return m, dumpDir +} + +// Happy path: the tar seam writes a good `.tar.tmp`; DumpAppVolumes promotes it to `.tar`, leaves no +// `.tmp`, and the content is exactly what the seam wrote. +func TestDumpAppVolumes_HappyAtomicPromote(t *testing.T) { + const vol = "app_data" + want := []byte("GOOD-TAR-CONTENT") + m, dumpDir := atomicTestManager(t, "app", vol, func(_, dir string) ([]byte, error) { + return nil, os.WriteFile(filepath.Join(dir, vol+".tar.tmp"), want, 0644) + }) + + if err := m.DumpAppVolumes("app"); err != nil { + t.Fatalf("DumpAppVolumes: %v", err) + } + got, err := os.ReadFile(filepath.Join(dumpDir, vol+".tar")) + if err != nil { + t.Fatalf("final .tar missing: %v", err) + } + if string(got) != string(want) { + t.Errorf("final .tar content = %q, want %q", got, want) + } + if _, err := os.Stat(filepath.Join(dumpDir, vol+".tar.tmp")); !os.IsNotExist(err) { + t.Errorf("a `.tar.tmp` was left behind after a successful dump") + } +} + +// THE F7 RED-PROOF: a good `.tar` already exists (the last restore point). The tar seam fails +// mid-write (writes a partial/0-byte `.tar.tmp` then errors, simulating a NFS cut). The ORIGINAL +// `.tar` MUST be byte-identical afterwards, and no 0-byte `.tar` may exist. Companion: revert +// DumpAppVolumes to the in-place `tar cf …/.tar` write → the marker is truncated → this fails. +func TestDumpAppVolumes_MidWriteFailurePreservesLastGood(t *testing.T) { + const vol = "app_data" + marker := []byte("LAST-GOOD-247-BYTE-DUMP-MARKER") // the campaign's 247-byte last-good dump, in spirit + + m, dumpDir := atomicTestManager(t, "app", vol, func(_, dir string) ([]byte, error) { + // Simulate a mid-write cut: partially write the tmp, then fail (as a dead NFS target would). + _ = os.WriteFile(filepath.Join(dir, vol+".tar.tmp"), []byte("PARTIAL-TRUNCATED"), 0644) + return []byte("tar: write error: Input/output error"), errTest + }) + // Pre-seed the last good restore point. + if err := os.MkdirAll(dumpDir, 0755); err != nil { + t.Fatal(err) + } + tarPath := filepath.Join(dumpDir, vol+".tar") + if err := os.WriteFile(tarPath, marker, 0644); err != nil { + t.Fatal(err) + } + + err := m.DumpAppVolumes("app") + if err == nil { + t.Fatal("DumpAppVolumes must report failure when the tar step fails") + } + + // The restore point is byte-identical — F7's whole point. + got, rerr := os.ReadFile(tarPath) + if rerr != nil { + t.Fatalf("the last good `.tar` was DESTROYED by a failed write (F7 regression): %v", rerr) + } + if string(got) != string(marker) { + t.Errorf("last good `.tar` was mutated by a failed write: got %q, want %q (F7 regression)", got, marker) + } + // No stray tmp and no 0-byte tar. + if _, err := os.Stat(filepath.Join(dumpDir, vol+".tar.tmp")); !os.IsNotExist(err) { + t.Errorf("the failed `.tar.tmp` was not cleaned up") + } + if info, _ := os.Stat(tarPath); info != nil && info.Size() == 0 { + t.Errorf("the restore point is now a 0-byte file (the exact F7 failure)") + } +} + +// A leftover `.tar.tmp` from a previously-killed run is cleaned up by the next dump and is never a +// restore point (its name ends `.tmp`, invisible to the `.tar` restore-point scan). +func TestDumpAppVolumes_LeftoverTmpCleaned(t *testing.T) { + const vol = "app_data" + m, dumpDir := atomicTestManager(t, "app", vol, func(_, dir string) ([]byte, error) { + return nil, os.WriteFile(filepath.Join(dir, vol+".tar.tmp"), []byte("new"), 0644) + }) + if err := os.MkdirAll(dumpDir, 0755); err != nil { + t.Fatal(err) + } + // A stale tmp for a DIFFERENT (removed) volume — must be swept. + stale := filepath.Join(dumpDir, "old_removed_vol.tar.tmp") + if err := os.WriteFile(stale, []byte("stale"), 0644); err != nil { + t.Fatal(err) + } + if err := m.DumpAppVolumes("app"); err != nil { + t.Fatalf("DumpAppVolumes: %v", err) + } + if _, err := os.Stat(stale); !os.IsNotExist(err) { + t.Errorf("a stale `.tar.tmp` was left behind (must be swept)") + } +} diff --git a/controller/internal/backup/volume_dumps_test.go b/controller/internal/backup/volume_dumps_test.go index 0b5a66a..0582d0b 100644 --- a/controller/internal/backup/volume_dumps_test.go +++ b/controller/internal/backup/volume_dumps_test.go @@ -30,8 +30,8 @@ func (f *volDumpFakeProvider) StopStack(name string) error { f.stopped = append(f.stopped, name) return nil } -func (f *volDumpFakeProvider) StartStack(string) error { return nil } -func (f *volDumpFakeProvider) RefreshAndIsRunning(string) bool { return true } +func (f *volDumpFakeProvider) StartStack(string) error { return nil } +func (f *volDumpFakeProvider) RefreshAndIsRunning(string) bool { return true } func (f *volDumpFakeProvider) GetStackRecoveryInfo(string) (RecoveryInfo, bool) { return RecoveryInfo{}, false } @@ -71,9 +71,9 @@ func TestRunVolumeDumps_GatesPrecedeDump(t *testing.T) { }, volumes: map[string][]string{ "nextcloud": {"nextcloud_nextcloud_html"}, - "rallly": nil, // volume-less — must NOT be stopped/dumped - "traefik": {"traefik_data"}, // protected — never considered - "diskapp": {"diskapp_data"}, // volume-bearing but drive disconnected + "rallly": nil, // volume-less — must NOT be stopped/dumped + "traefik": {"traefik_data"}, // protected — never considered + "diskapp": {"diskapp_data"}, // volume-bearing but drive disconnected }, hdd: map[string]string{"nextcloud": usbDrive, "diskapp": badDrive}, } diff --git a/controller/internal/web/templates/backups.html b/controller/internal/web/templates/backups.html index e3638e2..b9efcb8 100644 --- a/controller/internal/web/templates/backups.html +++ b/controller/internal/web/templates/backups.html @@ -12,6 +12,9 @@ {{if .Backup}}{{if .Backup.FlashError}}
{{.Backup.FlashError}}
{{end}}{{end}} +{{if .Backup}}{{if .Backup.SingleCopyWarning}} +
{{.Backup.SingleCopyWarning}}
+{{end}}{{end}}