client: prepare instance spec types for use in sled-agent API (#896)

The next step toward implementing virtual platforms in Omicron is to
move instance spec construction from sled-agent up to Nexus. This
requires instance spec types to appear in sled-agent's API. To allow
this:

- Direct the generated client to add a `schemars::JsonSchema` derive to
  all generated types.
- Add `rename = snake_case` attributes to instance spec enum types that
  sled-agent expects to reuse. Omicron requires all of its API
  specifications to pass the OpenAPI linting rules defined in the
  openapi-lint crate (https://github.com/oxidecomputer/openapi-lint);
  among these is a requirement that enum variants all have snake case
  names.

Author: gjcolombo

Cargo.lock

@@ -151,7 +151,6 @@ serde_arrays = "0.1"
 serde_derive = "1.0"
 serde_json = "1.0"
 serde_test = "1.0.138"
-serde_with = "3.11.0"
 slog = "2.7"
 slog-async = "2.8"
 slog-bunyan = "2.4.0"

Cargo.toml

@@ -27,6 +27,7 @@ dropshot = { workspace = true }
 futures.workspace = true
 hyper.workspace = true
 serde.workspace = true
+propolis_api_types.workspace = true
 propolis_types.workspace = true
 semver.workspace = true
 serde_json.workspace = true

bin/mock-server/Cargo.toml

@@ -8,6 +8,9 @@ use serde::{Deserialize, Serialize};
 progenitor::generate_api!(
     spec = "../../openapi/propolis-server.json",
     derives = [schemars::JsonSchema],
+    replace = {
+        SpecKey = propolis_api_types::instance_spec::SpecKey,
+    },
     patch = {
         InstanceMetadata = { derives = [Clone, Eq, PartialEq] },
         InstanceProperties = { derives = [ Clone, Eq, PartialEq ] },

bin/mock-server/src/lib/api_types.rs

@@ -22,9 +22,9 @@ use propolis_client::types::{
     GuestHypervisorInterface, HyperVFeatureFlag, I440Fx, InstanceEnsureRequest,
     InstanceInitializationMethod, InstanceMetadata, InstanceSpecGetResponse,
     InstanceSpecV0, NvmeDisk, QemuPvpanic, ReplacementComponent, SerialPort,
-    SerialPortNumber, VirtioDisk,
+    SerialPortNumber, SpecKey, VirtioDisk,
 };
-use propolis_client::{PciPath, SpecKey};
+use propolis_client::PciPath;
 use propolis_config_toml::spec::SpecConfig;
 use serde::{Deserialize, Serialize};
 use slog::{o, Drain, Level, Logger};

bin/propolis-cli/src/main.rs

@@ -12,7 +12,6 @@ crucible-client-types.workspace = true
 propolis_types.workspace = true
 schemars.workspace = true
 serde.workspace = true
-serde_with.workspace = true
 thiserror.workspace = true
 uuid.workspace = true
 

crates/propolis-api-types/Cargo.toml

@@ -114,14 +114,19 @@ pub struct CpuidEntry {
     Eq,
     PartialEq,
 )]
-#[serde(deny_unknown_fields)]
+#[serde(deny_unknown_fields, rename_all = "snake_case")]
 pub enum HyperVFeatureFlag {
     ReferenceTsc,
 }
 
 /// A hypervisor interface to expose to the guest.
 #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema, Default)]
-#[serde(deny_unknown_fields, tag = "type", content = "value")]
+#[serde(
+    deny_unknown_fields,
+    rename_all = "snake_case",
+    tag = "type",
+    content = "value"
+)]
 pub enum GuestHypervisorInterface {
     /// Expose a bhyve-like interface ("bhyve bhyve " as the hypervisor ID in
     /// leaf 0x4000_0000 and no additional leaves or features).

crates/propolis-api-types/src/instance_spec/components/board.rs

@@ -155,7 +155,6 @@
 
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use serde_with::{DeserializeFromStr, SerializeDisplay};
 
 pub use propolis_types::{CpuidIdent, CpuidValues, CpuidVendor, PciPath};
 use uuid::Uuid;
@@ -187,21 +186,20 @@ pub mod v0;
 // names if they have no UUIDs available or the most obvious UUID is in use
 // elsewhere. The key type's From impls will try to parse strings into UUIDs
 // before storing keys as strings.
-//
-// This type derives `SerializeDisplay` and `DeserializeFromStr` so that it can
-// be used as a map key when serializing to JSON, which requires strings (and
-// not objects) as keys.
 #[derive(
-    Clone,
-    Debug,
-    SerializeDisplay,
-    DeserializeFromStr,
-    Eq,
-    PartialEq,
-    Ord,
-    PartialOrd,
-    JsonSchema,
+    Clone, Debug, Serialize, Deserialize, Eq, PartialEq, Ord, PartialOrd,
 )]
+// Direct serde to use an untagged enum representation for this type. Since both
+// Uuid and String serialize to strings, this allows other types that contain a
+// Map<K = SpecKey> to derive Serialize and successfully serialize to JSON.
+// (This doesn't work with a tagged representation because JSON doesn't allow
+// maps to be used as map keys.)
+//
+// Note that this makes the order of variants matter: serde will pick the first
+// variant into which it can successfully deserialize an untagged enum value,
+// and the point is to use the UUID representation for any value that can be
+// interpreted as a UUID.
+#[serde(untagged)]
 pub enum SpecKey {
     Uuid(Uuid),
     Name(String),
@@ -217,10 +215,7 @@ impl std::fmt::Display for SpecKey {
 }
 
 impl std::str::FromStr for SpecKey {
-    // This conversion is infallible, but the error type needs to implement
-    // `Display` for `SpecKey` to derive `DeserializeFromStr`.
-    type Err = &'static str;
-
+    type Err = core::convert::Infallible;
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         Ok(match Uuid::parse_str(s) {
             Ok(uuid) => Self::Uuid(uuid),
@@ -244,6 +239,62 @@ impl From<Uuid> for SpecKey {
     }
 }
 
+// Manually implement JsonSchema to help Progenitor generate the expected enum
+// type for spec keys.
+impl JsonSchema for SpecKey {
+    fn schema_name() -> String {
+        "SpecKey".to_owned()
+    }
+
+    fn json_schema(
+        generator: &mut schemars::gen::SchemaGenerator,
+    ) -> schemars::schema::Schema {
+        use schemars::schema::*;
+        fn label_schema(label: &str, schema: Schema) -> Schema {
+            SchemaObject {
+                metadata: Some(
+                    Metadata {
+                        title: Some(label.to_string()),
+                        ..Default::default()
+                    }
+                    .into(),
+                ),
+                subschemas: Some(
+                    SubschemaValidation {
+                        all_of: Some(vec![schema]),
+                        ..Default::default()
+                    }
+                    .into(),
+                ),
+                ..Default::default()
+            }
+            .into()
+        }
+
+        SchemaObject {
+            metadata: Some(
+                Metadata {
+                    description: Some(
+                        "A key identifying a component in an instance spec."
+                            .to_string(),
+                    ),
+                    ..Default::default()
+                }
+                .into(),
+            ),
+            subschemas: Some(Box::new(SubschemaValidation {
+                one_of: Some(vec![
+                    label_schema("uuid", generator.subschema_for::<Uuid>()),
+                    label_schema("name", generator.subschema_for::<String>()),
+                ]),
+                ..Default::default()
+            })),
+            ..Default::default()
+        }
+        .into()
+    }
+}
+
 /// A versioned instance spec.
 #[derive(Clone, Debug, Deserialize, Serialize, JsonSchema)]
 #[serde(deny_unknown_fields, tag = "version", content = "spec")]

crates/propolis-api-types/src/instance_spec/mod.rs

@@ -9,7 +9,12 @@ use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 
 #[derive(Clone, Deserialize, Serialize, Debug, JsonSchema)]
-#[serde(deny_unknown_fields, tag = "type", content = "component")]
+#[serde(
+    deny_unknown_fields,
+    tag = "type",
+    content = "component",
+    rename_all = "snake_case"
+)]
 pub enum ComponentV0 {
     VirtioDisk(components::devices::VirtioDisk),
     NvmeDisk(components::devices::NvmeDisk),

crates/propolis-api-types/src/instance_spec/v0.rs

@@ -14,10 +14,10 @@ use propolis_client::{
     types::{
         ComponentV0, DlpiNetworkBackend, FileStorageBackend,
         MigrationFailureInjector, NvmeDisk, P9fs, PciPciBridge, SoftNpuP9,
-        SoftNpuPciPort, SoftNpuPort, VirtioDisk, VirtioNetworkBackend,
+        SoftNpuPciPort, SoftNpuPort, SpecKey, VirtioDisk, VirtioNetworkBackend,
         VirtioNic,
     },
-    PciPath, SpecKey,
+    PciPath,
 };
 use thiserror::Error;
 
@@ -115,7 +115,7 @@ impl TryFrom<&super::Config> for SpecConfig {
         };
 
         for (device_name, device) in config.devices.iter() {
-            let device_id = SpecKey::from(device_name.clone());
+            let device_id = SpecKey::Name(device_name.clone());
             let driver = device.driver.as_str();
             if device_name == MIGRATION_FAILURE_DEVICE_NAME {
                 const FAIL_EXPORTS: &str = "fail_exports";