enforce correct types for Ipv4NatEntry

nexus/db-model/src/ipv4_nat_entry.rs

@@ -3,45 +3,35 @@ use std::net::{Ipv4Addr, Ipv6Addr};
 use super::MacAddr;
 use crate::{
     schema::{ipv4_nat_entry, ipv4_nat_version},
-    SqlU16, SqlU32, Vni,
+    Ipv4Net, Ipv6Net, SqlU16, SqlU32, Vni,
 };
 use chrono::{DateTime, Utc};
 use omicron_common::api::external;
 use schemars::JsonSchema;
 use serde::Serialize;
 use uuid::Uuid;
 
-// TODO correctness
-// If we're not going to store ipv4 and ipv6
-// NAT entries in the same table, and we don't
-// need any of the special properties of the IpNetwork
-// column type, does it make sense to use a different
-// column type?
-/// Database representation of an Ipv4 NAT Entry.
+/// Values used to create an Ipv4NatEntry
 #[derive(Insertable, Debug, Clone)]
 #[diesel(table_name = ipv4_nat_entry)]
 pub struct Ipv4NatValues {
-    pub external_address: ipnetwork::IpNetwork,
+    pub external_address: Ipv4Net,
     pub first_port: SqlU16,
     pub last_port: SqlU16,
-    pub sled_address: ipnetwork::IpNetwork,
+    pub sled_address: Ipv6Net,
     pub vni: Vni,
     pub mac: MacAddr,
 }
 
-// TODO correctness
-// If we're not going to store ipv4 and ipv6
-// NAT entries in the same table, we should probably
-// make the types more restrictive to prevent an
-// accidental ipv6 entry from being created.
+/// Database representation of an Ipv4 NAT Entry.
 #[derive(Queryable, Debug, Clone, Selectable)]
 #[diesel(table_name = ipv4_nat_entry)]
 pub struct Ipv4NatEntry {
     pub id: Uuid,
-    pub external_address: ipnetwork::IpNetwork,
+    pub external_address: Ipv4Net,
     pub first_port: SqlU16,
     pub last_port: SqlU16,
-    pub sled_address: ipnetwork::IpNetwork,
+    pub sled_address: Ipv6Net,
     pub vni: Vni,
     pub mac: MacAddr,
     pub version_added: SqlU32,
@@ -68,6 +58,7 @@ impl Ipv4NatEntry {
     }
 }
 
+/// Database representation of an Ipv4 NAT Generation.
 #[derive(Queryable, Debug, Clone, Selectable)]
 #[diesel(table_name = ipv4_nat_version)]
 pub struct Ipv4NatGen {
@@ -91,26 +82,16 @@ pub struct Ipv4NatEntryView {
 
 impl From<Ipv4NatEntry> for Ipv4NatEntryView {
     fn from(value: Ipv4NatEntry) -> Self {
-        let external_address = match value.external_address.ip() {
-            std::net::IpAddr::V4(a) => a,
-            std::net::IpAddr::V6(_) => unreachable!(),
-        };
-
-        let sled_address = match value.sled_address.ip() {
-            std::net::IpAddr::V4(_) => unreachable!(),
-            std::net::IpAddr::V6(a) => a,
-        };
-
         let (gen, deleted) = match value.version_removed {
             Some(gen) => (*gen, true),
             None => (*value.version_added, false),
         };
 
         Self {
-            external_address,
+            external_address: value.external_address.ip(),
             first_port: value.first_port(),
             last_port: value.last_port(),
-            sled_address,
+            sled_address: value.sled_address.ip(),
             vni: value.vni.0,
             mac: *value.mac,
             gen,

nexus/db-queries/src/db/datastore/ipv4_nat_entry.rs

@@ -336,18 +336,20 @@ mod test {
 
         // Each change (creation / deletion) to the NAT table should increment the
         // version number of the row in the NAT table
-        let external_address =
-            ipnetwork::IpNetwork::try_from("10.0.0.100").unwrap();
+        let external_address = external::Ipv4Net(
+            ipnetwork::Ipv4Network::try_from("10.0.0.100").unwrap(),
+        );
 
-        let sled_address =
-            ipnetwork::IpNetwork::try_from("fd00:1122:3344:104::1").unwrap();
+        let sled_address = external::Ipv6Net(
+            ipnetwork::Ipv6Network::try_from("fd00:1122:3344:104::1").unwrap(),
+        );
 
         // Add a nat entry.
         let nat1 = Ipv4NatValues {
-            external_address,
+            external_address: external_address.into(),
             first_port: 0.into(),
             last_port: 999.into(),
-            sled_address,
+            sled_address: sled_address.into(),
             vni: Vni(external::Vni::random()),
             mac: MacAddr(
                 external::MacAddr::from_str("A8:40:25:F5:EB:2A").unwrap(),
@@ -372,10 +374,10 @@ mod test {
 
         // Add another nat entry.
         let nat2 = Ipv4NatValues {
-            external_address,
+            external_address: external_address.into(),
             first_port: 1000.into(),
             last_port: 1999.into(),
-            sled_address,
+            sled_address: sled_address.into(),
             vni: Vni(external::Vni::random()),
             mac: MacAddr(
                 external::MacAddr::from_str("A8:40:25:F5:EB:2B").unwrap(),

nexus/src/app/instance_network.rs

@@ -6,6 +6,7 @@
 
 use crate::app::sagas::retry_until_known_result;
 use ipnetwork::IpNetwork;
+use ipnetwork::Ipv6Network;
 use nexus_db_model::Ipv4NatValues;
 use nexus_db_model::Vni as DbVni;
 use nexus_db_queries::authz;
@@ -15,12 +16,13 @@ use nexus_db_queries::db::identity::Asset;
 use nexus_db_queries::db::lookup::LookupPath;
 use omicron_common::api::external::DataPageParams;
 use omicron_common::api::external::Error;
+use omicron_common::api::external::Ipv4Net;
+use omicron_common::api::external::Ipv6Net;
 use omicron_common::api::internal::nexus;
 use omicron_common::api::internal::shared::SwitchLocation;
 use sled_agent_client::types::DeleteVirtualNetworkInterfaceHost;
 use sled_agent_client::types::SetVirtualNetworkInterfaceHost;
 use std::collections::HashSet;
-use std::net::IpAddr;
 use std::str::FromStr;
 use std::sync::Arc;
 use uuid::Uuid;
@@ -351,6 +353,9 @@ impl super::Nexus {
             }
         }
 
+        let sled_address =
+            Ipv6Net(Ipv6Network::new(*sled_ip_address.ip(), 128).unwrap());
+
         for target_ip in ips
             .iter()
             .enumerate()
@@ -364,21 +369,14 @@ impl super::Nexus {
             .map(|(_, ip)| ip)
         {
             // For each external ip, add a nat entry to the database
-            let nat_entry = Ipv4NatValues {
-                external_address: target_ip.ip,
-                first_port: target_ip.first_port,
-                last_port: target_ip.last_port,
-                sled_address: IpNetwork::new(
-                    IpAddr::V6(*sled_ip_address.ip()),
-                    128,
-                )
-                .unwrap(),
-                vni: DbVni(network_interface.vni.clone().into()),
-                mac: nexus_db_model::MacAddr(
-                    omicron_common::api::external::MacAddr(mac_address),
-                ),
-            };
-            self.db_datastore.ensure_ipv4_nat_entry(opctx, nat_entry).await?;
+            self.ensure_nat_entry(
+                target_ip,
+                sled_address,
+                &network_interface,
+                mac_address,
+                opctx,
+            )
+            .await?;
         }
 
         // Notify dendrite that there are changes for it to reconcile.
@@ -391,6 +389,37 @@ impl super::Nexus {
         Ok(())
     }
 
+    async fn ensure_nat_entry(
+        &self,
+        target_ip: &nexus_db_model::ExternalIp,
+        sled_address: Ipv6Net,
+        network_interface: &sled_agent_client::types::NetworkInterface,
+        mac_address: macaddr::MacAddr6,
+        opctx: &OpContext,
+    ) -> Result<(), Error> {
+        match target_ip.ip {
+            IpNetwork::V4(v4net) => {
+                let nat_entry = Ipv4NatValues {
+                    external_address: Ipv4Net(v4net).into(),
+                    first_port: target_ip.first_port,
+                    last_port: target_ip.last_port,
+                    sled_address: sled_address.into(),
+                    vni: DbVni(network_interface.vni.clone().into()),
+                    mac: nexus_db_model::MacAddr(
+                        omicron_common::api::external::MacAddr(mac_address),
+                    ),
+                };
+                self.db_datastore
+                    .ensure_ipv4_nat_entry(opctx, nat_entry)
+                    .await?;
+            }
+            IpNetwork::V6(_v6net) => {
+                // TODO: implement handling of v6 nat.
+            }
+        };
+        Ok(())
+    }
+
     /// Attempts to delete all of the Dendrite NAT configuration for the
     /// instance identified by `authz_instance`.
     ///