fix(sync): restore the empty params table for a Build image

bin/core/src/sync/replace_ids.rs

@@ -114,15 +114,19 @@ impl ReplaceIds for Deployment {
         .unwrap_or(&String::new()),
     );
 
+    // Leave the id alone when the lookup misses, rather than blanking it.
+    // `Deployment::edit_config_object` renames this field to `build` and
+    // REMOVES `version` when it is 0.0.0 (which means `latest`), so a blanked
+    // id leaves the `params` table empty. `skip_empty_object` then drops the
+    // table, and `DeploymentImage` is adjacently tagged, so the emitted
+    // `image.type = "Build"` cannot be read back: `missing field \`params\``.
+    // Nothing clears this reference when the Build is deleted, unlike
+    // `delete_from_alerters` for alerter targets, so a dangling id is normal.
     if let DeploymentImage::Build { build_id, .. } = &mut config.image
     {
-      build_id.clone_from(
-        all
-          .builds
-          .get(build_id)
-          .map(|b| &b.name)
-          .unwrap_or(&String::new()),
-      );
+      if let Some(build) = all.builds.get(build_id) {
+        build_id.clone_from(&build.name);
+      }
     }
   }
 }

bin/core/src/sync/toml.rs

@@ -199,6 +199,27 @@ impl ToToml for Stack {
 }
 
 impl ToToml for Deployment {
+  fn push_additional(
+    resource: ResourceToml<Self::PartialConfig>,
+    toml: &mut String,
+  ) {
+    // Same hazard the Builder impl below works around, one level deeper. For a
+    // Build image, `edit_config_object` renames `build_id` to `build` and drops
+    // `version` when it is 0.0.0 (`latest`). If the build is also unset, every
+    // key is gone, `skip_empty_object` removes the whole `params` table, and the
+    // emitted `image.type = "Build"` cannot be deserialized back, because
+    // `DeploymentImage` is adjacently tagged. Restore the empty table so the
+    // file stays readable.
+    let empty_params = matches!(
+      &resource.config.image,
+      Some(DeploymentImage::Build { build_id, version })
+        if build_id.is_empty() && version.is_none()
+    );
+    if empty_params {
+      toml.push_str("\nimage.params = {}");
+    }
+  }
+
   fn edit_config_object(
     resource: &ResourceToml<Self::PartialConfig>,
     config: IndexMap<String, serde_json::Value>,

bin/core/tests/deployment_build_image_toml.rs

@@ -0,0 +1,75 @@
+//! Proves both halves of the Build-image TOML fix.
+//!
+//! 1. An empty `params` table is dropped by the sync's serializer options and
+//!    the result cannot be deserialized back, because `DeploymentImage` is
+//!    adjacently tagged. This is the bug.
+//! 2. Restoring `image.params = {}`, the way `Deployment::push_additional` now
+//!    does, makes the emitted TOML round trip.
+use komodo_client::entities::deployment::DeploymentImage;
+
+const OPTIONS: toml_pretty::Options = toml_pretty::Options {
+  tab: "  ",
+  skip_empty_string: true,
+  skip_empty_object: true,
+  max_inline_array_length: 30,
+  inline_array: false,
+};
+
+#[derive(Debug, serde::Deserialize)]
+struct Wrapper {
+  image: DeploymentImage,
+}
+
+/// What `edit_config_object` leaves for a Build image whose build is unset (or
+/// was blanked) and whose version is 0.0.0: `build_id` renamed to `build`, and
+/// `version` removed by the `version.is_none()` branch.
+fn emitted_config() -> String {
+  let config = serde_json::json!({
+    "image": { "type": "Build", "params": { "build": "" } },
+  });
+  toml_pretty::to_string(&config, OPTIONS).expect("serialize")
+}
+
+#[test]
+fn empty_build_params_is_dropped_and_cannot_be_read_back() {
+  let toml = emitted_config();
+  assert_eq!(toml.trim(), "image.type = \"Build\"");
+  let err = toml::from_str::<Wrapper>(&toml)
+    .expect_err("a params-less Build image must not deserialize");
+  assert!(
+    err.to_string().contains("params"),
+    "expected a missing-params error, got: {err}"
+  );
+}
+
+#[test]
+fn restoring_the_empty_params_table_round_trips() {
+  // Exactly what `Deployment::push_additional` appends.
+  let toml = format!("{}\nimage.params = {{}}", emitted_config());
+  let parsed: Wrapper = toml::from_str(&toml)
+    .expect("with params restored it must deserialize");
+  match parsed.image {
+    DeploymentImage::Build { build_id, version } => {
+      assert!(build_id.is_empty());
+      assert!(version.is_none());
+    }
+    other => panic!("wrong variant: {other:?}"),
+  }
+}
+
+#[test]
+fn a_populated_build_image_is_unaffected() {
+  let config = serde_json::json!({
+    "image": { "type": "Build", "params": { "build": "my-build" } },
+  });
+  let toml =
+    toml_pretty::to_string(&config, OPTIONS).expect("serialize");
+  assert!(toml.contains("image.params.build = \"my-build\""));
+  let parsed: Wrapper = toml::from_str(&toml).expect("round trip");
+  match parsed.image {
+    DeploymentImage::Build { build_id, .. } => {
+      assert_eq!(build_id, "my-build")
+    }
+    other => panic!("wrong variant: {other:?}"),
+  }
+}