Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 4 additions & 8 deletions common/src/api/external/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}
}

Expand All @@ -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,
}
}

Expand Down
4 changes: 2 additions & 2 deletions dev-tools/omdb/src/bin/omdb/nexus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

?

//! om.into_untyped_uuid()db commands that query or update specific Nexus instances

mod chicken_switches;
mod quiesce;
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions nexus/db-model/src/affinity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
}
Expand Down Expand Up @@ -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,
}
Expand Down
3 changes: 2 additions & 1 deletion nexus/db-model/src/support_bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -127,7 +128,7 @@ impl SupportBundle {
impl From<SupportBundle> 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,
Expand Down
4 changes: 2 additions & 2 deletions nexus/db-model/src/webhook_delivery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
12 changes: 6 additions & 6 deletions nexus/db-queries/src/db/datastore/affinity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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,
};
Expand Down Expand Up @@ -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!
Expand Down
4 changes: 2 additions & 2 deletions nexus/src/app/affinity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
29 changes: 14 additions & 15 deletions nexus/tests/integration_tests/support_bundles.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand All @@ -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<()> {
Expand Down Expand Up @@ -87,7 +89,7 @@ async fn bundles_list(

async fn bundle_get(
client: &ClientTestContext,
id: SupportBundleUuid,
id: Uuid,
) -> Result<SupportBundleInfo> {
let url = format!("{BUNDLES_URL}/{id}");
NexusRequest::object_get(client, &url)
Expand All @@ -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<()> {
Expand All @@ -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)
Expand Down Expand Up @@ -200,7 +199,7 @@ async fn bundle_create_expect_fail(

async fn bundle_download(
client: &ClientTestContext,
id: SupportBundleUuid,
id: Uuid,
) -> Result<bytes::Bytes> {
let url = format!("{BUNDLES_URL}/{id}/download");
let body = NexusRequest::new(
Expand All @@ -219,7 +218,7 @@ async fn bundle_download(

async fn bundle_download_head(
client: &ClientTestContext,
id: SupportBundleUuid,
id: Uuid,
) -> Result<usize> {
let url = format!("{BUNDLES_URL}/{id}/download");
let len = NexusRequest::new(
Expand All @@ -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<bytes::Bytes> {
Expand All @@ -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<()> {
Expand Down Expand Up @@ -298,7 +297,7 @@ async fn bundle_download_expect_fail(

async fn bundle_update_comment(
client: &ClientTestContext,
id: SupportBundleUuid,
id: Uuid,
comment: Option<String>,
) -> Result<SupportBundleInfo> {
use nexus_types::external_api::params::SupportBundleUpdate;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions nexus/tests/integration_tests/webhooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
3 changes: 1 addition & 2 deletions nexus/types/src/external_api/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Utc>,
pub reason_for_creation: String,
pub reason_for_failure: Option<String>,
Expand Down
6 changes: 3 additions & 3 deletions nexus/types/src/external_api/views.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can this be enforced somehow, like not including the uuid kinds dep in the Cargo.toml for this crate?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, I'll try. Though it's not necessary if we go the other route and just have typed UUIDs act like normal ones in the OpenAPI schema.

use oxnet::{Ipv4Net, Ipv6Net};
use schemars::JsonSchema;
use semver::Version;
Expand Down Expand Up @@ -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,
Expand Down
Loading
Loading