From 0c3e109c04b7320bebb6986dc14610b4a905ffd0 Mon Sep 17 00:00:00 2001 From: Tee8z Date: Tue, 20 Oct 2026 15:33:22 -0400 Subject: [PATCH] feat(agents): resume interrupted Sandbox credential binding A deploy that bound the Sandbox to its first Pod but never wrote the launch Secret used to strand the session. The next deploy now finishes the binding while the container has not started, with the Sandbox's own generation and an owner reference to that Pod. When the Secret write fails, the deploy keeps the first-Pod binding, because the write may have succeeded despite the error; the retry re-reads and verifies the Secret. A started session without credentials still requires explicit recovery (credentials_missing), and a deploy stops resuming before the reaper's bind timeout can end the session under it (binding_expired). Every launch Secret now carries BUZZ_CHECKPOINT_RESTORE_KEY (empty unless a recovered session restores a checkpoint). The value comes from the Sandbox, never the request, so a resumed binding keeps the restore its Sandbox was created with. The key is part of every launch fingerprint; Sandboxes created before it existed are still reused. That acceptance is one-way: an older provider refuses a Sandbox created by this version as a configuration change. Signed-off-by: Tee8z --- crates/buzz-backend-kubernetes/src/sandbox.rs | 200 +++++++++--- .../src/sandbox_fake.rs | 37 ++- .../src/sandbox_tests.rs | 294 +++++++++++++++++- crates/buzz-backend-kubernetes/src/wire.rs | 7 +- docs/remote-agents.md | 36 ++- 5 files changed, 507 insertions(+), 67 deletions(-) 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 |