Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 62 additions & 4 deletions docs/architecture/ssh-jump-hosts.md
Original file line number Diff line number Diff line change
Expand Up @@ -330,21 +330,79 @@ pub fn get_max_jump_hosts -> usize {
- No compilation warnings (after clippy allows)
- Successfully handles multi-hop scenarios

### SSH Config ProxyJump Integration (Issue #117 - Implemented)

**Implementation:** `src/executor/connection_manager.rs`, `src/app/initialization.rs`

The jump host resolution now integrates with SSH configuration files, automatically using `ProxyJump` directives when no CLI `-J` option is specified:

**Priority Order:**
1. **CLI `-J` option** (highest priority) - Explicitly specified jump hosts
2. **SSH config `ProxyJump` directive** - Per-host configuration from `~/.ssh/config`
3. **None** - Direct connection (no jump host)

**Implementation Details:**
```rust
// In connection_manager.rs execute_on_node_with_jump_hosts()
let ssh_config_jump_hosts = config
.ssh_config
.and_then(|ssh_config| ssh_config.get_proxy_jump(&node.host));

let effective_jump_hosts = if config.jump_hosts.is_some() {
config.jump_hosts // CLI takes precedence
} else {
ssh_config_jump_hosts.as_deref() // Fall back to SSH config
};
```

**Example SSH Config:**
```
Host *.internal
ProxyJump bastion.example.com

Host db.internal
ProxyJump db-gateway.example.com
```

**Usage:**
```bash
# Automatically uses bastion.example.com from SSH config
bssh -H web.internal "uptime"

# CLI option overrides SSH config
bssh -J custom-jump.example.com -H web.internal "uptime"

# Most specific SSH config pattern wins
bssh -H db.internal "uptime" # Uses db-gateway.example.com
```

**Benefits:**
- Seamless integration with existing SSH workflows
- Centralized jump host configuration
- Per-host or wildcard pattern support
- No need to specify `-J` for frequently accessed internal hosts

**Tests:**
- Added unit tests in `src/app/initialization.rs::tests`
- Tests verify CLI precedence over SSH config
- Tests verify wildcard pattern matching
- Tests verify fallback behavior

### Known Limitations

**Connection Pooling:**
- Jump host connections not pooled (same as direct connections)
- Each operation establishes fresh tunnel
- **Rationale:** russh session limitations prevent connection reuse

**Configuration File Support:**
- Jump hosts only supported via CLI `-J` flag currently
- Configuration file support for per-cluster jump hosts is not implemented
**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

### Future Enhancements

1. **Configuration File Support:**
1. **YAML Configuration File Support:**
```yaml
clusters:
production:
Expand Down
3 changes: 3 additions & 0 deletions src/app/dispatcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,7 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> {
use_agent: cli.use_agent,
use_password: cli.password,
recursive: *recursive,
ssh_config: Some(&ctx.ssh_config),
};
upload_file(params, source, destination).await
}
Expand All @@ -138,6 +139,7 @@ pub async fn dispatch_command(cli: &Cli, ctx: &AppContext) -> Result<()> {
use_agent: cli.use_agent,
use_password: cli.password,
recursive: *recursive,
ssh_config: Some(&ctx.ssh_config),
};
download_file(params, source, destination).await
}
Expand Down Expand Up @@ -407,6 +409,7 @@ async fn handle_exec_command(cli: &Cli, ctx: &AppContext, command: &str) -> Resu
sudo_password,
batch: cli.batch,
fail_fast: cli.fail_fast,
ssh_config: Some(&ctx.ssh_config),
};
execute_command(params).await
}
Expand Down
93 changes: 93 additions & 0 deletions src/app/initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,3 +345,96 @@ pub fn determine_use_keychain(ssh_config: &SshConfig, hostname: Option<&str>) ->
pub fn determine_use_keychain(_ssh_config: &SshConfig, _hostname: Option<&str>) -> bool {
false
}

/// Determine the effective jump hosts for a connection.
///
/// Priority order:
/// 1. CLI `-J` option (highest priority)
/// 2. SSH config `ProxyJump` directive for the hostname
/// 3. None (direct connection)
///
/// # Arguments
/// * `cli_jump_hosts` - Jump hosts specified via CLI `-J` option
/// * `ssh_config` - The loaded SSH configuration
/// * `hostname` - The target hostname to check for ProxyJump config
///
/// # Returns
/// The effective jump host specification, or None for direct connection
#[allow(dead_code)] // Used for documentation and potential future use
pub fn determine_effective_jump_hosts(
cli_jump_hosts: Option<&str>,
ssh_config: &SshConfig,
hostname: &str,
) -> Option<String> {
// CLI takes precedence
if let Some(jump) = cli_jump_hosts {
return Some(jump.to_string());
}

// Fall back to SSH config ProxyJump
ssh_config.get_proxy_jump(hostname)
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_determine_effective_jump_hosts_cli_takes_precedence() {
let ssh_config_content = r#"
Host example.com
ProxyJump bastion.example.com
"#;
let ssh_config = SshConfig::parse(ssh_config_content).unwrap();

// CLI jump host should take precedence over SSH config
let result = determine_effective_jump_hosts(
Some("cli-jump.example.com"),
&ssh_config,
"example.com",
);
assert_eq!(result, Some("cli-jump.example.com".to_string()));
}

#[test]
fn test_determine_effective_jump_hosts_falls_back_to_ssh_config() {
let ssh_config_content = r#"
Host example.com
ProxyJump bastion.example.com
"#;
let ssh_config = SshConfig::parse(ssh_config_content).unwrap();

// Should use SSH config when CLI jump host is not specified
let result = determine_effective_jump_hosts(None, &ssh_config, "example.com");
assert_eq!(result, Some("bastion.example.com".to_string()));
}

#[test]
fn test_determine_effective_jump_hosts_no_jump_host() {
let ssh_config = SshConfig::new();

// Should return None when no jump host is configured
let result = determine_effective_jump_hosts(None, &ssh_config, "example.com");
assert_eq!(result, None);
}

#[test]
fn test_determine_effective_jump_hosts_wildcard_pattern() {
let ssh_config_content = r#"
Host *.internal
ProxyJump gateway.company.com

Host db.internal
ProxyJump db-gateway.company.com
"#;
let ssh_config = SshConfig::parse(ssh_config_content).unwrap();

// Should match the most specific pattern
let result = determine_effective_jump_hosts(None, &ssh_config, "db.internal");
assert_eq!(result, Some("db-gateway.company.com".to_string()));

// Should match wildcard pattern
let result = determine_effective_jump_hosts(None, &ssh_config, "web.internal");
assert_eq!(result, Some("gateway.company.com".to_string()));
}
}
8 changes: 6 additions & 2 deletions src/commands/download.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,17 @@ pub async fn download_file(
}

let key_path_str = params.key_path.map(|p| p.to_string_lossy().to_string());
let executor = ParallelExecutor::new_with_all_options(
let mut executor = ParallelExecutor::new_with_all_options(
params.nodes.clone(),
params.max_parallel,
key_path_str.clone(),
params.strict_mode,
params.use_agent,
params.use_password,
);
if let Some(ssh_config) = params.ssh_config {
executor = executor.with_ssh_config(Some(ssh_config.clone()));
}

// Check if source contains glob pattern
let has_glob = validated_source.contains('*')
Expand Down Expand Up @@ -114,8 +117,9 @@ pub async fn download_file(
params.strict_mode,
params.use_agent,
params.use_password,
None, // No jump hosts from this code path yet
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
)
.await;

Expand Down
5 changes: 4 additions & 1 deletion src/commands/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use crate::forwarding::ForwardingType;
use crate::node::Node;
use crate::security::SudoPassword;
use crate::ssh::known_hosts::StrictHostKeyChecking;
use crate::ssh::SshConfig;
use crate::ui::OutputFormatter;
use crate::utils::output::save_outputs_to_files;

Expand All @@ -47,6 +48,7 @@ pub struct ExecuteCommandParams<'a> {
pub sudo_password: Option<Arc<SudoPassword>>,
pub batch: bool,
pub fail_fast: bool,
pub ssh_config: Option<&'a SshConfig>,
}

pub async fn execute_command(params: ExecuteCommandParams<'_>) -> Result<()> {
Expand Down Expand Up @@ -214,7 +216,8 @@ async fn execute_command_without_forwarding(params: ExecuteCommandParams<'_>) ->
.with_jump_hosts(params.jump_hosts.map(|s| s.to_string()))
.with_sudo_password(params.sudo_password)
.with_batch_mode(params.batch)
.with_fail_fast(params.fail_fast);
.with_fail_fast(params.fail_fast)
.with_ssh_config(params.ssh_config.cloned());

// Set keychain usage if on macOS
#[cfg(target_os = "macos")]
Expand Down
7 changes: 6 additions & 1 deletion src/commands/upload.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ use std::path::Path;
use crate::executor::ParallelExecutor;
use crate::node::Node;
use crate::ssh::known_hosts::StrictHostKeyChecking;
use crate::ssh::SshConfig;
use crate::ui::OutputFormatter;
use crate::utils::fs::{format_bytes, resolve_source_files};

Expand All @@ -30,6 +31,7 @@ pub struct FileTransferParams<'a> {
pub use_agent: bool,
pub use_password: bool,
pub recursive: bool,
pub ssh_config: Option<&'a SshConfig>,
}

pub async fn upload_file(
Expand Down Expand Up @@ -76,14 +78,17 @@ pub async fn upload_file(
);

let key_path_str = params.key_path.map(|p| p.to_string_lossy().to_string());
let executor = ParallelExecutor::new_with_all_options(
let mut executor = ParallelExecutor::new_with_all_options(
params.nodes.clone(),
params.max_parallel,
key_path_str.clone(),
params.strict_mode,
params.use_agent,
params.use_password,
);
if let Some(ssh_config) = params.ssh_config {
executor = executor.with_ssh_config(Some(ssh_config.clone()));
}

let mut total_success = 0;
let mut total_failed = 0;
Expand Down
Loading