feat: Allow specifying other canisters as controllers in the manifest

crates/icp-cli/src/commands/canister/create.rs

@@ -4,11 +4,13 @@ use anyhow::anyhow;
 use candid::{Nat, Principal};
 use clap::{ArgGroup, Args, Parser};
 use ic_management_canister_types::CanisterSettings as MgmtCanisterSettings;
+use icp::canister::resolve_controllers;
 use icp::context::Context;
 use icp::parsers::{CyclesAmount, DurationAmount, MemoryAmount};
+use icp::store_id::IdMapping;
 use icp::{Canister, context::CanisterSelection, prelude::*};
 use serde::Serialize;
-use tracing::info;
+use tracing::{info, warn};
 
 use crate::{
     commands::args,
@@ -112,22 +114,40 @@ pub(crate) struct CreateArgs {
 }
 
 impl CreateArgs {
+    /// Merge CLI settings with manifest defaults. Returns the merged `CanisterSettings`
+    /// and any controller canister names that could not be resolved (not yet created).
+    ///
+    /// `caller` is always injected into manifest-derived controller lists so the active identity
+    /// retains access after creation (ic00 `controllers: Some([])` would lock out the caller).
     pub(crate) fn canister_settings_with_default(
         &self,
         default: &Canister,
-    ) -> MgmtCanisterSettings {
-        MgmtCanisterSettings {
+        ids: &IdMapping,
+        caller: Principal,
+    ) -> (MgmtCanisterSettings, Vec<String>) {
+        // CLI --controller flags take precedence over manifest controllers.
+        let (controllers, unresolved): (Option<Vec<Principal>>, Vec<String>) =
+            if !self.controller.is_empty() {
+                (Some(self.controller.clone()), vec![])
+            } else if let Some(crefs) = &default.settings.controllers {
+                let (mut resolved, unresolved) = resolve_controllers(crefs, ids);
+                // The active identity must always be included so it retains access.
+                if !resolved.contains(&caller) {
+                    resolved.push(caller);
+                }
+                (Some(resolved), unresolved)
+            } else {
+                (None, vec![])
+            };
+
+        let settings = MgmtCanisterSettings {
             freezing_threshold: self
                 .settings
                 .freezing_threshold
                 .clone()
                 .or(default.settings.freezing_threshold.clone())
                 .map(|d| Nat::from(d.get())),
-            controllers: if self.controller.is_empty() {
-                None
-            } else {
-                Some(self.controller.clone())
-            },
+            controllers,
             reserved_cycles_limit: self
                 .settings
                 .reserved_cycles_limit
@@ -148,7 +168,8 @@ impl CreateArgs {
                 .or(default.settings.compute_allocation)
                 .map(Nat::from),
             ..Default::default()
-        }
+        };
+        (settings, unresolved)
     }
 
     fn create_target(&self) -> CreateTarget {
@@ -267,28 +288,49 @@ async fn create_project_canister(ctx: &Context, args: &CreateArgs) -> Result<(),
         return Ok(());
     }
 
+    let identity = ctx.get_identity(&selections.identity).await?;
+    let caller = identity
+        .sender()
+        .map_err(|e| anyhow!("failed to get caller principal: {e}"))?;
     let agent = ctx
         .get_agent_for_env(&selections.identity, &selections.environment)
         .await?;
-    let existing_canisters = ctx
+    let ids = ctx
         .ids_by_environment(&selections.environment)
         .await
-        .map_err(|e| anyhow!(e))?
-        .into_values()
-        .collect();
+        .map_err(|e| anyhow!(e))?;
 
     let create_operation = CreateOperation::new(
-        agent,
+        agent.clone(),
         args.create_target(),
         args.cycles.get(),
-        existing_canisters,
+        ids.values().copied().collect(),
     );
 
-    let canister_settings = args.canister_settings_with_default(&canister_info);
+    let (canister_settings, unresolved) =
+        args.canister_settings_with_default(&canister_info, &ids, caller);
+    for name in &unresolved {
+        warn!(
+            "Controller canister '{name}' for '{canister}' has not been created yet; \
+             it will be set as a controller once created."
+        );
+    }
+
     let id = create_operation.create(&canister_settings).await?;
 
     ctx.set_canister_id_for_env(&canister, id, &selections.environment)
         .await?;
+
+    crate::operations::settings::sync_controller_dependents(
+        ctx,
+        &agent,
+        args.proxy,
+        &canister,
+        &selections.environment,
+    )
+    .await
+    .map_err(|e| anyhow!(e))?;
+
     ctx.update_custom_domains(&selections.environment).await;
 
     if args.quiet {

crates/icp-cli/src/commands/canister/settings/sync.rs

@@ -2,6 +2,7 @@ use anyhow::bail;
 use candid::Principal;
 use clap::Args;
 use icp::context::{CanisterSelection, Context};
+use tracing::warn;
 
 use crate::commands::args::CanisterCommandArgs;
 
@@ -40,7 +41,19 @@ pub(crate) async fn exec(ctx: &Context, args: &SyncArgs) -> Result<(), anyhow::E
             &selections.environment,
         )
         .await?;
+    let ids = ctx
+        .ids_by_environment(&selections.environment)
+        .await
+        .map_err(|e| anyhow::anyhow!(e))?;
 
-    crate::operations::settings::sync_settings(&agent, args.proxy, &cid, &canister).await?;
+    let unresolved =
+        crate::operations::settings::sync_settings(&agent, args.proxy, &cid, &canister, &ids)
+            .await?;
+    for controller_name in &unresolved {
+        warn!(
+            "Controller canister '{controller_name}' for '{name}' has not been created yet; \
+             it will be set as a controller once created."
+        );
+    }
     Ok(())
 }

crates/icp-cli/src/commands/deploy.rs

@@ -22,7 +22,7 @@ use crate::{
         candid_compat::check_candid_compatibility_many,
         create::{CreateOperation, CreateTarget},
         install::{install_many, resolve_install_mode_and_status},
-        settings::sync_settings_many,
+        settings::{sync_controller_dependents, sync_settings_many},
         sync::sync_many,
     },
     options::{EnvironmentOpt, IdentityOpt},
@@ -199,6 +199,17 @@ pub(crate) async fn exec(ctx: &Context, args: &DeployArgs) -> Result<(), anyhow:
                     ctx.set_canister_id_for_env(canister_name, id, &environment_selection)
                         .await
                         .map_err(|e| anyhow!(e))?;
+                    // Apply controller settings for any already-created canister that was
+                    // waiting for this one to exist (e.g. created via `icp canister create`).
+                    sync_controller_dependents(
+                        ctx,
+                        &agent,
+                        args.proxy,
+                        canister_name,
+                        &environment_selection,
+                    )
+                    .await
+                    .map_err(|e| anyhow!(e))?;
                 }
                 Err(err) => {
                     error = Some(err.into());
@@ -248,15 +259,21 @@ pub(crate) async fn exec(ctx: &Context, args: &DeployArgs) -> Result<(), anyhow:
         args.proxy,
         &env.name,
         target_canisters.clone(),
-        canister_list,
+        canister_list.clone(),
         ctx.debug,
     )
     .await
     .map_err(|e| anyhow!(e))?;
 
-    sync_settings_many(agent.clone(), args.proxy, target_canisters, ctx.debug)
-        .await
-        .map_err(|e| anyhow!(e))?;
+    sync_settings_many(
+        agent.clone(),
+        args.proxy,
+        target_canisters,
+        canister_list,
+        ctx.debug,
+    )
+    .await
+    .map_err(|e| anyhow!(e))?;
 
     // Install the selected canisters