Major refactor to cleanly run tasks from files or inline

ExecutableTask needs to find a new home, and some other things still need to get
shuffled around. This commit does however copy a temp file over for execution
which seems to be the best possible scenario for safe execution

An example run:

❯ RUST_LOG=debug ./target/debug/zap task tasks/shell/bash.ztask -p "script=set -xe; pwd" -t zap-freebsd
Running task with: TaskOpts { task: "tasks/shell/bash.ztask", parameter: ["script=set -xe; pwd"], targets: "zap-freebsd" }
 INFO  zap > Task located, preparing to execute
 DEBUG handlebars::render > Rendering value: Path(Relative(([Named("script")], "script")))
 DEBUG handlebars::context > Accessing context value: AbsolutePath(["script"])
err: + pwd
/root

Cargo.lock

@@ -634,6 +634,7 @@ dependencies = [
 name = "zap-parser"
 version = "0.1.0"
 dependencies = [
+ "handlebars",
  "log",
  "pest",
  "pest_derive",

README.adoc

@@ -38,7 +38,8 @@ a wrapped shell/ruby/python script which does some specific piece of
 functionality. Tasks may also take parameters, which allow for some
 pluggability of new values.
 
-Tasks have some built-in parameters that should not be overridden.
+Tasks have some default parameters that should not be overridden in new task
+definitions.
 
 .Built-in Parameters
 |===

cli/src/main.rs

@@ -10,7 +10,7 @@ mod transport;
 
 use crate::inventory::*;
 use crate::transport::ssh::Ssh;
-use zap_parser::plan::Plan;
+use zap_parser::plan::{ExecutableTask, Plan};
 use zap_parser::task::Task;
 
 fn main() {
@@ -51,13 +51,7 @@ fn handle_plan(opts: PlanOpts, runner: &dyn crate::transport::Transport, invento
             info!("Plan located, preparing to execute");
             for task in plan.tasks {
                 info!("Running executable task: {:?}", task);
-                exit = execute_task_on(
+                exit = execute_task_on(opts.targets.clone(), &task, runner, &inventory);
-                    opts.targets.clone(),
-                    &task.task,
-                    &task.parameters,
-                    runner,
-                    &inventory,
-                );
             }
         }
         Err(err) => {
@@ -69,21 +63,16 @@ fn handle_plan(opts: PlanOpts, runner: &dyn crate::transport::Transport, invento
 
 fn execute_task_on(
     targets: String,
-    task: &Task,
+    task: &ExecutableTask,
-    parameters: &HashMap<String, String>,
     runner: &dyn crate::transport::Transport,
     inventory: &Inventory,
 ) -> i32 {
-    if let Some(script) = task.get_script() {
+    if let Some(group) = inventory.groups.iter().find(|g| g.name == targets) {
-        let command = render_command(&script, &parameters);
+        return runner.run_group(task, &group, &inventory);
+    }
 
-        if let Some(group) = inventory.groups.iter().find(|g| g.name == targets) {
+    if let Some(target) = inventory.targets.iter().find(|t| t.name == targets) {
-            return runner.run_group(&command, &group, &inventory);
+        return runner.run(task, &target);
-        }
-
-        if let Some(target) = inventory.targets.iter().find(|t| t.name == targets) {
-            return runner.run(&command, &target);
-        }
     }
     error!("Failed to locate a script to execute for the task!");
     return -1;
@@ -110,13 +99,10 @@ fn handle_task(opts: TaskOpts, runner: &dyn crate::transport::Transport, invento
                     parameters.insert(parts[0].to_string(), parts[1].to_string());
                 }
             }
-            std::process::exit(execute_task_on(
+
-                opts.targets,
+            let task = ExecutableTask::new(task, parameters);
-                &task,
+
-                &parameters,
+            std::process::exit(execute_task_on(opts.targets, &task, runner, &inventory));
-                runner,
-                &inventory,
-            ));
         }
         Err(err) => {
             println!("Failed to load task: {:?}", err);
@@ -135,38 +121,9 @@ fn handle_task(opts: TaskOpts, runner: &dyn crate::transport::Transport, invento
  * non-zero.
  */
 fn handle_cmd(opts: CmdOpts, runner: &dyn crate::transport::Transport, inventory: Inventory) {
-    let mut task = Task::new("Dynamic");
+    let mut task = ExecutableTask::new(Task::new("Dynamic"), HashMap::new());
-    task.inline = Some(opts.command);
+    task.task.script.inline = Some(opts.command);
-    let parameters = HashMap::new();
+    std::process::exit(execute_task_on(opts.targets, &task, runner, &inventory));
-    std::process::exit(execute_task_on(
-        opts.targets,
-        &task,
-        &parameters,
-        runner,
-        &inventory,
-    ));
-}
-
-/**
- * render_command will handle injecting the parameters for a given command
- * into the string where appropriate, using the Handlebars syntax.
- *
- * If the template fails to render, then this will just return the command it
- * was given
- */
-fn render_command(cmd: &str, parameters: &HashMap<String, String>) -> String {
-    use handlebars::Handlebars;
-
-    let handlebars = Handlebars::new();
-    match handlebars.render_template(cmd, parameters) {
-        Ok(rendered) => {
-            return rendered;
-        }
-        Err(err) => {
-            error!("Failed to render command ({:?}): {}", err, cmd);
-            return cmd.to_string();
-        }
-    }
 }
 
 #[derive(Debug, Options)]
@@ -238,26 +195,4 @@ struct PlanOpts {
 }
 
 #[cfg(test)]
-mod tests {
+mod tests {}
-    use super::*;
-
-    #[test]
-    fn test_render_command() {
-        let cmd = "echo \"{{msg}}\"";
-        let mut params = HashMap::new();
-        params.insert("msg".to_string(), "hello".to_string());
-
-        let output = render_command(&cmd, &params);
-        assert_eq!(output, "echo \"hello\"");
-    }
-
-    #[test]
-    fn test_render_command_bad_template() {
-        let cmd = "echo \"{{msg\"";
-        let mut params = HashMap::new();
-        params.insert("msg".to_string(), "hello".to_string());
-
-        let output = render_command(&cmd, &params);
-        assert_eq!(output, "echo \"{{msg\"");
-    }
-}

cli/src/transport/mod.rs

@@ -1,4 +1,5 @@
 use crate::inventory::{Group, Inventory, Target};
+use zap_parser::plan::ExecutableTask;
 
 pub mod ssh;
 
@@ -7,6 +8,6 @@ pub mod ssh;
  * connecting to targets
  */
 pub trait Transport {
-    fn run_group(&self, cmd: &str, group: &Group, inv: &Inventory) -> i32;
+    fn run_group(&self, cmd: &ExecutableTask, group: &Group, inv: &Inventory) -> i32;
-    fn run(&self, command: &str, target: &Target) -> i32;
+    fn run(&self, command: &ExecutableTask, target: &Target) -> i32;
 }

cli/src/transport/ssh.rs

@@ -1,10 +1,16 @@
 use crate::inventory::{Group, Inventory, Target};
 use crate::transport::Transport;
 
+use log::*;
 use serde::{Deserialize, Serialize};
 use ssh2::Session;
+use std::convert::TryInto;
 use std::io::prelude::*;
+use std::io::BufReader;
 use std::net::TcpStream;
+use std::path::Path;
+
+use zap_parser::plan::ExecutableTask;
 
 #[derive(Clone, Debug, Deserialize, Serialize)]
 pub struct Ssh {}
@@ -16,7 +22,7 @@ impl Default for Ssh {
 }
 
 impl Transport for Ssh {
-    fn run_group(&self, command: &str, group: &Group, inventory: &Inventory) -> i32 {
+    fn run_group(&self, command: &ExecutableTask, group: &Group, inventory: &Inventory) -> i32 {
         let mut status = 1;
         for target_name in group.targets.iter() {
             // XXX: This is inefficient
@@ -29,7 +35,8 @@ impl Transport for Ssh {
         }
         status
     }
-    fn run(&self, command: &str, target: &Target) -> i32 {
+
+    fn run(&self, command: &ExecutableTask, target: &Target) -> i32 {
         // Connect to the local SSH server
         let tcp = TcpStream::connect(format!("{}:22", target.uri)).unwrap();
         let mut sess = Session::new().unwrap();
@@ -51,13 +58,54 @@ impl Transport for Ssh {
                 .unwrap();
         }
 
-        let mut channel = sess.channel_session().unwrap();
+        let remote_script = "._zap_command";
-        channel.exec(command).unwrap();
 
-        let mut s = String::new();
+        if let Some(script) = command.task.script.as_bytes(Some(&command.parameters)) {
-        channel.read_to_string(&mut s).unwrap();
+            let mut remote_file = sess
-        print!("{}", s);
+                .scp_send(
-        channel.wait_close().expect("Failed to close the channel");
+                    Path::new(remote_script),
-        return channel.exit_status().unwrap();
+                    0o700,
+                    script
+                        .len()
+                        .try_into()
+                        .expect("Overflow converting the size of the generated file, yikes!"),
+                    None,
+                )
+                .unwrap();
+            remote_file.write(&script).unwrap();
+            // Close the channel and wait for the whole content to be tranferred
+            remote_file.send_eof().unwrap();
+            remote_file.wait_eof().unwrap();
+            remote_file.close().unwrap();
+            remote_file.wait_close().unwrap();
+
+            let mut channel = sess.channel_session().unwrap();
+            let stderr = channel.stderr();
+
+            channel.exec(&format!("./{}", remote_script)).unwrap();
+
+            let reader = BufReader::new(stderr);
+            for line in reader.lines() {
+                println!("err: {}", line.unwrap());
+            }
+
+            let mut s = String::new();
+            channel.read_to_string(&mut s).unwrap();
+            print!("{}", s);
+            channel.wait_close().expect("Failed to close the channel");
+            let exit = channel.exit_status().unwrap();
+
+            /*
+             * This seems a little dumb and hacky, but we need to clean up the file
+             * somehow and I'm not seeing anything that would allow me to just reach
+             * out and remove a file
+             */
+            let mut channel = sess.channel_session().unwrap();
+            channel.exec(&format!("rm -f {}", remote_script)).unwrap();
+            return exit;
+        } else {
+            error!("No script available to run for task!");
+            return -1;
+        }
     }
 }

parser/Cargo.toml

@@ -5,6 +5,7 @@ authors = ["R. Tyler Croy <rtyler@brokenco.de>"]
 edition = "2018"
 
 [dependencies]
+handlebars = "~3.5"
 log = "0.4"
 pest = "~2.1"
 pest_derive = "~2.1"

parser/src/plan.pest

@@ -26,11 +26,16 @@ identifier = { ASCII_ALPHA ~ (ASCII_ALPHANUMERIC | "_")* }
 opening_brace = _{ "{" }
 closing_brace = _{ "}" }
 equals        = _{ "=" }
-quote         = _{ "\"" }
+quote         = _{ "'" }
+triple_quote  = _{ "'''" }
+
+string        = { triple_quoted | single_quoted }
+single_quoted = ${ (quote ~ inner_single_str ~ quote) }
+inner_single_str = @{ (!(quote | "\\") ~ ANY)* ~ (escape ~ inner_single_str)? }
+
+triple_quoted = ${ (triple_quote ~ inner_triple_str ~ triple_quote) }
+inner_triple_str = @{ (!(triple_quote | "\\") ~ ANY)* ~ (escape ~ inner_triple_str)? }
 
-string        = { double_quoted }
-double_quoted = ${ (quote ~ inner_double_str ~ quote) }
-inner_double_str = @{ (!("\"" | "\\") ~ ANY)* ~ (escape ~ inner_double_str)? }
 escape    = @{ "\\" ~ ("\"" | "\\" | "r" | "n" | "t" | "0" | "'" | code | unicode) }
 code      = @{ "x" ~ hex_digit{2} }
 unicode   = @{ "u" ~ opening_brace ~ hex_digit{2, 6} ~ closing_brace }

parser/src/plan.rs

@@ -17,7 +17,7 @@ pub struct ExecutableTask {
 }
 
 impl ExecutableTask {
-    fn new(task: crate::task::Task, parameters: HashMap<String, String>) -> Self {
+    pub fn new(task: crate::task::Task, parameters: HashMap<String, String>) -> Self {
         Self { task, parameters }
     }
 }
@@ -138,10 +138,10 @@ fn parse_str(parser: &mut Pairs<Rule>) -> Result<String, PestError<Rule>> {
             Rule::string => {
                 return parse_str(&mut parsed.into_inner());
             }
-            Rule::double_quoted => {
+            Rule::single_quoted => {
                 return parse_str(&mut parsed.into_inner());
             }
-            Rule::inner_double_str => {
+            Rule::inner_single_str => {
                 return Ok(parsed.as_str().to_string());
             }
             _ => {}
@@ -168,12 +168,12 @@ mod tests {
  * It is expected to be run from the root of the project tree.
  */
 
-task "../tasks/echo.ztask" {
+task '../tasks/echo.ztask' {
-    msg = "Hello from the wonderful world of zplans!"
+    msg = 'Hello from the wonderful world of zplans!'
 }
 
-task "../tasks/echo.ztask" {
+task '../tasks/echo.ztask' {
-    msg = "This can actually take inline shells too: $(date)"
+    msg = 'This can actually take inline shells too: $(date)'
 }"#;
         let _plan = PlanParser::parse(Rule::planfile, buf)
             .unwrap()
@@ -183,12 +183,12 @@ task "../tasks/echo.ztask" {
 
     #[test]
     fn parse_plan_fn() {
-        let buf = r#"task "../tasks/echo.ztask" {
+        let buf = r#"task '../tasks/echo.ztask' {
-                        msg = "Hello from the wonderful world of zplans!"
+                        msg = 'Hello from the wonderful world of zplans!'
                     }
 
-                    task "../tasks/echo.ztask" {
+                    task '../tasks/echo.ztask' {
-                        msg = "This can actually take inline shells too: $(date)"
+                        msg = 'This can actually take inline shells too: $(date)'
                     }"#;
         let plan = Plan::from_str(buf).expect("Failed to parse the plan");
         assert_eq!(plan.tasks.len(), 2);

parser/src/task.pest

@@ -32,10 +32,11 @@ ptype = { "type" ~ equals ~ typedef }
 
 script = { "script"
           ~ opening_brace
-          ~ (script_inline)
+          ~ (script_inline | script_file)
           ~ closing_brace
           }
-script_inline = _{ "inline" ~ equals ~ string }
+script_inline = { "inline" ~ equals ~ string }
+script_file   = { "file" ~ equals ~ string }
 
 
 
@@ -49,11 +50,16 @@ identifier = { ASCII_ALPHA ~ (ASCII_ALPHANUMERIC | "_")* }
 opening_brace = _{ "{" }
 closing_brace = _{ "}" }
 equals        = _{ "=" }
-quote         = _{ "\"" }
+quote         = _{ "'" }
+triple_quote  = _{ "'''" }
+
+string        = { triple_quoted | single_quoted }
+single_quoted = ${ (quote ~ inner_single_str ~ quote) }
+inner_single_str = @{ (!(quote | "\\") ~ ANY)* ~ (escape ~ inner_single_str)? }
+
+triple_quoted = ${ (triple_quote ~ inner_triple_str ~ triple_quote) }
+inner_triple_str = @{ (!(triple_quote | "\\") ~ ANY)* ~ (escape ~ inner_triple_str)? }
 
-string        = { double_quoted }
-double_quoted = ${ (quote ~ inner_double_str ~ quote) }
-inner_double_str = @{ (!("\"" | "\\") ~ ANY)* ~ (escape ~ inner_double_str)? }
 escape    = @{ "\\" ~ ("\"" | "\\" | "r" | "n" | "t" | "0" | "'" | code | unicode) }
 code      = @{ "x" ~ hex_digit{2} }
 unicode   = @{ "u" ~ opening_brace ~ hex_digit{2, 6} ~ closing_brace }

parser/src/task.rs

@@ -1,33 +1,121 @@
+use log::*;
 use pest::error::Error as PestError;
 use pest::error::ErrorVariant;
 use pest::iterators::Pairs;
 use pest::Parser;
+use std::collections::HashMap;
+use std::fs::File;
+use std::io::Read;
 use std::path::PathBuf;
 
 #[derive(Parser)]
 #[grammar = "task.pest"]
 struct TaskParser;
 
+/**
+ * A Script represents something that can be executed oa a remote host.
+ *
+ * These come in two variants:
+ *   - Inline string of shell commands to run
+ *   - A script or binary file to transfer and execute
+ */
+#[derive(Clone, Debug)]
+pub struct Script {
+    /**
+     * Inline scripts will have parameters rendered into then with handlebars syntax
+     */
+    pub inline: Option<String>,
+    /**
+     * File scripts will be executed with the parameters passed as command line
+     * arguments, e.g. the "msg" parameter would be passed as:
+     *      ./file --msg=value
+     */
+    pub file: Option<PathBuf>,
+}
+
+impl Script {
+    fn new() -> Self {
+        Self {
+            inline: None,
+            file: None,
+        }
+    }
+
+    /**
+     * Return the script's contents as bytes
+     *
+     * This is useful for transferring the script to another host for execution
+     *
+     * If the `file` member is defined, that will be preferred, even if `inline` is also defined
+     */
+    pub fn as_bytes(&self, parameters: Option<&HashMap<String, String>>) -> Option<Vec<u8>> {
+        use handlebars::Handlebars;
+
+        if self.inline.is_some() && self.file.is_some() {
+            warn!("Both inline and file structs are defined for this script, only file will be used!\n({})",
+            self.inline.as_ref().unwrap());
+        }
+
+        if let Some(path) = &self.file {
+            match File::open(path) {
+                Ok(mut file) => {
+                    let mut buf = vec![];
+
+                    if let Ok(count) = file.read_to_end(&mut buf) {
+                        debug!("Read {} bytes of {}", count, path.display());
+                        return Some(buf);
+                    } else {
+                        error!("Failed to read the file {}", path.display());
+                    }
+                }
+                Err(err) => {
+                    error!("Failed to open the file at {},  {:?}", path.display(), err);
+                }
+            }
+        }
+
+        if let Some(inline) = &self.inline {
+            // Early exit if there are no parameters to render
+            if parameters.is_none() {
+                return Some(inline.as_bytes().to_vec());
+            }
+
+            let parameters = parameters.unwrap();
+
+            let handlebars = Handlebars::new();
+            match handlebars.render_template(inline, &parameters) {
+                Ok(rendered) => {
+                    return Some(rendered.as_bytes().to_vec());
+                }
+                Err(err) => {
+                    error!("Failed to render command ({:?}): {}", err, inline);
+                    return Some(inline.as_bytes().to_vec());
+                }
+            }
+        }
+
+        None
+    }
+}
+
 #[derive(Clone, Debug)]
 pub struct Task {
     pub name: String,
-    pub inline: Option<String>,
+    pub script: Script,
 }
 
 impl Task {
-    pub fn get_script(&self) -> Option<&String> {
-        self.inline.as_ref()
-    }
-
     pub fn new(name: &str) -> Self {
         Task {
             name: name.to_string(),
-            inline: None,
+            script: Script::new(),
         }
     }
 
     fn parse(parser: &mut Pairs<Rule>) -> Result<Self, PestError<Rule>> {
         let mut task: Option<Self> = None;
+        let mut inline = None;
+        let mut file = None;
 
         while let Some(parsed) = parser.next() {
             match parsed.as_rule() {
@@ -35,17 +123,27 @@ impl Task {
                     task = Some(Task::new(parsed.as_str()));
                 }
                 Rule::script => {
-                    let script = parse_str(&mut parsed.into_inner())?;
+                    for pair in parsed.into_inner() {
-
+                        match pair.as_rule() {
-                    if let Some(ref mut task) = task {
+                            Rule::script_inline => {
-                        task.inline = Some(script);
+                                inline = Some(parse_str(&mut pair.into_inner())?);
+                            }
+                            Rule::script_file => {
+                                let path = parse_str(&mut pair.into_inner())?;
+                                file = Some(PathBuf::from(path));
+                            }
+                            _ => {}
+                        }
                     }
                 }
                 _ => {}
             }
         }
 
-        if let Some(task) = task {
+        if let Some(mut task) = task {
+            task.script.inline = inline;
+            task.script.file = file;
+
             return Ok(task);
         } else {
             return Err(PestError::new_from_pos(
@@ -78,9 +176,6 @@ impl Task {
     }
 
     pub fn from_path(path: &PathBuf) -> Result<Self, PestError<Rule>> {
-        use std::fs::File;
-        use std::io::Read;
-
         match File::open(path) {
             Ok(mut file) => {
                 let mut contents = String::new();
@@ -118,10 +213,16 @@ fn parse_str(parser: &mut Pairs<Rule>) -> Result<String, PestError<Rule>> {
             Rule::string => {
                 return parse_str(&mut parsed.into_inner());
             }
-            Rule::double_quoted => {
+            Rule::triple_quoted => {
                 return parse_str(&mut parsed.into_inner());
             }
-            Rule::inner_double_str => {
+            Rule::single_quoted => {
+                return parse_str(&mut parsed.into_inner());
+            }
+            Rule::inner_single_str => {
+                return Ok(parsed.as_str().to_string());
+            }
+            Rule::inner_triple_str => {
                 return Ok(parsed.as_str().to_string());
             }
             _ => {}
@@ -146,12 +247,12 @@ mod tests {
                 parameters {
                     package {
                         required = true
-                        help = "Name of package to be installed"
+                        help = 'Name of package to be installed'
                         type = string
                     }
                 }
                 script {
-                    inline = "zypper in -y ${ZAP_PACKAGE}"
+                    inline = 'zypper in -y {{package}}'
                 }
             }"#;
         let _task = TaskParser::parse(Rule::task, buf).unwrap().next().unwrap();
@@ -161,7 +262,7 @@ mod tests {
     fn parse_no_parameters() {
         let buf = r#"task PrintEnv {
                 script {
-                    inline = "env"
+                    inline = 'env'
                 }
             }"#;
         let _task = TaskParser::parse(Rule::task, buf).unwrap().next().unwrap();
@@ -171,11 +272,28 @@ mod tests {
     fn parse_task_fn() {
         let buf = r#"task PrintEnv {
                 script {
-                    inline = "env"
+                    inline = 'env'
                 }
             }"#;
         let task = Task::from_str(buf).expect("Failed to parse the task");
         assert_eq!(task.name, "PrintEnv");
-        assert_eq!(task.get_script().unwrap(), "env");
+
+        let script = task.script;
+
+        assert_eq!(script.as_bytes(None).unwrap(), "env".as_bytes());
+    }
+
+    #[test]
+    fn parse_task_fn_with_triple_quotes() {
+        let buf = r#"task PrintEnv {
+                script {
+                    inline = '''env'''
+                }
+            }"#;
+        let task = Task::from_str(buf).expect("Failed to parse the task");
+        assert_eq!(task.name, "PrintEnv");
+
+        let script = task.script;
+        assert_eq!(script.as_bytes(None).unwrap(), "env".as_bytes());
     }
 }

tasks/echo.ztask

@@ -2,12 +2,12 @@ task Echo {
     parameters {
         msg {
             required = true
-            help = "String to echo back to the client"
+            help = 'String to echo back to the client'
             type = string
         }
     }
 
     script {
-        inline = "echo {{msg}}"
+        inline = 'echo "{{msg}}"'
     }
 }

tasks/shell/bash.ztask

@@ -0,0 +1,15 @@
+task Bash {
+    parameters {
+        script {
+            required = true
+            help = 'A script to run via the bash shell (assumes bash is in the defult PATH)'
+            type = string
+        }
+    }
+
+    script {
+        inline = '''#!/usr/bin/env bash
+            {{script}}
+        '''
+    }
+}