Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Unreleased

* fix: Validate explicit canister paths and throw an error if `canister.yaml` is not found

# v0.1.0-beta.3

* feat: Remove requirement that the user install `icp-cli-network-launcher`, auto-install it on first use
Expand Down
1 change: 1 addition & 0 deletions crates/icp-cli/src/operations/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use snafu::{ResultExt, Snafu};
use crate::progress::{ProgressManager, ProgressManagerSettings};

#[derive(Debug, Snafu)]
#[allow(clippy::enum_variant_names)]
pub(crate) enum SyncSettingsOperationError {
#[snafu(display("failed to fetch current canister settings for canister {canister}"))]
FetchCurrentSettings {
Expand Down
156 changes: 127 additions & 29 deletions crates/icp-cli/tests/project_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -139,34 +139,132 @@ fn glob_path() {
.success();
}

// TODO(or.ricon): This test is currently not passing, fix it.
// #[test]
// fn explicit_path_missing() {
// let ctx = TestContext::new();

// // Setup project
// let project_dir = ctx.create_project_dir("icp");

// // Project manifest
// let pm = r#"
// canisters:
// - my-canister
// "#;

// write_string(
// &project_dir.join("icp.yaml"), // path
// pm, // contents
// )
// .expect("failed to write project manifest");

// // Invoke build
// ctx.icp()
// .current_dir(project_dir)
// .args(["build"])
// .assert()
// .failure()
// .stderr(eq("Error: canister path must exist and be a directory \'my-canister\'").trim());
// }
#[test]
fn explicit_path_missing() {
let ctx = TestContext::new();

// Setup project
let project_dir = ctx.create_project_dir("icp");

// Project manifest
let pm = r#"
canisters:
- my-canister
"#;

write_string(
&project_dir.join("icp.yaml"), // path
pm, // contents
)
.expect("failed to write project manifest");

// Invoke project show
ctx.icp()
.current_dir(project_dir)
.args(["project", "show"])
.assert()
.failure()
.stderr(contains(
"could not locate a canister manifest at: 'my-canister'",
));
}

#[test]
fn explicit_path_missing_canister_yaml() {
let ctx = TestContext::new();

// Setup project
let project_dir = ctx.create_project_dir("icp");

// Project manifest
let pm = r#"
canisters:
- my-canister
"#;

write_string(
&project_dir.join("icp.yaml"), // path
pm, // contents
)
.expect("failed to write project manifest");

// Create directory but no canister.yaml
create_dir_all(&project_dir.join("my-canister")).expect("failed to create canister directory");

// Invoke project show
ctx.icp()
.current_dir(project_dir)
.args(["project", "show"])
.assert()
.failure()
.stderr(contains(
"could not locate a canister manifest at: 'my-canister'",
));
}

#[test]
fn explicit_path_with_subdirectory() {
let ctx = TestContext::new();

// Setup project
let project_dir = ctx.create_project_dir("icp");

// Project manifest
let pm = r#"
canisters:
- canisters/backend
- canisters/frontend
"#;

write_string(
&project_dir.join("icp.yaml"), // path
pm, // contents
)
.expect("failed to write project manifest");

// Backend canister manifest
let backend_cm = indoc! {r#"
name: backend
build:
steps:
- type: script
command: echo "build"
"#};

create_dir_all(&project_dir.join("canisters/backend"))
.expect("failed to create backend directory");

write_string(
&project_dir.join("canisters/backend/canister.yaml"),
backend_cm,
)
.expect("failed to write backend manifest");

// Frontend canister manifest
let frontend_cm = indoc! {r#"
name: frontend
build:
steps:
- type: script
command: echo "build"
"#};

create_dir_all(&project_dir.join("canisters/frontend"))
.expect("failed to create frontend directory");

write_string(
&project_dir.join("canisters/frontend/canister.yaml"),
frontend_cm,
)
.expect("failed to write frontend manifest");

// Invoke project show - should succeed
ctx.icp()
.current_dir(&project_dir)
.args(["project", "show"])
.assert()
.success();
}

#[test]
fn redefine_mainnet_network_disallowed() {
Expand All @@ -189,7 +287,7 @@ fn redefine_mainnet_network_disallowed() {
// Any command that loads the project should fail
ctx.icp()
.current_dir(project_dir)
.args(["build"])
.args(["project", "show"])
.assert()
.failure()
.stderr(contains("`mainnet` is a reserved network name"));
Expand Down
37 changes: 31 additions & 6 deletions crates/icp/src/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,8 @@ pub async fn consolidate_manifest(
for i in &m.canisters {
let ms = match i {
Item::Path(pattern) => {
let paths = match is_glob(pattern) {
let is_glob_pattern = is_glob(pattern);
let paths = match is_glob_pattern {
// Explicit path
false => vec![pdir.join(pattern)],

Expand All @@ -163,11 +164,35 @@ pub async fn consolidate_manifest(
}
};

let paths = paths
.into_iter()
.filter(|p| p.is_dir()) // Skip missing directories
.filter(|p| p.join(CANISTER_MANIFEST).exists()) // Skip non-canister directories
.collect::<Vec<_>>();
let paths = if is_glob_pattern {
// For glob patterns, filter out non-directories and non-canister directories
paths
.into_iter()
.filter(|p| p.is_dir())
.filter(|p| p.join(CANISTER_MANIFEST).exists())
.collect::<Vec<_>>()
} else {
// For explicit paths, validate that they exist and contain canister.yaml
let mut validated_paths = vec![];
for p in paths {
if !p.is_dir() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we just use p.is_file() ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Otherwise the errors should be fixed, p.is_dir to say the directory doesn't exist and the path joined with canister.yaml to say the canister definition doesn't exist.

return NotFoundSnafu {
kind: "canister".to_string(),
path: pattern.to_string(),
}
.fail();
}
if !p.join(CANISTER_MANIFEST).exists() {
return NotFoundSnafu {
kind: "canister".to_string(),
path: pattern.to_string(),
}
.fail();
}
validated_paths.push(p);
}
validated_paths
};

let mut ms = vec![];

Expand Down