refactor: move Username from domain to shared module and update SshCredentials

josecelano · josecelano · commit 933956c19829 · 2025-09-26T16:00:08.000+01:00
- Move src/domain/username.rs to src/shared/username.rs
- Update SshCredentials to use Username type instead of String for ssh_username field
- Update all usage sites across the codebase to use Username::new()
- Update SSH key setup functions to accept &amp;Username parameters
- All tests pass: 351 unit tests, full e2e test suite, all linters
diff --git a/src/application/commands/provision.rs b/src/application/commands/provision.rs
@@ -178,6 +178,8 @@ mod tests {
     use super::*;
     use tempfile::TempDir;
 
+    use crate::shared::Username;
+
     // Helper function to create mock dependencies for testing
     fn create_mock_dependencies() -> (
         Arc<TofuTemplateRenderer>,
@@ -195,7 +197,7 @@ mod tests {
         let ssh_credentials = SshCredentials::new(
             "dummy_key".into(),
             "dummy_key.pub".into(),
-            "testuser".to_string(),
+            Username::new("testuser").unwrap(),
         );
 
         let tofu_renderer = Arc::new(TofuTemplateRenderer::new(
@@ -219,8 +221,11 @@ mod tests {
 
         let ssh_key_path = temp_dir.path().join("test_key");
         let ssh_pub_key_path = temp_dir.path().join("test_key.pub");
-        let ssh_credentials =
-            SshCredentials::new(ssh_key_path, ssh_pub_key_path, "test_user".to_string());
+        let ssh_credentials = SshCredentials::new(
+            ssh_key_path,
+            ssh_pub_key_path,
+            Username::new("test_user").unwrap(),
+        );
 
         (
             tofu_renderer,
diff --git a/src/application/commands/test.rs b/src/application/commands/test.rs
@@ -111,13 +111,18 @@ mod tests {
     use std::net::{IpAddr, Ipv4Addr};
     use tempfile::TempDir;
 
+    use crate::shared::Username;
+
     // Helper function to create mock dependencies for testing
     fn create_mock_dependencies() -> (SshCredentials, IpAddr, TempDir) {
         let temp_dir = TempDir::new().expect("Failed to create temp dir");
         let ssh_key_path = temp_dir.path().join("test_key");
         let ssh_pub_key_path = temp_dir.path().join("test_key.pub");
-        let ssh_credentials =
-            SshCredentials::new(ssh_key_path, ssh_pub_key_path, "test_user".to_string());
+        let ssh_credentials = SshCredentials::new(
+            ssh_key_path,
+            ssh_pub_key_path,
+            Username::new("test_user").unwrap(),
+        );
         let instance_ip = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 100));
 
         (ssh_credentials, instance_ip, temp_dir)
diff --git a/src/application/steps/connectivity/wait_ssh_connectivity.rs b/src/application/steps/connectivity/wait_ssh_connectivity.rs
@@ -78,7 +78,8 @@ impl WaitForSSHConnectivityStep {
 mod tests {
     use std::net::{IpAddr, Ipv4Addr};
 
-    use crate::shared::ssh::SshCredentials;
+    use crate::shared::ssh::{SshConnection, SshCredentials};
+    use crate::shared::Username;
 
     use super::*;
 
@@ -88,7 +89,7 @@ mod tests {
         let credentials = SshCredentials::new(
             "/tmp/test_key".into(),
             "/tmp/test_key.pub".into(),
-            "testuser".to_string(),
+            Username::new("testuser").unwrap(),
         );
         let ssh_connection = SshConnection::with_default_port(credentials, instance_ip);
 
@@ -108,7 +109,7 @@ mod tests {
         let credentials = SshCredentials::new(
             "/home/user/.ssh/id_rsa".into(),
             "/home/user/.ssh/id_rsa.pub".into(),
-            "torrust".to_string(),
+            Username::new("torrust").unwrap(),
         );
         let ssh_connection = SshConnection::with_default_port(credentials, instance_ip);
 
@@ -128,7 +129,7 @@ mod tests {
         let credentials = SshCredentials::new(
             "/path/to/ssh/key".into(),
             "/path/to/ssh/key.pub".into(),
-            "admin".to_string(),
+            Username::new("admin").unwrap(),
         );
         let ssh_connection = SshConnection::with_default_port(credentials, instance_ip);
 
diff --git a/src/application/steps/validation/cloud_init.rs b/src/application/steps/validation/cloud_init.rs
@@ -75,6 +75,7 @@ mod tests {
     use std::path::PathBuf;
 
     use crate::shared::ssh::SshCredentials;
+    use crate::shared::Username;
 
     use super::*;
 
@@ -83,7 +84,7 @@ mod tests {
         let ssh_credentials = SshCredentials::new(
             PathBuf::from("test_key"),
             PathBuf::from("test_key.pub"),
-            "test_user".to_string(),
+            Username::new("test_user").unwrap(),
         );
         let host_ip = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 1));
         let ssh_connection = SshConnection::with_default_port(ssh_credentials, host_ip);
diff --git a/src/application/steps/validation/docker.rs b/src/application/steps/validation/docker.rs
@@ -74,6 +74,7 @@ mod tests {
     use std::path::PathBuf;
 
     use crate::shared::ssh::SshCredentials;
+    use crate::shared::Username;
 
     use super::*;
 
@@ -82,7 +83,7 @@ mod tests {
         let ssh_credentials = SshCredentials::new(
             PathBuf::from("test_key"),
             PathBuf::from("test_key.pub"),
-            "test_user".to_string(),
+            Username::new("test_user").unwrap(),
         );
         let host_ip = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 1));
         let ssh_connection = SshConnection::with_default_port(ssh_credentials, host_ip);
diff --git a/src/application/steps/validation/docker_compose.rs b/src/application/steps/validation/docker_compose.rs
@@ -80,6 +80,7 @@ mod tests {
     use std::path::PathBuf;
 
     use crate::shared::ssh::SshCredentials;
+    use crate::shared::Username;
 
     use super::*;
 
@@ -88,7 +89,7 @@ mod tests {
         let ssh_credentials = SshCredentials::new(
             PathBuf::from("test_key"),
             PathBuf::from("test_key.pub"),
-            "test_user".to_string(),
+            Username::new("test_user").unwrap(),
         );
         let host_ip = IpAddr::V4(Ipv4Addr::new(192, 168, 1, 1));
         let ssh_connection = SshConnection::with_default_port(ssh_credentials, host_ip);
diff --git a/src/bin/e2e_config_tests.rs b/src/bin/e2e_config_tests.rs
@@ -51,7 +51,6 @@ use torrust_tracker_deploy::e2e::tasks::run_configure_command::run_configure_com
 use tracing::{error, info};
 
 use torrust_tracker_deploy::config::InstanceName;
-use torrust_tracker_deploy::domain::Username;
 use torrust_tracker_deploy::e2e::environment::{TestEnvironment, TestEnvironmentType};
 use torrust_tracker_deploy::e2e::tasks::{
     container::{
@@ -62,6 +61,7 @@ use torrust_tracker_deploy::e2e::tasks::{
     run_configuration_validation::run_configuration_validation,
 };
 use torrust_tracker_deploy::logging::{self, LogFormat};
+use torrust_tracker_deploy::shared::Username;
 
 #[derive(Parser)]
 #[command(name = "e2e-config-tests")]
diff --git a/src/bin/e2e_provision_tests.rs b/src/bin/e2e_provision_tests.rs
@@ -23,7 +23,6 @@ use tracing::{error, info};
 
 // Import E2E testing infrastructure
 use torrust_tracker_deploy::config::InstanceName;
-use torrust_tracker_deploy::domain::Username;
 use torrust_tracker_deploy::e2e::environment::{TestEnvironment, TestEnvironmentType};
 use torrust_tracker_deploy::e2e::tasks::{
     preflight_cleanup::cleanup_lingering_resources,
@@ -33,6 +32,7 @@ use torrust_tracker_deploy::e2e::tasks::{
     },
 };
 use torrust_tracker_deploy::logging::{self, LogFormat};
+use torrust_tracker_deploy::shared::Username;
 
 #[derive(Parser)]
 #[command(name = "e2e-provision-tests")]
diff --git a/src/bin/e2e_tests_full.rs b/src/bin/e2e_tests_full.rs
@@ -28,7 +28,6 @@ use tracing::{error, info};
 
 // Import E2E testing infrastructure
 use torrust_tracker_deploy::config::InstanceName;
-use torrust_tracker_deploy::domain::Username;
 use torrust_tracker_deploy::e2e::environment::{TestEnvironment, TestEnvironmentType};
 use torrust_tracker_deploy::e2e::tasks::{
     preflight_cleanup::cleanup_lingering_resources,
@@ -40,6 +39,7 @@ use torrust_tracker_deploy::e2e::tasks::{
     },
 };
 use torrust_tracker_deploy::logging::{self, LogFormat};
+use torrust_tracker_deploy::shared::Username;
 
 #[derive(Parser)]
 #[command(name = "e2e-tests")]
diff --git a/src/config/mod.rs b/src/config/mod.rs
@@ -90,10 +90,11 @@ impl Config {
     /// # use std::net::{IpAddr, Ipv4Addr};
     /// # use std::path::PathBuf;
     /// # use torrust_tracker_deploy::config::{Config, SshCredentials, InstanceName};
+    /// # use torrust_tracker_deploy::shared::Username;
     /// let ssh_credentials = SshCredentials::new(
     ///     PathBuf::from("/home/user/.ssh/deploy_key"),
     ///     PathBuf::from("/home/user/.ssh/deploy_key.pub"),
-    ///     "ubuntu".to_string(),
+    ///     Username::new("ubuntu").unwrap(),
     /// );
     /// let instance_name = InstanceName::new("my-instance".to_string()).unwrap();
     /// let config = Config::new(
diff --git a/src/domain/mod.rs b/src/domain/mod.rs
@@ -7,13 +7,10 @@
 //!
 //! - `instance_name` - LXD instance name validation and management
 //! - `template` - Core template domain models and business logic
-//! - `username` - Linux system username validation and management
 
 pub mod instance_name;
 pub mod template;
-pub mod username;
 
 // Re-export commonly used domain types for convenience
 pub use instance_name::{InstanceName, InstanceNameError};
 pub use template::{TemplateEngine, TemplateEngineError, TemplateManager, TemplateManagerError};
-pub use username::{Username, UsernameError};
diff --git a/src/e2e/containers/actions/ssh_key_setup.rs b/src/e2e/containers/actions/ssh_key_setup.rs
@@ -11,6 +11,7 @@ use testcontainers::core::ExecCommand;
 
 use crate::e2e::containers::ContainerExecutor;
 use crate::shared::ssh::SshCredentials;
+use crate::shared::Username;
 
 /// Specific error types for SSH key setup operations
 #[derive(Debug, thiserror::Error)]
@@ -132,22 +133,28 @@ impl SshKeySetupAction {
         credentials: &SshCredentials,
     ) -> std::result::Result<(), SshKeySetupError> {
         Self::create_ssh_directory(container, &credentials.ssh_username).await?;
+
         Self::add_public_key_to_authorized_keys(
             container,
             &credentials.ssh_username,
             &credentials.ssh_pub_key_path,
         )
         .await?;
+
         Self::set_ssh_directory_permissions(
             container,
-            &credentials.ssh_username,
-            &format!("/home/{}/.ssh", credentials.ssh_username),
+            credentials.ssh_username.as_str(),
+            &format!("/home/{}/.ssh", credentials.ssh_username.as_str()),
         )
         .await?;
+
         Self::set_authorized_keys_permissions(
             container,
-            &credentials.ssh_username,
-            &format!("/home/{}/.ssh/authorized_keys", credentials.ssh_username),
+            credentials.ssh_username.as_str(),
+            &format!(
+                "/home/{}/.ssh/authorized_keys",
+                credentials.ssh_username.as_str()
+            ),
         )
         .await?;
 
@@ -157,9 +164,10 @@ impl SshKeySetupAction {
     /// Create the SSH directory for the user
     async fn create_ssh_directory<T: ContainerExecutor>(
         container: &T,
-        username: &str,
+        username: &Username,
     ) -> std::result::Result<(), SshKeySetupError> {
-        let command = ExecCommand::new(["mkdir", "-p", &format!("/home/{username}/.ssh")]);
+        let command =
+            ExecCommand::new(["mkdir", "-p", &format!("/home/{}/.ssh", username.as_str())]);
         container.exec(command).await.map_err(|source| {
             SshKeySetupError::SshDirectoryCreationFailed {
                 ssh_user: username.to_string(),
@@ -172,7 +180,7 @@ impl SshKeySetupAction {
     /// Add the public key to the `authorized_keys` file
     async fn add_public_key_to_authorized_keys<T: ContainerExecutor>(
         container: &T,
-        username: &str,
+        username: &Username,
         public_key_path: &Path,
     ) -> std::result::Result<(), SshKeySetupError> {
         let public_key = fs::read_to_string(public_key_path).map_err(|source| {
@@ -187,8 +195,9 @@ impl SshKeySetupAction {
             "sh",
             "-c",
             &format!(
-                "echo '{}' >> /home/{username}/.ssh/authorized_keys",
-                public_key.trim()
+                "echo '{}' >> /home/{}/.ssh/authorized_keys",
+                public_key.trim(),
+                username.as_str()
             ),
         ]);
         container.exec(command).await.map_err(|source| {
@@ -302,10 +311,11 @@ mod tests {
     // Helper function to create mock SSH credentials for testing
     #[allow(dead_code)]
     fn create_mock_ssh_credentials() -> SshCredentials {
+        use crate::shared::Username;
         SshCredentials::new(
             PathBuf::from("/mock/path/to/private_key"),
             PathBuf::from("/mock/path/to/public_key.pub"),
-            "testuser".to_string(),
+            Username::new("testuser").unwrap(),
         )
     }
 }
diff --git a/src/e2e/environment.rs b/src/e2e/environment.rs
@@ -26,7 +26,7 @@ use tracing::{info, warn};
 
 use crate::config::{Config, InstanceName, SshCredentials};
 use crate::container::Services;
-use crate::domain::Username;
+use crate::shared::Username;
 
 /// Errors that can occur during test environment creation and initialization
 #[derive(Debug, thiserror::Error)]
@@ -233,7 +233,7 @@ impl TestEnvironment {
         Ok(SshCredentials::new(
             temp_ssh_key,
             temp_ssh_pub_key,
-            ssh_user.to_string(),
+            ssh_user.clone(),
         ))
     }
 
diff --git a/src/e2e/tasks/container/preflight_cleanup.rs b/src/e2e/tasks/container/preflight_cleanup.rs
@@ -79,7 +79,7 @@ fn cleanup_hanging_docker_containers(env: &TestEnvironment) {
     // First, check if the container exists
     let check_result = command_executor.run_command(
         "docker",
-        &["ps", "-aq", "--filter", &format!("name={}", instance_name)],
+        &["ps", "-aq", "--filter", &format!("name={instance_name}")],
         None,
     );
 
diff --git a/src/e2e/tasks/container/run_provision_simulation.rs b/src/e2e/tasks/container/run_provision_simulation.rs
@@ -64,7 +64,7 @@ use crate::infrastructure::ansible::AnsibleTemplateRenderer;
 /// use torrust_tracker_deploy::e2e::tasks::container::run_provision_simulation::run_provision_simulation;
 /// use torrust_tracker_deploy::e2e::environment::{TestEnvironment, TestEnvironmentType};
 /// use torrust_tracker_deploy::config::InstanceName;
-/// use torrust_tracker_deploy::domain::Username;
+/// use torrust_tracker_deploy::shared::Username;
 ///
 /// #[tokio::main]
 /// async fn main() -> anyhow::Result<()> {
diff --git a/src/infrastructure/tofu/template/renderer/cloud_init.rs b/src/infrastructure/tofu/template/renderer/cloud_init.rs
@@ -30,7 +30,7 @@
 //! let ssh_credentials = SshCredentials::new(
 //!     PathBuf::from("fixtures/testing_rsa"),
 //!     PathBuf::from("fixtures/testing_rsa.pub"),
-//!     "username".to_string()
+//!     Username::new("username").unwrap()
 //! );
 //! let renderer = CloudInitTemplateRenderer::new(template_manager);
 //!
@@ -219,6 +219,8 @@ mod tests {
     use std::fs;
     use tempfile::TempDir;
 
+    use crate::shared::Username;
+
     /// Helper function to create mock SSH credentials for testing
     fn create_mock_ssh_credentials(temp_dir: &std::path::Path) -> SshCredentials {
         let ssh_priv_key_path = temp_dir.join("test_key");
@@ -233,7 +235,11 @@ mod tests {
         )
         .expect("Failed to write public key");
 
-        SshCredentials::new(ssh_priv_key_path, ssh_pub_key_path, "test_user".to_string())
+        SshCredentials::new(
+            ssh_priv_key_path,
+            ssh_pub_key_path,
+            Username::new("test_user").unwrap(),
+        )
     }
 
     /// Helper function to create a mock template manager with cloud-init template
@@ -329,7 +335,7 @@ users:
         let ssh_credentials = SshCredentials::new(
             non_existent_key.clone(),
             non_existent_key,
-            "test_user".to_string(),
+            Username::new("test_user").unwrap(),
         );
 
         let output_dir = TempDir::new().expect("Failed to create output dir");
diff --git a/src/infrastructure/tofu/template/renderer/mod.rs b/src/infrastructure/tofu/template/renderer/mod.rs
diff --git a/src/shared/mod.rs b/src/shared/mod.rs
diff --git a/src/shared/ssh/client.rs b/src/shared/ssh/client.rs
diff --git a/src/shared/ssh/connection.rs b/src/shared/ssh/connection.rs
diff --git a/src/shared/ssh/credentials.rs b/src/shared/ssh/credentials.rs
diff --git a/src/shared/username.rs b/src/shared/username.rs
