diff --git a/REPORT.md b/REPORT.md index da55c26..e33cfdd 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,47 +1,73 @@ -# REPORT — controller v0.69.0: remove dead infra-backup stubs + unused restic-password report field (2026-06-16) +# REPORT — controller v0.70.0: config-apply self-restart + geo-restriction UX fixes (2026-06-16) -**Deployed:** controller **v0.69.0** on guest 9201 / felhom-pve (bootstrap-managed; healthy, hub-reporting OK). -**Scope:** controller half of Phase-1 Infra Backup retirement (hub **v0.12.0**). See -`felhom.eu/documentation/audits/SPIKE-infra-backup-2026-06-15.md`. Commit `9f59bc2`. +**Deployed:** controller **v0.70.0** on guest 9201 / felhom-pve (bootstrap-managed; healthy, hub-reporting OK). +**Scope:** controller-only. Fixes found during live geo testing (rotating the Cloudflare API token). +**Commits (trunk, per-commit green gate):** A `ba87412`, B `02a4ba0`, C `09d75c1`, docs `c514c00`, live-fix `9b7a265`. -## What changed (pure dead-code removal — no behaviour change) +## What shipped -- **Removed `report.Pusher.PushInfraBackup`** (`internal/report/pusher.go`) — pushed the infra-backup - payload to the now-removed hub endpoint `POST /api/v1/infra-backup`. Dead since slice 8C; no callers. -- **Removed `notify.Notifier.NotifyBackupCompleted`** (`internal/notify/notifier.go`) and the - `backup_completed` event — caller-less since whole-guest backup moved to the agent in slice 8C. The - hub's backup-deadline check now reads the agent host-report's PBS snapshots instead. `NotifyBackupFailed` - and the DB-dump notifiers are untouched and still used. -- **Removed `report.BackupReport.ResticPassword`** (`internal/report/types.go`, `json:"restic_password"`). +1. **Config-apply self-restart (core fix).** `POST /api/config/apply` previously wrote the new + `controller.yaml` but logged "restart needed" and left stale in-process singletons — the Cloudflare + client is built once at startup, so a rotated CF token kept 403'ing until a manual LXC restart. Now: + byte-identical re-push → no-op (no write, no restart); changed config → write, respond 200 (flushed), + then **graceful self-restart** (`os.Exit(0)` after ~500 ms; container is `restart: unless-stopped`). + Exit is behind an injectable `Restarter` seam. Removed the stale wording and the dead `OnConfigApplied` + hook (Phase-1-retired infra-backup push). +2. **Manual "Vezérlő újraindítása" button** → `POST /api/selfrestart` (auth + CSRF via `/api/` mount), + same helper; confirm → POST → poll `GET /` every 2 s → reload. +3. **Immediate hub report push on geo change** (`Router.reportPushNow`, wired in `main.go`): a successful + geo save and a successful manual geo sync fire a non-blocking report push so the hub reflects the new + state within seconds instead of after the ~15-min cycle. Scoped to geo handlers. +4. **Always report `geo_restriction`** (`buildGeoRestrictionReport`): always populated (Enabled=false, + empty countries when never configured) so the hub always renders the section ("Inaktív" when off). +5. **Country autocomplete fixed.** See root cause below. -## STEP-2 gate — confirmed before removing ResticPassword +## Country autocomplete — root cause (diagnosed live, NOT the hypothesised JS throw) -The task required proving the live builder leaves `ResticPassword` empty (else it would be an ongoing -restic-password leak to the hub). Confirmed two ways: -- **Source:** `buildBackupReport` (`internal/report/builder.go`) constructs `BackupReport{Enabled:…}` + - `LastDBDump` only; the comment at the disk-tier seam states restic/snapshot fields are left zero. No - code anywhere assigns `BackupReport.ResticPassword`. -- **Live data (hub DB, read-only):** the latest controller reports (v0.68.3) carry **no** - `restic_password` field at all. So no ongoing leak — safe to remove. +`filterCountries` runs without error and **correctly populates** the list, but reveals it with +`list.style.display = ''`. The `.geo-country-list` CSS default is `display:none` (style.css:3084), so +clearing the *inline* style falls back to the stylesheet and the populated list stays hidden — **no +console error, just an invisible dropdown**. Proven live via `javascript_tool`: calling +`filterCountries('Német')` set `innerHTML` to the "Németország (DE)" option but computed `display` +stayed `none`; forcing `display:block` revealed it. Latent since the geo feature's first commit +(`e1fb852`), not a recent regression; the operator's "1 console error" was unrelated (the stray +DevTools filter noted in the task). **Fix:** reveal with `'block'`. -> **Flagged (separate, worse historical finding — not in this task's scope):** the hub's legacy -> `reports` table holds **thousands of historical rows with a plaintext `restic_password` value** from -> *old* controller versions that DID populate it. The current controller does not, and this removal -> makes the field structurally unsendable, but the historical rows persist on the hub — a distinct -> purge/rotation decision for the operator. +## Tests (non-hollow, per-commit green gate; companions proven red pre-fix) -## Verification +- A: `selfrestart_test.go` — changed config → restart once; **identical → not called (companion, red + without the no-op guard)**; invalid YAML → not called; `/api/selfrestart` → restart once. +- B: `geo_test.go` — geo save success → push once; **invalid country → no push (companion)**. + `geo_report_test.go` — `buildGeoRestrictionReport(nil)` → non-nil disabled (**companion vs old + nil-omit**); passthrough; disabled-nil-countries. +- C: template/CSS visibility fix — verified live (no Go unit test applies). -- `go build ./... && go test ./...` — green; `go vet` clean. No residual references to - `PushInfraBackup` / `NotifyBackupCompleted` / `BackupReport.ResticPassword` (the `config.go` - `ResticPasswordFile` is the separate restic-password-file *path* config, intentionally untouched). -- Built `felhom-controller:0.69.0`, deployed to guest 9201 via the bootstrap mechanism - (`docker pull` → `/etc/felhom-controller-image` → restart `felhom-controller-bootstrap.service`). -- Live: container `Up … (healthy)` on 0.69.0; startup clean (no errors/panics); hub connectivity - `HTTP 200`; `Hub report pushed successfully (8309 bytes)` — confirming the caller-less removal causes - no behaviour change and reporting still works. +## Live verification on guest 9201 (all 5 pass) -## Out of scope +- **(a) config-apply auto-restart:** no-op re-push → `"…változatlan…"`, container `StartedAt` unchanged; + changed config → `"…a vezérlő újraindul."` + `StartedAt` advanced (11:04:18 → 11:08:50) + healthy — **no + manual step**. +- **(b) restart button:** renders (`btn-restart-controller` in rendered HTML); `POST /api/selfrestart` + → `"Újraindítás folyamatban…"` + restart (11:14:19 → 11:14:50) + healthy. (Full button click not + automated — its `confirm()` dialog blocks the browser extension; the poll-reload JS is code-reviewed.) +- **(c) geo save → hub within seconds:** save → controller logs "Hub report pushed successfully" 1 s + later; hub's latest demo report `received_at = 11:15:52` (the save time) with + `geo_restriction = {"enabled":true,"allowed_countries":["HU"]}`. +- **(d) hub "Inaktív" when off:** geo disabled → report carries `geo_restriction = {"enabled":false,…}` + (present-but-disabled = the data the hub renders as "Inaktív"; pre-fix this field was omitted and the + section hidden). Demo re-enabled (HU) afterwards. +- **(e) country dropdown:** typing "Német" lists **Németország (DE)** on the deployed 0.70.0 (the + `'block'` fix; confirmed the deployed JS uses `'block'` and the list becomes visible). -Credential rotation (operator); the historical `reports`-table restic_password rows (flagged above); -the Komga healthcheck. +## Bug found + fixed during verification + +The restart button was initially placed inside the `{{if .RetrievalPassword}}` block, so it was hidden +on the demo (no retrieval password set). Moved outside the conditional (commit `9b7a265`), rebuilt + +redeployed 0.70.0, re-verified it renders. + +## Notes / cleanup + +- All temp files removed from the build host (incl. a copied `controller.yaml` that held plaintext + secrets), guest, and local; the demo's `controller.yaml` was restored to its clean pre-test content. +- Out of scope (untouched): the Komga healthcheck; extending the immediate-report-push pattern beyond + geo handlers.