[tufaceous-lib] make DeserializedManifest public and serializable, fix serialization

Cargo.toml

@@ -160,7 +160,6 @@ bootstrap-agent-client = { path = "clients/bootstrap-agent-client" }
 buf-list = { version = "1.0.3", features = ["tokio1"] }
 byteorder = "1.5.0"
 bytes = "1.5.0"
-bytesize = "1.3.0"
 camino = "1.1"
 camino-tempfile = "1.1.1"
 cancel-safe-futures = "0.1.5"
@@ -283,6 +282,7 @@ oximeter-producer = { path = "oximeter/producer" }
 p256 = "0.13"
 parse-display = "0.8.2"
 partial-io = { version = "0.5.4", features = ["proptest1", "tokio1"] }
+parse-size = "1.0.0"
 paste = "1.0.14"
 percent-encoding = "2.3.1"
 pem = "3.0"

tufaceous-lib/Cargo.toml

@@ -10,7 +10,6 @@ anyhow = { workspace = true, features = ["backtrace"] }
 async-trait.workspace = true
 buf-list.workspace = true
 bytes.workspace = true
-bytesize = { workspace = true, features = ["serde"] }
 camino.workspace = true
 camino-tempfile.workspace = true
 chrono.workspace = true
@@ -22,6 +21,7 @@ hex.workspace = true
 hubtools.workspace = true
 itertools.workspace = true
 omicron-common.workspace = true
+parse-size.workspace = true
 rand.workspace = true
 ring = { workspace = true, features = ["std"] }
 serde.workspace = true

tufaceous-lib/src/assemble/manifest.rs

@@ -5,18 +5,21 @@
 use std::collections::{BTreeMap, BTreeSet};
 
 use anyhow::{bail, ensure, Context, Result};
-use bytesize::ByteSize;
 use camino::{Utf8Path, Utf8PathBuf};
 use omicron_common::api::{
     external::SemverVersion, internal::nexus::KnownArtifactKind,
 };
-use serde::Deserialize;
+use parse_size::parse_size;
+use serde::{Deserialize, Serialize};
 
 use crate::{
     make_filler_text, ArtifactSource, CompositeControlPlaneArchiveBuilder,
     CompositeHostArchiveBuilder, CompositeRotArchiveBuilder,
 };
 
+static FAKE_MANIFEST_TOML: &str =
+    include_str!("../../../tufaceous/manifests/fake.toml");
+
 /// A list of components in a TUF repo representing a single update.
 #[derive(Clone, Debug)]
 pub struct ArtifactManifest {
@@ -36,10 +39,15 @@ impl ArtifactManifest {
 
     /// Deserializes a manifest from an input string.
     pub fn from_str(base_dir: &Utf8Path, input: &str) -> Result<Self> {
-        let de = toml::Deserializer::new(input);
-        let manifest: DeserializedManifest =
-            serde_path_to_error::deserialize(de)?;
+        let manifest = DeserializedManifest::from_str(input)?;
+        Self::from_deserialized(base_dir, manifest)
+    }
 
+    /// Creates a manifest from a [`DeserializedManifest`].
+    pub fn from_deserialized(
+        base_dir: &Utf8Path,
+        manifest: DeserializedManifest,
+    ) -> Result<Self> {
         // Replace all paths in the deserialized manifest with absolute ones,
         // and do some processing to support flexible manifests:
         //
@@ -58,6 +66,7 @@ impl ArtifactManifest {
         // `KnownArtifactKind`s. It would be nicer to enforce this more
         // statically and let serde do these checks, but that seems relatively
         // tricky in comparison to these checks.
+
         Ok(ArtifactManifest {
             system_version: manifest.system_version,
             artifacts: manifest
@@ -88,7 +97,7 @@ impl ArtifactManifest {
                             kind,
                             &data.version,
                         )
-                        .make_data(size.0 as usize);
+                        .make_data(size as usize);
                         ArtifactSource::Memory(fake_data.into())
                     }
                     DeserializedArtifactSource::CompositeHost {
@@ -210,8 +219,6 @@ impl ArtifactManifest {
 
     /// Returns a fake manifest. Useful for testing.
     pub fn new_fake() -> Self {
-        static FAKE_MANIFEST_TOML: &str =
-            include_str!("../../../tufaceous/manifests/fake.toml");
         // The base directory doesn't matter for fake manifests.
         Self::from_str(".".into(), FAKE_MANIFEST_TOML)
             .expect("the fake manifest is a valid manifest")
@@ -298,30 +305,51 @@ pub struct ArtifactData {
 /// we don't expose the `Deserialize` impl on `ArtifactManifest, forcing
 /// consumers to go through [`ArtifactManifest::from_path`] or
 /// [`ArtifactManifest::from_str`].
-#[derive(Clone, Debug, Deserialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 #[serde(rename_all = "snake_case")]
-struct DeserializedManifest {
-    system_version: SemverVersion,
+pub struct DeserializedManifest {
+    pub system_version: SemverVersion,
     #[serde(rename = "artifact")]
-    artifacts: BTreeMap<KnownArtifactKind, Vec<DeserializedArtifactData>>,
+    pub artifacts: BTreeMap<KnownArtifactKind, Vec<DeserializedArtifactData>>,
 }
 
-#[derive(Clone, Debug, Deserialize)]
+impl DeserializedManifest {
+    pub fn from_path(path: &Utf8Path) -> Result<Self> {
+        let input = fs_err::read_to_string(path)?;
+        Self::from_str(&input).with_context(|| {
+            format!("error deserializing manifest from {path}")
+        })
+    }
+
+    pub fn from_str(input: &str) -> Result<Self> {
+        let de = toml::Deserializer::new(input);
+        serde_path_to_error::deserialize(de)
+            .with_context(|| format!("error deserializing manifest"))
+    }
+
+    /// Returns the fake manifest.
+    pub fn fake() -> Self {
+        Self::from_str(FAKE_MANIFEST_TOML).unwrap()
+    }
+}
+
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 #[serde(rename_all = "snake_case")]
-struct DeserializedArtifactData {
+pub struct DeserializedArtifactData {
     pub name: String,
     pub version: SemverVersion,
     pub source: DeserializedArtifactSource,
 }
 
-#[derive(Clone, Debug, Deserialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 #[serde(tag = "kind", rename_all = "kebab-case")]
-enum DeserializedArtifactSource {
+pub enum DeserializedArtifactSource {
     File {
         path: Utf8PathBuf,
     },
     Fake {
-        size: ByteSize,
+        #[serde(deserialize_with = "deserialize_byte_size")]
+        size: u64,
     },
     CompositeHost {
         phase_1: DeserializedFileArtifactSource,
@@ -336,11 +364,16 @@ enum DeserializedArtifactSource {
     },
 }
 
-#[derive(Clone, Debug, Deserialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 #[serde(tag = "kind", rename_all = "snake_case")]
-enum DeserializedFileArtifactSource {
-    File { path: Utf8PathBuf },
-    Fake { size: ByteSize },
+pub enum DeserializedFileArtifactSource {
+    File {
+        path: Utf8PathBuf,
+    },
+    Fake {
+        #[serde(deserialize_with = "deserialize_byte_size")]
+        size: u64,
+    },
 }
 
 impl DeserializedFileArtifactSource {
@@ -354,18 +387,24 @@ impl DeserializedFileArtifactSource {
                     .with_context(|| format!("failed to read {path}"))?
             }
             DeserializedFileArtifactSource::Fake { size } => {
-                fake_attr.make_data(size.0 as usize)
+                fake_attr.make_data(*size as usize)
             }
         };
         f(data)
     }
 }
 
-#[derive(Clone, Debug, Deserialize, serde::Serialize)]
+#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
 #[serde(tag = "kind", rename_all = "snake_case")]
-enum DeserializedControlPlaneZoneSource {
-    File { path: Utf8PathBuf },
-    Fake { name: String, size: ByteSize },
+pub enum DeserializedControlPlaneZoneSource {
+    File {
+        path: Utf8PathBuf,
+    },
+    Fake {
+        name: String,
+        #[serde(deserialize_with = "deserialize_byte_size")]
+        size: u64,
+    },
 }
 
 impl DeserializedControlPlaneZoneSource {
@@ -383,10 +422,76 @@ impl DeserializedControlPlaneZoneSource {
                 (name, data)
             }
             DeserializedControlPlaneZoneSource::Fake { name, size } => {
-                let data = make_filler_text(size.0 as usize);
+                let data = make_filler_text(*size as usize);
                 (name.as_str(), data)
             }
         };
         f(name, data)
     }
 }
+
+fn deserialize_byte_size<'de, D>(deserializer: D) -> Result<u64, D::Error>
+where
+    D: serde::Deserializer<'de>,
+{
+    // Attempt to deserialize the size as either a string or an integer.
+
+    struct Visitor;
+
+    impl<'de> serde::de::Visitor<'de> for Visitor {
+        type Value = u64;
+
+        fn expecting(
+            &self,
+            formatter: &mut std::fmt::Formatter,
+        ) -> std::fmt::Result {
+            formatter
+                .write_str("a string representing a byte size or an integer")
+        }
+
+        fn visit_str<E>(self, value: &str) -> Result<Self::Value, E>
+        where
+            E: serde::de::Error,
+        {
+            parse_size(value).map_err(|_| {
+                serde::de::Error::invalid_value(
+                    serde::de::Unexpected::Str(value),
+                    &self,
+                )
+            })
+        }
+
+        // TOML uses i64, not u64
+        fn visit_i64<E>(self, value: i64) -> Result<Self::Value, E>
+        where
+            E: serde::de::Error,
+        {
+            Ok(value as u64)
+        }
+
+        fn visit_u64<E>(self, value: u64) -> Result<Self::Value, E>
+        where
+            E: serde::de::Error,
+        {
+            Ok(value)
+        }
+    }
+
+    deserializer.deserialize_any(Visitor)
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // Ensure that the fake manifest roundtrips after serialization and
+    // deserialization.
+    #[test]
+    fn fake_roundtrip() {
+        let manifest = DeserializedManifest::fake();
+        let toml = toml::to_string(&manifest).unwrap();
+        let deserialized = DeserializedManifest::from_str(&toml)
+            .expect("fake manifest is a valid manifest");
+        assert_eq!(manifest, deserialized);
+    }
+}