From d7a0bdb5fa46805a8a568d4949d65db180f68684 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 7 Nov 2023 12:23:28 -0600 Subject: [PATCH] note that we are relying on DB enum order for most specific, flip order --- nexus/db-queries/src/db/datastore/ip_pool.rs | 14 ++++++-------- schema/crdb/dbinit.sql | 8 ++++++-- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/nexus/db-queries/src/db/datastore/ip_pool.rs b/nexus/db-queries/src/db/datastore/ip_pool.rs index 9c3b8c100f..1d67dd6916 100644 --- 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::( &*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::( &*self.pool_connection_authorized(opctx).await?, diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index b70df563cb..0a941490e9 100644 --- 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