sundog: use JSON output to allow non-String generated model types

packages/workspaces/data-store-version

@@ -1 +1 @@
-0.0
+0.1

workspaces/Cargo.toml

@@ -18,7 +18,7 @@ members = [
 
     # Migrations will be listed here.  They won't need to be listed as a
     # workspace member when we add cross-workspace dependencies.
-    "api/migration/migrations/v1.1/testmigration",
+    "api/migration/migrations/v0.1/borkseed",
 
     "preinit/laika",
 

workspaces/api/apiserver/src/model.rs

@@ -88,7 +88,7 @@ pub struct UpdatesSettings {
     pub target_base_url: Option<String>,
 
     #[serde(skip_serializing_if = "Option::is_none")]
-    pub seed: Option<String>,
+    pub seed: Option<u32>,
 }
 
 #[derive(Debug, PartialEq, Serialize, Deserialize)]

workspaces/api/bork/Cargo.toml

@@ -6,4 +6,5 @@ edition = "2018"
 publish = false
 
 [dependencies]
-rand = "0.7.0"
+rand = "0.7.0"
+serde_json = "1"

workspaces/api/bork/src/main.rs

@@ -4,5 +4,11 @@ use rand::{thread_rng, Rng};
 
 fn main() {
     let mut rng = thread_rng();
-    println!("{}", rng.gen_range(0, 2048));
+    let val = rng.gen_range(0, 2048);
+
+    // sundog expects JSON-serialized output so that many types can be represented, allowing the
+    // API model to use more accurate types.
+    let output = serde_json::to_string(&val).expect("Unable to serialize val '{}' to JSON");
+
+    println!("{}", output);
 }

workspaces/api/migration/migrations/v0.1/borkseed/Cargo.toml

@@ -0,0 +1,11 @@
+[package]
+name = "borkseed-migration"
+version = "0.1.0"
+authors = ["Tom Kirchner <[email protected]>"]
+edition = "2018"
+publish = false
+
+[dependencies]
+migration-helpers = { path = "../../../migration-helpers" }
+serde_json = "1.0"
+snafu = "0.5"

workspaces/api/migration/migrations/v0.1/borkseed/src/main.rs

@@ -0,0 +1,84 @@
+#![deny(rust_2018_idioms)]
+
+use migration_helpers::{error, migrate, Migration, MigrationData, Result};
+use std::convert::TryFrom;
+
+/// We moved from String to u32 for the seed value generated by bork and used by updog.
+struct BorkSeedIntMigration;
+
+impl Migration for BorkSeedIntMigration {
+    fn forward(&mut self, mut input: MigrationData) -> Result<MigrationData> {
+        if let Some(seed_val) = input.data.get_mut("settings.updates.seed") {
+            // We have the seed setting; check its type to see what we can do.
+            if let Some(seed_str) = seed_val.as_str() {
+                // Confirm the string is a valid u32.
+                let seed: u32 = seed_str.parse().or_else(|e| {
+                    error::Migration {
+                        msg: format!("Existing update seed string is not a valid u32: {}", e),
+                    }
+                    .fail()
+                })?;
+                *seed_val = serde_json::Value::Number(seed.into());
+            } else if let Some(seed_num) = seed_val.as_u64() {
+                // We shouldn't find a number because the migration should only run against a
+                // version with a String seed, but... as long as it's a valid u32, allow it.
+                let seed = u32::try_from(seed_num).or_else(|e| {
+                    error::Migration {
+                        msg: format!("Existing update seed number(!) is not a valid u32: {}", e),
+                    }
+                    .fail()
+                })?;
+                *seed_val = serde_json::Value::Number(seed.into());
+            } else {
+                // Other type, shouldn't happen, error.
+                return error::Migration {
+                    msg: format!("Unsupported type of existing update seed: '{:?}'", seed_val),
+                }
+                .fail();
+            }
+        } else {
+            // If they don't have a seed, one will be generated on startup.
+        }
+        Ok(input)
+    }
+
+    fn backward(&mut self, mut input: MigrationData) -> Result<MigrationData> {
+        if let Some(seed_val) = input.data.get_mut("settings.updates.seed") {
+            // We have the seed setting; check its type to see what we can do.
+            if let Some(seed_num) = seed_val.as_u64() {
+                // Number back to string, just serialize.
+                let seed_str = serde_json::to_string(&seed_num).or_else(|e| {
+                    error::Migration {
+                        msg: format!("Existing update seed number failed serialization: {}", e),
+                    }
+                    .fail()
+                })?;
+                *seed_val = serde_json::Value::String(seed_str);
+            } else if let Some(seed_str) = seed_val.as_str() {
+                // We shouldn't find a string because the migration should only run against a
+                // version with a number seed, but... as long as it's a valid u32, allow it.
+                // JUST FOR VALIDATION:
+                let _seed: u32 = seed_str.parse().or_else(|e| {
+                    error::Migration {
+                        msg: format!("Existing update seed string(!) is not a valid u32: {}", e),
+                    }
+                    .fail()
+                })?;
+                // Do nothing; keep the original (valid) string.
+            } else {
+                // Other type, shouldn't happen, error.
+                return error::Migration {
+                    msg: format!("Unsupported type of existing update seed: '{:?}'", seed_val),
+                }
+                .fail();
+            }
+        } else {
+            // If they don't have a seed, one will be generated on startup.
+        }
+        Ok(input)
+    }
+}
+
+fn main() -> Result<()> {
+    migrate(BorkSeedIntMigration)
+}

workspaces/api/pluto/src/main.rs

@@ -73,6 +73,16 @@ mod error {
             source: serde_json::error::Error,
         },
 
+        #[snafu(display(
+            "Error serializing to JSON from command output '{}': {}",
+            output,
+            source
+        ))]
+        OutputJson {
+            output: String,
+            source: serde_json::error::Error,
+        },
+
         #[snafu(display(
             "Missing 'region' key in Instance Identity Document from IMDS: {}",
             path
@@ -233,6 +243,10 @@ fn main() -> Result<()> {
         _ => usage(),
     }?;
 
-    println!("{}", setting);
+    // sundog expects JSON-serialized output so that many types can be represented, allowing the
+    // API model to use more accurate types.
+    let output = serde_json::to_string(&setting).context(error::OutputJson { output: &setting })?;
+
+    println!("{}", output);
     Ok(())
 }

workspaces/api/sundog/src/main.rs

@@ -112,6 +112,24 @@ mod error {
             source: serde_json::Error,
         },
 
+        #[snafu(display(
+            "Error deserializing command output as JSON from {}: '{}' ;: input: {}",
+            generator,
+            source,
+            input,
+        ))]
+        CommandJson {
+            generator: String,
+            input: String,
+            source: serde_json::Error,
+        },
+
+        #[snafu(display("Can't deserialize {} from command output '{}'", expected, input,))]
+        CommandJsonType {
+            expected: &'static str,
+            input: serde_json::Value,
+        },
+
         #[snafu(display("Error deserializing HashMap to Settings: {}", source))]
         Deserialize { source: deserialization::Error },
 
@@ -126,9 +144,9 @@ mod error {
             source: tracing_subscriber::filter::LevelParseError,
         },
 
-        #[snafu(display("Error serializing command output '{}': {}", output, source))]
+        #[snafu(display("Error serializing command output '{}': {}", value, source))]
         SerializeScalar {
-            output: String,
+            value: serde_json::Value,
             source: datastore::ScalarError,
         },
     }
@@ -292,20 +310,35 @@ where
             }
         }
 
-        // Build a valid utf8 string from the stdout and trim any whitespace
-        let output = str::from_utf8(&result.stdout)
+        // Sundog programs are expected to output JSON, which allows them to represent types other
+        // than strings, which in turn allows our API model to use types more accurate than strings
+        // for generated settings.
+        //
+        // First, we pull the raw string from the process output.
+        let output_raw = str::from_utf8(&result.stdout)
             .context(error::GeneratorOutput {
                 program: generator.as_str(),
             })?
             .trim()
             .to_string();
-        trace!("Generator '{}' output: {}", &generator, &output);
+        trace!("Generator '{}' output: {}", &generator, &output_raw);
 
-        // The command output must be serialized since we intend to call the
-        // datastore-level construct `from_map` on it. `from_map` treats
-        // strings as a serialized structure.
+        // Next, we deserialize the text into a Value that can represent any JSON type.
+        let output_value: serde_json::Value =
+            serde_json::from_str(&output_raw).context(error::CommandJson {
+                generator: &generator,
+                input: &output_raw,
+            })?;
+
+        // Finally, we re-serialize the command output; we intend to call the datastore-level
+        // construct `from_map` on it, which expects serialized values.
+        //
+        // We have to go through the round-trip of serialization because the data store
+        // serialization format may not be the same as the format we choose for sundog.
         let serialized_output =
-            datastore::serialize_scalar(&output).context(error::SerializeScalar { output })?;
+            datastore::serialize_scalar(&output_value).context(error::SerializeScalar {
+                value: output_value,
+            })?;
         trace!("Serialized output: {}", &serialized_output);
 
         settings.insert(setting, serialized_output);

workspaces/deny.toml

@@ -31,6 +31,7 @@ skip = [
     { name = "apiserver", licenses = [] },
     { name = "block-party", licenses = [] },
     { name = "bork", licenses = [] },
+    { name = "borkseed-migration", licenses = [] },
     { name = "data_store_version", licenses = [] },
     { name = "growpart", licenses = [] },
     { name = "host-containers", licenses = [] },

workspaces/updater/updog/src/de.rs

@@ -12,17 +12,17 @@ use std::str::FromStr;
 /// Converts the bound key to an integer before insertion and catches duplicates
 pub(crate) fn deserialize_bound<'de, D>(
     deserializer: D,
-) -> Result<BTreeMap<u64, DateTime<Utc>>, D::Error>
+) -> Result<BTreeMap<u32, DateTime<Utc>>, D::Error>
 where
     D: Deserializer<'de>,
 {
     fn bound_to_int(
         key: String,
         time: DateTime<Utc>,
-        map: &mut BTreeMap<u64, DateTime<Utc>>,
+        map: &mut BTreeMap<u32, DateTime<Utc>>,
     ) -> Result<(), error::Error> {
         let bound = key
-            .parse::<u64>()
+            .parse::<u32>()
             .context(error::BadBound { bound_str: key })?;
         ensure!(
             map.insert(bound, time).is_none(),
@@ -35,7 +35,7 @@ where
     struct Visitor;
 
     impl<'de> serde::de::Visitor<'de> for Visitor {
-        type Value = BTreeMap<u64, DateTime<Utc>>;
+        type Value = BTreeMap<u32, DateTime<Utc>>;
 
         fn expecting(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result {
             formatter.write_str("a map")

workspaces/updater/updog/src/error.rs

@@ -75,7 +75,7 @@ pub(crate) enum Error {
     },
 
     #[snafu(display("Duplicate key ID: {}", keyid))]
-    DuplicateKeyId { backtrace: Backtrace, keyid: u64 },
+    DuplicateKeyId { backtrace: Backtrace, keyid: u32 },
 
     #[snafu(display("Duplicate version key: {}", key))]
     DuplicateVersionKey { backtrace: Backtrace, key: String },

workspaces/updater/updog/src/main.rs

@@ -32,7 +32,7 @@ const TARGET_ARCH: &str = "aarch64";
 
 const TRUSTED_ROOT_PATH: &str = "/usr/share/updog/root.json";
 const MIGRATION_PATH: &str = "/var/lib/thar/datastore/migrations";
-const MAX_SEED: u64 = 2048;
+const MAX_SEED: u32 = 2048;
 
 #[derive(Debug, Deserialize, PartialEq)]
 #[serde(rename_all = "kebab-case")]
@@ -49,7 +49,7 @@ enum Command {
 struct Config {
     metadata_base_url: String,
     target_base_url: String,
-    seed: u64,
+    seed: u32,
     // TODO API sourced configuration, eg.
     // blacklist: Option<Vec<Version>>,
     // mode: Option<{Automatic, Managed, Disabled}>
@@ -69,19 +69,19 @@ struct Update {
     version: Version,
     max_version: Version,
     #[serde(deserialize_with = "de::deserialize_bound")]
-    waves: BTreeMap<u64, DateTime<Utc>>,
+    waves: BTreeMap<u32, DateTime<Utc>>,
     images: Images,
 }
 
 impl Update {
-    pub fn update_wave(&self, seed: u64) -> Option<&DateTime<Utc>> {
+    pub fn update_wave(&self, seed: u32) -> Option<&DateTime<Utc>> {
         if let Some((_, wave)) = self.waves.range((Included(0), Included(seed))).last() {
             return Some(wave);
         }
         None
     }
 
-    fn update_ready(&self, seed: u64) -> bool {
+    fn update_ready(&self, seed: u32) -> bool {
         // Has this client's wave started
         if let Some(wave) = self.update_wave(seed) {
             return *wave <= Utc::now();
@@ -96,7 +96,7 @@ impl Update {
         true
     }
 
-    pub fn jitter(&self, seed: u64) -> Option<DateTime<Utc>> {
+    pub fn jitter(&self, seed: u32) -> Option<DateTime<Utc>> {
         let prev = self.update_wave(seed);
         let next = self
             .waves