Dstack-TEE / Dstack-TEE/dstack
certbot: shutdown cancels an in-flight ACME order and skips DNS-01 TXT cleanup
- Dominant language
- Rust
- Stars
- 544
- Forks
- 96
- Avg merge
- 17h 57m
- Merged PRs (30d)
- 117
Description
Follow-up to #924.
The shutdown path added in #924 cancels the daemon future rather than letting it unwind:
```rust
// dstack/certbot/cli/src/main.rs
tokio::select! {
_ = bot.run() => unreachable!("certbot daemon returned"),
result = shutdown_signal() => result?,
}
```
If the signal lands while `renew_inner` is mid-ACME-order, `bot.run()` is dropped at its current await point and the cleanup at the end of the DNS-01 flow never runs:
```rust
// dstack/certbot/src/acme_client.rs:185
if let Err(err) = self.dns01_client.remove_record(&challenge.id).await {
error!("failed to remove dns record {}: {err}", challenge.id);
}
```
Result: a stale `_acme-challenge` TXT record left in the DNS zone, plus a pending authorization at the CA.
This is **not a regression** — before #924 the default SIGTERM disposition killed the process at the same point with the same effect — and it is self-healing, because `set_txt_records` calls `remove_txt_records(&acme_domain)` before publishing new ones on the next attempt. But "stop the daemon cleanly" currently means "stop promptly", not "stop without leaving state behind", and the gap is worth closing.
## Proposal
Give the loop a cancellation token instead of dropping the future:
- check the token at the top of each iteration and in the interval wait (`select!` between `sleep(renew_interval)` and cancellation) — this covers the idle case, which is the overwhelmingly common one and is already instant today;
- for the in-flight case, either let the current renewal run to completion under a bounded grace period before exiting, or make the DNS-01 challenge cleanup drop-safe (scope guard) so cancellation at any await point still removes the TXT record.
The grace period must stay bounded — `renew_timeout` already caps a single renewal, so reusing it as the shutdown deadline is a reasonable ceiling.
Contributor guide
Research direction
Read dstack/certbot/cli/src/main.rs and dstack/certbot/src/acme_client.rs, especially the shutdown select, renew_inner flow, and DNS-01 cleanup near line 185. Trace the existing renew_timeout behavior and run the certbot tests or shutdown-related checks. Done means shutdown handles both idle and in-flight renewals within a bounded deadline without leaving the TXT record or authorization state behind.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend, cli, security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Mostly clear
- Newbie friendliness
- 52/100