Rework config parsing

[jmattheis]

See the gotify-server.env.example for how it works. The new https://gotify.net/docs/config will just reference gotify-server.env.example, so it should be standalone and understandable.

• No yaml config anymore
• The list syntax changed from a yaml format to comma separated.
• Support loading settings from a file via _FILE
• Allow setting defining a custom path via $GOTIFY_CONFIG_FILE
• Supports gotify-server.env.local for local development, you can put GOTIFY_SERVER_PORT in there, and it works for the dev ui and server.

I've initially wanted to use a library (https://github.com/caarlos0/env) for the env parsing, but it didn't support _FILE natively. and manually implementing it doesn't seems to complex.

Fixes #366
Fixes #392

[codecov]

Codecov Report

❌ Patch coverage is 75.50432% with 85 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.68%. Comparing base (67b6589) to head (14a8368).

Files with missing lines	Patch %	Lines
app.go	49.18%	30 Missing and 1 partial ⚠️
config/parse.go	59.15%	20 Missing and 9 partials ⚠️
config/migrate/migrate.go	88.09%	5 Missing and 5 partials ⚠️
config/file.go	73.33%	6 Missing and 2 partials ⚠️
config/error.go	0.00%	4 Missing ⚠️
config/config.go	97.43%	1 Missing and 1 partial ⚠️
api/oidc.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #967      +/-   ##
==========================================
+ Coverage   74.46%   74.68%   +0.21%     
==========================================
  Files          62       67       +5     
  Lines        3008     3314     +306     
==========================================
+ Hits         2240     2475     +235     
- Misses        605      658      +53     
- Partials      163      181      +18     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jmattheis]

@eternal-flame-AD If you have time, could you have a look at this? (It's not urgent)

[eternal-flame-AD]

To be frank I'm not super sold on this .. could you clarify what is the problem we are solving by "standalone and understandable"?

Biggest concerns:

1. It looks like the library has a .Marshal function so I would say we should at least try to make an automated config migration utility to decrease the friction to users.
2. It looks like you basically unrolled the entire config parsing in config.go. Could we have achieved all the benefits you listed by adding special cases to help configor (AND achieved backwards compatibility with YAML syntax) anyways?
3. The entire config discovery is done relative to the executable in a "python venv" style with only option to add one file to the front of the config list, which IMO strongly bias towards one kind of deployment, and moving the environment means the gotify executable has to move with the config file as well.
4. The switch to dotenv format from YAML would encourage users to run "lean" configs with only specific items overriden. This can lead to complex problems when they are running multiple configs on the same system.

[eternal-flame-AD]

Ahh, I understand. It's your project so in terms of what should be done I completely defer to you. I think I the only thing I hope could be sorted out before merging is the friction.

It's true that an average set-up require only several options to be overridden, but what's more than likely for an existing user to see is they copied the example.yml into an custom file and have modified and tuned over months or years. I don't like that suddenly you have to figure out which option you flipped and which you did not and manually transliterate it into env format.

One concrete change in the current arrangement that should work better (and aligned with other software with overlay config system like openssh):

Move the default values into the code itself, comment all lines out for the default config file, like this:

# EXAMPLE_OPTIN enables the example opt in option 
# GOTIFY_EXAMPLE_OPTIN=false

A migration utility should be simple that just emits the default config file except uncommented overridden value when the input YAML file has a non default value for that item. We currently don't have a command line interface so how that could be integrated is uncertain. We can defer this into a separate PR but this should be doable and smooth.

This has the additional benefit of making config inheritance more simple. You no longer have A -> default -> C result in C being completely masked by a fully filled default file.

[jmattheis]

It's your project so in terms of what should be done I completely defer to you.

Yes, but I'd like your opinion on if you think this improves things. If not, I'd rather not merge this (:.

It's true that an average set-up require only several options to be overridden, but what's more than likely for an existing user to see is they copied the example.yml into an custom file and have modified and tuned over months or years. I don't like that suddenly you have to figure out which option you flipped and which you did not and manually transliterate it into env format.

Okay, sounds good. Maybe we can add a cli, like this

$ gotify serve
$ gotify migrate-config

This would also allow us to add other actions to the cli, like creating an admin users.

A migration utility should be simple that just emits the default config file except uncommented overridden value when the input YAML file has a non default value for that item. We currently don't have a command line interface so how that could be integrated is uncertain. We can defer this into a separate PR but this should be doable and smooth.

Yeah, let's do this separately.

Move the default values into the code itself, comment all lines out for the default config file.

My reasoning for embedding the defaults values directly is, that this basically explicitly defines the default values, so when we later change the default values, they continue to have the old default values, at the time they've setup the config.

But you right, with config inheritance this makes it more easier to misconfiguration. I'll comment out all the options.

Do we then allow config inheritance, or only read the first found config file like described in #967 (comment)?

[jmattheis]

@eternal-flame-AD could you answer the first question so we can continue or stop with this PR?

[eternal-flame-AD]

Sorry for the delay this fell off my notifications . It's okay we can proceed and I can review it as is, I'm on a sleeper train tonight so I won't be able to review code until tomorrow.

[jmattheis]

Thanks and no problem, I've made these changes as fixup commits:

• Comment out all options in the example
• only load the first found env file.
• Make GOTIFY_CONFIG_FILE is exclusive, don't load any other files if set.
• change back to using the cwd as check for the gotify-server.env
• error when a file cannot be read

[eternal-flame-AD]

lgtm

[eternal-flame-AD]

I don't think the current setup requires this but would be nice to support escaping comma literals as value.

[jmattheis]

I've changed this to use csv parsing for the list. See last fixup commit.

[eternal-flame-AD]

One final thing to consider, not a blocker just what I have been thinking about. If we use env variables instead of "proprietary" config files is that it some systems use /etc/default/gotify to put the environment variables for global services (like on Alpine and Arch and Ubuntu), I am not sure whether it might be beneficial to use that instead of /etc/gotify/server.env

[jmattheis]

One final thing to consider, not a blocker just what I have been thinking about. If we use env variables instead of "proprietary" config files is that it some systems use /etc/default/gotify to put the environment variables for global services (like on Alpine and Arch and Ubuntu), I am not sure whether it might be beneficial to use that instead of /etc/gotify/server.env

Do you have resources recommending to do this, instead of using /etc/gotify? I've looked a bit but didn't found many resources about this. The only thing using this I know of is grub, but I feel like this is an edge-case. If gotify is packaged with a systemd unit file, they packager can already define /etc/default/gotify or similar in the unit file, I don't think gotify/server itself should read from /etc/default.