Skip to content

Commit

Permalink
chore: [IC-272] add fetch_canister_logs feature flag
Browse files Browse the repository at this point in the history
  • Loading branch information
maksymar committed Jan 29, 2024
1 parent 29aef13 commit d2bfbb4
Show file tree
Hide file tree
Showing 6 changed files with 188 additions and 65 deletions.
5 changes: 5 additions & 0 deletions rs/config/src/execution_environment.rs
Expand Up @@ -250,6 +250,10 @@ pub struct Config {

/// Indicates whether canister backup and restore feature is enabled or not.
pub canister_snapshots: FlagStatus,

// TODO(IC-272): remove this flag once the feature is enabled by default.
/// Indicates whether fetching canister logs API is enabled or not.
pub fetch_canister_logs: FlagStatus,
}

impl Default for Config {
Expand Down Expand Up @@ -319,6 +323,7 @@ impl Default for Config {
wasm_chunk_store: FlagStatus::Enabled,
stop_canister_timeout_duration: STOP_CANISTER_TIMEOUT_DURATION,
canister_snapshots: FlagStatus::Disabled,
fetch_canister_logs: FlagStatus::Disabled,
}
}
}
Expand Down
39 changes: 18 additions & 21 deletions rs/execution_environment/src/canister_manager.rs
Expand Up @@ -17,8 +17,8 @@ use ic_cycles_account_manager::{CyclesAccountManager, ResourceSaturation};
use ic_error_types::{ErrorCode, RejectCode, UserError};
use ic_ic00_types::{
CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallModeV2, CanisterStatusResultV2,
CanisterStatusType, InstallChunkedCodeArgs, InstallCodeArgsV2, LogVisibility,
Method as Ic00Method, StoredChunksReply, UploadChunkReply,
CanisterStatusType, InstallChunkedCodeArgs, InstallCodeArgsV2, Method as Ic00Method,
StoredChunksReply, UploadChunkReply,
};
use ic_interfaces::execution_environment::{
CanisterOutOfCyclesError, HypervisorError, IngressHistoryWriter, SubnetAvailableMemory,
Expand Down Expand Up @@ -437,6 +437,7 @@ impl CanisterManager {
provisional_whitelist: &ProvisionalWhitelist,
ingress: &SignedIngressContent,
effective_canister_id: Option<CanisterId>,
fetch_canister_logs: FlagStatus,
) -> Result<(), UserError> {
let method_name = ingress.method_name();
let sender = ingress.sender();
Expand Down Expand Up @@ -524,33 +525,29 @@ impl CanisterManager {
)),
}
},
None => Err(UserError::new(
None => Err(UserError::new(
ErrorCode::InvalidManagementPayload,
format!("Failed to decode payload for ic00 method: {}", method_name),
)),
}
},

Ok(Ic00Method::FetchCanisterLogs) => {
match effective_canister_id {
Some(canister_id) => {
let canister = state.canister_state(&canister_id).ok_or_else(|| UserError::new(
ErrorCode::CanisterNotFound,
format!("Canister {} not found", canister_id),
))?;
match canister.log_visibility() {
LogVisibility::Public => Ok(()),
LogVisibility::Controllers if canister.controllers().contains(&sender.get()) => Ok(()),
LogVisibility::Controllers => Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!("Caller {} is not allowed to call ic00 method {}", sender, method_name),
)),
}
},
None => Err(UserError::new(
ErrorCode::InvalidManagementPayload,
format!("Failed to decode payload for ic00 method: {}", method_name),
match fetch_canister_logs {
FlagStatus::Enabled => Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!(
"{} API is only accessible in non-replicated mode",
Ic00Method::FetchCanisterLogs
),
)),
FlagStatus::Disabled => Err(UserError::new(
ErrorCode::CanisterContractViolation,
format!(
"{} API is not enabled on this subnet",
Ic00Method::FetchCanisterLogs
),
))
}
},

Expand Down
32 changes: 22 additions & 10 deletions rs/execution_environment/src/execution_environment.rs
Expand Up @@ -1102,17 +1102,28 @@ impl ExecutionEnvironment {
Some((res, msg.take_cycles()))
}

Ok(Ic00Method::FetchCanisterLogs) => Some((
// TODO(IC-272).
Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!(
"{} API is not yet implemented.",
Ic00Method::FetchCanisterLogs
),
Ok(Ic00Method::FetchCanisterLogs) => match self.config.fetch_canister_logs {
FlagStatus::Enabled => Some((
Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!(
"{} API is only accessible in non-replicated mode",
Ic00Method::FetchCanisterLogs
),
)),
msg.take_cycles(),
)),
msg.take_cycles(),
)),
FlagStatus::Disabled => {
let err = Err(UserError::new(
ErrorCode::CanisterContractViolation,
format!(
"{} API is not enabled on this subnet",
Ic00Method::FetchCanisterLogs
),
));
Some((err, msg.take_cycles()))
}
},

Ok(Ic00Method::TakeCanisterSnapshot) => match self.config.canister_snapshots {
FlagStatus::Enabled => {
Expand Down Expand Up @@ -1872,6 +1883,7 @@ impl ExecutionEnvironment {
provisional_whitelist,
ingress,
effective_canister_id,
self.config.fetch_canister_logs,
);
}

Expand Down
57 changes: 23 additions & 34 deletions rs/execution_environment/src/execution_environment/tests.rs
Expand Up @@ -3272,9 +3272,13 @@ fn test_canister_settings_log_visibility_set_to_public() {
}

#[test]
fn test_fetch_canister_logs_should_accept_ingress_message_log_visibility_public_succeeds() {
fn test_fetch_canister_logs_should_accept_ingress_message_disabled() {
// Arrange.
let mut test = ExecutionTestBuilder::new().build();
// - disable the fetch_canister_logs API
// - set the log visibility to public so any user can read the logs
let mut test = ExecutionTestBuilder::new()
.with_fetch_canister_logs(FlagStatus::Disabled)
.build();
let canister_id = test.universal_canister().unwrap();
let not_a_controller = user_test_id(42);
test.set_log_visibility(canister_id, LogVisibility::Public)
Expand All @@ -3290,17 +3294,27 @@ fn test_fetch_canister_logs_should_accept_ingress_message_log_visibility_public_
.encode(),
);
// Assert.
assert_eq!(result, Ok(()));
// Expect error because the API is disabled.
assert_eq!(
result,
Err(UserError::new(
ErrorCode::CanisterContractViolation,
"fetch_canister_logs API is not enabled on this subnet"
))
);
}

#[test]
fn test_fetch_canister_logs_should_accept_ingress_message_log_visibility_invalid_controller_fails()
{
fn test_fetch_canister_logs_should_accept_ingress_message_enabled() {
// Arrange.
let mut test = ExecutionTestBuilder::new().build();
// - enable the fetch_canister_logs API
// - set the log visibility to public so any user can read the logs
let mut test = ExecutionTestBuilder::new()
.with_fetch_canister_logs(FlagStatus::Enabled)
.build();
let canister_id = test.universal_canister().unwrap();
let not_a_controller = user_test_id(42);
test.set_log_visibility(canister_id, LogVisibility::Controllers)
test.set_log_visibility(canister_id, LogVisibility::Public)
.unwrap();
// Act.
test.set_user_id(not_a_controller);
Expand All @@ -3313,37 +3327,12 @@ fn test_fetch_canister_logs_should_accept_ingress_message_log_visibility_invalid
.encode(),
);
// Assert.
// Expect error since `should_accept_ingress_message` is only called in replicated mode which is not supported.
assert_eq!(
result,
Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
format!(
"Caller {not_a_controller} is not allowed to call ic00 method fetch_canister_logs"
),
"fetch_canister_logs API is only accessible in non-replicated mode"
))
);
}

#[test]
fn test_fetch_canister_logs_should_accept_ingress_message_log_visibility_valid_controller_succeeds()
{
// Arrange.
let mut test = ExecutionTestBuilder::new().build();
let canister_id = test.universal_canister().unwrap();
let controller = user_test_id(42);
test.set_log_visibility(canister_id, LogVisibility::Controllers)
.unwrap();
test.set_controller(canister_id, controller.get()).unwrap();
// Act.
test.set_user_id(controller);
let result = test.should_accept_ingress_message(
test.state().metadata.own_subnet_id.into(),
Method::FetchCanisterLogs,
FetchCanisterLogsRequest {
canister_id: canister_id.into(),
}
.encode(),
);
// Assert.
assert_eq!(result, Ok(()));
}
115 changes: 115 additions & 0 deletions rs/execution_environment/tests/fetch_canister_logs.rs
@@ -0,0 +1,115 @@
use ic_config::execution_environment::Config as ExecutionConfig;
use ic_config::flag_status::FlagStatus;
use ic_config::subnet_config::SubnetConfig;
use ic_ic00_types::{
CanisterInstallMode, CanisterSettingsArgsBuilder, FetchCanisterLogsRequest, LogVisibility,
Payload,
};
use ic_registry_subnet_type::SubnetType;
use ic_state_machine_tests::{
ErrorCode, PrincipalId, StateMachine, StateMachineBuilder, StateMachineConfig, UserError,
};
use ic_test_utilities::universal_canister::UNIVERSAL_CANISTER_WASM;
use ic_types::{CanisterId, Cycles};

fn setup(fetch_canister_logs: FlagStatus) -> (StateMachine, CanisterId) {
let subnet_type = SubnetType::Application;
let config = StateMachineConfig::new(
SubnetConfig::new(subnet_type),
ExecutionConfig {
fetch_canister_logs,
..ExecutionConfig::default()
},
);
let env = StateMachineBuilder::new()
.with_config(Some(config))
.with_subnet_type(subnet_type)
.with_checkpoints_enabled(false)
.build();
let canister_id =
env.create_canister_with_cycles(None, Cycles::from(100_000_000_000_u128), None);
env.install_wasm_in_mode(
canister_id,
CanisterInstallMode::Install,
UNIVERSAL_CANISTER_WASM.to_vec(),
vec![],
)
.unwrap();

(env, canister_id)
}

#[test]
fn test_fetch_canister_logs_ingress_disabled() {
// Arrange.
// - disable the fetch_canister_logs API
// - set the log visibility to public so that any user can read the logs
let (env, canister_id) = setup(FlagStatus::Disabled);
let not_a_controller = PrincipalId::new_user_test_id(42);
env.update_settings(
&canister_id,
CanisterSettingsArgsBuilder::new()
.with_log_visibility(LogVisibility::Public)
.build(),
)
.unwrap();
// Act.
// Make an update call.
let result = env.execute_ingress_as(
not_a_controller,
CanisterId::ic_00(),
"fetch_canister_logs",
FetchCanisterLogsRequest {
canister_id: canister_id.into(),
}
.encode(),
);
// Assert.
// Expect to get an error because the fetch_canister_logs API is disabled,
// despite the fact that the log visibility is set to public.
assert_eq!(
result,
Err(UserError::new(
ErrorCode::CanisterContractViolation,
"fetch_canister_logs API is not enabled on this subnet"
))
);
}

#[test]
fn test_fetch_canister_logs_ingress_enabled() {
// Arrange.
// - enable the fetch_canister_logs API
// - set the log visibility to public so that any user can read the logs
let (env, canister_id) = setup(FlagStatus::Enabled);
let not_a_controller = PrincipalId::new_user_test_id(42);
env.update_settings(
&canister_id,
CanisterSettingsArgsBuilder::new()
.with_log_visibility(LogVisibility::Public)
.build(),
)
.unwrap();
// Act.
// Make an update call.
let result = env.execute_ingress_as(
not_a_controller,
CanisterId::ic_00(),
"fetch_canister_logs",
FetchCanisterLogsRequest {
canister_id: canister_id.into(),
}
.encode(),
);
// Assert.
// Expect error because an update calls are not allowed.
assert_eq!(
result,
Err(UserError::new(
ErrorCode::CanisterRejectedMessage,
"fetch_canister_logs API is only accessible in non-replicated mode"
))
);
}

// TODO(IC-272): add query call tests.
5 changes: 5 additions & 0 deletions rs/test_utilities/execution_environment/src/lib.rs
Expand Up @@ -1922,6 +1922,11 @@ impl ExecutionTestBuilder {
self
}

pub fn with_fetch_canister_logs(mut self, status: FlagStatus) -> Self {
self.execution_config.fetch_canister_logs = status;
self
}

pub fn with_time(mut self, time: Time) -> Self {
self.time = time;
self
Expand Down

0 comments on commit d2bfbb4

Please sign in to comment.