feat(rust): improve parsing of node config files

implementations/rust/ockam/ockam_command/Cargo.toml

@@ -110,6 +110,7 @@ mockito = "1.5.0"
 ockam_api = { path = "../ockam_api", version = "0.82.0", default-features = false, features = ["test-utils"] }
 ockam_macros = { path = "../ockam_macros", version = "^0.35.0" }
 proptest = "1.5.0"
+serial_test = "3.0.0"
 tempfile = "3.10.1"
 time = { version = "0.3", default-features = false, features = ["std", "local-offset"] }
 

implementations/rust/ockam/ockam_command/src/node/create.rs

@@ -195,6 +195,11 @@ impl Command for CreateCommand {
 impl CreateCommand {
     /// Return true if the command should be run in config mode
     fn should_run_config(&self) -> bool {
+        // Ignore the config args if it's a child process (i.e. only the top level process can run the config)
+        if self.foreground_args.child_process {
+            return false;
+        }
+
         let name_arg_is_a_config = !self.has_name_arg();
 
         let no_config_args = !name_arg_is_a_config

implementations/rust/ockam/ockam_command/src/node/create/config.rs

@@ -72,7 +72,7 @@ impl CreateCommand {
     /// or read the inline configuration
     #[instrument(skip_all, fields(app.event.command.configuration_file))]
     pub async fn get_node_config(&self) -> miette::Result<NodeConfig> {
-        let mut contents = match self.config_args.configuration.clone() {
+        let contents = match self.config_args.configuration.clone() {
             Some(contents) => contents,
             None => match parse_path_or_url(&self.name).await {
                 Ok(contents) => contents,
@@ -95,7 +95,7 @@ impl CreateCommand {
             std::env::set_var(key, value);
         }
         // Parse the configuration
-        let node_config = NodeConfig::parse(&mut contents)?;
+        let node_config = NodeConfig::parse(contents.clone())?;
         // Record the configuration contents if the node configuration was successfully parsed
         Span::current().record(
             APPLICATION_EVENT_COMMAND_CONFIGURATION_FILE.as_str(),
@@ -132,8 +132,8 @@ pub struct NodeConfig {
 }
 
 impl NodeConfig {
-    fn parse(contents: &mut String) -> miette::Result<Self> {
-        ConfigParser::parse(contents)
+    fn parse(mut contents: String) -> miette::Result<Self> {
+        ConfigParser::parse(&mut contents)
     }
 
     /// Merge the arguments of the node defined in the config with the arguments from the
@@ -403,8 +403,8 @@ mod tests {
         for file in files {
             let file = file.unwrap();
             let path = file.path();
-            let mut contents = std::fs::read_to_string(&path).unwrap();
-            let res = NodeConfig::parse(&mut contents);
+            let contents = std::fs::read_to_string(&path).unwrap();
+            let res = NodeConfig::parse(contents);
             res.unwrap();
         }
     }
@@ -463,8 +463,8 @@ mod tests {
         };
 
         // No node config, cli args should be used
-        let mut contents = String::new();
-        let mut config = NodeConfig::parse(&mut contents).unwrap();
+        let contents = String::new();
+        let mut config = NodeConfig::parse(contents).unwrap();
         config.merge(&cli_args).unwrap();
         let node = config.node.into_parsed_commands().unwrap().pop().unwrap();
         assert_eq!(node.tcp_listener_address, "127.0.0.1:1234");
@@ -476,15 +476,15 @@ mod tests {
         // Config used, cli args should override the overlapping args
         let config_enrollment_ticket = ExportedEnrollmentTicket::new_test();
         let config_enrollment_ticket_encoded = config_enrollment_ticket.to_string();
-        std::env::set_var("ENROLLMENT_TICKET", config_enrollment_ticket_encoded);
+        std::env::set_var(ENROLLMENT_TICKET, config_enrollment_ticket_encoded);
 
-        let mut contents = r#"
+        let contents = r#"
             ticket: $ENROLLMENT_TICKET
             name: n1
             tcp-listener-address: 127.0.0.1:5555
         "#
         .to_string();
-        let mut config = NodeConfig::parse(&mut contents).unwrap();
+        let mut config = NodeConfig::parse(contents).unwrap();
         config.merge(&cli_args).unwrap();
         let node = config.node.into_parsed_commands().unwrap().pop().unwrap();
         assert_eq!(node.name, "n1");

implementations/rust/ockam/ockam_command/src/run/config.rs

@@ -77,29 +77,29 @@ impl Config {
     pub async fn parse_and_run(
         ctx: &Context,
         opts: CommandGlobalOpts,
-        contents: &mut String,
+        contents: String,
     ) -> miette::Result<()> {
         Self::parse(contents)?.run(ctx, &opts).await
     }
 
-    pub(crate) fn parse(contents: &mut String) -> miette::Result<Self> {
-        ConfigParser::parse(contents)
+    pub(crate) fn parse(mut contents: String) -> miette::Result<Self> {
+        ConfigParser::parse(&mut contents)
     }
 }
 
 #[cfg(test)]
 mod tests {
-    use std::collections::BTreeMap;
-    use std::path::PathBuf;
-
+    use super::*;
+    use crate::node::config::ENROLLMENT_TICKET;
     use crate::run::parser::building_blocks::*;
     use crate::run::parser::VersionValue;
-
-    use super::*;
+    use serial_test::serial;
+    use std::collections::BTreeMap;
+    use std::path::PathBuf;
 
     #[test]
     fn parse_complete_config() {
-        let mut config = r#"
+        let config = r#"
             vaults:
               - v1
               - v2
@@ -152,7 +152,7 @@ mod tests {
               - r2
         "#
         .to_string();
-        let parsed = Config::parse(&mut config).unwrap();
+        let parsed = Config::parse(config).unwrap();
 
         let expected = Config {
             version: Version {
@@ -294,21 +294,22 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn resolve_variables() {
         std::env::set_var("SUFFIX", "node");
-        let mut config = r#"
+        let config = r#"
             variables:
               prefix: ockam
-              ticket_path: ./path/to/ticket
+              ENROLLMENT_TICKET: ./path/to/ticket
 
-            ticket: ${ticket_path}
+            ticket: ${ENROLLMENT_TICKET}
 
             nodes:
               - ${prefix}_n1_${SUFFIX}
               - ${prefix}_n2_${SUFFIX}
         "#
         .to_string();
-        let parsed = Config::parse(&mut config).unwrap();
+        let parsed = Config::parse(config).unwrap();
         let expected = Config {
             version: Version {
                 version: VersionValue::latest(),
@@ -335,14 +336,18 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn parse_demo_config_files() {
-        std::env::set_var("ENROLLMENT_TICKET", "ticket");
-        let files = std::fs::read_dir(demo_config_files_dir()).unwrap();
+        let files = std::fs::read_dir(demo_config_files_dir())
+            .unwrap()
+            .collect::<Vec<_>>();
+        assert_eq!(files.len(), 7);
         for file in files {
+            std::env::set_var(ENROLLMENT_TICKET, "ticket");
             let file = file.unwrap();
             let path = file.path();
-            let mut contents = std::fs::read_to_string(&path).unwrap();
-            match Config::parse(&mut contents) {
+            let contents = std::fs::read_to_string(&path).unwrap();
+            match Config::parse(contents) {
                 Ok(_) => {}
                 Err(e) => {
                     eprintln!("Error parsing file {path:?}: {e}");
@@ -353,10 +358,11 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn parse_demo_config_file_1() {
         let path = demo_config_files_dir().join("1.portal.single-machine.yaml");
-        let mut config = std::fs::read_to_string(path).unwrap();
-        let parsed = Config::parse(&mut config).unwrap();
+        let config = std::fs::read_to_string(path).unwrap();
+        let parsed = Config::parse(config).unwrap();
         assert_eq!(parsed.version.version, VersionValue::latest());
         assert_eq!(parsed.vaults.vaults, None);
         assert_eq!(parsed.identities.identities, None);
@@ -398,10 +404,11 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn parse_demo_config_file_2_inlet() {
         let path = demo_config_files_dir().join("2.portal.inlet.yaml");
-        let mut config = std::fs::read_to_string(path).unwrap();
-        let parsed = Config::parse(&mut config).unwrap();
+        let config = std::fs::read_to_string(path).unwrap();
+        let parsed = Config::parse(config).unwrap();
         assert_eq!(parsed.version.version, VersionValue::latest());
         assert_eq!(parsed.vaults.vaults, None);
         assert_eq!(parsed.identities.identities, None);
@@ -440,10 +447,11 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn parse_demo_config_file_2_outlet() {
         let path = demo_config_files_dir().join("2.portal.outlet.yaml");
-        let mut config = std::fs::read_to_string(path).unwrap();
-        let parsed = Config::parse(&mut config).unwrap();
+        let config = std::fs::read_to_string(path).unwrap();
+        let parsed = Config::parse(config).unwrap();
         assert_eq!(parsed.version.version, VersionValue::latest());
         assert_eq!(parsed.vaults.vaults, None);
         assert_eq!(parsed.identities.identities, None);

implementations/rust/ockam/ockam_command/src/run/mod.rs

@@ -46,7 +46,7 @@ impl RunCommand {
 
     #[instrument(skip_all, fields(app.event.command.configuration_file))]
     async fn async_run(&self, ctx: &Context, opts: CommandGlobalOpts) -> miette::Result<()> {
-        let mut contents = match &self.inline {
+        let contents = match &self.inline {
             Some(contents) => contents.to_string(),
             None => {
                 let path = match &self.recipe {
@@ -82,6 +82,6 @@ impl RunCommand {
             APPLICATION_EVENT_COMMAND_CONFIGURATION_FILE.as_str(),
             &contents,
         );
-        Config::parse_and_run(ctx, opts, &mut contents).await
+        Config::parse_and_run(ctx, opts, contents).await
     }
 }

implementations/rust/ockam/ockam_command/src/run/parser/building_blocks.rs

@@ -330,8 +330,8 @@ pub fn as_command_args(args: BTreeMap<ArgKey, ArgValue>) -> Vec<String> {
 }
 
 /// Return the command representation of the argument name and its value.
-fn as_command_arg(key: ArgKey, values: ArgValue) -> Vec<String> {
-    match values {
+fn as_command_arg(key: ArgKey, value: ArgValue) -> Vec<String> {
+    match value {
         ArgValue::List(values) => values
             .into_iter()
             .flat_map(|value| as_command_arg(key.clone(), value))

implementations/rust/ockam/ockam_command/src/run/parser/config.rs

@@ -8,8 +8,8 @@ pub struct ConfigParser;
 
 impl ConfigParser {
     pub fn parse<'de, T: Deserialize<'de>>(contents: &'de mut String) -> miette::Result<T> {
-        // Resolve the environment variables section
-        Variables::resolve(contents)?;
+        // Expand the environment variables section
+        Variables::expand(contents)?;
 
         // Parse the configuration file as the given T type
         serde_yaml::from_str(contents)
@@ -30,9 +30,9 @@ impl ConfigParser {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use crate::run::parser::building_blocks::ArgsToCommands;
     use crate::run::parser::resource::{Nodes, Relays};
     use serde::Serialize;
+    use serial_test::serial;
 
     #[derive(Debug, PartialEq, Serialize, Deserialize)]
     pub struct TestConfig {
@@ -43,40 +43,45 @@ mod tests {
     }
 
     #[test]
+    #[serial]
     fn parse_yaml_config() {
+        std::env::set_var("NODE_NAME", "node1");
         let mut contents = r#"
             variables:
-              var_b: true
+              var_s: node2
               var_i: 1 # comment
             nodes:
               # comment
-              - node1
-              - node2
+              - $NODE_NAME
+              - $var_s
             relays:
               r1:
                 at: /project/default
                 to: outlet-node
         "#
         .to_string();
 
-        let result = ConfigParser::parse::<TestConfig>(&mut contents).unwrap();
-        assert_eq!(std::env::var("var_b").unwrap(), "true");
-        assert_eq!(std::env::var("var_i").unwrap(), "1");
-        assert_eq!(result.nodes.nodes.unwrap().len(), 2);
+        let parsed = ConfigParser::parse::<TestConfig>(&mut contents).unwrap();
+        let nodes = parsed.nodes.into_parsed_commands().unwrap();
+        assert_eq!(nodes.len(), 2);
+        assert_eq!(nodes[0].name, "node1");
+        assert_eq!(nodes[1].name, "node2");
     }
 
     #[test]
+    #[serial]
     fn parse_json_config() {
+        std::env::set_var("NODE_NAME", "node1");
         let mut contents = r#"
             {
                 # comment
                 "variables": {
-                    "var_b": true,
+                    "var_s": "node2",
                     "var_i": 1, # trailing commas are supported
                 },
                 "nodes": [
-                    "node1",
-                    "node2",
+                    "$NODE_NAME",
+                    "$var_s",
                 ],
                 "relays": {
                     "r1": {
@@ -88,9 +93,10 @@ mod tests {
             }
         "#
         .to_string();
-        let result = ConfigParser::parse::<TestConfig>(&mut contents).unwrap();
-        assert_eq!(std::env::var("var_b").unwrap(), "true");
-        assert_eq!(std::env::var("var_i").unwrap(), "1");
-        assert_eq!(result.nodes.nodes.unwrap().len(), 2);
+        let parsed = ConfigParser::parse::<TestConfig>(&mut contents).unwrap();
+        let nodes = parsed.nodes.into_parsed_commands().unwrap();
+        assert_eq!(nodes.len(), 2);
+        assert_eq!(nodes[0].name, "node1");
+        assert_eq!(nodes[1].name, "node2");
     }
 }

implementations/rust/ockam/ockam_command/src/run/parser/resource/node.rs

@@ -7,7 +7,6 @@ use serde::{Deserialize, Serialize};
 use crate::node::CreateCommand;
 use crate::run::parser::building_blocks::{as_command_args, ArgKey, ArgValue};
 
-use crate::node::config::ENROLLMENT_TICKET;
 use crate::run::parser::resource::utils::parse_cmd_from_args;
 use crate::run::parser::resource::Resource;
 use crate::{node, Command, OckamSubcommand};
@@ -118,15 +117,15 @@ impl Node {
     }
 
     pub fn into_parsed_commands(self) -> Result<Vec<CreateCommand>> {
-        // Unset the `ENROLLMENT_TICKET` env var, so that the `node create` command
-        // doesn't try to run in config mode again.
-        std::env::remove_var(ENROLLMENT_TICKET);
         Ok(vec![Self::get_subcommand(&self.args())?])
     }
 
     fn get_subcommand(args: &[String]) -> Result<CreateCommand> {
         if let OckamSubcommand::Node(cmd) = parse_cmd_from_args(CreateCommand::NAME, args)? {
-            if let node::NodeSubcommand::Create(c) = cmd.subcommand {
+            if let node::NodeSubcommand::Create(mut c) = cmd.subcommand {
+                c.config_args.configuration = None;
+                c.config_args.variables = vec![];
+                c.config_args.enrollment_ticket = None;
                 return Ok(c);
             }
         }