diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index bf17b83149..d766fa75d6 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -139,7 +139,9 @@ impl DataStore { use db::schema::ip_pool; use db::schema::ip_pool_resource; - let authz_silo = opctx.authn.silo_required()?; + let authz_silo = opctx.authn.silo_required().internal_context( + "fetching link from an IP pool to current silo", + )?; ip_pool::table .inner_join(ip_pool_resource::table) @@ -419,7 +421,7 @@ impl DataStore { }) } - pub async fn ip_pool_association_list( + pub async fn ip_pool_silo_list( &self, opctx: &OpContext, authz_pool: &authz::IpPool, @@ -445,7 +447,7 @@ impl DataStore { } // TODO: should this error on conflict instead of updating? - pub async fn ip_pool_associate_resource( + pub async fn ip_pool_link_silo( &self, opctx: &OpContext, ip_pool_resource: IpPoolResource, @@ -695,7 +697,7 @@ impl DataStore { /// Delete IP pool assocation with resource unless there are outstanding /// IPs allocated from the pool in the associated silo - pub async fn ip_pool_dissociate_resource( + pub async fn ip_pool_unlink_silo( &self, opctx: &OpContext, association: &IpPoolResourceDelete, @@ -926,7 +928,7 @@ mod test { .await .expect("Failed to create IP pool"); datastore - .ip_pool_associate_resource( + .ip_pool_link_silo( &opctx, IpPoolResource { ip_pool_id: pool1_for_silo.id(), @@ -946,7 +948,7 @@ mod test { // now we can change that association to is_default=true and // it should update rather than erroring out datastore - .ip_pool_associate_resource( + .ip_pool_link_silo( &opctx, IpPoolResource { ip_pool_id: pool1_for_silo.id(), @@ -975,7 +977,7 @@ mod test { .await .expect("Failed to create pool"); let err = datastore - .ip_pool_associate_resource( + .ip_pool_link_silo( &opctx, IpPoolResource { ip_pool_id: second_silo_default.id(), @@ -990,7 +992,7 @@ mod test { // now remove the association and we should get nothing again datastore - .ip_pool_dissociate_resource( + .ip_pool_unlink_silo( &opctx, &IpPoolResourceDelete { ip_pool_id: pool1_for_silo.id(), @@ -999,7 +1001,7 @@ mod test { }, ) .await - .expect("Failed to dissociate IP pool from silo"); + .expect("Failed to unlink IP pool from silo"); let error = datastore.ip_pools_fetch_default(&opctx).await.unwrap_err(); assert_matches!(error, Error::ObjectNotFound { .. }); @@ -1050,7 +1052,7 @@ mod test { is_default: true, }; datastore - .ip_pool_associate_resource(&opctx, link) + .ip_pool_link_silo(&opctx, link) .await .expect("Failed to make IP pool default for silo"); diff --git a/nexus/db-queries/src/db/datastore/rack.rs b/nexus/db-queries/src/db/datastore/rack.rs index 32329ba23c..ca50607b29 100644 --- a/nexus/db-queries/src/db/datastore/rack.rs +++ b/nexus/db-queries/src/db/datastore/rack.rs @@ -773,7 +773,7 @@ impl DataStore { // make default for the internal silo. only need to do this if // the create went through, i.e., if it wasn't already there if internal_created { - self.ip_pool_associate_resource( + self.ip_pool_link_silo( opctx, db::model::IpPoolResource { ip_pool_id: internal_pool_id, diff --git a/nexus/db-queries/src/db/queries/external_ip.rs b/nexus/db-queries/src/db/queries/external_ip.rs index 1ab878dc1c..49403aac61 100644 --- a/nexus/db-queries/src/db/queries/external_ip.rs +++ b/nexus/db-queries/src/db/queries/external_ip.rs @@ -897,7 +897,7 @@ mod tests { is_default, }; self.db_datastore - .ip_pool_associate_resource(&self.opctx, association) + .ip_pool_link_silo(&self.opctx, association) .await .expect("Failed to associate IP pool with silo"); diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index 1435ebfa5d..6dd0d2988a 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -106,46 +106,43 @@ impl super::Nexus { Ok(pool) } - pub(crate) async fn ip_pool_association_list( + pub(crate) async fn ip_pool_silo_list( &self, opctx: &OpContext, pool_lookup: &lookup::IpPool<'_>, pagparams: &DataPageParams<'_, Uuid>, ) -> ListResultVec { - // TODO: is this the right action to check? let (.., authz_pool) = pool_lookup.lookup_for(authz::Action::ListChildren).await?; - self.db_datastore - .ip_pool_association_list(opctx, &authz_pool, pagparams) - .await + self.db_datastore.ip_pool_silo_list(opctx, &authz_pool, pagparams).await } - pub(crate) async fn ip_pool_associate_resource( + pub(crate) async fn ip_pool_link_silo( &self, opctx: &OpContext, pool_lookup: &lookup::IpPool<'_>, silo_link: ¶ms::IpPoolSiloLink, ) -> CreateResult { - let (.., authz_pool) = + let (authz_pool,) = pool_lookup.lookup_for(authz::Action::Modify).await?; - let (silo,) = self + let (authz_silo,) = self .silo_lookup(&opctx, silo_link.silo.clone())? - .lookup_for(authz::Action::Read) + .lookup_for(authz::Action::Modify) .await?; self.db_datastore - .ip_pool_associate_resource( + .ip_pool_link_silo( opctx, db::model::IpPoolResource { ip_pool_id: authz_pool.id(), resource_type: db::model::IpPoolResourceType::Silo, - resource_id: silo.id(), + resource_id: authz_silo.id(), is_default: silo_link.is_default, }, ) .await } - pub(crate) async fn ip_pool_dissociate_resource( + pub(crate) async fn ip_pool_unlink_silo( &self, opctx: &OpContext, pool_lookup: &lookup::IpPool<'_>, @@ -157,7 +154,7 @@ impl super::Nexus { silo_lookup.lookup_for(authz::Action::Modify).await?; self.db_datastore - .ip_pool_dissociate_resource( + .ip_pool_unlink_silo( opctx, &IpPoolResourceDelete { ip_pool_id: authz_pool.id(), @@ -178,7 +175,7 @@ impl super::Nexus { let (.., authz_pool) = pool_lookup.lookup_for(authz::Action::Modify).await?; let (.., authz_silo) = - silo_lookup.lookup_for(authz::Action::Read).await?; + silo_lookup.lookup_for(authz::Action::Modify).await?; self.db_datastore .ip_pool_set_default( diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index ea226d76ab..b45e0f0de4 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1420,7 +1420,7 @@ async fn ip_pool_silo_list( let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?; let assocs = nexus - .ip_pool_association_list(&opctx, &pool_lookup, &pag_params) + .ip_pool_silo_list(&opctx, &pool_lookup, &pag_params) .await? .into_iter() .map(|assoc| assoc.into()) @@ -1454,7 +1454,7 @@ async fn ip_pool_silo_link( let resource_assoc = resource_assoc.into_inner(); let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?; let assoc = nexus - .ip_pool_associate_resource(&opctx, &pool_lookup, &resource_assoc) + .ip_pool_link_silo(&opctx, &pool_lookup, &resource_assoc) .await?; Ok(HttpResponseCreated(assoc.into())) }; @@ -1480,9 +1480,7 @@ async fn ip_pool_silo_unlink( let path = path_params.into_inner(); let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?; let silo_lookup = nexus.silo_lookup(&opctx, path.silo)?; - nexus - .ip_pool_dissociate_resource(&opctx, &pool_lookup, &silo_lookup) - .await?; + nexus.ip_pool_unlink_silo(&opctx, &pool_lookup, &silo_lookup).await?; Ok(HttpResponseUpdatedNoContent()) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await