diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index 393fbb08a5..bf17b83149 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/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 { + 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 diff --git a/nexus/db-queries/src/db/datastore/project.rs b/nexus/db-queries/src/db/datastore/project.rs index abf699f5ed..e3927fdfc1 100644 --- a/nexus/db-queries/src/db/datastore/project.rs +++ b/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 { - 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)) - } } diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index 2a931d6786..1435ebfa5d 100644 --- a/nexus/src/app/ip_pool.rs +++ b/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 { + 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 { + 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, diff --git a/nexus/src/app/project.rs b/nexus/src/app/project.rs index 6e8727a889..2e852ba2d3 100644 --- a/nexus/src/app/project.rs +++ b/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::, _>>()?; Ok(shared::Policy { role_assignments }) } - - pub(crate) async fn project_ip_pools_list( - &self, - opctx: &OpContext, - project_lookup: &lookup::Project<'_>, - pagparams: &PaginatedBy<'_>, - ) -> ListResultVec { - 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>, - ) -> LookupResult> { - // 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) - } - } - } } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 0d29e1c22c..ea226d76ab 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/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,7 +1211,7 @@ 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", @@ -1219,14 +1219,8 @@ async fn project_policy_update( }] async fn project_ip_pool_list( rqctx: RequestContext>, - query_params: Query>, + query_params: Query, ) -> Result>, 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>, path_params: Path, - project: Query, ) -> Result, 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 diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index bff398a64b..278a02a336 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/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) diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 1211cebaf4..e72c351fcf 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/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 { diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index 99bda9a291..1313ea37b9 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -4,8 +4,6 @@ //! Integration tests for operating on IP Pools -use std::collections::HashSet; - use dropshot::test_util::ClientTestContext; use dropshot::HttpErrorResponseBody; use dropshot::ResultsPage; @@ -16,7 +14,10 @@ use nexus_db_queries::db::fixed_data::silo::DEFAULT_SILO; use nexus_test_utils::http_testing::AuthnMode; use nexus_test_utils::http_testing::NexusRequest; use nexus_test_utils::http_testing::RequestBuilder; +use nexus_test_utils::resource_helpers::create_instance; +use nexus_test_utils::resource_helpers::create_ip_pool; use nexus_test_utils::resource_helpers::create_project; +use nexus_test_utils::resource_helpers::link_ip_pool; use nexus_test_utils::resource_helpers::object_create; use nexus_test_utils::resource_helpers::object_create_error; use nexus_test_utils::resource_helpers::object_delete; @@ -25,14 +26,8 @@ use nexus_test_utils::resource_helpers::object_get; use nexus_test_utils::resource_helpers::object_get_error; use nexus_test_utils::resource_helpers::object_put; use nexus_test_utils::resource_helpers::objects_list_page_authz; -use nexus_test_utils::resource_helpers::{ - create_instance, create_instance_with, -}; use nexus_test_utils_macros::nexus_test; use nexus_types::external_api::params; -use nexus_types::external_api::params::ExternalIpCreate; -use nexus_types::external_api::params::InstanceDiskAttachment; -use nexus_types::external_api::params::InstanceNetworkInterfaceAttachment; use nexus_types::external_api::params::IpPoolCreate; use nexus_types::external_api::params::IpPoolSiloLink; use nexus_types::external_api::params::IpPoolSiloUpdate; @@ -810,48 +805,15 @@ async fn test_ip_pool_range_pagination(cptestctx: &ControlPlaneTestContext) { } #[nexus_test] -async fn test_ip_pool_list_usable_by_project( - cptestctx: &ControlPlaneTestContext, -) { +async fn test_ip_pool_list_in_silo(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; - let scoped_ip_pools_url = "/v1/ip-pools"; - let ip_pools_url = "/v1/system/ip-pools"; let mypool_name = "mypool"; - let mypool_silos_url = format!("{}/{}/silos", ip_pools_url, mypool_name); - let mypool_ip_pool_add_range_url = - format!("{}/{}/ranges/add", ip_pools_url, mypool_name); - let service_ip_pool_add_range_url = - "/v1/system/ip-pools-service/ranges/add".to_string(); - - // Create an org and project, and then try to make an instance with an IP from - // each range to which the project is expected have access. const PROJECT_NAME: &str = "myproj"; - const INSTANCE_NAME: &str = "myinst"; create_project(client, PROJECT_NAME).await; - let params = IpPoolCreate { - identity: IdentityMetadataCreateParams { - name: String::from(mypool_name).parse().unwrap(), - description: String::from("right on cue"), - }, - }; - NexusRequest::objects_post(client, ip_pools_url, ¶ms) - .authn_as(AuthnMode::PrivilegedUser) - .execute_and_parse_unwrap::() - .await; - - // associate pool with default silo, which is the privileged user's silo - let params = IpPoolSiloLink { - silo: NameOrId::Id(DEFAULT_SILO.id()), - is_default: true, - }; - NexusRequest::objects_post(client, &mypool_silos_url, ¶ms) - .authn_as(AuthnMode::PrivilegedUser) - .execute_and_parse_unwrap::() - .await; - - // Add an IP range to mypool + // create pool with range and link (as default pool) to default silo, which + // is the privileged user's silo let mypool_range = IpRange::V4( Ipv4Range::new( std::net::Ipv4Addr::new(10, 0, 0, 51), @@ -859,97 +821,35 @@ async fn test_ip_pool_list_usable_by_project( ) .unwrap(), ); - let created_range: IpPoolRange = NexusRequest::objects_post( - client, - &mypool_ip_pool_add_range_url, - &mypool_range, - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap() - .parsed_body() - .unwrap(); - assert_eq!( - mypool_range.first_address(), - created_range.range.first_address() - ); - assert_eq!(mypool_range.last_address(), created_range.range.last_address()); + create_ip_pool(client, mypool_name, Some(mypool_range)).await; + link_ip_pool(client, mypool_name, &DEFAULT_SILO.id(), true).await; - // add a service range we *don't* expect to see in the results - let service_range = IpRange::V4( + // create another pool and don't link it to anything + let otherpool_name = "other-pool"; + let otherpool_range = IpRange::V4( Ipv4Range::new( - std::net::Ipv4Addr::new(10, 0, 0, 101), - std::net::Ipv4Addr::new(10, 0, 0, 102), + std::net::Ipv4Addr::new(10, 0, 0, 53), + std::net::Ipv4Addr::new(10, 0, 0, 54), ) .unwrap(), ); + create_ip_pool(client, otherpool_name, Some(otherpool_range)).await; - let created_range: IpPoolRange = NexusRequest::objects_post( - client, - &service_ip_pool_add_range_url, - &service_range, - ) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap() - .parsed_body() - .unwrap(); - assert_eq!( - service_range.first_address(), - created_range.range.first_address() - ); - assert_eq!( - service_range.last_address(), - created_range.range.last_address() - ); - - // TODO: add non-service ip pools that the project *can't* use - - let list_url = format!("{}?project={}", scoped_ip_pools_url, PROJECT_NAME); - let list = NexusRequest::iter_collection_authn::( - client, &list_url, "", None, - ) - .await - .expect("Failed to list IP Pools") - .all_items; + let list = + objects_list_page_authz::(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); - // currently there's only one in the list, so this is overkill. But there might be more - let pool_names: HashSet = - list.iter().map(|pool| pool.identity.name.to_string()).collect(); + // fetch the pool directly too + let url = format!("/v1/ip-pools/{}", mypool_name); + let pool: IpPool = object_get(client, &url).await; + assert_eq!(pool.identity.name.as_str(), mypool_name); - // ensure we can view each pool returned - for pool_name in &pool_names { - let view_pool_url = format!( - "{}/{}?project={}", - scoped_ip_pools_url, pool_name, PROJECT_NAME - ); - let pool = NexusRequest::object_get(client, &view_pool_url) - .authn_as(AuthnMode::PrivilegedUser) - .execute_and_parse_unwrap::() - .await; - assert_eq!(pool.identity.name.as_str(), pool_name.as_str()); - } - - // ensure we can successfully create an instance with each of the pools we - // should be able to access - for pool_name in pool_names { - let instance_name = format!("{}-{}", INSTANCE_NAME, pool_name); - let pool_name = Some(Name::try_from(pool_name.to_string()).unwrap()); - create_instance_with( - client, - PROJECT_NAME, - &instance_name, - &InstanceNetworkInterfaceAttachment::Default, - Vec::::new(), - vec![ExternalIpCreate::Ephemeral { pool_name }], - ) - .await; - } + // fetching the other pool directly 404s + let url = format!("/v1/ip-pools/{}", otherpool_name); + object_get_error(client, &url, StatusCode::NOT_FOUND).await; } #[nexus_test] diff --git a/openapi/nexus.json b/openapi/nexus.json index e36d0232ef..ea3e8af8bc 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -2154,7 +2154,7 @@ "tags": [ "projects" ], - "summary": "List all IP Pools that can be used by a given project.", + "summary": "List all IP pools", "operationId": "project_ip_pool_list", "parameters": [ { @@ -2177,14 +2177,6 @@ "type": "string" } }, - { - "in": "query", - "name": "project", - "description": "Name or ID of the project", - "schema": { - "$ref": "#/components/schemas/NameOrId" - } - }, { "in": "query", "name": "sort_by", @@ -2212,9 +2204,7 @@ } }, "x-dropshot-pagination": { - "required": [ - "project" - ] + "required": [] } } }, @@ -2234,14 +2224,6 @@ "schema": { "$ref": "#/components/schemas/NameOrId" } - }, - { - "in": "query", - "name": "project", - "description": "Name or ID of the project", - "schema": { - "$ref": "#/components/schemas/NameOrId" - } } ], "responses": {