From 64bcd1636d81caa68d02b4e6b06d996ac37cecfb Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 23 Jun 2021 10:25:54 -0400 Subject: [PATCH 01/10] Switch from lazy_static to once_cell `OnceCell` is needed later, and this avoids having two dependencies that do the same thing. Wrangler has dependencies that use both, so this doesn't actually reduce compile times, but it will make it clear which library to use. --- Cargo.lock | 2 +- Cargo.toml | 2 +- src/install/mod.rs | 7 +++---- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 8d306af1b..4fd12ab1b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3295,10 +3295,10 @@ dependencies = [ "hyper-rustls", "ignore", "indicatif", - "lazy_static", "log 0.4.14", "notify", "number_prefix 0.4.0", + "once_cell", "openssl", "os-version", "path-slash", diff --git a/Cargo.toml b/Cargo.toml index d8581b554..5d39c5650 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -35,10 +35,10 @@ hyper = { version = "0.14.7", features = ["http2", "server", "runtime"] } hyper-rustls = "0.22.1" ignore = "0.4.17" indicatif = "0.15.0" -lazy_static = "1.4.0" log = "0.4.11" notify = "4.0.15" number_prefix = "0.4.0" +once_cell = "1" openssl = { version = "0.10.32", optional = true } os-version = "0.1.1" path-slash = "0.1.4" diff --git a/src/install/mod.rs b/src/install/mod.rs index babc27867..d7418adfa 100644 --- a/src/install/mod.rs +++ b/src/install/mod.rs @@ -12,11 +12,10 @@ use std::env; use std::fs; use std::path::{Path, PathBuf}; -use lazy_static::lazy_static; +use once_cell::sync::Lazy; -lazy_static! { - static ref CACHE: Cache = get_wrangler_cache().expect("Could not get Wrangler cache location"); -} +static CACHE: Lazy = + Lazy::new(|| get_wrangler_cache().expect("Could not get Wrangler cache location")); enum ToolDownload { NeedsInstall(Version), From 89e030e33def34777799f0a61763f412a463debb Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 22 Jun 2021 17:39:51 -0400 Subject: [PATCH 02/10] Use the None variant of `RouteId::account_id` Previously, it was always `Some` and the empty string was used to see if it wasn't filled out. This removes unnecessary checking for the empty string. --- src/deploy/zoneless.rs | 2 +- src/settings/toml/environment.rs | 4 ++-- src/settings/toml/manifest.rs | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/deploy/zoneless.rs b/src/deploy/zoneless.rs index d193dfb93..98a679dc3 100644 --- a/src/deploy/zoneless.rs +++ b/src/deploy/zoneless.rs @@ -16,7 +16,7 @@ impl ZonelessTarget { match route_config.account_id.as_ref() { // TODO: Deserialize empty strings to None; cannot do this for account id // yet without a large refactor. - Some(account_id) if !account_id.is_empty() => Ok(Self { + Some(account_id) => Ok(Self { script_name: script_name.to_string(), account_id: account_id.to_string(), }), diff --git a/src/settings/toml/environment.rs b/src/settings/toml/environment.rs index 487190598..ad8796b71 100644 --- a/src/settings/toml/environment.rs +++ b/src/settings/toml/environment.rs @@ -37,10 +37,10 @@ pub struct Environment { impl Environment { pub fn route_config( &self, - top_level_account_id: String, + top_level_account_id: Option, top_level_zone_id: Option, ) -> Option { - let account_id = self.account_id.clone().or(Some(top_level_account_id)); + let account_id = self.account_id.clone().or(top_level_account_id); let zone_id = self.zone_id.clone().or(top_level_zone_id); if self.workers_dev.is_none() && self.route.is_none() && self.routes.is_none() { diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index c6c99ec76..4230639e8 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -190,7 +190,7 @@ impl Manifest { fn route_config(&self) -> RouteConfig { RouteConfig { - account_id: Some(self.account_id.clone()), + account_id: self.account_id, workers_dev: self.workers_dev, route: self.route.clone(), routes: self.routes.clone(), From d278ea701f0bbbf2fc4f225295e6eb288b61463b Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 22 Jun 2021 17:40:50 -0400 Subject: [PATCH 03/10] Use an Option for account_id This makes it explicit where the account ID is required and where it's treated as the empty string. --- src/deploy/schedule.rs | 8 ++++++-- src/reporter/mod.rs | 2 +- src/settings/toml/manifest.rs | 26 +++++++++++++------------- 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/deploy/schedule.rs b/src/deploy/schedule.rs index 3a7cc2fa2..b9877332a 100644 --- a/src/deploy/schedule.rs +++ b/src/deploy/schedule.rs @@ -11,11 +11,15 @@ pub struct ScheduleTarget { } impl ScheduleTarget { - pub fn build(account_id: String, script_name: String, crons: Vec) -> Result { + pub fn build( + account_id: Option, + script_name: String, + crons: Vec, + ) -> Result { // TODO: add validation for expressions before pushing them to the API // we can do this once the cron parser is open sourced Ok(Self { - account_id, + account_id: account_id.unwrap_or_default(), script_name, crons, }) diff --git a/src/reporter/mod.rs b/src/reporter/mod.rs index 1a2ad72cf..9b534029b 100644 --- a/src/reporter/mod.rs +++ b/src/reporter/mod.rs @@ -202,7 +202,7 @@ fn load_project_info() -> ProjectInfo { .insert("script_name".into(), manifest.name); project_info .base - .insert("account_id".into(), manifest.account_id); + .insert("account_id".into(), manifest.account_id.unwrap_or_default()); project_info.base.insert( "zone_id".into(), manifest.zone_id.unwrap_or_else(|| "".into()), diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index 4230639e8..790670186 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -36,7 +36,7 @@ pub struct Manifest { #[serde(rename = "type")] pub target_type: TargetType, #[serde(default)] - pub account_id: String, + pub account_id: Option, pub workers_dev: Option, #[serde(default, with = "string_empty_as_none")] pub route: Option, @@ -190,7 +190,7 @@ impl Manifest { fn route_config(&self) -> RouteConfig { RouteConfig { - account_id: self.account_id, + account_id: self.account_id.clone(), workers_dev: self.workers_dev, route: self.route.clone(), routes: self.routes.clone(), @@ -256,7 +256,7 @@ impl Manifest { let crons = match env { Some(e) => { - let account_id = e.account_id.as_ref().unwrap_or(&self.account_id); + let account_id = e.account_id.as_ref().or_else(|| self.account_id.as_ref()); e.triggers .as_ref() .or_else(|| self.triggers.as_ref()) @@ -265,12 +265,12 @@ impl Manifest { None => self .triggers .as_ref() - .map(|t| (t.crons.as_slice(), &self.account_id)), + .map(|t| (t.crons.as_slice(), self.account_id.as_ref())), }; if let Some((crons, account)) = crons { let scheduled = - deploy::ScheduleTarget::build(account.clone(), script.clone(), crons.to_vec())?; + deploy::ScheduleTarget::build(account.cloned(), script.clone(), crons.to_vec())?; deployments.push(DeployTarget::Schedule(scheduled)); } @@ -288,20 +288,20 @@ impl Manifest { pub fn get_account_id(&self, environment_name: Option<&str>) -> Result { let environment = self.get_environment(environment_name)?; - let mut result = self.account_id.to_string(); + let mut result = self.account_id.clone(); if let Some(environment) = environment { if let Some(account_id) = &environment.account_id { - result = account_id.to_string(); + result = Some(account_id.to_string()); } } - if result.is_empty() { + if let Some(id) = result { + Ok(id) + } else { let mut msg = "Your configuration file is missing an account_id field".to_string(); if let Some(environment_name) = environment_name { msg.push_str(&format!(" in [env.{}]", environment_name)); } anyhow::bail!("{}", &msg) - } else { - Ok(result) } } @@ -340,10 +340,10 @@ impl Manifest { Not inherited: Must be defined for every environment individually. */ let mut target = Target { - target_type: self.target_type.clone(), // Top level - account_id: self.account_id.clone(), // Inherited + target_type: self.target_type.clone(), // Top level + account_id: self.account_id.clone().unwrap_or_default(), // Inherited webpack_config: self.webpack_config.clone(), // Inherited - build: self.build.clone(), // Inherited + build: self.build.clone(), // Inherited // importantly, the top level name will be modified // to include the name of the environment name: self.name.clone(), // Inherited From 033ef2470384154982fd779ec4f33a8c0f00b1be Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Tue, 22 Jun 2021 17:27:32 -0400 Subject: [PATCH 04/10] Introduce LazyAccountId This collects all handling of the account_id in one place, so that errors no longer have to be handled at each call-site. Making this a new type, rather than a function on `Manifest`, allows it to be used for other types as well, such as `Target`. Using a `OnceCell` rather than an `Option` has two benefits: 1. It does not allow setting the account twice, which would be a bug. 2. It allows setting the account with only an immutable reference, which reduces the amount of churn in calling functions. Note that this still deserializes the empty string as not having an `account_id` for backwards compatibility. --- src/commands/publish.rs | 3 - src/commands/whoami.rs | 2 +- src/deploy/zoneless.rs | 19 ++----- src/reporter/mod.rs | 11 +++- src/settings/toml/environment.rs | 2 +- src/settings/toml/manifest.rs | 94 +++++++++++++++++++++++++------- src/settings/toml/route.rs | 4 +- 7 files changed, 94 insertions(+), 41 deletions(-) diff --git a/src/commands/publish.rs b/src/commands/publish.rs index df623586a..18c789148 100644 --- a/src/commands/publish.rs +++ b/src/commands/publish.rs @@ -184,9 +184,6 @@ pub fn validate_bucket_location(bucket: &Path) -> Result<()> { fn validate_target_required_fields_present(target: &Target) -> Result<()> { let mut missing_fields = Vec::new(); - if target.account_id.is_empty() { - missing_fields.push("account_id") - }; if target.name.is_empty() { missing_fields.push("name") }; diff --git a/src/commands/whoami.rs b/src/commands/whoami.rs index 0549b643d..7b0910a3b 100644 --- a/src/commands/whoami.rs +++ b/src/commands/whoami.rs @@ -111,7 +111,7 @@ fn fetch_api_token_email( } /// Fetch the accounts associated with a user -fn fetch_accounts(user: &GlobalUser) -> Result> { +pub(crate) fn fetch_accounts(user: &GlobalUser) -> Result> { let client = http::cf_v4_client(user)?; let response = client.request(&account::ListAccounts { params: None }); match response { diff --git a/src/deploy/zoneless.rs b/src/deploy/zoneless.rs index 98a679dc3..bcbb75ed7 100644 --- a/src/deploy/zoneless.rs +++ b/src/deploy/zoneless.rs @@ -1,4 +1,4 @@ -use crate::commands::{subdomain::Subdomain, whoami::display_account_id_maybe}; +use crate::commands::subdomain::Subdomain; use crate::http; use crate::settings::global_user::GlobalUser; use crate::settings::toml::RouteConfig; @@ -13,18 +13,11 @@ pub struct ZonelessTarget { impl ZonelessTarget { pub fn build(script_name: &str, route_config: &RouteConfig) -> Result { - match route_config.account_id.as_ref() { - // TODO: Deserialize empty strings to None; cannot do this for account id - // yet without a large refactor. - Some(account_id) => Ok(Self { - script_name: script_name.to_string(), - account_id: account_id.to_string(), - }), - _ => { - display_account_id_maybe(); - anyhow::bail!("field `account_id` is required to deploy to workers.dev") - } - } + let account_id = route_config.account_id.load()?; + Ok(Self { + script_name: script_name.to_string(), + account_id: account_id.to_string(), + }) } pub fn deploy(&self, user: &GlobalUser) -> Result { diff --git a/src/reporter/mod.rs b/src/reporter/mod.rs index 9b534029b..b4c3a67a3 100644 --- a/src/reporter/mod.rs +++ b/src/reporter/mod.rs @@ -200,9 +200,14 @@ fn load_project_info() -> ProjectInfo { project_info .base .insert("script_name".into(), manifest.name); - project_info - .base - .insert("account_id".into(), manifest.account_id.unwrap_or_default()); + project_info.base.insert( + "account_id".into(), + manifest + .account_id + .if_present() + .cloned() + .unwrap_or_default(), + ); project_info.base.insert( "zone_id".into(), manifest.zone_id.unwrap_or_else(|| "".into()), diff --git a/src/settings/toml/environment.rs b/src/settings/toml/environment.rs index ad8796b71..6bca9a1c7 100644 --- a/src/settings/toml/environment.rs +++ b/src/settings/toml/environment.rs @@ -40,7 +40,7 @@ impl Environment { top_level_account_id: Option, top_level_zone_id: Option, ) -> Option { - let account_id = self.account_id.clone().or(top_level_account_id); + let account_id = self.account_id.clone().or(top_level_account_id).into(); let zone_id = self.zone_id.clone().or(top_level_zone_id); if self.workers_dev.is_none() && self.route.is_none() && self.routes.is_none() { diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index 790670186..15c97c62d 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -7,12 +7,15 @@ use std::str::FromStr; use config::{Config, File}; use anyhow::{anyhow, Result}; +use once_cell::unsync::OnceCell; use serde::{Deserialize, Serialize}; use serde_with::rust::string_empty_as_none; use super::UsageModel; +use crate::commands::whoami::fetch_accounts; use crate::commands::{validate_worker_name, whoami, DEFAULT_CONFIG_PATH}; use crate::deploy::{self, DeployTarget, DeploymentSet}; +use crate::settings::global_user::GlobalUser; use crate::settings::toml::builder::Builder; use crate::settings::toml::dev::Dev; use crate::settings::toml::durable_objects::DurableObjects; @@ -36,7 +39,7 @@ pub struct Manifest { #[serde(rename = "type")] pub target_type: TargetType, #[serde(default)] - pub account_id: Option, + pub account_id: LazyAccountId, pub workers_dev: Option, #[serde(default, with = "string_empty_as_none")] pub route: Option, @@ -239,7 +242,7 @@ impl Manifest { if let Some(env) = env { if let Some(env_route_cfg) = - env.route_config(self.account_id.clone(), self.zone_id.clone()) + env.route_config(self.account_id.if_present().cloned(), self.zone_id.clone()) { add_routed_deployments(&env_route_cfg) } else { @@ -256,7 +259,10 @@ impl Manifest { let crons = match env { Some(e) => { - let account_id = e.account_id.as_ref().or_else(|| self.account_id.as_ref()); + let account_id = e + .account_id + .as_ref() + .or_else(|| self.account_id.if_present()); e.triggers .as_ref() .or_else(|| self.triggers.as_ref()) @@ -265,7 +271,7 @@ impl Manifest { None => self .triggers .as_ref() - .map(|t| (t.crons.as_slice(), self.account_id.as_ref())), + .map(|t| (t.crons.as_slice(), self.account_id.if_present())), }; if let Some((crons, account)) = crons { @@ -288,21 +294,12 @@ impl Manifest { pub fn get_account_id(&self, environment_name: Option<&str>) -> Result { let environment = self.get_environment(environment_name)?; - let mut result = self.account_id.clone(); if let Some(environment) = environment { if let Some(account_id) = &environment.account_id { - result = Some(account_id.to_string()); - } - } - if let Some(id) = result { - Ok(id) - } else { - let mut msg = "Your configuration file is missing an account_id field".to_string(); - if let Some(environment_name) = environment_name { - msg.push_str(&format!(" in [env.{}]", environment_name)); + return Ok(account_id.to_string()); } - anyhow::bail!("{}", &msg) } + self.account_id.load().map(String::from) } pub fn get_target(&self, environment_name: Option<&str>, preview: bool) -> Result { @@ -340,10 +337,10 @@ impl Manifest { Not inherited: Must be defined for every environment individually. */ let mut target = Target { - target_type: self.target_type.clone(), // Top level - account_id: self.account_id.clone().unwrap_or_default(), // Inherited - webpack_config: self.webpack_config.clone(), // Inherited - build: self.build.clone(), // Inherited + target_type: self.target_type.clone(), // Top level + account_id: self.account_id.load()?.to_string(), // Inherited + webpack_config: self.webpack_config.clone(), // Inherited + build: self.build.clone(), // Inherited // importantly, the top level name will be modified // to include the name of the environment name: self.name.clone(), // Inherited @@ -512,6 +509,65 @@ impl Manifest { } } +#[derive(Debug, Clone, Default, PartialEq)] +pub struct LazyAccountId(OnceCell); + +impl Serialize for LazyAccountId { + fn serialize(&self, serializer: S) -> Result + where + S: serde::Serializer, + { + self.0.get().serialize(serializer) + } +} + +impl<'de> Deserialize<'de> for LazyAccountId { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + Ok(match Option::::deserialize(deserializer)? { + None => None, + Some(x) if x.is_empty() => None, + Some(x) => Some(x), + } + .into()) + } +} + +impl From> for LazyAccountId { + fn from(opt: Option) -> Self { + let cell = OnceCell::new(); + if let Some(val) = opt { + cell.set(val).unwrap(); + } + Self(cell) + } +} + +impl LazyAccountId { + /// Return the `account_id` in `wrangler.toml`, if present. + pub(crate) fn if_present(&self) -> Option<&String> { + self.0.get() + } + + /// Load the account ID, possibly prompting the user. + pub(crate) fn load(&self) -> Result<&String> { + self.0.get_or_try_init(|| { + if let Ok(user) = GlobalUser::new() { + let accounts = fetch_accounts(&user)?; + let account_id = match accounts.as_slice() { + [single] => single.id.clone(), + _ => todo!(), + }; + return Ok(account_id); + } + + todo!() + }) + } +} + impl FromStr for Manifest { type Err = toml::de::Error; diff --git a/src/settings/toml/route.rs b/src/settings/toml/route.rs index 45505cb76..53b1cd5ea 100644 --- a/src/settings/toml/route.rs +++ b/src/settings/toml/route.rs @@ -2,6 +2,8 @@ use serde::{Deserialize, Serialize}; use cloudflare::endpoints::workers::WorkersRoute; +use super::manifest::LazyAccountId; + #[derive(Clone, Debug, Deserialize, PartialEq, Serialize)] pub struct Route { pub id: Option, @@ -25,7 +27,7 @@ pub struct RouteConfig { pub route: Option, pub routes: Option>, pub zone_id: Option, - pub account_id: Option, + pub account_id: LazyAccountId, } impl RouteConfig { From a86b7a6807bd953e5559438d6ea2a6bcf16340a1 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 23 Jun 2021 11:27:03 -0400 Subject: [PATCH 05/10] Handle multiple accounts --- src/settings/toml/manifest.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index 15c97c62d..0b4fcf7da 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -557,9 +557,15 @@ impl LazyAccountId { if let Ok(user) = GlobalUser::new() { let accounts = fetch_accounts(&user)?; let account_id = match accounts.as_slice() { + [] => unreachable!("auth token without account?"), [single] => single.id.clone(), - _ => todo!(), + _multiple => { + StdOut::user_error("You have multiple accounts."); + whoami::display_account_id_maybe(); + anyhow::bail!("field `account_id` is required") + } }; + return Ok(account_id); } From 685a2473ae2bbb302c31bbff1d58cb35074abab9 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 23 Jun 2021 12:59:58 -0400 Subject: [PATCH 06/10] Load account lazily in Target too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This avoid the following test failure: ``` it_builds_with_webpack_target_webworker_with_custom_file stdout ---- Created fixture at /tmp/.tmpSzLIJb thread 'it_builds_with_webpack_target_webworker_with_custom_file' panicked at 'Build failed: Output { status: ExitStatus(ExitStatus(256)), stdout: "šŸ‘€ You have multiple accounts.\nPlease choose one from below and add it to `wrangler.toml` under `account_id`.\nšŸ•µ\u{fe0f} You can copy your account_id below\n+--------------------------+----------------------------------+\n| Account Name | Account ID |\n+--------------------------+----------------------------------+\n| Cloudflare Community Jam | 6174b0f52802f24a9f52e7015282898c |\n+--------------------------+----------------------------------+\n| Edge Workers | 615f1f0479e7014f0bebcd10d379f10e |\n+--------------------------+----------------------------------+\n", stderr: "Error: \n" }', tests/build.rs:377:5 ``` It also prevents any similar issues that aren't tested, by forcing each command that wants an account ID to load it explicitly. --- src/commands/dev/edge/setup.rs | 11 +++++---- src/commands/kv/key/delete.rs | 2 +- src/commands/kv/key/get.rs | 2 +- src/commands/kv/key/put.rs | 2 +- src/commands/kv/mod.rs | 28 ++++++++++----------- src/commands/kv/namespace/delete.rs | 2 +- src/commands/secret.rs | 38 ++++++++++++++--------------- src/commands/subdomain.rs | 15 ++++-------- src/kv/bulk.rs | 4 +-- src/kv/key.rs | 2 +- src/kv/namespace/delete.rs | 6 ++--- src/kv/namespace/list.rs | 2 +- src/kv/namespace/upsert.rs | 2 +- src/preview/upload.rs | 6 ++--- src/settings/toml/manifest.rs | 12 ++++----- src/settings/toml/target.rs | 3 ++- src/sites/mod.rs | 2 +- src/tail/session.rs | 6 ++--- src/upload/mod.rs | 6 +++-- 19 files changed, 73 insertions(+), 78 deletions(-) diff --git a/src/commands/dev/edge/setup.rs b/src/commands/dev/edge/setup.rs index bda55a099..618f652ab 100644 --- a/src/commands/dev/edge/setup.rs +++ b/src/commands/dev/edge/setup.rs @@ -41,7 +41,7 @@ pub(super) fn upload( }; let session_config = get_session_config(deploy_target); - let address = get_upload_address(target); + let address = get_upload_address(target)?; let script_upload_form = upload::form::build(target, asset_manifest, Some(session_config))?; @@ -144,11 +144,12 @@ fn get_session_address(target: &DeployTarget) -> String { } } -fn get_upload_address(target: &mut Target) -> String { - format!( +fn get_upload_address(target: &mut Target) -> Result { + Ok(format!( "https://api.cloudflare.com/client/v4/accounts/{}/workers/scripts/{}/edge-preview", - target.account_id, target.name - ) + target.account_id.load()?, + target.name + )) } fn get_exchange_url(deploy_target: &DeployTarget, user: &GlobalUser) -> Result { diff --git a/src/commands/kv/key/delete.rs b/src/commands/kv/key/delete.rs index 2165f7c0a..771829a65 100644 --- a/src/commands/kv/key/delete.rs +++ b/src/commands/kv/key/delete.rs @@ -26,7 +26,7 @@ pub fn delete(target: &Target, user: &GlobalUser, id: &str, key: &str) -> Result StdOut::working(&msg); let response = client.request(&DeleteKey { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, namespace_identifier: id, key, // this is url encoded within cloudflare-rs }); diff --git a/src/commands/kv/key/get.rs b/src/commands/kv/key/get.rs index bf74fcfa6..485b83bea 100644 --- a/src/commands/kv/key/get.rs +++ b/src/commands/kv/key/get.rs @@ -17,7 +17,7 @@ pub fn get(target: &Target, user: &GlobalUser, id: &str, key: &str) -> Result<() kv::validate_target(target)?; let api_endpoint = format!( "https://api.cloudflare.com/client/v4/accounts/{}/storage/kv/namespaces/{}/values/{}", - target.account_id, + target.account_id.load()?, id, kv::url_encode_key(key) ); diff --git a/src/commands/kv/key/put.rs b/src/commands/kv/key/put.rs index 3b5e492e3..486e5e359 100644 --- a/src/commands/kv/key/put.rs +++ b/src/commands/kv/key/put.rs @@ -53,7 +53,7 @@ pub fn put(target: &Target, user: &GlobalUser, data: KVMetaData) -> Result<()> { let api_endpoint = format!( "https://api.cloudflare.com/client/v4/accounts/{}/storage/kv/namespaces/{}/values/{}", - target.account_id, + target.account_id.load()?, &data.namespace_id, kv::url_encode_key(&data.key) ); diff --git a/src/commands/kv/mod.rs b/src/commands/kv/mod.rs index 508956389..d59f4108e 100644 --- a/src/commands/kv/mod.rs +++ b/src/commands/kv/mod.rs @@ -40,20 +40,20 @@ fn kv_help(error_code: u16) -> &'static str { } pub fn validate_target(target: &Target) -> Result<()> { - let mut missing_fields = Vec::new(); - - if target.account_id.is_empty() { - missing_fields.push("account_id") - }; - - if !missing_fields.is_empty() { - anyhow::bail!( - "Your configuration file is missing the following field(s): {:?}", - missing_fields - ) - } else { + // let mut missing_fields = Vec::new(); + + // if target.account_id.is_empty() { + // missing_fields.push("account_id") + // }; + + // if !missing_fields.is_empty() { + // anyhow::bail!( + // "Your configuration file is missing the following field(s): {:?}", + // missing_fields + // ) + // } else { Ok(()) - } + // } } fn check_duplicate_namespaces(target: &Target) -> bool { @@ -109,7 +109,7 @@ mod tests { #[test] fn it_can_detect_duplicate_bindings() { let target_with_dup_kv_bindings = Target { - account_id: "".to_string(), + account_id: None.into(), kv_namespaces: vec![ KvNamespace { id: "fake".to_string(), diff --git a/src/commands/kv/namespace/delete.rs b/src/commands/kv/namespace/delete.rs index 72be7c1bb..aa6cda586 100644 --- a/src/commands/kv/namespace/delete.rs +++ b/src/commands/kv/namespace/delete.rs @@ -27,7 +27,7 @@ pub fn run(target: &Target, user: &GlobalUser, id: &str) -> Result<()> { let msg = format!("Deleting namespace {}", id); StdOut::working(&msg); - let response = delete(client, target, id); + let response = delete(client, target.account_id.load()?, id); match response { Ok(_) => { StdOut::success("Success"); diff --git a/src/commands/secret.rs b/src/commands/secret.rs index 6e8ca19f4..18d75fb6d 100644 --- a/src/commands/secret.rs +++ b/src/commands/secret.rs @@ -7,8 +7,8 @@ use anyhow::Result; use crate::http; use crate::settings::global_user::GlobalUser; use crate::settings::toml::Target; +use crate::terminal::interactive; use crate::terminal::message::{Message, StdOut}; -use crate::terminal::{emoji, interactive}; use crate::upload; fn format_error(e: ApiFailure) -> String { @@ -16,21 +16,21 @@ fn format_error(e: ApiFailure) -> String { } fn validate_target(target: &Target) -> Result<()> { - let mut missing_fields = Vec::new(); - - if target.account_id.is_empty() { - missing_fields.push("account_id") - }; - - if !missing_fields.is_empty() { - anyhow::bail!( - "{} Your configuration file is missing the following field(s): {:?}", - emoji::WARN, - missing_fields - ) - } else { + // let mut missing_fields = Vec::new(); + + // if target.account_id.is_empty() { + // missing_fields.push("account_id") + // }; + + // if !missing_fields.is_empty() { + // anyhow::bail!( + // "{} Your configuration file is missing the following field(s): {:?}", + // emoji::WARN, + // missing_fields + // ) + // } else { Ok(()) - } + // } } // secret_errors() provides more detailed explanations of API error codes. @@ -96,7 +96,7 @@ pub fn create_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<( }; let response = client.request(&CreateSecret { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, script_name: &target.name, params: params.clone(), }); @@ -108,7 +108,7 @@ pub fn create_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<( Some(draft_upload_response) => match draft_upload_response { Ok(_) => { let retry_response = client.request(&CreateSecret { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, script_name: &target.name, params, }); @@ -149,7 +149,7 @@ pub fn delete_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<( let client = http::cf_v4_client(user)?; let response = client.request(&DeleteSecret { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, script_name: &target.name, secret_name: &name, }); @@ -167,7 +167,7 @@ pub fn list_secrets(user: &GlobalUser, target: &Target) -> Result<()> { let client = http::cf_v4_client(user)?; let response = client.request(&ListSecrets { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, script_name: &target.name, }); diff --git a/src/commands/subdomain.rs b/src/commands/subdomain.rs index 0930907e8..1d243c076 100644 --- a/src/commands/subdomain.rs +++ b/src/commands/subdomain.rs @@ -114,17 +114,12 @@ fn register_subdomain(name: &str, user: &GlobalUser, target: &Target) -> Result< name ); StdOut::working(&msg); - Subdomain::put(name, &target.account_id, user) + Subdomain::put(name, target.account_id.load()?, user) } pub fn set_subdomain(name: &str, user: &GlobalUser, target: &Target) -> Result<()> { - if target.account_id.is_empty() { - anyhow::bail!( - "{} You must provide an account_id in your configuration file before creating a subdomain!", - emoji::WARN - ) - } - let subdomain = Subdomain::get(&target.account_id, user)?; + let account_id = target.account_id.load()?; + let subdomain = Subdomain::get(account_id, user)?; if let Some(subdomain) = subdomain { if subdomain == name { let msg = format!("You have already registered {}.workers.dev", subdomain); @@ -132,7 +127,7 @@ pub fn set_subdomain(name: &str, user: &GlobalUser, target: &Target) -> Result<( return Ok(()); } else { // list all the affected scripts - let scripts = get_subdomain_scripts(&target.account_id, user)?; + let scripts = get_subdomain_scripts(account_id, user)?; let default_msg = format!("Are you sure you want to permanently move your subdomain from {}.workers.dev to {}.workers.dev?", subdomain, name); @@ -168,7 +163,7 @@ pub fn set_subdomain(name: &str, user: &GlobalUser, target: &Target) -> Result<( } pub fn get_subdomain(user: &GlobalUser, target: &Target) -> Result<()> { - let subdomain = Subdomain::get(&target.account_id, user)?; + let subdomain = Subdomain::get(target.account_id.load()?, user)?; if let Some(subdomain) = subdomain { let msg = format!("{}.workers.dev", subdomain); StdOut::info(&msg); diff --git a/src/kv/bulk.rs b/src/kv/bulk.rs index c1f739ddf..76a0f7025 100644 --- a/src/kv/bulk.rs +++ b/src/kv/bulk.rs @@ -47,7 +47,7 @@ pub fn put( for b in batch_keys_values(pairs) { match client.request(&WriteBulk { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, namespace_identifier: namespace_id, bulk_key_value_pairs: b.to_owned(), }) { @@ -74,7 +74,7 @@ pub fn delete( for b in batch_keys(keys) { match client.request(&DeleteBulk { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, namespace_identifier: namespace_id, bulk_keys: b.to_owned(), }) { diff --git a/src/kv/key.rs b/src/kv/key.rs index da8274cb6..880a045da 100644 --- a/src/kv/key.rs +++ b/src/kv/key.rs @@ -31,7 +31,7 @@ impl KeyList { keys_result: None, prefix: prefix.map(str::to_string), client, - account_id: target.account_id.to_owned(), + account_id: target.account_id.load()?.to_owned(), namespace_id: namespace_id.to_string(), cursor: None, init_fetch: false, diff --git a/src/kv/namespace/delete.rs b/src/kv/namespace/delete.rs index 6dbe0fe33..830a934e8 100644 --- a/src/kv/namespace/delete.rs +++ b/src/kv/namespace/delete.rs @@ -3,15 +3,13 @@ use cloudflare::framework::apiclient::ApiClient; use cloudflare::framework::response::{ApiFailure, ApiSuccess}; use cloudflare::framework::HttpApiClient; -use crate::settings::toml::Target; - pub fn delete( client: HttpApiClient, - target: &Target, + account_id: &str, id: &str, ) -> Result, ApiFailure> { client.request(&RemoveNamespace { - account_identifier: &target.account_id, + account_identifier: account_id, namespace_identifier: id, }) } diff --git a/src/kv/namespace/list.rs b/src/kv/namespace/list.rs index b5885f7e7..cc6b8f06b 100644 --- a/src/kv/namespace/list.rs +++ b/src/kv/namespace/list.rs @@ -23,7 +23,7 @@ pub fn list(client: &impl ApiClient, target: &Target) -> Result { diff --git a/src/kv/namespace/upsert.rs b/src/kv/namespace/upsert.rs index 2a20f2032..b29bd3fc8 100644 --- a/src/kv/namespace/upsert.rs +++ b/src/kv/namespace/upsert.rs @@ -17,7 +17,7 @@ pub enum UpsertedNamespace { pub fn upsert(target: &Target, user: &GlobalUser, title: String) -> Result { let client = http::cf_v4_client(user)?; - let response = create(&client, &target.account_id, &title); + let response = create(&client, target.account_id.load()?, &title); match response { Ok(success) => Ok(UpsertedNamespace::Created(success.result)), diff --git a/src/preview/upload.rs b/src/preview/upload.rs index 6f43600e2..02c1bfd22 100644 --- a/src/preview/upload.rs +++ b/src/preview/upload.rs @@ -123,9 +123,6 @@ pub fn upload( fn validate(target: &Target) -> Vec<&str> { let mut missing_fields = Vec::new(); - if target.account_id.is_empty() { - missing_fields.push("account_id") - }; if target.name.is_empty() { missing_fields.push("name") }; @@ -150,7 +147,8 @@ fn authenticated_upload( ) -> Result { let create_address = format!( "https://api.cloudflare.com/client/v4/accounts/{}/workers/scripts/{}/preview", - target.account_id, target.name + target.account_id.load()?, + target.name ); log::info!("address: {}", create_address); diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index 0b4fcf7da..9f59de720 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -7,7 +7,7 @@ use std::str::FromStr; use config::{Config, File}; use anyhow::{anyhow, Result}; -use once_cell::unsync::OnceCell; +use once_cell::sync::OnceCell; use serde::{Deserialize, Serialize}; use serde_with::rust::string_empty_as_none; @@ -337,10 +337,10 @@ impl Manifest { Not inherited: Must be defined for every environment individually. */ let mut target = Target { - target_type: self.target_type.clone(), // Top level - account_id: self.account_id.load()?.to_string(), // Inherited - webpack_config: self.webpack_config.clone(), // Inherited - build: self.build.clone(), // Inherited + target_type: self.target_type.clone(), // Top level + account_id: self.account_id.clone(), // Inherited + webpack_config: self.webpack_config.clone(), // Inherited + build: self.build.clone(), // Inherited // importantly, the top level name will be modified // to include the name of the environment name: self.name.clone(), // Inherited @@ -359,7 +359,7 @@ impl Manifest { if let Some(environment) = environment { target.name = self.worker_name(environment_name); if let Some(account_id) = &environment.account_id { - target.account_id = account_id.clone(); + target.account_id = Some(account_id.clone()).into(); } if let Some(webpack_config) = &environment.webpack_config { target.webpack_config = Some(webpack_config.clone()); diff --git a/src/settings/toml/target.rs b/src/settings/toml/target.rs index f3e1d3225..b0c3e7b2f 100644 --- a/src/settings/toml/target.rs +++ b/src/settings/toml/target.rs @@ -1,5 +1,6 @@ use super::durable_objects::DurableObjects; use super::kv_namespace::KvNamespace; +use super::manifest::LazyAccountId; use super::site::Site; use super::target_type::TargetType; use super::UsageModel; @@ -12,7 +13,7 @@ use std::path::PathBuf; #[derive(Clone, Debug, Default)] pub struct Target { - pub account_id: String, + pub account_id: LazyAccountId, pub kv_namespaces: Vec, pub durable_objects: Option, pub migrations: Option, diff --git a/src/sites/mod.rs b/src/sites/mod.rs index 2160e37ec..319a1951d 100644 --- a/src/sites/mod.rs +++ b/src/sites/mod.rs @@ -318,7 +318,7 @@ mod tests { fn make_target(site: Site) -> Target { Target { - account_id: "".to_string(), + account_id: None.into(), kv_namespaces: Vec::new(), durable_objects: None, migrations: None, diff --git a/src/tail/session.rs b/src/tail/session.rs index 49e17fa5c..72fb558d1 100644 --- a/src/tail/session.rs +++ b/src/tail/session.rs @@ -50,7 +50,7 @@ impl Session { let client = http::cf_v4_api_client_async(&user, HttpApiClientConfig::default())?; client .request(&DeleteTail { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, script_name: &target.name, tail_id: &tail_id, }) @@ -82,7 +82,7 @@ impl Session { let url = Some(url_); let response = client .request(&CreateTail { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, script_name: &target.name, params: CreateTailParams { url }, }) @@ -137,7 +137,7 @@ async fn get_tunnel_url(metrics_port: u16) -> Result { async fn send_heartbeat(target: &Target, client: &async_api::Client, tail_id: &str) -> Result<()> { let response = client .request(&SendTailHeartbeat { - account_identifier: &target.account_id, + account_identifier: target.account_id.load()?, script_name: &target.name, tail_id, }) diff --git a/src/upload/mod.rs b/src/upload/mod.rs index 696cffe1c..420cfe74d 100644 --- a/src/upload/mod.rs +++ b/src/upload/mod.rs @@ -18,7 +18,8 @@ pub fn script( ) -> Result<()> { let worker_addr = format!( "https://api.cloudflare.com/client/v4/accounts/{}/workers/scripts/{}", - target.account_id, target.name, + target.account_id.load()?, + target.name, ); let script_upload_form = form::build(target, asset_manifest, None)?; @@ -42,7 +43,8 @@ pub fn script( if let Some(usage_model) = target.usage_model { let addr = format!( "https://api.cloudflare.com/client/v4/accounts/{}/workers/scripts/{}/usage-model", - target.account_id, target.name, + target.account_id.load()?, + target.name, ); let res = client From 52e65ae4d9890c88264ec415f8044225798c8937 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 23 Jun 2021 14:03:58 -0400 Subject: [PATCH 07/10] Fix previews Previews have different behavior when an account is available than when it isn't. This restores that previous behavior: if an account is available, use authenticated previews, otherwise unauthenticated previews. Note that "an account is available" has become a slightly tricky question, since it's no longer guaranteed that it will be in the wrangler.toml. Instead, this checks if it is either in the .toml or if there is only a single account available to the authentication token. --- src/preview/upload.rs | 3 +++ src/settings/toml/manifest.rs | 23 +++++++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/src/preview/upload.rs b/src/preview/upload.rs index 02c1bfd22..491d27b2e 100644 --- a/src/preview/upload.rs +++ b/src/preview/upload.rs @@ -123,6 +123,9 @@ pub fn upload( fn validate(target: &Target) -> Vec<&str> { let mut missing_fields = Vec::new(); + if target.account_id.maybe_load().is_none() { + missing_fields.push("account_id") + }; if target.name.is_empty() { missing_fields.push("name") }; diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index 9f59de720..d21ad6411 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -547,10 +547,33 @@ impl From> for LazyAccountId { impl LazyAccountId { /// Return the `account_id` in `wrangler.toml`, if present. + /// + /// Use this with caution; prefer `maybe_load` instead where possible. pub(crate) fn if_present(&self) -> Option<&String> { self.0.get() } + /// If `account_id` can be inferred automatically, do so; + /// otherwise, return `None`. + /// + /// Note that *unlike* `load`, this will never prompt the user or warn. + pub(crate) fn maybe_load(&self) -> Option { + if let Some(id) = self.0.get() { + return Some(id.to_owned()); + } + + if let Some(mut accounts) = GlobalUser::new() + .ok() + .and_then(|user| fetch_accounts(&user).ok()) + { + if accounts.len() == 1 { + return Some(accounts.pop().unwrap().id); + } + } + + None + } + /// Load the account ID, possibly prompting the user. pub(crate) fn load(&self) -> Result<&String> { self.0.get_or_try_init(|| { From 1ee1f42c284b2cf596143b064fb123240061e926 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 23 Jun 2021 15:16:36 -0400 Subject: [PATCH 08/10] Use `maybe_load` instead of `if_present` for `load_project_info` This only affects `wrangler report`. The old behavior preserved the behavior from before, but was less helpful because it wouldn't include the ID if there was only a single ID for the account token. --- src/reporter/mod.rs | 6 +----- src/settings/toml/manifest.rs | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/reporter/mod.rs b/src/reporter/mod.rs index b4c3a67a3..d3912207e 100644 --- a/src/reporter/mod.rs +++ b/src/reporter/mod.rs @@ -202,11 +202,7 @@ fn load_project_info() -> ProjectInfo { .insert("script_name".into(), manifest.name); project_info.base.insert( "account_id".into(), - manifest - .account_id - .if_present() - .cloned() - .unwrap_or_default(), + manifest.account_id.maybe_load().unwrap_or_default(), ); project_info.base.insert( "zone_id".into(), diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index d21ad6411..34380d6a5 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -549,7 +549,7 @@ impl LazyAccountId { /// Return the `account_id` in `wrangler.toml`, if present. /// /// Use this with caution; prefer `maybe_load` instead where possible. - pub(crate) fn if_present(&self) -> Option<&String> { + fn if_present(&self) -> Option<&String> { self.0.get() } From dafdc2953e7bc705acbabaf5fdd06ce71da32436 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Wed, 23 Jun 2021 14:14:57 -0400 Subject: [PATCH 09/10] Remove unneeded `validate` function It no longer does anything now that the account ID is loaded on demand. --- src/commands/kv/bulk/delete.rs | 4 +--- src/commands/kv/bulk/put.rs | 3 --- src/commands/kv/key/delete.rs | 3 +-- src/commands/kv/key/get.rs | 1 - src/commands/kv/key/list.rs | 1 - src/commands/kv/key/put.rs | 2 -- src/commands/kv/mod.rs | 17 ----------------- src/commands/kv/namespace/delete.rs | 1 - src/commands/kv/namespace/list.rs | 3 --- src/commands/secret.rs | 23 ----------------------- src/sites/sync.rs | 1 - 11 files changed, 2 insertions(+), 57 deletions(-) diff --git a/src/commands/kv/bulk/delete.rs b/src/commands/kv/bulk/delete.rs index 1295c2ff4..d0ee3b008 100644 --- a/src/commands/kv/bulk/delete.rs +++ b/src/commands/kv/bulk/delete.rs @@ -9,16 +9,14 @@ use cloudflare::endpoints::workerskv::write_bulk::KeyValuePair; use anyhow::Result; use indicatif::{ProgressBar, ProgressStyle}; -use crate::commands::kv; use crate::kv::bulk::delete; use crate::kv::bulk::BATCH_KEY_MAX; use crate::settings::global_user::GlobalUser; use crate::settings::toml::Target; use crate::terminal::interactive; use crate::terminal::message::{Message, StdOut}; -pub fn run(target: &Target, user: &GlobalUser, namespace_id: &str, filename: &Path) -> Result<()> { - kv::validate_target(target)?; +pub fn run(target: &Target, user: &GlobalUser, namespace_id: &str, filename: &Path) -> Result<()> { match interactive::confirm(&format!( "Are you sure you want to delete all keys in {}?", filename.display() diff --git a/src/commands/kv/bulk/put.rs b/src/commands/kv/bulk/put.rs index d37aa109b..6d81f174d 100644 --- a/src/commands/kv/bulk/put.rs +++ b/src/commands/kv/bulk/put.rs @@ -9,15 +9,12 @@ use cloudflare::endpoints::workerskv::write_bulk::KeyValuePair; use anyhow::{anyhow, Result}; use indicatif::{ProgressBar, ProgressStyle}; -use crate::commands::kv::validate_target; use crate::kv::bulk::put; use crate::kv::bulk::BATCH_KEY_MAX; use crate::settings::global_user::GlobalUser; use crate::settings::toml::Target; use crate::terminal::message::{Message, StdErr}; pub fn run(target: &Target, user: &GlobalUser, namespace_id: &str, filename: &Path) -> Result<()> { - validate_target(target)?; - let pairs: Vec = match &metadata(filename) { Ok(file_type) if file_type.is_file() => { let data = fs::read_to_string(filename)?; diff --git a/src/commands/kv/key/delete.rs b/src/commands/kv/key/delete.rs index 771829a65..82d1475fd 100644 --- a/src/commands/kv/key/delete.rs +++ b/src/commands/kv/key/delete.rs @@ -3,14 +3,13 @@ use cloudflare::framework::apiclient::ApiClient; use anyhow::Result; -use crate::commands::kv::{format_error, validate_target}; +use crate::commands::kv::format_error; use crate::http; use crate::settings::global_user::GlobalUser; use crate::settings::toml::Target; use crate::terminal::interactive; use crate::terminal::message::{Message, StdOut}; pub fn delete(target: &Target, user: &GlobalUser, id: &str, key: &str) -> Result<()> { - validate_target(target)?; let client = http::cf_v4_client(user)?; match interactive::confirm(&format!("Are you sure you want to delete key \"{}\"?", key)) { diff --git a/src/commands/kv/key/get.rs b/src/commands/kv/key/get.rs index 485b83bea..1040243cf 100644 --- a/src/commands/kv/key/get.rs +++ b/src/commands/kv/key/get.rs @@ -14,7 +14,6 @@ use crate::settings::toml::Target; use std::io::{self, Write}; pub fn get(target: &Target, user: &GlobalUser, id: &str, key: &str) -> Result<()> { - kv::validate_target(target)?; let api_endpoint = format!( "https://api.cloudflare.com/client/v4/accounts/{}/storage/kv/namespaces/{}/values/{}", target.account_id.load()?, diff --git a/src/commands/kv/key/list.rs b/src/commands/kv/key/list.rs index d39da7d60..8b40a903d 100644 --- a/src/commands/kv/key/list.rs +++ b/src/commands/kv/key/list.rs @@ -17,7 +17,6 @@ pub fn list( namespace_id: &str, prefix: Option<&str>, ) -> Result<()> { - kv::validate_target(target)?; let client = http::cf_v4_client(&user)?; let key_list = KeyList::new(target, client, namespace_id, prefix)?; diff --git a/src/commands/kv/key/put.rs b/src/commands/kv/key/put.rs index 486e5e359..a3ac8273e 100644 --- a/src/commands/kv/key/put.rs +++ b/src/commands/kv/key/put.rs @@ -49,8 +49,6 @@ pub fn parse_metadata(arg: Option<&str>) -> Result> { } pub fn put(target: &Target, user: &GlobalUser, data: KVMetaData) -> Result<()> { - kv::validate_target(target)?; - let api_endpoint = format!( "https://api.cloudflare.com/client/v4/accounts/{}/storage/kv/namespaces/{}/values/{}", target.account_id.load()?, diff --git a/src/commands/kv/mod.rs b/src/commands/kv/mod.rs index d59f4108e..38bfeb99c 100644 --- a/src/commands/kv/mod.rs +++ b/src/commands/kv/mod.rs @@ -39,23 +39,6 @@ fn kv_help(error_code: u16) -> &'static str { } } -pub fn validate_target(target: &Target) -> Result<()> { - // let mut missing_fields = Vec::new(); - - // if target.account_id.is_empty() { - // missing_fields.push("account_id") - // }; - - // if !missing_fields.is_empty() { - // anyhow::bail!( - // "Your configuration file is missing the following field(s): {:?}", - // missing_fields - // ) - // } else { - Ok(()) - // } -} - fn check_duplicate_namespaces(target: &Target) -> bool { // HashSet for detecting duplicate namespace bindings let mut binding_names: HashSet = HashSet::new(); diff --git a/src/commands/kv/namespace/delete.rs b/src/commands/kv/namespace/delete.rs index aa6cda586..e08f8a7e3 100644 --- a/src/commands/kv/namespace/delete.rs +++ b/src/commands/kv/namespace/delete.rs @@ -9,7 +9,6 @@ use crate::terminal::message::{Message, StdOut}; use anyhow::Result; pub fn run(target: &Target, user: &GlobalUser, id: &str) -> Result<()> { - kv::validate_target(target)?; let client = http::cf_v4_client(user)?; match interactive::confirm(&format!( diff --git a/src/commands/kv/namespace/list.rs b/src/commands/kv/namespace/list.rs index 5d5b8a1b2..f9b811c1b 100644 --- a/src/commands/kv/namespace/list.rs +++ b/src/commands/kv/namespace/list.rs @@ -1,4 +1,3 @@ -use crate::commands::kv; use crate::http; use crate::kv::namespace::list; use crate::settings::global_user::GlobalUser; @@ -7,8 +6,6 @@ use crate::settings::toml::Target; use anyhow::Result; pub fn run(target: &Target, user: &GlobalUser) -> Result<()> { - kv::validate_target(target)?; - let client = http::cf_v4_client(user)?; let result = list(&client, target); match result { diff --git a/src/commands/secret.rs b/src/commands/secret.rs index 18d75fb6d..75e3d1234 100644 --- a/src/commands/secret.rs +++ b/src/commands/secret.rs @@ -15,24 +15,6 @@ fn format_error(e: ApiFailure) -> String { http::format_error(e, Some(&secret_errors)) } -fn validate_target(target: &Target) -> Result<()> { - // let mut missing_fields = Vec::new(); - - // if target.account_id.is_empty() { - // missing_fields.push("account_id") - // }; - - // if !missing_fields.is_empty() { - // anyhow::bail!( - // "{} Your configuration file is missing the following field(s): {:?}", - // emoji::WARN, - // missing_fields - // ) - // } else { - Ok(()) - // } -} - // secret_errors() provides more detailed explanations of API error codes. fn secret_errors(error_code: u16) -> &'static str { match error_code { @@ -71,8 +53,6 @@ pub fn upload_draft_worker( } pub fn create_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<()> { - validate_target(target)?; - let secret_value = interactive::get_user_input_multi_line(&format!( "Enter the secret text you'd like assigned to the variable {} on the script named {}:", name, target.name @@ -127,8 +107,6 @@ pub fn create_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<( } pub fn delete_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<()> { - validate_target(target)?; - match interactive::confirm(&format!( "Are you sure you want to permanently delete the variable {} on the script named {}?", name, target.name @@ -163,7 +141,6 @@ pub fn delete_secret(name: &str, user: &GlobalUser, target: &Target) -> Result<( } pub fn list_secrets(user: &GlobalUser, target: &Target) -> Result<()> { - validate_target(target)?; let client = http::cf_v4_client(user)?; let response = client.request(&ListSecrets { diff --git a/src/sites/sync.rs b/src/sites/sync.rs index 9d6960e00..04aec4dfe 100644 --- a/src/sites/sync.rs +++ b/src/sites/sync.rs @@ -19,7 +19,6 @@ pub fn sync( namespace_id: &str, path: &Path, ) -> Result<(Vec, Vec, AssetManifest)> { - kv::validate_target(target)?; // First, find all changed files in given local directory (aka files that are now stale // in Workers KV). From 47eecb3d5ce569a3267e145b38527fcc5887bb97 Mon Sep 17 00:00:00 2001 From: Joshua Nelson Date: Mon, 28 Jun 2021 17:09:45 -0400 Subject: [PATCH 10/10] Don't tell the user to add `account_id` after running `wrangler generate` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before, wrangler would give this message: ``` > wrangler generate Creating project called `worker`... Done! New project created /home/jnelson/src/test-project/worker/worker šŸ•µļø You can find your zone_id in the right sidebar of a zone's overview tab at https://dash.cloudflare.com šŸ•µļø You can copy your account_id below +----------------------------+----------------------------------+ | Account Name | Account ID | +----------------------------+----------------------------------+ | Jyn514@gmail.com's Account | e1706d218241c1230155b4f91b3218af | +----------------------------+----------------------------------+ šŸ•µļø You will need to update the following fields in the created wrangler.toml file before continuing: - account_id ``` Adding the account ID is no longer necessary, so don't suggest adding it. --- src/settings/toml/manifest.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index 34380d6a5..55efbbde5 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -414,9 +414,6 @@ impl Manifest { let account_id_env = env::var("CF_ACCOUNT_ID").is_ok(); let zone_id_env = env::var("CF_ZONE_ID").is_ok(); let mut top_level_fields: Vec = Vec::new(); - if !account_id_env { - top_level_fields.push("account_id".to_string()); - } if let Some(kv_namespaces) = &self.kv_namespaces { for kv_namespace in kv_namespaces { top_level_fields.push(format!(