diff --git a/nexus/db-model/src/schema_versions.rs b/nexus/db-model/src/schema_versions.rs index 23ad6b94e3c..f8a32331f91 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(259, 0, 0); +pub const SCHEMA_VERSION: Version = Version::new(260, 0, 0); /// List of all past database schema versions, in *reverse* order /// @@ -28,6 +28,7 @@ pub 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(260, "ereport-trim-serial-trailing-nulls"), KnownVersion::new(259, "vmm-failure-reason"), KnownVersion::new(258, "lookup-unmarked-ereports-by-class"), KnownVersion::new(257, "add-disk-adoption-requests"), diff --git a/nexus/tests/integration_tests/data_migrations/ereport_trim_serial_trailing_nulls.rs b/nexus/tests/integration_tests/data_migrations/ereport_trim_serial_trailing_nulls.rs new file mode 100644 index 00000000000..f50ef3167f1 --- /dev/null +++ b/nexus/tests/integration_tests/data_migrations/ereport_trim_serial_trailing_nulls.rs @@ -0,0 +1,165 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at https://mozilla.org/MPL/2.0/. + +use super::super::schema::{DataMigrationFns, MigrationContext}; +use futures::future::BoxFuture; +use pretty_assertions::assert_eq; +use uuid::Uuid; + +// Ereport restart IDs — one per test case. +const CLEAN_SERIAL: Uuid = + Uuid::from_u128(25800001_0000_0000_0000_000000000001); +const NULL_PADDED_SERIAL: Uuid = + Uuid::from_u128(25800001_0000_0000_0000_000000000002); +const NULL_SERIAL: Uuid = Uuid::from_u128(25800001_0000_0000_0000_000000000003); + +const COLLECTOR: Uuid = Uuid::from_u128(25800002_0000_0000_0000_000000000001); + +pub(crate) fn checks() -> DataMigrationFns { + DataMigrationFns::new().before(before).after(after) +} + +fn before<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async move { + // Insert ereport rows with various serial_number values: + // 1. A clean serial number with no trailing nulls. + // 2. A serial number with trailing null bytes. + // 3. A NULL serial_number (should remain NULL). + ctx.client + .batch_execute(&format!( + " + INSERT INTO omicron.public.ereport ( + restart_id, ena, time_collected, collector_id, + serial_number, report, reporter, slot_type, slot + ) VALUES + ('{CLEAN_SERIAL}', 1, now(), '{COLLECTOR}', + 'BRM42220031', + '{{}}', 'sp', 'sled', 0), + ('{NULL_PADDED_SERIAL}', 1, now(), '{COLLECTOR}', + E'2EKRJYKV\\x00\\x00\\x00', + '{{}}', 'sp', 'sled', 1), + ('{NULL_SERIAL}', 1, now(), '{COLLECTOR}', + NULL, + '{{}}', 'sp', 'sled', 2); + " + )) + .await + .expect( + "failed to insert test data for ereport-trim-serial-trailing-nulls" + ); + + // Verify that the null-padded serial number actually has the expected + // nulls in it. + let rows = ctx + .client + .query( + &format!( + "SELECT restart_id, serial_number + FROM omicron.public.ereport + WHERE restart_id IN ( + '{CLEAN_SERIAL}', + '{NULL_PADDED_SERIAL}', + '{NULL_SERIAL}' + ) + ORDER BY restart_id" + ), + &[], + ) + .await + .expect("failed to query ereport rows after migration"); + + assert_eq!(rows.len(), 3, "expected 3 test ereport rows"); + + for row in &rows { + let id: Uuid = row.get("restart_id"); + let serial: Option = row.get("serial_number"); + + match id { + id if id == CLEAN_SERIAL => { + assert_eq!(serial.as_deref(), Some("BRM42220031"),); + } + id if id == NULL_PADDED_SERIAL => { + assert_eq!( + serial.as_deref(), + Some("2EKRJYKV\0\0\0"), + "2EKRJYKV should be null-padded", + ); + } + id if id == NULL_SERIAL => { + assert_eq!(serial, None, "NULL serial_number be None"); + } + other => panic!("unexpected restart_id: {other}"), + } + } + }) +} + +fn after<'a>(ctx: &'a MigrationContext<'a>) -> BoxFuture<'a, ()> { + Box::pin(async move { + // Verify the serial numbers after the migration. + let rows = ctx + .client + .query( + &format!( + "SELECT restart_id, serial_number + FROM omicron.public.ereport + WHERE restart_id IN ( + '{CLEAN_SERIAL}', + '{NULL_PADDED_SERIAL}', + '{NULL_SERIAL}' + ) + ORDER BY restart_id" + ), + &[], + ) + .await + .expect("failed to query ereport rows after migration"); + + assert_eq!(rows.len(), 3, "expected 3 test ereport rows"); + + for row in &rows { + let id: Uuid = row.get("restart_id"); + let serial: Option = row.get("serial_number"); + + match id { + id if id == CLEAN_SERIAL => { + assert_eq!( + serial.as_deref(), + Some("BRM42220031"), + "clean serial number should be unchanged" + ); + } + id if id == NULL_PADDED_SERIAL => { + assert_eq!( + serial.as_deref(), + Some("2EKRJYKV"), + "serial with trailing NUL bytes should be trimmed" + ); + } + id if id == NULL_SERIAL => { + assert_eq!( + serial, None, + "NULL serial_number should remain NULL" + ); + } + other => panic!("unexpected restart_id: {other}"), + } + } + + // Clean up test data. + ctx.client + .batch_execute(&format!( + " + DELETE FROM omicron.public.ereport + WHERE restart_id IN ( + '{CLEAN_SERIAL}', + '{NULL_PADDED_SERIAL}', + '{NULL_SERIAL}' + ); + " + )) + .await + .unwrap(); + }) +} diff --git a/nexus/tests/integration_tests/data_migrations/mod.rs b/nexus/tests/integration_tests/data_migrations/mod.rs index d63210e7ccd..6f2b75396a1 100644 --- a/nexus/tests/integration_tests/data_migrations/mod.rs +++ b/nexus/tests/integration_tests/data_migrations/mod.rs @@ -35,6 +35,7 @@ mod delete_nexus_default_allow_firewall_rule; mod disk_types; mod drop_uninitialized_svc_enabled_not_online_state; mod ereport_everyone_gets_a_slot; +mod ereport_trim_serial_trailing_nulls; mod fix_leaked_bp_oximeter_read_policy_rows; mod fix_session_token_column_order; mod inv_clear_mupdate_override; @@ -90,6 +91,7 @@ pub(crate) fn get_migration_checks() -> BTreeMap { register!(delete_nexus_default_allow_firewall_rule); register!(drop_uninitialized_svc_enabled_not_online_state); register!(bgp_unnumbered_peer_cleanup); + register!(ereport_trim_serial_trailing_nulls); map } diff --git a/schema/crdb/dbinit.sql b/schema/crdb/dbinit.sql index 707c3270779..88a44f4f736 100644 --- a/schema/crdb/dbinit.sql +++ b/schema/crdb/dbinit.sql @@ -8626,7 +8626,7 @@ INSERT INTO omicron.public.db_metadata ( version, target_version ) VALUES - (TRUE, NOW(), NOW(), '259.0.0', NULL) + (TRUE, NOW(), NOW(), '260.0.0', NULL) ON CONFLICT DO NOTHING; COMMIT; diff --git a/schema/crdb/ereport-trim-serial-trailing-nulls/up.sql b/schema/crdb/ereport-trim-serial-trailing-nulls/up.sql new file mode 100644 index 00000000000..82fe8ace940 --- /dev/null +++ b/schema/crdb/ereport-trim-serial-trailing-nulls/up.sql @@ -0,0 +1,12 @@ +-- Strip trailing NUL (\0) characters from the serial_number column in the +-- ereport table. The service processor null-pads OXV2 serial numbers. MGS +-- was stripping these trailing NULLs when collecting the inventory, but +-- not when ingesting ereports. This was fixed in +-- https://github.com/oxidecomputer/management-gateway-service#491 +-- so we can now update any previously-nully ereport serials to match. +SET LOCAL disallow_full_table_scans = off; + +UPDATE omicron.public.ereport + SET serial_number = rtrim(serial_number, chr(0)) + WHERE serial_number IS NOT NULL + AND serial_number != rtrim(serial_number, chr(0));