Skip to content

Commit

Permalink
fix: use account id for listing zones
Browse files Browse the repository at this point in the history
Fixes #4944

Trying. to fetch `/zones` fails when it spans more than 500 zones. The fix to use an account id when doing so. This patch passes the account id to the zones call, threading it through all the functions that require it.
  • Loading branch information
threepointone committed Jun 28, 2024
1 parent 7ed675e commit 3dafbf4
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 45 deletions.
9 changes: 9 additions & 0 deletions .changeset/grumpy-bikes-prove.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"wrangler": patch
---

fix: use account id for listing zones

Fixes https://github.com/cloudflare/workers-sdk/issues/4944

Trying to fetch `/zones` fails when it spans more than 500 zones. The fix to use an account id when doing so. This patch passes the account id to the zones call, threading it through all the functions that require it.
45 changes: 33 additions & 12 deletions packages/wrangler/src/__tests__/zones.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,22 @@ describe("Zones", () => {
describe("getZoneForRoute", () => {
test("string route", async () => {
mockGetZones("example.com", [{ id: "example-id" }]);
expect(await getZoneForRoute("example.com/*")).toEqual({
expect(
await getZoneForRoute({
route: "example.com/*",
accountId: "some-account-id",
})
).toEqual({
host: "example.com",
id: "example-id",
});
});

test("string route (not a zone)", async () => {
mockGetZones("wrong.com", []);
await expect(getZoneForRoute("wrong.com/*")).rejects
.toMatchInlineSnapshot(`
await expect(
getZoneForRoute({ route: "wrong.com/*", accountId: "some-account-id" })
).rejects.toMatchInlineSnapshot(`
[Error: Could not find zone for \`wrong.com\`. Make sure the domain is set up to be proxied by Cloudflare.
For more details, refer to https://developers.cloudflare.com/workers/configuration/routing/routes/#set-up-a-route]
`);
Expand All @@ -86,7 +92,10 @@ describe("Zones", () => {
// when a zone_id is provided in the route
mockGetZones("example.com", [{ id: "example-id" }]);
expect(
await getZoneForRoute({ pattern: "example.com/*", zone_id: "other-id" })
await getZoneForRoute({
route: { pattern: "example.com/*", zone_id: "other-id" },
accountId: "some-account-id",
})
).toEqual({
host: "example.com",
id: "other-id",
Expand All @@ -98,8 +107,11 @@ describe("Zones", () => {
mockGetZones("example.com", [{ id: "example-id" }]);
expect(
await getZoneForRoute({
pattern: "some.third-party.com/*",
zone_id: "other-id",
route: {
pattern: "some.third-party.com/*",
zone_id: "other-id",
},
accountId: "some-account-id",
})
).toEqual({
host: "some.third-party.com",
Expand All @@ -111,8 +123,11 @@ describe("Zones", () => {
mockGetZones("example.com", [{ id: "example-id" }]);
expect(
await getZoneForRoute({
pattern: "example.com/*",
zone_name: "example.com",
route: {
pattern: "example.com/*",
zone_name: "example.com",
},
accountId: "some-account-id",
})
).toEqual({
host: "example.com",
Expand All @@ -123,8 +138,11 @@ describe("Zones", () => {
mockGetZones("example.com", [{ id: "example-id" }]);
expect(
await getZoneForRoute({
pattern: "subdomain.example.com/*",
zone_name: "example.com",
route: {
pattern: "subdomain.example.com/*",
zone_name: "example.com",
},
accountId: "some-account-id",
})
).toEqual({
host: "subdomain.example.com",
Expand All @@ -135,8 +153,11 @@ describe("Zones", () => {
mockGetZones("example.com", [{ id: "example-id" }]);
expect(
await getZoneForRoute({
pattern: "some.third-party.com/*",
zone_name: "example.com",
route: {
pattern: "some.third-party.com/*",
zone_name: "example.com",
},
accountId: "some-account-id",
})
).toEqual({
host: "some.third-party.com",
Expand Down
22 changes: 18 additions & 4 deletions packages/wrangler/src/deploy/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -878,7 +878,13 @@ export async function publishRoutes(
workerUrl,
scriptName,
notProd,
}: { workerUrl: string; scriptName: string; notProd: boolean }
accountId,
}: {
workerUrl: string;
scriptName: string;
notProd: boolean;
accountId: string;
}
): Promise<string[]> {
try {
return await fetchResult(`${workerUrl}/routes`, {
Expand All @@ -897,7 +903,11 @@ export async function publishRoutes(
if (isAuthenticationError(e)) {
// An authentication error is probably due to a known issue,
// where the user is logged in via an API token that does not have "All Zones".
return await publishRoutesFallback(routes, { scriptName, notProd });
return await publishRoutesFallback(routes, {
scriptName,
notProd,
accountId,
});
} else {
throw e;
}
Expand All @@ -911,7 +921,11 @@ export async function publishRoutes(
*/
async function publishRoutesFallback(
routes: Route[],
{ scriptName, notProd }: { scriptName: string; notProd: boolean }
{
scriptName,
notProd,
accountId,
}: { scriptName: string; notProd: boolean; accountId: string }
) {
if (notProd) {
throw new UserError(
Expand All @@ -932,7 +946,7 @@ async function publishRoutesFallback(
const activeZones = new Map<string, string>();
const routesToDeploy = new Map<string, string>();
for (const route of routes) {
const zone = await getZoneForRoute(route);
const zone = await getZoneForRoute({ route, accountId });
if (zone) {
activeZones.set(zone.id, zone.host);
routesToDeploy.set(
Expand Down
3 changes: 2 additions & 1 deletion packages/wrangler/src/dev.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -912,7 +912,8 @@ export async function validateDevServerSettings(
// we want it to exit the Wrangler process early to allow the user to fix it. Calling it here forces
// the error to be thrown where it will correctly exit the Wrangler process
if (args.remote) {
await getZoneIdForPreview(host, routes);
assert(config.account_id);
await getZoneIdForPreview({ host, routes, accountId: config.account_id });
}
const initialIp = args.ip || config.dev.ip;
const initialIpListenCheck = initialIp === "*" ? "0.0.0.0" : initialIp;
Expand Down
6 changes: 5 additions & 1 deletion packages/wrangler/src/dev/remote.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -701,7 +701,11 @@ export async function getWorkerAccountAndContext(props: {
};

// What zone should the realish preview for this Worker run on?
const zoneId = await getZoneIdForPreview(props.host, props.routes);
const zoneId = await getZoneIdForPreview({
host: props.host,
routes: props.routes,
accountId: props.accountId,
});

const workerContext: CfWorkerContext = {
env: props.env,
Expand Down
7 changes: 6 additions & 1 deletion packages/wrangler/src/tail/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import assert from "node:assert";
import { setTimeout } from "node:timers/promises";
import onExit from "signal-exit";
import { readConfig } from "../config";
Expand Down Expand Up @@ -101,7 +102,11 @@ export async function tailHandler(args: TailArgs) {

// Worker names can't contain "." (and most routes should), so use that as a discriminator
if (args.worker?.includes(".")) {
scriptName = await getWorkerForZone(args.worker);
assert(config.account_id);
scriptName = await getWorkerForZone({
worker: args.worker,
accountId: config.account_id,
});
if (args.format === "pretty") {
logger.log(`Connecting to worker ${scriptName} at route ${args.worker}`);
}
Expand Down
9 changes: 7 additions & 2 deletions packages/wrangler/src/triggers/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ export default async function triggersDeploy(props: Props): Promise<void> {
// to bother with all the error handling tomfoolery.
const routesWithOtherBindings: Record<string, string[]> = {};
for (const route of routes) {
const zone = await getZoneForRoute(route);
const zone = await getZoneForRoute({ route, accountId });
if (!zone) {
continue;
}
Expand Down Expand Up @@ -194,7 +194,12 @@ export default async function triggersDeploy(props: Props): Promise<void> {
// Update routing table for the script.
if (routesOnly.length > 0) {
deployments.push(
publishRoutes(routesOnly, { workerUrl, scriptName, notProd }).then(() => {
publishRoutes(routesOnly, {
workerUrl,
scriptName,
notProd,
accountId,
}).then(() => {
if (routesOnly.length > 10) {
return routesOnly
.slice(0, 9)
Expand Down
46 changes: 31 additions & 15 deletions packages/wrangler/src/zones.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,16 +49,20 @@ export function getHostFromRoute(route: Route): string | undefined {
* - We try to extract a host from it
* - We try to get a zone id from the host
*/
export async function getZoneForRoute(route: Route): Promise<Zone | undefined> {
export async function getZoneForRoute(from: {
route: Route;
accountId: string;
}): Promise<Zone | undefined> {
const { route, accountId } = from;
const host = getHostFromRoute(route);
let id: string | undefined;

if (typeof route === "object" && "zone_id" in route) {
id = route.zone_id;
} else if (typeof route === "object" && "zone_name" in route) {
id = await getZoneIdFromHost(route.zone_name);
id = await getZoneIdFromHost({ host: route.zone_name, accountId });
} else if (host) {
id = await getZoneIdFromHost(host);
id = await getZoneIdFromHost({ host, accountId });
}

return id && host ? { id, host } : undefined;
Expand Down Expand Up @@ -93,17 +97,19 @@ export function getHostFromUrl(urlLike: string): string | undefined {
return undefined;
}
}
export async function getZoneIdForPreview(
host: string | undefined,
routes: Route[] | undefined
) {
export async function getZoneIdForPreview(from: {
host: string | undefined;
routes: Route[] | undefined;
accountId: string;
}) {
const { host, routes, accountId } = from;
let zoneId: string | undefined;
if (host) {
zoneId = await getZoneIdFromHost(host);
zoneId = await getZoneIdFromHost({ host, accountId });
}
if (!zoneId && routes) {
const firstRoute = routes[0];
const zone = await getZoneForRoute(firstRoute);
const zone = await getZoneForRoute({ route: firstRoute, accountId });
if (zone) {
zoneId = zone.id;
}
Expand All @@ -117,14 +123,20 @@ export async function getZoneIdForPreview(
* For each domain-like part of the host (e.g. w.x.y.z) try to get a zone id for it by
* lopping off subdomains until we get a hit from the API.
*/
export async function getZoneIdFromHost(host: string): Promise<string> {
const hostPieces = host.split(".");
export async function getZoneIdFromHost(from: {
host: string;
accountId: string;
}): Promise<string> {
const hostPieces = from.host.split(".");

while (hostPieces.length > 1) {
const zones = await fetchListResult<{ id: string }>(
`/zones`,
{},
new URLSearchParams({ name: hostPieces.join(".") })
new URLSearchParams({
name: hostPieces.join("."),
"account.id": from.accountId,
})
);
if (zones.length > 0) {
return zones[0].id;
Expand All @@ -133,7 +145,7 @@ export async function getZoneIdFromHost(host: string): Promise<string> {
}

throw new UserError(
`Could not find zone for \`${host}\`. Make sure the domain is set up to be proxied by Cloudflare.\nFor more details, refer to https://developers.cloudflare.com/workers/configuration/routing/routes/#set-up-a-route`
`Could not find zone for \`${from.host}\`. Make sure the domain is set up to be proxied by Cloudflare.\nFor more details, refer to https://developers.cloudflare.com/workers/configuration/routing/routes/#set-up-a-route`
);
}

Expand Down Expand Up @@ -200,8 +212,12 @@ export function findClosestRoute(
/**
* Given a route (must be assigned and within the correct zone), return the name of the worker assigned to it
*/
export async function getWorkerForZone(worker: string) {
const zone = await getZoneForRoute(worker);
export async function getWorkerForZone(from: {
worker: string;
accountId: string;
}) {
const { worker, accountId } = from;
const zone = await getZoneForRoute({ route: worker, accountId });
if (!zone) {
throw new UserError(
`The route '${worker}' is not part of one of your zones. Either add this zone from the Cloudflare dashboard, or try using a route within one of your existing zones.`
Expand Down
18 changes: 9 additions & 9 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 3dafbf4

Please sign in to comment.