implement to/from sql for semver version

common/Cargo.toml

@@ -19,6 +19,7 @@ rand.workspace = true
 reqwest = { workspace = true, features = ["rustls-tls", "stream"] }
 ring.workspace = true
 schemars = { workspace = true, features = [ "chrono", "uuid1" ] }
+semver.workspace = true
 serde.workspace = true
 serde_derive.workspace = true
 serde_json.workspace = true

common/src/api/external/mod.rs

@@ -23,6 +23,7 @@ use futures::stream::StreamExt;
 use parse_display::Display;
 use parse_display::FromStr;
 use schemars::JsonSchema;
+use semver;
 use serde::Deserialize;
 use serde::Serialize;
 use serde_with::{DeserializeFromStr, SerializeDisplay};
@@ -322,6 +323,37 @@ impl JsonSchema for NameOrId {
     }
 }
 
+// TODO: remove wrapper for semver::Version once this PR goes through
+// https://github.com/GREsau/schemars/pull/195
+#[derive(Clone, Debug, Serialize, Deserialize, PartialEq, Eq)]
+pub struct SemverVersion(pub semver::Version);
+
+impl SemverVersion {
+    pub fn new(major: u64, minor: u64, patch: u64) -> Self {
+        Self(semver::Version::new(major, minor, patch))
+    }
+}
+
+impl JsonSchema for SemverVersion {
+    fn schema_name() -> String {
+        "SemverVersion".to_string()
+    }
+
+    fn json_schema(
+        _: &mut schemars::gen::SchemaGenerator,
+    ) -> schemars::schema::Schema {
+        schemars::schema::SchemaObject {
+            instance_type: Some(schemars::schema::InstanceType::String.into()),
+            string: Some(Box::new(schemars::schema::StringValidation {
+                pattern: Some(r"^\d+\.\d+\.\d+([\-\+].+)?$".to_owned()),
+                ..Default::default()
+            })),
+            ..Default::default()
+        }
+        .into()
+    }
+}
+
 /// Name for a built-in role
 #[derive(
     Clone,

common/src/sql/dbinit.sql

@@ -1433,7 +1433,7 @@ CREATE TABLE omicron.public.system_update (
     version STRING(40) NOT NULL
 );
 
--- TODO: wtf are the values here
+-- TODO: get values from RFD 300
 CREATE TYPE omicron.public.device_type AS ENUM (
     'disk'
 );
@@ -1460,13 +1460,6 @@ CREATE TABLE omicron.public.component_update (
  * Associate system updates with component updates. Not done with a
  * system_update_id field on component_update because the same component update
  * may be part of more than one system update.
- * 
- * TODO: is the one-to-many association necessary? Things would be simpler if we
- * created a new component_update for every (system update, component update).
- * Downsides: less flexibility, possibly a _lot_ of duplication, depending how
- * often component updates are actually unique. I suspect many system updates
- * will only change a couple of components, which means most component updates
- * will appear in more than one system update.
  */
 CREATE TABLE omicron.public.system_update_component_update (
     system_update_id UUID NOT NULL,

nexus/Cargo.toml

@@ -54,7 +54,6 @@ reqwest = { workspace = true, features = [ "json" ] }
 ring.workspace = true
 samael.workspace = true
 schemars = { workspace = true, features = ["chrono", "uuid1"] }
-semver.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 serde_urlencoded.workspace = true

nexus/db-model/Cargo.toml

@@ -22,6 +22,7 @@ pq-sys = "*"
 rand.workspace = true
 ref-cast.workspace = true
 schemars = { workspace = true, features = ["chrono", "uuid1"] }
+semver.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 steno.workspace = true

nexus/db-model/src/lib.rs

@@ -39,6 +39,7 @@ mod organization;
 mod oximeter_info;
 mod producer_endpoint;
 mod project;
+mod semver_version;
 mod system_update;
 // These actually represent subqueries, not real table.
 // However, they must be defined in the same crate as our tables
@@ -116,6 +117,7 @@ pub use region::*;
 pub use region_snapshot::*;
 pub use role_assignment::*;
 pub use role_builtin::*;
+pub use semver_version::*;
 pub use service::*;
 pub use service_kind::*;
 pub use silo::*;

nexus/db-model/src/semver_version.rs

@@ -0,0 +1,55 @@
+// 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 diesel::backend::{Backend, RawValue};
+use diesel::deserialize::{self, FromSql};
+use diesel::query_builder::bind_collector::RawBytesBindCollector;
+use diesel::serialize::{self, ToSql};
+use diesel::sql_types;
+use omicron_common::api::external;
+use serde::{Deserialize, Serialize};
+
+// We wrap semver::Version in external to impl JsonSchema, and we wrap it again
+// here to impl ToSql/FromSql
+
+#[derive(
+    Clone, Debug, AsExpression, FromSqlRow, Serialize, Deserialize, PartialEq,
+)]
+#[diesel(sql_type = sql_types::Text)]
+pub struct SemverVersion(external::SemverVersion);
+
+NewtypeFrom! { () pub struct SemverVersion(external::SemverVersion); }
+NewtypeDeref! { () pub struct SemverVersion(external::SemverVersion); }
+
+impl SemverVersion {
+    pub fn new(major: u64, minor: u64, patch: u64) -> Self {
+        Self(external::SemverVersion(semver::Version::new(major, minor, patch)))
+    }
+}
+
+impl<DB> ToSql<sql_types::Text, DB> for SemverVersion
+where
+    DB: Backend<BindCollector = RawBytesBindCollector<DB>>,
+    String: ToSql<sql_types::Text, DB>,
+{
+    fn to_sql<'a>(
+        &'a self,
+        out: &mut serialize::Output<'a, '_, DB>,
+    ) -> serialize::Result {
+        (self.0).0.to_string().to_sql(&mut out.reborrow())
+    }
+}
+
+impl<DB> FromSql<sql_types::Text, DB> for SemverVersion
+where
+    DB: Backend,
+    String: FromSql<sql_types::Text, DB>,
+{
+    fn from_sql(raw: RawValue<DB>) -> deserialize::Result<Self> {
+        String::from_sql(raw)?
+            .parse()
+            .map(|s| SemverVersion(external::SemverVersion(s)))
+            .map_err(|e| e.into())
+    }
+}

nexus/db-model/src/system_update.rs

@@ -5,6 +5,7 @@
 use crate::{
     impl_enum_type,
     schema::{component_update, system_update, system_update_component_update},
+    SemverVersion,
 };
 use db_macros::Asset;
 use nexus_types::{external_api::views, identity::Asset};
@@ -25,16 +26,14 @@ use uuid::Uuid;
 pub struct SystemUpdate {
     #[diesel(embed)]
     identity: SystemUpdateIdentity,
-    pub version: String,
+    pub version: SemverVersion,
 }
 
 impl From<SystemUpdate> for views::SystemUpdate {
     fn from(system_update: SystemUpdate) -> Self {
         Self {
             identity: system_update.identity(),
-            // TODO: figure out how to ser/de semver versions
-            // version: system_update.version,
-            version: views::SemverVersion::new(1, 0, 0),
+            version: system_update.version.into(),
         }
     }
 }
@@ -76,7 +75,7 @@ impl From<DeviceType> for views::DeviceType {
 pub struct ComponentUpdate {
     #[diesel(embed)]
     identity: ComponentUpdateIdentity,
-    pub version: String,
+    pub version: SemverVersion,
     pub device_type: DeviceType,
     pub parent_id: Option<Uuid>,
 }
@@ -94,9 +93,7 @@ impl From<ComponentUpdate> for views::ComponentUpdate {
     fn from(component_update: ComponentUpdate) -> Self {
         Self {
             identity: component_update.identity(),
-            // TODO: figure out how to ser/de semver versions
-            // version: system_update.version,
-            version: views::SemverVersion::new(1, 0, 0),
+            version: component_update.version.into(),
             device_type: component_update.device_type.into(),
             parent_id: component_update.parent_id,
         }

nexus/src/external_api/http_entrypoints.rs

@@ -10,10 +10,10 @@ use super::{
     console_api, device_auth, params, views,
     views::{
         ComponentUpdate, ComponentVersion, GlobalImage, Group,
-        IdentityProvider, Image, Organization, Project, Rack, Role,
-        SemverVersion, Silo, Sled, Snapshot, SshKey, SystemUpdate,
-        SystemVersion, User, UserBuiltin, VersionRange, VersionStatus,
-        VersionSteadyReason, Vpc, VpcRouter, VpcSubnet,
+        IdentityProvider, Image, Organization, Project, Rack, Role, Silo, Sled,
+        Snapshot, SshKey, SystemUpdate, SystemVersion, User, UserBuiltin,
+        VersionRange, VersionStatus, VersionSteadyReason, Vpc, VpcRouter,
+        VpcSubnet,
     },
 };
 use crate::authz;
@@ -68,6 +68,7 @@ use omicron_common::api::external::RouterRouteCreateParams;
 use omicron_common::api::external::RouterRouteKind;
 use omicron_common::api::external::RouterRouteUpdateParams;
 use omicron_common::api::external::Saga;
+use omicron_common::api::external::SemverVersion;
 use omicron_common::api::external::VpcFirewallRuleUpdateParams;
 use omicron_common::api::external::VpcFirewallRules;
 use omicron_common::bail_unless;
@@ -5139,13 +5140,13 @@ struct SystemUpdatePathParam {
     // TODO: do updates have names? Should this therefore be NameOrId? Do we
     // allow access by some other string identifier (like version) which, like
     // Name, is disjoint with the set of Uuids? NameOrVersion?
-    update_id: Uuid,
+    id: Uuid,
 }
 
 /// View system update
 #[endpoint {
      method = GET,
-     path = "/v1/system/update/updates/{update_id}",
+     path = "/v1/system/update/updates/{id}",
      tags = ["system"],
 }]
 async fn system_update_view(
@@ -5158,7 +5159,7 @@ async fn system_update_view(
     let handler = async {
         let opctx = OpContext::for_external_api(&rqctx).await?;
         let system_update =
-            nexus.system_update_fetch_by_id(&opctx, &path.update_id).await?;
+            nexus.system_update_fetch_by_id(&opctx, &path.id).await?;
         Ok(HttpResponseOk(system_update.into()))
     };
     apictx.external_latencies.instrument_dropshot_handler(&rqctx, handler).await
@@ -5167,7 +5168,7 @@ async fn system_update_view(
 /// View system update component tree
 #[endpoint {
     method = GET,
-    path = "/v1/system/update/updates/{update_id}/components",
+    path = "/v1/system/update/updates/{id}/components",
     tags = ["system"],
 }]
 async fn system_update_components_list(
@@ -5180,7 +5181,7 @@ async fn system_update_components_list(
     let handler = async {
         let opctx = OpContext::for_external_api(&rqctx).await?;
         let components = nexus
-            .system_update_list_components(&opctx, &path.update_id)
+            .system_update_list_components(&opctx, &path.id)
             .await?
             .into_iter()
             .map(|i| i.into())
@@ -5195,7 +5196,7 @@ async fn system_update_components_list(
 /// Start system update
 #[endpoint {
     method = POST,
-    path = "/v1/system/update/updates/{update_id}/start",
+    path = "/v1/system/update/updates/{id}/start",
     tags = ["system"],
 }]
 async fn system_update_start(
@@ -5219,7 +5220,7 @@ async fn system_update_start(
 /// Stop system update
 #[endpoint {
     method = POST,
-    path = "/v1/system/update/updates/{update_id}/stop",
+    path = "/v1/system/update/updates/{id}/stop",
     tags = ["system"],
 }]
 async fn system_update_stop(

nexus/tests/integration_tests/system_updates.rs

@@ -6,6 +6,7 @@ use dropshot::ResultsPage;
 use http::{method::Method, StatusCode};
 use nexus_test_utils::http_testing::{AuthnMode, NexusRequest};
 use nexus_test_utils_macros::nexus_test;
+use omicron_common::api::external::SemverVersion;
 use omicron_nexus::external_api::views;
 use uuid::Uuid;
 
@@ -30,8 +31,8 @@ async fn test_system_version(cptestctx: &ControlPlaneTestContext) {
         version,
         views::SystemVersion {
             version_range: views::VersionRange {
-                low: views::SemverVersion::new(0, 0, 1),
-                high: views::SemverVersion::new(0, 0, 2),
+                low: SemverVersion::new(0, 0, 1),
+                high: SemverVersion::new(0, 0, 2),
             },
             status: views::VersionStatus::Steady {
                 reason: views::VersionSteadyReason::Completed,

nexus/types/Cargo.toml

@@ -12,7 +12,6 @@ newtype_derive.workspace = true
 openssl.workspace = true
 openssl-sys.workspace = true
 openssl-probe.workspace = true
-semver.workspace = true
 schemars = { workspace = true, features = ["chrono", "uuid1"] }
 serde.workspace = true
 serde_json.workspace = true