From 384e9f820ea685d6f4d2cc9639b83df478b6b359 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Thu, 9 May 2024 17:19:30 +0100 Subject: [PATCH 1/8] refactor: [#852] eenrich field types in HealthCheckApi config struct --- packages/configuration/src/v1/health_check_api.rs | 6 ++++-- packages/test-helpers/src/configuration.rs | 4 ++-- src/bootstrap/jobs/health_check_api.rs | 5 +---- tests/servers/health_check_api/environment.rs | 5 +---- 4 files changed, 8 insertions(+), 12 deletions(-) diff --git a/packages/configuration/src/v1/health_check_api.rs b/packages/configuration/src/v1/health_check_api.rs index 1c2cd073a..b8bfd2c1b 100644 --- a/packages/configuration/src/v1/health_check_api.rs +++ b/packages/configuration/src/v1/health_check_api.rs @@ -1,3 +1,5 @@ +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + use serde::{Deserialize, Serialize}; use serde_with::serde_as; @@ -9,13 +11,13 @@ pub struct HealthCheckApi { /// The format is `ip:port`, for example `127.0.0.1:1313`. If you want to /// listen to all interfaces, use `0.0.0.0`. If you want the operating /// system to choose a random port, use port `0`. - pub bind_address: String, + pub bind_address: SocketAddr, } impl Default for HealthCheckApi { fn default() -> Self { Self { - bind_address: String::from("127.0.0.1:1313"), + bind_address: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 1313), } } } diff --git a/packages/test-helpers/src/configuration.rs b/packages/test-helpers/src/configuration.rs index 49cfdd390..c1ee95197 100644 --- a/packages/test-helpers/src/configuration.rs +++ b/packages/test-helpers/src/configuration.rs @@ -1,6 +1,6 @@ //! Tracker configuration factories for testing. use std::env; -use std::net::IpAddr; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use torrust_tracker_configuration::Configuration; use torrust_tracker_primitives::TrackerMode; @@ -39,7 +39,7 @@ pub fn ephemeral() -> Configuration { // Ephemeral socket address for Health Check API let health_check_api_port = 0u16; - config.health_check_api.bind_address = format!("127.0.0.1:{}", &health_check_api_port); + config.health_check_api.bind_address = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), health_check_api_port); // Ephemeral socket address for UDP tracker let udp_port = 0u16; diff --git a/src/bootstrap/jobs/health_check_api.rs b/src/bootstrap/jobs/health_check_api.rs index eec4d81a8..fdedaa3e9 100644 --- a/src/bootstrap/jobs/health_check_api.rs +++ b/src/bootstrap/jobs/health_check_api.rs @@ -35,10 +35,7 @@ use crate::servers::signals::Halted; /// /// It would panic if unable to send the `ApiServerJobStarted` notice. pub async fn start_job(config: &HealthCheckApi, register: ServiceRegistry) -> JoinHandle<()> { - let bind_addr = config - .bind_address - .parse::() - .expect("it should have a valid health check bind address"); + let bind_addr = config.bind_address; let (tx_start, rx_start) = oneshot::channel::(); let (tx_halt, rx_halt) = tokio::sync::oneshot::channel::(); diff --git a/tests/servers/health_check_api/environment.rs b/tests/servers/health_check_api/environment.rs index 0856985d5..c200beaeb 100644 --- a/tests/servers/health_check_api/environment.rs +++ b/tests/servers/health_check_api/environment.rs @@ -33,10 +33,7 @@ pub struct Environment { impl Environment { pub fn new(config: &Arc, registar: Registar) -> Self { - let bind_to = config - .bind_address - .parse::() - .expect("Tracker API bind_address invalid."); + let bind_to = config.bind_address; Self { registar, From 1475eadd25e7fbf208523f72f49188d3f28173c2 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 10 May 2024 11:12:53 +0100 Subject: [PATCH 2/8] refactor: [#852] eenrich field types in UdpTracker config struct --- packages/configuration/src/v1/udp_tracker.rs | 6 ++++-- packages/test-helpers/src/configuration.rs | 10 +++++----- src/bootstrap/jobs/udp_tracker.rs | 5 +---- src/servers/udp/server.rs | 9 +++------ tests/servers/udp/environment.rs | 5 +---- 5 files changed, 14 insertions(+), 21 deletions(-) diff --git a/packages/configuration/src/v1/udp_tracker.rs b/packages/configuration/src/v1/udp_tracker.rs index 254272bdd..1f772164e 100644 --- a/packages/configuration/src/v1/udp_tracker.rs +++ b/packages/configuration/src/v1/udp_tracker.rs @@ -1,3 +1,5 @@ +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + use serde::{Deserialize, Serialize}; #[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] @@ -8,13 +10,13 @@ pub struct UdpTracker { /// The format is `ip:port`, for example `0.0.0.0:6969`. If you want to /// listen to all interfaces, use `0.0.0.0`. If you want the operating /// system to choose a random port, use port `0`. - pub bind_address: String, + pub bind_address: SocketAddr, } impl Default for UdpTracker { fn default() -> Self { Self { enabled: false, - bind_address: String::from("0.0.0.0:6969"), + bind_address: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 6969), } } } diff --git a/packages/test-helpers/src/configuration.rs b/packages/test-helpers/src/configuration.rs index c1ee95197..c28cf553e 100644 --- a/packages/test-helpers/src/configuration.rs +++ b/packages/test-helpers/src/configuration.rs @@ -1,6 +1,6 @@ //! Tracker configuration factories for testing. use std::env; -use std::net::{IpAddr, Ipv4Addr, SocketAddr}; +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; use torrust_tracker_configuration::Configuration; use torrust_tracker_primitives::TrackerMode; @@ -44,7 +44,7 @@ pub fn ephemeral() -> Configuration { // Ephemeral socket address for UDP tracker let udp_port = 0u16; config.udp_trackers[0].enabled = true; - config.udp_trackers[0].bind_address = format!("127.0.0.1:{}", &udp_port); + config.udp_trackers[0].bind_address = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), udp_port); // Ephemeral socket address for HTTP tracker let http_port = 0u16; @@ -136,10 +136,10 @@ pub fn ephemeral_with_external_ip(ip: IpAddr) -> Configuration { pub fn ephemeral_ipv6() -> Configuration { let mut cfg = ephemeral(); - let ipv6 = format!("[::]:{}", 0); + let ipv6 = SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0)), 0); - cfg.http_api.bind_address.clone_from(&ipv6); - cfg.http_trackers[0].bind_address.clone_from(&ipv6); + cfg.http_api.bind_address.clone_from(&ipv6.to_string()); + cfg.http_trackers[0].bind_address.clone_from(&ipv6.to_string()); cfg.udp_trackers[0].bind_address = ipv6; cfg diff --git a/src/bootstrap/jobs/udp_tracker.rs b/src/bootstrap/jobs/udp_tracker.rs index e9e4bc642..bb1cdb492 100644 --- a/src/bootstrap/jobs/udp_tracker.rs +++ b/src/bootstrap/jobs/udp_tracker.rs @@ -27,10 +27,7 @@ use crate::servers::udp::server::{Launcher, UdpServer}; /// It will panic if the task did not finish successfully. #[must_use] pub async fn start_job(config: &UdpTracker, tracker: Arc, form: ServiceRegistrationForm) -> JoinHandle<()> { - let bind_to = config - .bind_address - .parse::() - .expect("it should have a valid udp tracker bind address"); + let bind_to = config.bind_address; let server = UdpServer::new(Launcher::new(bind_to)) .start(tracker, form) diff --git a/src/servers/udp/server.rs b/src/servers/udp/server.rs index f7092f377..b02b9802d 100644 --- a/src/servers/udp/server.rs +++ b/src/servers/udp/server.rs @@ -463,19 +463,16 @@ mod tests { let cfg = Arc::new(ephemeral_mode_public()); let tracker = initialize_with_configuration(&cfg); let config = &cfg.udp_trackers[0]; - - let bind_to = config - .bind_address - .parse::() - .expect("Tracker API bind_address invalid."); - + let bind_to = config.bind_address; let register = &Registar::default(); let stopped = UdpServer::new(Launcher::new(bind_to)); + let started = stopped .start(tracker, register.give_form()) .await .expect("it should start the server"); + let stopped = started.stop().await.expect("it should stop the server"); tokio::time::sleep(Duration::from_secs(1)).await; diff --git a/tests/servers/udp/environment.rs b/tests/servers/udp/environment.rs index 6ced1dbb7..c1fecbdd3 100644 --- a/tests/servers/udp/environment.rs +++ b/tests/servers/udp/environment.rs @@ -31,10 +31,7 @@ impl Environment { let config = Arc::new(configuration.udp_trackers[0].clone()); - let bind_to = config - .bind_address - .parse::() - .expect("Tracker API bind_address invalid."); + let bind_to = config.bind_address; let server = UdpServer::new(Launcher::new(bind_to)); From fc191f7b6f8eef730665f16739e016ed1b4b5820 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 10 May 2024 11:26:04 +0100 Subject: [PATCH 3/8] refactor: [#852] enrich field types in HttpTracker config struct If the next major config version the `TslConfig` shoud always contain valid file paths and the whole field should be optional in the parent strcut `HttpTracker`: ```rust pub struct HttpTracker { pub enabled: bool, pub bind_address: SocketAddr, pub ssl_enabled: bool, #[serde(flatten)] pub tsl_config: Optional, } pub struct TslConfig { pub ssl_cert_path: PathBuf, pub ssl_key_path: PathBuf, } ``` That mean, the user could provide it or not, but if it's provided file paths can't be empty. --- packages/configuration/src/lib.rs | 13 +++++++++++ packages/configuration/src/v1/http_tracker.rs | 22 +++++++++---------- packages/test-helpers/src/configuration.rs | 4 ++-- src/bootstrap/jobs/http_tracker.rs | 15 +++++++------ src/servers/http/server.rs | 17 +++++++------- tests/servers/http/environment.rs | 13 ++++++----- 6 files changed, 50 insertions(+), 34 deletions(-) diff --git a/packages/configuration/src/lib.rs b/packages/configuration/src/lib.rs index 20912990a..f1e316f10 100644 --- a/packages/configuration/src/lib.rs +++ b/packages/configuration/src/lib.rs @@ -11,6 +11,8 @@ use std::sync::Arc; use std::{env, fs}; use derive_more::Constructor; +use serde::{Deserialize, Serialize}; +use serde_with::{serde_as, NoneAsEmptyString}; use thiserror::Error; use torrust_tracker_located_error::{DynError, LocatedError}; @@ -157,3 +159,14 @@ impl From for Error { } } } + +#[serde_as] +#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone, Default)] +pub struct TslConfig { + /// Path to the SSL certificate file. + #[serde_as(as = "NoneAsEmptyString")] + pub ssl_cert_path: Option, + /// Path to the SSL key file. + #[serde_as(as = "NoneAsEmptyString")] + pub ssl_key_path: Option, +} diff --git a/packages/configuration/src/v1/http_tracker.rs b/packages/configuration/src/v1/http_tracker.rs index c2d5928e2..57b2d83a1 100644 --- a/packages/configuration/src/v1/http_tracker.rs +++ b/packages/configuration/src/v1/http_tracker.rs @@ -1,5 +1,9 @@ +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; + use serde::{Deserialize, Serialize}; -use serde_with::{serde_as, NoneAsEmptyString}; +use serde_with::serde_as; + +use crate::TslConfig; /// Configuration for each HTTP tracker. #[serde_as] @@ -11,25 +15,21 @@ pub struct HttpTracker { /// The format is `ip:port`, for example `0.0.0.0:6969`. If you want to /// listen to all interfaces, use `0.0.0.0`. If you want the operating /// system to choose a random port, use port `0`. - pub bind_address: String, + pub bind_address: SocketAddr, /// Weather the HTTP tracker will use SSL or not. pub ssl_enabled: bool, - /// Path to the SSL certificate file. Only used if `ssl_enabled` is `true`. - #[serde_as(as = "NoneAsEmptyString")] - pub ssl_cert_path: Option, - /// Path to the SSL key file. Only used if `ssl_enabled` is `true`. - #[serde_as(as = "NoneAsEmptyString")] - pub ssl_key_path: Option, + /// TSL config. Only used if `ssl_enabled` is true. + #[serde(flatten)] + pub tsl_config: TslConfig, } impl Default for HttpTracker { fn default() -> Self { Self { enabled: false, - bind_address: String::from("0.0.0.0:7070"), + bind_address: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)), 7070), ssl_enabled: false, - ssl_cert_path: None, - ssl_key_path: None, + tsl_config: TslConfig::default(), } } } diff --git a/packages/test-helpers/src/configuration.rs b/packages/test-helpers/src/configuration.rs index c28cf553e..e6f53f85b 100644 --- a/packages/test-helpers/src/configuration.rs +++ b/packages/test-helpers/src/configuration.rs @@ -49,7 +49,7 @@ pub fn ephemeral() -> Configuration { // Ephemeral socket address for HTTP tracker let http_port = 0u16; config.http_trackers[0].enabled = true; - config.http_trackers[0].bind_address = format!("127.0.0.1:{}", &http_port); + config.http_trackers[0].bind_address = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), http_port); // Ephemeral sqlite database let temp_directory = env::temp_dir(); @@ -139,7 +139,7 @@ pub fn ephemeral_ipv6() -> Configuration { let ipv6 = SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0)), 0); cfg.http_api.bind_address.clone_from(&ipv6.to_string()); - cfg.http_trackers[0].bind_address.clone_from(&ipv6.to_string()); + cfg.http_trackers[0].bind_address.clone_from(&ipv6); cfg.udp_trackers[0].bind_address = ipv6; cfg diff --git a/src/bootstrap/jobs/http_tracker.rs b/src/bootstrap/jobs/http_tracker.rs index 0a0638b78..88d643149 100644 --- a/src/bootstrap/jobs/http_tracker.rs +++ b/src/bootstrap/jobs/http_tracker.rs @@ -40,14 +40,15 @@ pub async fn start_job( version: Version, ) -> Option> { if config.enabled { - let socket = config - .bind_address - .parse::() - .expect("it should have a valid http tracker bind address"); + let socket = config.bind_address; - let tls = make_rust_tls(config.ssl_enabled, &config.ssl_cert_path, &config.ssl_key_path) - .await - .map(|tls| tls.expect("it should have a valid http tracker tls configuration")); + let tls = make_rust_tls( + config.ssl_enabled, + &config.tsl_config.ssl_cert_path, + &config.tsl_config.ssl_key_path, + ) + .await + .map(|tls| tls.expect("it should have a valid http tracker tls configuration")); match version { Version::V1 => Some(start_v1(socket, tls, tracker.clone(), form).await), diff --git a/src/servers/http/server.rs b/src/servers/http/server.rs index decc734c5..5791708ec 100644 --- a/src/servers/http/server.rs +++ b/src/servers/http/server.rs @@ -234,14 +234,15 @@ mod tests { let tracker = initialize_with_configuration(&cfg); let config = &cfg.http_trackers[0]; - let bind_to = config - .bind_address - .parse::() - .expect("Tracker API bind_address invalid."); - - let tls = make_rust_tls(config.ssl_enabled, &config.ssl_cert_path, &config.ssl_key_path) - .await - .map(|tls| tls.expect("tls config failed")); + let bind_to = config.bind_address; + + let tls = make_rust_tls( + config.ssl_enabled, + &config.tsl_config.ssl_cert_path, + &config.tsl_config.ssl_key_path, + ) + .await + .map(|tls| tls.expect("tls config failed")); let register = &Registar::default(); diff --git a/tests/servers/http/environment.rs b/tests/servers/http/environment.rs index f00da293e..e3aa6641e 100644 --- a/tests/servers/http/environment.rs +++ b/tests/servers/http/environment.rs @@ -31,13 +31,14 @@ impl Environment { let config = Arc::new(configuration.http_trackers[0].clone()); - let bind_to = config - .bind_address - .parse::() - .expect("Tracker API bind_address invalid."); + let bind_to = config.bind_address; - let tls = block_on(make_rust_tls(config.ssl_enabled, &config.ssl_cert_path, &config.ssl_key_path)) - .map(|tls| tls.expect("tls config failed")); + let tls = block_on(make_rust_tls( + config.ssl_enabled, + &config.tsl_config.ssl_cert_path, + &config.tsl_config.ssl_key_path, + )) + .map(|tls| tls.expect("tls config failed")); let server = HttpServer::new(Launcher::new(bind_to, tls)); From a2e718bb1d56b5977f53bed0e10b677db4b4e63e Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 10 May 2024 12:00:24 +0100 Subject: [PATCH 4/8] chore(deps): add dependency camino We are using `String` to represent a filepath. We are refactoring to enrich types in configuration. Filepath should be represented with `PathBuf` but it allows non UTF-8 chars, so it can't be serialized. Since we need to serialize config options (toml, json) is valid UTF-8 strings, we need a type that represents a valid fileptah but also is a valid UTF-8 strings. That makes impossible to use non-UTF8 fielpath. It seems that's a restriction accepted for a lot of projects including Rust `cargo`. --- Cargo.lock | 11 +++++++++++ Cargo.toml | 1 + cSpell.json | 1 + packages/configuration/Cargo.toml | 1 + 4 files changed, 14 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 0bbf0205a..e54601bcf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -720,6 +720,15 @@ version = "1.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "514de17de45fdb8dc022b1a7975556c53c86f9f0aa5f534b98977b171857c2c9" +[[package]] +name = "camino" +version = "1.1.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c59e92b5a388f549b863a7bea62612c09f24c8393560709a54558a9abdfb3b9c" +dependencies = [ + "serde", +] + [[package]] name = "cast" version = "0.3.0" @@ -3880,6 +3889,7 @@ dependencies = [ "axum-client-ip", "axum-extra", "axum-server", + "camino", "chrono", "clap", "crossbeam-skiplist", @@ -3938,6 +3948,7 @@ dependencies = [ name = "torrust-tracker-configuration" version = "3.0.0-alpha.12-develop" dependencies = [ + "camino", "derive_more", "figment", "serde", diff --git a/Cargo.toml b/Cargo.toml index d7aa9a31c..60652b160 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -37,6 +37,7 @@ axum = { version = "0", features = ["macros"] } axum-client-ip = "0" axum-extra = { version = "0", features = ["query"] } axum-server = { version = "0", features = ["tls-rustls"] } +camino = { version = "1.1.6", features = ["serde"] } chrono = { version = "0", default-features = false, features = ["clock"] } clap = { version = "4", features = ["derive", "env"] } crossbeam-skiplist = "0.1" diff --git a/cSpell.json b/cSpell.json index 2473e9c33..bd6c9d489 100644 --- a/cSpell.json +++ b/cSpell.json @@ -25,6 +25,7 @@ "Buildx", "byteorder", "callgrind", + "camino", "canonicalize", "canonicalized", "certbot", diff --git a/packages/configuration/Cargo.toml b/packages/configuration/Cargo.toml index a033dcea1..bac2132d5 100644 --- a/packages/configuration/Cargo.toml +++ b/packages/configuration/Cargo.toml @@ -15,6 +15,7 @@ rust-version.workspace = true version.workspace = true [dependencies] +camino = { version = "1.1.6", features = ["serde"] } derive_more = "0" figment = { version = "0.10.18", features = ["env", "test", "toml"] } serde = { version = "1", features = ["derive"] } From 3997cfa256a944b30b55f2d94ae5dbcf0670a696 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 10 May 2024 12:06:32 +0100 Subject: [PATCH 5/8] refactor: [#852] eenrich field types in TslConfig config struct --- packages/configuration/src/lib.rs | 5 +++-- src/bootstrap/jobs/http_tracker.rs | 4 ++-- src/bootstrap/jobs/mod.rs | 31 +++++++++++++++++++++++++++++- src/servers/http/server.rs | 4 ++-- tests/servers/http/environment.rs | 4 ++-- 5 files changed, 39 insertions(+), 9 deletions(-) diff --git a/packages/configuration/src/lib.rs b/packages/configuration/src/lib.rs index f1e316f10..239803f47 100644 --- a/packages/configuration/src/lib.rs +++ b/packages/configuration/src/lib.rs @@ -10,6 +10,7 @@ use std::collections::HashMap; use std::sync::Arc; use std::{env, fs}; +use camino::Utf8PathBuf; use derive_more::Constructor; use serde::{Deserialize, Serialize}; use serde_with::{serde_as, NoneAsEmptyString}; @@ -165,8 +166,8 @@ impl From for Error { pub struct TslConfig { /// Path to the SSL certificate file. #[serde_as(as = "NoneAsEmptyString")] - pub ssl_cert_path: Option, + pub ssl_cert_path: Option, /// Path to the SSL key file. #[serde_as(as = "NoneAsEmptyString")] - pub ssl_key_path: Option, + pub ssl_key_path: Option, } diff --git a/src/bootstrap/jobs/http_tracker.rs b/src/bootstrap/jobs/http_tracker.rs index 88d643149..d9e8bdabe 100644 --- a/src/bootstrap/jobs/http_tracker.rs +++ b/src/bootstrap/jobs/http_tracker.rs @@ -18,7 +18,7 @@ use log::info; use tokio::task::JoinHandle; use torrust_tracker_configuration::HttpTracker; -use super::make_rust_tls; +use super::make_rust_tls_from_path_buf; use crate::core; use crate::servers::http::server::{HttpServer, Launcher}; use crate::servers::http::Version; @@ -42,7 +42,7 @@ pub async fn start_job( if config.enabled { let socket = config.bind_address; - let tls = make_rust_tls( + let tls = make_rust_tls_from_path_buf( config.ssl_enabled, &config.tsl_config.ssl_cert_path, &config.tsl_config.ssl_key_path, diff --git a/src/bootstrap/jobs/mod.rs b/src/bootstrap/jobs/mod.rs index 2c12eb40e..dd855f7c6 100644 --- a/src/bootstrap/jobs/mod.rs +++ b/src/bootstrap/jobs/mod.rs @@ -28,7 +28,35 @@ pub async fn make_rust_tls(enabled: bool, cert: &Option, key: &Option, + key: &Option, +) -> Option> { + if !enabled { + info!("TLS not enabled"); + return None; + } + + if let (Some(cert), Some(key)) = (cert, key) { + info!("Using https: cert path: {cert}."); + info!("Using https: key path: {key}."); Some( RustlsConfig::from_pem_file(cert, key) @@ -77,6 +105,7 @@ use std::panic::Location; use std::sync::Arc; use axum_server::tls_rustls::RustlsConfig; +use camino::Utf8PathBuf; use log::info; use thiserror::Error; use torrust_tracker_located_error::{DynError, LocatedError}; diff --git a/src/servers/http/server.rs b/src/servers/http/server.rs index 5791708ec..a6f96634f 100644 --- a/src/servers/http/server.rs +++ b/src/servers/http/server.rs @@ -224,7 +224,7 @@ mod tests { use torrust_tracker_test_helpers::configuration::ephemeral_mode_public; use crate::bootstrap::app::initialize_with_configuration; - use crate::bootstrap::jobs::make_rust_tls; + use crate::bootstrap::jobs::make_rust_tls_from_path_buf; use crate::servers::http::server::{HttpServer, Launcher}; use crate::servers::registar::Registar; @@ -236,7 +236,7 @@ mod tests { let bind_to = config.bind_address; - let tls = make_rust_tls( + let tls = make_rust_tls_from_path_buf( config.ssl_enabled, &config.tsl_config.ssl_cert_path, &config.tsl_config.ssl_key_path, diff --git a/tests/servers/http/environment.rs b/tests/servers/http/environment.rs index e3aa6641e..e662cea7c 100644 --- a/tests/servers/http/environment.rs +++ b/tests/servers/http/environment.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use futures::executor::block_on; use torrust_tracker::bootstrap::app::initialize_with_configuration; -use torrust_tracker::bootstrap::jobs::make_rust_tls; +use torrust_tracker::bootstrap::jobs::make_rust_tls_from_path_buf; use torrust_tracker::core::Tracker; use torrust_tracker::servers::http::server::{HttpServer, Launcher, Running, Stopped}; use torrust_tracker::servers::registar::Registar; @@ -33,7 +33,7 @@ impl Environment { let bind_to = config.bind_address; - let tls = block_on(make_rust_tls( + let tls = block_on(make_rust_tls_from_path_buf( config.ssl_enabled, &config.tsl_config.ssl_cert_path, &config.tsl_config.ssl_key_path, From ceb30747cec6372b5244f797ad032c7db8c03d6b Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 10 May 2024 12:49:57 +0100 Subject: [PATCH 6/8] refactor: [#852] enrich field types in HttpApi config struct --- packages/configuration/src/v1/tracker_api.rs | 23 ++++--- packages/test-helpers/src/configuration.rs | 4 +- src/bootstrap/jobs/http_tracker.rs | 12 ++-- src/bootstrap/jobs/mod.rs | 68 ++++++++------------ src/bootstrap/jobs/tracker_apis.rs | 7 +- src/servers/apis/server.rs | 7 +- src/servers/http/server.rs | 12 ++-- tests/servers/api/environment.rs | 8 +-- tests/servers/http/environment.rs | 9 +-- 9 files changed, 56 insertions(+), 94 deletions(-) diff --git a/packages/configuration/src/v1/tracker_api.rs b/packages/configuration/src/v1/tracker_api.rs index 8749478c8..5089c496a 100644 --- a/packages/configuration/src/v1/tracker_api.rs +++ b/packages/configuration/src/v1/tracker_api.rs @@ -1,7 +1,10 @@ use std::collections::HashMap; +use std::net::{IpAddr, Ipv4Addr, SocketAddr}; use serde::{Deserialize, Serialize}; -use serde_with::{serde_as, NoneAsEmptyString}; +use serde_with::serde_as; + +use crate::TslConfig; pub type AccessTokens = HashMap; @@ -15,19 +18,16 @@ pub struct HttpApi { /// The format is `ip:port`, for example `0.0.0.0:6969`. If you want to /// listen to all interfaces, use `0.0.0.0`. If you want the operating /// system to choose a random port, use port `0`. - pub bind_address: String, + pub bind_address: SocketAddr, /// Weather the HTTP API will use SSL or not. pub ssl_enabled: bool, - /// Path to the SSL certificate file. Only used if `ssl_enabled` is `true`. - #[serde_as(as = "NoneAsEmptyString")] - pub ssl_cert_path: Option, - /// Path to the SSL key file. Only used if `ssl_enabled` is `true`. - #[serde_as(as = "NoneAsEmptyString")] - pub ssl_key_path: Option, + /// TSL config. Only used if `ssl_enabled` is true. + #[serde(flatten)] + pub tsl_config: TslConfig, /// Access tokens for the HTTP API. The key is a label identifying the /// token and the value is the token itself. The token is used to /// authenticate the user. All tokens are valid for all endpoints and have - /// the all permissions. + /// all permissions. pub access_tokens: AccessTokens, } @@ -35,10 +35,9 @@ impl Default for HttpApi { fn default() -> Self { Self { enabled: true, - bind_address: String::from("127.0.0.1:1212"), + bind_address: SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 1212), ssl_enabled: false, - ssl_cert_path: None, - ssl_key_path: None, + tsl_config: TslConfig::default(), access_tokens: [(String::from("admin"), String::from("MyAccessToken"))] .iter() .cloned() diff --git a/packages/test-helpers/src/configuration.rs b/packages/test-helpers/src/configuration.rs index e6f53f85b..08f570dc6 100644 --- a/packages/test-helpers/src/configuration.rs +++ b/packages/test-helpers/src/configuration.rs @@ -35,7 +35,7 @@ pub fn ephemeral() -> Configuration { // Ephemeral socket address for API let api_port = 0u16; config.http_api.enabled = true; - config.http_api.bind_address = format!("127.0.0.1:{}", &api_port); + config.http_api.bind_address = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), api_port); // Ephemeral socket address for Health Check API let health_check_api_port = 0u16; @@ -138,7 +138,7 @@ pub fn ephemeral_ipv6() -> Configuration { let ipv6 = SocketAddr::new(IpAddr::V6(Ipv6Addr::new(0, 0, 0, 0, 0, 0, 0, 0)), 0); - cfg.http_api.bind_address.clone_from(&ipv6.to_string()); + cfg.http_api.bind_address.clone_from(&ipv6); cfg.http_trackers[0].bind_address.clone_from(&ipv6); cfg.udp_trackers[0].bind_address = ipv6; diff --git a/src/bootstrap/jobs/http_tracker.rs b/src/bootstrap/jobs/http_tracker.rs index d9e8bdabe..d8a976b98 100644 --- a/src/bootstrap/jobs/http_tracker.rs +++ b/src/bootstrap/jobs/http_tracker.rs @@ -18,7 +18,7 @@ use log::info; use tokio::task::JoinHandle; use torrust_tracker_configuration::HttpTracker; -use super::make_rust_tls_from_path_buf; +use super::make_rust_tls; use crate::core; use crate::servers::http::server::{HttpServer, Launcher}; use crate::servers::http::Version; @@ -42,13 +42,9 @@ pub async fn start_job( if config.enabled { let socket = config.bind_address; - let tls = make_rust_tls_from_path_buf( - config.ssl_enabled, - &config.tsl_config.ssl_cert_path, - &config.tsl_config.ssl_key_path, - ) - .await - .map(|tls| tls.expect("it should have a valid http tracker tls configuration")); + let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config) + .await + .map(|tls| tls.expect("it should have a valid http tracker tls configuration")); match version { Version::V1 => Some(start_v1(socket, tls, tracker.clone(), form).await), diff --git a/src/bootstrap/jobs/mod.rs b/src/bootstrap/jobs/mod.rs index dd855f7c6..d288989b5 100644 --- a/src/bootstrap/jobs/mod.rs +++ b/src/bootstrap/jobs/mod.rs @@ -20,41 +20,13 @@ pub struct Started { pub address: std::net::SocketAddr, } -pub async fn make_rust_tls(enabled: bool, cert: &Option, key: &Option) -> Option> { +pub async fn make_rust_tls(enabled: bool, tsl_config: &TslConfig) -> Option> { if !enabled { info!("TLS not enabled"); return None; } - if let (Some(cert), Some(key)) = (cert, key) { - info!("Using https: cert path: {cert}."); - info!("Using https: key path: {key}."); - - Some( - RustlsConfig::from_pem_file(cert, key) - .await - .map_err(|err| Error::BadTlsConfig { - source: (Arc::new(err) as DynError).into(), - }), - ) - } else { - Some(Err(Error::MissingTlsConfig { - location: Location::caller(), - })) - } -} - -pub async fn make_rust_tls_from_path_buf( - enabled: bool, - cert: &Option, - key: &Option, -) -> Option> { - if !enabled { - info!("TLS not enabled"); - return None; - } - - if let (Some(cert), Some(key)) = (cert, key) { + if let (Some(cert), Some(key)) = (tsl_config.ssl_cert_path.clone(), tsl_config.ssl_key_path.clone()) { info!("Using https: cert path: {cert}."); info!("Using https: key path: {key}."); @@ -75,15 +47,23 @@ pub async fn make_rust_tls_from_path_buf( #[cfg(test)] mod tests { + use camino::Utf8PathBuf; + use torrust_tracker_configuration::TslConfig; + use super::make_rust_tls; #[tokio::test] async fn it_should_error_on_bad_tls_config() { - let (bad_cert_path, bad_key_path) = (Some("bad cert path".to_string()), Some("bad key path".to_string())); - let err = make_rust_tls(true, &bad_cert_path, &bad_key_path) - .await - .expect("tls_was_enabled") - .expect_err("bad_cert_and_key_files"); + let err = make_rust_tls( + true, + &TslConfig { + ssl_cert_path: Some(Utf8PathBuf::from("bad cert path")), + ssl_key_path: Some(Utf8PathBuf::from("bad key path")), + }, + ) + .await + .expect("tls_was_enabled") + .expect_err("bad_cert_and_key_files"); assert!(err .to_string() @@ -91,11 +71,17 @@ mod tests { } #[tokio::test] - async fn it_should_error_on_missing_tls_config() { - let err = make_rust_tls(true, &None, &None) - .await - .expect("tls_was_enabled") - .expect_err("missing_config"); + async fn it_should_error_on_missing_cert_or_key_paths() { + let err = make_rust_tls( + true, + &TslConfig { + ssl_cert_path: None, + ssl_key_path: None, + }, + ) + .await + .expect("tls_was_enabled") + .expect_err("missing_config"); assert_eq!(err.to_string(), "tls config missing"); } @@ -105,9 +91,9 @@ use std::panic::Location; use std::sync::Arc; use axum_server::tls_rustls::RustlsConfig; -use camino::Utf8PathBuf; use log::info; use thiserror::Error; +use torrust_tracker_configuration::TslConfig; use torrust_tracker_located_error::{DynError, LocatedError}; /// Error returned by the Bootstrap Process. diff --git a/src/bootstrap/jobs/tracker_apis.rs b/src/bootstrap/jobs/tracker_apis.rs index ffd7c7407..120c960ef 100644 --- a/src/bootstrap/jobs/tracker_apis.rs +++ b/src/bootstrap/jobs/tracker_apis.rs @@ -61,12 +61,9 @@ pub async fn start_job( version: Version, ) -> Option> { if config.enabled { - let bind_to = config - .bind_address - .parse::() - .expect("it should have a valid tracker api bind address"); + let bind_to = config.bind_address; - let tls = make_rust_tls(config.ssl_enabled, &config.ssl_cert_path, &config.ssl_key_path) + let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config) .await .map(|tls| tls.expect("it should have a valid tracker api tls configuration")); diff --git a/src/servers/apis/server.rs b/src/servers/apis/server.rs index e72890557..9317d6ec0 100644 --- a/src/servers/apis/server.rs +++ b/src/servers/apis/server.rs @@ -275,12 +275,9 @@ mod tests { let tracker = initialize_with_configuration(&cfg); - let bind_to = config - .bind_address - .parse::() - .expect("Tracker API bind_address invalid."); + let bind_to = config.bind_address; - let tls = make_rust_tls(config.ssl_enabled, &config.ssl_cert_path, &config.ssl_key_path) + let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config) .await .map(|tls| tls.expect("tls config failed")); diff --git a/src/servers/http/server.rs b/src/servers/http/server.rs index a6f96634f..7c8148f22 100644 --- a/src/servers/http/server.rs +++ b/src/servers/http/server.rs @@ -224,7 +224,7 @@ mod tests { use torrust_tracker_test_helpers::configuration::ephemeral_mode_public; use crate::bootstrap::app::initialize_with_configuration; - use crate::bootstrap::jobs::make_rust_tls_from_path_buf; + use crate::bootstrap::jobs::make_rust_tls; use crate::servers::http::server::{HttpServer, Launcher}; use crate::servers::registar::Registar; @@ -236,13 +236,9 @@ mod tests { let bind_to = config.bind_address; - let tls = make_rust_tls_from_path_buf( - config.ssl_enabled, - &config.tsl_config.ssl_cert_path, - &config.tsl_config.ssl_key_path, - ) - .await - .map(|tls| tls.expect("tls config failed")); + let tls = make_rust_tls(config.ssl_enabled, &config.tsl_config) + .await + .map(|tls| tls.expect("tls config failed")); let register = &Registar::default(); diff --git a/tests/servers/api/environment.rs b/tests/servers/api/environment.rs index dec4ccff2..cacde8af9 100644 --- a/tests/servers/api/environment.rs +++ b/tests/servers/api/environment.rs @@ -33,13 +33,9 @@ impl Environment { let config = Arc::new(configuration.http_api.clone()); - let bind_to = config - .bind_address - .parse::() - .expect("Tracker API bind_address invalid."); + let bind_to = config.bind_address; - let tls = block_on(make_rust_tls(config.ssl_enabled, &config.ssl_cert_path, &config.ssl_key_path)) - .map(|tls| tls.expect("tls config failed")); + let tls = block_on(make_rust_tls(config.ssl_enabled, &config.tsl_config)).map(|tls| tls.expect("tls config failed")); let server = ApiServer::new(Launcher::new(bind_to, tls)); diff --git a/tests/servers/http/environment.rs b/tests/servers/http/environment.rs index e662cea7c..61837c40f 100644 --- a/tests/servers/http/environment.rs +++ b/tests/servers/http/environment.rs @@ -2,7 +2,7 @@ use std::sync::Arc; use futures::executor::block_on; use torrust_tracker::bootstrap::app::initialize_with_configuration; -use torrust_tracker::bootstrap::jobs::make_rust_tls_from_path_buf; +use torrust_tracker::bootstrap::jobs::make_rust_tls; use torrust_tracker::core::Tracker; use torrust_tracker::servers::http::server::{HttpServer, Launcher, Running, Stopped}; use torrust_tracker::servers::registar::Registar; @@ -33,12 +33,7 @@ impl Environment { let bind_to = config.bind_address; - let tls = block_on(make_rust_tls_from_path_buf( - config.ssl_enabled, - &config.tsl_config.ssl_cert_path, - &config.tsl_config.ssl_key_path, - )) - .map(|tls| tls.expect("tls config failed")); + let tls = block_on(make_rust_tls(config.ssl_enabled, &config.tsl_config)).map(|tls| tls.expect("tls config failed")); let server = HttpServer::new(Launcher::new(bind_to, tls)); From 7519ecc70052555dcaf9b59f533327d190649cd6 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 10 May 2024 13:47:47 +0100 Subject: [PATCH 7/8] refactor: [#852] enrich field types in Configuration struct --- packages/configuration/src/lib.rs | 17 +++++++++++++++ packages/configuration/src/v1/mod.rs | 25 +++++++++------------- packages/test-helpers/src/configuration.rs | 6 +++--- src/bootstrap/logging.rs | 14 ++++++++---- src/servers/apis/mod.rs | 4 ++-- src/servers/http/v1/services/announce.rs | 5 +++-- src/servers/udp/handlers.rs | 2 +- 7 files changed, 46 insertions(+), 27 deletions(-) diff --git a/packages/configuration/src/lib.rs b/packages/configuration/src/lib.rs index 239803f47..85867816c 100644 --- a/packages/configuration/src/lib.rs +++ b/packages/configuration/src/lib.rs @@ -171,3 +171,20 @@ pub struct TslConfig { #[serde_as(as = "NoneAsEmptyString")] pub ssl_key_path: Option, } + +#[derive(Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] +#[serde(rename_all = "lowercase")] +pub enum LogLevel { + /// A level lower than all log levels. + Off, + /// Corresponds to the `Error` log level. + Error, + /// Corresponds to the `Warn` log level. + Warn, + /// Corresponds to the `Info` log level. + Info, + /// Corresponds to the `Debug` log level. + Debug, + /// Corresponds to the `Trace` log level. + Trace, +} diff --git a/packages/configuration/src/v1/mod.rs b/packages/configuration/src/v1/mod.rs index 25aa587b3..643235c03 100644 --- a/packages/configuration/src/v1/mod.rs +++ b/packages/configuration/src/v1/mod.rs @@ -236,8 +236,7 @@ pub mod tracker_api; pub mod udp_tracker; use std::fs; -use std::net::IpAddr; -use std::str::FromStr; +use std::net::{IpAddr, Ipv4Addr}; use figment::providers::{Env, Format, Serialized, Toml}; use figment::Figment; @@ -248,7 +247,7 @@ use self::health_check_api::HealthCheckApi; use self::http_tracker::HttpTracker; use self::tracker_api::HttpApi; use self::udp_tracker::UdpTracker; -use crate::{AnnouncePolicy, Error, Info}; +use crate::{AnnouncePolicy, Error, Info, LogLevel}; /// Core configuration for the tracker. #[allow(clippy::struct_excessive_bools)] @@ -256,7 +255,7 @@ use crate::{AnnouncePolicy, Error, Info}; pub struct Configuration { /// Logging level. Possible values are: `Off`, `Error`, `Warn`, `Info`, /// `Debug` and `Trace`. Default is `Info`. - pub log_level: Option, + pub log_level: Option, /// Tracker mode. See [`TrackerMode`] for more information. pub mode: TrackerMode, @@ -284,7 +283,7 @@ pub struct Configuration { /// is using a loopback IP address, the tracker assumes that the peer is /// in the same network as the tracker and will use the tracker's IP /// address instead. - pub external_ip: Option, + pub external_ip: Option, /// Weather the tracker should collect statistics about tracker usage. /// If enabled, the tracker will collect statistics like the number of /// connections handled, the number of announce requests handled, etc. @@ -330,7 +329,7 @@ impl Default for Configuration { let announce_policy = AnnouncePolicy::default(); let mut configuration = Configuration { - log_level: Option::from(String::from("info")), + log_level: Some(LogLevel::Info), mode: TrackerMode::Public, db_driver: DatabaseDriver::Sqlite3, db_path: String::from("./storage/tracker/lib/database/sqlite3.db"), @@ -338,7 +337,7 @@ impl Default for Configuration { min_announce_interval: announce_policy.interval_min, max_peer_timeout: 900, on_reverse_proxy: false, - external_ip: Some(String::from("0.0.0.0")), + external_ip: Some(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0))), tracker_usage_statistics: true, persistent_torrent_completed_stat: false, inactive_peer_cleanup_interval: 600, @@ -363,13 +362,7 @@ impl Configuration { /// and `None` otherwise. #[must_use] pub fn get_ext_ip(&self) -> Option { - match &self.external_ip { - None => None, - Some(external_ip) => match IpAddr::from_str(external_ip) { - Ok(external_ip) => Some(external_ip), - Err(_) => None, - }, - } + self.external_ip.as_ref().map(|external_ip| *external_ip) } /// Saves the default configuration at the given path. @@ -432,6 +425,8 @@ impl Configuration { #[cfg(test)] mod tests { + use std::net::{IpAddr, Ipv4Addr}; + use crate::v1::Configuration; use crate::Info; @@ -495,7 +490,7 @@ mod tests { fn configuration_should_contain_the_external_ip() { let configuration = Configuration::default(); - assert_eq!(configuration.external_ip, Some(String::from("0.0.0.0"))); + assert_eq!(configuration.external_ip, Some(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)))); } #[test] diff --git a/packages/test-helpers/src/configuration.rs b/packages/test-helpers/src/configuration.rs index 08f570dc6..0c7cc533a 100644 --- a/packages/test-helpers/src/configuration.rs +++ b/packages/test-helpers/src/configuration.rs @@ -2,7 +2,7 @@ use std::env; use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr}; -use torrust_tracker_configuration::Configuration; +use torrust_tracker_configuration::{Configuration, LogLevel}; use torrust_tracker_primitives::TrackerMode; use crate::random; @@ -28,7 +28,7 @@ pub fn ephemeral() -> Configuration { // For example: a test for the UDP tracker should disable the API and HTTP tracker. let mut config = Configuration { - log_level: Some("off".to_owned()), // Change to `debug` for tests debugging + log_level: Some(LogLevel::Off), // Change to `debug` for tests debugging ..Default::default() }; @@ -125,7 +125,7 @@ pub fn ephemeral_mode_private_whitelisted() -> Configuration { pub fn ephemeral_with_external_ip(ip: IpAddr) -> Configuration { let mut cfg = ephemeral(); - cfg.external_ip = Some(ip.to_string()); + cfg.external_ip = Some(ip); cfg } diff --git a/src/bootstrap/logging.rs b/src/bootstrap/logging.rs index 97e26919d..b71079b57 100644 --- a/src/bootstrap/logging.rs +++ b/src/bootstrap/logging.rs @@ -10,11 +10,10 @@ //! - `Trace` //! //! Refer to the [configuration crate documentation](https://docs.rs/torrust-tracker-configuration) to know how to change log settings. -use std::str::FromStr; use std::sync::Once; use log::{info, LevelFilter}; -use torrust_tracker_configuration::Configuration; +use torrust_tracker_configuration::{Configuration, LogLevel}; static INIT: Once = Once::new(); @@ -31,10 +30,17 @@ pub fn setup(cfg: &Configuration) { }); } -fn config_level_or_default(log_level: &Option) -> LevelFilter { +fn config_level_or_default(log_level: &Option) -> LevelFilter { match log_level { None => log::LevelFilter::Info, - Some(level) => LevelFilter::from_str(level).unwrap(), + Some(level) => match level { + LogLevel::Off => LevelFilter::Off, + LogLevel::Error => LevelFilter::Error, + LogLevel::Warn => LevelFilter::Warn, + LogLevel::Info => LevelFilter::Info, + LogLevel::Debug => LevelFilter::Debug, + LogLevel::Trace => LevelFilter::Trace, + }, } } diff --git a/src/servers/apis/mod.rs b/src/servers/apis/mod.rs index 2d4b3abe1..ef37026fe 100644 --- a/src/servers/apis/mod.rs +++ b/src/servers/apis/mod.rs @@ -130,8 +130,8 @@ //! > **NOTICE**: You can generate a self-signed certificate for localhost using //! OpenSSL. See [Let's Encrypt](https://letsencrypt.org/docs/certificates-for-localhost/). //! That's particularly useful for testing purposes. Once you have the certificate -//! you need to set the [`ssl_cert_path`](torrust_tracker_configuration::HttpApi::ssl_cert_path) -//! and [`ssl_key_path`](torrust_tracker_configuration::HttpApi::ssl_key_path) +//! you need to set the [`ssl_cert_path`](torrust_tracker_configuration::HttpApi::tsl_config.ssl_cert_path) +//! and [`ssl_key_path`](torrust_tracker_configuration::HttpApi::tsl_config.ssl_key_path) //! options in the configuration file with the paths to the certificate //! (`localhost.crt`) and key (`localhost.key`) files. //! diff --git a/src/servers/http/v1/services/announce.rs b/src/servers/http/v1/services/announce.rs index b37081045..5a0ae40e4 100644 --- a/src/servers/http/v1/services/announce.rs +++ b/src/servers/http/v1/services/announce.rs @@ -145,8 +145,9 @@ mod tests { fn tracker_with_an_ipv6_external_ip(stats_event_sender: Box) -> Tracker { let mut configuration = configuration::ephemeral(); - configuration.external_ip = - Some(IpAddr::V6(Ipv6Addr::new(0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969)).to_string()); + configuration.external_ip = Some(IpAddr::V6(Ipv6Addr::new( + 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, + ))); Tracker::new(&configuration, Some(stats_event_sender), statistics::Repo::new()).unwrap() } diff --git a/src/servers/udp/handlers.rs b/src/servers/udp/handlers.rs index 876f4c9fe..d8ca4680c 100644 --- a/src/servers/udp/handlers.rs +++ b/src/servers/udp/handlers.rs @@ -426,7 +426,7 @@ mod tests { } pub fn with_external_ip(mut self, external_ip: &str) -> Self { - self.configuration.external_ip = Some(external_ip.to_owned()); + self.configuration.external_ip = Some(external_ip.to_owned().parse().expect("valid IP address")); self } From b545b33c17b3f71591a98dba9aeab84294cd1a62 Mon Sep 17 00:00:00 2001 From: Jose Celano Date: Fri, 10 May 2024 15:59:58 +0100 Subject: [PATCH 8/8] refactor: [#852] extract Core configuration type --- packages/configuration/src/lib.rs | 4 +- packages/configuration/src/v1/core.rs | 88 +++++++++++++++ packages/configuration/src/v1/mod.rs | 101 +++--------------- packages/test-helpers/src/configuration.rs | 23 ++-- src/app.rs | 6 +- src/bootstrap/jobs/torrent_cleanup.rs | 4 +- src/bootstrap/logging.rs | 2 +- src/core/mod.rs | 16 +-- src/core/services/mod.rs | 2 +- .../http/v1/extractors/client_ip_sources.rs | 2 +- src/servers/http/v1/services/announce.rs | 2 +- src/servers/udp/handlers.rs | 2 +- 12 files changed, 134 insertions(+), 118 deletions(-) create mode 100644 packages/configuration/src/v1/core.rs diff --git a/packages/configuration/src/lib.rs b/packages/configuration/src/lib.rs index 85867816c..fdc021480 100644 --- a/packages/configuration/src/lib.rs +++ b/packages/configuration/src/lib.rs @@ -3,7 +3,7 @@ //! This module contains the configuration data structures for the //! Torrust Tracker, which is a `BitTorrent` tracker server. //! -//! The current version for configuration is [`v1`](crate::v1). +//! The current version for configuration is [`v1`]. pub mod v1; use std::collections::HashMap; @@ -172,7 +172,7 @@ pub struct TslConfig { pub ssl_key_path: Option, } -#[derive(Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] +#[derive(Serialize, Deserialize, PartialEq, Eq, PartialOrd, Ord, Debug, Hash, Clone)] #[serde(rename_all = "lowercase")] pub enum LogLevel { /// A level lower than all log levels. diff --git a/packages/configuration/src/v1/core.rs b/packages/configuration/src/v1/core.rs new file mode 100644 index 000000000..ed9074194 --- /dev/null +++ b/packages/configuration/src/v1/core.rs @@ -0,0 +1,88 @@ +use std::net::{IpAddr, Ipv4Addr}; + +use serde::{Deserialize, Serialize}; +use torrust_tracker_primitives::{DatabaseDriver, TrackerMode}; + +use crate::{AnnouncePolicy, LogLevel}; + +#[allow(clippy::struct_excessive_bools)] +#[derive(Serialize, Deserialize, PartialEq, Eq, Debug, Clone)] +pub struct Core { + /// Logging level. Possible values are: `Off`, `Error`, `Warn`, `Info`, + /// `Debug` and `Trace`. Default is `Info`. + pub log_level: Option, + /// Tracker mode. See [`TrackerMode`] for more information. + pub mode: TrackerMode, + + // Database configuration + /// Database driver. Possible values are: `Sqlite3`, and `MySQL`. + pub db_driver: DatabaseDriver, + /// Database connection string. The format depends on the database driver. + /// For `Sqlite3`, the format is `path/to/database.db`, for example: + /// `./storage/tracker/lib/database/sqlite3.db`. + /// For `Mysql`, the format is `mysql://db_user:db_user_password:port/db_name`, for + /// example: `root:password@localhost:3306/torrust`. + pub db_path: String, + + /// See [`AnnouncePolicy::interval`] + pub announce_interval: u32, + + /// See [`AnnouncePolicy::interval_min`] + pub min_announce_interval: u32, + /// Weather the tracker is behind a reverse proxy or not. + /// If the tracker is behind a reverse proxy, the `X-Forwarded-For` header + /// sent from the proxy will be used to get the client's IP address. + pub on_reverse_proxy: bool, + /// The external IP address of the tracker. If the client is using a + /// loopback IP address, this IP address will be used instead. If the peer + /// is using a loopback IP address, the tracker assumes that the peer is + /// in the same network as the tracker and will use the tracker's IP + /// address instead. + pub external_ip: Option, + /// Weather the tracker should collect statistics about tracker usage. + /// If enabled, the tracker will collect statistics like the number of + /// connections handled, the number of announce requests handled, etc. + /// Refer to the [`Tracker`](https://docs.rs/torrust-tracker) for more + /// information about the collected metrics. + pub tracker_usage_statistics: bool, + /// If enabled the tracker will persist the number of completed downloads. + /// That's how many times a torrent has been downloaded completely. + pub persistent_torrent_completed_stat: bool, + + // Cleanup job configuration + /// Maximum time in seconds that a peer can be inactive before being + /// considered an inactive peer. If a peer is inactive for more than this + /// time, it will be removed from the torrent peer list. + pub max_peer_timeout: u32, + /// Interval in seconds that the cleanup job will run to remove inactive + /// peers from the torrent peer list. + pub inactive_peer_cleanup_interval: u64, + /// If enabled, the tracker will remove torrents that have no peers. + /// The clean up torrent job runs every `inactive_peer_cleanup_interval` + /// seconds and it removes inactive peers. Eventually, the peer list of a + /// torrent could be empty and the torrent will be removed if this option is + /// enabled. + pub remove_peerless_torrents: bool, +} + +impl Default for Core { + fn default() -> Self { + let announce_policy = AnnouncePolicy::default(); + + Self { + log_level: Some(LogLevel::Info), + mode: TrackerMode::Public, + db_driver: DatabaseDriver::Sqlite3, + db_path: String::from("./storage/tracker/lib/database/sqlite3.db"), + announce_interval: announce_policy.interval, + min_announce_interval: announce_policy.interval_min, + max_peer_timeout: 900, + on_reverse_proxy: false, + external_ip: Some(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0))), + tracker_usage_statistics: true, + persistent_torrent_completed_stat: false, + inactive_peer_cleanup_interval: 600, + remove_peerless_torrents: true, + } + } +} diff --git a/packages/configuration/src/v1/mod.rs b/packages/configuration/src/v1/mod.rs index 643235c03..d19fedc87 100644 --- a/packages/configuration/src/v1/mod.rs +++ b/packages/configuration/src/v1/mod.rs @@ -78,7 +78,7 @@ //! //! Alternatively, you could setup a reverse proxy like Nginx or Apache to //! handle the SSL/TLS part and forward the requests to the tracker. If you do -//! that, you should set [`on_reverse_proxy`](crate::Configuration::on_reverse_proxy) +//! that, you should set [`on_reverse_proxy`](crate::v1::core::Core::on_reverse_proxy) //! to `true` in the configuration file. It's out of scope for this //! documentation to explain in detail how to setup a reverse proxy, but the //! configuration file should be something like this: @@ -230,86 +230,32 @@ //! [health_check_api] //! bind_address = "127.0.0.1:1313" //!``` +pub mod core; pub mod health_check_api; pub mod http_tracker; pub mod tracker_api; pub mod udp_tracker; use std::fs; -use std::net::{IpAddr, Ipv4Addr}; +use std::net::IpAddr; use figment::providers::{Env, Format, Serialized, Toml}; use figment::Figment; use serde::{Deserialize, Serialize}; -use torrust_tracker_primitives::{DatabaseDriver, TrackerMode}; +use self::core::Core; use self::health_check_api::HealthCheckApi; use self::http_tracker::HttpTracker; use self::tracker_api::HttpApi; use self::udp_tracker::UdpTracker; -use crate::{AnnouncePolicy, Error, Info, LogLevel}; +use crate::{Error, Info}; /// Core configuration for the tracker. -#[allow(clippy::struct_excessive_bools)] #[derive(Serialize, Deserialize, PartialEq, Eq, Debug)] pub struct Configuration { - /// Logging level. Possible values are: `Off`, `Error`, `Warn`, `Info`, - /// `Debug` and `Trace`. Default is `Info`. - pub log_level: Option, - /// Tracker mode. See [`TrackerMode`] for more information. - pub mode: TrackerMode, - - // Database configuration - /// Database driver. Possible values are: `Sqlite3`, and `MySQL`. - pub db_driver: DatabaseDriver, - /// Database connection string. The format depends on the database driver. - /// For `Sqlite3`, the format is `path/to/database.db`, for example: - /// `./storage/tracker/lib/database/sqlite3.db`. - /// For `Mysql`, the format is `mysql://db_user:db_user_password:port/db_name`, for - /// example: `root:password@localhost:3306/torrust`. - pub db_path: String, - - /// See [`AnnouncePolicy::interval`] - pub announce_interval: u32, - - /// See [`AnnouncePolicy::interval_min`] - pub min_announce_interval: u32, - /// Weather the tracker is behind a reverse proxy or not. - /// If the tracker is behind a reverse proxy, the `X-Forwarded-For` header - /// sent from the proxy will be used to get the client's IP address. - pub on_reverse_proxy: bool, - /// The external IP address of the tracker. If the client is using a - /// loopback IP address, this IP address will be used instead. If the peer - /// is using a loopback IP address, the tracker assumes that the peer is - /// in the same network as the tracker and will use the tracker's IP - /// address instead. - pub external_ip: Option, - /// Weather the tracker should collect statistics about tracker usage. - /// If enabled, the tracker will collect statistics like the number of - /// connections handled, the number of announce requests handled, etc. - /// Refer to the [`Tracker`](https://docs.rs/torrust-tracker) for more - /// information about the collected metrics. - pub tracker_usage_statistics: bool, - /// If enabled the tracker will persist the number of completed downloads. - /// That's how many times a torrent has been downloaded completely. - pub persistent_torrent_completed_stat: bool, - - // Cleanup job configuration - /// Maximum time in seconds that a peer can be inactive before being - /// considered an inactive peer. If a peer is inactive for more than this - /// time, it will be removed from the torrent peer list. - pub max_peer_timeout: u32, - /// Interval in seconds that the cleanup job will run to remove inactive - /// peers from the torrent peer list. - pub inactive_peer_cleanup_interval: u64, - /// If enabled, the tracker will remove torrents that have no peers. - /// The clean up torrent job runs every `inactive_peer_cleanup_interval` - /// seconds and it removes inactive peers. Eventually, the peer list of a - /// torrent could be empty and the torrent will be removed if this option is - /// enabled. - pub remove_peerless_torrents: bool, - - // Server jobs configuration + /// Core configuration. + #[serde(flatten)] + pub core: Core, /// The list of UDP trackers the tracker is running. Each UDP tracker /// represents a UDP server that the tracker is running and it has its own /// configuration. @@ -326,30 +272,13 @@ pub struct Configuration { impl Default for Configuration { fn default() -> Self { - let announce_policy = AnnouncePolicy::default(); - - let mut configuration = Configuration { - log_level: Some(LogLevel::Info), - mode: TrackerMode::Public, - db_driver: DatabaseDriver::Sqlite3, - db_path: String::from("./storage/tracker/lib/database/sqlite3.db"), - announce_interval: announce_policy.interval, - min_announce_interval: announce_policy.interval_min, - max_peer_timeout: 900, - on_reverse_proxy: false, - external_ip: Some(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0))), - tracker_usage_statistics: true, - persistent_torrent_completed_stat: false, - inactive_peer_cleanup_interval: 600, - remove_peerless_torrents: true, - udp_trackers: Vec::new(), - http_trackers: Vec::new(), + Self { + core: Core::default(), + udp_trackers: vec![UdpTracker::default()], + http_trackers: vec![HttpTracker::default()], http_api: HttpApi::default(), health_check_api: HealthCheckApi::default(), - }; - configuration.udp_trackers.push(UdpTracker::default()); - configuration.http_trackers.push(HttpTracker::default()); - configuration + } } } @@ -362,7 +291,7 @@ impl Configuration { /// and `None` otherwise. #[must_use] pub fn get_ext_ip(&self) -> Option { - self.external_ip.as_ref().map(|external_ip| *external_ip) + self.core.external_ip.as_ref().map(|external_ip| *external_ip) } /// Saves the default configuration at the given path. @@ -490,7 +419,7 @@ mod tests { fn configuration_should_contain_the_external_ip() { let configuration = Configuration::default(); - assert_eq!(configuration.external_ip, Some(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)))); + assert_eq!(configuration.core.external_ip, Some(IpAddr::V4(Ipv4Addr::new(0, 0, 0, 0)))); } #[test] diff --git a/packages/test-helpers/src/configuration.rs b/packages/test-helpers/src/configuration.rs index 0c7cc533a..86ed57b9e 100644 --- a/packages/test-helpers/src/configuration.rs +++ b/packages/test-helpers/src/configuration.rs @@ -27,10 +27,9 @@ pub fn ephemeral() -> Configuration { // todo: disable services that are not needed. // For example: a test for the UDP tracker should disable the API and HTTP tracker. - let mut config = Configuration { - log_level: Some(LogLevel::Off), // Change to `debug` for tests debugging - ..Default::default() - }; + let mut config = Configuration::default(); + + config.core.log_level = Some(LogLevel::Off); // Change to `debug` for tests debugging // Ephemeral socket address for API let api_port = 0u16; @@ -55,7 +54,7 @@ pub fn ephemeral() -> Configuration { let temp_directory = env::temp_dir(); let random_db_id = random::string(16); let temp_file = temp_directory.join(format!("data_{random_db_id}.db")); - temp_file.to_str().unwrap().clone_into(&mut config.db_path); + temp_file.to_str().unwrap().clone_into(&mut config.core.db_path); config } @@ -65,7 +64,7 @@ pub fn ephemeral() -> Configuration { pub fn ephemeral_with_reverse_proxy() -> Configuration { let mut cfg = ephemeral(); - cfg.on_reverse_proxy = true; + cfg.core.on_reverse_proxy = true; cfg } @@ -75,7 +74,7 @@ pub fn ephemeral_with_reverse_proxy() -> Configuration { pub fn ephemeral_without_reverse_proxy() -> Configuration { let mut cfg = ephemeral(); - cfg.on_reverse_proxy = false; + cfg.core.on_reverse_proxy = false; cfg } @@ -85,7 +84,7 @@ pub fn ephemeral_without_reverse_proxy() -> Configuration { pub fn ephemeral_mode_public() -> Configuration { let mut cfg = ephemeral(); - cfg.mode = TrackerMode::Public; + cfg.core.mode = TrackerMode::Public; cfg } @@ -95,7 +94,7 @@ pub fn ephemeral_mode_public() -> Configuration { pub fn ephemeral_mode_private() -> Configuration { let mut cfg = ephemeral(); - cfg.mode = TrackerMode::Private; + cfg.core.mode = TrackerMode::Private; cfg } @@ -105,7 +104,7 @@ pub fn ephemeral_mode_private() -> Configuration { pub fn ephemeral_mode_whitelisted() -> Configuration { let mut cfg = ephemeral(); - cfg.mode = TrackerMode::Listed; + cfg.core.mode = TrackerMode::Listed; cfg } @@ -115,7 +114,7 @@ pub fn ephemeral_mode_whitelisted() -> Configuration { pub fn ephemeral_mode_private_whitelisted() -> Configuration { let mut cfg = ephemeral(); - cfg.mode = TrackerMode::PrivateListed; + cfg.core.mode = TrackerMode::PrivateListed; cfg } @@ -125,7 +124,7 @@ pub fn ephemeral_mode_private_whitelisted() -> Configuration { pub fn ephemeral_with_external_ip(ip: IpAddr) -> Configuration { let mut cfg = ephemeral(); - cfg.external_ip = Some(ip); + cfg.core.external_ip = Some(ip); cfg } diff --git a/src/app.rs b/src/app.rs index 8bdc281a6..fcb01a696 100644 --- a/src/app.rs +++ b/src/app.rs @@ -67,7 +67,7 @@ pub async fn start(config: &Configuration, tracker: Arc) -> Vec) -> Vec 0 { - jobs.push(torrent_cleanup::start_job(config, &tracker)); + if config.core.inactive_peer_cleanup_interval > 0 { + jobs.push(torrent_cleanup::start_job(&config.core, &tracker)); } // Start Health Check API diff --git a/src/bootstrap/jobs/torrent_cleanup.rs b/src/bootstrap/jobs/torrent_cleanup.rs index 300813430..bd3b2e332 100644 --- a/src/bootstrap/jobs/torrent_cleanup.rs +++ b/src/bootstrap/jobs/torrent_cleanup.rs @@ -15,7 +15,7 @@ use std::sync::Arc; use chrono::Utc; use log::info; use tokio::task::JoinHandle; -use torrust_tracker_configuration::Configuration; +use torrust_tracker_configuration::v1::core::Core; use crate::core; @@ -25,7 +25,7 @@ use crate::core; /// /// Refer to [`torrust-tracker-configuration documentation`](https://docs.rs/torrust-tracker-configuration) for more info about that option. #[must_use] -pub fn start_job(config: &Configuration, tracker: &Arc) -> JoinHandle<()> { +pub fn start_job(config: &Core, tracker: &Arc) -> JoinHandle<()> { let weak_tracker = std::sync::Arc::downgrade(tracker); let interval = config.inactive_peer_cleanup_interval; diff --git a/src/bootstrap/logging.rs b/src/bootstrap/logging.rs index b71079b57..5c7e93811 100644 --- a/src/bootstrap/logging.rs +++ b/src/bootstrap/logging.rs @@ -19,7 +19,7 @@ static INIT: Once = Once::new(); /// It redirects the log info to the standard output with the log level defined in the configuration pub fn setup(cfg: &Configuration) { - let level = config_level_or_default(&cfg.log_level); + let level = config_level_or_default(&cfg.core.log_level); if level == log::LevelFilter::Off { return; diff --git a/src/core/mod.rs b/src/core/mod.rs index 83813a863..dbaf27e22 100644 --- a/src/core/mod.rs +++ b/src/core/mod.rs @@ -544,13 +544,13 @@ impl Tracker { stats_event_sender: Option>, stats_repository: statistics::Repo, ) -> Result { - let database = Arc::new(databases::driver::build(&config.db_driver, &config.db_path)?); + let database = Arc::new(databases::driver::build(&config.core.db_driver, &config.core.db_path)?); - let mode = config.mode; + let mode = config.core.mode; Ok(Tracker { //config, - announce_policy: AnnouncePolicy::new(config.announce_interval, config.min_announce_interval), + announce_policy: AnnouncePolicy::new(config.core.announce_interval, config.core.min_announce_interval), mode, keys: tokio::sync::RwLock::new(std::collections::HashMap::new()), whitelist: tokio::sync::RwLock::new(std::collections::HashSet::new()), @@ -560,11 +560,11 @@ impl Tracker { database, external_ip: config.get_ext_ip(), policy: TrackerPolicy::new( - config.remove_peerless_torrents, - config.max_peer_timeout, - config.persistent_torrent_completed_stat, + config.core.remove_peerless_torrents, + config.core.max_peer_timeout, + config.core.persistent_torrent_completed_stat, ), - on_reverse_proxy: config.on_reverse_proxy, + on_reverse_proxy: config.core.on_reverse_proxy, }) } @@ -1033,7 +1033,7 @@ mod tests { pub fn tracker_persisting_torrents_in_database() -> Tracker { let mut configuration = configuration::ephemeral(); - configuration.persistent_torrent_completed_stat = true; + configuration.core.persistent_torrent_completed_stat = true; tracker_factory(&configuration) } diff --git a/src/core/services/mod.rs b/src/core/services/mod.rs index 76c6a36f6..dec143568 100644 --- a/src/core/services/mod.rs +++ b/src/core/services/mod.rs @@ -21,7 +21,7 @@ use crate::core::Tracker; #[must_use] pub fn tracker_factory(config: &Configuration) -> Tracker { // Initialize statistics - let (stats_event_sender, stats_repository) = statistics::setup::factory(config.tracker_usage_statistics); + let (stats_event_sender, stats_repository) = statistics::setup::factory(config.core.tracker_usage_statistics); // Initialize Torrust tracker match Tracker::new(&Arc::new(config), stats_event_sender, stats_repository) { diff --git a/src/servers/http/v1/extractors/client_ip_sources.rs b/src/servers/http/v1/extractors/client_ip_sources.rs index 18eff26b3..1c6cdc636 100644 --- a/src/servers/http/v1/extractors/client_ip_sources.rs +++ b/src/servers/http/v1/extractors/client_ip_sources.rs @@ -16,7 +16,7 @@ //! the tracker will use the `X-Forwarded-For` header to get the client IP //! address. //! -//! See [`torrust_tracker_configuration::Configuration::on_reverse_proxy`]. +//! See [`torrust_tracker_configuration::Configuration::core.on_reverse_proxy`]. //! //! The tracker can also be configured to run without a reverse proxy. In this //! case, the tracker will use the IP address from the connection info. diff --git a/src/servers/http/v1/services/announce.rs b/src/servers/http/v1/services/announce.rs index 5a0ae40e4..9529f954c 100644 --- a/src/servers/http/v1/services/announce.rs +++ b/src/servers/http/v1/services/announce.rs @@ -145,7 +145,7 @@ mod tests { fn tracker_with_an_ipv6_external_ip(stats_event_sender: Box) -> Tracker { let mut configuration = configuration::ephemeral(); - configuration.external_ip = Some(IpAddr::V6(Ipv6Addr::new( + configuration.core.external_ip = Some(IpAddr::V6(Ipv6Addr::new( 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, 0x6969, ))); diff --git a/src/servers/udp/handlers.rs b/src/servers/udp/handlers.rs index d8ca4680c..d6d7a1065 100644 --- a/src/servers/udp/handlers.rs +++ b/src/servers/udp/handlers.rs @@ -426,7 +426,7 @@ mod tests { } pub fn with_external_ip(mut self, external_ip: &str) -> Self { - self.configuration.external_ip = Some(external_ip.to_owned().parse().expect("valid IP address")); + self.configuration.core.external_ip = Some(external_ip.to_owned().parse().expect("valid IP address")); self }