diff --git a/end-to-end-tests/src/bin/bootstrap.rs b/end-to-end-tests/src/bin/bootstrap.rs index 320547b960..b02bed4265 100644 --- a/end-to-end-tests/src/bin/bootstrap.rs +++ b/end-to-end-tests/src/bin/bootstrap.rs @@ -4,8 +4,8 @@ use end_to_end_tests::helpers::{generate_name, get_system_ip_pool}; use omicron_test_utils::dev::poll::{wait_for_condition, CondCheckError}; use oxide_client::types::{ ByteCount, DeviceAccessTokenRequest, DeviceAuthRequest, DeviceAuthVerify, - DiskCreate, DiskSource, IpPoolCreate, IpPoolLinkSilo, Ipv4Range, NameOrId, - SiloQuotasUpdate, + DiskCreate, DiskSource, IpPoolCreate, IpPoolLinkSilo, IpRange, Ipv4Range, + NameOrId, SiloQuotasUpdate, }; use oxide_client::{ ClientDisksExt, ClientHiddenExt, ClientProjectsExt, @@ -60,7 +60,7 @@ async fn main() -> Result<()> { client .ip_pool_range_add() .pool(pool_name) - .body(Ipv4Range { first, last }) + .body(IpRange::V4(Ipv4Range { first, last })) .send() .await?; diff --git a/nexus/src/app/ip_pool.rs b/nexus/src/app/ip_pool.rs index 87a7d98c91..fd73a18355 100644 --- a/nexus/src/app/ip_pool.rs +++ b/nexus/src/app/ip_pool.rs @@ -27,6 +27,7 @@ use omicron_common::api::external::NameOrId; use omicron_common::api::external::ResourceType; use omicron_common::api::external::UpdateResult; use ref_cast::RefCast; +use std::matches; use uuid::Uuid; /// Helper to make it easier to 404 on attempts to manipulate internal pools @@ -291,6 +292,19 @@ impl super::Nexus { return Err(not_found_from_lookup(pool_lookup)); } + // Disallow V6 ranges until IPv6 is fully supported by the networking + // subsystem. Instead of changing the API to reflect that (making this + // endpoint inconsistent with the rest) and changing it back when we + // add support, we accept them at the API layer and error here. It + // would be nice if we could do it in the datastore layer, but we'd + // have no way of creating IPv6 ranges for the purpose of testing IP + // pool utilization. + if matches!(range, IpRange::V6(_)) { + return Err(Error::invalid_request( + "IPv6 ranges are not allowed yet", + )); + } + self.db_datastore.ip_pool_add_range(opctx, &authz_pool, range).await } @@ -347,6 +361,18 @@ impl super::Nexus { let (authz_pool, ..) = self.db_datastore.ip_pools_service_lookup(opctx).await?; opctx.authorize(authz::Action::Modify, &authz_pool).await?; + // Disallow V6 ranges until IPv6 is fully supported by the networking + // subsystem. Instead of changing the API to reflect that (making this + // endpoint inconsistent with the rest) and changing it back when we + // add support, we accept them at the API layer and error here. It + // would be nice if we could do it in the datastore layer, but we'd + // have no way of creating IPv6 ranges for the purpose of testing IP + // pool utilization. + if matches!(range, IpRange::V6(_)) { + return Err(Error::invalid_request( + "IPv6 ranges are not allowed yet", + )); + } self.db_datastore.ip_pool_add_range(opctx, &authz_pool, range).await } diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 7910f9e985..41e5bce08a 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -1789,6 +1789,8 @@ async fn ip_pool_range_list( } /// Add range to IP pool +/// +/// IPv6 ranges are not allowed yet. #[endpoint { method = POST, path = "/v1/system/ip-pools/{pool}/ranges/add", @@ -1797,17 +1799,14 @@ async fn ip_pool_range_list( async fn ip_pool_range_add( rqctx: RequestContext>, path_params: Path, - // The plan is to change this back to shared::IpRange once IPv6 works - range_params: TypedBody, + range_params: TypedBody, ) -> 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 path = path_params.into_inner(); - // This is a bit of a hack to keep the service functions general with - // respect to IP version while disallowing IPv6 ranges at the API layer - let range = shared::IpRange::V4(range_params.into_inner()); + let range = range_params.into_inner(); let pool_lookup = nexus.ip_pool_lookup(&opctx, &path.pool)?; let out = nexus.ip_pool_add_range(&opctx, &pool_lookup, &range).await?; Ok(HttpResponseCreated(out.into())) @@ -1824,8 +1823,6 @@ async fn ip_pool_range_add( async fn ip_pool_range_remove( rqctx: RequestContext>, path_params: Path, - // Note that add is restricted to IPv4 while remove allows v4 or v6. This is - // meant as a safeguard in case existing systems have v6 ranges to remove. range_params: TypedBody, ) -> Result { let apictx = &rqctx.context(); @@ -1885,6 +1882,8 @@ async fn ip_pool_service_range_list( } /// Add IP range to Oxide service pool +/// +/// IPv6 ranges are not allowed yet. #[endpoint { method = POST, path = "/v1/system/ip-pools-service/ranges/add", @@ -1892,16 +1891,13 @@ async fn ip_pool_service_range_list( }] async fn ip_pool_service_range_add( rqctx: RequestContext>, - // The plan is to change this back to shared::IpRange once IPv6 works - range_params: TypedBody, + range_params: TypedBody, ) -> Result, HttpError> { let apictx = &rqctx.context(); let handler = async { let opctx = crate::context::op_context_for_external_api(&rqctx).await?; let nexus = &apictx.nexus; - // This is a bit of a hack to keep the service functions general with - // respect to IP version while disallowing IPv6 ranges at the API layer - let range = shared::IpRange::V4(range_params.into_inner()); + let range = range_params.into_inner(); let out = nexus.ip_pool_service_add_range(&opctx, &range).await?; Ok(HttpResponseCreated(out.into())) }; @@ -1916,8 +1912,6 @@ async fn ip_pool_service_range_add( }] async fn ip_pool_service_range_remove( rqctx: RequestContext>, - // Note that add is restricted to IPv4 while remove allows v4 or v6. This is - // meant as a safeguard in case existing systems have v6 ranges to remove. range_params: TypedBody, ) -> Result { let apictx = &rqctx.context(); diff --git a/nexus/tests/integration_tests/ip_pools.rs b/nexus/tests/integration_tests/ip_pools.rs index 4a66fbcfa1..cb5eade735 100644 --- a/nexus/tests/integration_tests/ip_pools.rs +++ b/nexus/tests/integration_tests/ip_pools.rs @@ -9,6 +9,8 @@ use dropshot::HttpErrorResponseBody; use dropshot::ResultsPage; use http::method::Method; use http::StatusCode; +use nexus_db_queries::authz; +use nexus_db_queries::context::OpContext; use nexus_db_queries::db::datastore::SERVICE_IP_POOL_NAME; use nexus_db_queries::db::fixed_data::silo::DEFAULT_SILO; use nexus_db_queries::db::fixed_data::silo::INTERNAL_SILO_ID; @@ -47,6 +49,7 @@ use nexus_types::external_api::views::SiloIpPool; use nexus_types::identity::Resource; use omicron_common::address::Ipv6Range; use omicron_common::api::external::IdentityMetadataUpdateParams; +use omicron_common::api::external::LookupType; use omicron_common::api::external::NameOrId; use omicron_common::api::external::SimpleIdentity; use omicron_common::api::external::{IdentityMetadataCreateParams, Name}; @@ -765,7 +768,7 @@ async fn create_pool(client: &ClientTestContext, name: &str) -> IpPool { async fn test_ip_pool_utilization_total(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; - create_pool(client, "p0").await; + let pool = create_pool(client, "p0").await; assert_ip_pool_utilization(client, "p0", 0, 0, 0, 0).await; @@ -783,20 +786,36 @@ async fn test_ip_pool_utilization_total(cptestctx: &ControlPlaneTestContext) { assert_ip_pool_utilization(client, "p0", 0, 5, 0, 0).await; - // now let's add a gigantic range just for fun + // Now let's add a gigantic range. This requires direct datastore + // shenanigans because adding IPv6 ranges through the API is currently not + // allowed. It's worth doing because we want this code to correctly handle + // IPv6 ranges when they are allowed again. + + let nexus = &cptestctx.server.apictx().nexus; + let datastore = nexus.datastore(); + let log = cptestctx.logctx.log.new(o!()); + let opctx = OpContext::for_tests(log, datastore.clone()); + let authz_pool = authz::IpPool::new( + authz::FLEET, + pool.identity.id, + LookupType::ByName("p0".to_string()), + ); + let big_range = IpRange::V6( Ipv6Range::new( - std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 1), + std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 0), std::net::Ipv6Addr::new( - 0xfd00, 0, 0, 0, 0xffff, 0xfff, 0xffff, 0xffff, + 0xfd00, 0, 0, 0xffff, 0xffff, 0xffff, 0xffff, 0xffff, ), ) .unwrap(), ); - object_create::(client, &add_url, &big_range).await; + datastore + .ip_pool_add_range(&opctx, &authz_pool, &big_range) + .await + .expect("could not add range"); - assert_ip_pool_utilization(client, "p0", 0, 5, 0, 18446480190918885375) - .await; + assert_ip_pool_utilization(client, "p0", 0, 5, 0, 2u128.pow(80)).await; } // Data for testing overlapping IP ranges @@ -966,15 +985,14 @@ async fn test_ip_pool_range_rejects_v6(cptestctx: &ControlPlaneTestContext) { object_create_error(client, add_url, &range, StatusCode::BAD_REQUEST) .await; - let msg = "unable to parse JSON body: first: invalid IPv4 address syntax"; - assert!(error.message.starts_with(msg)); + assert_eq!(error.message, "IPv6 ranges are not allowed yet"); // same deal with service pool let add_url = "/v1/system/ip-pools-service/ranges/add"; let error = object_create_error(client, add_url, &range, StatusCode::BAD_REQUEST) .await; - assert!(error.message.starts_with(msg)); + assert_eq!(error.message, "IPv6 ranges are not allowed yet"); } #[nexus_test] diff --git a/openapi/nexus.json b/openapi/nexus.json index 7c319dcb5f..55dd88af49 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -5432,6 +5432,7 @@ "system/networking" ], "summary": "Add range to IP pool", + "description": "IPv6 ranges are not allowed yet.", "operationId": "ip_pool_range_add", "parameters": [ { @@ -5448,7 +5449,7 @@ "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/Ipv4Range" + "$ref": "#/components/schemas/IpRange" } } }, @@ -5847,12 +5848,13 @@ "system/networking" ], "summary": "Add IP range to Oxide service pool", + "description": "IPv6 ranges are not allowed yet.", "operationId": "ip_pool_service_range_add", "requestBody": { "content": { "application/json": { "schema": { - "$ref": "#/components/schemas/Ipv4Range" + "$ref": "#/components/schemas/IpRange" } } },