From 6e39056a447580e109e5deddde15780e4d2c75fe Mon Sep 17 00:00:00 2001 From: Jeongkyu Shin Date: Wed, 28 Jan 2026 14:09:19 +0900 Subject: [PATCH] feat: Support per-jump-host SSH private key configuration Add support for configuring separate SSH keys for jump hosts independent of destination node keys. Jump hosts can now use structured YAML format with optional ssh_key field while maintaining backward compatibility with string format. Key changes: - Add ssh_key field to JumpHost struct - Create JumpHostConfig enum supporting Simple(String) and Detailed formats - Update config resolver with get_jump_host_with_key methods - Implement SSH key priority: jump host key > cluster key > agent > defaults - Add comprehensive tests for config parsing and auth priority - Update documentation and example config --- docs/architecture/ssh-jump-hosts.md | 93 +++++++++ example-config.yaml | 21 +- src/config/mod.rs | 4 +- src/config/resolver.rs | 92 +++++++-- src/config/types.rs | 66 ++++++- src/jump/chain/auth.rs | 140 ++++++++++++- src/jump/parser/host.rs | 24 ++- src/jump/parser/mod.rs | 20 ++ tests/jump_host_config_test.rs | 293 +++++++++++++++++++++++++++- 9 files changed, 719 insertions(+), 34 deletions(-) diff --git a/docs/architecture/ssh-jump-hosts.md b/docs/architecture/ssh-jump-hosts.md index 8d8b8f99..1ccaedb8 100644 --- a/docs/architecture/ssh-jump-hosts.md +++ b/docs/architecture/ssh-jump-hosts.md @@ -440,6 +440,99 @@ clusters: 2. SSH config `ProxyJump` directive 3. YAML config (node → cluster → global) +### Per-Jump-Host SSH Key Configuration (Issue #167 - Implemented) + +**Implementation:** `src/config/types.rs`, `src/jump/chain/auth.rs`, `src/jump/parser/host.rs` + +Jump hosts can now specify their own SSH private keys, separate from the destination node keys. + +**Configuration Format:** + +Supports both legacy string format and new structured format: + +```yaml +clusters: + internal: + nodes: + - host: internal1.private + - host: internal2.private + user: admin + ssh_key: ~/.ssh/destination_key # For destination nodes + + # Legacy string format (uses cluster ssh_key for jump host) + jump_host: jumpuser@bastion.example.com + + # OR new structured format with dedicated jump host key: + jump_host: + host: bastion.example.com + user: jumpuser + port: 22 # optional + ssh_key: ~/.ssh/jump_host_key # Jump host's own key +``` + +**Per-Node Jump Host Override:** + +```yaml +clusters: + hybrid: + nodes: + - host: behind-firewall.internal + jump_host: + host: gateway.example.com + user: gw_user + ssh_key: ~/.ssh/gateway_key # Specific key for this gateway + - host: direct-access.example.com + jump_host: "" # Direct connection + jump_host: default-bastion.example.com +``` + +**SSH Key Priority Order:** + +When authenticating to jump hosts, the following priority is used: + +1. **Jump host's own `ssh_key`** (from structured config) +2. **Cluster/defaults `ssh_key`** (fallback) +3. **SSH agent** (if use_agent=true and agent has keys) +4. **Default key files** (~/.ssh/id_*) + +**Implementation Details:** + +- `JumpHost` struct now has `ssh_key: Option` field +- `JumpHostConfig` enum supports both `Simple(String)` and `Detailed { host, user, port, ssh_key }` +- `#[serde(untagged)]` enables seamless deserialization of both formats +- Environment variable expansion works in `ssh_key` paths (e.g., `$HOME/.ssh/key`) +- Path expansion supports `~` tilde notation + +**Example Use Case:** + +```yaml +clusters: + secure: + nodes: + - host: db.internal + user: dbadmin + ssh_key: ~/.ssh/db_admin_key # For database access + jump_host: + host: bastion.example.com + user: bastion_user + ssh_key: ~/.ssh/bastion_key # Separate key for bastion +``` + +**Backward Compatibility:** + +- All existing configurations continue to work without changes +- String format `jump_host: "user@host:port"` still supported +- When no `ssh_key` is specified in jump_host config, falls back to cluster `ssh_key` +- Multi-hop chains work with mixed formats + +**Tests:** + +- Unit tests in `tests/jump_host_config_test.rs` +- Auth priority tests in `src/jump/chain/auth.rs::tests` +- Validates both simple and structured format deserialization +- Verifies environment variable expansion +- Confirms backward compatibility + ### Future Enhancements 1. **Jump Host Connection Pooling:** diff --git a/example-config.yaml b/example-config.yaml index 8fb08a0f..5bf0a759 100644 --- a/example-config.yaml +++ b/example-config.yaml @@ -57,17 +57,28 @@ clusters: - host: internal2.private - host: internal3.private user: admin # User for internal*.private (destination nodes) - jump_host: jumpuser@bastion.example.com # User 'jumpuser' for bastion (jump host) - # Alternative: jump_host: bastion.example.com # Uses your local username for bastion + ssh_key: ~/.ssh/destination_key # Key for destination nodes + # Legacy string format (uses cluster ssh_key for both jump host and destinations) + jump_host: jumpuser@bastion.example.com + # Alternative structured format with dedicated jump host key: + # jump_host: + # host: bastion.example.com + # user: jumpuser + # port: 22 # optional + # ssh_key: ~/.ssh/jump_host_key # Uses this key for bastion only - # Example: Mixed direct and jump host access + # Example: Mixed direct and jump host access with per-node jump host override hybrid: nodes: - host: behind-firewall.internal - jump_host: gateway.example.com # Needs jump host + # Per-node jump host with dedicated key + jump_host: + host: gateway.example.com + user: gw_user + ssh_key: ~/.ssh/gateway_key - host: direct-access.example.com jump_host: "" # Empty string disables jump host (direct connection) - jump_host: default-bastion.example.com # Default for cluster + jump_host: default-bastion.example.com # Default for cluster (string format) # Example: Multi-hop jump chain with environment variables secure: diff --git a/src/config/mod.rs b/src/config/mod.rs index 80c03497..c25b5c0e 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -25,6 +25,6 @@ mod utils; // Re-export public types pub use types::{ Cluster, ClusterDefaults, Config, Defaults, InteractiveConfig, InteractiveConfigUpdate, - InteractiveMode, KeyBindings, NodeConfig, + InteractiveMode, JumpHostConfig, KeyBindings, NodeConfig, }; -pub use utils::expand_tilde; +pub use utils::{expand_env_vars, expand_tilde}; diff --git a/src/config/resolver.rs b/src/config/resolver.rs index 5c908674..01723529 100644 --- a/src/config/resolver.rs +++ b/src/config/resolver.rs @@ -133,6 +133,24 @@ impl Config { /// /// Empty string (`""`) explicitly disables jump host inheritance. pub fn get_jump_host(&self, cluster_name: &str, node_index: usize) -> Option { + self.get_jump_host_with_key(cluster_name, node_index) + .map(|(conn_str, _)| conn_str) + } + + /// Get jump host with SSH key 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. + /// Returns tuple of (connection_string, optional_ssh_key_path) + pub fn get_jump_host_with_key( + &self, + cluster_name: &str, + node_index: usize, + ) -> Option<(String, Option)> { if let Some(cluster) = self.get_cluster(cluster_name) { // Check node-level first if let Some(NodeConfig::Detailed { @@ -140,25 +158,55 @@ impl Config { .. }) = cluster.nodes.get(node_index) { - if jh.is_empty() { - return None; // Explicitly disabled - } - return Some(expand_env_vars(jh)); + return self.process_jump_host_config(jh); } // Check cluster-level if let Some(jh) = &cluster.defaults.jump_host { - if jh.is_empty() { - return None; // Explicitly disabled - } - return Some(expand_env_vars(jh)); + return self.process_jump_host_config(jh); } } // Fall back to global default self.defaults .jump_host .as_ref() - .filter(|s| !s.is_empty()) - .map(|s| expand_env_vars(s)) + .and_then(|jh| self.process_jump_host_config(jh)) + } + + /// Process a JumpHostConfig and return (connection_string, optional_ssh_key_path) + fn process_jump_host_config( + &self, + config: &super::types::JumpHostConfig, + ) -> Option<(String, Option)> { + use super::types::JumpHostConfig; + + match config { + JumpHostConfig::Simple(s) => { + if s.is_empty() { + None // Explicitly disabled + } else { + Some((expand_env_vars(s), None)) + } + } + JumpHostConfig::Detailed { + host, + user, + port, + ssh_key, + } => { + let mut conn_str = String::new(); + if let Some(u) = user { + conn_str.push_str(&expand_env_vars(u)); + conn_str.push('@'); + } + conn_str.push_str(&expand_env_vars(host)); + if let Some(p) = port { + conn_str.push(':'); + conn_str.push_str(&p.to_string()); + } + let key = ssh_key.as_ref().map(|k| expand_env_vars(k)); + Some((conn_str, key)) + } + } } /// Get jump host for a cluster (cluster-level default). @@ -169,13 +217,26 @@ impl Config { /// /// Empty string (`""`) explicitly disables jump host inheritance. pub fn get_cluster_jump_host(&self, cluster_name: Option<&str>) -> Option { + self.get_cluster_jump_host_with_key(cluster_name) + .map(|(conn_str, _)| conn_str) + } + + /// Get jump host with SSH key 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. + /// Returns tuple of (connection_string, optional_ssh_key_path) + pub fn get_cluster_jump_host_with_key( + &self, + cluster_name: Option<&str>, + ) -> Option<(String, 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(expand_env_vars(jh)); + return self.process_jump_host_config(jh); } } } @@ -183,8 +244,7 @@ impl Config { self.defaults .jump_host .as_ref() - .filter(|s| !s.is_empty()) - .map(|s| expand_env_vars(s)) + .and_then(|jh| self.process_jump_host_config(jh)) } /// Get SSH keepalive interval for a cluster. diff --git a/src/config/types.rs b/src/config/types.rs index 9e9919b3..752293ba 100644 --- a/src/config/types.rs +++ b/src/config/types.rs @@ -30,6 +30,28 @@ pub struct Config { pub interactive: InteractiveConfig, } +/// Jump host configuration format. +/// +/// Supports both legacy string format and structured format with optional SSH key. +/// Uses `#[serde(untagged)]` to allow seamless deserialization of both formats. +#[derive(Debug, Serialize, Deserialize, Clone)] +#[serde(untagged)] +pub enum JumpHostConfig { + /// Structured format with optional ssh_key field + /// Must be listed first for serde to try matching object format before string + Detailed { + host: String, + #[serde(default)] + user: Option, + #[serde(default)] + port: Option, + #[serde(default)] + ssh_key: Option, + }, + /// Legacy string format: "[user@]hostname[:port]" + Simple(String), +} + /// Global default settings. #[derive(Debug, Serialize, Deserialize, Default, Clone)] pub struct Defaults { @@ -39,8 +61,9 @@ pub struct Defaults { pub parallel: Option, pub timeout: Option, /// Jump host specification for all connections. + /// Supports both string format and structured format with optional ssh_key. /// Empty string explicitly disables jump host inheritance. - pub jump_host: Option, + pub jump_host: Option, /// SSH keepalive interval in seconds. /// Sends keepalive packets to prevent idle connection timeouts. /// Default: 60 seconds. Set to 0 to disable. @@ -128,8 +151,9 @@ pub struct ClusterDefaults { pub parallel: Option, pub timeout: Option, /// Jump host specification for this cluster. + /// Supports both string format and structured format with optional ssh_key. /// Empty string explicitly disables jump host inheritance. - pub jump_host: Option, + pub jump_host: Option, /// SSH keepalive interval in seconds. /// Sends keepalive packets to prevent idle connection timeouts. /// Default: 60 seconds. Set to 0 to disable. @@ -151,9 +175,10 @@ pub enum NodeConfig { #[serde(default)] user: Option, /// Jump host specification for this node. + /// Supports both string format and structured format with optional ssh_key. /// Empty string explicitly disables jump host inheritance. #[serde(default)] - jump_host: Option, + jump_host: Option, }, } @@ -188,3 +213,38 @@ pub(super) fn default_broadcast_toggle() -> String { pub(super) fn default_quit() -> String { "Ctrl+Q".to_string() } + +impl JumpHostConfig { + /// Convert to a connection string for resolution + pub fn to_connection_string(&self) -> String { + match self { + JumpHostConfig::Simple(s) => s.clone(), + JumpHostConfig::Detailed { + host, + user, + port, + ssh_key: _, + } => { + let mut result = String::new(); + if let Some(u) = user { + result.push_str(u); + result.push('@'); + } + result.push_str(host); + if let Some(p) = port { + result.push(':'); + result.push_str(&p.to_string()); + } + result + } + } + } + + /// Get the SSH key path if specified + pub fn ssh_key(&self) -> Option<&str> { + match self { + JumpHostConfig::Simple(_) => None, + JumpHostConfig::Detailed { ssh_key, .. } => ssh_key.as_deref(), + } + } +} diff --git a/src/jump/chain/auth.rs b/src/jump/chain/auth.rs index 8fe6a01c..a12a21e6 100644 --- a/src/jump/chain/auth.rs +++ b/src/jump/chain/auth.rs @@ -67,8 +67,11 @@ async fn agent_has_identities() -> bool { /// Determine authentication method for a jump host /// -/// For now, uses the same authentication method as the destination. -/// In the future, this could be enhanced to support per-host authentication. +/// Priority order for SSH key selection: +/// 1. Jump host's own `ssh_key` field (from structured config) +/// 2. Cluster/defaults `key_path` (fallback, passed as parameter) +/// 3. SSH agent (if use_agent=true and agent has keys) +/// 4. Default key files (~/.ssh/id_*) pub(super) async fn determine_auth_method( jump_host: &JumpHost, key_path: Option<&Path>, @@ -76,8 +79,21 @@ pub(super) async fn determine_auth_method( use_password: bool, auth_mutex: &Mutex<()>, ) -> Result { - // For now, use the same auth method determination logic as the main SSH client - // This could be enhanced to support per-jump-host authentication in the future + // Priority 1: Use jump host's own ssh_key if provided + let effective_key_path = if let Some(ref jump_key) = jump_host.ssh_key { + use crate::config::{expand_env_vars, expand_tilde}; + let expanded_path = expand_env_vars(jump_key); + let path = Path::new(&expanded_path); + let expanded_tilde = if expanded_path.starts_with('~') { + expand_tilde(path) + } else { + path.to_path_buf() + }; + Some(expanded_tilde) + } else { + // Priority 2: Fall back to cluster/defaults key_path + key_path.map(|p| p.to_path_buf()) + }; // Cache agent availability check to avoid querying the agent multiple times // (each query involves socket connection and protocol handshake) @@ -131,7 +147,7 @@ pub(super) async fn determine_auth_method( // If agent is running but has no identities, fall through to try key files } - if let Some(key_path) = key_path { + if let Some(key_path) = effective_key_path.as_deref() { // SECURITY: Use Zeroizing to ensure key contents are cleared from memory let key_contents = Zeroizing::new( std::fs::read_to_string(key_path) @@ -795,4 +811,118 @@ dGVzdHMgLSBub3QgcmVhbAECAwQ= "Timeout should be 5 seconds" ); } + + /// Test: Jump host's own ssh_key takes priority over cluster key_path + #[tokio::test] + async fn test_jump_host_ssh_key_priority() { + // Save and clear SSH_AUTH_SOCK + let original_sock = env::var("SSH_AUTH_SOCK").ok(); + env::remove_var("SSH_AUTH_SOCK"); + + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + + // Create jump host's own key + let jump_key_path = create_test_ssh_key(&temp_dir, "jump_host_key"); + let jump_key_str = jump_key_path.to_string_lossy().to_string(); + + // Create cluster's key + let cluster_key_path = create_test_ssh_key(&temp_dir, "cluster_key"); + + // Create jump host with its own ssh_key + let jump_host = JumpHost::with_ssh_key( + "test.example.com".to_string(), + Some("testuser".to_string()), + Some(22), + Some(jump_key_str.clone()), + ); + + let auth_mutex = Mutex::new(()); + + // Call determine_auth_method with both jump host key and cluster key + let result = determine_auth_method( + &jump_host, + Some(cluster_key_path.as_path()), // Cluster key + false, // use_agent + false, // use_password + &auth_mutex, + ) + .await; + + assert!(result.is_ok(), "Should succeed with jump host's key"); + let auth_method = result.unwrap(); + + // Verify it used the jump host's key, not the cluster key + match auth_method { + AuthMethod::PrivateKeyFile { key_file_path, .. } => { + let path_str = key_file_path.to_string_lossy(); + assert!( + path_str.contains("jump_host_key"), + "Should use jump host's key (jump_host_key), got: {path_str}" + ); + assert!( + !path_str.contains("cluster_key"), + "Should NOT use cluster key, got: {path_str}" + ); + } + other => { + panic!("Expected PrivateKeyFile, got {:?}", other); + } + } + + // Restore SSH_AUTH_SOCK + if let Some(val) = original_sock { + env::set_var("SSH_AUTH_SOCK", val); + } + } + + /// Test: Falls back to cluster key when jump host has no ssh_key + #[tokio::test] + async fn test_fallback_to_cluster_key() { + // Save and clear SSH_AUTH_SOCK + let original_sock = env::var("SSH_AUTH_SOCK").ok(); + env::remove_var("SSH_AUTH_SOCK"); + + let temp_dir = TempDir::new().expect("Failed to create temp dir"); + let cluster_key_path = create_test_ssh_key(&temp_dir, "cluster_key"); + + // Create jump host WITHOUT its own ssh_key + let jump_host = JumpHost::new( + "test.example.com".to_string(), + Some("testuser".to_string()), + Some(22), + ); + + let auth_mutex = Mutex::new(()); + + let result = determine_auth_method( + &jump_host, + Some(cluster_key_path.as_path()), + false, + false, + &auth_mutex, + ) + .await; + + assert!(result.is_ok(), "Should succeed with cluster key"); + let auth_method = result.unwrap(); + + // Verify it used the cluster key + match auth_method { + AuthMethod::PrivateKeyFile { key_file_path, .. } => { + let path_str = key_file_path.to_string_lossy(); + assert!( + path_str.contains("cluster_key"), + "Should use cluster key, got: {path_str}" + ); + } + other => { + panic!("Expected PrivateKeyFile, got {:?}", other); + } + } + + // Restore SSH_AUTH_SOCK + if let Some(val) = original_sock { + env::set_var("SSH_AUTH_SOCK", val); + } + } } diff --git a/src/jump/parser/host.rs b/src/jump/parser/host.rs index 6842055d..0b635b75 100644 --- a/src/jump/parser/host.rs +++ b/src/jump/parser/host.rs @@ -28,12 +28,34 @@ pub struct JumpHost { pub host: String, /// SSH port (None means use default port 22 or config default) pub port: Option, + /// SSH private key path for this jump host (None means use cluster/default key) + pub ssh_key: Option, } impl JumpHost { /// Create a new jump host specification pub fn new(host: String, user: Option, port: Option) -> Self { - Self { user, host, port } + Self { + user, + host, + port, + ssh_key: None, + } + } + + /// Create a new jump host specification with optional SSH key + pub fn with_ssh_key( + host: String, + user: Option, + port: Option, + ssh_key: Option, + ) -> Self { + Self { + user, + host, + port, + ssh_key, + } } /// Get the effective username (provided or current user) diff --git a/src/jump/parser/mod.rs b/src/jump/parser/mod.rs index 39c2f83d..725e3351 100644 --- a/src/jump/parser/mod.rs +++ b/src/jump/parser/mod.rs @@ -23,6 +23,26 @@ pub use config::{get_max_jump_hosts, ABSOLUTE_MAX_JUMP_HOSTS, DEFAULT_MAX_JUMP_H pub use host::JumpHost; pub use main_parser::parse_jump_hosts; +/// Parse jump hosts and set SSH key for all hosts in the chain +/// +/// This is a convenience function for when you have a jump host specification +/// from config with an associated SSH key path. +pub fn parse_jump_hosts_with_key( + jump_spec: &str, + ssh_key: Option, +) -> anyhow::Result> { + let mut jump_hosts = parse_jump_hosts(jump_spec)?; + + // Set the SSH key for all jump hosts in the chain + if let Some(key) = ssh_key { + for jump_host in &mut jump_hosts { + jump_host.ssh_key = Some(key.clone()); + } + } + + Ok(jump_hosts) +} + // Internal use #[cfg(test)] diff --git a/tests/jump_host_config_test.rs b/tests/jump_host_config_test.rs index 6056fddb..a67aa959 100644 --- a/tests/jump_host_config_test.rs +++ b/tests/jump_host_config_test.rs @@ -12,13 +12,15 @@ // See the License for the specific language governing permissions and // limitations under the License. -//! Integration tests for jump_host configuration feature (issue #115) +//! Integration tests for jump_host configuration feature (issue #115 and #167) //! //! 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. +//! +//! Tests for issue #167 verify per-jump-host SSH key configuration. -use bssh::config::Config; +use bssh::config::{Config, JumpHostConfig}; /// Test that global default jump_host is applied to all clusters #[test] @@ -349,3 +351,290 @@ clusters: Some("cluster-bastion.example.com".to_string()) ); } + +// ===== Tests for issue #167: Per-jump-host SSH key configuration ===== + +/// Test JumpHostConfig simple format deserialization +#[test] +fn test_jump_host_config_simple_format_parsing() { + let yaml = r#" +clusters: + test: + nodes: + - host: node1 + jump_host: bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).expect("Failed to parse config"); + let cluster = config.get_cluster("test").expect("Cluster not found"); + + match &cluster.defaults.jump_host { + Some(JumpHostConfig::Simple(s)) => { + assert_eq!(s, "bastion.example.com"); + } + other => panic!("Expected Simple variant, got {:?}", other), + } +} + +/// Test JumpHostConfig detailed format with all fields +#[test] +fn test_jump_host_config_detailed_format_full() { + let yaml = r#" +clusters: + test: + nodes: + - host: node1 + jump_host: + host: bastion.example.com + user: jumpuser + port: 2222 + ssh_key: ~/.ssh/bastion_key +"#; + + let config: Config = serde_yaml::from_str(yaml).expect("Failed to parse config"); + let cluster = config.get_cluster("test").expect("Cluster not found"); + + match &cluster.defaults.jump_host { + Some(JumpHostConfig::Detailed { + host, + user, + port, + ssh_key, + }) => { + assert_eq!(host, "bastion.example.com"); + assert_eq!(user.as_deref(), Some("jumpuser")); + assert_eq!(*port, Some(2222)); + assert_eq!(ssh_key.as_deref(), Some("~/.ssh/bastion_key")); + } + other => panic!("Expected Detailed variant, got {:?}", other), + } +} + +/// Test JumpHostConfig detailed format with minimal fields +#[test] +fn test_jump_host_config_detailed_minimal_host_only() { + let yaml = r#" +clusters: + test: + nodes: + - host: node1 + jump_host: + host: bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).expect("Failed to parse config"); + let cluster = config.get_cluster("test").expect("Cluster not found"); + + match &cluster.defaults.jump_host { + Some(JumpHostConfig::Detailed { + host, + user, + port, + ssh_key, + }) => { + assert_eq!(host, "bastion.example.com"); + assert!(user.is_none()); + assert!(port.is_none()); + assert!(ssh_key.is_none()); + } + other => panic!("Expected Detailed variant, got {:?}", other), + } +} + +/// Test get_jump_host_with_key resolution with structured format +#[test] +fn test_get_cluster_jump_host_with_key_structured() { + let yaml = r#" +clusters: + test: + nodes: + - host: node1 + jump_host: + host: bastion.example.com + user: jumpuser + ssh_key: ~/.ssh/jump_key +"#; + + let config: Config = serde_yaml::from_str(yaml).expect("Failed to parse config"); + let (conn_str, ssh_key) = config + .get_cluster_jump_host_with_key(Some("test")) + .expect("Jump host not found"); + + assert_eq!(conn_str, "jumpuser@bastion.example.com"); + assert!(ssh_key.is_some()); + assert!(ssh_key.as_ref().unwrap().contains("jump_key")); +} + +/// Test get_jump_host_with_key returns None for simple format ssh_key +#[test] +fn test_get_cluster_jump_host_with_key_simple_no_key() { + let yaml = r#" +clusters: + test: + nodes: + - host: node1 + jump_host: bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).expect("Failed to parse config"); + let (conn_str, ssh_key) = config + .get_cluster_jump_host_with_key(Some("test")) + .expect("Jump host not found"); + + assert_eq!(conn_str, "bastion.example.com"); + assert!(ssh_key.is_none()); +} + +/// Test node-level jump host override with per-jump-host key +#[test] +fn test_node_override_jump_host_with_key() { + let yaml = r#" +clusters: + test: + nodes: + - host: node1 + jump_host: + host: special-bastion.example.com + ssh_key: ~/.ssh/special_key + jump_host: default-bastion.example.com +"#; + + let config: Config = serde_yaml::from_str(yaml).expect("Failed to parse config"); + let (conn_str, ssh_key) = config + .get_jump_host_with_key("test", 0) + .expect("Jump host not found"); + + assert_eq!(conn_str, "special-bastion.example.com"); + assert!(ssh_key.is_some()); + assert!(ssh_key.as_ref().unwrap().contains("special_key")); +} + +/// Test environment variable expansion in jump_host ssh_key field +#[test] +fn test_jump_host_ssh_key_env_expansion() { + std::env::set_var("TEST_JUMP_HOST", "env-bastion.example.com"); + std::env::set_var("TEST_JUMP_KEY", "/keys/env_key"); + + let yaml = r#" +clusters: + test: + nodes: + - host: node1 + jump_host: + host: ${TEST_JUMP_HOST} + ssh_key: ${TEST_JUMP_KEY} +"#; + + let config: Config = serde_yaml::from_str(yaml).expect("Failed to parse config"); + let (conn_str, ssh_key) = config + .get_cluster_jump_host_with_key(Some("test")) + .expect("Jump host not found"); + + assert_eq!(conn_str, "env-bastion.example.com"); + assert_eq!(ssh_key.as_deref(), Some("/keys/env_key")); + + std::env::remove_var("TEST_JUMP_HOST"); + std::env::remove_var("TEST_JUMP_KEY"); +} + +/// Test backward compatibility: both simple and structured formats work +#[test] +fn test_backward_compatibility_mixed_formats() { + let yaml = r#" +clusters: + legacy: + nodes: + - host: node1 + jump_host: user@bastion1.example.com:2222 + + modern: + nodes: + - host: node2 + jump_host: + host: bastion2.example.com + user: admin + port: 22 + ssh_key: ~/.ssh/bastion2_key +"#; + + let config: Config = serde_yaml::from_str(yaml).expect("Failed to parse config"); + + // Test legacy format + let (conn_str1, ssh_key1) = config + .get_cluster_jump_host_with_key(Some("legacy")) + .expect("Jump host not found"); + assert_eq!(conn_str1, "user@bastion1.example.com:2222"); + assert!(ssh_key1.is_none()); + + // Test modern format + let (conn_str2, ssh_key2) = config + .get_cluster_jump_host_with_key(Some("modern")) + .expect("Jump host not found"); + assert_eq!(conn_str2, "admin@bastion2.example.com:22"); + assert!(ssh_key2.is_some()); + assert!(ssh_key2.as_ref().unwrap().contains("bastion2_key")); +} + +/// Test JumpHostConfig to_connection_string method +#[test] +fn test_jump_host_config_to_connection_string() { + // Simple format + let config1 = JumpHostConfig::Simple("bastion.example.com".to_string()); + assert_eq!(config1.to_connection_string(), "bastion.example.com"); + + // Detailed format with all fields + let config2 = JumpHostConfig::Detailed { + host: "bastion.example.com".to_string(), + user: Some("admin".to_string()), + port: Some(2222), + ssh_key: Some("~/.ssh/key".to_string()), + }; + assert_eq!( + config2.to_connection_string(), + "admin@bastion.example.com:2222" + ); + + // Detailed format with only host + let config3 = JumpHostConfig::Detailed { + host: "bastion.example.com".to_string(), + user: None, + port: None, + ssh_key: Some("~/.ssh/key".to_string()), + }; + assert_eq!(config3.to_connection_string(), "bastion.example.com"); + + // Detailed format with user but no port + let config4 = JumpHostConfig::Detailed { + host: "bastion.example.com".to_string(), + user: Some("admin".to_string()), + port: None, + ssh_key: None, + }; + assert_eq!(config4.to_connection_string(), "admin@bastion.example.com"); +} + +/// Test JumpHostConfig ssh_key accessor method +#[test] +fn test_jump_host_config_ssh_key_accessor() { + // Simple format has no ssh_key + let config1 = JumpHostConfig::Simple("bastion.example.com".to_string()); + assert!(config1.ssh_key().is_none()); + + // Detailed format without ssh_key + let config2 = JumpHostConfig::Detailed { + host: "bastion.example.com".to_string(), + user: None, + port: None, + ssh_key: None, + }; + assert!(config2.ssh_key().is_none()); + + // Detailed format with ssh_key + let config3 = JumpHostConfig::Detailed { + host: "bastion.example.com".to_string(), + user: None, + port: None, + ssh_key: Some("~/.ssh/key".to_string()), + }; + assert_eq!(config3.ssh_key(), Some("~/.ssh/key")); +}