Overhaul Configuration and Settings for Torrust

[da2ce7]

Our current configuration system is a mess and not sustainable.

I suggest that we move to the Figment configuration library and conceptually rework our entire approach to configuration for the tracker.

[josecelano]

Related to: #127

Subtasks (UPDATED 2024-05)

• Config overhaul: migrate to figment #850
• Config overhaul: replace primitive types in configuration with richer types #852
• Config overhaul: remove default values from config file templates #856
• Config overhaul: missing toml file location in Figment errors #855
• Config overhaul: change env var names convention #851
• Config overhaul: version 2 for the configuration toml file (breaking changes) #878

More subtasks (UPDATED 2024-07)

After an internal discussion, we decided to make more changes:

• Config overhaul: split tracker mode #932
• Config overhaul: define current config version as version 2 #939
• Config overhaul: improve admin experience #935
• Config overhaul: rename log_level to threshold #936
• Config overhaul: add version and namespace to the configuration #937
• Config overhaul: remove secrets when final config is printed to console #948
• Config overhaul: lowercase for DatabaseDriver #950
• Config overhaul: use an explicit section for metadata #958
• Config overhaul: make some fields mandatory #938

[josecelano]

Some ideas related to configuration:

• Create an installer with a flag (argument) #134
• Configuration should be passed with a file or env vars. It can be passed as a whole file. Regarding env vars there is one for the whole file contents and others used to overwrite some values.
• We are allowing user to overwrite some options after loading the configuration file.
• It would be nice to validate the configuration before running the services when possible, in order to fail as soon as possible. We are doing that for one case. That implies to parse invalid configuration from a config file and create a secondary internal structure that is valid.
• It would be nice to allow users to define only the config options they want to change from the default configuration instead. See https://github.com/greatest-ape/aquatic/pull/191/files/c5843eedce6ccbc8f69e3b3f48ade84f705e70ba#r1527138893. This could be a problem if default values change. The user could not realise it's using different values.

[josecelano]

I've just realized we don't have the toml file path in Figment errors because we always pass Figment the toml file contents:

pub fn load(info: &Info) -> Result<Configuration, Error> {
    let figment = Figment::from(Serialized::defaults(Configuration::default()))
        .merge(Toml::string(&info.tracker_toml))
        .merge(Env::prefixed("TORRUST_TRACKER__").split("__"));

    let mut config: Configuration = figment.extract()?;

    // Deprecated manual overwritting of config options
    if let Some(ref token) = info.api_admin_token {
        config.override_api_admin_token(token);
    };

    Ok(config)
}

See: #854

Since we always pass Figment the contents of the toml file it does not know the location for the original toml file. We have to change the Info struct to provide the original info: contents or file and let Fgment load the file if needed. I will open a new issue for this when I finish the current open PR.

Error message:

$ cargo run
    Finished `dev` profile [optimized + debuginfo] target(s) in 0.10s
     Running `target/debug/torrust-tracker`
Loading default configuration file: `./share/default/config/tracker.development.sqlite3.toml` ...
thread 'main' panicked at src/bootstrap/config.rs:45:32:
called `Result::unwrap()` on an `Err` value: ConfigError { source: LocatedError { source: Error { tag: Tag(Default, 2), profile: Some(Profile(Uncased { string: "default" })), metadata: Some(Metadata { name: "TOML source string", source: None, provide_location: Some(Location { file: "packages/configuration/src/v1/mod.rs", line: 397, col: 14 }), interpolater:  }), path: ["health_check_api", "bind_address"], kind: Message("invalid socket address syntax"), prev: None }, location: Location { file: "packages/configuration/src/v1/mod.rs", line: 400, col: 41 } } }
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

The error should show the file location ./share/default/config/tracker.development.sqlite3.toml.

[josecelano]

Hi @da2ce7, I've finished the migration to Figment and also renamed the env vars. Is there anything else you wanted to refactor when you opened this issue?

I have also opened a new discussion to define the version 2 for the configuration:

#853

I guess we can define that for the milestone v.3.2.0 like the API v2.

PLease, reopen the issue if you miss something you want to refactor before releasing v3.0.0.

[josecelano]

Today, we had a meeting, and we decided to include some more changes.

Keep the version 1 for the TOML file

I'm using the internal version 1 v1 for the new version since I have totally removed the previous one. Since the config file is not too big and changes are easy to apply manually, I decided to start using versioning from the current one.

@da2ce7 wants to introduce versioning before the next major release v3.0.0 so we need to:

• Rename the namespace for the current one from v1 to v2.
• Add a field with the version in the toml file at the top of the files: version = 2.
• Recover the previous version and call it v1.
• Allow users to use the version 1 and migrate automatically? @da2ce7, If yes manually, automatically or with an update config script? I would suggest:
• Warning the user if it's using an old version. And ...
• Provide an update config script to update configuration from v1 to v2.

Some fields should not have a default value

In the current version, all config values have a default value. YOu can run the app without providing any config value at all.

@da2ce7 suggested forcing the admin to provide at least some critical options like:

• version
• logging level
• tracker mode

Add a namespace to the configuration

[torrust_tracker]
version = "1.0.0"

[torrust_tracker.logging]
log_level = "info"

{
  "torrust_tracker": {
    "version": "1.0.0",
    "logging": {
      "log_level": "info"
    }
  }
}

Rename some fields

For example. From

[logging]
log_level = "info"

To:

[logging]
threshold = "info"

Provide explicit values for enums

I see two ways of doing it.

First option:

[logging.threshold]
error = false
info = true
warm = false
debug = false
trace = false

Second option, provide a schema from TOML and JSON.

See: https://docs.rs/toml_schema/latest/toml_schema/

Third option:

[logging]
#threshold = "error"
threshold = "info"
#threshold = "warm"
#threshold = "debug"
#threshold = "trace"

Add all options in the temapltes uncommenting the default one.

Improve UX (for admins)

• Add a warning when the configuration does not enable any service (UDP tracker, HTTP tracker, or tracker API). It does not make sense to run the app without services enabled.
• Write the final config used to the logs when the tracker starts. We are only writing the config values when the source is an env var. We should do it always and print out the final configuration after emerging sources (defaults -> TOML file -> env vars).

[josecelano]

I forget to reopen the issue ☝🏼 to implement those extra changes.

[josecelano]

Hi @da2ce7, I'm also considering a change I have wanted to do for a long time.

I think tracker privacy (public and private) and torrent restrictions (whitelisted or not) are totally independent options.

Current

pub enum TrackerMode {
    Public,
    Listed,
    Private,
    PrivateListed,
}

[core]
mode = "public"

New Proposal

pub enum Privacy {
    Public,
    Private,
}

[core]
privacy = "public"
whitelisted = "true"

I think it's the right moment to do it. Any other suggestions for names @da2ce7 ?

Maybe an alternative to "whitelist"? See:

• https://english.stackexchange.com/questions/51088/alternative-terms-to-blacklist-and-whitelist
• https://chat.stackexchange.com/rooms/134051/discussion-on-answer-by-hugo-alternative-terms-to-blacklist-and-whitelist

[josecelano]

@da2ce7's proposal and I agree.

[core]
private = true
listed = true

[josecelano]

When you run the tracker with the default development config, you see the configuration with all services enabled in JSON:

Loading extra configuration from default configuration file: `./share/default/config/tracker.development.sqlite3.toml` ...
2024-07-03T16:11:27.971946Z  INFO torrust_tracker::bootstrap::logging: Logging initialized
2024-07-03T16:11:27.972578Z  INFO torrust_tracker::bootstrap::app: Configuration:
{
  "version": "2",
  "logging": {
    "threshold": "info"
  },
  "core": {
    "announce_policy": {
      "interval": 120,
      "interval_min": 120
    },
    "database": {
      "driver": "Sqlite3",
      "path": "./storage/tracker/lib/database/sqlite3.db"
    },
    "inactive_peer_cleanup_interval": 600,
    "listed": false,
    "net": {
      "external_ip": "0.0.0.0",
      "on_reverse_proxy": false
    },
    "private": false,
    "tracker_policy": {
      "max_peer_timeout": 900,
      "persistent_torrent_completed_stat": false,
      "remove_peerless_torrents": true
    },
    "tracker_usage_statistics": true
  },
  "udp_trackers": [
    {
      "bind_address": "0.0.0.0:6969"
    }
  ],
  "http_trackers": [
    {
      "bind_address": "0.0.0.0:7070",
      "tsl_config": null
    }
  ],
  "http_api": {
    "bind_address": "0.0.0.0:1212",
    "tsl_config": null,
    "access_tokens": {
      "admin": "MyAccessToken"
    }
  },
  "health_check_api": {
    "bind_address": "127.0.0.1:1313"
  }
}
2024-07-03T16:11:27.972620Z  INFO UDP TRACKER: Starting on: 0.0.0.0:6969
2024-07-03T16:11:27.972662Z  INFO UDP TRACKER: Started on: udp://0.0.0.0:6969
2024-07-03T16:11:27.972694Z  INFO HTTP TRACKER: Starting on: http://0.0.0.0:7070
2024-07-03T16:11:27.972786Z  INFO HTTP TRACKER: Started on: http://0.0.0.0:7070
2024-07-03T16:11:27.972900Z  INFO API: Starting on http://0.0.0.0:1212
2024-07-03T16:11:27.972905Z  INFO API: Started on http://0.0.0.0:1212
2024-07-03T16:11:27.972919Z  INFO HEALTH CHECK API: Starting on: http://127.0.0.1:1313
2024-07-03T16:11:27.972951Z  INFO HEALTH CHECK API: Started on: http://127.0.0.1:1313

In toml:

version = "2"

[logging]
threshold = "info"

[core]
inactive_peer_cleanup_interval = 600
listed = false
private = false
tracker_usage_statistics = true

  [core.announce_policy]
  interval = 120
  interval_min = 120

  [core.database]
  driver = "Sqlite3"
  path = "./storage/tracker/lib/database/sqlite3.db"

  [core.net]
  external_ip = "0.0.0.0"
  on_reverse_proxy = false

  [core.tracker_policy]
  max_peer_timeout = 900
  persistent_torrent_completed_stat = false
  remove_peerless_torrents = true

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

[[http_trackers]]
bind_address = "0.0.0.0:7070"

[http_api]
bind_address = "0.0.0.0:1212"

  [http_api.access_tokens]
  admin = "MyAccessToken"

[health_check_api]
bind_address = "127.0.0.1:1313"

That's the current state. Some more changes are pending to confirm or get feedback. See the list above. @da2ce7 @mario-nt @cgbosse