fix: prune env-omitted canisters from bundles

CHANGELOG.md

@@ -24,6 +24,7 @@ air-gapped signing
 
 ## Experimental
 
+* feat(bundle): `icp project bundle -e <ENV>` now decides what the bundle carries, not just what its build steps are told. The archive holds the canisters that environment contains and the bundled manifests declare only those, so a bundle built for one environment no longer ships every canister in the project. References the pruning would leave dangling go with them: another environment's `canisters:` list, a dependency's `canisters:` exposure list, an environment's `settings:`/`init_args:` override of a canister that is gone, and a controller naming a canister the environment does not hold — in a canister's own settings or in an environment's override of them — which is reported as a warning. The environment is now resolved rather than passed through as a name, so naming one the project does not declare — or one a workspace member does not declare — is an error, as it already was for `icp build` and `icp deploy`.
 * fix(dependencies)!: an environment's `canisters:` list is now honored in a workspace — a vendored project's own list used to be dropped, so all of its canisters deployed. Membership is decided locally and only locally: each project's `canisters:` list names that project's **own** canisters, so every canister's membership is decided by the manifest that declares it, and a vendored project holds the same canisters in an environment as it would deployed on its own. This is breaking for a root that named a dependency's canister — `canisters: [app, "vendor/openemail:frontend"]` is now rejected when the project is loaded, and keeping a dependency's canister out of an environment means editing that dependency. `canisters: []` likewise empties only the project that writes it, and a listed environment's canisters now come in declaration order. See [Which canisters an environment holds](docs/concepts/project-dependencies.md#which-canisters-an-environment-holds).
 * feat(signing): a canister call can now be signed on one machine and submitted from another, restoring what `dfx canister sign` / `dfx canister send` covered. `icp canister call --sign-only <FILE>` composes and signs a call and writes it to a JSON file instead of submitting it; `icp message send <FILE>` submits that file and prints the reply. So a machine that holds the key needs no network, and the machine with the network needs no key — it never resolves an identity at all. `-` writes to stdout and reads from stdin respectively.
   * Nothing is fetched while signing: the Candid interface comes from `--candid` or from the canister's local build artifact rather than from the canister itself, and `--root-key` must name a key (`mainnet` or a hex-encoded key) rather than `fetch`. `--proxy` is not supported.

crates/icp-cli/src/commands/project/bundle.rs

@@ -1,42 +1,56 @@
+use std::collections::HashSet;
+
 use anyhow::Context as _;
 use clap::{Args, ValueHint};
-use icp::context::Context;
+use icp::context::{Context, EnvironmentSelection};
 use icp::prelude::*;
+use tracing::warn;
 
 use icp::operations::bundle::create_bundle;
 
 use crate::render::rendered;
 
 /// Bundle a project into a self-contained deployable archive.
 ///
-/// Builds all project canisters and packages them with a rewritten manifest
-/// into a `.tar.gz` file. The rewritten manifest replaces all build steps
-/// with pre-built steps referencing the bundled WASM files. Asset sync
-/// directories are included in the archive.
+/// Builds the canisters the selected environment contains and packages them
+/// with a rewritten manifest into a `.tar.gz` file. The rewritten manifest
+/// replaces all build steps with pre-built steps referencing the bundled WASM
+/// files. Asset sync directories are included in the archive.
 ///
-/// Projects with script sync steps cannot be bundled.
+/// A canister with a script sync step cannot be bundled.
 #[derive(Args, Debug)]
 pub(crate) struct BundleArgs {
     /// Output path for the bundle archive (e.g. bundle.tar.gz)
     #[arg(long, short, value_hint = ValueHint::AnyPath)]
     pub(crate) output: PathBuf,
 
-    /// Environment the canisters are built for. Bundles are made to be deployed
-    /// elsewhere, so this defaults to `ic` rather than the usual `local`.
+    /// Environment the canisters are built for, and whose canisters the bundle
+    /// carries. Bundles are made to be deployed elsewhere, so this defaults to
+    /// `ic` rather than the usual `local`.
     #[arg(long, short = 'e', env = "ICP_ENVIRONMENT", default_value = IC)]
     pub(crate) environment: String,
 }
 
 pub(crate) async fn exec(ctx: &Context, args: &BundleArgs) -> Result<(), anyhow::Error> {
     let project = ctx.project.load().await.context("failed to load project")?;
+    let environment_selection = EnvironmentSelection::Named(args.environment.clone());
+    let env = ctx.get_environment(&environment_selection).await?;
 
     let canisters: Vec<_> = project.canisters.into_values().collect();
+    let selected: HashSet<String> = env.canisters.keys().cloned().collect();
+    if selected.is_empty() {
+        warn!(
+            "Environment '{}' contains no canisters; the bundle will carry none",
+            args.environment
+        );
+    }
 
     let pkg_cache = ctx.dirs.package_cache()?;
     rendered(ctx.debug, async |reporter| {
         create_bundle(
             &project.dir,
             canisters,
+            &selected,
             &args.environment,
             ctx.builder.clone(),
             ctx.artifacts.clone(),

crates/icp-cli/tests/bundle_tests.rs

@@ -917,6 +917,9 @@ fn bundle_builds_for_ic_by_default() {
                   commands:
                     - echo "$ICP_CLI_ENVIRONMENT" > '{recorded}'
                     - cp '{wasm_src}' "$ICP_WASM_OUTPUT_PATH"
+
+        environments:
+          - name: staging
     "#};
 
     write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest");
@@ -1608,6 +1611,214 @@ fn bundle_preserves_dependency_structure() {
     }
 }
 
+/// `-e` decides what the bundle carries, not merely what the build steps are
+/// told: the archive holds the canisters that environment contains, and the
+/// bundled manifests declare only those. Every reference the pruning would leave
+/// dangling — another environment's canister list, a dependency's exposure list,
+/// a controller — goes with them, so the extracted bundle still loads.
+#[test]
+fn bundle_carries_only_the_environments_canisters() {
+    let ctx = TestContext::new();
+    let project_dir = ctx.create_project_dir("icp");
+    let wasm_src = ctx.make_asset("example_icp_mo.wasm");
+
+    let build_step = formatdoc! {r#"
+        build:
+              steps:
+                - type: script
+                  command: cp '{wasm_src}' "$ICP_WASM_OUTPUT_PATH"
+    "#};
+
+    let dep_dir = project_dir.join("vendor/openemail");
+    create_dir_all(&dep_dir).expect("failed to create dependency dir");
+    write_string(
+        &dep_dir.join("icp.yaml"),
+        &formatdoc! {r#"
+            canisters:
+              - name: registry
+                {build_step}
+              - name: archive
+                {build_step}
+
+            environments:
+              - name: staging
+                canisters: [registry]
+              - name: prod
+                canisters: [archive]
+        "#},
+    )
+    .expect("failed to write dependency manifest");
+
+    // Each project names its own: staging is the root's `frontend` and
+    // openemail's `registry`, prod the root's `backend` and openemail's
+    // `archive`. `frontend` names `backend` as a controller, which staging does
+    // not contain — in its base settings and again in staging's own override of
+    // them.
+    write_string(
+        &project_dir.join("icp.yaml"),
+        &formatdoc! {r#"
+            canisters:
+              - name: frontend
+                settings:
+                  controllers: [backend]
+                {build_step}
+              - name: backend
+                {build_step}
+
+            dependencies:
+              - name: openemail
+                path: ./vendor/openemail
+                canisters: [registry, archive]
+
+            environments:
+              - name: staging
+                canisters: [frontend]
+                settings:
+                  frontend:
+                    controllers: [backend, "vendor/openemail:registry"]
+              - name: prod
+                canisters: [backend]
+                settings:
+                  backend:
+                    compute_allocation: 1
+        "#},
+    )
+    .expect("failed to write project manifest");
+
+    let bundle_path = project_dir.join("bundle.tar.gz");
+    ctx.icp()
+        .current_dir(&project_dir)
+        .args([
+            "project",
+            "bundle",
+            "--environment",
+            "staging",
+            "--output",
+            bundle_path.as_str(),
+        ])
+        .assert()
+        .success()
+        .stderr(contains("names 'backend' as a controller"));
+
+    let bundle_bytes = fs::read(bundle_path.as_std_path()).expect("failed to read bundle");
+    let gz = GzDecoder::new(BufReader::new(bundle_bytes.as_slice()));
+    let mut archive = Archive::new(gz);
+
+    let mut entries: Vec<String> = Vec::new();
+    let mut manifests: std::collections::HashMap<String, String> = std::collections::HashMap::new();
+    for entry in archive.entries().expect("failed to read archive entries") {
+        let mut entry = entry.expect("failed to read archive entry");
+        let path = entry
+            .path()
+            .expect("failed to get entry path")
+            .to_string_lossy()
+            .into_owned();
+        if path.ends_with("icp.yaml") {
+            let mut yaml = String::new();
+            entry
+                .read_to_string(&mut yaml)
+                .expect("failed to read manifest");
+            manifests.insert(path.clone(), yaml);
+        }
+        entries.push(path);
+    }
+
+    assert_eq!(
+        entries,
+        [
+            "icp.yaml",
+            "vendor/openemail/icp.yaml",
+            "canisters/frontend.wasm",
+            "vendor/openemail/canisters/registry.wasm",
+        ],
+        "bundle should carry only the staging canisters"
+    );
+
+    let root: serde_yaml::Value =
+        serde_yaml::from_str(&manifests["icp.yaml"]).expect("root manifest yaml is invalid");
+    assert_eq!(
+        root["canisters"][0]["name"],
+        serde_yaml::Value::from("frontend")
+    );
+    assert!(
+        root["canisters"][1].is_null(),
+        "root manifest should declare frontend alone: {:?}",
+        root["canisters"]
+    );
+    // The controller reference outlived the canister it named, so the bundle
+    // drops it rather than carry a name nothing declares.
+    assert_eq!(
+        root["canisters"][0]["settings"]["controllers"],
+        serde_yaml::Value::Sequence(vec![]),
+    );
+    assert_eq!(
+        root["dependencies"][0]["canisters"],
+        serde_yaml::Value::Sequence(vec!["registry".into()]),
+    );
+    assert_eq!(
+        root["environments"][0]["canisters"],
+        serde_yaml::Value::Sequence(vec!["frontend".into()]),
+    );
+    assert_eq!(
+        root["environments"][1]["canisters"],
+        serde_yaml::Value::Sequence(vec![]),
+        "prod named only canisters the bundle left out",
+    );
+    // An override the bundle keeps still has to lose the controllers it names
+    // that the bundle does not carry.
+    assert_eq!(
+        root["environments"][0]["settings"]["frontend"]["controllers"],
+        serde_yaml::Value::Sequence(vec!["vendor/openemail:registry".into()]),
+    );
+    // An override *of* a left-out canister goes entirely.
+    assert!(
+        root["environments"][1]["settings"]["backend"].is_null(),
+        "prod's override of a left-out canister should be dropped: {:?}",
+        root["environments"][1]["settings"]
+    );
+
+    let dep: serde_yaml::Value = serde_yaml::from_str(&manifests["vendor/openemail/icp.yaml"])
+        .expect("dependency manifest yaml is invalid");
+    assert_eq!(
+        dep["canisters"][0]["name"],
+        serde_yaml::Value::from("registry")
+    );
+    assert!(
+        dep["canisters"][1].is_null(),
+        "dependency manifest should declare registry alone: {:?}",
+        dep["canisters"]
+    );
+    assert_eq!(
+        dep["environments"][0]["canisters"],
+        serde_yaml::Value::Sequence(vec!["registry".into()]),
+    );
+    assert_eq!(
+        dep["environments"][1]["canisters"],
+        serde_yaml::Value::Sequence(vec![]),
+        "openemail's prod named only canisters the bundle left out",
+    );
+
+    // Nothing dangles: the extracted workspace consolidates, and its remaining
+    // canisters keep the store keys the source workspace gave them.
+    let bundle_dir = project_dir.join("bundle-extracted");
+    create_dir_all(&bundle_dir).expect("failed to create bundle-extracted dir");
+    let gz = GzDecoder::new(BufReader::new(bundle_bytes.as_slice()));
+    Archive::new(gz)
+        .unpack(bundle_dir.as_std_path())
+        .expect("failed to extract bundle");
+
+    ctx.icp()
+        .current_dir(&bundle_dir)
+        .args(["project", "show"])
+        .assert()
+        .success()
+        .stdout(
+            contains("vendor/openemail:registry")
+                .and(contains("backend").not())
+                .and(contains("vendor/openemail:archive").not()),
+        );
+}
+
 /// A dependency path that does not describe where the instance sits relative to
 /// the workspace root — an absolute path, or one traversing a symlink — must be
 /// rewritten to the instance's location in the archive. Reusing the declared path

crates/icp/src/manifest/mod.rs

@@ -25,7 +25,7 @@ pub use {
         SyncSteps,
     },
     dependency::DependencyManifest,
-    environment::EnvironmentManifest,
+    environment::{CanisterSelection, EnvironmentManifest},
     network::{ManagedMode, Mode, NetworkManifest},
     project::ProjectManifest,
 };