From 41056a78153a7a8fe0e4f1b4fc4eded54f5bac5f Mon Sep 17 00:00:00 2001 From: karencfv Date: Fri, 11 Jul 2025 18:27:18 +1200 Subject: [PATCH 1/5] Fix leaked bp_oximeter_read_policy rows --- nexus/db-model/src/schema_versions.rs | 1 + schema/crdb/dbinit.sql | 2 +- schema/crdb/fix-leaked-bp-oximeter-read-policy-rows/up1.sql | 3 +++ 3 files changed, 5 insertions(+), 1 deletion(-) create mode 100644 schema/crdb/fix-leaked-bp-oximeter-read-policy-rows/up1.sql diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 7a67315c41..7edb329f4d 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/nexus/db-model/src/schema_versions.rs @@ -28,6 +28,7 @@ static KNOWN_VERSIONS: LazyLock> = 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(162, "fix-leaked-bp-oximeter-read-policy-rows"), KnownVersion::new(161, "inv_cockroachdb_status"), KnownVersion::new(160, "tuf-trust-root"), KnownVersion::new(159, "sled-config-desired-host-phase-2"), diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 39ea3574f1..768f3caf78 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -6198,7 +6198,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '161.0.0', NULL) + (TRUE, NOW(), NOW(), '162.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/fix-leaked-bp-oximeter-read-policy-rows/up1.sql b/schema/crdb/fix-leaked-bp-oximeter-read-policy-rows/up1.sql new file mode 100644 index 0000000000..c92ce6e93f --- /dev/null +++ b/schema/crdb/fix-leaked-bp-oximeter-read-policy-rows/up1.sql @@ -0,0 +1,3 @@ +set local disallow_full_table_scans = off; + +DELETE FROM bp_oximeter_read_policy WHERE blueprint_id NOT IN (SELECT blueprint_id FROM blueprint); From 9f8a00031e1847a20e42b408944a2a3d617c67f9 Mon Sep 17 00:00:00 2001 From: karencfv Date: Mon, 14 Jul 2025 12:26:48 +1200 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=A4=A6=E2=80=8D=E2=99=80=EF=B8=8F?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- nexus/db-model/src/schema_versions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 7edb329f4d..4f06af2e4a 100644 --- a/nexus/db-model/src/schema_versions.rs +++ b/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(161, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(162, 0, 0); /// List of all past database schema versions, in *reverse* order /// From 7888da2e8063262c57b8e3cc65ba8dea8efeea4c Mon Sep 17 00:00:00 2001 From: karencfv Date: Tue, 15 Jul 2025 15:18:02 +1200 Subject: [PATCH 3/5] add test --- nexus/tests/integration_tests/schema.rs | 58 +++++++++++++++++++ .../up1.sql | 2 +- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/nexus/tests/integration_tests/schema.rs b/nexus/tests/integration_tests/schema.rs index 35ae89e519..b469a06d19 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -2650,6 +2650,60 @@ mod migration_156 { } } +const BP_OXIMETER_READ_POLICY_ID_0: &str = "5cb42909-d94a-4903-be72-330eea0325d9"; +const BP_OXIMETER_READ_POLICY_ID_1: &str = "142b62c2-9348-4530-9eed-7077351fb94b"; +const BP_OXIMETER_READ_POLICY_ID_2: &str = "3b5b7861-03aa-420a-a057-0a14347dc4c0"; +const BP_OXIMETER_READ_POLICY_ID_3: &str = "de7ab4c0-30d4-4e9e-b620-3a959a9d59dd"; + +// Insert two blueprints and 4 oximeter read policies, two of which do not have +// a corresponding blueprint +fn before_163_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async move { + ctx.client + .batch_execute( + &format!(" + INSERT INTO omicron.public.blueprint + (id, parent_blueprint_id, time_created, creator, comment, internal_dns_version, external_dns_version, cockroachdb_fingerprint, cockroachdb_setting_preserve_downgrade, target_release_minimum_generation) + VALUES + ( + '{BP_OXIMETER_READ_POLICY_ID_0}', NULL, now(), 'bob', 'hi', 1, 1, 'fingerprint', NULL, 1 + ), + ( + '{BP_OXIMETER_READ_POLICY_ID_1}', NULL, now(), 'bab', 'hi', 1, 1, 'fingerprint', NULL, 1 + ); + + INSERT INTO omicron.public.bp_oximeter_read_policy + (blueprint_id, version, oximeter_read_mode) + VALUES + ('{BP_OXIMETER_READ_POLICY_ID_0}', 1, 'cluster'), + ('{BP_OXIMETER_READ_POLICY_ID_1}', 2, 'cluster'), + ('{BP_OXIMETER_READ_POLICY_ID_2}', 3, 'cluster'), + ('{BP_OXIMETER_READ_POLICY_ID_3}', 4, 'cluster') + "), + ) + .await + .expect("failed to insert pre-migration rows for 163"); + }) +} + +// Validate that rows that do not have a corresponding blueprint are gone +fn after_163_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async move { + let rows = ctx + .client + .query("SELECT blueprint_id FROM omicron.public.bp_oximeter_read_policy;", &[]) + .await + .expect("failed to query post-migration bp_oximeter_read_policy table"); + assert_eq!(rows.len(), 2); + + let id_1: Uuid = (&rows[0]).get::<&str, Uuid>("blueprint_id"); + assert_eq!(id_1.to_string(), BP_OXIMETER_READ_POLICY_ID_1); + + let id_2: Uuid = (&rows[1]).get::<&str, Uuid>("blueprint_id"); + assert_eq!(id_2.to_string(), BP_OXIMETER_READ_POLICY_ID_0); + }) +} + // Lazily initializes all migration checks. The combination of Rust function // pointers and async makes defining a static table fairly painful, so we're // using lazy initialization instead. @@ -2736,6 +2790,10 @@ fn get_migration_checks() -> BTreeMap { .before(migration_156::before) .after(migration_156::after), ); + map.insert( + Version::new(163, 0, 0), + DataMigrationFns::new().before(before_163_0_0).after(after_163_0_0), + ); map } diff --git a/schema/crdb/fix-leaked-bp-oximeter-read-policy-rows/up1.sql b/schema/crdb/fix-leaked-bp-oximeter-read-policy-rows/up1.sql index c92ce6e93f..15f10f7779 100644 --- a/schema/crdb/fix-leaked-bp-oximeter-read-policy-rows/up1.sql +++ b/schema/crdb/fix-leaked-bp-oximeter-read-policy-rows/up1.sql @@ -1,3 +1,3 @@ set local disallow_full_table_scans = off; -DELETE FROM bp_oximeter_read_policy WHERE blueprint_id NOT IN (SELECT blueprint_id FROM blueprint); +DELETE FROM bp_oximeter_read_policy WHERE blueprint_id NOT IN (SELECT id FROM blueprint); From 68f41c1fbc5409ee9bc8450d9f60ca188f54d760 Mon Sep 17 00:00:00 2001 From: karencfv Date: Tue, 15 Jul 2025 15:21:54 +1200 Subject: [PATCH 4/5] fmt --- nexus/tests/integration_tests/schema.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/nexus/tests/integration_tests/schema.rs b/nexus/tests/integration_tests/schema.rs index b469a06d19..b878730a9e 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -2650,10 +2650,14 @@ mod migration_156 { } } -const BP_OXIMETER_READ_POLICY_ID_0: &str = "5cb42909-d94a-4903-be72-330eea0325d9"; -const BP_OXIMETER_READ_POLICY_ID_1: &str = "142b62c2-9348-4530-9eed-7077351fb94b"; -const BP_OXIMETER_READ_POLICY_ID_2: &str = "3b5b7861-03aa-420a-a057-0a14347dc4c0"; -const BP_OXIMETER_READ_POLICY_ID_3: &str = "de7ab4c0-30d4-4e9e-b620-3a959a9d59dd"; +const BP_OXIMETER_READ_POLICY_ID_0: &str = + "5cb42909-d94a-4903-be72-330eea0325d9"; +const BP_OXIMETER_READ_POLICY_ID_1: &str = + "142b62c2-9348-4530-9eed-7077351fb94b"; +const BP_OXIMETER_READ_POLICY_ID_2: &str = + "3b5b7861-03aa-420a-a057-0a14347dc4c0"; +const BP_OXIMETER_READ_POLICY_ID_3: &str = + "de7ab4c0-30d4-4e9e-b620-3a959a9d59dd"; // Insert two blueprints and 4 oximeter read policies, two of which do not have // a corresponding blueprint From 8ea9bfa306c1f18c7ab00fdb620be9ed81e64833 Mon Sep 17 00:00:00 2001 From: karencfv Date: Wed, 16 Jul 2025 09:01:22 +1200 Subject: [PATCH 5/5] improve test --- nexus/tests/integration_tests/schema.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/nexus/tests/integration_tests/schema.rs b/nexus/tests/integration_tests/schema.rs index 11b8d33c53..1d289007df 100644 --- a/nexus/tests/integration_tests/schema.rs +++ b/nexus/tests/integration_tests/schema.rs @@ -2695,7 +2695,10 @@ fn after_164_0_0<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { Box::pin(async move { let rows = ctx .client - .query("SELECT blueprint_id FROM omicron.public.bp_oximeter_read_policy;", &[]) + .query( + "SELECT blueprint_id FROM omicron.public.bp_oximeter_read_policy ORDER BY blueprint_id;", + &[], + ) .await .expect("failed to query post-migration bp_oximeter_read_policy table"); assert_eq!(rows.len(), 2);