diff --git a/nexus/db-queries/src/db/datastore/vpc.rs b/nexus/db-queries/src/db/datastore/vpc.rs index 7e1cf4a049b..4bc246e9444 100644 --- a/nexus/db-queries/src/db/datastore/vpc.rs +++ b/nexus/db-queries/src/db/datastore/vpc.rs @@ -1653,38 +1653,67 @@ impl DataStore { #[derive(Debug)] enum DeleteError { - IpPoolsExist, - IpAddressesExist, + RoutesExist, } self.transaction_retry_wrapper("vpc_delete_internet_gateway_no_cascade") .transaction(&conn, |conn| { let err = err.clone(); async move { + // Load gateway so we can get the vpc + use nexus_db_schema::schema::internet_gateway::dsl as igw; + let igw_info = igw::internet_gateway + .filter(igw::time_deleted.is_null()) + .filter(igw::id.eq(authz_igw.id())) + .select(InternetGateway::as_select()) + .first_async(&conn) + .await?; + + // Get the vpc's routers + use nexus_db_schema::schema::vpc_router::dsl as vr; + let vpc_routers = vr::vpc_router + .filter(vr::time_deleted.is_null()) + .filter(vr::vpc_id.eq(igw_info.vpc_id)) + .select(vr::id) + .load_async::(&conn) + .await?; + + // Check route associations + use nexus_db_schema::schema::router_route::dsl as rr; + let has_routes = rr::router_route + .filter(rr::time_deleted.is_null()) + .filter(rr::vpc_router_id.eq_any(vpc_routers)) + .filter(rr::target.eq(format!("inetgw:{}", igw_info.name()))) + .select(rr::id) + .first_async::(&conn) + .await + .optional()? + .is_some(); + if has_routes { + return Err(err.bail(DeleteError::RoutesExist)); + } + // Delete ip pool associations use nexus_db_schema::schema::internet_gateway_ip_pool::dsl as pool; - let count = pool::internet_gateway_ip_pool + let now = Utc::now(); + diesel::update(pool::internet_gateway_ip_pool) .filter(pool::time_deleted.is_null()) .filter(pool::internet_gateway_id.eq(authz_igw.id())) - .count() - .first_async::(&conn) + .set(pool::time_deleted.eq(now)) + .execute_async(&conn) .await?; - if count > 0 { - return Err(err.bail(DeleteError::IpPoolsExist)); - } // Delete ip address associations use nexus_db_schema::schema::internet_gateway_ip_address::dsl as addr; - let count = addr::internet_gateway_ip_address + let now = Utc::now(); + diesel::update(addr::internet_gateway_ip_address) .filter(addr::time_deleted.is_null()) .filter(addr::internet_gateway_id.eq(authz_igw.id())) - .count() - .first_async::(&conn) + .set(addr::time_deleted.eq(now)) + .execute_async(&conn) .await?; - if count > 0 { - return Err(err.bail(DeleteError::IpAddressesExist)); - } + // Delete internet gateway use nexus_db_schema::schema::internet_gateway::dsl; let now = Utc::now(); diesel::update(dsl::internet_gateway) @@ -1701,8 +1730,7 @@ impl DataStore { .map_err(|e| { if let Some(err) = err.take() { match err { - DeleteError::IpPoolsExist => Error::invalid_request("Ip pools referencing this gateway exist. To perform a cascading delete set the cascade option"), - DeleteError::IpAddressesExist => Error::invalid_request("Ip addresses referencing this gateway exist. To perform a cascading delete set the cascade option"), + DeleteError::RoutesExist => Error::invalid_request("Routes referencing this gateway exist. To perform a cascading delete set the cascade option"), } } else { public_error_from_diesel(e, ErrorHandler::Server) diff --git a/nexus/external-api/src/lib.rs b/nexus/external-api/src/lib.rs index d46b906c234..6b9ea678a91 100644 --- a/nexus/external-api/src/lib.rs +++ b/nexus/external-api/src/lib.rs @@ -86,6 +86,7 @@ api_versions!([ // | date-based version should be at the top of the list. // v // (next_yyyy_mm_dd_nn, IDENT), + (2026_08_08_00, INTERNET_GATEWAY_NO_CASCADE_DELETE), (2026_07_31_00, SET_TARGET_RELEASE_UPDATE_RECOVERY_DOCS), (2026_07_28_00, INTERNET_GATEWAY_CASCADE_DOCS), (2026_06_11_00, ADD_SYSTEM_IP_POOL_APIS), diff --git a/nexus/tests/integration_tests/internet_gateway.rs b/nexus/tests/integration_tests/internet_gateway.rs index 333dd74d70e..b9484512c05 100644 --- a/nexus/tests/integration_tests/internet_gateway.rs +++ b/nexus/tests/integration_tests/internet_gateway.rs @@ -4,6 +4,7 @@ use dropshot::{ResultsPage, test_util::ClientTestContext}; use http::{Method, StatusCode}; +use nexus_db_queries::db::DataStore; use nexus_db_queries::db::fixed_data::silo::DEFAULT_SILO; use nexus_test_utils::{ http_testing::{AuthnMode, NexusRequest}, @@ -44,6 +45,7 @@ use omicron_common::{ IdentityMetadataCreateParams, NameOrId, RouteDestination, RouteTarget, }, }; +use uuid::Uuid; type ControlPlaneTestContext = nexus_test_utils::ControlPlaneTestContext; @@ -107,7 +109,8 @@ async fn test_internet_gateway_basic_crud(ctx: &ControlPlaneTestContext) { .await; assert_eq!(igw_pools.len(), 1, "should now have one attached ip pool"); - // ensure we cannot delete the IP gateway without cascading + // ensure we cannot delete the gateway without cascading while a route + // (created in test_setup) still targets it expect_igw_delete_fail(c, PROJECT_NAME, VPC_NAME, IGW_NAME, false).await; // ensure we cannot detach the igw ip pool without cascading @@ -171,7 +174,11 @@ async fn test_internet_gateway_basic_crud(ctx: &ControlPlaneTestContext) { "should now have zero attached ip addresses" ); - // delete internet gateay + // remove the route targeting this gateway so a non-cascading delete is + // permitted (a route referencing an igw blocks a no-cascade delete) + delete_route(c, PROJECT_NAME, VPC_NAME, ROUTER_NAME, ROUTE_NAME).await; + + // delete internet gateway delete_internet_gateway(c, PROJECT_NAME, VPC_NAME, IGW_NAME, false).await; let igws = list_internet_gateways(c, PROJECT_NAME, VPC_NAME).await; assert_eq!(igws.len(), 1, "should now just have default gateway"); @@ -270,6 +277,77 @@ async fn test_internet_gateway_delete_cascade(ctx: &ControlPlaneTestContext) { expect_igw_addresses_not_found(c, PROJECT_NAME, VPC_NAME, IGW_NAME).await; } +// A non-cascading delete must fail when a route still targets the gateway, +// rather than silently leaving a dangling route reference behind. +#[nexus_test] +async fn test_internet_gateway_delete_no_cascade_fails_with_routes( + ctx: &ControlPlaneTestContext, +) { + let c = &ctx.external_client; + test_setup(c).await; + + // test_setup already created a route targeting IGW_NAME; create the gateway + // it points at. Note the gateway has no ip pool / address attachments, so + // the only thing blocking a no-cascade delete is the route. + create_internet_gateway(c, PROJECT_NAME, VPC_NAME, IGW_NAME).await; + + // a route references this gateway, so a non-cascading delete must fail + expect_igw_delete_fail(c, PROJECT_NAME, VPC_NAME, IGW_NAME, false).await; + + // the gateway should still be there + let igws = list_internet_gateways(c, PROJECT_NAME, VPC_NAME).await; + assert_eq!(igws.len(), 2, "gateway should survive the failed delete"); +} + +// A non-cascading delete should succeed when no routes target the gateway, and +// should soft-delete the gateway's ip pool / address attachments as it goes +// rather than leave them dangling against a deleted gateway. +#[nexus_test] +async fn test_internet_gateway_delete_no_cascade_removes_associations( + ctx: &ControlPlaneTestContext, +) { + let c = &ctx.external_client; + let datastore = ctx.server.server_context().nexus.datastore(); + test_setup(c).await; + + let gw = create_internet_gateway(c, PROJECT_NAME, VPC_NAME, IGW_NAME).await; + attach_ip_pool_to_igw( + c, + PROJECT_NAME, + VPC_NAME, + IGW_NAME, + IP_POOL_NAME, + IP_POOL_ATTACHMENT_NAME, + ) + .await; + attach_ip_address_to_igw( + c, + PROJECT_NAME, + VPC_NAME, + IGW_NAME, + IP_ADDRESS_ATTACHMENT.parse().unwrap(), + IP_ADDRESS_ATTACHMENT_NAME, + ) + .await; + + // Remove the route test_setup created targeting this gateway so the + // non-cascading delete isn't blocked; it should then succeed and take the + // pool / address attachments with it. + delete_route(c, PROJECT_NAME, VPC_NAME, ROUTER_NAME, ROUTE_NAME).await; + + delete_internet_gateway(c, PROJECT_NAME, VPC_NAME, IGW_NAME, false).await; + + let igws = list_internet_gateways(c, PROJECT_NAME, VPC_NAME).await; + assert_eq!(igws.len(), 1, "should now just have default gateway"); + + // The external API can't see the attachments once the gateway is gone (the + // list endpoints 404 on the missing gateway before ever looking at the + // association rows), so a "not found" check here would pass even if the rows + // were merely orphaned. Check the datastore directly that no *live* + // association rows remain for this gateway id. + assert_no_live_igw_associations(datastore, gw.identity.id).await; +} + #[nexus_test] async fn test_igw_ip_pool_attach_silo_user(ctx: &ControlPlaneTestContext) { let c = &ctx.external_client; @@ -737,6 +815,62 @@ async fn list_internet_gateway_ip_addresses( out.items } +// Assert, at the datastore layer, that no *live* ip pool / address association +// rows remain for a (now deleted) internet gateway. The external API can't see +// these once the gateway is gone, so this is the only way to distinguish +// "soft-deleted along with the gateway" from "orphaned with time_deleted still +// NULL". +async fn assert_no_live_igw_associations(datastore: &DataStore, igw_id: Uuid) { + let conn = datastore.pool_connection_for_tests().await.unwrap(); + use async_bb8_diesel::AsyncRunQueryDsl; + use diesel::ExpressionMethods; + use diesel::QueryDsl; + + use nexus_db_schema::schema::internet_gateway_ip_pool::dsl as pool_dsl; + let live_pools: i64 = pool_dsl::internet_gateway_ip_pool + .filter(pool_dsl::internet_gateway_id.eq(igw_id)) + .filter(pool_dsl::time_deleted.is_null()) + .count() + .first_async::(&*conn) + .await + .unwrap(); + assert_eq!( + live_pools, 0, + "ip pool associations should be soft-deleted along with the gateway", + ); + + use nexus_db_schema::schema::internet_gateway_ip_address::dsl as addr_dsl; + let live_addrs: i64 = addr_dsl::internet_gateway_ip_address + .filter(addr_dsl::internet_gateway_id.eq(igw_id)) + .filter(addr_dsl::time_deleted.is_null()) + .count() + .first_async::(&*conn) + .await + .unwrap(); + assert_eq!( + live_addrs, 0, + "ip address associations should be soft-deleted along with the gateway", + ); +} + +async fn delete_route( + client: &ClientTestContext, + project_name: &str, + vpc_name: &str, + router_name: &str, + route_name: &str, +) { + let url = format!( + "/v1/vpc-router-routes/{}?project={}&vpc={}&router={}", + route_name, project_name, vpc_name, router_name, + ); + NexusRequest::object_delete(client, &url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); +} + async fn expect_igw_not_found( client: &ClientTestContext, project_name: &str, diff --git a/nexus/types/versions/src/initial/internet_gateway.rs b/nexus/types/versions/src/initial/internet_gateway.rs index 96596749f3e..58bbc0d1552 100644 --- a/nexus/types/versions/src/initial/internet_gateway.rs +++ b/nexus/types/versions/src/initial/internet_gateway.rs @@ -57,8 +57,8 @@ pub struct InternetGatewayDeleteSelector { /// Name or ID of the VPC pub vpc: Option, /// Detach attached IP pools/addresses and delete any routes targeting - /// this gateway. Without `cascade`, delete fails if the gateway has any IP - /// pools or IP addresses attached. + /// this gateway. Without `cascade`, delete fails if the gateway has any + /// routes targeting it. #[serde(default)] pub cascade: bool, } diff --git a/openapi/nexus/nexus-2026073100.0.0-e92b67.json.gitstub b/openapi/nexus/nexus-2026073100.0.0-e92b67.json.gitstub new file mode 100644 index 00000000000..9e1d4ea4fa8 --- /dev/null +++ b/openapi/nexus/nexus-2026073100.0.0-e92b67.json.gitstub @@ -0,0 +1 @@ +3ed2b88b12923c3454c6b98fcee41c8db2249031:openapi/nexus/nexus-2026073100.0.0-e92b67.json diff --git a/openapi/nexus/nexus-2026073100.0.0-e92b67.json b/openapi/nexus/nexus-2026080800.0.0-cfe1b1.json similarity index 99% rename from openapi/nexus/nexus-2026073100.0.0-e92b67.json rename to openapi/nexus/nexus-2026080800.0.0-cfe1b1.json index f4783944934..4eaee9b3966 100644 --- a/openapi/nexus/nexus-2026073100.0.0-e92b67.json +++ b/openapi/nexus/nexus-2026080800.0.0-cfe1b1.json @@ -7,7 +7,7 @@ "url": "https://oxide.computer", "email": "api@oxide.computer" }, - "version": "2026073100.0.0" + "version": "2026080800.0.0" }, "paths": { "/device/auth": { @@ -5844,7 +5844,7 @@ { "in": "query", "name": "cascade", - "description": "Detach attached IP pools/addresses and delete any routes targeting this gateway. Without `cascade`, delete fails if the gateway has any IP pools or IP addresses attached.", + "description": "Detach attached IP pools/addresses and delete any routes targeting this gateway. Without `cascade`, delete fails if the gateway has any routes targeting it.", "schema": { "type": "boolean" } diff --git a/openapi/nexus/nexus-latest.json b/openapi/nexus/nexus-latest.json index 5056afa4db0..3e921c99add 120000 --- a/openapi/nexus/nexus-latest.json +++ b/openapi/nexus/nexus-latest.json @@ -1 +1 @@ -nexus-2026073100.0.0-e92b67.json \ No newline at end of file +nexus-2026080800.0.0-cfe1b1.json \ No newline at end of file