Skip to content

Commit

Permalink
refactor: [#581] migration to Figment
Browse files Browse the repository at this point in the history
Use Figment to load settings from toml file and allow overriding
settings with env vars.
  • Loading branch information
josecelano committed May 20, 2024
1 parent ccb2fef commit 018dfc3
Show file tree
Hide file tree
Showing 2 changed files with 151 additions and 64 deletions.
201 changes: 142 additions & 59 deletions src/config/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,18 +2,21 @@
pub mod v1;
pub mod validator;

use std::env;
use std::sync::Arc;
use std::{env, fs};

use camino::Utf8PathBuf;
use config::{Config, ConfigError, File, FileFormat};
use figment::providers::{Env, Format, Serialized, Toml};
use figment::Figment;
use serde::{Deserialize, Serialize};
use serde_with::{serde_as, NoneAsEmptyString};
use thiserror::Error;
use tokio::sync::RwLock;
use torrust_index_located_error::{Located, LocatedError};
use torrust_index_located_error::LocatedError;
use url::Url;

use crate::web::api::server::DynError;

pub type Settings = v1::Settings;
pub type Api = v1::api::Api;
pub type Auth = v1::auth::Auth;
Expand All @@ -26,10 +29,16 @@ pub type Tracker = v1::tracker::Tracker;
pub type Website = v1::website::Website;
pub type EmailOnSignup = v1::auth::EmailOnSignup;

/// Prefix for env vars that overwrite configuration options.
const CONFIG_OVERRIDE_PREFIX: &str = "TORRUST_INDEX_CONFIG_OVERRIDE_";
/// Path separator in env var names for nested values in configuration.
const CONFIG_OVERRIDE_SEPARATOR: &str = "__";

/// Information required for loading config
#[derive(Debug, Default, Clone)]
pub struct Info {
index_toml: String,
config_toml: Option<String>,
config_toml_path: String,
tracker_api_token: Option<String>,
auth_secret_key: Option<String>,
}
Expand All @@ -51,40 +60,33 @@ impl Info {
///
#[allow(clippy::needless_pass_by_value)]
pub fn new(
env_var_config: String,
env_var_path_config: String,
default_path_config: String,
env_var_config_toml: String,
env_var_config_toml_path: String,
default_config_toml_path: String,
env_var_tracker_api_token: String,
env_var_auth_secret_key: String,
) -> Result<Self, Error> {
let index_toml = if let Ok(index_toml) = env::var(&env_var_config) {
println!("Loading configuration from env var {env_var_config} ...");

index_toml
let config_toml = if let Ok(config_toml) = env::var(env_var_config_toml) {
println!("Loading configuration from environment variable {config_toml} ...");
Some(config_toml)
} else {
let config_path = if let Ok(config_path) = env::var(env_var_path_config) {
println!("Loading configuration file: `{config_path}` ...");

config_path
} else {
println!("Loading default configuration file: `{default_path_config}` ...");

default_path_config
};
None
};

fs::read_to_string(config_path)
.map_err(|e| Error::UnableToLoadFromConfigFile {
source: (Arc::new(e) as Arc<dyn std::error::Error + Send + Sync>).into(),
})?
.parse()
.map_err(|_e: std::convert::Infallible| Error::Infallible)?
let config_toml_path = if let Ok(config_toml_path) = env::var(env_var_config_toml_path) {
println!("Loading configuration from file: `{config_toml_path}` ...");
config_toml_path
} else {
println!("Loading configuration from default configuration file: `{default_config_toml_path}` ...");
default_config_toml_path
};

let tracker_api_token = env::var(env_var_tracker_api_token).ok();
let auth_secret_key = env::var(env_var_auth_secret_key).ok();

Ok(Self {
index_toml,
config_toml,
config_toml_path,
tracker_api_token,
auth_secret_key,
})
Expand All @@ -96,7 +98,7 @@ impl Info {
pub enum Error {
/// Unable to load the configuration from the environment variable.
/// This error only occurs if there is no configuration file and the
/// `TORRUST_TRACKER_CONFIG_TOML` environment variable is not set.
/// `TORRUST_INDEX_CONFIG_TOML` environment variable is not set.
#[error("Unable to load from Environmental Variable: {source}")]
UnableToLoadFromEnvironmentVariable {
source: LocatedError<'static, dyn std::error::Error + Send + Sync>,
Expand All @@ -109,12 +111,23 @@ pub enum Error {

/// Unable to load the configuration from the configuration file.
#[error("Failed processing the configuration: {source}")]
ConfigError { source: LocatedError<'static, ConfigError> },
ConfigError {
source: LocatedError<'static, dyn std::error::Error + Send + Sync>,
},

#[error("The error for errors that can never happen.")]
Infallible,
}

impl From<figment::Error> for Error {
#[track_caller]
fn from(err: figment::Error) -> Self {
Self::ConfigError {
source: (Arc::new(err) as DynError).into(),
}
}
}

/* todo:
Use https://crates.io/crates/torrust-tracker-primitives for TrackerMode.
Expand Down Expand Up @@ -218,7 +231,20 @@ impl Default for Configuration {
}

impl Configuration {
/// Loads the configuration from the `Info` struct. The whole
/// Loads the configuration from the `Info` struct.
///
/// # Errors
///
/// Will return `Err` if the environment variable does not exist or has a bad configuration.
pub fn load(info: &Info) -> Result<Configuration, Error> {
let settings = Self::load_settings(info)?;

Ok(Configuration {
settings: RwLock::new(settings),
})
}

/// Loads the settings from the `Info` struct. The whole
/// configuration in toml format is included in the `info.index_toml` string.
///
/// Optionally will override the:
Expand All @@ -229,23 +255,33 @@ impl Configuration {
/// # Errors
///
/// Will return `Err` if the environment variable does not exist or has a bad configuration.
pub fn load(info: &Info) -> Result<Configuration, Error> {
let config_builder = Config::builder()
.add_source(File::from_str(&info.index_toml, FileFormat::Toml))
.build()?;
let mut settings: Settings = config_builder.try_deserialize()?;
pub fn load_settings(info: &Info) -> Result<Settings, Error> {
let figment = if let Some(config_toml) = &info.config_toml {
// Config in env var has priority over config file path
Figment::from(Serialized::defaults(Settings::default()))
.merge(Toml::string(config_toml))
.merge(Env::prefixed(CONFIG_OVERRIDE_PREFIX).split(CONFIG_OVERRIDE_SEPARATOR))
} else {
Figment::from(Serialized::defaults(Settings::default()))
.merge(Toml::file(&info.config_toml_path))
.merge(Env::prefixed(CONFIG_OVERRIDE_PREFIX).split(CONFIG_OVERRIDE_SEPARATOR))
};

//println!("figment: {figment:#?}");

let mut settings: Settings = figment.extract()?;

if let Some(ref token) = info.tracker_api_token {
// todo: remove when using only Figment env var name: `TORRUST_INDEX_CONFIG_OVERRIDE_TRACKER__TOKEN`
settings.override_tracker_api_token(token);
};

if let Some(ref secret_key) = info.auth_secret_key {
// todo: remove when using only Figment env var name: `TORRUST_INDEX_CONFIG_OVERRIDE_AUTH__SECRET_KEY`
settings.override_auth_secret_key(secret_key);
};

Ok(Configuration {
settings: RwLock::new(settings),
})
Ok(settings)
}

pub async fn get_all(&self) -> Settings {
Expand Down Expand Up @@ -278,15 +314,6 @@ impl Configuration {
}
}

impl From<ConfigError> for Error {
#[track_caller]
fn from(err: ConfigError) -> Self {
Self::ConfigError {
source: Located(err).into(),
}
}
}

/// The public index configuration.
/// There is an endpoint to get this configuration.
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
Expand All @@ -304,7 +331,7 @@ fn parse_url(url_str: &str) -> Result<Url, url::ParseError> {
#[cfg(test)]
mod tests {

use crate::config::{Configuration, ConfigurationPublic, Info};
use crate::config::{Configuration, ConfigurationPublic, Info, Settings};

#[cfg(test)]
fn default_config_toml() -> String {
Expand Down Expand Up @@ -415,21 +442,30 @@ mod tests {

#[tokio::test]
async fn configuration_could_be_loaded_from_a_toml_string() {
let info = Info {
index_toml: default_config_toml(),
tracker_api_token: None,
auth_secret_key: None,
};
figment::Jail::expect_with(|jail| {
jail.create_dir("templates")?;
jail.create_file("templates/verify.html", "EMAIL TEMPLATE")?;

let info = Info {
config_toml: Some(default_config_toml()),
config_toml_path: String::new(),
tracker_api_token: None,
auth_secret_key: None,
};

let configuration = Configuration::load(&info).expect("Failed to load configuration from info");
let settings = Configuration::load_settings(&info).expect("Failed to load configuration from info");

assert_eq!(configuration.get_all().await, Configuration::default().get_all().await);
assert_eq!(settings, Settings::default());

Ok(())
});
}

#[tokio::test]
async fn configuration_should_allow_to_override_the_tracker_api_token_provided_in_the_toml_file() {
async fn configuration_should_allow_to_override_the_tracker_api_token_provided_in_the_toml_file_deprecated() {
let info = Info {
index_toml: default_config_toml(),
config_toml: Some(default_config_toml()),
config_toml_path: String::new(),
tracker_api_token: Some("OVERRIDDEN API TOKEN".to_string()),
auth_secret_key: None,
};
Expand All @@ -443,9 +479,33 @@ mod tests {
}

#[tokio::test]
async fn configuration_should_allow_to_override_the_authentication_secret_key_provided_in_the_toml_file() {
async fn configuration_should_allow_to_override_the_tracker_api_token_provided_in_the_toml_file() {
figment::Jail::expect_with(|jail| {
jail.create_dir("templates")?;
jail.create_file("templates/verify.html", "EMAIL TEMPLATE")?;

jail.set_env("TORRUST_INDEX_CONFIG_OVERRIDE_TRACKER__TOKEN", "OVERRIDDEN API TOKEN");

let info = Info {
config_toml: Some(default_config_toml()),
config_toml_path: String::new(),
tracker_api_token: None,
auth_secret_key: None,
};

let settings = Configuration::load_settings(&info).expect("Could not load configuration from file");

assert_eq!(settings.tracker.token, "OVERRIDDEN API TOKEN".to_string());

Ok(())
});
}

#[tokio::test]
async fn configuration_should_allow_to_override_the_authentication_secret_key_provided_in_the_toml_file_deprecated() {
let info = Info {
index_toml: default_config_toml(),
config_toml: Some(default_config_toml()),
config_toml_path: String::new(),
tracker_api_token: None,
auth_secret_key: Some("OVERRIDDEN AUTH SECRET KEY".to_string()),
};
Expand All @@ -458,6 +518,29 @@ mod tests {
);
}

#[tokio::test]
async fn configuration_should_allow_to_override_the_authentication_secret_key_provided_in_the_toml_file() {
figment::Jail::expect_with(|jail| {
jail.create_dir("templates")?;
jail.create_file("templates/verify.html", "EMAIL TEMPLATE")?;

jail.set_env("TORRUST_INDEX_CONFIG_OVERRIDE_AUTH__SECRET_KEY", "OVERRIDDEN AUTH SECRET KEY");

let info = Info {
config_toml: Some(default_config_toml()),
config_toml_path: String::new(),
tracker_api_token: None,
auth_secret_key: None,
};

let settings = Configuration::load_settings(&info).expect("Could not load configuration from file");

assert_eq!(settings.auth.secret_key, "OVERRIDDEN AUTH SECRET KEY".to_string());

Ok(())
});
}

mod syntax_checks {
// todo: use rich types in configuration structs for basic syntax checks.

Expand Down
14 changes: 9 additions & 5 deletions src/utils/parse_torrent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -115,40 +115,44 @@ pub fn calculate_info_hash(bytes: &[u8]) -> Result<InfoHash, DecodeTorrentFileEr

#[cfg(test)]
mod tests {
use std::path::Path;
use std::path::{Path, PathBuf};
use std::str::FromStr;

use crate::models::info_hash::InfoHash;

#[test]
fn it_should_calculate_the_original_info_hash_using_all_fields_in_the_info_key_dictionary() {
let torrent_path = Path::new(
let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
let torrent_relative_path = Path::new(
// cspell:disable-next-line
"tests/fixtures/torrents/6c690018c5786dbbb00161f62b0712d69296df97_with_custom_info_dict_key.torrent",
);
let torrent_path = root_dir.join(torrent_relative_path);

let original_info_hash = super::calculate_info_hash(&std::fs::read(torrent_path).unwrap()).unwrap();

assert_eq!(
original_info_hash,
InfoHash::from_str("6c690018c5786dbbb00161f62b0712d69296df97").unwrap()
InfoHash::from_str("6c690018c5786dbbb00161f62b0712d69296df97").unwrap() // DevSkim: ignore DS173237
);
}

#[test]
fn it_should_calculate_the_new_info_hash_ignoring_non_standard_fields_in_the_info_key_dictionary() {
let torrent_path = Path::new(
let root_dir = PathBuf::from(env!("CARGO_MANIFEST_DIR"));
let torrent_relative_path = Path::new(
// cspell:disable-next-line
"tests/fixtures/torrents/6c690018c5786dbbb00161f62b0712d69296df97_with_custom_info_dict_key.torrent",
);
let torrent_path = root_dir.join(torrent_relative_path);

let torrent = super::decode_torrent(&std::fs::read(torrent_path).unwrap()).unwrap();

// The infohash is not the original infohash of the torrent file,
// but the infohash of the info dictionary without the custom keys.
assert_eq!(
torrent.canonical_info_hash_hex(),
"8aa01a4c816332045ffec83247ccbc654547fedf".to_string()
"8aa01a4c816332045ffec83247ccbc654547fedf".to_string() // DevSkim: ignore DS173237
);
}
}

0 comments on commit 018dfc3

Please sign in to comment.