Merge pull request #2602 from Ruadhri17/custom-operation-subcommands

Add support for commands with multiple arguments in custom operations

crates/common/logged_command/Cargo.toml

@@ -11,6 +11,7 @@ repository = { workspace = true }
 [dependencies]
 log = { workspace = true }
 nix = { workspace = true }
+shell-words = { workspace = true }
 tokio = { workspace = true, features = [
     "fs",
     "io-util",
@@ -22,6 +23,7 @@ tokio = { workspace = true, features = [
 [dev-dependencies]
 anyhow = { workspace = true }
 tedge_test_utils = { workspace = true }
+tokio = { workspace = true, features = ["time"] }
 
 [lints]
 workspace = true

crates/common/logged_command/src/logged_command.rs

@@ -148,23 +148,40 @@ impl std::fmt::Display for LoggedCommand {
 }
 
 impl LoggedCommand {
-    pub fn new(program: impl AsRef<OsStr>) -> LoggedCommand {
+    pub fn new(program: impl AsRef<OsStr>) -> Result<LoggedCommand, std::io::Error> {
         let command_line = match program.as_ref().to_str() {
             None => format!("{:?}", program.as_ref()),
             Some(cmd) => cmd.to_string(),
         };
 
-        let mut command = Command::new(program);
+        let mut args = shell_words::split(&command_line)
+            .map_err(|err| std::io::Error::new(std::io::ErrorKind::InvalidData, err))?;
+
+        let mut command = match args.len() {
+            0 => {
+                return Err(std::io::Error::new(
+                    std::io::ErrorKind::InvalidData,
+                    "command line is empty.",
+                ))
+            }
+            1 => Command::new(&args[0]),
+            _ => {
+                let mut command = Command::new(args.remove(0));
+                command.args(&args);
+                command
+            }
+        };
+
         command
             .current_dir("/tmp")
             .stdin(Stdio::piped())
             .stdout(Stdio::piped())
             .stderr(Stdio::piped());
 
-        LoggedCommand {
+        Ok(LoggedCommand {
             command_line,
             command,
-        }
+        })
     }
 
     pub fn arg(&mut self, arg: impl AsRef<OsStr>) -> &mut LoggedCommand {
@@ -259,7 +276,7 @@ mod tests {
         let mut logger = BufWriter::new(log_file);
 
         // Prepare a command
-        let mut command = LoggedCommand::new("echo");
+        let mut command = LoggedCommand::new("echo").unwrap();
         command.arg("Hello").arg("World!");
 
         // Execute the command with logging
@@ -292,7 +309,7 @@ EOF
         let mut logger = BufWriter::new(log_file);
 
         // Prepare a command that triggers some content on stderr
-        let mut command = LoggedCommand::new("ls");
+        let mut command = LoggedCommand::new("ls").unwrap();
         command.arg("dummy-file");
 
         // Execute the command with logging
@@ -326,7 +343,7 @@ EOF
         let mut logger = BufWriter::new(log_file);
 
         // Prepare a command that cannot be executed
-        let command = LoggedCommand::new("dummy-command");
+        let command = LoggedCommand::new("dummy-command").unwrap();
 
         // Execute the command with logging
         let _ = command.execute(&mut logger).await;

crates/core/plugin_sm/src/plugin.rs

@@ -263,11 +263,14 @@ impl ExternalPluginCommand {
         maybe_module: Option<&SoftwareModule>,
     ) -> Result<LoggedCommand, SoftwareError> {
         let mut command = if let Some(sudo) = &self.sudo {
-            let mut command = LoggedCommand::new(sudo);
+            // Safe unwrap
+            let mut command = LoggedCommand::new(sudo).unwrap();
             command.arg(&self.path);
             command
         } else {
-            LoggedCommand::new(&self.path)
+            LoggedCommand::new(&self.path).map_err(|err| SoftwareError::ParseError {
+                reason: err.to_string(),
+            })?
         };
         command.arg(action);
 

crates/extensions/c8y_mapper_ext/src/converter.rs

@@ -793,7 +793,13 @@ impl CumulocityConverter {
         let command = command.to_owned();
         let payload = payload.to_string();
 
-        let mut logged = LoggedCommand::new(&command);
+        let mut logged =
+            LoggedCommand::new(&command).map_err(|e| CumulocityMapperError::ExecuteFailed {
+                error_message: e.to_string(),
+                command: command.to_string(),
+                operation_name: operation_name.to_string(),
+            })?;
+
         logged.arg(&payload);
 
         let maybe_child_process =

docs/src/operate/c8y/supported_operations.md

@@ -237,3 +237,25 @@ The command will be executed with tedge-mapper permission level so most of the s
 * `on_message` - The SmartRest template on which the operation will be executed.
 * `command` - The command to execute.
 * `result_format` - The expected command output format: `"text"` or `"csv"`, `"text"` being the default.
+
+:::info
+The `command` parameter accepts command arguments when provided as a one string, e.g.
+
+```toml title="file: /etc/tedge/operations/c8y/c8y_Command"
+[exec]
+  topic = "c8y/s/ds"
+  on_message = "511"
+  command = "python /etc/tedge/operations/command.py"
+  timeout = 10
+``` 
+
+Arguments will be parsed correctly as long as following features are not included in input: operators, variable assignments, tilde expansion, parameter expansion, command substitution, arithmetic expansion and pathname expansion. 
+
+In case those unsupported shell features are present, the syntax that introduce them is interpreted literally.
+
+Be aware that SmartREST payload is always added as the last argument. The command presented above will actually lead to following code execution
+
+```bash
+python /etc/tedge/operations/command.py $SMART_REST_PAYLOAD
+```
+:::

tests/RobotFramework/tests/cumulocity/custom_operation/custom_operation_multiple_args/c8y_Command_1

@@ -0,0 +1,4 @@
+[exec]
+topic = "c8y/s/ds"
+on_message = "511"
+command = "sh /etc/tedge/operations/command_1.sh"

tests/RobotFramework/tests/cumulocity/custom_operation/custom_operation_multiple_args/command_1.sh

@@ -0,0 +1,2 @@
+#!/bin/bash
+echo "command 1"

tests/RobotFramework/tests/cumulocity/custom_operation/custom_operation_multiple_args/operation_multiple_arguments_in_command.robot

@@ -0,0 +1,26 @@
+*** Settings ***
+Resource            ../../../../resources/common.resource
+Library             Cumulocity
+Library             ThinEdgeIO
+
+Test Setup          Custom Setup
+Test Teardown       Get Logs
+
+Test Tags           theme:c8y    theme:operation    theme:custom
+
+
+*** Test Cases ***
+Run the custom operation with multiple arguments
+    ${operation}=    Cumulocity.Create Operation    description=do something    fragments={"c8y_Command":{"text":""}}
+    Operation Should Be SUCCESSFUL    ${operation}
+    Should Be Equal    ${operation.to_json()["c8y_Command"]["result"]}    command 1\n
+
+
+*** Keywords ***
+Custom Setup
+    ${DEVICE_SN}=    Setup
+    Set Suite Variable    $DEVICE_SN
+    Device Should Exist    ${DEVICE_SN}
+    ThinEdgeIO.Transfer To Device    ${CURDIR}/command_1.sh    /etc/tedge/operations/
+    Execute Command    chmod a+x /etc/tedge/operations/command_1.sh
+    ThinEdgeIO.Transfer To Device    ${CURDIR}/c8y_Command_1    /etc/tedge/operations/c8y/c8y_Command