Skip to content

Commit

Permalink
feat: optionally send zone_id with a route
Browse files Browse the repository at this point in the history
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 is 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
  • Loading branch information
threepointone committed Apr 10, 2022
1 parent 6d253a1 commit c46671b
Show file tree
Hide file tree
Showing 9 changed files with 283 additions and 76 deletions.
15 changes: 15 additions & 0 deletions .changeset/dull-jobs-return.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
---
"wrangler": patch
---

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 is 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 https://github.com/cloudflare/wrangler2/issues/774
29 changes: 23 additions & 6 deletions packages/wrangler/src/__tests__/configuration.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -553,7 +553,12 @@ describe("normalizeAndValidateConfig()", () => {
compatibility_date: "2022-01-01",
compatibility_flags: ["FLAG1", "FLAG2"],
workers_dev: false,
routes: ["ROUTE_1", "ROUTE_2"],
routes: [
"ROUTE_1",
"ROUTE_2",
{ pattern: "ROUTE3", zone_id: "ZONE_ID_3" },
"ROUTE_4",
],
jsx_factory: "JSX_FACTORY",
jsx_fragment: "JSX_FRAGMENT",
tsconfig: "path/to/tsconfig",
Expand Down Expand Up @@ -630,7 +635,19 @@ describe("normalizeAndValidateConfig()", () => {
compatibility_date: 333,
compatibility_flags: [444, 555],
workers_dev: "BAD",
routes: [666, 777],
routes: [
666,
777,
{ pattern: 123, zone_id: "zone_id_1" },
{ pattern: "route_2", zone_id: 123 },
{ pattern: "route_3" },
{ zone_id: "zone_id_4" },
{ pattern: undefined },
{ pattern: "route_5", zone_id: "zone_id_5", some_other_key: 123 },
// this one's actually valid, but let's make sure
// it gets included in the error anyway
{ pattern: "route_6", zone_id: "zone_id_6" },
],
route: 888,
jsx_factory: 999,
jsx_fragment: 1000,
Expand All @@ -655,8 +672,8 @@ describe("normalizeAndValidateConfig()", () => {
expect(diagnostics.hasWarnings()).toBe(false);
expect(diagnostics.renderErrors()).toMatchInlineSnapshot(`
"Processing wrangler configuration:
- Expected \\"route\\" to be of type string but got 888.
- Expected \\"routes\\" to be of type string array but got [666,777].
- Expected \\"route\\" to be a route but got 888.
- Expected \\"routes\\" to be an array of routes but got [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},{\\"pattern\\":\\"route_6\\",\\"zone_id\\":\\"zone_id_6\\"}].
- Expected exactly one of the following fields [\\"routes\\",\\"route\\"].
- Expected \\"workers_dev\\" to be of type boolean but got \\"BAD\\".
- Expected \\"build.command\\" to be of type string but got 1444.
Expand Down Expand Up @@ -1797,8 +1814,8 @@ describe("normalizeAndValidateConfig()", () => {
"Processing wrangler configuration:
- \\"env.ENV1\\" environment configuration
- Expected \\"route\\" to be of type string but got 888.
- Expected \\"routes\\" to be of type string array but got [666,777].
- Expected \\"route\\" to be a route but got 888.
- Expected \\"routes\\" to be an array of routes but got [666,777].
- Expected exactly one of the following fields [\\"routes\\",\\"route\\"].
- Expected \\"workers_dev\\" to be of type boolean but got \\"BAD\\".
- Expected \\"build.command\\" to be of type string but got 1444.
Expand Down
15 changes: 15 additions & 0 deletions packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,21 @@ describe("wrangler dev", () => {
);
});

it("should, when provided, use a configured zone_id", async () => {
writeWranglerToml({
main: "index.js",
routes: [
{ pattern: "https://some-domain.com/*", zone_id: "some-zone-id" },
],
});
fs.writeFileSync("index.js", `export default {};`);
await runWrangler("dev");
expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
host: "some-domain.com",
id: "some-zone-id",
});
});

it("given a long host, it should use the longest subdomain that resolves to a zone", async () => {
writeWranglerToml({
main: "index.js",
Expand Down
83 changes: 81 additions & 2 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,83 @@ describe("publish", () => {
await runWrangler("publish ./index");
});

it("should publish to a route with a pattern/zone_id combo", async () => {
writeWranglerToml({
routes: [
// toml doesn't let arrays have mixed types, or we
// could have tested strings + {pattern,zone_id} combos
{ pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
{ pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
],
});
writeWorkerSource();
mockUpdateWorkerRequest({ enabled: false });
mockUploadWorkerRequest({ expectedType: "esm" });
mockPublishRoutesRequest({
routes: [
{ pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
{ pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
],
});
await runWrangler("publish ./index");
expect(std).toMatchInlineSnapshot(`
Object {
"err": "",
"out": "Uploaded test-name (TIMINGS)
Published test-name (TIMINGS)
*a-boring-website.com (zone: 54sdf7fsda)
example.com/some-route/* (zone: JGHFHG654gjcj)",
"warn": "",
}
`);
});

it("should publish to a route with a pattern/zone_id combo (service environments)", async () => {
writeWranglerToml({
env: {
staging: {
routes: [
// toml doesn't let arrays have mixed types, or we
// could have tested strings + {pattern,zone_id} combos
{ pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
{ pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
],
},
},
});
mockSubDomainRequest();
writeWorkerSource();
mockUpdateWorkerRequest({
enabled: false,
env: "staging",
legacyEnv: false,
});
mockUploadWorkerRequest({
expectedType: "esm",
env: "staging",
legacyEnv: false,
});
mockPublishRoutesRequest({
routes: [
{ pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
{ pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
],
env: "staging",
legacyEnv: false,
});
await runWrangler("publish ./index --legacy-env false --env staging");
expect(std).toMatchInlineSnapshot(`
Object {
"err": "",
"out": "Uploaded test-name (staging) (TIMINGS)
Published test-name (staging) (TIMINGS)
*a-boring-website.com (zone: 54sdf7fsda)
example.com/some-route/* (zone: JGHFHG654gjcj)",
"warn": "",
}
`);
});

it("should publish to legacy environment specific routes", async () => {
writeWranglerToml({
routes: ["example.com/some-route/*"],
Expand Down Expand Up @@ -3829,7 +3906,7 @@ function mockPublishRoutesRequest({
env = undefined,
legacyEnv = false,
}: {
routes: string[];
routes: (string | { pattern: string; zone_id: string })[];
env?: string | undefined;
legacyEnv?: boolean | undefined;
}) {
Expand All @@ -3849,7 +3926,9 @@ function mockPublishRoutesRequest({
}

expect(JSON.parse(body as string)).toEqual(
routes.map((pattern) => ({ pattern }))
routes.map((route) =>
typeof route !== "object" ? { pattern: route } : route
)
);
return null;
}
Expand Down
4 changes: 2 additions & 2 deletions packages/wrangler/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,9 @@ export interface DevConfig {
upstream_protocol: "https" | "http";

/**
* Host to forward requests to, defaults to the zone of project
* Host to forward requests to, defaults to the host of the first route of project
*/
host: string | undefined;
host: (string | { pattern: string; zone_id: string }) | undefined;
}

export type RawDevConfig = Partial<DevConfig>;
Expand Down
4 changes: 2 additions & 2 deletions packages/wrangler/src/config/environment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ interface EnvironmentInheritable {
*
* @inheritable
*/
routes: string[] | undefined;
routes: (string | { pattern: string; zone_id: string })[] | undefined;

/**
* A route that your worker should be published to. Literally
Expand All @@ -85,7 +85,7 @@ interface EnvironmentInheritable {
*
* @inheritable
*/
route: string | undefined;
route: (string | { pattern: string; zone_id: string }) | undefined;

/**
* Path to a custom tsconfig
Expand Down
98 changes: 82 additions & 16 deletions packages/wrangler/src/config/validation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -461,6 +461,84 @@ function normalizeAndValidateModulePaths(
return mapping;
}

/**
* Check whether a value has the shape of a route, which can be a string
* or an object that looks like {pattern: string, zone_id: string }
*/
function isValidRouteValue(item: unknown): boolean {
return (
!!item &&
(typeof item === "string" ||
(typeof item === "object" &&
// @ts-expect-error how do i refine this type?
typeof item.pattern === "string" &&
// @ts-expect-error how do i refine this type?
typeof item.zone_id === "string" &&
Object.keys(item).length === 2))
);
}

/**
* Validate that the field is a route.
*/
const isRoute: ValidatorFn = (diagnostics, field, value) => {
if (value !== undefined && !isValidRouteValue(value)) {
diagnostics.errors.push(
`Expected "${field}" to be a route but got ${JSON.stringify(value)}.`
);
return false;
}
return true;
};

/**
* Validate that the field is an array of routes.
*/
const isRouteArray: ValidatorFn = (diagnostics, field, value) => {
if (
value !== undefined &&
(!Array.isArray(value) || value.some((item) => !isValidRouteValue(item)))
) {
diagnostics.errors.push(
`Expected "${field}" to be an array of routes but got ${JSON.stringify(
value
)}.`
);
return false;
}
return true;
};

function validateRoute(
diagnostics: Diagnostics,
topLevelEnv: Environment | undefined,
rawEnv: RawEnvironment
): Config["route"] {
return inheritable(
diagnostics,
topLevelEnv,
rawEnv,
"route",
isRoute,
undefined
);
}

function validateRoutes(
diagnostics: Diagnostics,
topLevelEnv: Environment | undefined,
rawEnv: RawEnvironment
): Config["routes"] {
return inheritable(
diagnostics,
topLevelEnv,
rawEnv,
"routes",
all(isRouteArray, isMutuallyExclusiveWith(rawEnv, "route")),
undefined
);
}

/**
* Validate top-level environment configuration and return the normalized values.
*/
Expand Down Expand Up @@ -526,22 +604,10 @@ function normalizeAndValidateEnvironment(

experimental(diagnostics, rawEnv, "unsafe");

const route = inheritable(
diagnostics,
topLevelEnv,
rawEnv,
"route",
isString,
undefined
);
const routes = inheritable(
diagnostics,
topLevelEnv,
rawEnv,
"routes",
all(isStringArray, isMutuallyExclusiveWith(rawEnv, "route")),
undefined
);
const route = validateRoute(diagnostics, topLevelEnv, rawEnv);

const routes = validateRoutes(diagnostics, topLevelEnv, rawEnv);

const workers_dev = inheritable(
diagnostics,
topLevelEnv,
Expand Down
Loading

0 comments on commit c46671b

Please sign in to comment.