From b54ed7be53aec6edb910d4593fdf8a7ae1897a58 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Wed, 26 Aug 2026 13:07:17 -0700 Subject: [PATCH 1/2] prune omitted canisters from bundles --- CHANGELOG.md | 1 + crates/icp-cli/src/commands/project/bundle.rs | 30 ++- crates/icp-cli/src/operations/bundle.rs | 175 ++++++++++++++-- crates/icp-cli/tests/bundle_tests.rs | 192 ++++++++++++++++++ crates/icp/src/manifest/mod.rs | 2 +- docs/concepts/project-dependencies.md | 2 + 6 files changed, 379 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e886a6237..78c532bbe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ air-gapped signing ## Experimental +* feat(bundle): `icp project bundle -e ` 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, and a controller naming a canister the environment does not hold (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 ` composes and signs a call and writes it to a JSON file instead of submitting it; `icp message send ` 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. diff --git a/crates/icp-cli/src/commands/project/bundle.rs b/crates/icp-cli/src/commands/project/bundle.rs index b9217c0b8..4f2f6c225 100644 --- a/crates/icp-cli/src/commands/project/bundle.rs +++ b/crates/icp-cli/src/commands/project/bundle.rs @@ -1,38 +1,52 @@ +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 crate::operations::bundle::create_bundle; /// 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 = env.canisters.keys().cloned().collect(); + if selected.is_empty() { + warn!( + "Environment '{}' contains no canisters; the bundle will carry none", + args.environment + ); + } create_bundle( &project.dir, canisters, + &selected, &args.environment, ctx.builder.clone(), ctx.artifacts.clone(), diff --git a/crates/icp-cli/src/operations/bundle.rs b/crates/icp-cli/src/operations/bundle.rs index 6786fc7d5..7713abb33 100644 --- a/crates/icp-cli/src/operations/bundle.rs +++ b/crates/icp-cli/src/operations/bundle.rs @@ -14,7 +14,7 @@ use icp::{ canister::{ControllerRef, ManifestEnvVar, Settings, build::Build, wasm}, fs, manifest::{ - ArgsFormat, BuildStep, BuildSteps, CanisterManifest, DependencyManifest, + ArgsFormat, BuildStep, BuildSteps, CanisterManifest, CanisterSelection, DependencyManifest, EnvironmentManifest, Instructions, Item, LoadManifestFromPathError, ManagedMode, ManifestInitArgs, Mode, NetworkManifest, PROJECT_MANIFEST, ProjectManifest, SyncStep, SyncSteps, load_manifest_from_path, plugin, prebuilt, @@ -27,6 +27,7 @@ use icp::{ }; use snafu::{OptionExt, ResultExt, Snafu}; use tar::Builder; +use tracing::warn; use crate::operations::build::{BuildManyError, build_many_with_progress_bar}; @@ -321,9 +322,51 @@ struct Instance { canisters: Vec<(PathBuf, Canister)>, } +/// The canisters the selected environment leaves out of the bundle, and the +/// lookup needed to recognize a manifest's reference to one of them. +/// +/// A reference that survives into a bundled manifest names a canister that +/// manifest no longer declares, which the extracted bundle rejects at load. +struct Pruned<'a> { + /// Store keys of the canisters the environment does not hold. + dropped: &'a HashSet, + + /// Each workspace instance's store-key prefix, by its canonical directory. + prefixes_by_dir: &'a HashMap, + + /// The environment the bundle is built for, for diagnostics. + environment: &'a str, +} + +impl Pruned<'_> { + /// Whether a canister name written in one instance's manifest denotes a + /// canister the environment leaves out. + /// + /// A name that resolves to no instance in the workspace is left alone: it is + /// invalid, and reporting it is the manifest loader's job, not the bundler's. + fn drops(&self, instance: &Instance, name: &str) -> bool { + self.store_key(instance, name) + .is_some_and(|key| self.dropped.contains(&key)) + } + + /// The workspace store key a name written in one instance's manifest refers + /// to: either a bare local name, or `:` naming a + /// canister of a project that instance reaches through its dependencies. The + /// path is the one the store key's own prefix is built from, so resolving it + /// against the instance's directory gives that prefix back. + fn store_key(&self, instance: &Instance, name: &str) -> Option { + let Some((rel, local)) = name.rsplit_once(':') else { + return Some(override_store_key(&instance.prefix, name)); + }; + let dir = instance.dir.join(rel).canonicalize_utf8().ok()?; + Some(override_store_key(self.prefixes_by_dir.get(&dir)?, local)) + } +} + pub(crate) async fn create_bundle( project_dir: &Path, canisters: Vec<(PathBuf, Canister)>, + selected: &HashSet, environment: &str, builder: Arc, artifacts: Arc, @@ -331,6 +374,18 @@ pub(crate) async fn create_bundle( debug: bool, output: &Path, ) -> Result<(), BundleError> { + // The bundle carries the canisters the selected environment holds and no + // others: it is built for that environment, and a manifest that declared a + // canister the archive has no wasm for could not be deployed from the + // extraction. + let (canisters, left_out): (Vec<_>, Vec<_>) = canisters + .into_iter() + .partition(|(_, canister)| selected.contains(&canister.name)); + let dropped: HashSet = left_out + .into_iter() + .map(|(_, canister)| canister.name) + .collect(); + // A bundle mirrors the workspace: the root project at the archive root and // each dependency instance at its workspace-relative directory, so the // dependency declarations — and the store keys and `PUBLIC_CANISTER_ID` @@ -341,6 +396,15 @@ pub(crate) async fn create_bundle( project_dir, )?; validate_canisters(&instances)?; + let mut prefixes_by_dir: HashMap = HashMap::with_capacity(instances.len()); + for instance in &instances { + prefixes_by_dir.insert(canonicalize(&instance.dir)?, instance.prefix.clone()); + } + let pruned = Pruned { + dropped: &dropped, + prefixes_by_dir: &prefixes_by_dir, + environment, + }; let canonical_project_dir = canonicalize(project_dir)?; let canonical_sync_dirs = validate_source_paths(project_dir, &canisters, &canonical_project_dir)?; @@ -384,13 +448,18 @@ pub(crate) async fn create_bundle( let mut manifests: Vec = Vec::with_capacity(instances.len()); for instance in &instances { - let canister_items = - prepare_canisters(instance, &*artifacts, pkg_cache, &mut bundle_artifacts).await?; + let canister_items = prepare_canisters( + instance, + &pruned, + &*artifacts, + pkg_cache, + &mut bundle_artifacts, + ) + .await?; let networks = inline_networks(&instance.manifest.networks, &instance.dir).await?; let environments = inline_environments( - &instance.manifest.environments, - &instance.prefix, - &instance.dir, + instance, + &pruned, &canonical_project_dir, &canister_dirs, &owner_prefixes, @@ -401,7 +470,7 @@ pub(crate) async fn create_bundle( let manifest = ProjectManifest { canisters: canister_items, - dependencies: rewrite_dependencies(instance)?, + dependencies: rewrite_dependencies(instance, &pruned)?, networks, environments, }; @@ -487,7 +556,10 @@ fn relative_archive_path(from: &str, to: &str) -> String { /// instance sits relative to the workspace root — and therefore not where it sits /// in the archive either. For a plainly vendored layout the rewritten path is the /// same path, modulo a leading `./`. -fn rewrite_dependencies(instance: &Instance) -> Result, BundleError> { +fn rewrite_dependencies( + instance: &Instance, + pruned: &Pruned<'_>, +) -> Result, BundleError> { let declared = &instance.manifest.dependencies; let targets = &instance.dependency_prefixes; // `workspace_instances` resolves one prefix per declaration, in order. @@ -506,11 +578,34 @@ fn rewrite_dependencies(instance: &Instance) -> Result, .map(|(dep, target_prefix)| DependencyManifest { name: dep.name.clone(), path: relative_archive_path(&instance.prefix, target_prefix), - canisters: dep.canisters.clone(), + // The exposure list names the dependency's own canisters, so a + // left-out one is no longer there to expose. + canisters: prune_selection(dep.canisters.clone(), |name| { + pruned + .dropped + .contains(&override_store_key(target_prefix, name)) + }), }) .collect()) } +/// Drop from a canister selection every name the environment leaves out. A list +/// emptied by the pruning becomes `CanisterSelection::None`, which is what an +/// empty list means once written to a manifest and read back. +fn prune_selection( + selection: CanisterSelection, + drops: impl Fn(&str) -> bool, +) -> CanisterSelection { + let CanisterSelection::Named(mut names) = selection else { + return selection; + }; + names.retain(|name| !drops(name)); + match names.is_empty() { + true => CanisterSelection::None, + false => CanisterSelection::Named(names), + } +} + /// Whether an instance's archive directory stays inside the workspace root. /// /// The prefix is the instance's canonical directory relative to the canonical @@ -574,6 +669,7 @@ fn group_canisters( /// Build one instance's manifest items and collect the archive artifacts they reference. async fn prepare_canisters( instance: &Instance, + pruned: &Pruned<'_>, artifacts: &dyn store_artifact::Access, pkg_cache: &PackageCache, out: &mut BundleArtifacts, @@ -593,6 +689,7 @@ async fn prepare_canisters( canister_path, canister, &local_names, + pruned, artifacts, pkg_cache, out, @@ -609,6 +706,7 @@ async fn prepare_canister( canister_path: &Path, canister: &Canister, local_names: &HashMap<&str, &str>, + pruned: &Pruned<'_>, artifacts: &dyn store_artifact::Access, pkg_cache: &PackageCache, out: &mut BundleArtifacts, @@ -670,7 +768,12 @@ async fn prepare_canister( Ok(Item::Manifest(CanisterManifest { name: local.to_owned(), - settings: localize_controllers(canister.settings.clone().into(), local_names), + settings: localize_controllers( + canister.settings.clone().into(), + &canister.name, + local_names, + pruned, + ), init_args: canister.init_args.as_ref().map(convert_init_args), instructions: Instructions::BuildSync { build: BuildSteps { @@ -687,7 +790,8 @@ async fn prepare_canister( } /// Rewrite controller references from workspace store keys back to the local -/// names of the instance being written. +/// names of the instance being written, dropping the ones the selected +/// environment leaves out of the bundle. /// /// Consolidation translates a dependency's references to its own siblings into /// store keys, which contain `:` and so are not valid canister names. References @@ -696,9 +800,24 @@ async fn prepare_canister( /// same way from the bundle. fn localize_controllers( mut settings: Settings, + canister: &str, local_names: &HashMap<&str, &str>, + pruned: &Pruned<'_>, ) -> Settings { if let Some(controllers) = &mut settings.controllers { + // A reference consolidation has already resolved is spelled as the store + // key it resolved to, so the left-out keys are what to match against. + controllers.retain(|cref| match cref { + ControllerRef::CanisterName(name) if pruned.dropped.contains(name.as_str()) => { + warn!( + "Canister '{canister}' names '{name}' as a controller, which environment \ + '{}' does not contain; the bundle drops the reference.", + pruned.environment, + ); + false + } + _ => true, + }); for cref in controllers.iter_mut() { if let ControllerRef::CanisterName(name) = cref && let Some(local) = local_names.get(name.as_str()) @@ -844,15 +963,17 @@ fn override_base_dir<'a>( #[allow(clippy::too_many_arguments)] async fn inline_environments( - items: &[Item], - instance_prefix: &str, - instance_dir: &Path, + instance: &Instance, + pruned: &Pruned<'_>, canonical_project_dir: &Path, canister_dirs: &HashMap<&str, &Path>, owner_prefixes: &HashMap<&str, &str>, seen_archive_paths: &mut HashSet, init_args_files: &mut Vec, ) -> Result>, BundleError> { + let items = &instance.manifest.environments; + let instance_prefix = instance.prefix.as_str(); + let instance_dir = instance.dir.as_path(); let mut out = Vec::with_capacity(items.len()); for item in items { @@ -867,6 +988,13 @@ async fn inline_environments( } }; + // Before the overrides below are followed to the files they name: an + // override for a left-out canister resolves its paths against that + // canister's directory, which the bundle no longer knows. + if let Item::Manifest(ref mut env) = inlined { + prune_environment(env, instance, pruned); + } + if let Item::Manifest(ref mut env) = inlined && let Some(ref mut overrides) = env.init_args { @@ -957,6 +1085,25 @@ async fn inline_environments( Ok(out) } +/// Drop from one environment every reference to a canister the selected +/// environment leaves out of the bundle: the canisters it lists, and the +/// per-canister settings and init_args it overrides. +/// +/// The environment being pruned is not necessarily the one the bundle was built +/// for — a bundle keeps every environment its manifests declare, and each of +/// them can only ever hold canisters the bundle carries. +fn prune_environment(env: &mut EnvironmentManifest, instance: &Instance, pruned: &Pruned<'_>) { + env.canisters = prune_selection(std::mem::take(&mut env.canisters), |name| { + pruned.drops(instance, name) + }); + if let Some(settings) = &mut env.settings { + settings.retain(|name, _| !pruned.drops(instance, name)); + } + if let Some(init_args) = &mut env.init_args { + init_args.retain(|name, _| !pruned.drops(instance, name)); + } +} + /// Load `icp_appmanifest.yaml` if present, rewriting its top-level `images` paths to point at /// copies relocated under `images/` in the bundle. Returns `None` when the file is absent. fn prepare_app_manifest( diff --git a/crates/icp-cli/tests/bundle_tests.rs b/crates/icp-cli/tests/bundle_tests.rs index 74748e4e4..c998f76b2 100644 --- a/crates/icp-cli/tests/bundle_tests.rs +++ b/crates/icp-cli/tests/bundle_tests.rs @@ -830,6 +830,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"); @@ -1521,6 +1524,195 @@ 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. + 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] + - name: prod + canisters: [backend] + "#}, + ) + .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 = Vec::new(); + let mut manifests: std::collections::HashMap = 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", + ); + + 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 diff --git a/crates/icp/src/manifest/mod.rs b/crates/icp/src/manifest/mod.rs index 0a811e358..165b7c2ac 100644 --- a/crates/icp/src/manifest/mod.rs +++ b/crates/icp/src/manifest/mod.rs @@ -25,7 +25,7 @@ pub use { SyncStep, SyncSteps, }, dependency::DependencyManifest, - environment::EnvironmentManifest, + environment::{CanisterSelection, EnvironmentManifest}, network::{ManagedMode, Mode, NetworkManifest}, project::ProjectManifest, }; diff --git a/docs/concepts/project-dependencies.md b/docs/concepts/project-dependencies.md index 2c20416d7..52904a6a9 100644 --- a/docs/concepts/project-dependencies.md +++ b/docs/concepts/project-dependencies.md @@ -158,6 +158,8 @@ A vendored project must remain a complete `icp` project: it never references its `icp project bundle` packages a workspace by mirroring it: the root project's `icp.yaml` sits at the archive root, each dependency instance gets its own `icp.yaml` at the directory it occupies in the workspace, and the `dependencies:` declarations are preserved, each pointing at the directory its dependency occupies in the archive. For a plainly vendored layout that is the path the manifest already used; a path that does not describe the dependency's location relative to the workspace root — an absolute path, or one that traverses a symlink — is rewritten so the extracted bundle stays self-contained. Canister names stay as each project wrote them, a shared dependency remains a single instance, and canister discovery works from the extracted bundle exactly as it did in the source workspace. Extracting the archive gives you the same workspace with every build step replaced by its built wasm. +A bundle is built for one environment — `-e/--environment`, defaulting to `ic` — and it carries [the canisters that environment holds](#which-canisters-an-environment-holds) and no others. The bundled manifests declare only those canisters, and references the rest leave behind are dropped along with them: another environment's `canisters:` list loses the names it can no longer reach, a `dependencies:` entry's `canisters:` exposure list loses the ones the dependency no longer bundles, and a controller naming a canister the environment does not hold is dropped with a warning. So every environment in an extracted bundle holds at most what the bundle carries, and deploying the bundle to some other environment of its own deploys those same canisters. + Every dependency must resolve to a directory **inside** the workspace root, so that the archive can contain it. A dependency that resolves outside the root — `../elsewhere` declared by the root project, or a directory that is a symlink pointing out of the workspace — is rejected. For the same reason a vendored member that depends on a sibling cannot be bundled on its own (e.g. with `ICP_PROJECT_ROOT` pointing at the member): the sibling would fall outside the bundle. Bundle the workspace root instead. ## Limitations From 81df2832477146a7d0bd27be963cbb11b045d57a Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Wed, 26 Aug 2026 15:23:14 -0700 Subject: [PATCH 2/2] copilot --- CHANGELOG.md | 2 +- crates/icp-cli/src/operations/bundle.rs | 24 ++++++++++++++++++++++-- crates/icp-cli/tests/bundle_tests.rs | 21 ++++++++++++++++++++- docs/concepts/project-dependencies.md | 2 +- 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78c532bbe..828a727de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,7 +14,7 @@ air-gapped signing ## Experimental -* feat(bundle): `icp project bundle -e ` 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, and a controller naming a canister the environment does not hold (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`. +* feat(bundle): `icp project bundle -e ` 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 ` composes and signs a call and writes it to a JSON file instead of submitting it; `icp message send ` 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. diff --git a/crates/icp-cli/src/operations/bundle.rs b/crates/icp-cli/src/operations/bundle.rs index 7713abb33..969d27009 100644 --- a/crates/icp-cli/src/operations/bundle.rs +++ b/crates/icp-cli/src/operations/bundle.rs @@ -1086,8 +1086,9 @@ async fn inline_environments( } /// Drop from one environment every reference to a canister the selected -/// environment leaves out of the bundle: the canisters it lists, and the -/// per-canister settings and init_args it overrides. +/// environment leaves out of the bundle: the canisters it lists, the +/// per-canister settings and init_args it overrides, and the controllers those +/// settings name. /// /// The environment being pruned is not necessarily the one the bundle was built /// for — a bundle keeps every environment its manifests declare, and each of @@ -1098,6 +1099,25 @@ fn prune_environment(env: &mut EnvironmentManifest, instance: &Instance, pruned: }); if let Some(settings) = &mut env.settings { settings.retain(|name, _| !pruned.drops(instance, name)); + // An override's own controller list survives the pruning above, which + // only reaches the canister an override configures: a kept canister can + // still be handed a controller the bundle does not carry. + for (canister, overrides) in settings.iter_mut() { + let Some(controllers) = &mut overrides.controllers else { + continue; + }; + controllers.retain(|cref| match cref { + ControllerRef::CanisterName(name) if pruned.drops(instance, name) => { + warn!( + "Environment '{}' names '{name}' as a controller of '{canister}', which \ + environment '{}' does not contain; the bundle drops the reference.", + env.name, pruned.environment, + ); + false + } + _ => true, + }); + } } if let Some(init_args) = &mut env.init_args { init_args.retain(|name, _| !pruned.drops(instance, name)); diff --git a/crates/icp-cli/tests/bundle_tests.rs b/crates/icp-cli/tests/bundle_tests.rs index c998f76b2..9f05a546d 100644 --- a/crates/icp-cli/tests/bundle_tests.rs +++ b/crates/icp-cli/tests/bundle_tests.rs @@ -1565,7 +1565,8 @@ fn bundle_carries_only_the_environments_canisters() { // 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. + // not contain — in its base settings and again in staging's own override of + // them. write_string( &project_dir.join("icp.yaml"), &formatdoc! {r#" @@ -1585,8 +1586,14 @@ fn bundle_carries_only_the_environments_canisters() { 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"); @@ -1670,6 +1677,18 @@ fn bundle_carries_only_the_environments_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"); diff --git a/docs/concepts/project-dependencies.md b/docs/concepts/project-dependencies.md index 52904a6a9..80036ec63 100644 --- a/docs/concepts/project-dependencies.md +++ b/docs/concepts/project-dependencies.md @@ -158,7 +158,7 @@ A vendored project must remain a complete `icp` project: it never references its `icp project bundle` packages a workspace by mirroring it: the root project's `icp.yaml` sits at the archive root, each dependency instance gets its own `icp.yaml` at the directory it occupies in the workspace, and the `dependencies:` declarations are preserved, each pointing at the directory its dependency occupies in the archive. For a plainly vendored layout that is the path the manifest already used; a path that does not describe the dependency's location relative to the workspace root — an absolute path, or one that traverses a symlink — is rewritten so the extracted bundle stays self-contained. Canister names stay as each project wrote them, a shared dependency remains a single instance, and canister discovery works from the extracted bundle exactly as it did in the source workspace. Extracting the archive gives you the same workspace with every build step replaced by its built wasm. -A bundle is built for one environment — `-e/--environment`, defaulting to `ic` — and it carries [the canisters that environment holds](#which-canisters-an-environment-holds) and no others. The bundled manifests declare only those canisters, and references the rest leave behind are dropped along with them: another environment's `canisters:` list loses the names it can no longer reach, a `dependencies:` entry's `canisters:` exposure list loses the ones the dependency no longer bundles, and a controller naming a canister the environment does not hold is dropped with a warning. So every environment in an extracted bundle holds at most what the bundle carries, and deploying the bundle to some other environment of its own deploys those same canisters. +A bundle is built for one environment — `-e/--environment`, defaulting to `ic` — and it carries [the canisters that environment holds](#which-canisters-an-environment-holds) and no others. The bundled manifests declare only those canisters, and references the rest leave behind are dropped along with them: another environment's `canisters:` list loses the names it can no longer reach, a `dependencies:` entry's `canisters:` exposure list loses the ones the dependency no longer bundles, an environment's `settings:` and `init_args:` lose their overrides 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 — is dropped with a warning. So every environment in an extracted bundle holds at most what the bundle carries, and deploying the bundle to some other environment of its own deploys those same canisters. Every dependency must resolve to a directory **inside** the workspace root, so that the archive can contain it. A dependency that resolves outside the root — `../elsewhere` declared by the root project, or a directory that is a symlink pointing out of the workspace — is rejected. For the same reason a vendored member that depends on a sibling cannot be bundled on its own (e.g. with `ICP_PROJECT_ROOT` pointing at the member): the sibling would fall outside the bundle. Bundle the workspace root instead.