Skip to content

Commit

Permalink
non-TTY check for required variables:
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JacobMGEvans committed May 4, 2022
1 parent 370200f commit ce9db80
Show file tree
Hide file tree
Showing 4 changed files with 48 additions and 16 deletions.
31 changes: 28 additions & 3 deletions packages/wrangler/src/__tests__/publish.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ describe("publish", () => {
expect(std.err).toMatchInlineSnapshot(`""`);
});

it("should throw an error in non-TTY if 'account_id' & 'CLOUDFLARE_ACCOUNT_ID' is missing", async () => {
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",
Expand Down Expand Up @@ -233,10 +233,35 @@ describe("publish", () => {
await expect(runWrangler("publish index.js")).rejects.toThrowError();

expect(std.err).toMatchInlineSnapshot(`
"[31mX [41;31m[[41;97mERROR[41;31m][0m [1mMissing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info: TBD[0m
"[31mX [41;31m[[41;97mERROR[41;31m][0m [1mMissing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info[0m
"
`);
});
it("should", 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();

X [ERROR] Did not login, quitting...
expect(std.err).toMatchInlineSnapshot(`
"X [ERROR] Failed to automatically retrieve account IDs the logged in user,
Please set the appropriate \`account_id\` in your \`wrangler.toml\` file.

"
`);
Expand Down
16 changes: 7 additions & 9 deletions packages/wrangler/src/__tests__/secret.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ 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 { useMockIsTTY } from "./helpers/mock-istty";
import { mockOAuthFlow } from "./helpers/mock-oauth-flow";
import { useMockStdin } from "./helpers/mock-stdin";
import { runInTempDir } from "./helpers/run-in-tmp";
Expand All @@ -13,14 +12,13 @@ import { runWrangler } from "./helpers/run-wrangler";
describe("wrangler secret", () => {
const std = mockConsoleMethods();
const { mockGetMemberships } = mockOAuthFlow();
const { setIsTTY } = useMockIsTTY();

runInTempDir();
mockAccountId();
mockApiToken();

afterEach(() => {
unsetAllMocks();
setIsTTY(true);
});

describe("put", () => {
Expand Down Expand Up @@ -177,11 +175,12 @@ describe("wrangler secret", () => {
success: false,
result: [],
});
await expect(
runWrangler("secret put the-key --name script-name")
).rejects.toThrowErrorMatchingInlineSnapshot(
`"No account id found, quitting..."`
);
await expect(runWrangler("secret put the-key --name script-name"))
.rejects.toThrowErrorMatchingInlineSnapshot(`
"Failed to automatically retrieve account IDs the logged in user,
Please set the appropriate \`account_id\` in your \`wrangler.toml\` file.
"
`);
});

it("should use the account from wrangler.toml", async () => {
Expand All @@ -204,7 +203,6 @@ describe("wrangler secret", () => {
});

it("should error if a user has multiple accounts, and has not specified an account in wrangler.toml", async () => {
setIsTTY(false);
mockGetMemberships({
success: true,
result: [
Expand Down
6 changes: 6 additions & 0 deletions packages/wrangler/src/__tests__/whoami.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,18 @@ import { writeAuthConfigFile } from "../user";
import { getUserInfo, WhoAmI } from "../whoami";
import { setMockResponse } from "./helpers/mock-cfetch";
import { mockConsoleMethods } from "./helpers/mock-console";
import { useMockIsTTY } from "./helpers/mock-istty";
import { runInTempDir } from "./helpers/run-in-tmp";
import type { UserInfo } from "../whoami";

describe("getUserInfo()", () => {
runInTempDir({ homedir: "./home" });
const std = mockConsoleMethods();
const { setIsTTY } = useMockIsTTY();

beforeEach(() => {
setIsTTY(true);
});

it("should return undefined if there is no config file", async () => {
const userInfo = await getUserInfo();
Expand Down
11 changes: 7 additions & 4 deletions packages/wrangler/src/user.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -418,11 +418,9 @@ export function getAPIToken(): string | undefined {
!localAPIToken &&
!LocalState.accessToken?.value
) {
logger.error(
"Missing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info: TBD"
throw new Error(
"Missing 'CLOUDFLARE_API_TOKEN' from non-TTY environment, please see docs for more info"
);

return;
}

return localAPIToken ?? LocalState.accessToken?.value;
Expand Down Expand Up @@ -1140,6 +1138,11 @@ export async function getAccountId(
.join("\n")
);
}
} else {
if (!isInteractive)
throw new Error(
`Failed to automatically retrieve account IDs the logged in user,\nPlease set the appropriate \`account_id\` in your \`wrangler.toml\` file.\n`
);
}
return accountId;
}
Expand Down

0 comments on commit ce9db80

Please sign in to comment.