IP range add can only be IPv4 (#5107)

Closes #5085 

- ~~Change range add request body from `IpRange` to `Ipv4Range`, see
what breaks~~
- [x] Error on Ipv6 ranges in the service layer
- [x] Update tests and add new tests
- ~~May change range remove body to `Ipv4Range` for consistency even
though it is technically fine as long as add is restricted to `Ipv4` —
need to think about it~~ decided not to so we can still remove any
existing IPv6 ranges through the API

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",
@@ -1880,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",

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;
@@ -38,15 +40,16 @@ use nexus_types::external_api::params::IpPoolSiloUpdate;
 use nexus_types::external_api::params::IpPoolUpdate;
 use nexus_types::external_api::shared::IpRange;
 use nexus_types::external_api::shared::Ipv4Range;
-use nexus_types::external_api::shared::Ipv6Range;
 use nexus_types::external_api::shared::SiloIdentityMode;
 use nexus_types::external_api::views::IpPool;
 use nexus_types::external_api::views::IpPoolRange;
 use nexus_types::external_api::views::IpPoolSiloLink;
 use nexus_types::external_api::views::Silo;
 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
@@ -895,59 +914,9 @@ async fn test_ip_pool_range_overlapping_ranges_fails(
     };
     test_bad_ip_ranges(client, &ip_pool_add_range_url, &ipv4_range).await;
 
-    // Test data for IPv6 ranges that should fail due to overlap
-    let ipv6_range = TestRange {
-        base_range: IpRange::V6(
-            Ipv6Range::new(
-                std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 10),
-                std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 20),
-            )
-            .unwrap(),
-        ),
-        bad_ranges: vec![
-            // The exact same range
-            IpRange::V6(
-                Ipv6Range::new(
-                    std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 10),
-                    std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 20),
-                )
-                .unwrap(),
-            ),
-            // Overlaps below
-            IpRange::V6(
-                Ipv6Range::new(
-                    std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 5),
-                    std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 15),
-                )
-                .unwrap(),
-            ),
-            // Overlaps above
-            IpRange::V6(
-                Ipv6Range::new(
-                    std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 15),
-                    std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 25),
-                )
-                .unwrap(),
-            ),
-            // Contains the base range
-            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, 100),
-                )
-                .unwrap(),
-            ),
-            // Contained by the base range
-            IpRange::V6(
-                Ipv6Range::new(
-                    std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 12),
-                    std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 13),
-                )
-                .unwrap(),
-            ),
-        ],
-    };
-    test_bad_ip_ranges(client, &ip_pool_add_range_url, &ipv6_range).await;
+    // IPv6 tests removed along with support for IPv6 ranges in
+    // https://github.com/oxidecomputer/omicron/pull/5107
+    // Put them back when IPv6 ranges are supported again.
 }
 
 async fn test_bad_ip_ranges(
@@ -994,6 +963,38 @@ async fn test_bad_ip_ranges(
     }
 }
 
+// Support for IPv6 ranges removed in
+// https://github.com/oxidecomputer/omicron/pull/5107
+// Delete this test when we support IPv6 again.
+#[nexus_test]
+async fn test_ip_pool_range_rejects_v6(cptestctx: &ControlPlaneTestContext) {
+    let client = &cptestctx.external_client;
+
+    create_ip_pool(client, "p0", None).await;
+
+    let range = IpRange::V6(
+        Ipv6Range::new(
+            std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 10),
+            std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 20),
+        )
+        .unwrap(),
+    );
+
+    let add_url = "/v1/system/ip-pools/p0/ranges/add";
+    let error =
+        object_create_error(client, add_url, &range, StatusCode::BAD_REQUEST)
+            .await;
+
+    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_eq!(error.message, "IPv6 ranges are not allowed yet");
+}
+
 #[nexus_test]
 async fn test_ip_pool_range_pagination(cptestctx: &ControlPlaneTestContext) {
     let client = &cptestctx.external_client;
@@ -1026,17 +1027,17 @@ async fn test_ip_pool_range_pagination(cptestctx: &ControlPlaneTestContext) {
     // address, which sorts all IPv4 before IPv6, then within protocol versions
     // by their first address.
     let ranges = [
-        IpRange::V6(
-            Ipv6Range::new(
-                std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 11),
-                std::net::Ipv6Addr::new(0xfd00, 0, 0, 0, 0, 0, 0, 20),
+        IpRange::V4(
+            Ipv4Range::new(
+                std::net::Ipv4Addr::new(10, 0, 0, 3),
+                std::net::Ipv4Addr::new(10, 0, 0, 4),
             )
             .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),
+        IpRange::V4(
+            Ipv4Range::new(
+                std::net::Ipv4Addr::new(10, 0, 0, 5),
+                std::net::Ipv4Addr::new(10, 0, 0, 6),
             )
             .unwrap(),
         ),
@@ -1304,15 +1305,15 @@ async fn test_ip_pool_service(cptestctx: &ControlPlaneTestContext) {
     let ranges = [
         IpRange::V4(
             Ipv4Range::new(
-                std::net::Ipv4Addr::new(10, 0, 0, 1),
-                std::net::Ipv4Addr::new(10, 0, 0, 2),
+                std::net::Ipv4Addr::new(10, 0, 0, 3),
+                std::net::Ipv4Addr::new(10, 0, 0, 4),
             )
             .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),
+        IpRange::V4(
+            Ipv4Range::new(
+                std::net::Ipv4Addr::new(10, 0, 0, 1),
+                std::net::Ipv4Addr::new(10, 0, 0, 2),
             )
             .unwrap(),
         ),

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": [
           {
@@ -5847,6 +5848,7 @@
           "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": {