From 4b56ca85b1bdf5ac0acf48ea451359d011868ac6 Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Fri, 19 Dec 2025 18:04:25 +0900 Subject: [PATCH 1/4] feat: Add jump_host field support in config.yaml (issue #115) Add jump_host configuration support at three levels: - Global defaults (Defaults struct) - Cluster-level (ClusterDefaults struct) - Node-level (NodeConfig::Detailed variant) Resolution priority (highest to lowest): 1. Node-level jump_host 2. Cluster-level jump_host 3. Global default jump_host 4. CLI -J option (fallback) Empty string ("") explicitly disables jump host inheritance. Also enables -J support for ping/upload/download commands by: - Adding jump_hosts field to FileTransferParams - Passing resolved jump_hosts to ping_nodes function - Updating dispatcher to resolve and pass jump_hosts to all handlers Includes comprehensive unit tests for all resolution scenarios. --- src/app/dispatcher.rs | 21 ++++ src/commands/download.rs | 9 +- src/commands/ping.rs | 3 +- src/commands/upload.rs | 5 +- src/config/resolver.rs | 59 +++++++++- src/config/tests.rs | 226 +++++++++++++++++++++++++++++++++++++++ src/config/types.rs | 10 ++ 7 files changed, 326 insertions(+), 7 deletions(-) diff --git a/src/app/dispatcher.rs b/src/app/dispatcher.rs index 3b258e9d..bfbbddb6 100644 --- a/src/app/dispatcher.rs +++ b/src/app/dispatcher.rs @@ -79,6 +79,12 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> { let use_keychain = determine_use_keychain(&ctx.ssh_config, hostname_for_ssh_config.as_deref()); + // Resolve jump_hosts: CLI takes precedence, then config + let jump_hosts = cli.jump_hosts.clone().or_else(|| { + ctx.config + .get_cluster_jump_host(ctx.cluster_name.as_deref().or(cli.cluster.as_deref())) + }); + ping_nodes( ctx.nodes.clone(), ctx.max_parallel, @@ -90,6 +96,7 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> { use_keychain, cli.timeout, Some(cli.connect_timeout), + jump_hosts, ) .await } @@ -106,6 +113,12 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> { ctx.cluster_name.as_deref().or(cli.cluster.as_deref()), ); + // Resolve jump_hosts: CLI takes precedence, then config + let jump_hosts = cli.jump_hosts.clone().or_else(|| { + ctx.config + .get_cluster_jump_host(ctx.cluster_name.as_deref().or(cli.cluster.as_deref())) + }); + let params = FileTransferParams { nodes: ctx.nodes.clone(), max_parallel: ctx.max_parallel, @@ -115,6 +128,7 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> { use_password: cli.password, recursive: *recursive, ssh_config: Some(&ctx.ssh_config), + jump_hosts, }; upload_file(params, source, destination).await } @@ -131,6 +145,12 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> { ctx.cluster_name.as_deref().or(cli.cluster.as_deref()), ); + // Resolve jump_hosts: CLI takes precedence, then config + let jump_hosts = cli.jump_hosts.clone().or_else(|| { + ctx.config + .get_cluster_jump_host(ctx.cluster_name.as_deref().or(cli.cluster.as_deref())) + }); + let params = FileTransferParams { nodes: ctx.nodes.clone(), max_parallel: ctx.max_parallel, @@ -140,6 +160,7 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> { use_password: cli.password, recursive: *recursive, ssh_config: Some(&ctx.ssh_config), + jump_hosts, }; download_file(params, source, destination).await } diff --git a/src/commands/download.rs b/src/commands/download.rs index 1f8a8d62..86f7b7f0 100644 --- a/src/commands/download.rs +++ b/src/commands/download.rs @@ -56,7 +56,8 @@ pub async fn download_file( params.strict_mode, params.use_agent, params.use_password, - ); + ) + .with_jump_hosts(params.jump_hosts.clone()); if let Some(ssh_config) = params.ssh_config { executor = executor.with_ssh_config(Some(ssh_config.clone())); } @@ -117,9 +118,9 @@ pub async fn download_file( params.strict_mode, params.use_agent, params.use_password, - None, // No jump hosts from this code path (ssh_config handles ProxyJump) - None, // Use default connect timeout - params.ssh_config, // Pass ssh_config for ProxyJump resolution + params.jump_hosts.as_deref(), // Pass jump hosts from params + None, // Use default connect timeout + params.ssh_config, // Pass ssh_config for ProxyJump resolution ) .await; diff --git a/src/commands/ping.rs b/src/commands/ping.rs index 68ce2cc4..ed53cd92 100644 --- a/src/commands/ping.rs +++ b/src/commands/ping.rs @@ -32,6 +32,7 @@ pub async fn ping_nodes( #[cfg(target_os = "macos")] use_keychain: bool, timeout: Option, connect_timeout: Option, + jump_hosts: Option, ) -> Result<()> { println!( "{}", @@ -54,7 +55,7 @@ pub async fn ping_nodes( ) .with_timeout(ping_timeout) .with_connect_timeout(connect_timeout) - .with_jump_hosts(None); + .with_jump_hosts(jump_hosts); #[cfg(target_os = "macos")] let executor = executor.with_keychain(use_keychain); diff --git a/src/commands/upload.rs b/src/commands/upload.rs index 92871fd2..3e468ba5 100644 --- a/src/commands/upload.rs +++ b/src/commands/upload.rs @@ -32,6 +32,8 @@ pub struct FileTransferParams<'a> { pub use_password: bool, pub recursive: bool, pub ssh_config: Option<&'a SshConfig>, + /// Jump hosts specification for connections. + pub jump_hosts: Option, } pub async fn upload_file( @@ -85,7 +87,8 @@ pub async fn upload_file( params.strict_mode, params.use_agent, params.use_password, - ); + ) + .with_jump_hosts(params.jump_hosts.clone()); if let Some(ssh_config) = params.ssh_config { executor = executor.with_ssh_config(Some(ssh_config.clone())); } diff --git a/src/config/resolver.rs b/src/config/resolver.rs index 0c9c1391..f1b49f08 100644 --- a/src/config/resolver.rs +++ b/src/config/resolver.rs @@ -57,7 +57,9 @@ impl Config { n })? } - NodeConfig::Detailed { host, port, user } => { + NodeConfig::Detailed { + host, port, user, .. + } => { // Expand environment variables let expanded_host = expand_env_vars(host); @@ -121,4 +123,59 @@ impl Config { self.defaults.parallel } + + /// Get jump host for a specific node in a cluster. + /// + /// Resolution priority (highest to lowest): + /// 1. Node-level `jump_host` (in `NodeConfig::Detailed`) + /// 2. Cluster-level `jump_host` (in `ClusterDefaults`) + /// 3. Global default `jump_host` (in `Defaults`) + /// + /// Empty string (`""`) explicitly disables jump host inheritance. + pub fn get_jump_host(&self, cluster_name: &str, node_index: usize) -> Option { + if let Some(cluster) = self.get_cluster(cluster_name) { + // Check node-level first + if let Some(NodeConfig::Detailed { + jump_host: Some(jh), + .. + }) = cluster.nodes.get(node_index) + { + if jh.is_empty() { + return None; // Explicitly disabled + } + return Some(jh.clone()); + } + // Check cluster-level + if let Some(jh) = &cluster.defaults.jump_host { + if jh.is_empty() { + return None; // Explicitly disabled + } + return Some(jh.clone()); + } + } + // Fall back to global default + self.defaults.jump_host.clone().filter(|s| !s.is_empty()) + } + + /// Get jump host for a cluster (cluster-level default). + /// + /// Resolution priority (highest to lowest): + /// 1. Cluster-level `jump_host` (in `ClusterDefaults`) + /// 2. Global default `jump_host` (in `Defaults`) + /// + /// Empty string (`""`) explicitly disables jump host inheritance. + pub fn get_cluster_jump_host(&self, cluster_name: Option<&str>) -> Option { + if let Some(cluster_name) = cluster_name { + if let Some(cluster) = self.get_cluster(cluster_name) { + if let Some(jh) = &cluster.defaults.jump_host { + if jh.is_empty() { + return None; // Explicitly disabled + } + return Some(jh.clone()); + } + } + } + // Fall back to global default + self.defaults.jump_host.clone().filter(|s| !s.is_empty()) + } } diff --git a/src/config/tests.rs b/src/config/tests.rs index 34c9dc52..e8b57788 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -237,3 +237,229 @@ fn test_backendai_env_parsing() { std::env::remove_var("BACKENDAI_CLUSTER_ROLE"); } } + +#[test] +fn test_jump_host_global_default() { + let yaml = r#" +defaults: + user: admin + jump_host: bastion.example.com + +clusters: + production: + nodes: + - host: prod1.internal + - host: prod2.internal +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Both nodes should inherit global jump_host + assert_eq!( + config.get_jump_host("production", 0), + Some("bastion.example.com".to_string()) + ); + assert_eq!( + config.get_jump_host("production", 1), + Some("bastion.example.com".to_string()) + ); + + // get_cluster_jump_host should also return global default + assert_eq!( + config.get_cluster_jump_host(Some("production")), + Some("bastion.example.com".to_string()) + ); +} + +#[test] +fn test_jump_host_cluster_override() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - host: prod1.internal + - host: prod2.internal + jump_host: prod-bastion.example.com + + staging: + nodes: + - host: staging1.internal +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // production cluster nodes should use cluster jump_host + assert_eq!( + config.get_jump_host("production", 0), + Some("prod-bastion.example.com".to_string()) + ); + assert_eq!( + config.get_jump_host("production", 1), + Some("prod-bastion.example.com".to_string()) + ); + + // staging cluster should fall back to global default + assert_eq!( + config.get_jump_host("staging", 0), + Some("global-bastion.example.com".to_string()) + ); + + // get_cluster_jump_host should return cluster-level jump_host + assert_eq!( + config.get_cluster_jump_host(Some("production")), + Some("prod-bastion.example.com".to_string()) + ); + assert_eq!( + config.get_cluster_jump_host(Some("staging")), + Some("global-bastion.example.com".to_string()) + ); +} + +#[test] +fn test_jump_host_node_override() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - host: prod1.internal + jump_host: prod1-bastion.example.com + - host: prod2.internal + - host: prod3.internal + jump_host: prod3-bastion:2222 + jump_host: prod-bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // prod1 should use node-level jump_host + assert_eq!( + config.get_jump_host("production", 0), + Some("prod1-bastion.example.com".to_string()) + ); + + // prod2 should use cluster-level jump_host (no node override) + assert_eq!( + config.get_jump_host("production", 1), + Some("prod-bastion.example.com".to_string()) + ); + + // prod3 should use node-level jump_host with custom port + assert_eq!( + config.get_jump_host("production", 2), + Some("prod3-bastion:2222".to_string()) + ); +} + +#[test] +fn test_jump_host_explicit_disable() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - host: prod1.internal + - host: prod2.internal + jump_host: "" + jump_host: prod-bastion.example.com + + direct_access: + nodes: + - host: direct1.example.com + jump_host: "" +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // prod1 should use cluster jump_host + assert_eq!( + config.get_jump_host("production", 0), + Some("prod-bastion.example.com".to_string()) + ); + + // prod2 should have no jump_host (explicitly disabled with empty string) + assert_eq!(config.get_jump_host("production", 1), None); + + // direct_access cluster disables jump_host at cluster level + assert_eq!(config.get_jump_host("direct_access", 0), None); + + // get_cluster_jump_host should return None for explicitly disabled + assert_eq!(config.get_cluster_jump_host(Some("direct_access")), None); +} + +#[test] +fn test_jump_host_no_config() { + let yaml = r#" +defaults: + user: admin + +clusters: + production: + nodes: + - host: prod1.internal +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // No jump_host configured anywhere + assert_eq!(config.get_jump_host("production", 0), None); + assert_eq!(config.get_cluster_jump_host(Some("production")), None); + assert_eq!(config.get_cluster_jump_host(None), None); +} + +#[test] +fn test_jump_host_nonexistent_cluster() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - host: prod1.internal +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Nonexistent cluster should return global default from get_cluster_jump_host + assert_eq!( + config.get_cluster_jump_host(Some("nonexistent")), + Some("global-bastion.example.com".to_string()) + ); + + // get_jump_host for nonexistent cluster returns global default + assert_eq!( + config.get_jump_host("nonexistent", 0), + Some("global-bastion.example.com".to_string()) + ); +} + +#[test] +fn test_jump_host_simple_node_config() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - simple-node.internal + jump_host: prod-bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Simple node (string) should inherit cluster jump_host + // Simple nodes cannot have node-level jump_host override + assert_eq!( + config.get_jump_host("production", 0), + Some("prod-bastion.example.com".to_string()) + ); +} diff --git a/src/config/types.rs b/src/config/types.rs index 3a2f0a43..9cb859a2 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -38,6 +38,9 @@ pub struct Defaults { pub ssh_key: Option, pub parallel: Option, pub timeout: Option, + /// Jump host specification for all connections. + /// Empty string explicitly disables jump host inheritance. + pub jump_host: Option, } /// Interactive mode configuration. @@ -117,6 +120,9 @@ pub struct ClusterDefaults { pub ssh_key: Option, pub parallel: Option, pub timeout: Option, + /// Jump host specification for this cluster. + /// Empty string explicitly disables jump host inheritance. + pub jump_host: Option, } /// Node configuration within a cluster. @@ -130,6 +136,10 @@ pub enum NodeConfig { port: Option, #[serde(default)] user: Option, + /// Jump host specification for this node. + /// Empty string explicitly disables jump host inheritance. + #[serde(default)] + jump_host: Option, }, } From de8ae6d8a95e72b94581054617f0892ff338818d Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Fri, 19 Dec 2025 18:13:52 +0900 Subject: [PATCH 2/4] fix: Apply expand_env_vars to jump_host and add config fallback for exec/interactive - Add environment variable expansion to get_jump_host() and get_cluster_jump_host() - Add config fallback for jump_hosts in exec command (was only using CLI option) - Add config fallback for jump_hosts in interactive commands - Add test for environment variable expansion in jump_host - Update README with jump_host configuration documentation Addresses review feedback on PR #120 (issue #115) --- README.md | 40 ++++++++++++++++++++++++++++++++++- src/app/dispatcher.rs | 24 ++++++++++++++++++--- src/config/resolver.rs | 18 +++++++++++----- src/config/tests.rs | 48 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 121 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index d49f6958..a377679c 100644 --- a/README.md +++ b/README.md @@ -699,6 +699,7 @@ defaults: ssh_key: ~/.ssh/id_rsa parallel: 10 timeout: 300 # Command timeout in seconds (0 for unlimited) + jump_host: bastion.example.com # Global default jump host (optional) # Global interactive mode settings (optional) interactive: @@ -726,21 +727,58 @@ clusters: - user@web3.example.com:2222 ssh_key: ~/.ssh/prod_key timeout: 600 # Override default timeout for this cluster + jump_host: prod-bastion.example.com # Cluster-specific jump host # Cluster-specific interactive settings (overrides global) interactive: default_mode: single_node prompt_format: "prod> " work_dir: /var/www - + staging: nodes: - host: staging1.example.com port: 2200 user: deploy + jump_host: staging-bastion:2222 # Node-specific jump host - staging2.example.com user: staging_user + jump_host: "" # Explicitly disable jump host for this cluster +``` + +### Jump Host Configuration in config.yaml + +bssh supports configuring jump hosts at three levels in your configuration file, with environment variable support: + +**Priority Order** (highest to lowest): +1. **CLI `-J` option** - Always takes precedence +2. **Node-level** - Per-node `jump_host` in detailed node config +3. **Cluster-level** - `jump_host` in cluster defaults +4. **Global default** - `jump_host` in top-level defaults + +**Example:** +```yaml +defaults: + jump_host: ${BASTION_HOST} # Environment variables supported + +clusters: + production: + nodes: + - host: prod1.internal + - host: prod2.internal + jump_host: "" # Explicitly disable for this node + jump_host: prod-bastion.example.com + + direct-access: + nodes: + - host: direct.example.com + jump_host: "" # Empty string disables inheritance ``` +**Notes:** +- Empty string (`""`) explicitly disables jump host inheritance at any level +- Environment variables (`${VAR}` or `$VAR`) are expanded in jump_host values +- Node-level jump_host requires detailed node syntax (not simple hostname strings) + ## SSH Configuration Support bssh fully supports OpenSSH-compatible configuration files via the `-F` flag or default SSH config locations (`~/.ssh/config`, `/etc/ssh/ssh_config`). In addition to standard SSH directives, bssh supports advanced options for certificate-based authentication and port forwarding control. diff --git a/src/app/dispatcher.rs b/src/app/dispatcher.rs index bfbbddb6..961d843a 100644 --- a/src/app/dispatcher.rs +++ b/src/app/dispatcher.rs @@ -270,6 +270,12 @@ async fn handle_interactive_command( #[cfg(target_os = "macos")] let use_keychain = determine_use_keychain(&ctx.ssh_config, hostname.as_deref()); + // Resolve jump_hosts: CLI takes precedence, then config + let jump_hosts = cli.jump_hosts.clone().or_else(|| { + ctx.config + .get_cluster_jump_host(ctx.cluster_name.as_deref().or(cli.cluster.as_deref())) + }); + let interactive_cmd = InteractiveCommand { single_node: merged_mode.0, multiplex: merged_mode.1, @@ -286,7 +292,7 @@ async fn handle_interactive_command( #[cfg(target_os = "macos")] use_keychain, strict_mode: ctx.strict_mode, - jump_hosts: cli.jump_hosts.clone(), + jump_hosts, pty_config, use_pty, }; @@ -332,6 +338,12 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu #[cfg(target_os = "macos")] let use_keychain = determine_use_keychain(&ctx.ssh_config, hostname.as_deref()); + // Resolve jump_hosts: CLI takes precedence, then config + let jump_hosts = cli.jump_hosts.clone().or_else(|| { + ctx.config + .get_cluster_jump_host(ctx.cluster_name.as_deref().or(cli.cluster.as_deref())) + }); + let interactive_cmd = InteractiveCommand { single_node: true, multiplex: false, @@ -348,7 +360,7 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu #[cfg(target_os = "macos")] use_keychain, strict_mode: ctx.strict_mode, - jump_hosts: cli.jump_hosts.clone(), + jump_hosts, pty_config, use_pty, }; @@ -403,6 +415,12 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu None }; + // Resolve jump_hosts: CLI takes precedence, then config + let jump_hosts = cli.jump_hosts.clone().or_else(|| { + ctx.config + .get_cluster_jump_host(ctx.cluster_name.as_deref().or(cli.cluster.as_deref())) + }); + let params = ExecuteCommandParams { nodes: ctx.nodes.clone(), command, @@ -419,7 +437,7 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu no_prefix: cli.no_prefix, timeout, connect_timeout: Some(cli.connect_timeout), - jump_hosts: cli.jump_hosts.as_deref(), + jump_hosts: jump_hosts.as_deref(), port_forwards: if cli.has_port_forwards() { Some(cli.parse_port_forwards()?) } else { diff --git a/src/config/resolver.rs b/src/config/resolver.rs index f1b49f08..7865bfe4 100644 --- a/src/config/resolver.rs +++ b/src/config/resolver.rs @@ -143,18 +143,22 @@ impl Config { if jh.is_empty() { return None; // Explicitly disabled } - return Some(jh.clone()); + return Some(expand_env_vars(jh)); } // Check cluster-level if let Some(jh) = &cluster.defaults.jump_host { if jh.is_empty() { return None; // Explicitly disabled } - return Some(jh.clone()); + return Some(expand_env_vars(jh)); } } // Fall back to global default - self.defaults.jump_host.clone().filter(|s| !s.is_empty()) + self.defaults + .jump_host + .as_ref() + .filter(|s| !s.is_empty()) + .map(|s| expand_env_vars(s)) } /// Get jump host for a cluster (cluster-level default). @@ -171,11 +175,15 @@ impl Config { if jh.is_empty() { return None; // Explicitly disabled } - return Some(jh.clone()); + return Some(expand_env_vars(jh)); } } } // Fall back to global default - self.defaults.jump_host.clone().filter(|s| !s.is_empty()) + self.defaults + .jump_host + .as_ref() + .filter(|s| !s.is_empty()) + .map(|s| expand_env_vars(s)) } } diff --git a/src/config/tests.rs b/src/config/tests.rs index e8b57788..fbfab61c 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -463,3 +463,51 @@ clusters: Some("prod-bastion.example.com".to_string()) ); } + +#[test] +fn test_jump_host_env_var_expansion() { + // Set up test environment variables + unsafe { + std::env::set_var("TEST_BASTION_HOST", "bastion.example.com"); + std::env::set_var("TEST_BASTION_PORT", "2222"); + } + + let yaml = r#" +defaults: + jump_host: ${TEST_BASTION_HOST} + +clusters: + production: + nodes: + - host: prod1.internal + jump_host: $TEST_BASTION_HOST:$TEST_BASTION_PORT + - host: prod2.internal + jump_host: ${TEST_BASTION_HOST}:${TEST_BASTION_PORT} +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Node-level with $VAR syntax + assert_eq!( + config.get_jump_host("production", 0), + Some("bastion.example.com:2222".to_string()) + ); + + // Cluster-level with ${VAR} syntax + assert_eq!( + config.get_jump_host("production", 1), + Some("bastion.example.com:2222".to_string()) + ); + + // Global default with ${VAR} syntax + assert_eq!( + config.get_cluster_jump_host(Some("staging")), + Some("bastion.example.com".to_string()) + ); + + // Clean up + unsafe { + std::env::remove_var("TEST_BASTION_HOST"); + std::env::remove_var("TEST_BASTION_PORT"); + } +} From 68c2ed19d6c07752a2e02e22278d2d0600a0e4bb Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Fri, 19 Dec 2025 19:19:43 +0900 Subject: [PATCH 3/4] test: Add comprehensive tests for jump_host configuration feature Add missing test cases for issue #115: Unit tests in src/config/tests.rs: - test_jump_host_out_of_bounds_node_index: Edge case for invalid node index - test_jump_host_mixed_simple_detailed_nodes: Mixed node config inheritance - test_jump_host_with_port_format: Jump host with port specification - test_jump_host_multi_hop_format: Multi-hop jump host chain Integration tests in tests/jump_host_config_test.rs: - test_config_global_jump_host_applied_to_all_clusters - test_config_cluster_jump_host_overrides_global - test_config_node_jump_host_overrides_cluster - test_config_empty_string_disables_jump_host - test_config_jump_host_env_expansion - test_config_jump_host_full_format - test_config_jump_host_multi_hop - test_config_backward_compatibility - test_config_jump_host_resolution_priority - test_config_simple_nodes_inherit_cluster_jump_host Total new tests: 14 --- src/config/tests.rs | 150 ++++++++++++++ tests/jump_host_config_test.rs | 351 +++++++++++++++++++++++++++++++++ 2 files changed, 501 insertions(+) create mode 100644 tests/jump_host_config_test.rs diff --git a/src/config/tests.rs b/src/config/tests.rs index fbfab61c..6e3e7e29 100644 --- a/src/config/tests.rs +++ b/src/config/tests.rs @@ -511,3 +511,153 @@ clusters: std::env::remove_var("TEST_BASTION_PORT"); } } + +#[test] +fn test_jump_host_out_of_bounds_node_index() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - host: prod1.internal + jump_host: prod-bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Node at index 0 exists - should use cluster jump_host + assert_eq!( + config.get_jump_host("production", 0), + Some("prod-bastion.example.com".to_string()) + ); + + // Node at index 1 does not exist - should fall back to cluster level + assert_eq!( + config.get_jump_host("production", 1), + Some("prod-bastion.example.com".to_string()) + ); + + // Node at index 100 does not exist - should fall back to cluster level + assert_eq!( + config.get_jump_host("production", 100), + Some("prod-bastion.example.com".to_string()) + ); +} + +#[test] +fn test_jump_host_mixed_simple_detailed_nodes() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - simple-node1.internal + - host: detailed-node1.internal + jump_host: special-bastion.example.com + - simple-node2.internal + - host: detailed-node2.internal + jump_host: prod-bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Simple node at index 0 - inherits cluster jump_host + assert_eq!( + config.get_jump_host("production", 0), + Some("prod-bastion.example.com".to_string()) + ); + + // Detailed node at index 1 - uses node-level jump_host + assert_eq!( + config.get_jump_host("production", 1), + Some("special-bastion.example.com".to_string()) + ); + + // Simple node at index 2 - inherits cluster jump_host + assert_eq!( + config.get_jump_host("production", 2), + Some("prod-bastion.example.com".to_string()) + ); + + // Detailed node at index 3 without jump_host - inherits cluster jump_host + assert_eq!( + config.get_jump_host("production", 3), + Some("prod-bastion.example.com".to_string()) + ); +} + +#[test] +fn test_jump_host_with_port_format() { + let yaml = r#" +defaults: + jump_host: bastion.example.com:2222 + +clusters: + production: + nodes: + - host: prod1.internal + jump_host: prod-bastion:3333 + - host: prod2.internal + jump_host: cluster-bastion:4444 +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Node with port in jump_host + assert_eq!( + config.get_jump_host("production", 0), + Some("prod-bastion:3333".to_string()) + ); + + // Cluster with port in jump_host + assert_eq!( + config.get_jump_host("production", 1), + Some("cluster-bastion:4444".to_string()) + ); + + // Global default with port + assert_eq!( + config.get_cluster_jump_host(Some("staging")), + Some("bastion.example.com:2222".to_string()) + ); +} + +#[test] +fn test_jump_host_multi_hop_format() { + let yaml = r#" +defaults: + jump_host: hop1.example.com,hop2.example.com + +clusters: + production: + nodes: + - host: prod1.internal + jump_host: jumpA,jumpB,jumpC + - host: prod2.internal + jump_host: bastion1,bastion2 +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Node with multi-hop jump_host + assert_eq!( + config.get_jump_host("production", 0), + Some("jumpA,jumpB,jumpC".to_string()) + ); + + // Cluster with multi-hop jump_host + assert_eq!( + config.get_jump_host("production", 1), + Some("bastion1,bastion2".to_string()) + ); + + // Global default with multi-hop + assert_eq!( + config.get_cluster_jump_host(None), + Some("hop1.example.com,hop2.example.com".to_string()) + ); +} diff --git a/tests/jump_host_config_test.rs b/tests/jump_host_config_test.rs new file mode 100644 index 00000000..6056fddb --- /dev/null +++ b/tests/jump_host_config_test.rs @@ -0,0 +1,351 @@ +// Copyright 2025 Lablup Inc. and Jeongkyu Shin +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Integration tests for jump_host configuration feature (issue #115) +//! +//! These tests verify that jump_host can be configured in config.yaml +//! at global, cluster, and node levels, and that CLI -J option takes +//! precedence over configuration. + +use bssh::config::Config; + +/// Test that global default jump_host is applied to all clusters +#[test] +fn test_config_global_jump_host_applied_to_all_clusters() { + let yaml = r#" +defaults: + user: admin + jump_host: global-bastion.example.com + +clusters: + cluster_a: + nodes: + - host: a1.internal + - host: a2.internal + + cluster_b: + nodes: + - host: b1.internal +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // All clusters should inherit global jump_host + assert_eq!( + config.get_cluster_jump_host(Some("cluster_a")), + Some("global-bastion.example.com".to_string()) + ); + assert_eq!( + config.get_cluster_jump_host(Some("cluster_b")), + Some("global-bastion.example.com".to_string()) + ); +} + +/// Test that cluster-level jump_host overrides global default +#[test] +fn test_config_cluster_jump_host_overrides_global() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + with_override: + nodes: + - host: node1.internal + jump_host: cluster-bastion.example.com + + without_override: + nodes: + - host: node2.internal +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Cluster with override should use cluster jump_host + assert_eq!( + config.get_cluster_jump_host(Some("with_override")), + Some("cluster-bastion.example.com".to_string()) + ); + + // Cluster without override should use global jump_host + assert_eq!( + config.get_cluster_jump_host(Some("without_override")), + Some("global-bastion.example.com".to_string()) + ); +} + +/// Test that node-level jump_host overrides cluster default +#[test] +fn test_config_node_jump_host_overrides_cluster() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - host: node1.internal + jump_host: node1-bastion.example.com + - host: node2.internal + - host: node3.internal + jump_host: node3-bastion.example.com + jump_host: cluster-bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Node 0 has override + assert_eq!( + config.get_jump_host("production", 0), + Some("node1-bastion.example.com".to_string()) + ); + + // Node 1 uses cluster default + assert_eq!( + config.get_jump_host("production", 1), + Some("cluster-bastion.example.com".to_string()) + ); + + // Node 2 has override + assert_eq!( + config.get_jump_host("production", 2), + Some("node3-bastion.example.com".to_string()) + ); +} + +/// Test that empty string explicitly disables jump_host +#[test] +fn test_config_empty_string_disables_jump_host() { + let yaml = r#" +defaults: + jump_host: global-bastion.example.com + +clusters: + direct_access: + nodes: + - host: direct1.example.com + jump_host: "" + + mixed: + nodes: + - host: via_bastion.internal + - host: direct.internal + jump_host: "" + jump_host: cluster-bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Cluster with empty jump_host should have no jump_host + assert_eq!(config.get_cluster_jump_host(Some("direct_access")), None); + assert_eq!(config.get_jump_host("direct_access", 0), None); + + // Node with empty jump_host should have no jump_host + assert_eq!( + config.get_jump_host("mixed", 0), + Some("cluster-bastion.example.com".to_string()) + ); + assert_eq!(config.get_jump_host("mixed", 1), None); +} + +/// Test environment variable expansion in jump_host +#[test] +fn test_config_jump_host_env_expansion() { + // Set environment variables + unsafe { + std::env::set_var("BSSH_TEST_BASTION", "env-bastion.example.com"); + std::env::set_var("BSSH_TEST_PORT", "2222"); + } + + let yaml = r#" +defaults: + jump_host: ${BSSH_TEST_BASTION} + +clusters: + production: + nodes: + - host: node1.internal + jump_host: ${BSSH_TEST_BASTION}:${BSSH_TEST_PORT} + - host: node2.internal + jump_host: $BSSH_TEST_BASTION:$BSSH_TEST_PORT +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Global default with env var + assert_eq!( + config.get_cluster_jump_host(Some("nonexistent")), + Some("env-bastion.example.com".to_string()) + ); + + // Node-level with ${VAR} syntax + assert_eq!( + config.get_jump_host("production", 0), + Some("env-bastion.example.com:2222".to_string()) + ); + + // Cluster-level with $VAR syntax + assert_eq!( + config.get_jump_host("production", 1), + Some("env-bastion.example.com:2222".to_string()) + ); + + // Clean up + unsafe { + std::env::remove_var("BSSH_TEST_BASTION"); + std::env::remove_var("BSSH_TEST_PORT"); + } +} + +/// Test jump_host with user@host:port format +#[test] +fn test_config_jump_host_full_format() { + let yaml = r#" +defaults: + jump_host: jumpuser@bastion.example.com:2222 + +clusters: + production: + nodes: + - host: node1.internal +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Should preserve full user@host:port format + assert_eq!( + config.get_cluster_jump_host(Some("production")), + Some("jumpuser@bastion.example.com:2222".to_string()) + ); +} + +/// Test multi-hop jump_host chain +#[test] +fn test_config_jump_host_multi_hop() { + let yaml = r#" +defaults: + jump_host: hop1.example.com,hop2.example.com,hop3.example.com + +clusters: + production: + nodes: + - host: node1.internal +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Should preserve multi-hop chain + assert_eq!( + config.get_cluster_jump_host(Some("production")), + Some("hop1.example.com,hop2.example.com,hop3.example.com".to_string()) + ); +} + +/// Test backward compatibility - configs without jump_host should work +#[test] +fn test_config_backward_compatibility() { + let yaml = r#" +defaults: + user: admin + port: 22 + ssh_key: ~/.ssh/id_rsa + +clusters: + production: + nodes: + - web1.example.com + - web2.example.com + ssh_key: ~/.ssh/prod_key +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Should work without jump_host configured + assert_eq!(config.get_cluster_jump_host(Some("production")), None); + assert_eq!(config.get_jump_host("production", 0), None); + + // Other config should still work + assert_eq!(config.defaults.user, Some("admin".to_string())); + assert_eq!(config.defaults.port, Some(22)); + + let cluster = config.get_cluster("production").unwrap(); + assert_eq!(cluster.nodes.len(), 2); +} + +/// Test resolution priority: node > cluster > global +#[test] +fn test_config_jump_host_resolution_priority() { + let yaml = r#" +defaults: + jump_host: global.example.com + +clusters: + test: + nodes: + - host: node1.internal + jump_host: node-level.example.com + - host: node2.internal + - host: node3.internal + jump_host: "" + jump_host: cluster-level.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Node 0: node-level takes priority + assert_eq!( + config.get_jump_host("test", 0), + Some("node-level.example.com".to_string()) + ); + + // Node 1: no node-level, uses cluster-level + assert_eq!( + config.get_jump_host("test", 1), + Some("cluster-level.example.com".to_string()) + ); + + // Node 2: empty string at node-level disables (even though cluster has value) + assert_eq!(config.get_jump_host("test", 2), None); + + // get_cluster_jump_host should return cluster-level (not node-level) + assert_eq!( + config.get_cluster_jump_host(Some("test")), + Some("cluster-level.example.com".to_string()) + ); +} + +/// Test that simple node strings cannot have jump_host override +#[test] +fn test_config_simple_nodes_inherit_cluster_jump_host() { + let yaml = r#" +clusters: + production: + nodes: + - simple-node.internal + - user@another-simple.internal:2222 + jump_host: cluster-bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).unwrap(); + + // Both simple nodes should inherit cluster jump_host + assert_eq!( + config.get_jump_host("production", 0), + Some("cluster-bastion.example.com".to_string()) + ); + assert_eq!( + config.get_jump_host("production", 1), + Some("cluster-bastion.example.com".to_string()) + ); +} From 48d0f664286e8f51d6ae2070c8dee2de5d67bd3b Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Fri, 19 Dec 2025 20:15:02 +0900 Subject: [PATCH 4/4] docs: Add jump_host configuration documentation for issue #115 - Update manpage with jump_host configuration examples and section - Update configuration.md with jump_host data model and resolution - Update ssh-jump-hosts.md to mark YAML config as implemented - Add jump_host examples to example-config.yaml --- docs/architecture/configuration.md | 93 ++++++++++++++++++++++------- docs/architecture/ssh-jump-hosts.md | 85 +++++++++++++++++--------- docs/man/bssh.1 | 46 ++++++++++++++ example-config.yaml | 36 ++++++++++- 4 files changed, 208 insertions(+), 52 deletions(-) diff --git a/docs/architecture/configuration.md b/docs/architecture/configuration.md index e4316b5e..ebb5af21 100644 --- a/docs/architecture/configuration.md +++ b/docs/architecture/configuration.md @@ -74,39 +74,90 @@ The configuration system uses the following core data structures: ```rust pub struct Config { - pub clusters: HashMap, - pub default_cluster: Option, - pub ssh_config: SshConfig, + pub defaults: Defaults, + pub clusters: HashMap, + pub default_cluster: Option, + pub ssh_config: SshConfig, +} + +pub struct Defaults { + pub user: Option, + pub port: Option, + pub ssh_key: Option, + pub parallel: Option, + pub jump_host: Option, // Global default jump host } pub struct Cluster { - pub nodes: Vec, - pub ssh_key: Option, - pub user: Option, + pub nodes: Vec, + pub defaults: ClusterDefaults, + pub interactive: Option, +} + +pub struct ClusterDefaults { + pub user: Option, + pub port: Option, + pub ssh_key: Option, + pub jump_host: Option, // Cluster-level jump host +} + +// Node can be simple string or detailed config +pub enum NodeConfig { + Simple(String), // "host" or "user@host:port" + Detailed { + host: String, + port: Option, + user: Option, + jump_host: Option, // Node-level jump host + }, } ``` +### Jump Host Resolution + +Jump hosts are resolved with the following priority (highest to lowest): +1. **CLI `-J` option** - Always takes precedence +2. **SSH config `ProxyJump`** - From `~/.ssh/config` +3. **Node-level config** - Per-node `jump_host` field +4. **Cluster-level config** - Cluster `defaults.jump_host` +5. **Global defaults** - Top-level `defaults.jump_host` + +An empty string (`""`) explicitly disables jump host inheritance. + ### Configuration File Example ```yaml default_cluster: production +defaults: + user: admin + port: 22 + ssh_key: ~/.ssh/id_rsa + jump_host: global-bastion.example.com # Default for all clusters + clusters: - production: - nodes: - - host: node1.example.com - port: 22 - user: admin - - host: node2.example.com - port: 22 - user: admin - ssh_key: ~/.ssh/id_rsa - - staging: - nodes: - - host: staging1.example.com - - host: staging2.example.com - user: deploy + production: + nodes: + - host: node1.example.com + port: 22 + user: admin + - host: node2.example.com + port: 22 + user: admin + jump_host: node2-bastion.example.com # Node-specific override + ssh_key: ~/.ssh/id_rsa + jump_host: prod-bastion.example.com # Cluster-level jump host + + staging: + nodes: + - host: staging1.example.com + - host: staging2.example.com + user: deploy + + direct_access: + nodes: + - host: external.example.com + jump_host: "" # Explicitly disable jump host for this cluster ``` --- diff --git a/docs/architecture/ssh-jump-hosts.md b/docs/architecture/ssh-jump-hosts.md index 4474a32a..8d8b8f99 100644 --- a/docs/architecture/ssh-jump-hosts.md +++ b/docs/architecture/ssh-jump-hosts.md @@ -395,38 +395,67 @@ bssh -H db.internal "uptime" # Uses db-gateway.example.com - Each operation establishes fresh tunnel - **Rationale:** russh session limitations prevent connection reuse -**YAML Configuration File Support:** -- Jump hosts via YAML cluster config not yet implemented -- Only CLI `-J` and SSH config `ProxyJump` are supported -- **Future Enhancement:** Add `jump_hosts` field to cluster configuration +### YAML Configuration File Support (Issue #115 - Implemented) + +**Implementation:** `src/config/types.rs`, `src/config/resolver.rs` + +Jump hosts can now be configured in the YAML configuration file at three levels: + +**Configuration Levels (priority order):** +1. **Node-level** (highest) - Per-node `jump_host` field +2. **Cluster-level** - Cluster `defaults.jump_host` or inline `jump_host` +3. **Global defaults** - Top-level `defaults.jump_host` + +**Example Configuration:** +```yaml +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - host: web1.internal + jump_host: special-bastion.example.com # Node-level override + - host: web2.internal # Uses cluster jump_host + - host: direct.example.com + jump_host: "" # Disabled (direct connection) + jump_host: prod-bastion.example.com # Cluster-level + + direct_cluster: + nodes: + - external.example.com + jump_host: "" # Cluster disables inherited global jump_host +``` + +**Special Values:** +- Empty string (`""`) - Explicitly disables jump host inheritance +- Environment variables - Supports `${VAR}` and `$VAR` syntax + +**Resolution Methods:** +- `config.get_jump_host(cluster_name, node_index)` - Get effective jump host for a node +- `config.get_cluster_jump_host(Some(cluster_name))` - Get cluster-level jump host + +**Priority with CLI and SSH Config:** +1. CLI `-J` option (highest) +2. SSH config `ProxyJump` directive +3. YAML config (node → cluster → global) ### Future Enhancements -1. **YAML Configuration File Support:** - ```yaml - clusters: - production: - jump_hosts: "bastion1.example.com,bastion2.example.com" - nodes: - - internal-host1 - - internal-host2 - ``` - -2. **Jump Host Connection Pooling:** - - Reuse jump host connections across multiple target nodes - - Significant performance improvement for cluster operations - - Requires russh session lifecycle improvements - -3. **Smart Timeout Calculation:** - - Measure actual round-trip times per hop - - Adjust timeouts dynamically based on observed latency - - Provide faster failures for genuinely unreachable hosts - -4. **Parallel Jump Host Establishment:** - - When connecting to multiple targets through same jump hosts - - Establish jump chain once, multiplex to targets - - Reduces connection overhead for cluster operations +1. **Jump Host Connection Pooling:** + - Reuse jump host connections across multiple target nodes + - Significant performance improvement for cluster operations + - Requires russh session lifecycle improvements + +2. **Smart Timeout Calculation:** + - Measure actual round-trip times per hop + - Adjust timeouts dynamically based on observed latency + - Provide faster failures for genuinely unreachable hosts +3. **Parallel Jump Host Establishment:** + - When connecting to multiple targets through same jump hosts + - Establish jump chain once, multiplex to targets + - Reduces connection overhead for cluster operations --- diff --git a/docs/man/bssh.1 b/docs/man/bssh.1 index b34718d5..aade72c2 100644 --- a/docs/man/bssh.1 +++ b/docs/man/bssh.1 @@ -425,6 +425,7 @@ defaults: port: 22 ssh_key: ~/.ssh/id_rsa parallel: 10 + jump_host: bastion.example.com # Global default jump host # Global interactive mode settings (optional) interactive: @@ -443,6 +444,7 @@ clusters: - web2.example.com - user@web3.example.com:2222 ssh_key: ~/.ssh/prod_key + jump_host: prod-bastion.example.com # Cluster-level jump host # Cluster-specific interactive settings (overrides global) interactive: default_mode: single_node @@ -454,8 +456,52 @@ clusters: - host: staging1.example.com port: 2200 user: deploy + jump_host: staging-bastion.example.com # Node-level jump host - staging2.example.com user: staging_user + jump_host: "" # Empty string disables jump host for this cluster +.fi + +.SS Jump Host Configuration in YAML +Jump hosts can be configured at three levels with the following priority: + +.IP 1. 4 +\fBNode-level\fR (highest priority) - applies to specific node only +.IP 2. 4 +\fBCluster-level\fR - applies to all nodes in the cluster +.IP 3. 4 +\fBGlobal defaults\fR - applies to all clusters without explicit jump_host + +.PP +The CLI \fB-J\fR option always takes precedence over configuration file settings. + +.PP +Special values: +.IP \[bu] 2 +Empty string (\fB""\fR) - explicitly disables jump host inheritance +.IP \[bu] 2 +Environment variables supported: \fB${VAR}\fR or \fB$VAR\fR syntax + +.PP +Example with all levels: +.nf +defaults: + jump_host: global-bastion.example.com + +clusters: + production: + nodes: + - host: web1.internal + jump_host: special-bastion.example.com # Uses special-bastion + - host: web2.internal # Uses prod-bastion + - host: direct.example.com + jump_host: "" # Direct connection (no jump) + jump_host: prod-bastion.example.com + + direct_cluster: + nodes: + - host: external.example.com + jump_host: "" # Cluster disables inherited global jump_host .fi .SH SSH CONFIGURATION OPTIONS diff --git a/example-config.yaml b/example-config.yaml index 4434c3fd..df41eca1 100644 --- a/example-config.yaml +++ b/example-config.yaml @@ -4,13 +4,15 @@ defaults: port: 22 ssh_key: ~/.ssh/id_rsa parallel: 10 + # Global default jump host (optional) - used for all clusters unless overridden + # jump_host: bastion.example.com clusters: local: nodes: - localhost user: ${USER} - + dev: nodes: - dev1.example.com @@ -18,7 +20,7 @@ clusters: - dev3.example.com user: developer ssh_key: ~/.ssh/dev_key - + production: nodes: - host: prod1.example.com @@ -27,6 +29,34 @@ clusters: - host: prod2.example.com port: 2222 user: admin + # Node-level jump host override (optional) + # jump_host: prod2-bastion.example.com - host: prod3.example.com - host: prod4.example.com - ssh_key: ~/.ssh/prod_key \ No newline at end of file + ssh_key: ~/.ssh/prod_key + # Cluster-level jump host (optional) - applies to all nodes in this cluster + # jump_host: prod-bastion.example.com + + # Example: Cluster behind a jump host/bastion + internal: + nodes: + - host: internal1.private + - host: internal2.private + - host: internal3.private + user: admin + jump_host: bastion.example.com # All nodes accessible via bastion + + # Example: Mixed direct and jump host access + hybrid: + nodes: + - host: behind-firewall.internal + jump_host: gateway.example.com # Needs jump host + - host: direct-access.example.com + jump_host: "" # Empty string disables jump host (direct connection) + jump_host: default-bastion.example.com # Default for cluster + + # Example: Multi-hop jump chain with environment variables + secure: + nodes: + - host: target.secure.internal + jump_host: ${FIRST_HOP},${SECOND_HOP} # Comma-separated for multi-hop \ No newline at end of file