Updates default VPC Subnet behavior

common/Cargo.toml

@@ -15,6 +15,7 @@ http = "0.2.5"
 hyper = "0.14"
 ipnetwork = "0.18"
 macaddr = { version = "1.0.1", features = [ "serde_std" ] }
+rand = "0.8.4"
 reqwest = { version = "0.11", default-features = false, features = ["rustls-tls"] }
 ring = "0.16"
 schemars = { version = "0.8", features = [ "chrono", "uuid" ] }

common/src/api/external/mod.rs

@@ -1043,6 +1043,14 @@ impl From<steno::SagaStateView> for SagaState {
 #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
 pub struct Ipv4Net(pub ipnetwork::Ipv4Network);
 
+impl Ipv4Net {
+    /// Return `true` if this IPv4 subnetwork is from an RFC 1918 private
+    /// address space.
+    pub fn is_private(&self) -> bool {
+        self.0.network().is_private()
+    }
+}
+
 impl std::ops::Deref for Ipv4Net {
     type Target = ipnetwork::Ipv4Network;
     fn deref(&self) -> &Self::Target {
@@ -1099,6 +1107,38 @@ impl JsonSchema for Ipv4Net {
 #[derive(Clone, Copy, Debug, Deserialize, PartialEq, Serialize)]
 pub struct Ipv6Net(pub ipnetwork::Ipv6Network);
 
+impl Ipv6Net {
+    /// The length for all VPC IPv6 prefixes
+    pub const VPC_IPV6_PREFIX_LENGTH: u8 = 48;
+
+    /// The prefix length for all VPC Sunets
+    pub const VPC_SUBNET_IPV6_PREFIX_LENGTH: u8 = 64;
+
+    /// Return `true` if this subnetwork is in the IPv6 Unique Local Address
+    /// range defined in RFC 4193, e.g., `fd00:/8`
+    pub fn is_unique_local(&self) -> bool {
+        // TODO: Delegate to `Ipv6Addr::is_unique_local()` when stabilized.
+        self.0.network().octets()[0] == 0xfd
+    }
+
+    /// Return `true` if this subnetwork is a valid VPC prefix.
+    ///
+    /// This checks that the subnet is a unique local address, and has the VPC
+    /// prefix length required.
+    pub fn is_vpc_prefix(&self) -> bool {
+        self.is_unique_local()
+            && self.0.prefix() == Self::VPC_IPV6_PREFIX_LENGTH
+    }
+
+    /// Return `true` if this subnetwork is a valid VPC Subnet, given the VPC's
+    /// prefix.
+    pub fn is_vpc_subnet(&self, vpc_prefix: &Ipv6Net) -> bool {
+        self.is_unique_local()
+            && self.is_subnet_of(vpc_prefix.0)
+            && self.prefix() == Self::VPC_SUBNET_IPV6_PREFIX_LENGTH
+    }
+}
+
 impl std::ops::Deref for Ipv6Net {
     type Target = ipnetwork::Ipv6Network;
     fn deref(&self) -> &Self::Target {
@@ -2308,4 +2348,26 @@ mod test {
             parse_display::ParseError::new()
         );
     }
+
+    #[test]
+    fn test_ipv6_net_operations() {
+        use super::Ipv6Net;
+        assert!(Ipv6Net("fd00::/8".parse().unwrap()).is_unique_local());
+        assert!(!Ipv6Net("fe00::/8".parse().unwrap()).is_unique_local());
+
+        assert!(Ipv6Net("fd00::/48".parse().unwrap()).is_vpc_prefix());
+        assert!(!Ipv6Net("fe00::/48".parse().unwrap()).is_vpc_prefix());
+        assert!(!Ipv6Net("fd00::/40".parse().unwrap()).is_vpc_prefix());
+
+        let vpc_prefix = Ipv6Net("fd00::/48".parse().unwrap());
+        assert!(
+            Ipv6Net("fd00::/64".parse().unwrap()).is_vpc_subnet(&vpc_prefix)
+        );
+        assert!(
+            !Ipv6Net("fd10::/64".parse().unwrap()).is_vpc_subnet(&vpc_prefix)
+        );
+        assert!(
+            !Ipv6Net("fd00::/63".parse().unwrap()).is_vpc_subnet(&vpc_prefix)
+        );
+    }
 }

common/src/sql/dbinit.sql

@@ -411,8 +411,8 @@ CREATE TABLE omicron.public.vpc_subnet (
     /* Indicates that the object has been deleted */
     time_deleted TIMESTAMPTZ,
     vpc_id UUID NOT NULL,
-    ipv4_block INET,
-    ipv6_block INET
+    ipv4_block INET NOT NULL,
+    ipv6_block INET NOT NULL
 );
 
 /* Subnet and network interface names are unique per VPC, not project */

nexus/src/db/datastore.rs

@@ -78,6 +78,8 @@ use crate::db::{
     pagination::paginated,
     pagination::paginated_multicolumn,
     subnet_allocation::AllocateIpQuery,
+    subnet_allocation::FilterConflictingVpcSubnetRangesQuery,
+    subnet_allocation::SubnetError,
     update_and_check::{UpdateAndCheck, UpdateStatus},
 };
 
@@ -1571,6 +1573,7 @@ impl DataStore {
         match interface.ip {
             // Attempt an insert with a requested IP address
             Some(ip) => {
+                interface.subnet.contains(ip)?;
                 let row = NetworkInterface {
                     identity: interface.identity,
                     instance_id: interface.instance_id,
@@ -1596,11 +1599,10 @@ impl DataStore {
             }
             // Insert and allocate an IP address
             None => {
-                let block = interface.subnet.ipv4_block.ok_or_else(|| {
-                    Error::internal_error("assuming subnets all have v4 block")
-                })?;
                 let allocation_query = AllocateIpQuery {
-                    block: ipnetwork::IpNetwork::V4(block.0 .0),
+                    block: ipnetwork::IpNetwork::V4(
+                        interface.subnet.ipv4_block.0 .0,
+                    ),
                     interface,
                     now: Utc::now(),
                 };
@@ -2185,30 +2187,35 @@ impl DataStore {
             })
     }
 
+    /// Insert a VPC Subnet, checking for unique IP address ranges.
     pub async fn vpc_create_subnet(
         &self,
         subnet: VpcSubnet,
-    ) -> CreateResult<VpcSubnet> {
+    ) -> Result<VpcSubnet, SubnetError> {
         use db::schema::vpc_subnet::dsl;
-
         let name = subnet.name().clone();
-        let subnet = diesel::insert_into(dsl::vpc_subnet)
-            .values(subnet)
-            .on_conflict(dsl::id)
-            .do_nothing()
+        let values = FilterConflictingVpcSubnetRangesQuery(subnet);
+        diesel::insert_into(dsl::vpc_subnet)
+            .values(values)
             .returning(VpcSubnet::as_returning())
             .get_result_async(self.pool())
             .await
-            .map_err(|e| {
-                public_error_from_diesel_pool(
-                    e,
-                    ErrorHandler::Conflict(
-                        ResourceType::VpcSubnet,
-                        name.as_str(),
-                    ),
-                )
-            })?;
-        Ok(subnet)
+            .map_err(|err| {
+                if let PoolError::Connection(ConnectionError::Query(
+                    diesel::result::Error::NotFound,
+                )) = err
+                {
+                    SubnetError::OverlappingIpRange
+                } else {
+                    SubnetError::External(public_error_from_diesel_pool(
+                        err,
+                        ErrorHandler::Conflict(
+                            ResourceType::VpcSubnet,
+                            name.to_string().as_str(),
+                        ),
+                    ))
+                }
+            })
     }
 
     pub async fn vpc_delete_subnet(&self, subnet_id: &Uuid) -> DeleteResult {
@@ -3251,6 +3258,7 @@ mod test {
     // scan are, in fact, runnable without a FULL SCAN.
     #[tokio::test]
     async fn test_queries_do_not_require_full_table_scan() {
+        use omicron_common::api::external;
         let logctx =
             dev::test_setup_log("test_queries_do_not_require_full_table_scan");
         let mut db = test_setup_database(&logctx.log).await;
@@ -3278,6 +3286,29 @@ mod test {
             explanation
         );
 
+        let subnet = db::model::VpcSubnet::new(
+            Uuid::nil(),
+            Uuid::nil(),
+            external::IdentityMetadataCreateParams {
+                name: external::Name::try_from(String::from("name")).unwrap(),
+                description: String::from("description"),
+            },
+            external::Ipv4Net("172.30.0.0/22".parse().unwrap()),
+            external::Ipv6Net("fd00::/64".parse().unwrap()),
+        );
+        let values = FilterConflictingVpcSubnetRangesQuery(subnet);
+        let query =
+            diesel::insert_into(db::schema::vpc_subnet::dsl::vpc_subnet)
+                .values(values)
+                .returning(VpcSubnet::as_returning());
+        println!("{}", diesel::debug_query(&query));
+        let explanation = query.explain_async(datastore.pool()).await.unwrap();
+        assert!(
+            !explanation.contains("FULL SCAN"),
+            "Found an unexpected FULL SCAN: {}",
+            explanation,
+        );
+
         let _ = db.cleanup().await;
     }
 }

nexus/src/db/explain.rs

@@ -104,9 +104,8 @@ where
     Q: QueryFragment<Pg>,
 {
     fn walk_ast(&self, mut out: AstPass<Pg>) -> QueryResult<()> {
-        out.push_sql("EXPLAIN (");
+        out.push_sql("EXPLAIN ");
         self.query.walk_ast(out.reborrow())?;
-        out.push_sql(")");
         Ok(())
     }
 }

nexus/src/db/mod.rs

@@ -21,7 +21,7 @@ mod pool;
 mod saga_recovery;
 mod saga_types;
 mod sec_store;
-mod subnet_allocation;
+pub(crate) mod subnet_allocation;
 mod update_and_check;
 
 #[cfg(test)]