From d651b61ad424d2eddc0d64f421cc4437cc526fa8 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Thu, 15 Oct 2026 17:57:34 -0700 Subject: [PATCH] fix(ingress): retry a failed dns-01 issuance only after a new account process_domain retried every failed issuance once, after writing the CAA. The retry exists for one case: a fresh account refused by a CAA record naming an earlier one, which set_caa_record has just replaced. For any other failure it doubled the failed validations, so a domain that keeps failing reached Let's Encrypt's limit of five per hostname per hour within three passes. Retry only when the first attempt had to create the account and this container manages the CAA. Also drop the status 2 -> 0 mapping after the retry: run_pass already detects a saved certificate by checksum. Signed-off-by: Kevin Wang --- custom-domain/dstack-ingress/TESTING.md | 2 +- custom-domain/dstack-ingress/scripts/dns01.sh | 23 +++++++++++-------- 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/custom-domain/dstack-ingress/TESTING.md b/custom-domain/dstack-ingress/TESTING.md index 59fee0e..c279cff 100644 --- a/custom-domain/dstack-ingress/TESTING.md +++ b/custom-domain/dstack-ingress/TESTING.md @@ -313,7 +313,7 @@ operator's CNAME. Expect the reason quoted back, e.g. `issue allows Note also that the first pass on a fresh deployment necessarily runs before any CAA exists: the record names the ACME account, so the account has to exist first. `process_domain` issues, writes the CAA, and issues again only if the -first attempt failed. +first attempt failed and had to create the account. ## Traps diff --git a/custom-domain/dstack-ingress/scripts/dns01.sh b/custom-domain/dstack-ingress/scripts/dns01.sh index 7eb3031..2090c64 100644 --- a/custom-domain/dstack-ingress/scripts/dns01.sh +++ b/custom-domain/dstack-ingress/scripts/dns01.sh @@ -267,7 +267,7 @@ set_caa_record() { } process_domain() { - local domain="$1" status + local domain="$1" status retry=false if [ -n "${DELEGATION_ZONE:-}" ]; then # Delegation: the operator created three CNAMEs once, before deploying. @@ -303,20 +303,23 @@ process_domain() { fi # The CAA record names our ACME account, so the account has to exist first: - # try once, write CAA, try again. The first attempt is expected to fail when - # a CAA record from an earlier account is still in place. Retry only then: - # after a success (0) or "nothing to renew" (2) a second attempt would - # issue another certificate whenever renewal is due on every check. + # try once, write CAA, and try again if the first attempt failed while + # creating the account. That is the attempt a CAA record naming an earlier + # account refuses, and set_caa_record has just replaced it. Any other + # failure would only fail again, spending another of Let's Encrypt's five + # failed validations per hostname per hour on every pass. + if [ -n "${DELEGATION_ZONE:-}" ] || [ "$SET_CAA" = "true" ]; then + acme_account_file >/dev/null 2>&1 || retry=true + fi if issue_certificate "$domain"; then status=0; else status=$?; fi set_caa_record "$domain" - if [ "$status" -ne 0 ] && [ "$status" -ne 2 ]; then - echo "First certificate attempt failed for $domain, retrying after the CAA record is set" + # Never after a success (0) or "nothing to renew" (2): a second attempt + # would issue another certificate whenever renewal is due. + if [ "$retry" = true ] && [ "$status" -ne 0 ] && [ "$status" -ne 2 ]; then + echo "First certificate attempt failed for $domain, retrying now that the CAA record names the new account" if issue_certificate "$domain"; then status=0; else status=$?; fi - # The failed attempt may still have saved a certificate (e.g. killed by - # the timeout after writing it), so "nothing to renew" now is a change. - [ "$status" -ne 2 ] || status=0 fi return "$status" }