rebase on main

Created using spr 1.3.6-beta.1

Author: sunshowers

nexus/db-model/src/external_ip.rs

@@ -236,6 +236,18 @@ impl TryFrom<ExternalIp> for SourceNatConfigGeneric {
     }
 }
 
+/// The resource an external IP is being allocated for.
+#[derive(Debug, Clone)]
+pub enum IpAllocationTarget {
+    /// A silo-owned external IP: instance SNAT / ephemeral, probe ephemeral, or
+    /// a floating IP. Allocation verifies the pool is still linked to this silo
+    /// and assigned to silos.
+    Silo { silo_id: Uuid },
+    /// A system-service external IP, i.e., an Omicron zone. Allocation verifies
+    /// the pool is still assigned to system services.
+    SystemService,
+}
+
 /// An incomplete external IP, used to store state required for issuing the
 /// database query that selects an available IP and stores the resulting record.
 #[derive(Debug, Clone)]
@@ -245,7 +257,7 @@ pub struct IncompleteExternalIp {
     description: Option<String>,
     time_created: DateTime<Utc>,
     kind: IpKind,
-    is_service: bool,
+    target: IpAllocationTarget,
     is_probe: bool,
     parent_id: Option<Uuid>,
     pool_id: Uuid,
@@ -262,6 +274,7 @@ impl IncompleteExternalIp {
         id: Uuid,
         instance_id: Uuid,
         pool_id: Uuid,
+        silo_id: Uuid,
     ) -> Self {
         let kind = IpKind::SNat;
         Self {
@@ -270,7 +283,7 @@ impl IncompleteExternalIp {
             description: None,
             time_created: Utc::now(),
             kind,
-            is_service: false,
+            target: IpAllocationTarget::Silo { silo_id },
             is_probe: false,
             parent_id: Some(instance_id),
             pool_id,
@@ -281,15 +294,15 @@ impl IncompleteExternalIp {
         }
     }
 
-    pub fn for_ephemeral(id: Uuid, pool_id: Uuid) -> Self {
+    pub fn for_ephemeral(id: Uuid, pool_id: Uuid, silo_id: Uuid) -> Self {
         let kind = IpKind::Ephemeral;
         Self {
             id,
             name: None,
             description: None,
             time_created: Utc::now(),
             kind,
-            is_service: false,
+            target: IpAllocationTarget::Silo { silo_id },
             parent_id: None,
             is_probe: false,
             pool_id,
@@ -304,6 +317,7 @@ impl IncompleteExternalIp {
         id: Uuid,
         instance_id: Uuid,
         pool_id: Uuid,
+        silo_id: Uuid,
     ) -> Self {
         let kind = IpKind::Ephemeral;
         Self {
@@ -312,7 +326,7 @@ impl IncompleteExternalIp {
             description: None,
             time_created: Utc::now(),
             kind: IpKind::Ephemeral,
-            is_service: false,
+            target: IpAllocationTarget::Silo { silo_id },
             is_probe: true,
             parent_id: Some(instance_id),
             pool_id,
@@ -329,6 +343,7 @@ impl IncompleteExternalIp {
         description: &str,
         project_id: Uuid,
         pool_id: Uuid,
+        silo_id: Uuid,
     ) -> Self {
         let kind = IpKind::Floating;
         Self {
@@ -337,7 +352,7 @@ impl IncompleteExternalIp {
             description: Some(description.to_string()),
             time_created: Utc::now(),
             kind,
-            is_service: false,
+            target: IpAllocationTarget::Silo { silo_id },
             is_probe: false,
             parent_id: None,
             pool_id,
@@ -355,6 +370,7 @@ impl IncompleteExternalIp {
         project_id: Uuid,
         explicit_ip: IpAddr,
         pool_id: Uuid,
+        silo_id: Uuid,
     ) -> Self {
         let kind = IpKind::Floating;
         Self {
@@ -363,7 +379,7 @@ impl IncompleteExternalIp {
             description: Some(description.to_string()),
             time_created: Utc::now(),
             kind,
-            is_service: false,
+            target: IpAllocationTarget::Silo { silo_id },
             is_probe: false,
             parent_id: None,
             pool_id,
@@ -424,7 +440,7 @@ impl IncompleteExternalIp {
             description,
             time_created: Utc::now(),
             kind,
-            is_service: true,
+            target: IpAllocationTarget::SystemService,
             is_probe: false,
             parent_id: Some(zone_id.into_untyped_uuid()),
             pool_id,
@@ -455,8 +471,21 @@ impl IncompleteExternalIp {
         &self.kind
     }
 
-    pub fn is_service(&self) -> &bool {
-        &self.is_service
+    pub fn is_service(&self) -> bool {
+        match self.target {
+            IpAllocationTarget::Silo { .. } => false,
+            IpAllocationTarget::SystemService => true,
+        }
+    }
+
+    /// The silo that owns this external IP, if it is a silo-owned IP.
+    ///
+    /// Returns `None` for system-service IPs.
+    pub fn silo_id(&self) -> Option<&Uuid> {
+        match &self.target {
+            IpAllocationTarget::Silo { silo_id } => Some(silo_id),
+            IpAllocationTarget::SystemService => None,
+        }
     }
 
     pub fn is_probe(&self) -> &bool {

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

@@ -7,6 +7,7 @@
 use super::DataStore;
 use super::SQL_BATCH_SIZE;
 use crate::authz;
+use crate::authz::ApiResource;
 use crate::context::OpContext;
 use crate::db::collection_attach::AttachError;
 use crate::db::collection_attach::DatastoreAttachTarget;
@@ -52,6 +53,7 @@ use omicron_common::api::external::Error;
 use omicron_common::api::external::IdentityMetadataCreateParams;
 use omicron_common::api::external::ListResultVec;
 use omicron_common::api::external::LookupResult;
+use omicron_common::api::external::LookupType;
 use omicron_common::api::external::ResourceType;
 use omicron_common::api::external::UpdateResult;
 use omicron_common::api::external::http_pagination::PaginatedBy;
@@ -136,12 +138,14 @@ impl DataStore {
         instance_id: InstanceUuid,
         pool_id: Uuid,
     ) -> CreateResult<ExternalIp> {
+        let silo_id = opctx.authn.silo_required()?.id();
         let data = IncompleteExternalIp::for_instance_source_nat(
             ip_id,
             instance_id.into_untyped_uuid(),
             pool_id,
+            silo_id,
         );
-        self.allocate_external_ip(opctx, data).await
+        self.allocate_external_ip(opctx, data, LookupType::ById(pool_id)).await
     }
 
     /// Create an Ephemeral IP address for a probe.
@@ -161,12 +165,15 @@ impl DataStore {
                 ip_version,
             )
             .await?;
+        let silo_id = opctx.authn.silo_required()?.id();
+        let pool_lookup = authz_pool.lookup_type().clone();
         let data = IncompleteExternalIp::for_ephemeral_probe(
             ip_id,
             probe_id,
             authz_pool.id(),
+            silo_id,
         );
-        self.allocate_external_ip(opctx, data).await
+        self.allocate_external_ip(opctx, data, pool_lookup).await
     }
 
     /// Create an Ephemeral IP address for an instance.
@@ -206,11 +213,17 @@ impl DataStore {
             )
             .await?;
 
-        let data = IncompleteExternalIp::for_ephemeral(ip_id, authz_pool.id());
+        let silo_id = opctx.authn.silo_required()?.id();
+        let pool_lookup = authz_pool.lookup_type().clone();
+        let data = IncompleteExternalIp::for_ephemeral(
+            ip_id,
+            authz_pool.id(),
+            silo_id,
+        );
 
         // We might not be able to acquire a new IP, but in the event of an
         // idempotent or double attach this failure is allowed.
-        let temp_ip = self.allocate_external_ip(opctx, data).await;
+        let temp_ip = self.allocate_external_ip(opctx, data, pool_lookup).await;
         if let Err(e) = temp_ip {
             // Use the pool's version for lookup when the request didn't
             // specify one. This handles the case where an explicit pool was
@@ -327,6 +340,7 @@ impl DataStore {
             "project_id" => %project_id,
         );
 
+        let silo_id = opctx.authn.silo_required()?.id();
         let data = if let Some(ip) = explicit_ip {
             IncompleteExternalIp::for_floating_explicit(
                 ip_id,
@@ -335,6 +349,7 @@ impl DataStore {
                 project_id,
                 ip,
                 authz_pool.id(),
+                silo_id,
             )
         } else {
             IncompleteExternalIp::for_floating(
@@ -343,29 +358,39 @@ impl DataStore {
                 &identity.description,
                 project_id,
                 authz_pool.id(),
+                silo_id,
             )
         };
 
-        self.allocate_external_ip(opctx, data).await
+        let pool_lookup = authz_pool.lookup_type().clone();
+        self.allocate_external_ip(opctx, data, pool_lookup).await
     }
 
     async fn allocate_external_ip(
         &self,
         opctx: &OpContext,
         data: IncompleteExternalIp,
+        pool_lookup: LookupType,
     ) -> CreateResult<ExternalIp> {
         let conn = self.pool_connection_authorized(opctx).await?;
-        let ip = Self::allocate_external_ip_on_connection(&conn, data)
-            .await
-            .map_err(|err| err.into_public_ignore_retries())?;
+        let ip =
+            Self::allocate_external_ip_on_connection(&conn, data, pool_lookup)
+                .await
+                .map_err(|err| err.into_public_ignore_retries())?;
         Ok(ip)
     }
 
     /// Variant of [Self::allocate_external_ip] which may be called from a
     /// transaction context.
+    ///
+    /// `pool_lookup` describes how the pool being allocated from was looked up
+    /// originally. This is used to generate a correct error message when the
+    /// pool isn't found or is no longer linked, which depends on how the API
+    /// request specifed the pool in the first place.
     pub(crate) async fn allocate_external_ip_on_connection(
         conn: &async_bb8_diesel::Connection<DbConnection>,
         data: IncompleteExternalIp,
+        pool_lookup: LookupType,
     ) -> Result<ExternalIp, TransactionError<Error>> {
         // Name needs to be cloned out here (if present) to give users a
         // sensible error message on name collision.
@@ -426,7 +451,10 @@ impl DataStore {
                         ))
                     } else {
                         TransactionError::CustomError(
-                            crate::db::queries::external_ip::from_diesel(e),
+                            crate::db::queries::external_ip::from_diesel(
+                                e,
+                                pool_lookup.clone(),
+                            ),
                         )
                     }
                 }
@@ -435,7 +463,10 @@ impl DataStore {
                         return TransactionError::Database(e);
                     }
                     TransactionError::CustomError(
-                        crate::db::queries::external_ip::from_diesel(e),
+                        crate::db::queries::external_ip::from_diesel(
+                            e,
+                            pool_lookup.clone(),
+                        ),
                     )
                 }
             }
@@ -460,7 +491,8 @@ impl DataStore {
             zone_id,
             zone_kind,
         );
-        self.allocate_external_ip(opctx, data).await
+        self.allocate_external_ip(opctx, data, LookupType::ById(pool.id()))
+            .await
     }
 
     /// Variant of [Self::external_ip_allocate_omicron_zone] which may be called
@@ -479,7 +511,12 @@ impl DataStore {
             zone_id,
             zone_kind,
         );
-        Self::allocate_external_ip_on_connection(conn, data).await
+        Self::allocate_external_ip_on_connection(
+            conn,
+            data,
+            LookupType::ById(service_pool.id()),
+        )
+        .await
     }
 
     /// List one page of all external IPs allocated to internal services

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

@@ -582,10 +582,10 @@ impl DataStore {
 
         let (authz_pool, pool_version) = match pool {
             Some(authz_pool) => {
-                self.ip_pool_fetch_link(opctx, authz_pool.id())
-                    .await
-                    .map_err(|_| authz_pool.not_found())?;
-
+                // We intentionally skip checking the silo / pool link here.
+                // That's now done in the `NextExternalIp` allocation query
+                // itself, which has a CTE arm that ensures the link exists. We
+                // do still fetch the pool to validate its type and IP version.
                 let pool_record = {
                     ip_pool::table
                         .filter(ip_pool::id.eq(authz_pool.id()))

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

@@ -667,18 +667,22 @@ impl DataStore {
             zone_config.id,
             zone_config.zone_type.kind(),
         );
-        Self::allocate_external_ip_on_connection(conn, db_ip).await.map_err(
-            |err| {
-                error!(
-                    log,
-                    "Initializing Rack: Failed to allocate \
-                     IP address for {}",
-                     zone_report_str;
-                    "err" => %err,
-                );
-                RackInitError::AddingIp(err.into_public_ignore_retries())
-            },
-        )?;
+        Self::allocate_external_ip_on_connection(
+            conn,
+            db_ip,
+            LookupType::ById(service_pool.db_pool.id()),
+        )
+        .await
+        .map_err(|err| {
+            error!(
+                log,
+                "Initializing Rack: Failed to allocate \
+                 IP address for {}",
+                 zone_report_str;
+                "err" => %err,
+            );
+            RackInitError::AddingIp(err.into_public_ignore_retries())
+        })?;
 
         self.create_network_interface_raw_conn(conn, db_nic)
             .await