refactor: replace primitive String/&str with InstanceName type for LXD instance names

- Update LXD client methods to use &InstanceName parameters
- Update template variables context to use InstanceName field
- Update TofuTemplateRenderer to use InstanceName throughout
- Add Serialize trait to InstanceName for JSON template rendering
- Maintain backward compatibility with hardcoded values
- Add TODO comments for Phase 3 configurability
- Improve type safety with compile-time validation

Author: josecelano

src/command_wrappers/lxd/client.rs

@@ -21,7 +21,8 @@ use tracing::info;
 
 use crate::command::CommandExecutor;
 
-use super::instance::InstanceInfo;
+#[allow(unused_imports)]
+use super::instance::{InstanceInfo, InstanceName};
 use super::json_parser::LxdJsonParser;
 
 /// A specialized LXD client for instance management.
@@ -67,7 +68,7 @@ impl LxdClient {
     /// This function will return an error if:
     /// * LXD command execution fails
     /// * JSON parsing fails
-    pub fn get_instance_ip(&self, instance_name: &str) -> Result<Option<IpAddr>> {
+    pub fn get_instance_ip(&self, instance_name: &InstanceName) -> Result<Option<IpAddr>> {
         info!("Getting IP address for instance: {}", instance_name);
 
         let Some(instance) = self.get_instance_by_name(instance_name)? else {
@@ -108,7 +109,7 @@ impl LxdClient {
     /// * JSON parsing fails
     pub fn wait_for_instance_ip(
         &self,
-        instance_name: &str,
+        instance_name: &InstanceName,
         timeout_seconds: u64,
         poll_interval_seconds: u64,
     ) -> Result<IpAddr> {
@@ -162,14 +163,17 @@ impl LxdClient {
     /// This function will return an error if:
     /// * LXD command execution fails
     /// * JSON parsing fails
-    pub fn get_instance_by_name(&self, instance_name: &str) -> Result<Option<InstanceInfo>> {
+    pub fn get_instance_by_name(
+        &self,
+        instance_name: &InstanceName,
+    ) -> Result<Option<InstanceInfo>> {
         info!("Getting instance by name: {}", instance_name);
 
         let instances = self.list(Some(instance_name))?;
 
         Ok(instances
             .into_iter()
-            .find(|inst| inst.name.as_str() == instance_name))
+            .find(|inst| inst.name.as_str() == instance_name.as_str()))
     }
 
     /// List instances in JSON format
@@ -188,13 +192,13 @@ impl LxdClient {
     /// * The LXD command fails
     /// * LXD is not installed or accessible
     /// * JSON parsing fails
-    fn list(&self, instance_name: Option<&str>) -> Result<Vec<InstanceInfo>> {
+    fn list(&self, instance_name: Option<&InstanceName>) -> Result<Vec<InstanceInfo>> {
         info!("Listing LXD instances");
 
         let mut args = vec!["list", "--format=json"];
 
         if let Some(name) = instance_name {
-            args.push(name);
+            args.push(name.as_str());
             info!("Filtering by instance name: {}", name);
         }
 
@@ -223,10 +227,10 @@ impl LxdClient {
     /// This function will return an error if:
     /// * The LXD command fails with an unexpected error
     /// * LXD is not installed or accessible
-    pub fn delete_instance(&self, instance_name: &str, force: bool) -> Result<()> {
+    pub fn delete_instance(&self, instance_name: &InstanceName, force: bool) -> Result<()> {
         info!("Deleting LXD instance: {}", instance_name);
 
-        let mut args = vec!["delete", instance_name];
+        let mut args = vec!["delete", instance_name.as_str()];
         if force {
             args.push("--force");
         }

src/command_wrappers/lxd/instance/name.rs

@@ -18,6 +18,8 @@ use std::fmt;
 use std::str::FromStr;
 
 use anyhow::{anyhow, Result};
+#[allow(unused_imports)]
+use serde::Serialize;
 
 /// A validated LXD instance name following LXD naming requirements.
 ///
@@ -29,7 +31,7 @@ use anyhow::{anyhow, Result};
 ///
 /// These requirements ensure that the instance name can be used in DNS records,
 /// on the file system, in various security profiles and as the host name.
-#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+#[derive(Debug, Clone, PartialEq, Eq, Hash, Serialize)]
 pub struct InstanceName(String);
 
 impl InstanceName {

src/commands/provision.rs

@@ -20,6 +20,8 @@ use tracing::{info, instrument};
 use crate::ansible::AnsibleTemplateRenderer;
 use crate::command::CommandError;
 use crate::command_wrappers::ansible::AnsibleClient;
+#[allow(unused_imports)]
+use crate::command_wrappers::lxd::InstanceName;
 use crate::command_wrappers::opentofu::client::{InstanceInfo, OpenTofuError};
 use crate::command_wrappers::ssh::{credentials::SshCredentials, SshError};
 use crate::steps::{
@@ -196,7 +198,7 @@ mod tests {
             template_manager.clone(),
             temp_dir.path(),
             ssh_credentials.clone(),
-            "torrust-vm".to_string(), // TODO: Make this configurable in Phase 3
+            InstanceName::new("torrust-vm".to_string()).expect("Valid hardcoded instance name"), // TODO: Make this configurable in Phase 3
         ));
 
         let ansible_renderer = Arc::new(AnsibleTemplateRenderer::new(

src/container.rs

@@ -14,7 +14,7 @@ use std::sync::Arc;
 
 use crate::ansible::AnsibleTemplateRenderer;
 use crate::command_wrappers::ansible::AnsibleClient;
-use crate::command_wrappers::lxd::LxdClient;
+use crate::command_wrappers::lxd::{InstanceName, LxdClient};
 use crate::command_wrappers::opentofu::OpenTofuClient;
 use crate::config::Config;
 use crate::template::TemplateManager;
@@ -35,6 +35,11 @@ pub struct Services {
 
 impl Services {
     /// Create a new services container using the provided configuration
+    ///
+    /// # Panics
+    ///
+    /// Panics if the hardcoded "torrust-vm" instance name is invalid (should never happen
+    /// as it's a known valid name). This will be made configurable in Phase 3.
     #[must_use]
     pub fn new(config: &Config) -> Self {
         // Create template manager
@@ -56,7 +61,7 @@ impl Services {
             template_manager.clone(),
             config.build_dir.clone(),
             config.ssh_credentials.clone(),
-            "torrust-vm".to_string(), // TODO: Make this configurable in Phase 3
+            InstanceName::new("torrust-vm".to_string()).expect("Valid hardcoded instance name"), // TODO: Make this configurable in Phase 3
         );
 
         // Create configuration template renderer

src/e2e/tasks/preflight_cleanup.rs

@@ -6,7 +6,8 @@
 use std::fmt;
 use tracing::{info, warn};
 
-use crate::command_wrappers::lxd::client::LxdClient;
+#[allow(unused_imports)]
+use crate::command_wrappers::lxd::{client::LxdClient, InstanceName};
 use crate::command_wrappers::opentofu::{self, EmergencyDestroyError};
 use crate::e2e::environment::TestEnvironment;
 
@@ -275,7 +276,10 @@ fn cleanup_lxd_resources(_env: &TestEnvironment) {
     let lxd_client = LxdClient::new();
 
     // Clean up test instance if it exists
-    match lxd_client.delete_instance("torrust-vm", true) {
+    match lxd_client.delete_instance(
+        &InstanceName::new("torrust-vm".to_string()).expect("Valid hardcoded instance name"),
+        true,
+    ) {
         Ok(()) => {
             info!(
                 operation = "lxd_resources_cleanup",

src/template/wrappers/tofu/lxd/variables/context.rs

@@ -14,16 +14,20 @@
 //!
 //! ```rust
 //! use torrust_tracker_deploy::template::wrappers::tofu::lxd::variables::VariablesContext;
+//! use torrust_tracker_deploy::command_wrappers::lxd::instance::InstanceName;
 //!
 //! let context = VariablesContext::builder()
-//!     .with_instance_name("my-test-vm".to_string())
+//!     .with_instance_name(InstanceName::new("my-test-vm".to_string()).unwrap())
 //!     .build()
 //!     .unwrap();
 //! ```
 
 use serde::Serialize;
 use thiserror::Error;
 
+#[allow(unused_imports)]
+use crate::command_wrappers::lxd::instance::InstanceName;
+
 /// Errors that can occur when building the variables context
 #[derive(Error, Debug)]
 pub enum VariablesContextError {
@@ -39,7 +43,7 @@ pub enum VariablesContextError {
 #[derive(Debug, Clone, Serialize)]
 pub struct VariablesContext {
     /// The name of the VM/container instance to be created
-    pub instance_name: String,
+    pub instance_name: InstanceName,
 }
 
 /// Builder for creating `VariablesContext` instances
@@ -48,7 +52,7 @@ pub struct VariablesContext {
 /// to ensure all required fields are provided.
 #[derive(Debug, Default)]
 pub struct VariablesContextBuilder {
-    instance_name: Option<String>,
+    instance_name: Option<InstanceName>,
 }
 
 impl VariablesContextBuilder {
@@ -64,7 +68,7 @@ impl VariablesContextBuilder {
     ///
     /// * `instance_name` - The name to assign to the created instance
     #[must_use]
-    pub fn with_instance_name(mut self, instance_name: String) -> Self {
+    pub fn with_instance_name(mut self, instance_name: InstanceName) -> Self {
         self.instance_name = Some(instance_name);
         self
     }
@@ -103,17 +107,17 @@ mod tests {
     #[test]
     fn it_should_create_variables_context_with_instance_name() {
         let context = VariablesContext::builder()
-            .with_instance_name("test-vm".to_string())
+            .with_instance_name(InstanceName::new("test-vm".to_string()).unwrap())
             .build()
             .unwrap();
 
-        assert_eq!(context.instance_name, "test-vm");
+        assert_eq!(context.instance_name.as_str(), "test-vm");
     }
 
     #[test]
     fn it_should_serialize_to_json() {
         let context = VariablesContext::builder()
-            .with_instance_name("test-vm".to_string())
+            .with_instance_name(InstanceName::new("test-vm".to_string()).unwrap())
             .build()
             .unwrap();
 
@@ -125,12 +129,12 @@ mod tests {
     #[test]
     fn it_should_build_context_with_builder_pattern() {
         let result = VariablesContext::builder()
-            .with_instance_name("my-instance".to_string())
+            .with_instance_name(InstanceName::new("my-instance".to_string()).unwrap())
             .build();
 
         assert!(result.is_ok());
         let context = result.unwrap();
-        assert_eq!(context.instance_name, "my-instance");
+        assert_eq!(context.instance_name.as_str(), "my-instance");
     }
 
     #[test]

src/template/wrappers/tofu/lxd/variables/mod.rs

@@ -80,7 +80,7 @@ impl VariablesTemplate {
     /// Get the instance name value
     #[must_use]
     pub fn instance_name(&self) -> &str {
-        &self.context.instance_name
+        self.context.instance_name.as_str()
     }
 
     /// Render the template to a file at the specified output path
@@ -109,11 +109,12 @@ impl VariablesTemplate {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use crate::command_wrappers::lxd::instance::InstanceName;
     use tempfile::NamedTempFile;
 
     fn create_test_context() -> VariablesContext {
         VariablesContext::builder()
-            .with_instance_name("test-instance".to_string())
+            .with_instance_name(InstanceName::new("test-instance".to_string()).unwrap())
             .build()
             .unwrap()
     }
@@ -200,7 +201,10 @@ image = "ubuntu:24.04""#;
         let context = create_test_context();
         let variables_template = VariablesTemplate::new(&template_file, context).unwrap();
 
-        assert_eq!(variables_template.context().instance_name, "test-instance");
+        assert_eq!(
+            variables_template.context().instance_name.as_str(),
+            "test-instance"
+        );
     }
 
     #[test]
@@ -256,11 +260,14 @@ image = "ubuntu:24.04""#;
         let template_file =
             File::new("variables.tfvars.tera", "{{ instance_name }}".to_string()).unwrap();
         let context = VariablesContext::builder()
-            .with_instance_name("dynamic-vm".to_string())
+            .with_instance_name(InstanceName::new("dynamic-vm".to_string()).unwrap())
             .build()
             .unwrap();
 
         let variables_template = VariablesTemplate::new(&template_file, context).unwrap();
-        assert_eq!(variables_template.context().instance_name, "dynamic-vm");
+        assert_eq!(
+            variables_template.context().instance_name.as_str(),
+            "dynamic-vm"
+        );
     }
 }