make authz checks slightly more consistent, get rid of some assoc/dis…

…soc language

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");
 

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,

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");
 

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<db::model::IpPoolResource> {
-        // 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: &params::IpPoolSiloLink,
     ) -> CreateResult<db::model::IpPoolResource> {
-        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(

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