Hi Clément, Thanks for taking the time to review my change. I've responded inline below.
On Mon, Jan 29 2024, Clément Lassieur wrote: > This is great, thank you! I tested it, it worked. Could you please > just make sure lines fit within 80 columns? Yep, no worries. > Would it make sense now to run ‘update-certificates’ at end of the > activation stuff? We can't run it during activation, because nginx won't have started yet. However, I am planning a follow-up to add a one-shot service to run certbot after nginx starts. I'll see if I can add it to this series, but if I run into any issues I'll leave it for later. > And would it make sense to reload nginx after ‘update-certificates’ is > run? This would be a sensible default. There is an example in the manual of configuring certbot to reload nginx, so this should be straightforward to add. > gnu/services/certbot.scm:203:26: warning: "subjectAltName=~{DNS:~a~^,~}": > unsupported format option ~{, use (ice-9 format) instead Ha! I import (ice-9 format), but within the gexp (and then I don't use it, whoops!). Must be a leftover from a previous iteration. I'll fix this up. >> + ;; Due to the way certbot runs, we need to >> + ;; create the self-signed certificates in the >> + ;; archive folder and symlink them into the live >> + ;; folder. This mimics what certbot does well >> + ;; enough to make acquiring new certificates >> + ;; work. > > In another mail you say it doesn't work as well as you thought it did? > What doesn't work? This comment doesn't describe the code any more. In my first attempt I was trying to generate certificates in /etc/letsencrypt/live/ and get certbot to write over them when it ran. Unfortunately, it refused to do so. I then tried writing to /etc/letsencrypt/archive/ and symlinking into /etc/letsencrypt/live/ (which is what this comment describes), but that also failed. Certbot refuses to write over any existing files when fetching a certificate. It looks like other acme clients might be happier to overwrite existing files, but changing away from certbot seemed like more work than adding a deploy hook to do what we need. I'll follow up with a v2 of this patch when I get a chance. Carlo