Use enum instead of sentinel :: for addrconf uplink addresses

nexus/db-model/src/switch_interface.rs

@@ -9,11 +9,14 @@ use db_macros::Asset;
 use ipnetwork::IpNetwork;
 use nexus_db_schema::schema::{loopback_address, switch_vlan_interface_config};
 use nexus_types::external_api::networking as networking_types;
+use nexus_types::external_api::networking::LoopbackAddressIpNet;
 use nexus_types::identity::Asset;
 use omicron_uuid_kinds::LoopbackAddressKind;
 use omicron_uuid_kinds::TypedUuid;
+use oxnet::IpNet;
 use serde::{Deserialize, Serialize};
 use sled_agent_types::early_networking::SwitchSlot;
+use sled_agent_types::early_networking::UplinkIpNetError;
 use uuid::Uuid;
 
 impl_enum_type!(
@@ -103,7 +106,7 @@ pub struct LoopbackAddress {
     pub address_lot_block_id: Uuid,
     pub rsvd_address_lot_block_id: Uuid,
     pub rack_id: Uuid,
-    pub address: IpNetwork,
+    address: IpNetwork,
     pub anycast: bool,
     pub switch_slot: DbSwitchSlot,
 }
@@ -115,7 +118,7 @@ impl LoopbackAddress {
         rsvd_address_lot_block_id: Uuid,
         rack_id: Uuid,
         switch_slot: SwitchSlot,
-        address: IpNetwork,
+        address: LoopbackAddressIpNet,
         anycast: bool,
     ) -> Self {
         Self {
@@ -126,20 +129,30 @@ impl LoopbackAddress {
             rsvd_address_lot_block_id,
             rack_id,
             switch_slot: switch_slot.into(),
-            address,
+            address: IpNet::from(address).into(),
             anycast,
         }
     }
+
+    /// Return the address of this `LoopbackAddress`.
+    ///
+    /// Only fails if we've stored invalid data in the DB (i.e., an address that
+    /// contains an IP that we don't allow for loopback addresses).
+    pub fn address(&self) -> Result<LoopbackAddressIpNet, UplinkIpNetError> {
+        LoopbackAddressIpNet::try_from(IpNet::from(self.address))
+    }
 }
 
-impl Into<networking_types::LoopbackAddress> for LoopbackAddress {
-    fn into(self) -> networking_types::LoopbackAddress {
-        networking_types::LoopbackAddress {
-            id: self.identity().id,
-            address_lot_block_id: self.address_lot_block_id,
-            rack_id: self.rack_id,
-            switch_slot: self.switch_slot.into(),
-            address: self.address.into(),
-        }
+impl TryFrom<LoopbackAddress> for networking_types::LoopbackAddress {
+    type Error = UplinkIpNetError;
+
+    fn try_from(value: LoopbackAddress) -> Result<Self, Self::Error> {
+        Ok(Self {
+            id: value.identity().id,
+            address_lot_block_id: value.address_lot_block_id,
+            rack_id: value.rack_id,
+            switch_slot: value.switch_slot.into(),
+            address: value.address()?,
+        })
     }
 }

nexus/db-model/src/switch_port.rs

@@ -30,6 +30,7 @@ use omicron_uuid_kinds::TypedUuid;
 use oxnet::IpNet;
 use serde::{Deserialize, Serialize};
 use sled_agent_types::early_networking::ImportExportPolicy;
+use sled_agent_types::early_networking::InvalidIpAddrError;
 use sled_agent_types::early_networking::LinkFec;
 use sled_agent_types::early_networking::LinkSpeed;
 use sled_agent_types::early_networking::RouterLifetimeConfig;
@@ -38,6 +39,7 @@ use sled_agent_types::early_networking::RouterPeerIpAddr;
 use sled_agent_types::early_networking::RouterPeerIpAddrError;
 use sled_agent_types::early_networking::RouterPeerType;
 use sled_agent_types::early_networking::SwitchSlot;
+use sled_agent_types::early_networking::UplinkAddress;
 use uuid::Uuid;
 
 /// Extension trait on [`RouterPeerType`] for converting it to and from the way
@@ -987,7 +989,7 @@ pub struct SwitchPortAddressConfig {
     pub port_settings_id: Uuid,
     pub address_lot_block_id: Uuid,
     pub rsvd_address_lot_block_id: Uuid,
-    pub address: IpNetwork,
+    address: IpNetwork,
     pub interface_name: Name,
     pub vlan_id: Option<SqlU16>,
 }
@@ -997,39 +999,40 @@ impl SwitchPortAddressConfig {
         port_settings_id: Uuid,
         address_lot_block_id: Uuid,
         rsvd_address_lot_block_id: Uuid,
-        address: IpNetwork,
+        address: UplinkAddress,
         interface_name: Name,
         vlan_id: Option<u16>,
     ) -> Self {
+        // TODO-cleanup `switch_port_settings_address_config.address` is not
+        // nullable; we store addrconf addresses as the sentinel value `::/128`.
+        // We should consider reworking this to be consistent with BGP peers
+        // (e.g., store addrconf as `NULL`):
+        // https://github.com/oxidecomputer/omicron/issues/9832#issuecomment-4092974372
+        let address = address.ip_net_squashing_addrconf_to_unspecified();
         Self {
             port_settings_id,
             address_lot_block_id,
             rsvd_address_lot_block_id,
-            address,
+            address: address.into(),
             interface_name,
             vlan_id: vlan_id.map(|x| x.into()),
         }
     }
-}
 
-impl Into<networking_types::SwitchPortAddressConfig>
-    for SwitchPortAddressConfig
-{
-    fn into(self) -> networking_types::SwitchPortAddressConfig {
-        networking_types::SwitchPortAddressConfig {
-            port_settings_id: self.port_settings_id,
-            address_lot_block_id: self.address_lot_block_id,
-            address: self.address.into(),
-            interface_name: self.interface_name.into(),
-            vlan_id: self.vlan_id.map(|x| x.into()),
-        }
+    /// Return the address of this address config.
+    ///
+    /// Only fails if we've stored invalid data in the DB (i.e., an address that
+    /// contains an IP that we don't allow for uplink addresses).
+    pub fn address(&self) -> Result<UplinkAddress, InvalidIpAddrError> {
+        UplinkAddress::try_from_ip_net_treating_unspecified_as_addrconf(
+            self.address.into(),
+        )
     }
 }
 
 #[cfg(test)]
 mod tests {
     use super::*;
-    use sled_agent_types::early_networking::InvalidIpAddrError;
     use sled_agent_types::early_networking::RouterLifetimeConfig;
     use sled_agent_types::early_networking::RouterPeerIpAddr;
     use sled_agent_types::early_networking::RouterPeerType;

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

@@ -2,8 +2,6 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
 
-use std::net::IpAddr;
-
 use super::DataStore;
 use crate::authz;
 use crate::context::OpContext;
@@ -29,6 +27,7 @@ use omicron_common::api::external::{
 };
 use ref_cast::RefCast;
 use serde::{Deserialize, Serialize};
+use sled_agent_types::early_networking::UplinkAddress;
 use uuid::Uuid;
 
 #[derive(Clone, Debug, Deserialize, Serialize)]
@@ -299,7 +298,7 @@ pub(crate) type ReserveBlockTxnError = TransactionError<ReserveBlockError>;
 
 pub(crate) async fn try_reserve_block(
     lot_id: Uuid,
-    inet: IpNetwork,
+    addr: UplinkAddress,
     anycast: bool,
     conn: &Connection<DTraceConnection<PgConnection>>,
 ) -> Result<(AddressLotBlock, AddressLotReservedBlock), ReserveBlockTxnError> {
@@ -308,6 +307,22 @@ pub(crate) async fn try_reserve_block(
     use nexus_db_schema::schema::address_lot_rsvd_block;
     use nexus_db_schema::schema::address_lot_rsvd_block::dsl as rsvd_block_dsl;
 
+    // TODO-correctness What should we do with addrconf addresses? They don't
+    // have an IP to check against an address lot's range. Should address lots
+    // have a setting controlling whether to allow them?
+    //
+    // For now, treat addrconf addresses as though they have the address `::`.
+    // This requires us to have an address lot that includes the address `::`,
+    // which seems a little silly.
+    //
+    // https://github.com/oxidecomputer/omicron/issues/10103
+    //
+    // NOTE: We specifically call `.addr()` on the returned `IpNet` to remove
+    // any subnet prefix from the underlying `IpNet` value. We only store bare
+    // IP address values in `address_lot_rsvd_block`.
+    let inet =
+        IpNetwork::from(addr.ip_net_squashing_addrconf_to_unspecified().addr());
+
     // Ensure a lot block exists with the requested address.
 
     let block = block_dsl::address_lot_block
@@ -324,13 +339,16 @@ pub(crate) async fn try_reserve_block(
             )
         })?;
 
-    // Ensure the address is not already taken.
-
-    let no_reserve = match inet.ip() {
-        IpAddr::V4(a) => a.is_unspecified(),
-        IpAddr::V6(a) => a.is_unspecified() || a.is_unicast_link_local(),
+    // TODO-correctness See note above about handling addrconf - `no_reserve`
+    // here means we'll skip checking for a conflicting overlapping address lot,
+    // but our insert below will still fail if we try to insert more than one
+    // addrconf address.
+    let no_reserve = match addr {
+        UplinkAddress::AddrConf => true,
+        UplinkAddress::Static { .. } => false,
     };
 
+    // Ensure the address is not already taken.
     if !no_reserve {
         let results: Vec<Uuid> = if anycast {
             // Ensure that a non-anycast reservation has not already been made

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

@@ -1,8 +1,8 @@
 // This Source Code Form is subject to the terms of the Mozilla Public
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at https://mozilla.org/MPL/2.0/.
-use super::DataStore;
 
+use super::DataStore;
 use crate::authz;
 use crate::context::OpContext;
 use crate::db::datastore::address_lot::{
@@ -12,7 +12,6 @@ use crate::db::model::LoopbackAddress;
 use crate::db::pagination::paginated;
 use async_bb8_diesel::AsyncRunQueryDsl;
 use diesel::{ExpressionMethods, QueryDsl, SelectableHelper};
-use ipnetwork::IpNetwork;
 use nexus_db_errors::ErrorHandler;
 use nexus_db_errors::OptionalError;
 use nexus_db_errors::public_error_from_diesel;
@@ -42,10 +41,6 @@ impl DataStore {
         }
 
         let conn = self.pool_connection_authorized(opctx).await?;
-
-        let inet = IpNetwork::new(params.address, params.mask)
-            .map_err(|_| Error::invalid_request("invalid address"))?;
-
         let err = OptionalError::new();
 
         // TODO https://github.com/oxidecomputer/omicron/issues/2811
@@ -58,7 +53,7 @@ impl DataStore {
                     let (block, rsvd_block) =
                         crate::db::datastore::address_lot::try_reserve_block(
                             lot_id,
-                            inet.ip().into(),
+                            params.address.into(),
                             params.anycast,
                             &conn,
                         )
@@ -78,7 +73,7 @@ impl DataStore {
                         rsvd_block.id,
                         params.rack_id,
                         params.switch_slot,
-                        inet,
+                        params.address,
                         params.anycast,
                     );
 
@@ -108,7 +103,7 @@ impl DataStore {
                         e,
                         ErrorHandler::Conflict(
                             ResourceType::LoopbackAddress,
-                            &format!("lo {}", inet),
+                            &format!("lo {}", params.address),
                         ),
                     )
                 }

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

@@ -1551,7 +1551,7 @@ async fn switch_port_settings_get_on_conn(
         .load_async::<SwitchPortAddressConfig>(conn)
         .await?;
 
-    result.addresses = switch_port_address_view(conn, addresses).await?;
+    result.addresses = switch_port_address_view(conn, addresses, err).await?;
 
     Ok(result)
 }
@@ -1945,7 +1945,7 @@ async fn do_switch_port_settings_create(
             let (block, rsvd_block) =
                 crate::db::datastore::address_lot::try_reserve_block(
                     address_lot_id,
-                    address.address.addr().into(),
+                    address.address,
                     // TODO: Should we allow anycast addresses for switch_ports?
                     // anycast
                     false,
@@ -1963,7 +1963,7 @@ async fn do_switch_port_settings_create(
                 psid,
                 block.id,
                 rsvd_block.id,
-                address.address.into(),
+                address.address,
                 a.link_name.clone().into(),
                 address.vlan_id,
             ));
@@ -1977,7 +1977,7 @@ async fn do_switch_port_settings_create(
     .get_results_async(conn)
     .await?;
 
-    result.addresses = switch_port_address_view(conn, addresses).await?;
+    result.addresses = switch_port_address_view(conn, addresses, err).await?;
 
     Ok(result)
 }
@@ -2047,10 +2047,14 @@ impl<'a> BgpPeerProperties<'a> {
     }
 }
 
-async fn switch_port_address_view(
+async fn switch_port_address_view<E>(
     conn: &Connection<DTraceConnection<PgConnection>>,
     addresses: Vec<SwitchPortAddressConfig>,
-) -> Result<Vec<networking::SwitchPortAddressView>, diesel::result::Error> {
+    err: OptionalError<E>,
+) -> Result<Vec<networking::SwitchPortAddressView>, diesel::result::Error>
+where
+    E: SwitchPortSettingsInternalError,
+{
     use nexus_db_schema::schema::{address_lot, address_lot_block};
 
     let mut result = vec![];
@@ -2067,12 +2071,25 @@ async fn switch_port_address_view(
             .first_async::<AddressLot>(conn)
             .await?;
 
+        // Converting the address back to an `UplinkAddress` should never fail;
+        // convert it to an internal error if it does.
+        let uplink_address = match address.address() {
+            Ok(uplink_address) => uplink_address,
+            Err(reason) => {
+                return Err(err.bail(E::internal_error(format!(
+                    "invalid IP address in SwitchPortAddressConfig {}: {}",
+                    address.port_settings_id,
+                    InlineErrorChain::new(&reason),
+                ))));
+            }
+        };
+
         result.push(networking::SwitchPortAddressView {
             port_settings_id: address.port_settings_id,
             address_lot_id: lot.id(),
             address_lot_name: lot.name().clone(),
             address_lot_block_id: address.address_lot_block_id,
-            address: address.address.into(),
+            address: uplink_address,
             vlan_id: address.vlan_id.map(Into::into),
             interface_name: address.interface_name.into(),
         })