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 1/2] 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] From 72fb05b5cc0bcfa1dd97b7eeb1dae4e2624cde15 Mon Sep 17 00:00:00 2001 From: Jean-Gabriel Gill-Couture Date: Tue, 19 Aug 2025 17:56:06 -0400 Subject: [PATCH 2/2] fix(inventory_agent) : Agent now retreives correct dmidecode fields, fixed uuid generation which is unacceptable, fixed storage drive parsing, much better error handling, much more strict behavior which also leads to more complete output as missing fields will raise errors unless explicitely optional --- Cargo.lock | 1 - harmony_inventory_agent/Cargo.toml | 1 - harmony_inventory_agent/src/hwinfo.rs | 806 +++++++++++++++----------- 3 files changed, 477 insertions(+), 331 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e02ce86..b1295be 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2178,7 +2178,6 @@ dependencies = [ "serde", "serde_json", "sysinfo", - "uuid", ] [[package]] diff --git a/harmony_inventory_agent/Cargo.toml b/harmony_inventory_agent/Cargo.toml index a299ca0..3b1be2c 100644 --- a/harmony_inventory_agent/Cargo.toml +++ b/harmony_inventory_agent/Cargo.toml @@ -10,4 +10,3 @@ serde.workspace = true serde_json.workspace = true log.workspace = true env_logger.workspace = true -uuid.workspace = true diff --git a/harmony_inventory_agent/src/hwinfo.rs b/harmony_inventory_agent/src/hwinfo.rs index 51867fd..21045cc 100644 --- a/harmony_inventory_agent/src/hwinfo.rs +++ b/harmony_inventory_agent/src/hwinfo.rs @@ -1,3 +1,4 @@ +use log::debug; use serde::{Deserialize, Serialize}; use serde_json::Value; use std::fs; @@ -101,7 +102,7 @@ impl PhysicalHost { }) } - fn all_tools_available() -> Result<(), String>{ + fn all_tools_available() -> Result<(), String> { let required_tools = [ ("lsblk", "--version"), ("lspci", "--version"), @@ -140,7 +141,13 @@ impl PhysicalHost { cmd.stdout(std::process::Stdio::null()); cmd.stderr(std::process::Stdio::null()); - missing_tools.push(*tool); + if let Ok(status) = cmd.status() { + if !status.success() { + missing_tools.push(*tool); + } + } else { + missing_tools.push(*tool); + } } if !missing_tools.is_empty() { @@ -174,11 +181,11 @@ impl PhysicalHost { true } - fn gather_storage_drives() -> Vec { + fn gather_storage_drives() -> Result, String> { let mut drives = Vec::new(); // Use lsblk with JSON output for robust parsing - if let Ok(output) = Command::new("lsblk") + let output = Command::new("lsblk") .args([ "-d", "-o", @@ -189,132 +196,165 @@ impl PhysicalHost { "--json", ]) .output() - && output.status.success() - && let Ok(json) = serde_json::from_slice::(&output.stdout) - && let Some(blockdevices) = json.get("blockdevices").and_then(|v| v.as_array()) - { - for device in blockdevices { - let name = device - .get("name") - .and_then(|v| v.as_str()) - .unwrap_or("") - .to_string(); - if name.is_empty() { - continue; - } + .map_err(|e| format!("Failed to execute lsblk: {}", e))?; - let model = device - .get("model") - .and_then(|v| v.as_str()) - .map(|s| s.trim().to_string()) - .unwrap_or_default(); - - let serial = device - .get("serial") - .and_then(|v| v.as_str()) - .map(|s| s.trim().to_string()) - .unwrap_or_default(); - - let size_str = device.get("size").and_then(|v| v.as_str()).unwrap_or("0"); - let size_bytes = Self::parse_size(size_str).unwrap_or(0); - - let rotational = device - .get("rota") - .and_then(|v| v.as_bool()) - .unwrap_or(false); - - let wwn = device - .get("wwn") - .and_then(|v| v.as_str()) - .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty() && s != "null"); - - let device_path = Path::new("/sys/block").join(&name); - - let mut drive = StorageDrive { - name: name.clone(), - model, - serial, - size_bytes, - logical_block_size: Self::read_sysfs_u32( - &device_path.join("queue/logical_block_size"), - ) - .unwrap_or(512), - physical_block_size: Self::read_sysfs_u32( - &device_path.join("queue/physical_block_size"), - ) - .unwrap_or(512), - rotational, - wwn, - interface_type: Self::get_interface_type(&name, &device_path), - smart_status: Self::get_smart_status(&name), - }; - - // Enhance with additional sysfs info if available - if device_path.exists() { - if drive.model.is_empty() { - drive.model = Self::read_sysfs_string(&device_path.join("device/model")); - } - if drive.serial.is_empty() { - drive.serial = Self::read_sysfs_string(&device_path.join("device/serial")); - } - } - - drives.push(drive); - } + if !output.status.success() { + return Err(format!( + "lsblk command failed: {}", + String::from_utf8_lossy(&output.stderr) + )); } - drives + let json: Value = serde_json::from_slice(&output.stdout) + .map_err(|e| format!("Failed to parse lsblk JSON output: {}", e))?; + + let blockdevices = json + .get("blockdevices") + .and_then(|v| v.as_array()) + .ok_or("Invalid lsblk JSON: missing 'blockdevices' array")?; + + for device in blockdevices { + let name = device + .get("name") + .and_then(|v| v.as_str()) + .ok_or("Missing 'name' in lsblk device")? + .to_string(); + + if name.is_empty() { + continue; + } + + let model = device + .get("model") + .and_then(|v| v.as_str()) + .map(|s| s.trim().to_string()) + .unwrap_or_default(); + + let serial = device + .get("serial") + .and_then(|v| v.as_str()) + .map(|s| s.trim().to_string()) + .unwrap_or_default(); + + let size_str = device + .get("size") + .and_then(|v| v.as_str()) + .ok_or("Missing 'size' in lsblk device")?; + let size_bytes = Self::parse_size(size_str)?; + + let rotational = device + .get("rota") + .and_then(|v| v.as_bool()) + .ok_or("Missing 'rota' in lsblk device")?; + + let wwn = device + .get("wwn") + .and_then(|v| v.as_str()) + .map(|s| s.trim().to_string()) + .filter(|s| !s.is_empty() && s != "null"); + + let device_path = Path::new("/sys/block").join(&name); + + let logical_block_size = Self::read_sysfs_u32( + &device_path.join("queue/logical_block_size"), + ) + .map_err(|e| format!("Failed to read logical block size for {}: {}", name, e))?; + + let physical_block_size = Self::read_sysfs_u32( + &device_path.join("queue/physical_block_size"), + ) + .map_err(|e| format!("Failed to read physical block size for {}: {}", name, e))?; + + let interface_type = Self::get_interface_type(&name, &device_path)?; + let smart_status = Self::get_smart_status(&name)?; + + let mut drive = StorageDrive { + name: name.clone(), + model, + serial, + size_bytes, + logical_block_size, + physical_block_size, + rotational, + wwn, + interface_type, + smart_status, + }; + + // Enhance with additional sysfs info if available + if device_path.exists() { + if drive.model.is_empty() { + drive.model = Self::read_sysfs_string(&device_path.join("device/model")) + .map_err(|e| format!("Failed to read model for {}: {}", name, e))?; + } + if drive.serial.is_empty() { + drive.serial = Self::read_sysfs_string(&device_path.join("device/serial")) + .map_err(|e| format!("Failed to read serial for {}: {}", name, e))?; + } + } + + drives.push(drive); + } + + Ok(drives) } - fn gather_storage_controller() -> StorageController { + fn gather_storage_controller() -> Result { let mut controller = StorageController { name: "Unknown".to_string(), driver: "Unknown".to_string(), }; // Use lspci with JSON output if available - if let Ok(output) = Command::new("lspci") + let output = Command::new("lspci") .args(["-nn", "-d", "::0100", "-J"]) // Storage controllers class with JSON .output() - && output.status.success() - && let Ok(json) = serde_json::from_slice::(&output.stdout) - && let Some(devices) = json.as_array() - { - for device in devices { - if let Some(device_info) = device.as_object() - && let Some(name) = device_info - .get("device") - .and_then(|v| v.as_object()) - .and_then(|v| v.get("name")) - .and_then(|v| v.as_str()) - { - controller.name = name.to_string(); - break; + .map_err(|e| format!("Failed to execute lspci: {}", e))?; + + if output.status.success() { + let json: Value = serde_json::from_slice(&output.stdout) + .map_err(|e| format!("Failed to parse lspci JSON output: {}", e))?; + + if let Some(devices) = json.as_array() { + for device in devices { + if let Some(device_info) = device.as_object() + && let Some(name) = device_info + .get("device") + .and_then(|v| v.as_object()) + .and_then(|v| v.get("name")) + .and_then(|v| v.as_str()) + { + controller.name = name.to_string(); + break; + } } } } - // Fallback to text output if JSON fails - if controller.name == "Unknown" - && let Ok(output) = Command::new("lspci") + // Fallback to text output if JSON fails or no device found + if controller.name == "Unknown" { + let output = Command::new("lspci") .args(["-nn", "-d", "::0100"]) // Storage controllers class .output() - && output.status.success() - { - let output_str = String::from_utf8_lossy(&output.stdout); - if let Some(line) = output_str.lines().next() { - let parts: Vec<&str> = line.split(':').collect(); - if parts.len() > 2 { - controller.name = parts[2].trim().to_string(); + .map_err(|e| format!("Failed to execute lspci (fallback): {}", e))?; + + if output.status.success() { + let output_str = String::from_utf8_lossy(&output.stdout); + if let Some(line) = output_str.lines().next() { + let parts: Vec<&str> = line.split(':').collect(); + if parts.len() > 2 { + controller.name = parts[2].trim().to_string(); + } } } } // Try to get driver info from lsmod - if let Ok(output) = Command::new("lsmod").output() - && output.status.success() - { + let output = Command::new("lsmod") + .output() + .map_err(|e| format!("Failed to execute lsmod: {}", e))?; + + if output.status.success() { let output_str = String::from_utf8_lossy(&output.stdout); for line in output_str.lines() { if line.contains("ahci") @@ -331,67 +371,78 @@ impl PhysicalHost { } } - controller + Ok(controller) } - fn gather_memory_modules() -> Vec { + fn gather_memory_modules() -> Result, String> { let mut modules = Vec::new(); - if let Ok(output) = Command::new("dmidecode").arg("--type").arg("17").output() - && output.status.success() - { - let output_str = String::from_utf8_lossy(&output.stdout); - let sections: Vec<&str> = output_str.split("Memory Device").collect(); + let output = Command::new("dmidecode") + .arg("--type") + .arg("17") + .output() + .map_err(|e| format!("Failed to execute dmidecode: {}", e))?; - for section in sections.into_iter().skip(1) { - let mut module = MemoryModule { - size_bytes: 0, - speed_mhz: None, - manufacturer: None, - part_number: None, - serial_number: None, - rank: None, - }; + if !output.status.success() { + return Err(format!( + "dmidecode command failed: {}", + String::from_utf8_lossy(&output.stderr) + )); + } - for line in section.lines() { - let line = line.trim(); - if let Some(size_str) = line.strip_prefix("Size: ") { - if size_str != "No Module Installed" - && let Some((num, unit)) = size_str.split_once(' ') - && let Ok(num) = num.parse::() - { - module.size_bytes = match unit { - "MB" => num * 1024 * 1024, - "GB" => num * 1024 * 1024 * 1024, - "KB" => num * 1024, - _ => 0, - }; - } - } else if let Some(speed_str) = line.strip_prefix("Speed: ") { - if let Some((num, _unit)) = speed_str.split_once(' ') { - module.speed_mhz = num.parse().ok(); - } - } else if let Some(man) = line.strip_prefix("Manufacturer: ") { - module.manufacturer = Some(man.to_string()); - } else if let Some(part) = line.strip_prefix("Part Number: ") { - module.part_number = Some(part.to_string()); - } else if let Some(serial) = line.strip_prefix("Serial Number: ") { - module.serial_number = Some(serial.to_string()); - } else if let Some(rank) = line.strip_prefix("Rank: ") { - module.rank = rank.parse().ok(); + let output_str = String::from_utf8(output.stdout) + .map_err(|e| format!("Failed to parse dmidecode output: {}", e))?; + + let sections: Vec<&str> = output_str.split("Memory Device").collect(); + + for section in sections.into_iter().skip(1) { + let mut module = MemoryModule { + size_bytes: 0, + speed_mhz: None, + manufacturer: None, + part_number: None, + serial_number: None, + rank: None, + }; + + for line in section.lines() { + let line = line.trim(); + if let Some(size_str) = line.strip_prefix("Size: ") { + if size_str != "No Module Installed" + && let Some((num, unit)) = size_str.split_once(' ') + && let Ok(num) = num.parse::() + { + module.size_bytes = match unit { + "MB" => num * 1024 * 1024, + "GB" => num * 1024 * 1024 * 1024, + "KB" => num * 1024, + _ => 0, + }; } + } else if let Some(speed_str) = line.strip_prefix("Speed: ") { + if let Some((num, _unit)) = speed_str.split_once(' ') { + module.speed_mhz = num.parse().ok(); + } + } else if let Some(man) = line.strip_prefix("Manufacturer: ") { + module.manufacturer = Some(man.to_string()); + } else if let Some(part) = line.strip_prefix("Part Number: ") { + module.part_number = Some(part.to_string()); + } else if let Some(serial) = line.strip_prefix("Serial Number: ") { + module.serial_number = Some(serial.to_string()); + } else if let Some(rank) = line.strip_prefix("Rank: ") { + module.rank = rank.parse().ok(); } + } - if module.size_bytes > 0 { - modules.push(module); - } + if module.size_bytes > 0 { + modules.push(module); } } - modules + Ok(modules) } - fn gather_cpus(sys: &System) -> Vec { + fn gather_cpus(sys: &System) -> Result, String> { let mut cpus = Vec::new(); let global_cpu = sys.global_cpu_info(); @@ -403,232 +454,310 @@ impl PhysicalHost { frequency_mhz: global_cpu.frequency(), }); - cpus + Ok(cpus) } fn gather_chipset() -> Result { Ok(Chipset { - name: Self::read_dmi("board-product-name")?, - vendor: Self::read_dmi("board-manufacturer")?, + name: Self::read_dmi("baseboard-product-name")?, + vendor: Self::read_dmi("baseboard-manufacturer")?, }) } - fn gather_network_interfaces() -> Vec { + fn gather_network_interfaces() -> Result, String> { let mut interfaces = Vec::new(); let sys_net_path = Path::new("/sys/class/net"); - if let Ok(entries) = fs::read_dir(sys_net_path) { - for entry in entries.flatten() { - let iface_name = entry.file_name().into_string().unwrap_or_default(); - let iface_path = entry.path(); + let entries = fs::read_dir(sys_net_path) + .map_err(|e| format!("Failed to read /sys/class/net: {}", e))?; - // Skip virtual interfaces - if iface_name.starts_with("lo") - || iface_name.starts_with("docker") - || iface_name.starts_with("virbr") - || iface_name.starts_with("veth") - || iface_name.starts_with("br-") - || iface_name.starts_with("tun") - || iface_name.starts_with("wg") - { - continue; - } + for entry in entries { + let entry = entry.map_err(|e| format!("Failed to read directory entry: {}", e))?; + let iface_name = entry + .file_name() + .into_string() + .map_err(|_| "Invalid UTF-8 in interface name")?; + let iface_path = entry.path(); - // Check if it's a physical interface by looking for device directory - if !iface_path.join("device").exists() { - continue; - } - - let mac_address = Self::read_sysfs_string(&iface_path.join("address")); - let speed_mbps = Self::read_sysfs_u32(&iface_path.join("speed")); - let operstate = Self::read_sysfs_string(&iface_path.join("operstate")); - let mtu = Self::read_sysfs_u32(&iface_path.join("mtu")).unwrap_or(1500); - let driver = Self::read_sysfs_string(&iface_path.join("device/driver/module")); - let firmware_version = - Self::read_sysfs_opt_string(&iface_path.join("device/firmware_version")); - - // Get IP addresses using ip command with JSON output - let (ipv4_addresses, ipv6_addresses) = Self::get_interface_ips_json(&iface_name); - - interfaces.push(NetworkInterface { - name: iface_name, - mac_address, - speed_mbps, - is_up: operstate == "up", - mtu, - ipv4_addresses, - ipv6_addresses, - driver, - firmware_version, - }); + // Skip virtual interfaces + if iface_name.starts_with("lo") + || iface_name.starts_with("docker") + || iface_name.starts_with("virbr") + || iface_name.starts_with("veth") + || iface_name.starts_with("br-") + || iface_name.starts_with("tun") + || iface_name.starts_with("wg") + { + continue; } + + // Check if it's a physical interface by looking for device directory + if !iface_path.join("device").exists() { + continue; + } + + let mac_address = Self::read_sysfs_string(&iface_path.join("address")) + .map_err(|e| format!("Failed to read MAC address for {}: {}", iface_name, e))?; + + let speed_mbps = if iface_path.join("speed").exists() { + match Self::read_sysfs_u32(&iface_path.join("speed")) { + Ok(speed) => Some(speed), + Err(e) => { + debug!( + "Failed to read speed for {}: {} . This is expected to fail on wifi interfaces.", + iface_name, e + ); + None + } + } + } else { + None + }; + + let operstate = Self::read_sysfs_string(&iface_path.join("operstate")) + .map_err(|e| format!("Failed to read operstate for {}: {}", iface_name, e))?; + + let mtu = Self::read_sysfs_u32(&iface_path.join("mtu")) + .map_err(|e| format!("Failed to read MTU for {}: {}", iface_name, e))?; + + let driver = + Self::read_sysfs_symlink_basename(&iface_path.join("device/driver/module")) + .map_err(|e| format!("Failed to read driver for {}: {}", iface_name, e))?; + + let firmware_version = Self::read_sysfs_opt_string( + &iface_path.join("device/firmware_version"), + ) + .map_err(|e| format!("Failed to read firmware version for {}: {}", iface_name, e))?; + + // Get IP addresses using ip command with JSON output + let (ipv4_addresses, ipv6_addresses) = Self::get_interface_ips_json(&iface_name) + .map_err(|e| format!("Failed to get IP addresses for {}: {}", iface_name, e))?; + + interfaces.push(NetworkInterface { + name: iface_name, + mac_address, + speed_mbps, + is_up: operstate == "up", + mtu, + ipv4_addresses, + ipv6_addresses, + driver, + firmware_version, + }); } - interfaces + Ok(interfaces) } - fn gather_management_interface() -> Option { - // Try to detect common management interfaces + fn gather_management_interface() -> Result, String> { if Path::new("/dev/ipmi0").exists() { - Some(ManagementInterface { + Ok(Some(ManagementInterface { kind: "IPMI".to_string(), address: None, - firmware: Self::read_dmi("bios-version"), - }) + firmware: Some(Self::read_dmi("bios-version")?), + })) } else if Path::new("/sys/class/misc/mei").exists() { - Some(ManagementInterface { + Ok(Some(ManagementInterface { kind: "Intel ME".to_string(), address: None, firmware: None, - }) + })) } else { - None + Ok(None) } } - fn get_host_uuid() -> String { - Self::read_dmi("system-uuid").unwrap() + fn get_host_uuid() -> Result { + Self::read_dmi("system-uuid") } // Helper methods - fn read_sysfs_string(path: &Path) -> String { + fn read_sysfs_string(path: &Path) -> Result { fs::read_to_string(path) - .unwrap_or_default() - .trim() - .to_string() - } - - fn read_sysfs_opt_string(path: &Path) -> Option { - fs::read_to_string(path) - .ok() .map(|s| s.trim().to_string()) - .filter(|s| !s.is_empty()) + .map_err(|e| format!("Failed to read {}: {}", path.display(), e)) } - fn read_sysfs_u32(path: &Path) -> Option { - fs::read_to_string(path) - .ok() - .and_then(|s| s.trim().parse().ok()) - } - - // 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}" - )); - } + fn read_sysfs_opt_string(path: &Path) -> Result, String> { + match fs::read_to_string(path) { + Ok(s) => { + let s = s.trim().to_string(); + Ok(if s.is_empty() { None } else { Some(s) }) } - Err(e) => Err(format!("dmidecode command failed for field {field} : {e}")), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(e) => Err(format!("Failed to read {}: {}", path.display(), e)), } } - fn get_interface_type(device_name: &str, device_path: &Path) -> String { - if device_name.starts_with("nvme") { - "NVMe".to_string() - } else if device_name.starts_with("sd") { - "SATA".to_string() - } else if device_name.starts_with("hd") { - "IDE".to_string() - } else if device_name.starts_with("vd") { - "VirtIO".to_string() - } else { - // Try to determine from device path - Self::read_sysfs_string(&device_path.join("device/subsystem")) - .split('/') - .next_back() - .unwrap_or("Unknown") - .to_string() + fn read_sysfs_u32(path: &Path) -> Result { + fs::read_to_string(path) + .map_err(|e| format!("Failed to read {}: {}", path.display(), e))? + .trim() + .parse() + .map_err(|e| format!("Failed to parse {}: {}", path.display(), e)) + } + + fn read_sysfs_symlink_basename(path: &Path) -> Result { + match fs::read_link(path) { + Ok(target_path) => match target_path.file_name() { + Some(name_osstr) => match name_osstr.to_str() { + Some(name_str) => Ok(name_str.to_string()), + None => Err(format!( + "Symlink target basename is not valid UTF-8: {}", + target_path.display() + )), + }, + None => Err(format!( + "Symlink target has no basename: {} -> {}", + path.display(), + target_path.display() + )), + }, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Err(format!( + "Could not resolve symlink for path : {}", + path.display() + )), + Err(e) => Err(format!("Failed to read symlink {}: {}", path.display(), e)), } } - fn get_smart_status(device_name: &str) -> Option { - Command::new("smartctl") - .arg("-H") - .arg(format!("/dev/{}", device_name)) + fn read_dmi(field: &str) -> Result { + let output = Command::new("dmidecode") + .arg("-s") + .arg(field) .output() - .ok() - .filter(|output| output.status.success()) - .and_then(|output| String::from_utf8(output.stdout).ok()) - .and_then(|s| { - s.lines() - .find(|line| line.contains("SMART overall-health self-assessment")) - .and_then(|line| line.split(':').nth(1)) - .map(|s| s.trim().to_string()) + .map_err(|e| format!("Failed to execute dmidecode for field {}: {}", field, e))?; + + if !output.status.success() { + return Err(format!( + "dmidecode command failed for field {}: {}", + field, + String::from_utf8_lossy(&output.stderr) + )); + } + + String::from_utf8(output.stdout) + .map(|s| s.trim().to_string()) + .map_err(|e| { + format!( + "Failed to parse dmidecode output for field {}: {}", + field, e + ) }) } - fn parse_size(size_str: &str) -> Option { - if size_str.ends_with('T') { - size_str[..size_str.len() - 1] - .parse::() - .ok() - .map(|t| t * 1024 * 1024 * 1024 * 1024) - } else if size_str.ends_with('G') { - size_str[..size_str.len() - 1] - .parse::() - .ok() - .map(|g| g * 1024 * 1024 * 1024) - } else if size_str.ends_with('M') { - size_str[..size_str.len() - 1] - .parse::() - .ok() - .map(|m| m * 1024 * 1024) - } else if size_str.ends_with('K') { - size_str[..size_str.len() - 1] - .parse::() - .ok() - .map(|k| k * 1024) - } else if size_str.ends_with('B') { - size_str[..size_str.len() - 1].parse::().ok() + fn get_interface_type(device_name: &str, device_path: &Path) -> Result { + if device_name.starts_with("nvme") { + Ok("NVMe".to_string()) + } else if device_name.starts_with("sd") { + Ok("SATA".to_string()) + } else if device_name.starts_with("hd") { + Ok("IDE".to_string()) + } else if device_name.starts_with("vd") { + Ok("VirtIO".to_string()) } else { - size_str.parse::().ok() + // Try to determine from device path + let subsystem = Self::read_sysfs_string(&device_path.join("device/subsystem"))?; + Ok(subsystem + .split('/') + .next_back() + .unwrap_or("Unknown") + .to_string()) } } - fn get_interface_ips_json(iface_name: &str) -> (Vec, Vec) { + fn get_smart_status(device_name: &str) -> Result, String> { + let output = Command::new("smartctl") + .arg("-H") + .arg(format!("/dev/{}", device_name)) + .output() + .map_err(|e| format!("Failed to execute smartctl for {}: {}", device_name, e))?; + + if !output.status.success() { + return Ok(None); + } + + let stdout = String::from_utf8(output.stdout) + .map_err(|e| format!("Failed to parse smartctl output for {}: {}", device_name, e))?; + + for line in stdout.lines() { + if line.contains("SMART overall-health self-assessment") { + if let Some(status) = line.split(':').nth(1) { + return Ok(Some(status.trim().to_string())); + } + } + } + + Ok(None) + } + + fn parse_size(size_str: &str) -> Result { + debug!("Parsing size_str '{size_str}'"); + let size; + if size_str.ends_with('T') { + size = size_str[..size_str.len() - 1] + .parse::() + .map(|t| t * 1024.0 * 1024.0 * 1024.0 * 1024.0) + .map_err(|e| format!("Failed to parse T size '{}': {}", size_str, e)) + } else if size_str.ends_with('G') { + size = size_str[..size_str.len() - 1] + .parse::() + .map(|g| g * 1024.0 * 1024.0 * 1024.0) + .map_err(|e| format!("Failed to parse G size '{}': {}", size_str, e)) + } else if size_str.ends_with('M') { + size = size_str[..size_str.len() - 1] + .parse::() + .map(|m| m * 1024.0 * 1024.0) + .map_err(|e| format!("Failed to parse M size '{}': {}", size_str, e)) + } else if size_str.ends_with('K') { + size = size_str[..size_str.len() - 1] + .parse::() + .map(|k| k * 1024.0) + .map_err(|e| format!("Failed to parse K size '{}': {}", size_str, e)) + } else if size_str.ends_with('B') { + size = size_str[..size_str.len() - 1] + .parse::() + .map_err(|e| format!("Failed to parse B size '{}': {}", size_str, e)) + } else { + size = size_str + .parse::() + .map_err(|e| format!("Failed to parse size '{}': {}", size_str, e)) + } + + size.map(|s| s as u64) + } + + fn get_interface_ips_json(iface_name: &str) -> Result<(Vec, Vec), String> { let mut ipv4 = Vec::new(); let mut ipv6 = Vec::new(); // Get IPv4 addresses using JSON output - if let Ok(output) = Command::new("ip") + let output = Command::new("ip") .args(["-j", "-4", "addr", "show", iface_name]) .output() - && output.status.success() - && let Ok(json) = serde_json::from_slice::(&output.stdout) - && let Some(addrs) = json.as_array() - { + .map_err(|e| { + format!( + "Failed to execute ip command for IPv4 on {}: {}", + iface_name, e + ) + })?; + + if !output.status.success() { + return Err(format!( + "ip command for IPv4 on {} failed: {}", + iface_name, + String::from_utf8_lossy(&output.stderr) + )); + } + + let json: Value = serde_json::from_slice(&output.stdout).map_err(|e| { + format!( + "Failed to parse ip JSON output for IPv4 on {}: {}", + iface_name, e + ) + })?; + + if let Some(addrs) = json.as_array() { for addr_info in addrs { if let Some(addr_info_obj) = addr_info.as_object() && let Some(addr_info) = @@ -646,13 +775,32 @@ impl PhysicalHost { } // Get IPv6 addresses using JSON output - if let Ok(output) = Command::new("ip") + let output = Command::new("ip") .args(["-j", "-6", "addr", "show", iface_name]) .output() - && output.status.success() - && let Ok(json) = serde_json::from_slice::(&output.stdout) - && let Some(addrs) = json.as_array() - { + .map_err(|e| { + format!( + "Failed to execute ip command for IPv6 on {}: {}", + iface_name, e + ) + })?; + + if !output.status.success() { + return Err(format!( + "ip command for IPv6 on {} failed: {}", + iface_name, + String::from_utf8_lossy(&output.stderr) + )); + } + + let json: Value = serde_json::from_slice(&output.stdout).map_err(|e| { + format!( + "Failed to parse ip JSON output for IPv6 on {}: {}", + iface_name, e + ) + })?; + + if let Some(addrs) = json.as_array() { for addr_info in addrs { if let Some(addr_info_obj) = addr_info.as_object() && let Some(addr_info) = @@ -672,6 +820,6 @@ impl PhysicalHost { } } - (ipv4, ipv6) + Ok((ipv4, ipv6)) } }