diff --git a/Cargo.lock b/Cargo.lock index a7ede930456..fa15535ee26 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1516,6 +1516,17 @@ dependencies = [ "uuid", ] +[[package]] +name = "buzz-sandbox-reaper" +version = "0.1.0" +dependencies = [ + "buzz-backend-kubernetes", + "chrono", + "proptest", + "serde", + "serde_json", +] + [[package]] name = "buzz-sdk" version = "0.1.0" diff --git a/Cargo.toml b/Cargo.toml index d27c63d234c..1f74832f313 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -32,6 +32,7 @@ members = [ "crates/buzz-dev-mcp", "crates/buzz-voice", "crates/buzz-backend-kubernetes", + "crates/buzz-sandbox-reaper", "crates/buzz-datastore-tracing", "examples/countdown-bot", ] diff --git a/Justfile b/Justfile index c85b620fd87..c2710c39cfd 100644 --- a/Justfile +++ b/Justfile @@ -465,6 +465,8 @@ test-unit: # because nothing in CI runs `cargo test --workspace` — workspace # membership alone buys clippy/check, not a single executed test. cargo nextest run -p buzz-backend-kubernetes + # Reaper planning and fake-apiserver regressions need no cluster or S3. + cargo nextest run -p buzz-sandbox-reaper # Feature-flag crate coverage: run once with LaunchDarkly enabled. # This includes all default tests plus the cfg(feature="launchdarkly") # tests, avoiding duplicate default-only execution in the unit lane. diff --git a/crates/buzz-sandbox-reaper/Cargo.toml b/crates/buzz-sandbox-reaper/Cargo.toml new file mode 100644 index 00000000000..af73d23bbd4 --- /dev/null +++ b/crates/buzz-sandbox-reaper/Cargo.toml @@ -0,0 +1,20 @@ +[package] +name = "buzz-sandbox-reaper" +version.workspace = true +edition.workspace = true +rust-version.workspace = true +license.workspace = true +description = "In-cluster reaper for ended and orphaned Buzz Agent Sandbox sessions (docs/remote-agents.md §Sandbox lifecycle)" + +[[bin]] +name = "buzz-sandbox-reaper" +path = "src/main.rs" + +[dependencies] +buzz-backend-kubernetes = { path = "../buzz-backend-kubernetes" } +serde = { workspace = true } +serde_json = { workspace = true } +chrono = { workspace = true } + +[dev-dependencies] +proptest = { workspace = true } diff --git a/crates/buzz-sandbox-reaper/src/config.rs b/crates/buzz-sandbox-reaper/src/config.rs new file mode 100644 index 00000000000..32a355c506a --- /dev/null +++ b/crates/buzz-sandbox-reaper/src/config.rs @@ -0,0 +1,195 @@ +//! Reaper configuration: a JSON file mounted from the Terraform-managed +//! ConfigMap `buzz-sandbox-reaper/sandbox-reaper-config` (key `config.json`). +//! +//! The file is the only source of the namespaces the reaper may touch and of +//! each namespace's expected owner. The reaper has no cluster-scoped RBAC, so +//! it never discovers namespaces on its own. + +use std::collections::BTreeMap; + +use serde::Deserialize; + +/// The only config schema version this binary accepts. +pub const CONFIG_VERSION: u32 = 1; + +/// Parsed and validated configuration. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Config { + pub version: u32, + /// Bucket holding workspace checkpoints (`ListBucket` only). + pub checkpoint_bucket: String, + /// AWS region of the bucket. + pub checkpoint_region: String, + /// Namespace name to its developer binding. + pub namespaces: BTreeMap, +} + +/// One developer namespace. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct Namespace { + pub developer: String, + /// Owner pubkey (64 lowercase hex) every managed Sandbox must carry. + pub owner_pubkey: String, + /// Developer checkpoint prefix, e.g. `tee8z/`. + pub checkpoint_prefix: String, +} + +fn is_dns_label(name: &str) -> bool { + !name.is_empty() + && name.len() <= 63 + && name.starts_with(|c: char| c.is_ascii_lowercase() || c.is_ascii_digit()) + && name.ends_with(|c: char| c.is_ascii_lowercase() || c.is_ascii_digit()) + && name + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') +} + +fn is_owner(value: &str) -> bool { + value.len() == 64 + && value + .bytes() + .all(|b| b.is_ascii_digit() || (b'a'..=b'f').contains(&b)) +} + +/// Same shape the image's checkpoint helper enforces: +/// `[a-z0-9][a-z0-9-]{0,31}/`. +fn is_checkpoint_prefix(value: &str) -> bool { + let Some(body) = value.strip_suffix('/') else { + return false; + }; + (1..=32).contains(&body.len()) + && body + .bytes() + .next() + .is_some_and(|b| b.is_ascii_lowercase() || b.is_ascii_digit()) + && body + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'-') +} + +fn is_bucket(value: &str) -> bool { + (3..=63).contains(&value.len()) + && value + .bytes() + .all(|b| b.is_ascii_lowercase() || b.is_ascii_digit() || b == b'.' || b == b'-') + && value + .bytes() + .next() + .is_some_and(|b| b.is_ascii_lowercase() || b.is_ascii_digit()) + && value + .bytes() + .last() + .is_some_and(|b| b.is_ascii_lowercase() || b.is_ascii_digit()) +} + +impl Config { + /// Parse and validate. Every refusal names the offending field. + pub fn parse(text: &str) -> Result { + let config: Self = + serde_json::from_str(text).map_err(|e| format!("invalid reaper config: {e}"))?; + if config.version != CONFIG_VERSION { + return Err(format!( + "unsupported config version {} (expected {CONFIG_VERSION})", + config.version + )); + } + if !is_bucket(&config.checkpoint_bucket) { + return Err("checkpoint_bucket is not a valid S3 bucket name".into()); + } + if config.checkpoint_region.trim().is_empty() { + return Err("checkpoint_region is required".into()); + } + if config.namespaces.is_empty() { + return Err("namespaces must list at least one developer namespace".into()); + } + for (name, namespace) in &config.namespaces { + if !is_dns_label(name) { + return Err(format!("namespace {name:?} is not a valid namespace name")); + } + if namespace.developer.trim().is_empty() { + return Err(format!("namespaces.{name}.developer is required")); + } + if !is_owner(&namespace.owner_pubkey) { + return Err(format!( + "namespaces.{name}.owner_pubkey must be 64 lowercase hex characters" + )); + } + if !is_checkpoint_prefix(&namespace.checkpoint_prefix) { + return Err(format!( + "namespaces.{name}.checkpoint_prefix must match [a-z0-9][a-z0-9-]{{0,31}}/" + )); + } + } + Ok(config) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use serde_json::json; + + fn valid() -> serde_json::Value { + json!({ + "version": 1, + "checkpoint_bucket": "voltage-staging-agent-checkpoints-498537461460", + "checkpoint_region": "us-west-2", + "namespaces": {"buzz-agent-staging-tee8z": { + "developer": "tee8z", "owner_pubkey": "a".repeat(64), "checkpoint_prefix": "tee8z/" + }} + }) + } + + #[test] + fn parses_the_terraform_contract() { + let config = Config::parse(&valid().to_string()).unwrap(); + assert_eq!( + config.namespaces["buzz-agent-staging-tee8z"].developer, + "tee8z" + ); + } + + #[test] + fn refuses_each_invalid_field_and_names_it() { + const NS: &str = "/namespaces/buzz-agent-staging-tee8z"; + let cases = [ + ("/version", json!(2), "version"), + ( + "/checkpoint_bucket", + json!("Bad_Bucket"), + "checkpoint_bucket", + ), + ("/checkpoint_region", json!(" "), "checkpoint_region"), + ("/namespaces", json!({}), "namespaces"), + ("/extra", json!(true), "extra"), + ( + "/namespaces", + json!({"Bad_NS": valid()["namespaces"]["buzz-agent-staging-tee8z"]}), + "Bad_NS", + ), + ("/owner_pubkey", json!("A".repeat(64)), "owner_pubkey"), + ("/checkpoint_prefix", json!("tee8z"), "checkpoint_prefix"), + ("/checkpoint_prefix", json!("a/b/"), "checkpoint_prefix"), + ("/developer", json!(" "), "developer"), + ]; + for (field, bad, named) in cases { + let mut value = valid(); + let pointer = if value.pointer(field).is_some() || field == "/extra" { + field.to_string() + } else { + format!("{NS}{field}") + }; + if field == "/extra" { + value["extra"] = bad; + } else { + *value + .pointer_mut(&pointer) + .unwrap_or_else(|| panic!("test pointer {pointer} does not exist")) = bad; + } + let error = Config::parse(&value.to_string()).unwrap_err(); + assert!(error.contains(named), "{pointer}: {error}"); + } + } +} diff --git a/crates/buzz-sandbox-reaper/src/main.rs b/crates/buzz-sandbox-reaper/src/main.rs new file mode 100644 index 00000000000..6038455d579 --- /dev/null +++ b/crates/buzz-sandbox-reaper/src/main.rs @@ -0,0 +1,14 @@ +//! `buzz-sandbox-reaper`: the in-cluster sandbox reaper for Buzz Agent +//! Sandbox sessions (`docs/remote-agents.md` §Sandbox lifecycle). +#![cfg_attr( + not(test), + expect( + dead_code, + reason = "stack glue: the modules are exercised by their tests until the reconcile subcommand wires them up" + ) +)] + +mod config; +mod plan; + +fn main() {} diff --git a/crates/buzz-sandbox-reaper/src/plan.rs b/crates/buzz-sandbox-reaper/src/plan.rs new file mode 100644 index 00000000000..2355d552751 --- /dev/null +++ b/crates/buzz-sandbox-reaper/src/plan.rs @@ -0,0 +1,357 @@ +//! Pure planning: one classified session plus its checkpoint evidence becomes +//! at most one action. +//! +//! The action vocabulary is deliberately closed. There is no variant that sets +//! `operatingMode: Running` or creates a Sandbox, and [`Action::Delete`] is +//! produced only for an expired tombstone, so the reconciler cannot resurrect +//! a session or discard a live one (property-tested below). + +use buzz_backend_kubernetes::lifecycle::{ + EndedReason, Ignored, State, CHECKPOINT_MISSING, CHECKPOINT_UNCONFIGURED, + MISSING_CHECKPOINT_HOLD_SECS, +}; +use chrono::{DateTime, Duration, Utc}; + +/// What is known about a session's checkpoint. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Evidence { + /// Receipt and S3 listing agree on this key. + Verified(String), + /// The session was launched without checkpoint-on-stop. + Unconfigured, + /// No receipt, or the receipt and S3 disagree. + Missing, + /// Missing, but the session was recovered from this (still present) key, + /// which the tombstone carries forward. + Inherited(String), + /// S3 could not be asked; decide on a later pass. + Unavailable, +} + +impl Evidence { + /// Tombstone `checkpoint` annotation value; `None` while undecidable. + fn annotation(&self) -> Option { + match self { + Self::Verified(key) | Self::Inherited(key) => Some(key.clone()), + Self::Unconfigured => Some(CHECKPOINT_UNCONFIGURED.into()), + Self::Missing => Some(CHECKPOINT_MISSING.into()), + Self::Unavailable => None, + } + } +} + +/// The single action for one session on one pass. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum Action { + /// Nothing to do. + None, + /// Leave it alone and say why (Event). + Ignore(Ignored), + /// Completed without a checkpoint: hold until `until` (Event + metric). + Hold { until: DateTime }, + /// One CAS merge patch: Suspended + lifecycle annotations. + Tombstone { + reason: EndedReason, + checkpoint: String, + }, + /// Delete an Ended tombstone past retention (uid + resourceVersion + /// preconditions). + Delete, +} + +fn tombstone(reason: EndedReason, evidence: &Evidence) -> Action { + match evidence.annotation() { + Some(checkpoint) => Action::Tombstone { reason, checkpoint }, + None => Action::None, + } +} + +/// Does planning `state` depend on checkpoint evidence? Only states that +/// tombstone do; everything else is decided without asking S3. +pub fn needs_evidence(state: &State) -> bool { + matches!( + state, + State::Binding { abandoned: true } + | State::BindingIncomplete { expired: true } + | State::Ending { .. } + | State::Completed { .. } + | State::Replaced + | State::Lost { confirmed: true } + ) +} + +/// Plan one session. `hold_since` is the first apiserver time at which this +/// reaper saw the session Completed. The kubelet's `finishedAt` is not used: +/// the node clock writes it, and every timer uses the apiserver clock. The +/// caller keeps `hold_since` in memory, so a reaper restart starts the hold +/// again. That only delays the tombstone; it never shortens the hold. +pub fn plan( + state: &State, + evidence: &Evidence, + now: DateTime, + hold_since: DateTime, +) -> Action { + match state { + State::Ignored(why) => Action::Ignore(*why), + State::Binding { abandoned: true } => tombstone(EndedReason::BindAbandoned, evidence), + State::BindingIncomplete { expired: true } => { + tombstone(EndedReason::BindingFailed, evidence) + } + State::Ending { reason } => tombstone(*reason, evidence), + State::Replaced => tombstone(EndedReason::Replaced, evidence), + State::Lost { confirmed: true } => tombstone(EndedReason::Lost, evidence), + State::Completed { .. } => match evidence { + Evidence::Missing | Evidence::Inherited(_) => { + let until = hold_since + Duration::seconds(MISSING_CHECKPOINT_HOLD_SECS); + if now >= until { + tombstone(EndedReason::Completed, evidence) + } else { + Action::Hold { until } + } + } + _ => tombstone(EndedReason::Completed, evidence), + }, + State::Ended { expired: true } => Action::Delete, + State::Binding { abandoned: false } + | State::BindingIncomplete { expired: false } + | State::Active + | State::Draining + | State::Lost { confirmed: false } + | State::Ended { expired: false } => Action::None, + } +} + +#[cfg(test)] +mod tests { + use super::*; + use proptest::prelude::*; + + fn t(secs: i64) -> DateTime { + DateTime::from_timestamp(1_800_000_000 + secs, 0).unwrap() + } + + fn key() -> Evidence { + Evidence::Verified("dev/g/x.tar.gz".into()) + } + + #[test] + fn every_terminal_state_tombstones_with_its_reason() { + let cases = [ + ( + State::Binding { abandoned: true }, + EndedReason::BindAbandoned, + ), + ( + State::BindingIncomplete { expired: true }, + EndedReason::BindingFailed, + ), + (State::Replaced, EndedReason::Replaced), + (State::Lost { confirmed: true }, EndedReason::Lost), + ( + State::Ending { + reason: EndedReason::Stopped, + }, + EndedReason::Stopped, + ), + ]; + for (state, reason) in cases { + assert_eq!( + plan(&state, &Evidence::Missing, t(0), t(0)), + Action::Tombstone { + reason, + checkpoint: CHECKPOINT_MISSING.into() + }, + "{state:?}" + ); + assert_eq!( + plan(&state, &key(), t(0), t(0)), + Action::Tombstone { + reason, + checkpoint: "dev/g/x.tar.gz".into() + } + ); + assert_eq!( + plan(&state, &Evidence::Unavailable, t(0), t(0)), + Action::None + ); + } + } + + #[test] + fn completed_with_a_checkpoint_or_none_configured_tombstones_now() { + let completed = State::Completed { + finished_at: Some(t(0)), + message: None, + }; + assert_eq!( + plan(&completed, &key(), t(1), t(1)), + Action::Tombstone { + reason: EndedReason::Completed, + checkpoint: "dev/g/x.tar.gz".into() + } + ); + assert_eq!( + plan(&completed, &Evidence::Unconfigured, t(1), t(1)), + Action::Tombstone { + reason: EndedReason::Completed, + checkpoint: CHECKPOINT_UNCONFIGURED.into() + } + ); + assert_eq!( + plan(&completed, &Evidence::Unavailable, t(1), t(1)), + Action::None + ); + } + + /// The hold runs from the reaper's first (apiserver-time) observation, + /// never from the node-written `finishedAt`, however early that claims. + #[test] + fn completed_without_a_checkpoint_holds_for_24_hours_from_first_observation() { + let until = t(5 + MISSING_CHECKPOINT_HOLD_SECS); + for finished_at in [None, Some(t(-100_000)), Some(t(0))] { + let completed = State::Completed { + finished_at, + message: None, + }; + assert_eq!( + plan( + &completed, + &Evidence::Missing, + until - Duration::seconds(1), + t(5) + ), + Action::Hold { until }, + "{finished_at:?}" + ); + assert_eq!( + plan(&completed, &Evidence::Missing, until, t(5)), + Action::Tombstone { + reason: EndedReason::Completed, + checkpoint: CHECKPOINT_MISSING.into() + } + ); + } + } + + /// A recovered session that saved nothing of its own still holds, then + /// carries its restore checkpoint forward instead of `missing`. + #[test] + fn completed_with_an_inherited_checkpoint_holds_then_carries_it_forward() { + let completed = State::Completed { + finished_at: Some(t(0)), + message: None, + }; + let inherited = Evidence::Inherited("dev/g/x.tar.gz".into()); + let until = t(MISSING_CHECKPOINT_HOLD_SECS); + assert_eq!( + plan(&completed, &inherited, t(1), t(0)), + Action::Hold { until } + ); + assert_eq!( + plan(&completed, &inherited, until, t(0)), + Action::Tombstone { + reason: EndedReason::Completed, + checkpoint: "dev/g/x.tar.gz".into() + } + ); + assert_eq!( + plan(&State::Lost { confirmed: true }, &inherited, t(0), t(0)), + Action::Tombstone { + reason: EndedReason::Lost, + checkpoint: "dev/g/x.tar.gz".into() + } + ); + } + + #[test] + fn ended_tombstones_are_deleted_only_once_expired() { + assert_eq!( + plan( + &State::Ended { expired: true }, + &Evidence::Missing, + t(0), + t(0) + ), + Action::Delete + ); + assert_eq!( + plan(&State::Ended { expired: false }, &key(), t(0), t(0)), + Action::None + ); + } + + fn any_state() -> impl Strategy { + let reason = prop_oneof![ + Just(EndedReason::Stopped), + Just(EndedReason::Completed), + Just(EndedReason::Replaced), + Just(EndedReason::Lost), + Just(EndedReason::BindingFailed), + Just(EndedReason::BindAbandoned), + ]; + let ignored = prop_oneof![ + Just(Ignored::Unmarked), + Just(Ignored::ForeignOwner), + Just(Ignored::ForeignPod), + Just(Ignored::NotRunning), + Just(Ignored::Inconsistent), + ]; + prop_oneof![ + ignored.prop_map(State::Ignored), + any::().prop_map(|abandoned| State::Binding { abandoned }), + any::().prop_map(|expired| State::BindingIncomplete { expired }), + Just(State::Active), + Just(State::Draining), + reason.prop_map(|reason| State::Ending { reason }), + (proptest::option::of(0i64..200_000)).prop_map(|at| State::Completed { + finished_at: at.map(t), + message: None, + }), + Just(State::Replaced), + any::().prop_map(|confirmed| State::Lost { confirmed }), + any::().prop_map(|expired| State::Ended { expired }), + ] + } + + fn any_evidence() -> impl Strategy { + prop_oneof![ + "[a-z]{1,8}".prop_map(Evidence::Verified), + "[a-z]{1,8}".prop_map(Evidence::Inherited), + Just(Evidence::Unconfigured), + Just(Evidence::Missing), + Just(Evidence::Unavailable), + ] + } + + proptest! { + /// Never resurrect, never discard a live session: deletion only for + /// an expired tombstone, nothing at all for live or ignored sessions, + /// and tombstones always carry a decided checkpoint value. + #[test] + fn no_plan_resurrects_or_deletes_a_live_session( + state in any_state(), + evidence in any_evidence(), + now in 0i64..400_000, + since in 0i64..400_000, + ) { + let action = plan(&state, &evidence, t(now), t(since)); + if action == Action::Delete { + prop_assert_eq!(&state, &State::Ended { expired: true }); + } + if matches!(state, State::Active | State::Draining | State::Ended { .. }) + || matches!(state, State::Ignored(_)) + { + prop_assert!(matches!(action, Action::None | Action::Delete | Action::Ignore(_))); + } + if let Action::Tombstone { checkpoint, .. } = &action { + prop_assert!(!checkpoint.is_empty()); + prop_assert_ne!(&evidence, &Evidence::Unavailable); + } + // States decided without evidence really ignore it, so the + // reconciler may skip the S3 probe for them. + if !needs_evidence(&state) { + prop_assert_eq!(&action, &plan(&state, &Evidence::Missing, t(now), t(since))); + } + } + } +} diff --git a/docs/remote-agents.md b/docs/remote-agents.md index 508efe963a9..f75bb34885a 100644 --- a/docs/remote-agents.md +++ b/docs/remote-agents.md @@ -1522,7 +1522,10 @@ keys (`buzz-backend-kubernetes` library, `lifecycle.rs`): | `buzz.block.xyz/recovered-from` | the generation of the tombstone that a recovered session replaced | The reaper classifies each Sandbox on every pass. All timers use the -apiserver `Date` header, not the reaper's clock. +apiserver `Date` header, not the reaper's clock. The 24-hour hold of a +Completed session starts when the reaper first sees it Completed, not at the +container's `finishedAt` (the node clock writes that value). A reaper restart +starts the hold again; this only delays the tombstone. | State | Detection | Reaper action | | --- | --- | --- | diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index fededb68421..f3ca8862871 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -151,6 +151,10 @@ run_unit_tests() { run_test_step "buzz-backend-kubernetes tests" \ cargo test -p buzz-backend-kubernetes -- --nocapture + # Keep the reaper regressions in step with the nextest unit lane. + run_test_step "buzz-sandbox-reaper tests" \ + cargo test -p buzz-sandbox-reaper -- --nocapture + # Keep fallback parity with `just test-unit`: one LaunchDarkly-feature run # exercises both default and feature-gated buzz-feature-flags tests. run_test_step "buzz-feature-flags tests (launchdarkly)" \