search

caddyserver / dist

Resources for packaging and distributing Caddy
Apache License 2.0
116 stars 118 forks source link

service: Add `RuntimeDirectory` and `LogsDirectory` to unit #105

Open jbergstroem opened 9 months ago

jbergstroem commented 9 months ago

Caddy supports listening on a unix socket. To make it easier for users to manage said socket (including permissions), let systemd handle the creation of it.

francislavoie commented 9 months ago

So if I understand correctly, this will create /run/caddy dir owned by the caddy user?

https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#RuntimeDirectory=

I'm realizing there's also LogsDirectory, we should probably use that instead of this stuff: https://github.com/caddyserver/dist/blob/49a805b0196e8c9e394cfe3546f2cd568d6e37d1/scripts/postinstall.sh#L24

jbergstroem commented 9 months ago

So if I understand correctly, this will create /run/caddy dir owned by the caddy user?

freedesktop.org/software/systemd/man/latest/systemd.exec.html#RuntimeDirectory=

Yes.

I'm realizing there's also LogsDirectory, we should probably use that instead of this stuff:

https://github.com/caddyserver/dist/blob/49a805b0196e8c9e394cfe3546f2cd568d6e37d1/scripts/postinstall.sh#L24

Let me have a look (test) and iterate.

jbergstroem commented 9 months ago

Added the line locally and removed the log directory followed by a restart. The log directory was correctly created with 0755 permissions/caddy:caddy as owner. If we want other permissions, I can change that too.

jbergstroem commented 6 months ago

Ran into this issue again the other day! Any more input, review wise?

francislavoie commented 6 months ago

We create the directory so that it's there if users want to use it for access logs. I think it's a common enough need that it's harmless to do so just to save users a step which may be surprising/confusing to require otherwise.

WDYT about the RuntimeDirectory @carlwgeorge? Same idea here, it's just a nice-to-have I think which would save a manual step for most users.

carlwgeorge commented 6 months ago

It's not surprising or confusing for users who change root to ensure the directory they point it to exists. Why would it be surprising or confusing for users to ensure (or have systemd ensure) the directory for their custom logging or unix socket exists? Sure the directive is harmless, but I'm just not convinced it's necessary in the default unit file for everyone. That combined with the simplicity of unit file overrides for adding local directives makes me lean towards no on this.