Skip to content

Commit

Permalink
validate routes in dev and deploy
Browse files Browse the repository at this point in the history
  • Loading branch information
emily-shen committed Sep 3, 2024
1 parent c4f0d9e commit abbe295
Show file tree
Hide file tree
Showing 7 changed files with 187 additions and 39 deletions.
58 changes: 48 additions & 10 deletions packages/wrangler/src/__tests__/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1453,11 +1453,12 @@ Update them to point to this script instead?`,
routes: [{ pattern: "*.example.com", custom_domain: true }],
});
writeWorkerSource();
await expect(
runWrangler("deploy ./index")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Cannot use "*.example.com" as a Custom Domain; wildcard operators (*) are not allowed]`
);
await expect(runWrangler("deploy ./index")).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
*.example.com:
Wildcard operators (*) are not allowed in Custom Domains]
`);

writeWranglerToml({
routes: [
Expand All @@ -1467,11 +1468,12 @@ Update them to point to this script instead?`,
writeWorkerSource();
mockServiceScriptData({});

await expect(
runWrangler("deploy ./index")
).rejects.toThrowErrorMatchingInlineSnapshot(
`[Error: Cannot use "api.example.com/at/a/path" as a Custom Domain; paths are not allowed]`
);
await expect(runWrangler("deploy ./index")).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
api.example.com/at/a/path:
Paths are not allowed in Custom Domains]
`);
});

it("should not continue with publishing an override if user does not confirm", async () => {
Expand Down Expand Up @@ -1514,6 +1516,42 @@ Update them to point to this script instead?`,
});
});

it("should error on routes with paths if experimental assets are present", async () => {
writeWranglerToml({
routes: [
"simple.co.uk/path",
"simple.co.uk/*",
"simple.co.uk",
{ pattern: "route.co.uk/path", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk", zone_id: "asdfadsf" },
{ pattern: "custom.co.uk/path", custom_domain: true },
{ pattern: "custom.co.uk/*", custom_domain: true },
{ pattern: "custom.co.uk", custom_domain: true },
],
});
writeWorkerSource();
writeAssets([{ filePath: "asset.txt", content: "Content of file-1" }]);

await expect(
runWrangler(`deploy --experimental-assets="assets"`)
).rejects.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
simple.co.uk/path:
Routes with paths (except for the wildcard /*) are not allowed in Workers + Assets projects
route.co.uk/path:
Routes with paths (except for the wildcard /*) are not allowed in Workers + Assets projects
custom.co.uk/path:
Paths are not allowed in Custom Domains
custom.co.uk/*:
Wildcard operators (*) are not allowed in Custom Domains
Paths are not allowed in Custom Domains]
`);
});

it.todo("should error if it's a workers.dev route");
});

Expand Down
62 changes: 62 additions & 0 deletions packages/wrangler/src/__tests__/dev.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,68 @@ describe("wrangler dev", () => {
})
);
});
it("should error if custom domains with paths are passed in but allow paths on normal routes", async () => {
fs.writeFileSync("index.js", `export default {};`);
writeWranglerToml({
main: "index.js",
routes: [
"simple.co.uk/path",
"simple.co.uk/*",
"simple.co.uk",
{ pattern: "route.co.uk/path", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk", zone_id: "asdfadsf" },
{ pattern: "custom.co.uk/path", custom_domain: true },
{ pattern: "custom.co.uk/*", custom_domain: true },
{ pattern: "custom.co.uk", custom_domain: true },
],
});
await expect(runWrangler(`dev`)).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
custom.co.uk/path:
Paths are not allowed in Custom Domains
custom.co.uk/*:
Wildcard operators (*) are not allowed in Custom Domains
Paths are not allowed in Custom Domains]
`);
});
it("should error on routes with paths if experimental assets are present", async () => {
writeWranglerToml({
routes: [
"simple.co.uk/path",
"simple.co.uk/*",
"simple.co.uk",
{ pattern: "route.co.uk/path", zone_id: "asdfadsf" },
{ pattern: "route.co.uk/*", zone_id: "asdfadsf" },
{ pattern: "route.co.uk", zone_id: "asdfadsf" },
{ pattern: "custom.co.uk/path", custom_domain: true },
{ pattern: "custom.co.uk/*", custom_domain: true },
{ pattern: "custom.co.uk", custom_domain: true },
],
experimental_assets: {
directory: "assets",
},
});
fs.openSync("assets", "w");
await expect(runWrangler(`dev`)).rejects
.toThrowErrorMatchingInlineSnapshot(`
[Error: Invalid Routes:
simple.co.uk/path:
Routes with paths (except for the wildcard /*) are not allowed in Workers + Assets projects
route.co.uk/path:
Routes with paths (except for the wildcard /*) are not allowed in Workers + Assets projects
custom.co.uk/path:
Paths are not allowed in Custom Domains
custom.co.uk/*:
Wildcard operators (*) are not allowed in Custom Domains
Paths are not allowed in Custom Domains]
`);
});
});

describe("host", () => {
Expand Down
2 changes: 2 additions & 0 deletions packages/wrangler/src/api/startDevWorker/ConfigController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ async function resolveDevConfig(
routes: input.triggers?.filter(
(t): t is Extract<Trigger, { type: "route" }> => t.type === "route"
),
experimentalAssets: input.experimental?.assets?.directory,
},
config
);
Expand Down Expand Up @@ -163,6 +164,7 @@ async function resolveTriggers(
routes: input.triggers?.filter(
(t): t is Extract<Trigger, { type: "route" }> => t.type === "route"
),
experimentalAssets: input.experimental?.assets?.directory,
},
config
);
Expand Down
69 changes: 55 additions & 14 deletions packages/wrangler/src/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,60 @@ function errIsStartupErr(err: unknown): err is ParseError & { code: 10021 } {
return false;
}

export const validateRoutes = (
routes: Route[],
hasExperimentalAssets: boolean
) => {
const invalidRoutes: { [route: string]: string[] } = {};
for (const route of routes) {
if (typeof route !== "string" && route.custom_domain) {
if (route.pattern.includes("*")) {
invalidRoutes[route.pattern] = [
...(invalidRoutes[route.pattern] ?? []),
`Wildcard operators (*) are not allowed in Custom Domains`,
];
}
if (route.pattern.includes("/")) {
invalidRoutes[route.pattern] = [
...(invalidRoutes[route.pattern] ?? []),
`Paths are not allowed in Custom Domains`,
];
}
} else {
if (
typeof route === "string" &&
hasExperimentalAssets &&
route.includes("/") &&
!route.endsWith("/*")
) {
invalidRoutes[route] = [
...(invalidRoutes[route] ?? []),
`Routes with paths (except for the wildcard /*) are not allowed in Workers + Assets projects`,
];
}
if (
typeof route === "object" &&
hasExperimentalAssets &&
route.pattern.includes("/") &&
!route.pattern.endsWith("/*")
) {
invalidRoutes[route.pattern] = [
...(invalidRoutes[route.pattern] ?? []),
`Routes with paths (except for the wildcard /*) are not allowed in Workers + Assets projects`,
];
}
}
}
if (Object.keys(invalidRoutes).length > 0) {
throw new UserError(
`Invalid Routes:\n` +
Object.entries(invalidRoutes)
.map(([route, errors]) => `${route}:\n` + errors.join("\n"))
.join(`\n\n`)
);
}
};

export function renderRoute(route: Route): string {
let result = "";
if (typeof route === "string") {
Expand Down Expand Up @@ -380,20 +434,7 @@ See https://developers.cloudflare.com/workers/platform/compatibility-dates for m

const routes =
props.routes ?? config.routes ?? (config.route ? [config.route] : []) ?? [];
for (const route of routes) {
if (typeof route !== "string" && route.custom_domain) {
if (route.pattern.includes("*")) {
throw new UserError(
`Cannot use "${route.pattern}" as a Custom Domain; wildcard operators (*) are not allowed`
);
}
if (route.pattern.includes("/")) {
throw new UserError(
`Cannot use "${route.pattern}" as a Custom Domain; paths are not allowed`
);
}
}
}
validateRoutes(routes, Boolean(props.experimentalAssetsOptions));

const jsxFactory = props.jsxFactory || config.jsx_factory;
const jsxFragment = props.jsxFragment || config.jsx_fragment;
Expand Down
15 changes: 10 additions & 5 deletions packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
extractBindingsOfType,
} from "./api/startDevWorker/utils";
import { findWranglerToml, printBindings, readConfig } from "./config";
import { validateRoutes } from "./deploy/deploy";
import { getEntry } from "./deployment-bundle/entry";
import { getNodeCompatMode } from "./deployment-bundle/node-compat";
import { getBoundRegisteredWorkers } from "./dev-registry";
Expand Down Expand Up @@ -1214,12 +1215,13 @@ export function maskVars(

export async function getHostAndRoutes(
args:
| Pick<StartDevOptions, "host" | "routes">
| Pick<StartDevOptions, "host" | "routes" | "experimentalAssets">
| {
host?: string;
routes?: Extract<Trigger, { type: "route" }>[];
experimentalAssets?: string;
},
config: Pick<Config, "route" | "routes"> & {
config: Pick<Config, "route" | "routes" | "experimental_assets"> & {
dev: Pick<Config["dev"], "host">;
}
) {
Expand All @@ -1241,7 +1243,12 @@ export async function getHostAndRoutes(
return r.pattern;
}
});

if (routes) {
validateRoutes(
routes,
Boolean(args.experimentalAssets || config.experimental_assets)
);
}
return { host, routes };
}

Expand Down Expand Up @@ -1304,9 +1311,7 @@ export async function validateDevServerSettings(
config,
"dev"
);

const { host, routes } = await getHostAndRoutes(args, config);

// TODO: Remove this hack
// This function throws if the zone ID can't be found given the provided host and routes
// However, it's called as part of initialising a preview session, which is nested deep within
Expand Down
14 changes: 4 additions & 10 deletions packages/wrangler/src/triggers/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
sleep,
updateQueueConsumers,
updateQueueProducers,
validateRoutes,
} from "../deploy/deploy";
import { UserError } from "../errors";
import { logger } from "../logger";
Expand All @@ -17,6 +18,7 @@ import { getZoneForRoute } from "../zones";
import type { Config } from "../config";
import type { Route } from "../config/environment";
import type { RouteObject } from "../deploy/deploy";
import type { ExperimentalAssetsOptions } from "../experimental-assets";

type Props = {
config: Config;
Expand All @@ -28,6 +30,7 @@ type Props = {
legacyEnv: boolean | undefined;
dryRun: boolean | undefined;
experimentalVersions: boolean | undefined;
experimentalAssetsOptions: ExperimentalAssetsOptions | undefined;
};

export default async function triggersDeploy(
Expand All @@ -40,18 +43,9 @@ export default async function triggersDeploy(
props.routes ?? config.routes ?? (config.route ? [config.route] : []) ?? [];
const routesOnly: Array<Route> = [];
const customDomainsOnly: Array<RouteObject> = [];
validateRoutes(routes, Boolean(props.experimentalAssetsOptions));
for (const route of routes) {
if (typeof route !== "string" && route.custom_domain) {
if (route.pattern.includes("*")) {
throw new UserError(
`Cannot use "${route.pattern}" as a Custom Domain; wildcard operators (*) are not allowed`
);
}
if (route.pattern.includes("/")) {
throw new UserError(
`Cannot use "${route.pattern}" as a Custom Domain; paths are not allowed`
);
}
customDomainsOnly.push(route);
} else {
routesOnly.push(route);
Expand Down
6 changes: 6 additions & 0 deletions packages/wrangler/src/triggers/index.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { readConfig } from "../config";
import { processExperimentalAssetsArg } from "../experimental-assets";
import { getScriptName, isLegacyEnv, printWranglerBanner } from "../index";
import * as metrics from "../metrics";
import { requireAuth } from "../user";
Expand Down Expand Up @@ -57,6 +58,10 @@ export async function triggersDeployHandler(
await printWranglerBanner();

const config = readConfig(undefined, args);
const experimentalAssetsOptions = processExperimentalAssetsArg(
{ experimentalAssets: undefined },
config
);
await metrics.sendMetricsEvent(
"deploy worker triggers",
{},
Expand All @@ -77,5 +82,6 @@ export async function triggersDeployHandler(
legacyEnv: isLegacyEnv(config),
dryRun: args.dryRun,
experimentalVersions: args.experimentalJsonConfig,
experimentalAssetsOptions,
});
}

0 comments on commit abbe295

Please sign in to comment.