Skip to content

Commit 8015ff3

Browse files
committed
feat: optionally send zone_id with a route
This enables optionally passing a route as `{pattern: string, zone_id: string}`. There are scenarios where we need to explicitly pass a zone_id to the api, so this enables that. Some nuance: our toml parser unfortunately doesn't let you have mixed types in arrays, so if you have a single route with this special shape, then you'll have to do it for all of them. We can replace it with something more recent, but I'll do that in a subsequent PR. Further nuance: The errors from the api aren't super useful when invalid values are passed, but that's something to further work on. This also fixes some types in our cli parsing. Fixes #774
1 parent 664803e commit 8015ff3

File tree

9 files changed

+295
-69
lines changed

9 files changed

+295
-69
lines changed

.changeset/dull-jobs-return.md

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
"wrangler": patch
3+
---
4+
5+
feat: optionally send zone_id with a route
6+
7+
This enables optionally passing a route as `{pattern: string, zone_id: string}`. There are scenarios where we need to explicitly pass a zone_id to the api, so this enables that.
8+
9+
Some nuance: The errors from the api aren't super useful when invalid values are passed, but that's something to further work on.
10+
11+
This also fixes some types in our cli parsing.
12+
13+
Fixes https://github.com/cloudflare/wrangler2/issues/774

packages/wrangler/src/__tests__/configuration.test.ts

+25-6
Original file line numberDiff line numberDiff line change
@@ -553,7 +553,12 @@ describe("normalizeAndValidateConfig()", () => {
553553
compatibility_date: "2022-01-01",
554554
compatibility_flags: ["FLAG1", "FLAG2"],
555555
workers_dev: false,
556-
routes: ["ROUTE_1", "ROUTE_2"],
556+
routes: [
557+
"ROUTE_1",
558+
"ROUTE_2",
559+
{ pattern: "ROUTE3", zone_id: "ZONE_ID_3" },
560+
"ROUTE_4",
561+
],
557562
jsx_factory: "JSX_FACTORY",
558563
jsx_fragment: "JSX_FRAGMENT",
559564
tsconfig: "path/to/tsconfig",
@@ -630,7 +635,21 @@ describe("normalizeAndValidateConfig()", () => {
630635
compatibility_date: 333,
631636
compatibility_flags: [444, 555],
632637
workers_dev: "BAD",
633-
routes: [666, 777],
638+
routes: [
639+
666,
640+
777,
641+
// this one's valid, but we add it here to make sure
642+
// it doesn't get included in the error message
643+
"example.com/*",
644+
{ pattern: 123, zone_id: "zone_id_1" },
645+
{ pattern: "route_2", zone_id: 123 },
646+
{ pattern: "route_3" },
647+
{ zone_id: "zone_id_4" },
648+
{ pattern: undefined },
649+
{ pattern: "route_5", zone_id: "zone_id_5", some_other_key: 123 },
650+
// this one's valid too
651+
{ pattern: "route_6", zone_id: "zone_id_6" },
652+
],
634653
route: 888,
635654
jsx_factory: 999,
636655
jsx_fragment: 1000,
@@ -655,8 +674,8 @@ describe("normalizeAndValidateConfig()", () => {
655674
expect(diagnostics.hasWarnings()).toBe(false);
656675
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
657676
"Processing wrangler configuration:
658-
- Expected \\"route\\" to be of type string but got 888.
659-
- Expected \\"routes\\" to be of type string array but got [666,777].
677+
- Expected \\"route\\" to be either a string or an object with shape {pattern: string, zone_id: string}, but got 888.
678+
- Expected \\"routes\\" to be an array of either strings or objects with the shape {pattern: string, zone_id: string}, but these weren't valid: [666,777,{\\"pattern\\":123,\\"zone_id\\":\\"zone_id_1\\"},{\\"pattern\\":\\"route_2\\",\\"zone_id\\":123},{\\"pattern\\":\\"route_3\\"},{\\"zone_id\\":\\"zone_id_4\\"},{},{\\"pattern\\":\\"route_5\\",\\"zone_id\\":\\"zone_id_5\\",\\"some_other_key\\":123}].
660679
- Expected exactly one of the following fields [\\"routes\\",\\"route\\"].
661680
- Expected \\"workers_dev\\" to be of type boolean but got \\"BAD\\".
662681
- Expected \\"build.command\\" to be of type string but got 1444.
@@ -1797,8 +1816,8 @@ describe("normalizeAndValidateConfig()", () => {
17971816
"Processing wrangler configuration:
17981817
17991818
- \\"env.ENV1\\" environment configuration
1800-
- Expected \\"route\\" to be of type string but got 888.
1801-
- Expected \\"routes\\" to be of type string array but got [666,777].
1819+
- Expected \\"route\\" to be either a string or an object with shape {pattern: string, zone_id: string}, but got 888.
1820+
- Expected \\"routes\\" to be an array of either strings or objects with the shape {pattern: string, zone_id: string}, but these weren't valid: [666,777].
18021821
- Expected exactly one of the following fields [\\"routes\\",\\"route\\"].
18031822
- Expected \\"workers_dev\\" to be of type boolean but got \\"BAD\\".
18041823
- Expected \\"build.command\\" to be of type string but got 1444.

packages/wrangler/src/__tests__/dev.test.tsx

+15
Original file line numberDiff line numberDiff line change
@@ -218,6 +218,21 @@ describe("wrangler dev", () => {
218218
);
219219
});
220220

221+
it("should, when provided, use a configured zone_id", async () => {
222+
writeWranglerToml({
223+
main: "index.js",
224+
routes: [
225+
{ pattern: "https://some-domain.com/*", zone_id: "some-zone-id" },
226+
],
227+
});
228+
fs.writeFileSync("index.js", `export default {};`);
229+
await runWrangler("dev");
230+
expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
231+
host: "some-domain.com",
232+
id: "some-zone-id",
233+
});
234+
});
235+
221236
it("given a long host, it should use the longest subdomain that resolves to a zone", async () => {
222237
writeWranglerToml({
223238
main: "index.js",

packages/wrangler/src/__tests__/publish.test.ts

+89-2
Original file line numberDiff line numberDiff line change
@@ -311,6 +311,91 @@ describe("publish", () => {
311311
await runWrangler("publish ./index");
312312
});
313313

314+
it("should publish to a route with a pattern/zone_id combo", async () => {
315+
writeWranglerToml({
316+
routes: [
317+
"some-example.com/some-route/*",
318+
{ pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
319+
{ pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
320+
"more-examples.com/*",
321+
],
322+
});
323+
writeWorkerSource();
324+
mockUpdateWorkerRequest({ enabled: false });
325+
mockUploadWorkerRequest({ expectedType: "esm" });
326+
mockPublishRoutesRequest({
327+
routes: [
328+
"some-example.com/some-route/*",
329+
{ pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
330+
{ pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
331+
"more-examples.com/*",
332+
],
333+
});
334+
await runWrangler("publish ./index");
335+
expect(std).toMatchInlineSnapshot(`
336+
Object {
337+
"err": "",
338+
"out": "Uploaded test-name (TIMINGS)
339+
Published test-name (TIMINGS)
340+
some-example.com/some-route/*
341+
*a-boring-website.com (zone: 54sdf7fsda)
342+
example.com/some-route/* (zone: JGHFHG654gjcj)
343+
more-examples.com/*",
344+
"warn": "",
345+
}
346+
`);
347+
});
348+
349+
it("should publish to a route with a pattern/zone_id combo (service environments)", async () => {
350+
writeWranglerToml({
351+
env: {
352+
staging: {
353+
routes: [
354+
"some-example.com/some-route/*",
355+
{ pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
356+
{ pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
357+
"more-examples.com/*",
358+
],
359+
},
360+
},
361+
});
362+
mockSubDomainRequest();
363+
writeWorkerSource();
364+
mockUpdateWorkerRequest({
365+
enabled: false,
366+
env: "staging",
367+
legacyEnv: false,
368+
});
369+
mockUploadWorkerRequest({
370+
expectedType: "esm",
371+
env: "staging",
372+
legacyEnv: false,
373+
});
374+
mockPublishRoutesRequest({
375+
routes: [
376+
"some-example.com/some-route/*",
377+
{ pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
378+
{ pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
379+
"more-examples.com/*",
380+
],
381+
env: "staging",
382+
legacyEnv: false,
383+
});
384+
await runWrangler("publish ./index --legacy-env false --env staging");
385+
expect(std).toMatchInlineSnapshot(`
386+
Object {
387+
"err": "",
388+
"out": "Uploaded test-name (staging) (TIMINGS)
389+
Published test-name (staging) (TIMINGS)
390+
some-example.com/some-route/*
391+
*a-boring-website.com (zone: 54sdf7fsda)
392+
example.com/some-route/* (zone: JGHFHG654gjcj)
393+
more-examples.com/*",
394+
"warn": "",
395+
}
396+
`);
397+
});
398+
314399
it("should publish to legacy environment specific routes", async () => {
315400
writeWranglerToml({
316401
routes: ["example.com/some-route/*"],
@@ -3829,7 +3914,7 @@ function mockPublishRoutesRequest({
38293914
env = undefined,
38303915
legacyEnv = false,
38313916
}: {
3832-
routes: string[];
3917+
routes: (string | { pattern: string; zone_id: string })[];
38333918
env?: string | undefined;
38343919
legacyEnv?: boolean | undefined;
38353920
}) {
@@ -3849,7 +3934,9 @@ function mockPublishRoutesRequest({
38493934
}
38503935

38513936
expect(JSON.parse(body as string)).toEqual(
3852-
routes.map((pattern) => ({ pattern }))
3937+
routes.map((route) =>
3938+
typeof route !== "object" ? { pattern: route } : route
3939+
)
38533940
);
38543941
return null;
38553942
}

packages/wrangler/src/config/config.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -183,9 +183,9 @@ export interface DevConfig {
183183
upstream_protocol: "https" | "http";
184184

185185
/**
186-
* Host to forward requests to, defaults to the zone of project
186+
* Host to forward requests to, defaults to the host of the first route of project
187187
*/
188-
host: string | undefined;
188+
host: (string | { pattern: string; zone_id: string }) | undefined;
189189
}
190190

191191
export type RawDevConfig = Partial<DevConfig>;

packages/wrangler/src/config/environment.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ interface EnvironmentInheritable {
7474
*
7575
* @inheritable
7676
*/
77-
routes: string[] | undefined;
77+
routes: (string | { pattern: string; zone_id: string })[] | undefined;
7878

7979
/**
8080
* A route that your worker should be published to. Literally
@@ -85,7 +85,7 @@ interface EnvironmentInheritable {
8585
*
8686
* @inheritable
8787
*/
88-
route: string | undefined;
88+
route: (string | { pattern: string; zone_id: string }) | undefined;
8989

9090
/**
9191
* Path to a custom tsconfig

packages/wrangler/src/config/validation.ts

+95-16
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,97 @@ function normalizeAndValidateModulePaths(
461461
return mapping;
462462
}
463463

464+
/**
465+
* Check whether a value has the shape of a route, which can be a string
466+
* or an object that looks like {pattern: string, zone_id: string }
467+
*/
468+
function isValidRouteValue(item: unknown): boolean {
469+
return (
470+
!!item &&
471+
(typeof item === "string" ||
472+
(typeof item === "object" &&
473+
hasProperty(item, "pattern") &&
474+
typeof item.pattern === "string" &&
475+
hasProperty(item, "zone_id") &&
476+
typeof item.zone_id === "string" &&
477+
Object.keys(item).length === 2))
478+
);
479+
}
480+
481+
/**
482+
* Validate that the field is a route.
483+
*/
484+
const isRoute: ValidatorFn = (diagnostics, field, value) => {
485+
if (value !== undefined && !isValidRouteValue(value)) {
486+
diagnostics.errors.push(
487+
`Expected "${field}" to be either a string or an object with shape {pattern: string, zone_id: string}, but got ${JSON.stringify(
488+
value
489+
)}.`
490+
);
491+
return false;
492+
}
493+
return true;
494+
};
495+
496+
/**
497+
* Validate that the field is an array of routes.
498+
*/
499+
const isRouteArray: ValidatorFn = (diagnostics, field, value) => {
500+
if (value === undefined) {
501+
return true;
502+
}
503+
if (!Array.isArray(value)) {
504+
diagnostics.errors.push(
505+
`Expected "${field}" to be an array but got ${JSON.stringify(value)}.`
506+
);
507+
return false;
508+
}
509+
const invalidRoutes = [];
510+
for (const item of value) {
511+
if (!isValidRouteValue(item)) {
512+
invalidRoutes.push(item);
513+
}
514+
}
515+
if (invalidRoutes.length > 0) {
516+
diagnostics.errors.push(
517+
`Expected "${field}" to be an array of either strings or objects with the shape {pattern: string, zone_id: string}, but these weren't valid: ${JSON.stringify(
518+
invalidRoutes
519+
)}.`
520+
);
521+
}
522+
return invalidRoutes.length === 0;
523+
};
524+
525+
function validateRoute(
526+
diagnostics: Diagnostics,
527+
topLevelEnv: Environment | undefined,
528+
rawEnv: RawEnvironment
529+
): Config["route"] {
530+
return inheritable(
531+
diagnostics,
532+
topLevelEnv,
533+
rawEnv,
534+
"route",
535+
isRoute,
536+
undefined
537+
);
538+
}
539+
540+
function validateRoutes(
541+
diagnostics: Diagnostics,
542+
topLevelEnv: Environment | undefined,
543+
rawEnv: RawEnvironment
544+
): Config["routes"] {
545+
return inheritable(
546+
diagnostics,
547+
topLevelEnv,
548+
rawEnv,
549+
"routes",
550+
all(isRouteArray, isMutuallyExclusiveWith(rawEnv, "route")),
551+
undefined
552+
);
553+
}
554+
464555
/**
465556
* Validate top-level environment configuration and return the normalized values.
466557
*/
@@ -526,22 +617,10 @@ function normalizeAndValidateEnvironment(
526617

527618
experimental(diagnostics, rawEnv, "unsafe");
528619

529-
const route = inheritable(
530-
diagnostics,
531-
topLevelEnv,
532-
rawEnv,
533-
"route",
534-
isString,
535-
undefined
536-
);
537-
const routes = inheritable(
538-
diagnostics,
539-
topLevelEnv,
540-
rawEnv,
541-
"routes",
542-
all(isStringArray, isMutuallyExclusiveWith(rawEnv, "route")),
543-
undefined
544-
);
620+
const route = validateRoute(diagnostics, topLevelEnv, rawEnv);
621+
622+
const routes = validateRoutes(diagnostics, topLevelEnv, rawEnv);
623+
545624
const workers_dev = inheritable(
546625
diagnostics,
547626
topLevelEnv,

0 commit comments

Comments
 (0)