Initial schema updates disallowing default IP Pool for the internal silo

- Add database constraints that ensure that we can't set a default pool
  for the Oxide internal silo. Add tests for this specifically.
- This is part of #8948, but does not resolve it.

Author: bnaecker

nexus/db-model/src/schema_versions.rs

@@ -16,7 +16,7 @@ use std::{collections::BTreeMap, sync::LazyLock};
 ///
 /// 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: Version = Version::new(186, 0, 0);
+pub const SCHEMA_VERSION: Version = Version::new(187, 0, 0);
 
 /// List of all past database schema versions, in *reverse* order
 ///
@@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock<Vec<KnownVersion>> = LazyLock::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(187, "no-default-pool-for-internal-silo"),
         KnownVersion::new(186, "nexus-generation"),
         KnownVersion::new(185, "populate-db-metadata-nexus"),
         KnownVersion::new(184, "store-silo-admin-group-name"),

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

@@ -1489,9 +1489,6 @@ mod test {
             assert_eq!(is_internal, Ok(false));
 
             // now link it to the current silo, and it is still not internal.
-            //
-            // We're only making the IPv4 pool the default right now. See
-            // https://github.com/oxidecomputer/omicron/issues/8884 for more.
             let silo_id = opctx.authn.silo_required().unwrap().id();
             let is_default = matches!(version, IpVersion::V4);
             let link = IpPoolResource {
@@ -1514,6 +1511,76 @@ mod test {
         logctx.cleanup_successful();
     }
 
+    #[tokio::test]
+    async fn cannot_set_default_ip_pool_for_internal_silo() {
+        let logctx =
+            dev::test_setup_log("cannot_set_default_ip_pool_for_internal_silo");
+        let db = TestDatabase::new_with_datastore(&logctx.log).await;
+        let (opctx, datastore) = (db.opctx(), db.datastore());
+
+        for version in [IpVersion::V4, IpVersion::V6] {
+            // confirm internal pools appear as internal
+            let (authz_pool, pool) = datastore
+                .ip_pools_service_lookup(&opctx, version)
+                .await
+                .unwrap();
+            assert_eq!(pool.ip_version, version);
+
+            let is_internal =
+                datastore.ip_pool_is_internal(&opctx, &authz_pool).await;
+            assert_eq!(is_internal, Ok(true));
+
+            // Try to link it as the default.
+            let (authz_silo, ..) =
+                nexus_db_lookup::LookupPath::new(&opctx, datastore)
+                    .silo_id(nexus_types::silo::INTERNAL_SILO_ID)
+                    .lookup_for(authz::Action::Read)
+                    .await
+                    .expect("Should be able to lookup internal silo");
+            let link = IpPoolResource {
+                ip_pool_id: authz_pool.id(),
+                resource_type: IpPoolResourceType::Silo,
+                resource_id: authz_silo.id(),
+                is_default: true,
+            };
+            let Err(e) = datastore.ip_pool_link_silo(opctx, link.clone()).await
+            else {
+                panic!(
+                    "should have failed to link IP Pool to internal silo as a default"
+                );
+            };
+            let Error::InternalError { internal_message } = &e else {
+                panic!("should have received an internal error");
+            };
+            assert!(
+                internal_message.contains("failed to satisfy CHECK constraint")
+            );
+
+            // We can link it if it's not the default.
+            let link = IpPoolResource { is_default: false, ..link };
+            datastore.ip_pool_link_silo(opctx, link).await.expect(
+                "Should be able to link non-default pool to internal silo",
+            );
+
+            // Try to set it to the default, and ensure that this also fails.
+            let Err(e) = datastore
+                .ip_pool_set_default(opctx, &authz_pool, &authz_silo, true)
+                .await
+            else {
+                panic!("should have failed to set internal pool to default");
+            };
+            let Error::InternalError { internal_message } = &e else {
+                panic!("should have received an internal error");
+            };
+            assert!(
+                internal_message.contains("failed to satisfy CHECK constraint")
+            );
+        }
+
+        db.terminate().await;
+        logctx.cleanup_successful();
+    }
+
     // We're breaking out the utilization tests for IPv4 and IPv6 pools, since
     // pools only contain one version now.
     //

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

@@ -991,9 +991,9 @@ impl DataStore {
                 },
                 version,
             );
-
+            // Create the pool, and link it to the internal silo if needed. But
+            // we cannot set a default.
             let internal_pool_id = internal_pool.id();
-
             let internal_created = self
                 .ip_pool_create(opctx, internal_pool)
                 .await
@@ -1002,25 +1002,14 @@ impl DataStore {
                     Error::ObjectAlreadyExists { .. } => Ok(false),
                     _ => Err(e),
                 })?;
-
-            // make default for the internal silo. only need to do this if
-            // the create went through, i.e., if it wasn't already there
-            //
-            // TODO-completeness: We're linking both IP pools here, but only the
-            // IPv4 pool is set as a default. We need a way for the operator to
-            // control this, either at RSS or through the API. An alternative is
-            // to not set a default at all, even though both are linked.
-            //
-            // See https://github.com/oxidecomputer/omicron/issues/8884
             if internal_created {
-                let is_default = matches!(version, IpVersion::V4);
                 self.ip_pool_link_silo(
                     opctx,
                     db::model::IpPoolResource {
                         ip_pool_id: internal_pool_id,
                         resource_type: db::model::IpPoolResourceType::Silo,
                         resource_id: INTERNAL_SILO_ID,
-                        is_default,
+                        is_default: false,
                     },
                 )
                 .await?;

nexus/src/app/silo.rs

@@ -50,6 +50,7 @@ impl super::Nexus {
         let silo = self.silo_lookup(opctx, NameOrId::Id(silo.id()))?;
         Ok(silo)
     }
+
     pub fn silo_lookup<'a>(
         &'a self,
         opctx: &'a OpContext,

schema/crdb/dbinit.sql

@@ -2110,7 +2110,20 @@ CREATE TABLE IF NOT EXISTS omicron.public.ip_pool_resource (
 
     -- resource_type is redundant because resource IDs are globally unique, but
     -- logically it belongs here
-    PRIMARY KEY (ip_pool_id, resource_type, resource_id)
+    PRIMARY KEY (ip_pool_id, resource_type, resource_id),
+
+    -- Check that there are no default pools for the internal silo
+    CONSTRAINT internal_silo_has_no_default_pool CHECK (
+        -- A = linked to internal, B = (not default)
+        -- A -> B iff ¬A v B
+        -- ¬(linked to internal silo) OR (not default)
+        NOT (
+            resource_type = 'silo' AND
+            resource_id = '001de000-5110-4000-8000-000000000001'
+        )
+        OR NOT is_default
+    )
+
 );
 
 -- a given resource can only have one default ip pool
@@ -2123,6 +2136,7 @@ CREATE UNIQUE INDEX IF NOT EXISTS one_default_ip_pool_per_resource ON omicron.pu
 CREATE INDEX IF NOT EXISTS ip_pool_resource_id ON omicron.public.ip_pool_resource (
     resource_id
 );
+
 CREATE INDEX IF NOT EXISTS ip_pool_resource_ip_pool_id ON omicron.public.ip_pool_resource (
     ip_pool_id
 );
@@ -6602,7 +6616,7 @@ INSERT INTO omicron.public.db_metadata (
     version,
     target_version
 ) VALUES
-    (TRUE, NOW(), NOW(), '186.0.0', NULL)
+    (TRUE, NOW(), NOW(), '187.0.0', NULL)
 ON CONFLICT DO NOTHING;
 
 COMMIT;

schema/crdb/no-default-pool-for-internal-silo/up01.sql

@@ -0,0 +1,9 @@
+/*
+ * Ensure we have no default pool for the internal silo.
+ */
+SET LOCAL disallow_full_table_scans = 'off';
+UPDATE omicron.public.ip_pool_resource
+SET is_default = FALSE
+WHERE
+    resource_type = 'silo' AND
+    resource_id = '001de000-5110-4000-8000-000000000001';

schema/crdb/no-default-pool-for-internal-silo/up02.sql

@@ -0,0 +1,18 @@
+/*
+ * Add check constraint ensuring that, if the pool is linked
+ * to the internal Oxide services silo, it's not marked as
+ * a default pool.
+ */
+ALTER TABLE IF EXISTS
+omicron.public.ip_pool_resource
+ADD CONSTRAINT IF NOT EXISTS
+internal_silo_has_no_default_pool CHECK (
+    -- A = linked to internal, B = (not default)
+    -- A -> B iff ¬A v B
+    -- ¬(linked to internal silo) OR (not default)
+    NOT (
+        resource_type = 'silo' AND
+        resource_id = '001de000-5110-4000-8000-000000000001'
+    )
+    OR NOT is_default
+);