From 5e57d0cbf244256d7f418a262717e36f81e66fee Mon Sep 17 00:00:00 2001 From: Vincent Zhang <118719397+vincent-dfinity@users.noreply.github.com> Date: Thu, 3 Oct 2024 20:16:17 +0800 Subject: [PATCH] feat: Supports the allowed viewer list for the canister log. (#3921) --- CHANGELOG.md | 8 + docs/cli-reference/dfx-canister.mdx | 8 +- docs/dfx-json-schema.json | 29 +++- e2e/tests-dfx/canister_logs.bash | 26 ++++ e2e/tests-dfx/create.bash | 45 ++++++ e2e/tests-dfx/update_settings.bash | 80 ++++++++++ src/dfx-core/src/config/model/dfinity.rs | 12 +- src/dfx/src/commands/canister/create.rs | 19 ++- src/dfx/src/commands/canister/status.rs | 10 +- .../src/commands/canister/update_settings.rs | 76 +++++++--- .../src/lib/canister_logs/log_visibility.rs | 142 ++++++++++++++++++ src/dfx/src/lib/canister_logs/mod.rs | 1 + src/dfx/src/lib/ic_attributes/mod.rs | 12 +- src/dfx/src/lib/mod.rs | 1 + src/dfx/src/util/clap/parsers.rs | 8 + 15 files changed, 439 insertions(+), 38 deletions(-) create mode 100644 src/dfx/src/lib/canister_logs/log_visibility.rs create mode 100644 src/dfx/src/lib/canister_logs/mod.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e48759880..19db6531da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ # UNRELEASED +### feat: Support canister log allowed viewer list + +Added support for the canister log allowed viewer list, enabling specified users to access a canister's logs without needing to be set as the canister's controller. +Valid settings are: +- `--add-log-viewer`, `--remove-log-viewer` and `--set-log-viewer` flags with `dfx canister update-settings` +- `--log-viewer` flag with `dfx canister create` +- `canisters[].initialization_values.log_visibility.allowed_viewers` in `dfx.json` + # 0.24.1 ### feat: More PocketIC flags supported diff --git a/docs/cli-reference/dfx-canister.mdx b/docs/cli-reference/dfx-canister.mdx index 39e606227a..fd306c111c 100644 --- a/docs/cli-reference/dfx-canister.mdx +++ b/docs/cli-reference/dfx-canister.mdx @@ -280,7 +280,8 @@ You can use the following options with the `dfx canister create` command. | `--memory-allocation ` | Specifies how much memory the canister is allowed to use in total. This should be a value in the range [0..12 GiB]. A setting of 0 means the canister will have access to memory on a “best-effort” basis: It will only be charged for the memory it uses, but at any point in time may stop running if it tries to allocate more memory when there isn’t space available on the subnet. | | `--reserved-cycles-limit ` | Specifies the upper limit for the canister's reserved cycles. | | `--wasm-memory-limit ` | Specifies a soft upper limit for the canister's heap memory. | -| `--log-visibility ` | Specifies who is allowed to read the canister's logs. Can be either "controllers" or "public". | +| `--log-viewer ` | Specifies the principal as an allowed viewers. Can be specified more than once. Cannot be used with `--log-visibility`. | +| `--log-visibility ` | Specifies who can read the canister's logs: "controllers" or "public". For custom allowed viewers, use `--log-viewer`. | | `--no-wallet` | Performs the call with the user Identity as the Sender of messages. Bypasses the Wallet canister. Enabled by default. | | `--with-cycles ` | Specifies the initial cycle balance to deposit into the newly created canister. The specified amount needs to take the canister create fee into account. This amount is deducted from the wallet's cycle balance. | | `--specified-id ` | Attempts to create the canister with this Canister ID | @@ -1137,14 +1138,17 @@ You can specify the following options for the `dfx canister update-settings` com | Option | Description | |-------------------------------------------|------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| | `--add-controller ` | Add a principal to the list of controllers of the canister. | +| `--add-log-viewer ` | Add a principal to the list of log viewers of the canister. Can be specified more than once to add multiple log viewers. If current log visibility is `public` or `controllers`, it will be changed to the custom allowed viewer list. | | `-c`, `--compute-allocation ` | Specifies the canister's compute allocation. This should be a percent in the range [0..100]. | | `--confirm-very-long-freezing-threshold` | Freezing thresholds above ~1.5 years require this option as confirmation. | | `--set-controller ` | Specifies the identity name or the principal of the new controller. Can be specified more than once, indicating the canister will have multiple controllers. If any controllers are set with this parameter, any other controllers will be removed. | +| `--set-log-viewer ` | Specifies the the principal of the log viewer of the canister. Can be specified more than once, indicating the canister will have multiple log viewers. If any log viewers are set with this parameter, any other log viewers will be removed. If current log visibility is `public` or `controllers`, it will be changed to the custom allowed viewer list. | | `--memory-allocation ` | Specifies how much memory the canister is allowed to use in total. This should be a value in the range [0..12 GiB]. A setting of 0 means the canister will have access to memory on a “best-effort” basis: It will only be charged for the memory it uses, but at any point in time may stop running if it tries to allocate more memory when there isn’t space available on the subnet. | | `--reserved-cycles-limit ` | Specifies the upper limit of the canister's reserved cycles. | | `--wasm-memory-limit ` | Specifies a soft upper limit for the canister's heap memory. | -| `--log-visibility ` | Specifies who is allowed to read the canister's logs. Can be either "controllers" or "public". | +| `--log-visibility ` | Specifies who is allowed to read the canister's logs. Can be either "controllers" or "public". For custom allowed viewers, use `--set-log-viewer` or `--add-log-viewer`. | | `--remove-controller ` | Removes a principal from the list of controllers of the canister. | +| `--remove-log-viewer ` | Removes a principal from the list of log viewers of the canister. Can be specified more than once to remove multiple log viewers. | | `--freezing-threshold ` | Set the [freezing threshold](https://internetcomputer.org/docs/current/references/ic-interface-spec/#ic-create_canister) in seconds for a canister. This should be a value in the range [0..2^64^-1]. Very long thresholds require the `--confirm-very-long-freezing-threshold` option. | | `-y`, `--yes` | Skips yes/no checks by answering 'yes'. Such checks can result in loss of control, so this is not recommended outside of CI. | diff --git a/docs/dfx-json-schema.json b/docs/dfx-json-schema.json index ec157b6e98..3e7240d3a4 100644 --- a/docs/dfx-json-schema.json +++ b/docs/dfx-json-schema.json @@ -137,10 +137,29 @@ } }, "CanisterLogVisibility": { - "type": "string", - "enum": [ - "controllers", - "public" + "oneOf": [ + { + "type": "string", + "enum": [ + "controllers", + "public" + ] + }, + { + "type": "object", + "required": [ + "allowed_viewers" + ], + "properties": { + "allowed_viewers": { + "type": "array", + "items": { + "type": "string" + } + } + }, + "additionalProperties": false + } ] }, "CanisterMetadataSection": { @@ -947,7 +966,7 @@ }, "log_visibility": { "title": "Log Visibility", - "description": "Specifies who is allowed to read the canister's logs.\n\nCan be \"public\" or \"controllers\".", + "description": "Specifies who is allowed to read the canister's logs.\n\nCan be \"public\", \"controllers\" or \"allowed_viewers\" with a list of principals.", "anyOf": [ { "$ref": "#/definitions/CanisterLogVisibility" diff --git a/e2e/tests-dfx/canister_logs.bash b/e2e/tests-dfx/canister_logs.bash index 27f6de53c6..367d10fcae 100644 --- a/e2e/tests-dfx/canister_logs.bash +++ b/e2e/tests-dfx/canister_logs.bash @@ -65,3 +65,29 @@ dfx_canister_logs_tail_n_1() { assert_not_contains "Alice" assert_contains "Bob" } + +@test "canister logs only visible to allowed viewers." { + install_asset logs + dfx_start + dfx canister create --all + dfx build + dfx canister install e2e_project + dfx canister call e2e_project hello Alice + sleep 2 + + assert_command dfx canister logs e2e_project + assert_contains "Hello, Alice!" + + # Create identity for viewers. + assert_command dfx identity new --storage-mode plaintext alice + ALICE_PRINCIPAL=$(dfx identity get-principal --identity alice) + + assert_command_fail dfx canister logs e2e_project --identity alice + + assert_command dfx canister update-settings --add-log-viewer="${ALICE_PRINCIPAL}" e2e_project + assert_command dfx canister status e2e_project + assert_contains "${ALICE_PRINCIPAL}" + + assert_command dfx canister logs e2e_project --identity alice + assert_contains "Hello, Alice!" +} diff --git a/e2e/tests-dfx/create.bash b/e2e/tests-dfx/create.bash index 9c15e48f77..58f54097c9 100644 --- a/e2e/tests-dfx/create.bash +++ b/e2e/tests-dfx/create.bash @@ -352,3 +352,48 @@ teardown() { assert_contains 'Freezing threshold: 2_592_000' assert_contains 'Log visibility: controllers' } + +@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 + ALICE_PRINCIPAL=$(dfx identity get-principal --identity alice) + BOB_PRINCIPAL=$(dfx identity get-principal --identity bob) + + jq '.canisters.e2e_project_backend.initialization_values={ + "compute_allocation": 5, + "freezing_threshold": "7days", + "memory_allocation": "2 GiB", + "reserved_cycles_limit": 1000000000000, + "wasm_memory_limit": "1 GiB", + "log_visibility": { + "allowed_viewers" : + ['\""$ALICE_PRINCIPAL"\"', '\""$BOB_PRINCIPAL"\"'] + } + }' dfx.json | sponge dfx.json + dfx_start + assert_command dfx deploy e2e_project_backend --no-wallet + assert_command dfx canister status e2e_project_backend + assert_contains 'Memory allocation: 2_147_483_648' + assert_contains 'Compute allocation: 5' + assert_contains 'Reserved cycles limit: 1_000_000_000_000' + assert_contains 'Wasm memory limit: 1_073_741_824' + assert_contains 'Freezing threshold: 604_800' + 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/e2e/tests-dfx/update_settings.bash b/e2e/tests-dfx/update_settings.bash index 63af5cb86f..74d668cf61 100644 --- a/e2e/tests-dfx/update_settings.bash +++ b/e2e/tests-dfx/update_settings.bash @@ -66,6 +66,8 @@ teardown() { dfx_start assert_command dfx deploy e2e_project_backend assert_command dfx canister status e2e_project_backend + + # Test against a single canister. assert_contains "Log visibility: controllers" assert_command dfx canister update-settings e2e_project_backend --log-visibility public assert_command dfx canister status e2e_project_backend @@ -73,6 +75,84 @@ teardown() { assert_command dfx canister update-settings e2e_project_backend --log-visibility controllers assert_command dfx canister status e2e_project_backend assert_contains "Log visibility: controllers" + + # Test --all code path. + assert_command dfx canister update-settings --log-visibility public --all + assert_command dfx canister status e2e_project_backend + assert_contains "Log visibility: public" + assert_command dfx canister update-settings --log-visibility controllers --all + assert_command dfx canister status e2e_project_backend + assert_contains "Log visibility: controllers" +} + +@test "update 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_new + dfx_start + assert_command dfx deploy e2e_project_backend + assert_command dfx canister status e2e_project_backend + assert_contains "Log visibility: controllers" + + # Test against a single canister. + assert_command dfx canister update-settings --add-log-viewer="${ALICE_PRINCIPAL}" e2e_project_backend + assert_command dfx canister status e2e_project_backend + assert_contains "${ALICE_PRINCIPAL}" + + assert_command dfx canister update-settings --remove-log-viewer="${BOB_PRINCIPAL}" e2e_project_backend + assert_contains "'${BOB_PRINCIPAL}' is not in the allowed list" + + assert_command dfx canister update-settings --add-log-viewer="${BOB_PRINCIPAL}" --remove-log-viewer="${ALICE_PRINCIPAL}" e2e_project_backend + assert_command dfx canister status e2e_project_backend + assert_contains "${BOB_PRINCIPAL}" + assert_not_contains "${ALICE_PRINCIPAL}" + + assert_command dfx canister update-settings --set-log-viewer="${ALICE_PRINCIPAL}" e2e_project_backend + assert_command dfx canister status e2e_project_backend + assert_contains "${ALICE_PRINCIPAL}" + assert_not_contains "${BOB_PRINCIPAL}" + + assert_command dfx canister update-settings --remove-log-viewer="${ALICE_PRINCIPAL}" e2e_project_backend + assert_command dfx canister status e2e_project_backend + assert_contains "allowed viewers list is empty" + + assert_command dfx canister update-settings --add-log-viewer="${BOB_PRINCIPAL}" --add-log-viewer="${ALICE_PRINCIPAL}" e2e_project_backend + assert_command dfx canister status e2e_project_backend + assert_contains "${ALICE_PRINCIPAL}" + assert_contains "${BOB_PRINCIPAL}" + + assert_command dfx canister update-settings --remove-log-viewer="${ALICE_PRINCIPAL}" --remove-log-viewer="${BOB_PRINCIPAL}" e2e_project_backend + assert_command dfx canister status e2e_project_backend + assert_contains "allowed viewers list is empty" + + assert_command dfx canister update-settings --set-log-viewer="${BOB_PRINCIPAL}" --set-log-viewer="${ALICE_PRINCIPAL}" e2e_project_backend + assert_command dfx canister status e2e_project_backend + assert_contains "${ALICE_PRINCIPAL}" + assert_contains "${BOB_PRINCIPAL}" + + assert_command dfx canister update-settings --log-visibility controllers e2e_project_backend + assert_command dfx canister status e2e_project_backend + assert_contains "Log visibility: controllers" + + assert_command_fail dfx canister update-settings --remove-log-viewer="${BOB_PRINCIPAL}" e2e_project_backend + assert_contains "Removing reviewers is not allowed with 'public' or 'controllers' log visibility." + + # Test --all code path. + assert_command dfx canister update-settings --add-log-viewer="${ALICE_PRINCIPAL}" --all + assert_command dfx canister status e2e_project_backend + assert_contains "${ALICE_PRINCIPAL}" + + assert_command dfx canister update-settings --remove-log-viewer="${ALICE_PRINCIPAL}" --all + assert_command dfx canister status e2e_project_backend + assert_contains "allowed viewers list is empty" + + assert_command dfx canister update-settings --set-log-viewer="${ALICE_PRINCIPAL}" --all + assert_command dfx canister status e2e_project_backend + assert_contains "${ALICE_PRINCIPAL}" } @test "set controller" { diff --git a/src/dfx-core/src/config/model/dfinity.rs b/src/dfx-core/src/config/model/dfinity.rs index 6fe142b1f6..e26edada97 100644 --- a/src/dfx-core/src/config/model/dfinity.rs +++ b/src/dfx-core/src/config/model/dfinity.rs @@ -407,12 +407,14 @@ impl CanisterTypeProperties { } } -#[derive(Copy, Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] -#[serde(rename_all = "lowercase")] +#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)] +#[serde(rename_all = "snake_case")] pub enum CanisterLogVisibility { #[default] Controllers, Public, + #[schemars(with = "Vec::")] + AllowedViewers(Vec), } impl From for LogVisibility { @@ -420,6 +422,9 @@ impl From for LogVisibility { match value { CanisterLogVisibility::Controllers => LogVisibility::Controllers, CanisterLogVisibility::Public => LogVisibility::Public, + CanisterLogVisibility::AllowedViewers(viewers) => { + LogVisibility::AllowedViewers(viewers) + } } } } @@ -475,7 +480,7 @@ pub struct InitializationValues { /// # Log Visibility /// Specifies who is allowed to read the canister's logs. /// - /// Can be "public" or "controllers". + /// Can be "public", "controllers" or "allowed_viewers" with a list of principals. #[schemars(with = "Option")] pub log_visibility: Option, } @@ -1008,6 +1013,7 @@ impl ConfigInterface { .map_err(|e| GetLogVisibilityFailed(canister_name.to_string(), e))? .initialization_values .log_visibility + .clone() .map(|visibility| visibility.into())) } diff --git a/src/dfx/src/commands/canister/create.rs b/src/dfx/src/commands/canister/create.rs index f949ef5fa5..a5293f0f1c 100644 --- a/src/dfx/src/commands/canister/create.rs +++ b/src/dfx/src/commands/canister/create.rs @@ -1,3 +1,4 @@ +use crate::lib::canister_logs::log_visibility::LogVisibilityOpt; use crate::lib::deps::get_pull_canisters_in_config; use crate::lib::environment::Environment; use crate::lib::error::{DfxError, DfxResult}; @@ -9,7 +10,8 @@ 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, log_visibility_parser, - memory_allocation_parser, reserved_cycles_limit_parser, wasm_memory_limit_parser, + memory_allocation_parser, principal_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; @@ -92,9 +94,14 @@ pub struct CanisterCreateOpts { /// Specifies who is allowed to read the canister's logs. /// Can be either "controllers" or "public". - #[arg(long, value_parser = log_visibility_parser)] + #[arg(long, value_parser = log_visibility_parser, conflicts_with("log_viewer"))] log_visibility: Option, + /// Specifies the the principal of the log viewer of the canister. + /// Can be specified more than once. + #[arg(long, action = ArgAction::Append, value_parser = principal_parser, conflicts_with("log_visibility"))] + log_viewer: Option>, + /// Performs the call with the user Identity as the Sender of messages. /// Bypasses the Wallet canister. #[arg(long)] @@ -202,7 +209,9 @@ pub async fn exec( ) .with_context(|| format!("Failed to read Wasm memory limit of {canister_name}."))?; let log_visibility = get_log_visibility( - opts.log_visibility, + env, + LogVisibilityOpt::from(&opts.log_visibility, &opts.log_viewer).as_ref(), + None, Some(config_interface), Some(canister_name), ) @@ -285,7 +294,9 @@ pub async fn exec( ) .with_context(|| format!("Failed to read Wasm memory limit of {canister_name}."))?; let log_visibility = get_log_visibility( - opts.log_visibility.clone(), + env, + LogVisibilityOpt::from(&opts.log_visibility, &opts.log_viewer).as_ref(), + None, Some(config_interface), Some(canister_name), ) diff --git a/src/dfx/src/commands/canister/status.rs b/src/dfx/src/commands/canister/status.rs index 8bac6b2eb8..2270d2e6ee 100644 --- a/src/dfx/src/commands/canister/status.rs +++ b/src/dfx/src/commands/canister/status.rs @@ -55,9 +55,13 @@ async fn canister_status( LogVisibility::Controllers => "controllers".to_string(), LogVisibility::Public => "public".to_string(), LogVisibility::AllowedViewers(viewers) => { - let mut viewers: Vec<_> = viewers.iter().map(Principal::to_text).collect(); - viewers.sort(); - format!("allowed viewers: {}", viewers.join(", ")) + if viewers.is_empty() { + "allowed viewers list is empty".to_string() + } else { + let mut viewers: Vec<_> = viewers.iter().map(Principal::to_text).collect(); + viewers.sort(); + format!("allowed viewers: {}", viewers.join(", ")) + } } }; diff --git a/src/dfx/src/commands/canister/update_settings.rs b/src/dfx/src/commands/canister/update_settings.rs index 8367e6ff39..f6867c58af 100644 --- a/src/dfx/src/commands/canister/update_settings.rs +++ b/src/dfx/src/commands/canister/update_settings.rs @@ -1,3 +1,4 @@ +use crate::lib::canister_logs::log_visibility::LogVisibilityOpt; use crate::lib::diagnosis::DiagnosedError; use crate::lib::environment::Environment; use crate::lib::error::{DfxError, DfxResult}; @@ -8,8 +9,8 @@ use crate::lib::ic_attributes::{ use crate::lib::operations::canister::{get_canister_status, update_settings}; use crate::lib::root_key::fetch_root_key_if_needed; use crate::util::clap::parsers::{ - compute_allocation_parser, freezing_threshold_parser, log_visibility_parser, - memory_allocation_parser, reserved_cycles_limit_parser, wasm_memory_limit_parser, + compute_allocation_parser, freezing_threshold_parser, memory_allocation_parser, + reserved_cycles_limit_parser, wasm_memory_limit_parser, }; use anyhow::{bail, Context}; use byte_unit::Byte; @@ -20,7 +21,7 @@ use dfx_core::error::identity::InstantiateIdentityFromNameError::GetIdentityPrin use dfx_core::identity::CallSender; use fn_error_context::context; use ic_agent::identity::Identity; -use ic_utils::interfaces::management_canister::LogVisibility; +use ic_utils::interfaces::management_canister::StatusCallResult; /// Update one or more of a canister's settings (i.e its controller, compute allocation, or memory allocation.) #[derive(Parser, Debug)] @@ -88,10 +89,8 @@ pub struct UpdateSettingsOpts { #[arg(long, value_parser = wasm_memory_limit_parser)] wasm_memory_limit: Option, - /// Specifies who is allowed to read the canister's logs. - /// Can be either "controllers" or "public". - #[arg(long, value_parser = log_visibility_parser)] - log_visibility: Option, + #[command(flatten)] + log_visibility_opt: Option, /// Freezing thresholds above ~1.5 years require this flag as confirmation. #[arg(long)] @@ -157,11 +156,29 @@ pub async fn exec( get_reserved_cycles_limit(opts.reserved_cycles_limit, config_interface, canister_name)?; let wasm_memory_limit = get_wasm_memory_limit(opts.wasm_memory_limit, config_interface, canister_name)?; - let log_visibility = - get_log_visibility(opts.log_visibility.clone(), config_interface, canister_name)?; + let mut current_status: Option = None; + if let Some(log_visibility) = &opts.log_visibility_opt { + if log_visibility.require_current_settings() { + current_status = Some(get_canister_status(env, canister_id, call_sender).await?); + } + } + let log_visibility = get_log_visibility( + env, + opts.log_visibility_opt.as_ref(), + current_status.as_ref(), + config_interface, + canister_name, + )?; if let Some(added) = &opts.add_controller { - let status = get_canister_status(env, canister_id, call_sender).await?; - let mut existing_controllers = status.settings.controllers; + if current_status.is_none() { + current_status = Some(get_canister_status(env, canister_id, call_sender).await?); + } + let mut existing_controllers = current_status + .as_ref() + .unwrap() + .settings + .controllers + .clone(); for s in added { existing_controllers.push(controller_to_principal(env, s)?); } @@ -171,8 +188,11 @@ pub async fn exec( let controllers = if opts.add_controller.is_some() { controllers.as_mut().unwrap() } else { - let status = get_canister_status(env, canister_id, call_sender).await?; - controllers.get_or_insert(status.settings.controllers) + if current_status.is_none() { + current_status = + Some(get_canister_status(env, canister_id, call_sender).await?); + } + controllers.get_or_insert(current_status.unwrap().settings.controllers) }; let removed = removed .iter() @@ -240,15 +260,32 @@ pub async fn exec( Some(canister_name), ) .with_context(|| format!("Failed to get Wasm memory limit for {canister_name}."))?; + let mut current_status: Option = None; + if let Some(log_visibility) = &opts.log_visibility_opt { + if log_visibility.require_current_settings() { + current_status = + Some(get_canister_status(env, canister_id, call_sender).await?); + } + } let log_visibility = get_log_visibility( - opts.log_visibility.clone(), + env, + opts.log_visibility_opt.as_ref(), + current_status.as_ref(), Some(config_interface), Some(canister_name), ) .with_context(|| format!("Failed to get log visibility for {canister_name}."))?; if let Some(added) = &opts.add_controller { - let status = get_canister_status(env, canister_id, call_sender).await?; - let mut existing_controllers = status.settings.controllers; + if current_status.is_none() { + current_status = + Some(get_canister_status(env, canister_id, call_sender).await?); + } + let mut existing_controllers = current_status + .as_ref() + .unwrap() + .settings + .controllers + .clone(); for s in added { existing_controllers.push(controller_to_principal(env, s)?); } @@ -258,8 +295,11 @@ pub async fn exec( let controllers = if opts.add_controller.is_some() { controllers.as_mut().unwrap() } else { - let status = get_canister_status(env, canister_id, call_sender).await?; - controllers.get_or_insert(status.settings.controllers) + if current_status.is_none() { + current_status = + Some(get_canister_status(env, canister_id, call_sender).await?); + } + controllers.get_or_insert(current_status.unwrap().settings.controllers) }; let removed = removed .iter() diff --git a/src/dfx/src/lib/canister_logs/log_visibility.rs b/src/dfx/src/lib/canister_logs/log_visibility.rs new file mode 100644 index 0000000000..02e1829cb7 --- /dev/null +++ b/src/dfx/src/lib/canister_logs/log_visibility.rs @@ -0,0 +1,142 @@ +use crate::lib::environment::Environment; +use crate::lib::error::DfxResult; +use crate::util::clap::parsers::{log_visibility_parser, principal_parser}; +use anyhow::anyhow; +use candid::Principal; +use clap::{ArgAction, Args}; +use dfx_core::cli::ask_for_consent; +use ic_utils::interfaces::management_canister::{LogVisibility, StatusCallResult}; + +#[derive(Args, Clone, Debug, Default)] +pub struct 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("add_log_viewer"), + conflicts_with("remove_log_viewer"), + conflicts_with("set_log_viewer"), + )] + log_visibility: Option, + + /// Add a principal to the list of log viewers of the canister. + #[arg(long, action = ArgAction::Append, value_parser = principal_parser, conflicts_with("set_log_viewer"))] + add_log_viewer: Option>, + + /// Removes a principal from the list of log viewers of the canister. + #[arg(long, action = ArgAction::Append, value_parser = principal_parser, conflicts_with("set_log_viewer"))] + remove_log_viewer: Option>, + + /// Specifies the the principal of the log viewer of the canister. + /// Can be specified more than once. + #[arg( + long, + action = ArgAction::Append, + value_parser = principal_parser, + conflicts_with("add_log_viewer"), + conflicts_with("remove_log_viewer"), + )] + set_log_viewer: Option>, +} + +impl LogVisibilityOpt { + pub fn require_current_settings(&self) -> bool { + self.add_log_viewer.is_some() || self.remove_log_viewer.is_some() + } + + pub fn from( + log_visibility: &Option, + log_viewer: &Option>, + ) -> Option { + 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, + current_status: Option<&StatusCallResult>, + ) -> DfxResult { + let logger = env.get_logger(); + + // For public and controllers. + if let Some(log_visibility) = self.log_visibility.as_ref() { + return Ok(log_visibility.clone()); + } + + // For setting viewers. + if let Some(principals) = self.set_log_viewer.as_ref() { + return Ok(LogVisibility::AllowedViewers(principals.clone())); + } + + // Get the current viewer list for adding and removing, only for update-settings. + let mut current_visibility: Option = None; + let mut viewers = match current_status { + Some(status) => { + current_visibility = Some(status.settings.log_visibility.clone()); + match &status.settings.log_visibility { + LogVisibility::AllowedViewers(viewers) => viewers.clone(), + _ => vec![], + } + } + None => vec![], + }; + + // Adding. + if let Some(added) = self.add_log_viewer.as_ref() { + if let Some(LogVisibility::Public) = current_visibility { + let msg = "Current log is public to everyone. Adding log reviewers will make the log only visible to the reviewers."; + ask_for_consent(msg)?; + } + + for principal in added { + if !viewers.iter().any(|x| x == principal) { + viewers.push(*principal); + } + } + } + + // Removing. + if let Some(removed) = self.remove_log_viewer.as_ref() { + if let Some(visibility) = ¤t_visibility { + match visibility { + LogVisibility::Public | LogVisibility::Controllers => { + return Err(anyhow!("Removing reviewers is not allowed with 'public' or 'controllers' log visibility.")); + } + _ => (), + } + } + for principal in removed { + if let Some(idx) = viewers.iter().position(|x| x == principal) { + viewers.swap_remove(idx); + } else { + slog::warn!( + logger, + "Principal '{}' is not in the allowed list.", + principal.to_text() + ); + } + } + } + + Ok(LogVisibility::AllowedViewers(viewers)) + } +} diff --git a/src/dfx/src/lib/canister_logs/mod.rs b/src/dfx/src/lib/canister_logs/mod.rs new file mode 100644 index 0000000000..befc0b7994 --- /dev/null +++ b/src/dfx/src/lib/canister_logs/mod.rs @@ -0,0 +1 @@ +pub mod log_visibility; diff --git a/src/dfx/src/lib/ic_attributes/mod.rs b/src/dfx/src/lib/ic_attributes/mod.rs index 7d93589811..a7cf379a02 100644 --- a/src/dfx/src/lib/ic_attributes/mod.rs +++ b/src/dfx/src/lib/ic_attributes/mod.rs @@ -1,3 +1,5 @@ +use crate::lib::canister_logs::log_visibility::LogVisibilityOpt; +use crate::lib::environment::Environment; use crate::lib::error::DfxResult; use anyhow::{anyhow, Context, Error}; use byte_unit::Byte; @@ -7,7 +9,7 @@ use fn_error_context::context; use ic_utils::interfaces::management_canister::{ attributes::{ComputeAllocation, FreezingThreshold, MemoryAllocation, ReservedCyclesLimit}, builders::WasmMemoryLimit, - LogVisibility, + LogVisibility, StatusCallResult, }; use num_traits::ToPrimitive; use std::convert::TryFrom; @@ -218,12 +220,16 @@ pub fn get_wasm_memory_limit( } pub fn get_log_visibility( - log_visibility: Option, + env: &dyn Environment, + log_visibility: Option<&LogVisibilityOpt>, + current_settings: Option<&StatusCallResult>, config_interface: Option<&ConfigInterface>, canister_name: Option<&str>, ) -> DfxResult> { let log_visibility = match (log_visibility, config_interface, canister_name) { - (Some(log_visibility), _, _) => Some(log_visibility), + (Some(log_visibility), _, _) => { + Some(log_visibility.to_log_visibility(env, current_settings)?) + } (None, Some(config_interface), Some(canister_name)) => { config_interface.get_log_visibility(canister_name)? } diff --git a/src/dfx/src/lib/mod.rs b/src/dfx/src/lib/mod.rs index c1d33bc68f..8a93c56690 100644 --- a/src/dfx/src/lib/mod.rs +++ b/src/dfx/src/lib/mod.rs @@ -1,6 +1,7 @@ pub mod agent; pub mod builders; pub mod canister_info; +pub mod canister_logs; pub mod cycles_ledger_types; pub mod deps; pub mod dfxvm; diff --git a/src/dfx/src/util/clap/parsers.rs b/src/dfx/src/util/clap/parsers.rs index 443a599e0e..33f7c8fa21 100644 --- a/src/dfx/src/util/clap/parsers.rs +++ b/src/dfx/src/util/clap/parsers.rs @@ -1,4 +1,5 @@ use byte_unit::{Byte, ByteUnit}; +use candid::Principal; use ic_utils::interfaces::management_canister::LogVisibility; use icrc_ledger_types::icrc1::account::Subaccount; use rust_decimal::Decimal; @@ -137,6 +138,13 @@ pub fn log_visibility_parser(log_visibility: &str) -> Result Result { + match Principal::from_text(principal_text) { + Ok(principal) => Ok(principal), + _ => Err(("Failed to convert to a principal.").to_string()), + } +} + pub fn freezing_threshold_parser(freezing_threshold: &str) -> Result { freezing_threshold .parse::()