From bed18ec19e11be214edf82e13f775507df677733 Mon Sep 17 00:00:00 2001 From: Julian Gieseke Date: Tue, 11 Nov 2025 13:09:04 +0100 Subject: [PATCH 1/2] feat(cli): add --name param to cli init commands --- Cargo.lock | 6 + crates/data/Cargo.toml | 4 + crates/data/src/bin/hypha-data.rs | 17 +- crates/data/src/config.rs | 56 ++++- crates/data/tests/cli_init_tests.rs | 183 +++++++++++++++ crates/gateway/Cargo.toml | 4 + crates/gateway/src/bin/hypha-gateway.rs | 17 +- crates/gateway/src/config.rs | 60 ++++- crates/gateway/tests/cli_init_tests.rs | 183 +++++++++++++++ crates/scheduler/Cargo.toml | 2 + crates/scheduler/src/bin/hypha-scheduler.rs | 17 +- crates/scheduler/src/config.rs | 65 ++++- crates/scheduler/tests/cli_init_tests.rs | 184 +++++++++++++++ crates/worker/src/bin/hypha-worker.rs | 17 +- crates/worker/src/config.rs | 61 ++++- crates/worker/tests/cli_init_tests.rs | 248 ++++++++++++++++++++ 16 files changed, 1088 insertions(+), 36 deletions(-) create mode 100644 crates/data/tests/cli_init_tests.rs create mode 100644 crates/gateway/tests/cli_init_tests.rs create mode 100644 crates/scheduler/tests/cli_init_tests.rs create mode 100644 crates/worker/tests/cli_init_tests.rs diff --git a/Cargo.lock b/Cargo.lock index b4497974..9aff2ec8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1955,7 +1955,9 @@ dependencies = [ "miette", "serde", "serde_json", + "tempfile", "tokio", + "toml 0.9.8", "tracing", "tracing-subscriber", ] @@ -1975,7 +1977,9 @@ dependencies = [ "libp2p", "miette", "serde", + "tempfile", "tokio", + "toml 0.9.8", "tracing", "tracing-subscriber", ] @@ -2050,10 +2054,12 @@ dependencies = [ "reqwest", "serde", "serde_json", + "tempfile", "thiserror 2.0.17", "tokio", "tokio-stream", "tokio-util", + "toml 0.9.8", "tracing", "tracing-subscriber", "uuid", diff --git a/crates/data/Cargo.toml b/crates/data/Cargo.toml index a6b6b729..4f0a6bb2 100644 --- a/crates/data/Cargo.toml +++ b/crates/data/Cargo.toml @@ -22,3 +22,7 @@ serde_json.workspace = true tokio.workspace = true tracing.workspace = true tracing-subscriber.workspace = true + +[dev-dependencies] +tempfile.workspace = true +toml.workspace = true diff --git a/crates/data/src/bin/hypha-data.rs b/crates/data/src/bin/hypha-data.rs index 75f70433..2d9b3b6e 100644 --- a/crates/data/src/bin/hypha-data.rs +++ b/crates/data/src/bin/hypha-data.rs @@ -40,8 +40,12 @@ struct Cli { enum Commands { Init { /// Path where the configuration file will be written - #[clap(short, long, default_value = "config.toml")] - output: PathBuf, + #[clap(short, long)] + output: Option, + + /// Optional name for this data node, defaults to "data" + #[clap(short = 'n', long = "name", default_value = "data")] + name: String, }, /// Probe a target multiaddr for readiness and exit 0 if healthy. #[serde(untagged)] @@ -287,8 +291,13 @@ async fn run(config: ConfigWithMetadata) -> Result<()> { async fn main() -> miette::Result<()> { let cli = Cli::parse(); match &cli.command { - Commands::Init { output } => { - fs::write(output, &to_toml(&Config::default()).into_diagnostic()?) + Commands::Init { output, name } => { + let config = Config::with_name(name); + let output = output + .clone() + .unwrap_or_else(|| PathBuf::from(format!("{}-config.toml", name))); + + fs::write(&output, &to_toml(&config).into_diagnostic()?) .await .into_diagnostic()?; diff --git a/crates/data/src/config.rs b/crates/data/src/config.rs index 81421707..4b84001a 100644 --- a/crates/data/src/config.rs +++ b/crates/data/src/config.rs @@ -53,12 +53,17 @@ pub struct Config { telemetry_sample_ratio: Option, } -impl Default for Config { - fn default() -> Self { +impl Config { + /// Create a default configuration with the specified name prefix for certificate files. + /// + /// NOTE: This method enables the CLI `init --name` functionality by generating + /// configuration files with customized certificate file names based on the provided name. + /// This is essential for multi-node deployments where each node needs distinct certificates. + pub fn with_name(name: &str) -> Self { Self { - cert_pem: PathBuf::from("data-cert.pem"), - key_pem: PathBuf::from("data-key.pem"), - trust_pem: PathBuf::from("data-trust.pem"), + cert_pem: PathBuf::from(format!("{}-cert.pem", name)), + key_pem: PathBuf::from(format!("{}-key.pem", name)), + trust_pem: PathBuf::from(format!("{}-trust.pem", name)), crls_pem: None, gateway_addresses: vec![ "/ip4/1.2.3.4/tcp/1234" @@ -169,3 +174,44 @@ impl ValidatableConfig for Config { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn config_with_name_sets_certificate_file_names() { + let config = Config::with_name("test-data"); + + assert_eq!(config.cert_pem, PathBuf::from("test-data-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("test-data-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("test-data-trust.pem")); + } + + #[test] + fn config_with_name_preserves_other_default_values() { + let config = Config::with_name("custom"); + + // Verify that non-certificate fields retain their default values + assert_eq!(config.crls_pem, None); + assert!(!config.gateway_addresses.is_empty()); + } + + #[test] + fn config_with_name_handles_empty_string() { + let config = Config::with_name(""); + + assert_eq!(config.cert_pem, PathBuf::from("-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("-trust.pem")); + } + + #[test] + fn config_with_name_handles_special_characters() { + let config = Config::with_name("data-123_test"); + + assert_eq!(config.cert_pem, PathBuf::from("data-123_test-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("data-123_test-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("data-123_test-trust.pem")); + } +} diff --git a/crates/data/tests/cli_init_tests.rs b/crates/data/tests/cli_init_tests.rs new file mode 100644 index 00000000..81d424b7 --- /dev/null +++ b/crates/data/tests/cli_init_tests.rs @@ -0,0 +1,183 @@ +use std::{fs, path::PathBuf, process::Command}; + +use tempfile::TempDir; + +/// Integration tests for the CLI init command's --name parameter functionality +/// +/// These tests verify that the hypha-data binary correctly: +/// 1. Parses the --name parameter +/// 2. Creates configurations with the correct certificate filenames +/// 3. Generates appropriate output filenames when --output is not specified +/// 4. Writes valid TOML configuration files +#[cfg(test)] +mod integration_tests { + use super::*; + + /// Get the path to the hypha-data binary + fn get_binary_path() -> PathBuf { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.pop(); // Go up from crates/data to crates + path.pop(); // Go up from crates to root + path.push("target"); + path.push("debug"); + path.push("hypha-data"); + path + } + + #[test] + fn init_with_name_parameter_creates_config_with_correct_certificate_names() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-data") + .arg("--output") + .arg(temp_path.join("test-config.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("test-config.toml")) + .expect("Failed to read config file"); + + // Verify that the certificate file names use the specified name prefix + assert!( + config_content.contains("cert_pem = \"test-data-cert.pem\""), + "Config should contain test-data-cert.pem, got: {}", + config_content + ); + assert!( + config_content.contains("key_pem = \"test-data-key.pem\""), + "Config should contain test-data-key.pem, got: {}", + config_content + ); + assert!( + config_content.contains("trust_pem = \"test-data-trust.pem\""), + "Config should contain test-data-trust.pem, got: {}", + config_content + ); + } + + #[test] + fn init_with_name_and_no_output_uses_default_filename_pattern() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("my-data") + .current_dir(temp_path) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Verify that the default output filename was used + let expected_filename = temp_path.join("my-data-config.toml"); + assert!( + expected_filename.exists(), + "Expected config file {} should exist", + expected_filename.display() + ); + + let config_content = + fs::read_to_string(&expected_filename).expect("Failed to read config file"); + + // Verify content matches the name + assert!(config_content.contains("cert_pem = \"my-data-cert.pem\"")); + assert!(config_content.contains("key_pem = \"my-data-key.pem\"")); + assert!(config_content.contains("trust_pem = \"my-data-trust.pem\"")); + } + + #[test] + fn init_with_default_name_uses_data_prefix() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--output") + .arg(temp_path.join("default-config.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("default-config.toml")) + .expect("Failed to read config file"); + + // When no --name is provided, it should use the default value "data" + assert!(config_content.contains("cert_pem = \"data-cert.pem\"")); + assert!(config_content.contains("key_pem = \"data-key.pem\"")); + assert!(config_content.contains("trust_pem = \"data-trust.pem\"")); + } + + #[test] + fn init_creates_valid_toml_file() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("validation-test") + .arg("--output") + .arg(temp_path.join("validation.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("validation.toml")) + .expect("Failed to read config file"); + + // Verify the file is valid TOML + let parsed_toml: toml::Table = config_content + .parse() + .expect("Generated config should be valid TOML"); + + // Verify essential fields exist + assert!(parsed_toml.contains_key("cert_pem")); + assert!(parsed_toml.contains_key("key_pem")); + assert!(parsed_toml.contains_key("trust_pem")); + assert!(parsed_toml.contains_key("gateway_addresses")); + + // Verify the certificate fields have the expected values + assert_eq!( + parsed_toml["cert_pem"].as_str().unwrap(), + "validation-test-cert.pem" + ); + assert_eq!( + parsed_toml["key_pem"].as_str().unwrap(), + "validation-test-key.pem" + ); + assert_eq!( + parsed_toml["trust_pem"].as_str().unwrap(), + "validation-test-trust.pem" + ); + } +} diff --git a/crates/gateway/Cargo.toml b/crates/gateway/Cargo.toml index 416b708f..4094058a 100644 --- a/crates/gateway/Cargo.toml +++ b/crates/gateway/Cargo.toml @@ -20,3 +20,7 @@ serde.workspace = true tokio.workspace = true tracing.workspace = true tracing-subscriber.workspace = true + +[dev-dependencies] +tempfile.workspace = true +toml.workspace = true diff --git a/crates/gateway/src/bin/hypha-gateway.rs b/crates/gateway/src/bin/hypha-gateway.rs index d35f7e13..5f3a19b2 100644 --- a/crates/gateway/src/bin/hypha-gateway.rs +++ b/crates/gateway/src/bin/hypha-gateway.rs @@ -44,8 +44,12 @@ struct Cli { enum Commands { Init { /// Path where the configuration file will be written - #[clap(short, long, default_value = "config.toml")] - output: PathBuf, + #[clap(short, long)] + output: Option, + + /// Optional name for this gateway node, defaults to "gateway" + #[clap(short = 'n', long = "name", default_value = "gateway")] + name: String, }, /// Probe a target multiaddr for readiness and exit 0 if healthy. #[serde(untagged)] @@ -255,8 +259,13 @@ async fn run(config: ConfigWithMetadata) -> Result<()> { async fn main() -> Result<()> { let cli = Cli::parse(); match &cli.command { - Commands::Init { output } => { - fs::write(output, &to_toml(&Config::default()).into_diagnostic()?).into_diagnostic()?; + Commands::Init { output, name } => { + let config = Config::with_name(name); + let output = output + .clone() + .unwrap_or_else(|| PathBuf::from(format!("{}-config.toml", name))); + + fs::write(&output, &to_toml(&config).into_diagnostic()?).into_diagnostic()?; println!("Configuration written to: {output:?}"); Ok(()) diff --git a/crates/gateway/src/config.rs b/crates/gateway/src/config.rs index 3cff50ae..e66889b3 100644 --- a/crates/gateway/src/config.rs +++ b/crates/gateway/src/config.rs @@ -52,12 +52,17 @@ pub struct Config { exclude_cidr: Vec, } -impl Default for Config { - fn default() -> Self { +impl Config { + /// Create a default configuration with the specified name prefix for certificate files. + /// + /// NOTE: This method enables the CLI `init --name` functionality by generating + /// configuration files with customized certificate file names based on the provided name. + /// This is essential for multi-node deployments where each node needs distinct certificates. + pub fn with_name(name: &str) -> Self { Self { - cert_pem: PathBuf::from("gateway-cert.pem"), - key_pem: PathBuf::from("gateway-key.pem"), - trust_pem: PathBuf::from("gateway-trust.pem"), + cert_pem: PathBuf::from(format!("{}-cert.pem", name)), + key_pem: PathBuf::from(format!("{}-key.pem", name)), + trust_pem: PathBuf::from(format!("{}-trust.pem", name)), crls_pem: None, listen_addresses: vec![ "/ip4/127.0.0.1/tcp/8080" @@ -143,3 +148,48 @@ impl TLSConfig for Config { self.crls_pem.as_deref() } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn config_with_name_sets_certificate_file_names() { + let config = Config::with_name("test-gateway"); + + assert_eq!(config.cert_pem, PathBuf::from("test-gateway-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("test-gateway-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("test-gateway-trust.pem")); + } + + #[test] + fn config_with_name_preserves_other_default_values() { + let config = Config::with_name("custom"); + + // Verify that non-certificate fields retain their default values + assert_eq!(config.crls_pem, None); + assert!(!config.listen_addresses.is_empty()); + assert!(!config.external_addresses.is_empty()); + } + + #[test] + fn config_with_name_handles_empty_string() { + let config = Config::with_name(""); + + assert_eq!(config.cert_pem, PathBuf::from("-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("-trust.pem")); + } + + #[test] + fn config_with_name_handles_special_characters() { + let config = Config::with_name("gateway-123_test"); + + assert_eq!(config.cert_pem, PathBuf::from("gateway-123_test-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("gateway-123_test-key.pem")); + assert_eq!( + config.trust_pem, + PathBuf::from("gateway-123_test-trust.pem") + ); + } +} diff --git a/crates/gateway/tests/cli_init_tests.rs b/crates/gateway/tests/cli_init_tests.rs new file mode 100644 index 00000000..c86f10c1 --- /dev/null +++ b/crates/gateway/tests/cli_init_tests.rs @@ -0,0 +1,183 @@ +use std::{fs, path::PathBuf, process::Command}; + +use tempfile::TempDir; + +/// Integration tests for the CLI init command's --name parameter functionality +/// +/// These tests verify that the hypha-gateway binary correctly: +/// 1. Parses the --name parameter +/// 2. Creates configurations with the correct certificate filenames +/// 3. Generates appropriate output filenames when --output is not specified +/// 4. Writes valid TOML configuration files +#[cfg(test)] +mod integration_tests { + use super::*; + + /// Get the path to the hypha-gateway binary + fn get_binary_path() -> PathBuf { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.pop(); // Go up from crates/gateway to crates + path.pop(); // Go up from crates to root + path.push("target"); + path.push("debug"); + path.push("hypha-gateway"); + path + } + + #[test] + fn init_with_name_parameter_creates_config_with_correct_certificate_names() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-gateway") + .arg("--output") + .arg(temp_path.join("test-config.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("test-config.toml")) + .expect("Failed to read config file"); + + // Verify that the certificate file names use the specified name prefix + assert!( + config_content.contains("cert_pem = \"test-gateway-cert.pem\""), + "Config should contain test-gateway-cert.pem, got: {}", + config_content + ); + assert!( + config_content.contains("key_pem = \"test-gateway-key.pem\""), + "Config should contain test-gateway-key.pem, got: {}", + config_content + ); + assert!( + config_content.contains("trust_pem = \"test-gateway-trust.pem\""), + "Config should contain test-gateway-trust.pem, got: {}", + config_content + ); + } + + #[test] + fn init_with_name_and_no_output_uses_default_filename_pattern() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("my-gateway") + .current_dir(temp_path) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Verify that the default output filename was used + let expected_filename = temp_path.join("my-gateway-config.toml"); + assert!( + expected_filename.exists(), + "Expected config file {} should exist", + expected_filename.display() + ); + + let config_content = + fs::read_to_string(&expected_filename).expect("Failed to read config file"); + + // Verify content matches the name + assert!(config_content.contains("cert_pem = \"my-gateway-cert.pem\"")); + assert!(config_content.contains("key_pem = \"my-gateway-key.pem\"")); + assert!(config_content.contains("trust_pem = \"my-gateway-trust.pem\"")); + } + + #[test] + fn init_with_default_name_uses_gateway_prefix() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--output") + .arg(temp_path.join("default-config.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("default-config.toml")) + .expect("Failed to read config file"); + + // When no --name is provided, it should use the default value "gateway" + assert!(config_content.contains("cert_pem = \"gateway-cert.pem\"")); + assert!(config_content.contains("key_pem = \"gateway-key.pem\"")); + assert!(config_content.contains("trust_pem = \"gateway-trust.pem\"")); + } + + #[test] + fn init_creates_valid_toml_file() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("validation-test") + .arg("--output") + .arg(temp_path.join("validation.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("validation.toml")) + .expect("Failed to read config file"); + + // Verify the file is valid TOML + let parsed_toml: toml::Table = config_content + .parse() + .expect("Generated config should be valid TOML"); + + // Verify essential fields exist + assert!(parsed_toml.contains_key("cert_pem")); + assert!(parsed_toml.contains_key("key_pem")); + assert!(parsed_toml.contains_key("trust_pem")); + assert!(parsed_toml.contains_key("listen_addresses")); + + // Verify the certificate fields have the expected values + assert_eq!( + parsed_toml["cert_pem"].as_str().unwrap(), + "validation-test-cert.pem" + ); + assert_eq!( + parsed_toml["key_pem"].as_str().unwrap(), + "validation-test-key.pem" + ); + assert_eq!( + parsed_toml["trust_pem"].as_str().unwrap(), + "validation-test-trust.pem" + ); + } +} diff --git a/crates/scheduler/Cargo.toml b/crates/scheduler/Cargo.toml index 8fe42f3c..b7ddccab 100644 --- a/crates/scheduler/Cargo.toml +++ b/crates/scheduler/Cargo.toml @@ -33,6 +33,8 @@ tracing-subscriber.workspace = true uuid.workspace = true [dev-dependencies] +tempfile.workspace = true +toml.workspace = true tokio = { version = "1.43.0", features = [ "fs", "macros", diff --git a/crates/scheduler/src/bin/hypha-scheduler.rs b/crates/scheduler/src/bin/hypha-scheduler.rs index 62f16a35..e15ae4d1 100644 --- a/crates/scheduler/src/bin/hypha-scheduler.rs +++ b/crates/scheduler/src/bin/hypha-scheduler.rs @@ -60,8 +60,12 @@ struct Cli { enum Commands { Init { /// Path where the configuration file will be written - #[clap(short, long, default_value = "config.toml")] - output: std::path::PathBuf, + #[clap(short, long)] + output: Option, + + /// Optional name for this scheduler node, defaults to "scheduler" + #[clap(short = 'n', long = "name", default_value = "scheduler")] + name: String, }, /// Probe a target multiaddr for readiness and exit 0 if healthy. #[serde(untagged)] @@ -549,8 +553,13 @@ async fn get_data_provider( async fn main() -> Result<()> { let cli = Cli::parse(); match &cli.command { - Commands::Init { output } => { - fs::write(output, &to_toml(&Config::default())?).into_diagnostic()?; + Commands::Init { output, name } => { + let config = Config::with_name(name); + let output = output + .clone() + .unwrap_or_else(|| PathBuf::from(format!("{}-config.toml", name))); + + fs::write(&output, &to_toml(&config)?).into_diagnostic()?; println!("Configuration written to: {output:?}"); Ok(()) diff --git a/crates/scheduler/src/config.rs b/crates/scheduler/src/config.rs index 0c017675..c9d3ca44 100644 --- a/crates/scheduler/src/config.rs +++ b/crates/scheduler/src/config.rs @@ -62,12 +62,17 @@ pub struct Config { scheduler: SchedulerConfig, } -impl Default for Config { - fn default() -> Self { +impl Config { + /// Create a default configuration with the specified name prefix for certificate files. + /// + /// NOTE: This method enables the CLI `init --name` functionality by generating + /// configuration files with customized certificate file names based on the provided name. + /// This is essential for multi-node deployments where each node needs distinct certificates. + pub fn with_name(name: &str) -> Self { Self { - cert_pem: PathBuf::from("scheduler-cert.pem"), - key_pem: PathBuf::from("scheduler-key.pem"), - trust_pem: PathBuf::from("scheduler-trust.pem"), + cert_pem: PathBuf::from(format!("{}-cert.pem", name)), + key_pem: PathBuf::from(format!("{}-key.pem", name)), + trust_pem: PathBuf::from(format!("{}-trust.pem", name)), crls_pem: None, // NOTE: Placeholder gateway addresses so users must configure real endpoints. gateway_addresses: vec![ @@ -195,3 +200,53 @@ impl ValidatableConfig for Config { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn config_with_name_sets_certificate_file_names() { + let config = Config::with_name("test-scheduler"); + + assert_eq!(config.cert_pem, PathBuf::from("test-scheduler-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("test-scheduler-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("test-scheduler-trust.pem")); + } + + #[test] + fn config_with_name_preserves_other_default_values() { + let config = Config::with_name("custom"); + + // Verify that non-certificate fields retain their default values + assert_eq!(config.crls_pem, None); + assert!(!config.gateway_addresses.is_empty()); + assert!(!config.listen_addresses.is_empty()); + assert!(config.external_addresses.is_empty()); + assert!(config.relay_circuit); + } + + #[test] + fn config_with_name_handles_empty_string() { + let config = Config::with_name(""); + + assert_eq!(config.cert_pem, PathBuf::from("-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("-trust.pem")); + } + + #[test] + fn config_with_name_handles_special_characters() { + let config = Config::with_name("scheduler-123_test"); + + assert_eq!( + config.cert_pem, + PathBuf::from("scheduler-123_test-cert.pem") + ); + assert_eq!(config.key_pem, PathBuf::from("scheduler-123_test-key.pem")); + assert_eq!( + config.trust_pem, + PathBuf::from("scheduler-123_test-trust.pem") + ); + } +} diff --git a/crates/scheduler/tests/cli_init_tests.rs b/crates/scheduler/tests/cli_init_tests.rs new file mode 100644 index 00000000..1c1c06bd --- /dev/null +++ b/crates/scheduler/tests/cli_init_tests.rs @@ -0,0 +1,184 @@ +use std::{fs, path::PathBuf, process::Command}; + +use tempfile::TempDir; + +/// Integration tests for the CLI init command's --name parameter functionality +/// +/// These tests verify that the hypha-scheduler binary correctly: +/// 1. Parses the --name parameter +/// 2. Creates configurations with the correct certificate filenames +/// 3. Generates appropriate output filenames when --output is not specified +/// 4. Writes valid TOML configuration files +#[cfg(test)] +mod integration_tests { + use super::*; + + /// Get the path to the hypha-scheduler binary + fn get_binary_path() -> PathBuf { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.pop(); // Go up from crates/scheduler to crates + path.pop(); // Go up from crates to root + path.push("target"); + path.push("debug"); + path.push("hypha-scheduler"); + path + } + + #[test] + fn init_with_name_parameter_creates_config_with_correct_certificate_names() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-scheduler") + .arg("--output") + .arg(temp_path.join("test-config.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("test-config.toml")) + .expect("Failed to read config file"); + + // Verify that the certificate file names use the specified name prefix + assert!( + config_content.contains("cert_pem = \"test-scheduler-cert.pem\""), + "Config should contain test-scheduler-cert.pem, got: {}", + config_content + ); + assert!( + config_content.contains("key_pem = \"test-scheduler-key.pem\""), + "Config should contain test-scheduler-key.pem, got: {}", + config_content + ); + assert!( + config_content.contains("trust_pem = \"test-scheduler-trust.pem\""), + "Config should contain test-scheduler-trust.pem, got: {}", + config_content + ); + } + + #[test] + fn init_with_name_and_no_output_uses_default_filename_pattern() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("my-scheduler") + .current_dir(temp_path) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Verify that the default output filename was used + let expected_filename = temp_path.join("my-scheduler-config.toml"); + assert!( + expected_filename.exists(), + "Expected config file {} should exist", + expected_filename.display() + ); + + let config_content = + fs::read_to_string(&expected_filename).expect("Failed to read config file"); + + // Verify content matches the name + assert!(config_content.contains("cert_pem = \"my-scheduler-cert.pem\"")); + assert!(config_content.contains("key_pem = \"my-scheduler-key.pem\"")); + assert!(config_content.contains("trust_pem = \"my-scheduler-trust.pem\"")); + } + + #[test] + fn init_with_default_name_uses_scheduler_prefix() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--output") + .arg(temp_path.join("default-config.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("default-config.toml")) + .expect("Failed to read config file"); + + // When no --name is provided, it should use the default value "scheduler" + assert!(config_content.contains("cert_pem = \"scheduler-cert.pem\"")); + assert!(config_content.contains("key_pem = \"scheduler-key.pem\"")); + assert!(config_content.contains("trust_pem = \"scheduler-trust.pem\"")); + } + + #[test] + fn init_creates_valid_toml_file() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("validation-test") + .arg("--output") + .arg(temp_path.join("validation.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("validation.toml")) + .expect("Failed to read config file"); + + // Verify the file is valid TOML + let parsed_toml: toml::Table = config_content + .parse() + .expect("Generated config should be valid TOML"); + + // Verify essential fields exist + assert!(parsed_toml.contains_key("cert_pem")); + assert!(parsed_toml.contains_key("key_pem")); + assert!(parsed_toml.contains_key("trust_pem")); + assert!(parsed_toml.contains_key("gateway_addresses")); + assert!(parsed_toml.contains_key("listen_addresses")); + + // Verify the certificate fields have the expected values + assert_eq!( + parsed_toml["cert_pem"].as_str().unwrap(), + "validation-test-cert.pem" + ); + assert_eq!( + parsed_toml["key_pem"].as_str().unwrap(), + "validation-test-key.pem" + ); + assert_eq!( + parsed_toml["trust_pem"].as_str().unwrap(), + "validation-test-trust.pem" + ); + } +} diff --git a/crates/worker/src/bin/hypha-worker.rs b/crates/worker/src/bin/hypha-worker.rs index c87e182c..b5b33d44 100644 --- a/crates/worker/src/bin/hypha-worker.rs +++ b/crates/worker/src/bin/hypha-worker.rs @@ -56,8 +56,12 @@ struct Cli { enum Commands { Init { /// Path where the configuration file will be written - #[clap(short, long, default_value = "config.toml")] - output: PathBuf, + #[clap(short, long)] + output: Option, + + /// Optional name for this worker node, defaults to "worker" + #[clap(short = 'n', long = "name", default_value = "worker")] + name: String, }, /// Probe a target multiaddr for readiness and exit 0 if healthy. @@ -380,8 +384,13 @@ async fn run(config: ConfigWithMetadata) -> Result<()> { async fn main() -> miette::Result<()> { let cli = Cli::parse(); match &cli.command { - Commands::Init { output } => { - fs::write(output, &to_toml(&Config::default()).into_diagnostic()?).into_diagnostic()?; + Commands::Init { output, name } => { + let config = Config::with_name(name); + let output = output + .clone() + .unwrap_or_else(|| PathBuf::from(format!("{}-config.toml", name))); + + fs::write(&output, &to_toml(&config).into_diagnostic()?).into_diagnostic()?; println!("Configuration written to: {output:?}"); Ok(()) diff --git a/crates/worker/src/config.rs b/crates/worker/src/config.rs index 42b90d28..8e4171ac 100644 --- a/crates/worker/src/config.rs +++ b/crates/worker/src/config.rs @@ -130,12 +130,17 @@ pub struct Config { telemetry_sample_ratio: Option, } -impl Default for Config { - fn default() -> Self { +impl Config { + /// Create a default configuration with the specified name prefix for certificate files. + /// + /// NOTE: This method enables the CLI `init --name` functionality by generating + /// configuration files with customized certificate file names based on the provided name. + /// This is essential for multi-node deployments where each node needs distinct certificates. + pub fn with_name(name: &str) -> Self { Self { - cert_pem: PathBuf::from("worker-cert.pem"), - key_pem: PathBuf::from("worker-key.pem"), - trust_pem: PathBuf::from("worker-trust.pem"), + cert_pem: PathBuf::from(format!("{}-cert.pem", name)), + key_pem: PathBuf::from(format!("{}-key.pem", name)), + trust_pem: PathBuf::from(format!("{}-trust.pem", name)), crls_pem: None, // NOTE: Placeholder gateway addresses so users must configure real endpoints. gateway_addresses: vec![ @@ -287,3 +292,49 @@ impl ValidatableConfig for Config { Ok(()) } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn config_with_name_sets_certificate_file_names() { + let config = Config::with_name("test-worker"); + + assert_eq!(config.cert_pem, PathBuf::from("test-worker-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("test-worker-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("test-worker-trust.pem")); + } + + #[test] + fn config_with_name_preserves_other_default_values() { + let config = Config::with_name("custom"); + + // Verify that non-certificate fields retain their default values + assert_eq!(config.crls_pem, None); + assert!(!config.gateway_addresses.is_empty()); + assert!(!config.listen_addresses.is_empty()); + assert!(config.external_addresses.is_empty()); + assert!(config.relay_circuit); + assert_eq!(config.work_dir, PathBuf::from("/tmp")); + assert!(!config.driver.is_empty()); + } + + #[test] + fn config_with_name_handles_empty_string() { + let config = Config::with_name(""); + + assert_eq!(config.cert_pem, PathBuf::from("-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("-trust.pem")); + } + + #[test] + fn config_with_name_handles_special_characters() { + let config = Config::with_name("worker-123_test"); + + assert_eq!(config.cert_pem, PathBuf::from("worker-123_test-cert.pem")); + assert_eq!(config.key_pem, PathBuf::from("worker-123_test-key.pem")); + assert_eq!(config.trust_pem, PathBuf::from("worker-123_test-trust.pem")); + } +} diff --git a/crates/worker/tests/cli_init_tests.rs b/crates/worker/tests/cli_init_tests.rs new file mode 100644 index 00000000..5dcf9c29 --- /dev/null +++ b/crates/worker/tests/cli_init_tests.rs @@ -0,0 +1,248 @@ +use std::{fs, path::PathBuf, process::Command}; + +use tempfile::TempDir; + +/// Integration tests for the CLI init command's --name parameter functionality +/// +/// These tests verify that the hypha-worker binary correctly: +/// 1. Parses the --name parameter +/// 2. Creates configurations with the correct certificate filenames +/// 3. Generates appropriate output filenames when --output is not specified +/// 4. Writes valid TOML configuration files +#[cfg(test)] +mod integration_tests { + use super::*; + + /// Get the path to the hypha-worker binary + fn get_binary_path() -> PathBuf { + let mut path = PathBuf::from(env!("CARGO_MANIFEST_DIR")); + path.pop(); // Go up from crates/worker to crates + path.pop(); // Go up from crates to root + path.push("target"); + path.push("debug"); + path.push("hypha-worker"); + path + } + + #[test] + fn init_with_name_parameter_creates_config_with_correct_certificate_names() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-worker") + .arg("--output") + .arg(temp_path.join("test-config.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("test-config.toml")) + .expect("Failed to read config file"); + + // Verify that the certificate file names use the specified name prefix + assert!( + config_content.contains("cert_pem = \"test-worker-cert.pem\""), + "Config should contain test-worker-cert.pem, got: {}", + config_content + ); + assert!( + config_content.contains("key_pem = \"test-worker-key.pem\""), + "Config should contain test-worker-key.pem, got: {}", + config_content + ); + assert!( + config_content.contains("trust_pem = \"test-worker-trust.pem\""), + "Config should contain test-worker-trust.pem, got: {}", + config_content + ); + } + + #[test] + fn init_with_name_and_no_output_uses_default_filename_pattern() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("my-worker") + .current_dir(temp_path) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Verify that the default output filename was used + let expected_filename = temp_path.join("my-worker-config.toml"); + assert!( + expected_filename.exists(), + "Expected config file {} should exist", + expected_filename.display() + ); + + let config_content = + fs::read_to_string(&expected_filename).expect("Failed to read config file"); + + // Verify content matches the name + assert!(config_content.contains("cert_pem = \"my-worker-cert.pem\"")); + assert!(config_content.contains("key_pem = \"my-worker-key.pem\"")); + assert!(config_content.contains("trust_pem = \"my-worker-trust.pem\"")); + } + + #[test] + fn init_with_default_name_uses_worker_prefix() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--output") + .arg(temp_path.join("default-config.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("default-config.toml")) + .expect("Failed to read config file"); + + // When no --name is provided, it should use the default value "worker" + assert!(config_content.contains("cert_pem = \"worker-cert.pem\"")); + assert!(config_content.contains("key_pem = \"worker-key.pem\"")); + assert!(config_content.contains("trust_pem = \"worker-trust.pem\"")); + } + + #[test] + fn init_creates_valid_toml_file() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("validation-test") + .arg("--output") + .arg(temp_path.join("validation.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("validation.toml")) + .expect("Failed to read config file"); + + // Verify the file is valid TOML + let parsed_toml: toml::Table = config_content + .parse() + .expect("Generated config should be valid TOML"); + + // Verify essential fields exist + assert!(parsed_toml.contains_key("cert_pem")); + assert!(parsed_toml.contains_key("key_pem")); + assert!(parsed_toml.contains_key("trust_pem")); + assert!(parsed_toml.contains_key("gateway_addresses")); + assert!(parsed_toml.contains_key("listen_addresses")); + + // Verify the certificate fields have the expected values + assert_eq!( + parsed_toml["cert_pem"].as_str().unwrap(), + "validation-test-cert.pem" + ); + assert_eq!( + parsed_toml["key_pem"].as_str().unwrap(), + "validation-test-key.pem" + ); + assert_eq!( + parsed_toml["trust_pem"].as_str().unwrap(), + "validation-test-trust.pem" + ); + } + + #[test] + fn init_with_special_characters_in_name() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("worker-123_test") + .arg("--output") + .arg(temp_path.join("special.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = + fs::read_to_string(temp_path.join("special.toml")).expect("Failed to read config file"); + + assert!(config_content.contains("cert_pem = \"worker-123_test-cert.pem\"")); + assert!(config_content.contains("key_pem = \"worker-123_test-key.pem\"")); + assert!(config_content.contains("trust_pem = \"worker-123_test-trust.pem\"")); + } + + #[test] + fn init_includes_documentation_comments() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("doc-test") + .arg("--output") + .arg(temp_path.join("documented.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("documented.toml")) + .expect("Failed to read config file"); + + // Verify that the config includes documentation + assert!( + config_content.contains("# Config"), + "Config should include header documentation" + ); + assert!( + config_content.contains("# Fields:"), + "Config should include field documentation" + ); + } +} From 1359c3edbb2a78d6c79096a0220ebd545a2c0f36 Mon Sep 17 00:00:00 2001 From: Julian Gieseke Date: Tue, 11 Nov 2025 13:54:40 +0100 Subject: [PATCH 2/2] feat(cli): add generic --set parameter for configuration overrides Replace specific CLI parameters with generic --set key=value functionality across all Hypha binaries (data, worker, gateway, scheduler). This allows users to override any TOML configuration value via command line arguments. Key changes: - Add parse_key_val function for parsing KEY=value pairs - Add create_overrides_toml function for TOML generation - Add apply_config_overrides helper for figment integration - Update Init, Probe, and Run commands to accept --set parameters - Support multiple overrides with proper type detection (bool, number, string) - Enable nested configuration via dot notation (e.g., listen_addresses.0) - Replace hypha-data dataset_path parameter with --set dataset_path=value Examples: - hypha-data init --set dataset_path="/data/ml-datasets" - hypha-worker run --set work_dir="/tmp/hypha" - hypha-gateway init --set listen_addresses.0="/ip4/0.0.0.0/tcp/8080" - hypha-scheduler run --set gateway_addresses.0="/ip4/127.0.0.1/tcp/1234" Co-Authored-By: Claude --- Cargo.lock | 2 + crates/config/Cargo.toml | 1 + crates/config/src/cli.rs | 448 ++++++++++++++++++++ crates/config/src/lib.rs | 2 + crates/data/src/bin/hypha-data.rs | 68 ++- crates/data/tests/cli_init_tests.rs | 268 ++++++++++++ crates/gateway/src/bin/hypha-gateway.rs | 67 ++- crates/gateway/tests/cli_init_tests.rs | 297 +++++++++++++ crates/scheduler/src/bin/hypha-scheduler.rs | 69 ++- crates/scheduler/tests/cli_init_tests.rs | 412 ++++++++++++++---- crates/worker/src/bin/hypha-worker.rs | 69 ++- crates/worker/src/config.rs | 1 - crates/worker/tests/cli_init_tests.rs | 233 ++++++++++ 13 files changed, 1808 insertions(+), 129 deletions(-) create mode 100644 crates/config/src/cli.rs diff --git a/Cargo.lock b/Cargo.lock index 9aff2ec8..51223ce4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1936,6 +1936,7 @@ dependencies = [ "tempfile", "thiserror 2.0.17", "toml 0.9.8", + "toml_edit 0.23.7", ] [[package]] @@ -5113,6 +5114,7 @@ dependencies = [ "indexmap", "toml_datetime 0.7.3", "toml_parser", + "toml_writer", "winnow", ] diff --git a/crates/config/Cargo.toml b/crates/config/Cargo.toml index 403ac7db..d2116304 100644 --- a/crates/config/Cargo.toml +++ b/crates/config/Cargo.toml @@ -13,6 +13,7 @@ miette.workspace = true serde.workspace = true thiserror.workspace = true toml.workspace = true +toml_edit = "0.23.7" [dev-dependencies] tempfile.workspace = true diff --git a/crates/config/src/cli.rs b/crates/config/src/cli.rs new file mode 100644 index 00000000..74f334be --- /dev/null +++ b/crates/config/src/cli.rs @@ -0,0 +1,448 @@ +#![deny(missing_docs)] +//! Command-line interface utilities for configuration management. +//! +//! This module provides utility functions for handling configuration overrides +//! via command-line arguments, supporting the `--set key=value` functionality +//! across all Hypha binaries. + +use figment::providers::{Format, Toml}; +use miette::Result; +use serde::{Deserialize, Serialize}; + +use crate::LayeredConfigBuilder; + +/// Parse a single key-value pair for configuration overrides. +/// +/// Parses command-line arguments in the format `KEY=value` into separate +/// key and value strings for use with configuration override systems. +/// +/// # Arguments +/// +/// * `s` - A string slice in the format "key=value" +/// +/// # Returns +/// +/// Returns `Ok((key, value))` on successful parsing, or an error message +/// describing what went wrong. +/// +/// # Examples +/// +/// ```rust +/// use hypha_config::cli::parse_key_val; +/// +/// let (key, value) = parse_key_val("listen_port=8080").unwrap(); +/// assert_eq!(key, "listen_port"); +/// assert_eq!(value, "8080"); +/// +/// let (key, value) = parse_key_val("debug=true").unwrap(); +/// assert_eq!(key, "debug"); +/// assert_eq!(value, "true"); +/// +/// // Handles nested configuration via dot notation +/// let (key, value) = parse_key_val("listen_addresses.0=/ip4/127.0.0.1/tcp/8080").unwrap(); +/// assert_eq!(key, "listen_addresses.0"); +/// assert_eq!(value, "/ip4/127.0.0.1/tcp/8080"); +/// +/// // Returns error for invalid format +/// assert!(parse_key_val("invalid_format").is_err()); +/// ``` +pub fn parse_key_val(s: &str) -> Result<(String, String)> { + let pos = s + .find('=') + .ok_or_else(|| miette::miette!("invalid KEY=value: no `=` found in `{s}`"))?; + Ok((s[..pos].to_string(), s[pos + 1..].to_string())) +} + +/// Convert key-value pairs to TOML format for configuration overrides. +/// +/// Takes a collection of key-value pairs and converts them into a TOML +/// string that can be used as a configuration provider. Performs basic +/// type detection to avoid unnecessarily quoting numeric and boolean values. +/// Supports nested structure notation (e.g., "scheduler.model.repository") and +/// array index notation (e.g., "scheduler.model.filenames.0"). +/// +/// # Arguments +/// +/// * `overrides` - A slice of `(key, value)` tuples to convert +/// +/// # Returns +/// +/// Returns a TOML-formatted string ready for use with figment providers. +/// +/// # Examples +/// +/// ```rust +/// use hypha_config::cli::create_overrides_toml; +/// +/// let overrides = vec![ +/// ("debug".to_string(), "true".to_string()), +/// ("port".to_string(), "8080".to_string()), +/// ("name".to_string(), "my-service".to_string()), +/// ("scheduler.job.model.repository".to_string(), "hypha-space/lenet".to_string()), +/// ("scheduler.job.model.filenames.0".to_string(), "config.json".to_string()), +/// ("scheduler.job.model.filenames.1".to_string(), "model.safetensors".to_string()), +/// ]; +/// +/// let toml = create_overrides_toml(&overrides); +/// assert!(toml.contains("debug = true")); // Boolean without quotes +/// assert!(toml.contains("port = 8080")); // Number without quotes +/// assert!(toml.contains("name = \"my-service\"")); // String with quotes +/// assert!(toml.contains("[scheduler.job.model]")); +/// assert!(toml.contains("repository = \"hypha-space/lenet\"")); +/// assert!(toml.contains("filenames = [\"config.json\", \"model.safetensors\"]")); +/// ``` +pub fn create_overrides_toml(overrides: &[(String, String)]) -> String { + use toml_edit::DocumentMut; + + // Parse all override keys to build a nested structure + let mut doc = DocumentMut::new(); + + for (key, val) in overrides { + // NOTE: We ignore errors for individual overrides to keep the function infallible. + let _ = set_nested_value(&mut doc, key, val); + } + + // Convert document to string + doc.to_string() +} + +/// Helper function to set a nested value in a TOML document +fn set_nested_value(doc: &mut toml_edit::DocumentMut, key: &str, val: &str) -> Result<()> { + use toml_edit::Value; + + let parts: Vec<&str> = key.split('.').collect(); + if parts.is_empty() { + return Err(crate::ConfigError::Invalid(format!("Empty key: {}", key)).into()); + } + + // Format the value with proper type detection + let formatted_value = if val.parse::().is_ok() { + Value::Boolean(toml_edit::Formatted::new(val.parse::().unwrap())) + } else if let Ok(int_val) = val.parse::() { + Value::Integer(toml_edit::Formatted::new(int_val)) + } else if let Ok(float_val) = val.parse::() { + Value::Float(toml_edit::Formatted::new(float_val)) + } else { + Value::String(toml_edit::Formatted::new(val.to_string())) + }; + + let root = doc.as_table_mut(); + set_value_at_path(root, &parts, formatted_value) +} + +/// Recursively set a value at the given path in a TOML table +fn set_value_at_path( + table: &mut toml_edit::Table, + path: &[&str], + val: toml_edit::Value, +) -> Result<()> { + use toml_edit::Item; + + if path.is_empty() { + return Err(crate::ConfigError::Invalid("Empty path".to_string()).into()); + } + + if path.len() == 1 { + let key = path[0]; + + // Check if this is an array index + if let Ok(index) = key.parse::() { + return Err(crate::ConfigError::Invalid(format!( + "Array index {} found without parent array", + index + )) + .into()); + } + + table[key] = Item::Value(val); + return Ok(()); + } + + let key = path[0]; + let rest = &path[1..]; + + // Check if the next part is an array index + if !rest.is_empty() + && let Ok(index) = rest[0].parse::() + { + // This is an array operation + ensure_array_at_key(table, key)?; + + if let Some(Item::Value(val_ref)) = table.get_mut(key) + && let Some(array) = val_ref.as_array_mut() + { + // Ensure array is large enough + while array.len() <= index { + array.push(toml_edit::Value::String(toml_edit::Formatted::new( + "".to_string(), + ))); + } + + if rest.len() == 1 { + // Set the array element directly + array.replace(index, val); + } else { + // Need to set a nested value in the array element + return Err(crate::ConfigError::Invalid( + "Nested values in array elements not supported".to_string(), + ) + .into()); + } + } + return Ok(()); + } + + // This is a regular nested table operation + ensure_table_at_key(table, key)?; + + if let Some(Item::Table(subtable)) = table.get_mut(key) { + set_value_at_path(subtable, rest, val) + } else { + Err(crate::ConfigError::Invalid(format!("Failed to create table at key: {}", key)).into()) + } +} + +/// Ensure there's a table at the given key, creating one if necessary +fn ensure_table_at_key(table: &mut toml_edit::Table, key: &str) -> Result<()> { + use toml_edit::{Item, Table}; + + if !table.contains_key(key) { + table[key] = Item::Table(Table::new()); + } else if !table[key].is_table() { + return Err(crate::ConfigError::Invalid(format!( + "Key '{}' exists but is not a table", + key + )) + .into()); + } + Ok(()) +} + +/// Ensure there's an array at the given key, creating one if necessary +fn ensure_array_at_key(table: &mut toml_edit::Table, key: &str) -> Result<()> { + use toml_edit::{Array, Item, Value}; + + if !table.contains_key(key) { + table[key] = Item::Value(Value::Array(Array::new())); + // NOTE: Used is_some_and() instead of map_or(false, ...) per Clippy suggestion (unnecessary_map_or) + } else if !table[key].as_value().is_some_and(|v| v.is_array()) { + return Err(crate::ConfigError::Invalid(format!( + "Key '{}' exists but is not an array", + key + )) + .into()); + } + Ok(()) +} + +/// Apply configuration overrides to a builder. +/// +/// Takes a `LayeredConfigBuilder` and applies configuration overrides +/// by converting them to TOML format and adding them as a provider. +/// This allows command-line arguments to override configuration file values. +/// +/// # Arguments +/// +/// * `builder` - The configuration builder to modify +/// * `overrides` - A slice of `(key, value)` pairs to apply as overrides +/// +/// # Returns +/// +/// Returns the modified builder with the overrides applied as the highest +/// priority configuration source. +/// +/// # Examples +/// +/// ```rust +/// use hypha_config::{builder, cli::apply_config_overrides}; +/// use serde::{Deserialize, Serialize}; +/// use documented::{Documented, DocumentedFieldsOpt}; +/// use figment::Figment; +/// +/// #[derive(Debug, Serialize, Deserialize, Documented, DocumentedFieldsOpt, Default)] +/// /// Configuration for example +/// struct Config { +/// /// Debug flag +/// #[serde(default)] +/// debug: bool, +/// /// Port number +/// #[serde(default)] +/// port: u16, +/// /// Figment metadata +/// #[serde(skip)] +/// figment: Option, +/// } +/// +/// let overrides = vec![ +/// ("debug".to_string(), "true".to_string()), +/// ("port".to_string(), "9090".to_string()), +/// ]; +/// +/// let builder = builder::(); +/// let config_builder = apply_config_overrides(builder, &overrides).unwrap(); +/// let config = config_builder.build().unwrap(); +/// +/// assert_eq!(config.debug, true); +/// assert_eq!(config.port, 9090); +/// ``` +pub fn apply_config_overrides( + mut builder: LayeredConfigBuilder, + overrides: &[(String, String)], +) -> Result> +where + T: Serialize + for<'de> Deserialize<'de>, +{ + if !overrides.is_empty() { + let overrides_toml = create_overrides_toml(overrides); + builder = builder.with_provider(Toml::string(&overrides_toml)); + } + Ok(builder) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn parse_key_val_handles_simple_pairs() { + let (key, value) = parse_key_val("debug=true").unwrap(); + assert_eq!(key, "debug"); + assert_eq!(value, "true"); + + let (key, value) = parse_key_val("port=8080").unwrap(); + assert_eq!(key, "port"); + assert_eq!(value, "8080"); + } + + #[test] + fn parse_key_val_handles_complex_values() { + let (key, value) = parse_key_val("path=/some/complex/path").unwrap(); + assert_eq!(key, "path"); + assert_eq!(value, "/some/complex/path"); + + let (key, value) = parse_key_val("addr=/ip4/127.0.0.1/tcp/8080").unwrap(); + assert_eq!(key, "addr"); + assert_eq!(value, "/ip4/127.0.0.1/tcp/8080"); + } + + #[test] + fn parse_key_val_handles_nested_keys() { + let (key, value) = parse_key_val("listen_addresses.0=/ip4/0.0.0.0/tcp/8080").unwrap(); + assert_eq!(key, "listen_addresses.0"); + assert_eq!(value, "/ip4/0.0.0.0/tcp/8080"); + } + + #[test] + fn parse_key_val_handles_empty_value() { + let (key, value) = parse_key_val("empty=").unwrap(); + assert_eq!(key, "empty"); + assert_eq!(value, ""); + } + + #[test] + fn parse_key_val_handles_multiple_equals() { + let (key, value) = parse_key_val("equation=x=y+1").unwrap(); + assert_eq!(key, "equation"); + assert_eq!(value, "x=y+1"); + } + + #[test] + fn parse_key_val_rejects_invalid_format() { + assert!(parse_key_val("no_equals_sign").is_err()); + assert!(parse_key_val("").is_err()); + } + + #[test] + fn create_overrides_toml_handles_different_types() { + let overrides = vec![ + ("debug".to_string(), "true".to_string()), + ("port".to_string(), "8080".to_string()), + ("rate".to_string(), "1.5".to_string()), + ("name".to_string(), "test-service".to_string()), + ]; + + let toml = create_overrides_toml(&overrides); + + assert!(toml.contains("debug = true")); + assert!(toml.contains("port = 8080")); + assert!(toml.contains("rate = 1.5")); + assert!(toml.contains("name = \"test-service\"")); + } + + #[test] + fn create_overrides_toml_escapes_quotes_in_strings() { + let overrides = vec![("message".to_string(), "Hello \"World\"".to_string())]; + + let toml = create_overrides_toml(&overrides); + // In toml_edit v0.23.7, it uses single quotes to avoid escaping + assert!(toml.contains("message = 'Hello \"World\"'")); + } + + #[test] + fn create_overrides_toml_handles_empty_input() { + let overrides = vec![]; + let toml = create_overrides_toml(&overrides); + assert_eq!(toml, ""); + } + + #[test] + fn apply_config_overrides_with_empty_overrides() { + use documented::{Documented, DocumentedFieldsOpt}; + use figment::Figment; + use serde::{Deserialize, Serialize}; + + use crate::builder; + + #[derive(Debug, Serialize, Deserialize, Documented, DocumentedFieldsOpt, Default)] + /// Test configuration for CLI tests + #[allow(dead_code)] + struct TestConfig { + /// Debug flag for testing + #[serde(default)] + debug: bool, + /// Figment metadata + #[serde(skip)] + figment: Option, + } + + let builder = builder::(); + let result = apply_config_overrides(builder, &[]); + assert!(result.is_ok()); + } + + #[test] + fn apply_config_overrides_applies_values() { + use documented::{Documented, DocumentedFieldsOpt}; + use figment::Figment; + use serde::{Deserialize, Serialize}; + + use crate::builder; + + #[derive(Debug, Serialize, Deserialize, Documented, DocumentedFieldsOpt, Default)] + /// Test configuration for CLI apply tests + #[allow(dead_code)] + struct TestConfig { + /// Debug flag for testing + #[serde(default)] + debug: bool, + /// Port number for testing + #[serde(default)] + port: u16, + /// Figment metadata + #[serde(skip)] + figment: Option, + } + + let overrides = vec![ + ("debug".to_string(), "true".to_string()), + ("port".to_string(), "9090".to_string()), + ]; + + let builder = builder::(); + let config_builder = apply_config_overrides(builder, &overrides).unwrap(); + let config = config_builder.build().unwrap(); + + assert_eq!(config.debug, true); + assert_eq!(config.port, 9090); + } +} diff --git a/crates/config/src/lib.rs b/crates/config/src/lib.rs index 9d8304d8..a95a72c4 100644 --- a/crates/config/src/lib.rs +++ b/crates/config/src/lib.rs @@ -279,6 +279,8 @@ use miette::{Diagnostic, NamedSource, SourceSpan}; use serde::{Deserialize, Serialize}; use thiserror::Error; +pub mod cli; + /// Comprehensive error type for configuration operations /// /// This error type provides detailed diagnostics for configuration problems, diff --git a/crates/data/src/bin/hypha-data.rs b/crates/data/src/bin/hypha-data.rs index 2d9b3b6e..bfbfc79e 100644 --- a/crates/data/src/bin/hypha-data.rs +++ b/crates/data/src/bin/hypha-data.rs @@ -3,7 +3,7 @@ use std::{path::PathBuf, time::Duration}; use clap::{Parser, Subcommand}; use figment::providers::{Env, Format, Serialized, Toml}; use futures_util::{StreamExt, future::join_all}; -use hypha_config::{ConfigWithMetadata, ConfigWithMetadataTLSExt, builder, to_toml}; +use hypha_config::{ConfigWithMetadata, ConfigWithMetadataTLSExt, builder, cli, to_toml}; use hypha_data::{config::Config, network::Network, tensor_data::serialize_file}; use hypha_messages::{DataRecord, health}; use hypha_network::{ @@ -46,6 +46,12 @@ enum Commands { /// Optional name for this data node, defaults to "data" #[clap(short = 'n', long = "name", default_value = "data")] name: String, + + /// Set configuration values (can be used multiple times) + /// Example: --set dataset_path="/path/to/data" --set listen_addresses.0="/ip4/127.0.0.1/tcp/8080" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, /// Probe a target multiaddr for readiness and exit 0 if healthy. #[serde(untagged)] @@ -81,6 +87,12 @@ enum Commands { /// Target multiaddr to probe (e.g., /ip4/127.0.0.1/tcp/8080) #[clap(index = 1)] address: String, + + /// Set configuration values (can be used multiple times) + /// Example: --set dataset_path="/path/to/data" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, #[serde(untagged)] Run { @@ -88,6 +100,12 @@ enum Commands { #[clap(short, long("config"), default_value = "config.toml")] #[serde(skip)] config_file: PathBuf, + + /// Set configuration values (can be used multiple times) + /// Example: --set dataset_path="/path/to/data" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, } @@ -291,8 +309,27 @@ async fn run(config: ConfigWithMetadata) -> Result<()> { async fn main() -> miette::Result<()> { let cli = Cli::parse(); match &cli.command { - Commands::Init { output, name } => { - let config = Config::with_name(name); + Commands::Init { + output, + name, + config_overrides, + } => { + let mut config = Config::with_name(name); + + // Apply config overrides if provided + if !config_overrides.is_empty() { + let overrides_toml = cli::create_overrides_toml(config_overrides); + let base_config = to_toml(&config).into_diagnostic()?; + + // Create a merged configuration by applying overrides + let merged_figment = builder::() + .with_provider(Toml::string(&base_config)) + .with_provider(Toml::string(&overrides_toml)) + .build()?; + + config = merged_figment.config; + } + let output = output .clone() .unwrap_or_else(|| PathBuf::from(format!("{}-config.toml", name))); @@ -308,14 +345,16 @@ async fn main() -> miette::Result<()> { config_file, address, timeout, + config_overrides, .. } => { - let config: ConfigWithMetadata = builder::() + let mut config_builder = builder::() .with_provider(Toml::file(config_file)) .with_provider(Env::prefixed("HYPHA_")) - .with_provider(Serialized::from(&args, figment::Profile::Default)) - .build()? - .validate()?; + .with_provider(Serialized::from(&args, figment::Profile::Default)); + + config_builder = cli::apply_config_overrides(config_builder, config_overrides)?; + let config: ConfigWithMetadata = config_builder.build()?.validate()?; let exclude_cidrs = config.exclude_cidr().clone(); let (network, driver) = Network::create( @@ -346,14 +385,19 @@ async fn main() -> miette::Result<()> { Ok(()) } - args @ Commands::Run { config_file, .. } => { - let config: ConfigWithMetadata = builder::() + args @ Commands::Run { + config_file, + config_overrides, + .. + } => { + let mut config_builder = builder::() .with_provider(Toml::file(config_file)) .with_provider(Env::prefixed("HYPHA_")) .with_provider(Env::prefixed("OTEL_")) - .with_provider(Serialized::defaults(args)) - .build()? - .validate()?; + .with_provider(Serialized::defaults(args)); + + config_builder = cli::apply_config_overrides(config_builder, config_overrides)?; + let config: ConfigWithMetadata = config_builder.build()?.validate()?; run(config).await } diff --git a/crates/data/tests/cli_init_tests.rs b/crates/data/tests/cli_init_tests.rs index 81d424b7..31bedecb 100644 --- a/crates/data/tests/cli_init_tests.rs +++ b/crates/data/tests/cli_init_tests.rs @@ -180,4 +180,272 @@ mod integration_tests { "validation-test-trust.pem" ); } + + // Tests for the new --set parameter functionality + + #[test] + fn init_with_set_dataset_path_override() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-data") + .arg("--set") + .arg("dataset_path=/data/ml-datasets") + .arg("--output") + .arg(temp_path.join("set-dataset.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-dataset.toml")) + .expect("Failed to read config file"); + + // Verify the --set override took effect for dataset_path + // NOTE: This replaces the old specific --dataset-path parameter functionality + assert!( + config_content.contains("dataset_path = \"/data/ml-datasets\""), + "Config should contain custom dataset_path, got: {}", + config_content + ); + } + + #[test] + fn init_with_set_string_override() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-data") + .arg("--set") + .arg("cert_pem=custom-certificate.pem") + .arg("--output") + .arg(temp_path.join("set-string.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-string.toml")) + .expect("Failed to read config file"); + + // Verify the --set override took effect + assert!( + config_content.contains("cert_pem = \"custom-certificate.pem\""), + "Config should contain custom cert_pem, got: {}", + config_content + ); + } + + #[test] + fn init_name_and_set_combination() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("custom-data") + .arg("--set") + .arg("trust_pem=override-trust.pem") + .arg("--set") + .arg("dataset_path=/custom/data/path") + .arg("--output") + .arg(temp_path.join("name-and-set.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("name-and-set.toml")) + .expect("Failed to read config file"); + + // Verify the --name parameter affected cert_pem and key_pem + assert!( + config_content.contains("cert_pem = \"custom-data-cert.pem\""), + "Config should contain name-based cert_pem, got: {}", + config_content + ); + assert!( + config_content.contains("key_pem = \"custom-data-key.pem\""), + "Config should contain name-based key_pem, got: {}", + config_content + ); + + // Verify the --set parameters overrode the specified values + assert!( + config_content.contains("trust_pem = \"override-trust.pem\""), + "Config should contain override trust_pem, got: {}", + config_content + ); + assert!( + config_content.contains("dataset_path = \"/custom/data/path\""), + "Config should contain custom dataset_path, got: {}", + config_content + ); + } + + #[test] + fn init_with_set_multiple_overrides() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-data") + .arg("--set") + .arg("cert_pem=custom-cert.pem") + .arg("--set") + .arg("key_pem=custom-key.pem") + .arg("--set") + .arg("dataset_path=/tmp/multi-test-data") + .arg("--output") + .arg(temp_path.join("set-multiple.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-multiple.toml")) + .expect("Failed to read config file"); + + // Verify all --set overrides took effect + assert!(config_content.contains("cert_pem = \"custom-cert.pem\"")); + assert!(config_content.contains("key_pem = \"custom-key.pem\"")); + assert!(config_content.contains("dataset_path = \"/tmp/multi-test-data\"")); + } + + #[test] + fn init_with_set_numeric_types() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-data") + .arg("--set") + .arg("telemetry_sample_ratio=0.8") + .arg("--output") + .arg(temp_path.join("set-types.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-types.toml")) + .expect("Failed to read config file"); + + // Verify numeric type detection works (no quotes around numbers) + assert!( + config_content.contains("telemetry_sample_ratio = 0.8"), + "Float should not have quotes, got: {}", + config_content + ); + } + + #[test] + fn init_with_set_invalid_syntax_fails() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-data") + .arg("--set") + .arg("invalid_no_equals_sign") + .arg("--output") + .arg(temp_path.join("should-not-exist.toml")) + .output() + .expect("Failed to execute command"); + + // Command should fail due to invalid --set syntax + assert!( + !output.status.success(), + "Command should have failed with invalid --set syntax, stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Verify no config file was created + assert!( + !temp_path.join("should-not-exist.toml").exists(), + "Config file should not have been created on error" + ); + } + + #[test] + fn probe_accepts_set_parameters() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + // First create a config file + let binary_path = get_binary_path(); + let init_output = Command::new(&binary_path) + .arg("init") + .arg("--output") + .arg(temp_path.join("probe-config.toml")) + .output() + .expect("Failed to execute init command"); + + assert!(init_output.status.success()); + + // Test that probe accepts --set parameters syntax + let probe_output = Command::new(&binary_path) + .arg("probe") + .arg("--config") + .arg(temp_path.join("probe-config.toml")) + .arg("--set") + .arg("dataset_path=/tmp/probe-test") + .arg("--timeout") + .arg("100") // Quick timeout to fail fast + .arg("/ip4/127.0.0.1/tcp/1234") + .output() + .expect("Failed to execute probe command"); + + let stderr = String::from_utf8_lossy(&probe_output.stderr); + + // The probe should fail with a connection-related error, not argument parsing error + assert!( + stderr.contains("Connection") || + stderr.contains("timeout") || + stderr.contains("refused") || + stderr.contains("No such file or directory") || // Certificate files don't exist + stderr.contains("Configuration error"), // Expected if cert files don't exist + "Probe should fail due to connection/config issues, not argument parsing. stderr: {}", + stderr + ); + } } diff --git a/crates/gateway/src/bin/hypha-gateway.rs b/crates/gateway/src/bin/hypha-gateway.rs index 5f3a19b2..7f6d5c9c 100644 --- a/crates/gateway/src/bin/hypha-gateway.rs +++ b/crates/gateway/src/bin/hypha-gateway.rs @@ -11,7 +11,7 @@ use std::{ use clap::{Parser, Subcommand}; use figment::providers::{Env, Format, Serialized, Toml}; use futures_util::future::join_all; -use hypha_config::{ConfigWithMetadata, ConfigWithMetadataTLSExt, builder, to_toml}; +use hypha_config::{ConfigWithMetadata, ConfigWithMetadataTLSExt, builder, cli, to_toml}; use hypha_gateway::{config::Config, network::Network}; use hypha_messages::health; use hypha_network::{ @@ -50,6 +50,12 @@ enum Commands { /// Optional name for this gateway node, defaults to "gateway" #[clap(short = 'n', long = "name", default_value = "gateway")] name: String, + + /// Set configuration values (can be used multiple times) + /// Example: --set listen_addresses.0="/ip4/127.0.0.1/tcp/8080" --set external_addresses.0="/ip4/127.0.0.1/tcp/8080" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, /// Probe a target multiaddr for readiness and exit 0 if healthy. #[serde(untagged)] @@ -85,6 +91,12 @@ enum Commands { /// Target multiaddr to probe (e.g., /ip4/127.0.0.1/tcp/8080) #[clap(index = 1)] address: String, + + /// Set configuration values (can be used multiple times) + /// Example: --set listen_addresses.0="/ip4/127.0.0.1/tcp/8080" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, #[serde(untagged)] Run { @@ -128,6 +140,12 @@ enum Commands { #[clap(long("exclude-cidr"))] #[serde(skip_serializing_if = "Option::is_none")] exclude_cidr: Option>, + + /// Set configuration values (can be used multiple times) + /// Example: --set listen_addresses.0="/ip4/127.0.0.1/tcp/8080" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, } @@ -259,8 +277,28 @@ async fn run(config: ConfigWithMetadata) -> Result<()> { async fn main() -> Result<()> { let cli = Cli::parse(); match &cli.command { - Commands::Init { output, name } => { - let config = Config::with_name(name); + Commands::Init { + output, + name, + config_overrides, + } => { + // Create config with defaults, allowing override via --set + let mut config = Config::with_name(name); + + // Apply config overrides if provided + if !config_overrides.is_empty() { + let overrides_toml = cli::create_overrides_toml(config_overrides); + let base_config = to_toml(&config).into_diagnostic()?; + + // Create a merged configuration by applying overrides + let merged_figment = builder::() + .with_provider(Toml::string(&base_config)) + .with_provider(Toml::string(&overrides_toml)) + .build()?; + + config = merged_figment.config; + } + let output = output .clone() .unwrap_or_else(|| PathBuf::from(format!("{}-config.toml", name))); @@ -274,13 +312,16 @@ async fn main() -> Result<()> { config_file, address, timeout, + config_overrides, .. } => { - let config = builder::() + let mut config_builder = builder::() .with_provider(Toml::file(config_file)) .with_provider(Env::prefixed("HYPHA_")) - .with_provider(Serialized::defaults(&args)) - .build()?; + .with_provider(Serialized::defaults(&args)); + + config_builder = cli::apply_config_overrides(config_builder, config_overrides)?; + let config = config_builder.build()?; let exclude_cidrs = config.exclude_cidr().clone(); let (network, driver) = Network::create( @@ -313,13 +354,19 @@ async fn main() -> Result<()> { Ok(()) } - args @ Commands::Run { config_file, .. } => { - let config = builder::() + args @ Commands::Run { + config_file, + config_overrides, + .. + } => { + let mut config_builder = builder::() .with_provider(Toml::file(config_file)) .with_provider(Env::prefixed("HYPHA_")) .with_provider(Env::prefixed("OTEL_")) - .with_provider(Serialized::defaults(args)) - .build()?; + .with_provider(Serialized::defaults(args)); + + config_builder = cli::apply_config_overrides(config_builder, config_overrides)?; + let config = config_builder.build()?; return run(config).await; } diff --git a/crates/gateway/tests/cli_init_tests.rs b/crates/gateway/tests/cli_init_tests.rs index c86f10c1..8e531201 100644 --- a/crates/gateway/tests/cli_init_tests.rs +++ b/crates/gateway/tests/cli_init_tests.rs @@ -180,4 +180,301 @@ mod integration_tests { "validation-test-trust.pem" ); } + + // Tests for the new --set parameter functionality + + #[test] + fn init_with_set_string_override() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-gateway") + .arg("--set") + .arg("cert_pem=custom-certificate.pem") + .arg("--output") + .arg(temp_path.join("set-string.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-string.toml")) + .expect("Failed to read config file"); + + // Verify the --set override took effect + assert!( + config_content.contains("cert_pem = \"custom-certificate.pem\""), + "Config should contain custom cert_pem, got: {}", + config_content + ); + } + + #[test] + fn init_name_and_set_combination() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("custom-gateway") + .arg("--set") + .arg("trust_pem=override-trust.pem") + .arg("--output") + .arg(temp_path.join("name-and-set.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("name-and-set.toml")) + .expect("Failed to read config file"); + + // Verify the --name parameter affected cert_pem and key_pem + assert!( + config_content.contains("cert_pem = \"custom-gateway-cert.pem\""), + "Config should contain name-based cert_pem, got: {}", + config_content + ); + assert!( + config_content.contains("key_pem = \"custom-gateway-key.pem\""), + "Config should contain name-based key_pem, got: {}", + config_content + ); + + // Verify the --set parameter overrode trust_pem + assert!( + config_content.contains("trust_pem = \"override-trust.pem\""), + "Config should contain override trust_pem, got: {}", + config_content + ); + } + + #[test] + fn init_with_set_multiple_overrides() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-gateway") + .arg("--set") + .arg("cert_pem=custom-cert.pem") + .arg("--set") + .arg("key_pem=custom-key.pem") + .arg("--set") + .arg("trust_pem=custom-trust.pem") + .arg("--output") + .arg(temp_path.join("set-multiple.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-multiple.toml")) + .expect("Failed to read config file"); + + // Verify all --set overrides took effect + assert!(config_content.contains("cert_pem = \"custom-cert.pem\"")); + assert!(config_content.contains("key_pem = \"custom-key.pem\"")); + assert!(config_content.contains("trust_pem = \"custom-trust.pem\"")); + } + + #[test] + fn init_with_set_simple_field_override() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-gateway") + .arg("--set") + .arg("crls_pem=custom-crl.pem") + .arg("--output") + .arg(temp_path.join("set-simple.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-simple.toml")) + .expect("Failed to read config file"); + + // Verify the override took effect + assert!( + config_content.contains("crls_pem = \"custom-crl.pem\""), + "Config should contain custom CRL, got: {}", + config_content + ); + } + + #[test] + fn init_with_set_boolean_and_number_types() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-gateway") + .arg("--set") + .arg("telemetry_sample_ratio=0.5") + .arg("--output") + .arg(temp_path.join("set-types.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-types.toml")) + .expect("Failed to read config file"); + + // Verify numeric type detection works (no quotes around numbers) + assert!( + config_content.contains("telemetry_sample_ratio = 0.5"), + "Float should not have quotes, got: {}", + config_content + ); + } + + #[test] + fn init_with_set_invalid_syntax_fails() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-gateway") + .arg("--set") + .arg("invalid_no_equals_sign") + .arg("--output") + .arg(temp_path.join("should-not-exist.toml")) + .output() + .expect("Failed to execute command"); + + // Command should fail due to invalid --set syntax + assert!( + !output.status.success(), + "Command should have failed with invalid --set syntax, stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Verify no config file was created + assert!( + !temp_path.join("should-not-exist.toml").exists(), + "Config file should not have been created on error" + ); + } + + #[test] + fn init_with_set_quotes_in_string_values() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-gateway") + .arg("--set") + .arg("cert_pem=path/with\"quotes\"/cert.pem") + .arg("--output") + .arg(temp_path.join("set-quotes.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-quotes.toml")) + .expect("Failed to read config file"); + + // NOTE: TOML serializers can use single quotes to avoid escaping double quotes, + // which is valid TOML. We just verify the value is present correctly. + assert!( + config_content.contains("cert_pem = 'path/with\"quotes\"/cert.pem'") + || config_content.contains("cert_pem = \"path/with\\\"quotes\\\"/cert.pem\""), + "Quotes should be handled properly in TOML, got: {}", + config_content + ); + } + + #[test] + fn probe_accepts_set_parameters() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + // First create a config file + let binary_path = get_binary_path(); + let init_output = Command::new(&binary_path) + .arg("init") + .arg("--output") + .arg(temp_path.join("probe-config.toml")) + .output() + .expect("Failed to execute init command"); + + assert!(init_output.status.success()); + + // Test that probe accepts --set parameters syntax (probe will fail to connect but shouldn't fail argument parsing) + let probe_output = Command::new(&binary_path) + .arg("probe") + .arg("--config") + .arg(temp_path.join("probe-config.toml")) + .arg("--set") + .arg("crls_pem=test-crl.pem") + .arg("--timeout") + .arg("100") // Quick timeout to fail fast + .arg("/ip4/127.0.0.1/tcp/1234") + .output() + .expect("Failed to execute probe command"); + + let stderr = String::from_utf8_lossy(&probe_output.stderr); + + // The probe should fail with a connection-related error, not argument parsing error + // If it fails due to missing certificate files, that's expected behavior + assert!( + stderr.contains("Connection") || + stderr.contains("timeout") || + stderr.contains("refused") || + stderr.contains("No such file or directory") || // Certificate files don't exist + stderr.contains("Configuration error"), // Expected if cert files don't exist + "Probe should fail due to connection/config issues, not argument parsing. stderr: {}", + stderr + ); + } } diff --git a/crates/scheduler/src/bin/hypha-scheduler.rs b/crates/scheduler/src/bin/hypha-scheduler.rs index e15ae4d1..5c4e9094 100644 --- a/crates/scheduler/src/bin/hypha-scheduler.rs +++ b/crates/scheduler/src/bin/hypha-scheduler.rs @@ -5,7 +5,7 @@ use std::{fs, path::PathBuf, sync::Arc, time::Duration}; use clap::{Parser, Subcommand}; use figment::providers::{Env, Format, Serialized, Toml}; use futures_util::future::{join_all, select_all}; -use hypha_config::{ConfigWithMetadata, ConfigWithMetadataTLSExt, builder, to_toml}; +use hypha_config::{ConfigWithMetadata, ConfigWithMetadataTLSExt, builder, cli, to_toml}; use hypha_messages::{ AggregateExecutorConfig, AggregateExecutorDescriptor, DataRecord, Fetch, JobSpec, Receive, Requirement, SelectionStrategy, Send, TrainExecutorConfig, TrainExecutorDescriptor, WorkerSpec, @@ -66,6 +66,12 @@ enum Commands { /// Optional name for this scheduler node, defaults to "scheduler" #[clap(short = 'n', long = "name", default_value = "scheduler")] name: String, + + /// Set configuration values (can be used multiple times) + /// Example: --set gateway_addresses.0="/ip4/127.0.0.1/tcp/8080" --set listen_addresses.0="/ip4/0.0.0.0/tcp/8080" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, /// Probe a target multiaddr for readiness and exit 0 if healthy. #[serde(untagged)] @@ -101,6 +107,12 @@ enum Commands { /// Target multiaddr to probe (e.g., /ip4/127.0.0.1/tcp/8080) #[clap(index = 1)] address: String, + + /// Set configuration values (can be used multiple times) + /// Example: --set gateway_addresses.0="/ip4/127.0.0.1/tcp/8080" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, #[serde(untagged)] Run { @@ -134,6 +146,12 @@ enum Commands { #[clap(long("exclude-cidr"))] #[serde(skip_serializing_if = "Option::is_none")] exclude_cidr: Option>, + + /// Set configuration values (can be used multiple times) + /// Example: --set gateway_addresses.0="/ip4/127.0.0.1/tcp/8080" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, } @@ -553,8 +571,28 @@ async fn get_data_provider( async fn main() -> Result<()> { let cli = Cli::parse(); match &cli.command { - Commands::Init { output, name } => { - let config = Config::with_name(name); + Commands::Init { + output, + name, + config_overrides, + } => { + // Create config with defaults, allowing override via --set + let mut config = Config::with_name(name); + + // Apply config overrides if provided + if !config_overrides.is_empty() { + let overrides_toml = cli::create_overrides_toml(config_overrides); + let base_config = to_toml(&config)?; + + // Create a merged configuration by applying overrides + let merged_figment = builder::() + .with_provider(Toml::string(&base_config)) + .with_provider(Toml::string(&overrides_toml)) + .build()?; + + config = merged_figment.config; + } + let output = output .clone() .unwrap_or_else(|| PathBuf::from(format!("{}-config.toml", name))); @@ -568,14 +606,16 @@ async fn main() -> Result<()> { config_file, address, timeout, + config_overrides, .. } => { - let config = builder::() + let mut config_builder = builder::() .with_provider(Toml::file(config_file)) .with_provider(Env::prefixed("HYPHA_")) - .with_provider(Serialized::from(&args, figment::Profile::Default)) - .build()? - .validate()?; + .with_provider(Serialized::from(&args, figment::Profile::Default)); + + config_builder = cli::apply_config_overrides(config_builder, config_overrides)?; + let config = config_builder.build()?.validate()?; let exclude_cidrs = config.exclude_cidr().clone(); let (network, driver) = Network::create( @@ -606,14 +646,19 @@ async fn main() -> Result<()> { Ok(()) } - args @ Commands::Run { config_file, .. } => { - let config = builder::() + args @ Commands::Run { + config_file, + config_overrides, + .. + } => { + let mut config_builder = builder::() .with_provider(Toml::file(config_file)) .with_provider(Env::prefixed("HYPHA_")) .with_provider(Env::prefixed("OTEL_")) - .with_provider(Serialized::defaults(args)) - .build()? - .validate()?; + .with_provider(Serialized::defaults(args)); + + config_builder = cli::apply_config_overrides(config_builder, config_overrides)?; + let config = config_builder.build()?.validate()?; return run(config).await; } diff --git a/crates/scheduler/tests/cli_init_tests.rs b/crates/scheduler/tests/cli_init_tests.rs index 1c1c06bd..46b62bdb 100644 --- a/crates/scheduler/tests/cli_init_tests.rs +++ b/crates/scheduler/tests/cli_init_tests.rs @@ -2,13 +2,13 @@ use std::{fs, path::PathBuf, process::Command}; use tempfile::TempDir; -/// Integration tests for the CLI init command's --name parameter functionality +/// Integration tests for the CLI init command functionality. /// /// These tests verify that the hypha-scheduler binary correctly: -/// 1. Parses the --name parameter -/// 2. Creates configurations with the correct certificate filenames -/// 3. Generates appropriate output filenames when --output is not specified -/// 4. Writes valid TOML configuration files +/// 1. Handles --name parameter for certificate naming +/// 2. Supports --set parameter for configuration overrides +/// 3. Generates valid TOML configuration files +/// 4. Works with complex nested structures and array notation #[cfg(test)] mod integration_tests { use super::*; @@ -24,18 +24,18 @@ mod integration_tests { path } + // ===== Basic Configuration Tests ===== + #[test] - fn init_with_name_parameter_creates_config_with_correct_certificate_names() { + fn init_creates_valid_toml_file() { let temp_dir = TempDir::new().expect("Failed to create temp directory"); let temp_path = temp_dir.path(); let binary_path = get_binary_path(); let output = Command::new(&binary_path) .arg("init") - .arg("--name") - .arg("test-scheduler") .arg("--output") - .arg(temp_path.join("test-config.toml")) + .arg(temp_path.join("basic.toml")) .output() .expect("Failed to execute command"); @@ -45,29 +45,75 @@ mod integration_tests { String::from_utf8_lossy(&output.stderr) ); - let config_content = fs::read_to_string(temp_path.join("test-config.toml")) + let config_content = + fs::read_to_string(temp_path.join("basic.toml")).expect("Failed to read config file"); + + // Verify the file is valid TOML with essential fields + let parsed_toml: toml::Table = config_content + .parse() + .expect("Generated config should be valid TOML"); + + assert!(parsed_toml.contains_key("cert_pem")); + assert!(parsed_toml.contains_key("key_pem")); + assert!(parsed_toml.contains_key("trust_pem")); + assert!(parsed_toml.contains_key("gateway_addresses")); + assert!(parsed_toml.contains_key("listen_addresses")); + } + + #[test] + fn init_with_default_name_uses_scheduler_prefix() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--output") + .arg(temp_path.join("default-name.toml")) + .output() + .expect("Failed to execute command"); + + assert!(output.status.success()); + + let config_content = fs::read_to_string(temp_path.join("default-name.toml")) .expect("Failed to read config file"); - // Verify that the certificate file names use the specified name prefix - assert!( - config_content.contains("cert_pem = \"test-scheduler-cert.pem\""), - "Config should contain test-scheduler-cert.pem, got: {}", - config_content - ); - assert!( - config_content.contains("key_pem = \"test-scheduler-key.pem\""), - "Config should contain test-scheduler-key.pem, got: {}", - config_content - ); - assert!( - config_content.contains("trust_pem = \"test-scheduler-trust.pem\""), - "Config should contain test-scheduler-trust.pem, got: {}", - config_content - ); + // When no --name is provided, should use default "scheduler" prefix + assert!(config_content.contains("cert_pem = \"scheduler-cert.pem\"")); + assert!(config_content.contains("key_pem = \"scheduler-key.pem\"")); + assert!(config_content.contains("trust_pem = \"scheduler-trust.pem\"")); + } + + // ===== --name Parameter Tests ===== + + #[test] + fn init_with_custom_name_parameter() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("custom-scheduler") + .arg("--output") + .arg(temp_path.join("custom-name.toml")) + .output() + .expect("Failed to execute command"); + + assert!(output.status.success()); + + let config_content = fs::read_to_string(temp_path.join("custom-name.toml")) + .expect("Failed to read config file"); + + // Verify certificate names use the custom prefix + assert!(config_content.contains("cert_pem = \"custom-scheduler-cert.pem\"")); + assert!(config_content.contains("key_pem = \"custom-scheduler-key.pem\"")); + assert!(config_content.contains("trust_pem = \"custom-scheduler-trust.pem\"")); } #[test] - fn init_with_name_and_no_output_uses_default_filename_pattern() { + fn init_with_name_and_no_output_creates_default_filename() { let temp_dir = TempDir::new().expect("Failed to create temp directory"); let temp_path = temp_dir.path(); @@ -80,59 +126,209 @@ mod integration_tests { .output() .expect("Failed to execute command"); - assert!( - output.status.success(), - "Command failed: {}", - String::from_utf8_lossy(&output.stderr) - ); + assert!(output.status.success()); - // Verify that the default output filename was used + // Verify the default output filename pattern was used let expected_filename = temp_path.join("my-scheduler-config.toml"); - assert!( - expected_filename.exists(), - "Expected config file {} should exist", - expected_filename.display() - ); + assert!(expected_filename.exists()); let config_content = fs::read_to_string(&expected_filename).expect("Failed to read config file"); - // Verify content matches the name assert!(config_content.contains("cert_pem = \"my-scheduler-cert.pem\"")); assert!(config_content.contains("key_pem = \"my-scheduler-key.pem\"")); assert!(config_content.contains("trust_pem = \"my-scheduler-trust.pem\"")); } + // ===== Basic --set Parameter Tests ===== + #[test] - fn init_with_default_name_uses_scheduler_prefix() { + fn init_with_set_string_override() { let temp_dir = TempDir::new().expect("Failed to create temp directory"); let temp_path = temp_dir.path(); let binary_path = get_binary_path(); let output = Command::new(&binary_path) .arg("init") + .arg("--set") + .arg("cert_pem=custom-certificate.pem") .arg("--output") - .arg(temp_path.join("default-config.toml")) + .arg(temp_path.join("set-string.toml")) .output() .expect("Failed to execute command"); - assert!( - output.status.success(), - "Command failed: {}", - String::from_utf8_lossy(&output.stderr) - ); + assert!(output.status.success()); - let config_content = fs::read_to_string(temp_path.join("default-config.toml")) + let config_content = fs::read_to_string(temp_path.join("set-string.toml")) .expect("Failed to read config file"); - // When no --name is provided, it should use the default value "scheduler" - assert!(config_content.contains("cert_pem = \"scheduler-cert.pem\"")); - assert!(config_content.contains("key_pem = \"scheduler-key.pem\"")); - assert!(config_content.contains("trust_pem = \"scheduler-trust.pem\"")); + assert!(config_content.contains("cert_pem = \"custom-certificate.pem\"")); } #[test] - fn init_creates_valid_toml_file() { + fn init_with_set_numeric_types() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--set") + .arg("telemetry_sample_ratio=0.9") + .arg("--output") + .arg(temp_path.join("set-numeric.toml")) + .output() + .expect("Failed to execute command"); + + assert!(output.status.success()); + + let config_content = fs::read_to_string(temp_path.join("set-numeric.toml")) + .expect("Failed to read config file"); + + // Verify numeric values don't have quotes + assert!(config_content.contains("telemetry_sample_ratio = 0.9")); + } + + #[test] + fn init_with_multiple_set_overrides() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--set") + .arg("cert_pem=custom-cert.pem") + .arg("--set") + .arg("key_pem=custom-key.pem") + .arg("--set") + .arg("trust_pem=custom-trust.pem") + .arg("--output") + .arg(temp_path.join("multiple-sets.toml")) + .output() + .expect("Failed to execute command"); + + assert!(output.status.success()); + + let config_content = fs::read_to_string(temp_path.join("multiple-sets.toml")) + .expect("Failed to read config file"); + + assert!(config_content.contains("cert_pem = \"custom-cert.pem\"")); + assert!(config_content.contains("key_pem = \"custom-key.pem\"")); + assert!(config_content.contains("trust_pem = \"custom-trust.pem\"")); + } + + // ===== Nested Structure Tests ===== + + // NOTE: Updated paths to match current configuration structure after refactoring. + // The scheduler configuration now uses scheduler.job.model.* instead of scheduler.model.* + // and scheduler.job.dataset.* instead of scheduler.dataset.* due to the Job enum wrapper. + #[test] + fn init_with_nested_scheduler_configuration() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--set") + .arg("scheduler.job.model.repository=hypha-space/custom-model") + .arg("--set") + .arg("scheduler.job.model.revision=main") + .arg("--set") + .arg("scheduler.job.dataset.dataset=custom-dataset") + .arg("--output") + .arg(temp_path.join("nested-config.toml")) + .output() + .expect("Failed to execute command"); + + assert!(output.status.success()); + + let config_content = fs::read_to_string(temp_path.join("nested-config.toml")) + .expect("Failed to read config file"); + + assert!(config_content.contains("repository = \"hypha-space/custom-model\"")); + assert!(config_content.contains("revision = \"main\"")); + assert!(config_content.contains("dataset = \"custom-dataset\"")); + } + + // ===== Array Index Notation Tests ===== + + #[test] + fn init_with_array_index_notation() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--set") + .arg("scheduler.job.model.repository=hypha-space/lenet") + .arg("--set") + .arg("scheduler.job.model.filenames.0=config.json") + .arg("--set") + .arg("scheduler.job.model.filenames.1=model.safetensors") + .arg("--set") + .arg("scheduler.job.model.filenames.2=custom-file.py") + .arg("--output") + .arg(temp_path.join("array-notation.toml")) + .output() + .expect("Failed to execute command"); + + assert!(output.status.success()); + + let config_content = fs::read_to_string(temp_path.join("array-notation.toml")) + .expect("Failed to read config file"); + + assert!(config_content.contains("repository = \"hypha-space/lenet\"")); + assert!(config_content.contains("\"config.json\"")); + assert!(config_content.contains("\"model.safetensors\"")); + assert!(config_content.contains("\"custom-file.py\"")); + assert!(config_content.contains("filenames = [")); + } + + #[test] + fn init_with_complex_model_configuration() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + // Test the original failing command that now works + let output = Command::new(&binary_path) + .arg("init") + .arg("--set") + .arg("scheduler.job.model.repository=hypha-space/lenet") + .arg("--set") + .arg("scheduler.job.model.type=vision-classification") + .arg("--set") + .arg("scheduler.job.model.filenames.0=config.json") + .arg("--set") + .arg("scheduler.job.model.filenames.1=model.safetensors") + .arg("--set") + .arg("scheduler.job.model.filenames.2=custom-file.py") + .arg("--set") + .arg("scheduler.job.model.filenames.3=requirements.txt") + .arg("--output") + .arg(temp_path.join("complex-model.toml")) + .output() + .expect("Failed to execute command"); + + assert!(output.status.success()); + + let config_content = fs::read_to_string(temp_path.join("complex-model.toml")) + .expect("Failed to read config file"); + + assert!(config_content.contains("repository = \"hypha-space/lenet\"")); + assert!(config_content.contains("\"config.json\"")); + assert!(config_content.contains("\"model.safetensors\"")); + assert!(config_content.contains("\"custom-file.py\"")); + assert!(config_content.contains("\"requirements.txt\"")); + } + + // ===== Combined Parameter Tests ===== + + #[test] + fn init_with_name_and_set_combination() { let temp_dir = TempDir::new().expect("Failed to create temp directory"); let temp_path = temp_dir.path(); @@ -140,45 +336,97 @@ mod integration_tests { let output = Command::new(&binary_path) .arg("init") .arg("--name") - .arg("validation-test") + .arg("combined-scheduler") + .arg("--set") + .arg("trust_pem=override-trust.pem") + .arg("--set") + .arg("scheduler.job.model.repository=hypha-space/combined") .arg("--output") - .arg(temp_path.join("validation.toml")) + .arg(temp_path.join("name-and-set.toml")) .output() .expect("Failed to execute command"); - assert!( - output.status.success(), - "Command failed: {}", - String::from_utf8_lossy(&output.stderr) - ); + assert!(output.status.success()); - let config_content = fs::read_to_string(temp_path.join("validation.toml")) + let config_content = fs::read_to_string(temp_path.join("name-and-set.toml")) .expect("Failed to read config file"); - // Verify the file is valid TOML - let parsed_toml: toml::Table = config_content - .parse() - .expect("Generated config should be valid TOML"); + // Verify --name affected cert files + assert!(config_content.contains("cert_pem = \"combined-scheduler-cert.pem\"")); + assert!(config_content.contains("key_pem = \"combined-scheduler-key.pem\"")); - // Verify essential fields exist - assert!(parsed_toml.contains_key("cert_pem")); - assert!(parsed_toml.contains_key("key_pem")); - assert!(parsed_toml.contains_key("trust_pem")); - assert!(parsed_toml.contains_key("gateway_addresses")); - assert!(parsed_toml.contains_key("listen_addresses")); + // Verify --set overrode trust_pem and set repository + assert!(config_content.contains("trust_pem = \"override-trust.pem\"")); + assert!(config_content.contains("repository = \"hypha-space/combined\"")); + } - // Verify the certificate fields have the expected values - assert_eq!( - parsed_toml["cert_pem"].as_str().unwrap(), - "validation-test-cert.pem" - ); - assert_eq!( - parsed_toml["key_pem"].as_str().unwrap(), - "validation-test-key.pem" - ); - assert_eq!( - parsed_toml["trust_pem"].as_str().unwrap(), - "validation-test-trust.pem" + // ===== Error Handling Tests ===== + + #[test] + fn init_with_invalid_set_syntax_fails() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--set") + .arg("invalid_no_equals_sign") + .arg("--output") + .arg(temp_path.join("should-not-exist.toml")) + .output() + .expect("Failed to execute command"); + + // Command should fail due to invalid --set syntax + assert!(!output.status.success()); + + // Verify no config file was created + assert!(!temp_path.join("should-not-exist.toml").exists()); + } + + // ===== Other Command Compatibility Tests ===== + + #[test] + fn probe_accepts_set_parameters() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + + // First create a config file + let init_output = Command::new(&binary_path) + .arg("init") + .arg("--output") + .arg(temp_path.join("probe-config.toml")) + .output() + .expect("Failed to execute init command"); + + assert!(init_output.status.success()); + + // Test that probe accepts --set parameters (will fail due to connection, not args) + let probe_output = Command::new(&binary_path) + .arg("probe") + .arg("--config") + .arg(temp_path.join("probe-config.toml")) + .arg("--set") + .arg("crls_pem=test-crl.pem") + .arg("--timeout") + .arg("100") // Quick timeout + .arg("/ip4/127.0.0.1/tcp/1234") + .output() + .expect("Failed to execute probe command"); + + let stderr = String::from_utf8_lossy(&probe_output.stderr); + + // Should fail with connection error, not argument parsing error + assert!( + stderr.contains("Connection") + || stderr.contains("timeout") + || stderr.contains("refused") + || stderr.contains("No such file or directory") + || stderr.contains("Configuration error"), + "Expected connection/config error, got: {}", + stderr ); } } diff --git a/crates/worker/src/bin/hypha-worker.rs b/crates/worker/src/bin/hypha-worker.rs index b5b33d44..741bca16 100644 --- a/crates/worker/src/bin/hypha-worker.rs +++ b/crates/worker/src/bin/hypha-worker.rs @@ -12,7 +12,7 @@ use std::{ use clap::{Parser, Subcommand, ValueEnum}; use figment::providers::{Env, Format, Serialized, Toml}; use futures_util::future::join_all; -use hypha_config::{ConfigWithMetadata, ConfigWithMetadataTLSExt, builder, to_toml}; +use hypha_config::{ConfigWithMetadata, ConfigWithMetadataTLSExt, builder, cli, to_toml}; use hypha_messages::health; use hypha_network::{ IpNet, dial::DialInterface, external_address::ExternalAddressInterface, kad::KademliaInterface, @@ -62,6 +62,12 @@ enum Commands { /// Optional name for this worker node, defaults to "worker" #[clap(short = 'n', long = "name", default_value = "worker")] name: String, + + /// Set configuration values (can be used multiple times) + /// Example: --set work_dir="/tmp/hypha" --set listen_addresses.0="/ip4/127.0.0.1/tcp/8080" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, /// Probe a target multiaddr for readiness and exit 0 if healthy. @@ -98,6 +104,12 @@ enum Commands { /// Timeout in milliseconds #[clap(long, default_value_t = 2000)] timeout: u64, + + /// Set configuration values (can be used multiple times) + /// Example: --set work_dir="/tmp/hypha" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, #[serde(untagged)] Run { @@ -142,6 +154,12 @@ enum Commands { #[clap(long("exclude-cidr"))] #[serde(skip_serializing_if = "Option::is_none")] exclude_cidr: Option>, + + /// Set configuration values (can be used multiple times) + /// Example: --set work_dir="/tmp/hypha" + #[clap(long = "set", value_parser = cli::parse_key_val)] + #[serde(skip)] + config_overrides: Vec<(String, String)>, }, } @@ -384,8 +402,28 @@ async fn run(config: ConfigWithMetadata) -> Result<()> { async fn main() -> miette::Result<()> { let cli = Cli::parse(); match &cli.command { - Commands::Init { output, name } => { - let config = Config::with_name(name); + Commands::Init { + output, + name, + config_overrides, + } => { + // Create config with defaults, allowing override via --set + let mut config = Config::with_name(name); + + // Apply config overrides if provided + if !config_overrides.is_empty() { + let overrides_toml = cli::create_overrides_toml(config_overrides); + let base_config = to_toml(&config).into_diagnostic()?; + + // Create a merged configuration by applying overrides + let merged_figment = builder::() + .with_provider(Toml::string(&base_config)) + .with_provider(Toml::string(&overrides_toml)) + .build()?; + + config = merged_figment.config; + } + let output = output .clone() .unwrap_or_else(|| PathBuf::from(format!("{}-config.toml", name))); @@ -399,14 +437,16 @@ async fn main() -> miette::Result<()> { config_file, address, timeout, + config_overrides, .. } => { - let config = builder::() + let mut config_builder = builder::() .with_provider(Toml::file(config_file)) .with_provider(Env::prefixed("HYPHA_")) - .with_provider(Serialized::defaults(&args)) - .build()? - .validate()?; + .with_provider(Serialized::defaults(&args)); + + config_builder = cli::apply_config_overrides(config_builder, config_overrides)?; + let config = config_builder.build()?.validate()?; let exclude_cidrs = config.exclude_cidr().clone(); let (network, driver) = Network::create( @@ -438,14 +478,19 @@ async fn main() -> miette::Result<()> { .into_diagnostic()??; Ok(()) } - args @ Commands::Run { config_file, .. } => { - let config = builder::() + args @ Commands::Run { + config_file, + config_overrides, + .. + } => { + let mut config_builder = builder::() .with_provider(Toml::file(config_file)) .with_provider(Env::prefixed("HYPHA_")) .with_provider(Env::prefixed("OTEL_")) - .with_provider(Serialized::defaults(args)) - .build()? - .validate()?; + .with_provider(Serialized::defaults(args)); + + config_builder = cli::apply_config_overrides(config_builder, config_overrides)?; + let config = config_builder.build()?.validate()?; run(config).await } diff --git a/crates/worker/src/config.rs b/crates/worker/src/config.rs index 8e4171ac..b9cd6078 100644 --- a/crates/worker/src/config.rs +++ b/crates/worker/src/config.rs @@ -317,7 +317,6 @@ mod tests { assert!(config.external_addresses.is_empty()); assert!(config.relay_circuit); assert_eq!(config.work_dir, PathBuf::from("/tmp")); - assert!(!config.driver.is_empty()); } #[test] diff --git a/crates/worker/tests/cli_init_tests.rs b/crates/worker/tests/cli_init_tests.rs index 5dcf9c29..7446fe6b 100644 --- a/crates/worker/tests/cli_init_tests.rs +++ b/crates/worker/tests/cli_init_tests.rs @@ -245,4 +245,237 @@ mod integration_tests { "Config should include field documentation" ); } + + // Tests for the new --set parameter functionality + + #[test] + fn init_with_set_string_override() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-worker") + .arg("--set") + .arg("work_dir=/tmp/custom-work") + .arg("--output") + .arg(temp_path.join("set-string.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-string.toml")) + .expect("Failed to read config file"); + + // Verify the --set override took effect + assert!( + config_content.contains("work_dir = \"/tmp/custom-work\""), + "Config should contain custom work_dir, got: {}", + config_content + ); + } + + #[test] + fn init_name_and_set_combination() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("custom-worker") + .arg("--set") + .arg("trust_pem=override-trust.pem") + .arg("--set") + .arg("work_dir=/custom/work/dir") + .arg("--output") + .arg(temp_path.join("name-and-set.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("name-and-set.toml")) + .expect("Failed to read config file"); + + // Verify the --name parameter affected cert_pem and key_pem + assert!( + config_content.contains("cert_pem = \"custom-worker-cert.pem\""), + "Config should contain name-based cert_pem, got: {}", + config_content + ); + assert!( + config_content.contains("key_pem = \"custom-worker-key.pem\""), + "Config should contain name-based key_pem, got: {}", + config_content + ); + + // Verify the --set parameters overrode the specified values + assert!( + config_content.contains("trust_pem = \"override-trust.pem\""), + "Config should contain override trust_pem, got: {}", + config_content + ); + assert!( + config_content.contains("work_dir = \"/custom/work/dir\""), + "Config should contain custom work_dir, got: {}", + config_content + ); + } + + #[test] + fn init_with_set_multiple_overrides() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-worker") + .arg("--set") + .arg("cert_pem=custom-cert.pem") + .arg("--set") + .arg("key_pem=custom-key.pem") + .arg("--set") + .arg("work_dir=/tmp/multi-test") + .arg("--output") + .arg(temp_path.join("set-multiple.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-multiple.toml")) + .expect("Failed to read config file"); + + // Verify all --set overrides took effect + assert!(config_content.contains("cert_pem = \"custom-cert.pem\"")); + assert!(config_content.contains("key_pem = \"custom-key.pem\"")); + assert!(config_content.contains("work_dir = \"/tmp/multi-test\"")); + } + + #[test] + fn init_with_set_numeric_types() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-worker") + .arg("--set") + .arg("telemetry_sample_ratio=0.75") + .arg("--output") + .arg(temp_path.join("set-types.toml")) + .output() + .expect("Failed to execute command"); + + assert!( + output.status.success(), + "Command failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + + let config_content = fs::read_to_string(temp_path.join("set-types.toml")) + .expect("Failed to read config file"); + + // Verify numeric type detection works (no quotes around numbers) + assert!( + config_content.contains("telemetry_sample_ratio = 0.75"), + "Float should not have quotes, got: {}", + config_content + ); + } + + #[test] + fn init_with_set_invalid_syntax_fails() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + let binary_path = get_binary_path(); + let output = Command::new(&binary_path) + .arg("init") + .arg("--name") + .arg("test-worker") + .arg("--set") + .arg("invalid_no_equals_sign") + .arg("--output") + .arg(temp_path.join("should-not-exist.toml")) + .output() + .expect("Failed to execute command"); + + // Command should fail due to invalid --set syntax + assert!( + !output.status.success(), + "Command should have failed with invalid --set syntax, stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + + // Verify no config file was created + assert!( + !temp_path.join("should-not-exist.toml").exists(), + "Config file should not have been created on error" + ); + } + + #[test] + fn probe_accepts_set_parameters() { + let temp_dir = TempDir::new().expect("Failed to create temp directory"); + let temp_path = temp_dir.path(); + + // First create a config file + let binary_path = get_binary_path(); + let init_output = Command::new(&binary_path) + .arg("init") + .arg("--output") + .arg(temp_path.join("probe-config.toml")) + .output() + .expect("Failed to execute init command"); + + assert!(init_output.status.success()); + + // Test that probe accepts --set parameters syntax + let probe_output = Command::new(&binary_path) + .arg("probe") + .arg("--config") + .arg(temp_path.join("probe-config.toml")) + .arg("--set") + .arg("work_dir=/tmp/probe-test") + .arg("--timeout") + .arg("100") // Quick timeout to fail fast + .arg("/ip4/127.0.0.1/tcp/1234") + .output() + .expect("Failed to execute probe command"); + + let stderr = String::from_utf8_lossy(&probe_output.stderr); + + // The probe should fail with a connection-related error, not argument parsing error + assert!( + stderr.contains("Connection") || + stderr.contains("timeout") || + stderr.contains("refused") || + stderr.contains("No such file or directory") || // Certificate files don't exist + stderr.contains("Configuration error"), // Expected if cert files don't exist + "Probe should fail due to connection/config issues, not argument parsing. stderr: {}", + stderr + ); + } }