From ab84bbe905a162af24e334037bb10c90de38a7c0 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 29 Dec 2025 18:03:41 +0000 Subject: [PATCH 1/7] feat: add configurable Health Check API to tracker configuration - Add HealthCheckApiConfig domain type with bind_address field - Add HealthCheckApiSection DTO with validation (rejects port 0) - Default bind address: 127.0.0.1:1313 - Update TrackerConfig and TrackerSection with health_check_api field - Add comprehensive unit tests for validation logic - Update all test fixtures and E2E config generators - Regenerate environment-config.json schema - Fix all doc test examples The Health Check API is now fully configurable throughout the application with proper validation and comprehensive test coverage. --- project-words.txt | 2 + schemas/environment-config.json | 22 ++- .../create/config/environment_config.rs | 10 + .../tracker/health_check_api_section.rs | 125 +++++++++++++ .../create/config/tracker/mod.rs | 2 + .../create/config/tracker/tracker_section.rs | 26 ++- src/domain/environment/mod.rs | 4 +- src/domain/tracker/config.rs | 33 +++- src/domain/tracker/mod.rs | 8 +- src/domain/tracker/protocol.rs | 177 ++++++++++++++++++ .../template/wrappers/variables/context.rs | 19 +- .../template/renderer/project_generator.rs | 14 +- .../wrapper/tracker_config/context.rs | 15 +- .../subcommands/environment/config_loader.rs | 12 ++ .../create/subcommands/environment/tests.rs | 9 + src/presentation/controllers/tests/mod.rs | 9 + src/testing/e2e/containers/tracker_ports.rs | 3 + .../e2e/tasks/black_box/generate_config.rs | 3 + 18 files changed, 471 insertions(+), 22 deletions(-) create mode 100644 src/application/command_handlers/create/config/tracker/health_check_api_section.rs create mode 100644 src/domain/tracker/protocol.rs diff --git a/project-words.txt b/project-words.txt index f23f44d5..56083dd3 100644 --- a/project-words.txt +++ b/project-words.txt @@ -303,3 +303,5 @@ zeroize ключ конфиг файл +Datagram +connectionless diff --git a/schemas/environment-config.json b/schemas/environment-config.json index 682321f7..08f02a5a 100644 --- a/schemas/environment-config.json +++ b/schemas/environment-config.json @@ -155,6 +155,17 @@ "admin_password" ] }, + "HealthCheckApiSection": { + "type": "object", + "properties": { + "bind_address": { + "type": "string" + } + }, + "required": [ + "bind_address" + ] + }, "HetznerProviderSection": { "description": "Hetzner-specific configuration section\n\nUses raw `String` fields for JSON deserialization. Convert to domain\n`HetznerConfig` via `ProviderSection::to_provider_config()`.\n\n# Examples\n\n```rust\nuse torrust_tracker_deployer_lib::application::command_handlers::create::config::HetznerProviderSection;\n\nlet section = HetznerProviderSection {\n api_token: \"your-api-token\".to_string(),\n server_type: \"cx22\".to_string(),\n location: \"nbg1\".to_string(),\n image: \"ubuntu-24.04\".to_string(),\n};\n```", "type": "object", @@ -320,13 +331,17 @@ ] }, "TrackerSection": { - "description": "Tracker configuration section (application DTO)\n\nAggregates all tracker configuration sections: core, UDP trackers,\nHTTP trackers, and HTTP API.\n\n# Examples\n\n```json\n{\n \"core\": {\n \"database\": {\n \"driver\": \"sqlite3\",\n \"database_name\": \"tracker.db\"\n },\n \"private\": false\n },\n \"udp_trackers\": [\n { \"bind_address\": \"0.0.0.0:6969\" }\n ],\n \"http_trackers\": [\n { \"bind_address\": \"0.0.0.0:7070\" }\n ],\n \"http_api\": {\n \"bind_address\": \"0.0.0.0:1212\",\n \"admin_token\": \"MyAccessToken\"\n }\n}\n```", + "description": "Tracker configuration section (application DTO)\n\nAggregates all tracker configuration sections: core, UDP trackers,\nHTTP trackers, and HTTP API.\n\n# Examples\n\n```json\n{\n \"core\": {\n \"database\": {\n \"driver\": \"sqlite3\",\n \"database_name\": \"tracker.db\"\n },\n \"private\": false\n },\n \"udp_trackers\": [\n { \"bind_address\": \"0.0.0.0:6969\" }\n ],\n \"http_trackers\": [\n { \"bind_address\": \"0.0.0.0:7070\" }\n ],\n \"http_api\": {\n \"bind_address\": \"0.0.0.0:1212\",\n \"admin_token\": \"MyAccessToken\"\n },\n \"health_check_api\": {\n \"bind_address\": \"127.0.0.1:1313\"\n }\n}\n```", "type": "object", "properties": { "core": { "description": "Core tracker configuration (database, privacy mode)", "$ref": "#/$defs/TrackerCoreSection" }, + "health_check_api": { + "description": "Health Check API configuration", + "$ref": "#/$defs/HealthCheckApiSection" + }, "http_api": { "description": "HTTP API configuration", "$ref": "#/$defs/HttpApiSection" @@ -350,7 +365,8 @@ "core", "udp_trackers", "http_trackers", - "http_api" + "http_api", + "health_check_api" ] }, "UdpTrackerSection": { @@ -365,4 +381,4 @@ ] } } -} \ No newline at end of file +} diff --git a/src/application/command_handlers/create/config/environment_config.rs b/src/application/command_handlers/create/config/environment_config.rs index f99c311e..7b421df9 100644 --- a/src/application/command_handlers/create/config/environment_config.rs +++ b/src/application/command_handlers/create/config/environment_config.rs @@ -67,6 +67,9 @@ use super::tracker::TrackerSection; /// "http_api": { /// "bind_address": "0.0.0.0:1212", /// "admin_token": "MyAccessToken" +/// }, +/// "health_check_api": { +/// "bind_address": "127.0.0.1:1313" /// } /// }, /// "prometheus": { @@ -417,6 +420,7 @@ impl EnvironmentCreationConfig { bind_address: "0.0.0.0:1212".to_string(), admin_token: "MyAccessToken".to_string(), }, + health_check_api: super::tracker::HealthCheckApiSection::default(), }, prometheus: Some(PrometheusSection::default()), grafana: Some(GrafanaSection::default()), @@ -572,6 +576,9 @@ mod tests { "http_api": { "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }, + "health_check_api": { + "bind_address": "127.0.0.1:1313" } } }"#; @@ -633,6 +640,9 @@ mod tests { "http_api": { "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }, + "health_check_api": { + "bind_address": "127.0.0.1:1313" } } }"#; diff --git a/src/application/command_handlers/create/config/tracker/health_check_api_section.rs b/src/application/command_handlers/create/config/tracker/health_check_api_section.rs new file mode 100644 index 00000000..9556cc54 --- /dev/null +++ b/src/application/command_handlers/create/config/tracker/health_check_api_section.rs @@ -0,0 +1,125 @@ +use std::net::SocketAddr; + +use schemars::JsonSchema; +use serde::{Deserialize, Serialize}; + +use crate::application::command_handlers::create::config::errors::CreateConfigError; +use crate::domain::tracker::HealthCheckApiConfig; + +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema)] +pub struct HealthCheckApiSection { + pub bind_address: String, +} + +impl HealthCheckApiSection { + /// Converts this DTO to a domain `HealthCheckApiConfig` + /// + /// # Errors + /// + /// Returns `CreateConfigError::InvalidBindAddress` if the bind address cannot be parsed as a valid IP:PORT combination. + /// Returns `CreateConfigError::DynamicPortNotSupported` if port 0 (dynamic port assignment) is specified. + pub fn to_health_check_api_config(&self) -> Result { + // Validate that the bind address can be parsed as SocketAddr + let bind_address = self.bind_address.parse::().map_err(|e| { + CreateConfigError::InvalidBindAddress { + address: self.bind_address.clone(), + source: e, + } + })?; + + // Reject port 0 (dynamic port assignment) + if bind_address.port() == 0 { + return Err(CreateConfigError::DynamicPortNotSupported { + bind_address: self.bind_address.clone(), + }); + } + + Ok(HealthCheckApiConfig { bind_address }) + } +} + +impl Default for HealthCheckApiSection { + fn default() -> Self { + Self { + bind_address: "127.0.0.1:1313".to_string(), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_convert_to_domain_config_when_bind_address_is_valid() { + let section = HealthCheckApiSection { + bind_address: "127.0.0.1:1313".to_string(), + }; + + let config = section.to_health_check_api_config().unwrap(); + + assert_eq!( + config.bind_address, + "127.0.0.1:1313".parse::().unwrap() + ); + } + + #[test] + fn it_should_fail_when_bind_address_is_invalid() { + let section = HealthCheckApiSection { + bind_address: "invalid".to_string(), + }; + + let result = section.to_health_check_api_config(); + + assert!(result.is_err()); + assert!(matches!( + result.unwrap_err(), + CreateConfigError::InvalidBindAddress { .. } + )); + } + + #[test] + fn it_should_reject_dynamic_port_assignment() { + let section = HealthCheckApiSection { + bind_address: "0.0.0.0:0".to_string(), + }; + + let result = section.to_health_check_api_config(); + + assert!(result.is_err()); + assert!(matches!( + result.unwrap_err(), + CreateConfigError::DynamicPortNotSupported { .. } + )); + } + + #[test] + fn it_should_allow_ipv6_addresses() { + let section = HealthCheckApiSection { + bind_address: "[::1]:1313".to_string(), + }; + + let result = section.to_health_check_api_config(); + + assert!(result.is_ok()); + } + + #[test] + fn it_should_allow_any_port_except_zero() { + let section = HealthCheckApiSection { + bind_address: "127.0.0.1:8080".to_string(), + }; + + let result = section.to_health_check_api_config(); + + assert!(result.is_ok()); + } + + #[test] + fn it_should_provide_default_localhost_1313() { + let section = HealthCheckApiSection::default(); + + assert_eq!(section.bind_address, "127.0.0.1:1313"); + } +} diff --git a/src/application/command_handlers/create/config/tracker/mod.rs b/src/application/command_handlers/create/config/tracker/mod.rs index edc034af..6115fbc8 100644 --- a/src/application/command_handlers/create/config/tracker/mod.rs +++ b/src/application/command_handlers/create/config/tracker/mod.rs @@ -4,12 +4,14 @@ //! environment creation. These types use raw primitives (String) for //! JSON deserialization and convert to rich domain types (`SocketAddr`). +mod health_check_api_section; mod http_api_section; mod http_tracker_section; mod tracker_core_section; mod tracker_section; mod udp_tracker_section; +pub use health_check_api_section::HealthCheckApiSection; pub use http_api_section::HttpApiSection; pub use http_tracker_section::HttpTrackerSection; pub use tracker_core_section::{DatabaseSection, TrackerCoreSection}; diff --git a/src/application/command_handlers/create/config/tracker/tracker_section.rs b/src/application/command_handlers/create/config/tracker/tracker_section.rs index d7e4883c..5563c5b0 100644 --- a/src/application/command_handlers/create/config/tracker/tracker_section.rs +++ b/src/application/command_handlers/create/config/tracker/tracker_section.rs @@ -6,9 +6,14 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use super::{HttpApiSection, HttpTrackerSection, TrackerCoreSection, UdpTrackerSection}; +use super::{ + HealthCheckApiSection, HttpApiSection, HttpTrackerSection, TrackerCoreSection, + UdpTrackerSection, +}; use crate::application::command_handlers::create::config::errors::CreateConfigError; -use crate::domain::tracker::{HttpApiConfig, HttpTrackerConfig, TrackerConfig, UdpTrackerConfig}; +use crate::domain::tracker::{ + HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, TrackerConfig, UdpTrackerConfig, +}; /// Tracker configuration section (application DTO) /// @@ -35,6 +40,9 @@ use crate::domain::tracker::{HttpApiConfig, HttpTrackerConfig, TrackerConfig, Ud /// "http_api": { /// "bind_address": "0.0.0.0:1212", /// "admin_token": "MyAccessToken" +/// }, +/// "health_check_api": { +/// "bind_address": "127.0.0.1:1313" /// } /// } /// ``` @@ -48,6 +56,8 @@ pub struct TrackerSection { pub http_trackers: Vec, /// HTTP API configuration pub http_api: HttpApiSection, + /// Health Check API configuration + pub health_check_api: HealthCheckApiSection, } impl TrackerSection { @@ -75,11 +85,15 @@ impl TrackerSection { let http_api: HttpApiConfig = self.http_api.to_http_api_config()?; + let health_check_api: HealthCheckApiConfig = + self.health_check_api.to_health_check_api_config()?; + Ok(TrackerConfig { core, udp_trackers: udp_trackers?, http_trackers: http_trackers?, http_api, + health_check_api, }) } } @@ -113,6 +127,7 @@ impl Default for TrackerSection { bind_address: "0.0.0.0:1212".to_string(), admin_token: "MyAccessToken".to_string(), }, + health_check_api: HealthCheckApiSection::default(), } } } @@ -144,6 +159,7 @@ mod tests { bind_address: "0.0.0.0:1212".to_string(), admin_token: "MyAccessToken".to_string(), }, + health_check_api: HealthCheckApiSection::default(), }; let config = section.to_tracker_config().unwrap(); @@ -192,6 +208,7 @@ mod tests { bind_address: "0.0.0.0:1212".to_string(), admin_token: "MyAccessToken".to_string(), }, + health_check_api: HealthCheckApiSection::default(), }; let config = section.to_tracker_config().unwrap(); @@ -217,6 +234,7 @@ mod tests { bind_address: "0.0.0.0:1212".to_string(), admin_token: "MyAccessToken".to_string(), }, + health_check_api: HealthCheckApiSection::default(), }; let result = section.to_tracker_config(); @@ -247,6 +265,7 @@ mod tests { bind_address: "0.0.0.0:1212".to_string(), admin_token: "MyAccessToken".to_string(), }, + health_check_api: HealthCheckApiSection::default(), }; let json = serde_json::to_string(§ion).unwrap(); @@ -275,6 +294,9 @@ mod tests { "http_api": { "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }, + "health_check_api": { + "bind_address": "127.0.0.1:1313" } }"#; diff --git a/src/domain/environment/mod.rs b/src/domain/environment/mod.rs index f2789bcf..5276947c 100644 --- a/src/domain/environment/mod.rs +++ b/src/domain/environment/mod.rs @@ -127,8 +127,8 @@ pub use user_inputs::UserInputs; // Re-export tracker types for convenience pub use crate::domain::tracker::{ - DatabaseConfig, HttpApiConfig, HttpTrackerConfig, MysqlConfig, SqliteConfig, TrackerConfig, - TrackerCoreConfig, UdpTrackerConfig, + DatabaseConfig, HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, MysqlConfig, + SqliteConfig, TrackerConfig, TrackerCoreConfig, UdpTrackerConfig, }; // Re-export Prometheus types for convenience diff --git a/src/domain/tracker/config.rs b/src/domain/tracker/config.rs index 1a1f93bd..dccd5959 100644 --- a/src/domain/tracker/config.rs +++ b/src/domain/tracker/config.rs @@ -20,7 +20,7 @@ use crate::shared::ApiToken; /// ```rust /// use torrust_tracker_deployer_lib::domain::tracker::{ /// TrackerConfig, TrackerCoreConfig, DatabaseConfig, SqliteConfig, -/// UdpTrackerConfig, HttpTrackerConfig, HttpApiConfig +/// UdpTrackerConfig, HttpTrackerConfig, HttpApiConfig, HealthCheckApiConfig /// }; /// /// let tracker_config = TrackerConfig { @@ -40,6 +40,9 @@ use crate::shared::ApiToken; /// bind_address: "0.0.0.0:1212".parse().unwrap(), /// admin_token: "MyAccessToken".to_string().into(), /// }, +/// health_check_api: HealthCheckApiConfig { +/// bind_address: "127.0.0.1:1313".parse().unwrap(), +/// }, /// }; /// ``` #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -55,6 +58,9 @@ pub struct TrackerConfig { /// HTTP API configuration pub http_api: HttpApiConfig, + + /// Health Check API configuration + pub health_check_api: HealthCheckApiConfig, } /// Core tracker configuration options @@ -103,6 +109,22 @@ pub struct HttpApiConfig { pub admin_token: ApiToken, } +/// Health Check API configuration +/// +/// The Health Check API is a minimal HTTP endpoint used by Docker and container +/// orchestration tools to verify service health. It's separate from the main HTTP API. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct HealthCheckApiConfig { + /// Bind address (e.g., "127.0.0.1:1313") + /// + /// Conventionally uses port 1313, though this is configurable + #[serde( + serialize_with = "serialize_socket_addr", + deserialize_with = "deserialize_socket_addr" + )] + pub bind_address: SocketAddr, +} + impl Default for TrackerConfig { /// Returns a default tracker configuration suitable for development and testing /// @@ -132,6 +154,9 @@ impl Default for TrackerConfig { bind_address: "0.0.0.0:1212".parse().expect("valid address"), admin_token: "MyAccessToken".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().expect("valid address"), + }, } } } @@ -174,6 +199,9 @@ mod tests { bind_address: "0.0.0.0:1212".parse().unwrap(), admin_token: "test_token".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, }; assert_eq!(config.core.database.database_name(), "tracker.db"); @@ -197,6 +225,9 @@ mod tests { bind_address: "0.0.0.0:1212".parse().unwrap(), admin_token: "token123".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, }; let json = serde_json::to_value(&config).unwrap(); diff --git a/src/domain/tracker/mod.rs b/src/domain/tracker/mod.rs index 014718c9..9745793f 100644 --- a/src/domain/tracker/mod.rs +++ b/src/domain/tracker/mod.rs @@ -19,7 +19,7 @@ //! ```rust //! use torrust_tracker_deployer_lib::domain::tracker::{ //! TrackerConfig, TrackerCoreConfig, DatabaseConfig, SqliteConfig, -//! UdpTrackerConfig, HttpTrackerConfig, HttpApiConfig +//! UdpTrackerConfig, HttpTrackerConfig, HttpApiConfig, HealthCheckApiConfig //! }; //! //! let config = TrackerConfig { @@ -39,6 +39,9 @@ //! bind_address: "0.0.0.0:1212".parse().unwrap(), //! admin_token: "MyToken".to_string().into(), //! }, +//! health_check_api: HealthCheckApiConfig { +//! bind_address: "127.0.0.1:1313".parse().unwrap(), +//! }, //! }; //! ``` @@ -46,6 +49,7 @@ mod config; mod database; pub use config::{ - HttpApiConfig, HttpTrackerConfig, TrackerConfig, TrackerCoreConfig, UdpTrackerConfig, + HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, TrackerConfig, TrackerCoreConfig, + UdpTrackerConfig, }; pub use database::{DatabaseConfig, MysqlConfig, SqliteConfig}; diff --git a/src/domain/tracker/protocol.rs b/src/domain/tracker/protocol.rs new file mode 100644 index 00000000..c9a862ea --- /dev/null +++ b/src/domain/tracker/protocol.rs @@ -0,0 +1,177 @@ +//! Network protocol types for tracker services +//! +//! This module defines the protocol types used by tracker services +//! to distinguish between UDP and TCP based services. + +use std::fmt; +use std::str::FromStr; + +/// Network protocol used by tracker services +/// +/// Distinguishes between UDP and TCP protocols for socket binding validation. +/// UDP and TCP maintain separate port spaces in the operating system, allowing +/// the same port number to be used by both protocols simultaneously. +/// +/// # Examples +/// +/// ```rust +/// use torrust_tracker_deployer_lib::domain::tracker::Protocol; +/// +/// let udp = Protocol::Udp; +/// let tcp = Protocol::Tcp; +/// +/// assert_eq!(udp.to_string(), "UDP"); +/// assert_eq!(tcp.to_string(), "TCP"); +/// ``` +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum Protocol { + /// User Datagram Protocol - connectionless protocol + Udp, + /// Transmission Control Protocol - connection-oriented protocol + Tcp, +} + +/// Error type for protocol parsing failures +#[derive(Debug, Clone, PartialEq)] +pub enum ProtocolParseError { + /// Unknown protocol string provided + UnknownProtocol(String), +} + +impl fmt::Display for Protocol { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Udp => write!(f, "UDP"), + Self::Tcp => write!(f, "TCP"), + } + } +} + +impl FromStr for Protocol { + type Err = ProtocolParseError; + + fn from_str(s: &str) -> Result { + match s.to_uppercase().as_str() { + "UDP" => Ok(Self::Udp), + "TCP" => Ok(Self::Tcp), + _ => Err(ProtocolParseError::UnknownProtocol(s.to_string())), + } + } +} + +impl fmt::Display for ProtocolParseError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::UnknownProtocol(proto) => { + write!(f, "Unknown protocol: '{proto}'. Expected 'UDP' or 'TCP'") + } + } + } +} + +impl std::error::Error for ProtocolParseError {} + +#[cfg(test)] +mod tests { + use super::*; + + mod protocol_enum { + use super::*; + + #[test] + fn it_should_display_udp_as_uppercase_string() { + assert_eq!(Protocol::Udp.to_string(), "UDP"); + } + + #[test] + fn it_should_display_tcp_as_uppercase_string() { + assert_eq!(Protocol::Tcp.to_string(), "TCP"); + } + + #[test] + fn it_should_parse_udp_from_uppercase_string() { + assert_eq!("UDP".parse::().unwrap(), Protocol::Udp); + } + + #[test] + fn it_should_parse_udp_from_lowercase_string() { + assert_eq!("udp".parse::().unwrap(), Protocol::Udp); + } + + #[test] + fn it_should_parse_udp_from_mixed_case_string() { + assert_eq!("Udp".parse::().unwrap(), Protocol::Udp); + } + + #[test] + fn it_should_parse_tcp_from_uppercase_string() { + assert_eq!("TCP".parse::().unwrap(), Protocol::Tcp); + } + + #[test] + fn it_should_parse_tcp_from_lowercase_string() { + assert_eq!("tcp".parse::().unwrap(), Protocol::Tcp); + } + + #[test] + fn it_should_parse_tcp_from_mixed_case_string() { + assert_eq!("Tcp".parse::().unwrap(), Protocol::Tcp); + } + + #[test] + fn it_should_return_error_when_parsing_unknown_protocol() { + let result = "HTTP".parse::(); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + ProtocolParseError::UnknownProtocol("HTTP".to_string()) + ); + } + + #[test] + fn it_should_return_error_when_parsing_empty_string() { + let result = "".parse::(); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + ProtocolParseError::UnknownProtocol(String::new()) + ); + } + + #[test] + fn it_should_be_equal_when_same_protocol() { + assert_eq!(Protocol::Udp, Protocol::Udp); + assert_eq!(Protocol::Tcp, Protocol::Tcp); + } + + #[test] + fn it_should_not_be_equal_when_different_protocols() { + assert_ne!(Protocol::Udp, Protocol::Tcp); + } + + #[test] + fn it_should_be_hashable() { + use std::collections::HashSet; + + let mut set = HashSet::new(); + set.insert(Protocol::Udp); + set.insert(Protocol::Tcp); + set.insert(Protocol::Udp); // Duplicate + + assert_eq!(set.len(), 2); // Only two unique protocols + } + } + + mod protocol_parse_error { + use super::*; + + #[test] + fn it_should_display_helpful_error_message_for_unknown_protocol() { + let error = ProtocolParseError::UnknownProtocol("HTTP".to_string()); + assert_eq!( + error.to_string(), + "Unknown protocol: 'HTTP'. Expected 'UDP' or 'TCP'" + ); + } + } +} diff --git a/src/infrastructure/templating/ansible/template/wrappers/variables/context.rs b/src/infrastructure/templating/ansible/template/wrappers/variables/context.rs index 81a4f987..bc30cb00 100644 --- a/src/infrastructure/templating/ansible/template/wrappers/variables/context.rs +++ b/src/infrastructure/templating/ansible/template/wrappers/variables/context.rs @@ -184,8 +184,8 @@ mod tests { #[test] fn it_should_extract_tracker_ports_from_config() { use crate::domain::tracker::{ - DatabaseConfig, HttpApiConfig, HttpTrackerConfig, SqliteConfig, TrackerCoreConfig, - UdpTrackerConfig, + DatabaseConfig, HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, SqliteConfig, + TrackerCoreConfig, UdpTrackerConfig, }; let tracker_config = TrackerConfig { @@ -210,6 +210,9 @@ mod tests { bind_address: "0.0.0.0:1212".parse().unwrap(), admin_token: "MyAccessToken".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, }; let context = AnsibleVariablesContext::new(22, Some(&tracker_config), None).unwrap(); @@ -222,7 +225,7 @@ mod tests { #[test] fn it_should_handle_empty_tracker_lists() { use crate::domain::tracker::{ - DatabaseConfig, HttpApiConfig, SqliteConfig, TrackerCoreConfig, + DatabaseConfig, HealthCheckApiConfig, HttpApiConfig, SqliteConfig, TrackerCoreConfig, }; let tracker_config = TrackerConfig { @@ -238,6 +241,9 @@ mod tests { bind_address: "0.0.0.0:1212".parse().unwrap(), admin_token: "Token123".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, }; let context = AnsibleVariablesContext::new(22, Some(&tracker_config), None).unwrap(); @@ -250,8 +256,8 @@ mod tests { #[test] fn it_should_skip_invalid_bind_addresses() { use crate::domain::tracker::{ - DatabaseConfig, HttpApiConfig, HttpTrackerConfig, SqliteConfig, TrackerCoreConfig, - UdpTrackerConfig, + DatabaseConfig, HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, SqliteConfig, + TrackerCoreConfig, UdpTrackerConfig, }; let tracker_config = TrackerConfig { @@ -276,6 +282,9 @@ mod tests { bind_address: "0.0.0.0:1212".parse().unwrap(), admin_token: "Token".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, }; let context = AnsibleVariablesContext::new(22, Some(&tracker_config), None).unwrap(); diff --git a/src/infrastructure/templating/tracker/template/renderer/project_generator.rs b/src/infrastructure/templating/tracker/template/renderer/project_generator.rs index 988c86b3..de9cdee5 100644 --- a/src/infrastructure/templating/tracker/template/renderer/project_generator.rs +++ b/src/infrastructure/templating/tracker/template/renderer/project_generator.rs @@ -202,8 +202,8 @@ mod tests { #[test] fn it_should_render_tracker_toml_with_sqlite_database_path() { use crate::domain::environment::{ - DatabaseConfig, HttpApiConfig, HttpTrackerConfig, SqliteConfig, TrackerConfig, - TrackerCoreConfig, UdpTrackerConfig, + DatabaseConfig, HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, SqliteConfig, + TrackerConfig, TrackerCoreConfig, UdpTrackerConfig, }; let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); @@ -229,6 +229,9 @@ mod tests { bind_address: "0.0.0.0:1212".parse().unwrap(), admin_token: "test_token".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, }; generator @@ -245,8 +248,8 @@ mod tests { #[test] fn it_should_render_tracker_toml_with_mysql_connection_string() { use crate::domain::environment::{ - DatabaseConfig, HttpApiConfig, HttpTrackerConfig, MysqlConfig, TrackerConfig, - TrackerCoreConfig, UdpTrackerConfig, + DatabaseConfig, HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, MysqlConfig, + TrackerConfig, TrackerCoreConfig, UdpTrackerConfig, }; let temp_dir = tempfile::tempdir().expect("Failed to create temp dir"); @@ -276,6 +279,9 @@ mod tests { bind_address: "0.0.0.0:1212".parse().unwrap(), admin_token: "test_token".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, }; generator diff --git a/src/infrastructure/templating/tracker/template/wrapper/tracker_config/context.rs b/src/infrastructure/templating/tracker/template/wrapper/tracker_config/context.rs index c8ec6731..f143adab 100644 --- a/src/infrastructure/templating/tracker/template/wrapper/tracker_config/context.rs +++ b/src/infrastructure/templating/tracker/template/wrapper/tracker_config/context.rs @@ -24,7 +24,7 @@ use crate::domain::environment::TrackerConfig; /// /// ```rust /// use torrust_tracker_deployer_lib::infrastructure::templating::tracker::TrackerContext; -/// use torrust_tracker_deployer_lib::domain::environment::{TrackerConfig, TrackerCoreConfig, DatabaseConfig, SqliteConfig, UdpTrackerConfig, HttpTrackerConfig, HttpApiConfig}; +/// use torrust_tracker_deployer_lib::domain::environment::{TrackerConfig, TrackerCoreConfig, DatabaseConfig, SqliteConfig, UdpTrackerConfig, HttpTrackerConfig, HttpApiConfig, HealthCheckApiConfig}; /// /// let tracker_config = TrackerConfig { /// core: TrackerCoreConfig { @@ -44,6 +44,9 @@ use crate::domain::environment::TrackerConfig; /// bind_address: "0.0.0.0:1212".parse().unwrap(), /// admin_token: "MyToken".to_string().into(), /// }, +/// health_check_api: HealthCheckApiConfig { +/// bind_address: "127.0.0.1:1313".parse().unwrap(), +/// }, /// }; /// let context = TrackerContext::from_config(&tracker_config); /// ``` @@ -194,8 +197,8 @@ impl Default for TrackerContext { mod tests { use super::*; use crate::domain::environment::{ - DatabaseConfig, HttpApiConfig, HttpTrackerConfig, MysqlConfig, SqliteConfig, TrackerConfig, - TrackerCoreConfig, UdpTrackerConfig, + DatabaseConfig, HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, MysqlConfig, + SqliteConfig, TrackerConfig, TrackerCoreConfig, UdpTrackerConfig, }; use crate::shared::Password; @@ -222,6 +225,9 @@ mod tests { bind_address: "0.0.0.0:1212".parse().unwrap(), admin_token: "test_admin_token".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, } } @@ -268,6 +274,9 @@ mod tests { bind_address: "0.0.0.0:1212".parse().unwrap(), admin_token: "test_token".to_string().into(), }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, }; let context = TrackerContext::from_config(&config); diff --git a/src/presentation/controllers/create/subcommands/environment/config_loader.rs b/src/presentation/controllers/create/subcommands/environment/config_loader.rs index cedc1c16..0adcad1a 100644 --- a/src/presentation/controllers/create/subcommands/environment/config_loader.rs +++ b/src/presentation/controllers/create/subcommands/environment/config_loader.rs @@ -150,6 +150,9 @@ mod tests { "http_api": {{ "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }}, + "health_check_api": {{ + "bind_address": "127.0.0.1:1313" }} }} }}"# @@ -262,6 +265,9 @@ mod tests { "http_api": {{ "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }}, + "health_check_api": {{ + "bind_address": "127.0.0.1:1313" }} }} }}"# @@ -319,6 +325,9 @@ mod tests { "http_api": { "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }, + "health_check_api": { + "bind_address": "127.0.0.1:1313" } } }"#; @@ -381,6 +390,9 @@ mod tests { "http_api": {{ "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }}, + "health_check_api": {{ + "bind_address": "127.0.0.1:1313" }} }} }}"# diff --git a/src/presentation/controllers/create/subcommands/environment/tests.rs b/src/presentation/controllers/create/subcommands/environment/tests.rs index 81894488..f3c7cc6e 100644 --- a/src/presentation/controllers/create/subcommands/environment/tests.rs +++ b/src/presentation/controllers/create/subcommands/environment/tests.rs @@ -64,6 +64,9 @@ async fn it_should_create_environment_from_valid_config() { "http_api": {{ "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }}, + "health_check_api": {{ + "bind_address": "127.0.0.1:1313" }} }} }}"# @@ -185,6 +188,9 @@ async fn it_should_return_error_for_duplicate_environment() { "http_api": {{ "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }}, + "health_check_api": {{ + "bind_address": "127.0.0.1:1313" }} }} }}"# @@ -266,6 +272,9 @@ async fn it_should_create_environment_in_custom_working_dir() { "http_api": {{ "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }}, + "health_check_api": {{ + "bind_address": "127.0.0.1:1313" }} }} }}"# diff --git a/src/presentation/controllers/tests/mod.rs b/src/presentation/controllers/tests/mod.rs index 321740c0..8003c324 100644 --- a/src/presentation/controllers/tests/mod.rs +++ b/src/presentation/controllers/tests/mod.rs @@ -185,6 +185,9 @@ pub fn create_valid_config(path: &Path, env_name: &str) -> PathBuf { "http_api": {{ "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }}, + "health_check_api": {{ + "bind_address": "127.0.0.1:1313" }} }} }}"# @@ -301,6 +304,9 @@ pub fn create_config_with_invalid_name(path: &Path) -> PathBuf { "http_api": {{ "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }}, + "health_check_api": {{ + "bind_address": "127.0.0.1:1313" }} }} }}"# @@ -374,6 +380,9 @@ pub fn create_config_with_missing_keys(path: &Path) -> PathBuf { "http_api": { "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }, + "health_check_api": { + "bind_address": "127.0.0.1:1313" } } }"#; diff --git a/src/testing/e2e/containers/tracker_ports.rs b/src/testing/e2e/containers/tracker_ports.rs index 0b649abd..d6af3fcb 100644 --- a/src/testing/e2e/containers/tracker_ports.rs +++ b/src/testing/e2e/containers/tracker_ports.rs @@ -105,6 +105,9 @@ impl E2eConfigEnvironment { "http_api": { "bind_address": format!("0.0.0.0:{}", self.tracker_ports.http_api_port), "admin_token": "MyAccessToken" + }, + "health_check_api": { + "bind_address": "127.0.0.1:1313" } }, "prometheus": { diff --git a/src/testing/e2e/tasks/black_box/generate_config.rs b/src/testing/e2e/tasks/black_box/generate_config.rs index 536700b6..0107e95c 100644 --- a/src/testing/e2e/tasks/black_box/generate_config.rs +++ b/src/testing/e2e/tasks/black_box/generate_config.rs @@ -228,6 +228,9 @@ pub fn create_test_environment_config(environment_name: &str) -> String { "http_api": { "bind_address": "0.0.0.0:1212", "admin_token": "MyAccessToken" + }, + "health_check_api": { + "bind_address": "127.0.0.1:1313" } }, "prometheus": { From eca6dffa10c3afe008bec9a5f4ddb1327d34da8b Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 29 Dec 2025 18:22:53 +0000 Subject: [PATCH 2/7] refactor: [#255] extract tracker config types into submodules MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Convert config.rs into folder module (config/) - Extract TrackerCoreConfig → config/core.rs (2 tests) - Extract UdpTrackerConfig → config/udp.rs (3 tests) - Extract HttpTrackerConfig → config/http.rs (3 tests) - Extract HttpApiConfig → config/http_api.rs (3 tests) - Extract HealthCheckApiConfig → config/health_check_api.rs (3 tests) - Keep TrackerConfig in config/mod.rs (3 integration tests) - Improve code organization and maintainability - All 1582 tests passing --- src/domain/tracker/config/core.rs | 47 +++++++++++ src/domain/tracker/config/health_check_api.rs | 59 ++++++++++++++ src/domain/tracker/config/http.rs | 54 +++++++++++++ src/domain/tracker/config/http_api.rs | 64 +++++++++++++++ .../tracker/{config.rs => config/mod.rs} | 79 ++++--------------- src/domain/tracker/config/udp.rs | 54 +++++++++++++ 6 files changed, 292 insertions(+), 65 deletions(-) create mode 100644 src/domain/tracker/config/core.rs create mode 100644 src/domain/tracker/config/health_check_api.rs create mode 100644 src/domain/tracker/config/http.rs create mode 100644 src/domain/tracker/config/http_api.rs rename src/domain/tracker/{config.rs => config/mod.rs} (75%) create mode 100644 src/domain/tracker/config/udp.rs diff --git a/src/domain/tracker/config/core.rs b/src/domain/tracker/config/core.rs new file mode 100644 index 00000000..68bcf2c5 --- /dev/null +++ b/src/domain/tracker/config/core.rs @@ -0,0 +1,47 @@ +//! Core tracker configuration + +use serde::{Deserialize, Serialize}; + +use crate::domain::tracker::DatabaseConfig; + +/// Core tracker configuration options +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct TrackerCoreConfig { + /// Database configuration (`SQLite`, `MySQL`, etc.) + pub database: DatabaseConfig, + + /// Tracker mode: true for private tracker, false for public + pub private: bool, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::domain::tracker::{DatabaseConfig, SqliteConfig}; + + #[test] + fn it_should_create_core_config() { + let core = TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "tracker.db".to_string(), + }), + private: true, + }; + + assert_eq!(core.database.database_name(), "tracker.db"); + assert!(core.private); + } + + #[test] + fn it_should_serialize_core_config() { + let core = TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "test.db".to_string(), + }), + private: false, + }; + + let json = serde_json::to_value(&core).unwrap(); + assert_eq!(json["private"], false); + } +} diff --git a/src/domain/tracker/config/health_check_api.rs b/src/domain/tracker/config/health_check_api.rs new file mode 100644 index 00000000..2647e5bb --- /dev/null +++ b/src/domain/tracker/config/health_check_api.rs @@ -0,0 +1,59 @@ +//! Health Check API configuration + +use std::net::SocketAddr; + +use serde::{Deserialize, Serialize}; + +/// Health Check API configuration +/// +/// The Health Check API is a minimal HTTP endpoint used by Docker and container +/// orchestration tools to verify service health. It's separate from the main HTTP API. +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct HealthCheckApiConfig { + /// Bind address (e.g., "127.0.0.1:1313") + /// + /// Conventionally uses port 1313, though this is configurable + #[serde( + serialize_with = "crate::domain::tracker::config::serialize_socket_addr", + deserialize_with = "crate::domain::tracker::config::deserialize_socket_addr" + )] + pub bind_address: SocketAddr, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_create_health_check_api_config() { + let config = HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }; + + assert_eq!( + config.bind_address, + "127.0.0.1:1313".parse::().unwrap() + ); + } + + #[test] + fn it_should_serialize_health_check_api_config() { + let config = HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }; + + let json = serde_json::to_value(&config).unwrap(); + assert_eq!(json["bind_address"], "127.0.0.1:1313"); + } + + #[test] + fn it_should_deserialize_health_check_api_config() { + let json = r#"{"bind_address": "127.0.0.1:1313"}"#; + let config: HealthCheckApiConfig = serde_json::from_str(json).unwrap(); + + assert_eq!( + config.bind_address, + "127.0.0.1:1313".parse::().unwrap() + ); + } +} diff --git a/src/domain/tracker/config/http.rs b/src/domain/tracker/config/http.rs new file mode 100644 index 00000000..2bb1a946 --- /dev/null +++ b/src/domain/tracker/config/http.rs @@ -0,0 +1,54 @@ +//! HTTP tracker configuration + +use std::net::SocketAddr; + +use serde::{Deserialize, Serialize}; + +/// HTTP tracker bind configuration +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct HttpTrackerConfig { + /// Bind address (e.g., "0.0.0.0:7070") + #[serde( + serialize_with = "crate::domain::tracker::config::serialize_socket_addr", + deserialize_with = "crate::domain::tracker::config::deserialize_socket_addr" + )] + pub bind_address: SocketAddr, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_create_http_tracker_config() { + let config = HttpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }; + + assert_eq!( + config.bind_address, + "0.0.0.0:7070".parse::().unwrap() + ); + } + + #[test] + fn it_should_serialize_http_tracker_config() { + let json = serde_json::to_value(&HttpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }) + .unwrap(); + + assert_eq!(json["bind_address"], "0.0.0.0:7070"); + } + + #[test] + fn it_should_deserialize_http_tracker_config() { + let json = r#"{"bind_address": "0.0.0.0:7070"}"#; + let config: HttpTrackerConfig = serde_json::from_str(json).unwrap(); + + assert_eq!( + config.bind_address, + "0.0.0.0:7070".parse::().unwrap() + ); + } +} diff --git a/src/domain/tracker/config/http_api.rs b/src/domain/tracker/config/http_api.rs new file mode 100644 index 00000000..f920ff7a --- /dev/null +++ b/src/domain/tracker/config/http_api.rs @@ -0,0 +1,64 @@ +//! HTTP API configuration + +use std::net::SocketAddr; + +use serde::{Deserialize, Serialize}; + +use crate::shared::ApiToken; + +/// HTTP API configuration +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct HttpApiConfig { + /// Bind address (e.g., "0.0.0.0:1212") + #[serde( + serialize_with = "crate::domain::tracker::config::serialize_socket_addr", + deserialize_with = "crate::domain::tracker::config::deserialize_socket_addr" + )] + pub bind_address: SocketAddr, + + /// Admin access token for HTTP API authentication + pub admin_token: ApiToken, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_create_http_api_config() { + let config = HttpApiConfig { + bind_address: "0.0.0.0:1212".parse().unwrap(), + admin_token: "test_token".to_string().into(), + }; + + assert_eq!( + config.bind_address, + "0.0.0.0:1212".parse::().unwrap() + ); + assert_eq!(config.admin_token.expose_secret(), "test_token"); + } + + #[test] + fn it_should_serialize_http_api_config() { + let config = HttpApiConfig { + bind_address: "0.0.0.0:1212".parse().unwrap(), + admin_token: "token123".to_string().into(), + }; + + let json = serde_json::to_value(&config).unwrap(); + assert_eq!(json["bind_address"], "0.0.0.0:1212"); + assert_eq!(json["admin_token"], "token123"); + } + + #[test] + fn it_should_deserialize_http_api_config() { + let json = r#"{"bind_address": "0.0.0.0:1212", "admin_token": "MyToken"}"#; + let config: HttpApiConfig = serde_json::from_str(json).unwrap(); + + assert_eq!( + config.bind_address, + "0.0.0.0:1212".parse::().unwrap() + ); + assert_eq!(config.admin_token.expose_secret(), "MyToken"); + } +} diff --git a/src/domain/tracker/config.rs b/src/domain/tracker/config/mod.rs similarity index 75% rename from src/domain/tracker/config.rs rename to src/domain/tracker/config/mod.rs index dccd5959..6b642541 100644 --- a/src/domain/tracker/config.rs +++ b/src/domain/tracker/config/mod.rs @@ -7,8 +7,19 @@ use std::net::SocketAddr; use serde::{Deserialize, Serialize}; +mod core; +mod health_check_api; +mod http; +mod http_api; +mod udp; + +pub use core::TrackerCoreConfig; +pub use health_check_api::HealthCheckApiConfig; +pub use http::HttpTrackerConfig; +pub use http_api::HttpApiConfig; +pub use udp::UdpTrackerConfig; + use super::{DatabaseConfig, SqliteConfig}; -use crate::shared::ApiToken; /// Tracker deployment configuration /// @@ -63,68 +74,6 @@ pub struct TrackerConfig { pub health_check_api: HealthCheckApiConfig, } -/// Core tracker configuration options -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct TrackerCoreConfig { - /// Database configuration (`SQLite`, `MySQL`, etc.) - pub database: DatabaseConfig, - - /// Tracker mode: true for private tracker, false for public - pub private: bool, -} - -/// UDP tracker bind configuration -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct UdpTrackerConfig { - /// Bind address (e.g., "0.0.0.0:6868") - #[serde( - serialize_with = "serialize_socket_addr", - deserialize_with = "deserialize_socket_addr" - )] - pub bind_address: SocketAddr, -} - -/// HTTP tracker bind configuration -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct HttpTrackerConfig { - /// Bind address (e.g., "0.0.0.0:7070") - #[serde( - serialize_with = "serialize_socket_addr", - deserialize_with = "deserialize_socket_addr" - )] - pub bind_address: SocketAddr, -} - -/// HTTP API configuration -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct HttpApiConfig { - /// Bind address (e.g., "0.0.0.0:1212") - #[serde( - serialize_with = "serialize_socket_addr", - deserialize_with = "deserialize_socket_addr" - )] - pub bind_address: SocketAddr, - - /// Admin access token for HTTP API authentication - pub admin_token: ApiToken, -} - -/// Health Check API configuration -/// -/// The Health Check API is a minimal HTTP endpoint used by Docker and container -/// orchestration tools to verify service health. It's separate from the main HTTP API. -#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] -pub struct HealthCheckApiConfig { - /// Bind address (e.g., "127.0.0.1:1313") - /// - /// Conventionally uses port 1313, though this is configurable - #[serde( - serialize_with = "serialize_socket_addr", - deserialize_with = "deserialize_socket_addr" - )] - pub bind_address: SocketAddr, -} - impl Default for TrackerConfig { /// Returns a default tracker configuration suitable for development and testing /// @@ -161,14 +110,14 @@ impl Default for TrackerConfig { } } -fn serialize_socket_addr(addr: &SocketAddr, serializer: S) -> Result +pub(crate) fn serialize_socket_addr(addr: &SocketAddr, serializer: S) -> Result where S: serde::Serializer, { serializer.serialize_str(&addr.to_string()) } -fn deserialize_socket_addr<'de, D>(deserializer: D) -> Result +pub(crate) fn deserialize_socket_addr<'de, D>(deserializer: D) -> Result where D: serde::Deserializer<'de>, { diff --git a/src/domain/tracker/config/udp.rs b/src/domain/tracker/config/udp.rs new file mode 100644 index 00000000..a182d464 --- /dev/null +++ b/src/domain/tracker/config/udp.rs @@ -0,0 +1,54 @@ +//! UDP tracker configuration + +use std::net::SocketAddr; + +use serde::{Deserialize, Serialize}; + +/// UDP tracker bind configuration +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +pub struct UdpTrackerConfig { + /// Bind address (e.g., "0.0.0.0:6868") + #[serde( + serialize_with = "crate::domain::tracker::config::serialize_socket_addr", + deserialize_with = "crate::domain::tracker::config::deserialize_socket_addr" + )] + pub bind_address: SocketAddr, +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_create_udp_tracker_config() { + let config = UdpTrackerConfig { + bind_address: "0.0.0.0:6868".parse().unwrap(), + }; + + assert_eq!( + config.bind_address, + "0.0.0.0:6868".parse::().unwrap() + ); + } + + #[test] + fn it_should_serialize_udp_tracker_config() { + let config = UdpTrackerConfig { + bind_address: "0.0.0.0:6969".parse().unwrap(), + }; + + let json = serde_json::to_value(&config).unwrap(); + assert_eq!(json["bind_address"], "0.0.0.0:6969"); + } + + #[test] + fn it_should_deserialize_udp_tracker_config() { + let json = r#"{"bind_address": "0.0.0.0:6969"}"#; + let config: UdpTrackerConfig = serde_json::from_str(json).unwrap(); + + assert_eq!( + config.bind_address, + "0.0.0.0:6969".parse::().unwrap() + ); + } +} From 981547be8f2241bfcd7b866a3d26cbe439bfbe31 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 29 Dec 2025 18:32:49 +0000 Subject: [PATCH 3/7] refactor: [#255] move database module into config/core - Convert core.rs into folder module (config/core/) - Move database module into config/core/database/ - Update imports to reflect new structure - Database types now exported through config module hierarchy - Improves logical grouping (database is part of core config) - All 1582 tests passing - All linters passing --- src/domain/tracker/{ => config/core}/database/mod.rs | 0 src/domain/tracker/{ => config/core}/database/mysql.rs | 0 src/domain/tracker/{ => config/core}/database/sqlite.rs | 0 src/domain/tracker/config/{core.rs => core/mod.rs} | 5 +++-- src/domain/tracker/config/mod.rs | 4 +--- src/domain/tracker/mod.rs | 9 +++------ 6 files changed, 7 insertions(+), 11 deletions(-) rename src/domain/tracker/{ => config/core}/database/mod.rs (100%) rename src/domain/tracker/{ => config/core}/database/mysql.rs (100%) rename src/domain/tracker/{ => config/core}/database/sqlite.rs (100%) rename src/domain/tracker/config/{core.rs => core/mod.rs} (91%) diff --git a/src/domain/tracker/database/mod.rs b/src/domain/tracker/config/core/database/mod.rs similarity index 100% rename from src/domain/tracker/database/mod.rs rename to src/domain/tracker/config/core/database/mod.rs diff --git a/src/domain/tracker/database/mysql.rs b/src/domain/tracker/config/core/database/mysql.rs similarity index 100% rename from src/domain/tracker/database/mysql.rs rename to src/domain/tracker/config/core/database/mysql.rs diff --git a/src/domain/tracker/database/sqlite.rs b/src/domain/tracker/config/core/database/sqlite.rs similarity index 100% rename from src/domain/tracker/database/sqlite.rs rename to src/domain/tracker/config/core/database/sqlite.rs diff --git a/src/domain/tracker/config/core.rs b/src/domain/tracker/config/core/mod.rs similarity index 91% rename from src/domain/tracker/config/core.rs rename to src/domain/tracker/config/core/mod.rs index 68bcf2c5..00ca89ab 100644 --- a/src/domain/tracker/config/core.rs +++ b/src/domain/tracker/config/core/mod.rs @@ -2,7 +2,9 @@ use serde::{Deserialize, Serialize}; -use crate::domain::tracker::DatabaseConfig; +mod database; + +pub use database::{DatabaseConfig, MysqlConfig, SqliteConfig}; /// Core tracker configuration options #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] @@ -17,7 +19,6 @@ pub struct TrackerCoreConfig { #[cfg(test)] mod tests { use super::*; - use crate::domain::tracker::{DatabaseConfig, SqliteConfig}; #[test] fn it_should_create_core_config() { diff --git a/src/domain/tracker/config/mod.rs b/src/domain/tracker/config/mod.rs index 6b642541..27d4ca99 100644 --- a/src/domain/tracker/config/mod.rs +++ b/src/domain/tracker/config/mod.rs @@ -13,14 +13,12 @@ mod http; mod http_api; mod udp; -pub use core::TrackerCoreConfig; +pub use core::{DatabaseConfig, MysqlConfig, SqliteConfig, TrackerCoreConfig}; pub use health_check_api::HealthCheckApiConfig; pub use http::HttpTrackerConfig; pub use http_api::HttpApiConfig; pub use udp::UdpTrackerConfig; -use super::{DatabaseConfig, SqliteConfig}; - /// Tracker deployment configuration /// /// This structure mirrors the real tracker configuration but only includes diff --git a/src/domain/tracker/mod.rs b/src/domain/tracker/mod.rs index 9745793f..3689ef0b 100644 --- a/src/domain/tracker/mod.rs +++ b/src/domain/tracker/mod.rs @@ -5,8 +5,7 @@ //! //! # Module Structure //! -//! - `config` - Main `TrackerConfig` and component configurations -//! - `database` - Database configuration (`SQLite`, `MySQL`) +//! - `config` - Main `TrackerConfig` and component configurations (includes database) //! //! # Layer Separation //! @@ -46,10 +45,8 @@ //! ``` mod config; -mod database; pub use config::{ - HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, TrackerConfig, TrackerCoreConfig, - UdpTrackerConfig, + DatabaseConfig, HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, MysqlConfig, + SqliteConfig, TrackerConfig, TrackerCoreConfig, UdpTrackerConfig, }; -pub use database::{DatabaseConfig, MysqlConfig, SqliteConfig}; From 16ade4a85a47493c675f43a5603c895bf187bf24 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 29 Dec 2025 19:04:40 +0000 Subject: [PATCH 4/7] feat: [#255] add socket address uniqueness validation with tiered help system - Add BindingAddress value object combining SocketAddr with Protocol - Add TrackerConfig::validate() method to detect socket address conflicts - Add TrackerConfigError enum with DuplicateSocketAddress variant - Implement tiered help system (brief Display + detailed .help() method) - Integrate validation at DTO boundary (TrackerSection.to_tracker_config) - Add comprehensive unit tests for validation logic - Add integration tests for DTO validation Follows error handling conventions from docs/contributing/error-handling.md --- .../command_handlers/create/config/errors.rs | 26 + .../create/config/tracker/tracker_section.rs | 66 ++- src/domain/tracker/binding_address.rs | 143 +++++ src/domain/tracker/config/mod.rs | 493 ++++++++++++++++++ src/domain/tracker/mod.rs | 8 +- 5 files changed, 733 insertions(+), 3 deletions(-) create mode 100644 src/domain/tracker/binding_address.rs diff --git a/src/application/command_handlers/create/config/errors.rs b/src/application/command_handlers/create/config/errors.rs index b2ed885d..c3918032 100644 --- a/src/application/command_handlers/create/config/errors.rs +++ b/src/application/command_handlers/create/config/errors.rs @@ -7,6 +7,7 @@ use std::path::PathBuf; use thiserror::Error; +use crate::domain::tracker::TrackerConfigError; use crate::domain::EnvironmentNameError; use crate::domain::ProfileNameError; use crate::shared::UsernameError; @@ -105,6 +106,10 @@ pub enum CreateConfigError { /// Invalid Prometheus configuration #[error("Invalid Prometheus configuration: {0}")] InvalidPrometheusConfig(String), + + /// Tracker configuration validation failed + #[error("Tracker configuration validation failed: {0}")] + TrackerConfigValidation(#[from] TrackerConfigError), } impl CreateConfigError { @@ -424,6 +429,27 @@ impl CreateConfigError { Note: The template automatically adds the 's' suffix (e.g., 15 becomes '15s'),\n\ so you only need to specify the numeric value." } + Self::TrackerConfigValidation(_) => { + "Tracker configuration validation failed.\n\ + \n\ + This error indicates a problem with the tracker service configuration,\n\ + typically related to socket address (IP:Port:Protocol) conflicts.\n\ + \n\ + The error message above provides specific details about:\n\ + - Which services are in conflict\n\ + - The conflicting socket addresses\n\ + - Why the configuration is invalid\n\ + \n\ + Common issues:\n\ + 1. Multiple services on same TCP port (HTTP tracker + API)\n\ + 2. Duplicate UDP tracker ports\n\ + 3. Duplicate HTTP tracker ports\n\ + \n\ + Note: UDP and TCP can share the same port (different protocols),\n\ + but this is not recommended for clarity.\n\ + \n\ + Related: docs/external-issues/tracker/udp-tcp-port-sharing-allowed.md" + } } } } diff --git a/src/application/command_handlers/create/config/tracker/tracker_section.rs b/src/application/command_handlers/create/config/tracker/tracker_section.rs index 5563c5b0..76ae797f 100644 --- a/src/application/command_handlers/create/config/tracker/tracker_section.rs +++ b/src/application/command_handlers/create/config/tracker/tracker_section.rs @@ -68,6 +68,7 @@ impl TrackerSection { /// Returns error if any of the nested sections fail validation: /// - Invalid bind address formats /// - Invalid database configuration + /// - Socket address conflicts (multiple services on same IP:Port:Protocol) pub fn to_tracker_config(&self) -> Result { let core = self.core.to_tracker_core_config()?; @@ -88,13 +89,18 @@ impl TrackerSection { let health_check_api: HealthCheckApiConfig = self.health_check_api.to_health_check_api_config()?; - Ok(TrackerConfig { + let config = TrackerConfig { core, udp_trackers: udp_trackers?, http_trackers: http_trackers?, http_api, health_check_api, - }) + }; + + // Validate socket address uniqueness + config.validate().map_err(CreateConfigError::from)?; + + Ok(config) } } @@ -306,4 +312,60 @@ mod tests { assert_eq!(section.udp_trackers.len(), 1); assert_eq!(section.http_trackers.len(), 1); } + + #[test] + fn it_should_reject_configuration_with_duplicate_socket_addresses() { + // HTTP tracker and API on same port (TCP protocol conflict) + let section = TrackerSection { + core: TrackerCoreSection { + database: DatabaseSection::Sqlite { + database_name: "tracker.db".to_string(), + }, + private: false, + }, + udp_trackers: vec![], + http_trackers: vec![HttpTrackerSection { + bind_address: "0.0.0.0:7070".to_string(), + }], + http_api: HttpApiSection { + bind_address: "0.0.0.0:7070".to_string(), + admin_token: "token".to_string(), + }, + health_check_api: HealthCheckApiSection::default(), + }; + + let result = section.to_tracker_config(); + assert!(result.is_err()); + assert!(matches!( + result.unwrap_err(), + CreateConfigError::TrackerConfigValidation(_) + )); + } + + #[test] + fn it_should_accept_udp_and_tcp_on_same_port() { + // UDP and TCP can share the same port (different protocol spaces) + let section = TrackerSection { + core: TrackerCoreSection { + database: DatabaseSection::Sqlite { + database_name: "tracker.db".to_string(), + }, + private: false, + }, + udp_trackers: vec![UdpTrackerSection { + bind_address: "0.0.0.0:7070".to_string(), + }], + http_trackers: vec![HttpTrackerSection { + bind_address: "0.0.0.0:7070".to_string(), + }], + http_api: HttpApiSection { + bind_address: "0.0.0.0:1212".to_string(), + admin_token: "token".to_string(), + }, + health_check_api: HealthCheckApiSection::default(), + }; + + let result = section.to_tracker_config(); + assert!(result.is_ok()); + } } diff --git a/src/domain/tracker/binding_address.rs b/src/domain/tracker/binding_address.rs new file mode 100644 index 00000000..99702e90 --- /dev/null +++ b/src/domain/tracker/binding_address.rs @@ -0,0 +1,143 @@ +//! Binding address value object for tracker services +//! +//! This module provides a type-safe representation of socket addresses +//! with protocol information, used for validating tracker configurations. + +use std::fmt; +use std::net::SocketAddr; + +use super::Protocol; + +/// A binding address combining socket address and protocol +/// +/// Represents a complete socket binding specification including both the +/// network address (IP + port) and the protocol (UDP or TCP). This ensures +/// that socket address uniqueness validation accounts for protocol differences. +/// +/// # Examples +/// +/// ```rust +/// use torrust_tracker_deployer_lib::domain::tracker::{BindingAddress, Protocol}; +/// +/// let udp_addr = BindingAddress::new( +/// "0.0.0.0:6969".parse().unwrap(), +/// Protocol::Udp +/// ); +/// +/// let tcp_addr = BindingAddress::new( +/// "0.0.0.0:7070".parse().unwrap(), +/// Protocol::Tcp +/// ); +/// +/// assert_eq!(udp_addr.socket().port(), 6969); +/// assert_eq!(tcp_addr.protocol(), Protocol::Tcp); +/// ``` +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub struct BindingAddress { + socket: SocketAddr, + protocol: Protocol, +} + +impl BindingAddress { + /// Creates a new binding address + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::domain::tracker::{BindingAddress, Protocol}; + /// + /// let addr = BindingAddress::new( + /// "0.0.0.0:6969".parse().unwrap(), + /// Protocol::Udp + /// ); + /// ``` + #[must_use] + pub fn new(socket: SocketAddr, protocol: Protocol) -> Self { + Self { socket, protocol } + } + + /// Returns the socket address + #[must_use] + pub fn socket(&self) -> &SocketAddr { + &self.socket + } + + /// Returns the protocol + #[must_use] + pub fn protocol(&self) -> Protocol { + self.protocol + } +} + +impl fmt::Display for BindingAddress { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{} ({})", self.socket, self.protocol) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn it_should_create_binding_address_with_udp_protocol() { + let socket: SocketAddr = "0.0.0.0:6969".parse().unwrap(); + let addr = BindingAddress::new(socket, Protocol::Udp); + + assert_eq!(addr.socket(), &socket); + assert_eq!(addr.protocol(), Protocol::Udp); + } + + #[test] + fn it_should_create_binding_address_with_tcp_protocol() { + let socket: SocketAddr = "127.0.0.1:7070".parse().unwrap(); + let addr = BindingAddress::new(socket, Protocol::Tcp); + + assert_eq!(addr.socket(), &socket); + assert_eq!(addr.protocol(), Protocol::Tcp); + } + + #[test] + fn it_should_display_binding_address_with_protocol() { + let addr = BindingAddress::new("0.0.0.0:6969".parse().unwrap(), Protocol::Udp); + assert_eq!(addr.to_string(), "0.0.0.0:6969 (UDP)"); + + let addr = BindingAddress::new("127.0.0.1:7070".parse().unwrap(), Protocol::Tcp); + assert_eq!(addr.to_string(), "127.0.0.1:7070 (TCP)"); + } + + #[test] + fn it_should_consider_same_socket_different_protocol_as_different() { + let udp_addr = BindingAddress::new("0.0.0.0:7070".parse().unwrap(), Protocol::Udp); + let tcp_addr = BindingAddress::new("0.0.0.0:7070".parse().unwrap(), Protocol::Tcp); + + assert_ne!(udp_addr, tcp_addr); + } + + #[test] + fn it_should_consider_same_socket_same_protocol_as_equal() { + let addr1 = BindingAddress::new("0.0.0.0:7070".parse().unwrap(), Protocol::Tcp); + let addr2 = BindingAddress::new("0.0.0.0:7070".parse().unwrap(), Protocol::Tcp); + + assert_eq!(addr1, addr2); + } + + #[test] + fn it_should_consider_different_ips_same_port_as_different() { + let addr1 = BindingAddress::new("192.168.1.10:7070".parse().unwrap(), Protocol::Tcp); + let addr2 = BindingAddress::new("192.168.1.20:7070".parse().unwrap(), Protocol::Tcp); + + assert_ne!(addr1, addr2); + } + + #[test] + fn it_should_be_usable_as_hash_map_key() { + use std::collections::HashMap; + + let mut map = HashMap::new(); + let addr = BindingAddress::new("0.0.0.0:6969".parse().unwrap(), Protocol::Udp); + map.insert(addr, "UDP Tracker"); + + assert_eq!(map.get(&addr), Some(&"UDP Tracker")); + } +} diff --git a/src/domain/tracker/config/mod.rs b/src/domain/tracker/config/mod.rs index 27d4ca99..76975e2c 100644 --- a/src/domain/tracker/config/mod.rs +++ b/src/domain/tracker/config/mod.rs @@ -3,10 +3,14 @@ //! This module contains the main tracker configuration and component types //! used for deploying the Torrust Tracker. +use std::collections::HashMap; +use std::fmt; use std::net::SocketAddr; use serde::{Deserialize, Serialize}; +use super::{BindingAddress, Protocol}; + mod core; mod health_check_api; mod http; @@ -72,6 +76,190 @@ pub struct TrackerConfig { pub health_check_api: HealthCheckApiConfig, } +/// Error type for tracker configuration validation failures +#[derive(Debug, Clone, PartialEq)] +pub enum TrackerConfigError { + /// Multiple services attempting to bind to the same socket address + DuplicateSocketAddress { + /// The conflicting socket address + address: SocketAddr, + /// The protocol (UDP or TCP) + protocol: Protocol, + /// Names of services attempting to bind to this address + services: Vec, + }, +} + +impl fmt::Display for TrackerConfigError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::DuplicateSocketAddress { + address, + protocol, + services, + } => { + let services_list = services + .iter() + .map(|s| format!("'{s}'")) + .collect::>() + .join(", "); + write!( + f, + "Socket address conflict: {services_list} cannot bind to {address} ({protocol})\n\ + Tip: Assign different port numbers to each service" + ) + } + } + } +} + +impl std::error::Error for TrackerConfigError {} + +impl TrackerConfigError { + /// Get detailed troubleshooting guidance for this error + /// + /// This method provides comprehensive troubleshooting steps that can be + /// displayed to users when they need more help resolving the error. + #[must_use] + pub fn help(&self) -> String { + match self { + Self::DuplicateSocketAddress { + address, + protocol, + services, + } => { + use std::fmt::Write; + + let mut help = + String::from("Socket Address Conflict - Detailed Troubleshooting:\n\n"); + + help.push_str("Conflicting services:\n"); + for service in services { + let _ = writeln!(help, " - {service}: {address} ({protocol})"); + } + help.push('\n'); + + help.push_str("Why this fails:\n"); + let _ = write!( + help, + "Two services using the same protocol ({protocol}) cannot bind to the same\n\ + IP address and port number. The second service will fail with\n\ + \"Address already in use\" error.\n\n" + ); + + help.push_str("How to fix:\n"); + help.push_str( + "1. Assign different port numbers to each service\n\ + 2. Or configure only one service to use this address\n\n", + ); + + help.push_str("Note:\n"); + help.push_str( + "Services using different protocols (UDP vs TCP) CAN share the same port.\n\ + See: docs/external-issues/tracker/udp-tcp-port-sharing-allowed.md\n", + ); + + help + } + } + } +} + +impl TrackerConfig { + /// Validates the tracker configuration for socket address conflicts + /// + /// Checks that no two services using the same protocol attempt to bind + /// to the same socket address (IP + port). Services using different + /// protocols (UDP vs TCP) can share the same port number. + /// + /// # Errors + /// + /// Returns `TrackerConfigError::DuplicateSocketAddress` if multiple services + /// using the same protocol attempt to bind to the same socket address. + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::domain::tracker::{ + /// TrackerConfig, TrackerCoreConfig, DatabaseConfig, SqliteConfig, + /// UdpTrackerConfig, HttpTrackerConfig, HttpApiConfig, HealthCheckApiConfig + /// }; + /// + /// let config = TrackerConfig { + /// core: TrackerCoreConfig { + /// database: DatabaseConfig::Sqlite(SqliteConfig { + /// database_name: "tracker.db".to_string(), + /// }), + /// private: false, + /// }, + /// udp_trackers: vec![ + /// UdpTrackerConfig { bind_address: "0.0.0.0:6969".parse().unwrap() }, + /// ], + /// http_trackers: vec![ + /// HttpTrackerConfig { bind_address: "0.0.0.0:7070".parse().unwrap() }, + /// ], + /// http_api: HttpApiConfig { + /// bind_address: "0.0.0.0:1212".parse().unwrap(), + /// admin_token: "MyAccessToken".to_string().into(), + /// }, + /// health_check_api: HealthCheckApiConfig { + /// bind_address: "127.0.0.1:1313".parse().unwrap(), + /// }, + /// }; + /// + /// assert!(config.validate().is_ok()); + /// ``` + pub fn validate(&self) -> Result<(), TrackerConfigError> { + // Collect all binding addresses with their service names + let mut bindings: HashMap> = HashMap::new(); + + // Add UDP trackers + for (i, udp) in self.udp_trackers.iter().enumerate() { + let binding = BindingAddress::new(udp.bind_address, Protocol::Udp); + bindings + .entry(binding) + .or_default() + .push(format!("UDP Tracker #{}", i + 1)); + } + + // Add HTTP trackers + for (i, http) in self.http_trackers.iter().enumerate() { + let binding = BindingAddress::new(http.bind_address, Protocol::Tcp); + bindings + .entry(binding) + .or_default() + .push(format!("HTTP Tracker #{}", i + 1)); + } + + // Add HTTP API + let api_binding = BindingAddress::new(self.http_api.bind_address, Protocol::Tcp); + bindings + .entry(api_binding) + .or_default() + .push("HTTP API".to_string()); + + // Add Health Check API + let health_binding = BindingAddress::new(self.health_check_api.bind_address, Protocol::Tcp); + bindings + .entry(health_binding) + .or_default() + .push("Health Check API".to_string()); + + // Check for duplicates + for (binding, services) in bindings { + if services.len() > 1 { + return Err(TrackerConfigError::DuplicateSocketAddress { + address: *binding.socket(), + protocol: binding.protocol(), + services, + }); + } + } + + Ok(()) + } +} + impl Default for TrackerConfig { /// Returns a default tracker configuration suitable for development and testing /// @@ -214,4 +402,309 @@ mod tests { ); assert_eq!(config.http_api.admin_token.expose_secret(), "MyAccessToken"); } + + mod validation { + use super::*; + + #[test] + fn it_should_accept_valid_configuration_with_unique_addresses() { + let config = TrackerConfig { + core: TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "tracker.db".to_string(), + }), + private: false, + }, + udp_trackers: vec![UdpTrackerConfig { + bind_address: "0.0.0.0:6969".parse().unwrap(), + }], + http_trackers: vec![HttpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }], + http_api: HttpApiConfig { + bind_address: "0.0.0.0:1212".parse().unwrap(), + admin_token: "token".to_string().into(), + }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, + }; + + assert!(config.validate().is_ok()); + } + + #[test] + fn it_should_reject_duplicate_udp_tracker_ports() { + let config = TrackerConfig { + core: TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "tracker.db".to_string(), + }), + private: false, + }, + udp_trackers: vec![ + UdpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }, + UdpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }, + ], + http_trackers: vec![], + http_api: HttpApiConfig { + bind_address: "0.0.0.0:1212".parse().unwrap(), + admin_token: "token".to_string().into(), + }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, + }; + + let result = config.validate(); + assert!(result.is_err()); + + if let Err(TrackerConfigError::DuplicateSocketAddress { + address, + protocol, + services, + }) = result + { + assert_eq!(address, "0.0.0.0:7070".parse::().unwrap()); + assert_eq!(protocol, Protocol::Udp); + assert_eq!(services.len(), 2); + assert!(services.contains(&"UDP Tracker #1".to_string())); + assert!(services.contains(&"UDP Tracker #2".to_string())); + } else { + panic!("Expected DuplicateSocketAddress error"); + } + } + + #[test] + fn it_should_reject_duplicate_http_tracker_ports() { + let config = TrackerConfig { + core: TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "tracker.db".to_string(), + }), + private: false, + }, + udp_trackers: vec![], + http_trackers: vec![ + HttpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }, + HttpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }, + ], + http_api: HttpApiConfig { + bind_address: "0.0.0.0:1212".parse().unwrap(), + admin_token: "token".to_string().into(), + }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, + }; + + let result = config.validate(); + assert!(result.is_err()); + + if let Err(TrackerConfigError::DuplicateSocketAddress { + address, + protocol, + services, + }) = result + { + assert_eq!(address, "0.0.0.0:7070".parse::().unwrap()); + assert_eq!(protocol, Protocol::Tcp); + assert_eq!(services.len(), 2); + } else { + panic!("Expected DuplicateSocketAddress error"); + } + } + + #[test] + fn it_should_reject_http_tracker_and_api_conflict() { + let config = TrackerConfig { + core: TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "tracker.db".to_string(), + }), + private: false, + }, + udp_trackers: vec![], + http_trackers: vec![HttpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }], + http_api: HttpApiConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + admin_token: "token".to_string().into(), + }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, + }; + + let result = config.validate(); + assert!(result.is_err()); + + if let Err(TrackerConfigError::DuplicateSocketAddress { + address, + protocol, + services, + }) = result + { + assert_eq!(address, "0.0.0.0:7070".parse::().unwrap()); + assert_eq!(protocol, Protocol::Tcp); + assert_eq!(services.len(), 2); + assert!(services.contains(&"HTTP Tracker #1".to_string())); + assert!(services.contains(&"HTTP API".to_string())); + } else { + panic!("Expected DuplicateSocketAddress error"); + } + } + + #[test] + fn it_should_reject_http_tracker_and_health_check_api_conflict() { + let config = TrackerConfig { + core: TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "tracker.db".to_string(), + }), + private: false, + }, + udp_trackers: vec![], + http_trackers: vec![HttpTrackerConfig { + bind_address: "0.0.0.0:1313".parse().unwrap(), + }], + http_api: HttpApiConfig { + bind_address: "0.0.0.0:1212".parse().unwrap(), + admin_token: "token".to_string().into(), + }, + health_check_api: HealthCheckApiConfig { + bind_address: "0.0.0.0:1313".parse().unwrap(), + }, + }; + + let result = config.validate(); + assert!(result.is_err()); + + if let Err(TrackerConfigError::DuplicateSocketAddress { + address, + protocol, + services, + }) = result + { + assert_eq!(address, "0.0.0.0:1313".parse::().unwrap()); + assert_eq!(protocol, Protocol::Tcp); + assert_eq!(services.len(), 2); + assert!(services.contains(&"HTTP Tracker #1".to_string())); + assert!(services.contains(&"Health Check API".to_string())); + } else { + panic!("Expected DuplicateSocketAddress error"); + } + } + + #[test] + fn it_should_allow_udp_and_http_on_same_port() { + // This is valid because UDP and TCP use separate port spaces + let config = TrackerConfig { + core: TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "tracker.db".to_string(), + }), + private: false, + }, + udp_trackers: vec![UdpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }], + http_trackers: vec![HttpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }], + http_api: HttpApiConfig { + bind_address: "0.0.0.0:1212".parse().unwrap(), + admin_token: "token".to_string().into(), + }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, + }; + + assert!(config.validate().is_ok()); + } + + #[test] + fn it_should_allow_same_port_different_ips() { + let config = TrackerConfig { + core: TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "tracker.db".to_string(), + }), + private: false, + }, + udp_trackers: vec![], + http_trackers: vec![ + HttpTrackerConfig { + bind_address: "192.168.1.10:7070".parse().unwrap(), + }, + HttpTrackerConfig { + bind_address: "192.168.1.20:7070".parse().unwrap(), + }, + ], + http_api: HttpApiConfig { + bind_address: "0.0.0.0:1212".parse().unwrap(), + admin_token: "token".to_string().into(), + }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, + }; + + assert!(config.validate().is_ok()); + } + + #[test] + fn it_should_provide_clear_error_message_with_fix_instructions() { + let config = TrackerConfig { + core: TrackerCoreConfig { + database: DatabaseConfig::Sqlite(SqliteConfig { + database_name: "tracker.db".to_string(), + }), + private: false, + }, + udp_trackers: vec![], + http_trackers: vec![HttpTrackerConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + }], + http_api: HttpApiConfig { + bind_address: "0.0.0.0:7070".parse().unwrap(), + admin_token: "token".to_string().into(), + }, + health_check_api: HealthCheckApiConfig { + bind_address: "127.0.0.1:1313".parse().unwrap(), + }, + }; + + let error = config.validate().unwrap_err(); + let error_message = error.to_string(); + + // Verify brief error message contains essential information + assert!(error_message.contains("Socket address conflict")); + assert!(error_message.contains("'HTTP Tracker #1'")); + assert!(error_message.contains("'HTTP API'")); + assert!(error_message.contains("0.0.0.0:7070")); + assert!(error_message.contains("TCP")); + assert!(error_message.contains("Tip: Assign different port numbers")); + + // Verify detailed help contains comprehensive troubleshooting + let help = error.help(); + assert!(help.contains("Socket Address Conflict - Detailed Troubleshooting")); + assert!(help.contains("Conflicting services:")); + assert!(help.contains("HTTP Tracker #1")); + assert!(help.contains("HTTP API")); + assert!(help.contains("Why this fails:")); + assert!(help.contains("How to fix:")); + assert!(help.contains("docs/external-issues/tracker/udp-tcp-port-sharing-allowed.md")); + } + } } diff --git a/src/domain/tracker/mod.rs b/src/domain/tracker/mod.rs index 3689ef0b..aa9c9f0a 100644 --- a/src/domain/tracker/mod.rs +++ b/src/domain/tracker/mod.rs @@ -6,6 +6,8 @@ //! # Module Structure //! //! - `config` - Main `TrackerConfig` and component configurations (includes database) +//! - `binding_address` - Socket binding address with protocol information +//! - `protocol` - Network protocol types (UDP, TCP) //! //! # Layer Separation //! @@ -44,9 +46,13 @@ //! }; //! ``` +mod binding_address; mod config; +mod protocol; +pub use binding_address::BindingAddress; pub use config::{ DatabaseConfig, HealthCheckApiConfig, HttpApiConfig, HttpTrackerConfig, MysqlConfig, - SqliteConfig, TrackerConfig, TrackerCoreConfig, UdpTrackerConfig, + SqliteConfig, TrackerConfig, TrackerConfigError, TrackerCoreConfig, UdpTrackerConfig, }; +pub use protocol::{Protocol, ProtocolParseError}; From 97368e5ddad91683e1472bb52a9e6acaa59ef39e Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 29 Dec 2025 19:07:10 +0000 Subject: [PATCH 5/7] refactor: [#255] extract binding collection logic into separate method - Extract collect_bindings() private method from validate() - Improves Single Responsibility Principle compliance - Makes validate() method more focused on conflict detection - Better separation of concerns: collection vs validation --- src/domain/tracker/config/mod.rs | 36 ++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/domain/tracker/config/mod.rs b/src/domain/tracker/config/mod.rs index 76975e2c..810369b2 100644 --- a/src/domain/tracker/config/mod.rs +++ b/src/domain/tracker/config/mod.rs @@ -210,7 +210,28 @@ impl TrackerConfig { /// assert!(config.validate().is_ok()); /// ``` pub fn validate(&self) -> Result<(), TrackerConfigError> { - // Collect all binding addresses with their service names + let bindings = self.collect_bindings(); + + // Check for duplicates + for (binding, services) in bindings { + if services.len() > 1 { + return Err(TrackerConfigError::DuplicateSocketAddress { + address: *binding.socket(), + protocol: binding.protocol(), + services, + }); + } + } + + Ok(()) + } + + /// Collects all binding addresses with their service names + /// + /// Creates a map of binding addresses (socket + protocol) to service names. + /// This allows identifying which services are attempting to bind to the same + /// socket address with the same protocol. + fn collect_bindings(&self) -> HashMap> { let mut bindings: HashMap> = HashMap::new(); // Add UDP trackers @@ -245,18 +266,7 @@ impl TrackerConfig { .or_default() .push("Health Check API".to_string()); - // Check for duplicates - for (binding, services) in bindings { - if services.len() > 1 { - return Err(TrackerConfigError::DuplicateSocketAddress { - address: *binding.socket(), - protocol: binding.protocol(), - services, - }); - } - } - - Ok(()) + bindings } } From 170ce022938083e36c192ee99b8d84e1e071e6ad Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 29 Dec 2025 19:11:05 +0000 Subject: [PATCH 6/7] refactor: [#255] extract conflict checking logic into separate method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract check_for_conflicts() private method from validate() - Further improves Single Responsibility Principle - Now validate() is a clean orchestrator of two focused methods - Three-level separation: collect → check → report error --- src/domain/tracker/config/mod.rs | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/domain/tracker/config/mod.rs b/src/domain/tracker/config/mod.rs index 810369b2..693bd54f 100644 --- a/src/domain/tracker/config/mod.rs +++ b/src/domain/tracker/config/mod.rs @@ -211,8 +211,21 @@ impl TrackerConfig { /// ``` pub fn validate(&self) -> Result<(), TrackerConfigError> { let bindings = self.collect_bindings(); + Self::check_for_conflicts(bindings) + } - // Check for duplicates + /// Checks for socket address conflicts in the collected bindings + /// + /// Examines the binding map to find any addresses that have multiple + /// services attempting to use them with the same protocol. + /// + /// # Errors + /// + /// Returns `TrackerConfigError::DuplicateSocketAddress` if any binding + /// address is shared by multiple services. + fn check_for_conflicts( + bindings: HashMap>, + ) -> Result<(), TrackerConfigError> { for (binding, services) in bindings { if services.len() > 1 { return Err(TrackerConfigError::DuplicateSocketAddress { From dc153f070a14f4bb4b5df2307acad43ba9533e19 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Mon, 29 Dec 2025 19:14:37 +0000 Subject: [PATCH 7/7] refactor: [#255] eliminate duplication in binding collection - Extract register_binding() helper to remove repeated HashMap operations - Extract register_trackers() to handle multiple tracker instances generically - Add HasBindAddress trait for uniform tracker handling - Service names now declared once at call site (DRY principle) - Reduces code from ~30 lines to ~15 lines in collect_bindings() - Improves testability with focused helper methods --- src/domain/tracker/config/mod.rs | 99 ++++++++++++++++++++++++-------- 1 file changed, 76 insertions(+), 23 deletions(-) diff --git a/src/domain/tracker/config/mod.rs b/src/domain/tracker/config/mod.rs index 693bd54f..a3b1c20a 100644 --- a/src/domain/tracker/config/mod.rs +++ b/src/domain/tracker/config/mod.rs @@ -248,38 +248,91 @@ impl TrackerConfig { let mut bindings: HashMap> = HashMap::new(); // Add UDP trackers - for (i, udp) in self.udp_trackers.iter().enumerate() { - let binding = BindingAddress::new(udp.bind_address, Protocol::Udp); - bindings - .entry(binding) - .or_default() - .push(format!("UDP Tracker #{}", i + 1)); - } + Self::register_trackers( + &mut bindings, + &self.udp_trackers, + Protocol::Udp, + "UDP Tracker", + ); // Add HTTP trackers - for (i, http) in self.http_trackers.iter().enumerate() { - let binding = BindingAddress::new(http.bind_address, Protocol::Tcp); - bindings - .entry(binding) - .or_default() - .push(format!("HTTP Tracker #{}", i + 1)); - } + Self::register_trackers( + &mut bindings, + &self.http_trackers, + Protocol::Tcp, + "HTTP Tracker", + ); // Add HTTP API - let api_binding = BindingAddress::new(self.http_api.bind_address, Protocol::Tcp); - bindings - .entry(api_binding) - .or_default() - .push("HTTP API".to_string()); + Self::register_binding( + &mut bindings, + self.http_api.bind_address, + Protocol::Tcp, + "HTTP API", + ); // Add Health Check API - let health_binding = BindingAddress::new(self.health_check_api.bind_address, Protocol::Tcp); + Self::register_binding( + &mut bindings, + self.health_check_api.bind_address, + Protocol::Tcp, + "Health Check API", + ); + bindings - .entry(health_binding) - .or_default() - .push("Health Check API".to_string()); + } + /// Registers multiple tracker instances in the bindings map + /// + /// Creates numbered service names for each tracker instance (e.g., "UDP Tracker #1"). + fn register_trackers( + bindings: &mut HashMap>, + trackers: &[T], + protocol: Protocol, + service_name: &str, + ) where + T: HasBindAddress, + { + for (i, tracker) in trackers.iter().enumerate() { + let service_label = format!("{service_name} #{}", i + 1); + Self::register_binding(bindings, tracker.bind_address(), protocol, &service_label); + } + } + + /// Registers a single binding in the bindings map + /// + /// Associates the given service name with the socket address and protocol. + fn register_binding( + bindings: &mut HashMap>, + address: SocketAddr, + protocol: Protocol, + service_name: &str, + ) { + let binding = BindingAddress::new(address, protocol); bindings + .entry(binding) + .or_default() + .push(service_name.to_string()); + } +} + +/// Trait for types that have a bind address +/// +/// Used for generic tracker registration in validation logic. +trait HasBindAddress { + /// Returns the socket address this service binds to + fn bind_address(&self) -> SocketAddr; +} + +impl HasBindAddress for UdpTrackerConfig { + fn bind_address(&self) -> SocketAddr { + self.bind_address + } +} + +impl HasBindAddress for HttpTrackerConfig { + fn bind_address(&self) -> SocketAddr { + self.bind_address } }