Skip to content

Commit

Permalink
CI/CD check for required variables:
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JacobMGEvans committed Apr 27, 2022
1 parent 277b254 commit 87cb0d2
Show file tree
Hide file tree
Showing 8 changed files with 148 additions and 16 deletions.
8 changes: 8 additions & 0 deletions .changeset/eighty-yaks-jump.md
Original file line number Diff line number Diff line change
@@ -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
7 changes: 5 additions & 2 deletions package-lock.json

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

84 changes: 84 additions & 0 deletions packages/wrangler/src/__tests__/ci.test.ts
Original file line number Diff line number Diff line change
@@ -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]`
);
});
});
});
3 changes: 3 additions & 0 deletions packages/wrangler/src/__tests__/jest.setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
20 changes: 8 additions & 12 deletions packages/wrangler/src/__tests__/sentry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down Expand Up @@ -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;
});
});
34 changes: 34 additions & 0 deletions packages/wrangler/src/ci-check.ts
Original file line number Diff line number Diff line change
@@ -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`
);
}
}
}
3 changes: 2 additions & 1 deletion packages/wrangler/src/reporting.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down
5 changes: 4 additions & 1 deletion packages/wrangler/src/user.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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" +
Expand Down Expand Up @@ -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;

Expand Down

0 comments on commit 87cb0d2

Please sign in to comment.