274085 – www/caddy: Add forcereload command to rc script

Bug 274085 - www/caddy: Add forcereload command to rc script

Summary: www/caddy: Add forcereload command to rc script

Status:	Closed FIXED

Alias:	None

Product:	Ports & Packages
Classification:	Unclassified
Component:	Individual Port(s) (show other bugs)
Version:	Latest
Hardware:	Any Any

Importance:	--- Affects Some People
Assignee:	Adam Weinberger

URL:
Keywords:

Depends on:
Blocks:

Reported:	2023-09-25 18:08 UTC by Tom MTT.
Modified:	2023-10-14 02:21 UTC (History)
CC List:	1 user (show)

See Also:

Flags:	bugzilla: maintainer-feedback? (adamw)

Attachments
Proposed patch for www/caddy (793 bytes, patch) 2023-09-25 18:08 UTC, Tom MTT.	no flags	Details \| Diff
Proposed patch for www/caddy (with commit message) (1.29 KB, patch) 2023-09-25 19:45 UTC, Tom MTT.	no flags	Details \| Diff
Proposed patch for www/caddy (with commit message and reloadssl) (1.25 KB, patch) 2023-09-27 06:49 UTC, Tom MTT.	no flags	Details \| Diff
Show Obsolete (2) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Tom MTT. 2023-09-25 18:08:48 UTC

Created attachment 245223 [details]
Proposed patch for www/caddy

External TLS certificates (e.g. managed by acme.sh) are not reloaded by `caddy reload` unless Caddy's configuration has changed. Caddy added a `--force` flag which makes it reload everything (including TLS certs) no matter what for this use case.

Currently, the caddy rc script has a `reload` directive which does a regular Caddy reload (which doesn't reload external certificates). This patch adds a `forcereload` command which, as its name implies, makes Caddy reload everything (using `caddy reload --force`).

This way, Caddy can reload external certificates when they change (for instance, using acme.sh's `RenewHook`) without needing a full restart (thus keeping active connections alive) or `curl`-ing Caddy's admin API to do a full reload.

Thanks a lot in advance :)

Comment 1 Mina Galić freebsd_triage

2023-09-25 18:13:16 UTC

I like this patch.
It's nice and simple.
I would've put a big chunk of this PR's description into the commit message, but otherwise, it's looking really good.

Comment 2 Tom MTT. 2023-09-25 19:45:10 UTC

Created attachment 245232 [details]
Proposed patch for www/caddy (with commit message)

Oops, you're right. Completely forgot to add one.

Here's the revised patch with a proper commit message (taken and stripped down from my original bug report).

Comment 3 Adam Weinberger freebsd_committer

2023-09-26 10:28:57 UTC

This is a great idea!

I'm not sure about the name "forcereload" yet. On one hand, it's doing a 'reload --force' which suggests that it IS the correct name. But on the other hand, if the intent is to force SSL cert regeneration, perhaps the name should reflect that?

Comment 4 Mina Galić freebsd_triage

2023-09-26 10:49:34 UTC

(In reply to Adam Weinberger from comment #3)
(I suggested force-reload, I guess forcereload makes more sense in shell)
whether the name is the intent, or the effect can be debated.
A name like full-reload would make just as much sense.

We could also do a survey of existing rc scripts and see what they call their extra commands, so we're not just creating random outliers:

no one uses "force" in their extra_commands, but a lot use some kind of reload.
uwsgi has brutalreload, and haproxy has hardreload. There's a bunch of gracefuls, but no force. In the end however, this is a service specific command, so a service specific name names sense.

x11-fonts/xfs/files/xfs.in:extra_commands="reload"
www/wwwoffle/files/wwwoffled.in:extra_commands="reload"
www/varnish_exporter/files/varnish_exporter.in:extra_commands="reload"
www/varnish7/files/varnishd.in:extra_commands="status reload configtest"
www/uwsgi/files/uwsgi.in:extra_commands="reload brutalreload"
www/ufdbguard/files/ufdbguardd.in:extra_commands="configtest monitor reload rotatelog"
www/tomee/files/tomee.in:extra_commands="reload"
www/tomcat101/files/tomcat101.in:extra_commands="reload"
www/varnish6/files/varnishd.in:extra_commands="status reload configtest"
www/tomcat-devel/files/tomcat_devel.in:extra_commands="reload"
www/tomcat9/files/tomcat9.in:extra_commands="reload"
www/tomcat85/files/tomcat85.in:extra_commands="reload"
www/squid/files/squid.in:extra_commands="reload configtest"
www/squid-devel/files/squid.in:extra_commands="reload configtest"
www/py-graphite-api/files/graphiteapi.in:extra_commands="reload"
www/p5-Starman/files/starman.in:extra_commands="reload"
www/nginx/files/nginx.in:extra_commands="reload configtest upgrade gracefulstop"
www/nghttp2/files/nghttpx.in:extra_commands="reload reopenlogs"
www/nginx-devel/files/nginx.in:extra_commands="reload configtest upgrade gracefulstop"
www/lighttpd/files/lighttpd.in:extra_commands="reload graceful gracefulstop configtest"
www/kcgi/files/kfcgi.in:extra_commands="reload"
www/kannel/files/kannel_bearerbox.in:extra_commands="reload"
www/kannel/files/kannel_smsbox.in:extra_commands="reload"
www/kannel-sqlbox/files/kannel_sqlbox.in:extra_commands="reload"
www/kannel/files/kannel_wapbox.in:extra_commands="reload"
www/hs-postgrest/files/postgrest.in:extra_commands="reload"
www/h2o-devel/files/h2o.in:extra_commands="configtest reload"
www/h2o/files/h2o.in:extra_commands="configtest reload"
www/e2guardian/files/e2guardian.in:extra_commands="reload"
www/caddy/files/caddy.in:extra_commands="configtest reload"
www/caddy-custom/files/caddy.in:extra_commands="configtest reload"
www/apache24/files/apache24.in:extra_commands="reload graceful gracefulstop configtest"
www/angie/files/angie.in:extra_commands="configtest reload upgrade"
textproc/refdb/files/refdbd.in:extra_commands="reload"
textproc/opensearch-dashboards/files/opensearch-dashboards.in:extra_commands="reload"
textproc/kibana8/files/kibana.in:extra_commands="reload"
textproc/kibana7/files/kibana.in:extra_commands="reload"
sysutils/syslog-ng/files/syslog-ng.in:extra_commands="reload"
sysutils/smartmontools/files/smartd.in:extra_commands="reload report"
sysutils/sec/files/sec.in:extra_commands="reload"
sysutils/rsyslog8/files/rsyslogd.in:extra_commands="reload"
sysutils/rest-server/files/restserver.in:extra_commands="reload monitor"
sysutils/pies/files/pies.in:extra_commands="reload configtest"
sysutils/p5-App-Regather/files/regather.in:extra_commands="reload"
sysutils/nomad/files/nomad.in:extra_commands="reload"
sysutils/msyslog/files/msyslogd.in:extra_commands="reload"
sysutils/logstash8/files/logstash.in:extra_commands="configtest reload"
sysutils/logstash7/files/logstash.in:extra_commands="configtest reload"
sysutils/ipa/files/ipa.in:extra_commands="reload"
sysutils/healthd/files/healthd.in:extra_commands="reload"
sysutils/consul/files/consul.in:extra_commands="reload"
sysutils/consul_exporter/files/consul_exporter.in:extra_commands="reload"
sysutils/cbsd-mq-router/files/cbsd-mq-router.in:extra_commands="reload"
sysutils/cbsd-mq-api/files/cbsd-mq-api.in:extra_commands="reload"
sysutils/burp-devel/files/burp.in:extra_commands="reload monitor summary"
sysutils/burp/files/burp.in:extra_commands="reload monitor summary"
sysutils/bareos21-server/files/bareos-dir.in:extra_commands="reload configtest"
sysutils/bareos20-server/files/bareos-dir.in:extra_commands="reload configtest"
sysutils/bareos19-server/files/bareos-dir.in:extra_commands="reload configtest"
sysutils/bareos-server/files/bareos-dir.in:extra_commands="reload configtest"
sysutils/am-utils/files/amd.in:extra_commands="reload"
security/vault/files/vault.in:extra_commands="reload monitor"
security/tor-devel/files/tor.in:extra_commands="reload"
security/tinc/files/tincd.in:extra_commands="reload"
security/tinc-devel/files/tincd.in:extra_commands="reload"
security/tor/files/tor.in:extra_commands="reload"
security/strongswan/files/strongswan.in:extra_commands="reload statusall"
security/strongswan/files/strongswan.in:	extra_commands="reload statusall"
security/py-fail2ban/files/fail2ban.in:extra_commands="reload jailstatus"
security/ossec-hids-local/files/ossec-hids.in:extra_commands="help status reload ossec_conf"
security/openvpn/files/openvpn.in:extra_commands="reload softrestart stats"
security/openvpn-devel/files/openvpn.in:extra_commands="reload softrestart stats"
security/openssh-portable/files/openssh.in:extra_commands="configtest reload keygen"
security/i2pd/files/i2pd.in:extra_commands="reload"
security/fwknop/files/fwknopd.in:extra_commands="reload"
security/crowdsec/files/crowdsec.in:extra_commands="configtest reload"
print/cups/files/cupsd.in:extra_commands="reload"
net/wireguard-tools/files/wireguard_wgquick.in:extra_commands="reload status"
net/wireguard-tools/files/wireguard_lite.in:extra_commands="reload status"
net/samba416/files/samba_server.in:extra_commands="reload status configtest"
net/samba413/files/samba_server.in:extra_commands="reload status configtest"
net/relayd/files/relayd.in:extra_commands="reload"
net/radiator/files/radiator.in:extra_commands="reload"
net/pichi/files/pichi.in:extra_commands="reload"
net/parpd/files/parpd.in:extra_commands="reload"
net/openmdns/files/mdnsd.in:extra_commands="reload"
net/ntpa/files/ntpa.in:extra_commands="reload configtest"
net/netatalk3/files/netatalk.in:extra_commands="reload"
net/mrouted/files/mrouted.in:extra_commands="reload"
net/mlvpn/files/mlvpn.in:extra_commands="reload softrestart"
net/lavinmq/files/lavinmq.in:extra_commands="reload monitor"
net/hostapd/files/hostapd.in:extra_commands="reload"
net/hostapd-devel/files/hostapd.in:extra_commands="reload"
net/haproxy27/files/haproxy.in:extra_commands="reload configtest hardstop hardreload softreload"
net/hostapd29/files/hostapd.in:extra_commands="reload"
net/haproxy24/files/haproxy.in:extra_commands="reload configtest hardstop hardreload softreload"
net/haproxy22/files/haproxy.in:extra_commands="reload configtest hardstop hardreload softreload"
net/haproxy20/files/haproxy.in:extra_commands="reload configtest hardstop hardreload softreload"
net/haproxy/files/haproxy.in:extra_commands="reload configtest hardstop hardreload softreload"
net/haproxy-devel/files/haproxy.in:extra_commands="reload configtest hardstop hardreload softreload"
net/haproxy26/files/haproxy.in:extra_commands="reload configtest hardstop hardreload softreload"
net/haproxy25/files/haproxy.in:extra_commands="reload configtest hardstop hardreload softreload"
net/gemserv/files/gemserv.in:extra_commands="reload"
net/freeradius3/files/radiusd.in:extra_commands="reload debug"
net/exabgp4/files/exabgp.in:extra_commands="configtest reload reload_all"
net/dtcpclient/files/dtcpclient.in:extra_commands="reload"
net/dtcp/files/dtcpc.in:extra_commands="reload"
net/bird/files/bird6.in:extra_commands="reload"
net/bird/files/bird.in:extra_commands="reload"
net/babeld/files/babeld.in:extra_commands="reload"
net-p2p/uhub/files/uhub.in:extra_commands="reload"
net-p2p/cardano-node/files/cardano_node.in:extra_commands="status fetch reload"
net-mgmt/victoria-metrics/files/victoria-metrics.in:extra_commands="reload"
net-mgmt/victoria-metrics/files/vmagent.in:extra_commands="checkconfig reload"
net-mgmt/smokeping/files/smokeping.in:extra_commands="status configtest reload"
net-mgmt/riemann/files/riemann.in:extra_commands="reload"
net-mgmt/prometheus2/files/prometheus.in:extra_commands="reload"
net-mgmt/prometheus1/files/prometheus.in:extra_commands="reload"
net-mgmt/p0f/files/p0f.in:extra_commands="reload"
net-mgmt/netdisco/files/netdisco.in:extra_commands="reload status"
net-mgmt/nagios/files/nagios.in:extra_commands="reload configtest"
net-mgmt/icinga2/files/icinga2.in:extra_commands="reload checkconfig configtest"
net-mgmt/alertmanager/files/alertmanager.in:extra_commands="reload"
net-im/prosody/files/prosody.in:extra_commands="reload"
net-im/ejabberd/files/ejabberd.in:extra_commands="reload status"
net-im/biboumi/files/biboumi.in:extra_commands="reload"
misc/raspberrypi-gpioshutdown/files/gpioshutdown.in:extra_commands="unload reload"
mail/spamassassin/files/sa-spamd.in:extra_commands="reload"
mail/spamassassin-devel/files/sa-spamd.in:extra_commands="reload"
mail/rspamd/files/rspamd.in:extra_commands="reload configtest reopenlog"
mail/rspamd-devel/files/rspamd.in:extra_commands="reload configtest reopenlog"
mail/rbl-milter/files/rblmilter.in:extra_commands="reload"
mail/postfix/files/postfix.in:extra_commands="reload"
mail/postfix-postfwd/files/postfwd.in:extra_commands="reload"
mail/postfix-current/files/postfix.in:extra_commands="reload"
mail/popular/files/pproxy.in:extra_commands="reload"
mail/popular/files/pserv.in:extra_commands="reload"
mail/popular/files/pcheckd.in:extra_commands="reload"
mail/opendkim/files/milter-opendkim.in:extra_commands="reload"
mail/opendkim/files/milter-opendkim.in:extra_commands="reload"
mail/openarc/files/openarc.in:extra_commands="reload"
mail/noattach/files/noattach.in:extra_commands="reload"
mail/milter-manager/files/milter-manager.in:extra_commands="reload"
mail/masqmail/files/masqmail.in:extra_commands="reload"
mail/mailscanner/files/mta.in:extra_commands="reload"
mail/mailman/files/mailman.in:extra_commands="reload status reopen"
mail/exim/files/exim.in:extra_commands="reload"
mail/dovecot/files/dovecot.in:extra_commands="reload"
mail/cyrus-imapd38/files/imapd.in:extra_commands="reload"
mail/cyrus-imapd36/files/imapd.in:extra_commands="reload"
mail/cyrus-imapd34/files/imapd.in:extra_commands="reload"
mail/cyrus-imapd32/files/imapd.in:extra_commands="reload"
mail/cyrus-imapd30/files/imapd.in:extra_commands="reload"
mail/cyrus-imapd25/files/imapd.in:extra_commands="reload"
mail/cyrus-imapd24/files/imapd.in:extra_commands="reload"
mail/cyrus-imapd23/files/imapd.in:extra_commands="reload"
lang/php83/files/php-fpm.in:extra_commands="reload configtest logrotate"
lang/php82/files/php-fpm.in:extra_commands="reload configtest logrotate"
lang/php80/files/php-fpm.in:extra_commands="reload configtest logrotate"
lang/php81/files/php-fpm.in:extra_commands="reload configtest logrotate"
japanese/tiarra/files/tiarra.in:extra_commands="reload"
japanese/ebnetd/files/ndtpd.in:extra_commands="reload"
japanese/ebnetd/files/ebhttpd.in:extra_commands="reload"
japanese/ebnetd/files/ebnetd.in:extra_commands="reload"
irc/inspircd/files/inspircd.in:extra_commands="reload status"
irc/atheme-services/files/atheme-services.in:extra_commands="reload"
ftp/vsftpd/files/vsftpd6.in:extra_commands="reload"
ftp/vsftpd/files/vsftpd.in:extra_commands="reload"
ftp/proftpd/files/proftpd.in:extra_commands="reload"
dns/unbound/files/unbound.in:extra_commands="reload"
dns/opendnssec2/files/opendnssec.in:extra_commands="reload status"
dns/nsd/files/nsd.in:extra_commands="reload"
dns/gdnsd2/files/gdnsd.in:extra_commands="reload configtest"
dns/gdnsd3/files/gdnsd.in:extra_commands="stats reload configtest"
dns/dnsmasq/files/dnsmasq.in:extra_commands="reload logstats"
dns/dnsmasq-devel/files/dnsmasq.in:extra_commands="reload logstats"
dns/adsuck/files/adsuck.in:extra_commands="reload"
devel/py-buildbot/files/buildbot.in:extra_commands="check reload"
databases/redis70/files/redis.in:extra_commands="reload"
databases/redis6/files/redis.in:extra_commands="reload"
databases/redis62/files/redis.in:extra_commands="reload"
databases/redis/files/redis.in:extra_commands="reload"
databases/redis-devel/files/redis.in:extra_commands="reload"
databases/postgresql16-server/files/postgresql.in:extra_commands="reload initdb"
databases/postgresql15-server/files/postgresql.in:extra_commands="reload initdb"
databases/postgresql14-server/files/postgresql.in:extra_commands="reload initdb"
databases/postgresql13-server/files/postgresql.in:extra_commands="reload initdb"
databases/postgresql12-server/files/postgresql.in:extra_commands="reload initdb"
databases/postgresql11-server/files/postgresql.in:extra_commands="reload initdb"
databases/pgpool-II-44/files/pgpool.in:extra_commands="reload"
databases/pgpool-II-41/files/pgpool.in:extra_commands="reload"
databases/pgbouncer/files/pgbouncer.in:extra_commands="reload gracefulstop"
databases/pgagroal/files/pgagroal.in:extra_commands="reload"
databases/pgpool-II-42/files/pgpool.in:extra_commands="reload"
databases/p5-Bucardo/files/bucardo.in:extra_commands="reload kick"
databases/pgpool-II-40/files/pgpool.in:extra_commands="reload"
databases/pgpool-II-43/files/pgpool.in:extra_commands="reload"
databases/go-carbon/files/go-carbon.in:extra_commands="reload"
databases/galera/files/garb.sh.in:#extra_commands="reload"
databases/galera26/files/garb.sh.in:#extra_commands="reload"
comms/qpage/files/qpage.in:extra_commands="reload"
comms/conserver-com/files/conserver.in:extra_commands="reload reconnect reinit"
comms/atslog/files/atslogd.in:extra_commands="reload writedb rotate alltodb cleardb"
audio/murmur/files/murmur.in:extra_commands="reload"
audio/icecast/files/icecast.in:extra_commands="reload"
audio/icecast-kh/files/icecast.in:extra_commands="reload"

Comment 5 Adam Weinberger freebsd_committer

2023-09-26 10:59:00 UTC

(In reply to Mina Galić from comment #4)
> no one uses "force" in their extra_commands
Ahh, yes, that is true. Something was bugging me and I couldn't figure out why (hadn't had coffee yet). That's because "force" before a command name has special meaning for rc(8): it ignores the ${name}_enabled check, and that behavior is handled by the rc(8) subsystem itself.

So, 'forcereload' can't be used, and 'force-reload' is similar enough to be confusing (and could theoretically lead to forceforce-reload).

It'll definitely need a different name.

Comment 6 Tom MTT. 2023-09-26 11:52:06 UTC

(In reply to Adam Weinberger from comment #3)
> I'm not sure about the name "forcereload" yet. On one hand, it's doing a 'reload --force' which suggests that it IS the correct name. But on the other hand, if the intent is to force SSL cert regeneration, perhaps the name should reflect that?

While `reload --force` is mostly used when reloading external certificates, I don't think that naming the command to reflect that is a good idea. I can't think of anything in Caddy besides external TLS certs that need a forced config reload as of now, but maybe something will be added to Caddy in the future that would need a forced reload, too? Let me know what you think.

(In reply to Mina Galić from comment #4)
> (I suggested force-reload, I guess forcereload makes more sense in shell)

I actually thought about naming it force-reload but couldn't figure out how to use dashes for rc services commands. Don't think it's possible since shell variables can't have dashes in their name AFAIK.

(In reply to Adam Weinberger from comment #5)
> Ahh, yes, that is true. Something was bugging me and I couldn't figure out why (hadn't had coffee yet). That's because "force" before a command name has special meaning for rc(8): it ignores the ${name}_enabled check, and that behavior is handled by the rc(8) subsystem itself.

Oh, that's good to know. How about using the same command as Haproxy and naming it hardreload then?

I'll send another patch later once I hear your feedback :)

Comment 7 Adam Weinberger freebsd_committer

2023-09-27 04:15:22 UTC

(In reply to Tom MTT. from comment #6)

I feel like hardreload is getting less descriptive. I doubt end users will have any idea of what hardreload does and how it differs from reload. What about reloadssl?

Comment 8 Tom MTT. 2023-09-27 06:49:41 UTC

Created attachment 245270 [details]
Proposed patch for www/caddy (with commit message and reloadssl)

(In reply to Adam Weinberger from comment #7)
> What about reloadssl?

Alright then. Let's go for reloadssl :)

Comment 9 Tom MTT. 2023-10-05 07:20:55 UTC

bump :)

Comment 10 commit-hook freebsd_committer

2023-10-12 02:39:17 UTC

A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=528ac9e77df40d5e8bf424b8a1255be4d2986ec9

commit 528ac9e77df40d5e8bf424b8a1255be4d2986ec9
Author:     Tom MTT <tom@heimdall.pm>
AuthorDate: 2023-10-12 02:18:43 +0000
Commit:     Adam Weinberger <adamw@FreeBSD.org>
CommitDate: 2023-10-12 02:38:40 +0000

    www/caddy: Add reloadssl rc(8) command

    `service caddy reloadssl` instructs Caddy to reload its TLS
    certificates. This is useful when using Caddy with ACME DNS
    providers, as it allows for the renewal of certificates without
    manually deleting the old ones and restarting Caddy.

    PR:             274085

 www/caddy/files/caddy.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comment 11 Adam Weinberger freebsd_committer

2023-10-12 02:39:51 UTC

This is committed. Thank you so much for all your work, and for your patience!

Comment 12 commit-hook freebsd_committer

2023-10-14 02:21:55 UTC

A commit in branch 2023Q4 references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=ec2084ee9a16358571a69a73192d70c02b2e1aff

commit ec2084ee9a16358571a69a73192d70c02b2e1aff
Author:     Tom MTT <tom@heimdall.pm>
AuthorDate: 2023-10-12 02:18:43 +0000
Commit:     Adam Weinberger <adamw@FreeBSD.org>
CommitDate: 2023-10-14 02:20:00 +0000

    www/caddy: Add reloadssl rc(8) command

    `service caddy reloadssl` instructs Caddy to reload its TLS
    certificates. This is useful when using Caddy with ACME DNS
    providers, as it allows for the renewal of certificates without
    manually deleting the old ones and restarting Caddy.

    PR:             274085
    (cherry picked from commit 528ac9e77df40d5e8bf424b8a1255be4d2986ec9)

 www/caddy/files/caddy.in | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)