Skip to content

Commit

Permalink
project IP pools endpoint also includes is_default
Browse files Browse the repository at this point in the history
  • Loading branch information
david-crespo committed Jan 18, 2024
1 parent 270133e commit 532dd5a
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 14 deletions.
14 changes: 6 additions & 8 deletions nexus/db-queries/src/db/datastore/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ impl DataStore {
&self,
opctx: &OpContext,
pagparams: &PaginatedBy<'_>,
) -> ListResultVec<db::model::IpPool> {
) -> ListResultVec<(IpPool, IpPoolResource)> {
use db::schema::ip_pool;
use db::schema::ip_pool_resource;

Expand All @@ -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))
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion nexus/src/app/ip_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl super::Nexus {
&self,
opctx: &OpContext,
pagparams: &PaginatedBy<'_>,
) -> ListResultVec<db::model::IpPool> {
) -> ListResultVec<(db::model::IpPool, db::model::IpPoolResource)> {
self.db_datastore.current_silo_ip_pool_list(opctx, pagparams).await
}

Expand Down
15 changes: 12 additions & 3 deletions nexus/src/external_api/http_entrypoints.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1343,8 +1343,7 @@ async fn project_policy_update(
async fn project_ip_pool_list(
rqctx: RequestContext<Arc<ServerContext>>,
query_params: Query<PaginatedByNameOrId>,
// TODO: have this return SiloIpPool so it has is_default on it
) -> Result<HttpResponseOk<ResultsPage<IpPool>>, HttpError> {
) -> Result<HttpResponseOk<ResultsPage<views::SiloIpPool>>, HttpError> {
let apictx = rqctx.context();
let handler = async {
let nexus = &apictx.nexus;
Expand All @@ -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,
Expand Down Expand Up @@ -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<PaginatedById>,
// 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<HttpResponseOk<ResultsPage<views::IpPoolSilo>>, HttpError> {
let apictx = rqctx.context();
let handler = async {
Expand Down
7 changes: 5 additions & 2 deletions nexus/tests/integration_tests/ip_pools.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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::<IpPool>(client, "/v1/ip-pools").await.items;
let list = objects_list_page_authz::<SiloIpPool>(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);
Expand Down

0 comments on commit 532dd5a

Please sign in to comment.