From 1e57b1f9783b0270d29205509c48c0dc72e27837 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 25 Apr 2024 01:20:54 +0200 Subject: [PATCH 1/4] add blocking shell cmd execution --- src/config.rs | 22 +++++++++ src/controller.rs | 112 +++++++++++++++++++++++++++++++++++++++------- 2 files changed, 117 insertions(+), 17 deletions(-) diff --git a/src/config.rs b/src/config.rs index 30e0614..ef11200 100644 --- a/src/config.rs +++ b/src/config.rs @@ -38,6 +38,7 @@ pub enum GroupId { Hk = 1, Mode = 2, Action = 3, + Controller = 4, } pub const TEST_EVENT: EventU32TypedSev = @@ -212,6 +213,27 @@ pub mod mode_err { pub const WRONG_MODE: ResultU16 = ResultU16::new(GroupId::Mode as u8, 0); } +pub mod ctrl_err { + use super::*; + use satrs::res_code::ResultU16; + + #[resultcode] + pub const INVALID_CMD_FORMAT: ResultU16 = ResultU16::new(GroupId::Controller as u8, 0); + #[resultcode] + pub const SHELL_CMD_IO_ERROR: ResultU16 = ResultU16::new(GroupId::Controller as u8, 1); + #[resultcode] + pub const SHELL_CMD_EXECUTION_FAILURE: ResultU16 = ResultU16::new(GroupId::Controller as u8, 2); + #[resultcode] + pub const SHELL_CMD_INVALID_FORMAT: ResultU16 = ResultU16::new(GroupId::Controller as u8, 3); + + pub const CTRL_ERR_RESULTS: &[ResultU16Info] = &[ + INVALID_CMD_FORMAT_EXT, + SHELL_CMD_IO_ERROR_EXT, + SHELL_CMD_EXECUTION_FAILURE_EXT, + SHELL_CMD_INVALID_FORMAT_EXT, + ]; +} + pub mod pool { use satrs::pool::{StaticMemoryPool, StaticPoolConfig}; diff --git a/src/controller.rs b/src/controller.rs index 69c27c8..a738c5b 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -1,16 +1,23 @@ use num_enum::TryFromPrimitive; use satrs::{ - action::ActionRequest, + action::{ActionRequest, ActionRequestVariant}, + params::Params, pus::action::{ActionReplyPus, ActionReplyVariant}, request::{GenericMessage, MessageMetadata}, + res_code::ResultU16, }; use std::{ env::temp_dir, path::{Path, PathBuf}, + process::Command, sync::{atomic::AtomicBool, mpsc, Arc}, }; -use ops_sat_rs::config::{action_err::INVALID_ACTION_ID, HOME_PATH, STOP_FILE_NAME}; +use ops_sat_rs::config::{ + action_err::INVALID_ACTION_ID, + ctrl_err::{SHELL_CMD_EXECUTION_FAILURE, SHELL_CMD_INVALID_FORMAT, SHELL_CMD_IO_ERROR}, + HOME_PATH, STOP_FILE_NAME, +}; use crate::requests::CompositeRequest; @@ -18,6 +25,7 @@ use crate::requests::CompositeRequest; #[repr(u32)] pub enum ActionId { StopExperiment = 1, + ExecuteShellCommandBlocking = 2, } pub struct ExperimentController { @@ -69,34 +77,104 @@ impl ExperimentController { self.check_stop_file(); } + pub fn send_completion_success(&self, requestor: &MessageMetadata, action_req: &ActionRequest) { + let result = self.action_reply_tx.send(GenericMessage::new_action_reply( + *requestor, + action_req.action_id, + ActionReplyVariant::Completed, + )); + if result.is_err() { + log::error!("sending action reply failed"); + } + } + + pub fn send_completion_failure( + &self, + requestor: &MessageMetadata, + action_req: &ActionRequest, + error_code: ResultU16, + params: Option, + ) { + let result = self.action_reply_tx.send(GenericMessage::new_action_reply( + *requestor, + action_req.action_id, + ActionReplyVariant::CompletionFailed { error_code, params }, + )); + if result.is_err() { + log::error!("sending action reply failed"); + } + } + pub fn handle_action_request(&mut self, requestor: MessageMetadata, action_req: ActionRequest) { - let action_id = ActionId::try_from(action_req.action_id); - if action_id.is_err() { + let send_completion_failure = |error_code: ResultU16, params: Option| { let result = self.action_reply_tx.send(GenericMessage::new_action_reply( requestor, action_req.action_id, - ActionReplyVariant::CompletionFailed { - error_code: INVALID_ACTION_ID, - params: None, - }, + ActionReplyVariant::CompletionFailed { error_code, params }, )); if result.is_err() { log::error!("sending action reply failed"); } + }; + let action_id = ActionId::try_from(action_req.action_id); + if action_id.is_err() { + send_completion_failure(INVALID_ACTION_ID, None); return; } - let action_id = action_id.unwrap(); - match action_id { + match action_id.unwrap() { ActionId::StopExperiment => { self.stop_signal .store(true, std::sync::atomic::Ordering::Relaxed); - let result = self.action_reply_tx.send(GenericMessage::new_action_reply( - requestor, - action_req.action_id, - ActionReplyVariant::Completed, - )); - if result.is_err() { - log::error!("sending action reply failed"); + self.send_completion_success(&requestor, &action_req); + } + ActionId::ExecuteShellCommandBlocking => { + self.handle_shell_command_execution(&requestor, &action_req); + } + } + } + + pub fn handle_shell_command_execution( + &self, + requestor: &MessageMetadata, + action_req: &ActionRequest, + ) { + if let ActionRequestVariant::VecData(data) = &action_req.variant { + match String::from_utf8(data.to_vec()) { + Ok(cmd_str) => { + log::info!("executing command: {}", cmd_str); + match Command::new(cmd_str).status() { + Ok(status) => { + if status.success() { + self.send_completion_success(requestor, action_req); + } else { + log::warn!("execution of command failed: {}", status); + self.send_completion_failure( + requestor, + action_req, + SHELL_CMD_EXECUTION_FAILURE, + Some(status.to_string().into()), + ); + } + } + Err(e) => { + log::warn!("execution of command failed with IO error: {}", e); + self.send_completion_failure( + requestor, + action_req, + SHELL_CMD_IO_ERROR, + Some(e.to_string().into()), + ); + } + } + } + Err(e) => { + log::warn!("invalid utf-8 data in action request: {}", e); + self.send_completion_failure( + requestor, + action_req, + SHELL_CMD_INVALID_FORMAT, + Some(e.to_string().into()), + ); } } } From c5eddcb29203d03c0fe120fa3f2dd17ad75886fb Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 25 Apr 2024 16:45:00 +0200 Subject: [PATCH 2/4] add unittest for shell cmd executor --- Cargo.lock | 58 +++++++++++++++++++++++++++++-- Cargo.toml | 1 + src/controller.rs | 88 +++++++++++++++++++++++++++++++++++++++++------ 3 files changed, 134 insertions(+), 13 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a0d88f0..1c5ea25 100644 --- a/Cargo.lock +++ b/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" diff --git a/Cargo.toml b/Cargo.toml index 70493df..8dbdff1 100644 --- a/Cargo.toml +++ b/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. diff --git a/src/controller.rs b/src/controller.rs index a738c5b..6b210e3 100644 --- a/src/controller.rs +++ b/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, +} + #[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()) { - Ok(cmd_str) => { - log::info!("executing command: {}", cmd_str); - match Command::new(cmd_str).status() { + let shell_cmd_result: serde_json::Result = serde_json::from_slice(data); + match shell_cmd_result { + Ok(shell_cmd) => { + 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); - self.send_completion_failure( - requestor, - action_req, - SHELL_CMD_INVALID_FORMAT, - Some(e.to_string().into()), - ); + log::warn!("failed to deserialize shell command: {}", e); } } + } 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 + ) + } + } + } +} From eea2f76b9f593a1af680206c2942879ce639dc6d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 25 Apr 2024 16:49:15 +0200 Subject: [PATCH 3/4] remove stray printouts --- src/controller.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/controller.rs b/src/controller.rs index 6b210e3..b707f4e 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -150,14 +150,11 @@ impl ExperimentController { match shell_cmd_result { Ok(shell_cmd) => { 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, @@ -169,7 +166,6 @@ 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, From b4a84dbf20d4e0c78a389a9205ffa59ab5661a70 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 25 Apr 2024 17:11:52 +0200 Subject: [PATCH 4/4] that should do the job --- src/controller.rs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/controller.rs b/src/controller.rs index b707f4e..57321d1 100644 --- a/src/controller.rs +++ b/src/controller.rs @@ -23,9 +23,9 @@ use ops_sat_rs::config::{ use crate::requests::CompositeRequest; #[derive(Serialize, Deserialize, Debug)] -pub struct ShellCmd { - cmd: String, - args: Vec, +pub struct ShellCmd<'a> { + cmd: &'a str, + args: Vec<&'a str>, } #[derive(Debug, Clone, Copy, TryFromPrimitive)] @@ -231,10 +231,12 @@ mod tests { 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 args = vec![named_temp_file + .path() + .to_str() + .expect("converting path to str failed")]; + + let cmd = ShellCmd { cmd: "rm", args }; let cmd_serialized = serde_json::to_string(&cmd).expect("serialization failed"); let action_req = satrs::action::ActionRequest { action_id: ActionId::ExecuteShellCommandBlocking as u32,