From a93a3f9288873a9c5a60cdf065eb7baaf45902af Mon Sep 17 00:00:00 2001
From: Vincent Zhang <vincent.zhang@dfinity.org>
Date: Tue, 24 Sep 2024 17:41:09 +0800
Subject: [PATCH] Add '--log-viewer' argument for creating canister.

---
 e2e/tests-dfx/create.bash                     | 17 ++++++++++++-
 src/dfx/src/commands/canister/create.rs       | 19 +++++++++-----
 .../src/lib/canister_logs/log_visibility.rs   | 25 +++++++++++++++++++
 3 files changed, 54 insertions(+), 7 deletions(-)

diff --git a/e2e/tests-dfx/create.bash b/e2e/tests-dfx/create.bash
index 2befcf99c3..58f54097c9 100644
--- a/e2e/tests-dfx/create.bash
+++ b/e2e/tests-dfx/create.bash
@@ -353,7 +353,7 @@ teardown() {
   assert_contains 'Log visibility: controllers'
 }
 
-@test "create with log allowed viewer list" {
+@test "create with multiple log allowed viewer list in dfx.json" {
   # Create two identities
   assert_command dfx identity new --storage-mode plaintext alice
   assert_command dfx identity new --storage-mode plaintext bob
@@ -382,3 +382,18 @@ teardown() {
   assert_contains "${ALICE_PRINCIPAL}"
   assert_contains "${BOB_PRINCIPAL}"
 }
+
+@test "create with multiple log allowed viewer list" {
+  # Create two identities
+  assert_command dfx identity new --storage-mode plaintext alice
+  assert_command dfx identity new --storage-mode plaintext bob
+  ALICE_PRINCIPAL=$(dfx identity get-principal --identity alice)
+  BOB_PRINCIPAL=$(dfx identity get-principal --identity bob)
+
+  dfx_start
+  assert_command dfx canister create --all --log-viewer "${ALICE_PRINCIPAL}" --log-viewer "${BOB_PRINCIPAL}"  --no-wallet
+  assert_command dfx deploy e2e_project_backend --no-wallet
+  assert_command dfx canister status e2e_project_backend
+  assert_contains "${ALICE_PRINCIPAL}"
+  assert_contains "${BOB_PRINCIPAL}"
+}
diff --git a/src/dfx/src/commands/canister/create.rs b/src/dfx/src/commands/canister/create.rs
index be4284a091..7eb53b75f6 100644
--- a/src/dfx/src/commands/canister/create.rs
+++ b/src/dfx/src/commands/canister/create.rs
@@ -9,8 +9,8 @@ use crate::lib::ic_attributes::{
 use crate::lib::operations::canister::create_canister;
 use crate::lib::root_key::fetch_root_key_if_needed;
 use crate::util::clap::parsers::{
-    compute_allocation_parser, freezing_threshold_parser, memory_allocation_parser,
-    reserved_cycles_limit_parser, wasm_memory_limit_parser,
+    compute_allocation_parser, freezing_threshold_parser, log_visibility_parser,
+    memory_allocation_parser, reserved_cycles_limit_parser, wasm_memory_limit_parser,
 };
 use crate::util::clap::parsers::{cycle_amount_parser, icrc_subaccount_parser};
 use crate::util::clap::subnet_selection_opt::SubnetSelectionOpt;
@@ -21,6 +21,7 @@ use clap::{ArgAction, Parser};
 use dfx_core::error::identity::InstantiateIdentityFromNameError::GetIdentityPrincipalFailed;
 use dfx_core::identity::CallSender;
 use ic_agent::Identity as _;
+use ic_utils::interfaces::management_canister::LogVisibility;
 use icrc_ledger_types::icrc1::account::Subaccount;
 use slog::info;
 
@@ -90,8 +91,14 @@ pub struct CanisterCreateOpts {
     #[arg(long, value_parser = wasm_memory_limit_parser, hide = true)]
     wasm_memory_limit: Option<Byte>,
 
-    #[command(flatten)]
-    log_visibility_opt: Option<LogVisibilityOpt>,
+    /// Specifies who is allowed to read the canister's logs.
+    /// Can be either "controllers" or "public".
+    #[arg(long, value_parser = log_visibility_parser, conflicts_with("log_viewer"))]
+    log_visibility: Option<LogVisibility>,
+
+    /// Specifies the identity name or the principal of the new controller.
+    #[arg(long, action = ArgAction::Append, conflicts_with("log_visibility"))]
+    log_viewer: Option<Vec<String>>,
 
     /// Performs the call with the user Identity as the Sender of messages.
     /// Bypasses the Wallet canister.
@@ -201,7 +208,7 @@ pub async fn exec(
         .with_context(|| format!("Failed to read Wasm memory limit of {canister_name}."))?;
         let log_visibility = get_log_visibility(
             env,
-            opts.log_visibility_opt.as_ref(),
+            LogVisibilityOpt::from(&opts.log_visibility, &opts.log_viewer).as_ref(),
             None,
             Some(config_interface),
             Some(canister_name),
@@ -286,7 +293,7 @@ pub async fn exec(
                 .with_context(|| format!("Failed to read Wasm memory limit of {canister_name}."))?;
                 let log_visibility = get_log_visibility(
                     env,
-                    opts.log_visibility_opt.as_ref(),
+                    LogVisibilityOpt::from(&opts.log_visibility, &opts.log_viewer).as_ref(),
                     None,
                     Some(config_interface),
                     Some(canister_name),
diff --git a/src/dfx/src/lib/canister_logs/log_visibility.rs b/src/dfx/src/lib/canister_logs/log_visibility.rs
index a0d8befdfb..538ee6b07e 100644
--- a/src/dfx/src/lib/canister_logs/log_visibility.rs
+++ b/src/dfx/src/lib/canister_logs/log_visibility.rs
@@ -44,6 +44,31 @@ impl LogVisibilityOpt {
         self.add_log_viewer.is_some() || self.remove_log_viewer.is_some()
     }
 
+    pub fn from(
+        log_visibility: &Option<LogVisibility>,
+        log_viewer: &Option<Vec<String>>,
+    ) -> Option<LogVisibilityOpt> {
+        if let Some(log_visibility) = log_visibility {
+            return Some(LogVisibilityOpt {
+                log_visibility: Some(log_visibility.clone()),
+                add_log_viewer: None,
+                remove_log_viewer: None,
+                set_log_viewer: None,
+            });
+        }
+
+        if let Some(log_viewer) = log_viewer {
+            return Some(LogVisibilityOpt {
+                log_visibility: None,
+                add_log_viewer: None,
+                remove_log_viewer: None,
+                set_log_viewer: Some(log_viewer.clone()),
+            });
+        }
+
+        None
+    }
+
     pub fn to_log_visibility(
         &self,
         env: &dyn Environment,