Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: refactor wrangler login/logout/whoami to use defineCommand #7150

Merged
merged 7 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/good-eggs-lay.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"wrangler": patch
---

refactor: improve login/logout/whoami setup with the new internal registration utils
21 changes: 16 additions & 5 deletions packages/wrangler/src/__tests__/sentry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,18 +259,29 @@ describe("sentry", () => {
Object {
"colno": 0,
"context_line": "",
"filename": "/wrangler/packages/wrangler/src/index.ts",
"filename": "/wrangler/packages/wrangler/src/core/register-commands.ts",
"function": "",
"in_app": false,
"lineno": 0,
"module": "index.ts",
"module": "register-commands.ts",
"post_context": Array [],
"pre_context": Array [],
},
Object {
"colno": 0,
"context_line": "",
"filename": "/wrangler/packages/wrangler/src/user/commands.ts",
"function": "",
"in_app": false,
"lineno": 0,
"module": "commands.ts",
"post_context": Array [],
"pre_context": Array [],
},
Object {
"colno": 0,
"context_line": "",
"filename": "/wrangler/packages/wrangler/src/whoami.ts",
"filename": "/wrangler/packages/wrangler/src/user/whoami.ts",
"function": "",
"in_app": false,
"lineno": 0,
Expand All @@ -281,7 +292,7 @@ describe("sentry", () => {
Object {
"colno": 0,
"context_line": "",
"filename": "/wrangler/packages/wrangler/src/whoami.ts",
"filename": "/wrangler/packages/wrangler/src/user/whoami.ts",
"function": "",
"in_app": false,
"lineno": 0,
Expand All @@ -292,7 +303,7 @@ describe("sentry", () => {
Object {
"colno": 0,
"context_line": "",
"filename": "/wrangler/packages/wrangler/src/whoami.ts",
"filename": "/wrangler/packages/wrangler/src/user/whoami.ts",
"function": "",
"in_app": false,
"lineno": 0,
Expand Down
2 changes: 1 addition & 1 deletion packages/wrangler/src/__tests__/whoami.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { http, HttpResponse } from "msw";
import { writeAuthConfigFile } from "../user";
import { getUserInfo } from "../whoami";
import { getUserInfo } from "../user/whoami";
import { mockConsoleMethods } from "./helpers/mock-console";
import { useMockIsTTY } from "./helpers/mock-istty";
import {
Expand Down
9 changes: 8 additions & 1 deletion packages/wrangler/src/core/define-command.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,13 +101,20 @@ export type CommandDefinition<
* @default true
*/
printBanner?: boolean;

/**
* By default, wrangler will print warnings about wrangler.toml configuration.
* Set this value to `false` to skip printing these warnings.
*/
printConfigWarnings?: boolean;
Comment on lines +105 to +109
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was rebasing the changes on #6756 over to this PR and notice the same changes doesn't work here. The reason was that defineCommand will read the config for you and there was no way to disable the warning. You can find the full changes that enable this option in ec6d4cd

cc: @penalosa @emily-shen

};

/**
* A plain key-value object describing the CLI args for this command.
* Shared args can be defined as another plain object and spread into this.
*/
args: NamedArgDefs;
args?: NamedArgDefs;

/**
* Optionally declare some of the named args as positional args.
* The order of this array is the order they are expected in the command.
Expand Down
17 changes: 12 additions & 5 deletions packages/wrangler/src/core/register-commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ function walkTreeAndRegister(
// inference from positionalArgs
const commandPositionalArgsSuffix = def.positionalArgs
?.map((key) => {
const { demandOption, array } = def.args[key];
const { demandOption, array } = def.args?.[key] ?? {};
return demandOption
? `<${key}${array ? ".." : ""}>` // <key> or <key..>
: `[${key}${array ? ".." : ""}]`; // [key] or [key..]
Expand All @@ -124,21 +124,23 @@ function walkTreeAndRegister(
(def.metadata.hidden ? false : def.metadata.description) as string, // cast to satisfy typescript overload selection
function builder(subYargs) {
if (def.type === "command") {
yargs.options(def.args);
const args = def.args ?? {};

yargs.options(args);

// Yargs offers an `array: true` option that will always coerces the value to an array
// e.g. `--name foo` becomes `{ name: ["foo"] }` instead of `{ name: "foo" }`
// However, non-array arguments can still receives multiple values
// e.g. `--name foo --name bar` becomes `{ name: ["foo", "bar"] }` regardless of the `array` setting
// @see https://github.com/yargs/yargs/issues/1318
for (const [key, opt] of Object.entries(def.args)) {
for (const [key, opt] of Object.entries(args)) {
if (!opt.array) {
yargs.check(demandSingleValue(key));
}
}

for (const key of def.positionalArgs ?? []) {
yargs.positional(key, def.args[key]);
yargs.positional(key, args[key]);
}
} else if (def.type === "namespace") {
// this is our hacky way of printing --help text for incomplete commands
Expand Down Expand Up @@ -180,7 +182,12 @@ function createHandler(def: CommandDefinition) {
await def.validateArgs?.(args);

await def.handler(args, {
config: readConfig(args.config, args),
config: readConfig(
args.config,
args,
undefined,
!(def.behaviour?.printConfigWarnings ?? true)
),
errors: { UserError, FatalError },
logger,
fetchResult,
Expand Down
109 changes: 7 additions & 102 deletions packages/wrangler/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import makeCLI from "yargs";
import { version as wranglerVersion } from "../package.json";
import { ai } from "./ai";
import { cloudchamber } from "./cloudchamber";
import { loadDotEnv, readConfig } from "./config";
import { loadDotEnv } from "./config";
import { createCommandRegister } from "./core/register-commands";
import { d1 } from "./d1";
import { deleteHandler, deleteOptions } from "./delete";
Expand Down Expand Up @@ -48,9 +48,9 @@ import { hyperdrive } from "./hyperdrive/index";
import { initHandler, initOptions } from "./init";
import "./kv";
import "./workflows";
import "./user/commands";
import { demandSingleValue } from "./core";
import { logBuildFailure, logger, LOGGER_LEVELS } from "./logger";
import * as metrics from "./metrics";
import { mTlsCertificateCommands } from "./mtls-certificate/cli";
import { writeOutput } from "./output";
import { pages } from "./pages";
Expand All @@ -70,19 +70,13 @@ import { tailHandler, tailOptions } from "./tail";
import registerTriggersSubcommands from "./triggers";
import { typesHandler, typesOptions } from "./type-generation";
import { printWranglerBanner, updateCheck } from "./update-check";
import {
getAuthFromEnv,
listScopes,
login,
logout,
validateScopeKeys,
} from "./user";
import { getAuthFromEnv } from "./user";
import { whoami } from "./user/whoami";
import { debugLogFilepath } from "./utils/log-file";
import { vectorize } from "./vectorize/index";
import registerVersionsSubcommands from "./versions";
import registerVersionsDeploymentsSubcommands from "./versions/deployments";
import registerVersionsRollbackCommand from "./versions/rollback";
import { whoami } from "./whoami";
import { asJson } from "./yargs-types";
import type { Config } from "./config";
import type { LoggerLevel } from "./logger";
Expand Down Expand Up @@ -596,99 +590,10 @@ export function createCLIParser(argv: string[]) {
});

/******************** CMD GROUP ***********************/
// login
wrangler.command(
// this needs scopes as an option?
"login",
"🔓 Login to Cloudflare",
(yargs) => {
// TODO: This needs some copy editing
// I mean, this entire app does, but this too.
return yargs
.option("scopes-list", {
describe: "List all the available OAuth scopes with descriptions",
})
.option("browser", {
default: true,
type: "boolean",
describe: "Automatically open the OAuth link in a browser",
})
.option("scopes", {
describe: "Pick the set of applicable OAuth scopes when logging in",
array: true,
type: "string",
requiresArg: true,
});
// TODO: scopes
},
async (args) => {
await printWranglerBanner();
if (args["scopes-list"]) {
listScopes();
return;
}
if (args.scopes) {
if (args.scopes.length === 0) {
// don't allow no scopes to be passed, that would be weird
listScopes();
return;
}
if (!validateScopeKeys(args.scopes)) {
throw new CommandLineArgsError(
`One of ${args.scopes} is not a valid authentication scope. Run "wrangler login --scopes-list" to see the valid scopes.`
);
}
await login({ scopes: args.scopes, browser: args.browser });
return;
}
await login({ browser: args.browser });
const config = readConfig(args.config, args, undefined, true);
await metrics.sendMetricsEvent("login user", {
sendMetrics: config.send_metrics,
});

// TODO: would be nice if it optionally saved login
// credentials inside node_modules/.cache or something
// this way you could have multiple users on a single machine
}
);

// logout
wrangler.command(
// this needs scopes as an option?
"logout",
"🚪 Logout from Cloudflare",
() => {},
async (args) => {
await printWranglerBanner();
await logout();
const config = readConfig(undefined, args, undefined, true);
await metrics.sendMetricsEvent("logout user", {
sendMetrics: config.send_metrics,
});
}
);

// whoami
wrangler.command(
"whoami",
"🕵️ Retrieve your user information",
(yargs) => {
return yargs.option("account", {
type: "string",
describe:
"Show membership information for the given account (id or name).",
});
},
async (args) => {
await printWranglerBanner();
await whoami(args.account);
const config = readConfig(undefined, args);
await metrics.sendMetricsEvent("view accounts", {
sendMetrics: config.send_metrics,
});
}
);
register.registerNamespace("login");
register.registerNamespace("logout");
register.registerNamespace("whoami");

/******************************************************/
/* DEPRECATED COMMANDS */
Expand Down
104 changes: 104 additions & 0 deletions packages/wrangler/src/user/commands.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import { defineCommand } from "../core/define-command";
import { CommandLineArgsError } from "../errors";
import * as metrics from "../metrics";
import { listScopes, login, logout, validateScopeKeys } from "./user";
import { whoami } from "./whoami";

defineCommand({
command: "wrangler login",
metadata: {
description: "🔓 Login to Cloudflare",
owner: "Workers: Authoring and Testing",
status: "stable",
},
behaviour: {
printConfigWarnings: false,
},
args: {
"scopes-list": {
describe: "List all the available OAuth scopes with descriptions",
},
browser: {
default: true,
type: "boolean",
describe: "Automatically open the OAuth link in a browser",
},
scopes: {
describe: "Pick the set of applicable OAuth scopes when logging in",
array: true,
type: "string",
requiresArg: true,
},
},
async handler(args, { config }) {
if (args.scopesList) {
listScopes();
return;
}
if (args.scopes) {
if (args.scopes.length === 0) {
// don't allow no scopes to be passed, that would be weird
listScopes();
return;
}
if (!validateScopeKeys(args.scopes)) {
throw new CommandLineArgsError(
`One of ${args.scopes} is not a valid authentication scope. Run "wrangler login --scopes-list" to see the valid scopes.`
);
}
await login({ scopes: args.scopes, browser: args.browser });
return;
}
await login({ browser: args.browser });
await metrics.sendMetricsEvent("login user", {
sendMetrics: config.send_metrics,
});

// TODO: would be nice if it optionally saved login
// credentials inside node_modules/.cache or something
// this way you could have multiple users on a single machine
},
});

defineCommand({
command: "wrangler logout",
metadata: {
description: "🚪 Logout from Cloudflare",
owner: "Workers: Authoring and Testing",
status: "stable",
},
behaviour: {
printConfigWarnings: false,
},
async handler(_, { config }) {
await logout();
await metrics.sendMetricsEvent("logout user", {
sendMetrics: config.send_metrics,
});
},
});

defineCommand({
command: "wrangler whoami",
metadata: {
description: "🕵️ Retrieve your user information",
owner: "Workers: Authoring and Testing",
status: "stable",
},
behaviour: {
printConfigWarnings: false,
},
args: {
account: {
type: "string",
describe:
"Show membership information for the given account (id or name).",
},
},
async handler(args, { config }) {
await whoami(args.account);
await metrics.sendMetricsEvent("view accounts", {
sendMetrics: config.send_metrics,
});
},
});
Loading
Loading