Skip to content

Commit ed3ca43

Browse files
committed
fix(dstack-ingress): stop dns-01 running the first pass twice
dns-01 issues before it serves, so run_pass is called explicitly at startup and cert_loop then ran it again immediately. For a single domain that is four certbot invocations and two rounds of DNS writes on every container start, all to reach a state the first pass had already reached. The duplicate is inherited from upstream, where bootstrap and the renewal daemon both ran at startup with no initial sleep; the refactor kept it faithfully. tls-alpn-01 never had it because there is no bootstrap pass to duplicate. Also deduplicate CAA records after parsing rather than before. Resolvers disagree on the wire format -- Cloudflare returns RFC 3597 generic hex, Google the presentation form -- so one record arrives as two distinct strings and its rejection reason was printed twice.
1 parent 0dcd796 commit ed3ca43

3 files changed

Lines changed: 55 additions & 6 deletions

File tree

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

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -234,20 +234,26 @@ run_pass() {
234234
[ "$failed" -eq 0 ]
235235
}
236236

237+
# $1: seconds to wait before the first pass. dns-01 issues before it serves, so
238+
# the caller has already run a pass by the time we get here; without this the
239+
# loop would immediately repeat it -- two full passes, and for a single domain
240+
# four certbot invocations plus a duplicate round of DNS writes, on every start.
237241
cert_loop() {
238-
local attempt=0 delay
242+
local attempt=0 delay="${1:-0}"
239243
while true; do
244+
if [ "$delay" -gt 0 ]; then
245+
echo "[$(date)] Next certificate check in ${delay}s"
246+
sleep "$delay"
247+
fi
240248
if run_pass; then
241249
attempt=0
242250
delay=${RENEW_INTERVAL:-43200}
243251
else
244252
attempt=$((attempt + 1))
245253
delay=$((60 * attempt))
246254
[ "$delay" -gt 1800 ] && delay=1800
247-
echo "[$(date)] Pass failed (attempt ${attempt}); retrying in ${delay}s"
255+
echo "[$(date)] Pass failed (attempt ${attempt}); retrying" >&2
248256
fi
249-
echo "[$(date)] Next certificate check in ${delay}s"
250-
sleep "$delay"
251257
done
252258
}
253259

@@ -265,6 +271,6 @@ haproxy_emit_tls_frontend ":${PORT}"
265271
haproxy_emit_backends
266272

267273
evidence_start_server
268-
cert_loop &
274+
cert_loop "${RENEW_INTERVAL:-43200}" &
269275

270276
exec "$@"

custom-domain/dstack-ingress/scripts/dnsguide.py

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,15 @@ def check_caa(
265265
candidate = ".".join(labels[i:])
266266
# Union: one resolver seeing a restriction is enough to honour it.
267267
rdatas = query_union(candidate, RR_CAA, resolvers)
268-
parsed = [p for p in (_parse_caa(r) for r in rdatas) if p is not None]
268+
# Deduplicate after parsing, not before: resolvers disagree on the wire
269+
# format (one returns presentation form, another RFC 3597 generic hex),
270+
# so the same record arrives as two distinct strings and would otherwise
271+
# be reported, and counted, twice.
272+
parsed = []
273+
for rdata in rdatas:
274+
record = _parse_caa(rdata)
275+
if record is not None and record not in parsed:
276+
parsed.append(record)
269277
relevant = [p for p in parsed if p[1] == want_tag]
270278
if not parsed:
271279
continue # keep climbing; this level has no CAA record set

custom-domain/dstack-ingress/scripts/tests/test_dnsguide.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,41 @@ def test_account_unknown_skips_the_comparison(self):
8484
self.assertTrue(ok)
8585

8686

87+
class TestCaaCheckDeduplication(unittest.TestCase):
88+
"""One record seen twice, in two wire formats, must be reported once.
89+
90+
Resolvers disagree on CAA presentation: Cloudflare hands back RFC 3597
91+
generic hex, Google the human-readable form. The union is taken over raw
92+
rdata, so the same record arrives as two different strings.
93+
"""
94+
95+
PRESENTATION = '0 issue "letsencrypt.org;validationmethods=dns-01"'
96+
# The same record, generic-encoded: flags=0, tag len 5, "issue", value.
97+
GENERIC = (
98+
r"\# 44 00 05 69 73 73 75 65 6c 65 74 73 65 6e 63 72 79 70 74 2e 6f 72 67 "
99+
r"3b 76 61 6c 69 64 61 74 69 6f 6e 6d 65 74 68 6f 64 73 3d 64 6e 73 2d 30 31"
100+
)
101+
102+
def setUp(self):
103+
self._saved = dnsguide.query_union
104+
dnsguide.query_union = lambda name, rr, resolvers: (
105+
[self.GENERIC, self.PRESENTATION] if name == "app.example.com" else []
106+
)
107+
self.addCleanup(lambda: setattr(dnsguide, "query_union", self._saved))
108+
109+
def test_both_encodings_parse_to_the_same_record(self):
110+
self.assertEqual(
111+
dnsguide._parse_caa(self.GENERIC), dnsguide._parse_caa(self.PRESENTATION)
112+
)
113+
114+
def test_reason_is_not_repeated(self):
115+
ok, reason = dnsguide.check_caa(
116+
"app.example.com", "issue", "tls-alpn-01", "", "r1,r2"
117+
)
118+
self.assertFalse(ok)
119+
self.assertEqual(reason.count("excludes tls-alpn-01"), 1, reason)
120+
121+
87122
class TestTxtNormalisation(unittest.TestCase):
88123
def test_strips_quotes(self):
89124
self.assertEqual(dnsguide._unquote_txt('"abc:443"'), "abc:443")

0 commit comments

Comments
 (0)