diff --git a/CHANGELOG.md b/CHANGELOG.md index 24ab48f..3ace9c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,42 @@ ## Changelog +### UNRELEASED — E-2 Part 1: the backup-target role (foundation; NOT yet wired to a UI) + +**Status: foundation only. No version bump — nothing customer-visible changes yet.** The field is +written by `SetBackupTarget` and read by `BackupTargetPath`/`BackupTargetAssigned`, and by nothing +else. **The offer UI (Part 3), the degraded banner (Part 4) and the absent-target signal's +controller half (Part 5) are NOT in this commit** — tracked as E-2 in `OPEN-ITEMS.md` so this cannot +become a sixth "seam built but never wired" (the fifth, `NotifyStorageDisconnected`, was found by +E-2's own Phase 0 and is one of the things still to wire). + +`StoragePath` gains `BackupTarget bool` — the sibling role to `Schedulable`/`IsDefault`/`Kind`, +marking the drive the whole-guest vzdump is written to. + +**It is INTENT, not truth.** The authority is the agent's `backup.local_backup_target`; this records +what the customer ASSIGNED so the controller can render the state, notice the drive going absent, +and detect drift. Truth comes from the agent's `GET /backup/tiers`. + +Invariants, each pinned by a test asserting the CONSEQUENCE rather than the mechanism: + +- **A drive never acquires the role by appearing.** Registration does not set it; only an explicit + customer choice through `SetBackupTarget` does. Red-proofed: adding auto-elevation to + `AddStoragePath` fails `TestRegisteringDrivesNeverAssignsTheBackupTarget` with + `registering drives assigned the backup target "/mnt/hdd_1"`. +- **Exactly one carrier** — assigning moves the role rather than duplicating it. +- **Sticky** — a new, bigger, faster drive appearing does not steal an assigned target. +- **An absent target stays assigned.** Clearing on disconnect would be a silent retarget by + omission: the box would read "no target configured" instead of "your target drive is missing". +- **A network share is refused** — the role exists to survive a LOCAL disk failure, and a remote, + credential-bound share mounted at its own root (R-108) is a different risk model. + +Attributes may suggest and may refuse the absurd; they may never select. The reference hardware +settles it: demo-felhom's backup drive is an external **USB HDD**, and **both** demo boxes' drives +report `removable=0` — a transport rule would disqualify the reference drive, a removable rule would +find no candidate at all. + +Green gate: `go build` + `go vet` + `go test ./internal/{settings,web,quiesce}` all rc=0, run +separately from the commit. + ### v0.183.0 — C9-F1 + C9-F2: a restore that restored nothing, and a crash loop nobody saw (2026-07-28) Both are the same shape — the system reporting healthy while the customer is not — and both were diff --git a/controller/internal/settings/backup_target_role_test.go b/controller/internal/settings/backup_target_role_test.go new file mode 100644 index 0000000..cc7eb7c --- /dev/null +++ b/controller/internal/settings/backup_target_role_test.go @@ -0,0 +1,121 @@ +package settings + +import ( + "log" + "os" + "path/filepath" + "testing" +) + +func roleTestSettings(t *testing.T) *Settings { + t.Helper() + s, err := Load(filepath.Join(t.TempDir(), "settings.json"), log.New(os.Stderr, "", 0)) + if err != nil { + t.Fatalf("load: %v", err) + } + return s +} + +// SCENARIO C, the core of E-2 §3: a drive NEVER acquires the backup-target role by appearing. +// +// This is the invariant the whole role model exists for. Registration is the path a newly-plugged +// drive travels, and the failure it guards against is a USB stick silently becoming the destination +// of the whole-system backup. The test asserts the CONSEQUENCE (no target is assigned after +// registering drives) rather than the mechanism, so it still fails if a future caller sets the field +// through some other route in the registration path. +func TestRegisteringDrivesNeverAssignsTheBackupTarget(t *testing.T) { + s := roleTestSettings(t) + for _, p := range []string{"/mnt/hdd_1", "/mnt/usb_stick", "/mnt/nvme-1tb"} { + if err := s.AddStoragePath(StoragePath{Path: p, Label: p, Schedulable: true}); err != nil { + t.Fatalf("add %s: %v", p, err) + } + } + if got := s.BackupTargetPath(); got != "" { + t.Fatalf("registering drives assigned the backup target %q — a drive must never acquire a role "+ + "by appearing (E-2 §3); only an explicit customer choice may assign it", got) + } + if s.BackupTargetAssigned() { + t.Fatal("BackupTargetAssigned() is true after registration alone") + } +} + +// Exactly one target at a time — assigning a second must move the role, not duplicate it. Two paths +// both claiming to be the destination is not a cosmetic defect: the renderer and the absent-check +// would each pick a different one. +func TestBackupTargetIsExclusive(t *testing.T) { + s := roleTestSettings(t) + for _, p := range []string{"/mnt/a", "/mnt/b"} { + if err := s.AddStoragePath(StoragePath{Path: p, Schedulable: true}); err != nil { + t.Fatalf("add: %v", err) + } + } + if err := s.SetBackupTarget("/mnt/a"); err != nil { + t.Fatalf("set a: %v", err) + } + if err := s.SetBackupTarget("/mnt/b"); err != nil { + t.Fatalf("set b: %v", err) + } + var carriers []string + for _, sp := range s.GetStoragePaths() { + if sp.BackupTarget { + carriers = append(carriers, sp.Path) + } + } + if len(carriers) != 1 || carriers[0] != "/mnt/b" { + t.Fatalf("expected exactly one carrier (/mnt/b), got %v", carriers) + } +} + +// STICKINESS (E-2 §3): a new drive appearing must not move an assigned target. This is the other half +// of "never by appearing" — the first covers an unassigned box, this covers an assigned one. +func TestNewDriveDoesNotStealAnAssignedTarget(t *testing.T) { + s := roleTestSettings(t) + if err := s.AddStoragePath(StoragePath{Path: "/mnt/hdd_1", Schedulable: true}); err != nil { + t.Fatalf("add: %v", err) + } + if err := s.SetBackupTarget("/mnt/hdd_1"); err != nil { + t.Fatalf("set: %v", err) + } + // A bigger, faster, newer drive appears. It must change nothing. + if err := s.AddStoragePath(StoragePath{Path: "/mnt/nvme_huge", Schedulable: true}); err != nil { + t.Fatalf("add: %v", err) + } + if got := s.BackupTargetPath(); got != "/mnt/hdd_1" { + t.Fatalf("target moved to %q when a new drive appeared — the role is sticky", got) + } +} + +// An ABSENT target stays assigned. Clearing it on disconnect would be a silent retarget by omission: +// the box would read as "no target configured" (degraded) instead of "your target drive is missing" +// (alarm), and reconnecting would not restore the assignment. +func TestDisconnectDoesNotClearTheTarget(t *testing.T) { + s := roleTestSettings(t) + if err := s.AddStoragePath(StoragePath{Path: "/mnt/hdd_1", Schedulable: true}); err != nil { + t.Fatalf("add: %v", err) + } + if err := s.SetBackupTarget("/mnt/hdd_1"); err != nil { + t.Fatalf("set: %v", err) + } + if err := s.SetDisconnected("/mnt/hdd_1", true, nil); err != nil { + t.Fatalf("disconnect: %v", err) + } + if got := s.BackupTargetPath(); got != "/mnt/hdd_1" { + t.Fatalf("disconnect cleared the target (got %q) — an absent target must stay assigned and "+ + "alarm, never silently become 'unconfigured'", got) + } +} + +// A network share can never be the target: the role exists to survive a LOCAL disk failure, and a +// remote, credential-bound share mounted at its own root (R-108) is a different risk model. +func TestNetworkShareRefusedAsBackupTarget(t *testing.T) { + s := roleTestSettings(t) + if err := s.AddStoragePath(StoragePath{Path: "/mnt/felhom-drives/nas", Kind: StorageKindNetwork}); err != nil { + t.Fatalf("add: %v", err) + } + if err := s.SetBackupTarget("/mnt/felhom-drives/nas"); err == nil { + t.Fatal("a network share was accepted as the backup target") + } + if s.BackupTargetAssigned() { + t.Fatal("a refused assignment still set the role") + } +} diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index a825108..f34b50e 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -287,6 +287,28 @@ type StoragePath struct { // Kind discriminates a physical drive ("" / "drive") from a NAS network share ("network"). A network // share is bulk-media only and carries the fields below; the drive lifecycle does NOT apply to it. Kind string `json:"kind,omitempty"` + + // BackupTarget (E-2) marks the drive the WHOLE-GUEST vzdump is written to — the sibling role to + // Schedulable/IsDefault/Kind. + // + // THIS FIELD IS INTENT, NOT TRUTH. The authority is the agent's `backup.local_backup_target` in + // agent.json; this records which drive the customer ASSIGNED so the controller can (a) render the + // degraded/healthy state, (b) notice when the assigned drive goes absent, and (c) detect drift from + // what the agent is actually doing. Read the agent's `GET /backup/tiers` for truth; never assume + // this flag and the agent agree. + // + // THE RULE THIS EXISTS TO ENFORCE: a drive NEVER acquires this role by appearing. Nothing in the + // registration path sets it — only an explicit customer choice through SetBackupTarget does. + // Attributes (transport, removable, size) may SUGGEST a candidate and may refuse the absurd; they + // may never select one. The reference hardware settles it: demo-felhom's backup drive is an + // external USB HDD and BOTH demo boxes' drives report removable=0, so a transport rule would + // disqualify the reference drive and a removable rule would find no candidate at all. + // + // Exactly one path may carry it (SetBackupTarget clears the others), and it is STICKY: it does not + // move because a new drive appeared, and it is never cleared just because the drive is absent — + // an absent target must stay assigned and ALARM (E-2 Part 5), because silently retargeting is how + // a backup lands somewhere nobody expects. See [[storage-authz-redesign]] for the role vocabulary. + BackupTarget bool `json:"backup_target,omitempty"` // Network-storage descriptors (Kind=="network" only; mirror the agent A1 add request). NO password // is stored — the SMB credential is passed through to the agent at add-time and never persisted here. Protocol string `json:"protocol,omitempty"` // nfs | smb @@ -1018,6 +1040,75 @@ func (s *Settings) SetSchedulable(path string, schedulable bool) error { return fmt.Errorf("storage path %q not found", path) } +// SetBackupTarget assigns the whole-guest backup-target role to exactly one storage path, clearing it +// from every other. This is the ONLY writer of StoragePath.BackupTarget — registration must never set +// it (E-2 §3: a drive never acquires a role by appearing). +// +// Refusals, both structural rather than advisory: +// - a NETWORK share can never be the target. vzdump writes a multi-GB archive through the host, and +// the whole point of the role is surviving a local disk failure — a share that is itself remote, +// credential-bound and mounted at its own root (R-108) is a different risk model entirely. +// - a path that is not registered cannot hold a role. +// +// It deliberately does NOT refuse a DISCONNECTED path: reassigning to a drive that is currently absent +// is a legitimate recovery order ("this is the drive, go find it"), and the absent-target alarm is what +// covers the gap. Refusing here would instead force a silent retarget elsewhere. +func (s *Settings) SetBackupTarget(path string) error { + s.mu.Lock() + defer s.mu.Unlock() + idx := -1 + for i := range s.StoragePaths { + if s.StoragePaths[i].Path == path { + idx = i + break + } + } + if idx < 0 { + return fmt.Errorf("storage path %q not found", path) + } + if s.StoragePaths[idx].IsNetwork() { + return fmt.Errorf("a hálózati tárhely nem lehet a rendszermentés célja: %s", path) + } + for i := range s.StoragePaths { + s.StoragePaths[i].BackupTarget = i == idx + } + if s.log != nil { + s.log.Printf("[INFO] [settings] backup target assigned: %s", path) + } + return s.save() +} + +// ClearBackupTarget removes the role from every path, leaving the box with no assigned target (the +// degraded state). Separate from SetBackupTarget on purpose: "no target" is a real, nameable state +// that must be reachable deliberately, not an accident of passing an empty string to a setter. +func (s *Settings) ClearBackupTarget() error { + s.mu.Lock() + defer s.mu.Unlock() + for i := range s.StoragePaths { + s.StoragePaths[i].BackupTarget = false + } + if s.log != nil { + s.log.Printf("[INFO] [settings] backup target cleared — the box is now in the degraded (system-drive) state") + } + return s.save() +} + +// BackupTargetPath returns the assigned target's path, or "" when none is assigned (degraded). +func (s *Settings) BackupTargetPath() string { + s.mu.RLock() + defer s.mu.RUnlock() + for _, sp := range s.StoragePaths { + if sp.BackupTarget { + return sp.Path + } + } + return "" +} + +// BackupTargetAssigned reports whether ANY path carries the role. The negation is the degraded state +// the customer must be told about (E-2 Part 4). +func (s *Settings) BackupTargetAssigned() bool { return s.BackupTargetPath() != "" } + // SetStorageLabel updates the label for a storage path. func (s *Settings) SetStorageLabel(path, label string) error { s.mu.Lock()