Skip to content

Commit

Permalink
verify there are no linked silos before deleting pool
Browse files Browse the repository at this point in the history
  • Loading branch information
@david-crespo
david-crespo committed Dec 13, 2023
1 parent 69a482a commit 7c0ba9d
Show file tree
Hide file tree
Showing 3 changed files with 112 additions and 114 deletions.
18 changes: 18 additions & 0 deletions nexus/db-queries/src/db/datastore/ip_pool.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,7 @@ impl DataStore {
) -> DeleteResult {
use db::schema::ip_pool::dsl;
use db::schema::ip_pool_range;
use db::schema::ip_pool_resource;
opctx.authorize(authz::Action::Delete, authz_pool).await?;

// Verify there are no IP ranges still in this pool
Expand All @@ -268,6 +269,23 @@ impl DataStore {
));
}

// Verify there are no linked silos
let silo_link = ip_pool_resource::table
.filter(ip_pool_resource::ip_pool_id.eq(authz_pool.id()))
.select(ip_pool_resource::resource_id)
.limit(1)
.first_async::<Uuid>(
&*self.pool_connection_authorized(opctx).await?,
)
.await
.optional()
.map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))?;
if silo_link.is_some() {
return Err(Error::invalid_request(
"IP Pool cannot be deleted while it is linked to a silo",
));
}

// Delete the pool, conditional on the rcgen not having changed. This
// protects the delete from occuring if clients created a new IP range
// in between the above check for children and this query.
Expand Down
69 changes: 52 additions & 17 deletions nexus/test-utils/src/resource_helpers.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,39 +57,31 @@ where
.unwrap()
}

pub async fn object_create<InputType, OutputType>(
pub async fn object_get<OutputType>(
client: &ClientTestContext,
path: &str,
input: &InputType,
) -> OutputType
where
InputType: serde::Serialize,
OutputType: serde::de::DeserializeOwned,
{
NexusRequest::objects_post(client, path, input)
NexusRequest::object_get(client, path)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap_or_else(|e| {
panic!("failed to make \"POST\" request to {path}: {e}")
panic!("failed to make \"GET\" request to {path}: {e}")
})
.parsed_body()
.unwrap()
}

/// Make a POST, assert status code, return error response body
pub async fn object_create_error<InputType>(
pub async fn object_get_error(
client: &ClientTestContext,
path: &str,
input: &InputType,
status: StatusCode,
) -> HttpErrorResponseBody
where
InputType: serde::Serialize,
{
) -> HttpErrorResponseBody {
NexusRequest::new(
RequestBuilder::new(client, Method::POST, path)
.body(Some(&input))
RequestBuilder::new(client, Method::DELETE, path)
.expect_status(Some(status)),
)
.authn_as(AuthnMode::PrivilegedUser)
Expand All @@ -100,13 +92,39 @@ where
.unwrap()
}

pub async fn object_delete_error(
pub async fn object_create<InputType, OutputType>(
client: &ClientTestContext,
path: &str,
input: &InputType,
) -> OutputType
where
InputType: serde::Serialize,
OutputType: serde::de::DeserializeOwned,
{
NexusRequest::objects_post(client, path, input)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap_or_else(|e| {
panic!("failed to make \"POST\" request to {path}: {e}")
})
.parsed_body()
.unwrap()
}

/// Make a POST, assert status code, return error response body
pub async fn object_create_error<InputType>(
client: &ClientTestContext,
path: &str,
input: &InputType,
status: StatusCode,
) -> HttpErrorResponseBody {
) -> HttpErrorResponseBody
where
InputType: serde::Serialize,
{
NexusRequest::new(
RequestBuilder::new(client, Method::DELETE, path)
RequestBuilder::new(client, Method::POST, path)
.body(Some(&input))
.expect_status(Some(status)),
)
.authn_as(AuthnMode::PrivilegedUser)
Expand Down Expand Up @@ -147,6 +165,23 @@ pub async fn object_delete(client: &ClientTestContext, path: &str) {
});
}

pub async fn object_delete_error(
client: &ClientTestContext,
path: &str,
status: StatusCode,
) -> HttpErrorResponseBody {
NexusRequest::new(
RequestBuilder::new(client, Method::DELETE, path)
.expect_status(Some(status)),
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body::<HttpErrorResponseBody>()
.unwrap()
}

/// Create an IP pool with a single range for testing.
///
/// The IP range may be specified if it's important for testing the behavior
Expand Down
139 changes: 42 additions & 97 deletions nexus/tests/integration_tests/ip_pools.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ use nexus_test_utils::resource_helpers::create_project;
use nexus_test_utils::resource_helpers::object_create;
use nexus_test_utils::resource_helpers::object_create_error;
use nexus_test_utils::resource_helpers::object_delete;
use nexus_test_utils::resource_helpers::object_delete_error;
use nexus_test_utils::resource_helpers::object_get;
use nexus_test_utils::resource_helpers::object_get_error;
use nexus_test_utils::resource_helpers::object_put;
use nexus_test_utils::resource_helpers::objects_list_page_authz;
use nexus_test_utils::resource_helpers::{
Expand Down Expand Up @@ -76,34 +79,15 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
assert_eq!(ip_pools.len(), 0, "Expected empty list of IP pools");

// Verify 404 if the pool doesn't exist yet, both for creating or deleting
let error: HttpErrorResponseBody = NexusRequest::expect_failure(
client,
StatusCode::NOT_FOUND,
Method::GET,
&ip_pool_url,
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();
let error =
object_get_error(client, &ip_pool_url, StatusCode::NOT_FOUND).await;
assert_eq!(
error.message,
format!("not found: ip-pool with name \"{}\"", pool_name),
);
let error: HttpErrorResponseBody = NexusRequest::expect_failure(
client,
StatusCode::NOT_FOUND,
Method::DELETE,
&ip_pool_url,
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();

let error =
object_delete_error(client, &ip_pool_url, StatusCode::NOT_FOUND).await;
assert_eq!(
error.message,
format!("not found: ip-pool with name \"{}\"", pool_name),
Expand All @@ -116,20 +100,11 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
name: String::from(pool_name).parse().unwrap(),
description: String::from(description),
},
// silo: None,
// is_default: false,
};
let created_pool: IpPool =
NexusRequest::objects_post(client, ip_pools_url, &params)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();
object_create(client, ip_pools_url, &params).await;
assert_eq!(created_pool.identity.name, pool_name);
assert_eq!(created_pool.identity.description, description);
// assert_eq!(created_pool.silo_id, None);

let list = NexusRequest::iter_collection_authn::<IpPool>(
client,
Expand All @@ -143,27 +118,18 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
assert_eq!(list.len(), 1, "Expected exactly 1 IP pool");
assert_pools_eq(&created_pool, &list[0]);

let fetched_pool: IpPool = NexusRequest::object_get(client, &ip_pool_url)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();
let fetched_pool: IpPool = object_get(client, &ip_pool_url).await;
assert_pools_eq(&created_pool, &fetched_pool);

// Verify we get a conflict error if we insert it again
let error: HttpErrorResponseBody = NexusRequest::new(
RequestBuilder::new(client, Method::POST, ip_pools_url)
.body(Some(&params))
.expect_status(Some(StatusCode::BAD_REQUEST)),
let error = object_create_error(
client,
ip_pools_url,
&params,
StatusCode::BAD_REQUEST,
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();
.await;

assert_eq!(
error.message,
format!("already exists: ip-pool \"{}\"", pool_name)
Expand All @@ -178,27 +144,13 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
.unwrap(),
);
let created_range: IpPoolRange =
NexusRequest::objects_post(client, &ip_pool_add_range_url, &range)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();
object_create(client, &ip_pool_add_range_url, &range).await;
assert_eq!(range.first_address(), created_range.range.first_address());
assert_eq!(range.last_address(), created_range.range.last_address());
let error: HttpErrorResponseBody = NexusRequest::expect_failure(
client,
StatusCode::BAD_REQUEST,
Method::DELETE,
&ip_pool_url,
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();

let error: HttpErrorResponseBody =
object_delete_error(client, &ip_pool_url, StatusCode::BAD_REQUEST)
.await;
assert_eq!(
error.message,
"IP Pool cannot be deleted while it contains IP ranges",
Expand All @@ -219,13 +171,7 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
},
};
let modified_pool: IpPool =
NexusRequest::object_put(client, &ip_pool_url, Some(&updates))
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();
object_put(client, &ip_pool_url, &updates).await;
assert_eq!(modified_pool.identity.name, new_pool_name);
assert_eq!(modified_pool.identity.id, created_pool.identity.id);
assert_eq!(
Expand All @@ -242,27 +188,11 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) {
);

let fetched_modified_pool: IpPool =
NexusRequest::object_get(client, &new_ip_pool_url)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();
object_get(client, &new_ip_pool_url).await;
assert_pools_eq(&modified_pool, &fetched_modified_pool);

let error: HttpErrorResponseBody = NexusRequest::expect_failure(
client,
StatusCode::NOT_FOUND,
Method::GET,
&ip_pool_url,
)
.authn_as(AuthnMode::PrivilegedUser)
.execute()
.await
.unwrap()
.parsed_body()
.unwrap();
let error: HttpErrorResponseBody =
object_get_error(client, &ip_pool_url, StatusCode::NOT_FOUND).await;
assert_eq!(
error.message,
format!("not found: ip-pool with name \"{}\"", pool_name),
Expand Down Expand Up @@ -425,14 +355,29 @@ async fn test_ip_pool_silo_link(cptestctx: &ControlPlaneTestContext) {
StatusCode::BAD_REQUEST,
)
.await;
assert_eq!(error.error_code, Some("ObjectAlreadyExists".to_string()));
assert_eq!(error.error_code.unwrap(), "ObjectAlreadyExists");

// pool delete fails because it is linked to a silo
let error = object_delete_error(
client,
"/v1/system/ip-pools/p1",
StatusCode::BAD_REQUEST,
)
.await;
assert_eq!(
error.message,
"IP Pool cannot be deleted while it is linked to a silo",
);

// unlink silo (doesn't matter that it's a default)
let url = format!("/v1/system/ip-pools/p1/silos/{}", cptestctx.silo_name);
object_delete(client, &url).await;

let silos_p1 = silos_for_pool(client, "p1").await;
assert_eq!(silos_p1.items.len(), 0);

// now we can delete the pool too
object_delete(client, "/v1/system/ip-pools/p1").await;
}

#[nexus_test]
Expand Down

0 comments on commit 7c0ba9d

Please sign in to comment.