Skip to content

Commit

Permalink
Merge #992: Config overhaul: make some fields mandatory
Browse files Browse the repository at this point in the history
90ef14d feat!: [#938] add mandatory config options (Jose Celano)

Pull request description:

  Some configuration options are mandatory. The tracker will panic if the user doesn't provide an explicit value for them from one of the configuration sources: TOML or ENV VARS.

  The mandatory options are:

  ```toml
  [metadata]
  schema_version = "2.0.0"

  [logging]
  threshold = "info"

  [core]
  private = false
  listed = false
  ```

ACKs for top commit:
  josecelano:
    ACK 90ef14d

Tree-SHA512: 428d599f81842e97335e771a7aa99f88b5f885b7f0026c1883a21b0cbddb4afca9e255aa7cfba637f3e9b3ddeb1c5314a832bb072df7ca21ff73be068d4814d8
  • Loading branch information
josecelano committed Aug 2, 2024
2 parents f95b16a + 90ef14d commit 1ca90ad
Show file tree
Hide file tree
Showing 7 changed files with 133 additions and 12 deletions.
3 changes: 3 additions & 0 deletions packages/configuration/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -303,6 +303,9 @@ pub enum Error {

#[error("Unsupported configuration version: {version}")]
UnsupportedVersion { version: Version },

#[error("Missing mandatory configuration option. Option path: {path}")]
MissingMandatoryOption { path: String },
}

impl From<figment::Error> for Error {
Expand Down
112 changes: 100 additions & 12 deletions packages/configuration/src/v2_0_0/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -313,27 +313,33 @@ impl Configuration {
}

/// Loads the configuration from the `Info` struct. The whole
/// configuration in toml format is included in the `info.tracker_toml` string.
/// configuration in toml format is included in the `info.tracker_toml`
/// string.
///
/// Optionally will override the admin api token.
/// Configuration provided via env var has priority over config file path.
///
/// # Errors
///
/// Will return `Err` if the environment variable does not exist or has a bad configuration.
pub fn load(info: &Info) -> Result<Configuration, Error> {
// Load configuration provided by the user, prioritizing env vars
let figment = if let Some(config_toml) = &info.config_toml {
// Config in env var has priority over config file path
Figment::from(Serialized::defaults(Configuration::default()))
.merge(Toml::string(config_toml))
.merge(Env::prefixed(CONFIG_OVERRIDE_PREFIX).split(CONFIG_OVERRIDE_SEPARATOR))
Figment::from(Toml::string(config_toml)).merge(Env::prefixed(CONFIG_OVERRIDE_PREFIX).split(CONFIG_OVERRIDE_SEPARATOR))
} else {
Figment::from(Serialized::defaults(Configuration::default()))
.merge(Toml::file(&info.config_toml_path))
Figment::from(Toml::file(&info.config_toml_path))
.merge(Env::prefixed(CONFIG_OVERRIDE_PREFIX).split(CONFIG_OVERRIDE_SEPARATOR))
};

// Make sure user has provided the mandatory options.
Self::check_mandatory_options(&figment)?;

// Fill missing options with default values.
let figment = figment.join(Serialized::defaults(Configuration::default()));

// Build final configuration.
let config: Configuration = figment.extract()?;

// Make sure the provided schema version matches this version.
if config.metadata.schema_version != Version::new(VERSION_2_0_0) {
return Err(Error::UnsupportedVersion {
version: config.metadata.schema_version,
Expand All @@ -343,6 +349,28 @@ impl Configuration {
Ok(config)
}

/// Some configuration options are mandatory. The tracker will panic if
/// the user doesn't provide an explicit value for them from one of the
/// configuration sources: TOML or ENV VARS.
///
/// # Errors
///
/// Will return an error if a mandatory configuration option is only
/// obtained by default value (code), meaning the user hasn't overridden it.
fn check_mandatory_options(figment: &Figment) -> Result<(), Error> {
let mandatory_options = ["metadata.schema_version", "logging.threshold", "core.private", "core.listed"];

for mandatory_option in mandatory_options {
figment
.find_value(mandatory_option)
.map_err(|_err| Error::MissingMandatoryOption {
path: mandatory_option.to_owned(),
})?;
}

Ok(())
}

/// Saves the configuration to the configuration file.
///
/// # Errors
Expand Down Expand Up @@ -496,14 +524,25 @@ mod tests {
}

#[test]
fn configuration_should_use_the_default_values_when_an_empty_configuration_is_provided_by_the_user() {
fn configuration_should_use_the_default_values_when_only_the_mandatory_options_are_provided_by_the_user_via_toml_file() {
figment::Jail::expect_with(|jail| {
jail.create_file("tracker.toml", "")?;
jail.create_file(
"tracker.toml",
r#"
[metadata]
schema_version = "2.0.0"
[logging]
threshold = "info"
let empty_configuration = String::new();
[core]
listed = false
private = false
"#,
)?;

let info = Info {
config_toml: Some(empty_configuration),
config_toml: None,
config_toml_path: "tracker.toml".to_string(),
};

Expand All @@ -515,10 +554,49 @@ mod tests {
});
}

#[test]
fn configuration_should_use_the_default_values_when_only_the_mandatory_options_are_provided_by_the_user_via_toml_content() {
figment::Jail::expect_with(|_jail| {
let config_toml = r#"
[metadata]
schema_version = "2.0.0"
[logging]
threshold = "info"
[core]
listed = false
private = false
"#
.to_string();

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

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

assert_eq!(configuration, Configuration::default());

Ok(())
});
}

#[test]
fn default_configuration_could_be_overwritten_from_a_single_env_var_with_toml_contents() {
figment::Jail::expect_with(|_jail| {
let config_toml = r#"
[metadata]
schema_version = "2.0.0"
[logging]
threshold = "info"
[core]
listed = false
private = false
[core.database]
path = "OVERWRITTEN DEFAULT DB PATH"
"#
Expand All @@ -543,6 +621,16 @@ mod tests {
jail.create_file(
"tracker.toml",
r#"
[metadata]
schema_version = "2.0.0"
[logging]
threshold = "info"
[core]
listed = false
private = false
[core.database]
path = "OVERWRITTEN DEFAULT DB PATH"
"#,
Expand Down
7 changes: 7 additions & 0 deletions share/default/config/tracker.container.mysql.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ app = "torrust-tracker"
purpose = "configuration"
schema_version = "2.0.0"

[logging]
threshold = "info"

[core]
listed = false
private = false

[core.database]
driver = "mysql"
path = "mysql://db_user:db_user_secret_password@mysql:3306/torrust_tracker"
Expand Down
7 changes: 7 additions & 0 deletions share/default/config/tracker.container.sqlite3.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ app = "torrust-tracker"
purpose = "configuration"
schema_version = "2.0.0"

[logging]
threshold = "info"

[core]
listed = false
private = false

[core.database]
path = "/var/lib/torrust/tracker/database/sqlite3.db"

Expand Down
7 changes: 7 additions & 0 deletions share/default/config/tracker.development.sqlite3.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ app = "torrust-tracker"
purpose = "configuration"
schema_version = "2.0.0"

[logging]
threshold = "info"

[core]
listed = false
private = false

[[udp_trackers]]
bind_address = "0.0.0.0:6969"

Expand Down
7 changes: 7 additions & 0 deletions share/default/config/tracker.e2e.container.sqlite3.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,13 @@ app = "torrust-tracker"
purpose = "configuration"
schema_version = "2.0.0"

[logging]
threshold = "info"

[core]
listed = false
private = false

[core.database]
path = "/var/lib/torrust/tracker/database/sqlite3.db"

Expand Down
2 changes: 2 additions & 0 deletions share/default/config/tracker.udp.benchmarking.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ schema_version = "2.0.0"
threshold = "error"

[core]
listed = false
private = false
remove_peerless_torrents = false
tracker_usage_statistics = false

Expand Down

0 comments on commit 1ca90ad

Please sign in to comment.