Skip to content

feat(ssl): add inter-site jitter and clear rate-limit messaging to ssl-renew - #499

Open
mrrobot47 wants to merge 4 commits into
EasyEngine:developfrom
mrrobot47:fix/ssl-renew-rate-limit
Open

mrrobot47 wants to merge 4 commits into
EasyEngine:developfrom
mrrobot47:fix/ssl-renew-rate-limit

Conversation

@mrrobot47

@mrrobot47 mrrobot47 commented Jun 30, 2026 •

Copy link
Copy Markdown
Member

Problem

ssl-renew --all fires per-site ACME operations back-to-back with no spacing, and on a Let's Encrypt rate limit authorize() shows a misleading "you're in a local environment or using a non-public domain" message instead of saying it's a rate limit.

Fix

  • Jitter between actual renewals: in renew_ssl_cert(), after the "is renewal due / --force" check, a sleep( random_int( 1, 5 ) ) before every real renewal except the first in the process, to avoid bursting against Let's Encrypt's new-order/burst limits. Sites that aren't due (almost all of them on a normal night) and single-site ee site ssl-renew <site> never sleep. The flag is a process-wide static because --all renews every site in one process.
  • Clear rate-limit messaging: is_rate_limit_exception() detects rate limits via RateLimitedServerException (acmephp maps every rateLimited ACME problem to it), plus a rateLimited marker in the message. authorize() and both executeRenewal() catches then say "Let's Encrypt rate limit hit for: … Please wait before retrying." with the rate-limits docs link. A rate limit hit while revoking the previous authorizations (the first new-order of every renewal) is now caught there too, so the renewal reports the rate limit instead of crashing.

With this change a rate-limited site returns false and ssl-renew --all continues. Other uncaught exceptions or EE::error() in one site still abort the whole batch (pre-existing; out of scope).

Out of scope (follow-ups)

A fully failure-resilient --all with automatic backoff-and-retry, and removing the dead if ( $all ) branch in ssl_renew(), are larger changes left for later.

Note for merge

This edits executeRenewal()'s catch blocks, which #486 also edits — whichever merges second will need a small rebase to reconcile.

Testing

Manual: ssl-renew --all with several LE sites that aren't due runs without any sleep; with two or more due sites there is a 1–5s pause before each renewal after the first. The rate-limit path was verified against acmephp with a mocked 429 rateLimited response (both authorize() and the revoke-then-authorize renewal flow print the rate-limit message and return false); unrelated errors such as "Too many open files" or rejectedIdentifier keep their own messages.

Tested on Ubuntu 26.04 with EasyEngine 4.12.0 with five sites flagged as Let's Encrypt (three not due, two due), and on PHP 7.4 and 8.5 for the rate-limit handling.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

revokeAuthorizationChallenges() runs before authorize() on every renewal and also calls new-order, so a Let's Encrypt rate limit surfaced there as an uncaught RateLimitedServerException: the new rate-limit message was never shown and `ssl-renew --all` aborted on that site. Stop revoking on a rate limit and let authorize() report it.
The fallback matched any error containing "too many" (e.g. "Too many open files"), which would be reported as a Let's Encrypt rate limit. acmephp maps every `rateLimited` ACME error to RateLimitedServerException, so the class check plus the `rateLimited` marker is enough. Also correct the docblock, which claimed HTTP 429 detection the code never did.
…crypt

The jitter slept before every dispatched LE site in `ssl-renew --all`, including the sites whose certificates aren't due, which make no ACME calls. On the daily cron that added about 3s per site for nothing (roughly 5 minutes for 100 sites) while holding up the run. Move it into renew_ssl_cert() after the renewal-necessity check, so it only spaces out real renewals within one process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants