From 1e471928e2ed4d06083cfef0253176fa96bd114e Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Thu, 6 Nov 2025 01:29:51 -0500 Subject: [PATCH 1/9] settings sync command --- .../src/commands/canister/settings/mod.rs | 2 + .../src/commands/canister/settings/sync.rs | 78 ++++++++ crates/icp-cli/src/commands/deploy/mod.rs | 7 +- crates/icp-cli/src/main.rs | 6 + crates/icp-cli/src/operations/mod.rs | 1 + crates/icp-cli/src/operations/settings.rs | 124 ++++++++++++ .../icp-cli/tests/canister_settings_tests.rs | 183 ++++++++++++++++++ 7 files changed, 400 insertions(+), 1 deletion(-) create mode 100644 crates/icp-cli/src/commands/canister/settings/sync.rs create mode 100644 crates/icp-cli/src/operations/settings.rs diff --git a/crates/icp-cli/src/commands/canister/settings/mod.rs b/crates/icp-cli/src/commands/canister/settings/mod.rs index 54e07c6d4..a449f2b82 100644 --- a/crates/icp-cli/src/commands/canister/settings/mod.rs +++ b/crates/icp-cli/src/commands/canister/settings/mod.rs @@ -1,6 +1,7 @@ use clap::Subcommand; pub(crate) mod show; +pub(crate) mod sync; pub(crate) mod update; #[derive(Subcommand, Debug)] @@ -8,4 +9,5 @@ pub(crate) mod update; pub(crate) enum Command { Show(show::ShowArgs), Update(update::UpdateArgs), + Sync(sync::SyncArgs), } diff --git a/crates/icp-cli/src/commands/canister/settings/sync.rs b/crates/icp-cli/src/commands/canister/settings/sync.rs new file mode 100644 index 000000000..8c376f5cd --- /dev/null +++ b/crates/icp-cli/src/commands/canister/settings/sync.rs @@ -0,0 +1,78 @@ +use crate::{ + operations::settings::SyncSettingsOperationError, + options::{EnvironmentOpt, IdentityOpt, NetworkOpt}, +}; + +use clap::Args; +use ic_utils::interfaces::ManagementCanister; +use icp::{ + LoadError, + context::{ + Context, EnvironmentSelection, GetAgentForEnvError, GetCanisterIdForEnvError, + GetEnvironmentError, + }, +}; +use snafu::Snafu; + +#[derive(Debug, Args)] +pub(crate) struct SyncArgs { + name: String, + #[command(flatten)] + pub(crate) network: NetworkOpt, + #[command(flatten)] + pub(crate) environment: EnvironmentOpt, + #[command(flatten)] + pub(crate) identity: IdentityOpt, +} + +#[derive(Debug, Snafu)] +pub(crate) enum CommandError { + #[snafu(transparent)] + GetAgentForEnv { source: GetAgentForEnvError }, + #[snafu(transparent)] + GetEnvironment { source: GetEnvironmentError }, + #[snafu(transparent)] + GetCanisterIdForEnv { source: GetCanisterIdForEnvError }, + #[snafu(transparent)] + LoadProject { source: LoadError }, + + #[snafu(display("project does not contain a canister named '{name}'"))] + CanisterNotFound { name: String }, + + #[snafu(display("environment '{environment}' does not include canister '{name}'"))] + EnvironmentCanisterNotFound { name: String, environment: String }, + + #[snafu(transparent)] + SyncSettingsError { source: SyncSettingsOperationError }, +} + +pub(crate) async fn exec(ctx: &Context, args: &SyncArgs) -> Result<(), CommandError> { + let environment_selection: EnvironmentSelection = args.environment.clone().into(); + let name = &args.name; + + let p = ctx.project.load().await?; + let env = ctx.get_environment(&environment_selection).await?; + + let Some((_, canister)) = p.canisters.get(name) else { + return CanisterNotFoundSnafu { name }.fail(); + }; + + if !env.canisters.contains_key(&args.name) { + return EnvironmentCanisterNotFoundSnafu { + environment: &env.name, + name, + } + .fail(); + } + + let agent = ctx + .get_agent_for_env(&args.identity.clone().into(), &environment_selection) + .await?; + let cid = ctx + .get_canister_id_for_env(&args.name, &environment_selection) + .await?; + let mgmt = ManagementCanister::create(&agent); + + crate::operations::settings::sync_settings(&mgmt, &cid, canister).await?; + Ok(()) +} diff --git a/crates/icp-cli/src/commands/deploy/mod.rs b/crates/icp-cli/src/commands/deploy/mod.rs index a25447396..10f0dfec9 100644 --- a/crates/icp-cli/src/commands/deploy/mod.rs +++ b/crates/icp-cli/src/commands/deploy/mod.rs @@ -15,6 +15,7 @@ use crate::{ build::build_many_with_progress_bar, create::CreateOperation, install::{InstallOperationError, install_many}, + settings::sync_settings_many, sync::{SyncOperationError, sync_many}, }, options::{EnvironmentOpt, IdentityOpt}, @@ -225,13 +226,17 @@ pub(crate) async fn exec(ctx: &Context, args: &DeployArgs) -> Result<(), Command set_binding_env_vars_many( agent.clone(), &env.name, - target_canisters, + target_canisters.clone(), canister_list, ctx.debug, ) .await .map_err(|e| anyhow!(e))?; + sync_settings_many(agent.clone(), target_canisters, ctx.debug) + .await + .map_err(|e| anyhow!(e))?; + // Install the selected canisters let _ = ctx.term.write_line("\n\nInstalling canisters:"); diff --git a/crates/icp-cli/src/main.rs b/crates/icp-cli/src/main.rs index 7d57ceb40..7f2e2ef4f 100644 --- a/crates/icp-cli/src/main.rs +++ b/crates/icp-cli/src/main.rs @@ -194,6 +194,12 @@ async fn main() -> Result<(), Error> { .instrument(trace_span) .await? } + + commands::canister::settings::Command::Sync(args) => { + commands::canister::settings::sync::exec(&ctx, &args) + .instrument(trace_span) + .await? + } }, commands::canister::Command::Show(args) => { diff --git a/crates/icp-cli/src/operations/mod.rs b/crates/icp-cli/src/operations/mod.rs index 1f59b79eb..af173665f 100644 --- a/crates/icp-cli/src/operations/mod.rs +++ b/crates/icp-cli/src/operations/mod.rs @@ -2,4 +2,5 @@ pub(crate) mod binding_env_vars; pub(crate) mod build; pub(crate) mod create; pub(crate) mod install; +pub(crate) mod settings; pub(crate) mod sync; diff --git a/crates/icp-cli/src/operations/settings.rs b/crates/icp-cli/src/operations/settings.rs new file mode 100644 index 000000000..0ddb031db --- /dev/null +++ b/crates/icp-cli/src/operations/settings.rs @@ -0,0 +1,124 @@ +use std::collections::HashMap; + +use candid::Principal; +use futures::{StreamExt, stream::FuturesOrdered}; +use ic_agent::{Agent, AgentError}; +use ic_management_canister_types::EnvironmentVariable; +use ic_utils::interfaces::ManagementCanister; +use icp::{Canister, canister::Settings}; +use itertools::Itertools; +use snafu::{ResultExt, Snafu}; + +use crate::progress::{ProgressManager, ProgressManagerSettings}; + +#[derive(Debug, Snafu)] +pub(crate) enum SyncSettingsOperationError { + #[snafu(display("failed to fetch current canister settings for canister {canister}"))] + FetchCurrentSettings { + source: AgentError, + canister: Principal, + }, + #[snafu(display("invalid canister settings in manifest for canister {name}"))] + ValidateSettings { source: AgentError, name: String }, + #[snafu(display("failed to update canister settings for canister {canister}"))] + UpdateSettings { + source: AgentError, + canister: Principal, + }, +} + +pub(crate) async fn sync_settings( + mgmt: &ManagementCanister<'_>, + cid: &Principal, + canister: &Canister, +) -> Result<(), SyncSettingsOperationError> { + let (status,) = mgmt + .canister_status(cid) + .await + .context(FetchCurrentSettingsSnafu { canister: *cid })?; + let &Settings { + compute_allocation, + memory_allocation, + freezing_threshold, + reserved_cycles_limit, + wasm_memory_limit, + wasm_memory_threshold, + ref environment_variables, + } = &canister.settings; + let current_settings = status.settings; + let environment_variable_setting = + if let Some(configured_environment_variables) = &environment_variables { + let mut merged_environment_variables: HashMap<_, _> = current_settings + .environment_variables + .into_iter() + .map(|EnvironmentVariable { name, value }| (name, value)) + .collect(); + merged_environment_variables.extend(configured_environment_variables.clone()); + Some( + merged_environment_variables + .into_iter() + .map(|(name, value)| EnvironmentVariable { name, value }) + .collect_vec(), + ) + } else { + None + }; + + mgmt.update_settings(cid) + .with_optional_compute_allocation(compute_allocation) + .with_optional_memory_allocation(memory_allocation) + .with_optional_freezing_threshold(freezing_threshold) + .with_optional_reserved_cycles_limit(reserved_cycles_limit) + .with_optional_wasm_memory_limit(wasm_memory_limit) + .with_optional_wasm_memory_threshold(wasm_memory_threshold) + .with_optional_environment_variables(environment_variable_setting) + .build() + .context(ValidateSettingsSnafu { + name: &canister.name, + })? + .await + .context(UpdateSettingsSnafu { canister: *cid })?; + + Ok(()) +} + +pub(crate) async fn sync_settings_many( + agent: Agent, + target_canisters: Vec<(Principal, Canister)>, + debug: bool, +) -> Result<(), SyncSettingsOperationError> { + let mgmt = ManagementCanister::create(&agent); + + let mut futs = FuturesOrdered::new(); + let progress_manager = ProgressManager::new(ProgressManagerSettings { hidden: debug }); + + for (cid, info) in target_canisters { + let pb = progress_manager.create_progress_bar(&info.name); + + let settings_fn = { + let mgmt = mgmt.clone(); + let pb = pb.clone(); + + async move { + pb.set_message("Updating canister settings..."); + sync_settings(&mgmt, &cid, &info).await + } + }; + + futs.push_back(async move { + ProgressManager::execute_with_progress( + &pb, + settings_fn, + || "Canister settings updated successfully".to_string(), + |err| format!("Failed to update canister settings: {err}"), + ) + .await + }); + } + + while let Some(res) = futs.next().await { + res?; + } + + Ok(()) +} diff --git a/crates/icp-cli/tests/canister_settings_tests.rs b/crates/icp-cli/tests/canister_settings_tests.rs index eed270430..420b746c9 100644 --- a/crates/icp-cli/tests/canister_settings_tests.rs +++ b/crates/icp-cli/tests/canister_settings_tests.rs @@ -944,3 +944,186 @@ fn canister_settings_update_environment_variables() { .and(contains("Name: var3, Value: value3").not()), ); } + +#[test] +fn canister_settings_sync() { + let ctx = TestContext::new(); + + // Setup project + let project_dir = ctx.create_project_dir("icp"); + + // Use vendored WASM + let wasm = ctx.make_asset("example_icp_mo.wasm"); + + // Project manifest + let pm = formatdoc! {r#" + canisters: + - name: my-canister + build: + steps: + - type: script + command: cp {wasm} "$ICP_WASM_OUTPUT_PATH" + + {NETWORK_RANDOM_PORT} + {ENVIRONMENT_RANDOM_PORT} + "#}; + + write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest"); + + // Start network + let _g = ctx.start_network_in(&project_dir, "my-network"); + ctx.ping_until_healthy(&project_dir, "my-network"); + + // Deploy project + clients::icp(&ctx, &project_dir, Some("my-environment".to_string())) + .mint_cycles(200 * TRILLION); + + ctx.icp() + .current_dir(&project_dir) + .args([ + "deploy", + "--subnet", + common::SUBNET_ID, + "--environment", + "my-environment", + ]) + .assert() + .success(); + + // Test helpers for syncing settings and checking wasm memory limit + fn sync(ctx: &TestContext, project_dir: &Path) { + ctx.icp() + .current_dir(project_dir) + .args([ + "canister", + "settings", + "sync", + "my-canister", + "--environment", + "my-environment", + ]) + .assert() + .success(); + } + + fn confirm_wasm_memory_limit(ctx: &TestContext, project_dir: &Path, expected_limit: &str) { + ctx.icp() + .current_dir(project_dir) + .args([ + "canister", + "settings", + "show", + "my-canister", + "--environment", + "my-environment", + ]) + .assert() + .success() + .stderr(contains(format!("Wasm memory limit: {}", expected_limit))); + } + + // Initial value + confirm_wasm_memory_limit(&ctx, &project_dir, "3_221_225_472"); + + let pm_with_empty_settings = formatdoc! {r#" + canisters: + - name: my-canister + build: + steps: + - type: script + command: cp {wasm} "$ICP_WASM_OUTPUT_PATH" + settings: + + {NETWORK_RANDOM_PORT} + {ENVIRONMENT_RANDOM_PORT} + "#}; + + let pm_with_empty_wasm_limit = formatdoc! {r#" + canisters: + - name: my-canister + build: + steps: + - type: script + command: cp {wasm} "$ICP_WASM_OUTPUT_PATH" + settings: + wasm_memory_limit: ~ + + {NETWORK_RANDOM_PORT} + {ENVIRONMENT_RANDOM_PORT} + "#}; + + let pm_with_wasm_limit_4gb = formatdoc! {r#" + canisters: + - name: my-canister + build: + steps: + - type: script + command: cp {wasm} "$ICP_WASM_OUTPUT_PATH" + settings: + wasm_memory_limit: 4000000000 + + {NETWORK_RANDOM_PORT} + {ENVIRONMENT_RANDOM_PORT} + "#}; + + // Syncing a nonexistent setting should not override the default + write_string(&project_dir.join("icp.yaml"), &pm_with_empty_settings) + .expect("failed to write project manifest"); + sync(&ctx, &project_dir); + confirm_wasm_memory_limit(&ctx, &project_dir, "3_221_225_472"); + write_string(&project_dir.join("icp.yaml"), &pm_with_empty_wasm_limit) + .expect("failed to write project manifest"); + sync(&ctx, &project_dir); + confirm_wasm_memory_limit(&ctx, &project_dir, "3_221_225_472"); + // Setting wasm memory limit in the manifest and syncing should update the canister settings + write_string(&project_dir.join("icp.yaml"), &pm_with_wasm_limit_4gb) + .expect("failed to write project manifest"); + sync(&ctx, &project_dir); + confirm_wasm_memory_limit(&ctx, &project_dir, "4_000_000_000"); + // Existing settings should be overridden on sync + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "settings", + "update", + "my-canister", + "--environment", + "my-environment", + "--wasm-memory-limit", + "5GiB", + ]) + .assert() + .success(); + confirm_wasm_memory_limit(&ctx, &project_dir, "5_368_709_120"); + sync(&ctx, &project_dir); + confirm_wasm_memory_limit(&ctx, &project_dir, "4_000_000_000"); + // Syncing a nonexistent setting should not override a previously set setting + write_string(&project_dir.join("icp.yaml"), &pm).expect("failed to write project manifest"); + sync(&ctx, &project_dir); + confirm_wasm_memory_limit(&ctx, &project_dir, "4_000_000_000"); + write_string(&project_dir.join("icp.yaml"), &pm_with_empty_settings) + .expect("failed to write project manifest"); + sync(&ctx, &project_dir); + confirm_wasm_memory_limit(&ctx, &project_dir, "4_000_000_000"); + write_string(&project_dir.join("icp.yaml"), &pm_with_empty_wasm_limit) + .expect("failed to write project manifest"); + sync(&ctx, &project_dir); + confirm_wasm_memory_limit(&ctx, &project_dir, "4_000_000_000"); + ctx.icp() + .current_dir(&project_dir) + .args([ + "canister", + "settings", + "update", + "my-canister", + "--environment", + "my-environment", + "--wasm-memory-limit", + "5GiB", + ]) + .assert() + .success(); + sync(&ctx, &project_dir); + confirm_wasm_memory_limit(&ctx, &project_dir, "5_368_709_120"); +} From 4585e0cfae8eb5b572c6c72c4e3070e8b8d804b8 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Mon, 17 Nov 2025 09:06:28 -0800 Subject: [PATCH 2/9] . --- docs/cli-reference.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 35c77587a..691608039 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -16,6 +16,7 @@ This document contains the help content for the `icp-cli` command-line program. * [`icp-cli canister settings`↴](#icp-cli-canister-settings) * [`icp-cli canister settings show`↴](#icp-cli-canister-settings-show) * [`icp-cli canister settings update`↴](#icp-cli-canister-settings-update) +* [`icp-cli canister settings sync`↴](#icp-cli-canister-settings-sync) * [`icp-cli canister show`↴](#icp-cli-canister-show) * [`icp-cli canister start`↴](#icp-cli-canister-start) * [`icp-cli canister status`↴](#icp-cli-canister-status) @@ -249,6 +250,7 @@ List the canisters in an environment * `show` — * `update` — +* `sync` — @@ -301,6 +303,23 @@ List the canisters in an environment +## `icp-cli canister settings sync` + +**Usage:** `icp-cli canister settings sync [OPTIONS] ` + +###### **Arguments:** + +* `` + +###### **Options:** + +* `--network ` — Name of the network to target, conflicts with environment argument +* `--environment ` — Override the environment to connect to. By default, the local environment is used +* `--ic` — Shorthand for --environment=ic +* `--identity ` — The user identity to run this command as + + + ## `icp-cli canister show` Show a canister's details From ad11446d584ebd81dd4f590a7e5f07e7983ad712 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Mon, 17 Nov 2025 13:41:10 -0800 Subject: [PATCH 3/9] Add warning --- .../src/commands/canister/settings/update.rs | 73 ++++++++++++++++++- .../icp-cli/tests/canister_settings_tests.rs | 3 +- 2 files changed, 73 insertions(+), 3 deletions(-) diff --git a/crates/icp-cli/src/commands/canister/settings/update.rs b/crates/icp-cli/src/commands/canister/settings/update.rs index 74e8dea1c..f63386475 100644 --- a/crates/icp-cli/src/commands/canister/settings/update.rs +++ b/crates/icp-cli/src/commands/canister/settings/update.rs @@ -4,9 +4,9 @@ use byte_unit::{Byte, Unit}; use clap::{ArgAction, Args}; use ic_agent::{AgentError, export::Principal}; use ic_management_canister_types::{CanisterStatusResult, EnvironmentVariable, LogVisibility}; -use icp::{agent, identity, network}; +use icp::{LoadError, agent, identity, network}; -use icp::context::{Context, GetCanisterIdAndAgentError}; +use icp::context::{CanisterSelection, Context, GetCanisterIdAndAgentError}; use crate::commands::args; use icp::store_id::LookupIdError; @@ -142,6 +142,16 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command ) .await?; + let configured_settings = if let CanisterSelection::Named(name) = &selections.canister { + match ctx.project.load().await { + Ok(p) => p.canisters[name].1.settings.clone(), + Err(LoadError::Locate) => <_>::default(), + Err(e) => return Err(CommandError::Project(e)), + } + } else { + <_>::default() + }; + // Management Interface let mgmt = ic_utils::interfaces::ManagementCanister::create(&agent); @@ -169,6 +179,7 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command // Handle environment variables. let mut environment_variables: Option> = None; if let Some(environment_variables_opt) = &args.environment_variables { + maybe_warn_on_env_vars_change(&configured_settings, environment_variables_opt); environment_variables = get_environment_variables(environment_variables_opt, current_status.as_ref()); } @@ -181,21 +192,51 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command } } if let Some(compute_allocation) = args.compute_allocation { + if configured_settings.compute_allocation.is_some() { + eprintln!( + "Warning: Compute allocation is already set in icp.yaml; this new value will be overridden on next settings sync" + ) + } update = update.with_compute_allocation(compute_allocation); } if let Some(memory_allocation) = args.memory_allocation { + if configured_settings.memory_allocation.is_some() { + eprintln!( + "Warning: Memory allocation is already set in icp.yaml; this new value will be overridden on next settings sync" + ) + } update = update.with_memory_allocation(memory_allocation.as_u64()); } if let Some(freezing_threshold) = args.freezing_threshold { + if configured_settings.freezing_threshold.is_some() { + eprintln!( + "Warning: Freezing threshold is already set in icp.yaml; this new value will be overridden on next settings sync" + ) + } update = update.with_freezing_threshold(freezing_threshold); } if let Some(reserved_cycles_limit) = args.reserved_cycles_limit { + if configured_settings.reserved_cycles_limit.is_some() { + eprintln!( + "Warning: Reserved cycles limit is already set in icp.yaml; this new value will be overridden on next settings sync" + ) + } update = update.with_reserved_cycles_limit(reserved_cycles_limit); } if let Some(wasm_memory_limit) = args.wasm_memory_limit { + if configured_settings.wasm_memory_limit.is_some() { + eprintln!( + "Warning: Wasm memory limit is already set in icp.yaml; this new value will be overridden on next settings sync" + ) + } update = update.with_wasm_memory_limit(wasm_memory_limit.as_u64()); } if let Some(wasm_memory_threshold) = args.wasm_memory_threshold { + if configured_settings.wasm_memory_threshold.is_some() { + eprintln!( + "Warning: Wasm memory threshold is already set in icp.yaml; this new value will be overridden on next settings sync" + ) + } update = update.with_wasm_memory_threshold(wasm_memory_threshold.as_u64()); } if let Some(log_visibility) = log_visibility { @@ -395,3 +436,31 @@ fn get_environment_variables( None } + +fn maybe_warn_on_env_vars_change( + configured_settings: &icp::canister::Settings, + environment_variables_opt: &EnvironmentVariableOpt, +) { + if let Some(configured_vars) = &configured_settings.environment_variables { + if let Some(to_add) = &environment_variables_opt.add_environment_variable { + for add_var in to_add { + if configured_vars.contains_key(&add_var.name) { + eprintln!( + "Warning: Environment variable '{}' is already set in icp.yaml; this new value will be overridden on next settings sync", + add_var.name + ) + } + } + } + if let Some(to_remove) = &environment_variables_opt.remove_environment_variable { + for remove_var in to_remove { + if configured_vars.contains_key(remove_var) { + eprintln!( + "Warning: Environment variable '{}' is already set in icp.yaml; removing it here will be overridden on next settings sync", + remove_var + ) + } + } + } + } +} diff --git a/crates/icp-cli/tests/canister_settings_tests.rs b/crates/icp-cli/tests/canister_settings_tests.rs index 420b746c9..1c779d00b 100644 --- a/crates/icp-cli/tests/canister_settings_tests.rs +++ b/crates/icp-cli/tests/canister_settings_tests.rs @@ -1094,7 +1094,8 @@ fn canister_settings_sync() { "5GiB", ]) .assert() - .success(); + .success() + .stderr(contains("Wasm memory limit is already set in icp.yaml")); confirm_wasm_memory_limit(&ctx, &project_dir, "5_368_709_120"); sync(&ctx, &project_dir); confirm_wasm_memory_limit(&ctx, &project_dir, "4_000_000_000"); From 04da38d1ff8a05a402a47e65035ebda64d5c4bc7 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Tue, 18 Nov 2025 09:49:26 -0800 Subject: [PATCH 4/9] feedback --- crates/icp-cli/src/commands/canister/mod.rs | 1 + .../src/commands/canister/settings/mod.rs | 3 + .../src/commands/canister/settings/sync.rs | 57 +++++++++---------- .../src/commands/canister/settings/update.rs | 45 +++++++++------ crates/icp-cli/src/operations/settings.rs | 16 +++++- docs/cli-reference.md | 20 +++++-- 6 files changed, 87 insertions(+), 55 deletions(-) diff --git a/crates/icp-cli/src/commands/canister/mod.rs b/crates/icp-cli/src/commands/canister/mod.rs index e85e6a468..0657d1edb 100644 --- a/crates/icp-cli/src/commands/canister/mod.rs +++ b/crates/icp-cli/src/commands/canister/mod.rs @@ -34,6 +34,7 @@ pub(crate) enum Command { /// List the canisters in an environment List(list::ListArgs), + /// Commands to manage canister settings #[command(subcommand)] Settings(settings::Command), diff --git a/crates/icp-cli/src/commands/canister/settings/mod.rs b/crates/icp-cli/src/commands/canister/settings/mod.rs index a449f2b82..d91b177ce 100644 --- a/crates/icp-cli/src/commands/canister/settings/mod.rs +++ b/crates/icp-cli/src/commands/canister/settings/mod.rs @@ -7,7 +7,10 @@ pub(crate) mod update; #[derive(Subcommand, Debug)] #[allow(clippy::large_enum_variant)] pub(crate) enum Command { + /// Display a canister's settings Show(show::ShowArgs), + /// Change a canister's settings to specified values Update(update::UpdateArgs), + /// Synchronize a canister's settings with those defined in the project Sync(sync::SyncArgs), } diff --git a/crates/icp-cli/src/commands/canister/settings/sync.rs b/crates/icp-cli/src/commands/canister/settings/sync.rs index 8c376f5cd..2948f32a4 100644 --- a/crates/icp-cli/src/commands/canister/settings/sync.rs +++ b/crates/icp-cli/src/commands/canister/settings/sync.rs @@ -1,6 +1,5 @@ use crate::{ - operations::settings::SyncSettingsOperationError, - options::{EnvironmentOpt, IdentityOpt, NetworkOpt}, + commands::args::CanisterCommandArgs, operations::settings::SyncSettingsOperationError, }; use clap::Args; @@ -8,7 +7,7 @@ use ic_utils::interfaces::ManagementCanister; use icp::{ LoadError, context::{ - Context, EnvironmentSelection, GetAgentForEnvError, GetCanisterIdForEnvError, + AssertEnvContainsCanisterError, CanisterSelection, Context, GetCanisterIdAndAgentError, GetEnvironmentError, }, }; @@ -16,61 +15,59 @@ use snafu::Snafu; #[derive(Debug, Args)] pub(crate) struct SyncArgs { - name: String, #[command(flatten)] - pub(crate) network: NetworkOpt, - #[command(flatten)] - pub(crate) environment: EnvironmentOpt, - #[command(flatten)] - pub(crate) identity: IdentityOpt, + cmd_args: CanisterCommandArgs, } #[derive(Debug, Snafu)] pub(crate) enum CommandError { #[snafu(transparent)] - GetAgentForEnv { source: GetAgentForEnvError }, + GetIdAndAgent { source: GetCanisterIdAndAgentError }, + #[snafu(transparent)] GetEnvironment { source: GetEnvironmentError }, - #[snafu(transparent)] - GetCanisterIdForEnv { source: GetCanisterIdForEnvError }, + + #[snafu(display("Canister name must be used for settings sync"))] + PrincipalCanister, + #[snafu(transparent)] LoadProject { source: LoadError }, #[snafu(display("project does not contain a canister named '{name}'"))] CanisterNotFound { name: String }, - #[snafu(display("environment '{environment}' does not include canister '{name}'"))] - EnvironmentCanisterNotFound { name: String, environment: String }, + #[snafu(transparent)] + EnvironmentCanisterNotFound { + source: AssertEnvContainsCanisterError, + }, #[snafu(transparent)] SyncSettingsError { source: SyncSettingsOperationError }, } pub(crate) async fn exec(ctx: &Context, args: &SyncArgs) -> Result<(), CommandError> { - let environment_selection: EnvironmentSelection = args.environment.clone().into(); - let name = &args.name; + let selections = args.cmd_args.selections(); + let CanisterSelection::Named(name) = &selections.canister else { + return PrincipalCanisterSnafu.fail(); + }; let p = ctx.project.load().await?; - let env = ctx.get_environment(&environment_selection).await?; let Some((_, canister)) = p.canisters.get(name) else { return CanisterNotFoundSnafu { name }.fail(); }; - - if !env.canisters.contains_key(&args.name) { - return EnvironmentCanisterNotFoundSnafu { - environment: &env.name, - name, - } - .fail(); - } - - let agent = ctx - .get_agent_for_env(&args.identity.clone().into(), &environment_selection) + ctx.assert_env_contains_canister(name, &selections.environment) .await?; - let cid = ctx - .get_canister_id_for_env(&args.name, &environment_selection) + + let (cid, agent) = ctx + .get_canister_id_and_agent( + &selections.canister, + &selections.environment, + &selections.network, + &selections.identity, + ) .await?; + let mgmt = ManagementCanister::create(&agent); crate::operations::settings::sync_settings(&mgmt, &cid, canister).await?; diff --git a/crates/icp-cli/src/commands/canister/settings/update.rs b/crates/icp-cli/src/commands/canister/settings/update.rs index f63386475..79bca71f1 100644 --- a/crates/icp-cli/src/commands/canister/settings/update.rs +++ b/crates/icp-cli/src/commands/canister/settings/update.rs @@ -1,7 +1,9 @@ use std::collections::{HashMap, HashSet}; +use std::io::{self, Write}; use byte_unit::{Byte, Unit}; use clap::{ArgAction, Args}; +use console::Term; use ic_agent::{AgentError, export::Principal}; use ic_management_canister_types::{CanisterStatusResult, EnvironmentVariable, LogVisibility}; use icp::{LoadError, agent, identity, network}; @@ -129,6 +131,9 @@ pub(crate) enum CommandError { #[error(transparent)] GetCanisterIdAndAgent(#[from] GetCanisterIdAndAgentError), + + #[error("failed to write to terminal")] + WriteTerm(#[from] std::io::Error), } pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), CommandError> { @@ -179,7 +184,7 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command // Handle environment variables. let mut environment_variables: Option> = None; if let Some(environment_variables_opt) = &args.environment_variables { - maybe_warn_on_env_vars_change(&configured_settings, environment_variables_opt); + maybe_warn_on_env_vars_change(&ctx.term, &configured_settings, environment_variables_opt)?; environment_variables = get_environment_variables(environment_variables_opt, current_status.as_ref()); } @@ -193,49 +198,49 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command } if let Some(compute_allocation) = args.compute_allocation { if configured_settings.compute_allocation.is_some() { - eprintln!( + ctx.term.write_line( "Warning: Compute allocation is already set in icp.yaml; this new value will be overridden on next settings sync" - ) + )? } update = update.with_compute_allocation(compute_allocation); } if let Some(memory_allocation) = args.memory_allocation { if configured_settings.memory_allocation.is_some() { - eprintln!( + ctx.term.write_line( "Warning: Memory allocation is already set in icp.yaml; this new value will be overridden on next settings sync" - ) + )? } update = update.with_memory_allocation(memory_allocation.as_u64()); } if let Some(freezing_threshold) = args.freezing_threshold { if configured_settings.freezing_threshold.is_some() { - eprintln!( + ctx.term.write_line( "Warning: Freezing threshold is already set in icp.yaml; this new value will be overridden on next settings sync" - ) + )? } update = update.with_freezing_threshold(freezing_threshold); } if let Some(reserved_cycles_limit) = args.reserved_cycles_limit { if configured_settings.reserved_cycles_limit.is_some() { - eprintln!( + ctx.term.write_line( "Warning: Reserved cycles limit is already set in icp.yaml; this new value will be overridden on next settings sync" - ) + )? } update = update.with_reserved_cycles_limit(reserved_cycles_limit); } if let Some(wasm_memory_limit) = args.wasm_memory_limit { if configured_settings.wasm_memory_limit.is_some() { - eprintln!( + ctx.term.write_line( "Warning: Wasm memory limit is already set in icp.yaml; this new value will be overridden on next settings sync" - ) + )? } update = update.with_wasm_memory_limit(wasm_memory_limit.as_u64()); } if let Some(wasm_memory_threshold) = args.wasm_memory_threshold { if configured_settings.wasm_memory_threshold.is_some() { - eprintln!( + ctx.term.write_line( "Warning: Wasm memory threshold is already set in icp.yaml; this new value will be overridden on next settings sync" - ) + )? } update = update.with_wasm_memory_threshold(wasm_memory_threshold.as_u64()); } @@ -438,29 +443,33 @@ fn get_environment_variables( } fn maybe_warn_on_env_vars_change( + mut term: &Term, configured_settings: &icp::canister::Settings, environment_variables_opt: &EnvironmentVariableOpt, -) { +) -> Result<(), io::Error> { if let Some(configured_vars) = &configured_settings.environment_variables { if let Some(to_add) = &environment_variables_opt.add_environment_variable { for add_var in to_add { if configured_vars.contains_key(&add_var.name) { - eprintln!( + writeln!( + term, "Warning: Environment variable '{}' is already set in icp.yaml; this new value will be overridden on next settings sync", add_var.name - ) + )?; } } } if let Some(to_remove) = &environment_variables_opt.remove_environment_variable { for remove_var in to_remove { if configured_vars.contains_key(remove_var) { - eprintln!( + writeln!( + term, "Warning: Environment variable '{}' is already set in icp.yaml; removing it here will be overridden on next settings sync", remove_var - ) + )?; } } } } + Ok(()) } diff --git a/crates/icp-cli/src/operations/settings.rs b/crates/icp-cli/src/operations/settings.rs index 0ddb031db..29ba1ca40 100644 --- a/crates/icp-cli/src/operations/settings.rs +++ b/crates/icp-cli/src/operations/settings.rs @@ -46,10 +46,12 @@ pub(crate) async fn sync_settings( ref environment_variables, } = &canister.settings; let current_settings = status.settings; + let environment_variable_setting = if let Some(configured_environment_variables) = &environment_variables { let mut merged_environment_variables: HashMap<_, _> = current_settings .environment_variables + .clone() .into_iter() .map(|EnvironmentVariable { name, value }| (name, value)) .collect(); @@ -63,7 +65,19 @@ pub(crate) async fn sync_settings( } else { None }; - + if compute_allocation.is_some_and(|s| s == current_settings.compute_allocation) + && memory_allocation.is_some_and(|s| s == current_settings.memory_allocation) + && freezing_threshold.is_some_and(|s| s == current_settings.freezing_threshold) + && reserved_cycles_limit.is_some_and(|s| s == current_settings.reserved_cycles_limit) + && wasm_memory_limit.is_some_and(|s| s == current_settings.wasm_memory_limit) + && wasm_memory_threshold.is_some_and(|s| s == current_settings.wasm_memory_threshold) + && environment_variable_setting + .as_ref() + .is_some_and(|s| *s == current_settings.environment_variables) + { + // No changes needed + return Ok(()); + } mgmt.update_settings(cid) .with_optional_compute_allocation(compute_allocation) .with_optional_memory_allocation(memory_allocation) diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 691608039..40a394fb6 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -106,7 +106,7 @@ Perform canister operations against a network * `info` — Display a canister's information * `install` — Install a built WASM to a canister on a network * `list` — List the canisters in an environment -* `settings` — +* `settings` — Commands to manage canister settings * `show` — Show a canister's details * `start` — Start a canister on a network * `status` — Show the status of a canister @@ -244,18 +244,22 @@ List the canisters in an environment ## `icp-cli canister settings` +Commands to manage canister settings + **Usage:** `icp-cli canister settings ` ###### **Subcommands:** -* `show` — -* `update` — -* `sync` — +* `show` — Display a canister's settings +* `update` — Change a canister's settings to specified values +* `sync` — Synchronize a canister's settings with those defined in the project ## `icp-cli canister settings show` +Display a canister's settings + **Usage:** `icp-cli canister settings show [OPTIONS] ` ###### **Arguments:** @@ -273,6 +277,8 @@ List the canisters in an environment ## `icp-cli canister settings update` +Change a canister's settings to specified values + **Usage:** `icp-cli canister settings update [OPTIONS] ` ###### **Arguments:** @@ -305,11 +311,13 @@ List the canisters in an environment ## `icp-cli canister settings sync` -**Usage:** `icp-cli canister settings sync [OPTIONS] ` +Synchronize a canister's settings with those defined in the project + +**Usage:** `icp-cli canister settings sync [OPTIONS] ` ###### **Arguments:** -* `` +* `` — Name or principal of canister to target When using a name an environment must be specified ###### **Options:** From 9bc0cd09a45947a1e636a1879bc4e7636b0e1586 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Tue, 18 Nov 2025 09:55:09 -0800 Subject: [PATCH 5/9] that makes it stdout --- crates/icp-cli/tests/canister_settings_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/icp-cli/tests/canister_settings_tests.rs b/crates/icp-cli/tests/canister_settings_tests.rs index 1c779d00b..3d6f6410c 100644 --- a/crates/icp-cli/tests/canister_settings_tests.rs +++ b/crates/icp-cli/tests/canister_settings_tests.rs @@ -1095,7 +1095,7 @@ fn canister_settings_sync() { ]) .assert() .success() - .stderr(contains("Wasm memory limit is already set in icp.yaml")); + .stdout(contains("Wasm memory limit is already set in icp.yaml")); confirm_wasm_memory_limit(&ctx, &project_dir, "5_368_709_120"); sync(&ctx, &project_dir); confirm_wasm_memory_limit(&ctx, &project_dir, "4_000_000_000"); From 45b04a11bf41786269418d25b4ff87c3ae31e7fc Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Tue, 18 Nov 2025 09:57:39 -0800 Subject: [PATCH 6/9] . --- crates/icp-cli/src/operations/settings.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/crates/icp-cli/src/operations/settings.rs b/crates/icp-cli/src/operations/settings.rs index 29ba1ca40..c9688ee18 100644 --- a/crates/icp-cli/src/operations/settings.rs +++ b/crates/icp-cli/src/operations/settings.rs @@ -65,15 +65,15 @@ pub(crate) async fn sync_settings( } else { None }; - if compute_allocation.is_some_and(|s| s == current_settings.compute_allocation) - && memory_allocation.is_some_and(|s| s == current_settings.memory_allocation) - && freezing_threshold.is_some_and(|s| s == current_settings.freezing_threshold) - && reserved_cycles_limit.is_some_and(|s| s == current_settings.reserved_cycles_limit) - && wasm_memory_limit.is_some_and(|s| s == current_settings.wasm_memory_limit) - && wasm_memory_threshold.is_some_and(|s| s == current_settings.wasm_memory_threshold) + if compute_allocation.is_none_or(|s| s == current_settings.compute_allocation) + && memory_allocation.is_none_or(|s| s == current_settings.memory_allocation) + && freezing_threshold.is_none_or(|s| s == current_settings.freezing_threshold) + && reserved_cycles_limit.is_none_or(|s| s == current_settings.reserved_cycles_limit) + && wasm_memory_limit.is_none_or(|s| s == current_settings.wasm_memory_limit) + && wasm_memory_threshold.is_none_or(|s| s == current_settings.wasm_memory_threshold) && environment_variable_setting .as_ref() - .is_some_and(|s| *s == current_settings.environment_variables) + .is_none_or(|s| *s == current_settings.environment_variables) { // No changes needed return Ok(()); From d1ea0ac67584fcdaa468583b55db1095f0e367c1 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Tue, 18 Nov 2025 10:28:19 -0800 Subject: [PATCH 7/9] box some large errors to reduce result size for clippy lint --- .../src/commands/canister/settings/update.rs | 59 ++++++++++--------- crates/icp/src/agent.rs | 16 +++-- crates/icp/src/context/mod.rs | 5 +- crates/icp/src/identity/key.rs | 9 ++- 4 files changed, 50 insertions(+), 39 deletions(-) diff --git a/crates/icp-cli/src/commands/canister/settings/update.rs b/crates/icp-cli/src/commands/canister/settings/update.rs index 79bca71f1..fa7555cce 100644 --- a/crates/icp-cli/src/commands/canister/settings/update.rs +++ b/crates/icp-cli/src/commands/canister/settings/update.rs @@ -1,5 +1,5 @@ use std::collections::{HashMap, HashSet}; -use std::io::{self, Write}; +use std::io::Write; use byte_unit::{Byte, Unit}; use clap::{ArgAction, Args}; @@ -9,6 +9,7 @@ use ic_management_canister_types::{CanisterStatusResult, EnvironmentVariable, Lo use icp::{LoadError, agent, identity, network}; use icp::context::{CanisterSelection, Context, GetCanisterIdAndAgentError}; +use snafu::{ResultExt, Snafu}; use crate::commands::args; use icp::store_id::LookupIdError; @@ -106,34 +107,34 @@ pub(crate) struct UpdateArgs { environment_variables: Option, } -#[derive(Debug, thiserror::Error)] +#[derive(Debug, Snafu)] pub(crate) enum CommandError { - #[error(transparent)] - Project(#[from] icp::LoadError), + #[snafu(transparent)] + Project { source: icp::LoadError }, - #[error(transparent)] - Identity(#[from] identity::LoadError), + #[snafu(transparent)] + Identity { source: identity::LoadError }, - #[error(transparent)] - Access(#[from] network::AccessError), + #[snafu(transparent)] + Access { source: network::AccessError }, - #[error(transparent)] - Agent(#[from] agent::CreateAgentError), + #[snafu(transparent)] + Agent { source: agent::CreateAgentError }, - #[error("invalid environment variable '{variable}'")] + #[snafu(display("invalid environment variable '{variable}'"))] InvalidEnvironmentVariable { variable: String }, - #[error(transparent)] - Lookup(#[from] LookupIdError), + #[snafu(transparent)] + Lookup { source: LookupIdError }, - #[error(transparent)] - Update(#[from] AgentError), + #[snafu(transparent)] + Update { source: AgentError }, - #[error(transparent)] - GetCanisterIdAndAgent(#[from] GetCanisterIdAndAgentError), + #[snafu(transparent)] + GetCanisterIdAndAgent { source: GetCanisterIdAndAgentError }, - #[error("failed to write to terminal")] - WriteTerm(#[from] std::io::Error), + #[snafu(display("failed to write to terminal"))] + WriteTerm { source: std::io::Error }, } pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), CommandError> { @@ -151,7 +152,7 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command match ctx.project.load().await { Ok(p) => p.canisters[name].1.settings.clone(), Err(LoadError::Locate) => <_>::default(), - Err(e) => return Err(CommandError::Project(e)), + Err(e) => return Err(CommandError::Project { source: e }), } } else { <_>::default() @@ -200,7 +201,7 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command if configured_settings.compute_allocation.is_some() { ctx.term.write_line( "Warning: Compute allocation is already set in icp.yaml; this new value will be overridden on next settings sync" - )? + ).context(WriteTermSnafu)? } update = update.with_compute_allocation(compute_allocation); } @@ -208,7 +209,7 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command if configured_settings.memory_allocation.is_some() { ctx.term.write_line( "Warning: Memory allocation is already set in icp.yaml; this new value will be overridden on next settings sync" - )? + ).context(WriteTermSnafu)? } update = update.with_memory_allocation(memory_allocation.as_u64()); } @@ -216,7 +217,7 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command if configured_settings.freezing_threshold.is_some() { ctx.term.write_line( "Warning: Freezing threshold is already set in icp.yaml; this new value will be overridden on next settings sync" - )? + ).context(WriteTermSnafu)? } update = update.with_freezing_threshold(freezing_threshold); } @@ -224,7 +225,7 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command if configured_settings.reserved_cycles_limit.is_some() { ctx.term.write_line( "Warning: Reserved cycles limit is already set in icp.yaml; this new value will be overridden on next settings sync" - )? + ).context(WriteTermSnafu)? } update = update.with_reserved_cycles_limit(reserved_cycles_limit); } @@ -232,7 +233,7 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command if configured_settings.wasm_memory_limit.is_some() { ctx.term.write_line( "Warning: Wasm memory limit is already set in icp.yaml; this new value will be overridden on next settings sync" - )? + ).context(WriteTermSnafu)? } update = update.with_wasm_memory_limit(wasm_memory_limit.as_u64()); } @@ -240,7 +241,7 @@ pub(crate) async fn exec(ctx: &Context, args: &UpdateArgs) -> Result<(), Command if configured_settings.wasm_memory_threshold.is_some() { ctx.term.write_line( "Warning: Wasm memory threshold is already set in icp.yaml; this new value will be overridden on next settings sync" - )? + ).context(WriteTermSnafu)? } update = update.with_wasm_memory_threshold(wasm_memory_threshold.as_u64()); } @@ -446,7 +447,7 @@ fn maybe_warn_on_env_vars_change( mut term: &Term, configured_settings: &icp::canister::Settings, environment_variables_opt: &EnvironmentVariableOpt, -) -> Result<(), io::Error> { +) -> Result<(), CommandError> { if let Some(configured_vars) = &configured_settings.environment_variables { if let Some(to_add) = &environment_variables_opt.add_environment_variable { for add_var in to_add { @@ -455,7 +456,7 @@ fn maybe_warn_on_env_vars_change( term, "Warning: Environment variable '{}' is already set in icp.yaml; this new value will be overridden on next settings sync", add_var.name - )?; + ).context(WriteTermSnafu)?; } } } @@ -466,7 +467,7 @@ fn maybe_warn_on_env_vars_change( term, "Warning: Environment variable '{}' is already set in icp.yaml; removing it here will be overridden on next settings sync", remove_var - )?; + ).context(WriteTermSnafu)?; } } } diff --git a/crates/icp/src/agent.rs b/crates/icp/src/agent.rs index cfb361e89..b8326b868 100644 --- a/crates/icp/src/agent.rs +++ b/crates/icp/src/agent.rs @@ -2,16 +2,20 @@ use std::{sync::Arc, time::Duration}; use async_trait::async_trait; use ic_agent::{Agent, AgentError, Identity}; +use snafu::Snafu; use crate::prelude::*; -#[derive(Debug, thiserror::Error)] +#[derive(Debug, Snafu)] pub enum CreateAgentError { - #[error(transparent)] - Agent(#[from] AgentError), - - #[error(transparent)] - Unexpected(#[from] anyhow::Error), + #[snafu(transparent)] + Agent { + #[snafu(source(from(AgentError, Box::new)))] + source: Box, + }, + + #[snafu(transparent)] + Unexpected { source: anyhow::Error }, } #[async_trait] diff --git a/crates/icp/src/context/mod.rs b/crates/icp/src/context/mod.rs index d12f7f2c2..37f1273cf 100644 --- a/crates/icp/src/context/mod.rs +++ b/crates/icp/src/context/mod.rs @@ -9,7 +9,7 @@ use crate::{ identity::IdentitySelection, network::{Configuration as NetworkConfiguration, access::NetworkAccess}, project::DEFAULT_LOCAL_ENVIRONMENT_NAME, - store_id::IdMapping, + store_id::{IdMapping, LookupIdError}, }; use candid::Principal; use ic_agent::{Agent, Identity}; @@ -476,7 +476,8 @@ pub enum GetCanisterIdForEnvError { environment_name ))] CanisterIdLookup { - source: crate::store_id::LookupIdError, + #[snafu(source(from(LookupIdError, Box::new)))] + source: Box, canister_name: String, environment_name: String, }, diff --git a/crates/icp/src/identity/key.rs b/crates/icp/src/identity/key.rs index bd8f80e7c..ade67a65e 100644 --- a/crates/icp/src/identity/key.rs +++ b/crates/icp/src/identity/key.rs @@ -50,11 +50,16 @@ pub enum LoadIdentityError { #[snafu(display("failed to load PEM file `{path}`: failed to parse"))] ParsePemError { path: PathBuf, - source: pem::PemError, + #[snafu(source(from(pem::PemError, Box::new)))] + source: Box, }, #[snafu(display("failed to load PEM file `{path}`: failed to decipher key"))] - ParseKeyError { path: PathBuf, source: pkcs8::Error }, + ParseKeyError { + path: PathBuf, + #[snafu(source(from(pkcs8::Error, Box::new)))] + source: Box, + }, #[snafu(display("no identity found with name `{name}`"))] NoSuchIdentity { name: String }, From 250116119507c2f6709dee68431e47e6b6dcdc49 Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Tue, 18 Nov 2025 10:30:16 -0800 Subject: [PATCH 8/9] convert to snafu while I'm here From 76af09b7b984b48970e5a42666a3e3e812ea749a Mon Sep 17 00:00:00 2001 From: Adam Spofford Date: Tue, 18 Nov 2025 17:40:52 -0800 Subject: [PATCH 9/9] merge checks --- .../src/commands/canister/settings/sync.rs | 29 +++++-------------- 1 file changed, 7 insertions(+), 22 deletions(-) diff --git a/crates/icp-cli/src/commands/canister/settings/sync.rs b/crates/icp-cli/src/commands/canister/settings/sync.rs index 2948f32a4..4e1a79742 100644 --- a/crates/icp-cli/src/commands/canister/settings/sync.rs +++ b/crates/icp-cli/src/commands/canister/settings/sync.rs @@ -4,12 +4,9 @@ use crate::{ use clap::Args; use ic_utils::interfaces::ManagementCanister; -use icp::{ - LoadError, - context::{ - AssertEnvContainsCanisterError, CanisterSelection, Context, GetCanisterIdAndAgentError, - GetEnvironmentError, - }, +use icp::context::{ + CanisterSelection, Context, GetCanisterIdAndAgentError, GetEnvCanisterError, + GetEnvironmentError, }; use snafu::Snafu; @@ -31,15 +28,7 @@ pub(crate) enum CommandError { PrincipalCanister, #[snafu(transparent)] - LoadProject { source: LoadError }, - - #[snafu(display("project does not contain a canister named '{name}'"))] - CanisterNotFound { name: String }, - - #[snafu(transparent)] - EnvironmentCanisterNotFound { - source: AssertEnvContainsCanisterError, - }, + GetEnvCanister { source: GetEnvCanisterError }, #[snafu(transparent)] SyncSettingsError { source: SyncSettingsOperationError }, @@ -51,12 +40,8 @@ pub(crate) async fn exec(ctx: &Context, args: &SyncArgs) -> Result<(), CommandEr return PrincipalCanisterSnafu.fail(); }; - let p = ctx.project.load().await?; - - let Some((_, canister)) = p.canisters.get(name) else { - return CanisterNotFoundSnafu { name }.fail(); - }; - ctx.assert_env_contains_canister(name, &selections.environment) + let (_, canister) = ctx + .get_canister_and_path_for_env(name, &selections.environment) .await?; let (cid, agent) = ctx @@ -70,6 +55,6 @@ pub(crate) async fn exec(ctx: &Context, args: &SyncArgs) -> Result<(), CommandEr let mgmt = ManagementCanister::create(&agent); - crate::operations::settings::sync_settings(&mgmt, &cid, canister).await?; + crate::operations::settings::sync_settings(&mgmt, &cid, &canister).await?; Ok(()) }