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: dfx canister url #3346

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,12 @@

### fix: dfx deploy urls printed for asset canisters

### feat: new subcommand: dfx canister url

Dfx Canister URL is a new subcommand that allows you to print the URL of a canister on a specific network. Currently works for local and ic networks.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Dfx Canister URL is a new subcommand that allows you to print the URL of a canister on a specific network. Currently works for local and ic networks.
`dfx canister url` is a new subcommand that allows you to print the URL of a canister on a specific network. Currently works for local and ic networks.


Asset canisters will print the URL of the asset canister itself, while non-asset canisters will print the URL of the Candid UI interface.

### chore: --emulator parameter is deprecated and will be discontinued soon

Added warning that the `--emulator` is deprecated and will be discontinued soon.
Expand Down
49 changes: 49 additions & 0 deletions e2e/tests-dfx/canister_url.bash
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
#!/usr/bin/env bats

load ../utils/_

setup() {
standard_setup

dfx_new hello
}

teardown() {
dfx_stop

standard_teardown
}

@test "canister url performs as expected on local deploy" {
Copy link
Member

Choose a reason for hiding this comment

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

Could we have a test that demonstrates the expected behavior with a non-remote, non-pull canister on --network ic? The dfx deploy tests couldn't do this because they can't deploy to mainnet. The test would have to populate canister_ids.json.

dfx_new_frontend hello
dfx_start
dfx deploy
assert_command dfx canister url hello_backend
assert_eq "http://127.0.0.1:4943/?canisterId=be2us-64aaa-aaaaa-qaabq-cai&id=bkyz2-fmaaa-aaaaa-qaaaq-cai"
assert_command dfx canister url hello_frontend
assert_eq "http://127.0.0.1:4943/?canisterId=bd3sg-teaaa-aaaaa-qaaba-cai"
}

@test "canister url performs as expected on remote canisters" {
Copy link
Member

Choose a reason for hiding this comment

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

pullable canisters are different from remote canisters. Could we have a test for remote canisters too? They should show different urls depending on network local/ic, but don't need to deploy to mainnet in order to display a mainnet url.

# set dfx.json to string
echo '{"canisters": {"whoami": {"type": "pull", "id": "ivcos-eqaaa-aaaab-qablq-cai"}}}' > dfx.json
assert_command dfx canister url whoami --network ic
assert_eq "https://a4gq6-oaaaa-aaaab-qaa4q-cai.raw.icp0.io/?id=ivcos-eqaaa-aaaab-qablq-cai"
}

@test "missing ui canister error" {
dfx_start
dfx canister create hello_backend
assert_command_fail dfx canister url hello_backend
assert_contains "Network local does not have a ui canister id"
}

@test "missing local id error" {
assert_command_fail dfx canister url hello_backend
assert_contains "Cannot find canister id. Please issue 'dfx canister create hello_backend'"
}

@test "missing ic id error" {
assert_command_fail dfx canister url hello_backend --network ic
assert_contains "Cannot find canister id. Please issue 'dfx canister create hello_backend --network ic'."
}
1 change: 1 addition & 0 deletions src/dfx-core/src/canister/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
pub mod url;
use crate::{
cli::ask_for_consent,
error::canister::{CanisterBuilderError, CanisterInstallError},
Expand Down
105 changes: 105 additions & 0 deletions src/dfx-core/src/canister/url.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
use url::Host::Domain;
use url::ParseError;
use url::Url;

const MAINNET_CANDID_INTERFACE_PRINCIPAL: &str = "a4gq6-oaaaa-aaaab-qaa4q-cai";

pub fn format_frontend_url(provider: &Url, canister_id: &str) -> Url {
Copy link
Member

Choose a reason for hiding this comment

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

Please retain types in the method signature and convert to &str in the method

Suggested change
pub fn format_frontend_url(provider: &Url, canister_id: &str) -> Url {
pub fn format_frontend_url(provider: &Url, canister_id: &Principal) -> Url {

let mut url = Url::clone(&provider);
if let Some(Domain(domain)) = url.host() {
if domain.ends_with("icp-api.io") || domain.ends_with("ic0.app") {
Copy link
Member

Choose a reason for hiding this comment

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

Looking for substrings in domain names is always iffy, but I suspect this should be the following:

Suggested change
if domain.ends_with("icp-api.io") || domain.ends_with("ic0.app") {
if domain == "icp-api.io" || domain.ends_with(".icp-api.io") || domain == "ic0.app" || domain.ends_with(".ic0.app") {

let new_domain = domain.replace("icp-api.io", "icp0.io");
let new_domain = new_domain.replace("ic0.app", "icp0.io");
Comment on lines +11 to +12
Copy link
Member

Choose a reason for hiding this comment

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

Hostname manipulation is error-prone. What if the domain is xyz-ic0.app-aaa.icp0.io?

let host = format!("{}.{}", canister_id, new_domain);
let _ = url.set_host(Some(&host));
} else if domain.contains("localhost") {
Copy link
Member

Choose a reason for hiding this comment

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

What if it's not-a-localhost.my-domain.yay

let port = url.port().unwrap_or(4943);
let host = format!("localhost:{}", port);
Comment on lines +16 to +17
Copy link
Member

Choose a reason for hiding this comment

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

Is it necessary to set the port here? set_host() leaves the port unchanged.

Suggested change
let port = url.port().unwrap_or(4943);
let host = format!("localhost:{}", port);
let host = "localhost";

let query = format!("canisterId={}", canister_id);
url.set_host(Some(&host)).unwrap();
url.set_query(Some(&query));
} else {
let host = format!("{}.{}", canister_id, domain);
let _ = url.set_host(Some(&host));
}
} else {
let query = format!("canisterId={}", canister_id);
url.set_query(Some(&query));
}
url
}

pub fn format_ui_canister_url_ic(canister_id: &str) -> Result<Url, ParseError> {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
pub fn format_ui_canister_url_ic(canister_id: &str) -> Result<Url, ParseError> {
pub fn format_ui_canister_url_ic(canister_id: &Principal) -> Result<Url, ParseError> {

let url_result = Url::parse(
format!(
"https://{}.raw.icp0.io/?id={}",
MAINNET_CANDID_INTERFACE_PRINCIPAL, canister_id
)
.as_str(),
);
return url_result;
Comment on lines +33 to +40
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
let url_result = Url::parse(
format!(
"https://{}.raw.icp0.io/?id={}",
MAINNET_CANDID_INTERFACE_PRINCIPAL, canister_id
)
.as_str(),
);
return url_result;
Url::parse(
&format!(
"https://{}.raw.icp0.io/?id={}",
MAINNET_CANDID_INTERFACE_PRINCIPAL, canister_id
)
)

}

pub fn format_ui_canister_url_custom(
canister_id: &str,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
canister_id: &str,
canister_id: &Principal,

provider: &Url,
ui_canister_id: &str,
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
ui_canister_id: &str,
ui_canister_id: &Principal,

) -> Url {
let mut url = Url::clone(&provider);

if let Some(Domain(domain)) = url.host() {
let host = format!("{}.{}", ui_canister_id, domain);
let query = format!("id={}", canister_id);
url.set_host(Some(&host)).unwrap();
url.set_query(Some(&query));
} else {
let query = format!("canisterId={}&id={}", ui_canister_id, canister_id);
url.set_query(Some(&query));
}

return url;
}

#[cfg(test)]
mod test {
use crate::canister::url::format_frontend_url;
use url::Url;

#[test]
fn print_local_frontend() {
let provider1 = &Url::parse("http://127.0.0.1:4943").unwrap();
let provider2 = &Url::parse("http://localhost:4943").unwrap();
let provider3 = &Url::parse("http://127.0.0.1:8000").unwrap();
assert_eq!(
format_frontend_url(provider1, "ryjl3-tyaaa-aaaaa-aaaba-cai").as_str(),
"http://127.0.0.1:4943/?canisterId=ryjl3-tyaaa-aaaaa-aaaba-cai"
);
assert_eq!(
format_frontend_url(provider2, "ryjl3-tyaaa-aaaaa-aaaba-cai").as_str(),
"http://localhost:4943/?canisterId=ryjl3-tyaaa-aaaaa-aaaba-cai"
);
assert_eq!(
format_frontend_url(provider3, "ryjl3-tyaaa-aaaaa-aaaba-cai").as_str(),
"http://127.0.0.1:8000/?canisterId=ryjl3-tyaaa-aaaaa-aaaba-cai"
);
}

#[test]
fn print_ic_frontend() {
let provider1 = &Url::parse("https://ic0.app").unwrap();
let provider2 = &Url::parse("https://icp-api.io").unwrap();
let provider3 = &Url::parse("https://icp0.io").unwrap();
assert_eq!(
format_frontend_url(provider1, "ryjl3-tyaaa-aaaaa-aaaba-cai").as_str(),
"https://ryjl3-tyaaa-aaaaa-aaaba-cai.icp0.io/"
);
assert_eq!(
format_frontend_url(provider2, "ryjl3-tyaaa-aaaaa-aaaba-cai").as_str(),
"https://ryjl3-tyaaa-aaaaa-aaaba-cai.icp0.io/"
);
assert_eq!(
format_frontend_url(provider3, "ryjl3-tyaaa-aaaaa-aaaba-cai").as_str(),
"https://ryjl3-tyaaa-aaaaa-aaaba-cai.icp0.io/"
);
}
}
5 changes: 4 additions & 1 deletion src/dfx/src/commands/canister/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ mod status;
mod stop;
mod uninstall_code;
mod update_settings;
pub mod url;

/// Manages canisters deployed on a network replica.
#[derive(Parser)]
Expand Down Expand Up @@ -58,11 +59,12 @@ pub enum SubCommand {
Stop(stop::CanisterStopOpts),
UninstallCode(uninstall_code::UninstallCodeOpts),
UpdateSettings(update_settings::UpdateSettingsOpts),
Url(url::CanisterURLOpts),
}

pub fn exec(env: &dyn Environment, opts: CanisterOpts) -> DfxResult {
let agent_env;
let env = if matches!(&opts.subcmd, SubCommand::Id(_)) {
let env = if matches!(&opts.subcmd, SubCommand::Id(_) | SubCommand::Url(_)) {
env
} else {
agent_env = create_agent_environment(env, opts.network.to_network_name())?;
Expand Down Expand Up @@ -90,6 +92,7 @@ pub fn exec(env: &dyn Environment, opts: CanisterOpts) -> DfxResult {
SubCommand::Stop(v) => stop::exec(env, v, &call_sender).await,
SubCommand::UninstallCode(v) => uninstall_code::exec(env, v, &call_sender).await,
SubCommand::UpdateSettings(v) => update_settings::exec(env, v, &call_sender).await,
SubCommand::Url(v) => url::exec(env, v),
}
})
}
110 changes: 110 additions & 0 deletions src/dfx/src/commands/canister/url.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
use crate::lib::canister_info::CanisterInfo;
use crate::lib::error::DfxResult;
use crate::lib::network::network_opt::NetworkOpt;
use crate::lib::{environment::Environment, named_canister};
use anyhow::Context;
use candid::Principal;
use clap::Parser;
use dfx_core::canister::url::{
format_frontend_url, format_ui_canister_url_custom, format_ui_canister_url_ic,
};
use dfx_core::config::model::canister_id_store::CanisterIdStore;
use dfx_core::config::model::network_descriptor::NetworkDescriptor;
use dfx_core::network::provider::{create_network_descriptor, LocalBindDetermination};
use fn_error_context::context;
use url::Url;

/// Prints the URL of a canister.
#[derive(Parser)]
pub struct CanisterURLOpts {
/// Specifies the name of the canister.
canister: String,
#[command(flatten)]
network: NetworkOpt,
}

#[context("Failed to construct frontend url for canister {} on network '{}'.", canister_id, network.name)]
pub fn construct_frontend_url(
network: &NetworkDescriptor,
canister_id: &Principal,
) -> DfxResult<Url> {
let url = Url::parse(&network.providers[0]).with_context(|| {
format!(
"Failed to parse url for network provider {}.",
&network.providers[0]
)
})?;

Ok(format_frontend_url(&url, &canister_id.to_string()))
}

#[context("Failed to construct ui canister url for {} on network '{}'.", canister_id, network.name)]
pub fn construct_ui_canister_url(
network: &NetworkDescriptor,
canister_id: &Principal,
ui_canister_id: Option<Principal>,
) -> DfxResult<Url> {
let provider = Url::parse(&network.providers[0]).with_context(|| {
format!(
"Failed to parse url for network provider {}.",
&network.providers[0]
)
})?;
if network.is_ic {
let formatted_url = format_ui_canister_url_ic(&canister_id.to_string())?;
return Ok(formatted_url);
} else {
if let Some(ui_canister_id) = ui_canister_id {
let formatted_url = format_ui_canister_url_custom(
&canister_id.to_string(),
&provider,
&ui_canister_id.to_string().as_str(),
);
return Ok(formatted_url);
} else {
return Err(anyhow::anyhow!(
"Network {} does not have a ui canister id",
network.name
));
}
}
}

pub fn exec(env: &dyn Environment, opts: CanisterURLOpts) -> DfxResult {
env.get_config_or_anyhow()?;
let network_descriptor = create_network_descriptor(
env.get_config(),
env.get_networks_config(),
opts.network.to_network_name(),
None,
LocalBindDetermination::AsConfigured,
)?;
let canister_name = opts.canister.as_str();
let canister_id_store =
CanisterIdStore::new(env.get_logger(), &network_descriptor, env.get_config())?;
let canister_id =
Principal::from_text(canister_name).or_else(|_| canister_id_store.get(canister_name))?;
let config = env.get_config_or_anyhow()?;
let canister_info = CanisterInfo::load(&config, canister_name, Some(canister_id))?;

let ui_canister_id = named_canister::get_ui_canister_id(&canister_id_store);
// If the canister is an assets canister or has a frontend section, we can display a frontend url.
if let Some(canisters) = &config.get_config().canisters {
let canister_config = canisters.get(canister_name).unwrap();
Comment on lines +92 to +93
Copy link
Member

Choose a reason for hiding this comment

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

Never Nesting is more of an aspiration, but we can flatten this a bit, put the error messages next to the error conditions, and eliminate an unwrap:

Suggested change
if let Some(canisters) = &config.get_config().canisters {
let canister_config = canisters.get(canister_name).unwrap();
let canisters = config
.get_config()
.canisters
.as_ref()
.ok_or_else(|| anyhow::anyhow!("No canisters defined in dfx.json"))?;
let canister_config = canisters
.get(canister_name)
.ok_or_else(|| anyhow::anyhow!("Canister {} not found in dfx.json", canister_name))?;

let is_assets = canister_info.is_assets() || canister_config.frontend.is_some();
if is_assets {
let url = construct_frontend_url(&network_descriptor, &canister_id)?;
println!("{}", url.as_str());
Ok(())
} else {
let url = construct_ui_canister_url(&network_descriptor, &canister_id, ui_canister_id)?;
println!("{}", url.as_str());
Ok(())
}
Comment on lines +95 to +103
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if is_assets {
let url = construct_frontend_url(&network_descriptor, &canister_id)?;
println!("{}", url.as_str());
Ok(())
} else {
let url = construct_ui_canister_url(&network_descriptor, &canister_id, ui_canister_id)?;
println!("{}", url.as_str());
Ok(())
}
let url = if is_assets {
construct_frontend_url(&network_descriptor, &canister_id)?
} else {
construct_ui_canister_url(&network_descriptor, &canister_id, ui_canister_id)?
};
println!("{}", &url);
Ok(())

} else {
Err(anyhow::anyhow!(
"Canister {} does not have a frontend section",
canister_name
))
Comment on lines +105 to +108
Copy link
Member

Choose a reason for hiding this comment

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

Is this error message correct? It looks like the condition for getting here is if dfx.json does not define any canisters.

}
}
Loading
Loading