wip(inventory_agent): Refactoring for better error handling in progress

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",

harmony_inventory_agent/src/hwinfo.rs

@@ -83,20 +83,95 @@ pub struct ManagementInterface {
 }
 
 impl PhysicalHost {
-    pub fn gather() -> Self {
+    pub fn gather() -> Result<Self, String> {
         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::<Vec<String>>()
+                .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<StorageDrive> {
@@ -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<Chipset, String> {
+        Ok(Chipset {
+            name: Self::read_dmi("board-product-name")?,
+            vendor: Self::read_dmi("board-manufacturer")?,
+        })
     }
 
     fn gather_network_interfaces() -> Vec<NetworkInterface> {
@@ -436,16 +511,47 @@ impl PhysicalHost {
             .and_then(|s| s.trim().parse().ok())
     }
 
-    fn read_dmi(field: &str) -> Option<String> {
-        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<String, String> {
+        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 {

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]