From b55f70255f33e89227d649b83ac4bd9b43299404 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 1 Aug 2022 15:45:01 -0400 Subject: [PATCH 1/9] Add db structures for Oxide service IP pools --- common/src/sql/dbinit.sql | 17 ++++ nexus/db-model/src/ip_pool.rs | 7 ++ nexus/db-model/src/schema.rs | 1 + nexus/src/app/ip_pool.rs | 47 +++++++++ nexus/src/db/datastore/ip_pool.rs | 35 ++++++- nexus/src/db/queries/external_ip.rs | 3 +- nexus/src/external_api/http_entrypoints.rs | 108 +++++++++++++++++++++ 7 files changed, 216 insertions(+), 2 deletions(-) diff --git a/common/src/sql/dbinit.sql b/common/src/sql/dbinit.sql index 9d1e4b2157a..3a7af40b04c 100644 --- a/common/src/sql/dbinit.sql +++ b/common/src/sql/dbinit.sql @@ -969,6 +969,14 @@ CREATE TABLE omicron.public.ip_pool ( /* Optional ID of the project for which this pool is reserved. */ project_id UUID, + /* + * Optional rack ID, indicating this is a reserved pool for internal + * services on a specific rack. + * TODO(https://github.com/oxidecomputer/omicron/issues/1276): This + * should probably point to an AZ or fleet, not a rack. + */ + rack_id UUID, + /* The collection's child-resource generation number */ rcgen INT8 NOT NULL ); @@ -981,6 +989,15 @@ CREATE UNIQUE INDEX ON omicron.public.ip_pool ( ) WHERE time_deleted IS NULL; +/* + * Index ensuring uniqueness of IP pools by rack ID + */ +CREATE UNIQUE INDEX ON omicron.public.ip_pool ( + rack_id +) WHERE + rack_id IS NOT NULL AND + time_deleted IS NULL; + /* * IP Pools are made up of a set of IP ranges, which are start/stop addresses. * Note that these need not be CIDR blocks or well-behaved subnets with a diff --git a/nexus/db-model/src/ip_pool.rs b/nexus/db-model/src/ip_pool.rs index 137351edd05..75da25e86e2 100644 --- a/nexus/db-model/src/ip_pool.rs +++ b/nexus/db-model/src/ip_pool.rs @@ -31,6 +31,11 @@ pub struct IpPool { /// An optional ID of the project for which this pool is reserved. pub project_id: Option, + /// An optional ID of the rack for which this pool is reserved. + // TODO(https://github.com/oxidecomputer/omicron/issues/1276): This + // should probably point to an AZ or fleet, not a rack. + pub rack_id: Option, + /// Child resource generation number, for optimistic concurrency control of /// the contained ranges. pub rcgen: i64, @@ -40,6 +45,7 @@ impl IpPool { pub fn new( pool_identity: &external::IdentityMetadataCreateParams, project_id: Option, + rack_id: Option, ) -> Self { Self { identity: IpPoolIdentity::new( @@ -47,6 +53,7 @@ impl IpPool { pool_identity.clone(), ), project_id, + rack_id, rcgen: 0, } } diff --git a/nexus/db-model/src/schema.rs b/nexus/db-model/src/schema.rs index 811f9bb20cf..3cf057942f7 100644 --- a/nexus/db-model/src/schema.rs +++ b/nexus/db-model/src/schema.rs @@ -144,6 +144,7 @@ table! { time_modified -> Timestamptz, time_deleted -> Nullable, project_id -> Nullable, + rack_id -> Nullable, rcgen -> Int8, } } diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index e67069907af..a9c2fd6f4f9 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -128,4 +128,51 @@ impl super::Nexus { .await?; self.db_datastore.ip_pool_delete_range(opctx, &authz_pool, range).await } + + pub async fn ip_pool_service_list_ranges( + &self, + opctx: &OpContext, + rack_id: Uuid, + pagparams: &DataPageParams<'_, IpNetwork>, + ) -> ListResultVec { + let (authz_pool, ..) = self + .db_datastore + .ip_pools_lookup_by_rack_id( + opctx, + authz::Action::ListChildren, + rack_id, + ) + .await?; + self.db_datastore + .ip_pool_list_ranges(opctx, &authz_pool, pagparams) + .await + } + + pub async fn ip_pool_service_add_range( + &self, + opctx: &OpContext, + rack_id: Uuid, + range: &IpRange, + ) -> UpdateResult { + let (authz_pool, db_pool) = self + .db_datastore + .ip_pools_lookup_by_rack_id(opctx, authz::Action::Modify, rack_id) + .await?; + self.db_datastore + .ip_pool_add_range(opctx, &authz_pool, &db_pool, range) + .await + } + + pub async fn ip_pool_service_delete_range( + &self, + opctx: &OpContext, + rack_id: Uuid, + range: &IpRange, + ) -> DeleteResult { + let (authz_pool, ..) = self + .db_datastore + .ip_pools_lookup_by_rack_id(opctx, authz::Action::Modify, rack_id) + .await?; + self.db_datastore.ip_pool_delete_range(opctx, &authz_pool, range).await + } } diff --git a/nexus/src/db/datastore/ip_pool.rs b/nexus/src/db/datastore/ip_pool.rs index f9ea58958ab..984b6e1c6f1 100644 --- a/nexus/src/db/datastore/ip_pool.rs +++ b/nexus/src/db/datastore/ip_pool.rs @@ -32,6 +32,7 @@ use omicron_common::api::external::DataPageParams; use omicron_common::api::external::DeleteResult; use omicron_common::api::external::Error; use omicron_common::api::external::ListResultVec; +use omicron_common::api::external::LookupResult; use omicron_common::api::external::LookupType; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; @@ -56,6 +57,37 @@ impl DataStore { .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } + pub async fn ip_pools_lookup_by_rack_id( + &self, + opctx: &OpContext, + action: authz::Action, + rack_id: Uuid, + ) -> LookupResult<(authz::IpPool, IpPool)> { + use db::schema::ip_pool::dsl; + + opctx.authorize(authz::Action::Read, &authz::IP_POOL_LIST).await?; + let (authz_pool, pool) = dsl::ip_pool + .filter(dsl::rack_id.eq(Some(rack_id))) + .filter(dsl::time_deleted.is_null()) + .select(IpPool::as_select()) + .get_result_async(self.pool_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + .map(|ip_pool| { + ( + authz::IpPool::new( + authz::FLEET, + ip_pool.id(), + LookupType::ByCompositeId(rack_id.to_string()), + ), + ip_pool, + ) + })?; + opctx.authorize(action, &authz_pool).await?; + + Ok((authz_pool, pool)) + } + /// List IP Pools by their IDs pub async fn ip_pools_list_by_id( &self, @@ -94,7 +126,8 @@ impl DataStore { Some(authz_project.id()) } }; - let pool = IpPool::new(&new_pool.identity, project_id); + let rack_id = None; + let pool = IpPool::new(&new_pool.identity, project_id, rack_id); let pool_name = pool.name().as_str().to_string(); diesel::insert_into(dsl::ip_pool) .values(pool) diff --git a/nexus/src/db/queries/external_ip.rs b/nexus/src/db/queries/external_ip.rs index ffe66c22542..7e032e38a65 100644 --- a/nexus/src/db/queries/external_ip.rs +++ b/nexus/src/db/queries/external_ip.rs @@ -662,7 +662,8 @@ mod tests { name: String::from(name).parse().unwrap(), description: format!("ip pool {}", name), }, - None, + /* project_id= */ None, + /* rack_id= */ None, ); pool.project_id = project_id; diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 92c9a8b6945..9e92401d518 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -101,16 +101,24 @@ pub fn external_api() -> NexusApiDescription { api.register(project_policy_view)?; api.register(project_policy_update)?; + // Customer-Accessible IP Pools API api.register(ip_pool_list)?; api.register(ip_pool_create)?; api.register(ip_pool_view)?; api.register(ip_pool_delete)?; api.register(ip_pool_update)?; + // Customer-Accessible IP Pool Range API (used by instances) api.register(ip_pool_range_list)?; api.register(ip_pool_range_add)?; api.register(ip_pool_range_remove)?; + // Operator-Accessible IP Pool Range API (used by Oxide services) + api.register(ip_pool_service_range_list)?; + api.register(ip_pool_service_range_add)?; + api.register(ip_pool_service_range_remove)?; + + api.register(disk_list)?; api.register(disk_list)?; api.register(disk_create)?; api.register(disk_view)?; @@ -1361,6 +1369,106 @@ async fn ip_pool_range_remove( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +#[derive(Deserialize, JsonSchema)] +pub struct IpPoolServicePathParam { + pub rack_id: Uuid, +} + +/// List ranges for an IP pool used for Oxide services. +/// +/// Ranges are ordered by their first address. +#[endpoint { + method = GET, + path = "/ip-pools-service/{rack_id}/ranges", + tags = ["ip-pools"], +}] +async fn ip_pool_service_range_list( + rqctx: Arc>>, + path_params: Path, + query_params: Query, +) -> Result>, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let query = query_params.into_inner(); + let path = path_params.into_inner(); + let rack_id = path.rack_id; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let marker = match query.page { + WhichPage::First(_) => None, + WhichPage::Next(ref addr) => Some(addr), + }; + let pag_params = DataPageParams { + limit: rqctx.page_limit(&query)?, + direction: PaginationOrder::Ascending, + marker, + }; + let ranges = nexus + .ip_pool_service_list_ranges(&opctx, rack_id, &pag_params) + .await? + .into_iter() + .map(|range| range.into()) + .collect(); + Ok(HttpResponseOk(ResultsPage::new( + ranges, + &EmptyScanParams {}, + |range: &IpPoolRange, _| { + IpNetwork::from(range.range.first_address()) + }, + )?)) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/// Add a range to an IP pool used for Oxide services. +#[endpoint { + method = POST, + path = "/ip-pools-service/{rack_id}/ranges/add", + tags = ["ip-pools"], +}] +async fn ip_pool_service_range_add( + rqctx: Arc>>, + path_params: Path, + range_params: TypedBody, +) -> Result, HttpError> { + let apictx = &rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let rack_id = path.rack_id; + let range = range_params.into_inner(); + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let out = + nexus.ip_pool_service_add_range(&opctx, rack_id, &range).await?; + Ok(HttpResponseCreated(out.into())) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/// Remove a range from an IP pool used for Oxide services. +#[endpoint { + method = POST, + path = "/ip-pools-service/{rack_id}/ranges/remove", + tags = ["ip-pools"], +}] +async fn ip_pool_service_range_remove( + rqctx: Arc>>, + path_params: Path, + range_params: TypedBody, +) -> Result { + let apictx = &rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let rack_id = path.rack_id; + let range = range_params.into_inner(); + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + nexus.ip_pool_service_delete_range(&opctx, rack_id, &range).await?; + Ok(HttpResponseUpdatedNoContent()) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + // Disks /// List disks From 86f968d5c3f1006a06c4fe68e82ef76001d75d6d Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 1 Aug 2022 15:58:19 -0400 Subject: [PATCH 2/9] remove duplicated 'disk_list' (was a typo) --- nexus/src/external_api/http_entrypoints.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 9e92401d518..3fe9971bdbf 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -118,7 +118,6 @@ pub fn external_api() -> NexusApiDescription { api.register(ip_pool_service_range_add)?; api.register(ip_pool_service_range_remove)?; - api.register(disk_list)?; api.register(disk_list)?; api.register(disk_create)?; api.register(disk_view)?; From 867950d025d11fee1671f8cd530fa05fd92a1833 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 1 Aug 2022 16:00:16 -0400 Subject: [PATCH 3/9] docs --- nexus/src/app/ip_pool.rs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index a9c2fd6f4f9..04be4db3c97 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -129,6 +129,13 @@ impl super::Nexus { self.db_datastore.ip_pool_delete_range(opctx, &authz_pool, range).await } + // The "ip_pool_service_..." functions look up IP pools for Oxide service usage, + // rather than for VMs. As such, they're identified by rack UUID, not + // by pool names. + // + // TODO(https://github.com/oxidecomputer/omicron/issues/1276): Should be + // AZ UUID, probably. + pub async fn ip_pool_service_list_ranges( &self, opctx: &OpContext, From 5d5ef0abc44aa740e61f671a10e658cb663fd258 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 1 Aug 2022 16:14:38 -0400 Subject: [PATCH 4/9] openapi --- nexus/tests/output/nexus_tags.txt | 3 + openapi/nexus.json | 154 ++++++++++++++++++++++++++++++ 2 files changed, 157 insertions(+) diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index 17002ee17d1..ba5898e716c 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -70,6 +70,9 @@ ip_pool_list /ip-pools ip_pool_range_add /ip-pools/{pool_name}/ranges/add ip_pool_range_list /ip-pools/{pool_name}/ranges ip_pool_range_remove /ip-pools/{pool_name}/ranges/remove +ip_pool_service_range_add /ip-pools-service/{rack_id}/ranges/add +ip_pool_service_range_list /ip-pools-service/{rack_id}/ranges +ip_pool_service_range_remove /ip-pools-service/{rack_id}/ranges/remove ip_pool_update /ip-pools/{pool_name} ip_pool_view /ip-pools/{pool_name} diff --git a/openapi/nexus.json b/openapi/nexus.json index 2829591442c..0b845db62f6 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -1294,6 +1294,160 @@ } } }, + "/ip-pools-service/{rack_id}/ranges": { + "get": { + "tags": [ + "ip-pools" + ], + "summary": "List ranges for an IP pool used for Oxide services.", + "description": "Ranges are ordered by their first address.", + "operationId": "ip_pool_service_range_list", + "parameters": [ + { + "in": "path", + "name": "rack_id", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + }, + "style": "simple" + }, + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + }, + "style": "form" + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + }, + "style": "form" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IpPoolRangeResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": true + } + }, + "/ip-pools-service/{rack_id}/ranges/add": { + "post": { + "tags": [ + "ip-pools" + ], + "summary": "Add a range to an IP pool used for Oxide services.", + "operationId": "ip_pool_service_range_add", + "parameters": [ + { + "in": "path", + "name": "rack_id", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + }, + "style": "simple" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IpRange" + } + } + }, + "required": true + }, + "responses": { + "201": { + "description": "successful creation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IpPoolRange" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, + "/ip-pools-service/{rack_id}/ranges/remove": { + "post": { + "tags": [ + "ip-pools" + ], + "summary": "Remove a range from an IP pool used for Oxide services.", + "operationId": "ip_pool_service_range_remove", + "parameters": [ + { + "in": "path", + "name": "rack_id", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + }, + "style": "simple" + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IpRange" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/login": { "post": { "tags": [ From 1528831cb7f3dbdf07710f44f093e37b3d56e6ba Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Mon, 1 Aug 2022 23:14:28 -0400 Subject: [PATCH 5/9] Passing unauthorized tests now --- nexus/src/app/ip_pool.rs | 24 ++++++--- nexus/src/db/datastore/ip_pool.rs | 34 ++++++++++--- nexus/tests/integration_tests/endpoints.rs | 49 +++++++++++++++++++ nexus/tests/integration_tests/unauthorized.rs | 20 ++++++++ 4 files changed, 113 insertions(+), 14 deletions(-) diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index 04be4db3c97..f9e6885f95d 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -26,7 +26,16 @@ impl super::Nexus { opctx: &OpContext, new_pool: ¶ms::IpPoolCreate, ) -> CreateResult { - self.db_datastore.ip_pool_create(opctx, new_pool).await + self.db_datastore.ip_pool_create(opctx, new_pool, None).await + } + + pub async fn ip_pool_services_create( + &self, + opctx: &OpContext, + new_pool: ¶ms::IpPoolCreate, + rack_id: Uuid, + ) -> CreateResult { + self.db_datastore.ip_pool_create(opctx, new_pool, Some(rack_id)).await } pub async fn ip_pools_list_by_name( @@ -144,12 +153,9 @@ impl super::Nexus { ) -> ListResultVec { let (authz_pool, ..) = self .db_datastore - .ip_pools_lookup_by_rack_id( - opctx, - authz::Action::ListChildren, - rack_id, - ) + .ip_pools_lookup_by_rack_id(opctx, rack_id) .await?; + opctx.authorize(authz::Action::Read, &authz_pool).await?; self.db_datastore .ip_pool_list_ranges(opctx, &authz_pool, pagparams) .await @@ -163,8 +169,9 @@ impl super::Nexus { ) -> UpdateResult { let (authz_pool, db_pool) = self .db_datastore - .ip_pools_lookup_by_rack_id(opctx, authz::Action::Modify, rack_id) + .ip_pools_lookup_by_rack_id(opctx, rack_id) .await?; + opctx.authorize(authz::Action::Modify, &authz_pool).await?; self.db_datastore .ip_pool_add_range(opctx, &authz_pool, &db_pool, range) .await @@ -178,8 +185,9 @@ impl super::Nexus { ) -> DeleteResult { let (authz_pool, ..) = self .db_datastore - .ip_pools_lookup_by_rack_id(opctx, authz::Action::Modify, rack_id) + .ip_pools_lookup_by_rack_id(opctx, rack_id) .await?; + opctx.authorize(authz::Action::Modify, &authz_pool).await?; self.db_datastore.ip_pool_delete_range(opctx, &authz_pool, range).await } } diff --git a/nexus/src/db/datastore/ip_pool.rs b/nexus/src/db/datastore/ip_pool.rs index 984b6e1c6f1..ef6e6b53875 100644 --- a/nexus/src/db/datastore/ip_pool.rs +++ b/nexus/src/db/datastore/ip_pool.rs @@ -60,12 +60,24 @@ impl DataStore { pub async fn ip_pools_lookup_by_rack_id( &self, opctx: &OpContext, - action: authz::Action, rack_id: Uuid, ) -> LookupResult<(authz::IpPool, IpPool)> { use db::schema::ip_pool::dsl; - opctx.authorize(authz::Action::Read, &authz::IP_POOL_LIST).await?; + // Ensure the caller has the ability to look up these IP pools. + // If they don't, return "not found" instead of "forbidden". + opctx + .authorize(authz::Action::ListChildren, &authz::IP_POOL_LIST) + .await + .map_err(|e| match e { + Error::Forbidden => { + LookupType::ByCompositeId(format!("Rack ID: {rack_id}")) + .into_not_found(ResourceType::IpPool) + } + _ => e, + })?; + + // Look up this IP pool by rack ID. let (authz_pool, pool) = dsl::ip_pool .filter(dsl::rack_id.eq(Some(rack_id))) .filter(dsl::time_deleted.is_null()) @@ -78,13 +90,13 @@ impl DataStore { authz::IpPool::new( authz::FLEET, ip_pool.id(), - LookupType::ByCompositeId(rack_id.to_string()), + LookupType::ByCompositeId(format!( + "Rack ID: {rack_id}" + )), ), ip_pool, ) })?; - opctx.authorize(action, &authz_pool).await?; - Ok((authz_pool, pool)) } @@ -106,10 +118,15 @@ impl DataStore { .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } + /// Creates a new IP pool. + /// + /// - If `rack_id` is provided, this IP pool is used for Oxide + /// services. pub async fn ip_pool_create( &self, opctx: &OpContext, new_pool: ¶ms::IpPoolCreate, + rack_id: Option, ) -> CreateResult { use db::schema::ip_pool::dsl; opctx @@ -118,6 +135,12 @@ impl DataStore { let project_id = match new_pool.project.clone() { None => None, Some(project) => { + if let Some(_) = &rack_id { + return Err(Error::invalid_request( + "Internal Service IP pools cannot be project-scoped", + )); + } + let (.., authz_project) = LookupPath::new(opctx, self) .organization_name(&Name(project.organization)) .project_name(&Name(project.project)) @@ -126,7 +149,6 @@ impl DataStore { Some(authz_project.id()) } }; - let rack_id = None; let pool = IpPool::new(&new_pool.identity, project_id, rack_id); let pool_name = pool.name().as_str().to_string(); diesel::insert_into(dsl::ip_pool) diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index 2abbd6c0cfd..b77baf70669 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -315,6 +315,21 @@ lazy_static! { pub static ref DEMO_IP_POOL_RANGES_ADD_URL: String = format!("{}/add", *DEMO_IP_POOL_RANGES_URL); pub static ref DEMO_IP_POOL_RANGES_DEL_URL: String = format!("{}/remove", *DEMO_IP_POOL_RANGES_URL); + // IP Pools (Services) + pub static ref DEMO_IP_POOL_SERVICE_CREATE: params::IpPoolCreate = + params::IpPoolCreate { + identity: IdentityMetadataCreateParams { + name: "pool1".parse::().unwrap(), + description: String::from("an IP pool"), + }, + project: None, + }; + pub static ref DEMO_IP_POOLS_SERVICE_URL: &'static str = "/ip-pools-service"; + pub static ref DEMO_IP_POOL_SERVICE_URL: String = format!("{}/{}", *DEMO_IP_POOLS_SERVICE_URL, RACK_UUID); + pub static ref DEMO_IP_POOL_SERVICE_RANGES_URL: String = format!("{}/ranges", *DEMO_IP_POOL_SERVICE_URL); + pub static ref DEMO_IP_POOL_SERVICE_RANGES_ADD_URL: String = format!("{}/add", *DEMO_IP_POOL_SERVICE_RANGES_URL); + pub static ref DEMO_IP_POOL_SERVICE_RANGES_DEL_URL: String = format!("{}/remove", *DEMO_IP_POOL_SERVICE_RANGES_URL); + // Snapshots pub static ref DEMO_SNAPSHOT_NAME: Name = "demo-snapshot".parse().unwrap(); pub static ref DEMO_SNAPSHOT_URL: String = @@ -586,6 +601,40 @@ lazy_static! { ], }, + // IP Pool ranges endpoint (Oxide services) + VerifyEndpoint { + url: &*DEMO_IP_POOL_SERVICE_RANGES_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get + ], + }, + + // IP Pool ranges/add endpoint (Oxide services) + VerifyEndpoint { + url: &*DEMO_IP_POOL_SERVICE_RANGES_ADD_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Post( + serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() + ), + ], + }, + + // IP Pool ranges/delete endpoint (Oxide services) + VerifyEndpoint { + url: &*DEMO_IP_POOL_SERVICE_RANGES_DEL_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Post( + serde_json::to_value(&*DEMO_IP_POOL_RANGE).unwrap() + ), + ], + }, + /* Silos */ VerifyEndpoint { url: "/silos", diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index f1b8619ee8e..217f26cd21c 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -23,6 +23,8 @@ use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external::IdentityMetadata; use omicron_nexus::authn::external::spoof; +use std::str::FromStr; +use uuid::Uuid; // This test hits a list Nexus API endpoints using both unauthenticated and // unauthorized requests to make sure we get the expected behavior (generally: @@ -55,6 +57,24 @@ use omicron_nexus::authn::external::spoof; async fn test_unauthorized(cptestctx: &ControlPlaneTestContext) { DiskTest::new(cptestctx).await; let client = &cptestctx.external_client; + + let nexus = &cptestctx.server.apictx.nexus; + + // Creation of IP pools for internal services is not exposed through the + // API. We create an IP pool manually as a workaround. + let opctx = omicron_nexus::context::OpContext::for_tests( + cptestctx.logctx.log.new(o!()), + nexus.datastore().clone(), + ); + nexus + .ip_pool_services_create( + &opctx, + &DEMO_IP_POOL_SERVICE_CREATE, + Uuid::from_str(nexus_test_utils::RACK_UUID).unwrap(), + ) + .await + .unwrap(); + let log = &cptestctx.logctx.log; let mut setup_results = std::collections::BTreeMap::new(); From 899d0a52586c344eb7896bb3d8e5f0a3576ae512 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 2 Aug 2022 12:08:01 -0400 Subject: [PATCH 6/9] Limit overlap of service/non-service IP pools, add getter API --- nexus/src/app/ip_pool.rs | 52 ++++++++-- nexus/src/authz/omicron.polar | 2 + nexus/src/db/datastore/ip_pool.rs | 45 +++++---- nexus/src/external_api/http_entrypoints.rs | 25 +++++ nexus/src/populate.rs | 19 ++++ nexus/tests/integration_tests/endpoints.rs | 18 ++-- nexus/tests/integration_tests/unauthorized.rs | 20 ---- nexus/tests/output/nexus_tags.txt | 2 + openapi/nexus.json | 99 +++++++++++++++++++ 9 files changed, 228 insertions(+), 54 deletions(-) diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index f9e6885f95d..bcddb77190a 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -15,8 +15,10 @@ use ipnetwork::IpNetwork; 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::ListResultVec; use omicron_common::api::external::LookupResult; +use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use uuid::Uuid; @@ -100,10 +102,18 @@ impl super::Nexus { pool_name: &Name, pagparams: &DataPageParams<'_, IpNetwork>, ) -> ListResultVec { - let (.., authz_pool) = LookupPath::new(opctx, &self.db_datastore) - .ip_pool_name(pool_name) - .lookup_for(authz::Action::ListChildren) - .await?; + let (.., authz_pool, db_pool) = + LookupPath::new(opctx, &self.db_datastore) + .ip_pool_name(pool_name) + .fetch_for(authz::Action::ListChildren) + .await?; + if db_pool.rack_id.is_some() { + return Err(Error::not_found_by_name( + ResourceType::IpPool, + pool_name, + )); + } + self.db_datastore .ip_pool_list_ranges(opctx, &authz_pool, pagparams) .await @@ -120,6 +130,12 @@ impl super::Nexus { .ip_pool_name(pool_name) .fetch_for(authz::Action::Modify) .await?; + if db_pool.rack_id.is_some() { + return Err(Error::not_found_by_name( + ResourceType::IpPool, + pool_name, + )); + } self.db_datastore .ip_pool_add_range(opctx, &authz_pool, &db_pool, range) .await @@ -131,10 +147,17 @@ impl super::Nexus { pool_name: &Name, range: &IpRange, ) -> DeleteResult { - let (.., authz_pool) = LookupPath::new(opctx, &self.db_datastore) - .ip_pool_name(pool_name) - .lookup_for(authz::Action::Modify) - .await?; + let (.., authz_pool, db_pool) = + LookupPath::new(opctx, &self.db_datastore) + .ip_pool_name(pool_name) + .fetch_for(authz::Action::Modify) + .await?; + if db_pool.rack_id.is_some() { + return Err(Error::not_found_by_name( + ResourceType::IpPool, + pool_name, + )); + } self.db_datastore.ip_pool_delete_range(opctx, &authz_pool, range).await } @@ -145,6 +168,19 @@ impl super::Nexus { // TODO(https://github.com/oxidecomputer/omicron/issues/1276): Should be // AZ UUID, probably. + pub async fn ip_pool_service_fetch( + &self, + opctx: &OpContext, + rack_id: Uuid, + ) -> LookupResult { + let (authz_pool, db_pool) = self + .db_datastore + .ip_pools_lookup_by_rack_id(opctx, rack_id) + .await?; + opctx.authorize(authz::Action::Read, &authz_pool).await?; + Ok(db_pool) + } + pub async fn ip_pool_service_list_ranges( &self, opctx: &OpContext, diff --git a/nexus/src/authz/omicron.polar b/nexus/src/authz/omicron.polar index c9e414e0f56..cc1675494c3 100644 --- a/nexus/src/authz/omicron.polar +++ b/nexus/src/authz/omicron.polar @@ -436,3 +436,5 @@ has_permission(_actor: AuthenticatedActor, "query", _resource: Database); # The "db-init" user is the only one with the "init" role. has_permission(actor: AuthenticatedActor, "modify", _resource: Database) if actor = USER_DB_INIT; +has_permission(actor: AuthenticatedActor, "create_child", _resource: IpPoolList) + if actor = USER_DB_INIT; diff --git a/nexus/src/db/datastore/ip_pool.rs b/nexus/src/db/datastore/ip_pool.rs index ef6e6b53875..7d3a40ae390 100644 --- a/nexus/src/db/datastore/ip_pool.rs +++ b/nexus/src/db/datastore/ip_pool.rs @@ -50,6 +50,7 @@ impl DataStore { .authorize(authz::Action::ListChildren, &authz::IP_POOL_LIST) .await?; paginated(dsl::ip_pool, dsl::name, pagparams) + .filter(dsl::rack_id.is_null()) .filter(dsl::time_deleted.is_null()) .select(db::model::IpPool::as_select()) .get_results_async(self.pool_authorized(opctx).await?) @@ -57,6 +58,30 @@ impl DataStore { .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) } + /// List IP Pools by their IDs + pub async fn ip_pools_list_by_id( + &self, + opctx: &OpContext, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + use db::schema::ip_pool::dsl; + opctx + .authorize(authz::Action::ListChildren, &authz::IP_POOL_LIST) + .await?; + paginated(dsl::ip_pool, dsl::id, pagparams) + .filter(dsl::rack_id.is_null()) + .filter(dsl::time_deleted.is_null()) + .select(db::model::IpPool::as_select()) + .get_results_async(self.pool_authorized(opctx).await?) + .await + .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) + } + + /// Looks up an IP pool by a particular Rack ID. + /// + /// An index exists to look up pools by rack ID, but it is not a primary + /// key, which requires this lookup function to be used instead of the + /// [`LookupPath`] utility. pub async fn ip_pools_lookup_by_rack_id( &self, opctx: &OpContext, @@ -100,24 +125,6 @@ impl DataStore { Ok((authz_pool, pool)) } - /// List IP Pools by their IDs - pub async fn ip_pools_list_by_id( - &self, - opctx: &OpContext, - pagparams: &DataPageParams<'_, Uuid>, - ) -> ListResultVec { - use db::schema::ip_pool::dsl; - opctx - .authorize(authz::Action::ListChildren, &authz::IP_POOL_LIST) - .await?; - paginated(dsl::ip_pool, dsl::id, pagparams) - .filter(dsl::time_deleted.is_null()) - .select(db::model::IpPool::as_select()) - .get_results_async(self.pool_authorized(opctx).await?) - .await - .map_err(|e| public_error_from_diesel_pool(e, ErrorHandler::Server)) - } - /// Creates a new IP pool. /// /// - If `rack_id` is provided, this IP pool is used for Oxide @@ -198,6 +205,7 @@ impl DataStore { // in between the above check for children and this query. let now = Utc::now(); let updated_rows = diesel::update(dsl::ip_pool) + .filter(dsl::rack_id.is_null()) .filter(dsl::time_deleted.is_null()) .filter(dsl::id.eq(authz_pool.id())) .filter(dsl::rcgen.eq(db_pool.rcgen)) @@ -229,6 +237,7 @@ impl DataStore { use db::schema::ip_pool::dsl; opctx.authorize(authz::Action::Modify, authz_pool).await?; diesel::update(dsl::ip_pool) + .filter(dsl::rack_id.is_null()) .filter(dsl::id.eq(authz_pool.id())) .filter(dsl::time_deleted.is_null()) .set(updates) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 3fe9971bdbf..978e7287517 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -108,6 +108,9 @@ pub fn external_api() -> NexusApiDescription { api.register(ip_pool_delete)?; api.register(ip_pool_update)?; + // Operator-Accessible IP Pools API + api.register(ip_pool_service_view)?; + // Customer-Accessible IP Pool Range API (used by instances) api.register(ip_pool_range_list)?; api.register(ip_pool_range_add)?; @@ -1272,6 +1275,28 @@ async fn ip_pool_update( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +/// Fetch an IP pool +#[endpoint { + method = GET, + path = "/ip-pools-service/{rack_id}", + tags = ["ip-pools"], +}] +async fn ip_pool_service_view( + rqctx: Arc>>, + path_params: Path, +) -> Result, HttpError> { + let apictx = rqctx.context(); + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let rack_id = path.rack_id; + let handler = async { + let opctx = OpContext::for_external_api(&rqctx).await?; + let pool = nexus.ip_pool_service_fetch(&opctx, rack_id).await?; + Ok(HttpResponseOk(IpPool::from(pool))) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + type IpPoolRangePaginationParams = PaginationParams; /// List ranges for an IP pool diff --git a/nexus/src/populate.rs b/nexus/src/populate.rs index 85223aef2b1..062e314c219 100644 --- a/nexus/src/populate.rs +++ b/nexus/src/populate.rs @@ -44,10 +44,13 @@ use crate::context::OpContext; use crate::db::{self, DataStore}; +use crate::external_api::params; use futures::future::BoxFuture; use futures::FutureExt; use lazy_static::lazy_static; use omicron_common::api::external::Error; +use omicron_common::api::external::IdentityMetadataCreateParams; +use omicron_common::api::external::Name; use omicron_common::backoff; use std::sync::Arc; use uuid::Uuid; @@ -279,6 +282,22 @@ impl Populator for PopulateRack { datastore .rack_insert(opctx, &db::model::Rack::new(args.rack_id)) .await?; + + let params = params::IpPoolCreate { + identity: IdentityMetadataCreateParams { + name: "oxide-service-pool".parse::().unwrap(), + description: String::from("IP Pool for Oxide Services"), + }, + project: None, + }; + datastore + .ip_pool_create(opctx, ¶ms, Some(args.rack_id)) + .await + .map(|_| ()) + .or_else(|e| match e { + Error::ObjectAlreadyExists { .. } => Ok(()), + _ => Err(e), + })?; Ok(()) } .boxed() diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index b77baf70669..b40b3c60362 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -316,14 +316,6 @@ lazy_static! { pub static ref DEMO_IP_POOL_RANGES_DEL_URL: String = format!("{}/remove", *DEMO_IP_POOL_RANGES_URL); // IP Pools (Services) - pub static ref DEMO_IP_POOL_SERVICE_CREATE: params::IpPoolCreate = - params::IpPoolCreate { - identity: IdentityMetadataCreateParams { - name: "pool1".parse::().unwrap(), - description: String::from("an IP pool"), - }, - project: None, - }; pub static ref DEMO_IP_POOLS_SERVICE_URL: &'static str = "/ip-pools-service"; pub static ref DEMO_IP_POOL_SERVICE_URL: String = format!("{}/{}", *DEMO_IP_POOLS_SERVICE_URL, RACK_UUID); pub static ref DEMO_IP_POOL_SERVICE_RANGES_URL: String = format!("{}/ranges", *DEMO_IP_POOL_SERVICE_URL); @@ -601,6 +593,16 @@ lazy_static! { ], }, + // IP Pool endpoint (Oxide services) + VerifyEndpoint { + url: &*DEMO_IP_POOL_SERVICE_URL, + visibility: Visibility::Protected, + unprivileged_access: UnprivilegedAccess::None, + allowed_methods: vec![ + AllowedMethod::Get + ], + }, + // IP Pool ranges endpoint (Oxide services) VerifyEndpoint { url: &*DEMO_IP_POOL_SERVICE_RANGES_URL, diff --git a/nexus/tests/integration_tests/unauthorized.rs b/nexus/tests/integration_tests/unauthorized.rs index 217f26cd21c..f1b8619ee8e 100644 --- a/nexus/tests/integration_tests/unauthorized.rs +++ b/nexus/tests/integration_tests/unauthorized.rs @@ -23,8 +23,6 @@ use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external::IdentityMetadata; use omicron_nexus::authn::external::spoof; -use std::str::FromStr; -use uuid::Uuid; // This test hits a list Nexus API endpoints using both unauthenticated and // unauthorized requests to make sure we get the expected behavior (generally: @@ -57,24 +55,6 @@ use uuid::Uuid; async fn test_unauthorized(cptestctx: &ControlPlaneTestContext) { DiskTest::new(cptestctx).await; let client = &cptestctx.external_client; - - let nexus = &cptestctx.server.apictx.nexus; - - // Creation of IP pools for internal services is not exposed through the - // API. We create an IP pool manually as a workaround. - let opctx = omicron_nexus::context::OpContext::for_tests( - cptestctx.logctx.log.new(o!()), - nexus.datastore().clone(), - ); - nexus - .ip_pool_services_create( - &opctx, - &DEMO_IP_POOL_SERVICE_CREATE, - Uuid::from_str(nexus_test_utils::RACK_UUID).unwrap(), - ) - .await - .unwrap(); - let log = &cptestctx.logctx.log; let mut setup_results = std::collections::BTreeMap::new(); diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index ba5898e716c..1afd9626781 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -70,9 +70,11 @@ ip_pool_list /ip-pools ip_pool_range_add /ip-pools/{pool_name}/ranges/add ip_pool_range_list /ip-pools/{pool_name}/ranges ip_pool_range_remove /ip-pools/{pool_name}/ranges/remove +ip_pool_service_list /ip-pools-service ip_pool_service_range_add /ip-pools-service/{rack_id}/ranges/add ip_pool_service_range_list /ip-pools-service/{rack_id}/ranges ip_pool_service_range_remove /ip-pools-service/{rack_id}/ranges/remove +ip_pool_service_view /ip-pools-service/{rack_id} ip_pool_update /ip-pools/{pool_name} ip_pool_view /ip-pools/{pool_name} diff --git a/openapi/nexus.json b/openapi/nexus.json index 0b845db62f6..4d9416a6894 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -1294,6 +1294,105 @@ } } }, + "/ip-pools-service": { + "get": { + "tags": [ + "ip-pools" + ], + "summary": "List IP pools", + "operationId": "ip_pool_service_list", + "parameters": [ + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + }, + "style": "form" + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + }, + "style": "form" + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/NameOrIdSortMode" + }, + "style": "form" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IpPoolResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": true + } + }, + "/ip-pools-service/{rack_id}": { + "get": { + "tags": [ + "ip-pools" + ], + "summary": "Fetch an IP pool", + "operationId": "ip_pool_service_view", + "parameters": [ + { + "in": "path", + "name": "rack_id", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + }, + "style": "simple" + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IpPool" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/ip-pools-service/{rack_id}/ranges": { "get": { "tags": [ From e265f02b4b8eaec89dbac64a4d62c522363ba790 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 2 Aug 2022 12:23:25 -0400 Subject: [PATCH 7/9] Update tests, openapi --- nexus/tests/integration_tests/ip_pools.rs | 84 +++++++++++++++++++++++ nexus/tests/output/nexus_tags.txt | 1 - openapi/nexus.json | 60 ---------------- 3 files changed, 84 insertions(+), 61 deletions(-) diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index 4566388e767..0c469957fcb 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -679,6 +679,90 @@ async fn test_ip_range_delete_with_allocated_external_ip_fails( ); } +#[nexus_test] +async fn test_ip_pool_service(cptestctx: &ControlPlaneTestContext) { + let client = &cptestctx.external_client; + let ip_pool_url = + format!("/ip-pools-service/{}", nexus_test_utils::RACK_UUID); + let ip_pool_ranges_url = format!("{}/ranges", ip_pool_url); + let ip_pool_add_range_url = format!("{}/add", ip_pool_ranges_url); + let ip_pool_remove_range_url = format!("{}/remove", ip_pool_ranges_url); + + // View the pool, which should exist without explicit creation. + let fetched_pool: IpPool = NexusRequest::object_get(client, &ip_pool_url) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!(fetched_pool.identity.name, "oxide-service-pool"); + assert_eq!(fetched_pool.identity.description, "IP Pool for Oxide Services"); + + // Add some ranges. Pagination is tested more explicitly in the IP pool + // implementation, but we just check that these endpoints work here. + let ranges = [ + IpRange::V4( + Ipv4Range::new( + std::net::Ipv4Addr::new(10, 0, 0, 1), + std::net::Ipv4Addr::new(10, 0, 0, 2), + ) + .unwrap(), + ), + IpRange::V6( + Ipv6Range::new( + std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 0), + std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 10), + ) + .unwrap(), + ), + ]; + + let mut expected_ranges = Vec::with_capacity(ranges.len()); + for range in ranges.iter() { + let created_range: IpPoolRange = + NexusRequest::objects_post(client, &ip_pool_add_range_url, &range) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap() + .parsed_body() + .unwrap(); + assert_eq!(range.first_address(), created_range.range.first_address()); + assert_eq!(range.last_address(), created_range.range.last_address()); + expected_ranges.push(created_range); + } + expected_ranges + .sort_by(|a, b| a.range.first_address().cmp(&b.range.first_address())); + + // List the ranges. + let first_page = + objects_list_page_authz::(client, &ip_pool_ranges_url) + .await; + assert_eq!(first_page.items.len(), ranges.len()); + + let actual_ranges = first_page.items.iter(); + for (expected_range, actual_range) in + expected_ranges.iter().zip(actual_ranges) + { + assert_ranges_eq(expected_range, actual_range); + } + + // Remove both ranges, observe that the IP Pool is empty. + for range in ranges.iter() { + NexusRequest::objects_post(client, &ip_pool_remove_range_url, &range) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .unwrap(); + } + + let first_page = + objects_list_page_authz::(client, &ip_pool_ranges_url) + .await; + assert!(first_page.items.is_empty()); +} + fn assert_pools_eq(first: &IpPool, second: &IpPool) { assert_eq!(first.identity, second.identity); } diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index 1afd9626781..f670745a411 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -70,7 +70,6 @@ ip_pool_list /ip-pools ip_pool_range_add /ip-pools/{pool_name}/ranges/add ip_pool_range_list /ip-pools/{pool_name}/ranges ip_pool_range_remove /ip-pools/{pool_name}/ranges/remove -ip_pool_service_list /ip-pools-service ip_pool_service_range_add /ip-pools-service/{rack_id}/ranges/add ip_pool_service_range_list /ip-pools-service/{rack_id}/ranges ip_pool_service_range_remove /ip-pools-service/{rack_id}/ranges/remove diff --git a/openapi/nexus.json b/openapi/nexus.json index 4d9416a6894..a4a857fcc34 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -1294,66 +1294,6 @@ } } }, - "/ip-pools-service": { - "get": { - "tags": [ - "ip-pools" - ], - "summary": "List IP pools", - "operationId": "ip_pool_service_list", - "parameters": [ - { - "in": "query", - "name": "limit", - "description": "Maximum number of items returned by a single call", - "schema": { - "nullable": true, - "type": "integer", - "format": "uint32", - "minimum": 1 - }, - "style": "form" - }, - { - "in": "query", - "name": "page_token", - "description": "Token returned by previous call to retrieve the subsequent page", - "schema": { - "nullable": true, - "type": "string" - }, - "style": "form" - }, - { - "in": "query", - "name": "sort_by", - "schema": { - "$ref": "#/components/schemas/NameOrIdSortMode" - }, - "style": "form" - } - ], - "responses": { - "200": { - "description": "successful operation", - "content": { - "application/json": { - "schema": { - "$ref": "#/components/schemas/IpPoolResultsPage" - } - } - } - }, - "4XX": { - "$ref": "#/components/responses/Error" - }, - "5XX": { - "$ref": "#/components/responses/Error" - } - }, - "x-dropshot-pagination": true - } - }, "/ip-pools-service/{rack_id}": { "get": { "tags": [ From 73af7b44700bb378854c79d31b1b68f3c2f9f954 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 2 Aug 2022 12:57:16 -0400 Subject: [PATCH 8/9] fix tests wanting no content --- nexus/tests/integration_tests/ip_pools.rs | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index 0c469957fcb..238324fc016 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -750,11 +750,19 @@ async fn test_ip_pool_service(cptestctx: &ControlPlaneTestContext) { // Remove both ranges, observe that the IP Pool is empty. for range in ranges.iter() { - NexusRequest::objects_post(client, &ip_pool_remove_range_url, &range) - .authn_as(AuthnMode::PrivilegedUser) - .execute() - .await - .unwrap(); + NexusRequest::new( + RequestBuilder::new( + client, + Method::POST, + &ip_pool_remove_range_url, + ) + .body(Some(&range)) + .expect_status(Some(StatusCode::NO_CONTENT)), + ) + .authn_as(AuthnMode::PrivilegedUser) + .execute() + .await + .expect("Failed to delete IP range from a pool"); } let first_page = From 77ce28d1abe8f518ed8e0ff2f8514938ff49d2d0 Mon Sep 17 00:00:00 2001 From: Sean Klein Date: Tue, 2 Aug 2022 19:18:58 -0400 Subject: [PATCH 9/9] Review feedback, but fix openapi too --- nexus/src/external_api/http_entrypoints.rs | 2 +- openapi/nexus.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 978e7287517..2b0a6af814a 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1275,7 +1275,7 @@ async fn ip_pool_update( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } -/// Fetch an IP pool +/// Fetch an IP pool used for Oxide services. #[endpoint { method = GET, path = "/ip-pools-service/{rack_id}", diff --git a/openapi/nexus.json b/openapi/nexus.json index a4a857fcc34..bd1ca1e29ff 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -1299,7 +1299,7 @@ "tags": [ "ip-pools" ], - "summary": "Fetch an IP pool", + "summary": "Fetch an IP pool used for Oxide services.", "operationId": "ip_pool_service_view", "parameters": [ {