diff --git a/nexus/db-model/src/ip_pool.rs b/nexus/db-model/src/ip_pool.rs index 78bc21c09c..b6f496c99e 100644 --- a/nexus/db-model/src/ip_pool.rs +++ b/nexus/db-model/src/ip_pool.rs @@ -97,6 +97,25 @@ pub struct IpPoolResource { pub resource_id: Uuid, pub is_default: bool, } +impl From for views::IpPoolResourceType { + fn from(typ: IpPoolResourceType) -> Self { + match typ { + IpPoolResourceType::Fleet => Self::Fleet, + IpPoolResourceType::Silo => Self::Silo, + } + } +} + +impl From for views::IpPoolResource { + fn from(assoc: IpPoolResource) -> Self { + Self { + ip_pool_id: assoc.ip_pool_id, + resource_type: assoc.resource_type.into(), + resource_id: assoc.resource_id, + is_default: assoc.is_default, + } + } +} /// A range of IP addresses for an IP Pool. #[derive(Queryable, Insertable, Selectable, Clone, Debug)] diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index aee5a53d5a..c3df60c27a 100644 --- a/nexus/db-queries/src/db/datastore/ip_pool.rs +++ b/nexus/db-queries/src/db/datastore/ip_pool.rs @@ -312,6 +312,31 @@ impl DataStore { }) } + pub async fn ip_pool_association_list( + &self, + opctx: &OpContext, + authz_pool: &authz::IpPool, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + use db::schema::ip_pool; + use db::schema::ip_pool_resource; + + paginated( + ip_pool_resource::table, + ip_pool_resource::ip_pool_id, + pagparams, + ) + .inner_join(ip_pool::table) + .filter(ip_pool::id.eq(authz_pool.id())) + .filter(ip_pool::time_deleted.is_null()) + .select(IpPoolResource::as_select()) + .load_async::( + &*self.pool_connection_authorized(opctx).await?, + ) + .await + .map_err(|e| public_error_from_diesel(e, ErrorHandler::Server)) + } + pub async fn ip_pool_associate_resource( &self, opctx: &OpContext, @@ -559,6 +584,8 @@ mod test { use omicron_common::api::external::{Error, IdentityMetadataCreateParams}; use omicron_test_utils::dev; + // TODO: add calls to the list endpoint throughout all this + #[tokio::test] async fn test_default_ip_pools() { let logctx = dev::test_setup_log("test_default_ip_pools"); diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index da115cc972..95b1e3adbb 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -11,7 +11,6 @@ use nexus_db_queries::authz; use nexus_db_queries::context::OpContext; use nexus_db_queries::db; use nexus_db_queries::db::fixed_data::FLEET_ID; -// use nexus_db_queries::db::fixed_data::silo::INTERNAL_SILO_ID; use nexus_db_queries::db::lookup; use nexus_db_queries::db::lookup::LookupPath; use nexus_db_queries::db::model::Name; @@ -26,6 +25,7 @@ use omicron_common::api::external::NameOrId; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use ref_cast::RefCast; +use uuid::Uuid; /// Helper to make it easier to 404 on attempts to manipulate internal pools fn not_found_from_lookup(pool_lookup: &lookup::IpPool<'_>) -> Error { @@ -72,6 +72,20 @@ impl super::Nexus { self.db_datastore.ip_pool_create(opctx, pool).await } + pub(crate) async fn ip_pool_association_list( + &self, + opctx: &OpContext, + pool_lookup: &lookup::IpPool<'_>, + pagparams: &DataPageParams<'_, Uuid>, + ) -> ListResultVec { + // TODO: is this the right action to check? + let (.., authz_pool) = + pool_lookup.lookup_for(authz::Action::ListChildren).await?; + self.db_datastore + .ip_pool_association_list(opctx, &authz_pool, pagparams) + .await + } + pub(crate) async fn ip_pool_associate_resource( &self, opctx: &OpContext, diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 03ad0fbbdc..1b5bfaf25d 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -121,6 +121,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_association_list)?; api.register(ip_pool_association_create)?; api.register(ip_pool_association_delete)?; api.register(ip_pool_view)?; @@ -1322,7 +1323,48 @@ async fn ip_pool_update( // 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 +/// List IP pool resource associations +#[endpoint { + method = GET, + path = "/v1/system/ip-pools/{pool}/associations", + tags = ["system/networking"], +}] +async fn ip_pool_association_list( + rqctx: RequestContext>, + path_params: Path, + // paginating by resource_id because they're unique per pool. most robust + // option would be to paginate by a composite key representing the (pool, + // resource_type, resource) + query_params: 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 query = query_params.into_inner(); + let pag_params = data_page_params_for(&rqctx, &query)?; + + let path = path_params.into_inner(); + let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?; + + let assocs = nexus + .ip_pool_association_list(&opctx, &pool_lookup, &pag_params) + .await? + .into_iter() + .map(|assoc| assoc.into()) + .collect(); + + Ok(HttpResponseOk(ScanById::results_page( + &query, + assocs, + &|_, x: &views::IpPoolResource| x.resource_id, + )?)) + }; + apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await +} + +/// Associate an IP Pool with a silo or the fleet #[endpoint { method = POST, path = "/v1/system/ip-pools/{pool}/associations", @@ -1332,7 +1374,6 @@ async fn ip_pool_association_create( rqctx: RequestContext>, path_params: Path, resource_assoc: TypedBody, - // TODO: what does this return? Returning the association record seems silly ) -> Result, HttpError> { let apictx = rqctx.context(); let handler = async { @@ -1341,10 +1382,10 @@ async fn ip_pool_association_create( let path = path_params.into_inner(); let resource_assoc = resource_assoc.into_inner(); let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?; - nexus + let assoc = nexus .ip_pool_associate_resource(&opctx, &pool_lookup, &resource_assoc) .await?; - Ok(HttpResponseCreated(views::IpPoolResource {})) + Ok(HttpResponseCreated(assoc.into())) }; apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await } diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index 62ce4d2dde..bb62749689 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -6,6 +6,7 @@ use dropshot::test_util::ClientTestContext; use dropshot::HttpErrorResponseBody; +use dropshot::ResultsPage; use http::method::Method; use http::StatusCode; use nexus_db_queries::db::datastore::SERVICE_IP_POOL_NAME; @@ -31,6 +32,7 @@ use nexus_types::external_api::shared::Ipv6Range; use nexus_types::external_api::views::IpPool; use nexus_types::external_api::views::IpPoolRange; use nexus_types::external_api::views::IpPoolResource; +use nexus_types::external_api::views::IpPoolResourceType; use nexus_types::external_api::views::Silo; use omicron_common::api::external::IdentityMetadataUpdateParams; use omicron_common::api::external::NameOrId; @@ -346,8 +348,12 @@ async fn test_ip_pool_service_no_cud(cptestctx: &ControlPlaneTestContext) { async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; - let _created_pool = create_pool(client, "p0").await; - let _created_pool = create_pool(client, "p1").await; + let p0 = create_pool(client, "p0").await; + let p1 = create_pool(client, "p1").await; + + // there should be no associations + let assocs_p0 = get_associations(client, "p0").await; + assert_eq!(assocs_p0.items.len(), 0); // expect 404 on association if the specified silo doesn't exist let nonexistent_silo_id = Uuid::new_v4(); @@ -361,7 +367,7 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) { RequestBuilder::new( client, Method::POST, - "/v1/system/ip-pools/p1/associations", + "/v1/system/ip-pools/p0/associations", ) .body(Some(¶ms)) .expect_status(Some(StatusCode::NOT_FOUND)), @@ -378,7 +384,7 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) { format!("not found: silo with id \"{nonexistent_silo_id}\"") ); - // associate with silo that exists + // associate by name with silo that exists let params = params::IpPoolAssociationCreate::Silo(params::IpPoolAssociateSilo { // TODO: this is probably not the best silo ID to use @@ -387,24 +393,33 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) { }); let _: IpPoolResource = object_create( client, - &format!("/v1/system/ip-pools/p1/associations"), + &format!("/v1/system/ip-pools/p0/associations"), ¶ms, ) .await; - // TODO: test assocation worked, or at least comes back in association list - - // get silo ID so we can test assocation by ID as well + // get silo ID so we can test association by ID as well let silo_url = format!("/v1/system/silos/{}", cptestctx.silo_name); let silo = NexusRequest::object_get(client, &silo_url) .authn_as(AuthnMode::PrivilegedUser) .execute_and_parse_unwrap::() .await; + let silo_id = silo.identity.id; + + let assocs_p0 = get_associations(client, "p0").await; + let silo_assoc = IpPoolResource { + ip_pool_id: p0.identity.id, + resource_type: IpPoolResourceType::Silo, + resource_id: silo_id, + is_default: false, + }; + assert_eq!(assocs_p0.items.len(), 1); + assert_eq!(assocs_p0.items[0], silo_assoc); // TODO: dissociate silo // TODO: confirm dissociation - // associate same silo by ID + // associate same silo to other pool by ID let params = params::IpPoolAssociationCreate::Silo(params::IpPoolAssociateSilo { silo: NameOrId::Id(silo.identity.id), @@ -417,6 +432,14 @@ async fn test_ip_pool_with_silo(cptestctx: &ControlPlaneTestContext) { ) .await; + // association should look the same as the other one, except different pool ID + let assocs_p1 = get_associations(client, "p1").await; + assert_eq!(assocs_p1.items.len(), 1); + assert_eq!( + assocs_p1.items[0], + IpPoolResource { ip_pool_id: p1.identity.id, ..silo_assoc } + ); + // TODO: associating a resource that is already associated should be a noop // and return a success message @@ -466,6 +489,17 @@ fn get_names(pools: Vec) -> Vec { pools.iter().map(|p| p.identity.name.to_string()).collect() } +async fn get_associations( + client: &ClientTestContext, + id: &str, +) -> ResultsPage { + objects_list_page_authz::( + client, + &format!("/v1/system/ip-pools/{}/associations", id), + ) + .await +} + async fn create_pool(client: &ClientTestContext, name: &str) -> IpPool { let params = IpPoolCreate { identity: IdentityMetadataCreateParams { diff --git a/nexus/tests/output/nexus_tags.txt b/nexus/tests/output/nexus_tags.txt index 2039c33812..df2fdeed19 100644 --- a/nexus/tests/output/nexus_tags.txt +++ b/nexus/tests/output/nexus_tags.txt @@ -131,6 +131,7 @@ API operations found with tag "system/networking" OPERATION ID METHOD URL PATH ip_pool_association_create POST /v1/system/ip-pools/{pool}/associations ip_pool_association_delete DELETE /v1/system/ip-pools/{pool}/associations +ip_pool_association_list GET /v1/system/ip-pools/{pool}/associations 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/views.rs b/nexus/types/src/external_api/views.rs index c2cba6f3e0..b6dcad8553 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -246,9 +246,22 @@ pub struct IpPool { pub identity: IdentityMetadata, } -// TODO: placeholder response for IP pool associate POST -#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct IpPoolResource {} +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)] +#[serde(rename_all = "snake_case")] +pub enum IpPoolResourceType { + Fleet, + Silo, +} + +#[derive(Clone, Debug, Deserialize, Serialize, JsonSchema, PartialEq)] +pub struct IpPoolResource { + // TODO: is including the pool ID redundant? it's convenient to have and + // makes this response a cohesive whole + pub ip_pool_id: Uuid, + pub resource_type: IpPoolResourceType, + pub resource_id: Uuid, + pub is_default: bool, +} #[derive(Clone, Copy, Debug, Deserialize, Serialize, JsonSchema)] pub struct IpPoolRange { diff --git a/openapi/nexus.json b/openapi/nexus.json index dc0a406496..eadc31367c 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -4578,11 +4578,77 @@ } }, "/v1/system/ip-pools/{pool}/associations": { + "get": { + "tags": [ + "system/networking" + ], + "summary": "List IP pool resource associations", + "operationId": "ip_pool_association_list", + "parameters": [ + { + "in": "path", + "name": "pool", + "description": "Name or ID of the IP pool", + "required": true, + "schema": { + "$ref": "#/components/schemas/NameOrId" + } + }, + { + "in": "query", + "name": "limit", + "description": "Maximum number of items returned by a single call", + "schema": { + "nullable": true, + "type": "integer", + "format": "uint32", + "minimum": 1 + } + }, + { + "in": "query", + "name": "page_token", + "description": "Token returned by previous call to retrieve the subsequent page", + "schema": { + "nullable": true, + "type": "string" + } + }, + { + "in": "query", + "name": "sort_by", + "schema": { + "$ref": "#/components/schemas/IdSortMode" + } + } + ], + "responses": { + "200": { + "description": "successful operation", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/IpPoolResourceResultsPage" + } + } + } + }, + "4XX": { + "$ref": "#/components/responses/Error" + }, + "5XX": { + "$ref": "#/components/responses/Error" + } + }, + "x-dropshot-pagination": { + "required": [] + } + }, "post": { "tags": [ "system/networking" ], - "summary": "Associate an IP Pool with a silo or project", + "summary": "Associate an IP Pool with a silo or the fleet", "operationId": "ip_pool_association_create", "parameters": [ { @@ -12455,7 +12521,57 @@ ] }, "IpPoolResource": { - "type": "object" + "type": "object", + "properties": { + "ip_pool_id": { + "type": "string", + "format": "uuid" + }, + "is_default": { + "type": "boolean" + }, + "resource_id": { + "type": "string", + "format": "uuid" + }, + "resource_type": { + "$ref": "#/components/schemas/IpPoolResourceType" + } + }, + "required": [ + "ip_pool_id", + "is_default", + "resource_id", + "resource_type" + ] + }, + "IpPoolResourceResultsPage": { + "description": "A single page of results", + "type": "object", + "properties": { + "items": { + "description": "list of items on this page of results", + "type": "array", + "items": { + "$ref": "#/components/schemas/IpPoolResource" + } + }, + "next_page": { + "nullable": true, + "description": "token used to fetch the next page of results (if any)", + "type": "string" + } + }, + "required": [ + "items" + ] + }, + "IpPoolResourceType": { + "type": "string", + "enum": [ + "fleet", + "silo" + ] }, "IpPoolResultsPage": { "description": "A single page of results", diff --git a/schema/crdb/9.0.0/up4.sql b/schema/crdb/9.0.0/up4.sql index edcad062a8..52ce081453 100644 --- a/schema/crdb/9.0.0/up4.sql +++ b/schema/crdb/9.0.0/up4.sql @@ -1,4 +1,4 @@ --- copy existing fleet assocations into association table. treat all existing +-- copy existing fleet associations into association table. treat all existing -- pools as fleet-associated because that is the current behavior INSERT INTO ip_pool_resource (ip_pool_id, resource_type, resource_id, is_default) SELECT id, 'fleet', '001de000-1334-4000-8000-000000000000', is_default diff --git a/schema/crdb/9.0.0/up5.sql b/schema/crdb/9.0.0/up5.sql index 92f2201b86..656b45027b 100644 --- a/schema/crdb/9.0.0/up5.sql +++ b/schema/crdb/9.0.0/up5.sql @@ -1,4 +1,4 @@ --- copy existing ip_pool-to-silo assocations into association table +-- copy existing ip_pool-to-silo associations into association table INSERT INTO ip_pool_resource (ip_pool_id, resource_type, resource_id, is_default) SELECT id, 'silo', silo_id, is_default FROM ip_pool