Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Supports the allowed viewer list for the canister log. #3921

Merged
merged 24 commits into from
Oct 3, 2024
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
ec25f11
Add 'LogVisibilityOpt' to support viewer list from the command line.
vincent-dfinity Sep 19, 2024
9e05b27
Support add/set/remove log viewers.
vincent-dfinity Sep 19, 2024
103df8c
Removed unused code.
vincent-dfinity Sep 19, 2024
6372aeb
Fixed errors from running Lint.
vincent-dfinity Sep 20, 2024
faad6b3
Add dfx.json support.
vincent-dfinity Sep 20, 2024
5a0deb6
Addressed review comments.
vincent-dfinity Sep 20, 2024
dd7d9d7
Add warnings and ask for consent.
vincent-dfinity Sep 20, 2024
19495f2
Allow an empty allowed viewer list.
vincent-dfinity Sep 20, 2024
b0e2155
Merge branch 'master' into vincent/SDK-1798
vincent-dfinity Sep 22, 2024
1286e76
Add tests for log allowed viewer list, with the tests disabled for now.
vincent-dfinity Sep 23, 2024
284f009
Output error or warning without panic.
vincent-dfinity Sep 23, 2024
2cea35b
Add more tests.
vincent-dfinity Sep 24, 2024
e44c7c2
Update replica revision to include the log allowed viewer feature, an…
vincent-dfinity Sep 24, 2024
af477d3
Merge branch 'master' into vincent/SDK-1798
vincent-dfinity Sep 24, 2024
cb345d8
Improve the the allowed viewer output.
vincent-dfinity Sep 24, 2024
a93a3f9
Add '--log-viewer' argument for creating canister.
vincent-dfinity Sep 24, 2024
a685c3f
Add document and changelog
vincent-dfinity Sep 24, 2024
201e2b7
Address the review comments.
vincent-dfinity Sep 24, 2024
0e5e10c
Merge branch 'master' into vincent/SDK-1798
vincent-dfinity Sep 27, 2024
24588a0
Merge branch 'master' into vincent/SDK-1798
vincent-dfinity Oct 1, 2024
07b2ce4
Addressed review comments, mainly about reducing the usage of unwrap.
vincent-dfinity Oct 1, 2024
78871f8
Merge branch 'master' into vincent/SDK-1798
vincent-dfinity Oct 1, 2024
382b6bb
Merge branch 'master' into vincent/SDK-1798
sesi200 Oct 3, 2024
b2aa0ce
move changelog to unreleased header
sesi200 Oct 3, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,11 @@ license = "Apache-2.0"
[workspace.dependencies]
candid = "0.10.4"
candid_parser = "0.1.4"
ic-agent = { git = "https://github.com/dfinity/agent-rs.git", rev = "6e11a350112f9b907c4d590d8217f340e153d898" }
ic-agent = { git = "https://github.com/dfinity/agent-rs.git", rev = "c19d1b6c8e590bffb88444968e401fd15ded0c34" }
ic-asset = { path = "src/canisters/frontend/ic-asset" }
ic-cdk = "0.13.1"
ic-identity-hsm = { git = "https://github.com/dfinity/agent-rs.git", rev = "6e11a350112f9b907c4d590d8217f340e153d898" }
ic-utils = { git = "https://github.com/dfinity/agent-rs.git", rev = "6e11a350112f9b907c4d590d8217f340e153d898" }
ic-identity-hsm = { git = "https://github.com/dfinity/agent-rs.git", rev = "c19d1b6c8e590bffb88444968e401fd15ded0c34" }
ic-utils = { git = "https://github.com/dfinity/agent-rs.git", rev = "c19d1b6c8e590bffb88444968e401fd15ded0c34" }

aes-gcm = "0.10.3"
anyhow = "1.0.56"
Expand Down
29 changes: 24 additions & 5 deletions docs/dfx-json-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -137,10 +137,29 @@
}
},
"CanisterLogVisibility": {
"type": "string",
"enum": [
"controllers",
"public"
"oneOf": [
{
"type": "string",
"enum": [
"controllers",
"public"
]
},
{
"type": "object",
"required": [
"allowedviewers"
],
"properties": {
"allowedviewers": {
"type": "array",
"items": {
"type": "string"
}
}
},
"additionalProperties": false
}
]
},
"CanisterMetadataSection": {
Expand Down Expand Up @@ -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 \"allowedviewers\" with a list of Principals.",
"anyOf": [
{
"$ref": "#/definitions/CanisterLogVisibility"
Expand Down
13 changes: 11 additions & 2 deletions src/dfx-core/src/config/model/dfinity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -407,19 +407,27 @@ impl CanisterTypeProperties {
}
}

#[derive(Copy, Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "lowercase")]
vincent-dfinity marked this conversation as resolved.
Show resolved Hide resolved
pub enum CanisterLogVisibility {
#[default]
Controllers,
Public,
AllowedViewers(Vec<String>),
vincent-dfinity marked this conversation as resolved.
Show resolved Hide resolved
}

impl From<CanisterLogVisibility> for LogVisibility {
fn from(value: CanisterLogVisibility) -> Self {
match value {
CanisterLogVisibility::Controllers => LogVisibility::Controllers,
CanisterLogVisibility::Public => LogVisibility::Public,
CanisterLogVisibility::AllowedViewers(viewers) => {
let principals: Vec<_> = viewers
.iter()
.map(|v| Principal::from_text(v).unwrap())
.collect();
LogVisibility::AllowedViewers(principals)
}
}
}
}
Expand Down Expand Up @@ -475,7 +483,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 "allowedviewers" with a list of Principals.
#[schemars(with = "Option<CanisterLogVisibility>")]
pub log_visibility: Option<CanisterLogVisibility>,
}
Expand Down Expand Up @@ -1008,6 +1016,7 @@ impl ConfigInterface {
.map_err(|e| GetLogVisibilityFailed(canister_name.to_string(), e))?
.initialization_values
.log_visibility
.clone()
.map(|visibility| visibility.into()))
}

Expand Down
24 changes: 15 additions & 9 deletions src/dfx/src/commands/canister/create.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -8,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, 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 crate::util::clap::parsers::{cycle_amount_parser, icrc_subaccount_parser};
use crate::util::clap::subnet_selection_opt::SubnetSelectionOpt;
Expand All @@ -20,7 +21,6 @@ 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;

Expand Down Expand Up @@ -90,10 +90,8 @@ pub struct CanisterCreateOpts {
#[arg(long, value_parser = wasm_memory_limit_parser, hide = true)]
wasm_memory_limit: Option<Byte>,

/// 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<LogVisibility>,
#[command(flatten)]
log_visibility_opt: Option<LogVisibilityOpt>,

/// Performs the call with the user Identity as the Sender of messages.
/// Bypasses the Wallet canister.
Expand Down Expand Up @@ -202,10 +200,14 @@ 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,
opts.log_visibility_opt.as_ref(),
Some(config_interface),
Some(canister_name),
None,
call_sender,
)
.await
.with_context(|| format!("Failed to read log visibility of {canister_name}."))?;
create_canister(
env,
Expand Down Expand Up @@ -285,10 +287,14 @@ 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,
opts.log_visibility_opt.as_ref(),
Some(config_interface),
Some(canister_name),
None,
call_sender,
)
.await
.with_context(|| format!("Failed to read log visibility of {canister_name}."))?;
create_canister(
env,
Expand Down
12 changes: 10 additions & 2 deletions src/dfx/src/commands/canister/status.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,16 @@ async fn canister_status(
"Not Set".to_string()
};
let log_visibility = match status.settings.log_visibility {
LogVisibility::Controllers => "controllers",
LogVisibility::Public => "public",
LogVisibility::Controllers => "controllers".to_string(),
LogVisibility::Public => "public".to_string(),
LogVisibility::AllowedViewers(allowed_list) => {
let mut ids_str = "Allowed List:".to_string();
for principal in allowed_list {
ids_str.push(' ');
ids_str.push_str(principal.to_text().as_str());
}
ids_str
}
};

println!("Canister status call result for {canister}.\nStatus: {status}\nControllers: {controllers}\nMemory allocation: {memory_allocation}\nCompute allocation: {compute_allocation}\nFreezing threshold: {freezing_threshold}\nMemory Size: {memory_size:?}\nBalance: {balance} Cycles\nReserved: {reserved} Cycles\nReserved cycles limit: {reserved_cycles_limit}\nWasm memory limit: {wasm_memory_limit}\nModule hash: {module_hash}\nNumber of queries: {queries_total}\nInstructions spent in queries: {query_instructions_total}\nTotal query request payload size (bytes): {query_req_payload_total}\nTotal query response payload size (bytes): {query_resp_payload_total}\nLog visibility: {log_visibility}",
Expand Down
29 changes: 19 additions & 10 deletions src/dfx/src/commands/canister/update_settings.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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;
Expand All @@ -20,7 +21,6 @@ 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;

/// Update one or more of a canister's settings (i.e its controller, compute allocation, or memory allocation.)
#[derive(Parser, Debug)]
Expand Down Expand Up @@ -88,10 +88,8 @@ pub struct UpdateSettingsOpts {
#[arg(long, value_parser = wasm_memory_limit_parser)]
wasm_memory_limit: Option<Byte>,

/// 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<LogVisibility>,
#[command(flatten)]
log_visibility_opt: Option<LogVisibilityOpt>,

/// Freezing thresholds above ~1.5 years require this flag as confirmation.
#[arg(long)]
Expand Down Expand Up @@ -157,8 +155,15 @@ 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, config_interface, canister_name)?;
let log_visibility = get_log_visibility(
env,
opts.log_visibility_opt.as_ref(),
config_interface,
canister_name,
Some(canister_id),
call_sender,
)
.await?;
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;
Expand Down Expand Up @@ -241,10 +246,14 @@ pub async fn exec(
)
.with_context(|| format!("Failed to get Wasm memory limit for {canister_name}."))?;
let log_visibility = get_log_visibility(
opts.log_visibility,
env,
opts.log_visibility_opt.as_ref(),
Some(config_interface),
Some(canister_name),
Some(canister_id),
call_sender,
)
.await
.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?;
Expand Down
Loading
Loading