From d379a574a721038dbe23c72dd7c33e55c306a01f Mon Sep 17 00:00:00 2001 From: peytonr18 Date: Wed, 17 Jun 2026 14:39:57 -0700 Subject: [PATCH 1/4] feat(kvp): add ProvisioningReport abstraction over KvpPoolStore Add a ProvisioningReport that converts to ordered KVP entries via a new ToKvp trait and persists with write_report. ToKvp is the seam for a future diagnostics report to reuse write_report unchanged. --- libazureinit-kvp/Cargo.toml | 1 + libazureinit-kvp/src/lib.rs | 5 + libazureinit-kvp/src/report.rs | 370 +++++++++++++++++++++++++++++++++ 3 files changed, 376 insertions(+) create mode 100644 libazureinit-kvp/src/report.rs diff --git a/libazureinit-kvp/Cargo.toml b/libazureinit-kvp/Cargo.toml index 71943904..2e0ba442 100644 --- a/libazureinit-kvp/Cargo.toml +++ b/libazureinit-kvp/Cargo.toml @@ -9,6 +9,7 @@ license = "MIT" description = "Hyper-V KVP (Key-Value Pair) storage library for azure-init." [dependencies] +chrono = { version = "0.4", default-features = false, features = ["clock", "std"] } libc = "0.2" tracing = "0.1.40" diff --git a/libazureinit-kvp/src/lib.rs b/libazureinit-kvp/src/lib.rs index 4b85d644..52f391da 100644 --- a/libazureinit-kvp/src/lib.rs +++ b/libazureinit-kvp/src/lib.rs @@ -6,9 +6,14 @@ //! //! - [`KvpPoolStore`]: KVP pool file store with //! [`PoolMode`]-based policy. +//! - [`ProvisioningReport`]: structured provisioning health report that +//! converts into KVP entries via [`ToKvp`] and is persisted with +//! [`write_report`]. mod error; +mod report; mod store; pub use error::KvpError; +pub use report::{write_report, ProvisioningReport, ToKvp}; pub use store::{KvpPool, KvpPoolStore, PoolMode}; diff --git a/libazureinit-kvp/src/report.rs b/libazureinit-kvp/src/report.rs new file mode 100644 index 00000000..13433381 --- /dev/null +++ b/libazureinit-kvp/src/report.rs @@ -0,0 +1,370 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Structured provisioning report abstraction layered over the raw +//! [`KvpPoolStore`](crate::KvpPoolStore) key/value API. +//! +//! [`ProvisioningReport`] is a strongly-typed representation of a +//! provisioning health report. Instead of building ad-hoc key/value +//! strings at the call site, callers construct a report and convert it +//! into ordered KVP entries via [`ToKvp::to_kvp`], then persist it with +//! [`write_report`]. +//! +//! The [`ToKvp`] trait is the shared seam intended for future layering: +//! a diagnostics report can implement the same trait and reuse +//! [`write_report`] without changing this module. + +use chrono::Utc; + +use crate::{KvpError, KvpPoolStore}; + +/// Default value for the `pps_type` field when none is specified. +const DEFAULT_PPS_TYPE: &str = "None"; + +/// The current time formatted as an RFC 3339 string. +fn now_rfc3339() -> String { + Utc::now().to_rfc3339() +} +/// Conversion into ordered KVP entries. +/// +/// Implementors return key/value pairs in a deterministic order so the +/// resulting KVP records are stable and easy to assert against. This is +/// the shared seam that future report types (e.g. diagnostics) can +/// implement to reuse [`write_report`]. +pub trait ToKvp { + /// Return the report as ordered `(key, value)` pairs. + fn to_kvp(&self) -> Vec<(String, String)>; +} + +/// Outcome of a provisioning attempt. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +enum ReportResult { + /// Provisioning completed successfully. + Success, + /// Provisioning failed. + Error, +} + +impl ReportResult { + /// The wire string used in the `result` KVP field. + fn as_str(self) -> &'static str { + match self { + Self::Success => "success", + Self::Error => "error", + } + } +} + +impl std::fmt::Display for ReportResult { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + +/// A strongly-typed provisioning health report. +/// +/// Construct one with [`ProvisioningReport::success`] or +/// [`ProvisioningReport::error`], optionally attach extra context with +/// the builder methods, then convert to KVP entries with +/// [`ToKvp::to_kvp`] or persist with [`write_report`]. +/// +/// # Example +/// ```no_run +/// use libazureinit_kvp::{ +/// write_report, KvpPool, KvpPoolStore, PoolMode, ProvisioningReport, +/// }; +/// +/// # fn main() -> Result<(), libazureinit_kvp::KvpError> { +/// let store = KvpPoolStore::new(KvpPool::Guest, PoolMode::Safe)?; +/// +/// let report = ProvisioningReport::success( +/// format!("Azure-Init/{}", env!("CARGO_PKG_VERSION")), +/// "00000000-0000-0000-0000-000000000abc", +/// ) +/// .with_extra("build", "test-123"); +/// +/// write_report(&store, &report)?; +/// # Ok(()) +/// # } +/// ``` +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ProvisioningReport { + /// Provisioning outcome (`result` field). + result: ReportResult, + /// Reporting agent identifier (`agent` field). + agent: String, + /// Virtual machine identifier (`vm_id` field). + vm_id: String, + /// Report timestamp (`timestamp` field), set to the current time + /// (RFC 3339) when the report is constructed. + timestamp: String, + /// Pre-provisioning type (`pps_type` field). Defaults to `None`. + pps_type: String, + /// Failure reason (`reason` field). Present for error reports. + reason: Option, + /// Documentation URL (`documentation_url` field), if applicable. + documentation_url: Option, + /// Additional ordered key/value context (e.g. supporting data). + extra: Vec<(String, String)>, +} + +impl ProvisioningReport { + /// Create a successful provisioning report. + pub fn success(agent: impl Into, vm_id: impl Into) -> Self { + Self { + result: ReportResult::Success, + agent: agent.into(), + vm_id: vm_id.into(), + timestamp: now_rfc3339(), + pps_type: DEFAULT_PPS_TYPE.to_string(), + reason: None, + documentation_url: None, + extra: Vec::new(), + } + } + + /// Create a failed provisioning report with a failure reason. + pub fn error( + agent: impl Into, + vm_id: impl Into, + reason: impl Into, + ) -> Self { + Self { + result: ReportResult::Error, + agent: agent.into(), + vm_id: vm_id.into(), + timestamp: now_rfc3339(), + pps_type: DEFAULT_PPS_TYPE.to_string(), + reason: Some(reason.into()), + documentation_url: None, + extra: Vec::new(), + } + } + + /// Override the `pps_type` field (defaults to `None`). + pub fn with_pps_type(mut self, pps_type: impl Into) -> Self { + self.pps_type = pps_type.into(); + self + } + + /// Attach a documentation URL. + pub fn with_documentation_url(mut self, url: impl Into) -> Self { + self.documentation_url = Some(url.into()); + self + } + + /// Append an additional key/value pair. Extras are emitted in the + /// order they were added. + pub fn with_extra( + mut self, + key: impl Into, + value: impl Into, + ) -> Self { + self.extra.push((key.into(), value.into())); + self + } +} + +impl ToKvp for ProvisioningReport { + /// Emit entries in a deterministic order: + /// `result`, `reason` (if any), `agent`, extras (in order), + /// `pps_type`, `vm_id`, `timestamp`, `documentation_url` (if any). + fn to_kvp(&self) -> Vec<(String, String)> { + let mut entries = Vec::with_capacity(6 + self.extra.len()); + + entries.push(("result".to_string(), self.result.to_string())); + if let Some(reason) = &self.reason { + entries.push(("reason".to_string(), reason.clone())); + } + entries.push(("agent".to_string(), self.agent.clone())); + for (key, value) in &self.extra { + entries.push((key.clone(), value.clone())); + } + entries.push(("pps_type".to_string(), self.pps_type.clone())); + entries.push(("vm_id".to_string(), self.vm_id.clone())); + entries.push(("timestamp".to_string(), self.timestamp.clone())); + if let Some(url) = &self.documentation_url { + entries.push(("documentation_url".to_string(), url.clone())); + } + + entries + } +} + +/// Persist a report to the KVP store. +/// +/// Each entry from [`ToKvp::to_kvp`] is written with +/// [`KvpPoolStore::insert`] (upsert / last-write-wins), so re-emitting a +/// report collapses to a single record per key rather than accumulating +/// duplicates. The inserts are not transactional: an I/O error partway +/// through can leave entries written before the failure in the store. +pub fn write_report( + store: &KvpPoolStore, + report: &impl ToKvp, +) -> Result<(), KvpError> { + for (key, value) in report.to_kvp() { + store.insert(&key, &value)?; + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{KvpPool, PoolMode}; + use rstest::rstest; + use tempfile::TempDir; + + const VM_ID: &str = "00000000-0000-0000-0000-000000000abc"; + const AGENT: &str = "Azure-Init/0.0.0"; + const TS: &str = "2026-06-17T00:00:00+00:00"; + + fn safe_store(dir: &TempDir) -> KvpPoolStore { + KvpPoolStore::new_in(KvpPool::Guest, dir.path(), PoolMode::Safe) + .unwrap() + } + + /// Pin a report's timestamp so generated entries are deterministic. + fn with_ts(mut report: ProvisioningReport) -> ProvisioningReport { + report.timestamp = TS.to_string(); + report + } + + #[rstest] + #[case::success( + with_ts(ProvisioningReport::success(AGENT, VM_ID)), + vec![ + ("result", "success"), + ("agent", AGENT), + ("pps_type", "None"), + ("vm_id", VM_ID), + ("timestamp", TS), + ], + )] + #[case::success_with_extras( + with_ts( + ProvisioningReport::success(AGENT, VM_ID) + .with_extra("endpoint", "http://example.com") + .with_extra("status", "404"), + ), + vec![ + ("result", "success"), + ("agent", AGENT), + ("endpoint", "http://example.com"), + ("status", "404"), + ("pps_type", "None"), + ("vm_id", VM_ID), + ("timestamp", TS), + ], + )] + #[case::custom_pps_type( + with_ts( + ProvisioningReport::success(AGENT, VM_ID) + .with_pps_type("Savable"), + ), + vec![ + ("result", "success"), + ("agent", AGENT), + ("pps_type", "Savable"), + ("vm_id", VM_ID), + ("timestamp", TS), + ], + )] + #[case::error_with_documentation_url( + with_ts( + ProvisioningReport::error(AGENT, VM_ID, "failed to load sshd config") + .with_documentation_url("https://aka.ms/linuxprovisioningerror"), + ), + vec![ + ("result", "error"), + ("reason", "failed to load sshd config"), + ("agent", AGENT), + ("pps_type", "None"), + ("vm_id", VM_ID), + ("timestamp", TS), + ("documentation_url", "https://aka.ms/linuxprovisioningerror"), + ], + )] + #[case::error_without_documentation_url( + with_ts(ProvisioningReport::error(AGENT, VM_ID, "boom")), + vec![ + ("result", "error"), + ("reason", "boom"), + ("agent", AGENT), + ("pps_type", "None"), + ("vm_id", VM_ID), + ("timestamp", TS), + ], + )] + fn to_kvp_emits_expected_entries( + #[case] report: ProvisioningReport, + #[case] expected: Vec<(&str, &str)>, + ) { + let expected: Vec<(String, String)> = expected + .into_iter() + .map(|(k, v)| (k.to_string(), v.to_string())) + .collect(); + assert_eq!(report.to_kvp(), expected); + } + + #[test] + fn default_timestamp_is_populated() { + let report = ProvisioningReport::success(AGENT, VM_ID); + assert!(!report.timestamp.is_empty()); + } + + #[test] + fn write_report_round_trips_through_store() { + let dir = TempDir::new().unwrap(); + let store = safe_store(&dir); + + let report = with_ts( + ProvisioningReport::error(AGENT, VM_ID, "boom") + .with_extra("details", "bad config") + .with_documentation_url( + "https://aka.ms/linuxprovisioningerror", + ), + ); + + write_report(&store, &report).unwrap(); + + let entries = store.entries().unwrap(); + assert_eq!(entries.get("result").map(String::as_str), Some("error")); + assert_eq!(entries.get("reason").map(String::as_str), Some("boom")); + assert_eq!( + entries.get("details").map(String::as_str), + Some("bad config") + ); + assert_eq!(entries.get("vm_id").map(String::as_str), Some(VM_ID)); + assert_eq!(entries.get("timestamp").map(String::as_str), Some(TS)); + assert_eq!( + entries.get("documentation_url").map(String::as_str), + Some("https://aka.ms/linuxprovisioningerror") + ); + } + + #[test] + fn write_report_is_idempotent_upsert() { + let dir = TempDir::new().unwrap(); + let store = safe_store(&dir); + + let report = with_ts(ProvisioningReport::success(AGENT, VM_ID)); + write_report(&store, &report).unwrap(); + write_report(&store, &report).unwrap(); + + assert_eq!(store.len().unwrap(), report.to_kvp().len()); + } + + #[test] + fn write_report_propagates_store_error() { + let dir = TempDir::new().unwrap(); + let store = safe_store(&dir); + + let oversized = "x".repeat(store.max_value_size() + 1); + let report = ProvisioningReport::success(AGENT, VM_ID) + .with_extra("big", oversized); + + let result = write_report(&store, &report); + assert!(result.is_err()); + } +} From e9eade59acf9a21f9a6011d872a02d5e736c5d77 Mon Sep 17 00:00:00 2001 From: peytonr18 Date: Tue, 23 Jun 2026 14:06:54 -0700 Subject: [PATCH 2/4] fix(kvp): match legacy field order and add typed ReportPpsType Branch ProvisioningReport::encode per outcome so success extras trail the standard fields (failure layout unchanged), require a typed ReportPpsType on the constructors, rename error() -> failure(), and mark ReportPpsType non_exhaustive. Adds layout and pps_type wire-string tests. --- libazureinit-kvp/Cargo.toml | 1 + libazureinit-kvp/src/cli.rs | 214 +++++++++++++++++-- libazureinit-kvp/src/lib.rs | 6 +- libazureinit-kvp/src/report.rs | 371 ++++++++++++++++++++------------- 4 files changed, 433 insertions(+), 159 deletions(-) diff --git a/libazureinit-kvp/Cargo.toml b/libazureinit-kvp/Cargo.toml index 5f9e6e9a..06f9bc45 100644 --- a/libazureinit-kvp/Cargo.toml +++ b/libazureinit-kvp/Cargo.toml @@ -11,6 +11,7 @@ description = "Hyper-V KVP (Key-Value Pair) storage library for azure-init." [dependencies] chrono = { version = "0.4", default-features = false, features = ["clock", "std"] } clap = { version = "4.5.21", features = ["derive"] } +csv = "1" libc = "0.2" serde_json = "1.0.96" tracing = "0.1.40" diff --git a/libazureinit-kvp/src/cli.rs b/libazureinit-kvp/src/cli.rs index 72136676..b9d50b31 100644 --- a/libazureinit-kvp/src/cli.rs +++ b/libazureinit-kvp/src/cli.rs @@ -9,7 +9,10 @@ use std::process::ExitCode; use clap::{Parser, Subcommand, ValueEnum}; use serde_json::json; -use crate::{KvpError, KvpPool, KvpPoolStore, PoolMode}; +use crate::{ + write_report, KvpError, KvpPool, KvpPoolStore, PoolMode, + ProvisioningReport, ReportPpsType, +}; const EXIT_OK: u8 = 0; const EXIT_NOT_FOUND: u8 = 1; @@ -122,6 +125,35 @@ enum Command { }, /// Print whether the pool is stale (exit 0 if stale, 1 otherwise). IsStale, + /// Write a success provisioning health report, overriding any existing + /// `PROVISIONING_REPORT` record. + ReportSuccess { + /// Virtual machine identifier. + #[arg(long)] + vm_id: String, + /// Reporting agent identifier (e.g. Azure-Init/1.2.3). + #[arg(long)] + agent: String, + /// Optional human-readable message attached as an extra field. + #[arg(long)] + message: Option, + }, + /// Write a failure provisioning health report, overriding any existing + /// `PROVISIONING_REPORT` record. + ReportFailure { + /// Virtual machine identifier. + #[arg(long)] + vm_id: String, + /// Reporting agent identifier (e.g. Azure-Init/1.2.3). + #[arg(long)] + agent: String, + /// Failure reason. + #[arg(long)] + reason: String, + /// Optional documentation URL describing the failure. + #[arg(long)] + documentation_url: Option, + }, } #[derive(ValueEnum, Clone, Copy, Debug, PartialEq, Eq)] @@ -194,6 +226,17 @@ fn dispatch(cli: Cli, stdout: &mut W) -> Result { Ok(EXIT_OK) } Command::IsStale => is_stale(&store, stdout, output), + Command::ReportSuccess { + vm_id, + agent, + message, + } => report_success(&store, vm_id, agent, message), + Command::ReportFailure { + vm_id, + agent, + reason, + documentation_url, + } => report_failure(&store, vm_id, agent, reason, documentation_url), } } @@ -352,6 +395,37 @@ fn is_stale( Ok(if stale { EXIT_OK } else { EXIT_NOT_FOUND }) } +fn report_success( + store: &KvpPoolStore, + vm_id: String, + agent: String, + message: Option, +) -> Result { + let mut report = + ProvisioningReport::success(agent, vm_id, ReportPpsType::None); + if let Some(message) = message { + report = report.with_extra("message", message); + } + write_report(store, &report)?; + Ok(EXIT_OK) +} + +fn report_failure( + store: &KvpPoolStore, + vm_id: String, + agent: String, + reason: String, + documentation_url: Option, +) -> Result { + let mut report = + ProvisioningReport::failure(agent, vm_id, reason, ReportPpsType::None); + if let Some(url) = documentation_url { + report = report.with_documentation_url(url); + } + write_report(store, &report)?; + Ok(EXIT_OK) +} + fn writeln_json( stdout: &mut W, value: &serde_json::Value, @@ -510,12 +584,8 @@ mod tests { } fn store_at(dir: &TempDir) -> KvpPoolStore { - KvpPoolStore::new_in( - KvpPool::Guest, - dir.path().to_path_buf(), - PoolMode::Safe, - ) - .unwrap() + KvpPoolStore::new_in(KvpPool::Guest, dir.path(), PoolMode::Safe) + .unwrap() } fn run_dispatch(cli: Cli) -> (u8, String) { @@ -625,6 +695,125 @@ mod tests { )); } + #[test] + fn parse_report_success() { + let cli = Cli::parse_from([ + "libazureinit-kvp", + "report-success", + "--vm-id", + "vm-1", + "--agent", + "Azure-Init/0.0.0", + "--message", + "all good", + ]); + assert!(matches!( + cli.command, + Command::ReportSuccess { ref vm_id, ref agent, ref message } + if vm_id == "vm-1" + && agent == "Azure-Init/0.0.0" + && message.as_deref() == Some("all good") + )); + } + + #[test] + fn parse_report_failure() { + let cli = Cli::parse_from([ + "libazureinit-kvp", + "report-failure", + "--vm-id", + "vm-1", + "--agent", + "Azure-Init/0.0.0", + "--reason", + "boom", + "--documentation-url", + "https://aka.ms/x", + ]); + assert!(matches!( + cli.command, + Command::ReportFailure { + ref vm_id, + ref agent, + ref reason, + ref documentation_url, + } if vm_id == "vm-1" + && agent == "Azure-Init/0.0.0" + && reason == "boom" + && documentation_url.as_deref() == Some("https://aka.ms/x") + )); + } + + #[test] + fn dispatch_report_success_writes_single_record() { + let dir = TempDir::new().unwrap(); + let command = Command::ReportSuccess { + vm_id: "vm-1".into(), + agent: "Azure-Init/0.0.0".into(), + message: Some("hello".into()), + }; + let (code, _) = run_dispatch(cli(&dir, command)); + assert_eq!(code, EXIT_OK); + + let entries = store_at(&dir).entries().unwrap(); + assert_eq!(entries.len(), 1); + let value = entries.get("PROVISIONING_REPORT").unwrap(); + assert!(value.starts_with( + "result=success|agent=Azure-Init/0.0.0\ +|pps_type=None|vm_id=vm-1|timestamp=" + )); + assert!(value.ends_with("|message=hello")); + } + + #[test] + fn dispatch_report_failure_writes_single_record() { + let dir = TempDir::new().unwrap(); + let command = Command::ReportFailure { + vm_id: "vm-1".into(), + agent: "Azure-Init/0.0.0".into(), + reason: "boom".into(), + documentation_url: Some("https://aka.ms/x".into()), + }; + let (code, _) = run_dispatch(cli(&dir, command)); + assert_eq!(code, EXIT_OK); + + let entries = store_at(&dir).entries().unwrap(); + assert_eq!(entries.len(), 1); + let value = entries.get("PROVISIONING_REPORT").unwrap(); + assert!(value.starts_with( + "result=error|reason=boom|agent=Azure-Init/0.0.0\ +|pps_type=None|vm_id=vm-1|timestamp=" + )); + assert!(value.ends_with("|documentation_url=https://aka.ms/x")); + } + + #[test] + fn dispatch_report_overrides_previous_record() { + let dir = TempDir::new().unwrap(); + run_dispatch(cli( + &dir, + Command::ReportFailure { + vm_id: "vm-1".into(), + agent: "Azure-Init/0.0.0".into(), + reason: "boom".into(), + documentation_url: None, + }, + )); + run_dispatch(cli( + &dir, + Command::ReportSuccess { + vm_id: "vm-1".into(), + agent: "Azure-Init/0.0.0".into(), + message: None, + }, + )); + + let entries = store_at(&dir).entries().unwrap(); + assert_eq!(entries.len(), 1); + let value = entries.get("PROVISIONING_REPORT").unwrap(); + assert!(value.starts_with("result=success|")); + } + #[test] fn pool_arg_into_kvp_pool_covers_every_variant() { assert_eq!(KvpPool::from(PoolArg::External), KvpPool::External); @@ -966,15 +1155,11 @@ mod tests { assert_eq!(kvp_validation.to_string(), "KVP key must not be empty"); assert_eq!(kvp_validation.exit_code(), EXIT_USAGE_OR_VALIDATION); - let kvp_io = CliError::Kvp(KvpError::Io(io::Error::new( - io::ErrorKind::Other, - "kvp-io", - ))); + let kvp_io = CliError::Kvp(KvpError::Io(io::Error::other("kvp-io"))); assert!(kvp_io.to_string().contains("kvp-io")); assert_eq!(kvp_io.exit_code(), EXIT_IO); - let io_err = - CliError::Io(io::Error::new(io::ErrorKind::Other, "raw-io")); + let io_err = CliError::Io(io::Error::other("raw-io")); assert_eq!(io_err.to_string(), "raw-io"); assert_eq!(io_err.exit_code(), EXIT_IO); } @@ -984,8 +1169,7 @@ mod tests { let from_kvp: CliError = KvpError::EmptyKey.into(); assert!(matches!(from_kvp, CliError::Kvp(_))); - let from_io: CliError = - io::Error::new(io::ErrorKind::Other, "boom").into(); + let from_io: CliError = io::Error::other("boom").into(); assert!(matches!(from_io, CliError::Io(_))); } diff --git a/libazureinit-kvp/src/lib.rs b/libazureinit-kvp/src/lib.rs index 5d487c91..5f4d85e5 100644 --- a/libazureinit-kvp/src/lib.rs +++ b/libazureinit-kvp/src/lib.rs @@ -7,7 +7,7 @@ //! - [`KvpPoolStore`]: KVP pool file store with //! [`PoolMode`]-based policy. //! - [`ProvisioningReport`]: structured provisioning health report that -//! converts into KVP entries via [`ToKvp`] and is persisted with +//! is persisted as the single `PROVISIONING_REPORT` record with //! [`write_report`]. mod cli; @@ -17,5 +17,7 @@ mod store; pub use cli::run; pub use error::KvpError; -pub use report::{write_report, ProvisioningReport, ToKvp}; +pub use report::{ + write_report, ProvisioningReport, ReportPpsType, PROVISIONING_REPORT_KEY, +}; pub use store::{KvpPool, KvpPoolStore, PoolMode}; diff --git a/libazureinit-kvp/src/report.rs b/libazureinit-kvp/src/report.rs index 13433381..98274588 100644 --- a/libazureinit-kvp/src/report.rs +++ b/libazureinit-kvp/src/report.rs @@ -6,35 +6,26 @@ //! //! [`ProvisioningReport`] is a strongly-typed representation of a //! provisioning health report. Instead of building ad-hoc key/value -//! strings at the call site, callers construct a report and convert it -//! into ordered KVP entries via [`ToKvp::to_kvp`], then persist it with -//! [`write_report`]. -//! -//! The [`ToKvp`] trait is the shared seam intended for future layering: -//! a diagnostics report can implement the same trait and reuse -//! [`write_report`] without changing this module. +//! strings at the call site, callers construct a report and persist it +//! with [`write_report`], which serializes it into the single +//! pipe-delimited `PROVISIONING_REPORT` KVP record that the Azure/Hyper-V +//! host parses. use chrono::Utc; use crate::{KvpError, KvpPoolStore}; -/// Default value for the `pps_type` field when none is specified. -const DEFAULT_PPS_TYPE: &str = "None"; +/// KVP key under which the encoded provisioning health report is stored. +/// +/// The Azure/Hyper-V host parses this single key; its value is the +/// pipe-delimited `key=value|key=value|...` report produced by +/// [`write_report`]. +pub const PROVISIONING_REPORT_KEY: &str = "PROVISIONING_REPORT"; /// The current time formatted as an RFC 3339 string. fn now_rfc3339() -> String { Utc::now().to_rfc3339() } -/// Conversion into ordered KVP entries. -/// -/// Implementors return key/value pairs in a deterministic order so the -/// resulting KVP records are stable and easy to assert against. This is -/// the shared seam that future report types (e.g. diagnostics) can -/// implement to reuse [`write_report`]. -pub trait ToKvp { - /// Return the report as ordered `(key, value)` pairs. - fn to_kvp(&self) -> Vec<(String, String)>; -} /// Outcome of a provisioning attempt. #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -61,17 +52,56 @@ impl std::fmt::Display for ReportResult { } } +/// Pre-provisioning (PPS) type reported in the `pps_type` field. +/// +/// Mirrors the values cloud-init reports for the platform's +/// `PreprovisionedVMType` / IMDS `ppsType`. Marked `#[non_exhaustive]` +/// so new platform PPS types can be added without breaking downstream +/// `match` statements. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum ReportPpsType { + /// Not pre-provisioned (`None`). + None, + /// Pre-provisioned OS disk (`PreprovisionedOSDisk`). + OsDisk, + /// Running pre-provisioning (`Running`). + Running, + /// Savable pre-provisioning (`Savable`). + Savable, + /// Unknown pre-provisioning type (`Unknown`). + Unknown, +} + +impl ReportPpsType { + /// The wire string used in the `pps_type` KVP field. + fn as_str(self) -> &'static str { + match self { + Self::None => "None", + Self::OsDisk => "PreprovisionedOSDisk", + Self::Running => "Running", + Self::Savable => "Savable", + Self::Unknown => "Unknown", + } + } +} + +impl std::fmt::Display for ReportPpsType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str(self.as_str()) + } +} + /// A strongly-typed provisioning health report. /// /// Construct one with [`ProvisioningReport::success`] or -/// [`ProvisioningReport::error`], optionally attach extra context with -/// the builder methods, then convert to KVP entries with -/// [`ToKvp::to_kvp`] or persist with [`write_report`]. +/// [`ProvisioningReport::failure`], optionally attach extra context with +/// the builder methods, then persist it with [`write_report`]. /// /// # Example /// ```no_run /// use libazureinit_kvp::{ /// write_report, KvpPool, KvpPoolStore, PoolMode, ProvisioningReport, +/// ReportPpsType, /// }; /// /// # fn main() -> Result<(), libazureinit_kvp::KvpError> { @@ -80,6 +110,7 @@ impl std::fmt::Display for ReportResult { /// let report = ProvisioningReport::success( /// format!("Azure-Init/{}", env!("CARGO_PKG_VERSION")), /// "00000000-0000-0000-0000-000000000abc", +/// ReportPpsType::None, /// ) /// .with_extra("build", "test-123"); /// @@ -98,8 +129,8 @@ pub struct ProvisioningReport { /// Report timestamp (`timestamp` field), set to the current time /// (RFC 3339) when the report is constructed. timestamp: String, - /// Pre-provisioning type (`pps_type` field). Defaults to `None`. - pps_type: String, + /// Pre-provisioning type (`pps_type` field). + pps_type: ReportPpsType, /// Failure reason (`reason` field). Present for error reports. reason: Option, /// Documentation URL (`documentation_url` field), if applicable. @@ -110,13 +141,17 @@ pub struct ProvisioningReport { impl ProvisioningReport { /// Create a successful provisioning report. - pub fn success(agent: impl Into, vm_id: impl Into) -> Self { + pub fn success( + agent: impl Into, + vm_id: impl Into, + pps_type: ReportPpsType, + ) -> Self { Self { result: ReportResult::Success, agent: agent.into(), vm_id: vm_id.into(), timestamp: now_rfc3339(), - pps_type: DEFAULT_PPS_TYPE.to_string(), + pps_type, reason: None, documentation_url: None, extra: Vec::new(), @@ -124,29 +159,24 @@ impl ProvisioningReport { } /// Create a failed provisioning report with a failure reason. - pub fn error( + pub fn failure( agent: impl Into, vm_id: impl Into, reason: impl Into, + pps_type: ReportPpsType, ) -> Self { Self { result: ReportResult::Error, agent: agent.into(), vm_id: vm_id.into(), timestamp: now_rfc3339(), - pps_type: DEFAULT_PPS_TYPE.to_string(), + pps_type, reason: Some(reason.into()), documentation_url: None, extra: Vec::new(), } } - /// Override the `pps_type` field (defaults to `None`). - pub fn with_pps_type(mut self, pps_type: impl Into) -> Self { - self.pps_type = pps_type.into(); - self - } - /// Attach a documentation URL. pub fn with_documentation_url(mut self, url: impl Into) -> Self { self.documentation_url = Some(url.into()); @@ -165,47 +195,73 @@ impl ProvisioningReport { } } -impl ToKvp for ProvisioningReport { - /// Emit entries in a deterministic order: - /// `result`, `reason` (if any), `agent`, extras (in order), - /// `pps_type`, `vm_id`, `timestamp`, `documentation_url` (if any). - fn to_kvp(&self) -> Vec<(String, String)> { - let mut entries = Vec::with_capacity(6 + self.extra.len()); - - entries.push(("result".to_string(), self.result.to_string())); - if let Some(reason) = &self.reason { - entries.push(("reason".to_string(), reason.clone())); - } - entries.push(("agent".to_string(), self.agent.clone())); - for (key, value) in &self.extra { - entries.push((key.clone(), value.clone())); - } - entries.push(("pps_type".to_string(), self.pps_type.clone())); - entries.push(("vm_id".to_string(), self.vm_id.clone())); - entries.push(("timestamp".to_string(), self.timestamp.clone())); - if let Some(url) = &self.documentation_url { - entries.push(("documentation_url".to_string(), url.clone())); +impl ProvisioningReport { + /// Encode the report as a single pipe-delimited `key=value` string. + /// + /// - Success: `result`, `agent`, `pps_type`, `vm_id`, `timestamp`, + /// then any extras in insertion order. + /// - Failure: `result`, `reason`, `agent`, extras in insertion + /// order, `pps_type`, `vm_id`, `timestamp`, then + /// `documentation_url` (if any). + fn encode(&self) -> String { + let mut data = Vec::with_capacity(7 + self.extra.len()); + + data.push(format!("result={}", self.result)); + match self.result { + ReportResult::Success => { + data.push(format!("agent={}", self.agent)); + data.push(format!("pps_type={}", self.pps_type)); + data.push(format!("vm_id={}", self.vm_id)); + data.push(format!("timestamp={}", self.timestamp)); + for (key, value) in &self.extra { + data.push(format!("{key}={value}")); + } + } + ReportResult::Error => { + if let Some(reason) = &self.reason { + data.push(format!("reason={reason}")); + } + data.push(format!("agent={}", self.agent)); + for (key, value) in &self.extra { + data.push(format!("{key}={value}")); + } + data.push(format!("pps_type={}", self.pps_type)); + data.push(format!("vm_id={}", self.vm_id)); + data.push(format!("timestamp={}", self.timestamp)); + if let Some(url) = &self.documentation_url { + data.push(format!("documentation_url={url}")); + } + } } - entries + let mut writer = csv::WriterBuilder::new() + .delimiter(b'|') + .quote_style(csv::QuoteStyle::Necessary) + .from_writer(vec![]); + writer + .write_record(&data) + .expect("writing to an in-memory buffer cannot fail"); + let mut bytes = writer + .into_inner() + .expect("flushing an in-memory buffer cannot fail"); + if let Some(b'\n') = bytes.last() { + bytes.pop(); + } + String::from_utf8(bytes).expect("encoded report is valid UTF-8") } } -/// Persist a report to the KVP store. +/// Persist a report to the KVP store under [`PROVISIONING_REPORT_KEY`]. /// -/// Each entry from [`ToKvp::to_kvp`] is written with -/// [`KvpPoolStore::insert`] (upsert / last-write-wins), so re-emitting a -/// report collapses to a single record per key rather than accumulating -/// duplicates. The inserts are not transactional: an I/O error partway -/// through can leave entries written before the failure in the store. +/// The report is encoded into a single pipe-delimited value and written +/// with [`KvpPoolStore::insert`] (upsert / last-write-wins), so it +/// overrides any existing `PROVISIONING_REPORT` record rather than +/// accumulating duplicates. pub fn write_report( store: &KvpPoolStore, - report: &impl ToKvp, + report: &ProvisioningReport, ) -> Result<(), KvpError> { - for (key, value) in report.to_kvp() { - store.insert(&key, &value)?; - } - Ok(()) + store.insert(PROVISIONING_REPORT_KEY, &report.encode()) } #[cfg(test)] @@ -232,84 +288,112 @@ mod tests { #[rstest] #[case::success( - with_ts(ProvisioningReport::success(AGENT, VM_ID)), - vec![ - ("result", "success"), - ("agent", AGENT), - ("pps_type", "None"), - ("vm_id", VM_ID), - ("timestamp", TS), - ], + with_ts(ProvisioningReport::success(AGENT, VM_ID, ReportPpsType::None)), + "result=success|agent=Azure-Init/0.0.0|pps_type=None|vm_id=00000000-0000-0000-0000-000000000abc|timestamp=2026-06-17T00:00:00+00:00", )] #[case::success_with_extras( with_ts( - ProvisioningReport::success(AGENT, VM_ID) + ProvisioningReport::success(AGENT, VM_ID, ReportPpsType::None) .with_extra("endpoint", "http://example.com") .with_extra("status", "404"), ), - vec![ - ("result", "success"), - ("agent", AGENT), - ("endpoint", "http://example.com"), - ("status", "404"), - ("pps_type", "None"), - ("vm_id", VM_ID), - ("timestamp", TS), - ], + "result=success|agent=Azure-Init/0.0.0|pps_type=None|vm_id=00000000-0000-0000-0000-000000000abc|timestamp=2026-06-17T00:00:00+00:00|endpoint=http://example.com|status=404", )] #[case::custom_pps_type( - with_ts( - ProvisioningReport::success(AGENT, VM_ID) - .with_pps_type("Savable"), - ), - vec![ - ("result", "success"), - ("agent", AGENT), - ("pps_type", "Savable"), - ("vm_id", VM_ID), - ("timestamp", TS), - ], + with_ts(ProvisioningReport::success( + AGENT, + VM_ID, + ReportPpsType::Savable, + )), + "result=success|agent=Azure-Init/0.0.0|pps_type=Savable|vm_id=00000000-0000-0000-0000-000000000abc|timestamp=2026-06-17T00:00:00+00:00", )] #[case::error_with_documentation_url( with_ts( - ProvisioningReport::error(AGENT, VM_ID, "failed to load sshd config") - .with_documentation_url("https://aka.ms/linuxprovisioningerror"), + ProvisioningReport::failure( + AGENT, + VM_ID, + "failed to load sshd config", + ReportPpsType::None, + ) + .with_documentation_url("https://aka.ms/linuxprovisioningerror"), ), - vec![ - ("result", "error"), - ("reason", "failed to load sshd config"), - ("agent", AGENT), - ("pps_type", "None"), - ("vm_id", VM_ID), - ("timestamp", TS), - ("documentation_url", "https://aka.ms/linuxprovisioningerror"), - ], + "result=error|reason=failed to load sshd config|agent=Azure-Init/0.0.0|pps_type=None|vm_id=00000000-0000-0000-0000-000000000abc|timestamp=2026-06-17T00:00:00+00:00|documentation_url=https://aka.ms/linuxprovisioningerror", )] #[case::error_without_documentation_url( - with_ts(ProvisioningReport::error(AGENT, VM_ID, "boom")), - vec![ - ("result", "error"), - ("reason", "boom"), - ("agent", AGENT), - ("pps_type", "None"), - ("vm_id", VM_ID), - ("timestamp", TS), - ], + with_ts(ProvisioningReport::failure( + AGENT, + VM_ID, + "boom", + ReportPpsType::None, + )), + "result=error|reason=boom|agent=Azure-Init/0.0.0|pps_type=None|vm_id=00000000-0000-0000-0000-000000000abc|timestamp=2026-06-17T00:00:00+00:00", )] - fn to_kvp_emits_expected_entries( + fn encode_emits_expected_pipe_string( #[case] report: ProvisioningReport, - #[case] expected: Vec<(&str, &str)>, + #[case] expected: &str, ) { - let expected: Vec<(String, String)> = expected - .into_iter() - .map(|(k, v)| (k.to_string(), v.to_string())) - .collect(); - assert_eq!(report.to_kvp(), expected); + assert_eq!(report.encode(), expected); + } + + /// Pins each [`ReportPpsType`] variant to its exact wire string. + #[rstest] + #[case(ReportPpsType::None, "None")] + #[case(ReportPpsType::OsDisk, "PreprovisionedOSDisk")] + #[case(ReportPpsType::Running, "Running")] + #[case(ReportPpsType::Savable, "Savable")] + #[case(ReportPpsType::Unknown, "Unknown")] + fn pps_type_display_matches_wire_string( + #[case] pps_type: ReportPpsType, + #[case] expected: &str, + ) { + assert_eq!(pps_type.to_string(), expected); + } + + /// The success layout lists the standard fields first, then any + /// extras appended at the very end. + #[test] + fn success_layout_appends_extras_last() { + let report = with_ts( + ProvisioningReport::success(AGENT, VM_ID, ReportPpsType::None) + .with_extra("build", "test-123"), + ); + assert_eq!( + report.encode(), + "result=success|agent=Azure-Init/0.0.0|pps_type=None\ +|vm_id=00000000-0000-0000-0000-000000000abc\ +|timestamp=2026-06-17T00:00:00+00:00|build=test-123" + ); + } + + /// The failure layout lists reason and agent first, extras + /// (supporting data) before the standard fields, and + /// `documentation_url` last. + #[test] + fn failure_layout_places_extras_before_pps_type() { + let report = with_ts( + ProvisioningReport::failure( + AGENT, + VM_ID, + "boom", + ReportPpsType::None, + ) + .with_extra("details", "bad config") + .with_documentation_url("https://aka.ms/linuxprovisioningerror"), + ); + assert_eq!( + report.encode(), + "result=error|reason=boom|agent=Azure-Init/0.0.0\ +|details=bad config|pps_type=None\ +|vm_id=00000000-0000-0000-0000-000000000abc\ +|timestamp=2026-06-17T00:00:00+00:00\ +|documentation_url=https://aka.ms/linuxprovisioningerror" + ); } #[test] fn default_timestamp_is_populated() { - let report = ProvisioningReport::success(AGENT, VM_ID); + let report = + ProvisioningReport::success(AGENT, VM_ID, ReportPpsType::None); assert!(!report.timestamp.is_empty()); } @@ -319,27 +403,25 @@ mod tests { let store = safe_store(&dir); let report = with_ts( - ProvisioningReport::error(AGENT, VM_ID, "boom") - .with_extra("details", "bad config") - .with_documentation_url( - "https://aka.ms/linuxprovisioningerror", - ), + ProvisioningReport::failure( + AGENT, + VM_ID, + "boom", + ReportPpsType::None, + ) + .with_extra("details", "bad config") + .with_documentation_url("https://aka.ms/linuxprovisioningerror"), ); write_report(&store, &report).unwrap(); let entries = store.entries().unwrap(); - assert_eq!(entries.get("result").map(String::as_str), Some("error")); - assert_eq!(entries.get("reason").map(String::as_str), Some("boom")); - assert_eq!( - entries.get("details").map(String::as_str), - Some("bad config") - ); - assert_eq!(entries.get("vm_id").map(String::as_str), Some(VM_ID)); - assert_eq!(entries.get("timestamp").map(String::as_str), Some(TS)); + assert_eq!(entries.len(), 1); assert_eq!( - entries.get("documentation_url").map(String::as_str), - Some("https://aka.ms/linuxprovisioningerror") + entries.get(PROVISIONING_REPORT_KEY).map(String::as_str), + Some( + "result=error|reason=boom|agent=Azure-Init/0.0.0|details=bad config|pps_type=None|vm_id=00000000-0000-0000-0000-000000000abc|timestamp=2026-06-17T00:00:00+00:00|documentation_url=https://aka.ms/linuxprovisioningerror" + ) ); } @@ -348,11 +430,15 @@ mod tests { let dir = TempDir::new().unwrap(); let store = safe_store(&dir); - let report = with_ts(ProvisioningReport::success(AGENT, VM_ID)); + let report = with_ts(ProvisioningReport::success( + AGENT, + VM_ID, + ReportPpsType::None, + )); write_report(&store, &report).unwrap(); write_report(&store, &report).unwrap(); - assert_eq!(store.len().unwrap(), report.to_kvp().len()); + assert_eq!(store.len().unwrap(), 1); } #[test] @@ -361,8 +447,9 @@ mod tests { let store = safe_store(&dir); let oversized = "x".repeat(store.max_value_size() + 1); - let report = ProvisioningReport::success(AGENT, VM_ID) - .with_extra("big", oversized); + let report = + ProvisioningReport::success(AGENT, VM_ID, ReportPpsType::None) + .with_extra("big", oversized); let result = write_report(&store, &report); assert!(result.is_err()); From df8e3dc6d7d6fc8cb5392d5261cc8ef5974e1a83 Mon Sep 17 00:00:00 2001 From: peytonr18 Date: Tue, 7 Jul 2026 12:42:51 -0700 Subject: [PATCH 3/4] feat(kvp): make report vm-id optional, default agent, add --supporting-data --- libazureinit-kvp/Cargo.toml | 1 + libazureinit-kvp/src/cli.rs | 185 ++++++++++++++++++++++++----- libazureinit-kvp/src/lib.rs | 1 + libazureinit-kvp/src/vm_id.rs | 218 ++++++++++++++++++++++++++++++++++ libazureinit-kvp/tests/cli.rs | 44 +++++++ 5 files changed, 418 insertions(+), 31 deletions(-) create mode 100644 libazureinit-kvp/src/vm_id.rs diff --git a/libazureinit-kvp/Cargo.toml b/libazureinit-kvp/Cargo.toml index 06f9bc45..d58f862d 100644 --- a/libazureinit-kvp/Cargo.toml +++ b/libazureinit-kvp/Cargo.toml @@ -15,6 +15,7 @@ csv = "1" libc = "0.2" serde_json = "1.0.96" tracing = "0.1.40" +uuid = "1.3" [dev-dependencies] rstest = { version = "0.26", default-features = false } diff --git a/libazureinit-kvp/src/cli.rs b/libazureinit-kvp/src/cli.rs index b9d50b31..be816a80 100644 --- a/libazureinit-kvp/src/cli.rs +++ b/libazureinit-kvp/src/cli.rs @@ -19,6 +19,10 @@ const EXIT_NOT_FOUND: u8 = 1; const EXIT_USAGE_OR_VALIDATION: u8 = 2; const EXIT_IO: u8 = 3; +/// Default reporting agent identifier, derived from this crate's version +const DEFAULT_AGENT: &str = + concat!("libazureinit-kvp/", env!("CARGO_PKG_VERSION")); + /// Entry point for the `libazureinit-kvp` binary. pub fn run() -> ExitCode { let cli = Cli::parse(); @@ -130,22 +134,23 @@ enum Command { ReportSuccess { /// Virtual machine identifier. #[arg(long)] - vm_id: String, - /// Reporting agent identifier (e.g. Azure-Init/1.2.3). - #[arg(long)] + vm_id: Option, + /// Reporting agent identifier (e.g. libazureinit-kvp/0.1.0). + #[arg(long, default_value = DEFAULT_AGENT)] agent: String, - /// Optional human-readable message attached as an extra field. - #[arg(long)] - message: Option, + /// Additional key=value supporting data, comma-separated or the + /// flag repeated (e.g. --supporting-data k1=v1,k2=v2). + #[arg(long, value_delimiter = ',', value_parser = parse_key_value_pair)] + supporting_data: Vec<(String, String)>, }, /// Write a failure provisioning health report, overriding any existing /// `PROVISIONING_REPORT` record. ReportFailure { - /// Virtual machine identifier. - #[arg(long)] - vm_id: String, - /// Reporting agent identifier (e.g. Azure-Init/1.2.3). + /// Virtual machine identifier ID. #[arg(long)] + vm_id: Option, + /// Reporting agent identifier (e.g. libazureinit-kvp/0.1.0). + #[arg(long, default_value = DEFAULT_AGENT)] agent: String, /// Failure reason. #[arg(long)] @@ -153,6 +158,10 @@ enum Command { /// Optional documentation URL describing the failure. #[arg(long)] documentation_url: Option, + /// Additional key=value supporting data, comma-separated or the + /// flag repeated (e.g. --supporting-data k1=v1,k2=v2). + #[arg(long, value_delimiter = ',', value_parser = parse_key_value_pair)] + supporting_data: Vec<(String, String)>, }, } @@ -229,14 +238,22 @@ fn dispatch(cli: Cli, stdout: &mut W) -> Result { Command::ReportSuccess { vm_id, agent, - message, - } => report_success(&store, vm_id, agent, message), + supporting_data, + } => report_success(&store, vm_id, agent, supporting_data), Command::ReportFailure { vm_id, agent, reason, documentation_url, - } => report_failure(&store, vm_id, agent, reason, documentation_url), + supporting_data, + } => report_failure( + &store, + vm_id, + agent, + reason, + documentation_url, + supporting_data, + ), } } @@ -397,14 +414,15 @@ fn is_stale( fn report_success( store: &KvpPoolStore, - vm_id: String, + vm_id: Option, agent: String, - message: Option, + supporting_data: Vec<(String, String)>, ) -> Result { + let vm_id = resolve_vm_id(vm_id)?; let mut report = ProvisioningReport::success(agent, vm_id, ReportPpsType::None); - if let Some(message) = message { - report = report.with_extra("message", message); + for (key, value) in supporting_data { + report = report.with_extra(key, value); } write_report(store, &report)?; Ok(EXIT_OK) @@ -412,13 +430,18 @@ fn report_success( fn report_failure( store: &KvpPoolStore, - vm_id: String, + vm_id: Option, agent: String, reason: String, documentation_url: Option, + supporting_data: Vec<(String, String)>, ) -> Result { + let vm_id = resolve_vm_id(vm_id)?; let mut report = ProvisioningReport::failure(agent, vm_id, reason, ReportPpsType::None); + for (key, value) in supporting_data { + report = report.with_extra(key, value); + } if let Some(url) = documentation_url { report = report.with_documentation_url(url); } @@ -426,6 +449,32 @@ fn report_failure( Ok(EXIT_OK) } +/// Resolve the VM ID for a report, falling back to the current VM's ID when +/// `--vm-id` was not supplied. +fn resolve_vm_id(vm_id: Option) -> Result { + match vm_id { + Some(vm_id) => Ok(vm_id), + None => crate::vm_id::get_vm_id().ok_or_else(|| { + CliError::Usage( + "unable to determine the current VM ID automatically; \ + pass --vm-id explicitly" + .to_string(), + ) + }), + } +} + +/// Parse a single `key=value` supporting-data pair. +fn parse_key_value_pair(raw: &str) -> Result<(String, String), String> { + let (key, value) = raw.split_once('=').ok_or_else(|| { + format!("supporting data '{raw}' must be in key=value format") + })?; + if key.is_empty() { + return Err(format!("supporting data '{raw}' has an empty key")); + } + Ok((key.to_string(), value.to_string())) +} + fn writeln_json( stdout: &mut W, value: &serde_json::Value, @@ -704,15 +753,30 @@ mod tests { "vm-1", "--agent", "Azure-Init/0.0.0", - "--message", - "all good", + "--supporting-data", + "k1=v1,k2=v2", ]); assert!(matches!( cli.command, - Command::ReportSuccess { ref vm_id, ref agent, ref message } - if vm_id == "vm-1" + Command::ReportSuccess { ref vm_id, ref agent, ref supporting_data } + if vm_id.as_deref() == Some("vm-1") && agent == "Azure-Init/0.0.0" - && message.as_deref() == Some("all good") + && supporting_data == &vec![ + ("k1".to_string(), "v1".to_string()), + ("k2".to_string(), "v2".to_string()), + ] + )); + } + + #[test] + fn parse_report_success_defaults_agent_and_vm_id() { + let cli = Cli::parse_from(["libazureinit-kvp", "report-success"]); + assert!(matches!( + cli.command, + Command::ReportSuccess { ref vm_id, ref agent, ref supporting_data } + if vm_id.is_none() + && agent == DEFAULT_AGENT + && supporting_data.is_empty() )); } @@ -729,6 +793,8 @@ mod tests { "boom", "--documentation-url", "https://aka.ms/x", + "--supporting-data", + "details=bad config", ]); assert!(matches!( cli.command, @@ -737,20 +803,38 @@ mod tests { ref agent, ref reason, ref documentation_url, - } if vm_id == "vm-1" + ref supporting_data, + } if vm_id.as_deref() == Some("vm-1") && agent == "Azure-Init/0.0.0" && reason == "boom" && documentation_url.as_deref() == Some("https://aka.ms/x") + && supporting_data == &vec![ + ("details".to_string(), "bad config".to_string()), + ] )); } + #[test] + fn parse_report_rejects_supporting_data_without_equals() { + let result = Cli::try_parse_from([ + "libazureinit-kvp", + "report-success", + "--supporting-data", + "novalue", + ]); + assert!(result.is_err()); + } + #[test] fn dispatch_report_success_writes_single_record() { let dir = TempDir::new().unwrap(); let command = Command::ReportSuccess { - vm_id: "vm-1".into(), + vm_id: Some("vm-1".into()), agent: "Azure-Init/0.0.0".into(), - message: Some("hello".into()), + supporting_data: vec![ + ("endpoint".into(), "http://example.com".into()), + ("status".into(), "404".into()), + ], }; let (code, _) = run_dispatch(cli(&dir, command)); assert_eq!(code, EXIT_OK); @@ -762,17 +846,34 @@ mod tests { "result=success|agent=Azure-Init/0.0.0\ |pps_type=None|vm_id=vm-1|timestamp=" )); - assert!(value.ends_with("|message=hello")); + assert!(value.ends_with("|endpoint=http://example.com|status=404")); + } + + #[test] + fn dispatch_report_success_defaults_agent() { + let dir = TempDir::new().unwrap(); + let command = Command::ReportSuccess { + vm_id: Some("vm-1".into()), + agent: DEFAULT_AGENT.into(), + supporting_data: Vec::new(), + }; + let (code, _) = run_dispatch(cli(&dir, command)); + assert_eq!(code, EXIT_OK); + + let entries = store_at(&dir).entries().unwrap(); + let value = entries.get("PROVISIONING_REPORT").unwrap(); + assert!(value.contains(&format!("agent={DEFAULT_AGENT}"))); } #[test] fn dispatch_report_failure_writes_single_record() { let dir = TempDir::new().unwrap(); let command = Command::ReportFailure { - vm_id: "vm-1".into(), + vm_id: Some("vm-1".into()), agent: "Azure-Init/0.0.0".into(), reason: "boom".into(), documentation_url: Some("https://aka.ms/x".into()), + supporting_data: Vec::new(), }; let (code, _) = run_dispatch(cli(&dir, command)); assert_eq!(code, EXIT_OK); @@ -787,24 +888,46 @@ mod tests { assert!(value.ends_with("|documentation_url=https://aka.ms/x")); } + #[test] + fn dispatch_report_failure_places_supporting_data_before_pps_type() { + let dir = TempDir::new().unwrap(); + let command = Command::ReportFailure { + vm_id: Some("vm-1".into()), + agent: "Azure-Init/0.0.0".into(), + reason: "boom".into(), + documentation_url: None, + supporting_data: vec![("details".into(), "bad config".into())], + }; + let (code, _) = run_dispatch(cli(&dir, command)); + assert_eq!(code, EXIT_OK); + + let entries = store_at(&dir).entries().unwrap(); + let value = entries.get("PROVISIONING_REPORT").unwrap(); + assert!(value.starts_with( + "result=error|reason=boom|agent=Azure-Init/0.0.0\ +|details=bad config|pps_type=None|vm_id=vm-1|timestamp=" + )); + } + #[test] fn dispatch_report_overrides_previous_record() { let dir = TempDir::new().unwrap(); run_dispatch(cli( &dir, Command::ReportFailure { - vm_id: "vm-1".into(), + vm_id: Some("vm-1".into()), agent: "Azure-Init/0.0.0".into(), reason: "boom".into(), documentation_url: None, + supporting_data: Vec::new(), }, )); run_dispatch(cli( &dir, Command::ReportSuccess { - vm_id: "vm-1".into(), + vm_id: Some("vm-1".into()), agent: "Azure-Init/0.0.0".into(), - message: None, + supporting_data: Vec::new(), }, )); diff --git a/libazureinit-kvp/src/lib.rs b/libazureinit-kvp/src/lib.rs index 5f4d85e5..f17b66f7 100644 --- a/libazureinit-kvp/src/lib.rs +++ b/libazureinit-kvp/src/lib.rs @@ -14,6 +14,7 @@ mod cli; mod error; mod report; mod store; +mod vm_id; pub use cli::run; pub use error::KvpError; diff --git a/libazureinit-kvp/src/vm_id.rs b/libazureinit-kvp/src/vm_id.rs new file mode 100644 index 00000000..a8c1edbd --- /dev/null +++ b/libazureinit-kvp/src/vm_id.rs @@ -0,0 +1,218 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Self-contained VM ID lookup used to auto-populate provisioning reports. +//! +//! The VM ID is read from `/sys/class/dmi/id/product_uuid` and, on Gen1 VMs, +//! the first three UUID fields are byte-swapped from big-endian to native endianness. + +use std::fs; +use std::path::Path; + +use uuid::Uuid; + +/// Retrieves the current VM ID by reading `/sys/class/dmi/id/product_uuid` +/// and byte-swapping the result if the VM is Gen1. +/// +/// # Returns +/// - `Some(String)` containing the VM ID if retrieval is successful. +/// - `None` if the file is missing, empty, or cannot be read. +pub fn get_vm_id() -> Option { + private_get_vm_id(None, None, None) +} + +fn private_get_vm_id( + product_uuid_path: Option<&str>, + sysfs_efi_path: Option<&str>, + dev_efi_path: Option<&str>, +) -> Option { + let path = product_uuid_path.unwrap_or("/sys/class/dmi/id/product_uuid"); + + let system_uuid = match fs::read_to_string(path) { + Ok(s) => s.trim().to_lowercase(), + Err(err) => { + tracing::error!("Failed to read VM ID from {}: {}", path, err); + return None; + } + }; + + if system_uuid.is_empty() { + tracing::info!("VM ID file is empty at path: {}", path); + return None; + } + + if is_vm_gen1(sysfs_efi_path, dev_efi_path) { + match Uuid::parse_str(&system_uuid) { + Ok(uuid_parsed) => { + let swapped_uuid = + swap_uuid_to_little_endian(*uuid_parsed.as_bytes()); + Some(swapped_uuid.to_string()) + } + Err(err) => { + tracing::error!("invalid VM ID UUID '{system_uuid}': {err}"); + Some(system_uuid) + } + } + } else { + Some(system_uuid) + } +} + +/// Determines whether the VM is Gen1 (i.e. not UEFI/Gen2) based on EFI +/// detection. Returns `true` when neither EFI path exists. +fn is_vm_gen1( + sysfs_efi_path: Option<&str>, + dev_efi_path: Option<&str>, +) -> bool { + let sysfs_efi = sysfs_efi_path.unwrap_or("/sys/firmware/efi"); + let dev_efi = dev_efi_path.unwrap_or("/dev/efi"); + + // If *either* efi path exists, this is Gen2; if *neither* exist, Gen1. + !Path::new(sysfs_efi).exists() && !Path::new(dev_efi).exists() +} + +/// Converts the first three fields of a 16-byte array from big-endian to +/// the native endianness, then returns it as a `Uuid`. +fn swap_uuid_to_little_endian(mut bytes: [u8; 16]) -> Uuid { + let (d1, remainder) = bytes.split_at(std::mem::size_of::()); + let d1 = d1 + .try_into() + .map(u32::from_be_bytes) + .unwrap_or(0) + .to_ne_bytes(); + + let (d2, remainder) = remainder.split_at(std::mem::size_of::()); + let d2 = d2 + .try_into() + .map(u16::from_be_bytes) + .unwrap_or(0) + .to_ne_bytes(); + + let (d3, _) = remainder.split_at(std::mem::size_of::()); + let d3 = d3 + .try_into() + .map(u16::from_be_bytes) + .unwrap_or(0) + .to_ne_bytes(); + + let native_endian = d1.into_iter().chain(d2).chain(d3).collect::>(); + debug_assert_eq!(native_endian.len(), 8); + bytes[..native_endian.len()].copy_from_slice(&native_endian); + Uuid::from_bytes(bytes) +} + +#[cfg(test)] +mod tests { + use super::*; + use tempfile::TempDir; + + #[test] + fn gen1_when_no_efi_paths_exist() { + assert!(is_vm_gen1( + Some("/nonexistent_sysfs_efi"), + Some("/nonexistent_dev_efi") + )); + } + + #[test] + fn gen2_when_sysfs_efi_exists() { + let dir = TempDir::new().unwrap(); + let efi = dir.path().join("efi"); + fs::create_dir(&efi).unwrap(); + assert!(!is_vm_gen1( + Some(efi.to_str().unwrap()), + Some("/nonexistent_dev_efi") + )); + } + + #[test] + fn gen2_when_dev_efi_exists() { + let dir = TempDir::new().unwrap(); + let efi = dir.path().join("efi"); + fs::create_dir(&efi).unwrap(); + assert!(!is_vm_gen1( + Some("/nonexistent_sysfs_efi"), + Some(efi.to_str().unwrap()) + )); + } + + #[test] + fn reads_and_swaps_for_gen1() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("product_uuid"); + fs::write(&path, "550e8400-e29b-41d4-a716-446655440000").unwrap(); + + let actual = private_get_vm_id( + Some(path.to_str().unwrap()), + Some("/nonexistent_sysfs_efi"), + Some("/nonexistent_dev_efi"), + ) + .unwrap(); + + assert_eq!(actual, "00840e55-9be2-d441-a716-446655440000"); + } + + #[test] + fn reads_without_swap_for_gen2() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("product_uuid"); + fs::write(&path, "550E8400-E29B-41D4-A716-446655440000").unwrap(); + let efi = dir.path().join("efi"); + fs::create_dir(&efi).unwrap(); + + let actual = private_get_vm_id( + Some(path.to_str().unwrap()), + Some(efi.to_str().unwrap()), + Some("/nonexistent_dev_efi"), + ) + .unwrap(); + + assert_eq!(actual, "550e8400-e29b-41d4-a716-446655440000"); + } + + #[test] + fn returns_raw_value_when_gen1_uuid_is_unparseable() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("product_uuid"); + fs::write(&path, "not-a-uuid").unwrap(); + + // Gen1 (no EFI paths) but the content cannot be parsed as a UUID, + // so the raw lowercased value is returned unchanged. + let actual = private_get_vm_id( + Some(path.to_str().unwrap()), + Some("/nonexistent_sysfs_efi"), + Some("/nonexistent_dev_efi"), + ) + .unwrap(); + + assert_eq!(actual, "not-a-uuid"); + } + + #[test] + fn get_vm_id_public_wrapper_is_callable() { + // Exercises the public entry point. It reads the host's + // product_uuid if present, so the result is environment dependent; + // we only assert that invoking it does not panic. + let _ = get_vm_id(); + } + + #[test] + fn returns_none_for_empty_file() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("product_uuid"); + fs::write(&path, "\n").unwrap(); + + assert!(private_get_vm_id(Some(path.to_str().unwrap()), None, None) + .is_none()); + } + + #[test] + fn returns_none_for_missing_file() { + assert!(private_get_vm_id( + Some("/this/path/does/not/exist"), + None, + None + ) + .is_none()); + } +} diff --git a/libazureinit-kvp/tests/cli.rs b/libazureinit-kvp/tests/cli.rs index 046f55cd..ef485314 100644 --- a/libazureinit-kvp/tests/cli.rs +++ b/libazureinit-kvp/tests/cli.rs @@ -254,3 +254,47 @@ fn json_read_round_trips_value_with_equals_and_newline() { assert_eq!(value["key"], "url"); assert_eq!(value["value"], raw_value); } + +#[test] +fn report_success_defaults_agent_and_accepts_supporting_data() { + let dir = TempDir::new().unwrap(); + assert_success(kvp(&with_dir( + &dir, + &[ + "report-success", + "--vm-id", + "vm-1", + "--supporting-data", + "build=123,commit=abc", + ], + ))); + + let report = + assert_success(kvp(&with_dir(&dir, &["read", "PROVISIONING_REPORT"]))); + let expected_agent = + format!("agent=libazureinit-kvp/{}", env!("CARGO_PKG_VERSION")); + assert!(report.contains(&expected_agent), "report was: {report}"); + assert!(report.contains("vm_id=vm-1"), "report was: {report}"); + assert!(report.trim_end().ends_with("|build=123|commit=abc")); +} + +#[test] +fn report_failure_rejects_invalid_supporting_data() { + let dir = TempDir::new().unwrap(); + let output = kvp(&with_dir( + &dir, + &[ + "report-failure", + "--vm-id", + "vm-1", + "--reason", + "boom", + "--supporting-data", + "novalue", + ], + )); + assert_eq!(output.status.code(), Some(2)); + assert!(String::from_utf8(output.stderr) + .unwrap() + .contains("key=value")); +} From d8ec7726c7f07ce393b0e66744c30bba31e611ab Mon Sep 17 00:00:00 2001 From: peytonr18 Date: Tue, 7 Jul 2026 15:12:34 -0700 Subject: [PATCH 4/4] test(kvp): cover VM ID fallback and comma-aware supporting-data parsing Close the codecov gaps on the new CLI paths and rework --supporting-data so one flag can carry multiple comma-separated pairs, including values that contain literal commas. --- libazureinit-kvp/src/cli.rs | 326 +++++++++++++++++++++++++++++++++--- 1 file changed, 301 insertions(+), 25 deletions(-) diff --git a/libazureinit-kvp/src/cli.rs b/libazureinit-kvp/src/cli.rs index be816a80..fa347cc7 100644 --- a/libazureinit-kvp/src/cli.rs +++ b/libazureinit-kvp/src/cli.rs @@ -138,10 +138,11 @@ enum Command { /// Reporting agent identifier (e.g. libazureinit-kvp/0.1.0). #[arg(long, default_value = DEFAULT_AGENT)] agent: String, - /// Additional key=value supporting data, comma-separated or the - /// flag repeated (e.g. --supporting-data k1=v1,k2=v2). - #[arg(long, value_delimiter = ',', value_parser = parse_key_value_pair)] - supporting_data: Vec<(String, String)>, + /// Additional key=value supporting data, comma-separated + /// (e.g. --supporting-data k1=v1,k2=v2). The flag may also be + /// repeated. + #[arg(long, value_parser = parse_supporting_data)] + supporting_data: Vec, }, /// Write a failure provisioning health report, overriding any existing /// `PROVISIONING_REPORT` record. @@ -158,10 +159,22 @@ enum Command { /// Optional documentation URL describing the failure. #[arg(long)] documentation_url: Option, - /// Additional key=value supporting data, comma-separated or the - /// flag repeated (e.g. --supporting-data k1=v1,k2=v2). - #[arg(long, value_delimiter = ',', value_parser = parse_key_value_pair)] - supporting_data: Vec<(String, String)>, + /// Additional key=value supporting data, comma-separated + /// (e.g. --supporting-data k1=v1,k2=v2). The flag may also be + /// repeated. + /// + /// To keep a literal comma in a value, wrap that value in matching + /// single or double quotes. The shell strips quotes first, so wrap + /// the whole argument to let the inner quotes through: + /// + /// --supporting-data "k1='a,b',k2=v2" -> k1=a,b k2=v2 + /// + /// --supporting-data 'k1="a,b",k2=v2' -> k1=a,b k2=v2 + /// + /// Quotes are only special at the start of a value and must be + /// balanced. + #[arg(long, value_parser = parse_supporting_data)] + supporting_data: Vec, }, } @@ -416,12 +429,12 @@ fn report_success( store: &KvpPoolStore, vm_id: Option, agent: String, - supporting_data: Vec<(String, String)>, + supporting_data: Vec, ) -> Result { let vm_id = resolve_vm_id(vm_id)?; let mut report = ProvisioningReport::success(agent, vm_id, ReportPpsType::None); - for (key, value) in supporting_data { + for (key, value) in supporting_data.into_iter().flat_map(|data| data.0) { report = report.with_extra(key, value); } write_report(store, &report)?; @@ -434,12 +447,12 @@ fn report_failure( agent: String, reason: String, documentation_url: Option, - supporting_data: Vec<(String, String)>, + supporting_data: Vec, ) -> Result { let vm_id = resolve_vm_id(vm_id)?; let mut report = ProvisioningReport::failure(agent, vm_id, reason, ReportPpsType::None); - for (key, value) in supporting_data { + for (key, value) in supporting_data.into_iter().flat_map(|data| data.0) { report = report.with_extra(key, value); } if let Some(url) = documentation_url { @@ -452,9 +465,19 @@ fn report_failure( /// Resolve the VM ID for a report, falling back to the current VM's ID when /// `--vm-id` was not supplied. fn resolve_vm_id(vm_id: Option) -> Result { + resolve_vm_id_with(vm_id, crate::vm_id::get_vm_id) +} + +/// Resolve the VM ID using `lookup` to determine the current VM's ID when +/// `--vm-id` was not supplied. Split out from [`resolve_vm_id`] so the +/// auto-detection branches can be tested without touching the host's DMI data. +fn resolve_vm_id_with( + vm_id: Option, + lookup: impl FnOnce() -> Option, +) -> Result { match vm_id { Some(vm_id) => Ok(vm_id), - None => crate::vm_id::get_vm_id().ok_or_else(|| { + None => lookup().ok_or_else(|| { CliError::Usage( "unable to determine the current VM ID automatically; \ pass --vm-id explicitly" @@ -464,7 +487,102 @@ fn resolve_vm_id(vm_id: Option) -> Result { } } -/// Parse a single `key=value` supporting-data pair. +/// One or more `key=value` supporting-data pairs parsed from a single +/// `--supporting-data` argument. +#[derive(Clone, Debug, PartialEq, Eq)] +struct SupportingData(Vec<(String, String)>); + +/// Parse a `--supporting-data` argument into its `key=value` pairs. +/// +/// Fields are comma-separated. A value may be wrapped in matching single or +/// double quotes so it can contain literal commas; the quotes are honored +/// only when they wrap the *entire* value (the opening quote immediately +/// follows `=` and the matching quote ends the field) and are stripped from +/// the stored value. Empty fields (such as a trailing comma) are ignored. +/// +/// Supported (input -> parsed pairs): +/// - `k=v` -> `k`=`v` +/// - `k1=v1,k2=v2` -> `k1`=`v1`, `k2`=`v2` +/// - `k='a,b'` or `k="a,b"` -> `k`=`a,b` (quotes protect the comma) +/// - `k=a'b` -> `k`=`a'b` (a quote not at the value start is literal) +/// - `k=v,` -> `k`=`v` (trailing/empty field ignored) +/// +/// Rejected: +/// - `novalue` -> missing `=` +/// - `=v` -> empty key +/// - `k='a,b` -> unterminated quote +/// - `k='a,b'x` -> characters after a quoted value +fn parse_supporting_data(raw: &str) -> Result { + let mut pairs = Vec::new(); + for field in split_supporting_data_fields(raw)? { + if field.is_empty() { + continue; + } + pairs.push(parse_key_value_pair(field)?); + } + Ok(SupportingData(pairs)) +} + +/// Split a `--supporting-data` argument into `key=value` field slices on +/// top-level commas. See [`parse_supporting_data`] for the quoting rules. +fn split_supporting_data_fields(raw: &str) -> Result, String> { + let bytes = raw.as_bytes(); + let len = bytes.len(); + let mut fields = Vec::new(); + let mut idx = 0; + + loop { + let field_start = idx; + + while idx < len && bytes[idx] != b'=' && bytes[idx] != b',' { + idx += 1; + } + + if idx < len && bytes[idx] == b'=' { + idx += 1; + if idx < len && (bytes[idx] == b'\'' || bytes[idx] == b'"') { + let quote = bytes[idx]; + idx += 1; + let mut closed = false; + while idx < len { + let ch = bytes[idx]; + idx += 1; + if ch == quote { + closed = true; + break; + } + } + if !closed { + return Err(format!( + "supporting data '{raw}' has an unterminated quote" + )); + } + if idx < len && bytes[idx] != b',' { + return Err(format!( + "supporting data '{raw}' has unexpected characters \ + after a quoted value" + )); + } + } else { + while idx < len && bytes[idx] != b',' { + idx += 1; + } + } + } + + fields.push(&raw[field_start..idx]); + + if idx >= len { + break; + } + idx += 1; + } + + Ok(fields) +} + +/// Parse a single `key=value` supporting-data pair, stripping one layer of +/// surrounding single or double quotes from the value. fn parse_key_value_pair(raw: &str) -> Result<(String, String), String> { let (key, value) = raw.split_once('=').ok_or_else(|| { format!("supporting data '{raw}' must be in key=value format") @@ -472,16 +590,26 @@ fn parse_key_value_pair(raw: &str) -> Result<(String, String), String> { if key.is_empty() { return Err(format!("supporting data '{raw}' has an empty key")); } - Ok((key.to_string(), value.to_string())) + Ok((key.to_string(), unquote(value).to_string())) +} + +/// Strip one layer of matching surrounding single or double quotes. +fn unquote(value: &str) -> &str { + let bytes = value.as_bytes(); + if bytes.len() >= 2 { + let first = bytes[0]; + if (first == b'\'' || first == b'"') && bytes[bytes.len() - 1] == first + { + return &value[1..value.len() - 1]; + } + } + value } fn writeln_json( stdout: &mut W, value: &serde_json::Value, ) -> Result<(), CliError> { - // Serializing `serde_json::Value` to a String only fails on writer - // I/O errors, which `to_string` cannot produce, so this is safe to - // unwrap. let rendered = serde_json::to_string(value) .expect("serde_json::Value always serializes to a String"); stdout.write_all(rendered.as_bytes())?; @@ -761,10 +889,28 @@ mod tests { Command::ReportSuccess { ref vm_id, ref agent, ref supporting_data } if vm_id.as_deref() == Some("vm-1") && agent == "Azure-Init/0.0.0" - && supporting_data == &vec![ + && supporting_data == &vec![SupportingData(vec![ ("k1".to_string(), "v1".to_string()), ("k2".to_string(), "v2".to_string()), - ] + ])] + )); + } + + #[test] + fn parse_report_supporting_data_preserves_commas_in_values() { + let cli = Cli::parse_from([ + "libazureinit-kvp", + "report-success", + "--supporting-data", + "k1='foo,bar',k2=foo2", + ]); + assert!(matches!( + cli.command, + Command::ReportSuccess { ref supporting_data, .. } + if supporting_data == &vec![SupportingData(vec![ + ("k1".to_string(), "foo,bar".to_string()), + ("k2".to_string(), "foo2".to_string()), + ])] )); } @@ -808,9 +954,9 @@ mod tests { && agent == "Azure-Init/0.0.0" && reason == "boom" && documentation_url.as_deref() == Some("https://aka.ms/x") - && supporting_data == &vec![ + && supporting_data == &vec![SupportingData(vec![ ("details".to_string(), "bad config".to_string()), - ] + ])] )); } @@ -825,16 +971,143 @@ mod tests { assert!(result.is_err()); } + #[rstest] + #[case::key_value("k=v", Ok(("k".to_string(), "v".to_string())))] + #[case::value_with_commas( + "k=foo,bar", + Ok(("k".to_string(), "foo,bar".to_string())) + )] + #[case::quoted_value( + "k='foo,bar'", + Ok(("k".to_string(), "foo,bar".to_string())) + )] + #[case::missing_separator( + "novalue", + Err("supporting data 'novalue' must be in key=value format".to_string()) + )] + #[case::empty_key("=v", Err("supporting data '=v' has an empty key".to_string()))] + fn parse_key_value_pair_handles_input( + #[case] raw: &str, + #[case] expected: Result<(String, String), String>, + ) { + assert_eq!(parse_key_value_pair(raw), expected); + } + + #[rstest] + #[case::single("k=v", vec![("k", "v")])] + #[case::multiple("k1=v1,k2=v2", vec![("k1", "v1"), ("k2", "v2")])] + #[case::single_quoted_comma("k1='a,b',k2=v2", vec![("k1", "a,b"), ("k2", "v2")])] + #[case::double_quoted_comma("k1=\"a,b\"", vec![("k1", "a,b")])] + #[case::apostrophe_in_bare_value_is_literal( + "k1=it's,k2=v2", + vec![("k1", "it's"), ("k2", "v2")] + )] + #[case::quote_in_middle_is_literal("k=a'b", vec![("k", "a'b")])] + #[case::trailing_comma_ignored("k=v,", vec![("k", "v")])] + #[case::empty_input("", vec![])] + #[case::leading_comma_ignored(",k=v", vec![("k", "v")])] + #[case::only_commas_ignored(",,", vec![])] + #[case::consecutive_commas_ignored( + "k1=v1,,k2=v2", + vec![("k1", "v1"), ("k2", "v2")] + )] + #[case::empty_quoted_value("k=''", vec![("k", "")])] + #[case::empty_bare_value("k=", vec![("k", "")])] + #[case::bare_value_with_equals("k=a=b", vec![("k", "a=b")])] + #[case::quoted_value_with_equals_and_comma( + "k='a=b,c'", + vec![("k", "a=b,c")] + )] + #[case::mixed_quote_types( + "k1='a,b',k2=\"c,d\"", + vec![("k1", "a,b"), ("k2", "c,d")] + )] + #[case::quoted_value_then_trailing_comma("k='a,b',", vec![("k", "a,b")])] + fn parse_supporting_data_splits_and_unquotes( + #[case] raw: &str, + #[case] expected: Vec<(&str, &str)>, + ) { + let expected: Vec<(String, String)> = expected + .into_iter() + .map(|(key, value)| (key.to_string(), value.to_string())) + .collect(); + assert_eq!( + parse_supporting_data(raw).unwrap(), + SupportingData(expected) + ); + } + + #[rstest] + #[case::missing_separator( + "k1=v1,bad", + "supporting data 'bad' must be in key=value format" + )] + #[case::empty_key("=v", "supporting data '=v' has an empty key")] + #[case::unterminated_quote( + "k1='a,b", + "supporting data 'k1='a,b' has an unterminated quote" + )] + #[case::chars_after_quoted_value( + "k1='a,b'x", + "supporting data 'k1='a,b'x' has unexpected characters after a quoted value" + )] + #[case::chars_after_empty_quoted_value( + "k=''x", + "supporting data 'k=''x' has unexpected characters after a quoted value" + )] + #[case::double_quote_unterminated( + "k=\"a,b", + "supporting data 'k=\"a,b' has an unterminated quote" + )] + #[case::missing_key_on_quoted_field( + "k1=v1,'a,b'", + "supporting data ''a' must be in key=value format" + )] + fn parse_supporting_data_rejects_invalid_fields( + #[case] raw: &str, + #[case] expected: &str, + ) { + assert_eq!(parse_supporting_data(raw).unwrap_err(), expected); + } + + #[rstest] + #[case::explicit(Some("vm-1"), Some("auto-vm"), Ok("vm-1"))] + #[case::fallback_to_lookup(None, Some("auto-vm"), Ok("auto-vm"))] + #[case::lookup_fails( + None, + None, + Err("unable to determine the current VM ID automatically") + )] + fn resolve_vm_id_resolves_from_flag_or_lookup( + #[case] vm_id: Option<&str>, + #[case] lookup: Option<&str>, + #[case] expected: Result<&str, &str>, + ) { + let result = resolve_vm_id_with(vm_id.map(str::to_string), || { + lookup.map(str::to_string) + }); + + match expected { + Ok(vm) => assert_eq!(result.unwrap(), vm), + Err(needle) => { + let err = result.unwrap_err(); + assert!(matches!(err, CliError::Usage(_))); + assert_eq!(err.exit_code(), EXIT_USAGE_OR_VALIDATION); + assert!(err.to_string().contains(needle)); + } + } + } + #[test] fn dispatch_report_success_writes_single_record() { let dir = TempDir::new().unwrap(); let command = Command::ReportSuccess { vm_id: Some("vm-1".into()), agent: "Azure-Init/0.0.0".into(), - supporting_data: vec![ + supporting_data: vec![SupportingData(vec![ ("endpoint".into(), "http://example.com".into()), ("status".into(), "404".into()), - ], + ])], }; let (code, _) = run_dispatch(cli(&dir, command)); assert_eq!(code, EXIT_OK); @@ -896,7 +1169,10 @@ mod tests { agent: "Azure-Init/0.0.0".into(), reason: "boom".into(), documentation_url: None, - supporting_data: vec![("details".into(), "bad config".into())], + supporting_data: vec![SupportingData(vec![( + "details".into(), + "bad config".into(), + )])], }; let (code, _) = run_dispatch(cli(&dir, command)); assert_eq!(code, EXIT_OK);