feat: add log_visibility to canister settings manifest #312
Conversation
| if compute_allocation.is_none_or(|s| s == current_settings.compute_allocation) | ||
| if log_visibility_setting | ||
| .as_ref() | ||
| .is_none_or(|s| *s == current_settings.log_visibility) |
There was a problem hiding this comment.
is this going to work when the settings are for allowed principals and the array is not in the same order or do we not care?
There was a problem hiding this comment.
actually is was not possible to set allowed viewers via config yet. I just added this, too.
the updated code now also checks if the values for allowed_viewers (log_visibility) and environment_variables have changed so that we avoid unnecessary update calls.
or do we want to always trigger an update call, also if just the order is changed? 🤔
Add support for configuring log_visibility in icp.yaml and canister.yaml files. This allows users to set who can read canister logs directly in their project configuration: ```yaml settings: log_visibility: public # or "controllers" ``` Previously, log_visibility could only be changed via the CLI command `icp canister settings update --log-visibility`, not in manifest files. Changes: - Add LogVisibility enum to icp::canister module with JsonSchema support - Add log_visibility field to Settings struct - Update settings sync to apply log_visibility during deployment - Regenerate JSON schemas Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…d fix test for PocketIC limitation
marc0olo
force-pushed
the
marc0olo/fix-log-visibility-settings
branch
from
e549c20 to
2583a8a
Compare
| #[serde(rename_all = "lowercase")] | ||
| #[derive(Clone, Debug, Default, PartialEq, Serialize)] | ||
| #[serde(rename_all = "snake_case")] | ||
| pub enum LogVisibility { |
There was a problem hiding this comment.
Serialization, deserialization, and the schema shouldn't have three different structures. If there's multiple ways it can look, it should use multiple serde entities.
| if compute_allocation.is_none_or(|s| s == current_settings.compute_allocation) | ||
| if log_visibility_setting | ||
| .as_ref() | ||
| .is_none_or(|s| log_visibility_eq(s, ¤t_settings.log_visibility)) |
There was a problem hiding this comment.
Please also update icp canister settings update to warn for this field like the others.
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn canister_settings_sync_log_visibility() { |
There was a problem hiding this comment.
Maybe add a test case for allowed_principals?
|
@adamspofford-dfinity your feedback should be addressed now. here the summary of the recent follow-up commit:
The refactored implementation reduces duplication by having Serialize and |
3 participants
Summary
Add support for configuring
log_visibilityinicp.yamlandcanister.yamlmanifest files. This allows users to control who can read canister logs directly in their project configuration:Previously,
log_visibilitycould only be changed via the CLI commandicp canister settings update --log-visibility.Changes
Core functionality
LogVisibilityenum toicp::canistermodule withJsonSchemasupport for manifest validationlog_visibilityfield toSettingsstructcanister settings syncto applylog_visibilityduring deploymentlog_visibilityto cycles ledger duringcanister createanddeployFiles changed
crates/icp/src/canister/mod.rs- AddLogVisibilityenum and conversionscrates/icp-canister-interfaces/src/cycles_ledger.rs- AddLogVisibilitytoCanisterSettingsArgcrates/icp-cli/src/commands/canister/create.rs- Passlog_visibilityduring creationcrates/icp-cli/src/operations/settings.rs- Applylog_visibilityduring syncdocs/schemas/*.json- Updated JSON schemasTesting
Added comprehensive tests:
canister_create_with_settings- Tests manifestlog_visibilitywith create + sync workflowcanister_settings_sync_log_visibility- Tests syncinglog_visibilityfrom manifest after deployNote on PocketIC limitation
The test environment (PocketIC) uses a fake-cmc that doesn't support
log_visibilityduring canister creation. The real cycles ledger on mainnet does support it. The tests work around this by syncing settings after creation, which uses the management canister directly and works correctly.