From 9eb0a106f297200d8b971f1ca560366600acf2fe Mon Sep 17 00:00:00 2001 From: jspspike Date: Tue, 10 Aug 2021 13:26:38 -0500 Subject: [PATCH 1/5] Changed having no routes from fatal to warning --- src/settings/toml/manifest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index f326df307..205755a15 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -289,7 +289,7 @@ impl Manifest { }; if durable_objects.is_none() && deployments.is_empty() { - anyhow::bail!("Please specify your deployment routes or `wrangler_dev = true` inside of your configuration file. For more information, see: https://developers.cloudflare.com/workers/cli-wrangler/configuration#keys") + StdOut::warn("No depolyment routes specified, worker will not be triggered. Please specify your deployment routes or set `wrangler_dev = true` inside of your configuration file in order to trigger your worker. For more information, see: https://developers.cloudflare.com/workers/cli-wrangler/configuration#keys"); } Ok(deployments) From a68af0bf70e0ba19c06f5fd330230fa1a51e3556 Mon Sep 17 00:00:00 2001 From: jspspike Date: Thu, 12 Aug 2021 14:23:39 -0500 Subject: [PATCH 2/5] Allowed scripts with zone and no routes to published --- src/deploy/zoned.rs | 4 --- src/settings/toml/environment.rs | 6 ++++- src/settings/toml/manifest.rs | 16 ++++++++++++ src/settings/toml/tests/deployments.rs | 36 +++++++++++++------------- tests/fixtures/wrangler_toml.rs | 8 ++++-- 5 files changed, 45 insertions(+), 25 deletions(-) diff --git a/src/deploy/zoned.rs b/src/deploy/zoned.rs index e241338f0..47fcab93f 100644 --- a/src/deploy/zoned.rs +++ b/src/deploy/zoned.rs @@ -40,10 +40,6 @@ impl ZonedTarget { })) .collect(); - if routes.is_empty() { - anyhow::bail!("No routes specified"); - } - Ok(Self { zone_id: zone_id.to_owned(), routes, diff --git a/src/settings/toml/environment.rs b/src/settings/toml/environment.rs index 6bca9a1c7..1fdc37d10 100644 --- a/src/settings/toml/environment.rs +++ b/src/settings/toml/environment.rs @@ -43,7 +43,11 @@ impl Environment { 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() { + if self.workers_dev.is_none() + && self.zone_id.is_none() + && self.route.is_none() + && self.routes.is_none() + { None } else { Some(RouteConfig { diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index 205755a15..0fc844f2e 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -215,6 +215,11 @@ impl Manifest { let mut add_routed_deployments = |route_config: &RouteConfig| -> Result<()> { if route_config.is_zoned() { let zoned = deploy::ZonedTarget::build(&script, route_config)?; + + if zoned.routes.is_empty() { + return Ok(()); + } + // This checks all of the configured routes for the wildcard ending and warns // the user that their site may not work as expected without it. if self.site.is_some() { @@ -240,6 +245,17 @@ impl Manifest { deployments.push(DeployTarget::Zoneless(zoneless)); } + if !route_config.is_zoned() && !route_config.is_zoneless() { + if route_config.route.is_some() + || (route_config.routes.is_some() + && !route_config.routes.as_ref().unwrap().is_empty()) + { + anyhow::bail!( + "Routes specified with no zone, specify `zone_id` in your wrangler.toml" + ) + } + } + Ok(()) }; diff --git a/src/settings/toml/tests/deployments.rs b/src/settings/toml/tests/deployments.rs index fea16c4a9..625f34023 100644 --- a/src/settings/toml/tests/deployments.rs +++ b/src/settings/toml/tests/deployments.rs @@ -20,7 +20,7 @@ fn it_errors_on_empty_get_deployments() { let environment = None; - assert!(manifest.get_deployments(environment).is_err()); + assert!(manifest.get_deployments(environment).is_ok()); } #[test] @@ -109,7 +109,7 @@ fn it_errors_on_zoneless_get_deployments_workers_dev_false() { let environment = None; - assert!(manifest.get_deployments(environment).is_err()); + assert!(manifest.get_deployments(environment).is_ok()); } #[test] @@ -295,7 +295,7 @@ fn it_errors_on_single_route_get_deployments_empty_route() { let environment = None; - assert!(manifest.get_deployments(environment).is_err()); + assert!(manifest.get_deployments(environment).is_ok()); } #[test] @@ -309,7 +309,7 @@ fn it_errors_on_single_route_get_deployments_missing_route() { let environment = None; - assert!(manifest.get_deployments(environment).is_err()); + assert!(manifest.get_deployments(environment).is_ok()); } #[test] @@ -448,7 +448,7 @@ fn it_errors_on_multi_route_get_deployments_empty_routes_list() { let environment = None; - assert!(manifest.get_deployments(environment).is_err()); + assert!(manifest.get_deployments(environment).is_ok()); } #[test] @@ -462,7 +462,7 @@ fn it_errors_on_multi_route_get_deployments_empty_route() { let environment = None; - assert!(manifest.get_deployments(environment).is_err()); + assert!(manifest.get_deployments(environment).is_ok()); } #[test] @@ -536,7 +536,7 @@ fn when_top_level_empty_env_empty() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -552,7 +552,7 @@ fn when_top_level_empty_env_has_zone_id() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -569,7 +569,7 @@ fn when_top_level_empty_env_workers_dev_false() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -606,7 +606,7 @@ fn when_top_level_empty_zoned_single_route_env() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -636,7 +636,7 @@ fn when_top_level_empty_env_zoned_single_route_zone_id_only() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -678,7 +678,7 @@ fn when_top_level_empty_zoned_multi_route_env_routes_empty() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -694,7 +694,7 @@ fn when_top_level_empty_zoned_multi_route_env_route_empty() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -779,7 +779,7 @@ fn when_top_level_zoneless_env_zoneless_workers_dev_false() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -889,7 +889,7 @@ fn when_top_level_zoneless_env_zoned_multi_route_routes_list_empty() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -907,7 +907,7 @@ fn when_top_level_zoneless_env_zoned_multi_route_route_empty() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -1026,7 +1026,7 @@ fn when_top_level_zoned_env_zoneless_workers_dev_false() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] @@ -1065,7 +1065,7 @@ fn when_top_level_zoned_env_zoned_single_route_route_empty() { let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); - assert!(actual_deployments.is_err()); + assert!(actual_deployments.is_ok()); } #[test] diff --git a/tests/fixtures/wrangler_toml.rs b/tests/fixtures/wrangler_toml.rs index a346fa442..c09c2e721 100644 --- a/tests/fixtures/wrangler_toml.rs +++ b/tests/fixtures/wrangler_toml.rs @@ -124,11 +124,15 @@ impl WranglerToml { account_id: &'static str, workers_dev: bool, ) -> WranglerToml { - WranglerToml { + let wrangler_toml = WranglerToml { workers_dev: Some(workers_dev), account_id: Some(account_id), ..WranglerToml::webpack(name) - } + }; + + eprintln!("{:#?}", &wrangler_toml); + + wrangler_toml } pub fn zoned_single_route( From 563ed769c118eb2ff94bf4ca5285551eea9ded4f Mon Sep 17 00:00:00 2001 From: jspspike Date: Wed, 18 Aug 2021 12:04:51 -0500 Subject: [PATCH 3/5] Fixed typo in warning --- src/settings/toml/manifest.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index 0fc844f2e..10a1b599f 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -305,7 +305,7 @@ impl Manifest { }; if durable_objects.is_none() && deployments.is_empty() { - StdOut::warn("No depolyment routes specified, worker will not be triggered. Please specify your deployment routes or set `wrangler_dev = true` inside of your configuration file in order to trigger your worker. For more information, see: https://developers.cloudflare.com/workers/cli-wrangler/configuration#keys"); + StdOut::warn("No deployment routes specified, worker will not be triggered. Please specify your deployment routes or set `workers_dev = true` inside of your configuration file in order to trigger your worker. For more information, see: https://developers.cloudflare.com/workers/cli-wrangler/configuration#keys"); } Ok(deployments) From 5af37c5ca3ec0c58f8c67756dc62233cae10b28d Mon Sep 17 00:00:00 2001 From: jspspike Date: Wed, 18 Aug 2021 14:34:43 -0500 Subject: [PATCH 4/5] Made workers_dev be inherited by environemnts --- src/settings/toml/environment.rs | 4 +- src/settings/toml/manifest.rs | 24 ++++++----- src/settings/toml/tests/deployments.rs | 60 +++++++++++++++++++------- 3 files changed, 60 insertions(+), 28 deletions(-) diff --git a/src/settings/toml/environment.rs b/src/settings/toml/environment.rs index 1fdc37d10..82aa30083 100644 --- a/src/settings/toml/environment.rs +++ b/src/settings/toml/environment.rs @@ -39,9 +39,11 @@ impl Environment { &self, top_level_account_id: Option, top_level_zone_id: Option, + top_level_workers_dev: Option, ) -> Option { 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); + let workers_dev = self.workers_dev.or(top_level_workers_dev); if self.workers_dev.is_none() && self.zone_id.is_none() @@ -52,7 +54,7 @@ impl Environment { } else { Some(RouteConfig { account_id, - workers_dev: self.workers_dev, + workers_dev, route: self.route.clone(), routes: self.routes.clone(), zone_id, diff --git a/src/settings/toml/manifest.rs b/src/settings/toml/manifest.rs index 10a1b599f..a6737e42c 100644 --- a/src/settings/toml/manifest.rs +++ b/src/settings/toml/manifest.rs @@ -245,24 +245,26 @@ impl Manifest { deployments.push(DeployTarget::Zoneless(zoneless)); } - if !route_config.is_zoned() && !route_config.is_zoneless() { - if route_config.route.is_some() + if !route_config.is_zoned() + && !route_config.is_zoneless() + && (route_config.route.is_some() || (route_config.routes.is_some() - && !route_config.routes.as_ref().unwrap().is_empty()) - { - anyhow::bail!( - "Routes specified with no zone, specify `zone_id` in your wrangler.toml" - ) - } + && !route_config.routes.as_ref().unwrap().is_empty())) + { + anyhow::bail!( + "Routes specified with no zone, specify `zone_id` in your wrangler.toml" + ) } Ok(()) }; if let Some(env) = env { - if let Some(env_route_cfg) = - env.route_config(self.account_id.if_present().cloned(), self.zone_id.clone()) - { + if let Some(env_route_cfg) = env.route_config( + self.account_id.if_present().cloned(), + self.zone_id.clone(), + self.workers_dev, + ) { add_routed_deployments(&env_route_cfg) } else { let config = self.route_config(); diff --git a/src/settings/toml/tests/deployments.rs b/src/settings/toml/tests/deployments.rs index 625f34023..c604c1e48 100644 --- a/src/settings/toml/tests/deployments.rs +++ b/src/settings/toml/tests/deployments.rs @@ -840,9 +840,14 @@ fn when_top_level_zoneless_env_zoned_single_route_zone_id_missing() { let toml_string = toml::to_string(&test_toml).unwrap(); let manifest = Manifest::from_str(&toml_string).unwrap(); - let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); + let environment = Some(TEST_ENV_NAME); + let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)).unwrap(); + let expected_deployments = vec![DeployTarget::Zoneless(ZonelessTarget { + account_id: ACCOUNT_ID.to_string(), + script_name: manifest.worker_name(environment), + })]; - assert!(actual_deployments.is_err()); + assert_eq!(actual_deployments, expected_deployments); } #[test] @@ -861,15 +866,22 @@ fn when_top_level_zoneless_env_zoned_single_route() { let expected_name = manifest.worker_name(Some(TEST_ENV_NAME)); + let environment = Some(TEST_ENV_NAME); let expected_routes = vec![Route { script: Some(expected_name), pattern: PATTERN.to_string(), id: None, }]; - let expected_deployments = vec![DeployTarget::Zoned(ZonedTarget { - zone_id: ZONE_ID.to_string(), - routes: expected_routes, - })]; + let expected_deployments = vec![ + DeployTarget::Zoned(ZonedTarget { + zone_id: ZONE_ID.to_string(), + routes: expected_routes, + }), + DeployTarget::Zoneless(ZonelessTarget { + account_id: ACCOUNT_ID.to_string(), + script_name: manifest.worker_name(environment), + }), + ]; assert_eq!(actual_deployments, expected_deployments); } @@ -935,10 +947,16 @@ fn when_top_level_zoneless_env_zoned_multi_route_route_key_present() { }) .collect(); - let expected_deployments = vec![DeployTarget::Zoned(ZonedTarget { - routes: expected_routes, - zone_id: ZONE_ID.to_owned(), - })]; + let expected_deployments = vec![ + DeployTarget::Zoned(ZonedTarget { + routes: expected_routes, + zone_id: ZONE_ID.to_owned(), + }), + DeployTarget::Zoneless(ZonelessTarget { + account_id: ACCOUNT_ID.to_string(), + script_name: expected_name, + }), + ]; let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)).unwrap(); @@ -958,9 +976,13 @@ fn when_top_level_zoneless_env_zoned_multi_route_zone_id_missing() { let toml_string = toml::to_string(&test_toml).unwrap(); let manifest = Manifest::from_str(&toml_string).unwrap(); - let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)); + let actual_deployments = manifest.get_deployments(Some(TEST_ENV_NAME)).unwrap(); + let expected_deployments = vec![DeployTarget::Zoneless(ZonelessTarget { + account_id: ACCOUNT_ID.to_string(), + script_name: manifest.worker_name(Some(TEST_ENV_NAME)), + })]; - assert!(actual_deployments.is_err()); + assert_eq!(expected_deployments, actual_deployments); } #[test] @@ -989,10 +1011,16 @@ fn when_top_level_zoneless_env_zoned_multi_route() { }) .collect(); - let expected_deployments = vec![DeployTarget::Zoned(ZonedTarget { - zone_id: ZONE_ID.to_string(), - routes: expected_routes, - })]; + let expected_deployments = vec![ + DeployTarget::Zoned(ZonedTarget { + zone_id: ZONE_ID.to_string(), + routes: expected_routes, + }), + DeployTarget::Zoneless(ZonelessTarget { + account_id: ACCOUNT_ID.to_string(), + script_name: expected_name, + }), + ]; assert_eq!(actual_deployments, expected_deployments); } From 197338549aea545faf94aad3375652f2bba4e974 Mon Sep 17 00:00:00 2001 From: jspspike Date: Wed, 18 Aug 2021 15:08:48 -0500 Subject: [PATCH 5/5] Changed test names --- src/settings/toml/tests/deployments.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/settings/toml/tests/deployments.rs b/src/settings/toml/tests/deployments.rs index c604c1e48..2a66981c3 100644 --- a/src/settings/toml/tests/deployments.rs +++ b/src/settings/toml/tests/deployments.rs @@ -13,7 +13,7 @@ const ACCOUNT_ID: &str = "fakeaccountid"; // TOP LEVEL TESTS #[test] -fn it_errors_on_empty_get_deployments() { +fn it_can_get_empty_deployments() { let test_toml = WranglerToml::webpack("empty"); let toml_string = toml::to_string(&test_toml).unwrap(); let manifest = Manifest::from_str(&toml_string).unwrap(); @@ -100,7 +100,7 @@ fn it_errors_on_get_deployments_missing_account_id() { } #[test] -fn it_errors_on_zoneless_get_deployments_workers_dev_false() { +fn it_can_zoneless_get_deployments_workers_dev_false() { let script_name = "zoneless_false"; let workers_dev = false; let test_toml = WranglerToml::zoneless(script_name, ACCOUNT_ID, workers_dev); @@ -285,7 +285,7 @@ fn it_errors_on_single_route_get_deployments_missing_zone_id() { } #[test] -fn it_errors_on_single_route_get_deployments_empty_route() { +fn it_can_single_route_get_deployments_empty_route() { let script_name = "single_route_empty_route"; let pattern = ""; @@ -299,7 +299,7 @@ fn it_errors_on_single_route_get_deployments_empty_route() { } #[test] -fn it_errors_on_single_route_get_deployments_missing_route() { +fn it_can_single_route_get_deployments_missing_route() { let script_name = "single_route_missing_route"; let mut test_toml = WranglerToml::zoned_single_route(script_name, ZONE_ID, ""); @@ -438,7 +438,7 @@ fn it_errors_on_multi_route_get_deployments_missing_zone_id() { } #[test] -fn it_errors_on_multi_route_get_deployments_empty_routes_list() { +fn it_can_multi_route_get_deployments_empty_routes_list() { let script_name = "multi_route_empty_routes_list"; let patterns = []; @@ -452,7 +452,7 @@ fn it_errors_on_multi_route_get_deployments_empty_routes_list() { } #[test] -fn it_errors_on_multi_route_get_deployments_empty_route() { +fn it_can_multi_route_get_deployments_empty_route() { let script_name = "multi_route_empty_route"; let patterns = [""];