From 4663df7ff3cca8c4fc319f174bfb4f7454a08b68 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Fri, 31 Jul 2026 08:54:50 +0200 Subject: [PATCH] =?UTF-8?q?REPORT:=20agent=20v0.119.0=20=E2=80=94=20host?= =?UTF-8?q?=20addresses,=20deployed=20+=20published=20+=20vouched?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- REPORT.md | 162 ++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 108 insertions(+), 54 deletions(-) diff --git a/REPORT.md b/REPORT.md index 2913696..22f7135 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,76 +1,130 @@ -# REPORT — R-106 + R-109: the DR recipe stops guessing (agent v0.118.0, 2026-07-30) +# REPORT — the host report carries the box's addresses (agent v0.119.0, 2026-07-31) -**Overwritten** per the standing rule. Paired with **hub v0.83.0** — neither half is useful alone, see §5. +**Overwritten** per the standing rule. Pairs with **hub v0.85.0**; the agent half is useless alone. +Full live evidence: `felhom.eu/documentation/audits/host-addresses-visible-2026-07-31.md`. -## 1. What was wrong, confirmed live BEFORE the fix +## 1. Baselines -Downloaded from the hub, `GET /customers//dr-recipe.json`, both demo boxes: +| Repo | `main` @ start | Version before → after | +|---|---|---| +| felhom-agent | `6b5dade4dcc5` | **0.118.1 → 0.119.0** | -```json -"pbs": { "repo_id": "felhom-pbs", "namespace": "root", "latest_snapshot_id": "9201" } -``` +Clean-tree gate held before the build (`git status --porcelain` empty, `HEAD == origin/main`). -and **no `backup_target` key at all**, while `pve_storage` offered two plausible candidates: +## 2. What changed and why -```json -{ "name": "felhom-backup", "type": "local-dir", "content": "backup" } -{ "name": "local", "type": "local", "content": "backup,import,vztmpl,iso" } -``` +The hub could not show a managed box's LAN IP **anywhere**, because nothing reported one: +`HostMetrics` carried node/cpu/memory/disk/loadavg/uptime/temp/wrapper-sha and no address of any kind. -`/etc/pve/storage.cfg` on those same boxes says `namespace demo-felhom` and `namespace demo-hp`. +**Two plausible sources were checked and rejected before any code was written** — this is the +substantive part. `lan_resolver.host_ip` is an OPTIONAL config value, absent unless that feature is +configured. `DeriveHostIP(local_api.listen_addr)` returns `169.254.253.1`, because since the R-50 +island migration the local API binds a link-local address that is **byte-identical on every box**. +Either would have produced a confident wrong answer, which is worse than the blank it replaces. -## 2. Root causes, at source +New wire field **`addresses[]`**, one entry per (interface, address). Deliberately iface+cidr rather +than a single `lan_ip`: a Proxmox host legitimately holds several — a management bridge, a tailnet, +the WG tunnel — and picking one to call "the" LAN IP is a guess the agent is not entitled to make, +and is silently wrong on a box whose bridge is not `vmbr0`. The agent reports what exists; the hub +labels. -- **R-106** — `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 into the recipe. -- **R-109** — `BuildDRRecipeHostHalf` emitted `guests`/`drives`/`pve_storage`/`pbs` and nothing naming the - vzdump target. Benign until the 2026-07-28 target move gave every box a second `content=backup` dir - storage holding archives frozen at that date. +## 3. The filter was chosen by measurement, not reasoning -## 3. The fix +`ip -o addr show` on **both** demo boxes established that `IsGlobalUnicast()` is the whole rule: it +drops loopback, IPv6 link-local (`fe80::/10`, one per bridge) and IPv4 link-local (`169.254/16` — +exactly the island literal above). It needs **no veth/fwbr/tap denylist**, because on a Proxmox host +that per-guest plumbing carries no IP at all and self-excludes. -The namespace now resolves from the pbs **storage** (`StorageTarget.PBSNamespace`, from storage.cfg's -`namespace`) — the same field `vzdump --storage ` makes PVE read, so the recipe cannot disagree with -the backup that produced the snapshot. `backup_target` resolves from the **primary tier of -`cfg.Backup.BackupTiers()`** — the function the scheduler itself consults — and carries the mountpoint, -which is what actually separates `/mnt/hdd_1` from `/var/lib/vz`. - -**Unresolvable is recorded as unresolvable**, never defaulted: `namespace_state` and `backup_target.state` -are `resolved | unknown`, with distinct reasons (`agent_backup_config_unavailable`, `not_a_known_storage`, -`no_pbs_storage_observed`). An unconfigured namespace still reads `"root"` — that is an answer, not a gap. - -**The resolver reports the tier IN EFFECT, not the file on disk.** A backup-target move rewrites -`agent.json` and deliberately does not restart the agent (the E-1 lesson), so a disk re-read would name a -storage no archive had reached yet. +The unit fixtures are those two interface tables **transcribed verbatim, including the address-less +rows** — the no-denylist claim rests on those rows really being empty, and a fixture that omitted +them would have proved the claim by assuming it. ## 4. Files -`internal/hub/report.go`, `internal/storage/observe.go`, `internal/hub/dr_recipe.go`, -`internal/hub/collect.go`, `cmd/felhom-agent/main.go`, `internal/hub/testdata/host-report.golden.json`, -`REUSE.md`; tests in `internal/hub/{dr_recipe,contract,report}_test.go`, `internal/pbs/live_reporter_test.go`. +- `internal/hub/hostaddr.go` (**new**) — `HostAddress`, the `AddressEnumerator` seam, + `systemInterfaces`, `filterHostAddresses`, `Collector.collectAddresses` +- `internal/hub/hostaddr_test.go` (**new**) +- `internal/hub/report.go`, `internal/hub/collect.go` — the field + its population +- `internal/hub/contract_test.go`, `internal/hub/report_test.go`, + `internal/hub/testdata/host-report.golden.json` +- `CHANGELOG.md`, `REUSE.md` -## 5. The cross-repo half that is easy to miss +## 5. Commit -The hub's `AssembleDRRecipe` **allow-lists** top-level keys. `backup_target` would have been stored intact -and dropped before any operator saw it. The same mechanism already cost `offsite_restic` its whole -existence (**R-122**, found this session). `REUSE.md` now records this as a two-repo change. +`14642e3` — `v0.119.0 — the host report carries the box's addresses`, pushed to `main` (trunk-based). -## 6. Tests +## 6. Tests + red-proofs -9 new. Consequence-level: a box with two `content=backup` storages names the **live** one and not the -frozen one; `assertBackupCandidateAmbiguity` fails if the fixture ever stops posing that problem. Fixtures -are the storage set demo-felhom really had, provenance recorded in-file. Two tests run the real -`Collector.Collect()` path, one pinning that an **unwired seam yields unknown**. +Suite **874 → 883** (+9), `go build ./... && go vet ./... && go test ./...` **rc=0, 29 packages ok**. +Run as a command separate from the commit, per standing rule 1. -**4 red-proofs**, each mutation asserted to have landed before running: revert R-106 → the live `"root"` -symptom reproduces; make unknown guess a default → 5 assertions fire; drop the field → the cross-repo -contract guard fires; hollow the fixture → the ambiguity guard fires. +| # | Mutation | Observed | Reverted | +|---|---|---|---| +| 1 | drop the `!IsGlobalUnicast()` continue | `FAIL` — the island address and every `fe80::` leak through, on **both** host fixtures | yes | +| 2 | drop the `!i.Up` guard | `FAIL … a DOWN interface's address was reported` | yes | +| 3 | make `collectAddresses` return `[]HostAddress{}` (the inert-seam shape) | `FAIL … the collector did not run the filter over the enumerator's output` | yes | -Suite: **rc=0, 29 packages ok, 0 FAIL** (run separately from the commit). +**The repo's own no-nulls invariant caught the new field on the first run** — +`TestHostReport_FieldNamesAndEmptyCollections` failed because `addresses` marshalled as `null`. Fixed +by making the collector return a non-nil slice and setting `[]` in the fixture, matching how every +other collection is handled. That guard did its job unprompted. -## 7. Not touched +**The seam DEFAULTS to the real enumerator** (`addrEnum == nil` → `systemInterfaces`), deliberately +inverting the nil-reporter-means-off convention the optional stanzas use. Those gate on a config +feature; this one has no dependency and no flag, so a forgotten wiring call in `main.go` would have +shipped it silently empty — the inert-seam failure recorded four times in this project. -The backup machinery. This corrects the record, not the doing — and the record was never acted on -programmatically: the `host_loss` plan is surface-only (`internal/dr/plan.go:125`), so the blast radius was -exactly one operator reading a wrong value. `plan.PBS` inherits the fix for free. +## 7. Cross-repo contract + +`testdata/host-report.golden.json` is duplicated **byte-identically** in +`felhom.eu/hub/internal/api/testdata/`, and `TestHostReport_ContractMatchesGolden` fails on any +top-level key drift. Both goldens moved in this arc and were diffed to confirm they stayed identical; +`addresses[0]`'s key set is now asserted bidirectionally alongside the existing sections. + +## 8. Live validation + +**Method: the agent's own `--selftest=hub`**, which prints "the report it would send" — the wire +itself, not a proxy. On demo-felhom running the deployed 0.119.0: + +```json +"addresses": [ + { "iface": "tailscale0", "cidr": "100.70.170.35/32" }, + { "iface": "tailscale0", "cidr": "fd7a:115c:a1e0::5236:aa24/128" }, + { "iface": "vmbr0", "cidr": "192.168.0.162/24" }, + { "iface": "wg-felhom", "cidr": "10.77.0.2/32" } +] +``` + +Present: the LAN address. Absent: `lo`, `vmbr9`'s `169.254.253.1`, every `fe80::`. Matches the +`ip addr` ground truth exactly. + +## 9. Deploy + publish + vouch — all three, deliberately (R-115) + +| Leg | Result | +|---|---| +| Deployed `felhom-pve` (demo-felhom) | 0.118.1 → **0.119.0**, `systemctl is-active` → `active`, backup kept as `.bak-0.118.1` | +| Deployed `demo-hp` | 0.118.1 → **0.119.0**, `active`, backup kept | +| **Published** to Gitea | sha256 `e37aca824d0f57bb13b859a9a3cfcc472e84395d6f290ee90228b1933ab9fa43`, upload HTTP 201, **round-trip GET verified** | +| **Vouched** in the Day-0 manifest | read back: `agent=0.119.0`; golden `0.188.0`, `min_agent 0.113.0`, `wrapper_sha256` all preserved **verbatim** | + +`min_agent` deliberately **not** raised — `addresses[]` has no controller coupling, so holding the +controller floor for it would be an unnecessary fleet-wide write. + +**Trap hit and recorded:** the Gitea package listing sorted **lexicographically** ranks `0.96.0` above +`0.118.1`, which read as "nothing since 0.96.0 was ever published" — the exact shape R-120 recorded +for SQL `MAX()`. Re-sorted by semver before picking the version. Anyone auditing that endpoint by eye +will hit it too. + +## 10. Not covered + +- A box with **two LAN bridges** — handled by design (one row per interface) and unit-tested, but no + such box exists to observe. +- **IPv4-only / IPv6-only** hosts; every box here is dual-stacked via tailscale's ULA. +- No new privilege was taken: `net.Interfaces()` is a netlink/procfs read, so the sudoers fence is + untouched and the "a health check issues no block I/O" rule is honoured. + +## 11. Teardown + +This run provisioned nothing — no guest, no VM, no scratch customer. The only mutations are the two +in-place binary upgrades (previous binaries retained as `.bak-0.118.1` on both boxes) and the Day-0 +manifest vouch.