FM-601: Proxmox guest associations and guest-agent data - #102
Merged
Merged
Conversation
There was a problem hiding this comment.
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
Member
Author
|
All 19 findings addressed in 6bf4158: P1
P2s
P3s
|
There was a problem hiding this comment.
All reported issues were addressed across 11 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…agination documented in OpenAPI
Member
Author
|
Round-2 re-review addressed in 63fb3d5:
(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.) |
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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
fleet-provider-proxmox):guest_discoveron theProxmoxSourceport — the cluster snapshot names the guests, the guest config carries theirnetNMACs (normalized lowercase,normalize_macrefuses 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 reportsonline: falsewith 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.fleet-application/src/proxmox.rs):association_candidatecorrelates guests with Fleet machines by candidates — MAC evidence (agent interfaces + config MACs vs the machine's recordednet/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: whenmachine.read.sensitiveis denied, candidates come back empty rather than half-redacted lies (the redacted reference still carries the host, so the explicit gate is required).observe_guestrecords the guest's facts onto the confirmed machine as capability facts through the existingrecord_capabilitiesfunnel —pve/guest|vmid|node|agent|os|kernel,net/mac0…, provenanceproxmox/<version>. Availability honesty: an off QEMU guest's agent isunavailable(may be off, not agentless); an LXC guest's agent isunknown(no qemu-guest-agent by design). The machine funnel authorizes and audits the write; the Proxmox read is audited separately.GET /api/v1/proxmox/accounts/{id}/guestsandPOST /api/v1/proxmox/accounts/{id}/guests/{vmid}/observein the OpenAPI doc (client regenerated);fleetctl proxmox guestsandfleetctl proxmox observe-guest.Tests
cargo xtask verifypasses; 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_discoverreads cluster resources, guest config MACs, and QEMU guest-agent facts; off QEMU agents reportunavailable, LXC agents reportunknown, and failed guest configs warn and continue.observe_guestrecords guest facts onto the confirmed machine through the existing capability funnel withproxmox/<version>provenance.GET /api/v1/proxmox/accounts/{id}/guests,POST /api/v1/proxmox/accounts/{id}/guests/{vmid}/observe,fleetctl proxmox guests, andfleetctl proxmox observe-guest; guest lists paginate, and stale cursors are refused with 400.Written for commit 63fb3d5. Summary will update on new commits.