diff --git a/crates/buzz-backend-kubernetes/src/sandbox.rs b/crates/buzz-backend-kubernetes/src/sandbox.rs index e75fec52dc4..b80d361c6fd 100644 --- a/crates/buzz-backend-kubernetes/src/sandbox.rs +++ b/crates/buzz-backend-kubernetes/src/sandbox.rs @@ -10,10 +10,12 @@ use std::collections::BTreeMap; use std::time::Duration; use buzz_backend_kubernetes::lifecycle::{ - annotation, owned_pod, sandbox_resource, EndedReason, Lifecycle, OperatingMode, CHANNEL, - CHECKPOINTING, CHECKPOINTING_DISABLED, CHECKPOINTING_ENABLED, ENDED_REASON, GENERATION, - INITIAL_POD, OWNER, SANDBOX_BINDING_VERSION, SESSION, THREAD, + annotation, owned_pod, sandbox_resource, EndedReason, Lifecycle, OperatingMode, + BIND_TIMEOUT_SECS, CHANNEL, CHECKPOINTING, CHECKPOINTING_DISABLED, CHECKPOINTING_ENABLED, + ENDED_REASON, GENERATION, INITIAL_POD, OWNER, RESTORE_CHECKPOINT, SANDBOX_BINDING_VERSION, + SESSION, THREAD, }; +use chrono::{DateTime, Utc}; use k8s_openapi::api::core::v1::{EnvVar, EnvVarSource, ObjectFieldSelector, Pod, Secret}; use k8s_openapi::apimachinery::pkg::apis::meta::v1::OwnerReference; use kube::api::{Api, ListParams, PostParams}; @@ -26,6 +28,16 @@ use sha2::{Digest, Sha256}; use crate::wire::{ErrorCode, ProviderError}; use crate::{config::ProviderConfig, intent::Fingerprint, naming, pod}; +/// Secret env key carrying the checkpoint a recovered session restores from. +/// Always present (empty unless recovering) so its *key* is part of every +/// fingerprint and a later ordinary deploy matches the recovered Sandbox. +pub const RESTORE_KEY_ENV: &str = "BUZZ_CHECKPOINT_RESTORE_KEY"; + +/// How long one deploy waits for its binding, and then for startup. +const DEPLOY_DEADLINE_SECS: u64 = 120; +/// Allowance for skew between this host's clock and the apiserver's. +const CLOCK_SKEW_SECS: i64 = 60; + #[derive(Debug, Deserialize)] #[serde(deny_unknown_fields)] pub struct Options { @@ -182,9 +194,35 @@ fn verify_identity( Ok(()) } +/// Launch fingerprints a live Sandbox may carry and still be reused. +struct Fingerprints { + /// Every key this version writes, including [`RESTORE_KEY_ENV`]. + current: Fingerprint, + /// The same launch from a provider that predates [`RESTORE_KEY_ENV`]. + /// Accepted only on reuse, so Sandboxes created before this version keep + /// working while desktop versions are mixed; never written. + legacy: Fingerprint, +} + +impl Fingerprints { + fn new(cfg: &ProviderConfig, env: &BTreeMap) -> Self { + let keys = || env.keys().cloned(); + Self { + current: pod::intent_template(cfg, keys()).fingerprint(), + legacy: pod::intent_template(cfg, keys().filter(|key| key != RESTORE_KEY_ENV)) + .fingerprint(), + } + } + + fn accepts(&self, sandbox: &DynamicObject) -> bool { + let recorded = annotation(sandbox, naming::ANNOTATION_CREATE_INTENT); + recorded == Some(self.current.as_str()) || recorded == Some(self.legacy.as_str()) + } +} + /// The Sandbox is a live session this deploy may reuse. Every refusal names the /// lifecycle state so the desktop can offer the right explicit action. -fn verify_live(sandbox: &DynamicObject, fingerprint: &Fingerprint) -> Result<(), ProviderError> { +fn verify_live(sandbox: &DynamicObject, fingerprints: &Fingerprints) -> Result<(), ProviderError> { let refused = |code, message: &str| Err(ProviderError::refused(code, message)); if sandbox.metadata.deletion_timestamp.is_some() { return refused( @@ -225,7 +263,7 @@ fn verify_live(sandbox: &DynamicObject, fingerprint: &Fingerprint) -> Result<(), "Sandbox is not an active managed session; explicit recovery is required", ); } - if annotation(sandbox, naming::ANNOTATION_CREATE_INTENT) != Some(fingerprint.as_str()) { + if !fingerprints.accepts(sandbox) { return refused( ErrorCode::ConfigDiffers, "Sandbox launch configuration differs; end the session, then explicit recovery is required", @@ -294,7 +332,7 @@ struct Session<'a> { identity: &'a naming::AgentIdentity, scope: &'a Options, owner: &'a str, - fingerprint: Fingerprint, + fingerprints: Fingerprints, } impl<'a> Session<'a> { @@ -304,7 +342,7 @@ impl<'a> Session<'a> { identity: &'a naming::AgentIdentity, scope: &'a Options, owner: &'a str, - fingerprint: Fingerprint, + fingerprints: Fingerprints, ) -> Self { Self { sandboxes: api(client.clone(), namespace), @@ -314,7 +352,7 @@ impl<'a> Session<'a> { identity, scope, owner, - fingerprint, + fingerprints, } } } @@ -337,8 +375,8 @@ pub async fn deploy( return Err("Sandbox requires pod_options.active_deadline_seconds".into()); } let checkpointing = apply_session_env(scope, &mut env)?; - let fingerprint = pod::intent_template(cfg, env.keys().cloned()).fingerprint(); - let session = Session::new(client, &cfg.namespace, identity, scope, owner, fingerprint); + let fingerprints = Fingerprints::new(cfg, &env); + let session = Session::new(client, &cfg.namespace, identity, scope, owner, fingerprints); let existing = session .sandboxes .get_opt(&session.name) @@ -349,7 +387,7 @@ pub async fn deploy( None => create_new(&session, cfg, checkpointing).await?, }; verify_identity(&sandbox, identity, scope, owner)?; - verify_live(&sandbox, &session.fingerprint)?; + verify_live(&sandbox, &session.fingerprints)?; bind(&session, cfg, sandbox, env).await } @@ -395,8 +433,10 @@ fn apply_session_env(scope: &Options, env: &mut BTreeMap) -> Res if checkpointing { env.insert("BUZZ_ACP_CHECKPOINT_ON_STOP".into(), "true".into()); } - // Include the UID binding key in the fingerprint without hashing its value. + // Include the binding keys in the fingerprint without hashing their + // values; both are filled in from the Sandbox when the Secret is written. env.insert("BUZZ_SANDBOX_INITIAL_POD_UID".into(), String::new()); + env.insert(RESTORE_KEY_ENV.into(), String::new()); Ok(checkpointing) } @@ -448,7 +488,7 @@ async fn create_new( session.scope, session.owner, &new, - &session.fingerprint, + &session.fingerprints.current, )?; match session .sandboxes @@ -468,7 +508,39 @@ async fn create_new( } } +/// The reaper ends a never-started binding once its Pod is +/// [`BIND_TIMEOUT_SECS`] old, by a CAS on the Sandbox that a Secret write does +/// not bump. A deploy stops resuming early enough to write the Secret and see +/// the container start before then. +fn resume_window_closed(pod: &Pod, now: DateTime) -> bool { + let window = BIND_TIMEOUT_SECS - DEPLOY_DEADLINE_SECS as i64 - CLOCK_SKEW_SECS; + pod.metadata + .creation_timestamp + .as_ref() + .is_some_and(|created| now - created.0 >= chrono::Duration::seconds(window)) +} + +fn container_started(pod: &Pod) -> bool { + pod.status + .as_ref() + .and_then(|s| s.container_statuses.as_ref()) + .and_then(|containers| containers.iter().find(|c| c.name == "agent")) + .is_some_and(|c| { + c.restart_count > 0 + || c.last_state + .as_ref() + .is_some_and(|s| s.terminated.is_some() || s.running.is_some()) + || c.state + .as_ref() + .is_some_and(|s| s.terminated.is_some() || s.running.is_some()) + }) +} + /// Bind credentials to the Sandbox's first Pod and wait for it to start. +/// +/// Resumable: a session bound to its first Pod whose Secret is missing and +/// whose container never started is completed by whichever deploy observes +/// it, with the Sandbox's own generation and an ownerRef to that Pod. async fn bind( session: &Session<'_>, cfg: &ProviderConfig, @@ -480,8 +552,9 @@ async fn bind( let generation = annotation(&sandbox, GENERATION) .ok_or("Sandbox generation is missing")? .to_string(); - let deadline = tokio::time::Instant::now() + Duration::from_secs(120); + let deadline = tokio::time::Instant::now() + Duration::from_secs(DEPLOY_DEADLINE_SECS); loop { + let expired = tokio::time::Instant::now() >= deadline; let Some(child) = session .pods .get_opt(name) @@ -494,7 +567,7 @@ async fn bind( "Sandbox lost its original Pod; explicit recovery is required", )); } - if tokio::time::Instant::now() >= deadline { + if expired { return Err( "Sandbox controller has not created its Pod; inspect the Sandbox and retry" .into(), @@ -528,38 +601,25 @@ async fn bind( sandbox .annotations_mut() .insert(INITIAL_POD.into(), uid.into()); - sandbox = match session + match session .sandboxes .replace(name, &PostParams::default(), &sandbox) .await { - Ok(bound) => bound, + Ok(bound) => { + sandbox = bound; + } Err(e) if reason_is(&e, "Conflict") => { sandbox = refresh(session, &generation).await?; - if tokio::time::Instant::now() >= deadline { + if expired { return Err( "Sandbox kept changing during credential binding; retry deploy".into(), ); } - continue; } Err(e) => return Err(format!("bind Sandbox to its first Pod: {e}").into()), - }; - let secret = bound_secret(identity, cfg, &generation, name, uid, env); - session - .secrets - .create(&PostParams::default(), &secret) - .await - .map_err(|e| { - ProviderError::refused( - ErrorCode::BindingIncomplete, - format!( - "could not create bound Sandbox credentials ({}); explicit recovery is required", - api_failure(&e) - ), - ) - })?; - return wait_for_start(&session.pods, &sandbox, name, uid).await; + } + continue; }; if uid != initial_uid { return Err(ProviderError::refused( @@ -567,23 +627,52 @@ async fn bind( "Sandbox Pod was replaced; credentials remain bound to the original session, explicit recovery is required", )); } - let secret = session + let secret_name = identity.secret_name(&generation); + let secret = + session.secrets.get_opt(&secret_name).await.map_err(|e| { + format!("could not check Sandbox launch Secret: {}", api_failure(&e)) + })?; + if let Some(secret) = secret { + check_secret(&secret, identity, uid, &env)?; + return wait_for_start(&session.pods, &sandbox, name, uid).await; + } + if container_started(&child) { + return Err(ProviderError::refused( + ErrorCode::CredentialsMissing, + "Sandbox credentials are missing from a started session; explicit recovery is required", + )); + } + if expired { + return Err(ProviderError::refused( + ErrorCode::BindingIncomplete, + "Sandbox credential binding is incomplete; retry deploy", + )); + } + if resume_window_closed(&child, Utc::now()) { + return Err(ProviderError::refused( + ErrorCode::BindingExpired, + "Sandbox credential binding is too old to resume; explicit recovery is required once the session has ended", + )); + } + let secret = bound_secret(identity, cfg, &sandbox, &generation, name, uid, env.clone()); + match session .secrets - .get_opt(&identity.secret_name(&generation)) + .create(&PostParams::default(), &secret) .await - .map_err(|e| format!("could not check Sandbox launch Secret: {}", api_failure(&e)))?; - let Some(secret) = secret else { - if tokio::time::Instant::now() >= deadline { - return Err(ProviderError::refused( - ErrorCode::BindingIncomplete, - "Sandbox credential binding is incomplete; explicit recovery is required", - )); + { + // Either way, the next pass re-reads and verifies what was stored. + Ok(_) => {} + Err(e) if reason_is(&e, "AlreadyExists") => {} + Err(e) => { + // The write may have succeeded despite a lost response. + // Keep the Pod binding so retries verify the same session. + return Err(format!( + "could not create bound Sandbox credentials ({}); retry deploy", + api_failure(&e) + ) + .into()); } - tokio::time::sleep(Duration::from_millis(250)).await; - continue; - }; - check_secret(&secret, identity, uid, &env)?; - return wait_for_start(&session.pods, &sandbox, name, uid).await; + } } } @@ -595,7 +684,7 @@ async fn refresh(session: &Session<'_>, generation: &str) -> Result, generation: &str) -> Result Secret { env.insert("BUZZ_MANAGED_AGENT_START_NONCE".into(), generation.into()); env.insert("BUZZ_SANDBOX_INITIAL_POD_UID".into(), uid.into()); + // The restore key comes from the Sandbox, never from the request, so a + // resumed binding keeps the recovery its Sandbox was created with. + env.insert( + RESTORE_KEY_ENV.into(), + annotation(sandbox, RESTORE_CHECKPOINT) + .unwrap_or_default() + .into(), + ); let mut secret = pod::build_secret(identity, &cfg.namespace, generation, env); secret.labels_mut().insert( naming::LABEL_BINDING_VERSION.into(), @@ -673,7 +771,7 @@ async fn wait_for_start( name: &str, uid: &str, ) -> Result { - let deadline = tokio::time::Instant::now() + Duration::from_secs(120); + let deadline = tokio::time::Instant::now() + Duration::from_secs(DEPLOY_DEADLINE_SECS); loop { let child = pods .get_opt(name) diff --git a/crates/buzz-backend-kubernetes/src/sandbox_fake.rs b/crates/buzz-backend-kubernetes/src/sandbox_fake.rs index 8928fbd14ee..7ee8afc0fa0 100644 --- a/crates/buzz-backend-kubernetes/src/sandbox_fake.rs +++ b/crates/buzz-backend-kubernetes/src/sandbox_fake.rs @@ -4,7 +4,8 @@ //! It models only what the Sandbox paths touch: Sandbox CRUD with //! resourceVersion CAS, a controller that creates the Sandbox's Pod while it //! is Running, Pods whose agent container runs once their `envFrom` Secret -//! exists, and Secret create. +//! exists (or whose status a test overrides), and Secret create with failure +//! injection. use std::collections::BTreeMap; use std::sync::{Arc, Mutex}; @@ -22,8 +23,18 @@ pub struct State { pub secrets: BTreeMap, /// Pod UIDs by name; Pods are rendered from their Sandbox on read. pub pods: BTreeMap, + /// Pod `creationTimestamp`s (RFC 3339) by name; set when the controller + /// creates the Pod. + pub pod_created: BTreeMap, + /// Fields that replace the derived agent container status, such as + /// `restartCount` or `lastState`. + pub container_status: Option, pub rv: u64, pub uid: u64, + /// Fail this many Secret creates with a 500. + pub fail_secret_creates: usize, + /// Store the Secret, then fail this many create responses with a 500. + pub fail_secret_create_responses: usize, } pub type Shared = Arc>; @@ -64,8 +75,11 @@ impl State { if running && !self.pods.contains_key(name) { let uid = self.next_uid("pod"); self.pods.insert(name.into(), uid); + self.pod_created + .insert(name.into(), chrono::Utc::now().to_rfc3339()); } else if !running { self.pods.remove(name); + self.pod_created.remove(name); } } @@ -84,19 +98,26 @@ impl State { } else { json!({"waiting": {"reason": "CreateContainerConfigError"}}) }; + let mut container = json!({ + "name": "agent", "image": "i", "imageID": "", "ready": started, + "restartCount": 0, "state": state}); + if let Some(Value::Object(fields)) = &self.container_status { + for (key, value) in fields { + container[key] = value.clone(); + } + } Some(json!({ "apiVersion": "v1", "kind": "Pod", "metadata": { "name": name, "uid": uid, + "creationTimestamp": self.pod_created.get(name), "labels": template["metadata"]["labels"], "annotations": template["metadata"]["annotations"], "ownerReferences": [{"apiVersion": "agents.x-k8s.io/v1beta1", "kind": "Sandbox", "name": name, "uid": sandbox["metadata"]["uid"], "controller": true}], }, - "status": {"phase": "Pending", "containerStatuses": [{ - "name": "agent", "image": "i", "imageID": "", "ready": started, - "restartCount": 0, "state": state}]}, + "status": {"phase": "Pending", "containerStatuses": [container]}, })) } @@ -121,6 +142,10 @@ impl State { None => status(404, "NotFound"), }, ["api", "v1", "namespaces", _, "secrets"] if method == Method::POST => { + if self.fail_secret_creates > 0 { + self.fail_secret_creates -= 1; + return status(500, "InternalError"); + } let mut secret: Secret = serde_json::from_slice(body).unwrap(); let name = secret.metadata.name.clone().unwrap(); if self.secrets.contains_key(&name) { @@ -135,6 +160,10 @@ impl State { secret.metadata.uid = Some(self.next_uid("secret")); let stored = serde_json::to_value(&secret).unwrap(); self.secrets.insert(name, stored.clone()); + if self.fail_secret_create_responses > 0 { + self.fail_secret_create_responses -= 1; + return status(500, "InternalError"); + } ok(&stored) } _ => status(404, "NotFound"), diff --git a/crates/buzz-backend-kubernetes/src/sandbox_tests.rs b/crates/buzz-backend-kubernetes/src/sandbox_tests.rs index 63a2fa4d247..b6c530dfa49 100644 --- a/crates/buzz-backend-kubernetes/src/sandbox_tests.rs +++ b/crates/buzz-backend-kubernetes/src/sandbox_tests.rs @@ -17,12 +17,12 @@ fn custom_image_is_inside_a_real_sandbox_with_a_downward_uid() { channel_id: uuid::Uuid::new_v4().to_string(), thread_root: "c".repeat(64), }; - let fp = pod::intent_template(&cfg, vec![]).fingerprint(); + let fps = Fingerprints::new(&cfg, &BTreeMap::new()); let new = NewSession { generation: "generation", checkpointing: false, }; - let mut sandbox = build(&identity, &cfg, &scope, "owner", &new, &fp).unwrap(); + let mut sandbox = build(&identity, &cfg, &scope, "owner", &new, &fps.current).unwrap(); assert_eq!( sandbox.types.as_ref().unwrap().api_version, "agents.x-k8s.io/v1beta1" @@ -44,8 +44,9 @@ fn custom_image_is_inside_a_real_sandbox_with_a_downward_uid() { assert_eq!(spec["volumes"][0]["emptyDir"]["sizeLimit"], "50Gi"); assert_eq!(spec["containers"][0]["terminationMessagePolicy"], "File"); assert_eq!(annotation(&sandbox, CHECKPOINTING), Some("disabled")); + assert_eq!(annotation(&sandbox, RESTORE_CHECKPOINT), None); verify_identity(&sandbox, &identity, &scope, "owner").unwrap(); - verify_live(&sandbox, &fp).unwrap(); + verify_live(&sandbox, &fps).unwrap(); sandbox .annotations_mut() .insert(OWNER.into(), "different-owner".into()); @@ -420,7 +421,7 @@ fn sandbox_json(state: &fake::Shared, name: &str) -> serde_json::Value { } #[tokio::test(start_paused = true)] -async fn fresh_deploy_binds_and_reuses_its_session() { +async fn fresh_deploy_binds_and_always_carries_an_empty_restore_key() { let state = fake::Shared::default(); let (identity, cfg, scope) = (fake_identity(), fake_cfg(), fake_scope()); let name = deploy( @@ -454,6 +455,8 @@ async fn fresh_deploy_binds_and_reuses_its_session() { }) .collect(); assert_eq!(secret_owner_uids, [initial_pod_uid]); + assert!(annotations.get(RESTORE_CHECKPOINT).is_none()); + assert_eq!(secret_env(&state, RESTORE_KEY_ENV), [""]); assert_eq!( secret_env(&state, "BUZZ_MANAGED_AGENT_START_NONCE"), [annotations[GENERATION].as_str().unwrap()] @@ -586,3 +589,286 @@ async fn deploy_refuses_a_sandbox_that_is_not_a_reusable_live_session() { assert_eq!(guard.sandboxes[&name], before, "{case}: wrote the Sandbox"); } } + +/// The medium bug: bound to the first Pod, Secret never written, container +/// never started. A later deploy must finish the binding, not strand it. +#[tokio::test(start_paused = true)] +async fn a_bound_session_without_its_secret_is_resumed() { + let state = fake::Shared::default(); + let (identity, cfg, scope) = (fake_identity(), fake_cfg(), fake_scope()); + deploy( + fake::client(&state), + &identity, + &cfg, + &scope, + "owner", + plain_env(), + ) + .await + .unwrap(); + let name = scope.name(&identity); + let (generation, pod_uid) = { + let mut guard = state.lock().unwrap(); + guard.secrets.clear(); + let sandbox = &guard.sandboxes[&name]; + ( + sandbox["metadata"]["annotations"][GENERATION].clone(), + guard.pods[&name].clone(), + ) + }; + deploy( + fake::client(&state), + &identity, + &cfg, + &scope, + "owner", + plain_env(), + ) + .await + .unwrap(); + let guard = state.lock().unwrap(); + let (secret_name, secret) = guard.secrets.iter().next().unwrap(); + assert!(secret_name.ends_with(generation.as_str().unwrap())); + assert_eq!(secret["metadata"]["ownerReferences"][0]["uid"], pod_uid); + assert_eq!( + sandbox_json_locked(&guard, &name)["metadata"]["annotations"][GENERATION], + generation + ); +} + +fn sandbox_json_locked(state: &fake::State, name: &str) -> serde_json::Value { + state.sandboxes[name].clone() +} + +#[tokio::test(start_paused = true)] +async fn a_secret_create_failure_preserves_the_binding_and_the_retry_succeeds() { + assert_secret_create_retry(false).await; +} + +#[tokio::test(start_paused = true)] +async fn a_failed_secret_create_response_preserves_the_live_session() { + assert_secret_create_retry(true).await; +} + +async fn assert_secret_create_retry(stored_before_error: bool) { + let state = fake::Shared::default(); + if stored_before_error { + state.lock().unwrap().fail_secret_create_responses = 1; + } else { + state.lock().unwrap().fail_secret_creates = 1; + } + let (identity, cfg, scope) = (fake_identity(), fake_cfg(), fake_scope()); + let error = deploy( + fake::client(&state), + &identity, + &cfg, + &scope, + "owner", + plain_env(), + ) + .await + .unwrap_err(); + assert_eq!(error.code, None, "{error}"); + assert!(error.message.ends_with("retry deploy"), "{error}"); + let name = scope.name(&identity); + let sandbox = sandbox_json(&state, &name); + let pod_uid = state.lock().unwrap().pods[&name].clone(); + assert_eq!( + sandbox["metadata"]["annotations"][INITIAL_POD], pod_uid, + "an uncertain Secret write must not discard the original Pod binding" + ); + assert_eq!( + state.lock().unwrap().secrets.len(), + usize::from(stored_before_error) + ); + deploy( + fake::client(&state), + &identity, + &cfg, + &scope, + "owner", + plain_env(), + ) + .await + .unwrap(); + assert_eq!(sandbox_json(&state, &name), sandbox); + let guard = state.lock().unwrap(); + assert_eq!(guard.pods[&name], pod_uid); + assert_eq!(guard.secrets.len(), 1); + let secret = guard.secrets.values().next().unwrap(); + assert_eq!(secret["metadata"]["ownerReferences"][0]["uid"], pod_uid); +} + +#[test] +fn the_restore_key_is_fingerprinted_by_name_not_value() { + let cfg = fake_cfg(); + let scope = fake_scope(); + let mut plain = plain_env(); + apply_session_env(&scope, &mut plain).unwrap(); + assert_eq!(plain[RESTORE_KEY_ENV], ""); + let mut restored = plain.clone(); + restored.insert(RESTORE_KEY_ENV.into(), "dev/g/x.tar.gz".into()); + assert_eq!( + pod::intent_template(&cfg, plain.keys().cloned()).fingerprint(), + pod::intent_template(&cfg, restored.keys().cloned()).fingerprint() + ); + let mut without = plain.clone(); + without.remove(RESTORE_KEY_ENV); + assert_ne!( + pod::intent_template(&cfg, plain.keys().cloned()).fingerprint(), + pod::intent_template(&cfg, without.keys().cloned()).fingerprint() + ); +} + +/// Sandboxes created before the restore key existed carry a fingerprint +/// without it. Mixed desktop versions must keep reusing them; any other +/// fingerprint is still a configuration change. +#[tokio::test(start_paused = true)] +async fn a_sandbox_from_before_the_restore_key_is_still_reused() { + let state = fake::Shared::default(); + let (identity, cfg, scope) = (fake_identity(), fake_cfg(), fake_scope()); + let name = deploy( + fake::client(&state), + &identity, + &cfg, + &scope, + "owner", + plain_env(), + ) + .await + .unwrap(); + let mut env = plain_env(); + apply_session_env(&scope, &mut env).unwrap(); + let fingerprints = Fingerprints::new(&cfg, &env); + assert_ne!(fingerprints.legacy, fingerprints.current); + let set_intent = |value: &str| { + state.lock().unwrap().sandboxes.get_mut(&name).unwrap()["metadata"]["annotations"] + [naming::ANNOTATION_CREATE_INTENT] = value.into(); + }; + set_intent(fingerprints.legacy.as_str()); + let redeploy = || { + deploy( + fake::client(&state), + &identity, + &cfg, + &scope, + "owner", + plain_env(), + ) + }; + assert_eq!(redeploy().await.unwrap(), name); + set_intent(&"0".repeat(64)); + assert_eq!( + redeploy().await.unwrap_err().code, + Some(ErrorCode::ConfigDiffers) + ); +} + +/// Deploy once, then drop the launch Secret: a bound session whose +/// credentials were never written. +async fn unbound_credentials( + identity: &naming::AgentIdentity, + cfg: &ProviderConfig, + scope: &Options, +) -> (fake::Shared, String) { + let state = fake::Shared::default(); + let name = deploy( + fake::client(&state), + identity, + cfg, + scope, + "owner", + plain_env(), + ) + .await + .unwrap(); + state.lock().unwrap().secrets.clear(); + (state, name) +} + +/// A container that ever ran without its Secret is not resumed: writing +/// credentials now would hand them to a process that already started. +#[tokio::test(start_paused = true)] +async fn a_started_session_without_credentials_is_not_resumed() { + let (identity, cfg, scope) = (fake_identity(), fake_cfg(), fake_scope()); + for started in [ + json!({"restartCount": 1}), + json!({"lastState": {"terminated": {"exitCode": 1}}}), + json!({"state": {"running": {}}}), + ] { + let (state, _) = unbound_credentials(&identity, &cfg, &scope).await; + state.lock().unwrap().container_status = Some(started.clone()); + let error = deploy( + fake::client(&state), + &identity, + &cfg, + &scope, + "owner", + plain_env(), + ) + .await + .unwrap_err(); + assert_eq!(error.code, Some(ErrorCode::CredentialsMissing), "{started}"); + assert!(state.lock().unwrap().secrets.is_empty(), "{started}"); + } +} + +/// The restore key in the Secret is the Sandbox's, whatever the request +/// carries, so a resumed binding keeps the recovery it was created with. +#[tokio::test(start_paused = true)] +async fn the_restore_key_comes_from_the_sandbox_not_the_request() { + let (identity, cfg, scope) = (fake_identity(), fake_cfg(), fake_scope()); + for recorded in [Some("dev/generation/checkpoint.tar.gz"), None] { + let (state, name) = unbound_credentials(&identity, &cfg, &scope).await; + if let Some(key) = recorded { + state.lock().unwrap().sandboxes.get_mut(&name).unwrap()["metadata"]["annotations"] + [RESTORE_CHECKPOINT] = key.into(); + } + let mut env = plain_env(); + env.insert(RESTORE_KEY_ENV.into(), "dev/other/forged.tar.gz".into()); + deploy(fake::client(&state), &identity, &cfg, &scope, "owner", env) + .await + .unwrap(); + assert_eq!( + secret_env(&state, RESTORE_KEY_ENV), + [recorded.unwrap_or_default()] + ); + } +} + +/// Resuming stops before the reaper's bind timeout can end the session +/// under a deploy that is still writing credentials or waiting for startup. +#[tokio::test(start_paused = true)] +async fn a_binding_too_close_to_the_reaper_timeout_is_not_resumed() { + let (identity, cfg, scope) = (fake_identity(), fake_cfg(), fake_scope()); + let window = BIND_TIMEOUT_SECS - DEPLOY_DEADLINE_SECS as i64 - CLOCK_SKEW_SECS; + for (age, resumed) in [(window - 60, true), (window, false)] { + let (state, name) = unbound_credentials(&identity, &cfg, &scope).await; + let created = Utc::now() - chrono::Duration::seconds(age); + state + .lock() + .unwrap() + .pod_created + .insert(name, created.to_rfc3339()); + let result = deploy( + fake::client(&state), + &identity, + &cfg, + &scope, + "owner", + plain_env(), + ) + .await; + if resumed { + result.unwrap(); + assert_eq!(state.lock().unwrap().secrets.len(), 1); + } else { + assert_eq!( + result.unwrap_err().code, + Some(ErrorCode::BindingExpired), + "{age}" + ); + assert!(state.lock().unwrap().secrets.is_empty()); + } + } +} diff --git a/crates/buzz-backend-kubernetes/src/wire.rs b/crates/buzz-backend-kubernetes/src/wire.rs index 046a2139821..54160ec568d 100644 --- a/crates/buzz-backend-kubernetes/src/wire.rs +++ b/crates/buzz-backend-kubernetes/src/wire.rs @@ -158,8 +158,13 @@ pub enum ErrorCode { /// The session was launched against a different relay. RelayDiffers, /// The session is bound to its first Pod, but its credentials were not - /// written. + /// written within this deploy's deadline; a retry resumes the binding. BindingIncomplete, + /// The session is bound to its first Pod without credentials, and the + /// Pod is too old to resume before the reaper ends the binding. + BindingExpired, + /// The bound container started without its credentials. + CredentialsMissing, /// The Pod the session is bound to is gone. PodLost, /// The controller replaced the Pod the session is bound to. diff --git a/docs/remote-agents.md b/docs/remote-agents.md index a493f36868a..57520ba64fd 100644 --- a/docs/remote-agents.md +++ b/docs/remote-agents.md @@ -1464,11 +1464,9 @@ For Codex account enrollment, set `BUZZ_SETUP_MODE=codex-device-v1`, select `cod The provider supplies the agent/image binding, downward Pod UID, one-use nonce, and a 15-minute setup deadline. Retries preserve the original binding and deadline. The image must accept the Sandbox's 32-character generation. -**Limit: interrupted credential binding.** Deploy first records the Pod UID on the Sandbox, then creates the launch Secret. -If the Secret create fails after the UID is recorded, the deploy error names the Kubernetes reason and code. -Later deploys for that thread wait 120s for the Secret and then fail with "Sandbox credential binding is incomplete; explicit recovery is required". -To recover, delete the Sandbox. This discards the workspace; the container never started, so it contains no agent work. Then mention the agent in the thread again. -A later layer of this stack makes the binding resumable, so a retried deploy creates the missing Secret for the same Pod. +**Interrupted credential binding.** Deploy first records the Pod UID on the Sandbox, then creates the launch Secret. +If the Secret create fails after the UID is recorded, the deploy error names the Kubernetes reason and code, +and the next deploy for that thread resumes the binding for the same Pod (§Deploy repair). ### Sandbox lifecycle @@ -1498,7 +1496,7 @@ apiserver `Date` header, not the reaper's clock. | --- | --- | --- | | Binding | Running, no first-Pod binding, Sandbox younger than 1200s | none | | BindAbandoned | as Binding, Sandbox 1200s or older | tombstone `bind-abandoned` | -| BindingIncomplete | bound to this Pod, container never started | none before the Pod is 1200s old (planned: a deploy completes the binding); then tombstone `binding-failed` | +| BindingIncomplete | bound to this Pod, container never started | none before the Pod is 1200s old (a deploy completes the binding, §Deploy repair); then tombstone `binding-failed` | | Active | bound to this Pod, container running | none | | Draining | Pod deleting, or `lifecycle=ending` with the Pod present | none | | Ending | `lifecycle=ending`, Pod gone | tombstone with the recorded reason (`stopped`) | @@ -1521,6 +1519,28 @@ configuration lists for the namespace. It records a Warning Event for a managed Sandbox that it leaves alone (foreign owner, foreign Pod, inconsistent lifecycle). +**Deploy repair.** A deploy that bound the first Pod but did not write the +launch Secret is resumed by the next deploy while the container has not +started. The provider writes the Secret with the Sandbox's generation and +an owner reference to that Pod. When the Secret write fails, the provider +keeps the first-Pod binding, because the write may have succeeded despite +the error; the retry reads the Secret again and verifies it. A container +that ever started without its Secret is not resumed (`credentials_missing`). +The provider stops resuming once the Pod is 1020s old (the 1200s bind +timeout, less the 120s deploy deadline and 60s for clock skew), so the +reaper cannot tombstone the session while a deploy writes its credentials +or waits for startup (`binding_expired`). + +**Launch fingerprint compatibility.** Every launch Secret carries +`BUZZ_CHECKPOINT_RESTORE_KEY`, so the key is part of every launch +fingerprint. A provider of this version also accepts, on reuse only, the +fingerprint that an older provider recorded without that key. The +acceptance is one-way: an older provider refuses a Sandbox that this +version created, with `config_differs`. During a rollout with mixed desktop +versions, update every desktop that deploys to the same threads. The legacy +fingerprint will be removed once Sandboxes created before the restore key +have ended (30-day tombstone retention). + ### Sandbox error codes A Sandbox deploy refusal carries a stable `code` next to its `error` text. @@ -1539,7 +1559,9 @@ or reused, and a new refusal gets a new code. They are defined in | `identity_mismatch` | The Sandbox belongs to a different agent, owner, or thread, or lacks the management marker or binding version `2`. | none (use a new thread) | | `credentials_mismatch` | The launch Secret is bound to a different Pod or agent. | explicit recovery | | `relay_differs` | The session was launched against a different relay. | new thread or explicit recovery | -| `binding_incomplete` | The session is bound to its first Pod, but its launch Secret was not written. | explicit recovery | +| `binding_incomplete` | The session is bound to its first Pod, but its launch Secret was not written within this deploy's deadline. | retry | +| `binding_expired` | As `binding_incomplete`, but the Pod is too old to resume (§Deploy repair). | explicit recovery once the session has ended | +| `credentials_missing` | The bound container started without its launch Secret. | explicit recovery | | `pod_lost` | The Pod the session is bound to is gone. | explicit recovery | | `pod_replaced` | The controller replaced the bound Pod. | explicit recovery | | `pod_ended` | The bound Pod finished, is deleting, or its process exited during startup. | explicit recovery |