torrust-tracker
torrust-tracker copied to clipboard
Validate services configuration before starting them
These refactors allow proper configuration validation with unit tests. It also improves the error messages when the service configuration is wrong.
For example, this is the current error message if you enable SSL for the HTTP tracker but you don't provide the certificate and the key files:
Loading default configuration file: `./share/default/config/tracker.development.sqlite3.toml` ...
2024-04-11T17:13:28.016834819+01:00 [torrust_tracker::bootstrap::logging][INFO] logging initialized.
2024-04-11T17:13:28.017498045+01:00 [UDP TRACKER][INFO] Starting on: udp://0.0.0.0:6969
thread 'main' panicked at /home/developer/Documents/git/committer/me/github/torrust/torrust-tracker/src/app.rs:96:25:
Invalid HTTP Tracker configuration: missing SSL cert path, got: none
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
This is also related to something we have discussed in the past. The application should fail as soon as possible. If the provided configuration is wrong, it should panic before starting the service.
The idea is to progressively convert the current configuration structs into DTOs (plain configuration). Those DTOs are validated and converted into rich domain objects. For example:
// DTO: it's just the config toml file section parsed into Rust struct.
pub struct HttpTracker {
pub enabled: bool,
pub bind_address: String,
pub ssl_enabled: bool,
pub ssl_cert_path: Option<String>,
pub ssl_key_path: Option<String>,
}
// Validated configuration:
// - private fields
// - Domain types: SocketAddr, Path, ...
pub struct Config {
enabled: bool,
bind_address: SocketAddr,
ssl_enabled: bool,
ssl_cert_path: Option<Path>,
ssl_key_path: Option<Path>,
}
Subtasks
- [x] HTTP tracker
- [x] UDP tracker
- [x] Tracker API
- [x] Health Check API
- [ ] Core tracker
Codecov Report
Attention: Patch coverage is 50.77399% with 159 lines in your changes are missing coverage. Please review.
Project coverage is 78.15%. Comparing base (
9a1ce0e) to head (81b81ff).
Additional details and impacted files
@@ Coverage Diff @@
## develop #790 +/- ##
===========================================
- Coverage 78.93% 78.15% -0.78%
===========================================
Files 162 167 +5
Lines 9186 9486 +300
===========================================
+ Hits 7251 7414 +163
- Misses 1935 2072 +137
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
Hi, @da2ce7, this is a significant change in the configuration. However, in this PR, I'm just creating the new configuration type for validation and converting it back to the plain structure (the current one) to avoid changing the configuration structures for the whole app.
It would be nice if you review it, because you are also working on the configuration overhaul.
I decided to migrate to Figment before implementing this validation. See https://github.com/torrust/torrust-tracker/pull/808. This validation could be needed anyway, but all the validation added here could be implemented by just adding reacher types to the configuration structures.
Closed in favour of https://github.com/torrust/torrust-tracker/pull/808