fix: route deployment fallback for restricted permission API tokens (#…

…1188) * refactor: add assertion to avoid incorrect API resource paths * refactor: move zone finding into its own helpers This makes it available to use elsewhere in the code. * fix: fallback on old zone-based API when account-based route API fails While we wait for changes to the CF API to support API tokens that do not have "All Zone" permissions, this change provides a workaround for most scenarios. If the bulk-route request fails with an authorization error, then we fallback to the Wrangler 1 approach, which sends individual route updates via a zone-based endpoint. Fixes #651
cloudflare · Jun 9, 2022 · b44cc26 · b44cc26
1 parent 2d42882
commit b44cc26
Show file tree

Hide file tree

Showing 12 changed files with 473 additions and 177 deletions.
diff --git a/.changeset/honest-cobras-relate.md b/.changeset/honest-cobras-relate.md
@@ -0,0 +1,14 @@
+---
+"wrangler": patch
+---
+
+fix: fallback on old zone-based API when account-based route API fails
+
+While we wait for changes to the CF API to support API tokens that do not have
+"All Zone" permissions, this change provides a workaround for most scenarios.
+
+If the bulk-route request fails with an authorization error, then we fallback
+to the Wrangler 1 approach, which sends individual route updates via a zone-based
+endpoint.
+
+Fixes #651
diff --git a/packages/wrangler/src/__tests__/dev.test.tsx b/packages/wrangler/src/__tests__/dev.test.tsx
@@ -152,10 +152,12 @@ describe("wrangler dev", () => {
       fs.writeFileSync("index.js", `export default {};`);
       mockGetZones("some-host.com", [{ id: "some-zone-id" }]);
       await runWrangler("dev --host some-host.com");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
-        host: "some-host.com",
-        id: "some-zone-id",
-      });
+      expect((Dev as jest.Mock).mock.calls[0][0]).toEqual(
+        expect.objectContaining({
+          host: "some-host.com",
+          zone: "some-zone-id",
+        })
+      );
     });
 
     it("should read wrangler.toml's dev.host", async () => {
@@ -168,9 +170,7 @@ describe("wrangler dev", () => {
       fs.writeFileSync("index.js", `export default {};`);
       mockGetZones("some-host.com", [{ id: "some-zone-id" }]);
       await runWrangler("dev");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone.host).toEqual(
-        "some-host.com"
-      );
+      expect((Dev as jest.Mock).mock.calls[0][0].host).toEqual("some-host.com");
     });
 
     it("should read --route", async () => {
@@ -180,9 +180,7 @@ describe("wrangler dev", () => {
       fs.writeFileSync("index.js", `export default {};`);
       mockGetZones("some-host.com", [{ id: "some-zone-id" }]);
       await runWrangler("dev --route http://some-host.com/some/path/*");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone.host).toEqual(
-        "some-host.com"
-      );
+      expect((Dev as jest.Mock).mock.calls[0][0].host).toEqual("some-host.com");
     });
 
     it("should read wrangler.toml's routes", async () => {
@@ -196,9 +194,7 @@ describe("wrangler dev", () => {
       fs.writeFileSync("index.js", `export default {};`);
       mockGetZones("some-host.com", [{ id: "some-zone-id" }]);
       await runWrangler("dev");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone.host).toEqual(
-        "some-host.com"
-      );
+      expect((Dev as jest.Mock).mock.calls[0][0].host).toEqual("some-host.com");
     });
 
     it("should read wrangler.toml's environment specific routes", async () => {
@@ -220,9 +216,7 @@ describe("wrangler dev", () => {
       fs.writeFileSync("index.js", `export default {};`);
       mockGetZones("some-host.com", [{ id: "some-zone-id" }]);
       await runWrangler("dev --env staging");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone.host).toEqual(
-        "some-host.com"
-      );
+      expect((Dev as jest.Mock).mock.calls[0][0].host).toEqual("some-host.com");
     });
 
     it("should strip leading `*` from given host when deducing a zone id", async () => {
@@ -233,9 +227,7 @@ describe("wrangler dev", () => {
       fs.writeFileSync("index.js", `export default {};`);
       mockGetZones("some-host.com", [{ id: "some-zone-id" }]);
       await runWrangler("dev");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone.host).toEqual(
-        "some-host.com"
-      );
+      expect((Dev as jest.Mock).mock.calls[0][0].host).toEqual("some-host.com");
     });
 
     it("should strip leading `*.` from given host when deducing a zone id", async () => {
@@ -246,9 +238,7 @@ describe("wrangler dev", () => {
       fs.writeFileSync("index.js", `export default {};`);
       mockGetZones("some-host.com", [{ id: "some-zone-id" }]);
       await runWrangler("dev");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone.host).toEqual(
-        "some-host.com"
-      );
+      expect((Dev as jest.Mock).mock.calls[0][0].host).toEqual("some-host.com");
     });
 
     it("should, when provided, use a configured zone_id", async () => {
@@ -260,10 +250,12 @@ describe("wrangler dev", () => {
       });
       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",
-      });
+      expect((Dev as jest.Mock).mock.calls[0][0]).toEqual(
+        expect.objectContaining({
+          host: "some-domain.com",
+          zone: "some-zone-id",
+        })
+      );
     });
 
     it("should, when provided, use a zone_name to get a zone_id", async () => {
@@ -276,11 +268,13 @@ describe("wrangler dev", () => {
       fs.writeFileSync("index.js", `export default {};`);
       mockGetZones("some-zone.com", [{ id: "a-zone-id" }]);
       await runWrangler("dev");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
-        // note that it uses the provided zone_name as a host too
-        host: "some-zone.com",
-        id: "a-zone-id",
-      });
+      expect((Dev as jest.Mock).mock.calls[0][0]).toEqual(
+        expect.objectContaining({
+          // note that it uses the provided zone_name as a host too
+          host: "some-zone.com",
+          zone: "a-zone-id",
+        })
+      );
     });
 
     it("given a long host, it should use the longest subdomain that resolves to a zone", async () => {
@@ -292,10 +286,12 @@ describe("wrangler dev", () => {
       mockGetZones("222.333.some-host.com", []);
       mockGetZones("333.some-host.com", [{ id: "some-zone-id" }]);
       await runWrangler("dev --host 111.222.333.some-host.com");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
-        host: "111.222.333.some-host.com",
-        id: "some-zone-id",
-      });
+      expect((Dev as jest.Mock).mock.calls[0][0]).toEqual(
+        expect.objectContaining({
+          host: "111.222.333.some-host.com",
+          zone: "some-zone-id",
+        })
+      );
     });
 
     it("should, in order, use args.host/config.dev.host/args.routes/(config.route|config.routes)", async () => {
@@ -310,10 +306,12 @@ describe("wrangler dev", () => {
         routes: ["http://5.some-host.com/some/path/*"],
       });
       await runWrangler("dev");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
-        host: "5.some-host.com",
-        id: "some-zone-id-5",
-      });
+      expect((Dev as jest.Mock).mock.calls[0][0]).toEqual(
+        expect.objectContaining({
+          host: "5.some-host.com",
+          zone: "some-zone-id-5",
+        })
+      );
       (Dev as jest.Mock).mockClear();
 
       // config.route
@@ -323,10 +321,12 @@ describe("wrangler dev", () => {
         route: "https://4.some-host.com/some/path/*",
       });
       await runWrangler("dev");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
-        host: "4.some-host.com",
-        id: "some-zone-id-4",
-      });
+      expect((Dev as jest.Mock).mock.calls[0][0]).toEqual(
+        expect.objectContaining({
+          host: "4.some-host.com",
+          zone: "some-zone-id-4",
+        })
+      );
       (Dev as jest.Mock).mockClear();
 
       // --routes
@@ -336,10 +336,12 @@ describe("wrangler dev", () => {
         route: "https://4.some-host.com/some/path/*",
       });
       await runWrangler("dev --routes http://3.some-host.com/some/path/*");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
-        host: "3.some-host.com",
-        id: "some-zone-id-3",
-      });
+      expect((Dev as jest.Mock).mock.calls[0][0]).toEqual(
+        expect.objectContaining({
+          host: "3.some-host.com",
+          zone: "some-zone-id-3",
+        })
+      );
       (Dev as jest.Mock).mockClear();
 
       // config.dev.host
@@ -352,10 +354,12 @@ describe("wrangler dev", () => {
         route: "4.some-host.com/some/path/*",
       });
       await runWrangler("dev --routes http://3.some-host.com/some/path/*");
-      expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
-        host: "2.some-host.com",
-        id: "some-zone-id-2",
-      });
+      expect((Dev as jest.Mock).mock.calls[0][0]).toEqual(
+        expect.objectContaining({
+          host: "2.some-host.com",
+          zone: "some-zone-id-2",
+        })
+      );
       (Dev as jest.Mock).mockClear();
 
       // --host
@@ -370,10 +374,12 @@ describe("wrangler dev", () => {
       await runWrangler(
         "dev --routes http://3.some-host.com/some/path/* --host 1.some-host.com"
       );
-      expect((Dev as jest.Mock).mock.calls[0][0].zone).toEqual({
-        host: "1.some-host.com",
-        id: "some-zone-id-1",
-      });
+      expect((Dev as jest.Mock).mock.calls[0][0]).toEqual(
+        expect.objectContaining({
+          host: "1.some-host.com",
+          zone: "some-zone-id-1",
+        })
+      );
       (Dev as jest.Mock).mockClear();
     });
 

diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts
@@ -649,8 +649,85 @@ describe("publish", () => {
       await runWrangler("publish ./index --env dev --legacy-env false");
     });
 
+    it("should fallback to the Wrangler 1 zone-based API if the bulk-routes API fails", async () => {
+      writeWranglerToml({
+        routes: ["example.com/some-route/*"],
+      });
+      writeWorkerSource();
+      mockUpdateWorkerRequest({ enabled: false });
+      mockUploadWorkerRequest({ expectedType: "esm" });
+      // Simulate the bulk-routes API failing with a not authorized error.
+      mockUnauthorizedPublishRoutesRequest();
+      // Simulate that the worker has already been deployed to another route in this zone.
+      mockCollectKnownRoutesRequest([
+        {
+          pattern: "foo.example.com/other-route",
+          script: "test-name",
+        },
+      ]);
+      mockGetZoneFromHostRequest("example.com", "some-zone-id");
+      mockPublishRoutesFallbackRequest({
+        pattern: "example.com/some-route/*",
+        script: "test-name",
+      });
+      await runWrangler("publish ./index");
+
+      expect(std.err).toMatchInlineSnapshot(`""`);
+      expect(std.warn).toMatchInlineSnapshot(`
+        "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mThe current authentication token does not have 'All Zones' permissions.[0m
+
+          Falling back to using the zone-based API endpoint to update each route individually.
+          Note that there is no access to read or create routes associated with zones that the API token
+          does not have permission for.
+          No existing routes will been deleted in this case.
+
+
+        [33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mPreviously deployed routes:[0m
+
+          The following routes were already associated with this worker, and have not been deleted:
+           - \\"foo.example.com/other-route\\"
+          If these routes are not wanted then you can remove them in the dashboard.
+
+        "
+      `);
+      expect(std.out).toMatchInlineSnapshot(`
+        "Uploaded test-name (TIMINGS)
+        Published test-name (TIMINGS)
+          example.com/some-route/*"
+      `);
+    });
+
+    it("should error if the bulk-routes API fails and trying to push to a non-production environment", async () => {
+      writeWranglerToml({
+        routes: ["example.com/some-route/*"],
+        legacy_env: false,
+      });
+      writeWorkerSource();
+      mockUpdateWorkerRequest({ env: "staging", enabled: false });
+      mockUploadWorkerRequest({ env: "staging", expectedType: "esm" });
+      // Simulate the bulk-routes API failing with a not authorized error.
+      mockUnauthorizedPublishRoutesRequest({ env: "staging" });
+      // Simulate that the worker has already been deployed to another route in this zone.
+      mockCollectKnownRoutesRequest([
+        {
+          pattern: "foo.example.com/other-route",
+          script: "test-name",
+        },
+      ]);
+      mockGetZoneFromHostRequest("example.com", "some-zone-id");
+      mockPublishRoutesFallbackRequest({
+        pattern: "example.com/some-route/*",
+        script: "test-name",
+      });
+      await expect(runWrangler("publish ./index --env=staging")).rejects
+        .toThrowErrorMatchingInlineSnapshot(`
+              "Service environments combined with an API token that doesn't have 'All Zones' permissions is not supported.
+              Either turn off service environments by setting \`legacy_env = true\`, creating an API token with 'All Zones' permissions, or logging in via OAuth"
+            `);
+    });
+
     describe("custom domains", () => {
-      it("should publish routes marked with 'custom_domain' as seperate custom domains", async () => {
+      it("should publish routes marked with 'custom_domain' as separate custom domains", async () => {
         writeWranglerToml({
           routes: [{ pattern: "api.example.com", custom_domain: true }],
         });
@@ -5004,7 +5081,7 @@ addEventListener('fetch', event => {});`
           "debug": "",
           "err": "",
           "out": "--dry-run: exiting now.",
-          "warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling node.js compatibility mode for builtins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
+          "warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
 
         ",
         }
@@ -5048,7 +5125,7 @@ addEventListener('fetch', event => {});`
           "debug": "",
           "err": "",
           "out": "--dry-run: exiting now.",
-          "warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling node.js compatibility mode for builtins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
+          "warn": "[33m▲ [43;33m[[43;30mWARNING[43;33m][0m [1mEnabling node.js compatibility mode for built-ins and globals. This is experimental and has serious tradeoffs. Please see https://github.com/ionic-team/rollup-plugin-node-polyfills/ for more details.[0m
 
         ",
         }
@@ -5226,6 +5303,49 @@ function mockPublishRoutesRequest({
   );
 }
 
+function mockUnauthorizedPublishRoutesRequest({
+  env = undefined,
+  legacyEnv = false,
+}: {
+  env?: string | undefined;
+  legacyEnv?: boolean | undefined;
+} = {}) {
+  const servicesOrScripts = env && !legacyEnv ? "services" : "scripts";
+  const environment = env && !legacyEnv ? "/environments/:envName" : "";
+
+  setMockRawResponse(
+    `/accounts/:accountId/workers/${servicesOrScripts}/:scriptName${environment}/routes`,
+    "PUT",
+    () =>
+      createFetchResult(null, false, [
+        { message: "Authentication error", code: 10000 },
+      ])
+  );
+}
+
+function mockCollectKnownRoutesRequest(
+  routes: { pattern: string; script: string }[]
+) {
+  setMockResponse(`/zones/:zoneId/workers/routes`, "GET", () => routes);
+}
+
+function mockGetZoneFromHostRequest(host: string, zone: string) {
+  setMockResponse("/zones", (_uri, _init, queryParams) => {
+    expect(queryParams.get("name")).toEqual(host);
+    return [{ id: zone }];
+  });
+}
+
+function mockPublishRoutesFallbackRequest(route: {
+  pattern: string;
+  script: string;
+}) {
+  setMockResponse(`/zones/:zoneId/workers/routes`, "POST", (_url, { body }) => {
+    expect(JSON.parse(body as string)).toEqual(route);
+    return route.pattern;
+  });
+}
+
 function mockPublishCustomDomainsRequest({
   publishFlags,
   domains = [],