chore: make ctx.agent, ctx.identity private (#184)

Author: viviveevee

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

@@ -2,13 +2,17 @@
 // For now it's only used to set environment variables
 // Eventually we will add support for canister settings operation
 
-use std::collections::{HashMap, HashSet};
+use std::collections::HashSet;
 
 use clap::Args;
-use futures::{StreamExt, stream::FuturesOrdered};
+use futures::{StreamExt, future::try_join_all, stream::FuturesOrdered};
 use ic_agent::AgentError;
 use ic_utils::interfaces::management_canister::builders::EnvironmentVariable;
-use icp::{agent, identity, network};
+use icp::{
+    agent,
+    context::{GetAgentForEnvError, GetCanisterIdForEnvError, GetEnvironmentError},
+    identity, network,
+};
 use tracing::debug;
 
 use icp::context::Context;
@@ -17,8 +21,8 @@ use crate::{
     options::{EnvironmentOpt, IdentityOpt},
     progress::{ProgressManager, ProgressManagerSettings},
 };
-use icp::store_artifact::LookupError as LookupArtifactError;
-use icp::store_id::{Key, LookupError as LookupIdError};
+use icp::store_artifact::LookupArtifactError;
+use icp::store_id::LookupIdError;
 
 #[derive(Clone, Debug, Args)]
 pub(crate) struct BindingArgs {
@@ -40,27 +44,12 @@ pub(crate) enum CommandError {
     #[error(transparent)]
     Identity(#[from] identity::LoadError),
 
-    #[error("project does not contain an environment named '{name}'")]
-    EnvironmentNotFound { name: String },
-
     #[error(transparent)]
     Access(#[from] network::AccessError),
 
     #[error(transparent)]
     Agent(#[from] agent::CreateError),
 
-    #[error("project does not contain a canister named '{name}'")]
-    CanisterNotFound { name: String },
-
-    #[error("no canisters available to install")]
-    NoCanisters,
-
-    #[error("environment '{environment}' does not include canister '{canister}'")]
-    EnvironmentCanister {
-        environment: String,
-        canister: String,
-    },
-
     #[error("Could not find canister id(s) for '{}' in environment '{environment}' make sure they are created first", canister_names.join(", "))]
     CanisterNotCreated {
         environment: String,
@@ -75,66 +64,43 @@ pub(crate) enum CommandError {
 
     #[error(transparent)]
     InstallAgent(#[from] AgentError),
-}
 
-pub(crate) async fn exec(ctx: &Context, args: &BindingArgs) -> Result<(), CommandError> {
-    // Load the project
-    let p = ctx.project.load().await?;
+    #[error(transparent)]
+    GetAgentForEnv(#[from] GetAgentForEnvError),
 
-    // Load identity
-    let id = ctx.identity.load(args.identity.clone().into()).await?;
+    #[error(transparent)]
+    GetEnvironment(#[from] GetEnvironmentError),
 
-    // Load target environment
-    let env =
-        p.environments
-            .get(args.environment.name())
-            .ok_or(CommandError::EnvironmentNotFound {
-                name: args.environment.name().to_owned(),
-            })?;
+    #[error(transparent)]
+    GetCanisterIdForEnv(#[from] GetCanisterIdForEnvError),
+}
 
-    // Access network
-    let access = ctx.network.access(&env.network).await?;
+pub(crate) async fn exec(ctx: &Context, args: &BindingArgs) -> Result<(), CommandError> {
+    // Load target environment
+    let env = ctx.get_environment(args.environment.name()).await?;
 
     // Agent
-    let agent = ctx.agent.create(id, &access.url).await?;
-
-    if let Some(k) = access.root_key {
-        agent.set_root_key(k);
-    }
+    let agent = ctx
+        .get_agent_for_env(&args.identity.clone().into(), args.environment.name())
+        .await?;
 
-    let cnames = match args.names.is_empty() {
-        // No canisters specified
-        true => env.canisters.keys().cloned().collect(),
-
-        // Individual canisters specified
+    let target_canisters = match args.names.is_empty() {
+        true => env.get_canister_names(),
         false => args.names.clone(),
     };
 
-    for name in &cnames {
-        if !p.canisters.contains_key(name) {
-            return Err(CommandError::CanisterNotFound {
-                name: name.to_owned(),
-            });
-        }
-
-        if !env.canisters.contains_key(name) {
-            return Err(CommandError::EnvironmentCanister {
-                environment: env.name.to_owned(),
-                canister: name.to_owned(),
-            });
+    let env_canisters = &env.canisters;
+    let canisters = try_join_all(target_canisters.into_iter().map(|name| {
+        let env_name = args.environment.name();
+        async move {
+            let cid = ctx.get_canister_id_for_env(&name, env_name).await?;
+            let (_, info) = env_canisters
+                .get(&name)
+                .expect("Canister id exists but no canister info");
+            Ok::<_, CommandError>((name, cid, info))
         }
-    }
-
-    let cs = env
-        .canisters
-        .iter()
-        .filter(|(k, _)| cnames.contains(k))
-        .collect::<HashMap<_, _>>();
-
-    // Ensure at least one canister has been selected
-    if cs.is_empty() {
-        return Err(CommandError::NoCanisters);
-    }
+    }))
+    .await?;
 
     // Management Interface
     let mgmt = ic_utils::interfaces::ManagementCanister::create(&agent);
@@ -179,9 +145,9 @@ pub(crate) async fn exec(ctx: &Context, args: &BindingArgs) -> Result<(), Comman
         .map(|(n, p)| (format!("PUBLIC_CANISTER_ID:{n}"), p.to_text()))
         .collect::<Vec<(_, _)>>();
 
-    for (_, (_, c)) in cs {
+    for (name, cid, info) in canisters {
         // Create progress bar with standard configuration
-        let pb = progress_manager.create_progress_bar(&c.name);
+        let pb = progress_manager.create_progress_bar(&name);
 
         // Create an async closure that handles the operation for this specific canister
         let settings_fn = {
@@ -193,15 +159,8 @@ pub(crate) async fn exec(ctx: &Context, args: &BindingArgs) -> Result<(), Comman
                 // Indicate to user that the canister's environment variables are being set
                 pb.set_message("Updating environment variables...");
 
-                // Lookup the canister id
-                let cid = ctx.ids.lookup(&Key {
-                    network: env.network.name.to_owned(),
-                    environment: env.name.to_owned(),
-                    canister: c.name.to_owned(),
-                })?;
-
                 // Load the variables from the config files
-                let mut environment_variables = c
+                let mut environment_variables = info
                     .settings
                     .environment_variables
                     .to_owned()

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

@@ -5,7 +5,12 @@ use candid::{Decode, Encode, Nat};
 use clap::Args;
 use futures::{StreamExt, stream::FuturesOrdered};
 use ic_agent::{Agent, AgentError, export::Principal};
-use icp::{agent, identity, network, prelude::*};
+use icp::{
+    agent,
+    context::{GetAgentForEnvError, GetEnvironmentError},
+    identity, network,
+    prelude::*,
+};
 use icp_canister_interfaces::{
     cycles_ledger::{
         CYCLES_LEDGER_PRINCIPAL, CanisterSettingsArg, CreateCanisterArgs, CreateCanisterResponse,
@@ -22,7 +27,7 @@ use crate::{
     options::{EnvironmentOpt, IdentityOpt},
     progress::{ProgressManager, ProgressManagerSettings},
 };
-use icp::store_id::{Key, LookupError, RegisterError};
+use icp::store_id::{Key, LookupIdError, RegisterError};
 
 pub(crate) const DEFAULT_CANISTER_CYCLES: u128 = 2 * TRILLION;
 
@@ -85,9 +90,6 @@ pub(crate) enum CommandError {
     #[error(transparent)]
     Identity(#[from] identity::LoadError),
 
-    #[error("project does not contain an environment named '{name}'")]
-    EnvironmentNotFound { name: String },
-
     #[error(transparent)]
     Access(#[from] network::AccessError),
 
@@ -103,9 +105,6 @@ pub(crate) enum CommandError {
         canister: String,
     },
 
-    #[error("no canisters available to create")]
-    NoCanisters,
-
     #[error("canister exists already: {principal}")]
     CanisterExists { principal: Principal },
 
@@ -126,6 +125,12 @@ pub(crate) enum CommandError {
 
     #[error(transparent)]
     Unexpected(#[from] anyhow::Error),
+
+    #[error(transparent)]
+    GetAgentForEnv(#[from] GetAgentForEnvError),
+
+    #[error(transparent)]
+    GetEnvironment(#[from] GetEnvironmentError),
 }
 
 // Creates canister(s) by asking the cycles ledger to create them.
@@ -135,27 +140,15 @@ pub(crate) async fn exec(ctx: &Context, args: &CreateArgs) -> Result<(), Command
     // Load project
     let p = ctx.project.load().await?;
 
-    // Load identity
-    let id = ctx.identity.load(args.identity.clone().into()).await?;
-
     // Load target environment
-    let env =
-        p.environments
-            .get(args.environment.name())
-            .ok_or(CommandError::EnvironmentNotFound {
-                name: args.environment.name().to_owned(),
-            })?;
-
-    // Collect environment canisters
-    let cnames = match args.names.is_empty() {
-        // No canisters specified
-        true => env.canisters.keys().cloned().collect(),
-
-        // Individual canisters specified
+    let env = ctx.get_environment(args.environment.name()).await?;
+
+    let target_canisters = match args.names.is_empty() {
+        true => env.get_canister_names(),
         false => args.names.clone(),
     };
 
-    for name in &cnames {
+    for name in &target_canisters {
         if !p.canisters.contains_key(name) {
             return Err(CommandError::CanisterNotFound {
                 name: name.to_owned(),
@@ -170,17 +163,12 @@ pub(crate) async fn exec(ctx: &Context, args: &CreateArgs) -> Result<(), Command
         }
     }
 
-    let cs = env
+    let canister_infos = env
         .canisters
         .iter()
-        .filter(|(k, _)| cnames.contains(k))
+        .filter(|(k, _)| target_canisters.contains(k))
         .collect::<HashMap<_, _>>();
 
-    // Ensure at least one canister has been selected
-    if cs.is_empty() {
-        return Err(CommandError::NoCanisters);
-    }
-
     // Do we have any already existing canisters?
     let cexist: Vec<_> = env
         .canisters
@@ -196,15 +184,10 @@ pub(crate) async fn exec(ctx: &Context, args: &CreateArgs) -> Result<(), Command
         })
         .collect();
 
-    // Access network
-    let access = ctx.network.access(&env.network).await?;
-
     // Agent
-    let agent = ctx.agent.create(id, &access.url).await?;
-
-    if let Some(k) = access.root_key {
-        agent.set_root_key(k);
-    }
+    let agent = ctx
+        .get_agent_for_env(&args.identity.clone().into(), args.environment.name())
+        .await?;
 
     // Select which subnet to deploy the canisters to
     //
@@ -241,9 +224,10 @@ pub(crate) async fn exec(ctx: &Context, args: &CreateArgs) -> Result<(), Command
 
     let progress_manager = ProgressManager::new(ProgressManagerSettings { hidden: ctx.debug });
 
-    for (_, c) in cs.values() {
+    let env_ref = &env;
+    for (name, (_path, info)) in canister_infos.iter() {
         // Create progress bar with standard configuration
-        let pb = progress_manager.create_progress_bar(&c.name);
+        let pb = progress_manager.create_progress_bar(name);
 
         // Create an async closure that handles the operation for this specific canister
         let create_fn = {
@@ -257,9 +241,9 @@ pub(crate) async fn exec(ctx: &Context, args: &CreateArgs) -> Result<(), Command
 
                 // Create canister-network association-key
                 let k = Key {
-                    network: env.network.name.to_owned(),
-                    environment: env.name.to_owned(),
-                    canister: c.name.to_owned(),
+                    network: env_ref.network.name.to_owned(),
+                    environment: env_ref.name.to_owned(),
+                    canister: name.to_string(),
                 };
 
                 match ctx.ids.lookup(&k) {
@@ -269,7 +253,7 @@ pub(crate) async fn exec(ctx: &Context, args: &CreateArgs) -> Result<(), Command
                     }
 
                     // Doesn't exist (include)
-                    Err(LookupError::IdNotFound { .. }) => {}
+                    Err(LookupIdError::IdNotFound { .. }) => {}
 
                     // Lookup failed
                     Err(err) => panic!("{err}"),
@@ -280,7 +264,7 @@ pub(crate) async fn exec(ctx: &Context, args: &CreateArgs) -> Result<(), Command
                     freezing_threshold: cmd
                         .settings
                         .freezing_threshold
-                        .or(c.settings.freezing_threshold)
+                        .or(info.settings.freezing_threshold)
                         .map(Nat::from),
 
                     controllers: if cmd.controller.is_empty() {
@@ -292,19 +276,19 @@ pub(crate) async fn exec(ctx: &Context, args: &CreateArgs) -> Result<(), Command
                     reserved_cycles_limit: cmd
                         .settings
                         .reserved_cycles_limit
-                        .or(c.settings.reserved_cycles_limit)
+                        .or(info.settings.reserved_cycles_limit)
                         .map(Nat::from),
 
                     memory_allocation: cmd
                         .settings
                         .memory_allocation
-                        .or(c.settings.memory_allocation)
+                        .or(info.settings.memory_allocation)
                         .map(Nat::from),
 
                     compute_allocation: cmd
                         .settings
                         .compute_allocation
-                        .or(c.settings.compute_allocation)
+                        .or(info.settings.compute_allocation)
                         .map(Nat::from),
                 };