feat: Allow specifying sync steps in conjunction with recipes

CHANGELOG.md

@@ -8,6 +8,8 @@ air-gapped signing
 
 # Unreleased
 
+* feat: a canister that uses a `recipe` can now declare its own `sync` steps, which previously was rejected outright. They run after the sync steps the recipe renders, so a recipe's post-deployment work stays intact and yours is appended to it. `recipe` and `build` remain mutually exclusive.
+
 # v1.4.0
 
 * feat: a canister can now declare `upgrade_args` alongside `init_args`, in its own manifest and as a per-canister environment override. It is passed when `icp deploy` upgrades the canister, where `init_args` is passed when it installs or reinstalls it. It takes exactly the forms `init_args` does (inline Candid string, or `{ value | path, format }`), and paths resolve against the canister's own directory the same way. A canister that declares no `upgrade_args` is upgraded with its `init_args`, as before, and `--args` / `--args-file` still override whichever applies.

crates/icp-cli/tests/recipe_tests.rs

@@ -332,3 +332,67 @@ fn recipe_local_file_valid_checksum() {
         .assert()
         .success();
 }
+
+/// A canister may declare sync steps alongside a recipe; they land after the
+/// steps the recipe renders.
+#[test]
+fn recipe_with_manifest_sync_steps() {
+    let ctx = TestContext::new();
+
+    // Setup project
+    let project_dir = ctx.create_project_dir("icp");
+
+    // Recipe rendering a sync step of its own
+    write_string(
+        &project_dir.join("recipe.hbs"), // path
+        indoc! {r#"
+            build:
+              steps:
+                - type: script
+                  command: echo "test" > "$ICP_WASM_OUTPUT_PATH"
+            sync:
+              steps:
+                - type: script
+                  command: echo from-recipe
+        "#}, // contents
+    )
+    .expect("failed to write recipe template");
+
+    let pm = indoc! {"
+        canisters:
+          - name: my-canister
+            recipe:
+              type: file://./recipe.hbs
+            sync:
+              steps:
+                - type: script
+                  command: echo from-manifest
+    "};
+
+    write_string(
+        &project_dir.join("icp.yaml"), // path
+        pm,                            // contents
+    )
+    .expect("failed to write project manifest");
+
+    // The effective configuration holds both steps, the recipe's first
+    let assert = ctx
+        .icp()
+        .current_dir(project_dir)
+        .args(["project", "show"])
+        .assert()
+        .success();
+    let stdout = String::from_utf8(assert.get_output().stdout.clone())
+        .expect("`icp project show` output is not UTF-8");
+
+    let recipe_at = stdout
+        .find("echo from-recipe")
+        .unwrap_or_else(|| panic!("recipe's sync step missing from:\n{stdout}"));
+    let manifest_at = stdout
+        .find("echo from-manifest")
+        .unwrap_or_else(|| panic!("manifest's sync step missing from:\n{stdout}"));
+    assert!(
+        recipe_at < manifest_at,
+        "the manifest's sync step should follow the recipe's, got:\n{stdout}"
+    );
+}

crates/icp/src/manifest/canister.rs

@@ -170,29 +170,23 @@ impl<'de> Deserialize<'de> for CanisterManifest {
                 //
                 let has_recipe = temp_map.contains_key(&recipe_key);
                 let has_build = temp_map.contains_key(&build_key);
-                let has_sync = temp_map.contains_key(&sync_key);
 
-                match (has_recipe, has_build, has_sync) {
-                    (true, true, _) => {
+                match (has_recipe, has_build) {
+                    (true, true) => {
                         // Can't have a recipe and a build
                         Err(Error::custom(format!(
                             "Canister {name} cannot have both a `recipe` and a `build` section"
                         )))
                     }
-                    (true, false, true) => {
-                        // Can't have a recipe and a sync sections
-                        Err(Error::custom(format!(
-                            "Canister {name} cannot have both a `recipe` and a `sync` section"
-                        )))
-                    }
-                    (false, false, _) => {
+                    (false, false) => {
                         // We must have recipe or build
                         Err(Error::custom(format!(
                             "Canister {name} must have a `recipe` or a `build` section"
                         )))
                     }
-                    (true, false, false) => {
-                        // We have a a recipe
+                    (true, false) => {
+                        // We have a recipe, optionally with sync steps of its
+                        // own to run after the recipe's
                         let recipe: Recipe = serde_yaml::from_value(
                             temp_map
                                 .remove(&recipe_key)
@@ -203,6 +197,19 @@ impl<'de> Deserialize<'de> for CanisterManifest {
                             Error::custom(format!("Canister {name} failed to parse recipe: {}", e))
                         })?;
 
+                        // An explicit `sync: null` deserializes to `None`, as
+                        // it does in the build/sync variant
+                        let sync: Option<SyncSteps> =
+                            if let Some(sync_value) = temp_map.remove(&sync_key) {
+                                serde_yaml::from_value(sync_value).map_err(|e| {
+                                    Error::custom(format!(
+                                        "Canister {name} failed to parse sync instructions: {e}"
+                                    ))
+                                })?
+                            } else {
+                                None
+                            };
+
                         if !temp_map.is_empty() {
                             return Err(Error::custom(format!(
                                 "Unrecognized fields in canister `{name}`."
@@ -214,10 +221,10 @@ impl<'de> Deserialize<'de> for CanisterManifest {
                             settings,
                             init_args,
                             upgrade_args,
-                            instructions: Instructions::Recipe { recipe },
+                            instructions: Instructions::Recipe { recipe, sync },
                         })
                     }
-                    (false, true, _) => {
+                    (false, true) => {
                         // We have a build section
 
                         // Try to deserialize as BuildSync variant
@@ -262,6 +269,10 @@ impl<'de> Deserialize<'de> for CanisterManifest {
 pub enum Instructions {
     Recipe {
         recipe: Recipe,
+
+        /// Additional sync steps, run after the ones the recipe renders.
+        #[serde(skip_serializing_if = "Option::is_none")]
+        sync: Option<SyncSteps>,
     },
 
     BuildSync {
@@ -629,7 +640,8 @@ mod tests {
                         recipe_type: RecipeType::File("my-recipe".to_string()),
                         configuration: HashMap::new(),
                         sha256: None,
-                    }
+                    },
+                    sync: None,
                 },
             },
         );
@@ -659,7 +671,8 @@ mod tests {
                             ("key-2".to_string(), "value-2".into())
                         ]),
                         sha256: None,
-                    }
+                    },
+                    sync: None,
                 },
             },
         );
@@ -691,7 +704,8 @@ mod tests {
                             "9f86d081884c7d659a2feaa0c55ad015a3bf4f1b2b0b822cd15d6c15b0f00a08"
                                 .to_string()
                         ),
-                    }
+                    },
+                    sync: None,
                 },
             },
         );
@@ -722,12 +736,95 @@ mod tests {
                         recipe_type: RecipeType::File("my-recipe".to_string()),
                         configuration: HashMap::new(),
                         sha256: None,
-                    }
+                    },
+                    sync: None,
+                },
+            },
+        );
+    }
+
+    #[test]
+    fn recipe_with_sync() {
+        assert_eq!(
+            validate_canister_yaml(indoc! {r#"
+                    name: my-canister
+                    recipe:
+                      type: file://my-recipe
+                    sync:
+                      steps:
+                        - type: script
+                          command: echo hi
+                "#}),
+            CanisterManifest {
+                name: "my-canister".to_string(),
+                settings: ManifestSettings::default(),
+                init_args: None,
+                upgrade_args: None,
+                instructions: Instructions::Recipe {
+                    recipe: Recipe {
+                        recipe_type: RecipeType::File("my-recipe".to_string()),
+                        configuration: HashMap::new(),
+                        sha256: None,
+                    },
+                    sync: Some(SyncSteps {
+                        steps: vec![SyncStep::Script(script::Adapter {
+                            command: script::CommandField::Command("echo hi".to_string()),
+                        })]
+                    }),
+                },
+            },
+        );
+    }
+
+    #[test]
+    fn recipe_with_null_sync() {
+        assert_eq!(
+            validate_canister_yaml(indoc! {r#"
+                    name: my-canister
+                    recipe:
+                      type: file://my-recipe
+                    sync:
+                "#}),
+            CanisterManifest {
+                name: "my-canister".to_string(),
+                settings: ManifestSettings::default(),
+                init_args: None,
+                upgrade_args: None,
+                instructions: Instructions::Recipe {
+                    recipe: Recipe {
+                        recipe_type: RecipeType::File("my-recipe".to_string()),
+                        configuration: HashMap::new(),
+                        sha256: None,
+                    },
+                    sync: None,
                 },
             },
         );
     }
 
+    #[test]
+    fn recipe_with_invalid_sync() {
+        match serde_yaml::from_str::<CanisterManifest>(indoc! {r#"
+                name: my-canister
+                recipe:
+                  type: file://my-recipe
+                sync:
+                  steps:
+                    - type: nonsense
+        "#})
+        {
+            Ok(_) => panic!("an unknown sync step type should not deserialize"),
+            Err(err) => {
+                let err_msg = format!("{err}");
+                if !err_msg.contains("Canister my-canister failed to parse sync instructions") {
+                    panic!(
+                        "expected 'Canister my-canister failed to parse sync instructions' error but got: {err}"
+                    );
+                }
+            }
+        };
+    }
+
     #[test]
     fn build_steps() {
         assert_eq!(