diff --git a/CHANGELOG.md b/CHANGELOG.md index e66ad168a..4636d26da 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. 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 9b5ffacfd..288b3403e 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:?}"); @@ -255,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(); @@ -291,6 +313,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 +330,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 { @@ -341,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 @@ -557,51 +629,99 @@ impl DistributedCertBot { ) -> Result { // Get DNS credential (from config or default) let dns_cred = dns_credential_for(&self.kv_store, config)?; + let acme_url = self.acme_url()?; - // Create DNS client based on provider - let dns01_client = self.dns_client(domain, &dns_cred).await?; + if let Some(client) = self + .load_stored_acme_client(domain, &dns_cred, &acme_url) + .await? + { + info!("loaded global ACME account credentials from KvStore"); + return Ok(client); + } - // 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 - }; + // 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 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; + if let Err(err) = self.release_rotation_lock(&rotation_lock) { + error!("failed to release ACME rotation lock: {err:?}"); + } + client + } - // 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 + /// 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")?; - 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" - ); - } - info!("loaded global ACME account credentials from KvStore"); - 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"); + .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 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( + &self, + domain: &str, + dns_cred: &DnsCredential, + acme_url: &str, + ) -> Result { + if let Some(client) = self + .load_stored_acme_client(domain, dns_cred, acme_url) + .await? + { + info!("adopted the ACME account registered while this node waited for the lock"); + return Ok(client); } - // 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, @@ -823,6 +943,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"); @@ -870,32 +1040,164 @@ 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 + /// 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 = certbot_with_domain(data_dir.path()); + let config = test_zt_domain_config(); + + // 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(&config.domain, &config) + .await + { + Ok(_) => panic!("registration must not proceed while the lock is held"), + Err(err) => err, + }; + assert!( + err.to_string() + .contains("cannot register the shared ACME account"), "unexpected error: {err}" ); } + /// 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. + #[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}" ); } @@ -913,8 +1215,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 diff --git a/test-suites/full-stack-compose/README.md b/test-suites/full-stack-compose/README.md index 132b91acf..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 @@ -133,6 +176,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/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 0143ff6fb..4bcef2a4d 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" \ @@ -424,17 +428,32 @@ 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. +# `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 \ "$(jq -cn --arg u "http://10.0.2.2:${PEBBLE_HTTP_PORT}/dir" \ '{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}')" \ >/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" @@ -554,6 +573,110 @@ 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 expect=${2:-} deadline=$((SECONDS + 180)) out rc + log "reconciling CAA through Gateway node $node" + # 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. +# +# 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 +981,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 @@ -900,11 +1031,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"