fix(dstack-ingress): renew one lineage per run, and size the timeout to the wait

[kvinwang]

Stacked on #110 — base retargets to main once that merges. Review the second commit.

Two long-standing problems (both present since v1.0) that compound each other.

1. certbot renew was never scoped

certbot renew renews every lineage in /etc/letsencrypt/renewal unless given --cert-name, and we never gave it one:

cmd = self._build_certbot_command("renew", domain, "")   # no --cert-name

run_pass calls this once per domain, so N domains meant N runs each covering all N lineages. renew_certificate then reported the result against whichever domain asked, so domain A could be recorded as renewed because domain B was — and that flag is what drives evidence regeneration and the haproxy reload.

Each run is now scoped to the lineage it is named for.

2. The run timeout was a fixed 300 s

The dominant term inside a certbot run is the DNS propagation wait, which the plugin sleeps through in-process. A single fixed cap cannot fit every provider:

provider	CERTBOT_PROPAGATION_SECONDS	under the old 300 s cap
linode	300	never fits — dns-01 timed out every time, issuance included
cloudflare / namecheap	120	fine for one lineage; before --cert-name, three domains could not fit
route53	None	unaffected

The timeout is now derived from the wait (propagation + 180 s headroom, or DELEGATION_PROPAGATION_SECONDS under delegation) and CERTBOT_TIMEOUT overrides it. The renew path also gets a dedicated TimeoutExpired branch that names the knob instead of falling through to a generic handler.

Why --cert-name is safe here

renew_certificate detects a no-op by looking for "No renewals were attempted". That string comes from _renew_describe_results, which runs for the renew subcommand whether or not the lineage set was filtered — --cert-name only narrows the candidates. Verified against certbot 5.7.0, the version the image installs:

renewal.py:606:  notify(f"No {renewal_noun}s were attempted.")

So the existing reload-only-on-actual-renewal behaviour (#102) is preserved. Parsing prose is still fragile — a --deploy-hook marker, as the lego path already uses, would be better, but that is a separate change.

Testing

11 new cases in scripts/tests/test_certman.py (20 total in the file, all green, plus test_dnsguide.py 27 and the sanitizer suite):

• scope: plain domain, wildcard → bare lineage name, delegation mode, and certonly still using -d rather than --cert-name
• timeout: per-provider sizing, linode's 300 s fitting, delegation's own setting, its fallback, explicit override, invalid override ignored, provider with no propagation setting

Checked both ways — the five cases that assert the new behaviour fail against the previous code:

FAIL: test_renew_is_scoped_to_one_lineage
ERROR: test_renew_uses_the_bare_name_for_a_wildcard
FAIL: test_linode_default_propagation_fits
FAIL: test_delegation_uses_its_own_propagation_setting
FAIL: test_provider_without_propagation_still_gets_headroom

How it turned up

Testing 2.3. Two dns-01 renewals hit the 300 s cap; on dns-01 that fails the bootstrap pass, which runs before haproxy serves anything, so the container exits. My case was one cloudflare domain, so the arithmetic alone does not explain it — concurrent load on the same zone probably contributed — but chasing it surfaced both defects above, which are structural rather than incidental.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[kvinwang]

On CI: the workflows here trigger on pull_request: branches: [main, develop], so a PR based on another PR's branch gets no checks until it retargets. GitHub will retarget this to main automatically when #110 merges, and CI runs then.

In the meantime I ran the same checks locally on this branch:

• ./dev.sh check-all — what Basic Checks runs: All checks completed! (the shellcheck notes it prints are pre-existing, in unrelated files)
• python3 scripts/tests/test_certman.py — 20 tests, OK
• python3 scripts/tests/test_dnsguide.py — 27 tests, OK
• bash scripts/tests/test_sanitizers.sh — all passed

Reviewing just the second commit (6f2fe45) gives this PR's diff; the first is #110.

[kvinwang]

Independent review found a regression this PR introduced. Fixed in 4d62c57; two other findings refuted with evidence, one filed separately.

Taken — route53's budget was cut from 300 s to 180 s

route53 declares CERTBOT_PROPAGATION_SECONDS = None, so sizing purely from the wait gave it 0 + 180. A renewal that used to complete in between would now be killed. The sizing was meant to raise the cap where a provider needs more, not lower it anywhere.

Floored at the 300 s it replaced:

provider	before	after
cloudflare / namecheap	300	300
linode	300 (impossible — its own wait is 300)	480
route53	300	300

An explicit CERTBOT_TIMEOUT is still taken literally, floor included. New test walks every propagation value we ship (None, 0, 30, 120, 300) asserting none ends up under 300.

Refuted — one propagation wait per lineage, not per identifier

The concern was that a multi-SAN lineage might sleep once per identifier, making propagation + headroom insufficient. certbot 5.7.0 plugins/dns_common.py:

for achall in achalls:
    ...
    self._perform(domain, validation_domain_name, validation)
    responses.append(...)
# one sleep, after the loop
sleep(self.conf('propagation-seconds'))

_perform for each challenge, then a single sleep. The count is per lineage regardless of SAN count — and every lineage this image creates is single-domain anyway.

Refuted — --cert-name does not change the no-op wording

The concern was that if --cert-name altered certbot's summary, renew_certificate would return (True, True) and reload haproxy on every pass. certbot 5.7.0 _internal/renewal.py:606, in _renew_describe_results:

if not renew_successes and not renew_failures:
    notify(f"No {renewal_noun}s were attempted.")

That runs for the renew subcommand unconditionally; --cert-name only narrows which lineages become candidates. The detection is unchanged.

The broader point stands and I agree with it: keying on an English sentence is fragile across versions and locales. The lego path already uses a --deploy-hook marker file; doing the same for certbot is the right fix and I have left it out of this PR deliberately rather than bundle a third change.

Filed separately — wildcards never reach renew_certificate

certificate_exists() checks the unnormalised domain, so for *.example.com it stats a path that cannot exist (os.path.isfile does not glob) while certbot stores the lineage bare — confirmed in client.py _choose_lineagename. Consequences: auto always re-obtains, apply_renewal_window is unreachable, and every pass reports needs_evidence=True, so evidence is regenerated and haproxy reloaded every cycle for wildcard deployments — the behaviour #102 fixed, still live for wildcards.

This also means the wildcard --cert-name case here tests command construction for a path production cannot currently execute. I have kept the test — it is correct and becomes load-bearing once the existence check is fixed — but it does not demonstrate wildcard renewal, and the PR body should not be read as claiming it does.

Remaining known gaps, not addressed here: lineages with collision suffixes (example.com-0001) or explicit names are not discoverable from the domain, and a live cert whose renewal config is missing now fails explicitly instead of silently renewing everything else — which I consider an improvement, but it is a behaviour change worth knowing about.

24 cases in test_certman.py, ./dev.sh check-all green.