From 532dd5ad0ea6da45d873f380da0eef1be5ca4c12 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Thu, 18 Jan 2024 12:50:17 -0600 Subject: [PATCH] project IP pools endpoint also includes is_default --- nexus/db-queries/src/db/datastore/ip_pool.rs | 14 ++++++-------- nexus/src/app/ip_pool.rs | 2 +- nexus/src/external_api/http_entrypoints.rs | 15 ++++++++++++--- nexus/tests/integration_tests/ip_pools.rs | 7 +++++-- 4 files changed, 24 insertions(+), 14 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index a7f715771d..a6d7b6bc20 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -84,7 +84,7 @@ impl DataStore { &self, opctx: &OpContext, pagparams: &PaginatedBy<'_>, - ) -> ListResultVec { + ) -> ListResultVec<(IpPool, IpPoolResource)> { use db::schema::ip_pool; use db::schema::ip_pool_resource; @@ -108,13 +108,10 @@ impl DataStore { ), } .inner_join(ip_pool_resource::table) - .filter( - ip_pool_resource::resource_type - .eq(IpPoolResourceType::Silo) - .and(ip_pool_resource::resource_id.eq(silo_id)), - ) + .filter(ip_pool_resource::resource_type.eq(IpPoolResourceType::Silo)) + .filter(ip_pool_resource::resource_id.eq(silo_id)) .filter(ip_pool::time_deleted.is_null()) - .select(db::model::IpPool::as_select()) + .select(<(IpPool, IpPoolResource)>::as_select()) .get_results_async(&*self.pool_connection_authorized(opctx).await?) .await .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) @@ -961,7 +958,8 @@ mod test { .await .expect("Should list silo IP pools"); assert_eq!(silo_pools.len(), 1); - assert_eq!(silo_pools[0].id(), pool1_for_silo.id()); + assert_eq!(silo_pools[0].0.id(), pool1_for_silo.id()); + assert_eq!(silo_pools[0].1.is_default, false); // linking an already linked silo errors due to PK conflict let err = datastore diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index 876728fa4c..d1a9ecfbd2 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -78,7 +78,7 @@ impl super::Nexus { &self, opctx: &OpContext, pagparams: &PaginatedBy<'_>, - ) -> ListResultVec { + ) -> ListResultVec<(db::model::IpPool, db::model::IpPoolResource)> { self.db_datastore.current_silo_ip_pool_list(opctx, pagparams).await } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index ca569553fe..992c93c4a1 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1343,8 +1343,7 @@ async fn project_policy_update( async fn project_ip_pool_list( rqctx: RequestContext>, query_params: Query, - // TODO: have this return SiloIpPool so it has is_default on it -) -> Result>, HttpError> { +) -> Result>, HttpError> { let apictx = rqctx.context(); let handler = async { let nexus = &apictx.nexus; @@ -1357,7 +1356,10 @@ async fn project_ip_pool_list( .current_silo_ip_pool_list(&opctx, &paginated_by) .await? .into_iter() - .map(IpPool::from) + .map(|(pool, silo_link)| views::SiloIpPool { + identity: pool.identity(), + is_default: silo_link.is_default, + }) .collect(); Ok(HttpResponseOk(ScanByNameOrId::results_page( &query, @@ -1531,6 +1533,13 @@ async fn ip_pool_silo_list( // option would be to paginate by a composite key representing the (pool, // resource_type, resource) query_params: Query, + // TODO: this could just list views::Silo -- it's not like knowing silo_id + // and nothing else is particularly useful -- except we also want to say + // whether the pool is marked default on each silo. So one option would + // be to do the same as we did with SiloIpPool -- include is_default on + // whatever the thing is. Still... all we'd have to do to make this usable + // in both places would be to make it { ...IpPool, silo_id, silo_name, + // is_default } ) -> Result>, HttpError> { let apictx = rqctx.context(); let handler = async { diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index d97eda9a0b..440376361f 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -42,6 +42,7 @@ use nexus_types::external_api::views::IpPool; use nexus_types::external_api::views::IpPoolRange; use nexus_types::external_api::views::IpPoolSilo; use nexus_types::external_api::views::Silo; +use nexus_types::external_api::views::SiloIpPool; use nexus_types::identity::Resource; use omicron_common::api::external::IdentityMetadataUpdateParams; use omicron_common::api::external::NameOrId; @@ -933,12 +934,14 @@ async fn test_ip_pool_list_in_silo(cptestctx: &ControlPlaneTestContext) { ); create_ip_pool(client, otherpool_name, Some(otherpool_range)).await; - let list = - objects_list_page_authz::(client, "/v1/ip-pools").await.items; + let list = objects_list_page_authz::(client, "/v1/ip-pools") + .await + .items; // only mypool shows up because it's linked to my silo assert_eq!(list.len(), 1); assert_eq!(list[0].identity.name.to_string(), mypool_name); + assert!(list[0].is_default); // fetch the pool directly too let url = format!("/v1/ip-pools/{}", mypool_name);