Skip to content

Commit 1c1193d

Browse files
committed
fix: [#340] remove SSH key file existence validation from config parsing
SSH key files are external resources that should be validated at runtime when actually needed (during provision/configure), not during configuration parsing or validation. This allows: - Configs to be validated on machines without SSH keys - SSH keys to be on different machines than where config is validated - More flexible deployment workflows Changes: - Removed file existence checks from TryFrom<SshCredentialsConfig> - Updated validate command to skip file existence validation - Marked 5 obsolete tests as #[ignore] with explanatory comments - SSH keys now validated when SSH connections are attempted (runtime) Error handling remains user-friendly - provision command will fail with clear message if SSH keys don't exist when actually needed. Fixes #340
1 parent 2a4fc89 commit 1c1193d

7 files changed

Lines changed: 21 additions & 142 deletions

File tree

src/application/command_handlers/create/config/environment_config.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,7 +820,9 @@ mod tests {
820820
}
821821
}
822822

823+
// Note: SSH key file existence validation removed - checked at runtime instead
823824
#[test]
825+
#[ignore = "SSH key file existence no longer validated during config parsing"]
824826
fn it_should_return_error_when_private_key_file_not_found() {
825827
use std::env;
826828

@@ -857,7 +859,9 @@ mod tests {
857859
}
858860
}
859861

862+
// Note: SSH key file existence validation removed - checked at runtime instead
860863
#[test]
864+
#[ignore = "SSH key file existence no longer validated during config parsing"]
861865
fn it_should_return_error_when_public_key_file_not_found() {
862866
use std::env;
863867

src/application/command_handlers/create/config/ssh_credentials_config.rs

Lines changed: 6 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -120,18 +120,10 @@ impl TryFrom<SshCredentialsConfig> for SshCredentials {
120120
});
121121
}
122122

123-
// Validate SSH key files exist
124-
if !private_key_path.exists() {
125-
return Err(CreateConfigError::PrivateKeyNotFound {
126-
path: private_key_path,
127-
});
128-
}
129-
130-
if !public_key_path.exists() {
131-
return Err(CreateConfigError::PublicKeyNotFound {
132-
path: public_key_path,
133-
});
134-
}
123+
// Note: File existence is NOT validated here.
124+
// SSH keys are external resources that may not exist at config parsing time.
125+
// They will be validated at runtime when SSH connections are actually attempted.
126+
// This allows configs to be validated and stored even if keys are on different machines.
135127

136128
// Create domain credentials object
137129
Ok(SshCredentials::new(
@@ -275,55 +267,8 @@ mod tests {
275267
}
276268
}
277269

278-
#[test]
279-
fn it_should_return_error_when_private_key_file_not_found() {
280-
use std::env;
281-
282-
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
283-
let public_key_path = format!("{project_root}/fixtures/testing_rsa.pub");
284-
285-
let config = SshCredentialsConfig::new(
286-
"/nonexistent/private_key".to_string(),
287-
public_key_path,
288-
"torrust".to_string(),
289-
22,
290-
);
291-
292-
let result: Result<SshCredentials, CreateConfigError> = config.try_into();
293-
assert!(result.is_err());
294-
295-
match result.unwrap_err() {
296-
CreateConfigError::PrivateKeyNotFound { .. } => {
297-
// Expected error
298-
}
299-
other => panic!("Expected PrivateKeyNotFound error, got: {other:?}"),
300-
}
301-
}
302-
303-
#[test]
304-
fn it_should_return_error_when_public_key_file_not_found() {
305-
use std::env;
306-
307-
let project_root = env::var("CARGO_MANIFEST_DIR").expect("CARGO_MANIFEST_DIR not set");
308-
let private_key_path = format!("{project_root}/fixtures/testing_rsa");
309-
310-
let config = SshCredentialsConfig::new(
311-
private_key_path,
312-
"/nonexistent/public_key.pub".to_string(),
313-
"torrust".to_string(),
314-
22,
315-
);
316-
317-
let result: Result<SshCredentials, CreateConfigError> = config.try_into();
318-
assert!(result.is_err());
319-
320-
match result.unwrap_err() {
321-
CreateConfigError::PublicKeyNotFound { path } => {
322-
assert_eq!(path, PathBuf::from("/nonexistent/public_key.pub"));
323-
}
324-
other => panic!("Expected PublicKeyNotFound error, got: {other:?}"),
325-
}
326-
}
270+
// Note: Tests for file existence removed - file existence is now validated
271+
// at runtime when SSH connections are attempted, not during config parsing.
327272

328273
#[test]
329274
fn it_should_provide_correct_default_values_when_using_default_functions() {

src/application/command_handlers/create/tests/integration.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,9 @@ fn it_should_fail_with_invalid_environment_name() {
163163
}
164164
}
165165

166+
// Note: SSH key file existence validation removed - checked at runtime instead
166167
#[test]
168+
#[ignore = "SSH key file existence no longer validated during config parsing"]
167169
fn it_should_fail_when_ssh_private_key_not_found() {
168170
use crate::application::command_handlers::create::config::tracker::TrackerSection;
169171
use crate::application::command_handlers::create::config::{

src/application/command_handlers/validate/handler.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,7 @@ impl ValidateCommandHandler {
9292

9393
// Step 3: Convert to domain types (validates all constraints)
9494
// This includes:
95-
// - SSH key files must exist
95+
// - SSH key paths must be absolute (file existence checked at runtime)
9696
// - Port numbers must be valid
9797
// - Domain names must be well-formed
9898
// - All business rules must pass

src/presentation/controllers/create/subcommands/environment/config_loader.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,9 @@ mod tests {
288288
}
289289
}
290290

291+
// Note: SSH key file existence validation removed - checked at runtime instead
291292
#[test]
293+
#[ignore = "SSH key file existence no longer validated during config parsing"]
292294
fn it_should_return_error_for_missing_ssh_keys() {
293295
let temp_dir = TempDir::new().unwrap();
294296
let config_path = temp_dir.path().join("missing_keys.json");

src/presentation/controllers/create/tests/environment.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,9 @@ async fn it_should_reject_invalid_environment_name() {
105105
}
106106
}
107107

108+
// Note: SSH key file existence validation removed - checked at runtime instead
108109
#[tokio::test]
110+
#[ignore = "SSH key file existence no longer validated during config parsing"]
109111
async fn it_should_reject_missing_ssh_keys() {
110112
let context = TestContext::new();
111113
let config_path = create_config_with_missing_keys(context.working_dir());

tests/e2e/validate_command.rs

Lines changed: 4 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -121,86 +121,10 @@ fn it_should_report_invalid_json_when_configuration_file_has_malformed_json() {
121121
);
122122
}
123123

124-
#[test]
125-
fn it_should_report_missing_ssh_keys_when_key_files_do_not_exist() {
126-
// Verify dependencies before running tests
127-
verify_required_dependencies().expect("Dependency verification failed");
128-
129-
// Arrange: Create temporary workspace
130-
let temp_workspace = TempWorkspace::new().expect("Failed to create temp workspace");
131-
132-
// Create config with non-existent SSH keys
133-
let config_json = r#"{
134-
"environment": {
135-
"name": "test-missing-keys"
136-
},
137-
"ssh_credentials": {
138-
"private_key_path": "/tmp/nonexistent-private.key",
139-
"public_key_path": "/tmp/nonexistent-public.pub",
140-
"username": "torrust",
141-
"port": 22
142-
},
143-
"provider": {
144-
"provider": "lxd",
145-
"profile_name": "default"
146-
},
147-
"tracker": {
148-
"core": {
149-
"database": {
150-
"driver": "sqlite3",
151-
"database_name": "tracker.db"
152-
},
153-
"private": false
154-
},
155-
"udp_trackers": [
156-
{
157-
"bind_address": "0.0.0.0:6969"
158-
}
159-
],
160-
"http_trackers": [
161-
{
162-
"bind_address": "0.0.0.0:7070"
163-
}
164-
],
165-
"http_api": {
166-
"bind_address": "0.0.0.0:1212",
167-
"admin_token": "test"
168-
},
169-
"health_check_api": {
170-
"bind_address": "127.0.0.1:1313"
171-
}
172-
}
173-
}"#;
174-
175-
let config_path = temp_workspace.path().join("invalid-ssh.json");
176-
fs::write(&config_path, config_json).expect("Failed to write config");
177-
178-
// Act: Run validate command
179-
let result = ProcessRunner::new()
180-
.working_dir(temp_workspace.path())
181-
.run_validate_command(config_path.to_str().unwrap())
182-
.expect("Failed to run validate command");
183-
184-
// Assert: Command should fail
185-
assert!(
186-
!result.success(),
187-
"Validate command should fail for missing SSH keys"
188-
);
189-
190-
// Assert: Error message should mention SSH keys
191-
let stderr = result.stderr();
192-
assert!(
193-
stderr.contains("SSH")
194-
&& (stderr.contains("not found") || stderr.contains("does not exist")),
195-
"Expected error about missing SSH keys, got: {stderr}"
196-
);
197-
198-
// Assert: Help text should provide guidance
199-
assert!(
200-
stderr.contains("domain constraints") || stderr.contains("Common issues"),
201-
"Expected helpful troubleshooting tips, got: {stderr}"
202-
);
203-
}
124+
// Note: Test for missing SSH key files removed.
125+
// File existence is no longer validated during config validation.
126+
// SSH key files are external resources checked at runtime (provision/configure commands).
127+
// This allows configs to be validated even when SSH keys don't exist yet or are on different machines.
204128

205129
#[test]
206130
fn it_should_succeed_when_configuration_file_is_valid() {

0 commit comments

Comments
 (0)