puppet-letsencrypt icon indicating copy to clipboard operation
puppet-letsencrypt copied to clipboard

Avoid race condition when renewing certificates

Open deric opened this issue 2 years ago • 5 comments

Pull Request (PR) description

This PR makes sure that only single certificate can be processed at the same time. Make sure to obtain exclusive lock before executing renewal command.

flock command is used to obtain exclusive lock (with 30 seconds timeout).

This Pull Request (PR) fixes the following issues

Resolve errors like this, when randomized cron job time is not enough:

Another instance of Certbot is already running.

deric avatar Mar 30 '23 08:03 deric

@ekohl Yes, there are locking mechanisms in cerbot, but the process will end with an error instead of waiting for releasing the lock.

I agree it's pretty complex. Part of the problem is that:

$cron_minute = fqdn_rand(60, $title)
$cron_hour     = fqdn_rand(24, $title)

doesn't provide much flexibility. With ~20-30 certificates you can easily run into collisions. If you have only a few certificates you should be fine (or you could define those times manually).

deric avatar Aug 07 '23 13:08 deric

@deric my whole suggestion was to use the command certbot renew instead of one cronjob per certificate. certbot can figure out what needs renewel on its own. It's a big refactor for the whole module, but it would greatly simplify things.

ekohl avatar Aug 08 '23 10:08 ekohl

I agree with @ekohl . In the past I also had one systemd timer per certificate but that doesn't make much sense. certbot itself is really flexible with different hooks and can deal multiple certificates.

bastelfreak avatar Aug 08 '23 20:08 bastelfreak

@ekohl You're right, reducing per-site cron jobs to a single cron job would be definitely a better option.

deric avatar Aug 09 '23 06:08 deric

There's even an additional advantage: if multiple certificates are renewed at the same time you can reload/restart services which consume multiple certificates (Apache, nginx) only once instead of for each certificate.

ekohl avatar Aug 09 '23 08:08 ekohl