go-proxmox v0.7.0+ follow-ups: adopt new client options & evaluate new API surface

[wikkyk]

Following #813 (bump to go-proxmox v0.7.0), the release opens up new client options and API surface worth adopting/evaluating. Filing as one issue to start the discussion — happy to split into focused issues/PRs.

Refs: go-proxmox v0.7.0 release · README → Usage with Client Options.

Context: both client-construction sites — setupProxmoxClient() in cmd/main.go and pkg/scope/cluster.go — hand-roll an http.Transport/tls.Config and pass it via WithHTTPClient, with no timeout and no retry. RootCAs are built by internal/tlshelper (system roots merged with a custom cert).

---

Client options worth adopting

WithTimeout — resilience (recommended)

Neither client sets http.Client.Timeout, and the bare transport has no dial/TLS-handshake/idle timeouts, so a wedged PVE endpoint can block a reconcile worker indefinitely (we never wrap calls in context.WithTimeout). One option in NewAPIClient bounds every request and covers both sites at once:

opts = append(opts, proxmox.WithTimeout(60*time.Second)) // ideally flag/env-configurable

Ref: luthermonson/go-proxmox#327

TLS options — delete the duplicated hand-rolled transport (recommended)

WithInsecureSkipVerify() + WithRootCAs(pool) let us drop the http.Transport/http.Client boilerplate in both sites (go-proxmox then builds the transport off http.DefaultTransport, which also restores sane dial/idle timeouts). Keep tlshelper: WithRootCAs replaces the pool, it does not merge system roots —

// upstream: WithRootCAs(pool) => tc.RootCAs = pool   (replace, not append)
opts = append(opts, proxmox.WithRootCAs(rootCerts))      // rootCerts still from tlshelper
if insecure { opts = append(opts, proxmox.WithInsecureSkipVerify()) }

⚠️ Do not swap tlshelper.SystemRootsWithFile for upstream WithRootCAFile — it builds from an empty pool and would drop system-root trust (security regression). Heads-up: letting go-proxmox clone DefaultTransport also turns on ProxyFromEnvironment, so HTTP(S)_PROXY would start being honored — a conscious call to make.
Ref: luthermonson/go-proxmox#327

WithRetry — consider, scoped

Context-aware retries (respects ctx.Done(), so it won't fight controller-runtime requeue). The default condition retries net errors + 502/503/504/429 only. If we adopt it, scope a custom condition to also cover pveproxy's backend-unreachable codes — but not 500/501 (Proxmox overloads 500 for non-transient errors, e.g. 500 QEMU guest agent is not running):

proxmox.WithRetry(proxmox.WithRetryCondition(func(r *http.Response, err error) bool {
    if err != nil || r == nil { return err != nil }
    switch r.StatusCode { case 502, 503, 504, 429, 595, 596, 599: return true }
    return false
}))

Caveat: a net error on a non-idempotent POST (e.g. clone) is retried, but our clones use a pre-allocated VMID, so a replay errors as "VMID in use" rather than duplicating — and existing requeue + taskservice retry self-heal. Marginal value over what we already have; discussion welcome.
Ref: luthermonson/go-proxmox#326

WithRequestInterceptor — consider (observability)

No metrics/tracing on the Proxmox call path today. An interceptor (runs after auth headers; must return nil or it short-circuits the request) could add per-request metrics, OpenTelemetry spans, or correlation IDs across both sites.
Ref: luthermonson/go-proxmox#325

Not applicable / minor

• WithEagerAuth, WithOTP, WithDefaultRealm — documented no-ops under API-token auth (we're token-only via WithAPIToken).
• WithProxy / WithProxyFromEnvironment — no current need (feat(options): add WithProxy and WithProxyFromEnvironment luthermonson/go-proxmox#324); see the proxy heads-up under TLS above.
• WithUserAgent (minor) — we currently present the default go-proxmox/dev UA in PVE logs.

---

New API surface (nice-to-have)

Token permission preflight via Subdirs / diridx

v0.7.0 wraps PVE's directory-index endpoints as ACL-filtered capability probes ((*Cluster).Subdirs, (*Node).Subdirs). Our only startup check is Version(); a preflight could fail fast with a clear message on a misconfigured token instead of surfacing as a 403 deep inside clone/config. Ceiling: diridx reports read visibility, not the write privileges we actually need (VM.Clone/VM.Config/VM.Allocate, Datastore.AllocateSpace), so it catches gross misconfig only.
Ref: luthermonson/go-proxmox#319

Storage.Status — clone-target / scheduling checks

(*Storage).Status returns typed Active/Enabled/Shared + capacity. Could (a) gate clones on the target storage being active+enabled, and/or (b) make placement disk-aware — the scheduler accounts for memory only today (GetReservableMemoryBytes).
Ref: luthermonson/go-proxmox#319

Out of scope for now (listed for completeness)

SDN (luthermonson/go-proxmox#320); HA / backup jobs / replication (luthermonson/go-proxmox#274, luthermonson/go-proxmox#322); custom CPU models; bulk-action. All orthogonal to our stateless node-VM model.

---

Update — go-proxmox v0.7.1 → v0.8.0

v0.7.1 is a correctness-only patch (JSON-tag/scalar-type fixes on response structs). No new client options or API surface — nothing in this issue changes.

v0.8.0 adds two things relevant here:

TLS default reinforces the transport-cleanup item above

ensureTLSConfig now defaults to MinVersion: tls.VersionTLS12. That floor only applies when go-proxmox builds its own transport — capmox supplies its own via WithHTTPClient, and that hand-rolled tls.Config sets no MinVersion today. So the WithRootCAs/WithInsecureSkipVerify cleanup recommended above would now also inherit a TLS 1.2 minimum for free: one more reason to drop the hand-rolled transport rather than carry it forward.

VirtualMachineDeleteOptions — directly serves the HA track (#216, #791)

v0.8.0 changes VirtualMachine.Delete(ctx) → Delete(ctx, *VirtualMachineDeleteOptions), exposing Purge, SkipLock, and DestroyUnreferencedDisks (the optional DELETE /nodes/{node}/qemu/{vmid} query params). For ordinary stateless teardown nil is the right default — but Purge is precisely what the HA work needs:

• The ability for the CAPMOX provider to work with HA #216 / Proxmox HA support for ProxmoxMachines: register on create, delete with purge (fixes #216) #791: PVE rejects deleting an HA-managed VM without purge=1 ("used in HA resources and purge parameter not set"). Proxmox HA support for ProxmoxMachines: register on create, delete with purge (fixes #216) #791 currently hand-rolls this in a deleteVMWithPurge helper that issues a raw c.Delete(ctx, ".../qemu/{vmid}?purge=1", …) — explicitly because "go-proxmox's vm.Delete does not expose a purge parameter." As of v0.8.0 it does, so that helper can collapse to vm.Delete(ctx, &proxmox.VirtualMachineDeleteOptions{Purge: true}). Bonus: the hand-rolled path bypasses vm.Delete and therefore skips its cloud-init ISO cleanup (deleteCloudInitISO, unexported and unreachable from capmox); routing back through vm.Delete restores that cleanup for HA VMs.
• As the broader HA / failure-domain / resource-aware-scheduling work lands ([Proposal] Support for multi Proxmox Clusters (Datacenters) #370, feat: implement CAPI failure domain contract for v1alpha2 zones #717, feat(scheduler): tolerance-weighted CPU/memory aware VM scheduling #747) and VMs begin carrying HA / replication / backup-job references, Purge is the correct teardown switch to drop those references along with the VM.
• DestroyUnreferencedDisks stays off by default (footgun: it can wipe unreferenced disks on shared storage); SkipLock likewise off.

This doesn't override the "HA management API is out of scope" note above — capmox still wouldn't manage HA groups; it would only opt the existing delete path into upstream's purge flag instead of a bespoke query string. Natural follow-up once #816 (the v0.8.0 bump) and #791 are both merged.

---

🤖 Both the analysis and this write-up were done by Claude (Opus 4.8) via Claude Code, working against the go-proxmox v0.7.0–v0.8.0 source; the upstream migration guides (v0.7.0 · v0.8.0) have the wider context. Snippets above are illustrative, not full diffs.