From 3c49dc8ea42df6c84a4bc9495d0d8a7662163ae4 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 31 Aug 2026 10:24:29 +0200 Subject: [PATCH] =?UTF-8?q?v0.228.0=20=E2=80=94=20the=20off-site=20check?= =?UTF-8?q?=20reads=20the=20data;=20the=20debug=20page=20stops=20lying=20(?= =?UTF-8?q?R-399=20+=20R-400)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit R-399: monitoring.integrity.read_data_subset defaults to 100%. A pack damaged without changing its size made plain `restic check` report "no errors were found" on demo-hp 2026-08-30; every read-data form caught it. Cost on that 134 MB store: 35.0s structure vs 39.2s at 100%. "off" (any case) is the off token; empty means not-configured, therefore the default; a malformed value falls back to the DEFAULT, never to structure. A completed check over 5 minutes logs a WARN naming the duration, the depth and R-401 — operator log only, no hub event, no depth change. The depth is now recorded with the verdict (LastIntegrityDepth; empty = NOT RECORDED, never "structure"). R-400: 24 debug-page references, 17 dispatched, 7 dead — three of which fetched on page LOAD, so those panels were permanently blank. backup/crossdrive implemented; backup/infra, hub/infra-push, dr/infra-status, storage/watchdog-status and both storage/simulate-* deleted with their panels and JavaScript. scripts/debug_route_gate.py fails in both directions and is registered after the seven were resolved. 18 referenced, 18 dispatched, none orphaned. Corrections: the dead-field warning in report/types.go said the controller runs no integrity check and the notifiers are called from nowhere — both false since v0.227.0. controller.yaml.example gains its missing integrity: block. integrityCheckTimeout's "ships OFF" comment rewritten. --- .claude/rules/gates.md | 10 +- CHANGELOG.md | 83 ++++++++ CONTEXT.md | 41 +++- REUSE.md | 1 + controller/README.md | 17 +- controller/cmd/controller/main.go | 4 +- .../cmd/controller/r399_no_event_test.go | 71 +++++++ controller/configs/controller.yaml.example | 15 ++ controller/internal/backup/offbox.go | 7 + .../internal/backup/offbox_integrity.go | 136 ++++++++++-- .../internal/backup/r359_integrity_test.go | 51 ++--- controller/internal/backup/r399_depth_test.go | 193 ++++++++++++++++++ .../internal/backup/r399_slow_notice_test.go | 139 +++++++++++++ controller/internal/config/config.go | 26 ++- controller/internal/report/types.go | 14 +- controller/internal/settings/settings.go | 9 + controller/internal/web/handler_debug.go | 68 ++++++ .../internal/web/r400_debug_routes_test.go | 147 +++++++++++++ controller/internal/web/templates/debug.html | 105 ---------- controller/scripts/controller_gates.py | 7 +- controller/scripts/debug_route_gate.py | 65 ++++++ controller/scripts/test_controller_gates.py | 1 + controller/scripts/test_debug_route_gate.py | 109 ++++++++++ 23 files changed, 1144 insertions(+), 175 deletions(-) create mode 100644 controller/cmd/controller/r399_no_event_test.go create mode 100644 controller/internal/backup/r399_depth_test.go create mode 100644 controller/internal/backup/r399_slow_notice_test.go create mode 100644 controller/internal/web/r400_debug_routes_test.go create mode 100644 controller/scripts/debug_route_gate.py create mode 100644 controller/scripts/test_debug_route_gate.py diff --git a/.claude/rules/gates.md b/.claude/rules/gates.md index 2786793..130b5ef 100644 --- a/.claude/rules/gates.md +++ b/.claude/rules/gates.md @@ -7,10 +7,12 @@ paths: ["controller/**/*.go", "controller/**/*.html", "controller/**/*.css", "co ## The ONE entry point **Run `python3 controller/scripts/controller_gates.py` (from `controller/`) after ANY change in this -repo.** It runs all seven local gates — `template_id_gate`, `emoji_gate`, `native_confirm_gate`, -`offbox_rename_gate`, `app_row_dedup_gate`, `mojibake_gate`, `docker_run_volume_path_gate` — plus -`reuse_refs_check` and `instructions_gate` on the repo root, streaming each gate's own output and -exiting non-zero if any fails. +repo.** It runs the local gates — `template_id_gate`, `emoji_gate`, `native_confirm_gate`, +`offbox_rename_gate`, `app_row_dedup_gate`, `mojibake_gate`, `docker_run_volume_path_gate`, +`secret_in_markup_gate`, `retrieval_promise_gate`, `debug_route_gate` — plus `reuse_refs_check`, +`instructions_gate` and `observations_gate` on the repo root, streaming each gate's own output and +exiting non-zero if any fails. **The runner's `GATES` table is the list; this sentence is a pointer to +it, not a second copy** — it has already drifted once (it said "seven" while nine were registered). - `--fast` selects the gates that touch no network and no container runtime; today that is all of them. - **A missing gate script is a FAILURE, never a skip.** diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ed8c1c..82d0430 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,86 @@ +## v0.228.0 — the check reads the data, and the debug page stops lying (2026-08-31, R-399 + R-400) +**MinAgent: 0.129.0** (unchanged) + +### The off-site check now re-reads the stored data (R-399) + +`monitoring.integrity.read_data_subset` **defaults to `100%`**. Every box whose `controller.yaml` has +no `integrity:` block — which is every box in the fleet — now downloads and re-hashes the whole +off-site store on its weekly check instead of reading only the catalogue. + +**The fact that forced it:** on `demo-hp`, 2026-08-30, a pack was damaged **without changing its +size**. Plain `restic check` — the structure-and-index check every box ran — reported +`no errors were found` and exited clean. Every `--read-data*` form caught it. A structurally-verified +store is one whose rot is found at restore time, with a customer waiting. + +**The cost**, same store and day (140 829 678 B / 2 651 blobs / 67 snapshots): structure 35.0 s, +10% 35.9 s, 50% 37.3 s, **100% 39.2 s**. + +- **`off` (any case) is the new off token.** Absent or empty means *not configured*, therefore the + default; a setting with no off switch is not a setting, and emptiness cannot mean both things. +- **A malformed value falls back to the DEFAULT, never to structure.** Downgrading on a typo would + silently remove the protection this row adds — R-357's shape, a guard that opens quietly. +- **A completed check over 5 minutes logs a WARN** naming the duration, the depth and R-401. Operator + log only: no hub event, no customer alarm, and it never changes the depth by itself. A skip or an + unreachable store never warns — neither has a duration to judge. The threshold is deliberately + imprecise (≈7.6× the only full-depth number that exists) because a notice changes no behaviour, + while a precise number invented from one measurement on one 134 MB store would not. +- **The depth is now recorded with the verdict** — `settings.OffboxTarget.LastIntegrityDepth` and + `OffboxReportStatus.LastIntegrityDepth`. Empty means NOT RECORDED (a pre-0.228.0 controller), never + "structure": absence means the box cannot answer, following the `StatsKnown` precedent beside it. +- **R-401 filed with a TRIGGER, not a date:** revisit the depth when the timing WARN fires on any box. + No rotation schedule, size threshold or bandwidth budget is built here — every one of those would be + a number invented from a single data point. + +**R-87 (the restic tier is never restore-tested) stays OPEN.** Reading the bytes back out of the store +is not a restore. + +### The debug page stops lying (R-400) + +The shipped debug page referenced **24** `/api/debug/...` addresses and the dispatcher answered **17**. +Seven controls did nothing — and three of those seven were not buttons at all: `dr/infra-status` and +both `storage/watchdog-status` calls fetch on page **load**, so whole panels had been permanently +blank and nobody had to click anything to be misled. This is the page an operator opens when something +is already wrong. + +| control | disposition | why | +|---|---|---| +| `backup/crossdrive` | **IMPLEMENTED** | `Manager.RunTier2` is live; only the route was missing | +| `backup/infra` | **DELETED** | the disk-tier infra backup moved to the host agent in slice 8C | +| `hub/infra-push` | **DELETED** | `Pusher.PushInfraBackup` was removed 2026-06-16 (it pushed plaintext secrets) | +| `dr/infra-status` | **DELETED** | it rendered the two retired mechanisms above; fetched on page load | +| `storage/watchdog-status` | **DELETED** | the slice-8C watchdog is retired; the drive-gate reconcile replaced it and publishes no such status. Fetched on page load, twice | +| `storage/simulate-disconnect` | **DELETED** | no backing capability, and it WRITES storage state — a button that fakes a drive disconnect on a customer's machine is a foot-gun | +| `storage/simulate-reconnect` | **DELETED** | same | + +Each deletion took its panel and its JavaScript with it; the "Tárhely teszt" section went entirely. +A panel left behind renders nothing forever, which is how this class hides. + +**`controller/scripts/debug_route_gate.py` makes the class impossible.** Two lists and a difference: it +fails when the template references an address the dispatcher lacks, **and** when the dispatcher answers +one nothing references. Registered in `controller_gates.py` **after** the seven were resolved — a +registered-but-failing gate refuses every push. Ten lines on purpose. Red-proofed in both directions. +Tree after the change: **18 referenced addresses, 18 dispatched, none orphaned.** + +### Three corrections + +- `internal/report/types.go` — the dead-field warning block said *"the controller runs no integrity + check, and `NotifyIntegrityOK`/`NotifyIntegrityFailed` … are called from nowhere"*. **Both halves + became false in v0.227.0.** The fields stay dead and unrendered (`TestBackupReport_DeadFieldsStayZero` + still passes unmodified); only the *reason* changed. +- `configs/controller.yaml.example` had **no `integrity:` block at all** — an operator could not + discover the setting exists. Added, with both keys, the default, the off token and the measurement. +- `internal/backup/offbox_integrity.go` — `integrityCheckTimeout`'s comment said read-data "ships OFF + … whoever turns it on must revisit this number". Rewritten: it is now the number a large store meets + first, and the slow notice exists to say so long before it does. + +### Superseded tests + +Two R-359 tests asserted the ruling this release reverses, and are replaced rather than weakened. +`TestR359_StructureCheckPassesNoReadDataFlag` → `TestR399_AbsentConfigRunsFullDepth`. +`TestR359_MalformedReadDataSubsetIsTreatedAsOff` → `TestR399_MalformedFallsBackToTheDefault` (its NAME +was the defect: treating a typo as "off" is the quiet downgrade). Both are recorded in place, so a +later reader does not re-derive the old ruling from an absence. + ## v0.227.1 — the damage classifier matched restic's ordinary progress output (2026-08-30, R-359 follow-on) **MinAgent: 0.129.0** (unchanged) diff --git a/CONTEXT.md b/CONTEXT.md index 0ee04dd..8f2beae 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,7 +7,46 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" -Last updated: 2026-08-30 (v0.227.1 — R-359/R-397: the off-site store gets checked) +Last updated: 2026-08-31 (v0.228.0 — R-399/R-400: the check reads the data, the debug page stops lying) + +> **2026-08-31 — v0.228.0. TWO RULINGS, recorded so neither is re-litigated.** +> +> **1. The off-site integrity check ships at FULL depth — `--read-data-subset=100%` — and `off` is the +> way back.** Viktor's ruling, 2026-08-31, taken on a measurement rather than a claim: on `demo-hp`, +> 2026-08-30, a pack damaged **without changing its size** made plain `restic check` report +> `no errors were found` and exit clean; every read-data form caught it. Cost on that store +> (140 829 678 B / 2 651 blobs / 67 snapshots): structure 35.0 s, 10% 35.9 s, 50% 37.3 s, 100% 39.2 s. +> +> Three consequences that are decided, not open: +> - **Empty means "not configured", therefore the default.** It does NOT mean off. `off` (any case) is +> the off token, and it exists because a setting with no off switch is not a setting. +> - **A malformed value falls back to the DEFAULT, never to structure.** Falling back to structure +> would silently remove the protection on a typo — R-357's shape, a guard that opens quietly. +> - **The default lives in `internal/backup/offbox_integrity.go`, NOT in `config.applyDefaults`.** Both +> integrity defaults are resolved in one accessor each, beside the argument that justifies them; +> symmetry with the other `Monitoring` defaults is worth less than that. +> +> **The thing a future session will get wrong: there is exactly ONE data point, on a 134 MB store.** +> The cost curves are governed by different quantities — structure tracks the index, read-data tracks +> the data — so nothing here extrapolates. That is why v0.228.0 ships a *notice* (a WARN over 5 minutes +> naming R-401) and NOT a rotation schedule, a size threshold or a bandwidth budget. Every one of those +> would be a number invented from one measurement, which is the shape of the four production designs +> this project has already specced against nothing. **R-401's trigger is that WARN firing on any box, +> not a calendar date.** +> +> **2. Implement or delete FIRST, register the gate SECOND.** R-400 found seven debug-page controls +> with no handler. The gate that makes that impossible (`controller/scripts/debug_route_gate.py`) was +> written and registered only after all seven were resolved — a registered-but-failing gate refuses +> every push, exactly as `instructions_gate` established. The gate is deliberately ten lines: two lists +> and a difference, in both directions, because a cleverer gate needs maintaining and an unmaintained +> gate is how the class hides in the first place. **Keep `handleDebugAPI`'s exact-match switch with its +> `NotFound` default** — a prefix match would have made the original defect invisible instead of merely +> silent. +> +> **The shape worth remembering is worse than "seven dead buttons":** three of the seven fetched on +> page LOAD, so those panels were permanently blank on the page an operator opens when something is +> already wrong. + > **2026-08-30 — v0.227.0/v0.227.1. THREE RULINGS, recorded so none is re-litigated.** > diff --git a/REUSE.md b/REUSE.md index 95313f6..d8c111c 100644 --- a/REUSE.md +++ b/REUSE.md @@ -310,6 +310,7 @@ Cross-repo edges: ## 5. Extension points (where new features plug in) - **New storage web endpoint**: switch in `ServeStorageAPI` (controller/internal/web/storage_handlers.go); disk ops in `ServeDiskAPI` (controller/internal/web/agent_disk_handlers.go); backup in `ServeBackupAPI`; export in `ServeExportAPI`; debug in `handleDebugAPI` (debug-mode gated). +- **New debug-page control (R-400, v0.228.0)**: the control in `controller/internal/web/templates/debug.html` AND the `subpath == "…"` case in `controller/internal/web/handler_debug.go` are ONE change — `controller/scripts/debug_route_gate.py` fails on either half alone, in both directions (a reference with no case, and a case with no reference). Keep the dispatcher's EXACT-match switch with its `http.NotFound` default; a prefix match is what would have hidden the original defect. Handler shape: `debugTriggerDBDump` for a fire-and-forget trigger, `debugRunIntegrityCheck` for one the operator pressed to learn an ANSWER (synchronous). **Before this gate the page referenced 24 addresses and 17 were answered, and three of the seven dead ones fetched on page LOAD** — those panels were permanently blank on the page an operator opens when something is already wrong. - **New REST endpoint**: path dispatch in `Router.ServeHTTP` (controller/internal/api/router.go); use `writeJSON` + `limitBody`. - **New background job**: `sched.Every`/`sched.Daily` registration block in controller/cmd/controller/main.go. - **New template function**: `Server.templateFuncMap` (controller/internal/web/funcmap.go) — obey v2 state-suffix vocabulary. diff --git a/controller/README.md b/controller/README.md index 4f56a8d..324e3d0 100644 --- a/controller/README.md +++ b/controller/README.md @@ -1099,11 +1099,13 @@ jobs, the tier holding the customer's documents and photos had none. |---|---| | job | `offsite-integrity`, `sched.Daily` at **06:00** | | cadence | **due-ness, not a weekday** — runs when the last SUCCESSFUL check is older than `monitoring.integrity.max_age_days` (default **7**). A box switched off on its check day is checked the next day it is on | -| depth | structure + index by default. `monitoring.integrity.read_data_subset` (default **empty**) adds `--read-data-subset=`; a malformed value is refused at read time with a WARN and treated as empty | +| depth | **`--read-data-subset=100%` by default since v0.228.0 (R-399)** — the check re-reads and re-hashes every stored byte, not just the catalogue. `monitoring.integrity.read_data_subset`: absent or empty = the default; **`off`** (any case) = structure and index only; any form restic accepts (`10%`, `1/7`, `50M`) = itself. A malformed value WARNs and falls back to **the default**, never to `off` — a typo must not quietly remove the protection | +| why full depth | **the structure check does not detect a size-preserving pack corruption.** Measured on `demo-hp` 2026-08-30: a pack was damaged without changing its size, plain `restic check` reported `no errors were found` and exited clean, every read-data form caught it. Cost on that 134 MB store: 35.0 s structure vs **39.2 s** at 100% | +| slow notice | a **completed** check over `integritySlowNoticeThreshold` (**5 min**) logs a WARN naming the duration, the depth and **R-401**. Operator log only — no hub event, no customer alarm, and it never changes the depth by itself. A skip or an unreachable store never warns: neither has a duration to judge | | guard | takes the single-writer flag and **SKIPS rather than waits** | | timeout | 30 min (`integrityCheckTimeout`) — bounds a hung repository so it cannot pin the flag | | by hand | `POST /api/debug/backup/integrity` — same code path, due-ness ignored, **every other guard intact** | -| result | persisted on `settings.OffboxTarget` (`last_integrity_check`, `last_integrity_ok`) and published on `OffboxReportStatus` | +| result | persisted on `settings.OffboxTarget` (`last_integrity_check`, `last_integrity_ok`, **`last_integrity_depth`** — v0.228.0) and published on `OffboxReportStatus`. Depth empty = NOT RECORDED (a pre-0.228.0 controller), never "structure" | **Three outcomes, not two.** `Skipped` (a sibling operation held the flag), `Unreachable` (the repo could not be opened, or the check timed out) and failed are different facts. Only a failure notifies; @@ -2859,7 +2861,9 @@ The Hub serves three asset types per app: ### 13. Debug Mode -When `logging.level: "debug"` is set in `controller.yaml`, the controller exposes a full diagnostic dashboard at `/debug` with 9 testing sections. All debug endpoints are gated — at `info` level, the sidebar link disappears and all `/api/debug/*` routes return 404. +When `logging.level: "debug"` is set in `controller.yaml`, the controller exposes a full diagnostic dashboard at `/debug`. All debug endpoints are gated — at `info` level, the sidebar link disappears and all `/api/debug/*` routes return 404. + +**R-400 (v0.228.0): the table below is now MECHANICALLY pinned to the dispatcher.** `controller/scripts/debug_route_gate.py` compares every `/api/debug/...` reference in `debug.html` against every `subpath ==` case in `handler_debug.go` and fails on either difference. Before it existed the page referenced 24 addresses and 17 were answered; three of the seven dead ones fetched on page LOAD, so whole panels had been permanently blank. Six controls were deleted and one (`backup/crossdrive`) implemented — the "Tárhely teszt" section went entirely, which is why the section numbers below skip 4. #### Debug Page Sections @@ -2867,12 +2871,11 @@ When `logging.level: "debug"` is set in `controller.yaml`, the controller expose |---|---------|-----------|-------------| | 1 | Rendszer diagnosztika | `GET /api/debug/dump` | Full state dump: controller info, storage, stacks, network (guest-netns interfaces/route/DNS via the samba door, R-66; best-effort per item), scheduler, health, alerts. JSON download. | | 2 | Értesítés teszt | `POST /api/debug/event/test`, `GET /api/debug/event/history` | Send test events with configurable type/severity, view event history ring buffer. | -| 3 | Mentés teszt | `POST /api/debug/backup/dbdump` · `POST /api/debug/backup/integrity` | Trigger a DB dump, or run an off-site integrity check by hand. **`crossdrive` and `infra` are NOT implemented** — their buttons 404 (R-400). | -| 4 | Tárhely teszt | `POST /api/debug/storage/simulate-{disconnect,reconnect}`, `GET /api/debug/storage/watchdog-status` | Simulate drive disconnect/reconnect without unmounting. Per-path probe state with 5s auto-refresh. | -| 5 | Hub & Kapcsolatok | `POST /api/debug/hub/{push,infra-push,test-connectivity,preferences-sync}`, `POST /api/debug/gitea/test-connectivity` | Test Hub/Gitea connectivity with latency. Push reports and sync preferences. | +| 3 | Mentés teszt | `POST /api/debug/backup/dbdump` · `POST /api/debug/backup/crossdrive` · `POST /api/debug/backup/integrity` | Trigger a DB dump, run the Tier-2 (cross-drive) sweep over every deployed HDD-backed app, or run an off-site integrity check by hand. `crossdrive` is asynchronous and answers with the app list it started for; `integrity` is synchronous and answers with the verdict. **`backup/infra` was DELETED (R-400)** — the disk-tier infra backup moved to the host agent in slice 8C and nothing in this repo backs it. | +| 5 | Hub & Kapcsolatok | `POST /api/debug/hub/{push,test-connectivity,preferences-sync}`, `POST /api/debug/gitea/test-connectivity` | Test Hub/Gitea connectivity with latency. Push reports and sync preferences. **`hub/infra-push` was DELETED (R-400)** — `Pusher.PushInfraBackup` was removed 2026-06-16. | | — | Telemetria teszt | `GET /api/debug/telemetry` | Run the full telemetry collection pipeline on-demand (metrics query + log scan). Returns per-app table: container list, memory current/avg/peak, CPU avg, catalog limit, log error/warning counts, and top issues. Useful for verifying container→stack mapping and testing log scanner patterns without waiting for the 15-minute report cycle. | | 6 | Önfrissítés teszt | `POST /api/debug/selfupdate/dry-run` | Dry-run update check: current vs new image lines, compose writability, backup state. | -| 7 | DR / Telepítő varázsló | `POST /api/debug/dr/trigger-setup`, `GET /api/debug/dr/infra-status` | Infra backup status per drive. Trigger setup mode via marker file (requires "RESET" + infra backup pre-check). | +| 7 | DR / Telepítő varázsló | `POST /api/debug/dr/trigger-setup` | Trigger setup mode via marker file (requires "RESET"). **`dr/infra-status` and its panel were DELETED (R-400)** — it rendered the two retired infra-backup mechanisms above, and it fetched on page LOAD, so the panel had been permanently blank. | | 8 | Naplóviewer | `GET /api/debug/logs?level=&limit=&after=`, `GET /api/debug/agent-logs` | In-memory log viewer (last 5000 entries, spill-persisted across restart — fix-6), level filter, 2s auto-refresh, color-coded entries. Two tabs (v0.116.0): **Vezérlő** (own ring) and **Ügynök** (the agent's always-DEBUG ring proxied over the local API; a pre-0.83 agent renders the "available after the agent's next update" notice). | #### Key Implementation Details diff --git a/controller/cmd/controller/main.go b/controller/cmd/controller/main.go index d1eeb6c..14ff0f7 100644 --- a/controller/cmd/controller/main.go +++ b/controller/cmd/controller/main.go @@ -3134,7 +3134,7 @@ func runOffsiteIntegrityCheck(ctx context.Context, mgr *backup.Manager, n *notif // one alarm that means the customer's backups are damaged. return res case res.OK: - mgr.RecordIntegrityOutcome(res.RanAt, true) + mgr.RecordIntegrityVerdict(res) // severity `info`, which severityNotifies DROPS before either leg — so this mails NOBODY, by // design (08 §6.1). A weekly success e-mail is how people stop reading their alerts. It is // still pushed, because the hub stores it and the event stream is where "was it checked?" is @@ -3142,7 +3142,7 @@ func runOffsiteIntegrityCheck(ctx context.Context, mgr *backup.Manager, n *notif n.NotifyIntegrityOK(integrityOKMsg(res)) return res default: - mgr.RecordIntegrityOutcome(res.RanAt, false) + mgr.RecordIntegrityVerdict(res) // A FAILING store advances due-ness too: re-checking a broken repository every night is load // with no new information, and the hourly operator cooldown already governs the mail. // diff --git a/controller/cmd/controller/r399_no_event_test.go b/controller/cmd/controller/r399_no_event_test.go new file mode 100644 index 0000000..be18a94 --- /dev/null +++ b/controller/cmd/controller/r399_no_event_test.go @@ -0,0 +1,71 @@ +package main + +import ( + "go/ast" + "go/parser" + "go/token" + "testing" +) + +// B6 — R-399/R-401: SLOWNESS RAISES NO HUB EVENT. This asserts a NON-EFFECT. +// +// A "this took a while" event type would cost the severity contract, the grain table and three +// registers, and 08 §6.2's coarse-by-default rule points the other way. Worse, this project's +// dispatcher coerces any severity outside {info,warning,error,critical} to `info` and mails nobody +// while still returning 200 — so an event added carelessly here would be indistinguishable from one +// that works. +// +// AST, not strings.Contains: a commented-out call still contains the string, and this repo has a +// recorded case of a text-based wiring test passing the very red-proof it existed to fail. +func TestR399_SlownessRaisesNoHubEvent(t *testing.T) { + body := funcBody(t, "runOffsiteIntegrityCheck") + + // Every notifier method the one integrity caller invokes. + var notifies []string + ast.Inspect(body, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return true + } + if len(sel.Sel.Name) >= 6 && sel.Sel.Name[:6] == "Notify" { + notifies = append(notifies, sel.Sel.Name) + } + return true + }) + + // POSITIVE CONTROL. If this list is empty the test proves nothing — it would pass just as happily + // against a file it failed to walk. + allowed := map[string]bool{"NotifyIntegrityOK": true, "NotifyIntegrityFailed": true} + if len(notifies) == 0 { + t.Fatal("no Notify* call was found in runOffsiteIntegrityCheck — the walk found nothing, so " + + "the non-effect below would be asserted against an empty set and would pass vacuously") + } + for _, name := range notifies { + if !allowed[name] { + t.Errorf("runOffsiteIntegrityCheck calls %s — slowness and depth are OPERATOR notices in "+ + "the log, and the only two customer-facing notifications this job may raise are the "+ + "integrity pass and the integrity failure", name) + } + } +} + +// funcBody returns the body of a named top-level function in main.go. +func funcBody(t *testing.T, name string) *ast.BlockStmt { + t.Helper() + fset := token.NewFileSet() + f, err := parser.ParseFile(fset, "main.go", nil, 0) + if err != nil { + t.Fatalf("parse main.go: %v", err) + } + for _, decl := range f.Decls { + if fn, ok := decl.(*ast.FuncDecl); ok && fn.Name.Name == name && fn.Body != nil { + return fn.Body + } + } + t.Fatalf("func %s not found in main.go", name) + return nil +} diff --git a/controller/configs/controller.yaml.example b/controller/configs/controller.yaml.example index 4a108d7..6f0fcf3 100644 --- a/controller/configs/controller.yaml.example +++ b/controller/configs/controller.yaml.example @@ -83,6 +83,21 @@ monitoring: # ping_uuids: (deprecated — monitoring is now handled by the Hub event system) system_health_interval: "5m" health_check_schedule: "06:00" + # Off-site (restic) store integrity check — R-359 / R-399. + integrity: + max_age_days: 7 # A MAX AGE, not a weekday: the job runs daily and asks "is the last + # successful check older than this?", so a box that was off on its check + # day is checked the next day it is on. + read_data_subset: "" # HOW DEEP the check looks. Empty or absent = the default, 100% — every + # stored byte is downloaded and re-hashed. Set "off" to check the + # structure and index only; set "10%", "1/7" or "50M" for a partial + # re-read. A value restic does not accept WARNs and falls back to the + # default, never to "off". + # The default is 100% because the structure check does NOT detect a + # size-preserving pack corruption: measured on a real store 2026-08-30, + # `restic check` reported "no errors were found" over a damaged pack that + # every read-data form caught. Cost on that 134 MB store: 35.0 s at + # structure depth, 39.2 s at 100%. thresholds: disk_warn_percent: 80 disk_crit_percent: 90 diff --git a/controller/internal/backup/offbox.go b/controller/internal/backup/offbox.go index 6abf57e..3f707aa 100644 --- a/controller/internal/backup/offbox.go +++ b/controller/internal/backup/offbox.go @@ -1469,6 +1469,12 @@ type OffboxReportStatus struct { // timestamp is the field that says whether the bool means anything at all. LastIntegrityCheck string `json:"last_integrity_check,omitempty"` // RFC3339 LastIntegrityOK bool `json:"last_integrity_ok,omitempty"` + // LastIntegrityDepth (R-399) is how deep that verdict looked — "structure", or the subset that was + // re-read ("100%"). THIRD field, same argument as the two above and as StatsKnown: a hub that shows + // "checked, OK" without the depth shows the same words for a check that re-read every byte and one + // that only read the index, and those are different news. Absent = the box cannot answer (a + // controller older than v0.228.0), never "structure". + LastIntegrityDepth string `json:"last_integrity_depth,omitempty"` } // OffsiteStateNeedsCredential is the ONE declared state (v0.199.0, R-204 item 4 / R-193): this box @@ -1662,6 +1668,7 @@ func (m *Manager) OffboxReportStatus() *OffboxReportStatus { StatsKnown: t.StatsKnown, // R-331 — without it the hub cannot tell "empty" from "unmeasured" LastIntegrityCheck: t.LastIntegrityCheck, // R-359 LastIntegrityOK: t.LastIntegrityOK, + LastIntegrityDepth: t.LastIntegrityDepth, // R-399 AbandonPurgeRequested: t.AbandonPurgeRequested, // R-241: declared until the hub drops the package } } diff --git a/controller/internal/backup/offbox_integrity.go b/controller/internal/backup/offbox_integrity.go index 8c04608..6039a0d 100644 --- a/controller/internal/backup/offbox_integrity.go +++ b/controller/internal/backup/offbox_integrity.go @@ -41,14 +41,64 @@ import ( // any plausible structure check and far below "forever", so the failure mode of a wedged SFTP mount is // a released flag and a retry tomorrow, never a box whose backups stop because a check never returned. // -// It is deliberately NOT sized for a `--read-data-subset` run, which downloads pack data and can take -// hours. That option ships OFF (R-399); whoever turns it on must revisit this number, and this comment -// is the note that says so. +// R-399 CHANGED WHAT THIS NUMBER HAS TO COVER, and this paragraph replaces the one that said the +// opposite. Read-data now ships ON at 100% (see defaultIntegrityReadDataSubset below), so a check +// downloads and re-hashes the whole store every week. Measured on demo-hp 2026-08-30: 39.2 s at 100% +// against a 134 MB store, versus 35.0 s at structure depth. 30 minutes is still ~46x the only +// full-depth number that exists, so it is not resized here on the strength of one measurement — but +// it is now the number a LARGE store will meet first, and `integritySlowNoticeThreshold` exists to +// tell the operator long before that happens. R-401 owns the revisit. const integrityCheckTimeout = 30 * time.Minute +// integritySlowNoticeThreshold is the point at which a COMPLETED check has started costing real time +// and the depth setting needs revisiting (R-401). +// +// CHOSEN, and deliberately imprecise, from the single data point that exists: demo-hp's 134 MB store, +// 2026-08-30, 35.0 s at structure depth and 39.2 s at 100%. 5 minutes is ~7.6x the only full-depth +// number we have, so it cannot fire on anything resembling today's fleet; and it is well under +// integrityCheckTimeout, so the operator hears "this is getting slow" long before a check is killed +// for running too long. A notice changes no behaviour, so an imprecise number is cheap here — whereas +// a precise-looking threshold invented from one measurement on one small store would be the exact +// shape of the four production designs this project has already specced against nothing. +// A var, not a const, for ONE reason: the notice cannot otherwise be proven to fire through the real +// CheckOffboxIntegrity path — no test can make a check take five minutes. Tests lower it and restore +// it with defer. Nothing in production writes it. +var integritySlowNoticeThreshold = 5 * time.Minute + // defaultIntegrityMaxAgeDays is the max age of a SUCCESSFUL check before one is due again. const defaultIntegrityMaxAgeDays = 7 +// defaultIntegrityReadDataSubset is how deep an unconfigured box checks: ALL of it (R-399, Viktor's +// ruling of 2026-08-31). +// +// THE FACT THE DEFAULT RESTS ON, because it is the thing that stops someone turning it back down to +// save four seconds: **the structure check does not detect a size-preserving pack corruption.** On +// 2026-08-30 a pack in demo-hp's store was damaged WITHOUT changing its size; plain `restic check` +// reported `no errors were found` and exited clean, and every read-data form caught it. A store that +// is verified only structurally is a store whose rot is discovered at restore time, with a customer +// waiting. +// +// THE COST, measured the same day on the same store (140 829 678 B / 2 651 blobs / 67 snapshots): +// structure 35.0 s, 10% 35.9 s, 50% 37.3 s, 100% 39.2 s. Four seconds. +// +// WHAT IS NOT ESTABLISHED: how any of that behaves on a store one or two orders of magnitude larger. +// There is exactly ONE data point. That is why this is a constant and a notice (see +// integritySlowNoticeThreshold) rather than a rotation schedule, a size threshold or a bandwidth +// budget — every one of those would be a number invented from a single measurement. R-401. +// +// This default lives HERE and not in config.applyDefaults, deliberately, and defaultIntegrityMaxAgeDays +// beside it is the precedent: both integrity defaults are resolved in this package, in one accessor +// each, next to the reasoning that justifies them. Symmetry with the other Monitoring defaults is +// worth less than having the number and its argument in the same place. +const defaultIntegrityReadDataSubset = "100%" + +// integrityOffToken switches the deep check back off without a code change. +// +// A setting with no off switch is not a setting. Without this token there would be no way to return a +// box to structure depth: an EMPTY value means "not configured" and therefore the default (§8), so +// emptiness cannot also mean "off". Matched case-insensitively. +const integrityOffToken = "off" + // readDataSubsetRe accepts the forms restic documents for --read-data-subset: "n/m", a percentage // like "5%", or a size like "50M". Anything else is refused at read time rather than passed through — // a typo must not fail the whole check, which is what handing restic an unparsed value would do. @@ -80,21 +130,64 @@ type IntegrityResult struct { // integrityReadDataSubset resolves the configured subset spec, refusing anything malformed. // -// Fail-safe direction: an unrecognised value becomes "" (structure check only) with a WARN, never a -// passthrough. Handing restic `--read-data-subset=banana` fails the entire check, which would turn a -// typo in a config file into a store that silently stops being verified. +// FOUR inputs, three outcomes (§8's table): +// - absent or empty -> defaultIntegrityReadDataSubset. Empty is "not configured", never "off". +// - "off" (any case) -> "" , the structure-and-index check only. The one way to switch it back. +// - a form restic accepts -> itself, unchanged. An explicit value always wins. +// - anything else -> defaultIntegrityReadDataSubset, with a WARN naming the bad value. +// +// THE MALFORMED CASE FALLS BACK TO THE DEFAULT, NOT TO STRUCTURE, and the direction is the point. +// Handing restic `--read-data-subset=banana` fails the whole check, so a typo must not be passed +// through — but downgrading to structure depth on a typo would ALSO silently remove the protection +// R-399 exists to add, which is R-357's shape exactly: a guard that opens quietly. Falling back to the +// default keeps the protection and still says loudly that the config is wrong. func (m *Manager) integrityReadDataSubset() string { spec := strings.TrimSpace(m.cfg.Monitoring.Integrity.ReadDataSubset) if spec == "" { + return defaultIntegrityReadDataSubset + } + if strings.EqualFold(spec, integrityOffToken) { return "" } if !readDataSubsetRe.MatchString(spec) { - m.logger.Printf("[WARN] [offbox] integrity: read_data_subset %q is not a form restic accepts (n/m, N%%, or a size like 50M) — running the STRUCTURE check only", spec) - return "" + m.logger.Printf("[WARN] [offbox] integrity: read_data_subset %q is not a form restic accepts (n/m, N%%, a size like 50M, or %q) — falling back to the DEFAULT depth %q, not to a structure-only check, so a typo cannot quietly remove the protection", + spec, integrityOffToken, defaultIntegrityReadDataSubset) + return defaultIntegrityReadDataSubset } return spec } +// IntegrityDepthCode is the depth as a short RECORDED value, for the persisted verdict and the wire. +// +// "" is reserved to mean NOT RECORDED — a box older than v0.228.0, whose stored verdict cannot say how +// deep it looked. That follows the StatsKnown precedent on the same object: absence means "cannot +// answer", never an answer. So structure depth is written as the word "structure", not as "". +func IntegrityDepthCode(subset string) string { + if subset == "" { + return "structure" + } + return subset +} + +// noticeIfSlow logs an operator WARN when a COMPLETED check has started costing real time (R-401). +// +// COMPLETED ONLY. A skip has no duration to judge, and an unreachable store is "I could not look", +// which is not "I looked and it was slow" (§8). Pass and fail BOTH qualify: the notice and the failure +// alarm are independent facts and neither suppresses the other. +// +// It is a log line and NOTHING else — no hub event, no customer alarm. An event type costs the +// severity contract, the grain table and three registers, all to say "this took a while"; 08 §6.2's +// coarse-by-default rule points the other way. And it does NOT change the depth by itself: a notice +// that silently reconfigures the box would be a behaviour change wearing a notice's clothes. +func (m *Manager) noticeIfSlow(res IntegrityResult) { + if res.Skipped || res.Unreachable || res.Duration < integritySlowNoticeThreshold { + return + } + m.logger.Printf("[WARN] [offbox] integrity: the check took %s at depth %s (%s), over the %s notice threshold — R-401: the depth setting needs revisiting for a store this size. Nothing was changed automatically.", + res.Duration.Round(time.Second), IntegrityDepthCode(res.ReadDataSubset), + integrityDepthLabel(res.ReadDataSubset), integritySlowNoticeThreshold) +} + // integrityMaxAge returns the configured max age of a successful check, defaulting to 7 days. func (m *Manager) integrityMaxAge() time.Duration { d := m.cfg.Monitoring.Integrity.MaxAgeDays @@ -133,16 +226,29 @@ func (m *Manager) IntegrityDue(now time.Time) (due bool, last time.Time) { // the hourly operator cooldown already governs the mail, and the failure is already recorded where a // surface can read it. A skip or an unreachable repository does NOT reach here, so tomorrow tries // again. -// RecordIntegrityOutcome is the exported entry point; the caller in main.go owns the decision of WHEN -// a verdict counts, because only it knows whether the run was forced or scheduled. -func (m *Manager) RecordIntegrityOutcome(at time.Time, ok bool) { m.recordIntegrityOutcome(at, ok) } +// RecordIntegrityVerdict is the PRODUCTION entry point: it persists the verdict AND the depth it was +// reached at, in one write. The caller in main.go owns the decision of WHEN a verdict counts, because +// only it knows whether the run was forced or scheduled. +// +// The depth travels with the verdict because a stored result that does not say how deep it looked +// cannot be judged later: "checked, OK" means two different things at structure depth and at 100%, +// and the whole of R-399 is that difference. +func (m *Manager) RecordIntegrityVerdict(res IntegrityResult) { + m.recordIntegrityOutcome(res.RanAt, res.OK, IntegrityDepthCode(res.ReadDataSubset)) +} -func (m *Manager) recordIntegrityOutcome(at time.Time, ok bool) { +// RecordIntegrityOutcome records a verdict whose depth is not stated. It writes "" to the depth field, +// which reads as NOT RECORDED rather than as structure depth — see integrityDepthCode. Kept as the +// due-ness surface the R-359 tests drive; production goes through RecordIntegrityVerdict above. +func (m *Manager) RecordIntegrityOutcome(at time.Time, ok bool) { m.recordIntegrityOutcome(at, ok, "") } + +func (m *Manager) recordIntegrityOutcome(at time.Time, ok bool, depth string) { if err := m.settings.UpdateOffboxStatus(func(o *settings.OffboxTarget) { o.LastIntegrityCheck = at.UTC().Format(time.RFC3339) o.LastIntegrityOK = ok + o.LastIntegrityDepth = depth }); err != nil { - m.logger.Printf("[ERROR] [offbox] integrity: could not persist the check outcome: %v — the check RAN and its verdict was ok=%v, but due-ness did not advance, so it will run again tomorrow", err, ok) + m.logger.Printf("[ERROR] [offbox] integrity: could not persist the check outcome: %v — the check RAN and its verdict was ok=%v at depth %q, but due-ness did not advance, so it will run again tomorrow", err, ok, depth) } } @@ -198,6 +304,7 @@ func (m *Manager) CheckOffboxIntegrity(ctx context.Context) IntegrityResult { if err == nil { res.OK = true m.logger.Printf("[INFO] [offbox] integrity: check PASSED in %s (%s)", res.Duration.Round(time.Second), integrityDepthLabel(res.ReadDataSubset)) + m.noticeIfSlow(res) return res } @@ -221,6 +328,9 @@ func (m *Manager) CheckOffboxIntegrity(ctx context.Context) IntegrityResult { } m.logger.Printf("[ERROR] [offbox] integrity: check FAILED after %s — restic reported: %s", res.Duration.Round(time.Second), res.Output) + // A slow FAILING check gets the notice too. The two facts are independent and suppressing one + // because the other fired is how the second fact stops existing. + m.noticeIfSlow(res) return res } diff --git a/controller/internal/backup/r359_integrity_test.go b/controller/internal/backup/r359_integrity_test.go index bd2841f..3c3f47b 100644 --- a/controller/internal/backup/r359_integrity_test.go +++ b/controller/internal/backup/r359_integrity_test.go @@ -157,21 +157,17 @@ func TestR359_TimeoutIsNotDamage(t *testing.T) { } } -func TestR359_StructureCheckPassesNoReadDataFlag(t *testing.T) { - m, cap := newIntegrityManager(t, okRepo(nil)) - m.CheckOffboxIntegrity(context.Background()) - - argv := cap.checkArgv() - if argv == nil { - t.Fatal("no check ran") - } - for _, a := range argv { - if strings.HasPrefix(a, "--read-data") { - t.Fatalf("the DEFAULT check downloaded pack data (%q) — that is a bandwidth cost nobody "+ - "chose, and R-399 exists precisely so it is not chosen here", a) - } - } -} +// TestR359_StructureCheckPassesNoReadDataFlag was DELETED on 2026-08-31, superseded by R-399. +// +// It asserted that an unconfigured box passes NO --read-data flag. That was the correct contract on +// 2026-08-30, when nothing had measured the cost of a deeper check. The next day a size-preserving +// pack corruption was shown to PASS that structure-only check on real hardware, and Viktor ruled the +// default to full depth. The test is not weakened, it is inverted: its replacement is +// TestR399_AbsentConfigRunsFullDepth in r399_depth_test.go, and the off token it left room for is +// pinned by TestR399_OffTokenRunsStructureOnly. +// +// Recorded here rather than removed silently, so a later reader does not re-derive the old ruling +// from its absence. func TestR359_ReadDataSubsetIsPassedWhenConfigured(t *testing.T) { m, cap := newIntegrityManager(t, okRepo(nil)) @@ -192,25 +188,12 @@ func TestR359_ReadDataSubsetIsPassedWhenConfigured(t *testing.T) { } } -func TestR359_MalformedReadDataSubsetIsTreatedAsOff(t *testing.T) { - m, cap := newIntegrityManager(t, okRepo(nil)) - m.cfg.Monitoring.Integrity.ReadDataSubset = "banana" - res := m.CheckOffboxIntegrity(context.Background()) - - for _, a := range cap.checkArgv() { - if strings.HasPrefix(a, "--read-data") { - t.Fatalf("a malformed value was handed to restic (%q) — restic rejects it and the WHOLE "+ - "check fails, so one typo silently stops the store being verified at all", a) - } - } - if res.ReadDataSubset != "" { - t.Errorf("a refused value was still recorded as the depth: %q", res.ReadDataSubset) - } - if !strings.Contains(cap.logBuf.String(), "WARN") { - t.Error("a refused config value must say so — silence makes a typo indistinguishable from a " + - "deliberate structure-only setting") - } -} +// TestR359_MalformedReadDataSubsetIsTreatedAsOff was DELETED on 2026-08-31, superseded by R-399. +// +// Its NAME was the defect. Treating a typo as "off" downgrades the check silently, which is R-357's +// shape — a guard that opens quietly. The half of it that still holds (a malformed value never +// reaches restic, and it WARNs) is asserted by TestR399_MalformedFallsBackToTheDefault, which also +// pins the new direction: the fallback is the DEFAULT depth, never structure. func TestR359_MessageNeverCarriesResticOutputOrCredentials(t *testing.T) { // R-379: 615 bytes of raw database text reached a customer once. And offboxBaseArgs builds the repo diff --git a/controller/internal/backup/r399_depth_test.go b/controller/internal/backup/r399_depth_test.go new file mode 100644 index 0000000..d80a520 --- /dev/null +++ b/controller/internal/backup/r399_depth_test.go @@ -0,0 +1,193 @@ +package backup + +import ( + "context" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/config" +) + +// ── R-399 — the off-site check reads the DATA, not just the catalogue ──────────────────────────── +// +// THE MEASUREMENT THESE TESTS DEFEND. On demo-hp, 2026-08-30, a pack in a real 134 MB store was +// damaged WITHOUT changing its size. Plain `restic check` — the structure-and-index check that every +// box in the fleet ran — reported `no errors were found` and exited clean. Every `--read-data*` form +// caught it. Cost of the deeper run on that store: 39.2 s versus 35.0 s. +// +// So the assertions below are on the ARGUMENT LIST, never on the absence of an error. "The check +// passed" is exactly what the broken configuration produced; only the argv can tell the two apart. + +// readDataArg returns the --read-data* argument the check passed to restic, or "" when it passed none. +func readDataArg(argv []string) string { + for _, a := range argv { + if strings.HasPrefix(a, "--read-data") { + return a + } + } + return "" +} + +// A1 — the fleet's actual configuration: no integrity block at all. +func TestR399_AbsentConfigRunsFullDepth(t *testing.T) { + m, cap := newIntegrityManager(t, okRepo(nil)) + // Deliberately touch nothing: this is a box whose controller.yaml has no `integrity:` key. + res := m.CheckOffboxIntegrity(context.Background()) + + argv := cap.checkArgv() + if argv == nil { + t.Fatal("no check ran") + } + if got := readDataArg(argv); got != "--read-data-subset=100%" { + t.Fatalf("an unconfigured box ran at depth %q, want --read-data-subset=100%%; argv=%v\n"+ + "A structure-only run is what every box did before R-399, and it PASSED a size-preserving "+ + "pack corruption on real hardware — the store's rot would be found at restore time", + got, argv) + } + if res.ReadDataSubset != "100%" { + t.Errorf("the result did not record the depth it actually ran at: %+v", res) + } +} + +// A2 — an empty string is NOT the off switch. Empty means "not configured", so it means the default. +func TestR399_EmptyStringIsNotOff(t *testing.T) { + m, cap := newIntegrityManager(t, okRepo(nil)) + m.cfg.Monitoring.Integrity.ReadDataSubset = "" + m.CheckOffboxIntegrity(context.Background()) + + if got := readDataArg(cap.checkArgv()); got != "--read-data-subset=100%" { + t.Fatalf("an empty value was treated as OFF (%q) — emptiness must mean 'not configured', or "+ + "there is no way to distinguish an unset key from a deliberate downgrade", got) + } +} + +// A3 — the off switch. A setting with no off switch is not a setting. +func TestR399_OffTokenRunsStructureOnly(t *testing.T) { + m, cap := newIntegrityManager(t, okRepo(nil)) + m.cfg.Monitoring.Integrity.ReadDataSubset = "off" + res := m.CheckOffboxIntegrity(context.Background()) + + if got := readDataArg(cap.checkArgv()); got != "" { + t.Fatalf("the off token still downloaded pack data (%q) — there would be no way to switch the "+ + "deep check off without editing code", got) + } + if res.ReadDataSubset != "" { + t.Errorf("a structure-only run recorded a subset: %q", res.ReadDataSubset) + } + if code := IntegrityDepthCode(res.ReadDataSubset); code != "structure" { + t.Errorf("structure depth must be RECORDED as %q, not as an empty string a reader has to "+ + "interpret; got %q", "structure", code) + } +} + +// A4 — an operator writing "OFF" or "Off" in a YAML file means the same thing. +func TestR399_OffTokenIsCaseInsensitive(t *testing.T) { + for _, tok := range []string{"OFF", "Off", " oFf "} { + m, cap := newIntegrityManager(t, okRepo(nil)) + m.cfg.Monitoring.Integrity.ReadDataSubset = tok + m.CheckOffboxIntegrity(context.Background()) + if got := readDataArg(cap.checkArgv()); got != "" { + t.Errorf("%q was not recognised as the off token (argv carried %q)", tok, got) + } + } +} + +// A5 — an explicit value wins over both the default and the off token. +func TestR399_ExplicitValueWins(t *testing.T) { + m, cap := newIntegrityManager(t, okRepo(nil)) + m.cfg.Monitoring.Integrity.ReadDataSubset = "10%" + res := m.CheckOffboxIntegrity(context.Background()) + + if got := readDataArg(cap.checkArgv()); got != "--read-data-subset=10%" { + t.Fatalf("an explicit 10%% ran as %q — a chosen value must not be overridden by a default", got) + } + if res.ReadDataSubset != "10%" { + t.Errorf("result recorded %q, want 10%%", res.ReadDataSubset) + } +} + +// A6 — a typo falls back to the DEFAULT, not to structure depth. +// +// The direction is the whole point. Passing "banana" through fails the entire check; downgrading to +// structure would silently remove the protection R-399 exists to add, which is R-357's shape exactly — +// a guard that opens quietly. Falling back to the default keeps the protection and still says loudly +// that the config is wrong. +func TestR399_MalformedFallsBackToTheDefault(t *testing.T) { + m, cap := newIntegrityManager(t, okRepo(nil)) + m.cfg.Monitoring.Integrity.ReadDataSubset = "banana" + res := m.CheckOffboxIntegrity(context.Background()) + + argv := cap.checkArgv() + for _, a := range argv { + if strings.Contains(a, "banana") { + t.Fatalf("a malformed value was handed to restic (%q) — restic rejects it and the WHOLE "+ + "check fails, so one typo would stop the store being verified at all", a) + } + } + if got := readDataArg(argv); got != "--read-data-subset=100%" { + t.Fatalf("a typo downgraded the check to %q instead of falling back to the default 100%% — "+ + "a guard that opens quietly on a typo is R-357's failure", got) + } + if res.ReadDataSubset != "100%" { + t.Errorf("result recorded %q after a refused value, want the default", res.ReadDataSubset) + } + log := cap.logBuf.String() + if !strings.Contains(log, "WARN") || !strings.Contains(log, "banana") { + t.Errorf("a refused config value must WARN and NAME the bad value; log was:\n%s", log) + } +} + +// A7 — the depth is stated in the outcome, or the result cannot be judged afterwards. +func TestR399_DepthIsStatedInTheOutcome(t *testing.T) { + m, cap := newIntegrityManager(t, okRepo(nil)) + res := m.CheckOffboxIntegrity(context.Background()) + + if !strings.Contains(cap.logBuf.String(), "100%") { + t.Errorf("the PASSED log line does not say how deep the check looked:\n%s", cap.logBuf.String()) + } + // And it is PERSISTED with the verdict, so a later reader of the stored state can judge it too. + m.RecordIntegrityVerdict(res) + tgt := m.settings.GetOffboxTarget() + if tgt == nil || tgt.LastIntegrityDepth != "100%" { + t.Fatalf("the stored verdict does not carry its depth: %+v — 'checked, OK' means two different "+ + "things at structure depth and at 100%%", tgt) + } +} + +// A8 — THE SEAM TEST. Everything above sets the config field directly; this one proves the default +// survives the PRODUCTION resolution path: real YAML bytes -> config.LoadFromBytes -> the accessor -> +// the argv. A default that only works when a test hand-builds the struct is the inert-seam failure +// this project has shipped four times. +func TestR399_DefaultResolvesThroughTheRealConfigPath(t *testing.T) { + // A minimal but VALID controller.yaml — LoadFromBytes validates, and a config that fails + // validation would never reach a running box, so a fixture that skipped the required keys would + // not be the production path. + const yaml = ` +customer: + id: "demo-hp" + domain: "felhom.example.hu" +monitoring: + enabled: true + thresholds: + disk_warn_percent: 80 +` + loaded, err := config.LoadFromBytes([]byte(yaml)) + if err != nil { + t.Fatalf("the fixture config does not parse: %v", err) + } + if loaded.Monitoring.Integrity.ReadDataSubset != "" { + t.Fatalf("fixture wrong: the parsed config already carries a subset %q, so this test would "+ + "prove nothing about the ABSENT case", loaded.Monitoring.Integrity.ReadDataSubset) + } + + m, cap := newIntegrityManager(t, okRepo(nil)) + // Only the parsed Monitoring block is transplanted; Paths must stay pointing at the test's temp + // dir. The seam under test is config-parse -> Monitoring.Integrity -> integrityReadDataSubset. + m.cfg.Monitoring = loaded.Monitoring + m.CheckOffboxIntegrity(context.Background()) + + if got := readDataArg(cap.checkArgv()); got != "--read-data-subset=100%" { + t.Fatalf("a real controller.yaml with no `integrity:` block resolved to depth %q, want "+ + "--read-data-subset=100%% — that is every box in the fleet today", got) + } +} diff --git a/controller/internal/backup/r399_slow_notice_test.go b/controller/internal/backup/r399_slow_notice_test.go new file mode 100644 index 0000000..cd0878b --- /dev/null +++ b/controller/internal/backup/r399_slow_notice_test.go @@ -0,0 +1,139 @@ +package backup + +import ( + "context" + "errors" + "strings" + "testing" + "time" +) + +// ── R-399 / R-401 — the check tells the operator when it starts costing real time ──────────────── +// +// The 100% default rests on ONE measurement, on ONE 134 MB store: 39.2 s. It will not stay true. The +// notice is what makes that fact reach a person from the product rather than from a customer. +// +// It is a LOG LINE and nothing else — no hub event, no customer alarm, and it never changes the depth +// by itself. TestR399_SlownessRaisesNoHubEvent in cmd/controller pins the first of those; the other +// two are pinned here. + +// slowNoticeFragment is the ASCII-only fingerprint of the notice, chosen so no other line in this +// file's logs can match it. +const slowNoticeFragment = "R-401: the depth setting needs revisiting" + +// withSlowThreshold lowers the notice threshold for one test and restores it. +// +// No test can make a check take five minutes, so the threshold is the seam. Everything else runs +// through the real CheckOffboxIntegrity. +func withSlowThreshold(t *testing.T, d time.Duration) { + t.Helper() + prev := integritySlowNoticeThreshold + integritySlowNoticeThreshold = d + t.Cleanup(func() { integritySlowNoticeThreshold = prev }) +} + +// slowReply makes the `check` call take measurable time so a lowered threshold is genuinely exceeded +// rather than merely equalled. +func slowReply(out []byte, err error) func(args []string) ([]byte, error) { + return okRepo(func(args []string) ([]byte, error) { + time.Sleep(3 * time.Millisecond) + return out, err + }) +} + +// B1 — a slow check warns, and the warning names the duration and the depth. +func TestR399_SlowCheckWarns(t *testing.T) { + withSlowThreshold(t, time.Millisecond) + m, cap := newIntegrityManager(t, slowReply(nil, nil)) + res := m.CheckOffboxIntegrity(context.Background()) + + if !res.OK { + t.Fatalf("fixture: this run should pass; got %+v", res) + } + log := cap.logBuf.String() + if !strings.Contains(log, slowNoticeFragment) { + t.Fatalf("a check over the threshold produced no notice — the operator would keep spending a "+ + "customer's bandwidth every week and hear about it from the customer. Log:\n%s", log) + } + if !strings.Contains(log, "WARN") { + t.Errorf("the notice is not at WARN level:\n%s", log) + } + // It must name BOTH facts: how long, and how deep. Either alone is unactionable. + if !strings.Contains(log, "100%") { + t.Errorf("the notice does not name the depth that was slow:\n%s", log) + } + if !strings.Contains(log, "the check took") { + t.Errorf("the notice does not name the duration:\n%s", log) + } + // And it changes NOTHING by itself. A notice that silently reconfigured the box would be a + // behaviour change wearing a notice's clothes. + if m.integrityReadDataSubset() != "100%" { + t.Error("the notice altered the configured depth — it must only report") + } +} + +// B2 — a fast check is silent. A notice that fires every week is not a notice. +func TestR399_FastCheckIsSilent(t *testing.T) { + withSlowThreshold(t, time.Hour) + m, cap := newIntegrityManager(t, okRepo(nil)) + m.CheckOffboxIntegrity(context.Background()) + + if strings.Contains(cap.logBuf.String(), slowNoticeFragment) { + t.Fatalf("a check under the threshold warned anyway:\n%s", cap.logBuf.String()) + } +} + +// B3 — a SKIP has no duration to judge. +func TestR399_SkipNeverWarns(t *testing.T) { + withSlowThreshold(t, time.Nanosecond) // every non-zero duration would qualify + m, cap := newIntegrityManager(t, okRepo(nil)) + if err := m.AcquireRunningForTest(); err != nil { + t.Fatalf("fixture: %v", err) + } + defer m.ReleaseRunningForTest() + + res := m.CheckOffboxIntegrity(context.Background()) + if !res.Skipped { + t.Fatalf("fixture: expected a skip, got %+v", res) + } + if strings.Contains(cap.logBuf.String(), slowNoticeFragment) { + t.Fatalf("a skipped check produced a slowness notice — it never ran, so there is no duration "+ + "to judge:\n%s", cap.logBuf.String()) + } +} + +// B4 — "I could not look" is not "I looked and it was slow". +func TestR399_UnreachableNeverWarns(t *testing.T) { + withSlowThreshold(t, time.Nanosecond) + m, cap := newIntegrityManager(t, func(args []string) ([]byte, error) { + time.Sleep(3 * time.Millisecond) + return []byte("ssh: connect to host nas.local port 22: Connection refused"), errors.New("exit 1") + }) + res := m.CheckOffboxIntegrity(context.Background()) + if !res.Unreachable { + t.Fatalf("fixture: expected unreachable, got %+v", res) + } + if strings.Contains(cap.logBuf.String(), slowNoticeFragment) { + t.Fatalf("an unreachable store produced a slowness notice:\n%s", cap.logBuf.String()) + } +} + +// B5 — the notice and the failure verdict are INDEPENDENT facts. Neither suppresses the other. +func TestR399_SlowAndFailedProducesBoth(t *testing.T) { + withSlowThreshold(t, time.Millisecond) + const damaged = "pack 5b1f2c3d: not found in index\nrepository contains errors" + m, cap := newIntegrityManager(t, slowReply([]byte(damaged), errFake)) + res := m.CheckOffboxIntegrity(context.Background()) + + if res.OK || res.Unreachable { + t.Fatalf("fixture: expected a readable-and-damaged verdict, got %+v", res) + } + log := cap.logBuf.String() + if !strings.Contains(log, "check FAILED") { + t.Fatalf("the failure was not reported:\n%s", log) + } + if !strings.Contains(log, slowNoticeFragment) { + t.Fatalf("a slow FAILING check lost its slowness notice — suppressing one fact because the "+ + "other fired is how the second fact stops existing:\n%s", log) + } +} diff --git a/controller/internal/config/config.go b/controller/internal/config/config.go index ca54be7..9927d5b 100644 --- a/controller/internal/config/config.go +++ b/controller/internal/config/config.go @@ -188,11 +188,27 @@ type PingUUIDsConfig struct { // is on. R-341 is the failure that shape avoids: a dated check that was quietly missed for five days // because nothing asked again. // -// IntegrityReadDataSubset is EMPTY by default and that is a decision, not an oversight (R-399). An -// empty value runs restic's structure-and-index check, which downloads no pack data. A value like -// "5%" adds `--read-data-subset=5%`, which downloads and re-hashes that fraction of the store every -// run — a bandwidth and money cost that nothing has yet measured against the real store, so the -// default must not be chosen here. +// IntegrityReadDataSubset chooses HOW DEEP the check looks, and since 2026-08-31 it DEFAULTS TO +// FULL DEPTH — an absent or empty value re-reads 100% of the stored data (R-399, Viktor's ruling). +// +// This paragraph replaces one that argued the opposite. That argument was correct on the day it was +// written, when nothing had measured the cost; it was overtaken by a measurement the next day. +// +// **The structure check does not detect a size-preserving pack corruption.** On demo-hp, 2026-08-30, a +// pack was damaged WITHOUT changing its size: plain `restic check` reported `no errors were found` and +// exited clean, and every read-data form caught it. A structurally-verified store is one whose rot is +// found at restore time, with a customer waiting. That sentence is the reason for the default and it +// is what should stop anyone turning it back down to save four seconds. +// +// The cost, same store and day (140 829 678 B / 2 651 blobs / 67 snapshots): structure 35.0 s, +// 10% 35.9 s, 50% 37.3 s, 100% 39.2 s. +// +// Accepted values: absent or empty -> the default (100%); "off" (any case) -> structure and index +// only; any form restic accepts for --read-data-subset ("10%", "1/7", "50M") -> itself. Anything else +// WARNs and falls back to the default, never to structure — a typo must not quietly remove the +// protection. The default constant, the off token and that reasoning live in +// internal/backup/offbox_integrity.go, beside defaultIntegrityMaxAgeDays and NOT in applyDefaults: +// both integrity defaults are resolved in one accessor each, next to the argument that justifies them. type IntegrityConfig struct { MaxAgeDays int `yaml:"max_age_days"` ReadDataSubset string `yaml:"read_data_subset"` diff --git a/controller/internal/report/types.go b/controller/internal/report/types.go index 141ed6b..1d191b9 100644 --- a/controller/internal/report/types.go +++ b/controller/internal/report/types.go @@ -114,9 +114,17 @@ type BackupReport struct { // protected?". // // **The live numbers are in `Offsite` (backup.OffboxReportStatus) — read that instead**, and - // consult its `StatsKnown` before believing a zero. `IntegrityOK` has no source at all: the - // controller runs no integrity check, and `NotifyIntegrityOK`/`NotifyIntegrityFailed` exist and - // are called from nowhere — so it can only ever be a lie or a permanent "Unknown". + // consult its `StatsKnown` before believing a zero. That includes the integrity verdict: it is + // published as `Offsite.LastIntegrityCheck` / `LastIntegrityOK` / `LastIntegrityDepth`. + // + // CORRECTED 2026-08-31 (R-399). The sentence here used to read "`IntegrityOK` has no source at + // all: the controller runs no integrity check, and `NotifyIntegrityOK`/`NotifyIntegrityFailed` + // exist and are called from nowhere". BOTH HALVES BECAME FALSE IN v0.227.0 — the check runs weekly + // and both notifiers are called. The fields below are still dead and still unrendered, but the + // reason changed: not "nothing produces it" but "the live verdict lives on OffboxReportStatus, and + // these are kept only so historical reports already in the hub's store keep parsing". Giving them + // a value now would resurrect the card R-331 removed. A warning block asserting a fact that has + // stopped being true is how the next reader concludes the opposite of what it means. // // They are kept rather than deleted so historical reports already in the hub's store keep // parsing; pinned by TestBackupReport_DeadFieldsStayZero. diff --git a/controller/internal/settings/settings.go b/controller/internal/settings/settings.go index 88d598e..cb0edee 100644 --- a/controller/internal/settings/settings.go +++ b/controller/internal/settings/settings.go @@ -304,6 +304,15 @@ type OffboxTarget struct { // a dated check that is quietly missed and never catches up. LastIntegrityCheck string `json:"last_integrity_check,omitempty"` // RFC3339 LastIntegrityOK bool `json:"last_integrity_ok,omitempty"` + // LastIntegrityDepth (R-399) records HOW DEEP that verdict looked: "structure" for the + // structure-and-index check, or the subset spec that was re-read ("100%", "10%", "1/7", ...). + // + // It is a third field rather than a flag for the same reason there are two above: "checked, OK" + // means two different things at structure depth and at 100%, and a verdict that cannot say which + // cannot be judged afterwards. EMPTY means NOT RECORDED — a verdict written by a controller older + // than v0.228.0 — and never "structure"; absence means the box cannot answer, exactly as StatsKnown + // below establishes for the counts. + LastIntegrityDepth string `json:"last_integrity_depth,omitempty"` LastError string `json:"last_error,omitempty"` LastDuration string `json:"last_duration,omitempty"` RepoSizeHuman string `json:"repo_size_human,omitempty"` diff --git a/controller/internal/web/handler_debug.go b/controller/internal/web/handler_debug.go index 3747128..a14530c 100644 --- a/controller/internal/web/handler_debug.go +++ b/controller/internal/web/handler_debug.go @@ -65,6 +65,11 @@ func (s *Server) handleDebugAPI(w http.ResponseWriter, r *http.Request) { // Section 3: Backup testing (app-data only; disk-tier moved to host agent) case subpath == "backup/dbdump" && r.Method == http.MethodPost: s.debugTriggerDBDump(w, r) + // R-400 — the „Csak cross-drive" button has posted here since it was added and nothing answered. + // The capability was never missing: Manager.RunTier2 is live and the app config page already calls + // it. Only the debug route was absent, so this is IMPLEMENTED rather than deleted. + case subpath == "backup/crossdrive" && r.Method == http.MethodPost: + s.debugRunCrossDrive(w, r) // R-397 — the button at debug.html:83 has posted here since it was added and NOTHING answered. // Verified 2026-08-30: this dispatch had no such case, so pressing „Restic integritás" did // nothing at all. Seventh instance of built-but-never-wired in this project; filed as R-400 in its @@ -404,6 +409,66 @@ func (s *Server) debugTriggerDBDump(w http.ResponseWriter, r *http.Request) { writeDebugJSON(w, http.StatusOK, true, "DB dump elindítva", nil) } +// debugRunCrossDrive runs the Tier-2 (cross-drive) copy for every deployed HDD app (R-400). +// +// ASYNCHRONOUS, following debugTriggerDBDump above rather than the integrity button below: a Tier-2 +// sweep across every app copies real data and is bounded by disk speed, not by a timeout, so holding +// the operator's request open for it would time the browser out and teach nothing. The integrity +// button is synchronous because the operator pressed it to learn an ANSWER; this one is pressed to +// make something happen, and the log is where its outcome belongs. +// +// It reports WHICH apps it started for, not just „elindítva". A sweep that quietly matched zero apps +// and a sweep that matched eight are different facts, and a message that cannot tell them apart is +// how a button reads as working while doing nothing — the class this whole row exists to close. +func (s *Server) debugRunCrossDrive(w http.ResponseWriter, r *http.Request) { + if s.backupMgr == nil { + writeDebugJSON(w, http.StatusBadRequest, false, "Backup manager nincs konfigurálva", nil) + return + } + names := crossDriveTargets(s.stackMgr.GetStacks(), func(name string) bool { + return s.backupMgr.Tier2Info(name).IsHDDApp + }) + if len(names) == 0 { + writeDebugJSON(w, http.StatusOK, true, "Nincs olyan telepített alkalmazás, amelynek 2. mentése futtatható lenne", + map[string]interface{}{"apps": names, "count": 0}) + return + } + go func(apps []string) { + for _, name := range apps { + if err := s.backupMgr.RunTier2(name); err != nil { + s.logger.Printf("[WARN] [web] debug cross-drive run for %s failed: %v", name, err) + continue + } + s.logger.Printf("[INFO] [web] debug cross-drive run for %s completed", name) + } + }(names) + writeDebugJSON(w, http.StatusOK, true, + fmt.Sprintf("Cross-drive mentés elindítva %d alkalmazásra", len(names)), + map[string]interface{}{"apps": names, "count": len(names)}) +} + +// crossDriveTargets picks the apps a Tier-2 sweep should run for. +// +// A NAMED FUNCTION rather than an inline loop so both of its answers are assertable. The empty answer +// and the non-empty answer are the two outcomes a dead button and a working one are told apart by, and +// building a deployed HDD-backed stack inside a web test is not practical — so the selection is proven +// here and the dispatch is proven at the route. +func crossDriveTargets(all []stacks.Stack, isHDDApp func(name string) bool) []string { + names := make([]string, 0) + for _, st := range all { + if !st.Deployed { + continue + } + // Tier 2 is an off-DRIVE copy: an app with no HDD path has no second drive to copy to, and + // RunTier2 would return "no source drive" for every one of them. + if !isHDDApp(st.Name) { + continue + } + names = append(names, st.Name) + } + return names +} + // debugRunIntegrityCheck runs the off-site integrity check by hand (R-359/R-397). // // SYNCHRONOUS on purpose, unlike the DB-dump button beside it: the operator pressed this to learn an @@ -427,6 +492,9 @@ func (s *Server) debugRunIntegrityCheck(w http.ResponseWriter, r *http.Request) "unreachable": res.Unreachable, "duration_ms": res.Duration.Milliseconds(), "read_data_subset": res.ReadDataSubset, + // R-399: the depth as it is RECORDED, so the JSON says "structure" rather than an empty string + // a reader has to interpret. read_data_subset above stays raw for anyone parsing the argv. + "depth": backup.IntegrityDepthCode(res.ReadDataSubset), } switch { case res.Skipped: diff --git a/controller/internal/web/r400_debug_routes_test.go b/controller/internal/web/r400_debug_routes_test.go new file mode 100644 index 0000000..98bcaeb --- /dev/null +++ b/controller/internal/web/r400_debug_routes_test.go @@ -0,0 +1,147 @@ +package web + +import ( + "encoding/json" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" + + "gitea.dooplex.hu/admin/felhom-controller/internal/stacks" +) + +// ── R-400 — the debug page stops lying ─────────────────────────────────────────────────────────── +// +// Verified 2026-08-31 against the shipped tree: debug.html referenced 24 `/api/debug/...` addresses +// and handleDebugAPI answered 17. Seven controls did nothing. Three of the seven were not buttons — +// `dr/infra-status` and both `storage/watchdog-status` calls fetch on page LOAD, so those panels had +// been permanently empty and nobody had to click anything to be misled. This is the page an operator +// opens when something is already wrong. +// +// The gate in scripts/debug_route_gate.py makes the CLASS impossible. These tests name the individual +// DECISIONS, so that re-adding one of them is deliberate rather than accidental. + +// debugTemplateSource reads the shipped template from disk. The served page is rendered from this +// file, so a reference that is gone here is gone from the page — and reading the source is what lets +// a deletion be asserted without a browser (none is available on this host). +func debugTemplateSource(t *testing.T) string { + t.Helper() + b, err := os.ReadFile(filepath.Join("templates", "debug.html")) + if err != nil { + t.Fatalf("read debug.html: %v", err) + } + return string(b) +} + +// D2 — every control DELETED in Part 2.1, by name, with its reason. +func TestR400_DeletedControlsAreGoneFromTheTemplate(t *testing.T) { + src := debugTemplateSource(t) + + deleted := []struct{ ref, why string }{ + {"/api/debug/backup/infra", + "the disk-tier infra backup moved to the host agent (slice 8C); no function in this repo backs it"}, + {"/api/debug/hub/infra-push", + "Pusher.PushInfraBackup was removed 2026-06-16 — retired hub-side, and it had pushed plaintext secrets"}, + {"/api/debug/dr/infra-status", + "it rendered local infra backups and the hub infra push, both of which are the two retired mechanisms above"}, + {"/api/debug/storage/watchdog-status", + "the slice-8C storage watchdog is retired; the drive-gate reconcile replaced it and publishes no such status"}, + {"/api/debug/storage/simulate-disconnect", + "no backing capability, and it WRITES storage state — a button that fakes a drive disconnect on a customer's machine is a foot-gun"}, + {"/api/debug/storage/simulate-reconnect", "same as simulate-disconnect"}, + } + for _, d := range deleted { + if strings.Contains(src, d.ref) { + t.Errorf("%s is still referenced by debug.html — it was deleted because %s", d.ref, d.why) + } + } + + // POSITIVE CONTROL. Without it this test passes against a template it failed to read, or one that + // was emptied — the exact shape of an assertion that proves nothing. + for _, kept := range []string{"/api/debug/backup/integrity", "/api/debug/dr/trigger-setup"} { + if !strings.Contains(src, kept) { + t.Fatalf("positive control failed: %s is missing too, so the assertions above prove nothing "+ + "about what was deliberately deleted", kept) + } + } +} + +// D2b — the panels and the JavaScript went with the controls. A panel left behind renders nothing +// forever, which is how this whole class hides. +func TestR400_DeletedControlsLeftNoPanelOrScript(t *testing.T) { + src := debugTemplateSource(t) + // ASCII-only fragments (R-364): accented Hungarian through a template read is not the hazard here, + // but the rule is uniform and these identifiers are ASCII anyway. + for _, orphan := range []string{ + "watchdog-status", // the panel div and its two loaders + "renderWatchdogStatus", + "loadWatchdogStatus", + "simulateDisconnect", + "simulateReconnect", + "loadDRStatus", + "dr-status", + "btn-infra-backup", + "btn-hub-infra", + "section-storage", + } { + if strings.Contains(src, orphan) { + t.Errorf("%q survived the deletion — an orphaned panel or handler renders nothing forever "+ + "and reads as a working page", orphan) + } + } + // POSITIVE CONTROL: the neighbours that must stay. + for _, kept := range []string{"btn-dr-trigger", "triggerDR", "section-dr", "btn-crossdrive"} { + if !strings.Contains(src, kept) { + t.Fatalf("positive control failed: %q is gone too — the deletion took a neighbour with it", kept) + } + } +} + +// D1 — the one control IMPLEMENTED in Part 2.1 dispatches and answers with its JSON shape. +func TestR400_CrossDriveRouteDispatches(t *testing.T) { + // backupMgr nil is the fixture on purpose: it reaches the handler's own guard, which proves the + // route was DISPATCHED. A 404 here is the defect — that is precisely what the button got before. + s := newDebugServer(t, nil) + req := httptest.NewRequest(http.MethodPost, "/api/debug/backup/crossdrive", nil) + w := httptest.NewRecorder() + s.handleDebugAPI(w, req) + + if w.Code == http.StatusNotFound { + t.Fatal("POST /api/debug/backup/crossdrive returned 404 — the button still posts to nothing") + } + var env map[string]interface{} + if err := json.Unmarshal(w.Body.Bytes(), &env); err != nil { + t.Fatalf("the route answered with something that is not the debug JSON envelope: %q", w.Body.String()) + } + if _, has := env["ok"]; !has { + t.Errorf("no `ok` in the envelope: %v", env) + } +} + +// D1b — and it answers with the app list when a manager IS present. The empty sweep and the non-empty +// sweep are different facts, and „elindítva" alone cannot tell them apart. +func TestR400_CrossDriveReportsWhichAppsItStarted(t *testing.T) { + // The selection itself, both answers, through the same function the handler calls. + all := []stacks.Stack{ + {Name: "immich", Deployed: true}, + {Name: "vaultwarden", Deployed: true}, + {Name: "not-deployed", Deployed: false}, + } + hdd := func(name string) bool { return name == "immich" } + got := crossDriveTargets(all, hdd) + if len(got) != 1 || got[0] != "immich" { + t.Fatalf("the sweep picked %v — it must take deployed HDD-backed apps only: an app with no "+ + "second drive has nowhere to copy to, and an undeployed app has nothing to copy", got) + } + if none := crossDriveTargets(all, func(string) bool { return false }); len(none) != 0 { + t.Errorf("with no HDD app the sweep must be EMPTY, not a silent all-apps run: %v", none) + } + // NON-nil empty slice: it marshals as [] rather than null, so the JSON says "zero apps" instead of + // "no answer". + if none := crossDriveTargets(nil, hdd); none == nil { + t.Error("the empty answer is nil — it would marshal as `null`, which reads as 'unknown' rather " + + "than 'none', the same conflation R-331 cost a whole operator card") + } +} diff --git a/controller/internal/web/templates/debug.html b/controller/internal/web/templates/debug.html index e767a7d..e4610e6 100644 --- a/controller/internal/web/templates/debug.html +++ b/controller/internal/web/templates/debug.html @@ -82,24 +82,10 @@ - - - - -
-
-

Tárhely teszt

- ▶ -
- -
-
@@ -112,9 +98,6 @@ - - - @@ -168,7 +151,6 @@ ▶