Skip to content

Conversation

@ml-
Copy link
Collaborator

@ml- ml- commented Mar 10, 2020

Change removes local config.yml and used the (patched) example config from upstream instead.
Note: paths inside /var/lib changed. (Need migration script?)

Also a slightly hardened systemd service file, socket still missing. Let me know what matters in the end.

@ml-
Copy link
Collaborator Author

ml- commented Mar 11, 2020

There are still some issues:

Tests fail when there is a /etc/gotify/config.yml file on the system already and insufficient permissions to read it.
This ofc does not happen when using clean builld environment.

I'm not sure if I want the output in the journal since it includes the timestamp and is taking a lot of place. Is there a better logging output option if should we just write logs to a separate file like /var/log/gotify.log?

jmattheis added a commit to gotify/server that referenced this pull request Mar 11, 2020
Our config test ensures that the correct values will be extracted from
the config file and environment variables.
A globally defined config may change settings which are expected to have
default values.

See
gotify/server-aur-git#2 (comment)
@jmattheis
Copy link
Member

Yeah, we could add a redirect to file via systemd. Maybe /var/log/gotify/server.log because we also have a cli app which could log in the future.

The test failure should be fixed with gotify/server#279 in the meantime we should disable check or exclude the config package.

jmattheis added a commit to gotify/server that referenced this pull request Mar 11, 2020
Our config test ensures that the correct values will be extracted from
the config file and environment variables.
A globally defined config may change settings which are expected to have
default values.

See
gotify/server-aur-git#2 (comment)
- Add .install file to warn about default user
- Dropped tmpfiles.d in favor of (recommended) ConfigurationDirectory,
  LogsDirectory, StateDirectory
- Added `etc/gotify/config.yml` to `backup=()`
- Build UI with `NODE_ENV=production`
- Install config with 644 instead of 640
- Use absolute paths inside config
- Create `gotify` user without home
- Use `UMask=0027` instead of `0077` in service file
- Add `emptydirs` to `options=()`
Copy link
Member

@jmattheis jmattheis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@jmattheis jmattheis self-requested a review March 12, 2020 18:43
@ml- ml- merged commit 59b95bd into master Mar 13, 2020
@ml- ml- deleted the misc branch March 13, 2020 04:11
jmattheis added a commit to gotify/server that referenced this pull request Mar 14, 2020
Our config test ensures that the correct values will be extracted from
the config file and environment variables.
A globally defined config may change settings which are expected to have
default values.

See
gotify/server-aur-git#2 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants