From 896ddd3c0e23516881060ccf780628b35a130fd5 Mon Sep 17 00:00:00 2001 From: chesedo Date: Tue, 21 Nov 2023 14:39:48 +0000 Subject: [PATCH 01/11] tests: rename delete_project to stop_project since that is more accurate --- gateway/src/api/latest.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 917c76e82..72bccc044 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1140,7 +1140,7 @@ pub mod tests { .unwrap() }; - let delete_project = |project: &str| { + let stop_project = |project: &str| { Request::builder() .method("DELETE") .uri(format!("/projects/{project}")) @@ -1197,7 +1197,7 @@ pub mod tests { .unwrap(); router - .call(delete_project("matrix").with_header(&authorization)) + .call(stop_project("matrix").with_header(&authorization)) .map_ok(|resp| { assert_eq!(resp.status(), StatusCode::OK); }) @@ -1223,7 +1223,7 @@ pub mod tests { .unwrap(); router - .call(delete_project("reloaded").with_header(&authorization)) + .call(stop_project("reloaded").with_header(&authorization)) .map_ok(|resp| { assert_eq!(resp.status(), StatusCode::NOT_FOUND); }) From 184b2c05f7990c390807d0973b7cc2f1c18fd1e2 Mon Sep 17 00:00:00 2001 From: chesedo Date: Tue, 21 Nov 2023 16:00:14 +0000 Subject: [PATCH 02/11] tests: deleting a running project with nothing in it --- gateway/src/api/latest.rs | 112 ++++++++++++++++++++++++++++++++++++++ gateway/src/args.rs | 3 + gateway/src/lib.rs | 24 ++++++-- gateway/src/project.rs | 4 +- gateway/src/service.rs | 12 ++++ 5 files changed, 150 insertions(+), 5 deletions(-) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 72bccc044..458d5cb1d 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1109,6 +1109,7 @@ pub mod tests { use super::*; use crate::service::GatewayService; use crate::tests::{RequestBuilderExt, World}; + use crate::worker::Worker; #[tokio::test] async fn api_create_get_delete_projects() -> anyhow::Result<()> { @@ -1364,6 +1365,117 @@ pub mod tests { Ok(()) } + macro_rules! timed_loop { + (wait: $wait:literal$(, max: $max:literal)?, $block:block) => {{ + #[allow(unused_mut)] + #[allow(unused_variables)] + let mut tries = 0; + loop { + $block + tries += 1; + $(if tries > $max { + panic!("timed out in the loop"); + })? + ::tokio::time::sleep(::std::time::Duration::from_secs($wait)).await; + } + }}; + } + + #[tokio::test] + async fn api_delete_project_that_is_ready() -> anyhow::Result<()> { + let world = World::new().await; + let service = Arc::new(GatewayService::init(world.args(), world.pool(), "".into()).await); + let worker = Worker::new(); + + let (sender, mut receiver) = channel(256); + tokio::spawn({ + let worker_sender = worker.sender(); + async move { + while let Some(work) = receiver.recv().await { + // Forward tasks to an actual worker + worker_sender + .send(work) + .await + .map_err(|_| "could not send work") + .unwrap(); + } + } + }); + + let _worker = tokio::spawn(async move { + worker.start().await.unwrap(); + }); + + // Allow the spawns to start + tokio::time::sleep(Duration::from_secs(1)).await; + + let mut router = ApiBuilder::new() + .with_service(Arc::clone(&service)) + .with_sender(sender) + .with_default_routes() + .with_auth_service(world.context().auth_uri) + .into_router(); + + let neo_key = world.create_user("neo"); + + let authorization = Authorization::bearer(&neo_key).unwrap(); + + // Create a project and put it in the ready state + router + .call( + Request::builder() + .method("POST") + .uri(format!("/projects/matrix")) + .header("Content-Type", "application/json") + .body("{\"idle_minutes\": 3}".into()) + .unwrap() + .with_header(&authorization), + ) + .map_ok(|resp| { + assert_eq!(resp.status(), StatusCode::OK); + }) + .await + .unwrap(); + + timed_loop!(wait: 1, max: 12, { + let resp = router + .call( + Request::get("/projects/matrix") + .with_header(&authorization) + .body(Body::empty()) + .unwrap(), + ) + .await.unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); + let body = to_bytes(resp.into_body()).await.unwrap(); + let project: project::Response = serde_json::from_slice(&body).unwrap(); + + if project.state == project::State::Ready { + break; + } + + tokio::time::sleep(Duration::from_secs(1)).await; + }); + + router + .call( + Request::builder() + .method("DELETE") + .uri(format!("/projects/matrix/delete")) + .body(Body::empty()) + .unwrap() + .with_header(&authorization), + ) + .map_ok(|resp| { + assert_eq!(resp.status(), StatusCode::OK); + }) + .await + .unwrap(); + + Ok(()) + } + #[tokio::test(flavor = "multi_thread")] async fn status() { let world = World::new().await; diff --git a/gateway/src/args.rs b/gateway/src/args.rs index abfaa3474..4448a3108 100644 --- a/gateway/src/args.rs +++ b/gateway/src/args.rs @@ -74,4 +74,7 @@ pub struct ContextArgs { /// Api key for the user that has rights to start deploys #[arg(long, default_value = "gateway4deployes")] pub deploys_api_key: String, + + /// Allow tests to set some extra /etc/hosts + pub extra_hosts: Vec, } diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 322875c0e..41463d68e 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -506,11 +506,11 @@ pub mod tests { let control: i16 = Uniform::from(9000..10000).sample(&mut rand::thread_rng()); let user = control + 1; let bouncer = user + 1; - let auth = bouncer + 1; + let auth_port = bouncer + 1; let control = format!("127.0.0.1:{control}").parse().unwrap(); let user = format!("127.0.0.1:{user}").parse().unwrap(); let bouncer = format!("127.0.0.1:{bouncer}").parse().unwrap(); - let auth: SocketAddr = format!("127.0.0.1:{auth}").parse().unwrap(); + let auth: SocketAddr = format!("0.0.0.0:{auth_port}").parse().unwrap(); let auth_uri: Uri = format!("http://{auth}").parse().unwrap(); let auth_service = AuthService::new(auth); @@ -547,10 +547,26 @@ pub mod tests { prefix, provisioner_host, builder_host, - auth_uri: auth_uri.clone(), + // The started containers need to reach auth on the host. + // For this to work, the firewall should not be blocking traffic on the `SHUTTLE_TEST_NETWORK` interface. + // The following command can be used on NixOs to allow traffic on the interface. + // ``` + // sudo iptables -I nixos-fw -i -j nixos-fw-accept + // ``` + // + // Something like this should work on other systems. + // ``` + // sudo iptables -I INPUT -i -j ACCEPT + // ``` + auth_uri: format!("http://host.docker.internal:{auth_port}") + .parse() + .unwrap(), network_name, proxy_fqdn: FQDN::from_str("test.shuttleapp.rs").unwrap(), deploys_api_key: "gateway".to_string(), + + // Allow access to the auth on the host + extra_hosts: vec!["host.docker.internal:host-gateway".to_string()], }, }; @@ -609,7 +625,7 @@ pub mod tests { .lock() .unwrap() .users - .insert(user.to_string(), vec![Scope::Project, Scope::ProjectWrite]); + .insert(user.to_string(), AccountTier::Basic.into()); user.to_string() } diff --git a/gateway/src/project.rs b/gateway/src/project.rs index 1f29d1071..98465597f 100644 --- a/gateway/src/project.rs +++ b/gateway/src/project.rs @@ -792,6 +792,7 @@ impl ProjectCreating { builder_host, auth_uri, fqdn: public, + extra_hosts, .. } = ctx.container_settings(); @@ -863,7 +864,8 @@ impl ProjectCreating { "MemoryReservation": 4295000000i64, // 4 GiB soft limit, applied if host is low on memory // https://docs.docker.com/config/containers/resource_constraints/#cpu "CpuPeriod": 100000i64, - "CpuQuota": 400000i64 + "CpuQuota": 400000i64, + "ExtraHosts": extra_hosts, }); debug!( diff --git a/gateway/src/service.rs b/gateway/src/service.rs index 91742d369..d46e69990 100644 --- a/gateway/src/service.rs +++ b/gateway/src/service.rs @@ -65,6 +65,7 @@ pub struct ContainerSettingsBuilder { auth_uri: Option, network_name: Option, fqdn: Option, + extra_hosts: Option>, } impl Default for ContainerSettingsBuilder { @@ -83,6 +84,7 @@ impl ContainerSettingsBuilder { auth_uri: None, network_name: None, fqdn: None, + extra_hosts: None, } } @@ -95,6 +97,7 @@ impl ContainerSettingsBuilder { auth_uri, image, proxy_fqdn, + extra_hosts, .. } = args; self.prefix(prefix) @@ -104,6 +107,7 @@ impl ContainerSettingsBuilder { .auth_uri(auth_uri) .network_name(network_name) .fqdn(proxy_fqdn) + .extra_hosts(extra_hosts) .build() .await } @@ -143,12 +147,18 @@ impl ContainerSettingsBuilder { self } + pub fn extra_hosts(mut self, extra_hosts: &Vec) -> Self { + self.extra_hosts = Some(extra_hosts.iter().map(ToString::to_string).collect()); + self + } + pub async fn build(mut self) -> ContainerSettings { let prefix = self.prefix.take().unwrap(); let image = self.image.take().unwrap(); let provisioner_host = self.provisioner.take().unwrap(); let builder_host = self.builder.take().unwrap(); let auth_uri = self.auth_uri.take().unwrap(); + let extra_hosts = self.extra_hosts.take().unwrap(); let network_name = self.network_name.take().unwrap(); let fqdn = self.fqdn.take().unwrap(); @@ -161,6 +171,7 @@ impl ContainerSettingsBuilder { auth_uri, network_name, fqdn, + extra_hosts, } } } @@ -174,6 +185,7 @@ pub struct ContainerSettings { pub auth_uri: String, pub network_name: String, pub fqdn: String, + pub extra_hosts: Vec, } impl ContainerSettings { From 2155d05b7d20fd0c20065ba13ea392f808441f98 Mon Sep 17 00:00:00 2001 From: chesedo Date: Tue, 21 Nov 2023 17:39:31 +0000 Subject: [PATCH 03/11] refactor: remove unused hyper object --- gateway/src/lib.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 41463d68e..51d7d2ba6 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -489,7 +489,6 @@ pub mod tests { pub struct WorldContext { pub docker: Docker, pub container_settings: ContainerSettings, - pub hyper: HyperClient, pub auth_uri: Uri, } @@ -642,7 +641,6 @@ pub mod tests { WorldContext { docker: self.docker.clone(), container_settings: self.settings.clone(), - hyper: self.hyper.clone(), auth_uri: self.auth_uri.clone(), } } From 918383ca87d078831665abeb0458bedad40209b1 Mon Sep 17 00:00:00 2001 From: chesedo Date: Tue, 21 Nov 2023 17:44:51 +0000 Subject: [PATCH 04/11] refactor: remove unneeded hyper field --- gateway/src/lib.rs | 15 ++++++--------- gateway/src/project.rs | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 51d7d2ba6..fc50ba082 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -478,7 +478,6 @@ pub mod tests { docker: Docker, settings: ContainerSettings, args: StartArgs, - hyper: HyperClient, pool: SqlitePool, acme_client: AcmeClient, auth_service: Arc>, @@ -571,8 +570,6 @@ pub mod tests { let settings = ContainerSettings::builder().from_args(&args.context).await; - let hyper = HyperClient::builder().build(HttpConnector::new()); - let pool = SqlitePool::connect_with( SqliteConnectOptions::from_str("sqlite::memory:") .unwrap() @@ -591,7 +588,6 @@ pub mod tests { docker, settings, args, - hyper, pool, acme_client, auth_service, @@ -607,10 +603,6 @@ pub mod tests { self.pool.clone() } - pub fn client>(&self, addr: A) -> Client { - Client::new(addr).with_hyper_client(self.hyper.clone()) - } - pub fn fqdn(&self) -> FQDN { self.args().proxy_fqdn } @@ -634,6 +626,11 @@ pub mod tests { scopes.push(Scope::Admin) } } + + pub fn client>(addr: A) -> Client { + let hyper = HyperClient::builder().build(HttpConnector::new()); + Client::new(addr).with_hyper_client(hyper) + } } impl World { @@ -730,7 +727,7 @@ pub mod tests { } }); - let api_client = world.client(world.args.control); + let api_client = World::client(world.args.control); let api = ApiBuilder::new() .with_service(Arc::clone(&service)) .with_sender(log_out.clone()) diff --git a/gateway/src/project.rs b/gateway/src/project.rs index 98465597f..1794132be 100644 --- a/gateway/src/project.rs +++ b/gateway/src/project.rs @@ -1988,7 +1988,7 @@ pub mod tests { .unwrap() .unwrap(); - let client = world.client(target_addr); + let client = World::client(target_addr); client .request( From f4d75cc6283ebf0d990e90a2c76de6f4c9d907f2 Mon Sep 17 00:00:00 2001 From: chesedo Date: Tue, 21 Nov 2023 17:59:48 +0000 Subject: [PATCH 05/11] feat: make it easy to recreate a router --- gateway/src/api/latest.rs | 33 +-------------------------------- gateway/src/lib.rs | 34 ++++++++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 32 deletions(-) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 458d5cb1d..4329bdc67 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1109,7 +1109,6 @@ pub mod tests { use super::*; use crate::service::GatewayService; use crate::tests::{RequestBuilderExt, World}; - use crate::worker::Worker; #[tokio::test] async fn api_create_get_delete_projects() -> anyhow::Result<()> { @@ -1384,38 +1383,8 @@ pub mod tests { #[tokio::test] async fn api_delete_project_that_is_ready() -> anyhow::Result<()> { let world = World::new().await; - let service = Arc::new(GatewayService::init(world.args(), world.pool(), "".into()).await); - let worker = Worker::new(); - - let (sender, mut receiver) = channel(256); - tokio::spawn({ - let worker_sender = worker.sender(); - async move { - while let Some(work) = receiver.recv().await { - // Forward tasks to an actual worker - worker_sender - .send(work) - .await - .map_err(|_| "could not send work") - .unwrap(); - } - } - }); - - let _worker = tokio::spawn(async move { - worker.start().await.unwrap(); - }); - - // Allow the spawns to start - tokio::time::sleep(Duration::from_secs(1)).await; - - let mut router = ApiBuilder::new() - .with_service(Arc::clone(&service)) - .with_sender(sender) - .with_default_routes() - .with_auth_service(world.context().auth_uri) - .into_router(); + let mut router = world.router().await; let neo_key = world.create_user("neo"); let authorization = Authorization::bearer(&neo_key).unwrap(); diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index fc50ba082..6c26496d1 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -627,6 +627,40 @@ pub mod tests { } } + pub async fn router(&self) -> Router { + let service = Arc::new(GatewayService::init(self.args(), self.pool(), "".into()).await); + let worker = Worker::new(); + + let (sender, mut receiver) = channel(256); + tokio::spawn({ + let worker_sender = worker.sender(); + async move { + while let Some(work) = receiver.recv().await { + // Forward tasks to an actual worker + worker_sender + .send(work) + .await + .map_err(|_| "could not send work") + .unwrap(); + } + } + }); + + let _worker = tokio::spawn(async move { + worker.start().await.unwrap(); + }); + + // Allow the spawns to start + tokio::time::sleep(Duration::from_secs(1)).await; + + ApiBuilder::new() + .with_service(Arc::clone(&service)) + .with_sender(sender) + .with_default_routes() + .with_auth_service(self.context().auth_uri) + .into_router() + } + pub fn client>(addr: A) -> Client { let hyper = HyperClient::builder().build(HttpConnector::new()); Client::new(addr).with_hyper_client(hyper) From 7bcf3b83e2073b7da0e5e10c807811b6ed8448d5 Mon Sep 17 00:00:00 2001 From: chesedo Date: Tue, 21 Nov 2023 18:02:04 +0000 Subject: [PATCH 06/11] refactor: make it easier to get an authorization bearer --- gateway/src/api/latest.rs | 4 +--- gateway/src/lib.rs | 11 +++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 4329bdc67..242871b50 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1385,9 +1385,7 @@ pub mod tests { let world = World::new().await; let mut router = world.router().await; - let neo_key = world.create_user("neo"); - - let authorization = Authorization::bearer(&neo_key).unwrap(); + let authorization = world.create_authorization_bearer("neo"); // Create a project and put it in the ready state router diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 6c26496d1..301393bed 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -621,6 +621,17 @@ pub mod tests { user.to_string() } + pub fn create_authorization_bearer(&self, user: &str) -> Authorization { + self.auth_service + .lock() + .unwrap() + .users + .insert(user.to_string(), AccountTier::Basic.into()); + + let user_key = user.to_string(); + Authorization::bearer(&user_key).unwrap() + } + pub fn set_super_user(&self, user: &str) { if let Some(scopes) = self.auth_service.lock().unwrap().users.get_mut(user) { scopes.push(Scope::Admin) From 0d1a9799c51f7494677c711411da3a78705054b0 Mon Sep 17 00:00:00 2001 From: chesedo Date: Tue, 21 Nov 2023 18:35:22 +0000 Subject: [PATCH 07/11] refactor: make it easier to create a project for other tests --- gateway/src/api/latest.rs | 55 +---------------------- gateway/src/lib.rs | 91 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 53 deletions(-) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 242871b50..a16cacc02 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1108,7 +1108,7 @@ pub mod tests { use super::*; use crate::service::GatewayService; - use crate::tests::{RequestBuilderExt, World}; + use crate::tests::{RequestBuilderExt, RouterExt, World}; #[tokio::test] async fn api_create_get_delete_projects() -> anyhow::Result<()> { @@ -1364,22 +1364,6 @@ pub mod tests { Ok(()) } - macro_rules! timed_loop { - (wait: $wait:literal$(, max: $max:literal)?, $block:block) => {{ - #[allow(unused_mut)] - #[allow(unused_variables)] - let mut tries = 0; - loop { - $block - tries += 1; - $(if tries > $max { - panic!("timed out in the loop"); - })? - ::tokio::time::sleep(::std::time::Duration::from_secs($wait)).await; - } - }}; - } - #[tokio::test] async fn api_delete_project_that_is_ready() -> anyhow::Result<()> { let world = World::new().await; @@ -1388,42 +1372,7 @@ pub mod tests { let authorization = world.create_authorization_bearer("neo"); // Create a project and put it in the ready state - router - .call( - Request::builder() - .method("POST") - .uri(format!("/projects/matrix")) - .header("Content-Type", "application/json") - .body("{\"idle_minutes\": 3}".into()) - .unwrap() - .with_header(&authorization), - ) - .map_ok(|resp| { - assert_eq!(resp.status(), StatusCode::OK); - }) - .await - .unwrap(); - - timed_loop!(wait: 1, max: 12, { - let resp = router - .call( - Request::get("/projects/matrix") - .with_header(&authorization) - .body(Body::empty()) - .unwrap(), - ) - .await.unwrap(); - - assert_eq!(resp.status(), StatusCode::OK); - let body = to_bytes(resp.into_body()).await.unwrap(); - let project: project::Response = serde_json::from_slice(&body).unwrap(); - - if project.state == project::State::Ready { - break; - } - - tokio::time::sleep(Duration::from_secs(1)).await; - }); + let _project = router.create_project(&authorization, "matrix").await; router .call( diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 301393bed..04ed25226 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -280,6 +280,8 @@ pub mod tests { use sqlx::sqlite::SqliteConnectOptions; use sqlx::SqlitePool; use tokio::sync::mpsc::channel; + use tokio::time::sleep; + use tower::Service; use crate::acme::AcmeClient; use crate::api::latest::ApiBuilder; @@ -752,6 +754,95 @@ pub mod tests { } } + #[async_trait] + pub trait RouterExt { + /// Create a project and put it in the ready state + async fn create_project( + &mut self, + authorization: &Authorization, + project_name: &str, + ) -> TestProject; + } + + /// Helper struct to wrap a bunch of commands to run against a test project + pub struct TestProject<'a> { + router: &'a mut Router, + authorization: Authorization, + project_name: String, + } + + impl<'a> TestProject<'a> { + /// Wait a few seconds for the project to enter the desired state + pub async fn wait_for_state(&mut self, state: project::State) { + let mut tries = 0; + let project_name = &self.project_name; + + loop { + let resp = self + .router + .call( + Request::get(format!("/projects/{project_name}")) + .with_header(&self.authorization) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(resp.status(), StatusCode::OK); + let body = hyper::body::to_bytes(resp.into_body()).await.unwrap(); + let project: project::Response = serde_json::from_slice(&body).unwrap(); + + if project.state == state { + break; + } + + tries += 1; + if tries > 12 { + panic!("timed out waiting for state {state}"); + } + + sleep(Duration::from_secs(1)).await; + } + } + } + + #[async_trait] + impl RouterExt for Router { + async fn create_project( + &mut self, + authorization: &Authorization, + project_name: &str, + ) -> TestProject { + let authorization = authorization.clone(); + + self.call( + Request::builder() + .method("POST") + .uri(format!("/projects/{project_name}")) + .header("Content-Type", "application/json") + .body("{\"idle_minutes\": 3}".into()) + .unwrap() + .with_header(&authorization), + ) + .map_ok(|resp| { + assert_eq!(resp.status(), StatusCode::OK); + }) + .await + .unwrap(); + + let mut this = TestProject { + authorization, + project_name: project_name.to_string(), + router: self, + }; + + this.wait_for_state(project::State::Ready).await; + + this + } + } + #[tokio::test] async fn end_to_end() { let world = World::new().await; From 9f1fd298059dedbe42888dbf118b35f842e58007 Mon Sep 17 00:00:00 2001 From: chesedo Date: Wed, 22 Nov 2023 12:04:51 +0000 Subject: [PATCH 08/11] bug: deleting a project that is destroyed --- gateway/src/api/latest.rs | 54 +++++++++++++++++++++++++++++++++++++-- gateway/src/lib.rs | 50 +++++++++++++++++++++++++++++++++--- gateway/src/project.rs | 5 +++- gateway/src/task.rs | 16 ++++++++++++ 4 files changed, 118 insertions(+), 7 deletions(-) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index a16cacc02..a515766cd 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -37,6 +37,7 @@ use tokio::sync::{Mutex, MutexGuard}; use tower::ServiceBuilder; use tracing::{field, instrument, trace}; use ttl_cache::TtlCache; +use ulid::Ulid; use utoipa::openapi::security::{ApiKey, ApiKeyValue, SecurityScheme}; use utoipa::IntoParams; use utoipa::{Modify, OpenApi}; @@ -319,6 +320,24 @@ async fn delete_project( req: Request, ) -> Result, Error> { let project_name = scoped_user.scope.clone(); + let project = state.service.find_project(&project_name).await?; + let project_id = + Ulid::from_string(&project.project_id).expect("stored project id to be a valid ULID"); + + // Try to startup a destroyed project first + if project.state.is_destroyed() { + let handle = state + .service + .new_task() + .project(project_name.clone()) + .and_then(task::restart(project_id)) + .send(&state.sender) + .await?; + + // Wait for the project to be ready + handle.await; + } + let service = state.service.clone(); let sender = state.sender.clone(); @@ -1371,8 +1390,7 @@ pub mod tests { let mut router = world.router().await; let authorization = world.create_authorization_bearer("neo"); - // Create a project and put it in the ready state - let _project = router.create_project(&authorization, "matrix").await; + let mut project = router.create_project(&authorization, "matrix").await; router .call( @@ -1389,6 +1407,38 @@ pub mod tests { .await .unwrap(); + assert!(project.is_missing().await); + + Ok(()) + } + + #[tokio::test] + async fn api_delete_project_that_is_destroyed() -> anyhow::Result<()> { + let world = World::new().await; + + let mut router = world.router().await; + let authorization = world.create_authorization_bearer("neo"); + + let mut project = router.create_project(&authorization, "matrix").await; + project.destroy_project().await; + + router + .call( + Request::builder() + .method("DELETE") + .uri(format!("/projects/matrix/delete")) + .body(Body::empty()) + .unwrap() + .with_header(&authorization), + ) + .map_ok(|resp| { + assert_eq!(resp.status(), StatusCode::OK); + }) + .await + .unwrap(); + + assert!(project.is_missing().await); + Ok(()) } diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index 04ed25226..c4ef21981 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -765,13 +765,13 @@ pub mod tests { } /// Helper struct to wrap a bunch of commands to run against a test project - pub struct TestProject<'a> { - router: &'a mut Router, + pub struct TestProject { + router: Router, authorization: Authorization, project_name: String, } - impl<'a> TestProject<'a> { + impl TestProject { /// Wait a few seconds for the project to enter the desired state pub async fn wait_for_state(&mut self, state: project::State) { let mut tries = 0; @@ -805,6 +805,48 @@ pub mod tests { sleep(Duration::from_secs(1)).await; } } + + pub async fn is_missing(&mut self) -> bool { + let project_name = &self.project_name; + + let resp = self + .router + .call( + Request::get(format!("/projects/{project_name}")) + .with_header(&self.authorization) + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + resp.status() == StatusCode::NOT_FOUND + } + + pub async fn destroy_project(&mut self) { + let TestProject { + router, + authorization, + project_name, + } = self; + + router + .call( + Request::builder() + .method("DELETE") + .uri(format!("/projects/{project_name}")) + .body(Body::empty()) + .unwrap() + .with_header(authorization), + ) + .map_ok(|resp| { + assert_eq!(resp.status(), StatusCode::OK); + }) + .await + .unwrap(); + + self.wait_for_state(project::State::Destroyed).await; + } } #[async_trait] @@ -834,7 +876,7 @@ pub mod tests { let mut this = TestProject { authorization, project_name: project_name.to_string(), - router: self, + router: self.clone(), }; this.wait_for_state(project::State::Ready).await; diff --git a/gateway/src/project.rs b/gateway/src/project.rs index 1794132be..effe7c835 100644 --- a/gateway/src/project.rs +++ b/gateway/src/project.rs @@ -372,7 +372,10 @@ impl Project { | Self::Stopping(ProjectStopping { container, .. }) | Self::Stopped(ProjectStopped { container, .. }) | Self::Rebooting(ProjectRebooting { container, .. }) - | Self::Destroying(ProjectDestroying { container }) => Some(container.clone()), + | Self::Destroying(ProjectDestroying { container }) + | Self::Destroyed(ProjectDestroyed { + destroyed: Some(container), + }) => Some(container.clone()), Self::Errored(ProjectError { ctx: Some(ctx), .. }) => ctx.container(), Self::Errored(_) | Self::Creating(_) | Self::Destroyed(_) | Self::Deleted => None, } diff --git a/gateway/src/task.rs b/gateway/src/task.rs index 5c7785bb6..aeae7f43e 100644 --- a/gateway/src/task.rs +++ b/gateway/src/task.rs @@ -10,6 +10,7 @@ use tokio::sync::mpsc::Sender; use tokio::sync::oneshot; use tokio::time::{sleep, timeout}; use tracing::{error, trace, trace_span, warn}; +use ulid::Ulid; use uuid::Uuid; use crate::project::*; @@ -128,6 +129,21 @@ pub fn start() -> impl Task { }) } +/// Will force restart a project no matter the state it is in +pub fn restart(project_id: Ulid) -> impl Task { + run(move |ctx| async move { + let state = ctx + .state + .container() + .and_then(|container| ProjectCreating::from_container(container, 0).ok()) + .unwrap_or_else(|| { + ProjectCreating::new_with_random_initial_key(ctx.project_name, project_id, 1) + }); + + TaskResult::Done(Project::Creating(state)) + }) +} + pub fn start_idle_deploys() -> impl Task { run(|ctx| async move { match ctx.state { From 6640cd8e72cb659d7d66f2fba396557c78269bb0 Mon Sep 17 00:00:00 2001 From: chesedo Date: Wed, 22 Nov 2023 12:27:32 +0000 Subject: [PATCH 09/11] refactor: missing comments and optimizations --- gateway/src/lib.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index c4ef21981..bfe09de3f 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -623,14 +623,9 @@ pub mod tests { user.to_string() } + /// Create with the given name and return the authorization bearer for the user pub fn create_authorization_bearer(&self, user: &str) -> Authorization { - self.auth_service - .lock() - .unwrap() - .users - .insert(user.to_string(), AccountTier::Basic.into()); - - let user_key = user.to_string(); + let user_key = self.create_user(user); Authorization::bearer(&user_key).unwrap() } @@ -640,6 +635,7 @@ pub mod tests { } } + /// Create a router to make API calls against. Also starts up a worker to create actual Docker containers for all requests pub async fn router(&self) -> Router { let service = Arc::new(GatewayService::init(self.args(), self.pool(), "".into()).await); let worker = Worker::new(); @@ -754,6 +750,7 @@ pub mod tests { } } + /// Make it easy to perform common requests against the router for testing purposes #[async_trait] pub trait RouterExt { /// Create a project and put it in the ready state @@ -806,6 +803,7 @@ pub mod tests { } } + /// Is this project still available - aka has it been deleted pub async fn is_missing(&mut self) -> bool { let project_name = &self.project_name; @@ -823,6 +821,7 @@ pub mod tests { resp.status() == StatusCode::NOT_FOUND } + /// Destroy / stop a project. Like `cargo shuttle project stop` pub async fn destroy_project(&mut self) { let TestProject { router, From 4f2711318708bc7115170be01b708edd399f7505 Mon Sep 17 00:00:00 2001 From: chesedo Date: Wed, 22 Nov 2023 12:35:37 +0000 Subject: [PATCH 10/11] refactor: clippy suggestions --- gateway/src/api/latest.rs | 4 ++-- gateway/src/service.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index a515766cd..3b3b6f04d 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1396,7 +1396,7 @@ pub mod tests { .call( Request::builder() .method("DELETE") - .uri(format!("/projects/matrix/delete")) + .uri("/projects/matrix/delete") .body(Body::empty()) .unwrap() .with_header(&authorization), @@ -1426,7 +1426,7 @@ pub mod tests { .call( Request::builder() .method("DELETE") - .uri(format!("/projects/matrix/delete")) + .uri("/projects/matrix/delete") .body(Body::empty()) .unwrap() .with_header(&authorization), diff --git a/gateway/src/service.rs b/gateway/src/service.rs index d46e69990..9109a8d63 100644 --- a/gateway/src/service.rs +++ b/gateway/src/service.rs @@ -147,7 +147,7 @@ impl ContainerSettingsBuilder { self } - pub fn extra_hosts(mut self, extra_hosts: &Vec) -> Self { + pub fn extra_hosts(mut self, extra_hosts: &[S]) -> Self { self.extra_hosts = Some(extra_hosts.iter().map(ToString::to_string).collect()); self } From 9351114de7cb42c2edf3d8eece9155b699288283 Mon Sep 17 00:00:00 2001 From: chesedo Date: Wed, 22 Nov 2023 15:27:38 +0000 Subject: [PATCH 11/11] refactor: reduce test setup code --- Cargo.lock | 1 + gateway/Cargo.toml | 1 + gateway/src/api/latest.rs | 58 ++++++--------------------------------- gateway/src/lib.rs | 40 +++++++++++++++++++++++++-- 4 files changed, 49 insertions(+), 51 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 27ea2aefb..6d983032e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -5869,6 +5869,7 @@ dependencies = [ "sqlx", "strum 0.25.0", "tempfile", + "test-context", "tokio", "tonic 0.10.2", "tower", diff --git a/gateway/Cargo.toml b/gateway/Cargo.toml index d463233f7..9364c90da 100644 --- a/gateway/Cargo.toml +++ b/gateway/Cargo.toml @@ -65,3 +65,4 @@ portpicker = { workspace = true } ring = { workspace = true } snailquote = "0.3.1" tempfile = { workspace = true } +test-context = "0.1.4" diff --git a/gateway/src/api/latest.rs b/gateway/src/api/latest.rs index 3b3b6f04d..d052cb4bd 100644 --- a/gateway/src/api/latest.rs +++ b/gateway/src/api/latest.rs @@ -1117,17 +1117,19 @@ pub mod tests { use axum::headers::Authorization; use axum::http::Request; use futures::TryFutureExt; + use http::Method; use hyper::body::to_bytes; use hyper::StatusCode; use serde_json::Value; use shuttle_common::constants::limits::{MAX_PROJECTS_DEFAULT, MAX_PROJECTS_EXTRA}; + use test_context::test_context; use tokio::sync::mpsc::channel; use tokio::sync::oneshot; use tower::Service; use super::*; use crate::service::GatewayService; - use crate::tests::{RequestBuilderExt, RouterExt, World}; + use crate::tests::{RequestBuilderExt, TestProject, World}; #[tokio::test] async fn api_create_get_delete_projects() -> anyhow::Result<()> { @@ -1383,61 +1385,19 @@ pub mod tests { Ok(()) } + #[test_context(TestProject)] #[tokio::test] - async fn api_delete_project_that_is_ready() -> anyhow::Result<()> { - let world = World::new().await; - - let mut router = world.router().await; - let authorization = world.create_authorization_bearer("neo"); - - let mut project = router.create_project(&authorization, "matrix").await; - - router - .call( - Request::builder() - .method("DELETE") - .uri("/projects/matrix/delete") - .body(Body::empty()) - .unwrap() - .with_header(&authorization), - ) - .map_ok(|resp| { - assert_eq!(resp.status(), StatusCode::OK); - }) - .await - .unwrap(); - - assert!(project.is_missing().await); + async fn api_delete_project_that_is_ready(project: &mut TestProject) -> anyhow::Result<()> { + project.router_call(Method::DELETE, "/delete").await; Ok(()) } + #[test_context(TestProject)] #[tokio::test] - async fn api_delete_project_that_is_destroyed() -> anyhow::Result<()> { - let world = World::new().await; - - let mut router = world.router().await; - let authorization = world.create_authorization_bearer("neo"); - - let mut project = router.create_project(&authorization, "matrix").await; + async fn api_delete_project_that_is_destroyed(project: &mut TestProject) -> anyhow::Result<()> { project.destroy_project().await; - - router - .call( - Request::builder() - .method("DELETE") - .uri("/projects/matrix/delete") - .body(Body::empty()) - .unwrap() - .with_header(&authorization), - ) - .map_ok(|resp| { - assert_eq!(resp.status(), StatusCode::OK); - }) - .await - .unwrap(); - - assert!(project.is_missing().await); + project.router_call(Method::DELETE, "/delete").await; Ok(()) } diff --git a/gateway/src/lib.rs b/gateway/src/lib.rs index bfe09de3f..396a613e8 100644 --- a/gateway/src/lib.rs +++ b/gateway/src/lib.rs @@ -267,6 +267,7 @@ pub mod tests { use bollard::Docker; use fqdn::FQDN; use futures::prelude::*; + use http::Method; use hyper::client::HttpConnector; use hyper::http::uri::Scheme; use hyper::http::Uri; @@ -279,6 +280,7 @@ pub mod tests { use shuttle_common::models::project; use sqlx::sqlite::SqliteConnectOptions; use sqlx::SqlitePool; + use test_context::AsyncTestContext; use tokio::sync::mpsc::channel; use tokio::time::sleep; use tower::Service; @@ -674,9 +676,7 @@ pub mod tests { let hyper = HyperClient::builder().build(HttpConnector::new()); Client::new(addr).with_hyper_client(hyper) } - } - impl World { pub fn context(&self) -> WorldContext { WorldContext { docker: self.docker.clone(), @@ -846,6 +846,42 @@ pub mod tests { self.wait_for_state(project::State::Destroyed).await; } + + /// Send a request to the router for this project + pub async fn router_call(&mut self, method: Method, sub_path: &str) { + let project_name = &self.project_name; + + self.router + .call( + Request::builder() + .method(method) + .uri(format!("/projects/{project_name}{sub_path}")) + .body(Body::empty()) + .unwrap() + .with_header(&self.authorization), + ) + .map_ok(|resp| { + assert_eq!(resp.status(), StatusCode::OK); + }) + .await + .unwrap(); + } + } + + #[async_trait] + impl AsyncTestContext for TestProject { + async fn setup() -> Self { + let world = World::new().await; + + let mut router = world.router().await; + let authorization = world.create_authorization_bearer("neo"); + + router.create_project(&authorization, "matrix").await + } + + async fn teardown(mut self) { + assert!(self.is_missing().await, "test left a dangling project"); + } } #[async_trait]