Merge #657: Tracker Checker: pass configuration with env var

392ffab refactor: [#656] E2E runner. Pass config as env var to Tracker Checker (Jose Celano)
1bab582 feat: [#656] Tracker Checker sopports env var for config (Jose Celano)
7f43fbd chore: [656] add cargo dep feature (Jose Celano)
8543190 refactor: Tracker Checker: use clap and anyhow (Jose Celano)

Pull request description:

  Allows passing the configuration with an env var:

  Run providing a config file path (option or env var):

  ```console
  cargo run --bin tracker_checker -- --config-path "./share/default/config/tracker_checker.json"

  TORRUST_CHECKER_CONFIG_PATH="./share/default/config/tracker_checker.json" cargo run --bin tracker_checker
  ```

  Run providing the configuration:

  ```console
  TORRUST_CHECKER_CONFIG=$(cat "./share/default/config/tracker_checker.json") cargo run --bin tracker_checker
  ```

  This way we don't need to create a temp file in CI. See https://github.com/torrust/torrust-tracker/blob/develop/src/e2e/runner.rs#L71-L73

  ### Subtasks

  - [x] Use `clap` and `anyhow`.
  - [x] Add env var `TORRUST_TRACKER_CHECKER_CONFIG`.
  - [x] Use the new env var in the E2E runner binary.

ACKs for top commit:
  josecelano:
    ACK 392ffab

Tree-SHA512: abf25491bc04b1f07a0d60389a72ee5dca439591a82531f4847f47ee1a203613e05d17db78c8e71b0125e83db38c7fa2d3bb368210a0554459a4b570ab012dd4

Author: josecelano

Cargo.toml

@@ -72,7 +72,7 @@ uuid = { version = "1", features = ["v4"] }
 colored = "2.1.0"
 url = "2.5.0"
 tempfile = "3.9.0"
-clap = { version = "4.4.18", features = ["derive"]}
+clap = { version = "4.4.18", features = ["derive", "env"]}
 anyhow = "1.0.79"
 
 [dev-dependencies]

src/bin/tracker_checker.rs

@@ -1,7 +1,16 @@
 //! Program to run checks against running trackers.
 //!
+//! Run providing a config file path:
+//!
+//! ```text
+//! cargo run --bin tracker_checker -- --config-path "./share/default/config/tracker_checker.json"
+//! TORRUST_CHECKER_CONFIG_PATH="./share/default/config/tracker_checker.json" cargo run --bin tracker_checker
+//! ```
+//!
+//! Run providing the configuration:
+//!
 //! ```text
-//! cargo run --bin tracker_checker "./share/default/config/tracker_checker.json"
+//! TORRUST_CHECKER_CONFIG=$(cat "./share/default/config/tracker_checker.json") cargo run --bin tracker_checker
 //! ```
 use torrust_tracker::checker::app;
 

src/checker/app.rs

@@ -1,57 +1,54 @@
+use std::path::PathBuf;
 use std::sync::Arc;
 
+use anyhow::{Context, Result};
+use clap::Parser;
+
 use super::config::Configuration;
 use super::console::Console;
-use super::service::{CheckError, Service};
+use super::service::{CheckResult, Service};
 use crate::checker::config::parse_from_json;
 
-pub const NUMBER_OF_ARGUMENTS: usize = 2;
+#[derive(Parser, Debug)]
+#[clap(author, version, about, long_about = None)]
+struct Args {
+    /// Path to the JSON configuration file.
+    #[clap(short, long, env = "TORRUST_CHECKER_CONFIG_PATH")]
+    config_path: Option<PathBuf>,
+
+    /// Direct configuration content in JSON.
+    #[clap(env = "TORRUST_CHECKER_CONFIG", hide_env_values = true)]
+    config_content: Option<String>,
+}
 
 /// # Errors
 ///
-/// If some checks fails it will return a vector with all failing checks.
-///
-/// # Panics
-///
-/// Will panic if:
-///
-/// - It can't read the json configuration file.
-/// - The configuration file is invalid.
-pub async fn run() -> Result<(), Vec<CheckError>> {
-    let args = parse_arguments();
-    let config = setup_config(&args);
+/// Will return an error if the configuration was not provided.
+pub async fn run() -> Result<Vec<CheckResult>> {
+    let args = Args::parse();
+
+    let config = setup_config(args)?;
+
     let console_printer = Console {};
+
     let service = Service {
         config: Arc::new(config),
         console: console_printer,
     };
 
-    service.run_checks().await
-}
-
-pub struct Arguments {
-    pub config_path: String,
+    Ok(service.run_checks().await)
 }
 
-fn parse_arguments() -> Arguments {
-    let args: Vec<String> = std::env::args().collect();
-
-    if args.len() < NUMBER_OF_ARGUMENTS {
-        eprintln!("Usage:       cargo run --bin tracker_checker <PATH_TO_CONFIG_FILE>");
-        eprintln!("For example: cargo run --bin tracker_checker ./share/default/config/tracker_checker.json");
-        std::process::exit(1);
-    }
-
-    let config_path = &args[1];
-
-    Arguments {
-        config_path: config_path.to_string(),
+fn setup_config(args: Args) -> Result<Configuration> {
+    match (args.config_path, args.config_content) {
+        (Some(config_path), _) => load_config_from_file(&config_path),
+        (_, Some(config_content)) => parse_from_json(&config_content).context("invalid config format"),
+        _ => Err(anyhow::anyhow!("no configuration provided")),
     }
 }
 
-fn setup_config(args: &Arguments) -> Configuration {
-    let file_content = std::fs::read_to_string(args.config_path.clone())
-        .unwrap_or_else(|_| panic!("Can't read config file {}", args.config_path));
+fn load_config_from_file(path: &PathBuf) -> Result<Configuration> {
+    let file_content = std::fs::read_to_string(path).with_context(|| format!("can't read config file {path:?}"))?;
 
-    parse_from_json(&file_content).expect("Invalid config format")
+    parse_from_json(&file_content).context("invalid config format")
 }

src/checker/config.rs

@@ -1,3 +1,4 @@
+use std::error::Error;
 use std::fmt;
 use std::net::SocketAddr;
 
@@ -43,6 +44,8 @@ pub enum ConfigurationError {
     InvalidUrl(url::ParseError),
 }
 
+impl Error for ConfigurationError {}
+
 impl fmt::Display for ConfigurationError {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
         match self {

src/checker/service.rs

@@ -14,6 +14,8 @@ pub struct Service {
     pub(crate) console: Console,
 }
 
+pub type CheckResult = Result<(), CheckError>;
+
 #[derive(Debug)]
 pub enum CheckError {
     UdpError,
@@ -25,7 +27,7 @@ impl Service {
     /// # Errors
     ///
     /// Will return OK is all checks pass or an array with the check errors.
-    pub async fn run_checks(&self) -> Result<(), Vec<CheckError>> {
+    pub async fn run_checks(&self) -> Vec<CheckResult> {
         self.console.println("Running checks for trackers ...");
 
         self.check_udp_trackers();
@@ -50,23 +52,19 @@ impl Service {
         }
     }
 
-    async fn run_health_checks(&self) -> Result<(), Vec<CheckError>> {
+    async fn run_health_checks(&self) -> Vec<CheckResult> {
         self.console.println("Health checks ...");
 
-        let mut check_errors = vec![];
+        let mut check_results = vec![];
 
         for health_check_url in &self.config.health_checks {
             match self.run_health_check(health_check_url.clone()).await {
-                Ok(()) => {}
-                Err(err) => check_errors.push(err),
+                Ok(()) => check_results.push(Ok(())),
+                Err(err) => check_results.push(Err(err)),
             }
         }
 
-        if check_errors.is_empty() {
-            Ok(())
-        } else {
-            Err(check_errors)
-        }
+        check_results
     }
 
     fn check_udp_tracker(&self, address: &SocketAddr) {

src/e2e/mod.rs

@@ -1,5 +1,5 @@
 pub mod docker;
 pub mod logs_parser;
 pub mod runner;
-pub mod temp_dir;
+pub mod tracker_checker;
 pub mod tracker_container;

src/e2e/runner.rs

@@ -1,15 +1,9 @@
-use std::fs::File;
-use std::io::Write;
-use std::path::{Path, PathBuf};
-use std::process::Command;
-use std::{env, io};
-
 use log::{debug, info, LevelFilter};
 
 use super::tracker_container::TrackerContainer;
 use crate::e2e::docker::RunOptions;
 use crate::e2e::logs_parser::RunningServices;
-use crate::e2e::temp_dir::Handler;
+use crate::e2e::tracker_checker::{self};
 
 /* code-review:
      - We use always the same docker image name. Should we use a random image name (tag)?
@@ -19,10 +13,9 @@ use crate::e2e::temp_dir::Handler;
        Should we remove the image too?
 */
 
-pub const NUMBER_OF_ARGUMENTS: usize = 2;
+const NUMBER_OF_ARGUMENTS: usize = 2;
 const CONTAINER_IMAGE: &str = "torrust-tracker:local";
 const CONTAINER_NAME_PREFIX: &str = "tracker_";
-const TRACKER_CHECKER_CONFIG_FILE: &str = "tracker_checker.json";
 
 pub struct Arguments {
     pub tracker_config_path: String,
@@ -63,14 +56,10 @@ pub fn run() {
 
     assert_there_is_at_least_one_service_per_type(&running_services);
 
-    let temp_dir = create_temp_dir();
-
-    let tracker_checker_config_path =
-        create_tracker_checker_config_file(&running_services, temp_dir.temp_dir.path(), TRACKER_CHECKER_CONFIG_FILE);
+    let tracker_checker_config =
+        serde_json::to_string_pretty(&running_services).expect("Running services should be serialized into JSON");
 
-    // todo: inject the configuration with an env variable so that we don't have
-    // to create the temporary directory/file.
-    run_tracker_checker(&tracker_checker_config_path).expect("All tracker services should be running correctly");
+    tracker_checker::run(&tracker_checker_config).expect("All tracker services should be running correctly");
 
     // More E2E tests could be added here in the future.
     // For example: `cargo test ...` for only E2E tests, using this shared test env.
@@ -128,19 +117,6 @@ fn read_file(path: &str) -> String {
     std::fs::read_to_string(path).unwrap_or_else(|_| panic!("Can't read file {path}"))
 }
 
-fn create_temp_dir() -> Handler {
-    debug!(
-        "Current dir: {:?}",
-        env::current_dir().expect("It should return the current dir")
-    );
-
-    let temp_dir_handler = Handler::new().expect("A temp dir should be created");
-
-    info!("Temp dir created: {:?}", temp_dir_handler.temp_dir);
-
-    temp_dir_handler
-}
-
 fn assert_there_is_at_least_one_service_per_type(running_services: &RunningServices) {
     assert!(
         !running_services.udp_trackers.is_empty(),
@@ -155,64 +131,3 @@ fn assert_there_is_at_least_one_service_per_type(running_services: &RunningServi
         "At least one Health Check should be enabled in E2E tests configuration"
     );
 }
-
-fn create_tracker_checker_config_file(running_services: &RunningServices, config_path: &Path, config_name: &str) -> PathBuf {
-    let tracker_checker_config =
-        serde_json::to_string_pretty(&running_services).expect("Running services should be serialized into JSON");
-
-    let mut tracker_checker_config_path = PathBuf::from(&config_path);
-    tracker_checker_config_path.push(config_name);
-
-    write_tracker_checker_config_file(&tracker_checker_config_path, &tracker_checker_config);
-
-    tracker_checker_config_path
-}
-
-fn write_tracker_checker_config_file(config_file_path: &Path, config: &str) {
-    info!(
-        "Writing Tracker Checker configuration file: {:?} \n{config}",
-        config_file_path
-    );
-
-    let mut file = File::create(config_file_path).expect("Tracker checker config file to be created");
-
-    file.write_all(config.as_bytes())
-        .expect("Tracker checker config file to be written");
-}
-
-/// Runs the Tracker Checker.
-///
-/// For example:
-///
-/// ```text
-/// cargo run --bin tracker_checker "./share/default/config/tracker_checker.json"
-/// ```
-///
-/// # Errors
-///
-/// Will return an error if the tracker checker fails.
-///
-/// # Panics
-///
-/// Will panic if the config path is not a valid string.
-pub fn run_tracker_checker(config_path: &Path) -> io::Result<()> {
-    info!(
-        "Running Tracker Checker: cargo --bin tracker_checker {}",
-        config_path.display()
-    );
-
-    let path = config_path.to_str().expect("The path should be a valid string");
-
-    let status = Command::new("cargo")
-        .args(["run", "--bin", "tracker_checker", path])
-        .status()?;
-
-    if status.success() {
-        Ok(())
-    } else {
-        Err(io::Error::new(
-            io::ErrorKind::Other,
-            format!("Failed to run Tracker Checker with config file {path}"),
-        ))
-    }
-}

src/e2e/temp_dir.rs

@@ -0,0 +1,25 @@
+use std::io;
+use std::process::Command;
+
+use log::info;
+
+/// Runs the Tracker Checker.
+///
+/// # Errors
+///
+/// Will return an error if the Tracker Checker fails.
+pub fn run(config_content: &str) -> io::Result<()> {
+    info!("Running Tracker Checker: TORRUST_CHECKER_CONFIG=[config] cargo run --bin tracker_checker");
+    info!("Tracker Checker config:\n{config_content}");
+
+    let status = Command::new("cargo")
+        .env("TORRUST_CHECKER_CONFIG", config_content)
+        .args(["run", "--bin", "tracker_checker"])
+        .status()?;
+
+    if status.success() {
+        Ok(())
+    } else {
+        Err(io::Error::new(io::ErrorKind::Other, "Failed to run Tracker Checker"))
+    }
+}