non-TTY required variables checks (#852)

* non-TTY check for required variables: Added a check in non-TTY environments for `account_id`, `CLOUDFLARE_ACCOUNT_ID` and `CLOUDFLARE_API_TOKEN`. If `account_id` exists in `wrangler.toml` then `CLOUDFLARE_ACCOUNT_ID` is not needed in non-TTY scope. The `CLOUDFLARE_API_TOKEN` is necessary in non-TTY scope and will always error if missing. resolves #827 * non-TTY check for required variables: Added a check in non-TTY environments for `account_id`, `CLOUDFLARE_ACCOUNT_ID` and `CLOUDFLARE_API_TOKEN`. If `account_id` exists in `wrangler.toml` then `CLOUDFLARE_ACCOUNT_ID` is not needed in non-TTY scope. The `CLOUDFLARE_API_TOKEN` is necessary in non-TTY scope and will always error if missing. resolves #827
cloudflare · May 4, 2022 · 6283ad5 · 6283ad5
1 parent d0801b7
commit 6283ad5
Show file tree

Hide file tree

Showing 12 changed files with 271 additions and 58 deletions.
diff --git a/.changeset/eighty-yaks-jump.md b/.changeset/eighty-yaks-jump.md
@@ -0,0 +1,9 @@
+---
+"wrangler": patch
+---
+
+feat: non-TTY check for required variables
+Added a check in non-TTY environments for `account_id`, `CLOUDFLARE_ACCOUNT_ID` and `CLOUDFLARE_API_TOKEN`. If `account_id` exists in `wrangler.toml`
+then `CLOUDFLARE_ACCOUNT_ID` is not needed in non-TTY scope. The `CLOUDFLARE_API_TOKEN` is necessary in non-TTY scope and will always error if missing.
+
+resolves #827
diff --git a/packages/wrangler/src/__tests__/helpers/mock-istty.ts b/packages/wrangler/src/__tests__/helpers/mock-istty.ts
@@ -1,4 +1,5 @@
-const ORIGINAL_ISTTY = process.stdout.isTTY;
+const ORIGINAL_STDOUT_ISTTY = process.stdout.isTTY;
+const ORIGINAL_STDIN_ISTTY = process.stdin.isTTY;
 
 /**
  * Mock `process.stdout.isTTY`
@@ -9,14 +10,17 @@ export function useMockIsTTY() {
    */
   const setIsTTY = (isTTY: boolean) => {
     process.stdout.isTTY = isTTY;
+    process.stdin.isTTY = isTTY;
   };
 
   beforeEach(() => {
-    process.stdout.isTTY = ORIGINAL_ISTTY;
+    process.stdout.isTTY = ORIGINAL_STDOUT_ISTTY;
+    process.stdin.isTTY = ORIGINAL_STDIN_ISTTY;
   });
 
   afterEach(() => {
-    process.stdout.isTTY = ORIGINAL_ISTTY;
+    process.stdout.isTTY = ORIGINAL_STDOUT_ISTTY;
+    process.stdin.isTTY = ORIGINAL_STDIN_ISTTY;
   });
 
   return { setIsTTY };

diff --git a/packages/wrangler/src/__tests__/helpers/mock-oauth-flow.ts b/packages/wrangler/src/__tests__/helpers/mock-oauth-flow.ts
@@ -1,5 +1,6 @@
 import fetchMock from "jest-fetch-mock";
 import { Request } from "undici";
+import { getCloudflareApiBaseUrl } from "../../cfetch";
 import openInBrowser from "../../open-in-browser";
 import { mockHttpServer } from "./mock-http-server";
 
@@ -85,6 +86,28 @@ export const mockOAuthFlow = () => {
     return outcome;
   };
 
+  const mockGetMemberships = (args: {
+    success: boolean;
+    result: { id: string; account: { id: string; name: string } }[];
+  }) => {
+    const outcome = {
+      actual: new Request("https://example.org"),
+      expected: new Request(`${getCloudflareApiBaseUrl()}/memberships`, {
+        method: "GET",
+      }),
+    };
+
+    fetchMock.mockIf(outcome.expected.url, async (req) => {
+      outcome.actual = req;
+      return {
+        status: 200,
+        body: JSON.stringify(args),
+      };
+    });
+
+    return outcome;
+  };
+
   const mockGrantAccessToken = ({
     respondWith,
   }: {
@@ -150,10 +173,11 @@ export const mockOAuthFlow = () => {
   };
 
   return {
-    mockOAuthServerCallback,
+    mockGetMemberships,
+    mockGrantAccessToken,
     mockGrantAuthorization,
+    mockOAuthServerCallback,
     mockRevokeAuthorization,
-    mockGrantAccessToken,
     mockExchangeRefreshTokenForAccessToken,
   };
 };

diff --git a/packages/wrangler/src/__tests__/publish.test.ts b/packages/wrangler/src/__tests__/publish.test.ts
@@ -11,6 +11,7 @@ import {
   unsetMockFetchKVGetValues,
 } from "./helpers/mock-cfetch";
 import { mockConsoleMethods, normalizeSlashes } from "./helpers/mock-console";
+import { useMockIsTTY } from "./helpers/mock-istty";
 import { mockKeyListRequest } from "./helpers/mock-kv";
 import { mockOAuthFlow } from "./helpers/mock-oauth-flow";
 import { runInTempDir } from "./helpers/run-in-tmp";
@@ -27,18 +28,21 @@ describe("publish", () => {
   mockAccountId();
   mockApiToken();
   runInTempDir({ homedir: "./home" });
+  const { setIsTTY } = useMockIsTTY();
   const std = mockConsoleMethods();
   const {
     mockOAuthServerCallback,
     mockGrantAccessToken,
     mockGrantAuthorization,
+    mockGetMemberships,
   } = mockOAuthFlow();
 
   beforeEach(() => {
     // @ts-expect-error we're using a very simple setTimeout mock here
     jest.spyOn(global, "setTimeout").mockImplementation((fn, _period) => {
       setImmediate(fn);
     });
+    setIsTTY(true);
   });
 
   afterEach(() => {
@@ -116,6 +120,150 @@ describe("publish", () => {
       `);
       expect(std.err).toMatchInlineSnapshot(`""`);
     });
+
+    describe("non-TTY", () => {
+      const ENV_COPY = process.env;
+
+      afterEach(() => {
+        process.env = ENV_COPY;
+      });
+
+      it("should not throw an error in non-TTY if 'CLOUDFLARE_API_TOKEN' & 'account_id' are in scope", async () => {
+        process.env = {
+          CLOUDFLARE_API_TOKEN: "123456789",
+        };
+        setIsTTY(false);
+        writeWranglerToml({
+          account_id: "some-account-id",
+        });
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+
+        await runWrangler("publish index.js");
+
+        expect(std.out).toMatchInlineSnapshot(`
+          "Uploaded test-name (TIMINGS)
+          Published test-name (TIMINGS)
+            test-name.test-sub-domain.workers.dev"
+        `);
+        expect(std.err).toMatchInlineSnapshot(`""`);
+      });
+
+      it("should not throw an error if 'CLOUDFLARE_ACCOUNT_ID' & 'CLOUDFLARE_API_TOKEN' are in scope", async () => {
+        process.env = {
+          CLOUDFLARE_API_TOKEN: "hunter2",
+          CLOUDFLARE_ACCOUNT_ID: "some-account-id",
+        };
+        setIsTTY(false);
+        writeWranglerToml();
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+        mockGetMemberships({
+          success: true,
+          result: [],
+        });
+
+        await runWrangler("publish index.js");
+
+        expect(std.out).toMatchInlineSnapshot(`
+          "Uploaded test-name (TIMINGS)
+          Published test-name (TIMINGS)
+            test-name.test-sub-domain.workers.dev"
+        `);
+        expect(std.err).toMatchInlineSnapshot(`""`);
+      });
+
+      it("should throw an error in non-TTY & there is more than one account associated with API token", async () => {
+        setIsTTY(false);
+        process.env = {
+          CLOUDFLARE_API_TOKEN: "hunter2",
+          CLOUDFLARE_ACCOUNT_ID: undefined,
+        };
+        writeWranglerToml({
+          account_id: undefined,
+        });
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+        mockGetMemberships({
+          success: true,
+          result: [
+            { id: "IG-88", account: { id: "1701", name: "enterprise" } },
+            { id: "R2-D2", account: { id: "nx01", name: "enterprise-nx" } },
+          ],
+        });
+
+        await expect(runWrangler("publish index.js")).rejects
+          .toMatchInlineSnapshot(`
+                [Error: More than one account available but unable to select one in non-interactive mode.
+                Please set the appropriate \`account_id\` in your \`wrangler.toml\` file.
+                Available accounts are ("<name>" - "<id>"):
+                  "enterprise" - "1701")
+                  "enterprise-nx" - "nx01")]
+              `);
+      });
+
+      it("should throw error in non-TTY if 'CLOUDFLARE_API_TOKEN' is missing", async () => {
+        setIsTTY(false);
+        writeWranglerToml({
+          account_id: undefined,
+        });
+        process.env = {
+          CLOUDFLARE_API_TOKEN: undefined,
+          CLOUDFLARE_ACCOUNT_ID: "badwolf",
+        };
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+        mockGetMemberships({
+          success: true,
+          result: [
+            { id: "IG-88", account: { id: "1701", name: "enterprise" } },
+            { id: "R2-D2", account: { id: "nx01", name: "enterprise-nx" } },
+          ],
+        });
+
+        await expect(runWrangler("publish index.js")).rejects.toThrowError();
+
+        expect(std.err).toMatchInlineSnapshot(`
+          "[31mX [41;31m[[41;97mERROR[41;31m][0m [1mIn a non-interactive environment, it's necessary to set a CLOUDFLARE_API_TOKEN environment variable for wrangler to work. Please go to https://developers.cloudflare.com/api/tokens/create/ for instructions on how to create an api token, and assign its value to CLOUDFLARE_API_TOKEN.[0m
+
+          "
+        `);
+      });
+      it("should throw error with no account ID provided and no members retrieved", async () => {
+        setIsTTY(false);
+        writeWranglerToml({
+          account_id: undefined,
+        });
+        process.env = {
+          CLOUDFLARE_API_TOKEN: "picard",
+          CLOUDFLARE_ACCOUNT_ID: undefined,
+        };
+        writeWorkerSource();
+        mockSubDomainRequest();
+        mockUploadWorkerRequest();
+        mockOAuthServerCallback();
+        mockGetMemberships({
+          success: false,
+          result: [],
+        });
+
+        await expect(runWrangler("publish index.js")).rejects.toThrowError();
+
+        expect(std.err).toMatchInlineSnapshot(`
+          "[31mX [41;31m[[41;97mERROR[41;31m][0m [1mFailed to automatically retrieve account IDs for the logged in user. In a non-interactive environment, it is mandatory to specify an account ID, either by assigning its value to CLOUDFLARE_ACCOUNT_ID, or as \`account_id\` in your \`wrangler.toml\` file.[0m
+
+          "
+        `);
+      });
+    });
   });
 
   describe("environments", () => {

diff --git a/packages/wrangler/src/__tests__/secret.test.ts b/packages/wrangler/src/__tests__/secret.test.ts
@@ -1,16 +1,18 @@
 import * as fs from "node:fs";
 import * as TOML from "@iarna/toml";
-import fetchMock from "jest-fetch-mock";
 import { mockAccountId, mockApiToken } from "./helpers/mock-account-id";
 import { setMockResponse, unsetAllMocks } from "./helpers/mock-cfetch";
 import { mockConsoleMethods } from "./helpers/mock-console";
 import { mockConfirm, mockPrompt } from "./helpers/mock-dialogs";
+import { mockOAuthFlow } from "./helpers/mock-oauth-flow";
 import { useMockStdin } from "./helpers/mock-stdin";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import { runWrangler } from "./helpers/run-wrangler";
 
 describe("wrangler secret", () => {
   const std = mockConsoleMethods();
+  const { mockGetMemberships } = mockOAuthFlow();
+
   runInTempDir();
   mockAccountId();
   mockApiToken();
@@ -169,10 +171,14 @@ describe("wrangler secret", () => {
         mockAccountId({ accountId: null });
 
         it("should error if a user has no account", async () => {
+          mockGetMemberships({
+            success: false,
+            result: [],
+          });
           await expect(
             runWrangler("secret put the-key --name script-name")
           ).rejects.toThrowErrorMatchingInlineSnapshot(
-            `"No account id found, quitting..."`
+            `"Failed to automatically retrieve account IDs for the logged in user. In a non-interactive environment, it is mandatory to specify an account ID, either by assigning its value to CLOUDFLARE_ACCOUNT_ID, or as \`account_id\` in your \`wrangler.toml\` file."`
           );
         });
 
@@ -196,28 +202,24 @@ describe("wrangler secret", () => {
         });
 
         it("should error if a user has multiple accounts, and has not specified an account in wrangler.toml", async () => {
-          // This is a mock response for the request to the CF API memberships of the current user.
-          fetchMock.doMockOnce(async () => {
-            return {
-              body: JSON.stringify({
-                success: true,
-                result: [
-                  {
-                    id: "1",
-                    account: { id: "account-id-1", name: "account-name-1" },
-                  },
-                  {
-                    id: "2",
-                    account: { id: "account-id-2", name: "account-name-2" },
-                  },
-                  {
-                    id: "3",
-                    account: { id: "account-id-3", name: "account-name-3" },
-                  },
-                ],
-              }),
-            };
+          mockGetMemberships({
+            success: true,
+            result: [
+              {
+                id: "1",
+                account: { id: "account-id-1", name: "account-name-1" },
+              },
+              {
+                id: "2",
+                account: { id: "account-id-2", name: "account-name-2" },
+              },
+              {
+                id: "3",
+                account: { id: "account-id-3", name: "account-name-3" },
+              },
+            ],
           });
+
           await expect(runWrangler("secret put the-key --name script-name"))
             .rejects.toThrowErrorMatchingInlineSnapshot(`
                   "More than one account available but unable to select one in non-interactive mode.