fix external IPs tests, simplify pool lookup by name logic

Author: david-crespo

nexus/db-queries/src/db/datastore/external_ip.rs

@@ -64,7 +64,7 @@ impl DataStore {
                     &opctx,
                     authz::Action::CreateChild, // TODO: wtf
                     &name,
-                    Some(current_silo.id()),
+                    current_silo.id(),
                     None, // TODO: include current project ID
                 )
                 .await?

nexus/db-queries/src/db/datastore/ip_pool.rs

@@ -116,40 +116,32 @@ impl DataStore {
         opctx: &OpContext,
         action: authz::Action,
         name: &Name,
-        silo_id: Option<Uuid>,
+        silo_id: Uuid,
+        // TODO: project_id should always be defined, this is temporary
         project_id: Option<Uuid>,
     ) -> LookupResult<IpPool> {
         let (.., authz_pool, pool) = LookupPath::new(opctx, &self)
             .ip_pool_name(&name)
             .fetch_for(action)
             .await?;
 
-        if pool.silo_id == Some(*INTERNAL_SILO_ID) {
-            return Err(authz_pool.not_found());
-        }
+        // You can't look up a pool by name if it conflicts with your current
+        // scope, i.e., if it has a silo or project and those are different from
+        // your current silo or project
 
-        // You can't look up the internal pool by name, and you can't look up
-        // a pool by name if it conflicts with your current scope, i.e., if it
-        // has a silo or project and those are different from your current silo
-        // or project
         if let Some(pool_silo_id) = pool.silo_id {
-            if pool_silo_id == *INTERNAL_SILO_ID {
+            if pool_silo_id != silo_id {
                 return Err(authz_pool.not_found());
             }
-
-            if let Some(current_silo_id) = silo_id {
-                if current_silo_id != pool_silo_id {
-                    return Err(authz_pool.not_found());
-                }
-            }
         }
 
-        if let Some(pool_project_id) = pool.project_id {
-            if let Some(current_project_id) = project_id {
-                if current_project_id != pool_project_id {
+        match (pool.project_id, project_id) {
+            (Some(pool_project_id), Some(current_project_id)) => {
+                if pool_project_id != current_project_id {
                     return Err(authz_pool.not_found());
                 }
             }
+            _ => {}
         }
 
         Ok(pool)

nexus/db-queries/src/db/queries/external_ip.rs

@@ -816,7 +816,6 @@ mod tests {
     use crate::context::OpContext;
     use crate::db::datastore::DataStore;
     use crate::db::datastore::SERVICE_IP_POOL_NAME;
-    use crate::db::fixed_data::silo::INTERNAL_SILO_ID;
     use crate::db::identity::Resource;
     use crate::db::model::IpKind;
     use crate::db::model::IpPool;
@@ -862,14 +861,20 @@ mod tests {
             Self { logctx, opctx, db, db_datastore }
         }
 
-        async fn create_ip_pool(&self, name: &str, range: IpRange) {
+        async fn create_ip_pool(
+            &self,
+            name: &str,
+            range: IpRange,
+            is_default: bool,
+        ) {
+            let silo_id = self.opctx.authn.silo_required().unwrap().id();
             let pool = IpPool::new(
                 &IdentityMetadataCreateParams {
                     name: String::from(name).parse().unwrap(),
                     description: format!("ip pool {}", name),
                 },
-                Some(*INTERNAL_SILO_ID),
-                true,
+                Some(silo_id),
+                is_default,
             );
 
             use crate::db::schema::ip_pool::dsl as ip_pool_dsl;
@@ -1709,7 +1714,7 @@ mod tests {
             Ipv4Addr::new(10, 0, 0, 6),
         ))
         .unwrap();
-        context.create_ip_pool("p1", second_range).await;
+        context.create_ip_pool("p1", second_range, /*default*/ false).await;
 
         // Allocating an address on an instance in the second pool should be
         // respected, even though there are IPs available in the first.
@@ -1752,7 +1757,7 @@ mod tests {
         let last_address = Ipv4Addr::new(10, 0, 0, 6);
         let second_range =
             IpRange::try_from((first_address, last_address)).unwrap();
-        context.create_ip_pool("p1", second_range).await;
+        context.create_ip_pool("p1", second_range, /* default */ false).await;
 
         // Allocate all available addresses in the second pool.
         let instance_id = Uuid::new_v4();