refactor: [torrust#852] enrich field types in HttpApi config struct

Author: josecelano

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<String, String>;
 
@@ -15,30 +18,26 @@ 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<String>,
-    /// Path to the SSL key file. Only used if `ssl_enabled` is `true`.
-    #[serde_as(as = "NoneAsEmptyString")]
-    pub ssl_key_path: Option<String>,
+    /// 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,
 }
 
 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()

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;
 

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),

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<String>, key: &Option<String>) -> Option<Result<RustlsConfig, Error>> {
+pub async fn make_rust_tls(enabled: bool, tsl_config: &TslConfig) -> Option<Result<RustlsConfig, Error>> {
     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<Utf8PathBuf>,
-    key: &Option<Utf8PathBuf>,
-) -> Option<Result<RustlsConfig, Error>> {
-    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,27 +47,41 @@ 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()
             .contains("bad tls config: No such file or directory (os error 2)"));
     }
 
     #[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.

src/bootstrap/jobs/tracker_apis.rs

@@ -61,12 +61,9 @@ pub async fn start_job(
     version: Version,
 ) -> Option<JoinHandle<()>> {
     if config.enabled {
-        let bind_to = config
-            .bind_address
-            .parse::<std::net::SocketAddr>()
-            .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"));
 

src/servers/apis/server.rs

@@ -275,12 +275,9 @@ mod tests {
 
         let tracker = initialize_with_configuration(&cfg);
 
-        let bind_to = config
-            .bind_address
-            .parse::<std::net::SocketAddr>()
-            .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"));
 

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();
 

tests/servers/api/environment.rs

@@ -33,13 +33,9 @@ impl Environment<Stopped> {
 
         let config = Arc::new(configuration.http_api.clone());
 
-        let bind_to = config
-            .bind_address
-            .parse::<std::net::SocketAddr>()
-            .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));
 

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<Stopped> {
 
         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));