Skip to content

Commit 16ade4a

Browse files
committed
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
1 parent 981547b commit 16ade4a

5 files changed

Lines changed: 733 additions & 3 deletions

File tree

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use std::path::PathBuf;
88
use thiserror::Error;
99

10+
use crate::domain::tracker::TrackerConfigError;
1011
use crate::domain::EnvironmentNameError;
1112
use crate::domain::ProfileNameError;
1213
use crate::shared::UsernameError;
@@ -105,6 +106,10 @@ pub enum CreateConfigError {
105106
/// Invalid Prometheus configuration
106107
#[error("Invalid Prometheus configuration: {0}")]
107108
InvalidPrometheusConfig(String),
109+
110+
/// Tracker configuration validation failed
111+
#[error("Tracker configuration validation failed: {0}")]
112+
TrackerConfigValidation(#[from] TrackerConfigError),
108113
}
109114

110115
impl CreateConfigError {
@@ -424,6 +429,27 @@ impl CreateConfigError {
424429
Note: The template automatically adds the 's' suffix (e.g., 15 becomes '15s'),\n\
425430
so you only need to specify the numeric value."
426431
}
432+
Self::TrackerConfigValidation(_) => {
433+
"Tracker configuration validation failed.\n\
434+
\n\
435+
This error indicates a problem with the tracker service configuration,\n\
436+
typically related to socket address (IP:Port:Protocol) conflicts.\n\
437+
\n\
438+
The error message above provides specific details about:\n\
439+
- Which services are in conflict\n\
440+
- The conflicting socket addresses\n\
441+
- Why the configuration is invalid\n\
442+
\n\
443+
Common issues:\n\
444+
1. Multiple services on same TCP port (HTTP tracker + API)\n\
445+
2. Duplicate UDP tracker ports\n\
446+
3. Duplicate HTTP tracker ports\n\
447+
\n\
448+
Note: UDP and TCP can share the same port (different protocols),\n\
449+
but this is not recommended for clarity.\n\
450+
\n\
451+
Related: docs/external-issues/tracker/udp-tcp-port-sharing-allowed.md"
452+
}
427453
}
428454
}
429455
}

src/application/command_handlers/create/config/tracker/tracker_section.rs

Lines changed: 64 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ impl TrackerSection {
6868
/// Returns error if any of the nested sections fail validation:
6969
/// - Invalid bind address formats
7070
/// - Invalid database configuration
71+
/// - Socket address conflicts (multiple services on same IP:Port:Protocol)
7172
pub fn to_tracker_config(&self) -> Result<TrackerConfig, CreateConfigError> {
7273
let core = self.core.to_tracker_core_config()?;
7374

@@ -88,13 +89,18 @@ impl TrackerSection {
8889
let health_check_api: HealthCheckApiConfig =
8990
self.health_check_api.to_health_check_api_config()?;
9091

91-
Ok(TrackerConfig {
92+
let config = TrackerConfig {
9293
core,
9394
udp_trackers: udp_trackers?,
9495
http_trackers: http_trackers?,
9596
http_api,
9697
health_check_api,
97-
})
98+
};
99+
100+
// Validate socket address uniqueness
101+
config.validate().map_err(CreateConfigError::from)?;
102+
103+
Ok(config)
98104
}
99105
}
100106

@@ -306,4 +312,60 @@ mod tests {
306312
assert_eq!(section.udp_trackers.len(), 1);
307313
assert_eq!(section.http_trackers.len(), 1);
308314
}
315+
316+
#[test]
317+
fn it_should_reject_configuration_with_duplicate_socket_addresses() {
318+
// HTTP tracker and API on same port (TCP protocol conflict)
319+
let section = TrackerSection {
320+
core: TrackerCoreSection {
321+
database: DatabaseSection::Sqlite {
322+
database_name: "tracker.db".to_string(),
323+
},
324+
private: false,
325+
},
326+
udp_trackers: vec![],
327+
http_trackers: vec![HttpTrackerSection {
328+
bind_address: "0.0.0.0:7070".to_string(),
329+
}],
330+
http_api: HttpApiSection {
331+
bind_address: "0.0.0.0:7070".to_string(),
332+
admin_token: "token".to_string(),
333+
},
334+
health_check_api: HealthCheckApiSection::default(),
335+
};
336+
337+
let result = section.to_tracker_config();
338+
assert!(result.is_err());
339+
assert!(matches!(
340+
result.unwrap_err(),
341+
CreateConfigError::TrackerConfigValidation(_)
342+
));
343+
}
344+
345+
#[test]
346+
fn it_should_accept_udp_and_tcp_on_same_port() {
347+
// UDP and TCP can share the same port (different protocol spaces)
348+
let section = TrackerSection {
349+
core: TrackerCoreSection {
350+
database: DatabaseSection::Sqlite {
351+
database_name: "tracker.db".to_string(),
352+
},
353+
private: false,
354+
},
355+
udp_trackers: vec![UdpTrackerSection {
356+
bind_address: "0.0.0.0:7070".to_string(),
357+
}],
358+
http_trackers: vec![HttpTrackerSection {
359+
bind_address: "0.0.0.0:7070".to_string(),
360+
}],
361+
http_api: HttpApiSection {
362+
bind_address: "0.0.0.0:1212".to_string(),
363+
admin_token: "token".to_string(),
364+
},
365+
health_check_api: HealthCheckApiSection::default(),
366+
};
367+
368+
let result = section.to_tracker_config();
369+
assert!(result.is_ok());
370+
}
309371
}
Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
//! Binding address value object for tracker services
2+
//!
3+
//! This module provides a type-safe representation of socket addresses
4+
//! with protocol information, used for validating tracker configurations.
5+
6+
use std::fmt;
7+
use std::net::SocketAddr;
8+
9+
use super::Protocol;
10+
11+
/// A binding address combining socket address and protocol
12+
///
13+
/// Represents a complete socket binding specification including both the
14+
/// network address (IP + port) and the protocol (UDP or TCP). This ensures
15+
/// that socket address uniqueness validation accounts for protocol differences.
16+
///
17+
/// # Examples
18+
///
19+
/// ```rust
20+
/// use torrust_tracker_deployer_lib::domain::tracker::{BindingAddress, Protocol};
21+
///
22+
/// let udp_addr = BindingAddress::new(
23+
/// "0.0.0.0:6969".parse().unwrap(),
24+
/// Protocol::Udp
25+
/// );
26+
///
27+
/// let tcp_addr = BindingAddress::new(
28+
/// "0.0.0.0:7070".parse().unwrap(),
29+
/// Protocol::Tcp
30+
/// );
31+
///
32+
/// assert_eq!(udp_addr.socket().port(), 6969);
33+
/// assert_eq!(tcp_addr.protocol(), Protocol::Tcp);
34+
/// ```
35+
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
36+
pub struct BindingAddress {
37+
socket: SocketAddr,
38+
protocol: Protocol,
39+
}
40+
41+
impl BindingAddress {
42+
/// Creates a new binding address
43+
///
44+
/// # Examples
45+
///
46+
/// ```rust
47+
/// use torrust_tracker_deployer_lib::domain::tracker::{BindingAddress, Protocol};
48+
///
49+
/// let addr = BindingAddress::new(
50+
/// "0.0.0.0:6969".parse().unwrap(),
51+
/// Protocol::Udp
52+
/// );
53+
/// ```
54+
#[must_use]
55+
pub fn new(socket: SocketAddr, protocol: Protocol) -> Self {
56+
Self { socket, protocol }
57+
}
58+
59+
/// Returns the socket address
60+
#[must_use]
61+
pub fn socket(&self) -> &SocketAddr {
62+
&self.socket
63+
}
64+
65+
/// Returns the protocol
66+
#[must_use]
67+
pub fn protocol(&self) -> Protocol {
68+
self.protocol
69+
}
70+
}
71+
72+
impl fmt::Display for BindingAddress {
73+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
74+
write!(f, "{} ({})", self.socket, self.protocol)
75+
}
76+
}
77+
78+
#[cfg(test)]
79+
mod tests {
80+
use super::*;
81+
82+
#[test]
83+
fn it_should_create_binding_address_with_udp_protocol() {
84+
let socket: SocketAddr = "0.0.0.0:6969".parse().unwrap();
85+
let addr = BindingAddress::new(socket, Protocol::Udp);
86+
87+
assert_eq!(addr.socket(), &socket);
88+
assert_eq!(addr.protocol(), Protocol::Udp);
89+
}
90+
91+
#[test]
92+
fn it_should_create_binding_address_with_tcp_protocol() {
93+
let socket: SocketAddr = "127.0.0.1:7070".parse().unwrap();
94+
let addr = BindingAddress::new(socket, Protocol::Tcp);
95+
96+
assert_eq!(addr.socket(), &socket);
97+
assert_eq!(addr.protocol(), Protocol::Tcp);
98+
}
99+
100+
#[test]
101+
fn it_should_display_binding_address_with_protocol() {
102+
let addr = BindingAddress::new("0.0.0.0:6969".parse().unwrap(), Protocol::Udp);
103+
assert_eq!(addr.to_string(), "0.0.0.0:6969 (UDP)");
104+
105+
let addr = BindingAddress::new("127.0.0.1:7070".parse().unwrap(), Protocol::Tcp);
106+
assert_eq!(addr.to_string(), "127.0.0.1:7070 (TCP)");
107+
}
108+
109+
#[test]
110+
fn it_should_consider_same_socket_different_protocol_as_different() {
111+
let udp_addr = BindingAddress::new("0.0.0.0:7070".parse().unwrap(), Protocol::Udp);
112+
let tcp_addr = BindingAddress::new("0.0.0.0:7070".parse().unwrap(), Protocol::Tcp);
113+
114+
assert_ne!(udp_addr, tcp_addr);
115+
}
116+
117+
#[test]
118+
fn it_should_consider_same_socket_same_protocol_as_equal() {
119+
let addr1 = BindingAddress::new("0.0.0.0:7070".parse().unwrap(), Protocol::Tcp);
120+
let addr2 = BindingAddress::new("0.0.0.0:7070".parse().unwrap(), Protocol::Tcp);
121+
122+
assert_eq!(addr1, addr2);
123+
}
124+
125+
#[test]
126+
fn it_should_consider_different_ips_same_port_as_different() {
127+
let addr1 = BindingAddress::new("192.168.1.10:7070".parse().unwrap(), Protocol::Tcp);
128+
let addr2 = BindingAddress::new("192.168.1.20:7070".parse().unwrap(), Protocol::Tcp);
129+
130+
assert_ne!(addr1, addr2);
131+
}
132+
133+
#[test]
134+
fn it_should_be_usable_as_hash_map_key() {
135+
use std::collections::HashMap;
136+
137+
let mut map = HashMap::new();
138+
let addr = BindingAddress::new("0.0.0.0:6969".parse().unwrap(), Protocol::Udp);
139+
map.insert(addr, "UDP Tracker");
140+
141+
assert_eq!(map.get(&addr), Some(&"UDP Tracker"));
142+
}
143+
}

0 commit comments

Comments
 (0)