note that we are relying on DB enum order for most specific, flip order

oxidecomputer · Nov 7, 2023 · d7a0bdb · d7a0bdb
1 parent 9519ed1
commit d7a0bdb
Show file tree

Hide file tree

Showing 2 changed files with 12 additions and 10 deletions.
diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs
@@ -147,10 +147,12 @@ impl DataStore {
             )
             .filter(ip_pool_resource::is_default.eq(true))
             .filter(ip_pool::time_deleted.is_null())
-            // TODO: order by most specific first so we get the most specific
-            // when we select the first one. alphabetical desc technically
-            // works but come on. won't work when we have project association
-            .order(ip_pool_resource::resource_type.desc())
+            // Order by most specific first so we get the most specific.
+            // resource_type is an enum in the DB and therefore gets its order
+            // from the definition; it's not lexicographic. So correctness here
+            // relies on the types being most-specific-first in the definition.
+            // There are tests for this.
+            .order(ip_pool_resource::resource_type.asc())
             .select(IpPool::as_select())
             .first_async::<IpPool>(
                 &*self.pool_connection_authorized(opctx).await?,
@@ -304,10 +306,6 @@ impl DataStore {
                     .and(ip_pool_resource::resource_id.eq(*INTERNAL_SILO_ID)),
             )
             .filter(ip_pool::time_deleted.is_null())
-            // TODO: order by most specific first so we get the most specific
-            // when we select the first one. alphabetical desc technically
-            // works but come on. won't work when we have project association
-            .order(ip_pool_resource::resource_type.desc())
             .select(IpPool::as_select())
             .load_async::<IpPool>(
                 &*self.pool_connection_authorized(opctx).await?,

diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql
@@ -1470,9 +1470,13 @@ CREATE UNIQUE INDEX IF NOT EXISTS lookup_pool_by_name ON omicron.public.ip_pool
 ) WHERE
     time_deleted IS NULL;
 
+-- The order here is most-specific first, and it matters because we use this
+-- fact to select the most specific default in the case where there is both a
+-- silo default and a fleet default. If we were to add a project type, it should
+-- be added before silo.
 CREATE TYPE IF NOT EXISTS omicron.public.ip_pool_resource_type AS ENUM (
-    'fleet',
-    'silo'
+    'silo',
+    'fleet'
 );
 
 -- join table associating IP pools with resources like fleet or silo