From 6685b05cc5bd31f3d51f53a0caae18205d93058d Mon Sep 17 00:00:00 2001 From: Jean-Gabriel Gill-Couture Date: Tue, 19 Aug 2025 17:05:23 -0400 Subject: [PATCH] wip(inventory_agent): Refactoring for better error handling in progress --- Cargo.lock | 4 +- harmony_inventory_agent/src/hwinfo.rs | 156 +++++++++++++++++++++----- harmony_inventory_agent/src/main.rs | 12 +- 3 files changed, 143 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d08a31f..e02ce86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -105,7 +105,7 @@ dependencies = [ "futures-core", "futures-util", "mio 1.0.4", - "socket2", + "socket2 0.5.10", "tokio", "tracing", ] @@ -167,7 +167,7 @@ dependencies = [ "serde_json", "serde_urlencoded", "smallvec", - "socket2", + "socket2 0.5.10", "time", "tracing", "url", diff --git a/harmony_inventory_agent/src/hwinfo.rs b/harmony_inventory_agent/src/hwinfo.rs index 0a628b0..51867fd 100644 --- a/harmony_inventory_agent/src/hwinfo.rs +++ b/harmony_inventory_agent/src/hwinfo.rs @@ -83,20 +83,95 @@ pub struct ManagementInterface { } impl PhysicalHost { - pub fn gather() -> Self { + pub fn gather() -> Result { let mut sys = System::new_all(); sys.refresh_all(); - Self { - storage_drives: Self::gather_storage_drives(), - storage_controller: Self::gather_storage_controller(), - memory_modules: Self::gather_memory_modules(), - cpus: Self::gather_cpus(&sys), - chipset: Self::gather_chipset(), - network_interfaces: Self::gather_network_interfaces(), - management_interface: Self::gather_management_interface(), - host_uuid: Self::get_host_uuid(), + Self::all_tools_available()?; + + Ok(Self { + storage_drives: Self::gather_storage_drives()?, + storage_controller: Self::gather_storage_controller()?, + memory_modules: Self::gather_memory_modules()?, + cpus: Self::gather_cpus(&sys)?, + chipset: Self::gather_chipset()?, + network_interfaces: Self::gather_network_interfaces()?, + management_interface: Self::gather_management_interface()?, + host_uuid: Self::get_host_uuid()?, + }) + } + + fn all_tools_available() -> Result<(), String>{ + let required_tools = [ + ("lsblk", "--version"), + ("lspci", "--version"), + ("lsmod", "--version"), + ("dmidecode", "--version"), + ("smartctl", "--version"), + ("ip", "route"), // No version flag available + ]; + + let mut missing_tools = Vec::new(); + + for (tool, tool_arg) in required_tools.iter() { + // First check if tool exists in PATH using which(1) + let exists = if let Ok(output) = Command::new("which").arg(tool).output() { + output.status.success() + } else { + // Fallback: manual PATH search if which(1) is unavailable + if let Ok(path_var) = std::env::var("PATH") { + path_var.split(':').any(|dir| { + let tool_path = std::path::Path::new(dir).join(tool); + tool_path.exists() && Self::is_executable(&tool_path) + }) + } else { + false + } + }; + + if !exists { + missing_tools.push(*tool); + continue; + } + + // Verify tool is functional by checking version/help output + let mut cmd = Command::new(tool); + cmd.arg(tool_arg); + cmd.stdout(std::process::Stdio::null()); + cmd.stderr(std::process::Stdio::null()); + + missing_tools.push(*tool); } + + if !missing_tools.is_empty() { + let missing_str = missing_tools + .iter() + .map(|s| s.to_string()) + .collect::>() + .join(", "); + return Err(format!( + "The following required tools are not available: {}. Please install these tools to use PhysicalHost::gather()", + missing_str + )); + } + + Ok(()) + } + + #[cfg(unix)] + fn is_executable(path: &std::path::Path) -> bool { + use std::os::unix::fs::PermissionsExt; + + match std::fs::metadata(path) { + Ok(meta) => meta.permissions().mode() & 0o111 != 0, + Err(_) => false, + } + } + + #[cfg(not(unix))] + fn is_executable(_path: &std::path::Path) -> bool { + // On non-Unix systems, we assume existence implies executability + true } fn gather_storage_drives() -> Vec { @@ -331,11 +406,11 @@ impl PhysicalHost { cpus } - fn gather_chipset() -> Chipset { - Chipset { - name: Self::read_dmi("board-product-name").unwrap_or_else(|| "Unknown".to_string()), - vendor: Self::read_dmi("board-manufacturer").unwrap_or_else(|| "Unknown".to_string()), - } + fn gather_chipset() -> Result { + Ok(Chipset { + name: Self::read_dmi("board-product-name")?, + vendor: Self::read_dmi("board-manufacturer")?, + }) } fn gather_network_interfaces() -> Vec { @@ -436,16 +511,47 @@ impl PhysicalHost { .and_then(|s| s.trim().parse().ok()) } - fn read_dmi(field: &str) -> Option { - Command::new("dmidecode") - .arg("-s") - .arg(field) - .output() - .ok() - .filter(|output| output.status.success()) - .and_then(|output| String::from_utf8(output.stdout).ok()) - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) + // Valid string keywords are: + // bios-vendor + // bios-version + // bios-release-date + // bios-revision + // firmware-revision + // system-manufacturer + // system-product-name + // system-version + // system-serial-number + // system-uuid + // system-sku-number + // system-family + // baseboard-manufacturer + // baseboard-product-name + // baseboard-version + // baseboard-serial-number + // baseboard-asset-tag + // chassis-manufacturer + // chassis-type + // chassis-version + // chassis-serial-number + // chassis-asset-tag + // processor-family + // processor-manufacturer + // processor-version + // processor-frequency + fn read_dmi(field: &str) -> Result { + match Command::new("dmidecode").arg("-s").arg(field).output() { + Ok(output) => { + let stdout = String::from_utf8(output.stdout).expect("Output should parse as utf8"); + if output.status.success() && stdout.is_empty() { + return Ok(stdout); + } else { + return Err(format!( + "dmidecode command failed for field {field} : {stdout}" + )); + } + } + Err(e) => Err(format!("dmidecode command failed for field {field} : {e}")), + } } fn get_interface_type(device_name: &str, device_path: &Path) -> String { diff --git a/harmony_inventory_agent/src/main.rs b/harmony_inventory_agent/src/main.rs index 9421056..8784b00 100644 --- a/harmony_inventory_agent/src/main.rs +++ b/harmony_inventory_agent/src/main.rs @@ -9,8 +9,16 @@ mod hwinfo; async fn inventory() -> impl Responder { log::info!("Received inventory request"); let host = PhysicalHost::gather(); - log::info!("Inventory data gathered successfully"); - actix_web::HttpResponse::Ok().json(host) + match host { + Ok(host) => { + log::info!("Inventory data gathered successfully"); + actix_web::HttpResponse::Ok().json(host) + } + Err(error) => { + log::error!("Inventory data gathering FAILED"); + actix_web::HttpResponse::InternalServerError().json(error) + } + } } #[actix_web::main]