Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config overhaul: make some fields mandatory #992

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading