VPC Subnet Routing [2/2] -- Custom Routers and NIC 'transit IP' lists (…

…#5823)

This PR builds on #5777 to provide the Custom routers for subnets as
described in RFD21. This entails a few things:
* We remove the `unpublished = true` tag from the user API for VPC
routers and routes.
* Custom routers may be attached/detached to a VPC subnet using the
`custom_router` field in subnet `POST` and `PUT` requests.
* NICs now individually have a `transit_ips` list, which denotes an
additional set of CIDR blocks that a NIC is allowed to send and receive
traffic on. This is set during `POST` and/or `PUT` on instances which
are stopped. This is a key feature to enable software routing by
instances, as today's default behaviour drops any packets not matching
an assigned IP for an instance.
* I suspect there will be some discussion over the shape of this API, so
there isn't yet test coverage here until we know we're happy with it.
* Revisited which router routes can be created by users, e.g., better
validation on v4/v6 dest/target pairs.

There are some allowances around currently non-existent features:
* **Internet Gateways.** We allow unlimited use of one pseudo-gateway,
`inetgw:outbound`, which appears in our existing rules. Using this
target sends packets upstream as it does today.
* **VPC peering.** VPCs as destinations/targets are currently disallowed
in router routes.

Closes #2116.

common/src/api/external/mod.rs

@@ -1407,14 +1407,13 @@ pub struct RouterRoute {
     /// common identifying metadata
     #[serde(flatten)]
     pub identity: IdentityMetadata,
-
     /// The ID of the VPC Router to which the route belongs
     pub vpc_router_id: Uuid,
-
     /// Describes the kind of router. Set at creation. `read-only`
     pub kind: RouterRouteKind,
-
+    /// The location that matched packets should be forwarded to.
     pub target: RouteTarget,
+    /// Selects which traffic this routing rule will apply to.
     pub destination: RouteDestination,
 }
 
@@ -1979,6 +1978,11 @@ pub struct InstanceNetworkInterface {
     /// True if this interface is the primary for the instance to which it's
     /// attached.
     pub primary: bool,
+
+    /// A set of additional networks that this interface may send and
+    /// receive traffic on.
+    #[serde(default)]
+    pub transit_ips: Vec<IpNet>,
 }
 
 #[derive(

common/src/api/internal/shared.rs

@@ -57,6 +57,8 @@ pub struct NetworkInterface {
     pub vni: Vni,
     pub primary: bool,
     pub slot: u8,
+    #[serde(default)]
+    pub transit_ips: Vec<IpNet>,
 }
 
 /// An IP address and port range used for source NAT, i.e., making

illumos-utils/src/opte/port_manager.rs

@@ -440,6 +440,41 @@ impl PortManager {
             }
         }
 
+        // If there are any transit IPs set, allow them through.
+        // TODO: Currently set only in initial state.
+        //       This, external IPs, and cfg'able state
+        //       (DHCP?) are probably worth being managed by an RPW.
+        for block in &nic.transit_ips {
+            #[cfg(target_os = "illumos")]
+            {
+                use oxide_vpc::api::Direction;
+
+                // In principle if this were an operation on an existing
+                // port, we would explicitly undo the In addition if the
+                // Out addition fails.
+                // However, failure here will just destroy the port
+                // outright -- this should only happen if an excessive
+                // number of rules are specified.
+                hdl.allow_cidr(
+                    &port_name,
+                    super::net_to_cidr(*block),
+                    Direction::In,
+                )?;
+                hdl.allow_cidr(
+                    &port_name,
+                    super::net_to_cidr(*block),
+                    Direction::Out,
+                )?;
+            }
+
+            debug!(
+                self.inner.log,
+                "Added CIDR to in/out allowlist";
+                "port_name" => &port_name,
+                "cidr" => ?block,
+            );
+        }
+
         info!(
             self.inner.log,
             "Created OPTE port";

nexus/db-model/src/collection.rs

@@ -152,4 +152,8 @@ pub trait DatastoreAttachTargetConfig<ResourceType>:
     type ResourceTimeDeletedColumn: Column<Table = <Self::ResourceIdColumn as Column>::Table>
         + Default
         + ExpressionMethods;
+
+    /// Controls whether a resource may be attached to a new collection without
+    /// first being explicitly detached from the previous one
+    const ALLOW_FROM_ATTACHED: bool = false;
 }

nexus/db-model/src/network_interface.rs

@@ -13,6 +13,7 @@ use chrono::DateTime;
 use chrono::Utc;
 use db_macros::Resource;
 use diesel::AsChangeset;
+use ipnetwork::IpNetwork;
 use ipnetwork::NetworkSize;
 use nexus_types::external_api::params;
 use nexus_types::identity::Resource;
@@ -64,11 +65,13 @@ pub struct NetworkInterface {
     //
     // If user requests an address of either kind, give exactly that and not the other.
     // If neither is specified, auto-assign one of each?
-    pub ip: ipnetwork::IpNetwork,
+    pub ip: IpNetwork,
 
     pub slot: SqlU8,
     #[diesel(column_name = is_primary)]
     pub primary: bool,
+
+    pub transit_ips: Vec<IpNetwork>,
 }
 
 impl NetworkInterface {
@@ -102,6 +105,7 @@ impl NetworkInterface {
             vni: external::Vni::try_from(0).unwrap(),
             primary: self.primary,
             slot: *self.slot,
+            transit_ips: self.transit_ips.into_iter().map(Into::into).collect(),
         }
     }
 }
@@ -122,11 +126,13 @@ pub struct InstanceNetworkInterface {
     pub subnet_id: Uuid,
 
     pub mac: MacAddr,
-    pub ip: ipnetwork::IpNetwork,
+    pub ip: IpNetwork,
 
     pub slot: SqlU8,
     #[diesel(column_name = is_primary)]
     pub primary: bool,
+
+    pub transit_ips: Vec<IpNetwork>,
 }
 
 /// Service Network Interface DB model.
@@ -145,7 +151,7 @@ pub struct ServiceNetworkInterface {
     pub subnet_id: Uuid,
 
     pub mac: MacAddr,
-    pub ip: ipnetwork::IpNetwork,
+    pub ip: IpNetwork,
 
     pub slot: SqlU8,
     #[diesel(column_name = is_primary)]
@@ -242,6 +248,7 @@ impl NetworkInterface {
             ip: self.ip,
             slot: self.slot,
             primary: self.primary,
+            transit_ips: self.transit_ips,
         }
     }
 
@@ -290,6 +297,7 @@ impl From<InstanceNetworkInterface> for NetworkInterface {
             ip: iface.ip,
             slot: iface.slot,
             primary: iface.primary,
+            transit_ips: iface.transit_ips,
         }
     }
 }
@@ -313,6 +321,7 @@ impl From<ServiceNetworkInterface> for NetworkInterface {
             ip: iface.ip,
             slot: iface.slot,
             primary: iface.primary,
+            transit_ips: vec![],
         }
     }
 }
@@ -460,6 +469,7 @@ pub struct NetworkInterfaceUpdate {
     pub time_modified: DateTime<Utc>,
     #[diesel(column_name = is_primary)]
     pub primary: Option<bool>,
+    pub transit_ips: Vec<IpNetwork>,
 }
 
 impl From<InstanceNetworkInterface> for external::InstanceNetworkInterface {
@@ -472,6 +482,11 @@ impl From<InstanceNetworkInterface> for external::InstanceNetworkInterface {
             ip: iface.ip.ip(),
             mac: *iface.mac,
             primary: iface.primary,
+            transit_ips: iface
+                .transit_ips
+                .into_iter()
+                .map(Into::into)
+                .collect(),
         }
     }
 }
@@ -484,6 +499,11 @@ impl From<params::InstanceNetworkInterfaceUpdate> for NetworkInterfaceUpdate {
             description: params.identity.description,
             time_modified: Utc::now(),
             primary,
+            transit_ips: params
+                .transit_ips
+                .into_iter()
+                .map(Into::into)
+                .collect(),
         }
     }
 }

nexus/db-model/src/omicron_zone_config.rs

@@ -659,6 +659,7 @@ impl OmicronZoneNic {
             vni: omicron_common::api::external::Vni::try_from(*self.vni)
                 .context("parsing VNI")?,
             subnet: self.subnet.into(),
+            transit_ips: vec![],
         })
     }
 }

nexus/db-model/src/schema.rs

@@ -511,6 +511,7 @@ table! {
         ip -> Inet,
         slot -> Int2,
         is_primary -> Bool,
+        transit_ips -> Array<Inet>,
     }
 }
 
@@ -529,6 +530,7 @@ table! {
         ip -> Inet,
         slot -> Int2,
         is_primary -> Bool,
+        transit_ips -> Array<Inet>,
     }
 }
 joinable!(instance_network_interface -> instance (instance_id));

nexus/db-model/src/schema_versions.rs

@@ -17,7 +17,7 @@ use std::collections::BTreeMap;
 ///
 /// This must be updated when you change the database schema.  Refer to
 /// schema/crdb/README.adoc in the root of this repository for details.
-pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(78, 0, 0);
+pub const SCHEMA_VERSION: SemverVersion = SemverVersion::new(79, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -29,6 +29,7 @@ static KNOWN_VERSIONS: Lazy<Vec<KnownVersion>> = Lazy::new(|| {
         // |  leaving the first copy as an example for the next person.
         // v
         // KnownVersion::new(next_int, "unique-dirname-with-the-sql-files"),
+        KnownVersion::new(79, "nic-spoof-allow"),
         KnownVersion::new(78, "vpc-subnet-routing"),
         KnownVersion::new(77, "remove-view-for-v2p-mappings"),
         KnownVersion::new(76, "lookup-region-snapshot-by-snapshot-id"),

nexus/db-model/src/vpc_router.rs

@@ -4,7 +4,8 @@
 
 use super::{impl_enum_type, Generation, Name, RouterRoute};
 use crate::collection::DatastoreCollectionConfig;
-use crate::schema::{router_route, vpc_router};
+use crate::schema::{router_route, vpc_router, vpc_subnet};
+use crate::{DatastoreAttachTargetConfig, VpcSubnet};
 use chrono::{DateTime, Utc};
 use db_macros::Resource;
 use nexus_types::external_api::params;
@@ -41,8 +42,8 @@ pub struct VpcRouter {
     #[diesel(embed)]
     identity: VpcRouterIdentity,
 
-    pub vpc_id: Uuid,
     pub kind: VpcRouterKind,
+    pub vpc_id: Uuid,
     pub rcgen: Generation,
     pub resolved_version: i64,
 }
@@ -99,3 +100,16 @@ impl From<params::VpcRouterUpdate> for VpcRouterUpdate {
         }
     }
 }
+
+impl DatastoreAttachTargetConfig<VpcSubnet> for VpcRouter {
+    type Id = Uuid;
+
+    type CollectionIdColumn = vpc_router::dsl::id;
+    type CollectionTimeDeletedColumn = vpc_router::dsl::time_deleted;
+
+    type ResourceIdColumn = vpc_subnet::dsl::id;
+    type ResourceCollectionIdColumn = vpc_subnet::dsl::custom_router_id;
+    type ResourceTimeDeletedColumn = vpc_subnet::dsl::time_deleted;
+
+    const ALLOW_FROM_ATTACHED: bool = true;
+}

nexus/db-queries/src/db/collection_attach.rs

@@ -232,12 +232,26 @@ pub trait DatastoreAttachTarget<ResourceType>:
                 .filter(collection_table().primary_key().eq(collection_id))
                 .filter(Self::CollectionTimeDeletedColumn::default().is_null()),
         );
-        let resource_query = Box::new(
-            resource_query
-                .filter(resource_table().primary_key().eq(resource_id))
-                .filter(Self::ResourceTimeDeletedColumn::default().is_null())
-                .filter(Self::ResourceCollectionIdColumn::default().is_null()),
-        );
+        let resource_query = if Self::ALLOW_FROM_ATTACHED {
+            Box::new(
+                resource_query
+                    .filter(resource_table().primary_key().eq(resource_id))
+                    .filter(
+                        Self::ResourceTimeDeletedColumn::default().is_null(),
+                    ),
+            )
+        } else {
+            Box::new(
+                resource_query
+                    .filter(resource_table().primary_key().eq(resource_id))
+                    .filter(
+                        Self::ResourceTimeDeletedColumn::default().is_null(),
+                    )
+                    .filter(
+                        Self::ResourceCollectionIdColumn::default().is_null(),
+                    ),
+            )
+        };
 
         let update_resource_statement = update
             .into_boxed()

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

@@ -60,6 +60,7 @@ struct NicInfo {
     vni: db::model::Vni,
     primary: bool,
     slot: i16,
+    transit_ips: Vec<ipnetwork::IpNetwork>,
 }
 
 impl From<NicInfo> for omicron_common::api::internal::shared::NetworkInterface {
@@ -92,6 +93,7 @@ impl From<NicInfo> for omicron_common::api::internal::shared::NetworkInterface {
             vni: nic.vni.0,
             primary: nic.primary,
             slot: u8::try_from(nic.slot).unwrap(),
+            transit_ips: nic.transit_ips.iter().map(|v| (*v).into()).collect(),
         }
     }
 }
@@ -502,6 +504,7 @@ impl DataStore {
                 vpc::vni,
                 network_interface::is_primary,
                 network_interface::slot,
+                network_interface::transit_ips,
             ))
             .get_results_async::<NicInfo>(
                 &*self.pool_connection_authorized(opctx).await?,