CI/CD check for required variables:

Added a check in CI/CD 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 CI/CD scope. The `CLOUDFLARE_API_TOKEN` is necessary in CI/CD scope and will always error if missing.

resolves #827

Author: JacobMGEvans

.changeset/eighty-yaks-jump.md

@@ -0,0 +1,8 @@
+---
+"wrangler": patch
+---
+
+feat: Added a check in CI/CD 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 CI/CD scope. The `CLOUDFLARE_API_TOKEN` is necessary in CI/CD scope and will always error if missing.
+
+resolves #827

package-lock.json

@@ -0,0 +1,84 @@
+import { mockConsoleMethods } from "./helpers/mock-console";
+import { useMockIsTTY } from "./helpers/mock-istty";
+import { runInTempDir } from "./helpers/run-in-tmp";
+import { runWrangler } from "./helpers/run-wrangler";
+import writeWranglerToml from "./helpers/write-wrangler-toml";
+
+const ENV_COPY = process.env;
+mockConsoleMethods();
+runInTempDir();
+
+afterEach(() => {
+  process.env = ENV_COPY;
+});
+
+describe("CI", () => {
+  const { setIsTTY } = useMockIsTTY();
+  setIsTTY(false);
+
+  it("should not throw an error in CI if 'CLOUDFLARE_API_TOKEN' & 'account_id' are in scope", async () => {
+    writeWranglerToml({
+      account_id: "IG-88",
+    });
+
+    process.env = {
+      CLOUDFLARE_API_TOKEN: "123456789",
+    };
+
+    await runWrangler().catch((err) => {
+      expect(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: "IG-88",
+    };
+
+    await runWrangler().catch((err) => {
+      expect(err).toMatchInlineSnapshot(`""`);
+    });
+  });
+
+  it("should throw an error in CI if 'account_id' & 'CLOUDFLARE_ACCOUNT_ID' is missing", async () => {
+    writeWranglerToml({
+      account_id: undefined,
+    });
+
+    process.env = {
+      CLOUDFLARE_API_TOKEN: "hunter2",
+      CLOUDFLARE_ACCOUNT_ID: undefined,
+    };
+
+    await runWrangler().catch((err) => {
+      expect(err).toMatchInlineSnapshot(
+        `[Error: Missing "account_id" from "wrangler.toml" and "CLOUDFLARE_ACCOUNT_ID" from CI environment, one is required, please see docs for more info: TBD]`
+      );
+    });
+  });
+
+  it("should throw error in CI if 'CLOUDFLARE_API_TOKEN' is missing", async () => {
+    writeWranglerToml({
+      account_id: undefined,
+    });
+
+    process.env = {
+      CLOUDFLARE_API_TOKEN: undefined,
+      CLOUDFLARE_ACCOUNT_ID: "badwolf",
+    };
+    await runWrangler().catch((err) => {
+      expect(err).toMatchInlineSnapshot(
+        `[Error: Missing "CLOUDFLARE_API_TOKEN" from CI environment, please see docs for more info: TBD]`
+      );
+    });
+  });
+
+  it("should throw errors in CI if 'CLOUDFLARE_API_TOKEN', 'account_id' & 'CLOUDFLARE_ACCOUNT_ID is missing", async () => {
+    await runWrangler().catch((err) => {
+      expect(err).toMatchInlineSnapshot(
+        `[Error: Missing "account_id" from "wrangler.toml" and "CLOUDFLARE_ACCOUNT_ID" "CLOUDFLARE_API_TOKEN" from CI environment, please see docs for more info: TBD]`
+      );
+    });
+  });
+});

packages/wrangler/src/__tests__/ci.test.ts

@@ -71,3 +71,6 @@ jest.mock("../dev/dev", () => {
 // Make sure that we don't accidentally try to open a browser window when running tests.
 // We will actually provide a mock implementation for `openInBrowser()` within relevant tests.
 jest.mock("../open-in-browser");
+
+// Must mock `ciCheck()` implementation to prevent tests in actual CI throwing unnecessary errors.
+jest.mock("../ci-check.ts");

packages/wrangler/src/__tests__/jest.setup.ts

@@ -7,26 +7,27 @@ import * as Sentry from "@sentry/node";
 import prompts from "prompts";
 
 import { mockConsoleMethods } from "./helpers/mock-console";
+import { useMockIsTTY } from "./helpers/mock-istty";
 import { runInTempDir } from "./helpers/run-in-tmp";
 import { runWrangler } from "./helpers/run-wrangler";
 const { reportError } = jest.requireActual("../reporting");
 
 describe("Error Reporting", () => {
+  const { setIsTTY } = useMockIsTTY();
+
   runInTempDir({ homedir: "./home" });
   mockConsoleMethods();
   const reportingTOMLPath = ".wrangler/config/reporting.toml";
 
-  const originalTTY = process.stdout.isTTY;
   beforeEach(() => {
     jest.mock("@sentry/node");
     jest.spyOn(Sentry, "captureException");
-    process.stdout.isTTY = true;
+    setIsTTY(true);
   });
 
   afterEach(() => {
     jest.unmock("@sentry/node");
     jest.clearAllMocks();
-    process.stdout.isTTY = originalTTY;
   });
 
   it("should confirm user will allow error reporting usage", async () => {
@@ -127,20 +128,15 @@ describe("Error Reporting", () => {
   });
 
   it("should not prompt in non-TTY environment", async () => {
-    process.stdout.isTTY = false;
-
+    setIsTTY(false);
     await reportError(new Error("test error"), "testFalse");
 
-    const { error_tracking_opt, error_tracking_opt_date } = TOML.parse(
-      await fsp.readFile(path.join(os.homedir(), reportingTOMLPath), "utf-8")
-    );
-
-    expect(error_tracking_opt).toBe(false);
-    expect(error_tracking_opt_date).toBeTruthy();
+    expect(
+      fs.existsSync(path.join(os.homedir(), reportingTOMLPath, "utf-8"))
+    ).toBe(false);
 
     expect(Sentry.captureException).not.toHaveBeenCalledWith(
       new Error("test error")
     );
-    process.stdout.isTTY = originalTTY;
   });
 });

packages/wrangler/src/__tests__/sentry.test.ts

@@ -0,0 +1,34 @@
+import { readConfig } from "./config";
+import type { State } from "./user";
+
+/**
+ * Inside a CI/CD environment, you want to check if all the required variables are in scope
+ * and if not, throw a helpful error message with missing variables and documentation link.
+ */
+export function ciCheck(localState: State) {
+  if (process.stdout.isTTY) {
+    const config = readConfig(undefined, {});
+
+    if (
+      !localState.accessToken &&
+      !config.account_id &&
+      !process.env.CLOUDFLARE_ACCOUNT_ID
+    ) {
+      throw new Error(
+        `Missing "account_id" from "wrangler.toml" and "CLOUDFLARE_ACCOUNT_ID" "CLOUDFLARE_API_TOKEN" from CI environment, please see docs for more info: TBD`
+      );
+    }
+
+    if (!process.env.CLOUDFLARE_API_TOKEN) {
+      throw new Error(
+        `Missing "CLOUDFLARE_API_TOKEN" from CI environment, please see docs for more info: TBD`
+      );
+    }
+
+    if (!config.account_id && !process.env.CLOUDFLARE_ACCOUNT_ID) {
+      throw new Error(
+        `Missing "account_id" from "wrangler.toml" and "CLOUDFLARE_ACCOUNT_ID" from CI environment, one is required, please see docs for more info: TBD`
+      );
+    }
+  }
+}

packages/wrangler/src/ci-check.ts

@@ -92,7 +92,8 @@ function exceptionTransaction(error: Error, origin = "") {
 }
 
 export async function reportError(err: Error, origin = "") {
-  if (!process.stdout.isTTY) return await appendReportingDecision("false");
+  // If the user has not opted in to error reporting, we don't want to do anything in CI or non-interactive environments
+  if (!process.stdout.isTTY) return;
 
   const errorTrackingOpt = await reportingPermission();
 

packages/wrangler/src/reporting.ts

@@ -220,6 +220,7 @@ import Table from "ink-table";
 import React from "react";
 import { fetch } from "undici";
 import { getCloudflareApiBaseUrl } from "./cfetch";
+import { ciCheck } from "./ci-check";
 import { getEnvironmentVariableFactory } from "./environment-variables";
 import openInBrowser from "./open-in-browser";
 import { parseTOML, readFileSync } from "./parse";
@@ -256,7 +257,7 @@ interface PKCECodes {
 /**
  * The module level state of the authentication flow.
  */
-interface State extends AuthTokens {
+export interface State extends AuthTokens {
   authorizationCode?: string;
   codeChallenge?: string;
   codeVerifier?: string;
@@ -402,6 +403,7 @@ export function reinitialiseAuthTokens(config?: UserAuthConfig): void {
 }
 
 export function getAPIToken(): string | undefined {
+  ciCheck(LocalState);
   if (LocalState.apiToken) {
     console.warn(
       "It looks like you have used Wrangler 1's `config` command to login with an API token.\n" +
@@ -1071,6 +1073,7 @@ export function listScopes(message = "💁 Available scopes:"): void {
 export async function getAccountId(
   isInteractive = true
 ): Promise<string | undefined> {
+  ciCheck(LocalState);
   const apiToken = getAPIToken();
   if (!apiToken) return;