From b27d3ffc654b0e97bbb05ca5bdb655c5bec793fd Mon Sep 17 00:00:00 2001 From: Adrian Benavides Date: Tue, 29 Oct 2024 09:20:08 +0100 Subject: [PATCH] feat(rust): first argument of `node create` can contain an inline configuration --- .../ockam/ockam_command/src/node/create.rs | 13 ++- .../ockam_command/src/node/create/config.rs | 8 +- .../ockam_command/src/run/parser/config.rs | 27 ++++++ .../src/run/parser/resource/node.rs | 2 + .../src/run/parser/resource/nodes.rs | 3 +- .../ockam/ockam_command/src/value_parsers.rs | 25 +++++- .../ockam_command/tests/bats/local/nodes.bats | 18 ++++ .../tests/bats/orchestrator_enroll/nodes.bats | 90 +++++++++++++++++-- 8 files changed, 170 insertions(+), 16 deletions(-) diff --git a/implementations/rust/ockam/ockam_command/src/node/create.rs b/implementations/rust/ockam/ockam_command/src/node/create.rs index e58d38e6c84..5a4869e21fa 100644 --- a/implementations/rust/ockam/ockam_command/src/node/create.rs +++ b/implementations/rust/ockam/ockam_command/src/node/create.rs @@ -18,6 +18,7 @@ use ockam_api::{fmt_log, fmt_ok}; use ockam_core::{opentelemetry_context_parser, OpenTelemetryContext}; use ockam_node::Context; +use crate::node::config::NodeConfig; use crate::node::create::config::ConfigArgs; use crate::node::util::NodeManagerDefaults; use crate::service::config::Config; @@ -127,6 +128,7 @@ impl Default for CreateCommand { configuration: None, enrollment_ticket: None, variables: vec![], + started_from_configuration: false, }, tcp_listener_address: node_manager_defaults.tcp_listener_address, http_server: false, @@ -200,6 +202,11 @@ impl CreateCommand { return false; } + // Ignore the config args if the node was started from a configuration + if self.config_args.started_from_configuration { + return false; + } + let name_arg_is_a_config = !self.has_name_arg(); let no_config_args = !name_arg_is_a_config @@ -216,12 +223,14 @@ impl CreateCommand { || self.config_args.enrollment_ticket.is_some() } - /// Return true if the `name` argument is not a config file path or URL + /// Return true if the `name` argument is not a URL, a file path, or an inline config fn has_name_arg(&self) -> bool { + let is_url = is_url(&self.name).is_some(); let is_file = std::fs::metadata(&self.name) .map(|m| m.is_file()) .unwrap_or(false); - is_url(&self.name).is_none() && !is_file + let is_inline_config = serde_yaml::from_str::(&self.name).is_ok(); + !is_url && !is_file && !is_inline_config } fn parse_args(&self) -> miette::Result<()> { diff --git a/implementations/rust/ockam/ockam_command/src/node/create/config.rs b/implementations/rust/ockam/ockam_command/src/node/create/config.rs index 3269b51e028..b07bcbff9c6 100644 --- a/implementations/rust/ockam/ockam_command/src/node/create/config.rs +++ b/implementations/rust/ockam/ockam_command/src/node/create/config.rs @@ -3,7 +3,7 @@ use crate::node::CreateCommand; use crate::run::parser::config::ConfigParser; use crate::run::parser::resource::*; use crate::run::parser::Version; -use crate::value_parsers::{parse_key_val, parse_path_or_url}; +use crate::value_parsers::{parse_config_or_path_or_url, parse_key_val}; use crate::CommandGlobalOpts; use clap::Args; use miette::{miette, IntoDiagnostic}; @@ -36,6 +36,10 @@ pub struct ConfigArgs { /// Example: `--variable KEY1=VALUE1 --variable KEY2=VALUE2` #[arg(long = "variable", value_name = "VARIABLE", value_parser = parse_key_val::)] pub variables: Vec<(String, String)>, + + /// A flag used internally to indicate that the node was started from a configuration file. + #[arg(hide = true, long)] + pub started_from_configuration: bool, } impl CreateCommand { @@ -74,7 +78,7 @@ impl CreateCommand { pub async fn get_node_config(&self) -> miette::Result { let contents = match self.config_args.configuration.clone() { Some(contents) => contents, - None => match parse_path_or_url(&self.name).await { + None => match parse_config_or_path_or_url::(&self.name).await { Ok(contents) => contents, Err(err) => { // If just the enrollment ticket is passed, create a minimal configuration diff --git a/implementations/rust/ockam/ockam_command/src/run/parser/config.rs b/implementations/rust/ockam/ockam_command/src/run/parser/config.rs index ce88110a9e6..d73dbd2dd0e 100644 --- a/implementations/rust/ockam/ockam_command/src/run/parser/config.rs +++ b/implementations/rust/ockam/ockam_command/src/run/parser/config.rs @@ -99,4 +99,31 @@ mod tests { assert_eq!(nodes[0].name, "node1"); assert_eq!(nodes[1].name, "node2"); } + + #[test] + fn parse_without_variables_section() { + let mut contents = r#" + { + "nodes": [ + "node1", + "node2", + ], + } + "# + .to_string(); + let parsed = ConfigParser::parse::(&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] + fn parse_from_inline_yaml() { + let mut contents = "relay: r1".to_string(); + let parsed = ConfigParser::parse::(&mut contents).unwrap(); + let nodes = parsed.relays.into_parsed_commands(None).unwrap(); + assert_eq!(nodes.len(), 1); + assert_eq!(nodes[0].relay_name, "r1"); + } } diff --git a/implementations/rust/ockam/ockam_command/src/run/parser/resource/node.rs b/implementations/rust/ockam/ockam_command/src/run/parser/resource/node.rs index cda53068a8f..d2282230e18 100644 --- a/implementations/rust/ockam/ockam_command/src/run/parser/resource/node.rs +++ b/implementations/rust/ockam/ockam_command/src/run/parser/resource/node.rs @@ -40,6 +40,7 @@ impl Resource for Node { fn args(self) -> Vec { let mut args: BTreeMap = BTreeMap::new(); + args.insert("started-from-configuration".into(), true.into()); if let Some(name) = self.name { args.insert("name".into(), name); } @@ -126,6 +127,7 @@ impl Node { c.config_args.configuration = None; c.config_args.variables = vec![]; c.config_args.enrollment_ticket = None; + c.config_args.started_from_configuration = true; return Ok(c); } } diff --git a/implementations/rust/ockam/ockam_command/src/run/parser/resource/nodes.rs b/implementations/rust/ockam/ockam_command/src/run/parser/resource/nodes.rs index d57eb8f6e08..19c52bd3048 100644 --- a/implementations/rust/ockam/ockam_command/src/run/parser/resource/nodes.rs +++ b/implementations/rust/ockam/ockam_command/src/run/parser/resource/nodes.rs @@ -17,7 +17,8 @@ pub struct Nodes { impl Nodes { fn get_subcommand(args: &[String]) -> Result { 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.started_from_configuration = true; return Ok(c); } } diff --git a/implementations/rust/ockam/ockam_command/src/value_parsers.rs b/implementations/rust/ockam/ockam_command/src/value_parsers.rs index f47ccfe63fa..2a21ce62e2a 100644 --- a/implementations/rust/ockam/ockam_command/src/value_parsers.rs +++ b/implementations/rust/ockam/ockam_command/src/value_parsers.rs @@ -2,6 +2,7 @@ use crate::util::parsers::hostname_parser; use crate::CommandGlobalOpts; use miette::{miette, Context, IntoDiagnostic}; use ockam_api::cli_state::{EnrollmentTicket, ExportedEnrollmentTicket, LegacyEnrollmentTicket}; +use serde::Deserialize; use std::str::FromStr; use tracing::trace; use url::Url; @@ -47,13 +48,31 @@ pub async fn parse_enrollment_ticket( .await?) } -async fn parse_string_or_path_or_url(value: &str) -> miette::Result { +pub(crate) async fn parse_config_or_path_or_url<'de, T: Deserialize<'de>>( + value: &'de str, +) -> miette::Result { + match parse_path_or_url(value).await { + Ok(contents) => Ok(contents), + Err(_) => { + if serde_yaml::from_str::(value).is_ok() { + Ok(value.to_string()) + } else { + Err(miette!( + "Failed to parse value {} as a path, URL or configuration", + value + )) + } + } + } +} + +pub(crate) async fn parse_string_or_path_or_url(value: &str) -> miette::Result { parse_path_or_url(value) .await .or_else(|_| Ok(value.to_string())) } -pub async fn parse_path_or_url(value: &str) -> miette::Result { +pub(crate) async fn parse_path_or_url(value: &str) -> miette::Result { // If the URL is valid, download the contents if let Some(url) = is_url(value) { reqwest::get(url) @@ -65,7 +84,7 @@ pub async fn parse_path_or_url(value: &str) -> miette::Result { .into_diagnostic() .context("Failed to read contents from downloaded file") } - // If not, try to read the contents from a file + // Try to read the contents from a file else if tokio::fs::metadata(value).await.is_ok() { tokio::fs::read_to_string(value) .await diff --git a/implementations/rust/ockam/ockam_command/tests/bats/local/nodes.bats b/implementations/rust/ockam/ockam_command/tests/bats/local/nodes.bats index e23e5ff8a45..32ac5980726 100644 --- a/implementations/rust/ockam/ockam_command/tests/bats/local/nodes.bats +++ b/implementations/rust/ockam/ockam_command/tests/bats/local/nodes.bats @@ -119,6 +119,20 @@ teardown() { run_success $OCKAM node show n --output json assert_output --partial "\"name\": \"n\"" assert_output --partial "127.0.0.1:5432" + + run_success "$OCKAM" node create "{name: o, tcp-outlets: {db-outlet: {to: 5433, at: o}}}" + run_success $OCKAM node show o --output json + assert_output --partial "\"name\": \"o\"" + assert_output --partial "127.0.0.1:5433" + + run_success "$OCKAM" node create "name: p" + run_success $OCKAM node show p --output json + assert_output --partial "\"name\": \"p\"" + + run_success "$OCKAM" node create "name: q" --foreground & + sleep 3 + run_success $OCKAM node show q --output json + assert_output --partial "\"name\": \"q\"" } @test "node - node in foreground with configuration is deleted if something fails" { @@ -127,6 +141,10 @@ teardown() { run_failure "$OCKAM" node create --configuration "{name: n, tcp-outlets: {db-outlet: {to: \"localhost:65536\"}}}" run_success $OCKAM node show n --output json assert_output --partial "[]" + + run_failure "$OCKAM" node create "{name: n, tcp-outlets: {db-outlet: {to: \"localhost:65536\"}}}" + run_success $OCKAM node show n --output json + assert_output --partial "[]" } @test "node - create two nodes with the same inline configuration" { diff --git a/implementations/rust/ockam/ockam_command/tests/bats/orchestrator_enroll/nodes.bats b/implementations/rust/ockam/ockam_command/tests/bats/orchestrator_enroll/nodes.bats index c5a6c6797a3..78ea10679ba 100644 --- a/implementations/rust/ockam/ockam_command/tests/bats/orchestrator_enroll/nodes.bats +++ b/implementations/rust/ockam/ockam_command/tests/bats/orchestrator_enroll/nodes.bats @@ -18,7 +18,7 @@ teardown() { # ===== TESTS @test "nodes - create with config, admin enrolling twice with the project doesn't return error" { - $OCKAM project ticket --usage-count 10 >"$OCKAM_HOME/enrollment.ticket" + $OCKAM project ticket --usage-count 5 >"$OCKAM_HOME/enrollment.ticket" cat <"$OCKAM_HOME/config.yaml" name: n1 @@ -34,7 +34,7 @@ EOF ADMIN_HOME_DIR="$OCKAM_HOME" ticket_path="$ADMIN_HOME_DIR/enrollment.ticket" export RELAY_NAME=$(random_str) - $OCKAM project ticket --usage-count 10 --relay $RELAY_NAME >"$ticket_path" + $OCKAM project ticket --usage-count 5 --relay $RELAY_NAME >"$ticket_path" # User: try to enroll the same identity twice setup_home_dir @@ -59,7 +59,7 @@ EOF ADMIN_HOME_DIR="$OCKAM_HOME" ticket_path="$ADMIN_HOME_DIR/enrollment.ticket" export RELAY_NAME=$(random_str) - $OCKAM project ticket --usage-count 10 --relay $RELAY_NAME >"$ticket_path" + $OCKAM project ticket --usage-count 5 --relay $RELAY_NAME >"$ticket_path" # User: create a node in the foreground with a portal and using an enrollment ticket setup_home_dir @@ -161,7 +161,7 @@ EOF @test "nodes - create with config, download config and enrollment-ticket from URL" { random_file_name=$(random_str) ticket_relative_path=".tmp/$random_file_name.ticket" - $OCKAM project ticket --usage-count 10 >"$OCKAM_HOME_BASE/$ticket_relative_path" + $OCKAM project ticket --usage-count 5 >"$OCKAM_HOME_BASE/$ticket_relative_path" # Create a config file in the python server's root directory config_relative_path=".tmp/$random_file_name.config.yaml" @@ -170,12 +170,13 @@ name: n1 EOF # Using a proper url (with scheme) + setup_home_dir run_success "$OCKAM" node create "http://127.0.0.1:$PYTHON_SERVER_PORT/$config_relative_path" \ --enrollment-ticket "http://127.0.0.1:$PYTHON_SERVER_PORT/$ticket_relative_path" run_success "$OCKAM" message send --timeout 5 hello --to "/node/n1/secure/api/service/echo" # Without a scheme - run_success "$OCKAM" node delete --all -y + setup_home_dir run_success "$OCKAM" node create "127.0.0.1:$PYTHON_SERVER_PORT/$config_relative_path" \ --enrollment-ticket "127.0.0.1:$PYTHON_SERVER_PORT/$ticket_relative_path" run_success "$OCKAM" message send --timeout 5 hello --to "/node/n1/secure/api/service/echo" @@ -184,7 +185,9 @@ EOF @test "nodes - create with config, using the specified identity" { export RELAY_NAME=$(random_str) $OCKAM project ticket --relay "$RELAY_NAME" >"$OCKAM_HOME/enrollment.ticket" + ticket_path="$OCKAM_HOME/enrollment.ticket" + setup_home_dir cat <"$OCKAM_HOME/config.yaml" name: n1 identity: i1 @@ -193,7 +196,7 @@ EOF # The identity will be created and enrolled run_success "$OCKAM" node create "$OCKAM_HOME/config.yaml" \ - --enrollment-ticket "$OCKAM_HOME/enrollment.ticket" + --enrollment-ticket "$ticket_path" # Use the identity to send a message $OCKAM message send hi --identity i1 --to "/project/default/service/forward_to_$RELAY_NAME/secure/api/service/echo" @@ -201,9 +204,12 @@ EOF @test "nodes - create with config, using the specified enrollment ticket" { $OCKAM project ticket >"$OCKAM_HOME/enrollment.ticket" + ticket_path="$OCKAM_HOME/enrollment.ticket" + + setup_home_dir # The identity will be enrolled - run_success "$OCKAM" node create n1 --identity i1 --enrollment-ticket "$OCKAM_HOME/enrollment.ticket" + run_success "$OCKAM" node create n1 --identity i1 --enrollment-ticket "$ticket_path" # Check that the identity can reach the project run_success $OCKAM message send hi --identity i1 --to "/project/default/service/echo" @@ -211,7 +217,9 @@ EOF @test "nodes - create with config, using the specified enrollment ticket, overriding config" { $OCKAM project ticket >"$OCKAM_HOME/enrollment.ticket" + ticket_path="$OCKAM_HOME/enrollment.ticket" + setup_home_dir cat <"$OCKAM_HOME/config.yaml" ticket: other.ticket name: n2 @@ -219,7 +227,7 @@ identity: i2 EOF # The values from the config file will be overridden by the command line arguments - run_success "$OCKAM" node create n1 --identity i1 --enrollment-ticket "$OCKAM_HOME/enrollment.ticket" + run_success "$OCKAM" node create n1 --identity i1 --enrollment-ticket "$ticket_path" run_failure "$OCKAM" node show n2 run_failure "$OCKAM" identity show i2 @@ -231,6 +239,7 @@ EOF $OCKAM project ticket >"$OCKAM_HOME/enrollment.ticket" export ENROLLMENT_TICKET=$(cat "$OCKAM_HOME/enrollment.ticket") + setup_home_dir cat <"$OCKAM_HOME/config.yaml" ticket: ${ENROLLMENT_TICKET} name: n1 @@ -247,6 +256,7 @@ EOF $OCKAM project ticket >"$OCKAM_HOME/enrollment.ticket" export ENROLLMENT_TICKET=$(cat "$OCKAM_HOME/enrollment.ticket") + setup_home_dir cat <"$OCKAM_HOME/config.yaml" ticket: ${ENROLLMENT_TICKET} name: n1 @@ -264,6 +274,7 @@ EOF $OCKAM project ticket --output json >"$OCKAM_HOME/enrollment.ticket" export ENROLLMENT_TICKET="$OCKAM_HOME/enrollment.ticket" + setup_home_dir cat <"$OCKAM_HOME/config.yaml" ticket: ${ENROLLMENT_TICKET} name: n1 @@ -295,3 +306,66 @@ EOF # We just want to check that the command doesn't fail run_success "$OCKAM" node create "$OCKAM_HOME/node.yaml" } + +@test "nodes - create with inline config 1" { + $OCKAM project ticket --usage-count 5 >"$OCKAM_HOME/enrollment.ticket" + export ENROLLMENT_TICKET="$OCKAM_HOME/enrollment.ticket" + + setup_home_dir + run_success "$OCKAM" node create "{ \"name\": \"n1\" }" + run_success "$OCKAM" node show n1 + run_success $OCKAM message send hi --from n1 --to "/project/default/service/echo" +} + +@test "nodes - create with inline config 2" { + $OCKAM project ticket --usage-count 5 >"$OCKAM_HOME/enrollment.ticket" + export ENROLLMENT_TICKET="$OCKAM_HOME/enrollment.ticket" + + setup_home_dir + run_success "$OCKAM" node create "{ \"ticket\": \"$ENROLLMENT_TICKET\", \"name\": \"n2\" }" + run_success "$OCKAM" node show n2 + run_success $OCKAM message send hi --from n2 --to "/project/default/service/echo" +} + +@test "nodes - create with inline config 3" { + $OCKAM project ticket --usage-count 5 >"$OCKAM_HOME/enrollment.ticket" + ticket_path="$OCKAM_HOME/enrollment.ticket" + + setup_home_dir + run_success "$OCKAM" node create "{ \"name\": \"n3\" }" --enrollment-ticket "$ticket_path" + run_success "$OCKAM" node show n3 + run_success $OCKAM message send hi --from n3 --to "/project/default/service/echo" +} + +@test "nodes - create with inline config 4" { + $OCKAM project ticket --usage-count 5 >"$OCKAM_HOME/enrollment.ticket" + export ENROLLMENT_TICKET="$OCKAM_HOME/enrollment.ticket" + + setup_home_dir + run_success "$OCKAM" node create "{ \"name\": \"n4\" }" --foreground & + sleep 10 + run_success "$OCKAM" node show n4 + run_success $OCKAM message send hi --from n4 --to "/project/default/service/echo" +} + +@test "nodes - create with inline config 5" { + $OCKAM project ticket --usage-count 5 >"$OCKAM_HOME/enrollment.ticket" + export ENROLLMENT_TICKET="$OCKAM_HOME/enrollment.ticket" + + setup_home_dir + run_success "$OCKAM" node create "{ \"ticket\": \"$ENROLLMENT_TICKET\", \"name\": \"n5\" }" --foreground & + sleep 10 + run_success "$OCKAM" node show n5 + run_success $OCKAM message send hi --from n5 --to "/project/default/service/echo" +} + +@test "nodes - create with inline config 6" { + $OCKAM project ticket --usage-count 5 >"$OCKAM_HOME/enrollment.ticket" + ticket_path="$OCKAM_HOME/enrollment.ticket" + + setup_home_dir + run_success "$OCKAM" node create "{ \"name\": \"n6\" }" --enrollment-ticket "$ticket_path" --foreground & + sleep 10 + run_success "$OCKAM" node show n6 + run_success $OCKAM message send hi --from n6 --to "/project/default/service/echo" +}