Externalize pict-rs configuration, start centralizing configuration variables #160
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Internal/external nginx were sending duplicate headers, removed from internal - Opted X-Frame-Options DENY as default - Fixes #143
- Present/enabled in nginx.conf template
- Refs #109 - Co-authored by: Maximilian Praeger
- Rename pictrs.yml to vars.yml - Start moving some variables to vars.yml for easier management and discoverability
- Causes issues with how Ansible resolves variables - `server.com` != `root@server.com`, host_vars/server.com vs. host_vars/root@server.com
ticoombs
reviewed
Sep 4, 2023
Nutomic
reviewed
Sep 4, 2023
dessalines
approved these changes
Sep 4, 2023
ticoombs
reviewed
Sep 10, 2023
ticoombs
previously requested changes
Sep 10, 2023
codyro
dismissed
ticoombs’s stale review
Marked as resolved, won't allow merge. Semantic changes need another issue (beyond scope of this PR)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR was primarily looking to resolve #53 & #109. However, I ran across some things that required adjusting (which will benefit us when we tidy up everything a bit more!).
Special thanks to @mpraeger for the inspiration. I've added you as a co-author on the initial commit as I re-used some of your code & idea's.
Primary Changes
Remove the environmental variables defined in the
docker-compose.ymltemplate forpict-rsand move them to avars.ymlfile which is used to configure & maintain variables in a sane spot (instead of them floating around in the playbook) a8c859dPICTRS__SERVER__API_KEYdefault topostgres_passwordto ensure interoperability with prior versions, but this allows users to have more agency over what it’s set to. a8c859d#diff-fadf44a49d340433e809fac78150211bc15b8047e299b14bf891197a2f8a54fdR6Move
postgres_passwordvariable tovars.ymlinstead of being defined inlemmy.ymlandlemmy-almalinux.yml. This exposes it to users in case they have an existing database they would like to use or if they’d prefer to set it manually (it also helps clean up the playbooks by centralizing the configuration variables)inventory/host_vars/<fqdn>/passwords/postgrestoinventory/host_vars/<fqdn>/passwords/postgres.psk. The reason for this change is we’re currently not using thehost_varspath entirely correctly, so it’s attempting to parse the password file as a variable file and will puke due to an invalid syntax. By default, Ansible will read ininventory/host_vars/<fqdn>/{*.yml,*.yaml,*.ini,exec_file}files.Adjust the example inventory (
examples/hosts) to use a better format. Instead of usinguser@server.com, we opt to use theansible_user=userto avoid unpredictable variable loading. Using the above examples, if you haveuser@server.comin your inventory, Ansible will look for variables ininventory/host_vars/user@server.comdirectory instead of the expectedinventory/host_vars/server.comdirectory.Tested against:
Debian 10(UNTESTED, should work)Could someone verify at least one or more of the target distributions above? I didn't have time to test
pict-rswith S3, but I can confirm the environmental variables get populated and injected into the VM properly. We should try this to ensure there is no funny business when using S3 for object storage.