change the API back and just reject v6 range adds in service layer

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?;
 

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
     }
 

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<Arc<ServerContext>>,
     path_params: Path<params::IpPoolPath>,
-    // The plan is to change this back to shared::IpRange once IPv6 works
-    range_params: TypedBody<shared::Ipv4Range>,
+    range_params: TypedBody<shared::IpRange>,
 ) -> Result<HttpResponseCreated<IpPoolRange>, 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<Arc<ServerContext>>,
     path_params: Path<params::IpPoolPath>,
-    // 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<shared::IpRange>,
 ) -> Result<HttpResponseUpdatedNoContent, HttpError> {
     let apictx = &rqctx.context();
@@ -1885,23 +1882,22 @@ 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",
     tags = ["system/networking"],
 }]
 async fn ip_pool_service_range_add(
     rqctx: RequestContext<Arc<ServerContext>>,
-    // The plan is to change this back to shared::IpRange once IPv6 works
-    range_params: TypedBody<shared::Ipv4Range>,
+    range_params: TypedBody<shared::IpRange>,
 ) -> Result<HttpResponseCreated<IpPoolRange>, 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<Arc<ServerContext>>,
-    // 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<shared::IpRange>,
 ) -> Result<HttpResponseUpdatedNoContent, HttpError> {
     let apictx = &rqctx.context();

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::<IpRange, IpPoolRange>(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]

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"
               }
             }
           },