Use `figment` for configuration #808

josecelano · 2024-04-22T13:25:52Z

Relates to: #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.
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.
Use Default trait for config sections (not only root config).
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. 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 · 2024-04-22T13:33:10Z

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 ⚠️
packages/configuration/src/lib.rs	0.00%	3 Missing ⚠️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

josecelano · 2024-05-08T15:50:17Z

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.

It will replace the custom code for configuration inyection.

- Clone config strcuctures into a new mod `v1`. - Introduce versioning for configuration API. - Split config sections into submodules. TODO: - Still using root mod types in production. - Not using figment to build config in production.

This is part of the migration to Figment in the configuration. This expose new versioned types (version 1). However, those types still used the old Config crate. Replacement by Figment has not been done yet.

This replaces the crate `config` with `figment` in the Configuration implementation.

It was replaced by `figment`.

Enable Figment ability to overwrite all config options with env vars. We are currently overwriting only this value: ```toml [http_api.access_tokens] admin = "MyAccessToken" ``` With the env var `TORRUST_TRACKER_API_ADMIN_TOKEN`. The name we gave to the env var does nto follow Figment convention which is `TORRUST_TRACKER_HTTP_API.ACCESS_TOKENS.ADMIN`. We have to keep both options until we remove the old one in the rest of the code.

josecelano · 2024-05-08T16:41:48Z

It seems the en var name generated by Figment (TORRUST_TRACKER_HTTP_API.ACCESS_TOKENS.ADMIN) is not a valid Bash var name.

For example, the env var `TORRUST_TRACKER__HTTP_API__ACCESS_TOKENS__ADMIN` would be the config option: ``` [http_api.access_tokens] admin = "MyAccessToken" ``` It uses `__` double underscore becuase dots are not allowed in Bash names. See: https://www.gnu.org/software/bash/manual/bash.html#Definitions ``` name A word consisting solely of letters, numbers, and underscores, and beginning with a letter or underscore. Names are used as shell variable and function names. Also referred to as an identifier. ```

josecelano · 2024-05-09T07:27:26Z

It seems the en var name generated by Figment (TORRUST_TRACKER_HTTP_API.ACCESS_TOKENS.ADMIN) is not a valid Bash var name.

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

Until it's updated in this repor and the Index repo you can overwrite the admin token using the new env var name and the old one (deprecated): - New: `TORRUST_TRACKER__HTTP_API__ACCESS_TOKENS__ADMIN` - Old (deprecated): `TORRUST_TRACKER_API_ADMIN_TOKEN` THe new one uses exactly the strcuture and atribute names in the toml file, using `__` as the separator for levels. We can remove the test when we remove the deprecated name.

Now, you are able to run the tracler like this: ``` TORRUST_TRACKER_CONFIG="" cargo run ``` Default values will be used for the missing values in the provided configuration. In that case, none of the values have been provided, so it will use default values for all options.

josecelano · 2024-05-09T12:51:02Z

ACK 0252f30

josecelano · 2024-05-09T13:06:36Z

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.

da2ce7 · 2024-05-12T10:07:52Z

@josecelano I think that the double-underscore is the natural choice :)

josecelano · 2024-05-13T07:41:30Z

@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 #851. I would appreciate your feedback since this is a breaking change, and refactoring all the names in all repos takes quite time.

josecelano temporarily deployed to coverage April 22, 2024 13:25 — with GitHub Actions Inactive

josecelano requested a review from da2ce7 April 22, 2024 13:26

josecelano mentioned this pull request Apr 22, 2024

Validate services configuration before starting them #790

Closed

5 tasks

josecelano added the Needs Rebase Base Branch has Incompatibilities label May 7, 2024

josecelano force-pushed the use-figment-for-configuration branch from d925a3c to 16b0486 Compare May 7, 2024 17:20

josecelano temporarily deployed to coverage May 7, 2024 17:20 — with GitHub Actions Inactive

josecelano removed the Needs Rebase Base Branch has Incompatibilities label May 7, 2024

josecelano force-pushed the use-figment-for-configuration branch from 16b0486 to a8b4624 Compare May 8, 2024 11:12

josecelano temporarily deployed to coverage May 8, 2024 11:12 — with GitHub Actions Inactive

josecelano added this to the v3.0.0 milestone May 8, 2024

josecelano added the Code Cleanup / Refactoring Tidying and Making Neat label May 8, 2024

josecelano temporarily deployed to coverage May 8, 2024 11:58 — with GitHub Actions Inactive

josecelano temporarily deployed to coverage May 8, 2024 12:55 — with GitHub Actions Inactive

josecelano temporarily deployed to coverage May 8, 2024 14:37 — with GitHub Actions Inactive

josecelano force-pushed the use-figment-for-configuration branch from ad54bdf to 7eb469a Compare May 8, 2024 14:57

josecelano temporarily deployed to coverage May 8, 2024 14:57 — with GitHub Actions Inactive

josecelano temporarily deployed to coverage May 8, 2024 15:43 — with GitHub Actions Inactive

josecelano added 9 commits May 8, 2024 16:52

chore(deps): add dependency figment

7da52b1

It will replace the custom code for configuration inyection.

test: remove broken example in rustdoc

f0e0721

chore(deps): enable figment features: env, toml, test

157807c

refactor: reexport config versioned config types

002fb30

This is part of the migration to Figment in the configuration. This expose new versioned types (version 1). However, those types still used the old Config crate. Replacement by Figment has not been done yet.

refactor: replace Config by Figment in Configuration implementation

265d89d

This replaces the crate `config` with `figment` in the Configuration implementation.

chore: remove unused config dependenciy

5bd9494

It was replaced by `figment`.

josecelano force-pushed the use-figment-for-configuration branch from 2bb7f6d to 146b77d Compare May 8, 2024 15:57

josecelano temporarily deployed to coverage May 8, 2024 15:57 — with GitHub Actions Inactive

josecelano mentioned this pull request May 8, 2024

Question: Conventions to overwrite nested values with env vars SergioBenitez/Figment#103

Closed

josecelano added 2 commits May 8, 2024 17:51

refactor: move Configuration unit test to inner mods

632c8ba

refactor!: remove unused method in Configuration

b3a1442

josecelano temporarily deployed to coverage May 8, 2024 17:01 — with GitHub Actions Inactive

josecelano temporarily deployed to coverage May 9, 2024 07:18 — with GitHub Actions Inactive

refactor: implement Default for Configuration sections

69d7939

josecelano temporarily deployed to coverage May 9, 2024 12:00 — with GitHub Actions Inactive

josecelano added 2 commits May 9, 2024 13:07

docs: update env var name in toml config template files

b0c2f9f

josecelano linked an issue May 9, 2024 that may be closed by this pull request

Config overhaul: migrate to figment #850

Closed

6 tasks

josecelano temporarily deployed to coverage May 9, 2024 12:49 — with GitHub Actions Inactive

josecelano marked this pull request as ready for review May 9, 2024 12:51

josecelano merged commit a20c9d7 into torrust:develop May 9, 2024
17 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `figment` for configuration #808

Use `figment` for configuration #808

josecelano commented Apr 22, 2024 •

edited

Loading

codecov bot commented Apr 22, 2024 •

edited

Loading

josecelano commented May 8, 2024

josecelano commented May 8, 2024 •

edited

Loading

josecelano commented May 9, 2024 •

edited

Loading

josecelano commented May 9, 2024

josecelano commented May 9, 2024 •

edited

Loading

da2ce7 commented May 12, 2024

josecelano commented May 13, 2024 •

edited

Loading

Use figment for configuration #808

Use figment for configuration #808

Conversation

josecelano commented Apr 22, 2024 • edited Loading

Context

Subtasks without changing current config API (toml or struct)

FUTURE PR: Subtasks changing current config API (structs)

FUTURE PR: Subtasks changing current config API (toml and structs)

codecov bot commented Apr 22, 2024 • edited Loading

Codecov Report

josecelano commented May 8, 2024

josecelano commented May 8, 2024 • edited Loading

josecelano commented May 9, 2024 • edited Loading

josecelano commented May 9, 2024

josecelano commented May 9, 2024 • edited Loading

da2ce7 commented May 12, 2024

josecelano commented May 13, 2024 • edited Loading

Use `figment` for configuration #808

Use `figment` for configuration #808

josecelano commented Apr 22, 2024 •

edited

Loading

codecov bot commented Apr 22, 2024 •

edited

Loading

josecelano commented May 8, 2024 •

edited

Loading

josecelano commented May 9, 2024 •

edited

Loading

josecelano commented May 9, 2024 •

edited

Loading

josecelano commented May 13, 2024 •

edited

Loading