torrust-tracker
torrust-tracker copied to clipboard
Use `figment` for configuration
Relates to: https://github.com/torrust/torrust-tracker/pull/790
This PR migrates the configuration implementation to use Figment as suggested by @da2ce7 here.
Context
I was working on a configuration validation in this PR. I wanted to validate things like socket addresses earlier when we parse the configuration instead of waiting until the app launches the service. For example, the UDP tracker configuration contains the bind_address
:
[[udp_trackers]]
bind_address = "0.0.0.0:6969"
enabled = false
That configuration maps to a String
:
pub struct UdpTracker {
pub enabled: bool,
pub bind_address: String,
}
I realized that kind of very basic validation could be actually done just changing the types in the configuration. For example:
pub struct UdpTracker {
pub enabled: bool,
pub bind_address: ScoketAddr,
}
There are other functionalities like overwritting values in the config file with env vars that can be implemented with Figment (merge).
So I decided to migrate to Figment the current version and update the configuration after the migration.
Subtasks without changing current config API (toml or struct)
- Implement a new configuration mod with Figment.
- Reorganize configuration mods creating new submods for config file sections.
- Use
Default
trait for config sections (not only root config). - Introduce versioning for configuration, so that we can make breaking changes to the configuration in the future.
- Replace in production the configuration with the new Figment implementation.
Subtasks changing current config API (structs)
These changes do not change the toml file configuration, only the parsed type.
- Use reach types like SockeAddr to validate some types without even starting the services.
FUTURE PR: Subtasks changing current config API (toml and structs)
At this point an extra validation could be needed like the one described here. For example, if you enable TSL for the tracker API you must provide both the certificate path and the certificate key path:
[http_api]
bind_address = "127.0.0.1:1212"
enabled = true
ssl_cert_path = ""
ssl_enabled = false
ssl_key_path = ""
I think that type of validation could be implementented after parsing the inyected config or maybe just reorganizcing the toml file structure. For example:
No API enabled:
# No section [http_api]
API enabled but no TSL enabled:
[http_api]
bind_address = "127.0.0.1:1212"
API enabled with TSL enabled:
[http_api]
bind_address = "127.0.0.1:1212"
[http_api.tsl_config]
ssl_cert_path = "./storage/tracker/lib/tls/localhost.crt"
ssl_key_path = "./storage/tracker/lib/tls/localhost.key"
We would not need the enabled
field. If the section is present the feature is enabled. If it's not it means that feature is disabled.
These breaking changes will be added in a new v2
configuration in a new PR.
Codecov Report
Attention: Patch coverage is 94.62366%
with 10 lines
in your changes are missing coverage. Please review.
Project coverage is 78.76%. Comparing base (
31ab3a9
) to head (0252f30
).
Files | Patch % | Lines |
---|---|---|
packages/configuration/src/v1/mod.rs | 94.77% | 7 Missing :warning: |
packages/configuration/src/lib.rs | 0.00% | 3 Missing :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## develop #808 +/- ##
===========================================
- Coverage 78.78% 78.76% -0.02%
===========================================
Files 163 168 +5
Lines 9252 9291 +39
===========================================
+ Hits 7289 7318 +29
- Misses 1963 1973 +10
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
With Figment, you can overwrite all config options using env vars. I have enabled that feature. You only have to do this:
let figment = Figment::new()
.merge(Toml::file("Config.toml"))
.merge(Env::prefixed("TORRUST_TRACKER_"));
We were overwriting only one value:
# Please override the admin token setting the
# `TORRUST_TRACKER_API_ADMIN_TOKEN`
# environmental variable!
[http_api.access_tokens]
admin = "MyAccessToken"
With the env var TORRUST_TRACKER_API_ADMIN_TOKEN
. This env var does not follow Figment conventions. The Figment name for that value would be TORRUST_TRACKER_HTTP_API.ACCESS_TOKENS.ADMIN
. The current name will be deprecated when this PR is merged. We have to replace the old name everywhere with the new one TORRUST_TRACKER_HTTP_API.ACCESS_TOKENS.ADMIN
.
In the end, you can customize the separator in Figment. I'm using a double underscore to generate a valid Bash name:
TORRUST_TRACKER__HTTP_API__ACCESS_TOKENS__ADMIN
@da2ce7, any other preference for the separator? If we want to generate a valid Bash name we don't have other options.
https://www.gnu.org/software/bash/manual/bash.html#Definitions
ACK 0252f308183e8ea53988907e7553eda8952ebdc7
Finally, I decided to merge this PR at this point with:
- Migration to Figment done.
- A new feature. If you are OK with the default values, you can avoid passing config options. I mean you only need to specify the config values you want to overwrite.
I have updated the Overhaul Configuration issue by adding the next steps.
@josecelano I think that the double-underscore is the natural choice :)
@josecelano I think that the double-underscore is the natural choice :)
Hi @da2ce7, I've extended the question from only "which separator to use" (it seems __
is fine) to a whole re-design of the env var names. See https://github.com/torrust/torrust-tracker/issues/851. I would appreciate your feedback since this is a breaking change, and refactoring all the names in all repos takes quite time.