diff --git a/CHANGELOG.md b/CHANGELOG.md index 8511619..cadcdb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,72 @@ +## UNRELEASED — v0.130.0 candidate: the agent was the one leaking connections onto the off-site box (2026-08-20, R-344) + +> **Deliberately not a release heading yet, and the `release-complete` gate is doing its job by +> requiring that.** This build is **hand-installed on `demo-hp` only** so the fix can be proved +> against `demo-felhom` as an untouched control. Publishing it — tag + Gitea package — would put it +> on the control box through the self-update path and destroy the experiment. **When the operator +> authorises the publish, this heading becomes `## v0.130.0` in the same commit as the tag and the +> package**, and the gate then checks it for real. See R-347. + +**What was measured, before anything was changed.** Between 2026-08-18 09:51:22Z and 2026-08-20 +08:02:13Z, ep0's PBS proxy accumulated **388 established connections** — 194 from each demo box, on a +proxy whose descriptor ceiling is 65536 and whose runway at that rate was ~323 days. The connections +were held open on **both** sides: ep0 showed 388 while the two boxes showed 194 + 194, at two separate +instants, with the same source ports on each side, and **not one closed in a 31-minute window**. +`ss -tnp` on the boxes named the holder: **`felhom-agent`**, 194 of 194 on each, one PID. + +**`pvestatd` and `proxmox-backup-client` made 162,404 requests in that window and leaked zero.** They +are 99.5% of the traffic to that endpoint and 0% of the leak. The agent made 811 requests — of which +387 were `GET .../snapshots` — and leaked 388 sockets. One per call, within one. + +**The defect, and it is two things compounding.** + +- `internal/pbs/client.go` built its transport as a composite literal: + `&http.Transport{TLSClientConfig: tlsCfg}`. That takes **`IdleConnTimeout` zero, which does not mean + "use a sane default" — it means retain idle keep-alive connections FOREVER.** + `http.DefaultTransport` sets 90s; hand-rolling the transport (which every client here must do, + because they all pin TLS) silently discards it. +- `pbsTargetsFromPVE` (`cmd/felhom-agent/main.go`) builds **a fresh `pbs.Client` every cycle**, as its + own doc comment says, and drops the previous one. An abandoned `http.Transport` does **not** close + its connections — it becomes unreachable while its `persistConn` read-loop goroutine keeps the + socket alive. So each cycle stranded exactly one connection that nothing could ever close. + +The cadences reconcile without fitting: a 900s live-snapshot collect (184.7 cycles in the window) plus +a 6-hour verify loop (7.7) predicts 192.4 per box against **194 observed**. + +**The fix is one field, restored to the standard library's own value.** New leaf package +`internal/httpx` owns `DefaultIdleConnTimeout = 90 * time.Second` — 90s because that is what +`http.DefaultTransport` uses, so there is nothing invented here to justify or tune — and +`NewTransport(tlsCfg, idleConnTimeout)`, which returns a **fresh** transport (never shared: each caller +pins a different endpoint) and treats a zero or negative timeout as **use the default, never "no +timeout"**. `pbs.Config` gains an `IdleConnTimeout` field that production leaves unset; only tests set +it, to avoid a 90-second wait. + +**`internal/hub/client.go` and `internal/proxmox/client.go` carried the identical missing default and +were corrected in the same pass — but neither contributed to the ep0 leak, and this entry must not be +read as three leaks having been found.** Both are built **once per process**, so they held one idle +connection for the life of the daemon rather than accumulating, and neither talks to ep0:8007. + +**Tests, and what they deliberately do not assert.** `internal/pbs/client_leak_test.go` counts +connections **server-side** and models what `pbsTargetsFromPVE` actually does — build a client, use it +once, drop it on the floor — then asserts the connections go away. It does not assert `err == nil` and +it does not assert that some field holds some value; both were true of the leaking code. + +- **Red-proof 1 (the fix):** removing `IdleConnTimeout` from `NewTransport` fails the test with + *"after 5s the server still holds 5 open connection(s), want 0 (5 dialled in total)"* — the count is + in the message, so the failure cannot be mistaken for a timeout with another cause. Reverted. +- **Red-proof 2 (the fix that would be worse than the bug):** setting `DisableKeepAlives: true` also + makes the leak vanish — by dialling fresh for every request, which on a box polling ~40,000 times a + day is strictly worse than what we started with. **The leak test PASSES under that mutation**; + `TestPBSClient_KeepAliveStillReuses` is what catches it, failing with *"3 sequential requests over 3 + connection(s), want 1"*. Reverted. + +**What this release does NOT do.** It does not reduce the poll rate (**R-336 stays open, but re-scoped +— it was never the cause of this leak**), it does not refactor `pbsTargetsFromPVE` to cache or reuse +clients (a one-line default restores the standard behaviour; a lifecycle refactor adds +cache-invalidation questions for no measurable gain), it adds no `CloseIdleConnections` call, and it +**does not clear the 388 descriptors already stuck on ep0** — those persist until that proxy restarts, +which is not this change's to do. + ## v0.129.0 — a correct code for an earlier package stops being called wrong (2026-08-12, R-311) **The measurement this fixes.** On 2026-08-12 a recovery code that provably opens a RETAINED package diff --git a/REUSE.md b/REUSE.md index 804af99..23efdbe 100644 --- a/REUSE.md +++ b/REUSE.md @@ -86,6 +86,7 @@ | `Client.PoolAddVMID` | internal/proxmox/mutate.go | `PoolAddVMID(ctx, pool, vmid) error` | re-assert pool membership after a restore-over-existing (campaign-2 R2) | SYNC (no UPID, don't WaitTask); PVE `PUT /pools` is additive (merge, not replace) — `delete=1` removes; idempotent (already-member swallowed); needs `Pool.Allocate` at `/pool/`. `pct restore --pool` sets membership only at CREATE — a restore over an existing vmid drops it, so bring-up re-asserts post-restore | | `TLSConfig.build` / `normalizeFingerprint` | internal/proxmox/tls.go | `build() (*tls.Config, error)` | PVE leaf-cert SHA-256 pinning | No insecure default | | `pinnedTLS` | internal/pbs/pin.go | `pinnedTLS(fingerprint) (*tls.Config, error)` | PBS leaf pinning | Same model as PVE; 64-hex fingerprint normalized | +| `httpx.NewTransport` | internal/httpx/transport.go | `NewTransport(tlsCfg, idleConnTimeout) *http.Transport` | **EVERY** hand-rolled `http.Transport` in this repo — pbs, hub and proxmox all pin TLS, so none can use `http.DefaultTransport` | **R-344: never inline `&http.Transport{TLSClientConfig: ...}` again.** A composite literal takes `IdleConnTimeout` **zero, which means retain idle connections FOREVER** — `http.DefaultTransport` sets 90s and a literal does not inherit it. Combined with a client rebuilt per cycle and dropped (`pbsTargetsFromPVE`), that stranded **388 sockets on ep0 in 46 h**, held open on BOTH sides. `idleConnTimeout <= 0` means **use the default**, never "no timeout". Returns a **FRESH** transport every call — a shared one would pool connections across differently pinned endpoints. Pinned by `internal/pbs/client_leak_test.go` (server-side connection counting) + `internal/httpx/transport_test.go` | | `hub.Client.Report` | internal/hub/client.go | `Report(ctx, *HostReport) (*ControlEnvelope, error)` | the heartbeat | Typed `TransportError`/`HTTPError`, never contain the bearer token | | `hub.Loop` + `MultiObserver` | internal/hub/loop.go | `NewLoop(...)`; `MultiObserver(obs...)` | resilient report loop + envelope fan-out | Errors logged, loop continues; interval clamped 60–3600 s | | `provision.BackHalf.Provision` | internal/provision/backhalf.go | `Provision(ctx, Input) (Result, error)` | guest bootstrap back-half | mint→render→0600 write→chown 100000:100000→`pct set` ro bind→onboot; token NEVER logged/returned. Bootstrap `local_api.endpoint` = the caller's `cfg.LocalAPI.ListenAddr` (main.go) — moving the agent bind to the island moves the guest dial for free (R-50, no template) | diff --git a/cmd/felhom-agent/main.go b/cmd/felhom-agent/main.go index 605f5d0..84e9748 100644 --- a/cmd/felhom-agent/main.go +++ b/cmd/felhom-agent/main.go @@ -59,7 +59,7 @@ import ( // version is the agent version. Overridable at build time with // -ldflags "-X main.version="; defaults to the in-repo CHANGELOG version. -var version = "0.92.1" +var version = "0.130.0" // runGuestHook is the PVE hook body (`felhom-agent guest-hook `). On pre-start it // creates placeholder dirs for any absent bind-mount source so the guest always boots (the C1 net); diff --git a/internal/httpx/transport.go b/internal/httpx/transport.go new file mode 100644 index 0000000..9407ee7 --- /dev/null +++ b/internal/httpx/transport.go @@ -0,0 +1,59 @@ +// Package httpx holds the one HTTP-transport default this repo may not lose. +// +// Every client here pins TLS — PBS and PVE by leaf-cert SHA-256, the hub by an optional CA file — +// so none of them can use http.DefaultTransport and each hand-rolls its own. Hand-rolling silently +// discards DefaultTransport's settings, and one of them is load-bearing: +// +// Transport: &http.Transport{TLSClientConfig: tlsCfg} // IdleConnTimeout == 0 == NO timeout +// +// A zero IdleConnTimeout means idle keep-alive connections are retained FOREVER, not "use a sane +// default". Combined with a client that is rebuilt on a schedule and dropped (pbsTargetsFromPVE +// builds a fresh pbs.Client per cycle), every cycle strands one connection that nothing will ever +// close: the abandoned Transport becomes unreachable but its persistConn read-loop goroutine keeps +// the socket alive, and an unreachable Transport does not close its connections. +// +// Measured cost, live: 388 established connections accumulated on ep0's PBS proxy between +// 2026-08-18 09:51:22Z and 2026-08-20 08:02:13Z — 194 from each of the two boxes, held open on BOTH +// sides, one per agent poll cycle, on a proxy whose descriptor ceiling is 65536. See R-344 and +// felhom.eu/documentation/audits/SPIKE-ep0-established-connections-2026-08-20.md. +package httpx + +import ( + "crypto/tls" + "net/http" + "time" +) + +// DefaultIdleConnTimeout is how long an idle keep-alive connection is retained before it is closed. +// +// It is 90s because that is http.DefaultTransport's own value: the fix for R-344 restores a +// standard-library default rather than inventing a number, so there is nothing here to tune and +// nothing to justify. It is comfortably shorter than every cadence that drives these clients (the +// 15-minute live-snapshot collect and the 6-hour verify loop), so a connection abandoned by one +// cycle is closed long before the next. +const DefaultIdleConnTimeout = 90 * time.Second + +// NewTransport builds a FRESH *http.Transport pinned to tlsCfg, with the idle-connection timeout +// applied. +// +// Fresh, never shared: each caller pins a different endpoint, and a shared transport would pool +// connections across differently pinned servers. Reusing http.DefaultTransport for the same reason +// is not an option — it would drop the pin entirely. +// +// idleConnTimeout <= 0 means USE THE DEFAULT. It deliberately does not mean "no timeout": no-timeout +// is the bug this package exists to prevent, and an unset field must never be able to reintroduce +// it. Callers pass their configured value straight through; only tests pass a short one. +// +// Only IdleConnTimeout is set. The other DefaultTransport settings this transport also lacks +// (MaxIdleConns, TLSHandshakeTimeout, ExpectContinueTimeout) are deliberately left alone: none of +// them accumulates anything, every client bounds its whole request with http.Client.Timeout, and +// widening the change would have made the R-344 measurement unattributable. +func NewTransport(tlsCfg *tls.Config, idleConnTimeout time.Duration) *http.Transport { + if idleConnTimeout <= 0 { + idleConnTimeout = DefaultIdleConnTimeout + } + return &http.Transport{ + TLSClientConfig: tlsCfg, + IdleConnTimeout: idleConnTimeout, + } +} diff --git a/internal/httpx/transport_test.go b/internal/httpx/transport_test.go new file mode 100644 index 0000000..0338918 --- /dev/null +++ b/internal/httpx/transport_test.go @@ -0,0 +1,74 @@ +package httpx + +import ( + "crypto/tls" + "net/http" + "testing" + "time" +) + +// TestNewTransport_ZeroMeansDefaultNeverForever is the whole point of this package. +// +// http.Transport's zero IdleConnTimeout means "retain idle connections FOREVER". Any code path that +// can reach that zero reintroduces R-344, so an unset, zero or negative value must all land on the +// default. If someone later "simplifies" NewTransport by passing the argument straight through, +// this fails. +func TestNewTransport_ZeroMeansDefaultNeverForever(t *testing.T) { + for _, tc := range []struct { + name string + in time.Duration + want time.Duration + }{ + {"zero", 0, DefaultIdleConnTimeout}, + {"negative", -time.Hour, DefaultIdleConnTimeout}, + {"explicit short value (tests)", 50 * time.Millisecond, 50 * time.Millisecond}, + {"explicit long value", time.Hour, time.Hour}, + } { + t.Run(tc.name, func(t *testing.T) { + got := NewTransport(&tls.Config{MinVersion: tls.VersionTLS12}, tc.in).IdleConnTimeout + if got != tc.want { + t.Fatalf("IdleConnTimeout = %v, want %v", got, tc.want) + } + if got == 0 { + t.Fatal("IdleConnTimeout is 0 — that is 'never expire', which is the R-344 defect itself") + } + }) + } +} + +// TestDefaultIdleConnTimeout_MatchesTheStandardLibrary pins the number to its justification. +// +// 90s is not a tuned value; it is what http.DefaultTransport uses. Reading it off the standard +// library rather than hardcoding 90 means the constant cannot drift away from the reason given for +// it in the package doc. +func TestDefaultIdleConnTimeout_MatchesTheStandardLibrary(t *testing.T) { + std, ok := http.DefaultTransport.(*http.Transport) + if !ok { + t.Skip("http.DefaultTransport is not an *http.Transport in this Go build") + } + if DefaultIdleConnTimeout != std.IdleConnTimeout { + t.Fatalf("DefaultIdleConnTimeout = %v but http.DefaultTransport uses %v — the doc comment's justification no longer holds", + DefaultIdleConnTimeout, std.IdleConnTimeout) + } +} + +// TestNewTransport_IsFreshEveryCall guards the pooling property the pinning relies on. +// +// Each caller pins a DIFFERENT endpoint. A shared transport would pool connections across +// differently pinned servers, so returning a package-level singleton would be a security change +// dressed as a tidy-up. +func TestNewTransport_IsFreshEveryCall(t *testing.T) { + a := NewTransport(&tls.Config{MinVersion: tls.VersionTLS12}, 0) + b := NewTransport(&tls.Config{MinVersion: tls.VersionTLS12}, 0) + if a == b { + t.Fatal("NewTransport returned the SAME transport twice — connections would be pooled across differently pinned endpoints") + } +} + +// TestNewTransport_KeepsTheTLSConfig — the transport gains a field; it must lose nothing. +func TestNewTransport_KeepsTheTLSConfig(t *testing.T) { + cfg := &tls.Config{MinVersion: tls.VersionTLS12, InsecureSkipVerify: true} //nolint:gosec // test only + if got := NewTransport(cfg, 0).TLSClientConfig; got != cfg { + t.Fatalf("TLSClientConfig = %p, want the config passed in (%p) — the pin would be dropped", got, cfg) + } +} diff --git a/internal/hub/client.go b/internal/hub/client.go index 9ffc6ad..1859b57 100644 --- a/internal/hub/client.go +++ b/internal/hub/client.go @@ -15,6 +15,8 @@ import ( "time" "gitea.dooplex.hu/admin/felhom-agent/internal/config" + + "gitea.dooplex.hu/admin/felhom-agent/internal/httpx" ) const reportPath = "/api/v1/host-report" @@ -49,8 +51,11 @@ func NewClient(cfg config.HubConfig, logger *slog.Logger) (*Client, error) { tlsCfg.RootCAs = pool } hc := &http.Client{ - Timeout: time.Duration(cfg.TimeoutSeconds) * time.Second, - Transport: &http.Transport{TLSClientConfig: tlsCfg}, + Timeout: time.Duration(cfg.TimeoutSeconds) * time.Second, + // R-344, consistency only: this client is built ONCE per process, so it never accumulated + // and contributed nothing to the ep0 leak. It carried the same missing default, which over + // a tunnel is how one idle connection survives long enough to fail on next use. + Transport: httpx.NewTransport(tlsCfg, 0), } return newClient(cfg.URL, cfg.APIKey, cfg.HostID, hc, logger), nil } diff --git a/internal/pbs/client.go b/internal/pbs/client.go index 577af9d..b11eb05 100644 --- a/internal/pbs/client.go +++ b/internal/pbs/client.go @@ -10,6 +10,8 @@ import ( "net/url" "strings" "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/httpx" ) // Client is the PBS-API client for ONE PBS server. Construct with NewClient. It is pure (no @@ -31,6 +33,11 @@ type Config struct { Secret string // token secret (from .pw) Namespace string // PBS namespace (from storage.cfg `namespace`); "" = root. S4 per-customer tenancy. Timeout time.Duration + + // IdleConnTimeout bounds how long this client's idle keep-alive connections are retained. + // Zero means httpx.DefaultIdleConnTimeout (90s) — it does NOT mean "no timeout", which is the + // R-344 defect. Production leaves it unset; only tests set it, to avoid a 90-second wait. + IdleConnTimeout time.Duration } // NewClient builds a fingerprint-pinned, token-authed PBS client. @@ -55,8 +62,13 @@ func NewClient(cfg Config) (*Client, error) { authHeader: "PBSAPIToken=" + cfg.TokenID + ":" + cfg.Secret, namespace: cfg.Namespace, http: &http.Client{ - Timeout: timeout, - Transport: &http.Transport{TLSClientConfig: tlsCfg}, + Timeout: timeout, + // R-344: this transport MUST come from httpx. pbsTargetsFromPVE (cmd/felhom-agent/ + // main.go) builds a fresh Client every cycle and drops the previous one, so a + // transport with no idle timeout strands one connection per cycle, forever, on both + // sides. That leaked 388 sockets onto ep0 in 46 hours. Pinned by + // TestAbandonedClientsReleaseTheirConnections — do not inline an http.Transport here. + Transport: httpx.NewTransport(tlsCfg, cfg.IdleConnTimeout), }, }, nil } diff --git a/internal/pbs/client_leak_test.go b/internal/pbs/client_leak_test.go new file mode 100644 index 0000000..21d18aa --- /dev/null +++ b/internal/pbs/client_leak_test.go @@ -0,0 +1,173 @@ +package pbs + +import ( + "context" + "net" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/httpx" +) + +// connCounter is the SERVER-side observer. It counts what the server actually holds, which is the +// only thing that answers the question this file exists for: a client that believes it closed a +// connection, and a server still holding the socket, is precisely the R-344 shape. Asserting on +// anything client-side would be asserting the mechanism instead of the consequence. +type connCounter struct { + mu sync.Mutex + open int + total int // every connection ever accepted — how many times the client DIALLED +} + +func (c *connCounter) hook(_ net.Conn, s http.ConnState) { + c.mu.Lock() + defer c.mu.Unlock() + switch s { + case http.StateNew: + c.open++ + c.total++ + case http.StateClosed, http.StateHijacked: + c.open-- + } +} + +func (c *connCounter) counts() (open, total int) { + c.mu.Lock() + defer c.mu.Unlock() + return c.open, c.total +} + +// waitForOpen polls until the server holds want connections, or fails naming what it still holds. +func (c *connCounter) waitForOpen(t *testing.T, want int, within time.Duration, what string) { + t.Helper() + deadline := time.Now().Add(within) + for { + open, total := c.counts() + if open == want { + return + } + if time.Now().After(deadline) { + t.Fatalf("%s: after %s the server still holds %d open connection(s), want %d (%d dialled in total)", + what, within, open, want, total) + } + time.Sleep(5 * time.Millisecond) + } +} + +// newCountingPBSServer is newPBSTestServer plus a ConnState hook. Kept separate rather than +// changing the shared helper, so the existing tests are untouched by this file. +func newCountingPBSServer(t *testing.T, fn http.HandlerFunc) (*httptest.Server, string, *connCounter) { + t.Helper() + cc := &connCounter{} + ts := httptest.NewUnstartedServer(fn) + ts.Config.ConnState = cc.hook + ts.StartTLS() + t.Cleanup(ts.Close) + return ts, fingerprintOf(ts), cc +} + +// TestAbandonedClientsReleaseTheirConnections is Scenario A, and it is the load-bearing test for +// R-344. +// +// It models what pbsTargetsFromPVE actually does — build a client, use it once, drop it on the +// floor without closing anything — and asserts the CONSEQUENCE on the server: the connections go +// away. Before the fix every one of these stayed established forever on both sides; 388 of them +// accumulated on ep0 in 46 hours. +// +// Deliberately NOT asserted: that err == nil, or that IdleConnTimeout holds some value. Both were +// true of the leaking code. +func TestAbandonedClientsReleaseTheirConnections(t *testing.T) { + ts, fp, cc := newCountingPBSServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte(`{"data":[]}`)) + }) + host, port := hostPort(t, ts.URL) + + const cycles = 5 + for i := 0; i < cycles; i++ { + // One fresh client per "cycle", exactly as pbsTargetsFromPVE builds one per collect. + c, err := NewClient(Config{ + Server: host, Port: port, Fingerprint: fp, TokenID: "u@pbs!t", Secret: "s", + IdleConnTimeout: 50 * time.Millisecond, // production uses the 90s default + }) + if err != nil { + t.Fatal(err) + } + if _, err := c.Snapshots(context.Background(), "ds"); err != nil { + t.Fatalf("cycle %d: %v", i, err) + } + _ = c // dropped here — nothing closes it, nothing can + } + + if _, total := cc.counts(); total != cycles { + t.Fatalf("setup is not modelling the leak: want %d separate dials (one per abandoned client), got %d", cycles, total) + } + cc.waitForOpen(t, 0, 5*time.Second, "abandoned pbs.Clients") +} + +// TestAbandonedClientsReleaseTheirConnections_ProductionDefaultIsUsable pins the value that ships. +// +// The field being settable is exactly how it could silently become zero again — and zero used to +// mean "never expire". This asserts the production path (Config leaving it unset) lands on the +// standard-library default, so the leak cannot return through an unset field. +func TestPBSClient_UnsetIdleTimeoutUsesTheDefault(t *testing.T) { + for _, tc := range []struct { + name string + cfg time.Duration + want time.Duration + }{ + {"unset — the production path", 0, httpx.DefaultIdleConnTimeout}, + {"explicit zero is NOT no-timeout", 0, httpx.DefaultIdleConnTimeout}, + {"negative is NOT no-timeout", -time.Second, httpx.DefaultIdleConnTimeout}, + {"an explicit value is honoured", 3 * time.Second, 3 * time.Second}, + } { + t.Run(tc.name, func(t *testing.T) { + c, err := NewClient(Config{ + Server: "pbs.example", Fingerprint: strings.Repeat("ab", 32), + TokenID: "u@pbs!t", Secret: "s", IdleConnTimeout: tc.cfg, + }) + if err != nil { + t.Fatal(err) + } + tr, ok := c.http.Transport.(*http.Transport) + if !ok { + t.Fatalf("transport is %T, not *http.Transport — the httpx wiring was replaced", c.http.Transport) + } + if tr.IdleConnTimeout != tc.want { + t.Fatalf("IdleConnTimeout = %v, want %v (zero would mean connections are retained FOREVER — that is R-344)", tr.IdleConnTimeout, tc.want) + } + }) + } +} + +// TestPBSClient_KeepAliveStillReuses is Scenario C, and it is the guard against a "fix" that is +// worse than the bug. +// +// Disabling keep-alive entirely would also make the leak go away — by dialling a fresh connection +// for every single request, which on a box polling ~40,000 times a day is strictly worse than what +// we started with. The fix must retire IDLE connections without stopping reuse. +func TestPBSClient_KeepAliveStillReuses(t *testing.T) { + ts, fp, cc := newCountingPBSServer(t, func(w http.ResponseWriter, _ *http.Request) { + w.Write([]byte(`{"data":[]}`)) + }) + host, port := hostPort(t, ts.URL) + + c, err := NewClient(Config{ + Server: host, Port: port, Fingerprint: fp, TokenID: "u@pbs!t", Secret: "s", + IdleConnTimeout: 30 * time.Second, // long enough that reuse is what is being measured + }) + if err != nil { + t.Fatal(err) + } + for i := 0; i < 3; i++ { + if _, err := c.Snapshots(context.Background(), "ds"); err != nil { + t.Fatalf("request %d: %v", i, err) + } + } + if _, total := cc.counts(); total != 1 { + t.Fatalf("one client made 3 sequential requests over %d connection(s), want 1 — keep-alive reuse is broken, which would make the poll load WORSE than the leak", total) + } +} diff --git a/internal/proxmox/client.go b/internal/proxmox/client.go index b162059..f9c273b 100644 --- a/internal/proxmox/client.go +++ b/internal/proxmox/client.go @@ -10,6 +10,8 @@ import ( "net/url" "strings" "time" + + "gitea.dooplex.hu/admin/felhom-agent/internal/httpx" ) // doer is the minimal HTTP surface the client needs; *http.Client satisfies it. @@ -65,8 +67,10 @@ func NewClient(cfg Config) (*Client, error) { timeout = 30 * time.Second } hc := &http.Client{ - Timeout: timeout, - Transport: &http.Transport{TLSClientConfig: tlsCfg}, + Timeout: timeout, + // R-344, consistency only: built ONCE per process, so it never accumulated and contributed + // nothing to the ep0 leak. Same missing default, corrected for the same reason. + Transport: httpx.NewTransport(tlsCfg, 0), } return &Client{ base: strings.TrimRight(cfg.Endpoint, "/") + "/api2/json",