Skip to content

Commit

Permalink
Merge #808: Use figment for configuration
Browse files Browse the repository at this point in the history
0252f30 feat: allow users not to provide config option with default values (Jose Celano)
43942ce tests: add test for configuration with deprecated env var name (Jose Celano)
b0c2f9f docs: update env var name in toml config template files (Jose Celano)
69d7939 refactor: implement Default for Configuration sections (Jose Celano)
caae725 feat: use double underscore to split config env var names (Jose Celano)
b3a1442 refactor!: remove unused method in Configuration (Jose Celano)
632c8ba refactor: move Configuration unit test to inner mods (Jose Celano)
146b77d feat: enable overwrite Configuration values using env vars (Jose Celano)
5bd9494 chore: remove unused config dependenciy (Jose Celano)
265d89d refactor: replace Config by Figment in Configuration implementation (Jose Celano)
002fb30 refactor: reexport config versioned config types (Jose Celano)
e7d344c refactor: create new configuration v1 mod with figment (Jose Celano)
636e779 refactor: create new configuration v1 mod with figment (Jose Celano)
157807c chore(deps): enable figment features: env, toml, test (Jose Celano)
f0e0721 test: remove broken example in rustdoc (Jose Celano)
7da52b1 chore(deps): add dependency figment (Jose Celano)

Pull request description:

  Relates to: #790

  This PR migrates the configuration implementation to use [Figment](https://crates.io/crates/figment) as suggested by @da2ce7 [here](#401).

  ### Context

  I was working on a configuration validation in this [PR](#790). 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`:

  ```toml
  [[udp_trackers]]
  bind_address = "0.0.0.0:6969"
  enabled = false
  ```

  That configuration maps to a `String`:

  ```rust
  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:

  ```rust
  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)

  - [x] Implement a new configuration mod with Figment.
  - [x] Reorganize configuration mods creating new submods for config file sections.
  - [x] Introduce versioning for configuration, so that we can make breaking changes to the configuration in the future.
  - [x] Replace in production the configuration with the new Figment implementation.
  - [x] Use `Default` trait for config sections (not only root config).
  - [x] Allow users not to provide values when defaults are OK for them.

  ### FUTURE PR: 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](#790). For example, if  you enable TSL for the tracker API you must provide both the certificate path and the certificate key path:

  ```toml
  [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:

  ```toml
  # No section [http_api]
  ```

  API enabled but no TSL enabled:

  ```toml
  [http_api]
  bind_address = "127.0.0.1:1212"
  ```

  API enabled with TSL enabled:

  ```toml
  [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.

ACKs for top commit:
  josecelano:
    ACK 0252f30

Tree-SHA512: 94c7baa8566744c9e79bc9c56aae836d7a9b4272c42965c2dc88ec04b6a297b830c8eb7cb65d318c74e00aa5e27cc7c56784d6b083af04e98aadff4c82af5239
  • Loading branch information
josecelano committed May 9, 2024
2 parents 31ab3a9 + 0252f30 commit a20c9d7
Show file tree
Hide file tree
Showing 14 changed files with 867 additions and 875 deletions.
267 changes: 82 additions & 185 deletions Cargo.lock

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,11 @@ axum-extra = { version = "0", features = ["query"] }
axum-server = { version = "0", features = ["tls-rustls"] }
chrono = { version = "0", default-features = false, features = ["clock"] }
clap = { version = "4", features = ["derive", "env"] }
config = "0"
crossbeam-skiplist = "0.1"
dashmap = "5.5.3"
derive_more = "0"
fern = "0"
figment = "0.10.18"
futures = "0"
hex-literal = "0"
hyper = "1"
Expand Down Expand Up @@ -79,7 +79,7 @@ uuid = { version = "1", features = ["v4"] }
zerocopy = "0.7.33"

[package.metadata.cargo-machete]
ignored = ["serde_bytes", "crossbeam-skiplist", "dashmap", "parking_lot"]
ignored = ["crossbeam-skiplist", "dashmap", "figment", "parking_lot", "serde_bytes"]

[dev-dependencies]
local-ip-address = "0"
Expand All @@ -93,7 +93,7 @@ members = [
"packages/located-error",
"packages/primitives",
"packages/test-helpers",
"packages/torrent-repository"
"packages/torrent-repository",
]

[profile.dev]
Expand All @@ -107,5 +107,5 @@ lto = "fat"
opt-level = 3

[profile.release-debug]
inherits = "release"
debug = true
inherits = "release"
2 changes: 1 addition & 1 deletion packages/configuration/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ rust-version.workspace = true
version.workspace = true

[dependencies]
config = "0"
derive_more = "0"
figment = { version = "0.10.18", features = ["env", "test", "toml"] }
serde = { version = "1", features = ["derive"] }
serde_with = "3"
thiserror = "1"
Expand Down
Loading

0 comments on commit a20c9d7

Please sign in to comment.