Skip to content

FM-601: Proxmox guest associations and guest-agent data - #102

Merged
Andreas-Froyland merged 3 commits into
mainfrom
fm-601-proxmox-guest-associations
Sep 20, 2026
Merged

Andreas-Froyland merged 3 commits into
mainfrom
fm-601-proxmox-guest-associations

Conversation

@Andreas-Froyland

@Andreas-Froyland Andreas-Froyland commented Sep 20, 2026

Copy link
Copy Markdown
Member

Closes #101 (FM-601).

Implementation approach (from the issue)

FM-600 delivered the accounts, the pinned-fingerprint trust flow, and cluster-resource discovery. This PR adds the guest layer, read-only end to end.

What landed

  • Provider (fleet-provider-proxmox): guest_discover on the ProxmoxSource port — the cluster snapshot names the guests, the guest config carries their netN MACs (normalized lowercase, normalize_mac refuses non-MACs), and the QEMU Guest Agent carries the inner facts: info (online/version), network-get-interfaces (interfaces/MACs/addresses), get-osinfo (OS/kernel). Every surface degrades independently: an off guest's agent reports online: false with the honest warning; loopback-style interfaces without association evidence are skipped; a guest whose config fails warns and survives. A cluster entry without a node or vmid is isolated into warnings rather than guessed.
  • Association evidence (fleet-application/src/proxmox.rs): association_candidate correlates guests with Fleet machines by candidates — MAC evidence (agent interfaces + config MACs vs the machine's recorded net/mac* facts, outranking the rest), address evidence (agent IPs vs machine endpoint hosts), name evidence (guest name vs machine name, case-insensitive). Evidence only: candidates warn, never merge. The sensitive-detail rule from FM-213 applies: when machine.read.sensitive is denied, candidates come back empty rather than half-redacted lies (the redacted reference still carries the host, so the explicit gate is required).
  • Observation recording: observe_guest records the guest's facts onto the confirmed machine as capability facts through the existing record_capabilities funnel — pve/guest|vmid|node|agent|os|kernel, net/mac0…, provenance proxmox/<version>. Availability honesty: an off QEMU guest's agent is unavailable (may be off, not agentless); an LXC guest's agent is unknown (no qemu-guest-agent by design). The machine funnel authorizes and audits the write; the Proxmox read is audited separately.
  • Surfaces: GET /api/v1/proxmox/accounts/{id}/guests and POST /api/v1/proxmox/accounts/{id}/guests/{vmid}/observe in the OpenAPI doc (client regenerated); fleetctl proxmox guests and fleetctl proxmox observe-guest.
  • Trust gate: guest discovery sits behind the same explicit observe→confirm gate as cluster discovery — no credential-carrying call leaves Fleet for an unconfirmed account.

Tests

  • Provider unit tests: config MAC parsing, MAC normalization refusals, interface normalization/loopback skip.
  • Application tests over fakes: evidence-only candidates (address/MAC), MAC outranks address/name, unmatched guests list empty, sensitive-denial degradation, observe-guest fact recording (namespaces/provenance/status), off-vs-agentless honesty, unknown guest 404, trust gate, denial before ports.
  • Controller e2e over the real router + secret store: guests list with candidates, observe records onto the machine (verified through the machine read surface), unknown guest 404.
  • CLI render/parse contract tests. cargo xtask verify passes; live smoke remains on the epic's real-cluster checklist.

Non-goals (respected)

Lifecycle actions (epic #11), template/clone/snapshot (epic #12), Lab leases; nothing writes to PVE.


Summary by cubic

Closes #101 (FM-601). Proxmox accounts previously stopped at cluster discovery; now guests can be listed with association candidates and observed onto Fleet machines, read-only.

What changed

  • guest_discover reads cluster resources, guest config MACs, and QEMU guest-agent facts; off QEMU agents report unavailable, LXC agents report unknown, and failed guest configs warn and continue.
  • Association candidates are evidence-only and ranked by MAC, address, then name; address evidence now matches IPv6 endpoint hosts, and sensitive-denied reads return empty candidates instead of half-redacted ones.
  • observe_guest records guest facts onto the confirmed machine through the existing capability funnel with proxmox/<version> provenance.
  • Adds GET /api/v1/proxmox/accounts/{id}/guests, POST /api/v1/proxmox/accounts/{id}/guests/{vmid}/observe, fleetctl proxmox guests, and fleetctl proxmox observe-guest; guest lists paginate, and stale cursors are refused with 400.
  • Discovery stays behind the confirmed-fingerprint trust gate; lifecycle, template/clone/snapshot, and Lab lease actions remain out of scope.

Written for commit 63fb3d5. Summary will update on new commits.

Review in cubic

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 issues found across 11 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="crates/fleet-application/src/proxmox.rs">

<violation number="1" location="crates/fleet-application/src/proxmox.rs:1014">
P2: When more than 200 machines exist, guests cannot produce candidates for machines after this hard limit. Paginate through the complete machine set, or provide an association query that evaluates all machines without truncating the result.</violation>
</file>

<file name="crates/fleetctl/src/lib.rs">

<violation number="1" location="crates/fleetctl/src/lib.rs:2844">
P2: An empty guests response renders as the accounts list. The new guests block guards on `items.first().is_some_and(|item| item.get("vmid").is_some())`, so `{"items": []}` from `proxmox guests <account>` (a guest-less account is a valid input) fails the guard and falls through to the accounts-list branch, which prints "(no Proxmox accounts)". The enclosed `if items.is_empty()` is unreachable dead code: whenever this branch is entered the first item exists and carries `vmid`. Because both the guests and accounts endpoints return the same `{"items": [...], "page": ...}` envelope shape, the `vmid` probe cannot discriminate empty lists; the renderer needs an unambiguous marker (or command context) before "(no guests reported)" can ever run.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/fleetctl/src/lib.rs
Comment thread crates/fleet-api/src/proxmox.rs
Comment thread crates/fleet-api/src/proxmox.rs Outdated
Comment thread crates/fleet-controller/tests/proxmox.rs
Comment thread crates/fleet-application/src/proxmox.rs Outdated
Comment thread crates/providers/fleet-provider-proxmox/src/lib.rs Outdated
Comment thread crates/fleet-controller/tests/proxmox.rs
Comment thread crates/fleetctl/tests/cli.rs
Comment thread crates/fleetctl/src/lib.rs
Comment thread crates/fleet-application/src/proxmox.rs Outdated
@Andreas-Froyland

Copy link
Copy Markdown
Member Author

All 19 findings addressed in 6bf4158:

P1

  • proxmox guests never used the Proxmox renderer: the guests commands now dispatch to a dedicated command-scoped render_proxmox_guests (the guests and accounts endpoints share the page envelope, so the renderer is scoped by command rather than guessing from the payload — which also fixes the empty-list fall-through).

P2s

  • Empty guests response rendered as the accounts list: fixed by the command-scoped renderer; (no guests reported) is reachable and contract-tested.
  • Cluster-level warnings dropped: guests now returns a GuestSnapshot carrying warnings; the API reports them on the first page (they describe the snapshot, not a page of it).
  • IPv6 bracket retention: the host extraction now uses the shared reference_host helper, which strips IPv6 brackets (see the P3 dedup below).
  • All-zero loopback MAC as evidence: normalize_interface filters 00:00:00:00:00:00 before it can become evidence.
  • Constant fixture masked wrong-guest associations: the controller fixtures are keyed on the VMID path — qemu/100 carries MAC …:09/IP .241, qemu/101 …:01/.240 — and the test now asserts guest 100 has no candidate, so the association provably comes from the guest's own evidence.
  • Double info call: collapsed into one call; the misleading comment is gone.
  • Undocumented error statuses: both guest endpoints now document every reachable status from map_proxmox_error (404/409/400/500/502).
  • Unbounded guest snapshot: the guests endpoint takes limit/cursor (defaults shared from envelope), applies them before PageInfo, and advances the cursor by guest id.
  • Network call before machine authorization: observe_guest authorizes MachineUpdate for the machine before loading the secret or calling the guest-discovery port.
  • Silent MAC-less netN: config_macs returns warnings; a netN entry without a parseable MAC warns on the guest (regression test added).
  • >200 machines truncated: the machine page bound was the documented follow-up from FM-213's correlation; this PR keeps the bound but the snapshot's warnings do not hide it — a dedicated association query over all machines is the recorded follow-up for the association scale-up, and the bound is now commented at the call site. (If you'd rather I paginate through the full machine set now, say so and I'll add it to this PR.)
  • machine_id dead param: removed from guest_facts and its call site.
  • map_api_error duplication: discover now uses the shared helper too.
  • PveGuest.agent doc: describes the implemented behavior (None only for LXC; per-surface availability inside; config failure warns separately).

P3s

  • Fixture inconsistency: qemu/101 is now running, consistent with its live agent.
  • Render branches unpinned: the contract test pins "unreachable", the LXC "-" agent marker, empty candidates "-", and the empty-page "(no guests reported)" branch.
  • usage() missing the new verbs: both proxmox guests and proxmox observe-guest are in the help text.
  • Host-extraction duplication: reference_host is promoted to pub(crate) in onboarding (now IPv6-bracket-aware) and reused here.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 11 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread crates/fleet-api/src/proxmox.rs
Comment thread crates/fleet-api/src/proxmox.rs Outdated
@Andreas-Froyland

Copy link
Copy Markdown
Member Author

Round-2 re-review addressed in 63fb3d5:

  • Stale/malformed cursor silently restarting: the handler now returns invalid_request (400) via the shared invalid_request helper when the cursor names no guest in the snapshot.
  • limit/cursor absent from OpenAPI: both query parameters are in the listProxmoxGuests params annotation; the document and the generated client are regenerated.

(The 18:02 batch was round 1, addressed in the previous commit — the two findings above are the only new ones in the re-review.)

@Andreas-Froyland
Andreas-Froyland merged commit b3df986 into main Sep 20, 2026
12 checks passed
@Andreas-Froyland
Andreas-Froyland deleted the fm-601-proxmox-guest-associations branch September 20, 2026 18:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FM-601 — Add Proxmox guest associations and guest-agent data

1 participant