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

Commit

Permalink
Allow worker with no routes to be published (#2024)
Browse files Browse the repository at this point in the history 
* Changed having no routes from fatal to warning

* Allowed scripts with zone and no routes to published

* Fixed typo in warning

* Made workers_dev be inherited by environemnts

* Changed test names

Co-authored-by: Ashcon Partovi <[email protected]>
  • Loading branch information
@jspspike @Electroid
jspspike and Electroid authored Aug 27, 2021
1 parent 5675efa commit 59382e1
Show file tree
Hide file tree
Showing 5 changed files with 104 additions and 52 deletions.
4 changes: 0 additions & 4 deletions src/deploy/zoned.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 8 additions & 2 deletions src/settings/toml/environment.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,16 +39,22 @@ impl Environment {
&self,
top_level_account_id: Option<String>,
top_level_zone_id: Option<String>,
top_level_workers_dev: Option<bool>,
) -> Option<RouteConfig> {
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.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 {
account_id,
workers_dev: self.workers_dev,
workers_dev,
route: self.route.clone(),
routes: self.routes.clone(),
zone_id,
Expand Down
26 changes: 22 additions & 4 deletions src/settings/toml/manifest.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -240,13 +245,26 @@ impl Manifest {
deployments.push(DeployTarget::Zoneless(zoneless));
}

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"
)
}

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();
Expand Down Expand Up @@ -289,7 +307,7 @@ impl Manifest {
};

if durable_objects.is_none() && deployments.is_empty() {
anyhow::bail!("Please specify your deployment routes or `workers_dev = true` inside of your configuration file. 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)
Expand Down
108 changes: 68 additions & 40 deletions src/settings/toml/tests/deployments.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,14 @@ 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();

let environment = None;

assert!(manifest.get_deployments(environment).is_err());
assert!(manifest.get_deployments(environment).is_ok());
}

#[test]
Expand Down Expand Up @@ -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);
Expand All @@ -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]
Expand Down Expand Up @@ -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 = "";

Expand All @@ -295,11 +295,11 @@ 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]
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, "");
Expand All @@ -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]
Expand Down Expand Up @@ -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 = [];

Expand All @@ -448,11 +448,11 @@ 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]
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 = [""];

Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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]
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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]
Expand All @@ -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);
}
Expand All @@ -889,7 +901,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]
Expand All @@ -907,7 +919,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]
Expand Down Expand Up @@ -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();

Expand All @@ -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]
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -1026,7 +1054,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]
Expand Down Expand Up @@ -1065,7 +1093,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]
Expand Down
Loading

0 comments on commit 59382e1

Please sign in to comment.