diff --git a/Cargo.lock b/Cargo.lock index edc940ea68..f60974e0f8 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7025,6 +7025,7 @@ version = "0.3.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "96374855068f47402c3121c6eed88d29cb1de8f3ab27090e273e420bdabcf050" dependencies = [ + "futures", "parking_lot", ] diff --git a/crates/openshell-driver-docker/Cargo.toml b/crates/openshell-driver-docker/Cargo.toml index 7a5fa3fe3d..0b03fd3880 100644 --- a/crates/openshell-driver-docker/Cargo.toml +++ b/crates/openshell-driver-docker/Cargo.toml @@ -45,7 +45,7 @@ opentelemetry = { workspace = true } opentelemetry_sdk = { workspace = true, features = ["testing"] } prost-types = { workspace = true } tar = "0.4" -temp-env = "0.3" +temp-env = { version = "0.3", features = ["async_closure"] } tempfile = "3" tracing-subscriber = { workspace = true } diff --git a/crates/openshell-driver-docker/src/lib.rs b/crates/openshell-driver-docker/src/lib.rs index f19560ef97..ce3d9eed6e 100644 --- a/crates/openshell-driver-docker/src/lib.rs +++ b/crates/openshell-driver-docker/src/lib.rs @@ -1078,6 +1078,11 @@ impl DockerComputeDriver { } } } + // Container gone and no in-memory record survived (gateway + // restarted after an out-of-band `docker rm`). DeleteSandbox is + // the only thing that ever reclaims the token file, so reclaim it + // here too. + cleanup_sandbox_token_file_for_delete(sandbox_id, None, &self.config); return Ok(false); }; let Some(target) = summary_container_target(&container) else { diff --git a/crates/openshell-driver-docker/src/tests.rs b/crates/openshell-driver-docker/src/tests.rs index 98e7cd1c37..54ac942774 100644 --- a/crates/openshell-driver-docker/src/tests.rs +++ b/crates/openshell-driver-docker/src/tests.rs @@ -184,6 +184,29 @@ fn request_with_traceparent(message: T) -> Request { request } +async fn fake_docker_with_no_containers() -> (String, JoinHandle<()>) { + use tokio::io::{AsyncReadExt, AsyncWriteExt}; + + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server = tokio::spawn(async move { + while let Ok((mut stream, _)) = listener.accept().await { + openshell_core::net::set_tcp_nodelay_best_effort(&stream); + let mut scratch = [0_u8; 4096_usize]; + let _ = stream.read(&mut scratch).await; + let _ = stream + .write_all( + b"HTTP/1.1 200 OK\r\n\ + Content-Type: application/json\r\n\ + Content-Length: 2\r\n\r\n[]", + ) + .await; + let _ = stream.flush().await; + } + }); + (format!("http://{address}"), server) +} + async fn standalone_traced_client() -> ( TestDriverClient, tokio::sync::oneshot::Sender<()>, @@ -3327,3 +3350,77 @@ fn docker_oom_kill_stays_terminal_despite_137() { apply_docker_exit_classification(&mut sandbox, &state); assert_eq!(ready_reason(&sandbox), CONDITION_EXITED); } + +#[tokio::test] +async fn delete_sandbox_reclaims_token_file_when_container_and_pending_are_gone() { + let state_dir = tempfile::tempdir().unwrap(); + let (endpoint, server) = fake_docker_with_no_containers().await; + + temp_env::async_with_vars([("XDG_STATE_HOME", Some(state_dir.path()))], async { + let config = runtime_config(); + let mut driver = test_driver_with_config(config.clone()); + driver.docker = Arc::new( + Docker::connect_with_http(&endpoint, 5, bollard::API_DEFAULT_VERSION).unwrap(), + ); + + // Arrange the leak: token on disk, container gone, `pending` empty. + let token = openshell_core::driver_utils::sandbox_token_path( + "docker-sandbox-tokens", + Some(&config.sandbox_namespace), + "sandbox-1", + ) + .unwrap(); + + fs::create_dir_all(token.parent().unwrap()).unwrap(); + fs::write(&token, "jwt\n").unwrap(); + + let deleted = driver.delete_sandbox_inner("sandbox-1", "").await.unwrap(); + assert!(!deleted, "nothing was removed, must not claim a deletion"); + assert!(!token.exists(), "token file must be reclaimed"); + }) + .await; + + server.abort(); +} + +#[tokio::test] +async fn delete_sandbox_by_name_only_leaves_the_namespace_directory_alone() { + // `DeleteSandbox` accepts a name without an id. With no id there is no + // token path to derive, so the cleanup must be a no-op: deriving a path + // from an empty id yields `/sandbox.jwt`, whose parent is the + // shared namespace directory. + let state_dir = tempfile::tempdir().unwrap(); + let (endpoint, server) = fake_docker_with_no_containers().await; + + temp_env::async_with_vars([("XDG_STATE_HOME", Some(state_dir.path()))], async { + let config = runtime_config(); + let mut driver = test_driver_with_config(config.clone()); + driver.docker = Arc::new( + Docker::connect_with_http(&endpoint, 5, bollard::API_DEFAULT_VERSION).unwrap(), + ); + + let namespace_dir = openshell_core::driver_utils::sandbox_token_path( + "docker-sandbox-tokens", + Some(&config.sandbox_namespace), + "sandbox-1", + ) + .unwrap() + .parent() + .and_then(Path::parent) + .unwrap() + .to_path_buf(); + fs::create_dir_all(&namespace_dir).unwrap(); + + let deleted = driver.delete_sandbox_inner("", "sandbox-1").await.unwrap(); + + assert!(!deleted, "nothing was removed, must not claim a deletion"); + assert!( + namespace_dir.is_dir(), + "namespace directory must survive a name-only delete: {}", + namespace_dir.display() + ); + }) + .await; + + server.abort(); +}