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 28, 2022
1 parent 277b254 commit 78d5d30
Show file tree
Hide file tree
Showing 5 changed files with 128 additions and 14 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
96 changes: 96 additions & 0 deletions packages/wrangler/src/__tests__/ci.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
import { mockConsoleMethods } from "./helpers/mock-console";
import { runInTempDir } from "./helpers/run-in-tmp";
import { runWrangler } from "./helpers/run-wrangler";
import writeWranglerToml from "./helpers/write-wrangler-toml";

describe("CI", () => {
const ENV_COPY = process.env;
const std = mockConsoleMethods();
runInTempDir();

beforeEach(() => {
process.env = {
...ENV_COPY,
CI: "true",
};
});

afterEach(() => {
process.env = ENV_COPY;
});

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]`
);
});
});

it("should not throw missing Error in TTY environment", async () => {
process.env = {
...process.env,
CI: "false",
};
await runWrangler();
expect(std.err).toMatchInlineSnapshot(`""`);
});
});
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;
});
});
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
15 changes: 14 additions & 1 deletion packages/wrangler/src/user.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -409,7 +409,16 @@ export function getAPIToken(): string | undefined {
"If you wish to authenticate via an API token then please set the `CLOUDFLARE_API_TOKEN` environment variable."
);
}
return LocalState.accessToken?.value;

const localAPIToken = getCloudflareAPITokenFromEnv();

if (process.env.CI && !localAPIToken) {
throw new Error(
`Missing "CLOUDFLARE_API_TOKEN" from CI environment, please see docs for more info: TBD`
);
}

return localAPIToken ?? LocalState.accessToken?.value;
}

interface AccessContext {
Expand Down Expand Up @@ -1077,6 +1086,10 @@ export async function getAccountId(
const accountIdFromEnv = getCloudflareAccountIdFromEnv();
if (accountIdFromEnv) {
return accountIdFromEnv;
} else if (process.env.CI) {
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`
);
}

let response: Response;
Expand Down

0 comments on commit 78d5d30

Please sign in to comment.