Skip to content

Commit 140173f

Browse files
committed
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 <wy721@qq.com>
1 parent 9e8cf69 commit 140173f

2 files changed

Lines changed: 14 additions & 11 deletions

File tree

‎custom-domain/dstack-ingress/TESTING.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ operator's CNAME. Expect the reason quoted back, e.g. `issue allows
313313
Note also that the first pass on a fresh deployment necessarily runs before any
314314
CAA exists: the record names the ACME account, so the account has to exist
315315
first. `process_domain` issues, writes the CAA, and issues again only if the
316-
first attempt failed.
316+
first attempt failed and had to create the account.
317317

318318
## Traps
319319

‎custom-domain/dstack-ingress/scripts/dns01.sh‎

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ set_caa_record() {
267267
}
268268

269269
process_domain() {
270-
local domain="$1" status
270+
local domain="$1" status retry=false
271271

272272
if [ -n "${DELEGATION_ZONE:-}" ]; then
273273
# Delegation: the operator created three CNAMEs once, before deploying.
@@ -303,20 +303,23 @@ process_domain() {
303303
fi
304304

305305
# The CAA record names our ACME account, so the account has to exist first:
306-
# try once, write CAA, try again. The first attempt is expected to fail when
307-
# a CAA record from an earlier account is still in place. Retry only then:
308-
# after a success (0) or "nothing to renew" (2) a second attempt would
309-
# issue another certificate whenever renewal is due on every check.
306+
# try once, write CAA, and try again if the first attempt failed while
307+
# creating the account. That is the attempt a CAA record naming an earlier
308+
# account refuses, and set_caa_record has just replaced it. Any other
309+
# failure would only fail again, spending another of Let's Encrypt's five
310+
# failed validations per hostname per hour on every pass.
311+
if [ -n "${DELEGATION_ZONE:-}" ] || [ "$SET_CAA" = "true" ]; then
312+
acme_account_file >/dev/null 2>&1 || retry=true
313+
fi
310314
if issue_certificate "$domain"; then status=0; else status=$?; fi
311315

312316
set_caa_record "$domain"
313317

314-
if [ "$status" -ne 0 ] && [ "$status" -ne 2 ]; then
315-
echo "First certificate attempt failed for $domain, retrying after the CAA record is set"
318+
# Never after a success (0) or "nothing to renew" (2): a second attempt
319+
# would issue another certificate whenever renewal is due.
320+
if [ "$retry" = true ] && [ "$status" -ne 0 ] && [ "$status" -ne 2 ]; then
321+
echo "First certificate attempt failed for $domain, retrying now that the CAA record names the new account"
316322
if issue_certificate "$domain"; then status=0; else status=$?; fi
317-
# The failed attempt may still have saved a certificate (e.g. killed by
318-
# the timeout after writing it), so "nothing to renew" now is a change.
319-
[ "$status" -ne 2 ] || status=0
320323
fi
321324
return "$status"
322325
}

0 commit comments

Comments
 (0)