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 #774
cloudflare · Apr 11, 2022 · 184b547 · 184b547
1 parent 664803e
commit 184b547
Show file tree

Hide file tree

Showing 9 changed files with 295 additions and 69 deletions.
diff --git a/.changeset/dull-jobs-return.md b/.changeset/dull-jobs-return.md
@@ -0,0 +1,13 @@
+---
+"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: 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
diff --git a/packages/wrangler/src/__tests__/configuration.test.ts b/packages/wrangler/src/__tests__/configuration.test.ts
@@ -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",
@@ -630,7 +635,21 @@ describe("normalizeAndValidateConfig()", () => {
         compatibility_date: 333,
         compatibility_flags: [444, 555],
         workers_dev: "BAD",
-        routes: [666, 777],
+        routes: [
+          666,
+          777,
+          // this one's valid
+          "example.com/*",
+          { 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,
@@ -655,8 +674,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 either a string or an object with shape {pattern: string, zone_id: string}, but got 888.
+          - 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}].
           - 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.
@@ -1797,8 +1816,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 either a string or an object with shape {pattern: string, zone_id: string}, but got 888.
+            - 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].
             - 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.

diff --git a/packages/wrangler/src/__tests__/dev.test.tsx b/packages/wrangler/src/__tests__/dev.test.tsx
@@ -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",

diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts
@@ -311,6 +311,91 @@ describe("publish", () => {
       await runWrangler("publish ./index");
     });
 
+    it("should publish to a route with a pattern/zone_id combo", async () => {
+      writeWranglerToml({
+        routes: [
+          "some-example.com/some-route/*",
+          { pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
+          { pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
+          "more-examples.com/*",
+        ],
+      });
+      writeWorkerSource();
+      mockUpdateWorkerRequest({ enabled: false });
+      mockUploadWorkerRequest({ expectedType: "esm" });
+      mockPublishRoutesRequest({
+        routes: [
+          "some-example.com/some-route/*",
+          { pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
+          { pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
+          "more-examples.com/*",
+        ],
+      });
+      await runWrangler("publish ./index");
+      expect(std).toMatchInlineSnapshot(`
+        Object {
+          "err": "",
+          "out": "Uploaded test-name (TIMINGS)
+        Published test-name (TIMINGS)
+          some-example.com/some-route/*
+          *a-boring-website.com (zone: 54sdf7fsda)
+          example.com/some-route/* (zone: JGHFHG654gjcj)
+          more-examples.com/*",
+          "warn": "",
+        }
+      `);
+    });
+
+    it("should publish to a route with a pattern/zone_id combo (service environments)", async () => {
+      writeWranglerToml({
+        env: {
+          staging: {
+            routes: [
+              "some-example.com/some-route/*",
+              { pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
+              { pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
+              "more-examples.com/*",
+            ],
+          },
+        },
+      });
+      mockSubDomainRequest();
+      writeWorkerSource();
+      mockUpdateWorkerRequest({
+        enabled: false,
+        env: "staging",
+        legacyEnv: false,
+      });
+      mockUploadWorkerRequest({
+        expectedType: "esm",
+        env: "staging",
+        legacyEnv: false,
+      });
+      mockPublishRoutesRequest({
+        routes: [
+          "some-example.com/some-route/*",
+          { pattern: "*a-boring-website.com", zone_id: "54sdf7fsda" },
+          { pattern: "example.com/some-route/*", zone_id: "JGHFHG654gjcj" },
+          "more-examples.com/*",
+        ],
+        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)
+          some-example.com/some-route/*
+          *a-boring-website.com (zone: 54sdf7fsda)
+          example.com/some-route/* (zone: JGHFHG654gjcj)
+          more-examples.com/*",
+          "warn": "",
+        }
+      `);
+    });
+
     it("should publish to legacy environment specific routes", async () => {
       writeWranglerToml({
         routes: ["example.com/some-route/*"],
@@ -3829,7 +3914,7 @@ function mockPublishRoutesRequest({
   env = undefined,
   legacyEnv = false,
 }: {
-  routes: string[];
+  routes: (string | { pattern: string; zone_id: string })[];
   env?: string | undefined;
   legacyEnv?: boolean | undefined;
 }) {
@@ -3849,7 +3934,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;
     }

diff --git a/packages/wrangler/src/config/config.ts b/packages/wrangler/src/config/config.ts
@@ -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>;

diff --git a/packages/wrangler/src/config/environment.ts b/packages/wrangler/src/config/environment.ts
@@ -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
@@ -85,7 +85,7 @@ interface EnvironmentInheritable {
    *
    * @inheritable
    */
-  route: string | undefined;
+  route: (string | { pattern: string; zone_id: string }) | undefined;
 
   /**
    * Path to a custom tsconfig

diff --git a/packages/wrangler/src/config/validation.ts b/packages/wrangler/src/config/validation.ts
@@ -461,6 +461,97 @@ 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" &&
+        hasProperty(item, "pattern") &&
+        typeof item.pattern === "string" &&
+        hasProperty(item, "zone_id") &&
+        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 either a string or an object with shape {pattern: string, zone_id: string}, 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) {
+    return true;
+  }
+  if (!Array.isArray(value)) {
+    diagnostics.errors.push(
+      `Expected "${field}" to be an array but got ${JSON.stringify(value)}.`
+    );
+    return false;
+  }
+  const invalidRoutes = [];
+  for (const item of value) {
+    if (!isValidRouteValue(item)) {
+      invalidRoutes.push(item);
+    }
+  }
+  if (invalidRoutes.length > 0) {
+    diagnostics.errors.push(
+      `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(
+        invalidRoutes
+      )}.`
+    );
+  }
+  return invalidRoutes.length === 0;
+};
+
+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.
  */
@@ -526,22 +617,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,