diff --git a/common/src/api/external/mod.rs b/common/src/api/external/mod.rs index c5b249fd1c..f79e7a4b69 100644 --- a/common/src/api/external/mod.rs +++ b/common/src/api/external/mod.rs @@ -1154,27 +1154,6 @@ impl FromStr for IpNet { } } -#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] -#[serde(rename_all = "snake_case")] -pub enum VpcRouterKind { - System, - Custom, -} - -/// A VPC router defines a series of rules that indicate where traffic -/// should be sent depending on its destination. -#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] -pub struct VpcRouter { - /// common identifying metadata - #[serde(flatten)] - pub identity: IdentityMetadata, - - pub kind: VpcRouterKind, - - /// The VPC to which the router belongs. - pub vpc_id: Uuid, -} - /// A `RouteTarget` describes the possible locations that traffic matching a /// route destination can be sent. #[derive( diff --git a/nexus/src/db/datastore.rs b/nexus/src/db/datastore.rs index e7d05d2b0a..f07090a5e8 100644 --- a/nexus/src/db/datastore.rs +++ b/nexus/src/db/datastore.rs @@ -299,9 +299,7 @@ impl DataStore { // We look for valid datasets (non-deleted crucible datasets). .filter(dsl::size_used.is_not_null()) .filter(dsl::time_deleted.is_null()) - .filter(dsl::kind.eq(DatasetKind( - crate::internal_api::params::DatasetKind::Crucible, - ))) + .filter(dsl::kind.eq(DatasetKind::Crucible)) .order(dsl::size_used.asc()) // TODO: We admittedly don't actually *fail* any request for // running out of space - we try to send the request down to @@ -3308,7 +3306,9 @@ mod test { use crate::authz; use crate::db::explain::ExplainableAsync; use crate::db::identity::Resource; - use crate::db::model::{ConsoleSession, Organization, Project}; + use crate::db::model::{ + ConsoleSession, DatasetKind, Organization, Project, + }; use crate::external_api::params; use chrono::{Duration, Utc}; use nexus_test_utils::db::test_setup_database; @@ -3473,12 +3473,11 @@ mod test { let dataset_count = REGION_REDUNDANCY_THRESHOLD * 2; let bogus_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080); - let kind = - DatasetKind(crate::internal_api::params::DatasetKind::Crucible); let dataset_ids: Vec = (0..dataset_count).map(|_| Uuid::new_v4()).collect(); for id in &dataset_ids { - let dataset = Dataset::new(*id, zpool_id, bogus_addr, kind.clone()); + let dataset = + Dataset::new(*id, zpool_id, bogus_addr, DatasetKind::Crucible); datastore.dataset_upsert(dataset).await.unwrap(); } @@ -3549,12 +3548,11 @@ mod test { let dataset_count = REGION_REDUNDANCY_THRESHOLD; let bogus_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080); - let kind = - DatasetKind(crate::internal_api::params::DatasetKind::Crucible); let dataset_ids: Vec = (0..dataset_count).map(|_| Uuid::new_v4()).collect(); for id in &dataset_ids { - let dataset = Dataset::new(*id, zpool_id, bogus_addr, kind.clone()); + let dataset = + Dataset::new(*id, zpool_id, bogus_addr, DatasetKind::Crucible); datastore.dataset_upsert(dataset).await.unwrap(); } @@ -3610,12 +3608,11 @@ mod test { let dataset_count = REGION_REDUNDANCY_THRESHOLD - 1; let bogus_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080); - let kind = - DatasetKind(crate::internal_api::params::DatasetKind::Crucible); let dataset_ids: Vec = (0..dataset_count).map(|_| Uuid::new_v4()).collect(); for id in &dataset_ids { - let dataset = Dataset::new(*id, zpool_id, bogus_addr, kind.clone()); + let dataset = + Dataset::new(*id, zpool_id, bogus_addr, DatasetKind::Crucible); datastore.dataset_upsert(dataset).await.unwrap(); } @@ -3658,12 +3655,11 @@ mod test { let dataset_count = REGION_REDUNDANCY_THRESHOLD; let bogus_addr = SocketAddr::new(IpAddr::V4(Ipv4Addr::new(127, 0, 0, 1)), 8080); - let kind = - DatasetKind(crate::internal_api::params::DatasetKind::Crucible); let dataset_ids: Vec = (0..dataset_count).map(|_| Uuid::new_v4()).collect(); for id in &dataset_ids { - let dataset = Dataset::new(*id, zpool_id, bogus_addr, kind.clone()); + let dataset = + Dataset::new(*id, zpool_id, bogus_addr, DatasetKind::Crucible); datastore.dataset_upsert(dataset).await.unwrap(); } diff --git a/nexus/src/db/model.rs b/nexus/src/db/model.rs index ddacbd3063..ebc837db71 100644 --- a/nexus/src/db/model.rs +++ b/nexus/src/db/model.rs @@ -41,10 +41,19 @@ use uuid::Uuid; // TODO: Break up types into multiple files -/// This macro implements serialization and deserialization of an enum type -/// from our database into our model types. -/// See [`VpcRouterKindEnum`] and [`VpcRouterKind`] for a sample usage -macro_rules! impl_enum_type { +// TODO: The existence of both impl_enum_type and impl_enum_wrapper is a +// temporary state of affairs while we do the work of converting uses of +// impl_enum_wrapper to impl_enum_type. This is part of a broader initiative to +// move types out of the common crate into Nexus where possible. See +// https://github.com/oxidecomputer/omicron/issues/388 + +/// This macro implements serialization and deserialization of an enum type from +/// our database into our model types. This version wraps an enum imported from +/// the common crate in a struct so we can implement DB traits on it. We are +/// moving those enum definitions into this file and using impl_enum_type +/// instead, so eventually this macro will go away. See [`InstanceState`] for a +/// sample usage. +macro_rules! impl_enum_wrapper { ( $(#[$enum_meta:meta])* pub struct $diesel_type:ident; @@ -101,6 +110,70 @@ macro_rules! impl_enum_type { } } +/// This macro implements serialization and deserialization of an enum type from +/// our database into our model types. See [`VpcRouterKindEnum`] and +/// [`VpcRouterKind`] for a sample usage +macro_rules! impl_enum_type { + ( + $(#[$enum_meta:meta])* + pub struct $diesel_type:ident; + + $(#[$model_meta:meta])* + pub enum $model_type:ident; + $($enum_item:ident => $sql_value:literal)+ + ) => { + + $(#[$enum_meta])* + pub struct $diesel_type; + + $(#[$model_meta])* + pub enum $model_type { + $( + $enum_item, + )* + } + + impl ToSql<$diesel_type, DB> for $model_type + where + DB: Backend, + { + fn to_sql( + &self, + out: &mut serialize::Output, + ) -> serialize::Result { + match self { + $( + $model_type::$enum_item => { + out.write_all($sql_value)? + } + )* + } + Ok(IsNull::No) + } + } + + impl FromSql<$diesel_type, DB> for $model_type + where + DB: Backend + for<'a> BinaryRawValue<'a>, + { + fn from_sql(bytes: RawValue) -> deserialize::Result { + match DB::as_bytes(bytes) { + $( + $sql_value => { + Ok($model_type::$enum_item) + } + )* + _ => { + Err(concat!("Unrecognized enum variant for ", + stringify!{$model_type}) + .into()) + } + } + } + } + } +} + /// Newtype wrapper around [external::Name]. #[derive( Clone, @@ -634,7 +707,7 @@ impl_enum_type!( #[derive(Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize, PartialEq)] #[sql_type = "DatasetKindEnum"] - pub struct DatasetKind(pub internal_api::params::DatasetKind); + pub enum DatasetKind; // Enum values Crucible => b"crucible" @@ -644,7 +717,17 @@ impl_enum_type!( impl From for DatasetKind { fn from(k: internal_api::params::DatasetKind) -> Self { - Self(k) + match k { + internal_api::params::DatasetKind::Crucible => { + DatasetKind::Crucible + } + internal_api::params::DatasetKind::Cockroach => { + DatasetKind::Cockroach + } + internal_api::params::DatasetKind::Clickhouse => { + DatasetKind::Clickhouse + } + } } } @@ -687,7 +770,7 @@ impl Dataset { kind: DatasetKind, ) -> Self { let size_used = match kind { - DatasetKind(internal_api::params::DatasetKind::Crucible) => Some(0), + DatasetKind::Crucible => Some(0), _ => None, }; Self { @@ -1067,7 +1150,7 @@ impl Into for InstanceRuntimeState { } } -impl_enum_type!( +impl_enum_wrapper!( #[derive(SqlType, Debug)] #[postgres(type_name = "instance_state", type_schema = "public")] pub struct InstanceStateEnum; @@ -1587,9 +1670,9 @@ impl_enum_type!( #[postgres(type_name = "vpc_router_kind", type_schema = "public")] pub struct VpcRouterKindEnum; - #[derive(Clone, Debug, AsExpression, FromSqlRow)] + #[derive(Clone, Copy, Debug, AsExpression, FromSqlRow, PartialEq)] #[sql_type = "VpcRouterKindEnum"] - pub struct VpcRouterKind(pub external::VpcRouterKind); + pub enum VpcRouterKind; // Enum values System => b"system" @@ -1611,16 +1694,11 @@ impl VpcRouter { pub fn new( router_id: Uuid, vpc_id: Uuid, - kind: external::VpcRouterKind, + kind: VpcRouterKind, params: params::VpcRouterCreate, ) -> Self { let identity = VpcRouterIdentity::new(router_id, params.identity); - Self { - identity, - vpc_id, - kind: VpcRouterKind(kind), - rcgen: Generation::new(), - } + Self { identity, vpc_id, kind, rcgen: Generation::new() } } } @@ -1631,16 +1709,6 @@ impl DatastoreCollection for VpcRouter { type CollectionIdColumn = router_route::dsl::router_id; } -impl Into for VpcRouter { - fn into(self) -> external::VpcRouter { - external::VpcRouter { - identity: self.identity(), - vpc_id: self.vpc_id, - kind: self.kind.0, - } - } -} - #[derive(AsChangeset)] #[table_name = "vpc_router"] pub struct VpcRouterUpdate { @@ -1659,7 +1727,7 @@ impl From for VpcRouterUpdate { } } -impl_enum_type!( +impl_enum_wrapper!( #[derive(SqlType, Debug)] #[postgres(type_name = "router_route_kind", type_schema = "public")] pub struct RouterRouteKindEnum; @@ -1807,7 +1875,7 @@ impl From for RouterRouteUpdate { } } -impl_enum_type!( +impl_enum_wrapper!( #[derive(SqlType, Debug)] #[postgres(type_name = "vpc_firewall_rule_status", type_schema = "public")] pub struct VpcFirewallRuleStatusEnum; @@ -1822,7 +1890,7 @@ impl_enum_type!( NewtypeFrom! { () pub struct VpcFirewallRuleStatus(external::VpcFirewallRuleStatus); } NewtypeDeref! { () pub struct VpcFirewallRuleStatus(external::VpcFirewallRuleStatus); } -impl_enum_type!( +impl_enum_wrapper!( #[derive(SqlType, Debug)] #[postgres(type_name = "vpc_firewall_rule_direction", type_schema = "public")] pub struct VpcFirewallRuleDirectionEnum; @@ -1837,7 +1905,7 @@ impl_enum_type!( NewtypeFrom! { () pub struct VpcFirewallRuleDirection(external::VpcFirewallRuleDirection); } NewtypeDeref! { () pub struct VpcFirewallRuleDirection(external::VpcFirewallRuleDirection); } -impl_enum_type!( +impl_enum_wrapper!( #[derive(SqlType, Debug)] #[postgres(type_name = "vpc_firewall_rule_action", type_schema = "public")] pub struct VpcFirewallRuleActionEnum; @@ -1852,7 +1920,7 @@ impl_enum_type!( NewtypeFrom! { () pub struct VpcFirewallRuleAction(external::VpcFirewallRuleAction); } NewtypeDeref! { () pub struct VpcFirewallRuleAction(external::VpcFirewallRuleAction); } -impl_enum_type!( +impl_enum_wrapper!( #[derive(SqlType, Debug)] #[postgres(type_name = "vpc_firewall_rule_protocol", type_schema = "public")] pub struct VpcFirewallRuleProtocolEnum; @@ -2252,7 +2320,7 @@ impl RoleAssignmentBuiltin { } } -impl_enum_type!( +impl_enum_wrapper!( #[derive(SqlType, Debug, QueryId)] #[postgres(type_name = "update_artifact_kind", type_schema = "public")] pub struct UpdateArtifactKindEnum; diff --git a/nexus/src/external_api/http_entrypoints.rs b/nexus/src/external_api/http_entrypoints.rs index 70894ff6ea..a874ec0ebf 100644 --- a/nexus/src/external_api/http_entrypoints.rs +++ b/nexus/src/external_api/http_entrypoints.rs @@ -11,7 +11,8 @@ use crate::ServerContext; use super::{ console_api, params, views::{ - Organization, Project, Rack, Role, Sled, Snapshot, User, Vpc, VpcSubnet, + Organization, Project, Rack, Role, Sled, Snapshot, User, Vpc, + VpcRouter, VpcSubnet, }, }; use crate::context::OpContext; @@ -57,8 +58,6 @@ use omicron_common::api::external::RouterRouteUpdateParams; use omicron_common::api::external::Saga; use omicron_common::api::external::VpcFirewallRuleUpdateParams; use omicron_common::api::external::VpcFirewallRules; -use omicron_common::api::external::VpcRouter; -use omicron_common::api::external::VpcRouterKind; use ref_cast::RefCast; use schemars::JsonSchema; use serde::Deserialize; @@ -1863,7 +1862,7 @@ async fn vpc_routers_post( &path.organization_name, &path.project_name, &path.vpc_name, - &VpcRouterKind::Custom, + &db::model::VpcRouterKind::Custom, &create_params.into_inner(), ) .await?; diff --git a/nexus/src/external_api/params.rs b/nexus/src/external_api/params.rs index 84e5336f77..2775df390a 100644 --- a/nexus/src/external_api/params.rs +++ b/nexus/src/external_api/params.rs @@ -186,14 +186,14 @@ pub struct VpcSubnetUpdate { // VPC ROUTERS -/// Create-time parameters for a [`VpcRouter`](omicron_common::api::external::VpcRouter) +/// Create-time parameters for a [`VpcRouter`](crate::external_api::views::VpcRouter) #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct VpcRouterCreate { #[serde(flatten)] pub identity: IdentityMetadataCreateParams, } -/// Updateable properties of a [`VpcRouter`](omicron_common::api::external::VpcRouter) +/// Updateable properties of a [`VpcRouter`](crate::external_api::views::VpcRouter) #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)] pub struct VpcRouterUpdate { #[serde(flatten)] diff --git a/nexus/src/external_api/views.rs b/nexus/src/external_api/views.rs index 19a05af68d..76c18158b2 100644 --- a/nexus/src/external_api/views.rs +++ b/nexus/src/external_api/views.rs @@ -129,6 +129,42 @@ impl Into for model::VpcSubnet { } } +#[derive(Clone, Copy, Debug, Deserialize, Serialize, PartialEq, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum VpcRouterKind { + System, + Custom, +} + +impl From for VpcRouterKind { + fn from(k: model::VpcRouterKind) -> Self { + match k { + model::VpcRouterKind::Custom => Self::Custom, + model::VpcRouterKind::System => Self::System, + } + } +} + +/// A VPC router defines a series of rules that indicate where traffic +/// should be sent depending on its destination. +#[derive(ObjectIdentity, Clone, Debug, Deserialize, Serialize, JsonSchema)] +pub struct VpcRouter { + /// common identifying metadata + #[serde(flatten)] + pub identity: IdentityMetadata, + + pub kind: VpcRouterKind, + + /// The VPC to which the router belongs. + pub vpc_id: Uuid, +} + +impl From for VpcRouter { + fn from(r: model::VpcRouter) -> Self { + Self { identity: r.identity(), vpc_id: r.vpc_id, kind: r.kind.into() } + } +} + // RACKS /// Client view of an [`Rack`] diff --git a/nexus/src/nexus.rs b/nexus/src/nexus.rs index 43eea90f41..f5346cd807 100644 --- a/nexus/src/nexus.rs +++ b/nexus/src/nexus.rs @@ -13,6 +13,7 @@ use crate::db::identity::{Asset, Resource}; use crate::db::model::DatasetKind; use crate::db::model::Name; use crate::db::model::VpcRouter; +use crate::db::model::VpcRouterKind; use crate::db::model::VpcSubnet; use crate::db::subnet_allocation::NetworkInterfaceError; use crate::db::subnet_allocation::SubnetError; @@ -50,7 +51,6 @@ use omicron_common::api::external::RouterRouteKind; use omicron_common::api::external::RouterRouteUpdateParams; use omicron_common::api::external::UpdateResult; use omicron_common::api::external::VpcFirewallRuleUpdateParams; -use omicron_common::api::external::VpcRouterKind; use omicron_common::api::internal::nexus; use omicron_common::api::internal::nexus::DiskRuntimeState; use omicron_common::api::internal::nexus::UpdateArtifact; @@ -2282,7 +2282,7 @@ impl Nexus { // database query? This shouldn't affect correctness, assuming that a // router kind cannot be changed, but it might be able to save us a // database round-trip. - if db_router.kind.0 == VpcRouterKind::System { + if db_router.kind == VpcRouterKind::System { return Err(Error::MethodNotAllowed { internal_message: "Cannot delete system router".to_string(), }); diff --git a/nexus/test-utils/src/resource_helpers.rs b/nexus/test-utils/src/resource_helpers.rs index fb67db9707..3f7068e77d 100644 --- a/nexus/test-utils/src/resource_helpers.rs +++ b/nexus/test-utils/src/resource_helpers.rs @@ -16,10 +16,11 @@ use omicron_common::api::external::Disk; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::Instance; use omicron_common::api::external::InstanceCpuCount; -use omicron_common::api::external::VpcRouter; use omicron_nexus::crucible_agent_client::types::State as RegionState; use omicron_nexus::external_api::params; -use omicron_nexus::external_api::views::{Organization, Project, Vpc}; +use omicron_nexus::external_api::views::{ + Organization, Project, Vpc, VpcRouter, +}; use omicron_sled_agent::sim::SledAgent; use std::sync::Arc; use uuid::Uuid; diff --git a/nexus/tests/integration_tests/vpc_routers.rs b/nexus/tests/integration_tests/vpc_routers.rs index 86aeb98d25..547aa2d8c5 100644 --- a/nexus/tests/integration_tests/vpc_routers.rs +++ b/nexus/tests/integration_tests/vpc_routers.rs @@ -17,9 +17,9 @@ use nexus_test_utils::ControlPlaneTestContext; use nexus_test_utils_macros::nexus_test; use omicron_common::api::external::IdentityMetadataCreateParams; use omicron_common::api::external::IdentityMetadataUpdateParams; -use omicron_common::api::external::VpcRouter; -use omicron_common::api::external::VpcRouterKind; use omicron_nexus::external_api::params; +use omicron_nexus::external_api::views::VpcRouter; +use omicron_nexus::external_api::views::VpcRouterKind; #[nexus_test] async fn test_vpc_routers(cptestctx: &ControlPlaneTestContext) { diff --git a/openapi/nexus.json b/openapi/nexus.json index 231e98fa2a..7838f87252 100644 --- a/openapi/nexus.json +++ b/openapi/nexus.json @@ -6673,7 +6673,7 @@ ] }, "VpcRouterCreate": { - "description": "Create-time parameters for a [`VpcRouter`](omicron_common::api::external::VpcRouter)", + "description": "Create-time parameters for a [`VpcRouter`](crate::external_api::views::VpcRouter)", "type": "object", "properties": { "description": { @@ -6717,7 +6717,7 @@ ] }, "VpcRouterUpdate": { - "description": "Updateable properties of a [`VpcRouter`](omicron_common::api::external::VpcRouter)", + "description": "Updateable properties of a [`VpcRouter`](crate::external_api::views::VpcRouter)", "type": "object", "properties": { "description": {