Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- http-client: a caller can bound the response body (`http_request_bounded`, `PrpcClient::with_max_response_bytes`). Nothing is bounded by default — `dstack vmm logs --lines 100000` is a legitimate multi-megabyte fetch — but every client that talks to a guest agent opts in, in the gateway and in the VMM, because a CVM is untrusted and one of them polls on a timer against the whole fleet

### Fixed
- certbot: a certificate covering both a name and its wildcard (`example.com` and `*.example.com`) could never be issued over dns-01. The two authorizations are answered under one `_acme-challenge.example.com`, each with its own TXT value, and the publish step cleared every TXT record at that name before writing its own -- so the second authorization deleted the record answering the first, and the order failed with `Correct value not found for DNS challenge`. Clearing leftovers from an aborted run is now done once per challenge name per issuance, and the records for one name accumulate instead of replacing each other; cleanup afterwards is unchanged, deleting each record this run created by id
- gateway: a node removed via `RemoveNode` silently rejoined the cluster the next time it started, because every node re-registers its own sync address on boot. Once tombstone GC is collecting, that comeback is worse than an annoyance: a stale data directory diverges from every digest, and the divergence repair's full re-exchange resurrects records whose deletes the cluster already collected. Removal now writes a durable marker — a live record, so the GC can never eat it — that every gateway's sync endpoints enforce; a removed node's envelopes are refused until an operator re-admits it with `SetNodeUrl`. The refused node counts HTTP 403 sync rejections (`dstack_gateway_sync_rejected_total`), including removal lockouts and app-identity mismatches. Every gateway also exposes `dstack_gateway_node_last_seen_timestamp_seconds` per known node, so a long-offline gateway is a one-line alert instead of an ack-watermark puzzle
- gateway: deleted KV records left a tombstone that nothing ever collected, so every deregistered CVM stayed on disk for the life of the deployment. Tombstones every peer has acknowledged are now dropped once every `tombstone_gc_writes` replicated writes (default 10000, zero disables); the trigger counts replicated writes rather than reading a clock, so nodes in a cluster collect in the same window without depending on time synchronization. A `SetTombstoneGcConfig` admin RPC stores an operator override in the KV itself, replicating one pace to every node
- gateway: `Admin.RemoveCvm` now reports the outcome of the `inst/` tombstone alone. A failure to delete associated override or telemetry records is logged instead of failing the call, so a removal that did take effect is no longer reported as failed — which also aborted the local routing cleanup that follows it. Re-issuing a removal still sweeps up override and telemetry records orphaned by an earlier partial failure
Expand Down
65 changes: 60 additions & 5 deletions dstack/certbot/src/acme_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -338,11 +338,22 @@ impl AcmeClient {

let acme_domain = challenge_domain(challenge.identifier())?;
let dns_value = challenge.key_authorization().dns_value();
debug!("removing existing TXT record for {acme_domain}");
self.dns01_client
.remove_txt_records(&acme_domain)
.await
.context("failed to remove existing dns record")?;
// Clearing stale records is a per-name preparation step, not a
// per-authorization one. An order for `example.com` and
// `*.example.com` yields two authorizations that are both answered
// under `_acme-challenge.example.com`, each with its own value, and
// both values have to be live at validation time. Purging again for
// the second authorization would delete the record the first one
// just published, so one of the two challenges could never be
// answered and the order failed with "Correct value not found for
// DNS challenge".
if needs_purge(challenges, &acme_domain) {
debug!("removing existing TXT records for {acme_domain}");
self.dns01_client
.remove_txt_records(&acme_domain)
.await
.context("failed to remove existing dns record")?;
}
debug!(
"creating TXT record for {acme_domain} with TTL {}s",
self.dns_txt_ttl
Expand Down Expand Up @@ -649,6 +660,17 @@ impl AcmeClient {
}
}

/// Whether the stale TXT records under `acme_domain` still have to be cleared.
///
/// The purge runs once per challenge name per issuance: the records this run
/// has already published live under the names in `published`, and clearing
/// those would take an answered challenge back down.
fn needs_purge(published: &[Challenge], acme_domain: &str) -> bool {
!published
.iter()
.any(|challenge| challenge.acme_domain == acme_domain)
}

/// The name of the TXT record that answers a dns-01 challenge for `identifier`.
///
/// The record always lives under the bare name: a wildcard authorization for
Expand Down Expand Up @@ -922,3 +944,36 @@ mod challenge_domain_tests {
assert!(challenge_domain(&ip.authorized(false)).is_err());
}
}

#[cfg(test)]
mod purge_tests {
use super::{needs_purge, Challenge};

fn challenge(acme_domain: &str, dns_value: &str) -> Challenge {
Challenge {
id: format!("rec-{dns_value}"),
acme_domain: acme_domain.to_string(),
dns_value: dns_value.to_string(),
}
}

/// A base name and its wildcard are two authorizations answered under one
/// `_acme-challenge.<name>`. The first one clears whatever an aborted run
/// left behind; the second must publish alongside it instead of wiping it.
#[test]
fn a_name_is_cleared_once_per_issuance() {
let mut published = vec![];
assert!(needs_purge(&published, "_acme-challenge.example.com"));

published.push(challenge("_acme-challenge.example.com", "value-for-base"));
assert!(!needs_purge(&published, "_acme-challenge.example.com"));
}

/// A SAN list can span zones, and clearing one challenge name says nothing
/// about the others.
#[test]
fn an_untouched_name_is_still_cleared() {
let published = vec![challenge("_acme-challenge.example.com", "value-for-base")];
assert!(needs_purge(&published, "_acme-challenge.example.org"));
}
}
Loading