From 1de49fc0f9a333dd98d59eb4297323d65d5224d3 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Wed, 11 Oct 2023 11:08:49 -0500 Subject: [PATCH] let's add an endpoint! --- nexus/src/external_api/http_entrypoints.rs | 31 ++++++++ nexus/test-utils/src/resource_helpers.rs | 14 ++++ nexus/tests/integration_tests/endpoints.rs | 2 - nexus/tests/integration_tests/instances.rs | 4 +- nexus/tests/integration_tests/ip_pools.rs | 46 +++++------ nexus/tests/output/nexus_tags.txt | 1 + nexus/types/src/external_api/params.rs | 1 + openapi/nexus.json | 92 ++++++++++++++++------ 8 files changed, 139 insertions(+), 52 deletions(-) diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 8ee78a1b07..5f502306e8 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -116,6 +116,7 @@ pub(crate) fn external_api() -> NexusApiDescription { // Operator-Accessible IP Pools API api.register(ip_pool_list)?; api.register(ip_pool_create)?; + api.register(ip_pool_associate)?; api.register(ip_pool_view)?; api.register(ip_pool_delete)?; api.register(ip_pool_update)?; @@ -1300,6 +1301,36 @@ async fn ip_pool_update( apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } +// TODO: associate just seems like the wrong word and I'd like to change it +// across the board. What I really mean is "make available to" or "make availale +// for use in" +/// Associate an IP Pool with a silo or project +#[endpoint { + method = POST, + path = "/v1/system/ip-pools/{pool}/associate", + tags = ["system/networking"], +}] +async fn ip_pool_associate( + rqctx: RequestContext>, + path_params: Path, + resource_assoc: TypedBody, + // TODO: what does this return? Returning the association record seems silly +) -> Result { + let apictx = rqctx.context(); + let handler = async { + let opctx = crate::context::op_context_for_external_api(&rqctx).await?; + let nexus = &apictx.nexus; + let path = path_params.into_inner(); + let resource_assoc = resource_assoc.into_inner(); + let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?; + nexus + .ip_pool_associate_resource(&opctx, &pool_lookup, &resource_assoc) + .await?; + Ok(HttpResponseUpdatedNoContent()) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + /// Fetch the IP pool used for Oxide services #[endpoint { method = GET, diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index 35a4f6242b..ffbfca1d8e 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -12,6 +12,7 @@ use dropshot::test_util::ClientTestContext; use dropshot::HttpErrorResponseBody; use dropshot::Method; use http::StatusCode; +use nexus_db_queries::db::fixed_data::FLEET_ID; use nexus_test_interface::NexusServer; use nexus_types::external_api::params; use nexus_types::external_api::params::PhysicalDiskKind; @@ -143,6 +144,19 @@ pub async fn create_ip_pool( }, ) .await; + + // match previous behavior, makes pool available for use anywhere in fleet + let _: () = object_create( + client, + &format!("/v1/system/ip-pools/{pool_name}/associate"), + ¶ms::IpPoolResource { + resource_id: *FLEET_ID, + resource_type: params::IpPoolResourceType::Fleet, + is_default: false, + }, + ) + .await; + // TODO: associate with fleet as a non-default like before? let range = populate_ip_pool(client, pool_name, ip_range).await; (pool, range) diff --git a/nexus/tests/integration_tests/endpoints.rs b/nexus/tests/integration_tests/endpoints.rs index e9ae11c21f..0f7a0d6070 100644 --- a/nexus/tests/integration_tests/endpoints.rs +++ b/nexus/tests/integration_tests/endpoints.rs @@ -456,8 +456,6 @@ lazy_static! { name: DEMO_IP_POOL_NAME.clone(), description: String::from("an IP pool"), }, - silo: None, - is_default: true, }; pub static ref DEMO_IP_POOL_PROJ_URL: String = format!("/v1/ip-pools/{}?project={}", *DEMO_IP_POOL_NAME, *DEMO_PROJECT_NAME); diff --git a/nexus/tests/integration_tests/instances.rs b/nexus/tests/integration_tests/instances.rs index 83fff2fbab..fff61bd7c4 100644 --- a/nexus/tests/integration_tests/instances.rs +++ b/nexus/tests/integration_tests/instances.rs @@ -3416,8 +3416,8 @@ async fn test_instance_ephemeral_ip_from_correct_pool( name: pool_name.parse().unwrap(), description: String::from("an ip pool"), }, - silo: Some(NameOrId::Id(DEFAULT_SILO.id())), - is_default: true, + // silo: Some(NameOrId::Id(DEFAULT_SILO.id())), + // is_default: true, }, ) .await; diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index 639ccd788a..5084ab4f6f 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -28,7 +28,7 @@ use nexus_types::external_api::shared::Ipv6Range; use nexus_types::external_api::views::IpPool; use nexus_types::external_api::views::IpPoolRange; use omicron_common::api::external::IdentityMetadataUpdateParams; -use omicron_common::api::external::NameOrId; +// use omicron_common::api::external::NameOrId; use omicron_common::api::external::{IdentityMetadataCreateParams, Name}; use omicron_nexus::TestInterfaces; use sled_agent_client::TestInterfaces as SledTestInterfaces; @@ -61,8 +61,8 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) { assert_eq!(ip_pools.len(), 1, "Expected to see default IP pool"); assert_eq!(ip_pools[0].identity.name, "default",); - assert_eq!(ip_pools[0].silo_id, None); - assert!(ip_pools[0].is_default); + // assert_eq!(ip_pools[0].silo_id, None); + // assert!(ip_pools[0].is_default); // Verify 404 if the pool doesn't exist yet, both for creating or deleting let error: HttpErrorResponseBody = NexusRequest::expect_failure( @@ -105,8 +105,8 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) { name: String::from(pool_name).parse().unwrap(), description: String::from(description), }, - silo: None, - is_default: false, + // silo: None, + // is_default: false, }; let created_pool: IpPool = NexusRequest::objects_post(client, ip_pools_url, ¶ms) @@ -118,7 +118,7 @@ async fn test_ip_pool_basic_crud(cptestctx: &ControlPlaneTestContext) { .unwrap(); assert_eq!(created_pool.identity.name, pool_name); assert_eq!(created_pool.identity.description, description); - assert_eq!(created_pool.silo_id, None); + // assert_eq!(created_pool.silo_id, None); let list = NexusRequest::iter_collection_authn::( client, @@ -288,11 +288,10 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) { // silo: Some(NameOrId::Name(cptestctx.silo_name.clone())), // is_default: false, }; - let created_pool = create_pool(client, ¶ms).await; - assert_eq!(created_pool.identity.name, "p0"); + let _created_pool = create_pool(client, ¶ms).await; - let silo_id = - created_pool.silo_id.expect("Expected pool to have a silo_id"); + // let silo_id = + // created_pool.silo_id.expect("Expected pool to have a silo_id"); // now we'll create another IP pool using that silo ID let params = IpPoolCreate { @@ -300,12 +299,11 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) { name: String::from("p1").parse().unwrap(), description: String::from(""), }, - silo: Some(NameOrId::Id(silo_id)), - is_default: false, + // silo: Some(NameOrId::Id(silo_id)), + // is_default: false, }; - let created_pool = create_pool(client, ¶ms).await; - assert_eq!(created_pool.identity.name, "p1"); - assert_eq!(created_pool.silo_id.unwrap(), silo_id); + let _created_pool = create_pool(client, ¶ms).await; + // assert_eq!(created_pool.silo_id.unwrap(), silo_id); // expect 404 if the specified silo doesn't exist let bad_silo_params = IpPoolCreate { @@ -313,10 +311,10 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) { name: String::from("p2").parse().unwrap(), description: String::from(""), }, - silo: Some(NameOrId::Name( - String::from("not-a-thing").parse().unwrap(), - )), - is_default: false, + // silo: Some(NameOrId::Name( + // String::from("not-a-thing").parse().unwrap(), + // )), + // is_default: false, }; let error: HttpErrorResponseBody = NexusRequest::new( RequestBuilder::new(client, Method::POST, "/v1/system/ip-pools") @@ -374,8 +372,8 @@ async fn test_ip_pool_range_overlapping_ranges_fails( name: String::from(pool_name).parse().unwrap(), description: String::from(description), }, - silo: None, - is_default: false, + // silo: None, + // is_default: false, }; let created_pool: IpPool = NexusRequest::objects_post(client, ip_pools_url, ¶ms) @@ -557,8 +555,6 @@ async fn test_ip_pool_range_pagination(cptestctx: &ControlPlaneTestContext) { name: String::from(pool_name).parse().unwrap(), description: String::from(description), }, - silo: None, - is_default: false, }; let created_pool: IpPool = NexusRequest::objects_post(client, ip_pools_url, ¶ms) @@ -695,8 +691,8 @@ async fn test_ip_pool_list_usable_by_project( name: String::from(mypool_name).parse().unwrap(), description: String::from("right on cue"), }, - silo: None, - is_default: false, + // silo: None, + // is_default: false, }; NexusRequest::objects_post(client, ip_pools_url, ¶ms) .authn_as(AuthnMode::PrivilegedUser) diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index 1d7f5556c2..06a5651622 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -129,6 +129,7 @@ system_metric GET /v1/system/metrics/{metric_nam API operations found with tag "system/networking" OPERATION ID METHOD URL PATH +ip_pool_associate POST /v1/system/ip-pools/{pool}/associate ip_pool_create POST /v1/system/ip-pools ip_pool_delete DELETE /v1/system/ip-pools/{pool} ip_pool_list GET /v1/system/ip-pools diff --git a/nexus/types/src/external_api/params.rs b/nexus/types/src/external_api/params.rs index 7fa984b4c0..4bba13a530 100644 --- a/nexus/types/src/external_api/params.rs +++ b/nexus/types/src/external_api/params.rs @@ -741,6 +741,7 @@ pub struct IpPoolUpdate { } #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] +#[serde(rename_all = "snake_case")] pub enum IpPoolResourceType { Fleet, Silo, diff --git a/openapi/nexus.json b/openapi/nexus.json index 9dda94f283..23948208da 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -4577,6 +4577,47 @@ } } }, + "/v1/system/ip-pools/{pool}/associate": { + "post": { + "tags": [ + "system/networking" + ], + "summary": "Associate an IP Pool with a silo or project", + "operationId": "ip_pool_associate", + "parameters": [ + { + "in": "path", + "name": "pool", + "description": "Name or ID of the IP pool", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IpPoolResource" + } + } + }, + "required": true + }, + "responses": { + "204": { + "description": "resource updated" + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + } + } + }, "/v1/system/ip-pools/{pool}/ranges": { "get": { "tags": [ @@ -11512,9 +11553,6 @@ "type": "string", "format": "uuid" }, - "is_default": { - "type": "boolean" - }, "name": { "description": "unique, mutable, user-controlled identifier for each resource", "allOf": [ @@ -11523,11 +11561,6 @@ } ] }, - "silo_id": { - "nullable": true, - "type": "string", - "format": "uuid" - }, "time_created": { "description": "timestamp when this resource was created", "type": "string", @@ -11542,7 +11575,6 @@ "required": [ "description", "id", - "is_default", "name", "time_created", "time_modified" @@ -11555,22 +11587,8 @@ "description": { "type": "string" }, - "is_default": { - "description": "Whether the IP pool is considered a default pool for its scope (fleet or silo). If a pool is marked default and is associated with a silo, instances created in that silo will draw IPs from that pool unless another pool is specified at instance create time.", - "default": false, - "type": "boolean" - }, "name": { "$ref": "#/components/schemas/Name" - }, - "silo": { - "nullable": true, - "description": "If an IP pool is associated with a silo, instance IP allocations in that silo can draw from that pool.", - "allOf": [ - { - "$ref": "#/components/schemas/NameOrId" - } - ] } }, "required": [ @@ -11625,6 +11643,34 @@ "items" ] }, + "IpPoolResource": { + "description": "Parameters for associating an IP pool with a resource (fleet, silo)", + "type": "object", + "properties": { + "is_default": { + "type": "boolean" + }, + "resource_id": { + "type": "string", + "format": "uuid" + }, + "resource_type": { + "$ref": "#/components/schemas/IpPoolResourceType" + } + }, + "required": [ + "is_default", + "resource_id", + "resource_type" + ] + }, + "IpPoolResourceType": { + "type": "string", + "enum": [ + "fleet", + "silo" + ] + }, "IpPoolResultsPage": { "description": "A single page of results", "type": "object",