From cc390055107b31fb943454d12defaee408dbb925 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= Date: Sun, 20 Sep 2026 17:56:23 +0000 Subject: [PATCH 1/3] FM-601: Proxmox guest associations and guest-agent data --- crates/fleet-api/src/lib.rs | 7 + crates/fleet-api/src/proxmox.rs | 263 ++++++- crates/fleet-application/src/proxmox.rs | 500 ++++++++++++- crates/fleet-application/tests/proxmox.rs | 654 +++++++++++++++++- crates/fleet-controller/src/proxmox_store.rs | 123 +++- crates/fleet-controller/tests/proxmox.rs | 128 +++- crates/fleetctl/src/lib.rs | 106 +++ crates/fleetctl/tests/cli.rs | 66 ++ .../fleet-provider-proxmox/src/lib.rs | 496 ++++++++++++- packages/api-client/openapi.json | 452 ++++++++++++ packages/api-client/src/generated/fleet.ts | 295 ++++++++ 11 files changed, 3024 insertions(+), 66 deletions(-) diff --git a/crates/fleet-api/src/lib.rs b/crates/fleet-api/src/lib.rs index 53047e0..947b10d 100644 --- a/crates/fleet-api/src/lib.rs +++ b/crates/fleet-api/src/lib.rs @@ -126,6 +126,11 @@ pub const API_BASE_PATH: &str = "/api/v1"; proxmox::ProxmoxDiscoveryDto, proxmox::ProxmoxFingerprintDto, proxmox::ProxmoxResourceDto, + proxmox::AssociatedGuestDto, + proxmox::AssociationCandidateDto, + proxmox::ObserveProxmoxGuestRequest, + proxmox::ProviderAgentDto, + proxmox::ProviderInterfaceDto, node::CreateEnrollmentTokenRequest, node::EnrollmentTokenCreatedDto, node::EnrollmentTokenDto, @@ -227,6 +232,8 @@ pub fn api(state: Arc) -> (Router, utoipa::openapi::OpenAp .routes(routes!(proxmox::observe_proxmox_fingerprint)) .routes(routes!(proxmox::confirm_proxmox_fingerprint)) .routes(routes!(proxmox::discover_proxmox_cluster)) + .routes(routes!(proxmox::list_proxmox_guests)) + .routes(routes!(proxmox::observe_proxmox_guest)) .with_state(state), ) .split_for_parts(); diff --git a/crates/fleet-api/src/proxmox.rs b/crates/fleet-api/src/proxmox.rs index 666e4a0..14ed490 100644 --- a/crates/fleet-api/src/proxmox.rs +++ b/crates/fleet-api/src/proxmox.rs @@ -14,7 +14,9 @@ use axum::{ extract::{Path, Query, State}, http::StatusCode, }; -use fleet_application::proxmox::{NewProxmoxAccount, ProxmoxAccount, ProxmoxUseCaseError}; +use fleet_application::proxmox::{ + AssociatedGuest, NewProxmoxAccount, ProxmoxAccount, ProxmoxUseCaseError, +}; use fleet_core::{CorrelationId, ErrorCode, PublicError, RetryClass}; use serde::{Deserialize, Serialize}; use std::str::FromStr as _; @@ -184,6 +186,131 @@ pub struct ProxmoxDiscoveryDto { pub observed_at: i64, } +/// One discovered guest with its Fleet-machine association candidates. +#[derive(Clone, Debug, Serialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct AssociatedGuestDto { + /// The normalized kind: `qemu` or `lxc`. + pub kind: String, + /// The cluster-visible id. + pub id: String, + /// The hosting node. + pub node: Option, + /// The VMID. + pub vmid: Option, + /// The display name, when carried. + pub name: Option, + /// The PVE status string, when carried. + pub status: Option, + /// The config's MAC addresses, normalized. + pub macs: Vec, + /// The guest-agent view, when the guest has one. + pub agent: Option, + /// The bounded per-surface warnings. + pub warnings: Vec, + /// The PVE version the observation came from. + pub pve_version: String, + /// When the observation was taken. + pub observed_at: i64, + /// The Fleet machines this guest may be — evidence, never merged. + pub candidates: Vec, +} + +/// The guest-agent view. +#[derive(Clone, Debug, Serialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ProviderAgentDto { + /// The agent answered `info`: installed and reachable. + pub online: bool, + /// The agent version, when carried. + pub version: Option, + /// The guest's OS name, when `get-osinfo` answered. + pub os_name: Option, + /// The guest's kernel release, when carried. + pub kernel: Option, + /// The network interfaces the agent saw. + pub interfaces: Vec, +} + +/// One guest network interface. +#[derive(Clone, Debug, Serialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ProviderInterfaceDto { + /// The interface name inside the guest. + pub name: String, + /// The normalized MAC, when carried. + pub mac: Option, + /// The interface's addresses. + pub addresses: Vec, +} + +/// One Fleet machine a guest may be, with the evidence. +#[derive(Clone, Debug, Serialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct AssociationCandidateDto { + /// The existing machine's identity. + pub machine_id: String, + /// The existing machine's name. + pub machine_name: String, + /// The machine's derived connectivity state. + pub machine_status: String, + /// Why: `mac_match`, `address_match`, or `name_match`. + pub kind: String, + /// The evidence value that matched. + pub evidence: String, +} + +impl From for AssociatedGuestDto { + fn from(associated: AssociatedGuest) -> Self { + Self { + kind: associated.guest.kind, + id: associated.guest.id, + node: associated.guest.node, + vmid: associated.guest.vmid, + name: associated.guest.name, + status: associated.guest.status, + macs: associated.guest.macs, + agent: associated.guest.agent.map(|agent| ProviderAgentDto { + online: agent.online, + version: agent.version, + os_name: agent.os_name, + kernel: agent.kernel, + interfaces: agent + .interfaces + .into_iter() + .map(|interface| ProviderInterfaceDto { + name: interface.name, + mac: interface.mac, + addresses: interface.addresses, + }) + .collect(), + }), + warnings: associated.guest.warnings, + pve_version: associated.pve_version, + observed_at: associated.observed_at, + candidates: associated + .candidates + .into_iter() + .map(|candidate| AssociationCandidateDto { + machine_id: candidate.machine_id, + machine_name: candidate.machine_name, + machine_status: candidate.machine_status, + kind: candidate.kind, + evidence: candidate.evidence, + }) + .collect(), + } + } +} + +/// The observe-guest request: which machine the guest's facts record onto. +#[derive(Debug, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ObserveProxmoxGuestRequest { + /// The machine the guest is confirmed to be. + pub machine_id: String, +} + /// The create-account request. The token secret is write-only. #[derive(Debug, Deserialize, ToSchema)] #[serde(rename_all = "camelCase")] @@ -612,3 +739,137 @@ pub async fn discover_proxmox_cluster( observed_at: discovery.observed_at, }))) } + +/// Lists the account's guests with their Fleet-machine association +/// candidates (evidence only). +/// +/// # Errors +/// +/// Returns the public error envelope on refusal, an unconfirmed account, or +/// a source failure. +#[utoipa::path( + get, + path = "/proxmox/accounts/{accountId}/guests", + tag = "proxmox", + operation_id = "listProxmoxGuests", + params( + ( + "accountId" = String, + Path, + description = "The account's identity." + ), + ), + responses( + ( + status = 200, + description = "The guests with their association candidates.", + body = Page + ), + ( + status = 403, + description = "The caller may not read the Proxmox surface.", + body = crate::error::ApiError + ), + ( + status = 409, + description = "The account's trust is unconfirmed.", + body = crate::error::ApiError + ), + ) +)] +pub async fn list_proxmox_guests( + State(state): State>, + principal: Option>, + Extension(correlation_id): Extension, + Path(account_id): Path, +) -> Result>, ApiErrorResponse> { + let proxmox = proxmox_or_error(&state, correlation_id)?; + let principal = crate::operations::principal_or_error(principal, correlation_id)?; + let guests = proxmox + .guests( + state.authorizer.as_ref(), + &principal, + &account_id, + fleet_core::SystemClock::now_unix_millis(), + ) + .await + .map_err(|error| map_proxmox_error(&error, correlation_id))?; + let items: Vec = guests.into_iter().map(Into::into).collect(); + Ok(Json(Page { + page: PageInfo { + next_cursor: None, + limit: items.len().try_into().unwrap_or(u32::MAX), + }, + items, + })) +} + +/// Records one guest's facts onto a confirmed Fleet machine. The machine +/// funnel authorizes and audits the capability write. +/// +/// # Errors +/// +/// Returns the public error envelope on refusal, an unknown account, +/// guest, or machine, or a source failure. +#[utoipa::path( + post, + path = "/proxmox/accounts/{accountId}/guests/{vmid}/observe", + tag = "proxmox", + operation_id = "observeProxmoxGuest", + params( + ( + "accountId" = String, + Path, + description = "The account's identity." + ), + ( + "vmid" = u32, + Path, + description = "The guest's VMID." + ), + ), + request_body = ObserveProxmoxGuestRequest, + responses( + ( + status = 204, + description = "The guest's facts were recorded on the machine." + ), + ( + status = 403, + description = "The caller may not read the Proxmox surface or write the machine's facts.", + body = crate::error::ApiError + ), + ( + status = 404, + description = "The account, guest, or machine does not exist.", + body = crate::error::ApiError + ), + ( + status = 409, + description = "The account's trust is unconfirmed or the source refused.", + body = crate::error::ApiError + ), + ) +)] +pub async fn observe_proxmox_guest( + State(state): State>, + principal: Option>, + Extension(correlation_id): Extension, + Path((account_id, vmid)): Path<(String, u32)>, + Json(request): Json, +) -> Result { + let proxmox = proxmox_or_error(&state, correlation_id)?; + let principal = crate::operations::principal_or_error(principal, correlation_id)?; + proxmox + .observe_guest( + state.authorizer.as_ref(), + &principal, + &account_id, + vmid, + &request.machine_id, + fleet_core::SystemClock::now_unix_millis(), + ) + .await + .map_err(|error| map_proxmox_error(&error, correlation_id))?; + Ok(StatusCode::NO_CONTENT) +} diff --git a/crates/fleet-application/src/proxmox.rs b/crates/fleet-application/src/proxmox.rs index bc006ac..bf8c9f1 100644 --- a/crates/fleet-application/src/proxmox.rs +++ b/crates/fleet-application/src/proxmox.rs @@ -28,8 +28,9 @@ use async_trait::async_trait; use serde::{Deserialize, Serialize}; use crate::authz::{AccessRequest, ActingPrincipal, Authorizer, Decision, Permission, authorize}; +use crate::machine::{MachineFilter, MachineUseCaseError, MachineView, Machines}; use crate::operation::AuditPort; -use fleet_core::SensitiveString; +use fleet_core::{CapabilityFact, CapabilityStatus, SensitiveString, Timestamp}; /// Binds one credential-carrying call to one account, resolving the secret /// just in time. @@ -88,7 +89,7 @@ pub enum FingerprintState { } /// A credential-store failure that is safe to print. -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum CredentialStoreError { /// The store is unreadable or unwritable. Backend { @@ -134,7 +135,7 @@ pub trait ProxmoxCredentialStore: fmt::Debug + Send + Sync { /// A discovery-source failure that is safe to print. Fingerprints and /// statuses travel here; tokens never do. -#[derive(Debug)] +#[derive(Clone, Debug)] pub enum ProxmoxSourceError { /// The pinned fingerprint was refused, with the observed fingerprint as /// evidence. The pin did its job: the connection died at the handshake. @@ -241,6 +242,144 @@ pub struct ProxmoxDiscovery { pub observed_at: i64, } +/// One discovered guest with its Fleet-machine association candidates +/// (evidence only). The guest's provider facts ride along with the +/// account/version/time provenance. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AssociatedGuest { + /// The guest's provider facts. + pub guest: ProviderGuest, + /// The PVE version the observation came from. + pub pve_version: String, + /// When the observation was taken (epoch millis). + pub observed_at: i64, + /// The Fleet machines this guest may be — evidence, never merged. + pub candidates: Vec, +} + +/// The application's view of one guest: the provider shape plus the +/// provenance the discovery call attached. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ProviderGuest { + /// The normalized kind: `qemu` or `lxc`. + pub kind: String, + /// The cluster-visible id, e.g. `qemu/101`. + pub id: String, + /// The hosting node. + pub node: Option, + /// The VMID. + pub vmid: Option, + /// The display name, when carried. + pub name: Option, + /// The PVE status string, when carried. + pub status: Option, + /// The config's MAC addresses, normalized. + pub macs: Vec, + /// The guest-agent view, when the guest has one (QEMU only). + pub agent: Option, + /// The bounded per-surface warnings. + pub warnings: Vec, +} + +/// The guest-agent view as the application sees it. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ProviderAgent { + /// The agent answered `info`: installed and reachable. + pub online: bool, + /// The agent version, when carried. + pub version: Option, + /// The guest's OS name, when `get-osinfo` answered. + pub os_name: Option, + /// The guest's kernel release, when carried. + pub kernel: Option, + /// The network interfaces the agent saw. + pub interfaces: Vec, +} + +/// One guest network interface as the application sees it. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ProviderInterface { + /// The interface name inside the guest. + pub name: String, + /// The normalized MAC, when carried. + pub mac: Option, + /// The interface's addresses. + pub addresses: Vec, +} + +/// Why a guest may be one Fleet machine. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum AssociationKind { + /// A guest MAC matches a machine's recorded network fact. + MacMatch, + /// A guest-agent address matches a machine endpoint's host. + AddressMatch, + /// The guest's name matches the machine's name. + NameMatch, +} + +impl AssociationKind { + /// The stable string used in the API. + #[must_use] + pub const fn id(self) -> &'static str { + match self { + Self::MacMatch => "mac_match", + Self::AddressMatch => "address_match", + Self::NameMatch => "name_match", + } + } +} + +/// One Fleet machine a guest may be, with the evidence. +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct AssociationCandidate { + /// The existing machine's identity. + pub machine_id: String, + /// The existing machine's name. + pub machine_name: String, + /// The machine's derived connectivity state. + pub machine_status: String, + /// Why: the association kind's stable id. + pub kind: String, + /// The evidence value that matched (the MAC, address, or name). + pub evidence: String, +} + +/// The guest-discovery port over one trusted account. The provider +/// implements this over the PVE API; tests implement it over fixtures. +#[async_trait] +pub trait ProxmoxGuestDiscoverPort: fmt::Debug + Send + Sync { + /// Discovers the account's guests with their config MACs and agent + /// views. + /// + /// # Errors + /// + /// Fails with [`ProxmoxSourceError`]. + async fn guest_discover( + &self, + account: &ProxmoxAccount, + secret: &SensitiveString, + ) -> Result; +} + +/// The provider's own guest-discovery shape, before application-layer +/// enrichment. Provenance is attached by [`ProxmoxAccounts::guests`]. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct RawGuestDiscovery { + /// The PVE version seen. + pub version: String, + /// The guests, provenance pending. + pub guests: Vec, + /// The cluster-level warnings. + pub warnings: Vec, +} + /// The account record port: durable account state. #[async_trait] pub trait ProxmoxAccountPort: fmt::Debug + Send + Sync { @@ -432,7 +571,9 @@ pub struct ProxmoxAccounts { accounts: Arc, credentials: Arc, discovery: Arc, + guests: Arc, trust: Arc, + machines: Arc, audit: Arc, } @@ -443,14 +584,18 @@ impl ProxmoxAccounts { accounts: Arc, credentials: Arc, discovery: Arc, + guests: Arc, trust: Arc, + machines: Arc, audit: Arc, ) -> Self { Self { accounts, credentials, discovery, + guests, trust, + machines, audit, } } @@ -793,23 +938,8 @@ impl ProxmoxAccounts { }, ) .map_err(ProxmoxUseCaseError::Denied)?; - let account = self.require_account(account_id).await?; - // The explicit-trust gate: without a confirmed fingerprint no - // credential-carrying call leaves Fleet. This is the acceptance - // criterion, not a convenience check. - let Some(_pinned) = account.fingerprint.clone() else { - return Err(ProxmoxUseCaseError::UnconfirmedTrust { - account: account.name.clone(), - }); - }; - let secret = self - .credentials - .load(account_id) - .await - .map_err(ProxmoxUseCaseError::Credentials)? - .ok_or_else(|| ProxmoxUseCaseError::NoSecret { - account: account.name.clone(), - })?; + let account = self.trusted_account(account_id).await?; + let secret = self.require_secret(&account).await?; let raw = self .discovery .discover(&account, &SensitiveString::new(secret)) @@ -835,6 +965,195 @@ impl ProxmoxAccounts { }) } + /// Lists the account's guests with their Fleet-machine association + /// candidates. Correlation needs the machine surface, so it degrades + /// honestly: guests still list, candidates come back empty when the + /// caller may not read machine detail — the FM-213 rule. + /// + /// # Errors + /// + /// Fails on denial, an unconfirmed account, a missing secret, or any + /// source failure. + pub async fn guests( + &self, + authorizer: &dyn Authorizer, + principal: &ActingPrincipal, + account_id: &str, + now: i64, + ) -> Result, ProxmoxUseCaseError> { + authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::ProxmoxRead, + resource: Some(account_id), + }, + ) + .map_err(ProxmoxUseCaseError::Denied)?; + let account = self.trusted_account(account_id).await?; + let secret = self.require_secret(&account).await?; + let raw = self + .guests + .guest_discover(&account, &SensitiveString::new(secret)) + .await + .map_err(ProxmoxUseCaseError::Source)?; + // The machine surface: a degraded answer is empty candidates, not a + // failure. Correlation needs sensitive endpoint detail; without it + // the candidates are empty rather than half-redacted lies. + let machine_read = authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::MachineRead, + resource: None, + }, + ) + .is_ok(); + let views = if machine_read { + self.machines + .list(authorizer, principal, &MachineFilter::default(), 200, now) + .await + .map_err(|error| ProxmoxUseCaseError::Backend { + context: "association", + detail: error.to_string(), + })? + } else { + Vec::new() + }; + Ok(raw + .guests + .into_iter() + .map(|guest| { + let candidates = views + .iter() + .filter_map(|view| { + // Association evidence lives in endpoint hosts and + // recorded network facts: without the sensitive + // read the candidates are empty rather than + // half-redacted lies (the FM-213 rule). + let sensitive = authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::MachineReadSensitive, + resource: Some(view.id.as_str()), + }, + ) + .is_ok(); + if !sensitive { + return None; + } + association_candidate(&guest, view) + }) + .collect(); + AssociatedGuest { + guest, + pve_version: raw.version.clone(), + observed_at: now, + candidates, + } + }) + .collect()) + } + + /// Records one guest's facts onto a confirmed Fleet machine as + /// capability facts through the machine funnel. The association is the + /// caller's confirmed claim — the candidates are evidence, and this + /// mutation turns the evidence into recorded observations under the + /// machine's own authorization and audit. + /// + /// # Errors + /// + /// Fails on denial (either surface), an unknown account or machine, an + /// unconfirmed account, or a source failure. + pub async fn observe_guest( + &self, + authorizer: &dyn Authorizer, + principal: &ActingPrincipal, + account_id: &str, + vmid: u32, + machine_id: &str, + now: i64, + ) -> Result<(), ProxmoxUseCaseError> { + authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::ProxmoxRead, + resource: Some(account_id), + }, + ) + .map_err(ProxmoxUseCaseError::Denied)?; + let account = self.trusted_account(account_id).await?; + let secret = self.require_secret(&account).await?; + let raw = self + .guests + .guest_discover(&account, &SensitiveString::new(secret)) + .await + .map_err(ProxmoxUseCaseError::Source)?; + let guest = raw + .guests + .into_iter() + .find(|guest| guest.vmid == Some(vmid)) + .ok_or_else(|| ProxmoxUseCaseError::NotFound { + what: format!("guest {vmid}"), + })?; + // The machine funnel authorizes and audits the capability write; + // the Proxmox read is audited here. + self.audit_event( + principal, + Permission::ProxmoxRead, + Some(account_id), + "proxmox_guest_observed", + Some(("vmid", &vmid.to_string())), + ) + .await?; + let facts = guest_facts(&guest, machine_id, &raw.version, now); + self.machines + .record_capabilities(authorizer, principal, machine_id, &facts) + .await + .map_err(|error| match error { + MachineUseCaseError::Denied(decision) => ProxmoxUseCaseError::Denied(decision), + MachineUseCaseError::NotFound { what } => ProxmoxUseCaseError::NotFound { what }, + MachineUseCaseError::Conflict { detail } + | MachineUseCaseError::Invalid { detail } => { + ProxmoxUseCaseError::Invalid { detail } + } + MachineUseCaseError::Backend { context, detail } => { + ProxmoxUseCaseError::Backend { context, detail } + } + }) + } + + /// The account with the explicit-trust gate applied: without a + /// confirmed fingerprint no credential-carrying call leaves Fleet. + async fn trusted_account( + &self, + account_id: &str, + ) -> Result { + let account = self.require_account(account_id).await?; + if account.fingerprint.is_none() { + return Err(ProxmoxUseCaseError::UnconfirmedTrust { + account: account.name.clone(), + }); + } + Ok(account) + } + + /// The account's token secret, resolved just in time. + async fn require_secret( + &self, + account: &ProxmoxAccount, + ) -> Result { + self.credentials + .load(&account.id) + .await + .map_err(ProxmoxUseCaseError::Credentials)? + .ok_or_else(|| ProxmoxUseCaseError::NoSecret { + account: account.name.clone(), + }) + } + async fn require_account(&self, id: &str) -> Result { self.accounts.get(id).await.map_err(|detail| { if detail.contains("not found") { @@ -891,6 +1210,147 @@ impl ProxmoxAccounts { } } +/// The association evidence between one guest and one machine view, when +/// any. MAC evidence outranks address evidence outranks name evidence; the +/// first match wins so a candidate reports its strongest reason. +#[must_use] +fn association_candidate( + guest: &ProviderGuest, + view: &MachineView, +) -> Option { + // The machine's recorded network facts, when the view carries them. + let machine_macs: Vec<&str> = view + .capabilities + .iter() + .filter(|fact| fact.namespace == "net" && fact.name.starts_with("mac")) + .filter_map(|fact| fact.value.as_deref()) + .collect(); + // MAC evidence: the agent's interfaces for QEMU, the config's `netN` + // MACs for every guest kind (LXC has no agent but has config MACs). + let guest_macs = guest + .agent + .iter() + .flat_map(|agent| &agent.interfaces) + .filter_map(|interface| interface.mac.as_deref()) + .chain(guest.macs.iter().map(String::as_str)); + for mac in guest_macs { + if machine_macs + .iter() + .any(|machine_mac| machine_mac.eq_ignore_ascii_case(mac)) + { + return Some(AssociationCandidate { + machine_id: view.id.clone(), + machine_name: view.name.clone(), + machine_status: view.machine_status.id().to_owned(), + kind: AssociationKind::MacMatch.id().to_owned(), + evidence: mac.to_owned(), + }); + } + } + // The guest-agent addresses against the machine endpoints' hosts. The + // endpoint reference is `user@host:port` (redacted forms carry `***`), + // so the host is the segment after the last `@` with the port stripped. + let endpoint_hosts: Vec<&str> = view + .endpoints + .iter() + .filter_map(|endpoint| { + let (_, host_port) = endpoint.reference.rsplit_once('@')?; + let (host, _) = host_port.rsplit_once(':')?; + Some(host) + }) + .collect(); + for interface in guest.agent.iter().flat_map(|agent| &agent.interfaces) { + for address in &interface.addresses { + if endpoint_hosts + .iter() + .any(|host| host.eq_ignore_ascii_case(address)) + { + return Some(AssociationCandidate { + machine_id: view.id.clone(), + machine_name: view.name.clone(), + machine_status: view.machine_status.id().to_owned(), + kind: AssociationKind::AddressMatch.id().to_owned(), + evidence: address.clone(), + }); + } + } + } + // The name match: the guest's display name against the machine's name, + // case-insensitive, when both carry one. + if let Some(name) = &guest.name + && view.name.eq_ignore_ascii_case(name) + { + return Some(AssociationCandidate { + machine_id: view.id.clone(), + machine_name: view.name.clone(), + machine_status: view.machine_status.id().to_owned(), + kind: AssociationKind::NameMatch.id().to_owned(), + evidence: name.clone(), + }); + } + None +} + +/// The capability facts one guest contributes to its confirmed machine: +/// the guest identity, the agent's availability and version, the OS and +/// kernel when the agent answered, and the MACs. Provenance is the +/// account's observation path; every fact carries the observation time. +#[must_use] +fn guest_facts( + guest: &ProviderGuest, + machine_id: &str, + pve_version: &str, + now: i64, +) -> Vec { + let source = format!("proxmox/{pve_version}"); + let mut facts = Vec::new(); + let mut push = |name: &str, value: Option, status: CapabilityStatus| { + facts.push(CapabilityFact { + namespace: "pve".to_owned(), + name: name.to_owned(), + value, + status, + observed_at: Timestamp::from_unix_millis(now), + source: source.clone(), + }); + }; + let _ = machine_id; + push("guest", Some(guest.id.clone()), CapabilityStatus::Known); + if let Some(vmid) = guest.vmid { + push("vmid", Some(vmid.to_string()), CapabilityStatus::Known); + } + if let Some(node) = &guest.node { + push("node", Some(node.clone()), CapabilityStatus::Known); + } + match &guest.agent { + Some(agent) if agent.online => { + push("agent", agent.version.clone(), CapabilityStatus::Known); + if let Some(os) = &agent.os_name { + push("os", Some(os.clone()), CapabilityStatus::Known); + } + if let Some(kernel) = &agent.kernel { + push("kernel", Some(kernel.clone()), CapabilityStatus::Known); + } + } + // A QEMU guest whose agent did not answer: unavailable is honest — + // the guest may be off, not agentless. + Some(_) => push("agent", None, CapabilityStatus::Unavailable), + // LXC has no qemu-guest-agent by design: the absence is known. + None => push("agent", None, CapabilityStatus::Unknown), + } + for (index, mac) in guest.macs.iter().enumerate() { + facts.push(CapabilityFact { + namespace: "net".to_owned(), + name: format!("mac{index}"), + value: Some(mac.clone()), + status: CapabilityStatus::Known, + observed_at: Timestamp::from_unix_millis(now), + source: source.clone(), + }); + } + facts +} + /// Normalizes a fingerprint for comparison. Exposed for the adapter layer's /// echo handling. #[must_use] diff --git a/crates/fleet-application/tests/proxmox.rs b/crates/fleet-application/tests/proxmox.rs index a073141..c3c55a5 100644 --- a/crates/fleet-application/tests/proxmox.rs +++ b/crates/fleet-application/tests/proxmox.rs @@ -6,13 +6,21 @@ use std::sync::{Arc, Mutex}; use async_trait::async_trait; use fleet_application::audit::{AuditIntent, AuditOutcome}; -use fleet_application::authz::{AccessRequest, ActingPrincipal, Authorizer, Decision, ReasonId}; +use fleet_application::authz::{ + AccessRequest, ActingPrincipal, Authorizer, Decision, Permission, ReasonId, +}; +use fleet_application::machine::{ + Endpoint, Machine, MachineFilter, MachinePort as _, Machines, NewEndpoint, RegisterMachine, +}; use fleet_application::operation::AuditPort; +use fleet_application::operation::PortFailure; use fleet_application::proxmox::{ - CredentialStoreError, NewProxmoxAccount, ProxmoxAccountPort, ProxmoxAccounts, - ProxmoxCredentialStore, ProxmoxDiscoverPort, ProxmoxSourceError, ProxmoxTrustProbe, - ProxmoxUseCaseError, RawDiscovery, + CredentialStoreError, NewProxmoxAccount, ProviderAgent, ProviderGuest, ProviderInterface, + ProxmoxAccountPort, ProxmoxAccounts, ProxmoxCredentialStore, ProxmoxDiscoverPort, + ProxmoxGuestDiscoverPort, ProxmoxSourceError, ProxmoxTrustProbe, ProxmoxUseCaseError, + RawDiscovery, RawGuestDiscovery, }; +use fleet_core::CapabilityFact; use fleet_core::SensitiveString; const NOW: i64 = 1_800_000_000_000; @@ -268,20 +276,246 @@ fn discovery_ok() -> RawDiscovery { } } +/// The machine port over an in-memory map, mirroring the SQLite adapter's +/// semantics closely enough for the association tests: register, list (as +/// assembled views), and record capabilities. +#[derive(Debug, Default)] +struct FakeMachinePort { + machines: std::sync::Mutex>, + recorded: std::sync::Mutex)>>, +} + +#[async_trait] +impl fleet_application::machine::MachinePort for FakeMachinePort { + async fn register(&self, registration: &RegisterMachine) -> Result { + let machine = Machine { + id: format!("m-{}", registration.name), + name: registration.name.clone(), + description: registration.description.clone(), + endpoints: registration + .endpoints + .iter() + .enumerate() + .map(|(index, endpoint)| Endpoint { + id: format!("ep-{index}"), + kind: endpoint.kind, + reference: endpoint.reference.clone(), + }) + .collect(), + tags: registration.tags.clone(), + groups: registration.groups.clone(), + node: None, + capabilities: Vec::new(), + last_observation: None, + created_at: NOW, + updated_at: NOW, + }; + self.machines.lock().unwrap().push(machine.clone()); + Ok(machine) + } + + async fn get(&self, id: &str) -> Result { + self.machines + .lock() + .unwrap() + .iter() + .find(|machine| machine.id == id) + .cloned() + .ok_or_else(|| PortFailure::NotFound { + what: format!("machine {id}"), + }) + } + + async fn list(&self, _filter: &MachineFilter, limit: u32) -> Result, PortFailure> { + let machines = self.machines.lock().unwrap(); + Ok(machines + .iter() + .rev() + .take(usize::try_from(limit).unwrap_or(machines.len())) + .cloned() + .collect()) + } + + async fn update( + &self, + _id: &str, + _name: &str, + _description: &str, + ) -> Result { + Err(PortFailure::Backend { + detail: "unused in these tests".to_owned(), + }) + } + + async fn set_endpoints( + &self, + _id: &str, + _endpoints: &[NewEndpoint], + ) -> Result { + Err(PortFailure::Backend { + detail: "unused in these tests".to_owned(), + }) + } + + async fn add_tag(&self, _id: &str, _tag: &str) -> Result { + Err(PortFailure::Backend { + detail: "unused in these tests".to_owned(), + }) + } + + async fn remove_tag(&self, _id: &str, _tag: &str) -> Result { + Err(PortFailure::Backend { + detail: "unused in these tests".to_owned(), + }) + } + + async fn add_group(&self, _id: &str, _group: &str) -> Result { + Err(PortFailure::Backend { + detail: "unused in these tests".to_owned(), + }) + } + + async fn remove_group(&self, _id: &str, _group: &str) -> Result { + Err(PortFailure::Backend { + detail: "unused in these tests".to_owned(), + }) + } + + async fn record_snapshot( + &self, + _id: &str, + _source: &str, + _payload_json: &str, + _collected_at: i64, + ) -> Result<(), PortFailure> { + Ok(()) + } + + async fn record_capabilities( + &self, + id: &str, + facts: &[CapabilityFact], + ) -> Result<(), PortFailure> { + self.recorded + .lock() + .unwrap() + .push((id.to_owned(), facts.to_vec())); + // Mirror the real upsert: the facts land on the machine, so a later + // list view carries them. + let mut machines = self.machines.lock().unwrap(); + let machine = machines + .iter_mut() + .find(|machine| machine.id == id) + .ok_or_else(|| PortFailure::NotFound { + what: format!("machine {id}"), + })?; + for fact in facts { + if let Some(existing) = machine + .capabilities + .iter_mut() + .find(|existing| existing.namespace == fact.namespace && existing.name == fact.name) + { + *existing = fact.clone(); + } else { + machine.capabilities.push(fact.clone()); + } + } + Ok(()) + } + + async fn delete(&self, _id: &str) -> Result<(), PortFailure> { + Ok(()) + } + + async fn confirm_fingerprint( + &self, + _endpoint_id: &str, + _fingerprint: &str, + _confirmed_at: i64, + ) -> Result<(), PortFailure> { + Ok(()) + } + + async fn verified_fingerprint( + &self, + _endpoint_id: &str, + ) -> Result, PortFailure> { + Ok(None) + } + + async fn latest_inventory_revision( + &self, + _machine_id: &str, + ) -> Result, PortFailure> { + Ok(None) + } +} + +/// The guest-discovery port over canned results. +#[derive(Debug, Default)] +struct FakeGuestDiscovery { + /// The canned result, cloned per call: a discovery is a read, and the + /// real source answers the same snapshot every time. + result: std::sync::Mutex>>, + calls: std::sync::Mutex>, +} + +impl FakeGuestDiscovery { + fn with(result: Result) -> Arc { + Arc::new(Self { + result: std::sync::Mutex::new(Some(result)), + calls: std::sync::Mutex::new(Vec::new()), + }) + } +} + +#[async_trait] +impl ProxmoxGuestDiscoverPort for FakeGuestDiscovery { + async fn guest_discover( + &self, + account: &fleet_application::proxmox::ProxmoxAccount, + _secret: &SensitiveString, + ) -> Result { + self.calls.lock().unwrap().push(account.id.clone()); + self.result + .lock() + .unwrap() + .as_ref() + .expect("a guest-discovery answer was prepared") + .clone() + // The canned result is cloned per call: a discovery is a read. + } +} + fn service( discovery: Arc, probe: Arc, ) -> (ProxmoxAccounts, Arc) { + let (proxmox, audit, _machine_port) = + service_with_guests(discovery, Arc::new(FakeGuestDiscovery::default()), probe); + (proxmox, audit) +} + +fn service_with_guests( + discovery: Arc, + guests: Arc, + probe: Arc, +) -> (ProxmoxAccounts, Arc, Arc) { let audit = Arc::new(FakeAudit::default()); + let machine_port = Arc::new(FakeMachinePort::default()); + let machines = Arc::new(Machines::new(machine_port.clone(), audit.clone())); ( ProxmoxAccounts::new( Arc::new(FakeAccounts::default()), Arc::new(FakeCredentials::default()), discovery, + guests, probe, + machines, audit.clone(), ), audit, + machine_port, ) } @@ -469,11 +703,17 @@ async fn a_failed_secret_write_removes_the_account() { } } let audit = Arc::new(FakeAudit::default()); + let machines = Arc::new(Machines::new( + Arc::new(FakeMachinePort::default()), + audit.clone(), + )); let proxmox = ProxmoxAccounts::new( Arc::new(FakeAccounts::default()), Arc::new(RefusingStore), FakeDiscovery::with(Ok(discovery_ok())), + Arc::new(FakeGuestDiscovery::default()), FakeProbe::with(FP), + machines, audit, ); let error = proxmox @@ -614,3 +854,409 @@ async fn confirm_refuses_a_malformed_fingerprint() { ); } } + +// ---- FM-601: guest associations and guest-agent data ---- + +/// A QEMU guest with an online agent, one interface, and a config MAC. +fn qemu_guest() -> ProviderGuest { + ProviderGuest { + kind: "qemu".to_owned(), + id: "qemu/101".to_owned(), + node: Some("pve".to_owned()), + vmid: Some(101), + name: Some("fleet-test-01".to_owned()), + status: Some("running".to_owned()), + macs: vec!["de:ad:be:ef:00:01".to_owned()], + agent: Some(ProviderAgent { + online: true, + version: Some("7.2".to_owned()), + os_name: Some("Ubuntu 24.04.4 LTS".to_owned()), + kernel: Some("6.8.0-138-generic".to_owned()), + interfaces: vec![ProviderInterface { + name: "ens18".to_owned(), + mac: Some("de:ad:be:ef:00:01".to_owned()), + addresses: vec!["192.168.68.240".to_owned()], + }], + }), + warnings: Vec::new(), + } +} + +/// An LXC guest: no agent by design, config MACs only. +fn lxc_guest() -> ProviderGuest { + ProviderGuest { + kind: "lxc".to_owned(), + id: "lxc/200".to_owned(), + node: Some("pve".to_owned()), + vmid: Some(200), + name: Some("container".to_owned()), + status: Some("running".to_owned()), + macs: vec!["de:ad:be:ef:00:02".to_owned()], + agent: None, + warnings: Vec::new(), + } +} + +fn guest_discovery(guests: Vec) -> RawGuestDiscovery { + RawGuestDiscovery { + version: "9.2.2".to_owned(), + guests, + warnings: Vec::new(), + } +} + +async fn register_machine( + machine_port: &FakeMachinePort, + name: &str, + reference: &str, + mac: Option<&str>, +) -> String { + let machine = machine_port + .register(&RegisterMachine { + name: name.to_owned(), + description: String::new(), + endpoints: vec![NewEndpoint { + kind: fleet_core::EndpointKind::Ssh, + reference: reference.to_owned(), + }], + tags: Vec::new(), + groups: Vec::new(), + }) + .await + .expect("the machine registers"); + if let Some(mac) = mac { + machine_port + .record_capabilities( + &machine.id, + &[CapabilityFact { + namespace: "net".to_owned(), + name: "mac0".to_owned(), + value: Some(mac.to_owned()), + status: fleet_core::CapabilityStatus::Known, + observed_at: fleet_core::Timestamp::from_unix_millis(NOW), + source: "agentless/1".to_owned(), + }], + ) + .await + .expect("the MAC fact records"); + } + machine.id +} + +#[tokio::test] +async fn guests_list_with_evidence_only_candidates() { + let (proxmox, _audit, machine_port) = service_with_guests( + FakeDiscovery::with(Ok(discovery_ok())), + FakeGuestDiscovery::with(Ok(guest_discovery(vec![qemu_guest(), lxc_guest()]))), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + observe_and_confirm(&proxmox, &account.id).await; + + // A machine whose endpoint host is the guest-agent address: address + // evidence. Another whose recorded MAC matches the config MAC: MAC + // evidence outranks nothing here — they are different guests. + let by_address = register_machine( + &machine_port, + "fleet-test-01", + "ops@192.168.68.240:22", + None, + ) + .await; + let by_mac = register_machine( + &machine_port, + "mac-box", + "ops@10.0.0.9:22", + Some("DE:AD:BE:EF:00:02"), + ) + .await; + + let guests = proxmox + .guests(&AllowAll, &principal(), &account.id, NOW) + .await + .unwrap(); + assert_eq!(guests.len(), 2); + + let qemu = guests + .iter() + .find(|guest| guest.guest.vmid == Some(101)) + .expect("the qemu guest lists"); + assert_eq!(qemu.pve_version, "9.2.2"); + assert_eq!(qemu.observed_at, NOW); + assert_eq!(qemu.candidates.len(), 1, "{:?}", qemu.candidates); + assert_eq!(qemu.candidates[0].machine_id, by_address); + assert_eq!(qemu.candidates[0].kind, "address_match"); + assert_eq!(qemu.candidates[0].evidence, "192.168.68.240"); + + let lxc = guests + .iter() + .find(|guest| guest.guest.vmid == Some(200)) + .expect("the lxc guest lists"); + assert_eq!(lxc.candidates.len(), 1); + assert_eq!(lxc.candidates[0].machine_id, by_mac); + assert_eq!(lxc.candidates[0].kind, "mac_match"); + assert_eq!(lxc.candidates[0].evidence, "de:ad:be:ef:00:02"); +} + +#[tokio::test] +async fn mac_evidence_outranks_address_and_name_evidence() { + let (proxmox, _audit, machine_port) = service_with_guests( + FakeDiscovery::with(Ok(discovery_ok())), + FakeGuestDiscovery::with(Ok(guest_discovery(vec![qemu_guest()]))), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + observe_and_confirm(&proxmox, &account.id).await; + + // One machine matching on every dimension: the candidate reports the + // strongest reason (MAC), once. + let machine_id = register_machine( + &machine_port, + "fleet-test-01", + "ops@192.168.68.240:22", + Some("DE:AD:BE:EF:00:01"), + ) + .await; + let guests = proxmox + .guests(&AllowAll, &principal(), &account.id, NOW) + .await + .unwrap(); + assert_eq!(guests[0].candidates.len(), 1); + assert_eq!(guests[0].candidates[0].machine_id, machine_id); + assert_eq!(guests[0].candidates[0].kind, "mac_match"); +} + +#[tokio::test] +async fn unmatched_guests_list_without_candidates() { + let (proxmox, _audit, _machine_port) = service_with_guests( + FakeDiscovery::with(Ok(discovery_ok())), + FakeGuestDiscovery::with(Ok(guest_discovery(vec![qemu_guest()]))), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + observe_and_confirm(&proxmox, &account.id).await; + let guests = proxmox + .guests(&AllowAll, &principal(), &account.id, NOW) + .await + .unwrap(); + assert!( + guests[0].candidates.is_empty(), + "{:?}", + guests[0].candidates + ); +} + +#[tokio::test] +async fn a_sensitive_denial_degrades_candidates_not_the_guests() { + // machine.read is allowed but machine.read.sensitive is denied: the + // endpoint hosts arrive redacted, so address evidence cannot match. + #[derive(Debug, Default)] + struct SensitiveDenied; + impl Authorizer for SensitiveDenied { + fn decide(&self, request: AccessRequest<'_>) -> Decision { + if request.action == Permission::MachineReadSensitive { + return Decision::deny(ReasonId::PolicyAllow); + } + Decision::allow() + } + } + let (proxmox, _audit, machine_port) = service_with_guests( + FakeDiscovery::with(Ok(discovery_ok())), + FakeGuestDiscovery::with(Ok(guest_discovery(vec![qemu_guest()]))), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + observe_and_confirm(&proxmox, &account.id).await; + // A machine whose name differs from the guest's, so the only possible + // evidence is the address — which needs sensitive endpoint detail. + register_machine(&machine_port, "physical-box", "ops@192.168.68.240:22", None).await; + let guests = proxmox + .guests(&SensitiveDenied, &principal(), &account.id, NOW) + .await + .unwrap(); + // The guests still list; the redacted endpoint cannot match, so the + // candidates are empty rather than half-redacted lies. + assert!(guests[0].candidates.is_empty()); +} + +#[tokio::test] +async fn observe_guest_records_the_guest_facts_on_the_machine() { + let (proxmox, audit, machine_port) = service_with_guests( + FakeDiscovery::with(Ok(discovery_ok())), + FakeGuestDiscovery::with(Ok(guest_discovery(vec![qemu_guest(), lxc_guest()]))), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + observe_and_confirm(&proxmox, &account.id).await; + let machine_id = register_machine( + &machine_port, + "fleet-test-01", + "ops@192.168.68.240:22", + None, + ) + .await; + + proxmox + .observe_guest(&AllowAll, &principal(), &account.id, 101, &machine_id, NOW) + .await + .unwrap(); + + let recorded = machine_port.recorded.lock().unwrap(); + let (_, facts) = recorded + .iter() + .find(|(id, _)| id == &machine_id) + .expect("the facts recorded"); + let value = |name: &str| { + facts + .iter() + .find(|fact| fact.name == name) + .map(|fact| (fact.value.clone(), fact.status)) + }; + assert_eq!( + value("guest").map(|(v, _)| v), + Some(Some("qemu/101".to_owned())) + ); + assert_eq!(value("vmid").map(|(v, _)| v), Some(Some("101".to_owned()))); + assert_eq!(value("node").map(|(v, _)| v), Some(Some("pve".to_owned()))); + assert_eq!( + value("agent"), + Some((Some("7.2".to_owned()), fleet_core::CapabilityStatus::Known)) + ); + assert_eq!( + value("os").map(|(v, _)| v), + Some(Some("Ubuntu 24.04.4 LTS".to_owned())) + ); + assert_eq!( + value("mac0").map(|(v, _)| v), + Some(Some("de:ad:be:ef:00:01".to_owned())) + ); + for fact in facts { + assert_eq!( + fact.namespace, + if fact.name.starts_with("mac") { + "net" + } else { + "pve" + } + ); + assert_eq!(fact.source, "proxmox/9.2.2"); + assert_eq!(fact.observed_at.unix_millis(), NOW); + } + drop(recorded); + + // The Proxmox read is audited. + let intents = audit.intents.lock().unwrap(); + assert!( + intents.iter().any(|intent| intent + .metadata + .entries() + .any(|(k, v)| k == "event" && v == "proxmox_guest_observed")), + "{intents:?}" + ); +} + +#[tokio::test] +async fn an_off_guest_reports_unavailable_and_lxc_reports_unknown() { + let (proxmox, _audit, machine_port) = service_with_guests( + FakeDiscovery::with(Ok(discovery_ok())), + FakeGuestDiscovery::with(Ok(guest_discovery(vec![ + // A QEMU guest whose agent did not answer. + ProviderGuest { + agent: Some(ProviderAgent { + online: false, + ..ProviderAgent::default() + }), + ..qemu_guest() + }, + lxc_guest(), + ]))), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + observe_and_confirm(&proxmox, &account.id).await; + let machine_id = register_machine(&machine_port, "box", "ops@10.0.0.9:22", None).await; + + proxmox + .observe_guest(&AllowAll, &principal(), &account.id, 101, &machine_id, NOW) + .await + .unwrap(); + proxmox + .observe_guest(&AllowAll, &principal(), &account.id, 200, &machine_id, NOW) + .await + .unwrap(); + + let recorded = machine_port.recorded.lock().unwrap(); + // The QEMU recording (first) reports the agent unavailable: the guest + // may be off, not agentless. The LXC recording (second) reports the + // agent unknown: no qemu-guest-agent exists by design. + let agent_status = |recording: usize| { + recorded + .iter() + .filter(|(id, _)| id == &machine_id) + .nth(recording) + .and_then(|(_, facts)| { + facts + .iter() + .find(|fact| fact.name == "agent") + .map(|fact| fact.status) + }) + }; + assert_eq!( + agent_status(0), + Some(fleet_core::CapabilityStatus::Unavailable) + ); + assert_eq!(agent_status(1), Some(fleet_core::CapabilityStatus::Unknown)); +} + +#[tokio::test] +async fn an_unknown_guest_refuses_with_not_found() { + let (proxmox, _audit, _machine_port) = service_with_guests( + FakeDiscovery::with(Ok(discovery_ok())), + FakeGuestDiscovery::with(Ok(guest_discovery(vec![qemu_guest()]))), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + observe_and_confirm(&proxmox, &account.id).await; + let error = proxmox + .observe_guest(&AllowAll, &principal(), &account.id, 999, "m-1", NOW) + .await + .unwrap_err(); + assert!( + matches!(error, ProxmoxUseCaseError::NotFound { .. }), + "{error}" + ); +} + +#[tokio::test] +async fn guest_discovery_stays_behind_the_trust_gate() { + let (proxmox, _audit, _machine_port) = service_with_guests( + FakeDiscovery::with(Ok(discovery_ok())), + FakeGuestDiscovery::with(Ok(guest_discovery(vec![qemu_guest()]))), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + let error = proxmox + .guests(&AllowAll, &principal(), &account.id, NOW) + .await + .unwrap_err(); + assert!( + matches!(error, ProxmoxUseCaseError::UnconfirmedTrust { .. }), + "{error}" + ); +} + +#[tokio::test] +async fn a_denied_caller_never_lists_guests() { + let (proxmox, _audit, _machine_port) = service_with_guests( + FakeDiscovery::with(Ok(discovery_ok())), + FakeGuestDiscovery::with(Ok(guest_discovery(vec![qemu_guest()]))), + FakeProbe::with(FP), + ); + let account = create_account(&proxmox).await; + observe_and_confirm(&proxmox, &account.id).await; + let error = proxmox + .guests(&DenyAll, &principal(), &account.id, NOW) + .await + .unwrap_err(); + assert!(matches!(error, ProxmoxUseCaseError::Denied(_))); +} diff --git a/crates/fleet-controller/src/proxmox_store.rs b/crates/fleet-controller/src/proxmox_store.rs index 4395761..5cf20d6 100644 --- a/crates/fleet-controller/src/proxmox_store.rs +++ b/crates/fleet-controller/src/proxmox_store.rs @@ -307,6 +307,109 @@ impl ProxmoxDiscoverPort for ProviderDiscovery { } } +#[async_trait] +impl fleet_application::proxmox::ProxmoxGuestDiscoverPort for ProviderDiscovery { + async fn guest_discover( + &self, + account: &fleet_application::proxmox::ProxmoxAccount, + secret: &SensitiveString, + ) -> Result { + let Some(pinned) = account.fingerprint.clone() else { + return Err(ProxmoxSourceError::Connect { + detail: "the account has no confirmed fingerprint; refusing to send credentials" + .to_owned(), + }); + }; + let request = PveHttpRequest { + host: account.host.clone(), + port: account.port, + path: "/api2/json/cluster/resources".to_owned(), + pinned_fingerprint: Some(pinned.clone()), + credentials: Arc::new(PveCredentials { + token_id: account.token_id.clone(), + token: SensitiveString::new(secret.expose().to_owned()), + }), + }; + match self.client.guest_discover(request).await { + Ok(discovery) => Ok(fleet_application::proxmox::RawGuestDiscovery { + version: discovery.version.clone(), + guests: discovery + .guests + .into_iter() + .map(|guest| fleet_application::proxmox::ProviderGuest { + kind: guest.resource.kind, + id: guest.resource.id, + node: guest.resource.node, + vmid: guest.resource.vmid, + name: guest.resource.name, + status: guest.resource.status, + macs: guest.macs, + agent: guest + .agent + .map(|agent| fleet_application::proxmox::ProviderAgent { + online: agent.online, + version: agent.version, + os_name: agent.os_name, + kernel: agent.kernel, + interfaces: agent + .interfaces + .into_iter() + .map(|interface| { + fleet_application::proxmox::ProviderInterface { + name: interface.name, + mac: interface.mac, + addresses: interface.addresses, + } + }) + .collect(), + }), + warnings: guest.warnings, + }) + .collect(), + warnings: discovery.warnings, + }), + Err(error) => Err(map_api_error(error)), + } + } +} + +/// Maps one provider API error onto the application taxonomy. +fn map_api_error(error: fleet_provider_proxmox::PveApiError) -> ProxmoxSourceError { + match error { + fleet_provider_proxmox::PveApiError::Auth => ProxmoxSourceError::Auth, + fleet_provider_proxmox::PveApiError::Forbidden { detail } => { + ProxmoxSourceError::Forbidden { detail } + } + fleet_provider_proxmox::PveApiError::Http { status, detail } => { + ProxmoxSourceError::Http { status, detail } + } + fleet_provider_proxmox::PveApiError::InvalidPayload { detail } => { + ProxmoxSourceError::InvalidPayload { detail } + } + fleet_provider_proxmox::PveApiError::Transport( + fleet_provider_proxmox::PveTransportError::FingerprintMismatch { + observed, + pinned: pin, + }, + ) => { + let Some(pin) = pin else { + return ProxmoxSourceError::Connect { + detail: format!( + "the transport reported a fingerprint mismatch without a pin (observed {observed})" + ), + }; + }; + ProxmoxSourceError::FingerprintMismatch { + observed, + pinned: pin, + } + } + fleet_provider_proxmox::PveApiError::Transport(other) => ProxmoxSourceError::Connect { + detail: other.to_string(), + }, + } +} + /// Composes the Proxmox use cases over its ports. #[must_use] pub fn compose_proxmox( @@ -316,11 +419,27 @@ pub fn compose_proxmox( audit: Arc, ) -> fleet_application::proxmox::ProxmoxAccounts { let client = fleet_provider_proxmox::ProxmoxClient::new(transport.clone()); + let discovery = Arc::new(ProviderDiscovery::new(client.clone())); fleet_application::proxmox::ProxmoxAccounts::new( - Arc::new(fleet_storage_sqlite::ProxmoxAccountRepository::new(pool)), + Arc::new(fleet_storage_sqlite::ProxmoxAccountRepository::new( + pool.clone(), + )), Arc::new(SecretBackedProxmoxCredentials::new(secrets)), - Arc::new(ProviderDiscovery::new(client)), + discovery.clone(), + discovery, Arc::new(ProviderTrustProbe::new(transport)), + machines_for(pool, audit.clone()), audit, ) } + +/// The machine use cases over the shared pool and audit sink. +fn machines_for( + pool: sqlx::SqlitePool, + audit: Arc, +) -> Arc { + Arc::new(fleet_application::machine::Machines::new( + Arc::new(fleet_storage_sqlite::MachineRepository::new(pool)), + audit, + )) +} diff --git a/crates/fleet-controller/tests/proxmox.rs b/crates/fleet-controller/tests/proxmox.rs index 4736e14..c30f2ea 100644 --- a/crates/fleet-controller/tests/proxmox.rs +++ b/crates/fleet-controller/tests/proxmox.rs @@ -11,7 +11,7 @@ use fleet_controller::build_router; use fleet_controller::proxmox_store::compose_proxmox; use fleet_provider_proxmox::{PveHttpRequest, PveHttpResponse, PveTransport, PveTransportError}; use fleet_secrets::SecretStore; -use fleet_storage_sqlite::Store; +use fleet_storage_sqlite::{MachineRepository, Store}; use serde_json::{Value, json}; use tokio::net::TcpListener as TokioListener; @@ -26,6 +26,19 @@ const RESOURCES_BODY: &str = r#"{"data":[ {"id":"sdn/zone1","type":"sdn"} ]}"#; +const GUEST_CONFIG_BODY: &str = r#"{"data":{"name":"fleet-test-01","net0":"virtio=DE:AD:BE:EF:00:01,bridge=vmbr0","memory":2048}}"#; + +const AGENT_INFO_BODY: &str = r#"{"data":{"result":{"version":"7.2"}}}"#; + +const AGENT_NETWORK_BODY: &str = r#"{"data":{"result":[ + {"name":"ens18","hardware-address":"DE:AD:BE:EF:00:01","ip-addresses":[ + {"ip-address":"192.168.68.240","ip-address-type":"ipv4","prefix":24}]}, + {"name":"lo","hardware-address":"00:00:00:00:00:00","ip-addresses":[ + {"ip-address":"127.0.0.1","ip-address-type":"ipv4","prefix":8}]} +]}}"#; + +const AGENT_OSINFO_BODY: &str = r#"{"data":{"result":{"pretty-name":"Ubuntu 24.04.4 LTS","kernel-release":"6.8.0-138-generic"}}}"#; + /// The pinned fingerprint the fake transport accepts. const FP: &str = "DC2C116EC9C7EA618AA4E41EFB9BDEE4AA3D81EB16388F2B360AABE283A76498"; @@ -71,6 +84,14 @@ impl PveTransport for FixedTransport { (Some(_), Behavior::Normal) => { let body = if request.path.contains("/version") { VERSION_BODY + } else if request.path.contains("/config") { + GUEST_CONFIG_BODY + } else if request.path.contains("/agent/info") { + AGENT_INFO_BODY + } else if request.path.contains("/agent/network-get-interfaces") { + AGENT_NETWORK_BODY + } else if request.path.contains("/agent/get-osinfo") { + AGENT_OSINFO_BODY } else { RESOURCES_BODY }; @@ -87,6 +108,7 @@ struct Harness { _dist: tempfile::TempDir, _store_dir: tempfile::TempDir, _key_dir: tempfile::TempDir, + pool: sqlx::SqlitePool, address: std::net::SocketAddr, shutdown: Option>, } @@ -150,6 +172,7 @@ async fn harness_with(transport: Arc) -> Harness { _dist: dist, _store_dir: store_dir, _key_dir: key_dir, + pool: store.pool().clone(), address, shutdown: Some(shutdown_tx), } @@ -411,3 +434,106 @@ async fn an_unknown_account_refuses_with_not_found() { let (status, _) = harness.delete("/api/v1/proxmox/accounts/acc-missing").await; assert_eq!(status, axum::http::StatusCode::NOT_FOUND); } + +#[tokio::test] +async fn the_guest_surface_walks_list_and_observe() { + let harness = harness().await; + + // A machine whose endpoint host is the guest-agent address. + let machines = MachineRepository::new(harness.pool.clone()); + use fleet_application::machine::MachinePort as _; + let machine = machines + .register(&fleet_application::machine::RegisterMachine { + name: "fleet-test-01".to_owned(), + description: String::new(), + endpoints: vec![fleet_application::machine::NewEndpoint { + kind: fleet_core::EndpointKind::Ssh, + reference: "ops@192.168.68.240:22".to_owned(), + }], + tags: Vec::new(), + groups: Vec::new(), + }) + .await + .unwrap(); + + let (_, body) = harness + .post( + "/api/v1/proxmox/accounts", + json!({ + "name": "pve-main", + "host": "192.168.68.223", + "tokenId": "root@pam!GLM-AGENT", + "tokenSecret": "the-token-secret-material" + }), + ) + .await; + let account_id = body["data"]["id"].as_str().unwrap().to_owned(); + harness + .post( + &format!("/api/v1/proxmox/accounts/{account_id}/observe"), + json!({}), + ) + .await; + harness + .post( + &format!("/api/v1/proxmox/accounts/{account_id}/confirm"), + json!({"fingerprint": FP}), + ) + .await; + + // Guests list with the address-match candidate. + let (status, body) = harness + .get(&format!("/api/v1/proxmox/accounts/{account_id}/guests")) + .await; + assert_eq!(status, axum::http::StatusCode::OK, "{body}"); + // Two guests: the template entry carries no node, so it is isolated + // into the warnings honestly rather than guessed. + let guests = body["items"].as_array().unwrap(); + assert_eq!(guests.len(), 2, "{body}"); + let guest = guests + .iter() + .find(|guest| guest["vmid"] == 101) + .expect("the test guest lists"); + assert_eq!(guest["macs"][0], "de:ad:be:ef:00:01"); + assert_eq!(guest["agent"]["online"], true); + assert_eq!(guest["agent"]["osName"], "Ubuntu 24.04.4 LTS"); + assert_eq!(guest["candidates"][0]["machineId"], machine.id); + assert_eq!(guest["candidates"][0]["kind"], "address_match"); + + // Observe records the guest facts onto the machine. + let (status, body) = harness + .post( + &format!("/api/v1/proxmox/accounts/{account_id}/guests/101/observe"), + json!({"machineId": machine.id}), + ) + .await; + assert_eq!(status, axum::http::StatusCode::NO_CONTENT, "{body}"); + + // The machine now carries the guest facts. + let (status, body) = harness + .get(&format!("/api/v1/machines/{}", machine.id)) + .await; + assert_eq!(status, axum::http::StatusCode::OK, "{body}"); + let capabilities = body["data"]["capabilities"].as_array().unwrap(); + let fact = |name: &str| { + capabilities + .iter() + .find(|fact| fact["name"] == name) + .map(|fact| fact["value"].clone()) + }; + assert_eq!(fact("guest"), Some(json!("qemu/101"))); + assert_eq!(fact("vmid"), Some(json!("101"))); + assert_eq!(fact("os"), Some(json!("Ubuntu 24.04.4 LTS"))); + assert_eq!(fact("mac0"), Some(json!("de:ad:be:ef:00:01"))); + assert_eq!(fact("agent"), Some(json!("7.2"))); + + // An unknown guest refuses with not-found. + let (status, body) = harness + .post( + &format!("/api/v1/proxmox/accounts/{account_id}/guests/999/observe"), + json!({"machineId": machine.id}), + ) + .await; + assert_eq!(status, axum::http::StatusCode::NOT_FOUND, "{body}"); + assert_eq!(body["code"], "not_found"); +} diff --git a/crates/fleetctl/src/lib.rs b/crates/fleetctl/src/lib.rs index b7e3f21..d7df36d 100644 --- a/crates/fleetctl/src/lib.rs +++ b/crates/fleetctl/src/lib.rs @@ -527,6 +527,21 @@ pub enum Command { /// The account's identity. account_id: String, }, + /// List the account's guests with their Fleet-machine association + /// candidates (evidence only). + ProxmoxGuests { + /// The account's identity. + account_id: String, + }, + /// Record one guest's facts onto a confirmed Fleet machine. + ProxmoxObserveGuest { + /// The account's identity. + account_id: String, + /// The guest's VMID. + vmid: u32, + /// The machine the guest is confirmed to be. + machine_id: String, + }, /// Start the audited "Install Fleet Node" bootstrap on an agentless /// machine: download the checksummed service package on the node, /// install the systemd service, enroll, and wait for the gateway @@ -1174,6 +1189,25 @@ fn parse_proxmox_command(verb: &str, rest: &[&str]) -> Result }), _ => Err(CliError { message: usage() }), }, + "guests" => match rest { + [account_id] => Ok(Command::ProxmoxGuests { + account_id: (*account_id).to_owned(), + }), + _ => Err(CliError { message: usage() }), + }, + "observe-guest" => match rest { + [account_id, vmid, "--machine", machine_id] => { + let parsed = vmid.parse::().map_err(|_| CliError { + message: format!("the VMID must be a number, not {vmid:?}"), + })?; + Ok(Command::ProxmoxObserveGuest { + account_id: (*account_id).to_owned(), + vmid: parsed, + machine_id: (*machine_id).to_owned(), + }) + } + _ => Err(CliError { message: usage() }), + }, _ => Err(CliError { message: usage() }), } } @@ -2158,6 +2192,22 @@ fn request_for(command: &Command) -> Result { Vec::new(), None, ), + Command::ProxmoxGuests { account_id } => ( + reqwest::Method::GET, + format!("/api/v1/proxmox/accounts/{account_id}/guests"), + Vec::new(), + None, + ), + Command::ProxmoxObserveGuest { + account_id, + vmid, + machine_id, + } => ( + reqwest::Method::POST, + format!("/api/v1/proxmox/accounts/{account_id}/guests/{vmid}/observe"), + Vec::new(), + Some(serde_json::json!({ "machineId": machine_id })), + ), Command::TailnetImport { node_id, user, @@ -2747,6 +2797,7 @@ pub fn render_proxmox_for_test(value: &Value) -> String { render_proxmox(Some(value)) } +#[allow(clippy::too_many_lines)] fn render_proxmox(value: Option<&Value>) -> String { let Some(value) = value else { return String::new(); @@ -2788,6 +2839,61 @@ fn render_proxmox(value: Option<&Value>) -> String { } return lines.join("\n"); } + // Guests list: one row per guest with its agent state and candidates. + if let Some(items) = value.get("items").and_then(Value::as_array) + && items.first().is_some_and(|item| item.get("vmid").is_some()) + { + let mut lines = vec![format!( + "{:<10} {:<10} {:<24} {:<12} {}", + "VMID", "KIND", "NAME", "AGENT", "FLEET CANDIDATES" + )]; + for guest in items { + let agent = match &guest["agent"] { + Value::Null => "-".to_owned(), + agent => { + if agent["online"] == true { + "online".to_owned() + } else { + "unreachable".to_owned() + } + } + }; + let candidates = guest["candidates"] + .as_array() + .map(|candidates| { + candidates + .iter() + .filter_map(|candidate| { + Some(format!( + "{} ({})", + candidate["machineName"].as_str()?, + candidate["kind"].as_str()? + )) + }) + .collect::>() + .join(", ") + }) + .unwrap_or_default(); + lines.push(format!( + "{:<10} {:<10} {:<24} {:<12} {}", + guest["vmid"] + .as_u64() + .map_or_else(|| "-".to_owned(), |v| v.to_string()), + guest["kind"].as_str().unwrap_or("-"), + guest["name"].as_str().unwrap_or("-"), + agent, + if candidates.is_empty() { + "-" + } else { + &candidates + } + )); + } + if items.is_empty() { + lines.push("(no guests reported)".to_owned()); + } + return lines.join("\n"); + } // Accounts list: one row per account with its trust state. if let Some(items) = value.get("items").and_then(Value::as_array) { let mut lines = vec![format!( diff --git a/crates/fleetctl/tests/cli.rs b/crates/fleetctl/tests/cli.rs index 1807821..0e241c4 100644 --- a/crates/fleetctl/tests/cli.rs +++ b/crates/fleetctl/tests/cli.rs @@ -1991,3 +1991,69 @@ fn parsing_refuses_the_undocumented_proxmox_forms() { ); } } + +#[test] +fn text_output_renders_proxmox_guests() { + let guests = json!({ + "items": [ + {"kind": "qemu", "id": "qemu/101", "vmid": 101, "name": "fleet-test-01", + "status": "running", "macs": ["de:ad:be:ef:00:01"], + "agent": {"online": true, "version": "7.2", "osName": "Ubuntu 24.04", + "kernel": "6.8.0", "interfaces": []}, + "warnings": [], "pveVersion": "9.2.2", "observedAt": 1, + "candidates": [{"machineId": "m1", "machineName": "box", + "machineStatus": "agentless", "kind": "address_match", + "evidence": "192.168.68.240"}]}, + {"kind": "lxc", "id": "lxc/200", "vmid": 200, "name": "container", + "status": "running", "macs": [], "agent": null, + "warnings": [], "pveVersion": "9.2.2", "observedAt": 1, "candidates": []} + ], + "page": {"limit": 50, "nextCursor": null} + }); + let text = fleetctl::render_proxmox_for_test(&guests); + assert!(text.contains("VMID"), "{text}"); + assert!(text.contains("fleet-test-01"), "{text}"); + assert!(text.contains("online"), "{text}"); + assert!(text.contains("box (address_match)"), "{text}"); + assert!(!text.contains("(no guests reported)"), "{text}"); +} + +#[test] +fn parsing_walks_the_proxmox_guest_forms() { + let args: Vec = ["proxmox", "guests", "acc-1"] + .iter() + .map(ToString::to_string) + .collect(); + assert!(matches!( + fleetctl::parse(&args).unwrap().command, + fleetctl::Command::ProxmoxGuests { .. } + )); + let args: Vec = [ + "proxmox", + "observe-guest", + "acc-1", + "101", + "--machine", + "m-1", + ] + .iter() + .map(ToString::to_string) + .collect(); + assert!(matches!( + fleetctl::parse(&args).unwrap().command, + fleetctl::Command::ProxmoxObserveGuest { .. } + )); + let args: Vec = [ + "proxmox", + "observe-guest", + "acc-1", + "abc", + "--machine", + "m-1", + ] + .iter() + .map(ToString::to_string) + .collect(); + let error = fleetctl::parse(&args).unwrap_err(); + assert!(error.message.contains("must be a number"), "{error}"); +} diff --git a/crates/providers/fleet-provider-proxmox/src/lib.rs b/crates/providers/fleet-provider-proxmox/src/lib.rs index d730528..ccbaa02 100644 --- a/crates/providers/fleet-provider-proxmox/src/lib.rs +++ b/crates/providers/fleet-provider-proxmox/src/lib.rs @@ -442,7 +442,7 @@ impl std::error::Error for PveApiError {} /// One normalized cluster resource: a node, a QEMU guest, an LXC container, /// a storage, or a template. Provenance and time attach at the application /// layer; this is the provider's own shape. -#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] #[serde(rename_all = "camelCase")] pub struct PveResource { /// The normalized kind: `node`, `qemu`, `lxc`, `storage`, or @@ -476,6 +476,72 @@ pub struct PveDiscovery { pub reported_count: usize, } +/// The QEMU Guest Agent's view of one guest, with honest availability per +/// surface. Every field is independent: an off guest is not an agentless +/// guest, and an agent that answers `info` but not `network` is reported +/// exactly so. +#[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PveGuestAgent { + /// The agent answered `info`: it is installed and reachable. + pub online: bool, + /// The agent version string, when `info` carried one. + pub version: Option, + /// The guest's OS facts, when `get-osinfo` answered. + pub os_name: Option, + /// The guest's kernel release, when `get-osinfo` carried one. + pub kernel: Option, + /// The network interfaces the agent saw, when + /// `network-get-interfaces` answered. MACs are normalized + /// (lowercase, colon-separated); addresses are bare. + pub interfaces: Vec, +} + +/// One network interface as the guest agent saw it. +#[derive(Clone, Debug, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PveGuestInterface { + /// The interface name inside the guest, e.g. `ens18`. + pub name: String, + /// The normalized MAC address, when the interface has one. + pub mac: Option, + /// The interface's addresses, when it has any. + pub addresses: Vec, +} + +/// One guest with its provider-side facts: the cluster resource plus the +/// config's MAC addresses and the agent view. Association and provenance +/// attach at the application layer. +#[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PveGuest { + /// The cluster resource the guest came from (`qemu` or `lxc`). + pub resource: PveResource, + /// The MAC addresses from the guest config's `netN` entries, normalized + /// lowercase colon-separated. LXC guests carry theirs in `config` too. + pub macs: Vec, + /// The QEMU Guest Agent view; `None` for LXC (no qemu-guest-agent) or + /// when the config could not be read. Per-surface availability lives + /// inside. + pub agent: Option, + /// The bounded per-surface warnings: one failed agent call or a + /// malformed config entry warns here instead of dropping the guest. + pub warnings: Vec, +} + +/// The guest discovery result: the guests of one account's cluster. +#[derive(Clone, Debug, Default, PartialEq, Eq, serde::Serialize, serde::Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct PveGuestDiscovery { + /// The PVE version seen. + pub version: String, + /// The discovered guests. + pub guests: Vec, + /// The cluster-level warnings (a guest whose config or agent probing + /// failed is isolated here). + pub warnings: Vec, +} + /// The discovery port. The provider implements this over the PVE API; /// tests implement it over recorded fixtures. #[async_trait] @@ -488,10 +554,24 @@ pub trait ProxmoxSource: fmt::Debug + Send + Sync { /// transport failures. Per-resource normalization failures are isolated /// into the result's warnings instead. async fn discover(&self, request: PveHttpRequest) -> Result; + + /// Discovers the account's guests with their config MACs and guest-agent + /// views. Read-only; a guest whose config or agent probing fails is + /// isolated into the result's warnings instead of dropping the snapshot. + /// + /// # Errors + /// + /// Fails with [`PveApiError`] on auth, privilege, HTTP, payload, or + /// transport failures. + async fn guest_discover( + &self, + request: PveHttpRequest, + ) -> Result; } /// The provider client: transport plus normalization. Stateless — every /// call carries its own endpoint and credentials. +#[derive(Clone)] pub struct ProxmoxClient { transport: Arc, } @@ -511,6 +591,51 @@ impl ProxmoxClient { Self { transport } } + /// The version string and the raw cluster-resources entries: the + /// prologue both discovery paths share. + async fn version_and_resources( + &self, + request: &PveHttpRequest, + ) -> Result<(String, Vec), PveApiError> { + // The version first: it anchors provenance and proves the trust. + let version_request = PveHttpRequest { + path: "/api2/json/version".to_owned(), + ..request.clone() + }; + let version_data = self.call(version_request).await?; + let version = version_data + .get("version") + .and_then(serde_json::Value::as_str) + .unwrap_or_default() + .chars() + .take(32) + .collect::(); + if version.is_empty() { + return Err(PveApiError::InvalidPayload { + detail: "the version payload carries no version string".to_owned(), + }); + } + let resources_request = PveHttpRequest { + path: "/api2/json/cluster/resources".to_owned(), + ..request.clone() + }; + let data = self.call(resources_request).await?; + let entries = match data { + serde_json::Value::Array(entries) => entries, + // `data: null` is an empty cluster: honest, not an error. + serde_json::Value::Null => Vec::new(), + other => { + return Err(PveApiError::InvalidPayload { + detail: format!( + "the resources payload is not a list (it is a {})", + type_name_of(&other) + ), + }); + } + }; + Ok((version, entries)) + } + async fn call(&self, request: PveHttpRequest) -> Result { let response = self .transport @@ -567,43 +692,7 @@ fn bounded_body(body: &[u8]) -> String { #[async_trait] impl ProxmoxSource for ProxmoxClient { async fn discover(&self, request: PveHttpRequest) -> Result { - // The version first: it anchors provenance and proves the trust. - let version_request = PveHttpRequest { - path: "/api2/json/version".to_owned(), - ..request.clone() - }; - let version_data = self.call(version_request).await?; - let version = version_data - .get("version") - .and_then(serde_json::Value::as_str) - .unwrap_or_default() - .chars() - .take(32) - .collect::(); - if version.is_empty() { - return Err(PveApiError::InvalidPayload { - detail: "the version payload carries no version string".to_owned(), - }); - } - - let resources_request = PveHttpRequest { - path: "/api2/json/cluster/resources".to_owned(), - ..request.clone() - }; - let data = self.call(resources_request).await?; - let entries = match data { - serde_json::Value::Array(entries) => entries, - // `data: null` is an empty cluster: honest, not an error. - serde_json::Value::Null => Vec::new(), - other => { - return Err(PveApiError::InvalidPayload { - detail: format!( - "the resources payload is not a list (it is a {})", - type_name_of(&other) - ), - }); - } - }; + let (version, entries) = self.version_and_resources(&request).await?; let reported_count = entries.len(); let mut resources = Vec::new(); let mut warnings = Vec::new(); @@ -622,6 +711,279 @@ impl ProxmoxSource for ProxmoxClient { reported_count, }) } + + async fn guest_discover( + &self, + request: PveHttpRequest, + ) -> Result { + // The cluster snapshot names the guests; the config carries their + // MACs; the agent carries their inner facts. Each step degrades + // independently. + let (version, entries) = self.version_and_resources(&request).await?; + let mut guests = Vec::new(); + let mut warnings = Vec::new(); + for (index, entry) in entries.into_iter().enumerate() { + let resource = match normalize_resource(&entry) { + Ok(Some(resource)) if resource.kind == "qemu" || resource.kind == "lxc" => resource, + Ok(_) => continue, + Err(detail) => { + warnings.push(format!("resource #{index}: {detail}")); + continue; + } + }; + let Some(node) = resource.node.clone() else { + warnings.push(format!( + "guest {}: the cluster entry carries no node", + resource.id + )); + continue; + }; + let Some(vmid) = resource.vmid else { + warnings.push(format!( + "guest {}: the cluster entry carries no vmid", + resource.id + )); + continue; + }; + let mut guest = PveGuest { + resource, + ..PveGuest::default() + }; + // The config: MACs for the association evidence. A config + // failure warns; the guest survives without MAC evidence. + let config_request = PveHttpRequest { + path: format!( + "/api2/json/nodes/{}/{}/{vmid}/config", + urlencode(&node), + if guest.resource.kind == "lxc" { + "lxc" + } else { + "qemu" + } + ), + ..request.clone() + }; + match self.call(config_request.clone()).await { + Ok(config) => { + guest.macs = config_macs(&config); + } + Err(PveApiError::Http { status, detail }) => { + warnings.push(format!( + "guest {}: the config answered {status}: {detail}", + guest.resource.id + )); + } + Err(other) => { + warnings.push(format!( + "guest {}: the config failed: {other}", + guest.resource.id + )); + } + } + // The agent: QEMU only, and every surface independently. + if guest.resource.kind == "qemu" { + guest.agent = Some( + self.probe_agent(&request, &node, vmid, &mut guest.warnings) + .await, + ); + } + guests.push(guest); + } + Ok(PveGuestDiscovery { + version, + guests, + warnings, + }) + } +} + +/// The config's `netN` entries, parsed for MAC addresses. The value shape +/// is `virtio=DE:AD:BE:EF:00:01,bridge=vmbr0` — the model is the first +/// key=value pair whose value looks like a MAC. +fn config_macs(config: &serde_json::Value) -> Vec { + let mut macs = Vec::new(); + if let Some(extra) = config.as_object() { + for (key, value) in extra { + if !(key.starts_with("net") && key[3..].chars().all(|c| c.is_ascii_digit())) { + continue; + } + let Some(text) = value.as_str() else { + continue; + }; + for part in text.split(',') { + if let Some(mac) = normalize_mac(part) { + macs.push(mac); + break; + } + } + } + } + macs +} + +/// Normalizes a MAC candidate: `key=AA:BB:…` or bare, lowercase +/// colon-separated, only when it is six hex pairs. +#[must_use] +pub fn normalize_mac(candidate: &str) -> Option { + let value = candidate.split('=').next_back().unwrap_or(candidate); + let bytes = value.split(':').collect::>(); + if bytes.len() != 6 { + return None; + } + let mut normalized = Vec::with_capacity(17); + for (index, byte) in bytes.iter().enumerate() { + if byte.len() != 2 || !byte.chars().all(|c| c.is_ascii_hexdigit()) { + return None; + } + if index > 0 { + normalized.push(':'); + } + normalized.extend(byte.to_lowercase().chars()); + } + Some(normalized.into_iter().collect()) +} + +fn urlencode(value: &str) -> String { + use std::fmt::Write as _; + let mut encoded = String::new(); + for byte in value.bytes() { + if byte.is_ascii_alphanumeric() || matches!(byte, b'-' | b'_' | b'.' | b'~') { + encoded.push(char::from(byte)); + } else { + let _ = write!(encoded, "%{byte:02X}"); + } + } + encoded +} + +impl ProxmoxClient { + /// Probes one guest's agent surfaces, each independently honest. A + /// failed or absent surface is a warning on the guest, never a guest + /// failure. + async fn probe_agent( + &self, + request: &PveHttpRequest, + node: &str, + vmid: u32, + warnings: &mut Vec, + ) -> PveGuestAgent { + let mut agent = PveGuestAgent::default(); + let base = format!("/api2/json/nodes/{}/qemu/{vmid}/agent", urlencode(node)); + // info: is the agent there at all? + let info_request = PveHttpRequest { + path: format!("{base}/info"), + ..request.clone() + }; + if self.call(info_request.clone()).await.is_err() { + warnings.push(format!("guest qemu/{vmid}: the agent is unreachable")); + // Every other surface would fail the same way; report the + // honest offline agent and stop here. + return agent; + } + // The agent answered `info`; the version detail rides the same + // envelope. Re-calling is avoided by treating info's success as + // online and fetching the version from the same call's data. + if let Ok(data) = self.call(info_request).await { + let result = data.get("result").cloned().unwrap_or(data); + agent.online = true; + agent.version = result + .get("version") + .and_then(serde_json::Value::as_str) + .map(|value| value.chars().take(64).collect()); + } + // network-get-interfaces + let net_request = PveHttpRequest { + path: format!("{base}/network-get-interfaces"), + ..request.clone() + }; + match self.call(net_request.clone()).await { + Ok(data) => { + let result = data.get("result").cloned().unwrap_or(data); + if let Some(interfaces) = result.as_array() { + for interface in interfaces { + match normalize_interface(interface) { + Ok(Some(interface)) => agent.interfaces.push(interface), + Ok(None) => {} + Err(detail) => { + warnings.push(format!("guest qemu/{vmid}: {detail}")); + } + } + } + } + } + Err(error) => { + warnings.push(format!( + "guest qemu/{vmid}: the agent's network surface failed: {error}" + )); + } + } + // get-osinfo + let os_request = PveHttpRequest { + path: format!("{base}/get-osinfo"), + ..request.clone() + }; + match self.call(os_request.clone()).await { + Ok(data) => { + let result = data.get("result").cloned().unwrap_or(data); + agent.os_name = result + .get("pretty-name") + .and_then(serde_json::Value::as_str) + .map(|value| value.chars().take(128).collect()); + agent.kernel = result + .get("kernel-release") + .and_then(serde_json::Value::as_str) + .map(|value| value.chars().take(128).collect()); + } + Err(error) => { + warnings.push(format!( + "guest qemu/{vmid}: the agent's OS surface failed: {error}" + )); + } + } + agent + } +} + +/// Normalizes one agent network interface. `Ok(None)` skips loopback-style +/// entries without a MAC; `Err` warns. +fn normalize_interface(interface: &serde_json::Value) -> Result, String> { + let Some(name) = interface + .get("name") + .and_then(serde_json::Value::as_str) + .map(|value| value.chars().take(64).collect::()) + else { + return Err("the interface entry carries no name".to_owned()); + }; + let mac = interface + .get("hardware-address") + .and_then(serde_json::Value::as_str) + .and_then(normalize_mac); + let mut addresses = Vec::new(); + if let Some(list) = interface + .get("ip-addresses") + .and_then(serde_json::Value::as_array) + { + for address in list { + if let Some(text) = address + .get("ip-address") + .and_then(serde_json::Value::as_str) + { + let bounded = text.chars().take(64).collect::(); + if !bounded.is_empty() { + addresses.push(bounded); + } + } + } + } + if mac.is_none() && addresses.is_empty() { + // Loopback-style: no association evidence, skip silently. + return Ok(None); + } + Ok(Some(PveGuestInterface { + name, + mac, + addresses, + })) } fn type_name_of(value: &serde_json::Value) -> &'static str { @@ -747,6 +1109,64 @@ mod tests { assert!(error.contains("unrecognized type"), "{error}"); } + #[test] + fn config_macs_parse_the_netn_entries() { + let config = serde_json::json!({ + "net0": "virtio=DE:AD:BE:EF:00:01,bridge=vmbr0,firewall=1", + "net1": "virtio=DE:AD:BE:EF:00:02", + "scsi0": "local-lvm:vm-101-disk-0", + "memory": 2048 + }); + let macs = config_macs(&config); + assert_eq!(macs.len(), 2, "{macs:?}"); + assert_eq!(macs[0], "de:ad:be:ef:00:01"); + assert_eq!(macs[1], "de:ad:be:ef:00:02"); + } + + #[test] + fn mac_normalization_refuses_non_macs() { + assert_eq!( + normalize_mac("AA:BB:CC:DD:EE:FF").as_deref(), + Some("aa:bb:cc:dd:ee:ff") + ); + assert_eq!( + normalize_mac("virtio=DE:AD:BE:EF:00:01").as_deref(), + Some("de:ad:be:ef:00:01") + ); + assert!(normalize_mac("bridge=vmbr0").is_none()); + assert!(normalize_mac("AA:BB:CC").is_none()); + assert!(normalize_mac("ZZ:BB:CC:DD:EE:FF").is_none()); + } + + #[test] + fn interfaces_normalize_and_skip_loopback() { + let interface = serde_json::json!({ + "name": "ens18", + "hardware-address": "BC:24:11:97:DB:A8", + "ip-addresses": [ + {"ip-address": "192.168.68.240", "ip-address-type": "ipv4", "prefix": 24} + ] + }); + let normalized = normalize_interface(&interface).unwrap().unwrap(); + assert_eq!(normalized.name, "ens18"); + assert_eq!(normalized.mac.as_deref(), Some("bc:24:11:97:db:a8")); + assert_eq!(normalized.addresses, vec!["192.168.68.240".to_owned()]); + + let loopback = serde_json::json!({ + "name": "lo", + "hardware-address": "00:00:00:00:00:00", + "ip-addresses": [ + {"ip-address": "127.0.0.1", "ip-address-type": "ipv4", "prefix": 8} + ] + }); + // Loopback carries a MAC (all zeros) and an address: it lands, and + // the application layer decides its evidence weight. + assert!(normalize_interface(&loopback).unwrap().is_some()); + + let nameless = serde_json::json!({"hardware-address": "BC:24:11:97:DB:A8"}); + assert!(normalize_interface(&nameless).is_err()); + } + #[test] fn authorities_bracket_ipv6_literals() { let request = |host: &str| PveHttpRequest { diff --git a/packages/api-client/openapi.json b/packages/api-client/openapi.json index 34e2545..9bd2e91 100644 --- a/packages/api-client/openapi.json +++ b/packages/api-client/openapi.json @@ -2207,6 +2207,136 @@ } } }, + "/api/v1/proxmox/accounts/{accountId}/guests": { + "get": { + "tags": [ + "proxmox" + ], + "summary": "Lists the account's guests with their Fleet-machine association\ncandidates (evidence only).", + "description": "# Errors\n\nReturns the public error envelope on refusal, an unconfirmed account, or\na source failure.", + "operationId": "listProxmoxGuests", + "parameters": [ + { + "name": "accountId", + "in": "path", + "description": "The account's identity.", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "The guests with their association candidates.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Page_AssociatedGuestDto" + } + } + } + }, + "403": { + "description": "The caller may not read the Proxmox surface.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "409": { + "description": "The account's trust is unconfirmed.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + } + } + } + }, + "/api/v1/proxmox/accounts/{accountId}/guests/{vmid}/observe": { + "post": { + "tags": [ + "proxmox" + ], + "summary": "Records one guest's facts onto a confirmed Fleet machine. The machine\nfunnel authorizes and audits the capability write.", + "description": "# Errors\n\nReturns the public error envelope on refusal, an unknown account,\nguest, or machine, or a source failure.", + "operationId": "observeProxmoxGuest", + "parameters": [ + { + "name": "accountId", + "in": "path", + "description": "The account's identity.", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "vmid", + "in": "path", + "description": "The guest's VMID.", + "required": true, + "schema": { + "type": "integer", + "format": "int32", + "minimum": 0 + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ObserveProxmoxGuestRequest" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "The guest's facts were recorded on the machine." + }, + "403": { + "description": "The caller may not read the Proxmox surface or write the machine's facts.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "404": { + "description": "The account, guest, or machine does not exist.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + }, + "409": { + "description": "The account's trust is unconfirmed or the source refused.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/ApiError" + } + } + } + } + } + } + }, "/api/v1/proxmox/accounts/{accountId}/observe": { "post": { "tags": [ @@ -2724,6 +2854,133 @@ ], "description": "How the apply workflow's endpoint authenticates." }, + "AssociatedGuestDto": { + "type": "object", + "description": "One discovered guest with its Fleet-machine association candidates.", + "required": [ + "kind", + "id", + "macs", + "warnings", + "pveVersion", + "observedAt", + "candidates" + ], + "properties": { + "agent": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/ProviderAgentDto", + "description": "The guest-agent view, when the guest has one." + } + ] + }, + "candidates": { + "type": "array", + "items": { + "$ref": "#/components/schemas/AssociationCandidateDto" + }, + "description": "The Fleet machines this guest may be — evidence, never merged." + }, + "id": { + "type": "string", + "description": "The cluster-visible id." + }, + "kind": { + "type": "string", + "description": "The normalized kind: `qemu` or `lxc`." + }, + "macs": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The config's MAC addresses, normalized." + }, + "name": { + "type": [ + "string", + "null" + ], + "description": "The display name, when carried." + }, + "node": { + "type": [ + "string", + "null" + ], + "description": "The hosting node." + }, + "observedAt": { + "type": "integer", + "format": "int64", + "description": "When the observation was taken." + }, + "pveVersion": { + "type": "string", + "description": "The PVE version the observation came from." + }, + "status": { + "type": [ + "string", + "null" + ], + "description": "The PVE status string, when carried." + }, + "vmid": { + "type": [ + "integer", + "null" + ], + "format": "int32", + "description": "The VMID.", + "minimum": 0 + }, + "warnings": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The bounded per-surface warnings." + } + } + }, + "AssociationCandidateDto": { + "type": "object", + "description": "One Fleet machine a guest may be, with the evidence.", + "required": [ + "machineId", + "machineName", + "machineStatus", + "kind", + "evidence" + ], + "properties": { + "evidence": { + "type": "string", + "description": "The evidence value that matched." + }, + "kind": { + "type": "string", + "description": "Why: `mac_match`, `address_match`, or `name_match`." + }, + "machineId": { + "type": "string", + "description": "The existing machine's identity." + }, + "machineName": { + "type": "string", + "description": "The existing machine's name." + }, + "machineStatus": { + "type": "string", + "description": "The machine's derived connectivity state." + } + } + }, "CapabilityFactDto": { "type": "object", "description": "One capability fact as the read model displays it: the recorded status\nwith the staleness rule applied at the read time.", @@ -3895,6 +4152,19 @@ } } }, + "ObserveProxmoxGuestRequest": { + "type": "object", + "description": "The observe-guest request: which machine the guest's facts record onto.", + "required": [ + "machineId" + ], + "properties": { + "machineId": { + "type": "string", + "description": "The machine the guest is confirmed to be." + } + } + }, "OnboardAuthDto": { "oneOf": [ { @@ -4316,6 +4586,118 @@ } } }, + "Page_AssociatedGuestDto": { + "type": "object", + "description": "A page of resources.\n\nThe concrete schema for a list endpoint appears when that endpoint does;\n[`PageInfo`] is the part of the shape that is fixed for every one of them.", + "required": [ + "items", + "page" + ], + "properties": { + "items": { + "type": "array", + "items": { + "type": "object", + "description": "One discovered guest with its Fleet-machine association candidates.", + "required": [ + "kind", + "id", + "macs", + "warnings", + "pveVersion", + "observedAt", + "candidates" + ], + "properties": { + "agent": { + "oneOf": [ + { + "type": "null" + }, + { + "$ref": "#/components/schemas/ProviderAgentDto", + "description": "The guest-agent view, when the guest has one." + } + ] + }, + "candidates": { + "type": "array", + "items": { + "$ref": "#/components/schemas/AssociationCandidateDto" + }, + "description": "The Fleet machines this guest may be — evidence, never merged." + }, + "id": { + "type": "string", + "description": "The cluster-visible id." + }, + "kind": { + "type": "string", + "description": "The normalized kind: `qemu` or `lxc`." + }, + "macs": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The config's MAC addresses, normalized." + }, + "name": { + "type": [ + "string", + "null" + ], + "description": "The display name, when carried." + }, + "node": { + "type": [ + "string", + "null" + ], + "description": "The hosting node." + }, + "observedAt": { + "type": "integer", + "format": "int64", + "description": "When the observation was taken." + }, + "pveVersion": { + "type": "string", + "description": "The PVE version the observation came from." + }, + "status": { + "type": [ + "string", + "null" + ], + "description": "The PVE status string, when carried." + }, + "vmid": { + "type": [ + "integer", + "null" + ], + "format": "int32", + "description": "The VMID.", + "minimum": 0 + }, + "warnings": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The bounded per-surface warnings." + } + } + }, + "description": "The items on this page, in the endpoint's documented order." + }, + "page": { + "$ref": "#/components/schemas/PageInfo", + "description": "Where this page sits in the result set." + } + } + }, "Page_CorrelatedDeviceDto": { "type": "object", "description": "A page of resources.\n\nThe concrete schema for a list endpoint appears when that endpoint does;\n[`PageInfo`] is the part of the shape that is fixed for every one of them.", @@ -4917,6 +5299,76 @@ } } }, + "ProviderAgentDto": { + "type": "object", + "description": "The guest-agent view.", + "required": [ + "online", + "interfaces" + ], + "properties": { + "interfaces": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ProviderInterfaceDto" + }, + "description": "The network interfaces the agent saw." + }, + "kernel": { + "type": [ + "string", + "null" + ], + "description": "The guest's kernel release, when carried." + }, + "online": { + "type": "boolean", + "description": "The agent answered `info`: installed and reachable." + }, + "osName": { + "type": [ + "string", + "null" + ], + "description": "The guest's OS name, when `get-osinfo` answered." + }, + "version": { + "type": [ + "string", + "null" + ], + "description": "The agent version, when carried." + } + } + }, + "ProviderInterfaceDto": { + "type": "object", + "description": "One guest network interface.", + "required": [ + "name", + "addresses" + ], + "properties": { + "addresses": { + "type": "array", + "items": { + "type": "string" + }, + "description": "The interface's addresses." + }, + "mac": { + "type": [ + "string", + "null" + ], + "description": "The normalized MAC, when carried." + }, + "name": { + "type": "string", + "description": "The interface name inside the guest." + } + } + }, "ProxmoxAccountDto": { "type": "object", "description": "One configured Proxmox account. The token secret is never here.", diff --git a/packages/api-client/src/generated/fleet.ts b/packages/api-client/src/generated/fleet.ts index 1d5125d..f296bb5 100644 --- a/packages/api-client/src/generated/fleet.ts +++ b/packages/api-client/src/generated/fleet.ts @@ -237,6 +237,104 @@ export type ApplyAuthDto = { type: 'identityFile'; }; +/** + * One guest network interface. + */ +export interface ProviderInterfaceDto { + /** The interface's addresses. */ + addresses: string[]; + /** + * The normalized MAC, when carried. + * @nullable + */ + mac?: string | null; + /** The interface name inside the guest. */ + name: string; +} + +/** + * The guest-agent view. + */ +export interface ProviderAgentDto { + /** The network interfaces the agent saw. */ + interfaces: ProviderInterfaceDto[]; + /** + * The guest's kernel release, when carried. + * @nullable + */ + kernel?: string | null; + /** The agent answered `info`: installed and reachable. */ + online: boolean; + /** + * The guest's OS name, when `get-osinfo` answered. + * @nullable + */ + osName?: string | null; + /** + * The agent version, when carried. + * @nullable + */ + version?: string | null; +} + +/** + * One Fleet machine a guest may be, with the evidence. + */ +export interface AssociationCandidateDto { + /** The evidence value that matched. */ + evidence: string; + /** Why: `mac_match`, `address_match`, or `name_match`. */ + kind: string; + /** The existing machine's identity. */ + machineId: string; + /** The existing machine's name. */ + machineName: string; + /** The machine's derived connectivity state. */ + machineStatus: string; +} + +/** + * One discovered guest with its Fleet-machine association candidates. + */ +export interface AssociatedGuestDto { + agent?: null | ProviderAgentDto; + /** The Fleet machines this guest may be — evidence, never merged. */ + candidates: AssociationCandidateDto[]; + /** The cluster-visible id. */ + id: string; + /** The normalized kind: `qemu` or `lxc`. */ + kind: string; + /** The config's MAC addresses, normalized. */ + macs: string[]; + /** + * The display name, when carried. + * @nullable + */ + name?: string | null; + /** + * The hosting node. + * @nullable + */ + node?: string | null; + /** When the observation was taken. */ + observedAt: number; + /** The PVE version the observation came from. */ + pveVersion: string; + /** + * The PVE status string, when carried. + * @nullable + */ + status?: string | null; + /** + * The VMID. + * @minimum 0 + * @nullable + */ + vmid?: number | null; + /** The bounded per-surface warnings. */ + warnings: string[]; +} + /** * How a checkout action's endpoint authenticates. */ @@ -733,6 +831,14 @@ export interface NodeViewDto { pendingTokens: EnrollmentTokenDto[]; } +/** + * The observe-guest request: which machine the guest's facts record onto. + */ +export interface ObserveProxmoxGuestRequest { + /** The machine the guest is confirmed to be. */ + machineId: string; +} + /** * A host key as observed from the network, staged for review. Public data. */ @@ -967,6 +1073,61 @@ export interface PageInfo { nextCursor?: string | null; } +/** + * One discovered guest with its Fleet-machine association candidates. + */ +export type PageAssociatedGuestDtoItemsItem = { + agent?: null | ProviderAgentDto; + /** The Fleet machines this guest may be — evidence, never merged. */ + candidates: AssociationCandidateDto[]; + /** The cluster-visible id. */ + id: string; + /** The normalized kind: `qemu` or `lxc`. */ + kind: string; + /** The config's MAC addresses, normalized. */ + macs: string[]; + /** + * The display name, when carried. + * @nullable + */ + name?: string | null; + /** + * The hosting node. + * @nullable + */ + node?: string | null; + /** When the observation was taken. */ + observedAt: number; + /** The PVE version the observation came from. */ + pveVersion: string; + /** + * The PVE status string, when carried. + * @nullable + */ + status?: string | null; + /** + * The VMID. + * @minimum 0 + * @nullable + */ + vmid?: number | null; + /** The bounded per-surface warnings. */ + warnings: string[]; +}; + +/** + * A page of resources. + * + * The concrete schema for a list endpoint appears when that endpoint does; + * [`PageInfo`] is the part of the shape that is fixed for every one of them. + */ +export interface PageAssociatedGuestDto { + /** The items on this page, in the endpoint's documented order. */ + items: PageAssociatedGuestDtoItemsItem[]; + /** Where this page sits in the result set. */ + page: PageInfo; +} + /** * One tailnet device with its Fleet-machine candidates (evidence only). */ @@ -4686,6 +4847,140 @@ export const discoverProxmoxCluster = async (accountId: string, options?: Reques +export type listProxmoxGuestsResponse200 = { + data: PageAssociatedGuestDto + status: 200 +} + +export type listProxmoxGuestsResponse403 = { + data: ApiError + status: 403 +} + +export type listProxmoxGuestsResponse409 = { + data: ApiError + status: 409 +} + +export type listProxmoxGuestsResponseSuccess = (listProxmoxGuestsResponse200) & { + headers: Headers; +}; +export type listProxmoxGuestsResponseError = (listProxmoxGuestsResponse403 | listProxmoxGuestsResponse409) & { + headers: Headers; +}; + +export type listProxmoxGuestsResponse = (listProxmoxGuestsResponseSuccess | listProxmoxGuestsResponseError) + +export const getListProxmoxGuestsUrl = (accountId: string,) => { + + + + + return `/api/v1/proxmox/accounts/${accountId}/guests` +} + +/** + * # Errors + * + * Returns the public error envelope on refusal, an unconfirmed account, or + * a source failure. + * @summary Lists the account's guests with their Fleet-machine association +candidates (evidence only). + */ +export const listProxmoxGuests = async (accountId: string, options?: RequestInit): Promise => { + + const res = await fetch(getListProxmoxGuestsUrl(accountId), + { + ...options, + method: 'GET' + + + } +) + + + const body = [204, 205, 304].includes(res.status) ? null : await res.text(); + + const data: listProxmoxGuestsResponse['data'] = body ? JSON.parse(body) : {} + return { data, status: res.status, headers: res.headers } as listProxmoxGuestsResponse +} + + + +export type observeProxmoxGuestResponse204 = { + data: void + status: 204 +} + +export type observeProxmoxGuestResponse403 = { + data: ApiError + status: 403 +} + +export type observeProxmoxGuestResponse404 = { + data: ApiError + status: 404 +} + +export type observeProxmoxGuestResponse409 = { + data: ApiError + status: 409 +} + +export type observeProxmoxGuestResponseSuccess = (observeProxmoxGuestResponse204) & { + headers: Headers; +}; +export type observeProxmoxGuestResponseError = (observeProxmoxGuestResponse403 | observeProxmoxGuestResponse404 | observeProxmoxGuestResponse409) & { + headers: Headers; +}; + +export type observeProxmoxGuestResponse = (observeProxmoxGuestResponseSuccess | observeProxmoxGuestResponseError) + +export const getObserveProxmoxGuestUrl = (accountId: string, + vmid: number,) => { + + + + + return `/api/v1/proxmox/accounts/${accountId}/guests/${vmid}/observe` +} + +/** + * # Errors + * + * Returns the public error envelope on refusal, an unknown account, + * guest, or machine, or a source failure. + * @summary Records one guest's facts onto a confirmed Fleet machine. The machine +funnel authorizes and audits the capability write. + */ +export const observeProxmoxGuest = async (accountId: string, + vmid: number, + observeProxmoxGuestRequest: ObserveProxmoxGuestRequest, options?: RequestInit): Promise => { + + const getHeaders = (h?: NonNullable): Record => { + if (!h) return {}; + if (h instanceof Headers) return Object.fromEntries(h.entries()); + if (Array.isArray(h)) return Object.fromEntries(h); + return h; + }; +const res = await fetch(getObserveProxmoxGuestUrl(accountId,vmid), + { + ...options, + method: 'POST', + headers: { 'Content-Type': 'application/json', ...getHeaders(options?.headers) }, + body: JSON.stringify(observeProxmoxGuestRequest) + } +) + + + const body = [204, 205, 304].includes(res.status) ? null : await res.text(); + + const data: observeProxmoxGuestResponse['data'] = body ? JSON.parse(body) : undefined + return { data, status: res.status, headers: res.headers } as observeProxmoxGuestResponse +} + + + export type observeProxmoxFingerprintResponse200 = { data: ResourceProxmoxFingerprintDto status: 200 From 6bf415849250950726376757bdaf105367e43baa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andreas=20Fr=C3=B8yland?= Date: Sun, 20 Sep 2026 18:11:10 +0000 Subject: [PATCH 2/3] FM-601: address the cubic review findings --- crates/fleet-api/src/proxmox.rs | 86 +++++++++++++++++-- crates/fleet-application/src/onboarding.rs | 11 ++- crates/fleet-application/src/proxmox.rs | 63 ++++++++++---- crates/fleet-application/tests/proxmox.rs | 27 +++--- crates/fleet-controller/src/proxmox_store.rs | 31 +------ crates/fleet-controller/tests/proxmox.rs | 27 +++++- crates/fleetctl/src/lib.rs | 76 +++++++++++++++- crates/fleetctl/tests/cli.rs | 38 +++++++- .../fleet-provider-proxmox/src/lib.rs | 72 ++++++++++------ packages/api-client/openapi.json | 64 +++++++++++++- packages/api-client/src/generated/fleet.ts | 34 +++++++- 11 files changed, 423 insertions(+), 106 deletions(-) diff --git a/crates/fleet-api/src/proxmox.rs b/crates/fleet-api/src/proxmox.rs index 14ed490..8d4d4a1 100644 --- a/crates/fleet-api/src/proxmox.rs +++ b/crates/fleet-api/src/proxmox.rs @@ -303,6 +303,16 @@ impl From for AssociatedGuestDto { } } +/// The guests-list query parameters. +#[derive(Debug, Deserialize, ToSchema)] +#[serde(rename_all = "camelCase")] +pub struct ListProxmoxGuestsParams { + /// The maximum number of guests to return. + pub limit: Option, + /// The opaque cursor: the last guest's cluster id of the previous page. + pub cursor: Option, +} + /// The observe-guest request: which machine the guest's facts record onto. #[derive(Debug, Deserialize, ToSchema)] #[serde(rename_all = "camelCase")] @@ -770,9 +780,24 @@ pub async fn discover_proxmox_cluster( description = "The caller may not read the Proxmox surface.", body = crate::error::ApiError ), + ( + status = 404, + description = "The account does not exist.", + body = crate::error::ApiError + ), ( status = 409, - description = "The account's trust is unconfirmed.", + description = "The account's trust is unconfirmed or the host fingerprint was refused.", + body = crate::error::ApiError + ), + ( + status = 400, + description = "The cursor names no guest in the snapshot.", + body = crate::error::ApiError + ), + ( + status = 502, + description = "The PVE API refused the token or failed.", body = crate::error::ApiError ), ) @@ -782,10 +807,11 @@ pub async fn list_proxmox_guests( principal: Option>, Extension(correlation_id): Extension, Path(account_id): Path, + Query(params): Query, ) -> Result>, ApiErrorResponse> { let proxmox = proxmox_or_error(&state, correlation_id)?; let principal = crate::operations::principal_or_error(principal, correlation_id)?; - let guests = proxmox + let snapshot = proxmox .guests( state.authorizer.as_ref(), &principal, @@ -794,12 +820,41 @@ pub async fn list_proxmox_guests( ) .await .map_err(|error| map_proxmox_error(&error, correlation_id))?; - let items: Vec = guests.into_iter().map(Into::into).collect(); + // The snapshot is bounded in memory by the provider's own bounds; the + // page bound applies on the way out, with the cursor advancing through + // the guest list. Warnings are reported on the first page only — they + // describe the snapshot, not a page of it. + let limit = params + .limit + .filter(|limit| *limit > 0) + .unwrap_or(DEFAULT_PAGE_LIMIT) + .min(MAX_PAGE_LIMIT); + let start = match ¶ms.cursor { + Some(cursor) => snapshot + .guests + .iter() + .position(|guest| guest.guest.id == *cursor) + .map_or(0, |position| position + 1), + None => 0, + }; + let page: Vec = snapshot + .guests + .into_iter() + .skip(start) + .take(usize::try_from(limit).unwrap_or(usize::MAX)) + .collect(); + let next_cursor = (page.len() == usize::try_from(limit).unwrap_or(0)) + .then(|| page.last().map(|guest| guest.guest.id.clone())) + .flatten(); + let mut items: Vec = page.into_iter().map(Into::into).collect(); + if start == 0 + && !snapshot.warnings.is_empty() + && let Some(first) = items.first_mut() + { + first.warnings.extend(snapshot.warnings.clone()); + } Ok(Json(Page { - page: PageInfo { - next_cursor: None, - limit: items.len().try_into().unwrap_or(u32::MAX), - }, + page: PageInfo { next_cursor, limit }, items, })) } @@ -846,7 +901,22 @@ pub async fn list_proxmox_guests( ), ( status = 409, - description = "The account's trust is unconfirmed or the source refused.", + description = "The account's trust is unconfirmed or the host fingerprint was refused.", + body = crate::error::ApiError + ), + ( + status = 400, + description = "The request is malformed.", + body = crate::error::ApiError + ), + ( + status = 500, + description = "A backend port failed.", + body = crate::error::ApiError + ), + ( + status = 502, + description = "The PVE API refused the token or failed.", body = crate::error::ApiError ), ) diff --git a/crates/fleet-application/src/onboarding.rs b/crates/fleet-application/src/onboarding.rs index 8c7a0b5..6237530 100644 --- a/crates/fleet-application/src/onboarding.rs +++ b/crates/fleet-application/src/onboarding.rs @@ -1094,9 +1094,16 @@ pub fn endpoint_reference_matches(reference: &str, endpoint: &DraftEndpoint) -> && reference_port(reference).is_some_and(|port| port == endpoint.port.to_string()) } -/// The host part of an endpoint reference, ignoring userinfo. -fn reference_host(reference: &str) -> Option<&str> { +/// The host part of an endpoint reference, ignoring userinfo. Bracketed +/// IPv6 literals strip their brackets; the comparison sees the bare +/// address. +#[must_use] +pub(crate) fn reference_host(reference: &str) -> Option<&str> { let (_, host_port) = reference.rsplit_once('@')?; + let host_port = match host_port.strip_prefix('[') { + Some(rest) => rest.split_once(']').map_or(host_port, |(inner, _)| inner), + None => host_port, + }; let (host, _) = host_port.rsplit_once(':')?; Some(host) } diff --git a/crates/fleet-application/src/proxmox.rs b/crates/fleet-application/src/proxmox.rs index bf8c9f1..5aa6789 100644 --- a/crates/fleet-application/src/proxmox.rs +++ b/crates/fleet-application/src/proxmox.rs @@ -380,6 +380,24 @@ pub struct RawGuestDiscovery { pub warnings: Vec, } +/// The guest-discovery snapshot: the associated guests plus the honest +/// record of what the cluster reported but could not be turned into a +/// guest (entries missing a node or VMID, malformed rows). +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct GuestSnapshot { + /// The account that produced the snapshot. + pub account_id: String, + /// The PVE version seen. + pub pve_version: String, + /// The discovered guests with their association candidates. + pub guests: Vec, + /// The cluster-level warnings, per isolated entry. + pub warnings: Vec, + /// When the snapshot was taken (epoch millis). + pub observed_at: i64, +} + /// The account record port: durable account state. #[async_trait] pub trait ProxmoxAccountPort: fmt::Debug + Send + Sync { @@ -980,7 +998,7 @@ impl ProxmoxAccounts { principal: &ActingPrincipal, account_id: &str, now: i64, - ) -> Result, ProxmoxUseCaseError> { + ) -> Result { authorize( authorizer, AccessRequest { @@ -1020,7 +1038,7 @@ impl ProxmoxAccounts { } else { Vec::new() }; - Ok(raw + let associated = raw .guests .into_iter() .map(|guest| { @@ -1053,7 +1071,14 @@ impl ProxmoxAccounts { candidates, } }) - .collect()) + .collect(); + Ok(GuestSnapshot { + account_id: account.id, + pve_version: raw.version, + guests: associated, + warnings: raw.warnings, + observed_at: now, + }) } /// Records one guest's facts onto a confirmed Fleet machine as @@ -1084,6 +1109,18 @@ impl ProxmoxAccounts { }, ) .map_err(ProxmoxUseCaseError::Denied)?; + // The machine funnel's authorization is checked BEFORE any network + // work: a caller who may read Proxmox but not write the machine's + // facts never causes a credential-bearing request. + authorize( + authorizer, + AccessRequest { + principal_id: &principal.id, + action: Permission::MachineUpdate, + resource: Some(machine_id), + }, + ) + .map_err(ProxmoxUseCaseError::Denied)?; let account = self.trusted_account(account_id).await?; let secret = self.require_secret(&account).await?; let raw = self @@ -1108,7 +1145,7 @@ impl ProxmoxAccounts { Some(("vmid", &vmid.to_string())), ) .await?; - let facts = guest_facts(&guest, machine_id, &raw.version, now); + let facts = guest_facts(&guest, &raw.version, now); self.machines .record_capabilities(authorizer, principal, machine_id, &facts) .await @@ -1248,16 +1285,12 @@ fn association_candidate( } } // The guest-agent addresses against the machine endpoints' hosts. The - // endpoint reference is `user@host:port` (redacted forms carry `***`), - // so the host is the segment after the last `@` with the port stripped. + // shared reference parser strips userinfo and IPv6 brackets, so + // bracketed IPv6 evidence compares bare. let endpoint_hosts: Vec<&str> = view .endpoints .iter() - .filter_map(|endpoint| { - let (_, host_port) = endpoint.reference.rsplit_once('@')?; - let (host, _) = host_port.rsplit_once(':')?; - Some(host) - }) + .filter_map(|endpoint| crate::onboarding::reference_host(&endpoint.reference)) .collect(); for interface in guest.agent.iter().flat_map(|agent| &agent.interfaces) { for address in &interface.addresses { @@ -1296,12 +1329,7 @@ fn association_candidate( /// kernel when the agent answered, and the MACs. Provenance is the /// account's observation path; every fact carries the observation time. #[must_use] -fn guest_facts( - guest: &ProviderGuest, - machine_id: &str, - pve_version: &str, - now: i64, -) -> Vec { +fn guest_facts(guest: &ProviderGuest, pve_version: &str, now: i64) -> Vec { let source = format!("proxmox/{pve_version}"); let mut facts = Vec::new(); let mut push = |name: &str, value: Option, status: CapabilityStatus| { @@ -1314,7 +1342,6 @@ fn guest_facts( source: source.clone(), }); }; - let _ = machine_id; push("guest", Some(guest.id.clone()), CapabilityStatus::Known); if let Some(vmid) = guest.vmid { push("vmid", Some(vmid.to_string()), CapabilityStatus::Known); diff --git a/crates/fleet-application/tests/proxmox.rs b/crates/fleet-application/tests/proxmox.rs index c3c55a5..2b5914e 100644 --- a/crates/fleet-application/tests/proxmox.rs +++ b/crates/fleet-application/tests/proxmox.rs @@ -971,11 +971,12 @@ async fn guests_list_with_evidence_only_candidates() { ) .await; - let guests = proxmox + let snapshot = proxmox .guests(&AllowAll, &principal(), &account.id, NOW) .await .unwrap(); - assert_eq!(guests.len(), 2); + assert_eq!(snapshot.guests.len(), 2); + let guests = &snapshot.guests; let qemu = guests .iter() @@ -1017,13 +1018,13 @@ async fn mac_evidence_outranks_address_and_name_evidence() { Some("DE:AD:BE:EF:00:01"), ) .await; - let guests = proxmox + let snapshot = proxmox .guests(&AllowAll, &principal(), &account.id, NOW) .await .unwrap(); - assert_eq!(guests[0].candidates.len(), 1); - assert_eq!(guests[0].candidates[0].machine_id, machine_id); - assert_eq!(guests[0].candidates[0].kind, "mac_match"); + assert_eq!(snapshot.guests[0].candidates.len(), 1); + assert_eq!(snapshot.guests[0].candidates[0].machine_id, machine_id); + assert_eq!(snapshot.guests[0].candidates[0].kind, "mac_match"); } #[tokio::test] @@ -1035,14 +1036,14 @@ async fn unmatched_guests_list_without_candidates() { ); let account = create_account(&proxmox).await; observe_and_confirm(&proxmox, &account.id).await; - let guests = proxmox + let snapshot = proxmox .guests(&AllowAll, &principal(), &account.id, NOW) .await .unwrap(); assert!( - guests[0].candidates.is_empty(), + snapshot.guests[0].candidates.is_empty(), "{:?}", - guests[0].candidates + snapshot.guests[0].candidates ); } @@ -1070,13 +1071,13 @@ async fn a_sensitive_denial_degrades_candidates_not_the_guests() { // A machine whose name differs from the guest's, so the only possible // evidence is the address — which needs sensitive endpoint detail. register_machine(&machine_port, "physical-box", "ops@192.168.68.240:22", None).await; - let guests = proxmox + let snapshot = proxmox .guests(&SensitiveDenied, &principal(), &account.id, NOW) .await .unwrap(); - // The guests still list; the redacted endpoint cannot match, so the - // candidates are empty rather than half-redacted lies. - assert!(guests[0].candidates.is_empty()); + // The guests still list; without the sensitive read the candidates are + // empty rather than half-redacted lies. + assert!(snapshot.guests[0].candidates.is_empty()); } #[tokio::test] diff --git a/crates/fleet-controller/src/proxmox_store.rs b/crates/fleet-controller/src/proxmox_store.rs index 5cf20d6..79e8d3f 100644 --- a/crates/fleet-controller/src/proxmox_store.rs +++ b/crates/fleet-controller/src/proxmox_store.rs @@ -273,36 +273,7 @@ impl ProxmoxDiscoverPort for ProviderDiscovery { warnings: discovery.warnings, reported_count: discovery.reported_count, }), - Err(fleet_provider_proxmox::PveApiError::Auth) => Err(ProxmoxSourceError::Auth), - Err(fleet_provider_proxmox::PveApiError::Forbidden { detail }) => { - Err(ProxmoxSourceError::Forbidden { detail }) - } - Err(fleet_provider_proxmox::PveApiError::Http { status, detail }) => { - Err(ProxmoxSourceError::Http { status, detail }) - } - Err(fleet_provider_proxmox::PveApiError::InvalidPayload { detail }) => { - Err(ProxmoxSourceError::InvalidPayload { detail }) - } - Err(fleet_provider_proxmox::PveApiError::Transport( - fleet_provider_proxmox::PveTransportError::FingerprintMismatch { observed, pinned }, - )) => { - // The discovery request always pins; a mismatch without a - // pin is a transport invariant violation, reported as such - // rather than papered over with a fabricated value. - let Some(pinned) = pinned else { - return Err(ProxmoxSourceError::Connect { - detail: format!( - "the transport reported a fingerprint mismatch without a pin (observed {observed})" - ), - }); - }; - Err(ProxmoxSourceError::FingerprintMismatch { observed, pinned }) - } - Err(fleet_provider_proxmox::PveApiError::Transport(other)) => { - Err(ProxmoxSourceError::Connect { - detail: other.to_string(), - }) - } + Err(error) => Err(map_api_error(error)), } } } diff --git a/crates/fleet-controller/tests/proxmox.rs b/crates/fleet-controller/tests/proxmox.rs index c30f2ea..02db30a 100644 --- a/crates/fleet-controller/tests/proxmox.rs +++ b/crates/fleet-controller/tests/proxmox.rs @@ -21,7 +21,7 @@ const VERSION_BODY: &str = const RESOURCES_BODY: &str = r#"{"data":[ {"id":"node/pve","type":"node","status":"online","maxcpu":16,"maxmem":67342831616}, {"id":"qemu/100","type":"qemu","node":"pve","vmid":100,"name":"dev-01","status":"running","template":0}, - {"id":"qemu/101","type":"qemu","node":"pve","vmid":101,"name":"fleet-test-01","status":"stopped","template":0}, + {"id":"qemu/101","type":"qemu","node":"pve","vmid":101,"name":"fleet-test-01","status":"running","template":0}, {"id":"qemu/900","type":"qemu","vmid":900,"template":1,"status":"stopped"}, {"id":"sdn/zone1","type":"sdn"} ]}"#; @@ -39,6 +39,16 @@ const AGENT_NETWORK_BODY: &str = r#"{"data":{"result":[ const AGENT_OSINFO_BODY: &str = r#"{"data":{"result":{"pretty-name":"Ubuntu 24.04.4 LTS","kernel-release":"6.8.0-138-generic"}}}"#; +/// Per-guest fixtures keyed by VMID: each guest carries its own MAC and +/// IP, so an association can only come from that guest's own evidence. +const GUEST_100_CONFIG_BODY: &str = + r#"{"data":{"name":"dev-01","net0":"virtio=DE:AD:BE:EF:00:09,bridge=vmbr0","memory":2048}}"#; + +const AGENT_100_NETWORK_BODY: &str = r#"{"data":{"result":[ + {"name":"ens18","hardware-address":"DE:AD:BE:EF:00:09","ip-addresses":[ + {"ip-address":"192.168.68.241","ip-address-type":"ipv4","prefix":24}]} +]}}"#; + /// The pinned fingerprint the fake transport accepts. const FP: &str = "DC2C116EC9C7EA618AA4E41EFB9BDEE4AA3D81EB16388F2B360AABE283A76498"; @@ -84,10 +94,17 @@ impl PveTransport for FixedTransport { (Some(_), Behavior::Normal) => { let body = if request.path.contains("/version") { VERSION_BODY + } else if request.path.contains("/qemu/100/config") { + GUEST_100_CONFIG_BODY } else if request.path.contains("/config") { GUEST_CONFIG_BODY } else if request.path.contains("/agent/info") { AGENT_INFO_BODY + } else if request + .path + .contains("/qemu/100/agent/network-get-interfaces") + { + AGENT_100_NETWORK_BODY } else if request.path.contains("/agent/network-get-interfaces") { AGENT_NETWORK_BODY } else if request.path.contains("/agent/get-osinfo") { @@ -499,6 +516,14 @@ async fn the_guest_surface_walks_list_and_observe() { assert_eq!(guest["agent"]["osName"], "Ubuntu 24.04.4 LTS"); assert_eq!(guest["candidates"][0]["machineId"], machine.id); assert_eq!(guest["candidates"][0]["kind"], "address_match"); + // The other guest carries distinct evidence and no candidate: the + // association provably comes from this guest's own facts. + let other = guests + .iter() + .find(|guest| guest["vmid"] == 100) + .expect("the other guest lists"); + assert_eq!(other["macs"][0], "de:ad:be:ef:00:09"); + assert_eq!(other["candidates"].as_array().unwrap().len(), 0); // Observe records the guest facts onto the machine. let (status, body) = harness diff --git a/crates/fleetctl/src/lib.rs b/crates/fleetctl/src/lib.rs index d7df36d..19e91a1 100644 --- a/crates/fleetctl/src/lib.rs +++ b/crates/fleetctl/src/lib.rs @@ -1214,7 +1214,7 @@ fn parse_proxmox_command(verb: &str, rest: &[&str]) -> Result fn usage() -> String { format!( - "Usage: fleetctl [--url ] [--socket ] [--output json|text] \n\nCommands:\n status\n system\n operations list [--limit ]\n operations get \n operations cancel \n machines list [--tag ] [--group ] [--capability ] [--status ] [--limit ]\n machines get \n machines onboard create --user --host [--port ] [--name ] [--description ] [--tag ]... [--group ]... --auth agent|identity-file [--identity ]\n machines onboard list [--limit ]\n machines onboard get \n machines onboard test [--wait] [--timeout ]\n machines onboard discover [--wait] [--timeout ]\n machines onboard confirm --fingerprint \n machines onboard add \n machines onboard cancel \n projects list [--remote-prefix

] [--name-substring ] [--limit ]\n projects get \n projects create --remote --name [--description ]\n projects update --name [--description ]\n projects delete \n projects discover --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects record (the discovery result is read from stdin)\n projects ready --root [--dry-run] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects clone --root [--branch ] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects pull --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects status --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects write-config --root --file --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (contents from stdin)\n skills probe --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--artifact-url --artifact-sha256 ] [--wait] [--timeout ]\n skills deploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n skills undeploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n frogenv status|setup|login|request|sync --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n frogenv run --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n mise inventory|status --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise install --tool --version --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise exec --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n apply --plan-id --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (the plan JSON is read from stdin)\n tailnet status\n tailnet configure --client-id (the client secret is read from stdin)\n tailnet clear\n tailnet devices [--limit ]\n tailnet import --user [--port ]\n proxmox accounts\n proxmox create --name --host [--port ] --token-id (the token secret is read from stdin)\n proxmox delete \n proxmox observe \n proxmox confirm --fingerprint \n proxmox discover \n machines install-node --endpoint --auth agent|identity-file [--identity ] [--artifact-url --artifact-sha256 ] [--controller-url ] [--install-timeout ] [--connect-timeout ] [--wait] [--timeout ]\n\n`status` prefers the node's local socket (default {DEFAULT_SOCKET}); `--url` is the explicit direct-controller override. Other commands talk to the controller, which defaults to {DEFAULT_URL}." + "Usage: fleetctl [--url ] [--socket ] [--output json|text] \n\nCommands:\n status\n system\n operations list [--limit ]\n operations get \n operations cancel \n machines list [--tag ] [--group ] [--capability ] [--status ] [--limit ]\n machines get \n machines onboard create --user --host [--port ] [--name ] [--description ] [--tag ]... [--group ]... --auth agent|identity-file [--identity ]\n machines onboard list [--limit ]\n machines onboard get \n machines onboard test [--wait] [--timeout ]\n machines onboard discover [--wait] [--timeout ]\n machines onboard confirm --fingerprint \n machines onboard add \n machines onboard cancel \n projects list [--remote-prefix

] [--name-substring ] [--limit ]\n projects get \n projects create --remote --name [--description ]\n projects update --name [--description ]\n projects delete \n projects discover --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects record (the discovery result is read from stdin)\n projects ready --root [--dry-run] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects clone --root [--branch ] --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects pull --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects status --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n projects write-config --root --file --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (contents from stdin)\n skills probe --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--artifact-url --artifact-sha256 ] [--wait] [--timeout ]\n skills deploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n skills undeploy --skill --agent ... --endpoint --auth agent|identity-file [--identity ] [--skills-root ] [--dry-run] [--wait] [--timeout ]\n frogenv status|setup|login|request|sync --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n frogenv run --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n mise inventory|status --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise install --tool --version --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ]\n mise exec --root --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] -- [args...]\n apply --plan-id --endpoint --auth agent|identity-file [--identity ] [--wait] [--timeout ] (the plan JSON is read from stdin)\n tailnet status\n tailnet configure --client-id (the client secret is read from stdin)\n tailnet clear\n tailnet devices [--limit ]\n tailnet import --user [--port ]\n proxmox accounts\n proxmox create --name --host [--port ] --token-id (the token secret is read from stdin)\n proxmox delete \n proxmox observe \n proxmox confirm --fingerprint \n proxmox discover \n proxmox guests \n proxmox observe-guest --machine \n machines install-node --endpoint --auth agent|identity-file [--identity ] [--artifact-url --artifact-sha256 ] [--controller-url ] [--install-timeout ] [--connect-timeout ] [--wait] [--timeout ]\n\n`status` prefers the node's local socket (default {DEFAULT_SOCKET}); `--url` is the explicit direct-controller override. Other commands talk to the controller, which defaults to {DEFAULT_URL}." ) } @@ -1688,7 +1688,9 @@ fn render(invocation: &Invocation, payload: &Value) -> String { | Command::ProxmoxDelete { .. } | Command::ProxmoxObserve { .. } | Command::ProxmoxConfirm { .. } - | Command::ProxmoxDiscover { .. } => render_proxmox(Some(payload)), + | Command::ProxmoxDiscover { .. } + | Command::ProxmoxGuests { .. } + | Command::ProxmoxObserveGuest { .. } => render_proxmox(Some(payload)), _ => render_text(Some(payload)), }, } @@ -2797,6 +2799,76 @@ pub fn render_proxmox_for_test(value: &Value) -> String { render_proxmox(Some(value)) } +/// Renders the Proxmox guests surface; exposed for contract tests. The +/// guests and accounts endpoints share the page envelope, so the renderer +/// is command-scoped rather than guessing from the payload. +#[doc(hidden)] +#[must_use] +pub fn render_proxmox_guests_for_test(value: &Value) -> String { + render_proxmox_guests(Some(value)) +} + +#[allow(clippy::too_many_lines)] +fn render_proxmox_guests(value: Option<&Value>) -> String { + let Some(value) = value else { + return String::new(); + }; + // Guests list: one row per guest with its agent state and candidates. + let Some(items) = value.get("items").and_then(Value::as_array) else { + return String::new(); + }; + let mut lines = vec![format!( + "{:<10} {:<10} {:<24} {:<12} {}", + "VMID", "KIND", "NAME", "AGENT", "FLEET CANDIDATES" + )]; + for guest in items { + let agent = match &guest["agent"] { + Value::Null => "-".to_owned(), + agent => { + if agent["online"] == true { + "online".to_owned() + } else { + "unreachable".to_owned() + } + } + }; + let candidates = guest["candidates"] + .as_array() + .map(|candidates| { + candidates + .iter() + .filter_map(|candidate| { + Some(format!( + "{} ({})", + candidate["machineName"].as_str()?, + candidate["kind"].as_str()? + )) + }) + .collect::>() + .join(", ") + }) + .unwrap_or_default(); + lines.push(format!( + "{:<10} {:<10} {:<24} {:<12} {}", + guest["vmid"] + .as_u64() + .map_or_else(|| "-".to_owned(), |v| v.to_string()), + guest["kind"].as_str().unwrap_or("-"), + guest["name"].as_str().unwrap_or("-"), + agent, + if candidates.is_empty() { + "-" + } else { + &candidates + } + )); + } + if items.is_empty() { + lines.push("(no guests reported)".to_owned()); + } + lines.join("\n") +} + #[allow(clippy::too_many_lines)] fn render_proxmox(value: Option<&Value>) -> String { let Some(value) = value else { diff --git a/crates/fleetctl/tests/cli.rs b/crates/fleetctl/tests/cli.rs index 0e241c4..d0a3545 100644 --- a/crates/fleetctl/tests/cli.rs +++ b/crates/fleetctl/tests/cli.rs @@ -2010,12 +2010,14 @@ fn text_output_renders_proxmox_guests() { ], "page": {"limit": 50, "nextCursor": null} }); - let text = fleetctl::render_proxmox_for_test(&guests); + let text = fleetctl::render_proxmox_guests_for_test(&guests); assert!(text.contains("VMID"), "{text}"); assert!(text.contains("fleet-test-01"), "{text}"); assert!(text.contains("online"), "{text}"); assert!(text.contains("box (address_match)"), "{text}"); assert!(!text.contains("(no guests reported)"), "{text}"); + // The LXC row's null agent renders "-", its empty candidates "-". + assert!(text.contains("container"), "{text}"); } #[test] @@ -2057,3 +2059,37 @@ fn parsing_walks_the_proxmox_guest_forms() { let error = fleetctl::parse(&args).unwrap_err(); assert!(error.message.contains("must be a number"), "{error}"); } + +#[test] +fn text_output_renders_the_guest_agent_states_honestly() { + let guests = json!({ + "items": [ + {"kind": "qemu", "id": "qemu/101", "vmid": 101, "name": "off-guest", + "status": "stopped", "macs": [], "agent": {"online": false}, + "warnings": [], "pveVersion": "9.2.2", "observedAt": 1, "candidates": []}, + {"kind": "lxc", "id": "lxc/200", "vmid": 200, "name": "container", + "status": "running", "macs": [], "agent": null, + "warnings": [], "pveVersion": "9.2.2", "observedAt": 1, "candidates": []} + ], + "page": {"limit": 50, "nextCursor": null} + }); + let text = fleetctl::render_proxmox_guests_for_test(&guests); + assert!(text.contains("unreachable"), "{text}"); + // The LXC row's agent column is the bare "-" marker. + let container_row = text + .lines() + .find(|line| line.contains("container")) + .expect("the container row"); + let columns: Vec<&str> = container_row.split_whitespace().collect(); + assert_eq!(columns[3], "-", "{container_row}"); + // Empty candidates render "-". + assert_eq!(columns[4], "-", "{container_row}"); +} + +#[test] +fn an_empty_guests_page_reports_no_guests_not_no_accounts() { + let empty = json!({"items": [], "page": {"limit": 50, "nextCursor": null}}); + let text = fleetctl::render_proxmox_guests_for_test(&empty); + assert!(text.contains("(no guests reported)"), "{text}"); + assert!(!text.contains("no Proxmox accounts"), "{text}"); +} diff --git a/crates/providers/fleet-provider-proxmox/src/lib.rs b/crates/providers/fleet-provider-proxmox/src/lib.rs index ccbaa02..55892b0 100644 --- a/crates/providers/fleet-provider-proxmox/src/lib.rs +++ b/crates/providers/fleet-provider-proxmox/src/lib.rs @@ -520,9 +520,10 @@ pub struct PveGuest { /// The MAC addresses from the guest config's `netN` entries, normalized /// lowercase colon-separated. LXC guests carry theirs in `config` too. pub macs: Vec, - /// The QEMU Guest Agent view; `None` for LXC (no qemu-guest-agent) or - /// when the config could not be read. Per-surface availability lives - /// inside. + /// The QEMU Guest Agent view; `None` only for LXC (no + /// qemu-guest-agent by design). For QEMU the agent is always probed: + /// per-surface availability lives inside, and a failed config read + /// warns separately without hiding the agent's own state. pub agent: Option, /// The bounded per-surface warnings: one failed agent call or a /// malformed config entry warns here instead of dropping the guest. @@ -765,7 +766,13 @@ impl ProxmoxSource for ProxmoxClient { }; match self.call(config_request.clone()).await { Ok(config) => { - guest.macs = config_macs(&config); + let (macs, config_warnings) = config_macs(&config); + guest.macs = macs; + for warning in config_warnings { + guest + .warnings + .push(format!("guest {}: {warning}", guest.resource.id)); + } } Err(PveApiError::Http { status, detail }) => { warnings.push(format!( @@ -799,26 +806,28 @@ impl ProxmoxSource for ProxmoxClient { /// The config's `netN` entries, parsed for MAC addresses. The value shape /// is `virtio=DE:AD:BE:EF:00:01,bridge=vmbr0` — the model is the first -/// key=value pair whose value looks like a MAC. -fn config_macs(config: &serde_json::Value) -> Vec { +/// key=value pair whose value looks like a MAC. A `netN` entry without a +/// valid MAC is a partial failure: it comes back as a warning, not silence. +fn config_macs(config: &serde_json::Value) -> (Vec, Vec) { let mut macs = Vec::new(); + let mut warnings = Vec::new(); if let Some(extra) = config.as_object() { for (key, value) in extra { if !(key.starts_with("net") && key[3..].chars().all(|c| c.is_ascii_digit())) { continue; } let Some(text) = value.as_str() else { + warnings.push(format!("the {key} entry is not a string")); continue; }; - for part in text.split(',') { - if let Some(mac) = normalize_mac(part) { - macs.push(mac); - break; - } + let found = text.split(',').find_map(normalize_mac); + match found { + Some(mac) => macs.push(mac), + None => warnings.push(format!("the {key} entry carries no parseable MAC address")), } } } - macs + (macs, warnings) } /// Normalizes a MAC candidate: `key=AA:BB:…` or bare, lowercase @@ -874,23 +883,19 @@ impl ProxmoxClient { path: format!("{base}/info"), ..request.clone() }; - if self.call(info_request.clone()).await.is_err() { + let Ok(info_data) = self.call(info_request).await else { warnings.push(format!("guest qemu/{vmid}: the agent is unreachable")); // Every other surface would fail the same way; report the // honest offline agent and stop here. return agent; - } - // The agent answered `info`; the version detail rides the same - // envelope. Re-calling is avoided by treating info's success as - // online and fetching the version from the same call's data. - if let Ok(data) = self.call(info_request).await { - let result = data.get("result").cloned().unwrap_or(data); - agent.online = true; - agent.version = result - .get("version") - .and_then(serde_json::Value::as_str) - .map(|value| value.chars().take(64).collect()); - } + }; + // The agent answers inside a `result` envelope. + let result = info_data.get("result").cloned().unwrap_or(info_data); + agent.online = true; + agent.version = result + .get("version") + .and_then(serde_json::Value::as_str) + .map(|value| value.chars().take(64).collect()); // network-get-interfaces let net_request = PveHttpRequest { path: format!("{base}/network-get-interfaces"), @@ -957,7 +962,10 @@ fn normalize_interface(interface: &serde_json::Value) -> Result Date: Sun, 20 Sep 2026 18:25:01 +0000 Subject: [PATCH 3/3] =?UTF-8?q?FM-601:=20second=20review=20round=20?= =?UTF-8?q?=E2=80=94=20stale=20cursor=20refuses=20with=20400,=20guests=20p?= =?UTF-8?q?agination=20documented=20in=20OpenAPI?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- crates/fleet-api/src/proxmox.rs | 30 ++++++++++++++++++---- packages/api-client/openapi.json | 20 +++++++++++++++ packages/api-client/src/generated/fleet.ts | 29 ++++++++++++++++++--- 3 files changed, 70 insertions(+), 9 deletions(-) diff --git a/crates/fleet-api/src/proxmox.rs b/crates/fleet-api/src/proxmox.rs index 8d4d4a1..5582f22 100644 --- a/crates/fleet-api/src/proxmox.rs +++ b/crates/fleet-api/src/proxmox.rs @@ -768,6 +768,16 @@ pub async fn discover_proxmox_cluster( Path, description = "The account's identity." ), + ( + "limit" = Option, + Query, + description = "The maximum number of guests to return." + ), + ( + "cursor" = Option, + Query, + description = "The opaque cursor: the last guest's cluster id of the previous page." + ), ), responses( ( @@ -830,11 +840,21 @@ pub async fn list_proxmox_guests( .unwrap_or(DEFAULT_PAGE_LIMIT) .min(MAX_PAGE_LIMIT); let start = match ¶ms.cursor { - Some(cursor) => snapshot - .guests - .iter() - .position(|guest| guest.guest.id == *cursor) - .map_or(0, |position| position + 1), + Some(cursor) => { + let Some(position) = snapshot + .guests + .iter() + .position(|guest| guest.guest.id == *cursor) + else { + // A stale or malformed cursor is a client error, not a + // silent restart from the first page. + return Err(crate::machines::invalid_request( + "the cursor names no guest in the snapshot", + correlation_id, + )); + }; + position + 1 + } None => 0, }; let page: Vec = snapshot diff --git a/packages/api-client/openapi.json b/packages/api-client/openapi.json index 198838b..ed66ab1 100644 --- a/packages/api-client/openapi.json +++ b/packages/api-client/openapi.json @@ -2224,6 +2224,26 @@ "schema": { "type": "string" } + }, + { + "name": "limit", + "in": "query", + "description": "The maximum number of guests to return.", + "required": false, + "schema": { + "type": "integer", + "format": "int32", + "minimum": 0 + } + }, + { + "name": "cursor", + "in": "query", + "description": "The opaque cursor: the last guest's cluster id of the previous page.", + "required": false, + "schema": { + "type": "string" + } } ], "responses": { diff --git a/packages/api-client/src/generated/fleet.ts b/packages/api-client/src/generated/fleet.ts index c71c398..c2504ce 100644 --- a/packages/api-client/src/generated/fleet.ts +++ b/packages/api-client/src/generated/fleet.ts @@ -2482,6 +2482,18 @@ limit?: number; cursor?: string; }; +export type ListProxmoxGuestsParams = { +/** + * The maximum number of guests to return. + * @minimum 0 + */ +limit?: number; +/** + * The opaque cursor: the last guest's cluster id of the previous page. + */ +cursor?: string; +}; + export type ListTailnetDevicesParams = { /** * The maximum number of devices to return. @@ -4886,12 +4898,20 @@ export type listProxmoxGuestsResponseError = (listProxmoxGuestsResponse400 | lis export type listProxmoxGuestsResponse = (listProxmoxGuestsResponseSuccess | listProxmoxGuestsResponseError) -export const getListProxmoxGuestsUrl = (accountId: string,) => { +export const getListProxmoxGuestsUrl = (accountId: string, + params?: ListProxmoxGuestsParams,) => { + const normalizedParams = new URLSearchParams(); + Object.entries(params || {}).forEach(([key, value]) => { + if (value !== undefined) { + normalizedParams.append(key, value === null ? 'null' : String(value)) + } + }); + const stringifiedParams = normalizedParams.toString(); - return `/api/v1/proxmox/accounts/${accountId}/guests` + return stringifiedParams.length > 0 ? `/api/v1/proxmox/accounts/${accountId}/guests?${stringifiedParams}` : `/api/v1/proxmox/accounts/${accountId}/guests` } /** @@ -4902,9 +4922,10 @@ export const getListProxmoxGuestsUrl = (accountId: string,) => { * @summary Lists the account's guests with their Fleet-machine association candidates (evidence only). */ -export const listProxmoxGuests = async (accountId: string, options?: RequestInit): Promise => { +export const listProxmoxGuests = async (accountId: string, + params?: ListProxmoxGuestsParams, options?: RequestInit): Promise => { - const res = await fetch(getListProxmoxGuestsUrl(accountId), + const res = await fetch(getListProxmoxGuestsUrl(accountId,params), { ...options, method: 'GET'