diff --git a/docs/issues/287-docker-compose-topology-refactoring-epic.md b/docs/issues/287-docker-compose-topology-refactoring-epic.md index fe692417..304b9cb3 100644 --- a/docs/issues/287-docker-compose-topology-refactoring-epic.md +++ b/docs/issues/287-docker-compose-topology-refactoring-epic.md @@ -38,11 +38,14 @@ Implementation follows a 5-PR strategy (original scope), with Phase 3 added as a ### Phase 3: Port Topology (Added Extension) > **Note**: Phase 3 was part of the original refactoring plan analysis (see PORT-01 through PORT-11 rules in [docker-compose-topology-domain-model.md](../refactors/plans/docker-compose-topology-domain-model.md#port-exposure-rules)) but was not included in the initial 5-PR strategy. It follows the same pattern as networks and completes the topology domain model. - -- [ ] [#298](https://github.com/torrust/torrust-tracker-deployer/issues/298) - [Refactor] Phase 3: Create Port Topology Domain Model (P3.1, P3.2, P3.3, P3.4) → [Spec](298-phase-3-port-topology-domain-model.md) - - P3.1: Create Port domain types (`PortBinding`, `Protocol`) - - P3.2: Extend `ServiceTopology` with port derivation - - P3.3: Add cross-service port conflict validation +> +> **Implementation Note**: Phase 3 is split into two PRs for better reviewability. PR #298 delivers the domain layer foundation (P3.1-P3.3), while a follow-up PR will integrate ports into the template (P3.4). + +- [ ] [#298](https://github.com/torrust/torrust-tracker-deployer/issues/298) - [Refactor] Phase 3: Port Topology Domain Model - Foundation (P3.1, P3.2, P3.3) → [Spec](298-phase-3-port-topology-domain-model.md) + - P3.1: Create Port domain types (`PortBinding`, reuse `Protocol`) + - P3.2: Extend `ServiceTopology` with ports field + - P3.3: Add cross-service port conflict validation with `help()` method +- [ ] TBD - [Refactor] Phase 3: Port Topology Template Integration (P3.4) - P3.4: Update template to use derived ports with descriptions ## PR Dependencies @@ -60,7 +63,10 @@ PR 4 (Phase 1) PR 5 (Phase 2) │ ▼ -PR 6 (Phase 3) ◄─── Extension: Port topology (added post-completion) +PR 6 (Phase 3 Foundation) ◄─── Domain types: PortBinding, validation, help() + │ + ▼ +PR 7 (Phase 3 Template) ◄─── Template integration (P3.4) - follow-up ``` ## Key Decisions @@ -71,6 +77,8 @@ PR 6 (Phase 3) ◄─── Extension: Port topology (added post-completion) - ~47 domain rules identified with test specifications - Network descriptions rendered as YAML comments for sysadmin documentation - Port topology follows same pattern as networks (Phase 3) +- Phase 3 split into foundation (domain) and integration (template) PRs for reviewability +- `PortConflict` error includes `help()` method per DDD error handling conventions ## Related diff --git a/docs/issues/298-phase-3-port-topology-domain-model.md b/docs/issues/298-phase-3-port-topology-domain-model.md index 636340b8..3c474256 100644 --- a/docs/issues/298-phase-3-port-topology-domain-model.md +++ b/docs/issues/298-phase-3-port-topology-domain-model.md @@ -2,7 +2,16 @@ **Epic**: [#287](https://github.com/torrust/torrust-tracker-deployer/issues/287) - Docker Compose Topology Domain Model Refactoring **Related Plan**: [docs/refactors/plans/docker-compose-topology-domain-model.md](../../refactors/plans/docker-compose-topology-domain-model.md) -**Status**: Draft +**Status**: In Progress (P3.1-P3.3 Complete) + +## Implementation Notes + +> **PR Strategy**: This phase is split into two PRs for better reviewability: +> +> - **PR #298 (this PR)**: Domain layer foundation - P3.1, P3.2, P3.3 +> - **Follow-up PR**: Template integration - P3.4 +> +> The domain types are stable and well-tested. Template integration is a larger change that benefits from being a separate, focused PR. ## Overview @@ -177,46 +186,60 @@ services: ## Tasks -### P3.1: Create Port Domain Types +### P3.1: Create Port Domain Types ✅ -- [ ] Create `Protocol` enum (or reuse from `domain/tracker`) -- [ ] Create `PortBinding` struct with description support -- [ ] Add `PortDefinition` context type for template rendering -- [ ] Add unit tests for port binding creation +- [x] Reuse `Protocol` enum from `domain/tracker/protocol.rs` +- [x] Create `PortBinding` struct with description support +- [x] Add convenience constructors: `tcp()`, `udp()`, `localhost_tcp()` +- [x] Add `docker_compose_binding()` method for YAML rendering +- [x] Add unit tests for port binding creation (10 tests) -### P3.2: Extend ServiceTopology with Ports +### P3.2: Extend ServiceTopology with Ports ✅ -- [ ] Add `ports: Vec` to `ServiceTopology` -- [ ] Implement port derivation for each service type: - - Tracker: UDP ports, HTTP ports (TLS-dependent), API (TLS-dependent) - - Caddy: 80, 443, 443/udp (fixed) - - Prometheus: 9090 (localhost) - - Grafana: 3000 (TLS-dependent) - - MySQL: none -- [ ] Add unit tests for each service's port derivation +- [x] Add `ports: Vec` to `ServiceTopology` +- [x] Add `new()` constructor with ports parameter +- [x] Add `with_networks()` for backward compatibility +- [x] Add `ports()` and `has_ports()` getters with `#[must_use]` +- [x] Make all fields private with getters (DDD compliance) +- [x] Add unit tests for port field (4 tests) -### P3.3: Add Cross-Service Port Validation +### P3.3: Add Cross-Service Port Validation ✅ -- [ ] Implement `DockerComposeTopology::validate_port_uniqueness()` -- [ ] Create `TopologyError::PortConflict` variant -- [ ] Add tests for conflict detection scenarios +- [x] Implement `DockerComposeTopology::validate_port_uniqueness()` +- [x] Create `PortConflict` error type with full context +- [x] Create `TopologyError` enum with `help()` method (DDD error conventions) +- [x] Add tests for conflict detection scenarios (6 tests) -### P3.4: Update Template and Context +### P3.4: Update Template and Context (Follow-up PR) - [ ] Create `PortDefinition` with `binding()` and `description()` for template - [ ] Update `DockerComposeContext` to use derived ports - [ ] Simplify `docker-compose.yml.tera` port sections - [ ] Remove conditional port logic from template +- [ ] Implement port derivation for each service type: + - Tracker: UDP ports, HTTP ports (TLS-dependent), API (TLS-dependent) + - Caddy: 80, 443, 443/udp (fixed) + - Prometheus: 9090 (localhost) + - Grafana: 3000 (TLS-dependent) + - MySQL: none ## Acceptance Criteria +### PR #298 (Domain Foundation) ✅ + +- [x] `PortBinding` type with description support created +- [x] `ServiceTopology` extended with ports field +- [x] Cross-service port conflicts detected with actionable `help()` message +- [x] All fields private with getters (DDD compliance) +- [x] No duplication with `TrackerConfig` validation (different scopes) +- [x] All existing E2E tests pass +- [x] Pre-commit checks pass + +### Follow-up PR (Template Integration) + - [ ] All PORT-\* rules from refactoring plan are implemented in domain - [ ] Port descriptions render as YAML comments in output -- [ ] Cross-service port conflicts are detected and reported with actionable error - [ ] Template has no conditional port logic (just loops) -- [ ] No duplication with `TrackerConfig` validation (different scopes) -- [ ] All existing E2E tests pass -- [ ] Pre-commit checks pass ## Testing Strategy diff --git a/src/domain/topology/aggregate.rs b/src/domain/topology/aggregate.rs index 00f28629..62f42060 100644 --- a/src/domain/topology/aggregate.rs +++ b/src/domain/topology/aggregate.rs @@ -1,13 +1,14 @@ //! Docker Compose Topology Aggregate //! //! This module defines the [`DockerComposeTopology`] aggregate that collects -//! all service topologies and derives the required networks. +//! all service topologies and derives the required networks and ports. //! //! ## Design //! //! The topology aggregate is the single source of truth for: //! - Which services are enabled //! - Which networks each service uses +//! - Which ports each service exposes //! - The complete list of required networks (derived from services) //! //! ## Invariants @@ -15,28 +16,97 @@ //! - Every network used by a service is included in `required_networks()` //! - No orphan networks (networks defined but not used by any service) //! - Networks are returned in deterministic order +//! - Port bindings include descriptions for documentation -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; +use super::error::PortConflict; use super::network::Network; +use super::port::PortBinding; use super::service::Service; /// Topology information for a single service /// -/// Contains the service identifier and its network assignments. +/// Contains the service identifier, its network assignments, and port bindings. +/// +/// # Examples +/// +/// ```rust +/// use torrust_tracker_deployer_lib::domain::topology::{ +/// ServiceTopology, Service, Network, PortBinding +/// }; +/// +/// // Service with ports +/// let topology = ServiceTopology::new( +/// Service::Tracker, +/// vec![Network::Database, Network::Metrics], +/// vec![PortBinding::udp(6969, "BitTorrent UDP announce")], +/// ); +/// +/// assert_eq!(topology.service(), Service::Tracker); +/// assert_eq!(topology.networks().len(), 2); +/// assert_eq!(topology.ports().len(), 1); +/// +/// // Service without ports +/// let db_topology = ServiceTopology::with_networks( +/// Service::MySQL, +/// vec![Network::Database], +/// ); +/// +/// assert!(!db_topology.has_ports()); +/// ``` #[derive(Debug, Clone)] pub struct ServiceTopology { /// The service this topology describes - pub service: Service, + service: Service, /// Networks this service is connected to - pub networks: Vec, + networks: Vec, + /// Ports this service exposes to the host + ports: Vec, } impl ServiceTopology { - /// Creates a new service topology + /// Creates a new service topology with networks and ports + #[must_use] + pub fn new(service: Service, networks: Vec, ports: Vec) -> Self { + Self { + service, + networks, + ports, + } + } + + /// Creates a new service topology with networks only (no ports) + /// + /// Convenience constructor for services that don't expose ports + /// (e.g., `MySQL` which is internal-only). #[must_use] - pub fn new(service: Service, networks: Vec) -> Self { - Self { service, networks } + pub fn with_networks(service: Service, networks: Vec) -> Self { + Self::new(service, networks, vec![]) + } + + /// Returns the service this topology describes + #[must_use] + pub fn service(&self) -> Service { + self.service + } + + /// Returns the networks this service is connected to + #[must_use] + pub fn networks(&self) -> &[Network] { + &self.networks + } + + /// Returns the ports this service exposes + #[must_use] + pub fn ports(&self) -> &[PortBinding] { + &self.ports + } + + /// Returns whether this service exposes any ports + #[must_use] + pub fn has_ports(&self) -> bool { + !self.ports.is_empty() } } @@ -55,8 +125,8 @@ impl ServiceTopology { /// }; /// /// let topology = DockerComposeTopology::new(vec![ -/// ServiceTopology::new(Service::Tracker, vec![Network::Database, Network::Metrics]), -/// ServiceTopology::new(Service::MySQL, vec![Network::Database]), +/// ServiceTopology::with_networks(Service::Tracker, vec![Network::Database, Network::Metrics]), +/// ServiceTopology::with_networks(Service::MySQL, vec![Network::Database]), /// ]); /// /// let required = topology.required_networks(); @@ -94,7 +164,7 @@ impl DockerComposeTopology { let unique: HashSet = self .services .iter() - .flat_map(|s| s.networks.iter().copied()) + .flat_map(|s| s.networks().iter().copied()) .collect(); // Return in deterministic order for template stability @@ -108,6 +178,68 @@ impl DockerComposeTopology { pub fn services(&self) -> &[ServiceTopology] { &self.services } + + /// Validates that no two services bind to the same host port + /// + /// Docker Compose will fail at startup if two services try to bind + /// to the same host port. This method detects such conflicts early, + /// providing a clear error message. + /// + /// Note: Binding to different IPs (e.g., 127.0.0.1:8080 and 0.0.0.0:8080) + /// is still considered a conflict since 0.0.0.0 includes all interfaces. + /// + /// # Errors + /// + /// Returns [`PortConflict`] when two services expose the same host port. + /// The error includes details about both conflicting services and their + /// port bindings, enabling actionable error messages. + /// + /// # Examples + /// + /// ```rust + /// use torrust_tracker_deployer_lib::domain::topology::{ + /// DockerComposeTopology, ServiceTopology, Service, PortBinding + /// }; + /// + /// let topology = DockerComposeTopology::new(vec![ + /// ServiceTopology::new( + /// Service::Tracker, + /// vec![], + /// vec![PortBinding::tcp(9090, "Health check")] + /// ), + /// ServiceTopology::new( + /// Service::Prometheus, + /// vec![], + /// vec![PortBinding::tcp(9090, "Web UI")] // Conflict! + /// ), + /// ]); + /// + /// assert!(topology.validate_port_uniqueness().is_err()); + /// ``` + pub fn validate_port_uniqueness(&self) -> Result<(), PortConflict> { + // Track which service has bound each host port + let mut port_bindings: HashMap = HashMap::new(); + + for service_topology in &self.services { + for binding in service_topology.ports() { + let host_port = binding.host_port(); + + if let Some((first_service, first_binding)) = port_bindings.get(&host_port) { + return Err(PortConflict { + host_port, + first_service: *first_service, + first_binding: first_binding.clone(), + second_service: service_topology.service(), + second_binding: binding.clone(), + }); + } + + port_bindings.insert(host_port, (service_topology.service(), binding.clone())); + } + } + + Ok(()) + } } #[cfg(test)] @@ -119,21 +251,47 @@ mod tests { #[test] fn it_should_create_service_topology_with_networks() { - let topology = - ServiceTopology::new(Service::Tracker, vec![Network::Database, Network::Metrics]); + let topology = ServiceTopology::with_networks( + Service::Tracker, + vec![Network::Database, Network::Metrics], + ); - assert_eq!(topology.service, Service::Tracker); - assert_eq!(topology.networks.len(), 2); - assert!(topology.networks.contains(&Network::Database)); - assert!(topology.networks.contains(&Network::Metrics)); + assert_eq!(topology.service(), Service::Tracker); + assert_eq!(topology.networks().len(), 2); + assert!(topology.networks().contains(&Network::Database)); + assert!(topology.networks().contains(&Network::Metrics)); } #[test] fn it_should_create_service_topology_with_empty_networks() { - let topology = ServiceTopology::new(Service::Tracker, vec![]); + let topology = ServiceTopology::with_networks(Service::Tracker, vec![]); + + assert_eq!(topology.service(), Service::Tracker); + assert!(topology.networks().is_empty()); + } + + #[test] + fn it_should_create_service_topology_with_ports() { + let topology = ServiceTopology::new( + Service::Tracker, + vec![Network::Metrics], + vec![ + PortBinding::udp(6969, "BitTorrent UDP announce"), + PortBinding::tcp(7070, "HTTP tracker announce"), + ], + ); - assert_eq!(topology.service, Service::Tracker); - assert!(topology.networks.is_empty()); + assert_eq!(topology.service(), Service::Tracker); + assert_eq!(topology.ports().len(), 2); + assert!(topology.has_ports()); + } + + #[test] + fn it_should_have_no_ports_when_using_with_networks() { + let topology = ServiceTopology::with_networks(Service::MySQL, vec![Network::Database]); + + assert!(topology.ports().is_empty()); + assert!(!topology.has_ports()); } } @@ -143,9 +301,12 @@ mod tests { #[test] fn it_should_derive_required_networks_from_all_services() { let topology = DockerComposeTopology::new(vec![ - ServiceTopology::new(Service::Tracker, vec![Network::Database, Network::Metrics]), - ServiceTopology::new(Service::MySQL, vec![Network::Database]), - ServiceTopology::new(Service::Prometheus, vec![Network::Metrics]), + ServiceTopology::with_networks( + Service::Tracker, + vec![Network::Database, Network::Metrics], + ), + ServiceTopology::with_networks(Service::MySQL, vec![Network::Database]), + ServiceTopology::with_networks(Service::Prometheus, vec![Network::Metrics]), ]); let required = topology.required_networks(); @@ -156,7 +317,7 @@ mod tests { #[test] fn it_should_not_have_orphan_networks() { - let topology = DockerComposeTopology::new(vec![ServiceTopology::new( + let topology = DockerComposeTopology::new(vec![ServiceTopology::with_networks( Service::Tracker, vec![Network::Metrics], )]); @@ -175,10 +336,10 @@ mod tests { fn it_should_return_networks_in_deterministic_order() { // Create topology with networks added in non-alphabetical order let topology = DockerComposeTopology::new(vec![ - ServiceTopology::new(Service::Caddy, vec![Network::Proxy]), - ServiceTopology::new(Service::Tracker, vec![Network::Database]), - ServiceTopology::new(Service::Prometheus, vec![Network::Metrics]), - ServiceTopology::new(Service::Grafana, vec![Network::Visualization]), + ServiceTopology::with_networks(Service::Caddy, vec![Network::Proxy]), + ServiceTopology::with_networks(Service::Tracker, vec![Network::Database]), + ServiceTopology::with_networks(Service::Prometheus, vec![Network::Metrics]), + ServiceTopology::with_networks(Service::Grafana, vec![Network::Visualization]), ]); let required = topology.required_networks(); @@ -199,8 +360,8 @@ mod tests { #[test] fn it_should_deduplicate_networks_used_by_multiple_services() { let topology = DockerComposeTopology::new(vec![ - ServiceTopology::new(Service::Tracker, vec![Network::Metrics]), - ServiceTopology::new(Service::Prometheus, vec![Network::Metrics]), + ServiceTopology::with_networks(Service::Tracker, vec![Network::Metrics]), + ServiceTopology::with_networks(Service::Prometheus, vec![Network::Metrics]), ]); let required = topology.required_networks(); @@ -221,8 +382,10 @@ mod tests { #[test] fn it_should_return_empty_when_services_have_no_networks() { - let topology = - DockerComposeTopology::new(vec![ServiceTopology::new(Service::Tracker, vec![])]); + let topology = DockerComposeTopology::new(vec![ServiceTopology::with_networks( + Service::Tracker, + vec![], + )]); let required = topology.required_networks(); @@ -236,8 +399,10 @@ mod tests { #[test] fn it_should_configure_minimal_deployment() { // Tracker only, no optional services - let topology = - DockerComposeTopology::new(vec![ServiceTopology::new(Service::Tracker, vec![])]); + let topology = DockerComposeTopology::new(vec![ServiceTopology::with_networks( + Service::Tracker, + vec![], + )]); let required = topology.required_networks(); @@ -247,8 +412,8 @@ mod tests { #[test] fn it_should_configure_deployment_with_mysql() { let topology = DockerComposeTopology::new(vec![ - ServiceTopology::new(Service::Tracker, vec![Network::Database]), - ServiceTopology::new(Service::MySQL, vec![Network::Database]), + ServiceTopology::with_networks(Service::Tracker, vec![Network::Database]), + ServiceTopology::with_networks(Service::MySQL, vec![Network::Database]), ]); let required = topology.required_networks(); @@ -260,12 +425,12 @@ mod tests { #[test] fn it_should_configure_deployment_with_monitoring() { let topology = DockerComposeTopology::new(vec![ - ServiceTopology::new(Service::Tracker, vec![Network::Metrics]), - ServiceTopology::new( + ServiceTopology::with_networks(Service::Tracker, vec![Network::Metrics]), + ServiceTopology::with_networks( Service::Prometheus, vec![Network::Metrics, Network::Visualization], ), - ServiceTopology::new(Service::Grafana, vec![Network::Visualization]), + ServiceTopology::with_networks(Service::Grafana, vec![Network::Visualization]), ]); let required = topology.required_networks(); @@ -278,13 +443,16 @@ mod tests { #[test] fn it_should_configure_full_http_deployment() { let topology = DockerComposeTopology::new(vec![ - ServiceTopology::new(Service::Tracker, vec![Network::Database, Network::Metrics]), - ServiceTopology::new(Service::MySQL, vec![Network::Database]), - ServiceTopology::new( + ServiceTopology::with_networks( + Service::Tracker, + vec![Network::Database, Network::Metrics], + ), + ServiceTopology::with_networks(Service::MySQL, vec![Network::Database]), + ServiceTopology::with_networks( Service::Prometheus, vec![Network::Metrics, Network::Visualization], ), - ServiceTopology::new(Service::Grafana, vec![Network::Visualization]), + ServiceTopology::with_networks(Service::Grafana, vec![Network::Visualization]), ]); let required = topology.required_networks(); @@ -299,20 +467,20 @@ mod tests { #[test] fn it_should_configure_full_https_deployment() { let topology = DockerComposeTopology::new(vec![ - ServiceTopology::new( + ServiceTopology::with_networks( Service::Tracker, vec![Network::Database, Network::Metrics, Network::Proxy], ), - ServiceTopology::new(Service::MySQL, vec![Network::Database]), - ServiceTopology::new( + ServiceTopology::with_networks(Service::MySQL, vec![Network::Database]), + ServiceTopology::with_networks( Service::Prometheus, vec![Network::Metrics, Network::Visualization], ), - ServiceTopology::new( + ServiceTopology::with_networks( Service::Grafana, vec![Network::Visualization, Network::Proxy], ), - ServiceTopology::new(Service::Caddy, vec![Network::Proxy]), + ServiceTopology::with_networks(Service::Caddy, vec![Network::Proxy]), ]); let required = topology.required_networks(); @@ -328,8 +496,8 @@ mod tests { fn it_should_configure_https_minimal_deployment() { // Tracker + Caddy only (HTTPS with no monitoring or database) let topology = DockerComposeTopology::new(vec![ - ServiceTopology::new(Service::Tracker, vec![Network::Proxy]), - ServiceTopology::new(Service::Caddy, vec![Network::Proxy]), + ServiceTopology::with_networks(Service::Tracker, vec![Network::Proxy]), + ServiceTopology::with_networks(Service::Caddy, vec![Network::Proxy]), ]); let required = topology.required_networks(); @@ -338,4 +506,122 @@ mod tests { assert!(required.contains(&Network::Proxy)); } } + + mod port_validation { + use super::*; + + #[test] + fn it_should_succeed_when_no_port_conflicts() { + let topology = DockerComposeTopology::new(vec![ + ServiceTopology::new( + Service::Tracker, + vec![], + vec![ + PortBinding::udp(6969, "UDP announce"), + PortBinding::tcp(7070, "HTTP announce"), + ], + ), + ServiceTopology::new( + Service::Prometheus, + vec![], + vec![PortBinding::tcp(9090, "Web UI")], + ), + ]); + + assert!(topology.validate_port_uniqueness().is_ok()); + } + + #[test] + fn it_should_fail_when_same_host_port_bound_by_two_services() { + let topology = DockerComposeTopology::new(vec![ + ServiceTopology::new( + Service::Tracker, + vec![], + vec![PortBinding::tcp(9090, "Health check")], + ), + ServiceTopology::new( + Service::Prometheus, + vec![], + vec![PortBinding::tcp(9090, "Web UI")], + ), + ]); + + let result = topology.validate_port_uniqueness(); + + assert!(result.is_err()); + let conflict = result.unwrap_err(); + assert_eq!(conflict.host_port, 9090); + } + + #[test] + fn it_should_succeed_when_services_have_no_ports() { + let topology = DockerComposeTopology::new(vec![ + ServiceTopology::with_networks(Service::MySQL, vec![Network::Database]), + ServiceTopology::with_networks(Service::Tracker, vec![Network::Database]), + ]); + + assert!(topology.validate_port_uniqueness().is_ok()); + } + + #[test] + fn it_should_succeed_when_empty_topology() { + let topology = DockerComposeTopology::new(vec![]); + + assert!(topology.validate_port_uniqueness().is_ok()); + } + + #[test] + fn it_should_allow_same_container_port_on_different_host_ports() { + // Both services use container port 80 but mapped to different host ports + let topology = DockerComposeTopology::new(vec![ + ServiceTopology::new( + Service::Tracker, + vec![], + vec![PortBinding::new( + 8080, + 80, + crate::domain::tracker::Protocol::Tcp, + None, + "HTTP", + )], + ), + ServiceTopology::new( + Service::Prometheus, + vec![], + vec![PortBinding::new( + 9090, + 80, + crate::domain::tracker::Protocol::Tcp, + None, + "HTTP", + )], + ), + ]); + + assert!(topology.validate_port_uniqueness().is_ok()); + } + + #[test] + fn it_should_include_conflict_details_in_error() { + let topology = DockerComposeTopology::new(vec![ + ServiceTopology::new( + Service::Tracker, + vec![], + vec![PortBinding::tcp(9090, "Health check")], + ), + ServiceTopology::new( + Service::Prometheus, + vec![], + vec![PortBinding::tcp(9090, "Prometheus UI")], + ), + ]); + + let conflict = topology.validate_port_uniqueness().unwrap_err(); + + assert_eq!(conflict.first_service, Service::Tracker); + assert_eq!(conflict.second_service, Service::Prometheus); + assert_eq!(conflict.first_binding.description(), "Health check"); + assert_eq!(conflict.second_binding.description(), "Prometheus UI"); + } + } } diff --git a/src/domain/topology/error.rs b/src/domain/topology/error.rs new file mode 100644 index 00000000..781137fc --- /dev/null +++ b/src/domain/topology/error.rs @@ -0,0 +1,166 @@ +//! Topology validation errors +//! +//! This module defines errors that can occur when validating the Docker +//! Compose topology, particularly around port conflicts. + +use std::fmt; + +use super::port::PortBinding; +use super::service::Service; + +/// Error indicating a port conflict between services +/// +/// Occurs when two services expose the same host port, which would cause +/// a bind error when starting the Docker Compose stack. +#[derive(Debug, Clone)] +pub struct PortConflict { + /// The host port that is bound multiple times + pub host_port: u16, + /// The first service binding this port + pub first_service: Service, + /// The port binding from the first service + pub first_binding: PortBinding, + /// The second service (conflicting) binding this port + pub second_service: Service, + /// The port binding from the second service + pub second_binding: PortBinding, +} + +impl fmt::Display for PortConflict { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!( + f, + "Port {} is bound by both {} ({}) and {} ({})", + self.host_port, + self.first_service.name(), + self.first_binding.docker_compose_binding(), + self.second_service.name(), + self.second_binding.docker_compose_binding(), + ) + } +} + +impl std::error::Error for PortConflict {} + +/// Errors that can occur when building or validating a topology +#[derive(Debug)] +pub enum TopologyError { + /// A port conflict was detected between two services + PortConflict(PortConflict), +} + +impl fmt::Display for TopologyError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + TopologyError::PortConflict(conflict) => write!(f, "{conflict}"), + } + } +} + +impl std::error::Error for TopologyError { + fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { + match self { + TopologyError::PortConflict(conflict) => Some(conflict), + } + } +} + +impl From for TopologyError { + fn from(conflict: PortConflict) -> Self { + TopologyError::PortConflict(conflict) + } +} + +impl TopologyError { + /// Returns guidance on how to resolve this error + /// + /// Following the DDD practices for expressive error types, this method + /// explains what went wrong and suggests how to fix it. + #[must_use] + pub fn help(&self) -> String { + match self { + TopologyError::PortConflict(conflict) => { + format!( + "Two services are trying to bind to the same host port {}. \ + Either change the host port for '{}' or '{}', \ + or modify the configuration to avoid the conflict. \ + Docker cannot start if two services bind the same port.", + conflict.host_port, + conflict.first_service.name(), + conflict.second_service.name(), + ) + } + } + } +} + +#[cfg(test)] +mod tests { + use crate::domain::tracker::Protocol; + + use super::*; + + #[test] + fn it_should_format_port_conflict_with_both_services() { + let conflict = PortConflict { + host_port: 9090, + first_service: Service::Tracker, + first_binding: PortBinding::tcp(9090, "Health check"), + second_service: Service::Prometheus, + second_binding: PortBinding::tcp(9090, "Web UI"), + }; + + let message = conflict.to_string(); + + assert!(message.contains("9090")); + assert!(message.contains("tracker")); + assert!(message.contains("prometheus")); + } + + #[test] + fn it_should_include_protocol_in_conflict_message_for_udp() { + let conflict = PortConflict { + host_port: 6969, + first_service: Service::Tracker, + first_binding: PortBinding::new(6969, 6969, Protocol::Udp, None, "UDP announce"), + second_service: Service::Tracker, + second_binding: PortBinding::new(6969, 6970, Protocol::Udp, None, "Another UDP"), + }; + + let message = conflict.to_string(); + + assert!(message.contains("6969:6969/udp")); + assert!(message.contains("6969:6970/udp")); + } + + #[test] + fn it_should_provide_help_for_port_conflict_resolution() { + let conflict = PortConflict { + host_port: 9090, + first_service: Service::Tracker, + first_binding: PortBinding::tcp(9090, "Health check"), + second_service: Service::Prometheus, + second_binding: PortBinding::tcp(9090, "Web UI"), + }; + + let error = TopologyError::from(conflict); + let help = error.help(); + + assert!( + help.contains("9090"), + "Help should mention the conflicting port" + ); + assert!( + help.contains("tracker"), + "Help should mention the first service" + ); + assert!( + help.contains("prometheus"), + "Help should mention the second service" + ); + assert!( + help.contains("change") || help.contains("modify"), + "Help should suggest a fix" + ); + } +} diff --git a/src/domain/topology/mod.rs b/src/domain/topology/mod.rs index dabb7d83..4e41d4f3 100644 --- a/src/domain/topology/mod.rs +++ b/src/domain/topology/mod.rs @@ -1,12 +1,13 @@ //! Docker Compose Topology Domain Types //! //! This module contains domain types for Docker Compose topology elements, -//! including networks, services, and their relationships. +//! including networks, ports, services, and their relationships. //! //! ## Design Principles //! //! These types represent business concepts with strong typing: //! - Type-safe network references (no string typos) +//! - Type-safe port bindings with protocol awareness //! - Type-safe service identification //! - Single source of truth for network names //! - Domain-level invariant enforcement @@ -14,15 +15,21 @@ //! ## Components //! //! - [`Network`] - Docker Compose network enum representing isolation boundaries +//! - [`PortBinding`] - Port mapping with protocol and description //! - [`Service`] - Docker Compose service enum for type-safe service identification //! - [`DockerComposeTopology`] - Aggregate that derives required networks from services //! - [`ServiceTopology`] - Topology information for a single service +//! - [`TopologyError`] - Validation errors (e.g., port conflicts) pub mod aggregate; +pub mod error; pub mod network; +pub mod port; pub mod service; // Re-export main types for convenience pub use aggregate::{DockerComposeTopology, ServiceTopology}; +pub use error::{PortConflict, TopologyError}; pub use network::Network; +pub use port::PortBinding; pub use service::Service; diff --git a/src/domain/topology/port.rs b/src/domain/topology/port.rs new file mode 100644 index 00000000..11f7ebe0 --- /dev/null +++ b/src/domain/topology/port.rs @@ -0,0 +1,308 @@ +//! Port binding domain types for Docker Compose topology +//! +//! This module defines port binding types that represent how container ports +//! are exposed to the host network. +//! +//! ## Design Principles +//! +//! - Type-safe port bindings (no string typos in port specifications) +//! - Self-documenting with descriptions for sysadmin context +//! - Protocol-aware (UDP vs TCP have separate port spaces) +//! - Supports host IP binding (e.g., localhost-only for Prometheus) +//! +//! ## Port Rules (from refactoring plan) +//! +//! The following rules are encoded in the port derivation logic: +//! +//! - PORT-01: Tracker needs ports if UDP OR HTTP without TLS OR API without TLS +//! - PORT-02: UDP ports always exposed (UDP doesn't use TLS) +//! - PORT-03: HTTP ports WITHOUT TLS exposed directly +//! - PORT-04: HTTP ports WITH TLS NOT exposed (Caddy handles) +//! - PORT-05: API exposed only when no TLS +//! - PORT-06: API NOT exposed when TLS +//! - PORT-07: Grafana 3000 exposed only without TLS +//! - PORT-08: Grafana 3000 NOT exposed with TLS +//! - PORT-09: Caddy always exposes 80, 443, 443/udp +//! - PORT-10: Prometheus 9090 on localhost only +//! - PORT-11: `MySQL` no exposed ports + +use std::fmt; +use std::net::IpAddr; + +use crate::domain::tracker::Protocol; + +/// A port binding for Docker Compose +/// +/// Represents how a container port is exposed to the host network. +/// The description provides context for sysadmins inspecting the +/// rendered docker-compose.yml file. +/// +/// # Examples +/// +/// ```rust +/// use std::net::IpAddr; +/// use torrust_tracker_deployer_lib::domain::topology::PortBinding; +/// use torrust_tracker_deployer_lib::domain::tracker::Protocol; +/// +/// // UDP tracker port - exposed on all interfaces +/// let udp_port = PortBinding::new( +/// 6969, +/// 6969, +/// Protocol::Udp, +/// None, +/// "BitTorrent UDP announce", +/// ); +/// assert_eq!(udp_port.docker_compose_binding(), "6969:6969/udp"); +/// +/// // Prometheus - localhost only +/// let prometheus_port = PortBinding::new( +/// 9090, +/// 9090, +/// Protocol::Tcp, +/// Some("127.0.0.1".parse().unwrap()), +/// "Prometheus metrics (localhost only)", +/// ); +/// assert_eq!(prometheus_port.docker_compose_binding(), "127.0.0.1:9090:9090"); +/// ``` +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PortBinding { + /// Port on the host machine + host_port: u16, + /// Port inside the container + container_port: u16, + /// Network protocol (TCP or UDP) + protocol: Protocol, + /// Host IP to bind to (None = 0.0.0.0, all interfaces) + host_ip: Option, + /// Description of what this port is for (rendered as YAML comment) + description: &'static str, +} + +impl PortBinding { + /// Creates a new port binding + /// + /// # Arguments + /// + /// * `host_port` - Port number on the host machine + /// * `container_port` - Port number inside the container + /// * `protocol` - Network protocol (TCP or UDP) + /// * `host_ip` - Optional IP to bind to (None = all interfaces) + /// * `description` - Short description for sysadmin documentation + #[must_use] + pub fn new( + host_port: u16, + container_port: u16, + protocol: Protocol, + host_ip: Option, + description: &'static str, + ) -> Self { + Self { + host_port, + container_port, + protocol, + host_ip, + description, + } + } + + /// Creates a TCP port binding on all interfaces + /// + /// Convenience constructor for common TCP port mappings where + /// host and container ports are the same. + #[must_use] + pub fn tcp(port: u16, description: &'static str) -> Self { + Self::new(port, port, Protocol::Tcp, None, description) + } + + /// Creates a UDP port binding on all interfaces + /// + /// Convenience constructor for common UDP port mappings where + /// host and container ports are the same. + #[must_use] + pub fn udp(port: u16, description: &'static str) -> Self { + Self::new(port, port, Protocol::Udp, None, description) + } + + /// Creates a TCP port binding on localhost only + /// + /// Used for services that should not be exposed externally, + /// like Prometheus. + #[must_use] + pub fn localhost_tcp(port: u16, description: &'static str) -> Self { + Self::new( + port, + port, + Protocol::Tcp, + Some(IpAddr::V4(std::net::Ipv4Addr::LOCALHOST)), + description, + ) + } + + /// Returns the host port + #[must_use] + pub fn host_port(&self) -> u16 { + self.host_port + } + + /// Returns the container port + #[must_use] + pub fn container_port(&self) -> u16 { + self.container_port + } + + /// Returns the protocol + #[must_use] + pub fn protocol(&self) -> Protocol { + self.protocol + } + + /// Returns the host IP if specified + #[must_use] + pub fn host_ip(&self) -> Option { + self.host_ip + } + + /// Returns the description + #[must_use] + pub fn description(&self) -> &'static str { + self.description + } + + /// Returns the Docker Compose port binding string + /// + /// Formats the port binding for docker-compose.yml: + /// - `"80:80"` - TCP on all interfaces + /// - `"6969:6969/udp"` - UDP on all interfaces + /// - `"127.0.0.1:9090:9090"` - TCP on localhost only + #[must_use] + pub fn docker_compose_binding(&self) -> String { + let protocol_suffix = match self.protocol { + Protocol::Tcp => String::new(), + Protocol::Udp => "/udp".to_string(), + }; + + match self.host_ip { + Some(ip) => format!( + "{}:{}:{}{}", + ip, self.host_port, self.container_port, protocol_suffix + ), + None => format!( + "{}:{}{}", + self.host_port, self.container_port, protocol_suffix + ), + } + } +} + +impl fmt::Display for PortBinding { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self.docker_compose_binding()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + mod port_binding_creation { + use super::*; + + #[test] + fn it_should_create_tcp_port_binding_on_all_interfaces() { + let port = PortBinding::tcp(7070, "HTTP tracker announce"); + + assert_eq!(port.host_port(), 7070); + assert_eq!(port.container_port(), 7070); + assert_eq!(port.protocol(), Protocol::Tcp); + assert!(port.host_ip().is_none()); + assert_eq!(port.description(), "HTTP tracker announce"); + } + + #[test] + fn it_should_create_udp_port_binding_on_all_interfaces() { + let port = PortBinding::udp(6969, "BitTorrent UDP announce"); + + assert_eq!(port.host_port(), 6969); + assert_eq!(port.container_port(), 6969); + assert_eq!(port.protocol(), Protocol::Udp); + assert!(port.host_ip().is_none()); + assert_eq!(port.description(), "BitTorrent UDP announce"); + } + + #[test] + fn it_should_create_localhost_only_port_binding() { + let port = PortBinding::localhost_tcp(9090, "Prometheus metrics"); + + assert_eq!(port.host_port(), 9090); + assert_eq!(port.protocol(), Protocol::Tcp); + assert_eq!( + port.host_ip(), + Some(IpAddr::V4(std::net::Ipv4Addr::LOCALHOST)) + ); + } + + #[test] + fn it_should_create_port_binding_with_different_host_and_container_ports() { + let port = PortBinding::new(8080, 80, Protocol::Tcp, None, "Remapped HTTP"); + + assert_eq!(port.host_port(), 8080); + assert_eq!(port.container_port(), 80); + } + } + + mod docker_compose_binding { + use super::*; + + #[test] + fn it_should_format_tcp_port_without_protocol_suffix() { + let port = PortBinding::tcp(7070, "HTTP tracker"); + + assert_eq!(port.docker_compose_binding(), "7070:7070"); + } + + #[test] + fn it_should_format_udp_port_with_udp_suffix() { + let port = PortBinding::udp(6969, "UDP tracker"); + + assert_eq!(port.docker_compose_binding(), "6969:6969/udp"); + } + + #[test] + fn it_should_format_localhost_binding_with_ip_prefix() { + let port = PortBinding::localhost_tcp(9090, "Prometheus"); + + assert_eq!(port.docker_compose_binding(), "127.0.0.1:9090:9090"); + } + + #[test] + fn it_should_format_different_host_and_container_ports() { + let port = PortBinding::new(8080, 80, Protocol::Tcp, None, "Remapped"); + + assert_eq!(port.docker_compose_binding(), "8080:80"); + } + + #[test] + fn it_should_format_udp_with_localhost_binding() { + let port = PortBinding::new( + 5353, + 53, + Protocol::Udp, + Some(IpAddr::V4(std::net::Ipv4Addr::LOCALHOST)), + "Local DNS", + ); + + assert_eq!(port.docker_compose_binding(), "127.0.0.1:5353:53/udp"); + } + } + + mod display_trait { + use super::*; + + #[test] + fn it_should_display_as_docker_compose_binding() { + let port = PortBinding::tcp(80, "HTTP"); + + assert_eq!(format!("{port}"), "80:80"); + } + } +}