Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Commit

Permalink
Allow running wrangler dev without an account_id
Browse files Browse the repository at this point in the history
Previously, running it with no account_id and no config in ~/.wrangler would give an error:

```
Error: config path does not exist /home/jnelson/.wrangler/config/default.toml. Try running `wrangler login` or `wrangler config`
```

This is not technically a regression from loading account_id lazily, because in wrangler 1.17 it would just error earlier that account_id was required:
```
🕵️  You can find your account_id in the right sidebar of your account's Workers page
Error: field `account_id` is required to deploy to workers.dev
```

However, it seems odd that you'd have to have an account to run an unauthenticated preview.
This changes wrangler to only require an account_id or config file if you're actually deploying your site.

This also changes the tests not to expect that wrangler requires an account_id just to load the available deployments.
  • Loading branch information
jyn514 committed Aug 18, 2021
1 parent 20a5cd4 commit c6c4a00
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 34 deletions.
11 changes: 6 additions & 5 deletions src/commands/dev/edge/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,19 +129,20 @@ fn get_session_config(target: &DeployTarget) -> serde_json::Value {
}
}

fn get_session_address(target: &DeployTarget) -> String {
match target {
fn get_session_address(target: &DeployTarget) -> Result<String> {
let addr = match target {
DeployTarget::Zoned(config) => format!(
"https://api.cloudflare.com/client/v4/zones/{}/workers/edge-preview",
config.zone_id
),
// TODO: zoneless is probably wrong
DeployTarget::Zoneless(config) => format!(
"https://api.cloudflare.com/client/v4/accounts/{}/workers/subdomain/edge-preview",
config.account_id
config.account_id.load()?,
),
_ => unreachable!(),
}
};
Ok(addr)
}

fn get_upload_address(target: &mut Target) -> Result<String> {
Expand All @@ -154,7 +155,7 @@ fn get_upload_address(target: &mut Target) -> Result<String> {

fn get_exchange_url(deploy_target: &DeployTarget, user: &GlobalUser) -> Result<Url> {
let client = crate::http::legacy_auth_client(user);
let address = get_session_address(deploy_target);
let address = get_session_address(deploy_target)?;
let url = Url::parse(&address)?;
let response = client.get(url).send()?.error_for_status()?;
let text = &response.text()?;
Expand Down
11 changes: 6 additions & 5 deletions src/deploy/zoneless.rs
Original file line number Diff line number Diff line change
@@ -1,36 +1,37 @@
use crate::commands::subdomain::Subdomain;
use crate::http;
use crate::settings::global_user::GlobalUser;
use crate::settings::toml::target::LazyAccountId;
use crate::settings::toml::RouteConfig;

use anyhow::Result;

#[derive(Clone, Debug, PartialEq)]
pub struct ZonelessTarget {
pub account_id: String,
pub account_id: LazyAccountId,
pub script_name: String,
}

impl ZonelessTarget {
pub fn build(script_name: &str, route_config: &RouteConfig) -> Result<Self> {
let account_id = route_config.account_id.load()?;
Ok(Self {
script_name: script_name.to_string(),
account_id: account_id.to_string(),
account_id: route_config.account_id.clone(),
})
}

pub fn deploy(&self, user: &GlobalUser) -> Result<String> {
log::info!("publishing to workers.dev subdomain");
log::info!("checking that subdomain is registered");
let subdomain = match Subdomain::get(&self.account_id, user)? {
let subdomain = match Subdomain::get(self.account_id.load()?, user)? {
Some(subdomain) => subdomain,
None => anyhow::bail!("Before publishing to workers.dev, you must register a subdomain. Please choose a name for your subdomain and run `wrangler subdomain <name>`.")
};

let sd_worker_addr = format!(
"https://api.cloudflare.com/client/v4/accounts/{}/workers/scripts/{}/subdomain",
self.account_id, self.script_name,
self.account_id.load()?,
self.script_name,
);

let client = http::legacy_auth_client(user);
Expand Down
2 changes: 1 addition & 1 deletion src/settings/toml/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ mod manifest;
pub mod migrations;
mod route;
mod site;
mod target;
pub(crate) mod target;
mod target_type;
mod triggers;

Expand Down
2 changes: 1 addition & 1 deletion src/settings/toml/target.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::durable_objects::DurableObjects;
use super::kv_namespace::KvNamespace;
use super::manifest::LazyAccountId;
pub(crate) use super::manifest::LazyAccountId;
use super::site::Site;
use super::target_type::TargetType;
use super::UsageModel;
Expand Down
82 changes: 60 additions & 22 deletions src/settings/toml/tests/deployments.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::str::FromStr;

use crate::deploy::{DeployTarget, ScheduleTarget, ZonedTarget, ZonelessTarget};
use crate::settings::global_user::GlobalUser;
use crate::settings::toml::route::Route;
use crate::settings::toml::Manifest;

Expand Down Expand Up @@ -44,7 +45,7 @@ fn it_can_get_multiple_deployments() {
zone_id: ZONE_ID.to_owned(),
}),
DeployTarget::Zoneless(ZonelessTarget {
account_id: ACCOUNT_ID.to_owned(),
account_id: Some(ACCOUNT_ID.to_string()).into(),
script_name: script_name.to_owned(),
}),
];
Expand All @@ -66,7 +67,7 @@ fn it_can_get_a_top_level_zoneless_get_deployments() {
let actual_deployments = manifest.get_deployments(environment).unwrap();
let expected_deployments = vec![DeployTarget::Zoneless(ZonelessTarget {
script_name: script_name.to_string(),
account_id: ACCOUNT_ID.to_string(),
account_id: Some(ACCOUNT_ID.to_string()).into(),
})];

assert_eq!(actual_deployments, expected_deployments);
Expand All @@ -85,19 +86,26 @@ fn it_errors_on_get_deployments_missing_name() {
assert!(manifest.get_deployments(environment).is_err());
}

#[test]
fn it_errors_on_get_deployments_missing_account_id() {
let script_name = "zoneless_no_account_id";
let workers_dev = true;
let mut test_toml = WranglerToml::zoneless(script_name, ACCOUNT_ID, workers_dev);
test_toml.account_id = None;
let toml_string = toml::to_string(&test_toml).unwrap();
let manifest = Manifest::from_str(&toml_string).unwrap();

let environment = None;

assert!(manifest.get_deployments(environment).is_err());
}
// #[test]
// fn it_errors_on_get_deployments_missing_account_id() {
// let script_name = "zoneless_no_account_id";
// let workers_dev = true;
// let mut test_toml = WranglerToml::zoneless(script_name, ACCOUNT_ID, workers_dev);
// test_toml.account_id = None;
// let toml_string = toml::to_string(&test_toml).unwrap();
// let manifest = Manifest::from_str(&toml_string).unwrap();

// // assert!(manifest.get_deployments(environment).is_err());
// use crate::commands::dev::edge::setup::Session;
// let target = manifest.get_target(None, false);
// for deployment in manifest.get_deployments(None)? {
// if Session::new(manifest.target(), GlobalUser::new(), deploy).is_err() {
// return;
// }
// }

// panic!("at least one deployment should have required an account ID");
// }

#[test]
fn it_errors_on_zoneless_get_deployments_workers_dev_false() {
Expand Down Expand Up @@ -505,7 +513,22 @@ fn it_gets_deployments_with_route_and_workers_dev_true() {

let environment = None;

assert!(manifest.get_deployments(environment).is_err());
let deployments = manifest.get_deployments(environment).unwrap();
match dbg!(&*deployments) {
[DeployTarget::Zoned(ZonedTarget { zone_id, routes }), DeployTarget::Zoneless(ZonelessTarget {
account_id: _,
script_name: actual_script_name,
})] => {
assert!(
script_name == actual_script_name
&& zone_id == "samplezoneid"
&& matches!(&**routes,
[Route { id: None, script: Some(actual_script_name), pattern }]
if script_name == actual_script_name && pattern == PATTERN)
);
}
_ => panic!("deployments didn't match"),
}
}

#[test]
Expand All @@ -520,7 +543,22 @@ fn it_gets_deployments_with_routes_and_workers_dev_true() {

let environment = None;

assert!(manifest.get_deployments(environment).is_err());
let deployments = manifest.get_deployments(environment).unwrap();
match dbg!(&*deployments) {
[DeployTarget::Zoned(ZonedTarget { zone_id, routes }), DeployTarget::Zoneless(ZonelessTarget {
account_id: _,
script_name: actual_script_name,
})] => {
assert!(
script_name == actual_script_name
&& zone_id == "samplezoneid"
&& matches!(&**routes,
[Route { id: None, script: Some(actual_script_name), pattern }]
if script_name == actual_script_name && pattern == PATTERN)
);
}
_ => panic!("deployments didn't match"),
}
}

// ENVIRONMENT TESTS
Expand Down Expand Up @@ -587,7 +625,7 @@ fn when_top_level_empty_env_workers_dev_true() {
let actual_deployments = manifest.get_deployments(environment).unwrap();
let expected_deployments = vec![DeployTarget::Zoneless(ZonelessTarget {
script_name: manifest.worker_name(environment),
account_id: account_id.to_string(),
account_id: Some(account_id.to_string()).into(),
})];

assert_eq!(actual_deployments, expected_deployments);
Expand Down Expand Up @@ -760,7 +798,7 @@ fn when_top_level_zoneless_env_empty() {
let actual_deployments = manifest.get_deployments(environment).unwrap();
let expected_deployments = vec![DeployTarget::Zoneless(ZonelessTarget {
script_name: manifest.worker_name(environment),
account_id: ACCOUNT_ID.to_string(),
account_id: Some(ACCOUNT_ID.to_string()).into(),
})];

assert_eq!(actual_deployments, expected_deployments);
Expand Down Expand Up @@ -797,7 +835,7 @@ fn when_top_level_zoneless_env_zoneless_workers_dev_true() {
let environment = Some(TEST_ENV_NAME);
let actual_deployments = manifest.get_deployments(environment).unwrap();
let expected_deployments = vec![DeployTarget::Zoneless(ZonelessTarget {
account_id: ACCOUNT_ID.to_string(),
account_id: Some(ACCOUNT_ID.to_string()).into(),
script_name: manifest.worker_name(environment),
})];

Expand All @@ -820,7 +858,7 @@ fn when_top_level_zoneless_env_zoned_single_route_empty() {
let environment = Some(TEST_ENV_NAME);
let actual_deployments = manifest.get_deployments(environment).unwrap();
let expected_deployments = vec![DeployTarget::Zoneless(ZonelessTarget {
account_id: ACCOUNT_ID.to_string(),
account_id: Some(ACCOUNT_ID.to_string()).into(),
script_name: manifest.worker_name(environment),
})];

Expand Down Expand Up @@ -1044,7 +1082,7 @@ fn when_top_level_zoned_env_zoneless_workers_dev_true() {
let environment = Some(TEST_ENV_NAME);
let actual_deployments = manifest.get_deployments(environment).unwrap();
let expected_deployments = vec![DeployTarget::Zoneless(ZonelessTarget {
account_id: ACCOUNT_ID.to_string(),
account_id: Some(ACCOUNT_ID.to_string()).into(),
script_name: manifest.worker_name(environment),
})];

Expand Down

0 comments on commit c6c4a00

Please sign in to comment.