core: Reloading with SIGUSR1 if config never changed via admin

[francislavoie]

This is a long-standing issue (since 2.0!) where we haven't support reloading via signals because of the redesign of config handling, being that we'd support changing config via the admin API.

There's only one scenario that makes sense for us to support reloading via signal, and that's when you run Caddy with caddy run (and not caddy start or caddy file-server etc), because the process which loaded the config is the same as the one which would receive the signal, and we actually know the config file used when initially loading loading.

If running with caddy run --resume (e.g. start from the saved config, i.e. in an API-first setup) then reloading via SIGUSR1 also doesn't make sense, since you should also be using the API to trigger the reload.

So to make this possible, we record the last known config file (and config adapter) during cmdRun() (i.e. caddy run), and we also register a callback to perform the reload (necessary because Caddy's core cannot use config adapters since it would cause circular dependencies, so it needs to be deferred back to the cmd package to perform the config change).

If we receive any config change via API (caddy reload which calls /load, or APIs call to /config for full or partial changes), then we clear the last known config. We also pass a header in caddy reload's API call with the known config file & adapter, so we can check "is the config file different?" so we can retain the last config if it's still the same file.

And finally, the signal handling: if we get a SIGUSR1 signal, then it acts like a forced config reload (even if the config is unchanged, reload anyway), but only if we have a last known config recorded.

Assistance Disclosure

Used RooCode + GPT4.1 for most of the work on this, with lots of iteration and manual adjustments along the way.

[mholt]

Neat, thanks for working on this.

I have a few suggestions/comments, but overall looking good 👍

[mholt]

Also sorry for the lint error. I guess I rebased with a bad commit on master.

[mholt]

Hm, no, I'm starting to think the linter is borked. It just started erroring out of nowhere.

[mholt]

Thank you Francis!

There is an error: var errReloadFromSourceUnavailable is unused (unused) -- but it seems to be a false positive, as it is used in sigtrap_posix.

EDIT: Ohhh the Windows linter failed here. That makes sense since sigtrap_posix is only defined on Posix systems. Can we put a //nolint:unused on that line?

[matburton]

If running with caddy run --resume (e.g. start from the saved config, i.e. in an API-first setup) then reloading via SIGUSR1 also doesn't make sense, since you should also be using the API to trigger the reload.

It's worth noting that using the CLI, caddy reload and the API, /config or /load are both a bit painful or dangerous to use even for caddy run --resume situations.

caddy reload forces you to submit the current config, but doesn't allow you to supply the ETag from /config/, meaning it doesn't provide a way to avoid data races. Also caddy reload can only be used in situations where the API is reachable by TCP AFAIK, e.g. excluding cases where the API is available over a unix socket or cases where the API is behind any kind of auth mechanism, even Caddy's own basic_auth.

Using the API directly is similar. POST /load ignores the If-Match header AFAIK and requires the current configuration, meaning it doesn't provide a way to avoid data races either. Hence your only option really is to write some code which calls GET /config/ to fetch the current configuration and the corresponding ETag, then calls POST /config/ with both the If-Match and the Cache-Control: must-revalidate header, and loop to retry upon a status code of 412.

That's a lot of knowledge for a Caddy user to have, and a lot of burden for them for something simple like giving Caddy a nudge when a certificate on disk has changed.

Having typed that all out maybe this would be welcome as an issue 🤣 should I raise it?

[francislavoie]

caddy reload forces you to submit the current config, but doesn't allow you to supply the ETag from /config/, meaning it doesn't provide a way to avoid data races.

Ah good point, yeah we could fetch the config in the command to compare ETag, seems like a good idea. PRs welcome for that.

Also caddy reload can only be used in situations where the API is reachable by TCP AFAIK

I'm pretty sure it supports unix sockets too just fine. AFAIK you can just specify --address unix//path/to/file, it takes the same as the admin global option https://caddyserver.com/docs/caddyfile/options#admin. If the config you pass to caddy reload has admin set then it uses that address anyway, which means you don't need the --address flag if the admin address isn't changing.

or cases where the API is behind any kind of auth mechanism, even Caddy's own basic_auth.

That's probably the wrong way to do it. You should probably set up https://caddyserver.com/docs/json/admin/remote/ which lets you use certs to authenticate.

Either way, all this seems way out of scope of this PR, this is meant for users of Caddyfile (or other flat config files, whatever adapter), not API users. If you're using the API then it's obviously very much within your control to use the API the "right way" like you described.

[matburton]

Either way, all this seems way out of scope of this PR

Sorry I only intended to write a quick note, but that comment got out of hand.

I'm pretty sure it supports unix sockets too just fine

Ah. I was going off the docs which says:
Note that only TCP addresses are supported at this time.
I'll check that unix sockets work and if they do I'll try submitting a PR to remove that from the docs 👍

PRs welcome for that.

I'm totally new to Go, but it can't hurt for me to have a try when I get some free time 👍

That's probably the wrong way to do it

Yes sorry that was a bad example.