diff --git a/REPORT-r106-r109-recipe-completeness.md b/REPORT-r106-r109-recipe-completeness.md new file mode 100644 index 0000000..d29b277 --- /dev/null +++ b/REPORT-r106-r109-recipe-completeness.md @@ -0,0 +1,115 @@ +# REPORT — R-106 + R-109 (+ R-122): closing the recipe-completeness set (2026-07-30) + +Non-overwritten sibling per `CLAUDE.md:82-87` — the shared `REPORT.md` holds R-117 and is not touched. + +Shipped: **agent v0.118.0** (`felhom-agent` `1c8a67e`) + **hub v0.83.0**. Neither half is useful alone. + +## Part 0 — the answers, before the fix + +### 0.1 Which items are actually open, and R-105/R-106's registration + +`OPEN-ITEMS.md` calls itself "the single source of truth for open work" (`:1`), with `ROADMAP.md` keeping +"the full history and reasoning" (`:3-4`). + +| item | `ROADMAP.md` | `OPEN-ITEMS.md` | verdict | +|---|---|---|---| +| R-105 | row, `READY — 2026-07-28` (`:108`) | **absent** | **open but UNREGISTERED** | +| R-106 | row, `READY — 2026-07-28` (`:109`) | **absent** | **open but UNREGISTERED** | +| R-108 | row (`:111`) | row (`:50`) | registered | +| R-109 | row (`:112`) | row (`:61`) | registered | + +So R-109's own cell — "third recipe-completeness defect beside R-105/R-106" — was the **only** place in the +register naming two open items. That is exactly the thread-loss the register exists to prevent, and it is +itself a finding (filed **R-123**). Both now have rows. + +**The set this task closes is R-106 + R-109**, matching the arc's stated definition of done (`OPEN-ITEMS.md:14`). +**R-105 is NOT in it** and was not worked: it is M-sized and is about three *hub-held DR records* being `{}` +(`hosts.dr_record_json`, `host_escrow.directive_json`, and the `drives` third — already traced and populated +by the 2026-07-28 target move). Different fields, different owner, different size. + +### 0.2 Where the recipe is generated — three producers, not two + +| half | repo | function | +|---|---|---| +| host (guests/pbs/drives/pve_storage) | `felhom-agent` | `BuildDRRecipeHostHalf`, `internal/hub/dr_recipe.go:86` | +| app (customer/apps/offsite_restic) | `felhom-controller` | `controller/internal/report/dr_recipe.go` | +| **assembly + delivery** | `felhom.eu/hub` | `AssembleDRRecipe`, `internal/store/dr_recipe.go:104`; served by `handleDRRecipeDownload`, `internal/web/dr_recipe.go:14`, route `internal/web/server.go:439` | + +R-109's "host-half" is therefore the **agent**, and the field must also pass the **hub's** allow-list — see §2. + +### 0.3 What the namespace field actually contained — verified, and the brief was RIGHT + +The eleven-session-old brief held up. Live, pre-fix, from the hub for **both** boxes: + +```json +"pbs": { "repo_id": "felhom-pbs", "namespace": "root", "latest_snapshot_id": "9201" } +``` + +against `/etc/pve/storage.cfg` on the same boxes: + +``` +pbs: felhom-pbs + datastore felhom-offsite + namespace demo-felhom # demo-hp reads: namespace demo-hp +``` + +Traced to source: `Snapshot.Namespace` decodes `ns` (`internal/pbs/client.go:97`), which PBS does not echo +per item once the list is namespace-scoped via `?ns=` (`:118-120`) → always empty → `ToHub` normalises empty +to `"root"` (`internal/pbs/report.go:22-25`) → `latestPBSCoord` writes it in. + +**The authority taken, and why:** storage.cfg's `namespace` on the pbs storage. It is the same field +`vzdump --storage ` makes PVE read, and the agent's own verify client is built from it +(`cmd/felhom-agent/main.go:1164`). Deriving the recipe from anything else is how it drifts again. + +## 1. R-109's ambiguity is real, in the boxes' own pre-fix recipe + +```json +"pve_storage": [ + { "name": "local-lvm", "type": "lvmthin", "content": "images,rootdir" }, + { "name": "felhom-backup", "type": "local-dir", "content": "backup" }, + { "name": "felhom-pbs", "type": "pbs", "content": "backup" }, + { "name": "local", "type": "local", "content": "backup,import,vztmpl,iso" } +] +``` + +No `backup_target` key anywhere. `felhom-backup` (live, `/mnt/hdd_1`) and `local` (`/var/lib/vz`, archives +frozen 2026-07-28) are both `content=backup` dir storages; `local` is also the *historically* correct answer, +which is what makes guessing it so easy. + +## 2. R-122 — a fourth defect, found here, and it had already shipped + +`AssembleDRRecipe`'s `hostHalfShape`/`appHalfShape` are **allow-lists** dressed as forward-compat. The +controller has emitted `offsite_restic` since fork-4 (`controller/internal/report/dr_recipe.go:39-41`, "so DR +knows WHERE to recover from"); `appHalfShape` never listed the key. Verified both ways: + +- **stored**: `dr_recipe.app_half_json` carries it for all three real customers — + `peti-felhom`, `demo-felhom` (`u629488-sub1.your-storagebox.de:23/home/felhom-repo`), `demo-hp`. +- **delivered**: the downloaded recipe's top-level keys were + `recipe_version, customer, guests, pbs, drives, pve_storage, apps` — **no `offsite_restic`**. + +So a restorer reading the recipe had **no offsite location at all**, for the whole life of the feature, with +a green suite throughout — because the test fixture `drAppHalf` is hand-written and omits the field. + +**Deviation from the task's §7.10 ("Findings — filed as R-n, none fixed"), stated rather than absorbed:** +I fixed it. Reasons — (a) Part 0 authorises working the real set if it differs; (b) it is the same +symptom the task is named for (the recipe is incomplete), and the worst instance, a whole section missing; +(c) it is in the *same two structs* R-109 forced me to edit, and leaving one of three known keys off a +drop-list I was already correcting would be indefensible. It is filed as R-122 with a SHIPPED disposition. + +## 3. The before/after recipe — same box, quoted + +Full text in `documentation/audits/R106-R109-recipe-completeness-2026-07-30.md` §5. + +## 4. Findings filed (none of them fixed except R-122, see §2) + +| id | finding | +|---|---| +| **R-122** | `AssembleDRRecipe` allow-list dropped `offsite_restic` for the feature's whole life — **FIXED here**, hub v0.83.0 | +| **R-123** | R-105 and R-106 were `READY` in `ROADMAP.md` with no `OPEN-ITEMS.md` row — referenced only inside R-109's prose. Registered here | +| **R-124** | The recipe spells PBS's root namespace `"root"`, but the PBS API spells it `""` and there is no namespace literally named `root` — a restorer pasting it into `pct restore --ns root` would fail. Pre-existing wire convention, deliberately unchanged; documented at `PBSRootNamespace` | + +## 5. Not done, and why + +- **R-105, R-108, D5** — out of scope by the task's §6. R-108 blocks D5; starting either would leave both half-done. +- **The backup machinery** — untouched. This corrects the record, not the doing. +- **demo-felhom's agent was not upgraded past what the deploy step covers** — see the audit §6. diff --git a/REUSE.md b/REUSE.md index 6a76c0e..5ef2264 100644 --- a/REUSE.md +++ b/REUSE.md @@ -196,6 +196,7 @@ - **New site gate**: append to scripts/site_gates.py; new website pages go into its `PAGES` list. - **New artifact kind (Day-0)**: consts `pkg*`/`file*` (hub/internal/web/server.go ~L27), `ArtifactManifest` fields + settings keys (hub/internal/store/store.go ~L905), `handleSetArtifacts`, `artifactManifestResponse` (handler.go), and the install script's verify step. - **New host-install step**: `step_*` function in scripts/felhom-host-install.sh using `run()`/`fetch_raw`/`die` helpers; keep dry-run coverage. +- **New DR-recipe section**: `hostHalfShape`/`appHalfShape` **and** `AssembledRecipe` (hub/internal/store/dr_recipe.go) — those shape structs are **ALLOW-LISTS, not forward-compat**: a section only the emitter knows about is stored intact and **silently dropped** before the operator downloads it. No error, no log, no red test. That is R-122: the controller emitted `offsite_restic` from fork-4, all three real customers had it stored, and no delivered recipe ever contained it. Then extend `TestAssembleDRRecipe_CarriesEveryEmittedSection` (same commit) and, for a host-half section, the agent's `DRRecipeHostHalf` + BOTH copies of `testdata/host-report.golden.json` (byte-identical, cross-repo). ## 6. Known duplication (observed — NOT fixed) diff --git a/documentation/backlog/OPEN-ITEMS.md b/documentation/backlog/OPEN-ITEMS.md index c421081..ede7aa7 100644 --- a/documentation/backlog/OPEN-ITEMS.md +++ b/documentation/backlog/OPEN-ITEMS.md @@ -58,7 +58,11 @@ State: `BLOCKED` · `READY` · `WAITING-ON-OPERATOR` · `WATCHING`. Every row ha | **E-2b** | ~~`NotifyStorageDisconnected`/`Reconnected` defined and called NOWHERE — a drive going absent emitted no event on any channel~~ | **SHIPPED + PROVEN-LIVE** (controller v0.184.1 + agent v0.112.0 + hub v0.81.0, 2026-07-29) | — | Seam wired in `ReconcileDriveGates`; a target drive raises the specific `backup_target_absent` instead. **A keying bug was caught before deploy:** `a.Path` is the registered GUEST path, not the agent's host `MountPath`, so the target branch was unreachable — every absent drive, target included, fell through to the generic event (v0.184.1). Tests observe the WIRE (httptest hub), not a mock | — | | **E-2c** | ~~E-1 put the whole-guest backups on a drive `POST /disks/eject` would eject~~ | **SHIPPED + PROVEN-LIVE** (agent v0.112.0, 2026-07-29) | — | Eject + decommission refuse 409 on the backup-target mount, naming the storage and the remedy. **Live on BOTH boxes:** demo-hp `/mnt/nvme-1tb` and demo-felhom `/mnt/hdd_1` both refused, drives unmoved. NOT a role reclassification — `RoleForStorage` untouched, because on both boxes that drive is ALSO the enrolled user-data drive; `TestEjectStillAllowedOnANonTargetDrive` pins the non-over-correction and `/var/lib/vz` is still refused by the PRE-EXISTING role gate, not this one | — | | **PETI** | **`peti-felhom` deliberately NOT migrated.** Its whole-guest backup still shares a device with its guest, so a drive failure there is **offsite-only recovery** | **ACCEPTED RISK — parked** | operator's next visit (tester reinstalling from scratch) | **Accepted until the reinstall; re-evaluate if that slips past ~2026-09-01.** Do not migrate, do not touch | operator | -| **R-109** | **The DR recipe records no backup target.** It lists every storage's name/type/content but never which one holds the local archives — and each demo box now carries TWO `content=backup` dir storages, `felhom-backup` (live) and `local` (frozen 2026-07-28 archives) | READY (XS) | — | Add the resolved `BackupTarget()` to the host-half. Third recipe-completeness defect beside R-105/R-106 | CC | +| **R-109** | ~~The DR recipe records no backup target~~ | **SHIPPED + PROVEN-LIVE** (agent v0.118.0 + hub v0.83.0, 2026-07-30) | — | `backup_target` resolves from the PRIMARY tier of `cfg.Backup.BackupTiers()` — the function the scheduler consults, not a re-derivation — plus the mountpoint, which is what actually separates `/mnt/hdd_1` from `/var/lib/vz`. Three states, and unresolvable is recorded as unresolvable (`agent_backup_config_unavailable` / `not_a_known_storage`), never a default. **The resolver reads the daemon-start config on purpose:** a target move rewrites `agent.json` and deliberately does NOT restart, so a disk re-read would name a storage no archive had reached. **Needed a HUB half nobody had scoped** — `AssembleDRRecipe` allow-lists top-level keys, so the field would have been stored intact and dropped before any operator saw it (→ **R-122**). Evidence: `audits/R106-R109-recipe-completeness-2026-07-30.md` | — | +| **R-106** | ~~The DR recipe records the PBS namespace as `"root"` on every box~~ | **SHIPPED + PROVEN-LIVE** (agent v0.118.0, 2026-07-30) | — | **Was open-but-UNREGISTERED on this page until 2026-07-30 (→ R-123)** — `ROADMAP.md:109` had it READY and the only mention here was inside R-109's prose. Namespace now resolves from the pbs STORAGE (storage.cfg's `namespace`), the same field `vzdump --storage ` makes PVE read, so the recipe cannot disagree with the backup that produced the snapshot. An unconfigured namespace still reads `"root"` — that is an ANSWER, and `namespace_state` separates it from not knowing. Live: `demo-felhom` and `demo-hp` now report their own namespaces. Evidence: same audit | — | +| **R-122** | ~~`AssembleDRRecipe` silently DROPPED `offsite_restic` — the offsite recovery location never reached any recipe~~ | **SHIPPED** (hub v0.83.0, 2026-07-30) | — | **Found 2026-07-30 while scoping R-109's hub half; it had already shipped and nobody knew.** The controller has emitted `offsite_restic` since fork-4 (*"so DR knows WHERE to recover from"*), the hub stored it for **all three real customers**, and `appHalfShape` never listed the key — so no delivered recipe has ever contained it. No error, no log, green suite, because the fixture `drAppHalf` is hand-written and omits the field. `hostHalfShape`/`appHalfShape` are **ALLOW-LISTS dressed as forward-compat**; `TestAssembleDRRecipe_CarriesEveryEmittedSection` is now the guard, built on halves read verbatim out of the live `dr_recipe` table. `REUSE.md` (both repos) records that a recipe section is a TWO-REPO change | — | +| **R-123** | **R-105 and R-106 were `READY` in `ROADMAP.md` with no row on THIS page** — each referenced only inside R-109's prose, which is precisely the thread-loss the register exists to prevent | **PARTLY CLOSED** (2026-07-30) | — | R-106 registered above (and shipped). **R-105 still needs a row** — it is M-sized, is about three hub-held DR records being `{}`, and is NOT part of the recipe-completeness set that shipped today. **The process gap is the real item:** nothing checks that a `READY` ROADMAP row has an OPEN-ITEMS row. A grep-level gate would catch it | CC | +| **R-124** | **The recipe spells PBS's root namespace `"root"`, but the PBS API spells it `""`** and no namespace is literally named `root` — an operator pasting the field into `pct restore --ns root` gets a failure | READY (XS) | — | Pre-existing wire convention (`ToHub` has normalised empty→`"root"` since slice 6), deliberately NOT changed under R-106 so the field's meaning did not shift mid-fix. Documented at `hub.PBSRootNamespace`. Affects only a box with no `namespace` line — **no real customer today**, all three are per-customer. Fix = emit `""` + rely on `namespace_state`, or emit a `--ns`-ready form | CC | | **R-89** | Retention as a per-customer **commercial** policy on the hub | READY (increment 2) | — | Policy object + reconciler → ep0 prune job; keep box tokens write-only | CC | | **R-92** | Hub PBS-DR gauge is 0.1 GB-granular — small deltas unverifiable | READY (XS) | — | Widen precision when retention becomes customer-visible | CC | | **R-93** | `drill-r50` is both a blocked customer and the only drift fixture | READY (XS) | — | Retire it for a synthetic fixture, or unblock + silence per-customer | CC | diff --git a/documentation/backlog/ROADMAP.md b/documentation/backlog/ROADMAP.md index acc4eac..1732db4 100644 --- a/documentation/backlog/ROADMAP.md +++ b/documentation/backlog/ROADMAP.md @@ -106,10 +106,10 @@ | R-103 | **The Tier-2 no-coverage refusal names the working action but does not route to it.** v0.183.0 refuses up front without stopping the app and tells the customer to use „Visszaállítás indítása" on the other page; it does not take them there | S | READY — 2026-07-28 | **Was C9-F1b.** Deliberately its own item: it puts a DESTRUCTIVE operation (overwrites live data with the backup state) behind a button reached via a NON-destructive one, so the confirm copy must carry that difference. Flips: nothing until shipped; `07` §10.2 | | R-104 | **An interrupted offsite run leaves an exclusive restic lock the existing self-heal cannot reach.** `resticStep` has `unlock --remove-all` (`internal/backup/offbox.go:634-648`) but `ensureOffboxRepo`'s probe fails first, `classifyResticProbe` (`:77-93`) has no lock case → `"other"` → fail-fast; `ClassifyOffsiteFailure` likewise, so the operator is told *„A távoli mentés ismeretlen okból nem sikerült"* for a precisely-known, self-healable condition | S | READY — 2026-07-28 | **Was C9-F3.** Reachable by any interruption — container restart, OOM, network drop, host reboot mid-backup. The tier stays dead until a human runs `restic unlock --remove-all`. Flips: the offsite row in map §C; `07` §8 row 15 | | R-105 | **Three hub-held DR records are empty on the entire live fleet.** `hosts.dr_record_json` = `{}` on all 3 hosts; `host_escrow.directive_json` = `{}` on both escrowed hosts; `dr_recipe.host_half.drives` = `[]` on every customer **including two with enrolled data drives** (916 GB USB on demo-felhom, 938 GB NVMe on demo-hp) | M | READY — 2026-07-28 | These are exactly the fields a host-loss recovery reads: `05-hub-architecture.md:175-176,186` names the slim DR record as one of four durable sources; `06-offsite-connectivity.md:148-150` says the escrow upload carried the DR directive; `felhom-agent/internal/dr/plan.go:34-35` makes `PlannedDrive` the re-attach-by-`durable_id` wrong-disk guard. **The three may have different causes** — `isUserDataDrive` (`internal/hub/dr_recipe.go:129-136`) requires type `usb`/`local-dir` **and** a non-empty `DurableID` **and** `MountPath`, and which of the three fails was not traced. Evidence: `architecture/_recovery-inventory-2026-07-28.md` Part D2.3. **UPDATE 2026-07-28 (vzdump-target move): the `drives` third is TRACED and now POPULATED on both demo boxes.** Cause: the enrolled data drives were never PVE storages at all — only agent-generated systemd mounts — so they never entered `report.StorageTargets` and `isUserDataDrive` never saw them. Giving each drive a `dir` storage at its own mountpoint supplied all three required fields at once (type `local-dir`, fs-UUID durable id, mount path), and the recipe now emits `uuid:91d2dc2d-…`/`/mnt/nvme-1tb` on demo-hp and `uuid:47a3361a-…`/`/mnt/hdd_1` on demo-felhom. **The other two fields (`hosts.dr_record_json`, `host_escrow.directive_json`) are untouched and still `{}`** — this narrows R-105, it does not close it. See `runbooks/RUNBOOK-vzdump-target-move-2026-07-29.md` §5.4. Flips: `07` §4 | -| R-106 | **The DR recipe records the PBS namespace as `"root"` on every box** while the real namespaces are `demo-felhom` / `demo-hp` | XS | READY — 2026-07-28 | Traced to source: `Snapshot.Namespace` decodes a `ns` field (`felhom-agent/internal/pbs/client.go:97`) that PBS does not echo per item when the request is already namespace-scoped via `?ns=` (`:117-120`), so it is empty and `ToHub` normalises empty → `"root"` (`internal/pbs/report.go:23-27`), which `latestPBSCoord` writes into the recipe (`internal/hub/dr_recipe.go:149`). `latest_snapshot_id: "9201"` is **not** a defect — it is `BackupID`, documented as a coordinate (`:56`). Verified against ep0's real namespaces and a live `pct restore` command line carrying `--ns demo-felhom`. Flips: `07` §4 | +| R-106 | ~~The DR recipe records the PBS namespace as `"root"` on every box~~ | XS | **SHIPPED** — agent v0.118.0, 2026-07-30 | Namespace resolves from the pbs STORAGE (storage.cfg `namespace`) — the field `vzdump --storage ` makes PVE read — not from the listed snapshot, whose `ns` PBS omits once the list is `?ns=`-scoped. `namespace_state` distinguishes a genuine root namespace from not knowing. Live on both boxes. Follow-on: **R-124** (the `"root"` spelling is not `--ns`-pasteable). `audits/R106-R109-recipe-completeness-2026-07-30.md`; flipped `07` §4 | | R-107 | **No offsite action unpacks the named-volume tars Tier-3 captures on every run.** `ReconstituteFromOffsite` skips the unit outright (`internal/backup/offbox_reconstitute.go:284-289`); `PlaceOffsiteRestore` places it only when the live unit is ABSENT (`internal/backup/offbox_restore.go:352-356`). The DB **is** replayed, from the scratch unit's `db-dumps/` | M | READY — 2026-07-28 | 52 of 53 catalog templates keep data in Docker named volumes, so for most apps the offsite tier carries the volume data and cannot itself put it back. A two-step route exists (full restore → place → Tier-1 restore, which does unpack tars) but **no single action does it and no UI routes it** — the R-103 shape one tier over. Sibling of R-102: both are "captured and never read". Flips: the offsite-restore row in map §C; `07` §6.3, §7.2 | | R-108 | **Network storage can host an app's namespace, and FileBrowser binds a network share at its ROOT — this BLOCKS D5.** For local drives FileBrowser binds `/userdata` only (`internal/web/handlers.go:2450-2460`); for network paths it binds the share **root** with `:rslave` (`:2432`) and serves it with `download: true` (`internal/infra/infra.go:326`). Nothing stops an app's namespace landing there: `GetSchedulableStoragePaths()` has no `IsNetwork()` filter (`internal/settings/settings.go:904-914`, feeds the deploy dropdown at `handlers.go:462-473`), the per-app migrate target list has none (`handlers.go:674-679`), and `handleStorageMigrateApp` does not call `refuseNetworkLifecycle` though its whole-namespace sibling does (`internal/web/storage_handlers.go:397` vs `:410-424`) | M | **READY — blocks an architectural target** — 2026-07-28 | **Today this is not a secret leak** — the recovery unit's `app.yaml` is secret-stripped (`internal/backup/recovery_unit.go:73`). It becomes one under **D5**, which would put app secrets in the local unit precisely so Tier-1/Tier-2 restore stop needing the guest. Verified LIVE in demo-hp's generated compose, where the asymmetry is visible rather than inferred: `- /mnt/felhom-drives/nvme-1tb/userdata:/srv/nvme-1tb` beside `- /mnt/felhom-drives/Felhom-Share:/srv/Felhom-Share:rslave`. **Second effect, independent of D5:** `.fab` bundles already carry **plaintext** secrets by design with an OPTIONAL password (`internal/appexport/export.go:484,506-511,307`) and `storageDriveList()` does not filter network paths (`internal/web/handler_export.go:377-387`), so one can be exported onto a NAS today. Full sweep of every read surface: `07-backup-architecture.md` §10.1. Flips: `07` §7.3 (D5 target → adopted) once closed | -| R-109 | **The DR recipe records no backup target.** `BuildDRRecipeHostHalf` (`felhom-agent/internal/hub/dr_recipe.go:86`) emits `guests`, `drives`, `pve_storage` and the PBS coordinate, but nothing that says WHICH storage holds the local whole-guest archives. Harmless while the target was the well-known `local`; not harmless now — after the 2026-07-28 vzdump-target move each demo box carries **two** `content=backup` dir storages, `felhom-backup` (live) and `local` (frozen 2026-07-28 archives, never refreshed again) | XS | READY — 2026-07-28 | Found by `runbooks/RUNBOOK-vzdump-target-move-2026-07-29.md` Phase 0 §5 and confirmed live in the recipe both boxes emit. A restorer reading the recipe cannot distinguish the live target from the stale one, and picking wrong yields a silently outdated guest. Add the resolved `BackupTarget()` to the host-half. Sits with **R-105**/**R-106** as the third recipe-completeness defect. Flips: `07` §4 | +| R-109 | ~~The DR recipe records no backup target~~ | XS | **SHIPPED** — agent v0.118.0 + hub v0.83.0, 2026-07-30 | `backup_target` resolves from the PRIMARY tier of `cfg.Backup.BackupTiers()` (the scheduler's own function) + the mountpoint that separates `/mnt/hdd_1` from `/var/lib/vz`; unresolvable is recorded as unresolvable, never defaulted. Required an unscoped HUB half: `AssembleDRRecipe` allow-lists top-level keys → **R-122**, a fourth recipe-completeness defect that had already shipped (`offsite_restic` dropped for the feature's whole life). `audits/R106-R109-recipe-completeness-2026-07-30.md`; flipped `07` §4 | | E-2 | **Drive-role machinery around the moved vzdump target.** The 2026-07-28 runbook proved the architecture change by hand on both demo boxes; this is the machinery: a **backup-target role** on `StoragePath` beside `Schedulable`/`IsDefault`/`Kind`; **assignment in the storage wizard** (suggest by attribute, refuse the absurd, never decide by transport or `removable` — on the reference hardware demo-felhom's target IS a USB HDD and BOTH drives report `removable=0`); **unassigned drives do nothing automatically**; **stickiness** (never silently retarget); `felhom-host-install.sh` creating the target with `--is_mountpoint 1` **and** issuing the `FelhomAgentStore` ACL; **absent-target policy**; **retention/space accounting** on a drive the customer shares; the honest **single-drive label**; remaining fleet migration | M | READY — 2026-07-28 | Full scope + rationale in `runbooks/RUNBOOK-vzdump-target-move-2026-07-29.md` §7. Two traps already paid for live: the storage `path` must BE the mountpoint or the agent reports the target `disconnected` forever (`internal/storage/observe.go:321`), and the per-storage `FelhomAgentStore` grant is mandatory or every backup 403s. Absent-drive behaviour today is **fail-loudly, no silent retarget** (`is_mountpoint 1` proven live) — which is NOT the intended fall-back-and-alarm design. Flips: matrix row 4 | ## P3 — post-alpha diff --git a/hub/CHANGELOG.md b/hub/CHANGELOG.md index 69123b7..f434a93 100644 --- a/hub/CHANGELOG.md +++ b/hub/CHANGELOG.md @@ -1,3 +1,47 @@ +## v0.83.0 — R-109 + R-122: the recipe assembly stops dropping sections (2026-07-30) + +Pairs with **agent v0.118.0** (R-106 + R-109). The agent half is useless without this one. + +**`AssembleDRRecipe`'s two shape structs are ALLOW-LISTS, and nobody had noticed.** The doc comment sold +`hostHalfShape`/`appHalfShape` as forward-compat — "encoding/json drops any unknown top-level key" — which +is true and is also the trap: a section an emitter adds is **silently discarded** until it is named in both +the shape struct and `AssembledRecipe`. No error, no log, no failing test. The section is simply not in the +file the operator downloads. + +**R-122 (found this session) — that already happened, and it shipped.** The controller has emitted +`offsite_restic` since fork-4 — the offsite restic repo's non-secret coordinates, whose entire purpose is +"so DR knows WHERE to recover from". The hub stored it intact for **every real customer** +(`peti-felhom`, `demo-felhom`, `demo-hp` all carry it in `dr_recipe.app_half_json` today) and `appHalfShape` +never listed the key, so no delivered recipe has ever contained it. Verified both ways before the fix: the +stored half has it, `GET /customers/demo-felhom/dr-recipe.json` did not. + +**R-109 — and it would have happened again the same day.** The agent's new `backup_target` (which storage +holds the local whole-guest archives) is a new top-level host-half section. Without this commit it would +have been stored and dropped exactly like `offsite_restic`, and the R-109 fix would have read as shipped +while changing nothing an operator can see. + +Both keys are now on `hostHalfShape` / `appHalfShape` / `AssembledRecipe`, and the allow-list comment says +what it actually is, plus the rule: **adding a recipe section is a TWO-REPO change.** + +Tests: 3 new, all consequence-level and all built on the halves production **really stores** — read verbatim +out of the hub's own `dr_recipe` table (the pre-existing `drHostHalf`/`drAppHalf` constants are hand-written +and OMITTED `offsite_restic`, which is precisely why the drop stayed green for the feature's whole life). +`TestAssembleDRRecipe_CarriesEveryEmittedSection` enumerates every section both emitters produce and fails +on any that does not survive assembly — the guard the allow-list needed and never had. +`..._NamesTheLiveBackupTargetAmongTwoCandidates` asserts the delivered recipe names `felhom-backup` at +`/mnt/hdd_1` and not the frozen `local`, and refuses to run if the fixture stops posing that problem. +`..._UnknownBackupTargetSurvivesVerbatim` pins that the agent's explicit unknown reaches the operator AS an +unknown and does not acquire a `storage_id` on the way through. + +Red-proofs: 2, each mutation asserted to have landed before running — drop `offsite_restic` from the +allow-list (the R-122 defect restored) → 2 tests fail; drop `backup_target` → 4 fail, naming the section. +`go build` + `go vet` rc=0; suite rc=0, 17 packages, 0 FAIL. + +- `internal/store/dr_recipe.go` — `backup_target` + `offsite_restic` on both the shape structs and + `AssembledRecipe`; the allow-list warning. +- `internal/store/testdata/dr-recipe.golden.json` — both new sections + `namespace_state`. +- `internal/api/testdata/host-report.golden.json` — synced byte-identical with the agent's copy (`f4bc3554…`). + ## v0.82.0 — R-120: the vouch path refuses a golden the fleet has already outrun (2026-07-30) **The mechanism half of R-120.** The golden's version *is* the controller it bakes diff --git a/hub/internal/api/testdata/host-report.golden.json b/hub/internal/api/testdata/host-report.golden.json index 5943e36..3ba3c29 100644 --- a/hub/internal/api/testdata/host-report.golden.json +++ b/hub/internal/api/testdata/host-report.golden.json @@ -141,7 +141,8 @@ ], "pbs": { "repo_id": "felhom-pbs", - "namespace": "root", + "namespace": "felhom-spike", + "namespace_state": "resolved", "latest_snapshot_id": "9001" }, "drives": [ @@ -154,7 +155,13 @@ ], "pve_storage": [ { "name": "local-lvm", "type": "lvmthin", "content": "rootdir,images" }, - { "name": "usb-backup", "type": "usb", "content": "backup" } - ] + { "name": "usb-backup", "type": "usb", "content": "backup" }, + { "name": "felhom-pbs", "type": "pbs", "content": "backup" } + ], + "backup_target": { + "state": "resolved", + "storage_id": "usb-backup", + "mount_path": "/mnt/usb-backup" + } } } diff --git a/hub/internal/store/dr_recipe.go b/hub/internal/store/dr_recipe.go index ae5b968..dceaa49 100644 --- a/hub/internal/store/dr_recipe.go +++ b/hub/internal/store/dr_recipe.go @@ -79,22 +79,40 @@ type AssembledRecipe struct { PBS json.RawMessage `json:"pbs,omitempty"` Drives json.RawMessage `json:"drives,omitempty"` PVEStorage json.RawMessage `json:"pve_storage,omitempty"` - Apps json.RawMessage `json:"apps,omitempty"` + // BackupTarget (R-109) names WHICH storage holds the local whole-guest archives — the agent's + // host-half emits it from v0.118.0. + BackupTarget json.RawMessage `json:"backup_target,omitempty"` + Apps json.RawMessage `json:"apps,omitempty"` + // OffsiteRestic (R-122) is the offsite restic repo's non-secret coordinates — WHERE to recover from. + // The controller has emitted it since fork-4 and the hub dropped it for the whole time; see the + // allow-list warning on appHalfShape. + OffsiteRestic json.RawMessage `json:"offsite_restic,omitempty"` } // hostHalfShape / appHalfShape capture only the top-level keys the assembly stitches; encoding/json // drops any unknown top-level key (forward-compat — a newer half with extra sections still parses). +// +// THAT FORWARD-COMPAT IS ALSO A TRAP, and it has already cost one shipped section. These two structs are +// ALLOW-LISTS: a section an emitter adds is silently discarded here until it is named in BOTH the shape +// struct and AssembledRecipe. `offsite_restic` proved it — the controller emitted it from fork-4, the hub +// stored it intact for every customer, and the delivered recipe never contained it because nothing here +// listed the key. Nothing failed; the section simply was not there (R-122). +// +// SO: adding a section to either half is a TWO-REPO change. TestAssembleDRRecipe_CarriesEveryEmittedSection +// pins the current set against captured real halves — extend it in the same commit as any new section. type hostHalfShape struct { RecipeVersion int `json:"recipe_version"` Guests json.RawMessage `json:"guests"` PBS json.RawMessage `json:"pbs"` Drives json.RawMessage `json:"drives"` PVEStorage json.RawMessage `json:"pve_storage"` + BackupTarget json.RawMessage `json:"backup_target"` } type appHalfShape struct { RecipeVersion int `json:"recipe_version"` Customer json.RawMessage `json:"customer"` Apps json.RawMessage `json:"apps"` + OffsiteRestic json.RawMessage `json:"offsite_restic"` } // AssembleDRRecipe stitches the two stored halves into one operator-facing recipe. Either half may be @@ -112,6 +130,7 @@ func AssembleDRRecipe(rec *DRRecipe) (AssembledRecipe, error) { return out, err } out.Guests, out.PBS, out.Drives, out.PVEStorage = h.Guests, h.PBS, h.Drives, h.PVEStorage + out.BackupTarget = h.BackupTarget if h.RecipeVersion > out.RecipeVersion { out.RecipeVersion = h.RecipeVersion } @@ -122,6 +141,7 @@ func AssembleDRRecipe(rec *DRRecipe) (AssembledRecipe, error) { return out, err } out.Customer, out.Apps = a.Customer, a.Apps + out.OffsiteRestic = a.OffsiteRestic if a.RecipeVersion > out.RecipeVersion { out.RecipeVersion = a.RecipeVersion } diff --git a/hub/internal/store/dr_recipe_test.go b/hub/internal/store/dr_recipe_test.go index 205bafb..a5f633f 100644 --- a/hub/internal/store/dr_recipe_test.go +++ b/hub/internal/store/dr_recipe_test.go @@ -15,15 +15,50 @@ import ( const drHostHalf = `{ "recipe_version": 1, "guests": [ { "vmid": 9201, "cores": 4, "memory_bytes": 12884901888, "disk_bytes": 34359738368 } ], - "pbs": { "repo_id": "felhom-pbs", "namespace": "root", "latest_snapshot_id": "9201" }, + "pbs": { "repo_id": "felhom-pbs", "namespace": "root", "namespace_state": "resolved", "latest_snapshot_id": "9201" }, "drives": [ { "durable_id": "uuid:da9e7089-cf8e-4617-adcb-a377743fae00", "mount_path": "/mnt/felhom-usb", "intent": "enrolled", "total_bytes": 1000000000000 } ], - "pve_storage": [ { "name": "local-lvm", "type": "lvmthin", "content": "rootdir,images" }, { "name": "felhom-usb", "type": "usb", "content": "backup" } ] + "pve_storage": [ { "name": "local-lvm", "type": "lvmthin", "content": "rootdir,images" }, { "name": "felhom-usb", "type": "usb", "content": "backup" } ], + "backup_target": { "state": "resolved", "storage_id": "felhom-usb", "mount_path": "/mnt/felhom-usb" } }` const drAppHalf = `{ "recipe_version": 1, "customer": { "id": "cust-demo", "display": "Demo Customer", "domain": "demo-felhom.eu" }, - "apps": [ { "catalog_ref": "romm", "enabled": true, "storage_bindings": [ { "container_path": "/roms", "drive": "felhom-flash", "subpath": "userdata/roms" } ] } ] + "apps": [ { "catalog_ref": "romm", "enabled": true, "storage_bindings": [ { "container_path": "/roms", "drive": "felhom-flash", "subpath": "userdata/roms" } ] } ], + "offsite_restic": { "host": "u629488-sub1.your-storagebox.de", "user": "u629488-sub1", "port": 23, "repo_path": "/home/felhom-repo" } +}` + +// capturedHostHalf / capturedAppHalf are the halves demo-felhom's controller and agent REALLY stored, +// read verbatim out of the hub's own dr_recipe table on 2026-07-30 (customer_id='demo-felhom'), with only +// the app list truncated for length. They exist because drHostHalf/drAppHalf above are hand-written and +// OMIT sections production supplies — and that omission is exactly why R-122 survived: `offsite_restic` +// was stored for every real customer and silently dropped by the assembly, with a green suite throughout. +// +// capturedHostHalf carries the POST-FIX host half (agent v0.118.0: backup_target + namespace_state); the +// pre-fix one is quoted in the CHANGELOG entry for comparison. +const capturedHostHalf = `{ + "recipe_version": 1, + "guests": [ { "vmid": 9201, "cores": 4, "memory_bytes": 12884901888, "disk_bytes": 34359738368 } ], + "pbs": { "repo_id": "felhom-pbs", "namespace": "demo-felhom", "namespace_state": "resolved", "latest_snapshot_id": "9201" }, + "drives": [ { "durable_id": "uuid:47a3361a-91e0-4831-a69d-27f540ed3f48", "mount_path": "/mnt/hdd_1", "intent": "enrolled", "total_bytes": 983351140352 } ], + "pve_storage": [ + { "name": "local-lvm", "type": "lvmthin", "content": "images,rootdir" }, + { "name": "felhom-backup", "type": "local-dir", "content": "backup" }, + { "name": "felhom-pbs", "type": "pbs", "content": "backup" }, + { "name": "local", "type": "local", "content": "backup,import,vztmpl,iso" } + ], + "backup_target": { "state": "resolved", "storage_id": "felhom-backup", "mount_path": "/mnt/hdd_1" } +}` + +const capturedAppHalf = `{ + "recipe_version": 1, + "customer": { "id": "demo-felhom", "display": "Demo Ügyfél", "domain": "demo-felhom.eu" }, + "apps": [ + { "catalog_ref": "bookstack", "enabled": true, "storage_bindings": [] }, + { "catalog_ref": "immich", "enabled": true, "storage_bindings": [ + { "container_path": "/external/photos", "drive": "hdd_1", "subpath": "userdata/media/photos" } ] } + ], + "offsite_restic": { "host": "u629488-sub1.your-storagebox.de", "user": "u629488-sub1", "port": 23, "repo_path": "/home/felhom-repo" } }` // TestDRRecipe_StoreRoundTrip: each half upserts independently and preserves the other; GetDRRecipe @@ -196,6 +231,113 @@ func TestAssembleDRRecipe_NoSecrets(t *testing.T) { walk(v) } +// TestAssembleDRRecipe_CarriesEveryEmittedSection is the guard the allow-list needed and never had. +// +// It asserts the CONSEQUENCE — what an operator downloading the recipe actually receives — against the +// halves production really stores, not hand-written structs. Every section either emitter produces must +// survive to the assembled output. When the hostHalfShape/appHalfShape allow-lists and the emitters drift +// apart, this fails; that drift is what dropped `offsite_restic` for the entire life of the feature +// (R-122) and would have dropped `backup_target` on the day R-109 shipped. +func TestAssembleDRRecipe_CarriesEveryEmittedSection(t *testing.T) { + asm, err := AssembleDRRecipe(&DRRecipe{ + CustomerID: "demo-felhom", RecipeVersion: 1, + HostHalfJSON: capturedHostHalf, AppHalfJSON: capturedAppHalf, + }) + if err != nil { + t.Fatalf("captured real halves failed to assemble: %v", err) + } + b, _ := json.Marshal(asm) + var got map[string]any + if err := json.Unmarshal(b, &got); err != nil { + t.Fatal(err) + } + // Every section BOTH emitters produce today. A new section added to either half without a row here + // (and in the shape struct) is the defect this test exists to catch. + for _, section := range []string{ + "recipe_version", "customer", "guests", "pbs", "drives", "pve_storage", "backup_target", + "apps", "offsite_restic", + } { + if _, ok := got[section]; !ok { + t.Errorf("section %q was DROPPED by the assembly — the operator's recipe does not contain it "+ + "(add it to hostHalfShape/appHalfShape AND AssembledRecipe)", section) + } + } + // And the values, not just the keys: a present-but-empty section is the same gap wearing a hat. + if !jsonContains(t, asm.BackupTarget, "felhom-backup") || !jsonContains(t, asm.BackupTarget, "/mnt/hdd_1") { + t.Errorf("backup_target reached the operator without naming the live target: %s", asm.BackupTarget) + } + if !jsonContains(t, asm.OffsiteRestic, "u629488-sub1.your-storagebox.de") { + t.Errorf("offsite_restic reached the operator without the repo host: %s", asm.OffsiteRestic) + } + if !jsonContains(t, asm.PBS, "demo-felhom") { + t.Errorf("pbs coord lost the per-customer namespace: %s", asm.PBS) + } +} + +// TestAssembleDRRecipe_NamesTheLiveBackupTargetAmongTwoCandidates is the R-109 consequence at the DELIVERY +// boundary: the recipe an operator downloads for a box with two content=backup storages must name the live +// one unambiguously. Assembly-level, because a correct host half that the hub drops helps nobody. +func TestAssembleDRRecipe_NamesTheLiveBackupTargetAmongTwoCandidates(t *testing.T) { + asm, err := AssembleDRRecipe(&DRRecipe{HostHalfJSON: capturedHostHalf, AppHalfJSON: capturedAppHalf}) + if err != nil { + t.Fatal(err) + } + // The fixture must still pose the problem: two plausible content=backup storages. + var storages []struct{ Name, Type, Content string } + if err := json.Unmarshal(asm.PVEStorage, &storages); err != nil { + t.Fatal(err) + } + candidates := 0 + for _, s := range storages { + if strings.Contains(s.Content, "backup") && (s.Type == "local-dir" || s.Type == "local") { + candidates++ + } + } + if candidates < 2 { + t.Fatalf("fixture no longer poses the R-109 problem: %d content=backup dir storages", candidates) + } + + var bt struct { + State string `json:"state"` + StorageID string `json:"storage_id"` + MountPath string `json:"mount_path"` + } + if len(asm.BackupTarget) == 0 { + t.Fatal("backup_target absent from the delivered recipe — the restorer must still guess (R-109)") + } + if err := json.Unmarshal(asm.BackupTarget, &bt); err != nil { + t.Fatal(err) + } + if bt.State != "resolved" || bt.StorageID != "felhom-backup" || bt.MountPath != "/mnt/hdd_1" { + t.Errorf("delivered backup_target = %+v, want resolved/felhom-backup at /mnt/hdd_1", bt) + } + if bt.StorageID == "local" || bt.MountPath == "/var/lib/vz" { + t.Errorf("delivered recipe names the FROZEN target %q at %q", bt.StorageID, bt.MountPath) + } +} + +// TestAssembleDRRecipe_UnknownBackupTargetSurvivesVerbatim: the agent's explicit unknown must reach the +// operator AS an unknown. Passing it through as a resolved-looking section, or dropping it so the recipe +// merely lacks the field, would both turn "I could not tell" into something else. +func TestAssembleDRRecipe_UnknownBackupTargetSurvivesVerbatim(t *testing.T) { + const unknownHost = `{ + "recipe_version": 1, "guests": [], "drives": [], "pve_storage": [], + "backup_target": { "state": "unknown", "reason": "agent_backup_config_unavailable" } + }` + asm, err := AssembleDRRecipe(&DRRecipe{HostHalfJSON: unknownHost}) + if err != nil { + t.Fatal(err) + } + if !jsonContains(t, asm.BackupTarget, "unknown") || + !jsonContains(t, asm.BackupTarget, "agent_backup_config_unavailable") { + t.Errorf("the unknown state did not survive assembly: %s", asm.BackupTarget) + } + // It must not have acquired a target on the way through. + if strings.Contains(string(asm.BackupTarget), "storage_id") { + t.Errorf("assembly invented a storage_id for an unknown target: %s", asm.BackupTarget) + } +} + func jsonContains(t *testing.T, raw json.RawMessage, substr string) bool { t.Helper() return len(raw) > 0 && string(raw) != "null" && strings.Contains(string(raw), substr) diff --git a/hub/internal/store/testdata/dr-recipe.golden.json b/hub/internal/store/testdata/dr-recipe.golden.json index 42cd50c..0196b38 100644 --- a/hub/internal/store/testdata/dr-recipe.golden.json +++ b/hub/internal/store/testdata/dr-recipe.golden.json @@ -4,7 +4,7 @@ "guests": [ { "vmid": 9201, "cores": 4, "memory_bytes": 12884901888, "disk_bytes": 34359738368 } ], - "pbs": { "repo_id": "felhom-pbs", "namespace": "root", "latest_snapshot_id": "9201" }, + "pbs": { "repo_id": "felhom-pbs", "namespace": "root", "namespace_state": "resolved", "latest_snapshot_id": "9201" }, "drives": [ { "durable_id": "uuid:da9e7089-cf8e-4617-adcb-a377743fae00", @@ -17,6 +17,7 @@ { "name": "local-lvm", "type": "lvmthin", "content": "rootdir,images" }, { "name": "felhom-usb", "type": "usb", "content": "backup" } ], + "backup_target": { "state": "resolved", "storage_id": "felhom-usb", "mount_path": "/mnt/felhom-usb" }, "apps": [ { "catalog_ref": "romm", @@ -25,5 +26,11 @@ { "container_path": "/roms", "drive": "felhom-flash", "subpath": "userdata/roms" } ] } - ] + ], + "offsite_restic": { + "host": "u629488-sub1.your-storagebox.de", + "user": "u629488-sub1", + "port": 23, + "repo_path": "/home/felhom-repo" + } } diff --git a/manifests/hub.yaml b/manifests/hub.yaml index 970eb2e..a901fb5 100644 --- a/manifests/hub.yaml +++ b/manifests/hub.yaml @@ -125,7 +125,7 @@ spec: spec: containers: - name: hub - image: gitea.dooplex.hu/admin/felhom-hub:0.82.0 + image: gitea.dooplex.hu/admin/felhom-hub:0.83.0 ports: - containerPort: 8080 name: http