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 b2b80ff
Show file tree
Hide file tree
Showing 8 changed files with 268 additions and 76 deletions.
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 b2b80ff

Please sign in to comment.