From c208ddf30879ff69635c132c6922b63287d759d4 Mon Sep 17 00:00:00 2001 From: kisfenyo Date: Mon, 22 Jun 2026 17:52:12 +0200 Subject: [PATCH] =?UTF-8?q?docs:=20v0.74.0=20REPORT/CONTEXT/README=20?= =?UTF-8?q?=E2=80=94=20agent=20connection-leak=20fix=20+=20live=20proof?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.8 (1M context) --- CONTEXT.md | 14 ++++++ REPORT.md | 104 +++++++++++++++++++++++-------------------- controller/README.md | 6 +++ 3 files changed, 75 insertions(+), 49 deletions(-) diff --git a/CONTEXT.md b/CONTEXT.md index c2a7f21..171a2db 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -7,6 +7,20 @@ > > Ask Claude Code: "Please update CONTEXT.md with what we did today" +Last updated: 2026-06-22 (v0.74.0 — controller→agent connection-leak fix) + +> **2026-06-22 — v0.74.0 (deployed on demo guest 9201): fixed the controller→agent connection leak.** +> `Server.agentClient()` built a new `agentapi.Client` (new bare `http.Transport`, `IdleConnTimeout:0`) +> per call → one leaked idle ESTABLISHED socket per agent call to `192.168.0.162:8443`, exhausting the +> ephemeral source-port range after ~5 days of controller uptime → EADDRNOTAVAIL, which had taken down +> storage UI + host-metrics + whole-guest backup (found in the 2026-06-22 unattended campaign). Fix: +> memoize ONE shared client via `sync.Once` + harden the Transport (`MaxIdleConnsPerHost:2`, +> `IdleConnTimeout:90s`). Live-proven: idle sockets to `:8443` stay flat at 2 across a 120-call burst +> (pre-fix grew ~1/call → 132). Agent/firewall untouched. Diagnosis + campaign findings live in +> `felhom.eu/documentation/tests/unattended-test-campaign-2026-06-22-*.md`. **Separate open item:** the +> defense-in-depth host firewall rule scoping `:8443` to the guest bridge subnet is still absent +> (pve-firewall disabled) — to be closed independently. + Last updated: 2026-06-13 (v0.60.0 backlog-Medium cleanup) > **Live version: controller v0.60.0** (deployed on demo guest 9201), agent **v0.30.0** (AGENT-001 deployed), hub **v0.11.0**. diff --git a/REPORT.md b/REPORT.md index f68639e..49a63aa 100644 --- a/REPORT.md +++ b/REPORT.md @@ -1,58 +1,64 @@ -# REPORT — controller v0.73.0: DR recipe (customer + apps half) +# REPORT — controller v0.74.0: fix the controller→agent connection leak -**TASK — DR recipe slice (controller half).** Emit the secret-free customer + apps half of the -reconstruction recipe as an additive hub-report section. Grounded in `SPIKE-dr-recipe-2026-06-16.md`. -**The controller emitter is the boundary enforcement point** — it is the component that distinguishes -secret from non-secret deploy fields, so v1 ships only an allowlist and the no-secrets boundary test -lives here. +## Baseline → target +`felhom-controller` `main` v0.73.0 → **v0.74.0**. Controller-only change; agent/hub/firewall untouched. -## Implementation +## Problem +`Server.agentClient()` built a fresh `agentapi.Client` (hence a fresh bare `http.Transport`, +`IdleConnTimeout:0`) on **every** agent API call and discarded it without closing idle connections. +The agent's keep-alive left one idle `ESTABLISHED` socket per call to `192.168.0.162:8443`; they +accumulated (~5.8k/day) until the ephemeral source-port range for that tuple exhausted → +`connect: cannot assign requested address` (EADDRNOTAVAIL), disabling storage UI, host-metrics, and +whole-guest backup after ~5 days of controller uptime. (`:8006`/pveproxy was immune — the controller +never dials it.) Diagnosis: +`felhom.eu/documentation/tests/unattended-test-campaign-2026-06-22-8443-diagnosis.md`. -- `internal/report/dr_recipe.go`: - - `DRRecipeAppHalf{recipe_version, customer{id,display,domain}, apps[]}`. - - `BuildDRRecipeAppHalf(custID, custName, domain, stacks, composeReader)` — pure given the reader; - one `AppRecipe` per DEPLOYED, non-protected stack. - - `AppRecipe{catalog_ref, enabled, storage_bindings}` — the **entire v1 surface**. `buildAppRecipe` - reads NOTHING from `AppConfig.Env`. - - `appStorageBindings(composeYAML, hddPath)` — pure compose parser; each `${HDD_PATH}`/`${USERDATA_PATH}` - volume bind → `{container_path, drive=basename(HDD_PATH), subpath}`; named volumes excluded. - - `readComposeFile` — the production reader (best-effort; unreadable → no bindings, never a failure). -- `Report.DRRecipe *DRRecipeAppHalf` wired into `BuildReport` (customer fields + `GetStacks()`). +## Change (commit `2a5b88f`) +- `internal/web/server.go` — `Server` gains `agentCli *agentapi.Client`, `agentCliErr error`, + `agentCliOnce sync.Once` (+ the `agentapi` import). +- `internal/web/agent_disk_handlers.go` — `agentClient()` memoizes the build via `agentCliOnce` and + **returns one shared client** (cfg.LocalAPI is static per process; a config-apply self-restarts the + controller). The empty-endpoint "not configured" guard stays OUTSIDE the `Once`. Signature and all + 19 call sites unchanged; `*agentapi.Client`/`*http.Client` are concurrency-safe so no extra locking. +- `internal/agentapi/client.go` — `New` Transport hardened: `MaxIdleConns:4`, + `MaxIdleConnsPerHost:2`, `IdleConnTimeout:90s` (was bare, `IdleConnTimeout:0`). Added optional + `Client.Close()` (CloseIdleConnections) hygiene helper. -## The boundary (the Phase-1 lesson) +## Tests (green; both red-proofed) +- `go build ./... && go vet ./... && go test ./...` — all green. +- **T1** `TestAgentClient_ReusesSameInstance` (web) — two `agentClient()` calls return the identical + pointer; `TestAgentClient_UnconfiguredErrors` — empty endpoint still errors. + Red-proof: reverting to per-call `agentapi.New` → pointers differ → FAIL (shown, reverted). +- **T2** `TestNew_TransportIdlePoolBounded` (agentapi, white-box) — `IdleConnTimeout>0` AND + `MaxIdleConnsPerHost>0`. Red-proof: bare Transport → `IdleConnTimeout==0` → FAIL (shown, reverted). -The recipe carries ONLY `{catalog_ref, enabled, storage_bindings}` — identifiers/paths. It NEVER touches -`AppConfig.Env`, where the controller keeps `ENC:` secrets. This is an **allowlist** (a new field is -excluded by default), the inverse of the retired infra-backup that shipped `restic_password` / -`cf_api_token`. Secrets stay in the PBS whole-CT snapshot + escrow, recovered with R, never here. +## Deploy +Built+pushed `gitea.dooplex.hu/admin/felhom-controller:0.74.0` on 192.168.0.180 (digest +`sha256:2e85376e…`), deployed to guest 9201 via the bootstrap path (`docker pull` → pin +`/etc/felhom-controller-image` → restart `felhom-controller-bootstrap.service`). `docker inspect`: +`image=:0.74.0 status=running health=healthy`. +> Process note: the first build packaged stale source (the build server's `~/git/felhom-controller` +> was at v0.73.0 — the required `git -C ~/git/felhom-controller pull` step had been skipped, so the +> image was the old code mislabeled `:0.74.0`; the live test still leaked). Pulled the source to +> `2a5b88f` and rebuilt — second image (`sha256:2e85376e…`) is the real fix. -## Tests (the load-bearing boundary test + companion) +## LIVE acceptance — leak is gone (the real proof) +Burst of **120 agent calls** (alternating `/api/disks` + `/api/host-metrics`) against the controller in +guest 9201, sampling idle `ESTABLISHED` sockets to `192.168.0.162:8443` from the controller's netns +(`nsenter -t -n ss -tn state established dst 192.168.0.162:8443 | grep -c 192.168`): -- `TestBuildAppRecipe_NoSecrets` — emit a recipe for an app whose `Env` carries an `ENC:` value + a - token-shaped value; assert NONE of the values and NO credential-shaped key survive; assert the - allowlisted facts DID emit (non-vacuous). -- `TestBuildAppRecipe_AllowlistIsLoadBearing` — the companion/red-proof: a guard-removed shape leaks the - token; the production emitter does not. -- `TestAppStorageBindings` (+ `_NoHDD`) — pins the compose parse (roms + resources bindings; named volume - excluded; rootfs app → 0 bindings). -- `TestBuildDRRecipeAppHalf` — assemble-correctness (deployed + non-protected only) + whole-half secret - sweep. -- **Live red-proof:** forcing `buildAppRecipe` to dump `Env` made `TestBuildAppRecipe_NoSecrets` and the - companion FAIL (caught the token value + the `DB_PASSWORD`/`SECRET_KEY`/`IGDB_CLIENT_SECRET` keys); - reverted → green. +| stage | pre-fix image (stale build) | **fixed image (2e85376e)** | +|---|---|---| +| before | 8 | **1** | +| after 40 calls | 48 | **2** | +| after 80 calls | 90 | **2** | +| after 120 calls | 132 | **2** | -## Versioning +**Fixed: flat at 2 (= MaxIdleConnsPerHost), independent of call count.** Pre-fix grew ~1/call. Agent +endpoints still return real data (`/api/disks` lists disks, `/api/host-metrics` returns host CPU/mem). -`recipe_version=1`; read is ignore-unknown for forward-compat. The hub assembles this half with the -agent's storage/guest/PBS half (agent v0.38.0) into one customer recipe. - -## Gate / deploy - -`go build`, `go vet`, `go test ./...` all green (local + build server). Built + pushed image v0.73.0; -deployed to guest 9201 (bootstrap-managed). - -## Deferred (NOT in this slice) - -Free-form non-secret deploy fields (the SPIKE's `non_secret_deploy_fields`) — v1 is the three-field -allowlist only; additional fields land incrementally behind the same allowlist test. No re-provisioning -automation; no recovery-mode consumption. +## Not touched / separate open item +The agent, its bridge-IP `ListenAddr` bind (deliberate defense-in-depth), and all firewall rules were +**not** changed (controller-only fix). **Separate open item (NOT addressed here):** the defense-in-depth +host firewall rule scoping `:8443` to the guest bridge subnet is still absent (`pve-firewall` disabled, +no 8443 rule) — close it independently of this fix. diff --git a/controller/README.md b/controller/README.md index e7963bc..603b1fd 100644 --- a/controller/README.md +++ b/controller/README.md @@ -717,6 +717,12 @@ not just those with HDD data. Non-HDD apps can configure destination, method, an > `EjectDisk`/`FormatDisk(…, confirmed, durableID)`; `DiskInfo.role`+capacity; > `FormatResult.{role,needs_confirmation,durable_id}`; `ErrNeedsConfirmation` (user-data) vs > `ErrFormatRefused` (system/backup). `FormatResult.PendingOp.OpsignCommand()` for the operator path. +> - **(v0.74.0) Client lifecycle — ONE shared client, reused.** `Server.agentClient()` builds the +> `agentapi.Client` once (memoized via `sync.Once`) and returns the same instance to all ~19 call +> sites; the `http.Transport` is bounded + expiring (`MaxIdleConnsPerHost:2`, `IdleConnTimeout:90s`). +> This replaced a per-call `agentapi.New(...)` that leaked one idle ESTABLISHED socket per call and +> exhausted the ephemeral source-port range to the agent's `:8443` after ~5 days (EADDRNOTAVAIL). +> Safe because `cfg.LocalAPI` is static per process (a config-apply triggers a graceful self-restart). > - The **`StoragePath` registry** (`settings.go`: `AddStoragePath`/default/schedulable/label) is unchanged. > - **(v0.64.0) `AutoDiscoverStoragePaths` is now ADDITIVE** — it no longer bails when the registry is > non-empty; instead it registers only deployed-app paths missing from the registry. It never removes