From 430e4c00cdd214a6f1cf80fe2908d224b1b7c918 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 26 Aug 2025 11:53:23 -0500 Subject: [PATCH 1/3] take TypedUuids out of the external API --- common/src/api/external/mod.rs | 12 ++---- dev-tools/omdb/src/bin/omdb/nexus.rs | 4 +- nexus/db-model/src/affinity.rs | 5 ++- nexus/db-model/src/support_bundle.rs | 3 +- nexus/db-model/src/webhook_delivery.rs | 4 +- nexus/db-queries/src/db/datastore/affinity.rs | 12 +++--- nexus/src/app/affinity.rs | 4 +- .../integration_tests/support_bundles.rs | 29 +++++++------- nexus/tests/integration_tests/webhooks.rs | 12 +++--- nexus/types/src/external_api/shared.rs | 3 +- nexus/types/src/external_api/views.rs | 6 +-- openapi/nexus-internal.json | 7 +--- openapi/nexus.json | 39 +++++-------------- 13 files changed, 57 insertions(+), 83 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 1f9268400f2..9d33afbf896 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -22,8 +22,6 @@ use dropshot::HttpError; pub use dropshot::PaginationOrder; pub use error::*; use futures::stream::BoxStream; -use omicron_uuid_kinds::GenericUuid; -use omicron_uuid_kinds::InstanceUuid; use oxnet::IpNet; use oxnet::Ipv4Net; use parse_display::Display; @@ -1301,13 +1299,13 @@ pub enum AffinityGroupMember { /// /// Instances can belong to up to 16 affinity groups. // See: INSTANCE_MAX_AFFINITY_GROUPS - Instance { id: InstanceUuid, name: Name, run_state: InstanceState }, + Instance { id: Uuid, name: Name, run_state: InstanceState }, } impl SimpleIdentityOrName for AffinityGroupMember { fn id(&self) -> Uuid { match self { - AffinityGroupMember::Instance { id, .. } => *id.as_untyped_uuid(), + AffinityGroupMember::Instance { id, .. } => *id, } } @@ -1332,15 +1330,13 @@ pub enum AntiAffinityGroupMember { /// /// Instances can belong to up to 16 anti-affinity groups. // See: INSTANCE_MAX_ANTI_AFFINITY_GROUPS - Instance { id: InstanceUuid, name: Name, run_state: InstanceState }, + Instance { id: Uuid, name: Name, run_state: InstanceState }, } impl SimpleIdentityOrName for AntiAffinityGroupMember { fn id(&self) -> Uuid { match self { - AntiAffinityGroupMember::Instance { id, .. } => { - *id.as_untyped_uuid() - } + AntiAffinityGroupMember::Instance { id, .. } => *id, } } diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 9fdc54a9209..ee183d662b9 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -2,7 +2,7 @@ // 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/. -//! omdb commands that query or update specific Nexus instances +//! om.into_untyped_uuid()db commands that query or update specific Nexus instances mod chicken_switches; mod quiesce; @@ -4072,7 +4072,7 @@ async fn cmd_nexus_support_bundles_list( user_comment: String, } let rows = support_bundles.into_iter().map(|sb| SupportBundleInfo { - id: *sb.id, + id: sb.id.into_untyped_uuid(), time_created: sb.time_created, reason_for_creation: sb.reason_for_creation, reason_for_failure: sb diff --git a/nexus/db-model/src/affinity.rs b/nexus/db-model/src/affinity.rs index dbef84e9672..3ac58a64c69 100644 --- a/nexus/db-model/src/affinity.rs +++ b/nexus/db-model/src/affinity.rs @@ -23,6 +23,7 @@ use omicron_uuid_kinds::AffinityGroupKind; use omicron_uuid_kinds::AffinityGroupUuid; use omicron_uuid_kinds::AntiAffinityGroupKind; use omicron_uuid_kinds::AntiAffinityGroupUuid; +use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::InstanceKind; use omicron_uuid_kinds::InstanceUuid; use uuid::Uuid; @@ -229,7 +230,7 @@ impl AffinityGroupInstanceMembership { run_state: external::InstanceState, ) -> external::AffinityGroupMember { external::AffinityGroupMember::Instance { - id: self.instance_id.into(), + id: self.instance_id.into_untyped_uuid(), name: member_name, run_state, } @@ -257,7 +258,7 @@ impl AntiAffinityGroupInstanceMembership { run_state: external::InstanceState, ) -> external::AntiAffinityGroupMember { external::AntiAffinityGroupMember::Instance { - id: self.instance_id.into(), + id: self.instance_id.into_untyped_uuid(), name: member_name, run_state, } diff --git a/nexus/db-model/src/support_bundle.rs b/nexus/db-model/src/support_bundle.rs index b9dc9f55b17..9414f52ab71 100644 --- a/nexus/db-model/src/support_bundle.rs +++ b/nexus/db-model/src/support_bundle.rs @@ -11,6 +11,7 @@ use nexus_types::external_api::shared::SupportBundleInfo as SupportBundleView; use nexus_types::external_api::shared::SupportBundleState as SupportBundleStateView; use omicron_uuid_kinds::DatasetKind; use omicron_uuid_kinds::DatasetUuid; +use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::OmicronZoneKind; use omicron_uuid_kinds::OmicronZoneUuid; use omicron_uuid_kinds::SupportBundleKind; @@ -127,7 +128,7 @@ impl SupportBundle { impl From for SupportBundleView { fn from(bundle: SupportBundle) -> Self { Self { - id: bundle.id.into(), + id: bundle.id.into_untyped_uuid(), time_created: bundle.time_created, reason_for_creation: bundle.reason_for_creation, reason_for_failure: bundle.reason_for_failure, diff --git a/nexus/db-model/src/webhook_delivery.rs b/nexus/db-model/src/webhook_delivery.rs index 923eb86405d..b86aba48a08 100644 --- a/nexus/db-model/src/webhook_delivery.rs +++ b/nexus/db-model/src/webhook_delivery.rs @@ -125,9 +125,9 @@ impl WebhookDelivery { attempts.sort_by_key(|a| a.attempt); views::AlertDelivery { id: self.id.into_untyped_uuid(), - receiver_id: self.rx_id.into(), + receiver_id: self.rx_id.into_untyped_uuid(), alert_class: alert_class.as_str().to_owned(), - alert_id: self.alert_id.into(), + alert_id: self.alert_id.into_untyped_uuid(), state: self.state.into(), trigger: self.triggered_by.into(), attempts: views::AlertDeliveryAttempts::Webhook(attempts), diff --git a/nexus/db-queries/src/db/datastore/affinity.rs b/nexus/db-queries/src/db/datastore/affinity.rs index d9a8486e8c8..21da5fa8b34 100644 --- a/nexus/db-queries/src/db/datastore/affinity.rs +++ b/nexus/db-queries/src/db/datastore/affinity.rs @@ -545,7 +545,7 @@ impl DataStore { .into_iter() .map(|(id, name, instance_state, migration_id, vmm_state)| { Ok(external::AffinityGroupMember::Instance { - id: InstanceUuid::from_untyped_uuid(id), + id, name: name.into(), run_state: InstanceStateComputer::compute_state_from( &instance_state, @@ -613,7 +613,7 @@ impl DataStore { )); }; Ok(external::AntiAffinityGroupMember::Instance { - id: InstanceUuid::from_untyped_uuid(id), + id, name: name.into(), run_state: InstanceStateComputer::compute_state_from( &instance_state, @@ -1874,7 +1874,7 @@ mod tests { external::AntiAffinityGroupMember::Instance { id, .. - } if id == instance, + } if id == instance.into_untyped_uuid(), )); // We can delete the member and observe an empty member list @@ -1952,7 +1952,7 @@ mod tests { // Add the instance as a member to the group let member = external::AffinityGroupMember::Instance { - id: instance, + id: instance.into_untyped_uuid(), name: name.try_into().unwrap(), run_state: external::InstanceState::Stopped, }; @@ -2131,7 +2131,7 @@ mod tests { // Add the instance as a member to the group let member = external::AntiAffinityGroupMember::Instance { - id: instance, + id: instance.into_untyped_uuid(), name: name.try_into().unwrap(), run_state: external::InstanceState::Stopped, }; @@ -2473,7 +2473,7 @@ mod tests { external::AntiAffinityGroupMember::Instance { id, .. - } if id == instance, + } if id == instance.into_untyped_uuid(), )); // We can delete the member and observe an empty member list -- even // though it's running! diff --git a/nexus/src/app/affinity.rs b/nexus/src/app/affinity.rs index be2edd4ff73..bc89677158c 100644 --- a/nexus/src/app/affinity.rs +++ b/nexus/src/app/affinity.rs @@ -319,7 +319,7 @@ impl super::Nexus { ) .await?; Ok(external::AffinityGroupMember::Instance { - id: member, + id: authz_instance.id(), name: instance.name().clone(), // TODO: This is kinda a lie - the current implementation of // "affinity_group_member_instance_add" relies on the instance @@ -349,7 +349,7 @@ impl super::Nexus { ) .await?; Ok(external::AntiAffinityGroupMember::Instance { - id: member, + id: authz_instance.id(), name: instance.name().clone(), // TODO: This is kinda a lie - the current implementation of // "anti_affinity_group_member_instance_add" relies on the instance diff --git a/nexus/tests/integration_tests/support_bundles.rs b/nexus/tests/integration_tests/support_bundles.rs index 215936dbecb..c5e1b328894 100644 --- a/nexus/tests/integration_tests/support_bundles.rs +++ b/nexus/tests/integration_tests/support_bundles.rs @@ -20,9 +20,11 @@ use nexus_types::external_api::shared::SupportBundleState; use nexus_types::internal_api::background::SupportBundleCleanupReport; use nexus_types::internal_api::background::SupportBundleCollectionReport; use nexus_types::internal_api::background::SupportBundleEreportStatus; +use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::SupportBundleUuid; use serde::Deserialize; use std::io::Cursor; +use uuid::Uuid; use zip::read::ZipArchive; type ControlPlaneTestContext = @@ -41,7 +43,7 @@ const BUNDLES_URL: &str = "/experimental/v1/system/support-bundles"; async fn expect_not_found( client: &ClientTestContext, - bundle_id: SupportBundleUuid, + bundle_id: Uuid, bundle_url: &str, method: Method, ) -> Result<()> { @@ -87,7 +89,7 @@ async fn bundles_list( async fn bundle_get( client: &ClientTestContext, - id: SupportBundleUuid, + id: Uuid, ) -> Result { let url = format!("{BUNDLES_URL}/{id}"); NexusRequest::object_get(client, &url) @@ -100,7 +102,7 @@ async fn bundle_get( async fn bundle_get_expect_fail( client: &ClientTestContext, - id: SupportBundleUuid, + id: Uuid, expected_status: StatusCode, expected_message: &str, ) -> Result<()> { @@ -126,10 +128,7 @@ async fn bundle_get_expect_fail( Ok(()) } -async fn bundle_delete( - client: &ClientTestContext, - id: SupportBundleUuid, -) -> Result<()> { +async fn bundle_delete(client: &ClientTestContext, id: Uuid) -> Result<()> { let url = format!("{BUNDLES_URL}/{id}"); NexusRequest::object_delete(client, &url) .authn_as(AuthnMode::PrivilegedUser) @@ -200,7 +199,7 @@ async fn bundle_create_expect_fail( async fn bundle_download( client: &ClientTestContext, - id: SupportBundleUuid, + id: Uuid, ) -> Result { let url = format!("{BUNDLES_URL}/{id}/download"); let body = NexusRequest::new( @@ -219,7 +218,7 @@ async fn bundle_download( async fn bundle_download_head( client: &ClientTestContext, - id: SupportBundleUuid, + id: Uuid, ) -> Result { let url = format!("{BUNDLES_URL}/{id}/download"); let len = NexusRequest::new( @@ -244,7 +243,7 @@ async fn bundle_download_head( async fn bundle_download_range( client: &ClientTestContext, - id: SupportBundleUuid, + id: Uuid, value: &str, expected_content_range: &str, ) -> Result { @@ -270,7 +269,7 @@ async fn bundle_download_range( async fn bundle_download_expect_fail( client: &ClientTestContext, - id: SupportBundleUuid, + id: Uuid, expected_status: StatusCode, expected_message: &str, ) -> Result<()> { @@ -298,7 +297,7 @@ async fn bundle_download_expect_fail( async fn bundle_update_comment( client: &ClientTestContext, - id: SupportBundleUuid, + id: Uuid, comment: Option, ) -> Result { use nexus_types::external_api::params::SupportBundleUpdate; @@ -357,7 +356,7 @@ async fn activate_bundle_collection_background_task( async fn test_support_bundle_not_found(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; - let id = SupportBundleUuid::new_v4(); + let id = Uuid::new_v4(); expect_not_found( &client, @@ -489,7 +488,7 @@ async fn test_support_bundle_lifecycle(cptestctx: &ControlPlaneTestContext) { assert_eq!( output.collection_report, Some(SupportBundleCollectionReport { - bundle: bundle.id, + bundle: SupportBundleUuid::from_untyped_uuid(bundle.id), listed_in_service_sleds: true, listed_sps: true, activated_in_db_ok: true, @@ -592,7 +591,7 @@ async fn test_support_bundle_range_requests( assert_eq!( output.collection_report, Some(SupportBundleCollectionReport { - bundle: bundle.id, + bundle: SupportBundleUuid::from_untyped_uuid(bundle.id), listed_in_service_sleds: true, listed_sps: true, activated_in_db_ok: true, diff --git a/nexus/tests/integration_tests/webhooks.rs b/nexus/tests/integration_tests/webhooks.rs index f7d6568be71..0c415e312f1 100644 --- a/nexus/tests/integration_tests/webhooks.rs +++ b/nexus/tests/integration_tests/webhooks.rs @@ -926,8 +926,8 @@ async fn test_retry_backoff(cptestctx: &ControlPlaneTestContext) { ); let delivery = dbg!(&deliveries.all_items[0]); - assert_eq!(delivery.receiver_id.into_untyped_uuid(), webhook.identity.id); - assert_eq!(delivery.alert_id, id); + assert_eq!(delivery.receiver_id, webhook.identity.id); + assert_eq!(delivery.alert_id, *id.as_untyped_uuid()); assert_eq!(delivery.alert_class, "test.foo"); assert_eq!(delivery.state, views::AlertDeliveryState::Pending); expect_delivery_attempts( @@ -1000,8 +1000,8 @@ async fn test_retry_backoff(cptestctx: &ControlPlaneTestContext) { ); let delivery = dbg!(&deliveries.all_items[0]); - assert_eq!(delivery.receiver_id.into_untyped_uuid(), webhook.identity.id); - assert_eq!(delivery.alert_id, id); + assert_eq!(delivery.receiver_id, webhook.identity.id); + assert_eq!(delivery.alert_id, *id.as_untyped_uuid()); assert_eq!(delivery.alert_class, "test.foo"); assert_eq!(delivery.state, views::AlertDeliveryState::Pending); expect_delivery_attempts( @@ -1069,8 +1069,8 @@ async fn test_retry_backoff(cptestctx: &ControlPlaneTestContext) { deliveries.all_items ); let delivery = dbg!(&deliveries.all_items[0]); - assert_eq!(delivery.receiver_id.into_untyped_uuid(), webhook.identity.id); - assert_eq!(delivery.alert_id, id); + assert_eq!(delivery.receiver_id, webhook.identity.id); + assert_eq!(delivery.alert_id, *id.as_untyped_uuid()); assert_eq!(delivery.alert_class, "test.foo"); assert_eq!(delivery.state, views::AlertDeliveryState::Delivered); expect_delivery_attempts( diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index 10f38bf7df4..732b4706c01 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -12,7 +12,6 @@ use chrono::DateTime; use chrono::Utc; use omicron_common::api::external::Name; use omicron_common::api::internal::shared::NetworkInterface; -use omicron_uuid_kinds::SupportBundleUuid; use parse_display::FromStr; use schemars::JsonSchema; use serde::Deserialize; @@ -642,7 +641,7 @@ pub enum SupportBundleState { #[derive(Debug, Clone, JsonSchema, Serialize, Deserialize)] pub struct SupportBundleInfo { - pub id: SupportBundleUuid, + pub id: Uuid, pub time_created: DateTime, pub reason_for_creation: String, pub reason_for_failure: Option, diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index d6a81606b25..ef62933d822 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -19,7 +19,7 @@ use omicron_common::api::external::{ Digest, Error, FailureDomain, IdentityMetadata, InstanceState, Name, ObjectIdentity, SimpleIdentity, SimpleIdentityOrName, }; -use omicron_uuid_kinds::{AlertReceiverUuid, AlertUuid}; +// Use plain `Uuid` in external views to avoid leaking typed UUIDs in OpenAPI use oxnet::{Ipv4Net, Ipv6Net}; use schemars::JsonSchema; use semver::Version; @@ -1308,13 +1308,13 @@ pub struct AlertDelivery { pub id: Uuid, /// The UUID of the alert receiver that this event was delivered to. - pub receiver_id: AlertReceiverUuid, + pub receiver_id: Uuid, /// The event class. pub alert_class: String, /// The UUID of the event. - pub alert_id: AlertUuid, + pub alert_id: Uuid, /// The state of this delivery. pub state: AlertDeliveryState, diff --git a/openapi/nexus-internal.json b/openapi/nexus-internal.json index 6915ad92c21..82331b85632 100644 --- a/openapi/nexus-internal.json +++ b/openapi/nexus-internal.json @@ -8573,7 +8573,8 @@ "type": "object", "properties": { "id": { - "$ref": "#/components/schemas/TypedUuidForSupportBundleKind" + "type": "string", + "format": "uuid" }, "reason_for_creation": { "type": "string" @@ -8843,10 +8844,6 @@ "type": "string", "format": "uuid" }, - "TypedUuidForSupportBundleKind": { - "type": "string", - "format": "uuid" - }, "TypedUuidForUpstairsRepairKind": { "type": "string", "format": "uuid" diff --git a/openapi/nexus.json b/openapi/nexus.json index 922efaf0264..28fd4f87a78 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -13826,7 +13826,8 @@ "type": "object", "properties": { "id": { - "$ref": "#/components/schemas/TypedUuidForInstanceKind" + "type": "string", + "format": "uuid" }, "name": { "$ref": "#/components/schemas/Name" @@ -13993,11 +13994,8 @@ }, "alert_id": { "description": "The UUID of the event.", - "allOf": [ - { - "$ref": "#/components/schemas/TypedUuidForAlertKind" - } - ] + "type": "string", + "format": "uuid" }, "attempts": { "description": "Individual attempts to deliver this webhook event, and their outcomes.", @@ -14014,11 +14012,8 @@ }, "receiver_id": { "description": "The UUID of the alert receiver that this event was delivered to.", - "allOf": [ - { - "$ref": "#/components/schemas/TypedUuidForAlertReceiverKind" - } - ] + "type": "string", + "format": "uuid" }, "state": { "description": "The state of this delivery.", @@ -14514,7 +14509,8 @@ "type": "object", "properties": { "id": { - "$ref": "#/components/schemas/TypedUuidForInstanceKind" + "type": "string", + "format": "uuid" }, "name": { "$ref": "#/components/schemas/Name" @@ -24585,7 +24581,8 @@ "type": "object", "properties": { "id": { - "$ref": "#/components/schemas/TypedUuidForSupportBundleKind" + "type": "string", + "format": "uuid" }, "reason_for_creation": { "type": "string" @@ -25995,22 +25992,6 @@ } } }, - "TypedUuidForAlertKind": { - "type": "string", - "format": "uuid" - }, - "TypedUuidForAlertReceiverKind": { - "type": "string", - "format": "uuid" - }, - "TypedUuidForInstanceKind": { - "type": "string", - "format": "uuid" - }, - "TypedUuidForSupportBundleKind": { - "type": "string", - "format": "uuid" - }, "UninitializedSled": { "description": "A sled that has not been added to an initialized rack yet", "type": "object", From 6fcb8f5b01c232369f8397fa5729e2278b66e223 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 26 Aug 2025 14:17:31 -0500 Subject: [PATCH 2/3] fix bad search and replace --- dev-tools/omdb/src/bin/omdb/nexus.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index ee183d662b9..70aec043b3c 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -2,7 +2,7 @@ // 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/. -//! om.into_untyped_uuid()db commands that query or update specific Nexus instances +//! omdb commands that query or update specific Nexus instances mod chicken_switches; mod quiesce; From 823d0d5e5b09d57727b39d711755ae6ad21d0839 Mon Sep 17 00:00:00 2001 From: David Crespo Date: Tue, 26 Aug 2025 15:16:50 -0500 Subject: [PATCH 3/3] do everything much more simply with a schemars annotation --- common/src/api/external/mod.rs | 22 +++++++++++--- dev-tools/omdb/src/bin/omdb/nexus.rs | 2 +- nexus/db-model/src/affinity.rs | 5 ++-- nexus/db-model/src/support_bundle.rs | 3 +- nexus/db-model/src/webhook_delivery.rs | 4 +-- nexus/db-queries/src/db/datastore/affinity.rs | 12 ++++---- nexus/src/app/affinity.rs | 4 +-- .../integration_tests/support_bundles.rs | 29 ++++++++++--------- nexus/tests/integration_tests/webhooks.rs | 12 ++++---- nexus/types/src/external_api/shared.rs | 4 ++- nexus/types/src/external_api/views.rs | 8 +++-- 11 files changed, 61 insertions(+), 44 deletions(-) diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index 9d33afbf896..b5e3ad3d922 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -22,6 +22,8 @@ use dropshot::HttpError; pub use dropshot::PaginationOrder; pub use error::*; use futures::stream::BoxStream; +use omicron_uuid_kinds::GenericUuid; +use omicron_uuid_kinds::InstanceUuid; use oxnet::IpNet; use oxnet::Ipv4Net; use parse_display::Display; @@ -1299,13 +1301,18 @@ pub enum AffinityGroupMember { /// /// Instances can belong to up to 16 affinity groups. // See: INSTANCE_MAX_AFFINITY_GROUPS - Instance { id: Uuid, name: Name, run_state: InstanceState }, + Instance { + #[schemars(with = "Uuid")] + id: InstanceUuid, + name: Name, + run_state: InstanceState, + }, } impl SimpleIdentityOrName for AffinityGroupMember { fn id(&self) -> Uuid { match self { - AffinityGroupMember::Instance { id, .. } => *id, + AffinityGroupMember::Instance { id, .. } => *id.as_untyped_uuid(), } } @@ -1330,13 +1337,20 @@ pub enum AntiAffinityGroupMember { /// /// Instances can belong to up to 16 anti-affinity groups. // See: INSTANCE_MAX_ANTI_AFFINITY_GROUPS - Instance { id: Uuid, name: Name, run_state: InstanceState }, + Instance { + #[schemars(with = "Uuid")] + id: InstanceUuid, + name: Name, + run_state: InstanceState, + }, } impl SimpleIdentityOrName for AntiAffinityGroupMember { fn id(&self) -> Uuid { match self { - AntiAffinityGroupMember::Instance { id, .. } => *id, + AntiAffinityGroupMember::Instance { id, .. } => { + *id.as_untyped_uuid() + } } } diff --git a/dev-tools/omdb/src/bin/omdb/nexus.rs b/dev-tools/omdb/src/bin/omdb/nexus.rs index 70aec043b3c..d4bd569b60a 100644 --- a/dev-tools/omdb/src/bin/omdb/nexus.rs +++ b/dev-tools/omdb/src/bin/omdb/nexus.rs @@ -4072,7 +4072,7 @@ async fn cmd_nexus_support_bundles_list( user_comment: String, } let rows = support_bundles.into_iter().map(|sb| SupportBundleInfo { - id: sb.id.into_untyped_uuid(), + id: sb.id, time_created: sb.time_created, reason_for_creation: sb.reason_for_creation, reason_for_failure: sb diff --git a/nexus/db-model/src/affinity.rs b/nexus/db-model/src/affinity.rs index 3ac58a64c69..dbef84e9672 100644 --- a/nexus/db-model/src/affinity.rs +++ b/nexus/db-model/src/affinity.rs @@ -23,7 +23,6 @@ use omicron_uuid_kinds::AffinityGroupKind; use omicron_uuid_kinds::AffinityGroupUuid; use omicron_uuid_kinds::AntiAffinityGroupKind; use omicron_uuid_kinds::AntiAffinityGroupUuid; -use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::InstanceKind; use omicron_uuid_kinds::InstanceUuid; use uuid::Uuid; @@ -230,7 +229,7 @@ impl AffinityGroupInstanceMembership { run_state: external::InstanceState, ) -> external::AffinityGroupMember { external::AffinityGroupMember::Instance { - id: self.instance_id.into_untyped_uuid(), + id: self.instance_id.into(), name: member_name, run_state, } @@ -258,7 +257,7 @@ impl AntiAffinityGroupInstanceMembership { run_state: external::InstanceState, ) -> external::AntiAffinityGroupMember { external::AntiAffinityGroupMember::Instance { - id: self.instance_id.into_untyped_uuid(), + id: self.instance_id.into(), name: member_name, run_state, } diff --git a/nexus/db-model/src/support_bundle.rs b/nexus/db-model/src/support_bundle.rs index 9414f52ab71..b9dc9f55b17 100644 --- a/nexus/db-model/src/support_bundle.rs +++ b/nexus/db-model/src/support_bundle.rs @@ -11,7 +11,6 @@ use nexus_types::external_api::shared::SupportBundleInfo as SupportBundleView; use nexus_types::external_api::shared::SupportBundleState as SupportBundleStateView; use omicron_uuid_kinds::DatasetKind; use omicron_uuid_kinds::DatasetUuid; -use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::OmicronZoneKind; use omicron_uuid_kinds::OmicronZoneUuid; use omicron_uuid_kinds::SupportBundleKind; @@ -128,7 +127,7 @@ impl SupportBundle { impl From for SupportBundleView { fn from(bundle: SupportBundle) -> Self { Self { - id: bundle.id.into_untyped_uuid(), + id: bundle.id.into(), time_created: bundle.time_created, reason_for_creation: bundle.reason_for_creation, reason_for_failure: bundle.reason_for_failure, diff --git a/nexus/db-model/src/webhook_delivery.rs b/nexus/db-model/src/webhook_delivery.rs index b86aba48a08..923eb86405d 100644 --- a/nexus/db-model/src/webhook_delivery.rs +++ b/nexus/db-model/src/webhook_delivery.rs @@ -125,9 +125,9 @@ impl WebhookDelivery { attempts.sort_by_key(|a| a.attempt); views::AlertDelivery { id: self.id.into_untyped_uuid(), - receiver_id: self.rx_id.into_untyped_uuid(), + receiver_id: self.rx_id.into(), alert_class: alert_class.as_str().to_owned(), - alert_id: self.alert_id.into_untyped_uuid(), + alert_id: self.alert_id.into(), state: self.state.into(), trigger: self.triggered_by.into(), attempts: views::AlertDeliveryAttempts::Webhook(attempts), diff --git a/nexus/db-queries/src/db/datastore/affinity.rs b/nexus/db-queries/src/db/datastore/affinity.rs index 21da5fa8b34..d9a8486e8c8 100644 --- a/nexus/db-queries/src/db/datastore/affinity.rs +++ b/nexus/db-queries/src/db/datastore/affinity.rs @@ -545,7 +545,7 @@ impl DataStore { .into_iter() .map(|(id, name, instance_state, migration_id, vmm_state)| { Ok(external::AffinityGroupMember::Instance { - id, + id: InstanceUuid::from_untyped_uuid(id), name: name.into(), run_state: InstanceStateComputer::compute_state_from( &instance_state, @@ -613,7 +613,7 @@ impl DataStore { )); }; Ok(external::AntiAffinityGroupMember::Instance { - id, + id: InstanceUuid::from_untyped_uuid(id), name: name.into(), run_state: InstanceStateComputer::compute_state_from( &instance_state, @@ -1874,7 +1874,7 @@ mod tests { external::AntiAffinityGroupMember::Instance { id, .. - } if id == instance.into_untyped_uuid(), + } if id == instance, )); // We can delete the member and observe an empty member list @@ -1952,7 +1952,7 @@ mod tests { // Add the instance as a member to the group let member = external::AffinityGroupMember::Instance { - id: instance.into_untyped_uuid(), + id: instance, name: name.try_into().unwrap(), run_state: external::InstanceState::Stopped, }; @@ -2131,7 +2131,7 @@ mod tests { // Add the instance as a member to the group let member = external::AntiAffinityGroupMember::Instance { - id: instance.into_untyped_uuid(), + id: instance, name: name.try_into().unwrap(), run_state: external::InstanceState::Stopped, }; @@ -2473,7 +2473,7 @@ mod tests { external::AntiAffinityGroupMember::Instance { id, .. - } if id == instance.into_untyped_uuid(), + } if id == instance, )); // We can delete the member and observe an empty member list -- even // though it's running! diff --git a/nexus/src/app/affinity.rs b/nexus/src/app/affinity.rs index bc89677158c..be2edd4ff73 100644 --- a/nexus/src/app/affinity.rs +++ b/nexus/src/app/affinity.rs @@ -319,7 +319,7 @@ impl super::Nexus { ) .await?; Ok(external::AffinityGroupMember::Instance { - id: authz_instance.id(), + id: member, name: instance.name().clone(), // TODO: This is kinda a lie - the current implementation of // "affinity_group_member_instance_add" relies on the instance @@ -349,7 +349,7 @@ impl super::Nexus { ) .await?; Ok(external::AntiAffinityGroupMember::Instance { - id: authz_instance.id(), + id: member, name: instance.name().clone(), // TODO: This is kinda a lie - the current implementation of // "anti_affinity_group_member_instance_add" relies on the instance diff --git a/nexus/tests/integration_tests/support_bundles.rs b/nexus/tests/integration_tests/support_bundles.rs index c5e1b328894..215936dbecb 100644 --- a/nexus/tests/integration_tests/support_bundles.rs +++ b/nexus/tests/integration_tests/support_bundles.rs @@ -20,11 +20,9 @@ use nexus_types::external_api::shared::SupportBundleState; use nexus_types::internal_api::background::SupportBundleCleanupReport; use nexus_types::internal_api::background::SupportBundleCollectionReport; use nexus_types::internal_api::background::SupportBundleEreportStatus; -use omicron_uuid_kinds::GenericUuid; use omicron_uuid_kinds::SupportBundleUuid; use serde::Deserialize; use std::io::Cursor; -use uuid::Uuid; use zip::read::ZipArchive; type ControlPlaneTestContext = @@ -43,7 +41,7 @@ const BUNDLES_URL: &str = "/experimental/v1/system/support-bundles"; async fn expect_not_found( client: &ClientTestContext, - bundle_id: Uuid, + bundle_id: SupportBundleUuid, bundle_url: &str, method: Method, ) -> Result<()> { @@ -89,7 +87,7 @@ async fn bundles_list( async fn bundle_get( client: &ClientTestContext, - id: Uuid, + id: SupportBundleUuid, ) -> Result { let url = format!("{BUNDLES_URL}/{id}"); NexusRequest::object_get(client, &url) @@ -102,7 +100,7 @@ async fn bundle_get( async fn bundle_get_expect_fail( client: &ClientTestContext, - id: Uuid, + id: SupportBundleUuid, expected_status: StatusCode, expected_message: &str, ) -> Result<()> { @@ -128,7 +126,10 @@ async fn bundle_get_expect_fail( Ok(()) } -async fn bundle_delete(client: &ClientTestContext, id: Uuid) -> Result<()> { +async fn bundle_delete( + client: &ClientTestContext, + id: SupportBundleUuid, +) -> Result<()> { let url = format!("{BUNDLES_URL}/{id}"); NexusRequest::object_delete(client, &url) .authn_as(AuthnMode::PrivilegedUser) @@ -199,7 +200,7 @@ async fn bundle_create_expect_fail( async fn bundle_download( client: &ClientTestContext, - id: Uuid, + id: SupportBundleUuid, ) -> Result { let url = format!("{BUNDLES_URL}/{id}/download"); let body = NexusRequest::new( @@ -218,7 +219,7 @@ async fn bundle_download( async fn bundle_download_head( client: &ClientTestContext, - id: Uuid, + id: SupportBundleUuid, ) -> Result { let url = format!("{BUNDLES_URL}/{id}/download"); let len = NexusRequest::new( @@ -243,7 +244,7 @@ async fn bundle_download_head( async fn bundle_download_range( client: &ClientTestContext, - id: Uuid, + id: SupportBundleUuid, value: &str, expected_content_range: &str, ) -> Result { @@ -269,7 +270,7 @@ async fn bundle_download_range( async fn bundle_download_expect_fail( client: &ClientTestContext, - id: Uuid, + id: SupportBundleUuid, expected_status: StatusCode, expected_message: &str, ) -> Result<()> { @@ -297,7 +298,7 @@ async fn bundle_download_expect_fail( async fn bundle_update_comment( client: &ClientTestContext, - id: Uuid, + id: SupportBundleUuid, comment: Option, ) -> Result { use nexus_types::external_api::params::SupportBundleUpdate; @@ -356,7 +357,7 @@ async fn activate_bundle_collection_background_task( async fn test_support_bundle_not_found(cptestctx: &ControlPlaneTestContext) { let client = &cptestctx.external_client; - let id = Uuid::new_v4(); + let id = SupportBundleUuid::new_v4(); expect_not_found( &client, @@ -488,7 +489,7 @@ async fn test_support_bundle_lifecycle(cptestctx: &ControlPlaneTestContext) { assert_eq!( output.collection_report, Some(SupportBundleCollectionReport { - bundle: SupportBundleUuid::from_untyped_uuid(bundle.id), + bundle: bundle.id, listed_in_service_sleds: true, listed_sps: true, activated_in_db_ok: true, @@ -591,7 +592,7 @@ async fn test_support_bundle_range_requests( assert_eq!( output.collection_report, Some(SupportBundleCollectionReport { - bundle: SupportBundleUuid::from_untyped_uuid(bundle.id), + bundle: bundle.id, listed_in_service_sleds: true, listed_sps: true, activated_in_db_ok: true, diff --git a/nexus/tests/integration_tests/webhooks.rs b/nexus/tests/integration_tests/webhooks.rs index 0c415e312f1..f7d6568be71 100644 --- a/nexus/tests/integration_tests/webhooks.rs +++ b/nexus/tests/integration_tests/webhooks.rs @@ -926,8 +926,8 @@ async fn test_retry_backoff(cptestctx: &ControlPlaneTestContext) { ); let delivery = dbg!(&deliveries.all_items[0]); - assert_eq!(delivery.receiver_id, webhook.identity.id); - assert_eq!(delivery.alert_id, *id.as_untyped_uuid()); + assert_eq!(delivery.receiver_id.into_untyped_uuid(), webhook.identity.id); + assert_eq!(delivery.alert_id, id); assert_eq!(delivery.alert_class, "test.foo"); assert_eq!(delivery.state, views::AlertDeliveryState::Pending); expect_delivery_attempts( @@ -1000,8 +1000,8 @@ async fn test_retry_backoff(cptestctx: &ControlPlaneTestContext) { ); let delivery = dbg!(&deliveries.all_items[0]); - assert_eq!(delivery.receiver_id, webhook.identity.id); - assert_eq!(delivery.alert_id, *id.as_untyped_uuid()); + assert_eq!(delivery.receiver_id.into_untyped_uuid(), webhook.identity.id); + assert_eq!(delivery.alert_id, id); assert_eq!(delivery.alert_class, "test.foo"); assert_eq!(delivery.state, views::AlertDeliveryState::Pending); expect_delivery_attempts( @@ -1069,8 +1069,8 @@ async fn test_retry_backoff(cptestctx: &ControlPlaneTestContext) { deliveries.all_items ); let delivery = dbg!(&deliveries.all_items[0]); - assert_eq!(delivery.receiver_id, webhook.identity.id); - assert_eq!(delivery.alert_id, *id.as_untyped_uuid()); + assert_eq!(delivery.receiver_id.into_untyped_uuid(), webhook.identity.id); + assert_eq!(delivery.alert_id, id); assert_eq!(delivery.alert_class, "test.foo"); assert_eq!(delivery.state, views::AlertDeliveryState::Delivered); expect_delivery_attempts( diff --git a/nexus/types/src/external_api/shared.rs b/nexus/types/src/external_api/shared.rs index 732b4706c01..58088a9e07b 100644 --- a/nexus/types/src/external_api/shared.rs +++ b/nexus/types/src/external_api/shared.rs @@ -12,6 +12,7 @@ use chrono::DateTime; use chrono::Utc; use omicron_common::api::external::Name; use omicron_common::api::internal::shared::NetworkInterface; +use omicron_uuid_kinds::SupportBundleUuid; use parse_display::FromStr; use schemars::JsonSchema; use serde::Deserialize; @@ -641,7 +642,8 @@ pub enum SupportBundleState { #[derive(Debug, Clone, JsonSchema, Serialize, Deserialize)] pub struct SupportBundleInfo { - pub id: Uuid, + #[schemars(with = "Uuid")] + pub id: SupportBundleUuid, pub time_created: DateTime, pub reason_for_creation: String, pub reason_for_failure: Option, diff --git a/nexus/types/src/external_api/views.rs b/nexus/types/src/external_api/views.rs index ef62933d822..b406c9299f6 100644 --- a/nexus/types/src/external_api/views.rs +++ b/nexus/types/src/external_api/views.rs @@ -19,7 +19,7 @@ use omicron_common::api::external::{ Digest, Error, FailureDomain, IdentityMetadata, InstanceState, Name, ObjectIdentity, SimpleIdentity, SimpleIdentityOrName, }; -// Use plain `Uuid` in external views to avoid leaking typed UUIDs in OpenAPI +use omicron_uuid_kinds::{AlertReceiverUuid, AlertUuid}; use oxnet::{Ipv4Net, Ipv6Net}; use schemars::JsonSchema; use semver::Version; @@ -1308,13 +1308,15 @@ pub struct AlertDelivery { pub id: Uuid, /// The UUID of the alert receiver that this event was delivered to. - pub receiver_id: Uuid, + #[schemars(with = "Uuid")] + pub receiver_id: AlertReceiverUuid, /// The event class. pub alert_class: String, /// The UUID of the event. - pub alert_id: Uuid, + #[schemars(with = "Uuid")] + pub alert_id: AlertUuid, /// The state of this delivery. pub state: AlertDeliveryState,