torrust / torrust-tracker

A modern and feature-rich (private) BitTorrent tracker.
https://torrust.com
GNU Affero General Public License v3.0
357 stars 40 forks source link

Config overhaul: make some fields mandatory #938

Closed josecelano closed 2 months ago

josecelano commented 3 months ago

Parent issue: https://github.com/torrust/torrust-tracker/issues/401 Relates to: https://github.com/torrust/torrust-tracker/issues/401#issuecomment-2180566059 Depends on: https://github.com/torrust/torrust-tracker/issues/936

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 system admin (who is responsible to setup the app) to provide at least some critical options like:

version = "2"

[logging]
threshold = "info"

[core]
private = false
listed = false

They are already mandatory, but we have defined server default values, meaning you can omit the file in the TOML or JSON file, and it will use the default value. We have to remove the default values to force the users to provide the values.

Hi @da2ce7, is that list OK? (I'm already using new names even if, in some cases, those changes might not been applied yet)

UPDATE (2024-08-02)

Metadata section has changed.

[metadata]
app = "torrust-tracker"
purpose = "configuration"
schema_version = "2.0.0"

[logging]
threshold = "info"

[core]
private = false
listed = false
josecelano commented 3 months ago

Hey @da2ce7, to be honest, I don't see the point of making those fields mandatory. I think a public, non-listed tracker with the info threshold is a good default for running a tracker. The admin must add services anyway.

If you think that the default should be more restrictive, maybe we can change the default values to:

[logging]
threshold = "debug"

[core]
private = true
listed = true

In practice, that would somehow force the admin to review and change those values, which are safer (private as listed) and more friendly (debug) in case something is wrong with the configuration.

In general, I like the idea of running the service without knowing anything about the configuration for the tracker's probably most common use: the current default values.

On the other hand, there are other values we have not included in the first list that are more critical: the secrets. The API has a default "admin" token.

[http_api]
bind_address = "0.0.0.0:1212"

[http_api.access_tokens]
admin = "MyAccessToken"

You can omit the access_tokens key, and the API will be accessible (by default, only from localhost).

josecelano commented 2 months ago

I will implement this by checking that the following options don't come from default values (meaning they came from TOML or ENV VAR sources) instead of removing the default value, which can be helpful for other purposes like testing. In the end, we only want to make sure the user sets these values explicitly via TOML or ENV VARs.

[metadata]
schema_version = "2.0.0"

[logging]
threshold = "info"

[core]
private = false
listed = false

cc @da2ce7

josecelano commented 2 months ago

Hi @da2ce7 should secrets also be mandatory? For example `http_api.access_tokes.admin.