From 1b6e31675b4d8034762e5777eefe5dd61080b5d3 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 19 Oct 2026 02:59:34 +0200 Subject: [PATCH] The local whole-guest tier keeps one restore point per day for 3 days: keep-daily instead of keep-last (R-1149, operator D17, unreleased) --- CHANGELOG.md | 11 ++++ cmd/felhom-agent/main.go | 7 +-- internal/backup/prune_test.go | 4 +- internal/config/backup_tiers_test.go | 32 +++++++++++ internal/config/config.go | 24 +++++++- internal/localapi/backup_cap_test.go | 86 +++++++++++++++++++++++++++- 6 files changed, 153 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14ee10a..4cbb91f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,17 @@ Committed, not released — the release and its vouch are the operator's. `deferUpload: false` → „json mode must defer the upload"; the `Upload:` line removed → „has no Upload". **Residual:** the customer's typed-back check after the claim is browser-only, so „confirmed" here means „claimed" — a customer who opens the code and closes the page without writing it down has replaced the package. +- **R-1149 — the local whole-guest tier keeps one restore point per day for 3 days** (operator decision D17, + 2026-10-19). The primary (local) tier's per-run prune is now `--prune-backups keep-daily=N` (N = + `local_backup_retention`, default 3) instead of `keep-last=N` — the same at most 3 archives, but a run of extra + backups inside one day replaces only that day's point, so with the R-973 cap a flood cannot push out the earlier + days' copies. Additional tiers keep their explicit `keep_last` (the weekly off-site „last 2" is two weeks). + `config.BackupTier.PruneSpec()` is the one place the spec is built (runDaemon used to format keep-last itself); the + „backup tier armed" log line names it. Tests `TestBackupTierPruneSpec_R1149_LocalKeepsOnePerDay` and the + consequence `TestBackupCap_R1149_LocalFloodKeepsOlderDays` (the agent's real spec, a Proxmox-shaped prune model, + three nights then a day-long flood → the two earlier nights survive). Red-proved: keep-last for the primary → + „primary tier prune = "keep-last=3", want "keep-daily=3"" and „holds 0 copies made before the compromise". No + config or sudoers change. - **R-973 — the local-API whole-guest backup route has a cadence cap.** Every successful run prunes its tier, so a compromised controller calling `POST /backup` in a loop pushed out every pre-compromise restore point (local keep-last 3; off-site via ep0's keep-last 2). Now (`internal/localapi/backup_cap.go`): a tier whose cadence is ≤ 24 h diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index d3d180c..1f714c3 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -1947,10 +1947,7 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St var tierRunners []*backup.BackupRunner var tierTargets []string for _, t := range backupTiers { - prune := "" - if t.KeepLast > 0 { - prune = fmt.Sprintf("keep-last=%d", t.KeepLast) - } + prune := t.PruneSpec() // primary: keep-daily=N (R-1149); an extra tier: keep-last=N; 0 → no prune // Pruning a PBS target is allowed ONLY for an additional tier with an explicit keep_last // (the primary's target AND retention both default, so it could prune the DR by accident). allowPBSPrune := !t.Primary && t.KeepLast > 0 @@ -1968,7 +1965,7 @@ func buildLocalAPIServer(cfg config.Config, px *proxmox.Client, store *backup.St Service: r, }) logger.Info("backup tier armed", "target", t.TargetID, "cadence", t.Cadence.String(), - "keep_last", t.KeepLast, "wait_timeout", t.WaitTimeout.String(), + "keep_last", t.KeepLast, "prune", prune, "wait_timeout", t.WaitTimeout.String(), "prune_pbs_allowed", allowPBSPrune, "primary", t.Primary) } // Guest data-drive passthrough (slice 10 P2): a root-CLI runner for the `pct set` bind + chown diff --git a/internal/backup/prune_test.go b/internal/backup/prune_test.go index 19aa819..de48799 100644 --- a/internal/backup/prune_test.go +++ b/internal/backup/prune_test.go @@ -93,7 +93,7 @@ func TestPrune_KeepLastClamp(t *testing.T) { t.Fatalf("keep-last must NEVER be < 1 (would prune the fresh backup), got %d for %d", b.KeepLast(), c.set) } } - if spec := (config.BackupConfig{}).PruneBackupsSpec(); spec != "keep-last=3" { - t.Fatalf("default PruneBackupsSpec = %q, want keep-last=3", spec) + if spec := (config.BackupConfig{}).PruneBackupsSpec(); spec != "keep-daily=3" { + t.Fatalf("default PruneBackupsSpec = %q, want keep-daily=3 (R-1149)", spec) } } diff --git a/internal/config/backup_tiers_test.go b/internal/config/backup_tiers_test.go index 54c419d..c5f9d32 100644 --- a/internal/config/backup_tiers_test.go +++ b/internal/config/backup_tiers_test.go @@ -180,3 +180,35 @@ func TestBackupTiers_WaitTimeoutIsPerTier(t *testing.T) { t.Fatalf("wait bounds are shared between tiers — the whole point is that they differ: %+v", tiers) } } + +// R-1149 (operator decision D17, 2026-10-19): the local whole-guest tier keeps one restore point per day for N days +// (`--prune-backups keep-daily=N`), not the newest N; an additional tier keeps its explicit count. +// +// COMPANION RED-PROOF (observed 2026-10-19): make PruneSpec return keep-last for the primary → this fails with +// `primary tier prune = "keep-last=3", want "keep-daily=3"`. Restored. +func TestBackupTierPruneSpec_R1149_LocalKeepsOnePerDay(t *testing.T) { + b := BackupConfig{ + LocalBackupTarget: "local", + LocalBackupRetention: 3, + ExtraTargets: []BackupTargetConfig{ + {TargetID: "felhom-pbs", CadenceSeconds: 604800, KeepLast: 2}, + {TargetID: "nas", CadenceSeconds: 86400}, + }, + } + tiers, _ := b.BackupTiers() + if len(tiers) != 3 { + t.Fatalf("want 3 tiers, got %d", len(tiers)) + } + for _, c := range []struct { + i int + want string + }{{0, "keep-daily=3"}, {1, "keep-last=2"}, {2, ""}} { + if got := tiers[c.i].PruneSpec(); got != c.want { + t.Fatalf("%s tier prune = %q, want %q", map[bool]string{true: "primary", false: tiers[c.i].TargetID}[tiers[c.i].Primary], got, c.want) + } + } + // The unset retention defaults to 3 days, never to „prune everything". + if got := (BackupConfig{}).PruneBackupsSpec(); got != "keep-daily=3" { + t.Fatalf("default local prune = %q, want keep-daily=3", got) + } +} diff --git a/internal/config/config.go b/internal/config/config.go index 3218371..4afb776 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -391,7 +391,8 @@ type BackupConfig struct { // hub-served per-guest policy is slice 10; this is the agent-local cadence. BackupCadenceSeconds int `json:"backup_cadence_seconds"` - // LocalBackupRetention is keep-last=N for the per-run `--prune-backups` on a LOCAL vzdump target — + // LocalBackupRetention is N for the per-run `--prune-backups keep-daily=N` on a LOCAL vzdump target (keep-last=N + // before R-1149) — // so the agent's own local whole-guest backups can't pile up and refill root (the felhom-pve incident; // the host_disk + storage_fill checkers are the detectors, this is the preventive default). 0/unset → // default 3; ALWAYS clamped to ≥1 by KeepLast() so a mis-config can never prune the fresh backup. @@ -538,9 +539,26 @@ func (b BackupConfig) KeepLast() int { return b.LocalBackupRetention } -// PruneBackupsSpec returns the PVE `--prune-backups` value for the local vzdump (e.g. "keep-last=3"). +// PruneBackupsSpec returns the PVE `--prune-backups` value for the local vzdump: "keep-daily=N" (R-1149, operator +// decision D17 2026-10-19 — one restore point per day for N days; `07` §6.1). keep-last=N had the same disk cost but +// let N extra runs on one day push out every older point; keep-daily keeps the newest copy of each of the N newest +// days that have one, so a flood inside one day replaces only that day's point. Pinned by +// TestBackupTierPruneSpec_R1149_LocalKeepsOnePerDay and, as the consequence, TestBackupCap_R1149_LocalFloodKeepsOlderDays. func (b BackupConfig) PruneBackupsSpec() string { - return fmt.Sprintf("keep-last=%d", b.KeepLast()) + return fmt.Sprintf("keep-daily=%d", b.KeepLast()) +} + +// PruneSpec is this tier's per-run `--prune-backups` value, or "" for never prune. The PRIMARY (local) tier keeps one +// point per day (keep-daily, R-1149, from KeepLast() — always ≥1); an additional tier keeps its explicit keep_last +// count (a weekly off-site tier's „last 2" is two weeks, and PBS prunes stay opt-in per tier, R-82). +func (t BackupTier) PruneSpec() string { + if t.KeepLast <= 0 { + return "" + } + if t.Primary { + return fmt.Sprintf("keep-daily=%d", t.KeepLast) + } + return fmt.Sprintf("keep-last=%d", t.KeepLast) } // BackupCadence returns the per-guest /backup/due window: positive as-is, else 24h default. diff --git a/internal/localapi/backup_cap_test.go b/internal/localapi/backup_cap_test.go index 6037d53..59c0b7c 100644 --- a/internal/localapi/backup_cap_test.go +++ b/internal/localapi/backup_cap_test.go @@ -12,6 +12,7 @@ import ( "time" "gitea.dooplex.hu/admin/felhom-agent/internal/backup" + "gitea.dooplex.hu/admin/felhom-agent/internal/config" "gitea.dooplex.hu/admin/felhom-agent/internal/hub" ) @@ -25,11 +26,46 @@ type keepLastTier struct { mu sync.Mutex clock *testClock keepLast int + // spec, when set, is the `--prune-backups` value the agent passes (R-1149) and the tier prunes the way Proxmox + // reads it: keep-last=N → the newest N; keep-daily=N → the newest copy of each of the N newest days with one. + spec string + t *testing.T archives []time.Time fail bool runs int } +// pruneBySpec applies a Proxmox `--prune-backups` spec to archives (oldest first) and returns what is kept. +func pruneBySpec(t *testing.T, archives []time.Time, spec string) []time.Time { + var n int + var mode string + if _, err := fmt.Sscanf(spec, "keep-last=%d", &n); err == nil { + mode = "last" + } else if _, err := fmt.Sscanf(spec, "keep-daily=%d", &n); err == nil { + mode = "daily" + } else { + t.Fatalf("the model does not know the prune spec %q", spec) + } + var kept []time.Time + days := map[string]bool{} + for i := len(archives) - 1; i >= 0; i-- { // newest first, as Proxmox walks them + a := archives[i] + switch mode { + case "last": + if len(kept) < n { + kept = append([]time.Time{a}, kept...) + } + case "daily": + d := a.Format("2006-01-02") + if !days[d] && len(days) < n { + days[d] = true + kept = append([]time.Time{a}, kept...) + } + } + } + return kept +} + func (k *keepLastTier) BackupWithSnapshotHook(_ context.Context, vmid int, _ func()) (hub.Backup, error) { k.mu.Lock() defer k.mu.Unlock() @@ -39,7 +75,9 @@ func (k *keepLastTier) BackupWithSnapshotHook(_ context.Context, vmid int, _ fun return hub.Backup{VMID: vmid, StartedAt: now.Format(time.RFC3339)}, fmt.Errorf("vzdump failed") } k.archives = append(k.archives, now) - if len(k.archives) > k.keepLast { + if k.spec != "" { + k.archives = pruneBySpec(k.t, k.archives, k.spec) + } else if len(k.archives) > k.keepLast { k.archives = k.archives[len(k.archives)-k.keepLast:] } return hub.Backup{VMID: vmid, Success: true, Archive: "x", StartedAt: now.Format(time.RFC3339)}, nil @@ -291,3 +329,49 @@ func TestBackupCadenceCap(t *testing.T) { } } } + +// R-1149 — the consequence of keep-daily on the local tier, with the agent's REAL prune spec (config.BackupTiers → +// PruneSpec, what runDaemon hands the runner). Three honest nights, then a compromise at 01:00 that floods +// POST /backup for the rest of the day. The cap lets 3 runs through inside the rolling 24 h (two at once, one when +// last night's success ages out). With keep-last 3 those three were the only copies left; with keep-daily 3 the two +// earlier nights survive the whole day. +// +// COMPANION RED-PROOF (observed 2026-10-19): make BackupTier.PruneSpec return keep-last for the primary → this fails +// with "after a day of flooding the local tier holds 0 copies made before the compromise (kept […]) — want the 2 +// earlier nights". Restored. +func TestBackupCap_R1149_LocalFloodKeepsOlderDays(t *testing.T) { + tiers, _ := config.BackupConfig{LocalBackupTarget: "local", LocalBackupRetention: 3}.BackupTiers() + spec := tiers[0].PruneSpec() + day := time.Date(2026, 6, 10, 0, 0, 0, 0, time.UTC) + clock := &testClock{t: day} + local := &keepLastTier{clock: clock, keepLast: 3, spec: spec, t: t} + srv := capServer(t, filepath.Join(t.TempDir(), "s.json"), clock, local, &keepLastTier{clock: clock, keepLast: 2}) + for d := 3; d >= 1; d-- { // the nights of D-3, D-2, D-1 at 03:00 + clock.t = day.Add(-time.Duration(d)*24*time.Hour + 3*time.Hour) + if code := runBackup(t, srv, "local"); code != http.StatusAccepted { + t.Fatalf("seeding night D-%d: %d", d, code) + } + } + compromise := day.Add(time.Hour) + ran := 0 + for clock.t = compromise; clock.t.Before(day.Add(24 * time.Hour)); clock.Advance(10 * time.Minute) { + if runBackup(t, srv, "local") == http.StatusAccepted { + ran++ + } + } + if ran != 3 { + t.Fatalf("the flood ran %d backups on day D — want 3 (the cap)", ran) + } + local.mu.Lock() + kept := append([]time.Time(nil), local.archives...) + local.mu.Unlock() + pre := 0 + for _, a := range kept { + if a.Before(compromise) { + pre++ + } + } + if pre != 2 { + t.Fatalf("after a day of flooding the local tier holds %d copies made before the compromise (kept %v) — want the 2 earlier nights", pre, kept) + } +}