add unittest for shell cmd executor

Cargo.lock

@@ -113,6 +113,12 @@ version = "1.3.2"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a"
 
+[[package]]
+name = "bitflags"
+version = "2.5.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "cf4b9d6a944f767f8e5e0db018570623c85f3d925ac718db4e06d0187adb21c1"
+
 [[package]]
 name = "bumpalo"
 version = "3.16.0"
@@ -295,6 +301,22 @@ version = "1.0.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "5443807d6dff69373d433ab9ef5378ad8df50ca6298caf15de6e52e24aaf54d5"
 
+[[package]]
+name = "errno"
+version = "0.3.8"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "a258e46cdc063eb8519c00b9fc845fc47bcfca4130e2f08e88665ceda8474245"
+dependencies = [
+ "libc",
+ "windows-sys 0.52.0",
+]
+
+[[package]]
+name = "fastrand"
+version = "2.0.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "658bd65b1cf4c852a3cc96f18a8ce7b5640f6b703f905c7d74532294c2a63984"
+
 [[package]]
 name = "fern"
 version = "0.6.2"
@@ -495,6 +517,12 @@ version = "0.2.153"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9c198f91728a82281a64e1f4f9eeb25d82cb32a5de251c6bd1b5154d63a8e7bd"
 
+[[package]]
+name = "linux-raw-sys"
+version = "0.4.13"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "01cda141df6706de531b6c46c3a33ecca755538219bd484262fa09410c13539c"
+
 [[package]]
 name = "log"
 version = "0.4.21"
@@ -540,7 +568,7 @@ version = "0.26.4"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "598beaf3cc6fdd9a5dfb1630c2800c7acd31df7aaf0f565796fba2b53ca1af1b"
 dependencies = [
- "bitflags",
+ "bitflags 1.3.2",
  "cfg-if",
  "libc",
  "memoffset",
@@ -619,6 +647,7 @@ dependencies = [
  "serde_json",
  "socket2",
  "strum",
+ "tempfile",
  "thiserror",
  "toml",
 ]
@@ -687,7 +716,7 @@ version = "0.4.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "4722d768eff46b75989dd134e5c353f0d6296e5aaa3132e776cbdb56be7731aa"
 dependencies = [
- "bitflags",
+ "bitflags 1.3.2",
 ]
 
 [[package]]
@@ -719,6 +748,19 @@ version = "0.8.3"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "adad44e29e4c806119491a7f06f03de4d1af22c3a680dd47f1e6e179439d1f56"
 
+[[package]]
+name = "rustix"
+version = "0.38.34"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "70dc5ec042f7a43c4a73241207cecc9873a06d45debb38b329f8541d85c2730f"
+dependencies = [
+ "bitflags 2.5.0",
+ "errno",
+ "libc",
+ "linux-raw-sys",
+ "windows-sys 0.52.0",
+]
+
 [[package]]
 name = "rustversion"
 version = "1.0.15"
@@ -936,6 +978,18 @@ dependencies = [
  "unicode-ident",
 ]
 
+[[package]]
+name = "tempfile"
+version = "3.10.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "85b77fafb263dd9d05cbeac119526425676db3784113aa9295c88498cbf8bff1"
+dependencies = [
+ "cfg-if",
+ "fastrand",
+ "rustix",
+ "windows-sys 0.52.0",
+]
+
 [[package]]
 name = "thiserror"
 version = "1.0.59"

Cargo.toml

@@ -33,6 +33,7 @@ version = ">=0.1.2, <0.2"
 
 [dev-dependencies]
 env_logger = "0.11"
+tempfile = "3"
 
 # I don't think we need insane performance. If anything, a small binary is easier to upload
 # to the satellite.

src/controller.rs

@@ -6,6 +6,7 @@ use satrs::{
     request::{GenericMessage, MessageMetadata},
     res_code::ResultU16,
 };
+use serde::{Deserialize, Serialize};
 use std::{
     env::temp_dir,
     path::{Path, PathBuf},
@@ -21,6 +22,12 @@ use ops_sat_rs::config::{
 
 use crate::requests::CompositeRequest;
 
+#[derive(Serialize, Deserialize, Debug)]
+pub struct ShellCmd {
+    cmd: String,
+    args: Vec<String>,
+}
+
 #[derive(Debug, Clone, Copy, TryFromPrimitive)]
 #[repr(u32)]
 pub enum ActionId {
@@ -139,14 +146,18 @@ impl ExperimentController {
         action_req: &ActionRequest,
     ) {
         if let ActionRequestVariant::VecData(data) = &action_req.variant {
-            match String::from_utf8(data.to_vec()) {
+            let shell_cmd_result: serde_json::Result<ShellCmd> = serde_json::from_slice(data);
-                Ok(cmd_str) => {
+            match shell_cmd_result {
-                    log::info!("executing command: {}", cmd_str);
+                Ok(shell_cmd) => {
-                    match Command::new(cmd_str).status() {
+                    log::info!("executing shell cmd {:?}", shell_cmd);
+                    println!("executing shell cmd {:?}", shell_cmd);
+                    match Command::new(shell_cmd.cmd).args(shell_cmd.args).status() {
                         Ok(status) => {
                             if status.success() {
+                                println!("cmd exec success");
                                 self.send_completion_success(requestor, action_req);
                             } else {
+                                println!("cmd exec failed with {}", status);
                                 log::warn!("execution of command failed: {}", status);
                                 self.send_completion_failure(
                                     requestor,
@@ -158,6 +169,7 @@ impl ExperimentController {
                         }
                         Err(e) => {
                             log::warn!("execution of command failed with IO error: {}", e);
+                            println!("cmd exec failed with {}", e);
                             self.send_completion_failure(
                                 requestor,
                                 action_req,
@@ -168,15 +180,12 @@ impl ExperimentController {
                     }
                 }
                 Err(e) => {
-                    log::warn!("invalid utf-8 data in action request: {}", e);
+                    log::warn!("failed to deserialize shell command: {}", e);
-                    self.send_completion_failure(
-                        requestor,
-                        action_req,
-                        SHELL_CMD_INVALID_FORMAT,
-                        Some(e.to_string().into()),
-                    );
                 }
             }
+        } else {
+            log::warn!("no shell command was supplied for shell command action command");
+            self.send_completion_failure(requestor, action_req, SHELL_CMD_INVALID_FORMAT, None);
         }
     }
 
@@ -204,3 +213,60 @@ impl ExperimentController {
         check_at_path(self.home_path_stop_file.as_path());
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use std::sync::{mpsc, Arc};
+
+    use tempfile::NamedTempFile;
+
+    use super::*;
+
+    fn init() {
+        env_logger::builder().is_test(true).init();
+    }
+
+    #[test]
+    fn test_shell_cmd_exection() {
+        init();
+        let (composite_req_tx, composite_req_rx) = mpsc::channel();
+        let (action_reply_tx, action_reply_rx) = mpsc::channel();
+        let stop_signal = Arc::default();
+        let mut exp_ctrl =
+            ExperimentController::new(composite_req_rx, action_reply_tx, stop_signal);
+        let named_temp_file = NamedTempFile::new().expect("creating temp file failed");
+        let cmd = ShellCmd {
+            cmd: "rm".to_string(),
+            args: vec![named_temp_file.path().to_string_lossy().to_string()],
+        };
+        let cmd_serialized = serde_json::to_string(&cmd).expect("serialization failed");
+        let action_req = satrs::action::ActionRequest {
+            action_id: ActionId::ExecuteShellCommandBlocking as u32,
+            variant: satrs::action::ActionRequestVariant::VecData(cmd_serialized.into_bytes()),
+        };
+        composite_req_tx
+            .send(GenericMessage::new(
+                MessageMetadata::new(1, 2),
+                CompositeRequest::Action(action_req),
+            ))
+            .expect("sending action request failed");
+        exp_ctrl.perform_operation();
+        assert!(!named_temp_file.path().exists());
+        let action_reply = action_reply_rx
+            .try_recv()
+            .expect("receiving action reply failed");
+        assert_eq!(
+            action_reply.message.action_id,
+            ActionId::ExecuteShellCommandBlocking as u32
+        );
+        match action_reply.message.variant {
+            ActionReplyVariant::Completed => (),
+            _ => {
+                panic!(
+                    "unexecpted action reply variant {:?}",
+                    action_reply.message.variant
+                )
+            }
+        }
+    }
+}