Added ADR for the changes to load env vars on top of installer config

This commit is contained in:
Alejandro Celaya 2022-01-15 17:17:22 +01:00
parent 199d976e3d
commit f53305c404
2 changed files with 51 additions and 0 deletions

View file

@ -0,0 +1,50 @@
# Update env vars behavior to have precedence over installer options
* Status: Accepted
* Date: 2022-01-15
## Context and problem statement
Shlink supports providing configuration via the installer tool that generates a config file that gets merged with the rest of the config, or via environment variables.
It is potentially possible to combine both, but if you do so, you will find out the installer tool config has precedence over env vars, which is not very intuitive.
A [Twitter survey](https://twitter.com/shlinkio/status/1480614855006732289) has also showed up all participants also found the behavior should be the opposite.
## Considered option
* Move the logic to read env vars to another config file which always overrides installer options.
* Move the logic to read env vars to a config post-processor which overrides config dynamically, only if the appropriate env var had been defined.
* Make the installer generate a config file which also includes the logic to load env vars on it.
* Make the installer no longer generate the config structure, and instead generate a map with env vars and their values. Then Shlink would define those env vars if not defined already.
## Decision outcome
The most viable option was finally to re-think the installer tool, and make it generate a map of env vars and their values.
Then Shlink reads this as the first config file, which sets the values as env vars if not yet defined, and later on, the values are read as usual wherever needed.
## Pros and Cons of the Options
### Read all env vars in a single config file
* Bad: This option had to be discarded, as it would always override the installer config no matter what.
### Read all env vars in a config post-processor
* Good because it would not require any change in the installer.
* Bad because it requires moving all env var reading logic somewhere else, while having it together with their contextual config is quite convenient.
### Make the installer generate a config file which also reads env vars
* Good because it would not require changing Shlink.
* Bad because it requires looking for a new way to generate the installer config.
* Bad because it would mean reading the env vars in multiple places.
### Re-think the installer to no longer generate internal config, and instead, just define values for regular env vars
* Bad because it requires changes both in Shlink and the installer.
* Bad because it's more error-prone, and the option with higher chances to introduce a regression.
* Good because it finally decouples Shlink internal config (which is an implementation detail) from any external tool, including the installer, allowing to change it at will.
* Good because it opens the door to eventually simplify the installer. For the moment, it requires a bit of extra logic to support importing the old config.
* Good because it allows keeping the logic to read env vars next to the config where it applies.

View file

@ -2,6 +2,7 @@
Here listed you will find the different architectural decisions taken in the project, including all the reasoning behind it, options considered, and final outcome.
* [2022-01-15 Update env vars behavior to have precedence over installer options](2022-01-15-update-env-vars-behavior-to-have-precedence-over-installer-options.md)
* [2021-08-05 Migrate to a new caching library](2021-08-05-migrate-to-a-new-caching-library.md)
* [2021-02-07 Track visits to 'base_url', 'invalid_short_url' and 'regular_404'](2021-02-07-track-visits-to-base-url-invalid-short-url-and-regular-404.md)
* [2021-01-17 Support restrictions and permissions in API keys](2021-01-17-support-restrictions-and-permissions-in-api-keys.md)