Skip to content

Commit 4f91133

Browse files
committed
refactor: [torrust#521]: validate HTTP tracker config before starting the service
It shows a more friendly error message and it's easier to test.
1 parent 24b82f3 commit 4f91133

File tree

7 files changed

+208
-58
lines changed

7 files changed

+208
-58
lines changed
+182
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
//! Validated configuration for the HTTP Tracker service.
2+
//!
3+
//! [``crate::HttpTracker``] is a DTO containing the parsed data from the toml
4+
//! file.
5+
//!
6+
//! This configuration is a first level of validation that can be perform
7+
//! statically without running the service.
8+
//!
9+
//! For example, if SSL is enabled you must provide the certificate path. That
10+
//! can be validated. However, this validation does not check if the
11+
//! certificate is valid.
12+
use serde::{Deserialize, Serialize};
13+
use thiserror::Error;
14+
15+
use crate::HttpTracker;
16+
17+
/// Errors that can occur when validating the plan configuration.
18+
#[derive(Error, Debug, PartialEq)]
19+
pub enum ValidationError {
20+
/// Missing SSL cert path.
21+
#[error("missing SSL cert path, got: {ssl_cert_path}")]
22+
MissingSslCertPath { ssl_cert_path: String },
23+
/// Missing SSL key path.
24+
#[error("missing SSL key path, got: {ssl_key_path}")]
25+
MissingSslKeyPath { ssl_key_path: String },
26+
}
27+
28+
/// Configuration for each HTTP tracker.
29+
#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)]
30+
pub struct Config {
31+
enabled: bool,
32+
bind_address: String, // todo: use SocketAddr
33+
ssl_enabled: bool,
34+
ssl_cert_path: Option<String>, // todo: use Path
35+
ssl_key_path: Option<String>, // todo: use Path
36+
}
37+
38+
impl Config {
39+
#[must_use]
40+
pub fn is_enabled(&self) -> bool {
41+
self.enabled
42+
}
43+
}
44+
45+
impl TryFrom<HttpTracker> for Config {
46+
type Error = ValidationError;
47+
48+
fn try_from(config: HttpTracker) -> Result<Self, Self::Error> {
49+
if config.ssl_enabled {
50+
match config.ssl_cert_path.clone() {
51+
Some(ssl_cert_path) => {
52+
if ssl_cert_path.is_empty() {
53+
Err(ValidationError::MissingSslCertPath {
54+
ssl_cert_path: String::new(),
55+
})
56+
} else {
57+
Ok(())
58+
}
59+
}
60+
None => Err(ValidationError::MissingSslCertPath {
61+
ssl_cert_path: String::new(),
62+
}),
63+
}?;
64+
65+
match config.ssl_key_path.clone() {
66+
Some(ssl_key_path) => {
67+
if ssl_key_path.is_empty() {
68+
Err(ValidationError::MissingSslKeyPath {
69+
ssl_key_path: String::new(),
70+
})
71+
} else {
72+
Ok(())
73+
}
74+
}
75+
None => Err(ValidationError::MissingSslKeyPath {
76+
ssl_key_path: String::new(),
77+
}),
78+
}?;
79+
}
80+
81+
Ok(Self {
82+
enabled: config.enabled,
83+
bind_address: config.bind_address,
84+
ssl_enabled: config.ssl_enabled,
85+
ssl_cert_path: config.ssl_cert_path,
86+
ssl_key_path: config.ssl_key_path,
87+
})
88+
}
89+
}
90+
91+
impl From<Config> for HttpTracker {
92+
fn from(config: Config) -> Self {
93+
Self {
94+
enabled: config.enabled,
95+
bind_address: config.bind_address,
96+
ssl_enabled: config.ssl_enabled,
97+
ssl_cert_path: config.ssl_cert_path,
98+
ssl_key_path: config.ssl_key_path,
99+
}
100+
}
101+
}
102+
103+
#[cfg(test)]
104+
mod tests {
105+
106+
mod when_ssl_is_enabled {
107+
use crate::http_tracker::{Config, ValidationError};
108+
use crate::HttpTracker;
109+
110+
#[test]
111+
fn it_should_return_an_error_when_ssl_is_enabled_but_the_cert_path_is_not_provided() {
112+
let plain_config = HttpTracker {
113+
enabled: true,
114+
bind_address: "127.0.0.1:7070".to_string(),
115+
ssl_enabled: true,
116+
ssl_cert_path: None,
117+
ssl_key_path: Some("./localhost.key".to_string()),
118+
};
119+
120+
assert_eq!(
121+
Config::try_from(plain_config),
122+
Err(ValidationError::MissingSslCertPath {
123+
ssl_cert_path: String::new()
124+
})
125+
);
126+
}
127+
128+
#[test]
129+
fn it_should_return_an_error_when_ssl_is_enabled_but_the_cert_path_is_empty() {
130+
let plain_config = HttpTracker {
131+
enabled: true,
132+
bind_address: "127.0.0.1:7070".to_string(),
133+
ssl_enabled: true,
134+
ssl_cert_path: Some(String::new()),
135+
ssl_key_path: Some("./localhost.key".to_string()),
136+
};
137+
138+
assert_eq!(
139+
Config::try_from(plain_config),
140+
Err(ValidationError::MissingSslCertPath {
141+
ssl_cert_path: String::new()
142+
})
143+
);
144+
}
145+
146+
#[test]
147+
fn it_should_return_an_error_when_ssl_is_enabled_but_the_key_path_is_not_provided() {
148+
let plain_config = HttpTracker {
149+
enabled: true,
150+
bind_address: "127.0.0.1:7070".to_string(),
151+
ssl_enabled: true,
152+
ssl_cert_path: Some("./localhost.crt".to_string()),
153+
ssl_key_path: None,
154+
};
155+
156+
assert_eq!(
157+
Config::try_from(plain_config),
158+
Err(ValidationError::MissingSslKeyPath {
159+
ssl_key_path: String::new()
160+
})
161+
);
162+
}
163+
164+
#[test]
165+
fn it_should_return_an_error_when_ssl_is_enabled_but_the_key_path_is_empty() {
166+
let plain_config = HttpTracker {
167+
enabled: true,
168+
bind_address: "127.0.0.1:7070".to_string(),
169+
ssl_enabled: true,
170+
ssl_cert_path: Some("./localhost.crt".to_string()),
171+
ssl_key_path: Some(String::new()),
172+
};
173+
174+
assert_eq!(
175+
Config::try_from(plain_config),
176+
Err(ValidationError::MissingSslKeyPath {
177+
ssl_key_path: String::new()
178+
})
179+
);
180+
}
181+
}
182+
}

packages/configuration/src/lib.rs

+5-10
Original file line numberDiff line numberDiff line change
@@ -229,6 +229,10 @@
229229
//! [health_check_api]
230230
//! bind_address = "127.0.0.1:1313"
231231
//!```
232+
pub mod http_tracker;
233+
pub mod tracker_api;
234+
pub mod udp_tracker;
235+
232236
use std::collections::HashMap;
233237
use std::net::IpAddr;
234238
use std::str::FromStr;
@@ -261,16 +265,7 @@ pub struct Info {
261265
}
262266

263267
impl Info {
264-
/// Build Configuration Info
265-
///
266-
/// # Examples
267-
///
268-
/// ```
269-
/// use torrust_tracker_configuration::Info;
270-
///
271-
/// let result = Info::new(env_var_config, env_var_path_config, default_path_config, env_var_api_admin_token);
272-
/// assert_eq!(result, );
273-
/// ```
268+
/// Build Configuration Info.
274269
///
275270
/// # Errors
276271
///
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+

src/app.rs

+19-14
Original file line numberDiff line numberDiff line change
@@ -75,21 +75,26 @@ pub async fn start(config: &Configuration, tracker: Arc<core::Tracker>) -> Vec<J
7575
}
7676

7777
// Start the HTTP blocks
78-
for http_tracker_config in &config.http_trackers {
79-
if !http_tracker_config.enabled {
80-
continue;
81-
}
78+
for plain_http_tracker_config in &config.http_trackers {
79+
match torrust_tracker_configuration::http_tracker::Config::try_from(plain_http_tracker_config.clone()) {
80+
Ok(http_tracker_config) => {
81+
if !http_tracker_config.is_enabled() {
82+
continue;
83+
}
8284

83-
if let Some(job) = http_tracker::start_job(
84-
http_tracker_config,
85-
tracker.clone(),
86-
registar.give_form(),
87-
servers::http::Version::V1,
88-
)
89-
.await
90-
{
91-
jobs.push(job);
92-
};
85+
if let Some(job) = http_tracker::start_job(
86+
&http_tracker_config.into(),
87+
tracker.clone(),
88+
registar.give_form(),
89+
servers::http::Version::V1,
90+
)
91+
.await
92+
{
93+
jobs.push(job);
94+
};
95+
}
96+
Err(err) => panic!("Invalid HTTP Tracker configuration: {err}"),
97+
}
9398
}
9499

95100
// Start HTTP API

tests/servers/api/v1/contract/configuration.rs

-33
This file was deleted.

tests/servers/api/v1/contract/mod.rs

-1
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
11
pub mod authentication;
2-
pub mod configuration;
32
pub mod context;
43
pub mod fixtures;

0 commit comments

Comments
 (0)