From 37eda20dc39afb188d62f32f115d43ec8514ab9d Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 25 Aug 2026 08:11:16 -0700 Subject: [PATCH 1/5] fix(gateway): register the shared ACME account under the rotation lock --- CHANGELOG.md | 1 + dstack/gateway/src/distributed_certbot.rs | 122 +++++++++++++++++++++- 2 files changed, 119 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e66ad168a..23fa2a681 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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 - certbot: editing `domains` in `certbot.toml` had no effect once a certificate existed. Issuance was skipped whenever `live/cert.pem` was present, whatever names it carried, and renewal read its name list back off that certificate rather than the configuration -- so an added or removed name never reached the CA, and the mismatch survived every renewal. The live certificate's DNS names are now compared against the configured list (as sets, case- and trailing-dot-insensitive) and a mismatch reissues, logging both lists. A reissue that fails does not take the renewal check down with it: a name the CA will not validate is reported on every cycle, while the certificate actually being served keeps renewing, and the failure is still what the run returns unless the renewal committed something of its own - certbot: `certbot cfg` attached every comment after `cf_api_url` to the wrong key, because an absent optional field shifts the generated document's keys out of step with the struct's. The template's documentation is now looked up by key name +- gateway: a fresh cluster could register several ACME accounts at once. The shared account was registered lazily, on whichever renewal first found the credentials record empty, under no lock but the per-domain renewal lock -- so two domains, or two nodes, starting together each spent a rate-limited registration, and the last-writer-wins credentials record kept exactly one of them. The attestation written beside it races under its own key, so the account the cluster ends up using need not be the one it can prove it holds. Registration now takes the same lock rotation takes and re-reads the record under it, adopting an account another node registered while it waited; the DNS provider client is built only after the lock is granted, so a refused attempt costs no provider API call - 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 diff --git a/dstack/gateway/src/distributed_certbot.rs b/dstack/gateway/src/distributed_certbot.rs index 9b5ffacfd..60215e634 100644 --- a/dstack/gateway/src/distributed_certbot.rs +++ b/dstack/gateway/src/distributed_certbot.rs @@ -558,9 +558,6 @@ impl DistributedCertBot { // Get DNS credential (from config or default) let dns_cred = dns_credential_for(&self.kv_store, config)?; - // Create DNS client based on provider - let dns01_client = self.dns_client(domain, &dns_cred).await?; - // Use ACME URL from certbot config, fall back to default if not set let config = self.config()?; let acme_url = if config.acme_url.is_empty() { @@ -590,6 +587,69 @@ impl DistributedCertBot { ); } info!("loaded global ACME account credentials from KvStore"); + let dns01_client = self.dns_client(domain, &dns_cred).await?; + return AcmeClient::load( + dns01_client, + &creds.acme_credentials, + dns_cred.max_dns_wait, + dns_cred.dns_txt_ttl, + ) + .await + .context("failed to load ACME client from KvStore credentials"); + } + + // Registering is the one step in this function that cannot be repeated + // harmlessly. Each run spends a rate-limited registration at the CA, and + // the credentials record is last-writer-wins with no compare-and-swap, + // so a concurrent registration's account is simply dropped -- while the + // attestation written beside it, under its own key with its own + // last-writer-wins race, may well be the one that survives. Renewal + // locks are per domain and do not help: a fresh cluster registers from + // however many domains and nodes start at once. Take the rotation lock, + // which exists for exactly this key, and look again before spending one. + let _serialized = self.caa_lock.lock().await; + let Some(rotation_lock) = self.try_acquire_rotation_lock() else { + bail!( + "another node is registering or rotating the ACME account; \ + retry after it finishes" + ); + }; + let client = self + .register_or_adopt_account(domain, &dns_cred, acme_url) + .await; + if let Err(err) = self.release_rotation_lock(&rotation_lock) { + error!("failed to release ACME rotation lock: {err:?}"); + } + client + } + + /// Register the cluster's shared ACME account, or adopt the one that + /// appeared while this call was waiting for the lock. + /// + /// Called with the rotation lock held. The re-read is the point: without it + /// every waiter registers an account of its own the moment it is let + /// through, which is the race the lock was taken to avoid. + async fn register_or_adopt_account( + &self, + domain: &str, + dns_cred: &DnsCredential, + acme_url: &str, + ) -> Result { + if let Some(creds) = self + .kv_store + .get_acme_credentials() + .context("call RotateAcmeCredentials to replace the stored ACME credentials")? + { + if !acme_url_matches(&creds.acme_credentials, acme_url).context( + "invalid ACME credentials in KvStore; call RotateAcmeCredentials to replace them", + )? { + bail!( + "stored ACME credentials are for a different ACME directory; \ + call RotateAcmeCredentials to switch directories" + ); + } + info!("adopted the ACME account registered while this node waited for the lock"); + let dns01_client = self.dns_client(domain, dns_cred).await?; return AcmeClient::load( dns01_client, &creds.acme_credentials, @@ -600,8 +660,8 @@ impl DistributedCertBot { .context("failed to load ACME client from KvStore credentials"); } - // Create new global ACME account info!("creating new global ACME account at {acme_url}"); + let dns01_client = self.dns_client(domain, dns_cred).await?; let client = AcmeClient::new_account( acme_url, dns01_client, @@ -885,6 +945,60 @@ mod tests { ); } + /// First-use registration must wait on the same lock rotation takes, or a + /// fresh cluster registers one account per node that happens to start a + /// renewal -- and the credentials record, being last-writer-wins, keeps + /// exactly one of them. + /// + /// Reaching the lock at all is the assertion: the DNS provider client is + /// built only after the lock is granted, so an unreachable provider (as + /// configured here) cannot be what this run fails on. + #[tokio::test] + async fn first_use_registration_waits_for_the_rotation_lock() { + let data_dir = tempfile::tempdir().expect("failed to create temp dir"); + let certbot = test_certbot(data_dir.path()); + certbot + .kv_store + .save_dns_credential(&DnsCredential { + id: "cred-1".to_string(), + name: "unreachable".to_string(), + provider: DnsProvider::Cloudflare { + api_token: "token".to_string(), + api_url: Some("http://127.0.0.1:1/client/v4".to_string()), + }, + max_dns_wait: Duration::from_secs(1), + dns_txt_ttl: 60, + created_at: 0, + updated_at: 0, + }) + .expect("failed to store dns credential"); + let config = ZtDomainConfig { + domain: "app.example.com".to_string(), + dns_cred_id: Some("cred-1".to_string()), + port: 443, + node: None, + priority: 0, + }; + + // Another node is mid-registration or mid-rotation. + assert!(certbot + .kv_store + .try_acquire_rotation_lock(ROTATION_LOCK_TIMEOUT_SECS) + .is_some()); + + let err = match certbot + .get_or_create_acme_client("app.example.com", &config) + .await + { + Ok(_) => panic!("registration must not proceed while the lock is held"), + Err(err) => err, + }; + assert!( + err.to_string().contains("registering or rotating"), + "unexpected error: {err}" + ); + } + #[tokio::test] async fn rotate_acme_credentials_rejects_concurrent_runs() { let data_dir = tempfile::tempdir().expect("failed to create temp dir"); From b0b73f26aecd98141aef446855cdf0bd02193b67 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 25 Aug 2026 21:32:03 -0700 Subject: [PATCH 2/5] fix(gateway): one lock for every operation on the shared ACME account Rotation, CAA reconciliation, and first-use registration all read or re-pin the same account, so they now take the same KV lock. Two consequences. Registration under the lock cannot be reached from inside a region that already holds it: set_caa_all held the in-process caa_lock across its per-domain loop, and a fresh cluster's first SetCaa registers from inside that loop -- which, with a non-reentrant tokio Mutex and no timeout, hung the task and left the lock held for the life of the process. set_caa_all now registers the account before it takes the lock, where the steady state costs one KV read. Dropping caa_lock for the KV lock also widens what is ordered: CAA reconciliation was serialized within a process only, so one node's SetCaa could interleave with another's rotation over the same zone. Reconciling rewrites a zone's issuer records in place -- guard, sweep, write, unguard -- and two runs over one zone delete each other's records, which can leave the ";" guards behind and block issuance until a later run succeeds. --- CHANGELOG.md | 2 +- dstack/gateway/src/distributed_certbot.rs | 364 +++++++++++++++------- dstack/gateway/src/kv/mod.rs | 7 +- 3 files changed, 263 insertions(+), 110 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23fa2a681..4636d26da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,7 +37,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - 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 - certbot: editing `domains` in `certbot.toml` had no effect once a certificate existed. Issuance was skipped whenever `live/cert.pem` was present, whatever names it carried, and renewal read its name list back off that certificate rather than the configuration -- so an added or removed name never reached the CA, and the mismatch survived every renewal. The live certificate's DNS names are now compared against the configured list (as sets, case- and trailing-dot-insensitive) and a mismatch reissues, logging both lists. A reissue that fails does not take the renewal check down with it: a name the CA will not validate is reported on every cycle, while the certificate actually being served keeps renewing, and the failure is still what the run returns unless the renewal committed something of its own - certbot: `certbot cfg` attached every comment after `cf_api_url` to the wrong key, because an absent optional field shifts the generated document's keys out of step with the struct's. The template's documentation is now looked up by key name -- gateway: a fresh cluster could register several ACME accounts at once. The shared account was registered lazily, on whichever renewal first found the credentials record empty, under no lock but the per-domain renewal lock -- so two domains, or two nodes, starting together each spent a rate-limited registration, and the last-writer-wins credentials record kept exactly one of them. The attestation written beside it races under its own key, so the account the cluster ends up using need not be the one it can prove it holds. Registration now takes the same lock rotation takes and re-reads the record under it, adopting an account another node registered while it waited; the DNS provider client is built only after the lock is granted, so a refused attempt costs no provider API call +- gateway: a fresh cluster could register several ACME accounts at once. The shared account was registered lazily, on whichever renewal first found the credentials record empty, under no lock but the per-domain renewal lock -- so two domains, or two nodes, starting together each spent a rate-limited registration, and the last-writer-wins credentials record kept exactly one of them. The attestation written beside it races under its own key, so the account the cluster ends up using need not be the one it can prove it holds. One lock in the KV store now covers every operation over the shared account -- rotation, CAA reconciliation, and first-use registration -- so they are ordered across nodes and not merely within one process, as CAA reconciliation was before. Registration re-reads the record under that lock and adopts an account another node registered while it waited, and builds the DNS provider client only after the lock is granted, so a refused attempt costs no provider API call - 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 diff --git a/dstack/gateway/src/distributed_certbot.rs b/dstack/gateway/src/distributed_certbot.rs index 60215e634..f6c631c06 100644 --- a/dstack/gateway/src/distributed_certbot.rs +++ b/dstack/gateway/src/distributed_certbot.rs @@ -15,7 +15,6 @@ use certbot::{AcmeClient, Dns01Client}; use dstack_guest_agent_rpc::v0::RawQuoteArgs; use ra_tls::attestation::QuoteContentType; use ra_tls::rcgen::KeyPair; -use tokio::sync::Mutex; use tracing::{error, info, warn}; use crate::cert_store::CertResolver; @@ -39,11 +38,6 @@ pub struct DistributedCertBot { kv_store: Arc, cert_resolver: Arc, write_notifier: Option>, - /// Serializes CAA reconciliation and credential rotation within this process. - /// - /// Credential rotation is additionally guarded across nodes by a - /// best-effort lock in WaveKV; see [`KvStore::try_acquire_rotation_lock`]. - caa_lock: Mutex<()>, } impl DistributedCertBot { @@ -56,7 +50,6 @@ impl DistributedCertBot { kv_store, cert_resolver, write_notifier, - caa_lock: Default::default(), } } @@ -66,6 +59,28 @@ impl DistributedCertBot { } } + /// Acquire the lock that guards the cluster's shared ACME account. + /// + /// One lock covers every operation over that account -- rotation, CAA + /// reconciliation, and first-use registration -- because they all read or + /// re-pin the same thing, and the record they publish is last-writer-wins + /// with no compare-and-swap. It lives in WaveKV rather than in this + /// process, so it orders two nodes as well as two tasks; see + /// [`KvStore::try_acquire_rotation_lock`] for its best-effort limits. + /// + /// The lock is not reentrant, by design: a stale holder must expire rather + /// than be re-entered. A caller that holds it across a sequence of + /// per-domain work must therefore make sure nothing inside that sequence + /// takes it again -- see [`Self::ensure_acme_account`]. + fn acquire_acme_lock(&self, operation: &str) -> Result { + self.try_acquire_rotation_lock().with_context(|| { + format!( + "cannot {operation}: another rotation, CAA reconciliation, or ACME account \ + registration holds the shared ACME lock; retry after it finishes" + ) + }) + } + fn try_acquire_rotation_lock(&self) -> Option { let lock = self .kv_store @@ -124,13 +139,11 @@ impl DistributedCertBot { /// /// This RPC re-pins issuance to the new account; it does not deactivate the /// old ACME account at the CA. + /// + /// Runs under the shared ACME lock, so a concurrent reconciliation or + /// first-use registration on any node is refused for the duration. pub async fn rotate_acme_credentials(&self) -> Result<(String, usize)> { - let Ok(_guard) = self.caa_lock.try_lock() else { - bail!("ACME credential rotation or CAA reconciliation is already in progress"); - }; - let Some(rotation_lock) = self.try_acquire_rotation_lock() else { - bail!("another node is rotating ACME credentials; retry after it finishes"); - }; + let rotation_lock = self.acquire_acme_lock("rotate ACME credentials")?; let result = self.do_rotate_acme_credentials().await; if let Err(err) = self.release_rotation_lock(&rotation_lock) { error!("failed to release ACME rotation lock: {err:?}"); @@ -291,6 +304,12 @@ impl DistributedCertBot { /// Set CAA records for every configured ZT domain. /// + /// Runs under the shared ACME lock, so a rotation or another + /// reconciliation -- on this node or any other -- is refused for the + /// duration rather than interleaved with it. Reconciling a domain rewrites + /// its issuer records in place (guard, sweep, write, unguard), and two runs + /// over the same zone delete each other's records. + /// /// Reconciliation is per-domain best effort: a failing domain is logged and the /// remaining domains are still reconciled, so one misconfigured domain cannot /// leave the rest unauthorized. Failures are reported together in the returned @@ -302,15 +321,50 @@ impl DistributedCertBot { /// succeeds. The guard window can also fail an ACME order that is in flight for /// the same domain; the periodic renewal task retries, so that is transient. pub async fn set_caa_all(&self) -> Result<()> { - let Ok(_guard) = self.caa_lock.try_lock() else { - bail!("ACME credential rotation or CAA reconciliation is already in progress"); - }; let configs = self.kv_store.list_zt_domain_configs(); if configs.is_empty() { warn!("no ZT-Domain configured, no CAA records to set"); return Ok(()); } + // Before the lock, never under it: registration takes the same lock. + self.ensure_acme_account(&configs).await?; + let rotation_lock = self.acquire_acme_lock("reconcile CAA records")?; + let result = self.do_set_caa_all(configs).await; + if let Err(err) = self.release_rotation_lock(&rotation_lock) { + error!("failed to release ACME rotation lock: {err:?}"); + } + result + } + /// Register the cluster's shared ACME account if it does not exist yet. + /// + /// Called before taking the ACME lock, by callers that hold it across + /// per-domain work that would otherwise register lazily from inside the + /// locked region and be refused by their own lock. The record is read + /// first, so the steady state -- an account already registered -- costs one + /// KV read and no provider or CA round trip. + /// + /// Any configured domain will do: the account is shared, and registration + /// only needs one domain's DNS credential to build a client, the same way + /// rotation uses the first domain's. + async fn ensure_acme_account(&self, configs: &[ZtDomainConfig]) -> Result<()> { + let stored = self + .kv_store + .get_acme_credentials() + .context("call RotateAcmeCredentials to replace the stored ACME credentials")?; + if stored.is_some() { + return Ok(()); + } + let Some(config) = configs.first() else { + return Ok(()); + }; + self.get_or_create_acme_client(&config.domain, config) + .await + .context("failed to register the shared ACME account")?; + Ok(()) + } + + async fn do_set_caa_all(&self, configs: Vec) -> Result<()> { let total = configs.len(); let mut failed = Vec::new(); for config in configs { @@ -566,36 +620,12 @@ impl DistributedCertBot { &config.acme_url }; - // Try to load global ACME credentials from KvStore. A corrupt record - // is an error, not absence: falling through to account registration - // would silently create an account that the account-bound CAA records - // refuse, and burn a rate-limited registration. - let stored_creds = self - .kv_store - .get_acme_credentials() - .context("call RotateAcmeCredentials to replace the stored ACME credentials")?; - if let Some(creds) = stored_creds { - if !acme_url_matches(&creds.acme_credentials, acme_url).context( - "invalid ACME credentials in KvStore; call RotateAcmeCredentials to replace them", - )? { - // Registering a fresh account here would leave every domain's - // CAA pinned to the old account and block issuance; rotation - // re-pins CAA along with the switch. - bail!( - "stored ACME credentials are for a different ACME directory; \ - call RotateAcmeCredentials to switch directories" - ); - } + if let Some(client) = self + .load_stored_acme_client(domain, &dns_cred, acme_url) + .await? + { info!("loaded global ACME account credentials from KvStore"); - let dns01_client = self.dns_client(domain, &dns_cred).await?; - return AcmeClient::load( - dns01_client, - &creds.acme_credentials, - dns_cred.max_dns_wait, - dns_cred.dns_txt_ttl, - ) - .await - .context("failed to load ACME client from KvStore credentials"); + return Ok(client); } // Registering is the one step in this function that cannot be repeated @@ -605,15 +635,9 @@ impl DistributedCertBot { // attestation written beside it, under its own key with its own // last-writer-wins race, may well be the one that survives. Renewal // locks are per domain and do not help: a fresh cluster registers from - // however many domains and nodes start at once. Take the rotation lock, - // which exists for exactly this key, and look again before spending one. - let _serialized = self.caa_lock.lock().await; - let Some(rotation_lock) = self.try_acquire_rotation_lock() else { - bail!( - "another node is registering or rotating the ACME account; \ - retry after it finishes" - ); - }; + // however many domains and nodes start at once. Take the shared ACME + // lock and look again before spending a registration. + let rotation_lock = self.acquire_acme_lock("register the shared ACME account")?; let client = self .register_or_adopt_account(domain, &dns_cred, acme_url) .await; @@ -623,10 +647,52 @@ impl DistributedCertBot { client } + /// Build an ACME client from the cluster's stored account credentials, or + /// `None` if no account has been registered yet. + /// + /// A corrupt record is an error, not absence: falling through to account + /// registration would silently create an account that the account-bound CAA + /// records refuse, and burn a rate-limited registration. So is a record for + /// a different ACME directory -- registering a fresh account there would + /// leave every domain's CAA pinned to the old account and block issuance, + /// while rotation re-pins CAA along with the switch. + async fn load_stored_acme_client( + &self, + domain: &str, + dns_cred: &DnsCredential, + acme_url: &str, + ) -> Result> { + let Some(creds) = self + .kv_store + .get_acme_credentials() + .context("call RotateAcmeCredentials to replace the stored ACME credentials")? + else { + return Ok(None); + }; + if !acme_url_matches(&creds.acme_credentials, acme_url).context( + "invalid ACME credentials in KvStore; call RotateAcmeCredentials to replace them", + )? { + bail!( + "stored ACME credentials are for a different ACME directory; \ + call RotateAcmeCredentials to switch directories" + ); + } + let dns01_client = self.dns_client(domain, dns_cred).await?; + let client = AcmeClient::load( + dns01_client, + &creds.acme_credentials, + dns_cred.max_dns_wait, + dns_cred.dns_txt_ttl, + ) + .await + .context("failed to load ACME client from KvStore credentials")?; + Ok(Some(client)) + } + /// Register the cluster's shared ACME account, or adopt the one that /// appeared while this call was waiting for the lock. /// - /// Called with the rotation lock held. The re-read is the point: without it + /// Called with the ACME lock held. The re-read is the point: without it /// every waiter registers an account of its own the moment it is let /// through, which is the race the lock was taken to avoid. async fn register_or_adopt_account( @@ -635,29 +701,12 @@ impl DistributedCertBot { dns_cred: &DnsCredential, acme_url: &str, ) -> Result { - if let Some(creds) = self - .kv_store - .get_acme_credentials() - .context("call RotateAcmeCredentials to replace the stored ACME credentials")? + if let Some(client) = self + .load_stored_acme_client(domain, dns_cred, acme_url) + .await? { - if !acme_url_matches(&creds.acme_credentials, acme_url).context( - "invalid ACME credentials in KvStore; call RotateAcmeCredentials to replace them", - )? { - bail!( - "stored ACME credentials are for a different ACME directory; \ - call RotateAcmeCredentials to switch directories" - ); - } info!("adopted the ACME account registered while this node waited for the lock"); - let dns01_client = self.dns_client(domain, dns_cred).await?; - return AcmeClient::load( - dns01_client, - &creds.acme_credentials, - dns_cred.max_dns_wait, - dns_cred.dns_txt_ttl, - ) - .await - .context("failed to load ACME client from KvStore credentials"); + return Ok(client); } info!("creating new global ACME account at {acme_url}"); @@ -883,6 +932,56 @@ mod tests { DistributedCertBot::new(kv_store, Arc::new(CertResolver::new()), None) } + /// A DNS credential pointing at a closed port: any provider API call fails + /// fast, so a test that reaches one fails instead of hanging on the network. + fn unreachable_dns_credential() -> DnsCredential { + DnsCredential { + id: "cred-1".to_string(), + name: "unreachable".to_string(), + provider: DnsProvider::Cloudflare { + api_token: "token".to_string(), + api_url: Some("http://127.0.0.1:1/client/v4".to_string()), + }, + max_dns_wait: Duration::from_secs(1), + dns_txt_ttl: 60, + created_at: 0, + updated_at: 0, + } + } + + fn test_zt_domain_config() -> ZtDomainConfig { + ZtDomainConfig { + domain: "app.example.com".to_string(), + dns_cred_id: Some("cred-1".to_string()), + port: 443, + node: None, + priority: 0, + } + } + + /// A certbot with one ZT domain whose DNS provider is unreachable. + fn certbot_with_domain(data_dir: &std::path::Path) -> DistributedCertBot { + let certbot = test_certbot(data_dir); + certbot + .kv_store + .save_dns_credential(&unreachable_dns_credential()) + .expect("failed to store dns credential"); + certbot + .kv_store + .save_zt_domain_config(&test_zt_domain_config()) + .expect("failed to store zt domain config"); + certbot + } + + fn save_credentials_for(certbot: &DistributedCertBot, acme_url: &str) { + certbot + .kv_store + .save_acme_credentials(&CertCredentials { + acme_credentials: format!(r#"{{"acme_url":"{acme_url}"}}"#), + }) + .expect("failed to store acme credentials"); + } + #[test] fn lock_writes_wake_the_persistent_push_path() { let data_dir = tempfile::tempdir().expect("failed to create temp dir"); @@ -930,21 +1029,58 @@ mod tests { .expect("set_caa_all should succeed without domains"); } + /// CAA reconciliation rewrites a zone's issuer records in place -- guard, + /// sweep, write, unguard -- so two runs over the same zone can delete each + /// other's records and leave the guards behind, which blocks issuance until + /// a later run succeeds. The lock that orders them is the shared ACME one, + /// so it orders another node's rotation against this run too. #[tokio::test] async fn set_caa_all_rejects_concurrent_runs() { let data_dir = tempfile::tempdir().expect("failed to create temp dir"); - let certbot = test_certbot(data_dir.path()); - let _guard = certbot.caa_lock.lock().await; + let certbot = certbot_with_domain(data_dir.path()); + // An account already exists, so this run has nothing to register and + // reaches the lock it takes for the reconciliation itself. + save_credentials_for(&certbot, DEFAULT_ACME_URL); + assert!(certbot + .kv_store + .try_acquire_rotation_lock(ROTATION_LOCK_TIMEOUT_SECS) + .is_some()); + let err = certbot .set_caa_all() .await .expect_err("a concurrent run should be rejected"); assert!( - err.to_string().contains("already in progress"), + err.to_string().contains("cannot reconcile CAA records"), "unexpected error: {err}" ); } + /// Registration takes the same lock this run holds for the reconciliation, + /// and the lock is not reentrant. Registering up front is what keeps a + /// fresh cluster's first `SetCaa` from refusing -- or deadlocking on -- its + /// own lock. + #[tokio::test] + async fn set_caa_all_registers_the_account_before_taking_the_lock() { + let data_dir = tempfile::tempdir().expect("failed to create temp dir"); + let certbot = certbot_with_domain(data_dir.path()); + + let err = tokio::time::timeout(Duration::from_secs(30), certbot.set_caa_all()) + .await + .expect("set_caa_all must not block on its own lock") + .expect_err("the unreachable DNS provider should fail the run"); + assert!( + err.to_string() + .contains("failed to register the shared ACME account"), + "unexpected error: {err:?}" + ); + // A failed run leaves the lock free for the next attempt. + assert!(certbot + .kv_store + .try_acquire_rotation_lock(ROTATION_LOCK_TIMEOUT_SECS) + .is_some()); + } + /// First-use registration must wait on the same lock rotation takes, or a /// fresh cluster registers one account per node that happens to start a /// renewal -- and the credentials record, being last-writer-wins, keeps @@ -956,29 +1092,8 @@ mod tests { #[tokio::test] async fn first_use_registration_waits_for_the_rotation_lock() { let data_dir = tempfile::tempdir().expect("failed to create temp dir"); - let certbot = test_certbot(data_dir.path()); - certbot - .kv_store - .save_dns_credential(&DnsCredential { - id: "cred-1".to_string(), - name: "unreachable".to_string(), - provider: DnsProvider::Cloudflare { - api_token: "token".to_string(), - api_url: Some("http://127.0.0.1:1/client/v4".to_string()), - }, - max_dns_wait: Duration::from_secs(1), - dns_txt_ttl: 60, - created_at: 0, - updated_at: 0, - }) - .expect("failed to store dns credential"); - let config = ZtDomainConfig { - domain: "app.example.com".to_string(), - dns_cred_id: Some("cred-1".to_string()), - port: 443, - node: None, - priority: 0, - }; + let certbot = certbot_with_domain(data_dir.path()); + let config = test_zt_domain_config(); // Another node is mid-registration or mid-rotation. assert!(certbot @@ -987,29 +1102,63 @@ mod tests { .is_some()); let err = match certbot - .get_or_create_acme_client("app.example.com", &config) + .get_or_create_acme_client(&config.domain, &config) .await { Ok(_) => panic!("registration must not proceed while the lock is held"), Err(err) => err, }; assert!( - err.to_string().contains("registering or rotating"), + err.to_string() + .contains("cannot register the shared ACME account"), "unexpected error: {err}" ); } + /// Whoever the lock lets through next must look at the record again. Its + /// contents decide the outcome -- adopt, or refuse a directory switch -- + /// and reaching either one proves no second registration was spent. + #[tokio::test] + async fn registration_rereads_the_record_under_the_lock() { + let data_dir = tempfile::tempdir().expect("failed to create temp dir"); + let certbot = certbot_with_domain(data_dir.path()); + // The account that appeared while this call waited is registered at a + // different ACME directory than the one this run is configured for. + save_credentials_for(&certbot, "https://acme.test/directory"); + + let err = match certbot + .register_or_adopt_account( + "app.example.com", + &unreachable_dns_credential(), + DEFAULT_ACME_URL, + ) + .await + { + Ok(_) => panic!("a directory switch must not be made by registering"), + Err(err) => err, + }; + assert!( + err.to_string().contains("different ACME directory"), + "unexpected error: {err:?}" + ); + } + + /// Two rotations on this node are ordered by the same lock that orders two + /// nodes: the lock lives in the KV store, so a second in-process run sees + /// the first one's record. #[tokio::test] async fn rotate_acme_credentials_rejects_concurrent_runs() { let data_dir = tempfile::tempdir().expect("failed to create temp dir"); let certbot = test_certbot(data_dir.path()); - let _guard = certbot.caa_lock.lock().await; + let _held = certbot + .try_acquire_rotation_lock() + .expect("lock should be free"); let err = certbot .rotate_acme_credentials() .await .expect_err("a concurrent run should be rejected"); assert!( - err.to_string().contains("already in progress"), + err.to_string().contains("cannot rotate ACME credentials"), "unexpected error: {err}" ); } @@ -1027,8 +1176,7 @@ mod tests { .await .expect_err("rotation should be rejected while the KV lock is held"); assert!( - err.to_string() - .contains("another node is rotating ACME credentials"), + err.to_string().contains("cannot rotate ACME credentials"), "unexpected error: {err}" ); } diff --git a/dstack/gateway/src/kv/mod.rs b/dstack/gateway/src/kv/mod.rs index e0b3cea28..62bdc3870 100644 --- a/dstack/gateway/src/kv/mod.rs +++ b/dstack/gateway/src/kv/mod.rs @@ -2135,7 +2135,12 @@ impl KvStore { Ok(()) } - /// Try to acquire the global ACME credential rotation lock. + /// Try to acquire the lock over the cluster's shared ACME account. + /// + /// Every operation over that account takes it -- rotation, CAA + /// reconciliation, and first-use registration -- so they are ordered + /// against each other across nodes as well as within one; see + /// [`crate::distributed_certbot::DistributedCertBot`]. /// /// Returns the lock value that was written; pass it back to /// [`Self::release_rotation_lock`] so a rotation that outlived the timeout From 981a3ca0412a6469a2f6476aba00bca1101ee5da Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 25 Aug 2026 21:32:03 -0700 Subject: [PATCH 3/5] test(gateway): cover the ACME lock in unit tests and the full-stack E2E Nothing under the ACME lock may take it again. `set_caa` now loads the stored account instead of registering one, so the locked region cannot reach `acquire_acme_lock` at all -- the caller registers up front, and the property holds by construction rather than by argument. The E2E suite never called SetCaa or RotateAcmeCredentials, so the two admin operations that take the cluster-wide lock ran nowhere outside single-process unit tests. Both nodes now reconcile CAA, then one rotates the shared account and the other issues with it, which is also what proves the switch reached the cluster rather than one process. The zone is read back from the mock provider after each step: exactly one `issue` and one `issuewild` pinned to the same account, no ";" guard left behind -- the state a reconciliation that was interleaved, or that died halfway, does not produce. Admin RPCs are bounded by `--max-time` so an RPC that blocks forever fails the suite where the cause is obvious, instead of hanging it until the job timeout. --- dstack/gateway/src/distributed_certbot.rs | 67 +++++++++--- test-suites/full-stack-compose/README.md | 8 ++ .../full-stack-compose/scripts/runner.sh | 100 ++++++++++++++++++ 3 files changed, 161 insertions(+), 14 deletions(-) diff --git a/dstack/gateway/src/distributed_certbot.rs b/dstack/gateway/src/distributed_certbot.rs index f6c631c06..288b3403e 100644 --- a/dstack/gateway/src/distributed_certbot.rs +++ b/dstack/gateway/src/distributed_certbot.rs @@ -268,6 +268,15 @@ impl DistributedCertBot { self.kv_store.get_certbot_config() } + /// The ACME directory this cluster issues from. + fn acme_url(&self) -> Result { + let config = self.config()?; + if config.acme_url.is_empty() { + return Ok(DEFAULT_ACME_URL.to_string()); + } + Ok(config.acme_url) + } + /// Initialize all ZT-Domain certificates pub async fn init_all(&self) -> Result<()> { let configs = self.kv_store.list_zt_domain_configs(); @@ -395,13 +404,22 @@ impl DistributedCertBot { /// The domain in the config is the base domain and certificates are issued for /// `*.{domain}`, which the CAA lookup covers by climbing to the base domain. /// - /// The written CAA value pins `accounturi` to the global ACME account, so this - /// reuses the account from the KV store and registers one if none exists yet. + /// The written CAA value pins `accounturi` to the global ACME account, which + /// the caller has already registered; see [`Self::ensure_acme_account`]. async fn set_caa(&self, domain: &str, config: &ZtDomainConfig) -> Result<()> { + // Load only. This runs under the ACME lock, which is not reentrant, so + // registering here would refuse the run that took it -- and the caller + // has already registered the account before taking it. Nothing under + // the lock may call [`Self::acquire_acme_lock`], and keeping the + // registering variant out of this path is what makes that structural + // rather than a rule to remember. + let dns_cred = dns_credential_for(&self.kv_store, config)?; + let acme_url = self.acme_url()?; let acme_client = self - .get_or_create_acme_client(domain, config) + .load_stored_acme_client(domain, &dns_cred, &acme_url) .await - .context("failed to initialize ACME client")?; + .context("failed to initialize ACME client")? + .context("no shared ACME account is registered for this cluster")?; acme_client .set_caa_records(&[domain.to_string()]) .await @@ -611,17 +629,10 @@ impl DistributedCertBot { ) -> Result { // Get DNS credential (from config or default) let dns_cred = dns_credential_for(&self.kv_store, config)?; - - // Use ACME URL from certbot config, fall back to default if not set - let config = self.config()?; - let acme_url = if config.acme_url.is_empty() { - DEFAULT_ACME_URL - } else { - &config.acme_url - }; + let acme_url = self.acme_url()?; if let Some(client) = self - .load_stored_acme_client(domain, &dns_cred, acme_url) + .load_stored_acme_client(domain, &dns_cred, &acme_url) .await? { info!("loaded global ACME account credentials from KvStore"); @@ -639,7 +650,7 @@ impl DistributedCertBot { // lock and look again before spending a registration. let rotation_lock = self.acquire_acme_lock("register the shared ACME account")?; let client = self - .register_or_adopt_account(domain, &dns_cred, acme_url) + .register_or_adopt_account(domain, &dns_cred, &acme_url) .await; if let Err(err) = self.release_rotation_lock(&rotation_lock) { error!("failed to release ACME rotation lock: {err:?}"); @@ -1115,6 +1126,34 @@ mod tests { ); } + /// Nothing inside the locked region may take the lock again. With an + /// account already registered, a run reaches the DNS provider -- it fails + /// there, on the reconciliation itself, and never on its own lock. + #[tokio::test] + async fn set_caa_all_does_not_take_its_own_lock_again() { + let data_dir = tempfile::tempdir().expect("failed to create temp dir"); + let certbot = certbot_with_domain(data_dir.path()); + save_credentials_for(&certbot, DEFAULT_ACME_URL); + + let err = tokio::time::timeout(Duration::from_secs(30), certbot.set_caa_all()) + .await + .expect("set_caa_all must not block on its own lock") + .expect_err("the unreachable DNS provider should fail the run"); + let msg = format!("{err:#}"); + assert!( + msg.contains("failed to set CAA records for 1/1 domains"), + "unexpected error: {msg}" + ); + assert!( + !msg.contains("shared ACME lock"), + "the run refused its own lock: {msg}" + ); + assert!(certbot + .kv_store + .try_acquire_rotation_lock(ROTATION_LOCK_TIMEOUT_SECS) + .is_some()); + } + /// Whoever the lock lets through next must look at the record again. Its /// contents decide the outcome -- adopt, or refuse a directory switch -- /// and reaching either one proves no second registration was spent. diff --git a/test-suites/full-stack-compose/README.md b/test-suites/full-stack-compose/README.md index 132b91acf..427a7f682 100644 --- a/test-suites/full-stack-compose/README.md +++ b/test-suites/full-stack-compose/README.md @@ -133,6 +133,14 @@ rejected if they say image verification or self-authorization is disabled. 8. Force certificate issuance through each upgraded node against a mock DNS API that rejects anything except the original bearer token. This proves the DNS credential value survived even though the current admin API redacts it. +9. Reconcile CAA through each upgraded node, then rotate the shared ACME account + through one node and issue through the other. These are the two admin + operations that take the cluster-wide ACME lock, and nothing else in the + suite calls them: a reconciliation that refuses or blocks on the lock it + holds itself is invisible to single-process unit tests. The zone is read back + from the mock provider after each step and must carry exactly one `issue` and + one `issuewild` record, both pinned to the same account -- the rotated one + after rotation -- with no `;` guard left behind. This is a production-style **rolling two-node** zero-downtime assertion. The suite does not claim that rebooting a single Gateway CVM can preserve that diff --git a/test-suites/full-stack-compose/scripts/runner.sh b/test-suites/full-stack-compose/scripts/runner.sh index 0143ff6fb..afd6c0448 100755 --- a/test-suites/full-stack-compose/scripts/runner.sh +++ b/test-suites/full-stack-compose/scripts/runner.sh @@ -393,7 +393,11 @@ admin_curl() { local node=$1 method=$2 data=${3:-'{}'} admin out code read -r _ admin _ < <(gateway_ports "$node") out=$(mktemp) + # Bounded on purpose: an admin RPC that blocks forever -- on a lock it holds + # itself, say -- must fail this suite rather than hang it until the job + # timeout, where the cause is far less obvious. code=$(curl -sS -o "$out" -w '%{http_code}' -X POST \ + --max-time "${ADMIN_CURL_MAX_TIME:-300}" \ -H "Authorization: Bearer ${GATEWAY_ADMIN_TOKEN}" \ -H 'Content-Type: application/json' \ "http://127.0.0.1:${admin}/prpc/Admin.${method}?json" \ @@ -554,6 +558,94 @@ assert_gateway_dns_credential_usable() { die "Gateway node $node could not issue with its persisted DNS credential" } +# Read every record the mock Cloudflare API currently holds. The endpoint is +# unauthenticated on purpose: it is the suite's view of the zone, not one of the +# provider operations the gateway performs. +mock_dns_records() { + curl -sS "http://127.0.0.1:${MOCK_CF_HTTP_PORT}/api/records" +} + +# Assert the zone ends up in the exact state a finished reconciliation leaves: +# one `issue` and one `issuewild` record pinned to one account, and no `;` guard +# left behind. Reconciliation rewrites these in place -- guard, sweep, write, +# unguard -- so a run that was interleaved with another, or that died halfway, +# shows up here as a duplicate, a missing tag, or a surviving guard that blocks +# every future issuance for the name. +# +# Writes the pinned account URI to $WORK_DIR/gateway-caa-account. Pass an +# expected URI to require a specific account. +assert_gateway_caa_records() { + local expect=${1:-} parsed account + parsed=$(mock_dns_records | jq -c --arg n "$BASE_DOMAIN" ' + [ .records[] + | select(.type == "CAA") + | select((.name | ascii_downcase) == ($n | ascii_downcase)) + | .content + | capture("^(?[0-9]+) +(?[a-z]+) +\"(?.*)\"$") ]') \ + || die "could not read CAA records from the mock DNS API" + printf '%s\n' "$parsed" > "$WORK_DIR/gateway-caa-records.json" + + jq -e 'map(select(.value == ";")) | length == 0' <<<"$parsed" >/dev/null \ + || die "reconciliation left guard CAA records behind: $parsed" + local tag + for tag in issue issuewild; do + jq -e --arg t "$tag" 'map(select(.tag == $t)) | length == 1' <<<"$parsed" >/dev/null \ + || die "expected exactly one $tag CAA record for $BASE_DOMAIN: $parsed" + done + account=$(jq -r ' + map(select(.tag == "issue")) | .[0].value + | capture("accounturi=(?[^;]+)$") | .uri' <<<"$parsed") + [[ -n "$account" && "$account" != null ]] \ + || die "CAA records do not pin an ACME account: $parsed" + jq -e --arg a "$account" 'map(select(.value | endswith("accounturi=" + $a))) | length == 2' \ + <<<"$parsed" >/dev/null \ + || die "issue and issuewild pin different accounts: $parsed" + if [[ -n "$expect" ]]; then + [[ "$account" == "$expect" ]] \ + || die "CAA pins $account, expected the rotated account $expect" + fi + printf '%s' "$account" > "$WORK_DIR/gateway-caa-account" +} + +# Reconcile CAA through an upgraded node. +# +# On the current code one cluster-wide lock covers rotation, reconciliation, and +# first-use account registration. A reconciliation that refuses -- or blocks on +# -- the lock it holds itself is invisible to single-process unit tests and to +# every issuance path this suite already exercises, because nothing else calls +# SetCaa. +assert_gateway_caa_reconcile() { + local node=$1 + log "reconciling CAA through upgraded Gateway node $node" + admin_curl "$node" SetCaa >/dev/null \ + || die "Gateway node $node could not reconcile CAA records" + assert_gateway_caa_records + log "Gateway node $node pinned CAA to $(cat "$WORK_DIR/gateway-caa-account")" +} + +# Rotate the shared ACME account, then require the cluster to issue with it. +# +# Rotation registers a replacement account, publishes it, and re-pins every +# domain's CAA under the same lock. Issuing afterwards through the *other* node +# is what proves the switch reached the whole cluster rather than one process. +assert_gateway_acme_rotation() { + local node=$1 peer=$2 previous result account + previous=$(cat "$WORK_DIR/gateway-caa-account") + log "rotating the shared ACME account through Gateway node $node" + result=$(admin_curl "$node" RotateAcmeCredentials) \ + || die "Gateway node $node could not rotate the ACME account" + printf '%s\n' "$result" > "$WORK_DIR/gateway-rotate-acme.json" + account=$(jq -r '.account_uri // ""' <<<"$result") + [[ -n "$account" ]] || die "rotation returned no account URI: $result" + [[ "$account" != "$previous" ]] \ + || die "rotation kept the previous account $account" + jq -e '(.domains_updated // 0) >= 1' <<<"$result" >/dev/null \ + || die "rotation re-pinned no domain: $result" + assert_gateway_caa_records "$account" + log "rotated to $account and re-pinned CAA" + assert_gateway_dns_credential_usable "$peer" +} + render_app() { local label=$1 mode=$2 meta app_id hash image_ref=$APP_IMAGE_ID # The v0.5.11 Docker Compose treats a local image ID (`sha256:...`) as a @@ -858,6 +950,14 @@ phase_upgrade() { assert_gateway_dns_credential_usable 1 assert_gateway_dns_credential_usable 2 + # CAA reconciliation and ACME account rotation are the two admin operations + # that take the cluster-wide ACME lock, and neither runs anywhere else in this + # suite. Both nodes reconcile, then one rotates and the other issues with the + # replacement account. + assert_gateway_caa_reconcile 1 + assert_gateway_caa_reconcile 2 + assert_gateway_acme_rotation 2 1 + assert_no_insecure_shortcuts save_vm_logs # dstack-kms runs in an inner container, whose stdout is not part of the CVM From 0fedbb8f49772b2773133d7770c76481a11e1a6c Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 25 Aug 2026 21:32:03 -0700 Subject: [PATCH 4/5] test(gateway): split the full-stack E2E into phases, add a certbot phase The suite was one indivisible run: KMS and Gateway 0.5.8 rolled to current, with two apps. A change confined to one area had to pay for all of it, and one area it could never reach at all -- a fresh cluster's first ACME account registration, because 0.5.8 registers the shared account long before the current binary starts. A phase is now the unit a run can be limited to (`./run-e2e.sh --phase certbot`, or DSTACK_E2E_PHASE). Each is self-contained: it deploys what it needs and asserts on it. The `certbot` phase brings up a current KMS and two current Gateway nodes, then reconciles CAA on a cluster holding no account -- registering one from inside the region that holds the cluster-wide ACME lock -- issues against it, requires the second node to adopt that account rather than register its own, and rotates. Only the upgrade phase boots the v0.5.11 compatibility image, so no other phase requires it to be present, and none widens the authorization allowlist with an OS digest it will never launch. `run-upgrade-e2e.sh` stays as a wrapper. --- test-suites/full-stack-compose/README.md | 55 +++- test-suites/full-stack-compose/run-e2e.sh | 274 ++++++++++++++++++ .../full-stack-compose/run-upgrade-e2e.sh | 244 +--------------- .../scripts/render-config.sh | 12 +- .../full-stack-compose/scripts/runner.sh | 72 ++++- 5 files changed, 406 insertions(+), 251 deletions(-) create mode 100755 test-suites/full-stack-compose/run-e2e.sh diff --git a/test-suites/full-stack-compose/README.md b/test-suites/full-stack-compose/README.md index 427a7f682..bb7810825 100644 --- a/test-suites/full-stack-compose/README.md +++ b/test-suites/full-stack-compose/README.md @@ -3,7 +3,7 @@ SPDX-FileCopyrightText: © 2026 Phala Network SPDX-License-Identifier: Apache-2.0 --> -# Production-compatible KMS/Gateway upgrade E2E +# Production-compatible KMS/Gateway E2E This suite runs the stateful services and applications in **real TDX CVMs**. Docker Compose is used only for host infrastructure (VMM, authorization, @@ -73,16 +73,36 @@ would make a passing result irrelevant to production: - Docker and WireGuard kernel support 3. Provide an unpacked dstack image directory containing `digest.txt` and - `sha256sum.txt` for both the current image (`DSTACK_E2E_IMAGE_NAME`) and the - v0.5.11 compatibility image (`DSTACK_E2E_OLD_IMAGE_NAME`). Copy - `.env.example` to `.env` when paths or ports differ. + `sha256sum.txt` for the current image (`DSTACK_E2E_IMAGE_NAME`). The + `upgrade` phase additionally needs the v0.5.11 compatibility image + (`DSTACK_E2E_OLD_IMAGE_NAME`); no other phase boots it, and no other phase + requires it to be present. Copy `.env.example` to `.env` when paths or ports + differ. ## Run ```bash -DOCKER_BUILDKIT=0 ./run-upgrade-e2e.sh +DOCKER_BUILDKIT=0 ./run-e2e.sh # the full upgrade suite +DOCKER_BUILDKIT=0 ./run-e2e.sh --phase certbot # certbot/ACME only ``` +### Phases + +A phase is the unit a run can be limited to. Each one is self-contained --- it +deploys what it needs and asserts on it --- so a change confined to one area +does not have to pay for the whole suite. `DSTACK_E2E_PHASE` selects the same +thing as `--phase`. + +| Phase | Deploys | Covers | +| --- | --- | --- | +| `upgrade` (default) | KMS 0.5.8 + current, two Gateway nodes rolled 0.5.8 -> current, two apps | Everything below: key/identity continuity across the upgrade, durable Gateway state, zero-downtime rolling upgrade | +| `certbot` | Current KMS, two current Gateway nodes | A fresh cluster's first ACME account registration, CAA reconciliation, ACME account rotation | + +The `certbot` phase is the only one that reaches a fresh cluster's *first* +account registration on current code. The `upgrade` phase cannot: Gateway 0.5.8 +registers the shared account long before the current binary starts, so every +current-code run there takes the load path instead. + The defaults pull these released images from Docker Hub: - `dstacktee/dstack-kms:0.5.8@sha256:9650dcb47dad0065470f432f00e78e012912214ef1a5b1d7272918817e61a26d` @@ -92,7 +112,30 @@ The driver checks each released binary's `--version` output before deploying anything. Set `DSTACK_E2E_SKIP_CURRENT_BUILD=true` only when the current musl binaries were already built. -## Upgrade sequence and assertions +## `certbot` phase assertions + +1. Deploy current KMS (bootstrapped fresh, not onboarded) and two current + Gateway CVMs under one pinned Gateway app ID, then configure Pebble and the + mock Cloudflare DNS API. +2. Reconcile CAA on a cluster that holds **no** ACME account. This registers one + from inside the region that holds the cluster-wide ACME lock, which is the + ordering that has to be right: a run that refuses --- or blocks on --- its own + lock fails here. +3. Issue the wildcard certificate against the account that registration + published, and require the CAA records to still pin it. +4. Reconcile CAA through the *other* node. It must adopt the same account rather + than register a second one, which the last-writer-wins credentials record + would otherwise silently lose. +5. Rotate the shared account through one node and force issuance through the + other, which is what proves the switch reached the cluster rather than one + process. + +After every step the zone is read back from the mock provider: exactly one +`issue` and one `issuewild` record, both pinned to the same account (the rotated +one after rotation), and no `;` guard left behind --- the state a reconciliation +that was interleaved, or that died halfway, does not produce. + +## `upgrade` sequence and assertions ### KMS 0.5.8 to current diff --git a/test-suites/full-stack-compose/run-e2e.sh b/test-suites/full-stack-compose/run-e2e.sh new file mode 100755 index 000000000..cf0e91048 --- /dev/null +++ b/test-suites/full-stack-compose/run-e2e.sh @@ -0,0 +1,274 @@ +#!/usr/bin/env bash +# SPDX-FileCopyrightText: © 2026 Phala Network +# SPDX-License-Identifier: Apache-2.0 +set -euo pipefail + +SCRIPT_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) +REPO_DIR=$(cd -- "$SCRIPT_DIR/../.." && pwd) +STATE_DIR="$SCRIPT_DIR/state" +ENV_FILE=${DSTACK_E2E_ENV_FILE:-$SCRIPT_DIR/.env} + +# Phases are the unit a run can be limited to. Each one is self-contained: it +# deploys what it needs and asserts on it, so a change that only touches one +# area does not have to pay for the whole suite. +# +# upgrade KMS/Gateway 0.5.8 -> current rolling upgrade (the full suite) +# certbot current-only KMS + two-node Gateway; ACME account registration, +# CAA reconciliation, and account rotation +PHASE=${DSTACK_E2E_PHASE:-upgrade} +while (( $# )); do + case "$1" in + --phase) PHASE=${2:?--phase needs a value}; shift 2 ;; + --phase=*) PHASE=${1#--phase=}; shift ;; + -h|--help) + sed -n '/^# Phases are/,/^PHASE=/p' "${BASH_SOURCE[0]}" | sed 's/^# \?//;$d' + exit 0 ;; + *) echo "ERROR: unknown argument $1" >&2; exit 1 ;; + esac +done +case "$PHASE" in + upgrade|certbot) ;; + *) echo "ERROR: unknown phase $PHASE (expected upgrade or certbot)" >&2; exit 1 ;; +esac +export DSTACK_E2E_PHASE="$PHASE" + +setting() { + local name=$1 fallback=$2 line value + if [[ -v $name ]]; then + printf '%s' "${!name}" + return + fi + if [[ -f "$ENV_FILE" ]]; then + line=$(grep -E "^[[:space:]]*${name}=" "$ENV_FILE" | tail -n1 || true) + if [[ -n "$line" ]]; then + value=${line#*=} + value=${value%$'\r'} + if [[ "$value" == \"*\" && "$value" == *\" ]]; then + value=${value:1:${#value}-2} + elif [[ "$value" == \'*\' && "$value" == *\' ]]; then + value=${value:1:${#value}-2} + fi + printf '%s' "$value" + return + fi + fi + printf '%s' "$fallback" +} + +OLD_KMS_IMAGE=$(setting DSTACK_E2E_OLD_KMS_IMAGE \ + dstacktee/dstack-kms:0.5.8@sha256:9650dcb47dad0065470f432f00e78e012912214ef1a5b1d7272918817e61a26d) +OLD_GATEWAY_IMAGE=$(setting DSTACK_E2E_OLD_GATEWAY_IMAGE \ + dstacktee/dstack-gateway:0.5.8@sha256:6eb1dc1a5000f37cc5b0322d3fdb71e7f2e31859b5e3a611634919278cee2411) +APP_IMAGE=$(setting DSTACK_E2E_APP_IMAGE nginx:alpine) +KEEP_STACK=$(setting DSTACK_E2E_KEEP_STACK true) +CLEAN_STATE=$(setting DSTACK_E2E_UPGRADE_CLEAN_STATE true) +SKIP_BUILD=$(setting DSTACK_E2E_SKIP_CURRENT_BUILD false) +# dstack's build metadata deliberately embeds a 20-hex abbreviated revision. +CURRENT_REV=$(git -C "$REPO_DIR" rev-parse --short=20 HEAD) +CURRENT_VERSION=$(sed -n 's/^version = "\([^"]*\)"/\1/p' \ + "$REPO_DIR/dstack/Cargo.toml" | head -n1) +[[ -n "$CURRENT_VERSION" ]] || { + echo "ERROR: could not read current workspace version" >&2 + exit 1 +} + +COMPOSE=(docker compose -f "$SCRIPT_DIR/compose.yml") +if [[ -f "$ENV_FILE" ]]; then + COMPOSE=(docker compose --env-file "$ENV_FILE" -f "$SCRIPT_DIR/compose.yml") +fi + +log() { printf '[%(%H:%M:%S)T] %s\n' -1 "$*"; } +die() { log "ERROR: $*" >&2; exit 1; } +compose() { "${COMPOSE[@]}" "$@"; } + +need_bin() { + [[ -x "$1" ]] || die "missing executable $1" +} + +pull_released_image() { + local image=$1 component=$2 + log "pulling released $component image from Docker Hub: $image" + docker pull "$image" || die "cannot pull released $component image $image" +} + +reset_state() { + log "resetting Compose stack and E2E state" + compose down --remove-orphans >/dev/null 2>&1 || true + docker run --rm -v "$STATE_DIR:/state" alpine:3.22 sh -c \ + 'find /state -mindepth 1 ! -name .gitkeep -exec rm -rf {} +' +} + +build_current_binaries() { + if [[ "$SKIP_BUILD" == true ]]; then + log "using prebuilt current musl KMS/Gateway binaries" + else + log "building current KMS/Gateway as production-style static musl binaries" + cargo build --manifest-path "$REPO_DIR/dstack/Cargo.toml" \ + --release --target x86_64-unknown-linux-musl \ + -p dstack-kms -p dstack-gateway + fi + need_bin "$REPO_DIR/dstack/target/x86_64-unknown-linux-musl/release/dstack-kms" + need_bin "$REPO_DIR/dstack/target/x86_64-unknown-linux-musl/release/dstack-gateway" +} + +prepare_container_artifacts() { + local artifact_dir="$STATE_DIR/artifacts/images" + local context_dir="$STATE_DIR/image-build" + local rev current_kms_image current_gateway_image + local old_kms_id old_gateway_id current_kms_id current_gateway_id app_id + rev=$(git -C "$REPO_DIR" rev-parse --short=16 HEAD) + current_kms_image="dstack-e2e-kms-current:${rev}" + current_gateway_image="dstack-e2e-gateway-current:${rev}" + mkdir -p "$artifact_dir" "$context_dir/kms" "$context_dir/gateway" + + cp "$REPO_DIR/dstack/target/x86_64-unknown-linux-musl/release/dstack-kms" \ + "$context_dir/kms/dstack-kms" + cat > "$context_dir/kms/Dockerfile" < "$context_dir/gateway/Dockerfile" </dev/null || echo "") + + log "saving content-addressed images for import inside CVMs" + docker save -o "$artifact_dir/kms-current.tar" "$current_kms_image" + docker save -o "$artifact_dir/gateway-current.tar" "$current_gateway_image" + # The released images are still the base layers the current binaries are + # built on, so they are pulled either way; only the upgrade phase boots them. + if [[ "$PHASE" == upgrade ]]; then + docker save -o "$artifact_dir/kms-0.5.8.tar" "$OLD_KMS_IMAGE" + docker save -o "$artifact_dir/gateway-0.5.8.tar" "$OLD_GATEWAY_IMAGE" + docker save -o "$artifact_dir/app.tar" "$APP_IMAGE" + fi + + cat > "$STATE_DIR/artifacts/images.env" </dev/null \ + || die "$OLD_KMS_IMAGE is not KMS 0.5.8" + grep -E '^dstack-gateway v0\.5\.8 \(git:' "$STATE_DIR/work/gateway-old.version.txt" >/dev/null \ + || die "$OLD_GATEWAY_IMAGE is not Gateway 0.5.8" + grep -E "^dstack-kms v${CURRENT_VERSION//./\\.} \\(git:" \ + "$STATE_DIR/work/kms-current.version.txt" >/dev/null \ + || die "locally built KMS is not current v$CURRENT_VERSION" + grep -E "^dstack-gateway v${CURRENT_VERSION//./\\.} \\(git:" \ + "$STATE_DIR/work/gateway-current.version.txt" >/dev/null \ + || die "locally built Gateway is not current v$CURRENT_VERSION" + grep -F "$CURRENT_REV" "$STATE_DIR/work/kms-current.version.txt" >/dev/null \ + || die "KMS binary was not built from current revision $CURRENT_REV" + grep -F "$CURRENT_REV" "$STATE_DIR/work/gateway-current.version.txt" >/dev/null \ + || die "Gateway binary was not built from current revision $CURRENT_REV" +} + +wait_local_key_provider() { + local port deadline status + port=$(setting DSTACK_E2E_KEY_PROVIDER_PORT 13443) + deadline=$((SECONDS + 120)) + log "waiting for production SGX Local-Key-Provider on 127.0.0.1:${port}" + while (( SECONDS < deadline )); do + status=$(compose ps --format json local-keyprovider 2>/dev/null \ + | jq -rs 'map(select(.Service == "local-keyprovider"))[0].Health // ""' 2>/dev/null \ + || true) + if [[ "$status" == healthy ]]; then + log "Local-Key-Provider enclave is healthy" + return 0 + fi + sleep 2 + done + compose logs --tail=200 aesmd local-keyprovider >&2 || true + die "Local-Key-Provider did not become healthy; fix SGX/DCAP/PCCS provisioning rather than disabling attestation" +} + +on_exit() { + local rc=$? + if (( rc != 0 )); then + log "E2E failed; recent infrastructure logs follow" + compose logs --tail=250 auth artifacts vmm runner >&2 || true + fi + if [[ "$KEEP_STACK" != true ]]; then + compose down --remove-orphans >/dev/null 2>&1 || true + else + log "leaving stack running for inspection (DSTACK_E2E_KEEP_STACK=true)" + fi + exit "$rc" +} +trap on_exit EXIT + +main() { + need_bin "$REPO_DIR/dstack/target/release/dstack" + need_bin "$REPO_DIR/dstack/target/release/dstack-auth" + need_bin "$REPO_DIR/dstack/target/release/dstack-vmm" + need_bin "$REPO_DIR/dstack/target/release/supervisor" + + pull_released_image "$OLD_KMS_IMAGE" kms + pull_released_image "$OLD_GATEWAY_IMAGE" gateway + if [[ "$PHASE" == upgrade ]]; then + pull_released_image "$APP_IMAGE" application + fi + build_current_binaries + [[ "$CLEAN_STATE" == true ]] && reset_state + + log "building E2E infrastructure" + compose build init-config mock-cf-dns-api aesmd local-keyprovider + compose run --rm init-config + prepare_container_artifacts + + log "starting authorization, artifact, attestation, ACME and VMM infrastructure" + compose up -d mock-cf-dns-api pebble aesmd local-keyprovider auth artifacts + wait_local_key_provider + compose up -d vmm + + log "running the $PHASE phase" + # Keep the complete runner transcript in state/work even though the runner is + # an ephemeral Compose container. This is especially important for failures + # during deployment, before a per-VM log file exists. + compose run --rm --no-deps \ + -e DSTACK_E2E_PHASE="$PHASE" \ + -e DSTACK_E2E_CURRENT_VERSION="$CURRENT_VERSION" \ + -e DSTACK_E2E_CURRENT_REV="$CURRENT_REV" runner \ + 2>&1 | tee "$STATE_DIR/work/runner.log" + + log "$PHASE E2E success" + log "artifacts: $STATE_DIR/work" +} + +main "$@" diff --git a/test-suites/full-stack-compose/run-upgrade-e2e.sh b/test-suites/full-stack-compose/run-upgrade-e2e.sh index b0ee43b55..5c7298f3f 100755 --- a/test-suites/full-stack-compose/run-upgrade-e2e.sh +++ b/test-suites/full-stack-compose/run-upgrade-e2e.sh @@ -1,244 +1,8 @@ #!/usr/bin/env bash # SPDX-FileCopyrightText: © 2026 Phala Network # SPDX-License-Identifier: Apache-2.0 +# +# Compatibility wrapper: the driver now takes a --phase, and the upgrade +# scenario is one of them. set -euo pipefail - -SCRIPT_DIR=$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) -REPO_DIR=$(cd -- "$SCRIPT_DIR/../.." && pwd) -STATE_DIR="$SCRIPT_DIR/state" -ENV_FILE=${DSTACK_E2E_ENV_FILE:-$SCRIPT_DIR/.env} - -setting() { - local name=$1 fallback=$2 line value - if [[ -v $name ]]; then - printf '%s' "${!name}" - return - fi - if [[ -f "$ENV_FILE" ]]; then - line=$(grep -E "^[[:space:]]*${name}=" "$ENV_FILE" | tail -n1 || true) - if [[ -n "$line" ]]; then - value=${line#*=} - value=${value%$'\r'} - if [[ "$value" == \"*\" && "$value" == *\" ]]; then - value=${value:1:${#value}-2} - elif [[ "$value" == \'*\' && "$value" == *\' ]]; then - value=${value:1:${#value}-2} - fi - printf '%s' "$value" - return - fi - fi - printf '%s' "$fallback" -} - -OLD_KMS_IMAGE=$(setting DSTACK_E2E_OLD_KMS_IMAGE \ - dstacktee/dstack-kms:0.5.8@sha256:9650dcb47dad0065470f432f00e78e012912214ef1a5b1d7272918817e61a26d) -OLD_GATEWAY_IMAGE=$(setting DSTACK_E2E_OLD_GATEWAY_IMAGE \ - dstacktee/dstack-gateway:0.5.8@sha256:6eb1dc1a5000f37cc5b0322d3fdb71e7f2e31859b5e3a611634919278cee2411) -APP_IMAGE=$(setting DSTACK_E2E_APP_IMAGE nginx:alpine) -KEEP_STACK=$(setting DSTACK_E2E_KEEP_STACK true) -CLEAN_STATE=$(setting DSTACK_E2E_UPGRADE_CLEAN_STATE true) -SKIP_BUILD=$(setting DSTACK_E2E_SKIP_CURRENT_BUILD false) -# dstack's build metadata deliberately embeds a 20-hex abbreviated revision. -CURRENT_REV=$(git -C "$REPO_DIR" rev-parse --short=20 HEAD) -CURRENT_VERSION=$(sed -n 's/^version = "\([^"]*\)"/\1/p' \ - "$REPO_DIR/dstack/Cargo.toml" | head -n1) -[[ -n "$CURRENT_VERSION" ]] || { - echo "ERROR: could not read current workspace version" >&2 - exit 1 -} - -COMPOSE=(docker compose -f "$SCRIPT_DIR/compose.yml") -if [[ -f "$ENV_FILE" ]]; then - COMPOSE=(docker compose --env-file "$ENV_FILE" -f "$SCRIPT_DIR/compose.yml") -fi - -log() { printf '[%(%H:%M:%S)T] %s\n' -1 "$*"; } -die() { log "ERROR: $*" >&2; exit 1; } -compose() { "${COMPOSE[@]}" "$@"; } - -need_bin() { - [[ -x "$1" ]] || die "missing executable $1" -} - -pull_released_image() { - local image=$1 component=$2 - log "pulling released $component image from Docker Hub: $image" - docker pull "$image" || die "cannot pull released $component image $image" -} - -reset_state() { - log "resetting Compose stack and E2E state" - compose down --remove-orphans >/dev/null 2>&1 || true - docker run --rm -v "$STATE_DIR:/state" alpine:3.22 sh -c \ - 'find /state -mindepth 1 ! -name .gitkeep -exec rm -rf {} +' -} - -build_current_binaries() { - if [[ "$SKIP_BUILD" == true ]]; then - log "using prebuilt current musl KMS/Gateway binaries" - else - log "building current KMS/Gateway as production-style static musl binaries" - cargo build --manifest-path "$REPO_DIR/dstack/Cargo.toml" \ - --release --target x86_64-unknown-linux-musl \ - -p dstack-kms -p dstack-gateway - fi - need_bin "$REPO_DIR/dstack/target/x86_64-unknown-linux-musl/release/dstack-kms" - need_bin "$REPO_DIR/dstack/target/x86_64-unknown-linux-musl/release/dstack-gateway" -} - -prepare_container_artifacts() { - local artifact_dir="$STATE_DIR/artifacts/images" - local context_dir="$STATE_DIR/image-build" - local rev current_kms_image current_gateway_image - local old_kms_id old_gateway_id current_kms_id current_gateway_id app_id - rev=$(git -C "$REPO_DIR" rev-parse --short=16 HEAD) - current_kms_image="dstack-e2e-kms-current:${rev}" - current_gateway_image="dstack-e2e-gateway-current:${rev}" - mkdir -p "$artifact_dir" "$context_dir/kms" "$context_dir/gateway" - - cp "$REPO_DIR/dstack/target/x86_64-unknown-linux-musl/release/dstack-kms" \ - "$context_dir/kms/dstack-kms" - cat > "$context_dir/kms/Dockerfile" < "$context_dir/gateway/Dockerfile" < "$STATE_DIR/artifacts/images.env" </dev/null \ - || die "$OLD_KMS_IMAGE is not KMS 0.5.8" - grep -E '^dstack-gateway v0\.5\.8 \(git:' "$STATE_DIR/work/gateway-old.version.txt" >/dev/null \ - || die "$OLD_GATEWAY_IMAGE is not Gateway 0.5.8" - grep -E "^dstack-kms v${CURRENT_VERSION//./\\.} \\(git:" \ - "$STATE_DIR/work/kms-current.version.txt" >/dev/null \ - || die "locally built KMS is not current v$CURRENT_VERSION" - grep -E "^dstack-gateway v${CURRENT_VERSION//./\\.} \\(git:" \ - "$STATE_DIR/work/gateway-current.version.txt" >/dev/null \ - || die "locally built Gateway is not current v$CURRENT_VERSION" - grep -F "$CURRENT_REV" "$STATE_DIR/work/kms-current.version.txt" >/dev/null \ - || die "KMS binary was not built from current revision $CURRENT_REV" - grep -F "$CURRENT_REV" "$STATE_DIR/work/gateway-current.version.txt" >/dev/null \ - || die "Gateway binary was not built from current revision $CURRENT_REV" -} - -wait_local_key_provider() { - local port deadline status - port=$(setting DSTACK_E2E_KEY_PROVIDER_PORT 13443) - deadline=$((SECONDS + 120)) - log "waiting for production SGX Local-Key-Provider on 127.0.0.1:${port}" - while (( SECONDS < deadline )); do - status=$(compose ps --format json local-keyprovider 2>/dev/null \ - | jq -rs 'map(select(.Service == "local-keyprovider"))[0].Health // ""' 2>/dev/null \ - || true) - if [[ "$status" == healthy ]]; then - log "Local-Key-Provider enclave is healthy" - return 0 - fi - sleep 2 - done - compose logs --tail=200 aesmd local-keyprovider >&2 || true - die "Local-Key-Provider did not become healthy; fix SGX/DCAP/PCCS provisioning rather than disabling attestation" -} - -on_exit() { - local rc=$? - if (( rc != 0 )); then - log "upgrade E2E failed; recent infrastructure logs follow" - compose logs --tail=250 auth artifacts vmm runner >&2 || true - fi - if [[ "$KEEP_STACK" != true ]]; then - compose down --remove-orphans >/dev/null 2>&1 || true - else - log "leaving stack running for inspection (DSTACK_E2E_KEEP_STACK=true)" - fi - exit "$rc" -} -trap on_exit EXIT - -main() { - need_bin "$REPO_DIR/dstack/target/release/dstack" - need_bin "$REPO_DIR/dstack/target/release/dstack-auth" - need_bin "$REPO_DIR/dstack/target/release/dstack-vmm" - need_bin "$REPO_DIR/dstack/target/release/supervisor" - - pull_released_image "$OLD_KMS_IMAGE" kms - pull_released_image "$OLD_GATEWAY_IMAGE" gateway - pull_released_image "$APP_IMAGE" application - build_current_binaries - [[ "$CLEAN_STATE" == true ]] && reset_state - - log "building E2E infrastructure" - compose build init-config mock-cf-dns-api aesmd local-keyprovider - compose run --rm init-config - prepare_container_artifacts - - log "starting authorization, artifact, attestation, ACME and VMM infrastructure" - compose up -d mock-cf-dns-api pebble aesmd local-keyprovider auth artifacts - wait_local_key_provider - compose up -d vmm - - log "running production-compatible KMS/Gateway rolling-upgrade E2E" - # Keep the complete runner transcript in state/work even though the runner is - # an ephemeral Compose container. This is especially important for failures - # during deployment, before a per-VM log file exists. - DSTACK_E2E_PHASE=upgrade compose run --rm --no-deps \ - -e DSTACK_E2E_PHASE=upgrade \ - -e DSTACK_E2E_CURRENT_VERSION="$CURRENT_VERSION" \ - -e DSTACK_E2E_CURRENT_REV="$CURRENT_REV" runner \ - 2>&1 | tee "$STATE_DIR/work/runner.log" - - log "upgrade E2E success" - log "artifacts: $STATE_DIR/work" -} - -main "$@" +exec "$(dirname -- "${BASH_SOURCE[0]}")/run-e2e.sh" --phase upgrade "$@" diff --git a/test-suites/full-stack-compose/scripts/render-config.sh b/test-suites/full-stack-compose/scripts/render-config.sh index 0a2314923..efe17c71a 100755 --- a/test-suites/full-stack-compose/scripts/render-config.sh +++ b/test-suites/full-stack-compose/scripts/render-config.sh @@ -13,6 +13,7 @@ IMAGE_ROOT=${DSTACK_E2E_IMAGE_ROOT:-/images} IMAGE_NAME=${DSTACK_E2E_IMAGE_NAME:-dstack-0.6.0} OLD_IMAGE_NAME=${DSTACK_E2E_OLD_IMAGE_NAME:-dstack-0.5.11} PLATFORM=${DSTACK_E2E_PLATFORM:-tdx} +PHASE=${DSTACK_E2E_PHASE:-upgrade} VMM_PORT=${DSTACK_E2E_VMM_PORT:-29080} AUTH_PORT=${DSTACK_E2E_AUTH_PORT:-28011} @@ -113,7 +114,10 @@ package_os_image() { } OS_IMAGE_HASH=$(package_os_image "$IMAGE_NAME") -if [[ "$OLD_IMAGE_NAME" == "$IMAGE_NAME" ]]; then +# Only the upgrade phase boots the compatibility image. A phase that never +# deploys one must not require it to be present -- and must not widen the +# authorization allowlist with a second OS digest it will never launch. +if [[ "$OLD_IMAGE_NAME" == "$IMAGE_NAME" || "$PHASE" != upgrade ]]; then OLD_OS_IMAGE_HASH=$OS_IMAGE_HASH else OLD_OS_IMAGE_HASH=$(package_os_image "$OLD_IMAGE_NAME") @@ -284,5 +288,9 @@ echo " artifacts: http://10.0.2.2:${ARTIFACT_PORT}" echo " old KMS: https://${KMS_RPC_DOMAIN}:${KMS_OLD_HOST_PORT}" echo " latest KMS: https://${KMS_RPC_DOMAIN}:${KMS_LATEST_HOST_PORT}" echo " current image: ${IMAGE_NAME} (${OS_IMAGE_HASH})" -echo " old image: ${OLD_IMAGE_NAME} (${OLD_OS_IMAGE_HASH})" +if [[ "$OLD_OS_IMAGE_HASH" == "$OS_IMAGE_HASH" ]]; then + echo " old image: not used by phase ${PHASE}" +else + echo " old image: ${OLD_IMAGE_NAME} (${OLD_OS_IMAGE_HASH})" +fi echo " gateway app: ${GATEWAY_APP_ID}" diff --git a/test-suites/full-stack-compose/scripts/runner.sh b/test-suites/full-stack-compose/scripts/runner.sh index afd6c0448..9c2adde07 100755 --- a/test-suites/full-stack-compose/scripts/runner.sh +++ b/test-suites/full-stack-compose/scripts/runner.sh @@ -428,6 +428,14 @@ wait_gateway() { } bootstrap_gateway() { + configure_gateway_certbot + issue_first_gateway_cert +} + +# Certbot/DNS/ZT-Domain configuration only. Split out of the first issuance so a +# phase can put something between them -- a fresh cluster reconciling CAA before +# it holds an ACME account, for one. +configure_gateway_certbot() { log "configuring Gateway cluster through node 1" admin_curl 1 SetCertbotConfig \ "$(jq -cn --arg u "http://10.0.2.2:${PEBBLE_HTTP_PORT}/dir" \ @@ -439,6 +447,9 @@ bootstrap_gateway() { admin_curl 1 AddZtDomain \ "$(jq -cn --arg d "$BASE_DOMAIN" '{domain:$d,port:443,priority:100}')" \ >/dev/null +} + +issue_first_gateway_cert() { admin_curl 1 RenewZtDomainCert \ "$(jq -cn --arg d "$BASE_DOMAIN" '{domain:$d,force:true}')" \ | tee "$WORK_DIR/gateway-renew-cert.json" @@ -615,11 +626,11 @@ assert_gateway_caa_records() { # every issuance path this suite already exercises, because nothing else calls # SetCaa. assert_gateway_caa_reconcile() { - local node=$1 - log "reconciling CAA through upgraded Gateway node $node" + local node=$1 expect=${2:-} + log "reconciling CAA through Gateway node $node" admin_curl "$node" SetCaa >/dev/null \ || die "Gateway node $node could not reconcile CAA records" - assert_gateway_caa_records + assert_gateway_caa_records "$expect" log "Gateway node $node pinned CAA to $(cat "$WORK_DIR/gateway-caa-account")" } @@ -1000,11 +1011,66 @@ on_exit() { } trap on_exit EXIT +# Certbot/ACME behaviour on the current code alone, with no upgrade sources and +# no compatibility image: one KMS and a two-node Gateway cluster, both current. +# +# This is the only phase that reaches a fresh cluster's *first* ACME account +# registration on current code. The upgrade phase cannot: Gateway 0.5.8 +# registers the account long before the current binary starts, so every current +# run there takes the load path. +phase_certbot() { + [[ "$PLATFORM" == tdx ]] || die "certbot phase requires TDX" + wait_vmm + clean_start + + render_kms latest "$CURRENT_KMS_IMAGE_ID" kms-current.tar + deploy_kms_onboard latest "$KMS_LATEST_HOST_PORT" + wait_onboard latest "$KMS_LATEST_HOST_PORT" + authorize_kms_from_attestation latest + onboard_rpc "$KMS_LATEST_HOST_PORT" Bootstrap \ + "$(jq -cn --arg d "$KMS_RPC_DOMAIN" '{domain:$d}')" \ + "$WORK_DIR/kms-latest.bootstrap.json" + finish_onboarding latest "$KMS_LATEST_HOST_PORT" + + render_gateway_manifests latest "$CURRENT_GATEWAY_IMAGE_ID" gateway-current.tar + write_gateway_env 1 + write_gateway_env 2 + deploy_gateway 1 latest "$KMS_LATEST_URL" + gateway_version 1 latest + deploy_gateway 2 latest "$KMS_LATEST_URL" + gateway_version 2 latest + + configure_gateway_certbot + + # No account exists yet, so this run registers one -- from inside the region + # that holds the cluster-wide ACME lock, which is exactly the ordering that + # has to be got right. A run that refuses or blocks on its own lock fails + # here; nothing else in this suite reaches it. + local account + assert_gateway_caa_reconcile 1 + account=$(cat "$WORK_DIR/gateway-caa-account") + + # Issue against the account that registration just published, then reconcile + # from the other node: it must adopt the same account rather than register a + # second one, which is what the credentials record being last-writer-wins + # would otherwise silently lose. + issue_first_gateway_cert + assert_gateway_caa_records "$account" + assert_gateway_caa_reconcile 2 "$account" + + assert_gateway_acme_rotation 2 1 + + save_vm_logs + log "gateway certbot/ACME E2E success" + cleanup_after +} + main() { need_bin /workspace/target/release/dstack [[ -s "$STATE_DIR/artifacts/images.env" ]] || die "missing prepared image metadata" case "$PHASE" in upgrade) phase_upgrade ;; + certbot) phase_certbot ;; *) die "unknown DSTACK_E2E_PHASE=$PHASE" ;; esac log "Work artifacts: $WORK_DIR" From e7532bec4b5f6fcc886dfc177c9845f6a5460084 Mon Sep 17 00:00:00 2001 From: Kevin Wang Date: Tue, 25 Aug 2026 21:32:03 -0700 Subject: [PATCH 5/5] test(gateway): make the certbot phase survive the locks it races Two things a current-only cluster hits that an upgraded one never does. `max_dns_wait: 0` is rejected by current code at credential creation -- an issuance that never waits for propagation cannot succeed against a real provider -- and only 0.5.8 ever accepted it. The upgrade phase configures the cluster while the nodes still run 0.5.8, so the value survives into current code as stored state and the validation is never reached. One second is as good as none against a Pebble configured to validate unconditionally. Adding a ZT domain starts an issuance, and on a fresh cluster that issuance registers the shared ACME account -- so an operator's SetCaa, issued right after, races it for the cluster-wide lock and is refused. That refusal is deliberate and says to retry after the holder finishes, which is seconds for a registration. So retry, the way this suite already retries the per-domain certificate lock. Any other error still fails immediately. --- .../full-stack-compose/scripts/runner.sh | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/test-suites/full-stack-compose/scripts/runner.sh b/test-suites/full-stack-compose/scripts/runner.sh index 9c2adde07..4bcef2a4d 100755 --- a/test-suites/full-stack-compose/scripts/runner.sh +++ b/test-suites/full-stack-compose/scripts/runner.sh @@ -435,6 +435,10 @@ bootstrap_gateway() { # Certbot/DNS/ZT-Domain configuration only. Split out of the first issuance so a # phase can put something between them -- a fresh cluster reconciling CAA before # it holds an ACME account, for one. +# `max_dns_wait` is one second rather than zero: current code rejects zero at +# creation (an issuance that never waits for propagation cannot succeed against +# a real provider), and only 0.5.8 ever accepted it. Pebble is configured to +# validate unconditionally, so one second is as good as none here. configure_gateway_certbot() { log "configuring Gateway cluster through node 1" admin_curl 1 SetCertbotConfig \ @@ -442,7 +446,7 @@ configure_gateway_certbot() { '{acme_url:$u, renew_before_expiration_secs:3600}')" >/dev/null admin_curl 1 CreateDnsCredential \ "$(jq -cn --arg u "http://10.0.2.2:${MOCK_CF_HTTP_PORT}/client/v4" \ - '{name:"mock-cloudflare",provider_type:"cloudflare",cf_api_token:"test-token",cf_api_url:$u,set_as_default:true,dns_txt_ttl:1,max_dns_wait:0}')" \ + '{name:"mock-cloudflare",provider_type:"cloudflare",cf_api_token:"test-token",cf_api_url:$u,set_as_default:true,dns_txt_ttl:1,max_dns_wait:1}')" \ >/dev/null admin_curl 1 AddZtDomain \ "$(jq -cn --arg d "$BASE_DOMAIN" '{domain:$d,port:443,priority:100}')" \ @@ -626,12 +630,28 @@ assert_gateway_caa_records() { # every issuance path this suite already exercises, because nothing else calls # SetCaa. assert_gateway_caa_reconcile() { - local node=$1 expect=${2:-} + local node=$1 expect=${2:-} deadline=$((SECONDS + 180)) out rc log "reconciling CAA through Gateway node $node" - admin_curl "$node" SetCaa >/dev/null \ - || die "Gateway node $node could not reconcile CAA records" - assert_gateway_caa_records "$expect" - log "Gateway node $node pinned CAA to $(cat "$WORK_DIR/gateway-caa-account")" + # A fresh cluster registers its shared ACME account from whichever path + # reaches it first, and adding a ZT domain starts an issuance that does + # exactly that. Both take the cluster-wide ACME lock, and the loser is + # refused rather than queued -- deliberately: the refusal says to retry after + # the holder finishes, which is seconds for a registration. Retry, the way + # this suite already retries the per-domain certificate lock. + while (( SECONDS < deadline )); do + rc=0 + out=$(admin_curl "$node" SetCaa 2>&1) || rc=$? + if (( rc == 0 )); then + assert_gateway_caa_records "$expect" + log "Gateway node $node pinned CAA to $(cat "$WORK_DIR/gateway-caa-account")" + return + fi + grep -q "holds the shared ACME lock" <<<"$out" \ + || die "Gateway node $node could not reconcile CAA records: $out" + log "Gateway node $node is waiting for the shared ACME lock" + sleep 3 + done + die "Gateway node $node never acquired the shared ACME lock" } # Rotate the shared ACME account, then require the cluster to issue with it.