couldn't help it, had to fix the silo-scoped pools endpoints

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

@@ -40,6 +40,7 @@ use omicron_common::api::external::CreateResult;
 use omicron_common::api::external::DataPageParams;
 use omicron_common::api::external::DeleteResult;
 use omicron_common::api::external::Error;
+use omicron_common::api::external::InternalContext;
 use omicron_common::api::external::ListResultVec;
 use omicron_common::api::external::LookupResult;
 use omicron_common::api::external::LookupType;
@@ -86,6 +87,47 @@ impl DataStore {
         .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
     }
 
+    /// List IP pools linked to the current silo
+    pub async fn silo_ip_pools_list(
+        &self,
+        opctx: &OpContext,
+        pagparams: &PaginatedBy<'_>,
+    ) -> ListResultVec<db::model::IpPool> {
+        use db::schema::ip_pool;
+        use db::schema::ip_pool_resource;
+
+        // From the developer user's point of view, we treat IP pools linked to
+        // their silo as silo resources, so they can list them if they can list
+        // silo children
+        let authz_silo =
+            opctx.authn.silo_required().internal_context("listing IP pools")?;
+        opctx.authorize(authz::Action::ListChildren, &authz_silo).await?;
+
+        let silo_id = authz_silo.id();
+
+        match pagparams {
+            PaginatedBy::Id(pagparams) => {
+                paginated(ip_pool::table, ip_pool::id, pagparams)
+            }
+            PaginatedBy::Name(pagparams) => paginated(
+                ip_pool::table,
+                ip_pool::name,
+                &pagparams.map_name(|n| Name::ref_cast(n)),
+            ),
+        }
+        .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::time_deleted.is_null())
+        .select(db::model::IpPool::as_select())
+        .get_results_async(&*self.pool_connection_authorized(opctx).await?)
+        .await
+        .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
+    }
+
     /// Look up whether the given pool is available to users in the current
     /// silo, i.e., whether there is an entry in the association table linking
     /// the pool with that silo

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

@@ -27,7 +27,6 @@ use crate::transaction_retry::OptionalError;
 use async_bb8_diesel::AsyncRunQueryDsl;
 use chrono::Utc;
 use diesel::prelude::*;
-use nexus_db_model::IpPoolResourceType;
 use omicron_common::api::external::http_pagination::PaginatedBy;
 use omicron_common::api::external::CreateResult;
 use omicron_common::api::external::DeleteResult;
@@ -348,41 +347,4 @@ impl DataStore {
                 )
             })
     }
-
-    /// List IP Pools accessible to a project
-    pub async fn project_ip_pools_list(
-        &self,
-        opctx: &OpContext,
-        authz_project: &authz::Project,
-        pagparams: &PaginatedBy<'_>,
-    ) -> ListResultVec<db::model::IpPool> {
-        use db::schema::ip_pool;
-        use db::schema::ip_pool_resource;
-
-        opctx.authorize(authz::Action::ListChildren, authz_project).await?;
-
-        let silo_id = opctx.authn.silo_required().unwrap().id();
-
-        match pagparams {
-            PaginatedBy::Id(pagparams) => {
-                paginated(ip_pool::table, ip_pool::id, pagparams)
-            }
-            PaginatedBy::Name(pagparams) => paginated(
-                ip_pool::table,
-                ip_pool::name,
-                &pagparams.map_name(|n| Name::ref_cast(n)),
-            ),
-        }
-        .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::time_deleted.is_null())
-        .select(db::model::IpPool::as_select())
-        .get_results_async(&*self.pool_connection_authorized(opctx).await?)
-        .await
-        .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server))
-    }
 }

nexus/src/app/ip_pool.rs

@@ -10,11 +10,13 @@ use ipnetwork::IpNetwork;
 use nexus_db_model::IpPoolResourceDelete;
 use nexus_db_model::IpPoolResourceType;
 use nexus_db_queries::authz;
+use nexus_db_queries::authz::ApiResource;
 use nexus_db_queries::context::OpContext;
 use nexus_db_queries::db;
 use nexus_db_queries::db::lookup;
 use nexus_db_queries::db::lookup::LookupPath;
 use nexus_db_queries::db::model::Name;
+use nexus_types::identity::Resource;
 use omicron_common::api::external::http_pagination::PaginatedBy;
 use omicron_common::api::external::CreateResult;
 use omicron_common::api::external::DataPageParams;
@@ -73,6 +75,37 @@ impl super::Nexus {
         self.db_datastore.ip_pool_create(opctx, pool).await
     }
 
+    // TODO: this is used by a developer user to see what IP pools they can use
+    // in their silo, so it would be nice to say which one is the default
+
+    /// List IP pools in current silo
+    pub(crate) async fn silo_ip_pools_list(
+        &self,
+        opctx: &OpContext,
+        pagparams: &PaginatedBy<'_>,
+    ) -> ListResultVec<db::model::IpPool> {
+        self.db_datastore.silo_ip_pools_list(opctx, pagparams).await
+    }
+
+    // Look up pool by name or ID, but only return it if it's linked to the
+    // current silo
+    pub async fn silo_ip_pool_fetch<'a>(
+        &'a self,
+        opctx: &'a OpContext,
+        pool: &'a NameOrId,
+    ) -> LookupResult<db::model::IpPool> {
+        let (authz_pool, pool) =
+            self.ip_pool_lookup(opctx, pool)?.fetch().await?;
+
+        // 404 if no link is found in the current silo
+        let link = self.db_datastore.ip_pool_fetch_link(opctx, pool.id()).await;
+        if link.is_err() {
+            return Err(authz_pool.not_found());
+        }
+
+        Ok(pool)
+    }
+
     pub(crate) async fn ip_pool_association_list(
         &self,
         opctx: &OpContext,

nexus/src/app/project.rs

@@ -8,7 +8,6 @@ use crate::app::sagas;
 use crate::external_api::params;
 use crate::external_api::shared;
 use anyhow::Context;
-use nexus_db_model::Name;
 use nexus_db_queries::authn;
 use nexus_db_queries::authz;
 use nexus_db_queries::context::OpContext;
@@ -24,7 +23,6 @@ use omicron_common::api::external::ListResultVec;
 use omicron_common::api::external::LookupResult;
 use omicron_common::api::external::NameOrId;
 use omicron_common::api::external::UpdateResult;
-use ref_cast::RefCast;
 use std::sync::Arc;
 
 impl super::Nexus {
@@ -149,40 +147,4 @@ impl super::Nexus {
             .collect::<Result<Vec<_>, _>>()?;
         Ok(shared::Policy { role_assignments })
     }
-
-    pub(crate) async fn project_ip_pools_list(
-        &self,
-        opctx: &OpContext,
-        project_lookup: &lookup::Project<'_>,
-        pagparams: &PaginatedBy<'_>,
-    ) -> ListResultVec<db::model::IpPool> {
-        let (.., authz_project) =
-            project_lookup.lookup_for(authz::Action::ListChildren).await?;
-
-        self.db_datastore
-            .project_ip_pools_list(opctx, &authz_project, pagparams)
-            .await
-    }
-
-    pub fn project_ip_pool_lookup<'a>(
-        &'a self,
-        opctx: &'a OpContext,
-        pool: &'a NameOrId,
-        _project_lookup: &Option<lookup::Project<'_>>,
-    ) -> LookupResult<lookup::IpPool<'a>> {
-        // TODO(2148, 2056): check that the given project has access (if one
-        // is provided to the call) once that relation is implemented
-        match pool {
-            NameOrId::Name(name) => {
-                let pool = LookupPath::new(opctx, &self.db_datastore)
-                    .ip_pool_name(Name::ref_cast(name));
-                Ok(pool)
-            }
-            NameOrId::Id(id) => {
-                let pool =
-                    LookupPath::new(opctx, &self.db_datastore).ip_pool_id(*id);
-                Ok(pool)
-            }
-        }
-    }
 }

nexus/src/external_api/http_entrypoints.rs

@@ -42,7 +42,7 @@ use nexus_db_queries::db::identity::Resource;
 use nexus_db_queries::db::lookup::ImageLookup;
 use nexus_db_queries::db::lookup::ImageParentLookup;
 use nexus_db_queries::db::model::Name;
-use nexus_types::external_api::{params::ProjectSelector, views::SiloQuotas};
+use nexus_types::external_api::views::SiloQuotas;
 use nexus_types::{
     external_api::views::{SledInstance, Switch},
     identity::AssetIdentityMetadata,
@@ -1211,22 +1211,16 @@ async fn project_policy_update(
 
 // IP Pools
 
-/// List all IP Pools that can be used by a given project.
+/// List all IP pools
 #[endpoint {
     method = GET,
     path = "/v1/ip-pools",
     tags = ["projects"],
 }]
 async fn project_ip_pool_list(
     rqctx: RequestContext<Arc<ServerContext>>,
-    query_params: Query<PaginatedByNameOrId<params::ProjectSelector>>,
+    query_params: Query<PaginatedByNameOrId>,
 ) -> Result<HttpResponseOk<ResultsPage<IpPool>>, HttpError> {
-    // Per https://github.com/oxidecomputer/omicron/issues/2148
-    // This is currently the same list as /v1/system/ip-pools, that is to say,
-    // IP pools that are *available to* a given project, those being ones that
-    // are not the internal pools for Oxide service usage. This may change
-    // in the future as the scoping of pools is further developed, but for now,
-    // this is literally a near-duplicate of `ip_pool_list`:
     let apictx = rqctx.context();
     let handler = async {
         let nexus = &apictx.nexus;
@@ -1235,10 +1229,8 @@ async fn project_ip_pool_list(
         let scan_params = ScanByNameOrId::from_query(&query)?;
         let paginated_by = name_or_id_pagination(&pag_params, scan_params)?;
         let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
-        let project_lookup =
-            nexus.project_lookup(&opctx, scan_params.selector.clone())?;
         let pools = nexus
-            .project_ip_pools_list(&opctx, &project_lookup, &paginated_by)
+            .silo_ip_pools_list(&opctx, &paginated_by)
             .await?
             .into_iter()
             .map(IpPool::from)
@@ -1261,23 +1253,13 @@ async fn project_ip_pool_list(
 async fn project_ip_pool_view(
     rqctx: RequestContext<Arc<ServerContext>>,
     path_params: Path<params::IpPoolPath>,
-    project: Query<params::OptionalProjectSelector>,
 ) -> Result<HttpResponseOk<views::IpPool>, HttpError> {
     let apictx = rqctx.context();
     let handler = async {
         let opctx = crate::context::op_context_for_external_api(&rqctx).await?;
         let nexus = &apictx.nexus;
         let pool_selector = path_params.into_inner().pool;
-        let project_lookup = if let Some(project) = project.into_inner().project
-        {
-            Some(nexus.project_lookup(&opctx, ProjectSelector { project })?)
-        } else {
-            None
-        };
-        let (_authz_pool, pool) = nexus
-            .project_ip_pool_lookup(&opctx, &pool_selector, &project_lookup)?
-            .fetch()
-            .await?;
+        let pool = nexus.silo_ip_pool_fetch(&opctx, &pool_selector).await?;
         Ok(HttpResponseOk(IpPool::from(pool)))
     };
     apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await

nexus/test-utils/src/resource_helpers.rs

@@ -81,7 +81,7 @@ pub async fn object_get_error(
     status: StatusCode,
 ) -> HttpErrorResponseBody {
     NexusRequest::new(
-        RequestBuilder::new(client, Method::DELETE, path)
+        RequestBuilder::new(client, Method::GET, path)
             .expect_status(Some(status)),
     )
     .authn_as(AuthnMode::PrivilegedUser)

nexus/tests/integration_tests/endpoints.rs

@@ -500,8 +500,7 @@ lazy_static! {
         };
 
     // IP Pools
-    pub static ref DEMO_IP_POOLS_PROJ_URL: String =
-        format!("/v1/ip-pools?project={}", *DEMO_PROJECT_NAME);
+    pub static ref DEMO_IP_POOLS_PROJ_URL: String = "/v1/ip-pools".to_string();
     pub static ref DEMO_IP_POOLS_URL: &'static str = "/v1/system/ip-pools";
     pub static ref DEMO_IP_POOL_NAME: Name = "default".parse().unwrap();
     pub static ref DEMO_IP_POOL_CREATE: params::IpPoolCreate =
@@ -511,8 +510,7 @@ lazy_static! {
                 description: String::from("an IP pool"),
             },
         };
-    pub static ref DEMO_IP_POOL_PROJ_URL: String =
-        format!("/v1/ip-pools/{}?project={}", *DEMO_IP_POOL_NAME, *DEMO_PROJECT_NAME);
+    pub static ref DEMO_IP_POOL_PROJ_URL: String = format!("/v1/ip-pools/{}", *DEMO_IP_POOL_NAME);
     pub static ref DEMO_IP_POOL_URL: String = format!("/v1/system/ip-pools/{}", *DEMO_IP_POOL_NAME);
     pub static ref DEMO_IP_POOL_UPDATE: params::IpPoolUpdate =
         params::IpPoolUpdate {