From 65303d41b0f132393e344d080f0fa2b8dfb088b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ey=C3=BE=C3=B3r=20Magn=C3=BAsson?= Date: Tue, 20 Apr 2021 22:35:21 +0000 Subject: [PATCH] fix(enterprise): ensure exit code and --yes flag work --- core/src/commands/enterprise/helpers.ts | 97 +++++++++++++++++-- .../enterprise/secrets/secrets-create.ts | 31 +++--- .../enterprise/secrets/secrets-delete.ts | 21 ++-- .../enterprise/secrets/secrets-list.ts | 43 +------- .../commands/enterprise/users/users-create.ts | 32 ++---- .../commands/enterprise/users/users-delete.ts | 21 ++-- .../commands/enterprise/users/users-list.ts | 35 +++---- core/src/logger/util.ts | 9 +- 8 files changed, 144 insertions(+), 145 deletions(-) diff --git a/core/src/commands/enterprise/helpers.ts b/core/src/commands/enterprise/helpers.ts index bd100e8e7f..ca98fc9177 100644 --- a/core/src/commands/enterprise/helpers.ts +++ b/core/src/commands/enterprise/helpers.ts @@ -6,7 +6,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { GetProjectResponse } from "@garden-io/platform-api-types" +import { GetProjectResponse, SecretResponse, UserResponse } from "@garden-io/platform-api-types" import { EnterpriseApi } from "../../enterprise/api" import { dedent } from "../../util/string" @@ -16,12 +16,47 @@ import minimatch from "minimatch" import pluralize from "pluralize" import chalk from "chalk" import inquirer from "inquirer" +import { CommandError } from "../../exceptions" +import { CommandResult } from "../base" + +export interface DeleteResult { + id: number + status: string +} export interface ApiCommandError { identifier: string | number message?: string } +export interface SecretResult { + id: number + createdAt: string + updatedAt: string + name: string + environment?: { + name: string + id: number + } + user?: { + name: string + id: number + vcsUsername: string + } +} + +export interface UserResult { + id: number + createdAt: string + updatedAt: string + name: string + vcsUsername: string + groups: { + id: number + name: string + }[] +} + export const noApiMsg = (action: string, resource: string) => dedent` Unable to ${action} ${resource}. Make sure the project is configured for Garden Enterprise and that you're logged in. ` @@ -31,21 +66,61 @@ export async function getProject(api: EnterpriseApi, projectUid: string) { return res.data } -export function handleBulkOperationResult({ +export function makeUserFromResponse(user: UserResponse): UserResult { + return { + id: user.id, + name: user.name, + vcsUsername: user.vcsUsername, + createdAt: user.createdAt, + updatedAt: user.updatedAt, + groups: user.groups.map((g) => ({ id: g.id, name: g.name })), + } +} + +export function makeSecretFromResponse(res: SecretResponse): SecretResult { + const secret = { + name: res.name, + id: res.id, + updatedAt: res.updatedAt, + createdAt: res.createdAt, + } + if (res.environment) { + secret["environment"] = { + name: res.environment.name, + id: res.environment.id, + } + } + if (res.user) { + secret["user"] = { + name: res.user.name, + id: res.user.id, + vcsUsername: res.user.vcsUsername, + } + } + return secret +} + +/** + * Helper function for consistenly logging outputs for enterprise bulk operation commands. + * + * Throws if any errors exist after logging the relavant output. + */ +export function handleBulkOperationResult({ log, + results, errors, action, cmdLog, resource, - successCount, }: { log: LogEntry cmdLog: LogEntry + results: T[] errors: ApiCommandError[] action: "create" | "delete" - successCount: number resource: "secret" | "user" -}) { +}): CommandResult { + const successCount = results.length const totalCount = errors.length + successCount log.info("") @@ -66,9 +141,7 @@ export function handleBulkOperationResult({ }) .join("\n") log.error(dedent` - Failed ${actionVerb} ${errors.length}/${totalCount} ${pluralize(resource)}. - - See errors below: + Failed ${actionVerb} ${errors.length}/${totalCount} ${pluralize(resource)}. See errors below: ${errorMsgs}\n `) @@ -81,7 +154,15 @@ export function handleBulkOperationResult({ log.info({ msg: `Successfully ${action === "create" ? "created" : "deleted"} ${successCount} ${resourceStr}!`, }) + log.info("") } + + // Ensure command exits with code 1. + if (errors.length > 0) { + throw new CommandError("Command failed.", { errors }) + } + + return { result: results } } export function applyFilter(filter: string[], val?: string | string[]) { diff --git a/core/src/commands/enterprise/secrets/secrets-create.ts b/core/src/commands/enterprise/secrets/secrets-create.ts index d25aef4f41..471d851cc8 100644 --- a/core/src/commands/enterprise/secrets/secrets-create.ts +++ b/core/src/commands/enterprise/secrets/secrets-create.ts @@ -13,16 +13,18 @@ import { readFile } from "fs-extra" import { printHeader } from "../../../logger/util" import { Command, CommandParams, CommandResult } from "../../base" -import { ApiCommandError, getProject, handleBulkOperationResult, noApiMsg } from "../helpers" +import { + ApiCommandError, + getProject, + handleBulkOperationResult, + makeSecretFromResponse, + noApiMsg, + SecretResult, +} from "../helpers" import { dedent, deline } from "../../../util/string" import { StringsParameter, PathParameter, IntegerParameter, StringParameter } from "../../../cli/params" import { StringMap } from "../../../config/common" -export interface SecretsCreateCommandResult { - errors: ApiCommandError[] - results: CreateSecretResponse[] -} - export const secretsCreateArgs = { secrets: new StringsParameter({ help: deline`The names and values of the secrets to create, separated by '='. @@ -76,12 +78,7 @@ export class SecretsCreateCommand extends Command { printHeader(headerLog, "Create secrets", "lock") } - async action({ - garden, - log, - opts, - args, - }: CommandParams): Promise> { + async action({ garden, log, opts, args }: CommandParams): Promise> { // Apparently TS thinks that optional params are always defined so we need to cast them to their // true type here. const envName = opts["scope-to-env"] as string | undefined @@ -157,14 +154,14 @@ export class SecretsCreateCommand extends Command { let count = 1 const errors: ApiCommandError[] = [] - const results: CreateSecretResponse[] = [] + const results: SecretResult[] = [] for (const [name, value] of secretsToCreate) { cmdLog.setState({ msg: `Creating secrets... → ${count}/${secretsToCreate.length}` }) count++ try { const body = { environmentId, userId, projectId: project.id, name, value } const res = await api.post(`/secrets`, { body }) - results.push(res) + results.push(makeSecretFromResponse(res.data)) } catch (err) { errors.push({ identifier: name, @@ -173,15 +170,13 @@ export class SecretsCreateCommand extends Command { } } - handleBulkOperationResult({ + return handleBulkOperationResult({ log, cmdLog, action: "create", resource: "secret", errors, - successCount: results.length, + results, }) - - return { result: { errors, results } } } } diff --git a/core/src/commands/enterprise/secrets/secrets-delete.ts b/core/src/commands/enterprise/secrets/secrets-delete.ts index 1b9f09d8c2..76da3d9d19 100644 --- a/core/src/commands/enterprise/secrets/secrets-delete.ts +++ b/core/src/commands/enterprise/secrets/secrets-delete.ts @@ -12,12 +12,7 @@ import { CommandError, ConfigurationError } from "../../../exceptions" import { printHeader } from "../../../logger/util" import { dedent, deline } from "../../../util/string" import { Command, CommandParams, CommandResult } from "../../base" -import { ApiCommandError, confirmDelete, handleBulkOperationResult, noApiMsg } from "../helpers" - -export interface SecretsDeleteCommandResult { - errors: ApiCommandError[] - results: BaseResponse[] -} +import { ApiCommandError, confirmDelete, DeleteResult, handleBulkOperationResult, noApiMsg } from "../helpers" export const secretsDeleteArgs = { ids: new StringsParameter({ @@ -44,7 +39,7 @@ export class SecretsDeleteCommand extends Command { printHeader(headerLog, "Delete secrets", "lock") } - async action({ garden, args, log }: CommandParams): Promise> { + async action({ garden, args, log, opts }: CommandParams): Promise> { const secretsToDelete = (args.ids || []).map((id) => parseInt(id, 10)) if (secretsToDelete.length === 0) { throw new CommandError(`No secret IDs provided.`, { @@ -52,7 +47,7 @@ export class SecretsDeleteCommand extends Command { }) } - if (!(await confirmDelete("secret", secretsToDelete.length))) { + if (!opts.yes && !(await confirmDelete("secret", secretsToDelete.length))) { return {} } @@ -65,13 +60,13 @@ export class SecretsDeleteCommand extends Command { let count = 1 const errors: ApiCommandError[] = [] - const results: BaseResponse[] = [] + const results: DeleteResult[] = [] for (const id of secretsToDelete) { cmdLog.setState({ msg: `Deleting secrets... → ${count}/${secretsToDelete.length}` }) count++ try { const res = await api.delete(`/secrets/${id}`) - results.push(res) + results.push({ id, status: res.status }) } catch (err) { errors.push({ identifier: id, @@ -80,15 +75,13 @@ export class SecretsDeleteCommand extends Command { } } - handleBulkOperationResult({ + return handleBulkOperationResult({ log, cmdLog, errors, action: "delete", resource: "secret", - successCount: results.length, + results, }) - - return { result: { errors, results } } } } diff --git a/core/src/commands/enterprise/secrets/secrets-list.ts b/core/src/commands/enterprise/secrets/secrets-list.ts index e79aecae1e..b62940c2e7 100644 --- a/core/src/commands/enterprise/secrets/secrets-list.ts +++ b/core/src/commands/enterprise/secrets/secrets-list.ts @@ -13,27 +13,11 @@ import { ListSecretsResponse } from "@garden-io/platform-api-types" import { printHeader } from "../../../logger/util" import { dedent, deline, renderTable } from "../../../util/string" import { Command, CommandParams, CommandResult } from "../../base" -import { applyFilter, getProject, noApiMsg } from "../helpers" +import { applyFilter, getProject, makeSecretFromResponse, noApiMsg, SecretResult } from "../helpers" import chalk from "chalk" import { sortBy } from "lodash" import { StringsParameter } from "../../../cli/params" -interface Secret { - id: number - createdAt: string - updatedAt: string - name: string - environment?: { - name: string - id: number - } - user?: { - name: string - id: number - vcsUsername: string - } -} - export const secretsListOpts = { "filter-envs": new StringsParameter({ help: deline`Filter on environment. Use comma as a separator to filter on multiple environments. @@ -67,7 +51,7 @@ export class SecretsListCommand extends Command<{}, Opts> { printHeader(headerLog, "List secrets", "lock") } - async action({ garden, log, opts }: CommandParams<{}, Opts>): Promise> { + async action({ garden, log, opts }: CommandParams<{}, Opts>): Promise> { const envFilter = opts["filter-envs"] || [] const nameFilter = opts["filter-names"] || [] const userFilter = opts["filter-user-ids"] || [] @@ -81,28 +65,7 @@ export class SecretsListCommand extends Command<{}, Opts> { const q = stringify({ projectId: project.id }) const res = await api.get(`/secrets?${q}`) - const secrets = res.data.map((secret) => { - const ret: Secret = { - name: secret.name, - id: secret.id, - updatedAt: secret.updatedAt, - createdAt: secret.createdAt, - } - if (secret.environment) { - ret["environment"] = { - name: secret.environment.name, - id: secret.environment.id, - } - } - if (secret.user) { - ret["user"] = { - name: secret.user.name, - id: secret.user.id, - vcsUsername: secret.user.vcsUsername, - } - } - return ret - }) + const secrets = res.data.map((secret) => makeSecretFromResponse(secret)) log.info("") diff --git a/core/src/commands/enterprise/users/users-create.ts b/core/src/commands/enterprise/users/users-create.ts index 8524fa4a87..6dccb0ea28 100644 --- a/core/src/commands/enterprise/users/users-create.ts +++ b/core/src/commands/enterprise/users/users-create.ts @@ -13,25 +13,16 @@ import { readFile } from "fs-extra" import { printHeader } from "../../../logger/util" import { Command, CommandParams, CommandResult } from "../../base" -import { ApiCommandError, handleBulkOperationResult, noApiMsg } from "../helpers" +import { ApiCommandError, handleBulkOperationResult, makeUserFromResponse, noApiMsg, UserResult } from "../helpers" import { dedent, deline } from "../../../util/string" import { StringsParameter, PathParameter } from "../../../cli/params" import { StringMap } from "../../../config/common" import { chunk } from "lodash" import Bluebird = require("bluebird") +// This is the limit set by the API. const MAX_USERS_PER_REQUEST = 100 -export interface ErrorResponse { - message: string - status: string -} - -export interface UsersCreateCommandResult { - errors: ApiCommandError[] - results: UserResponse[] -} - export const secretsCreateArgs = { users: new StringsParameter({ help: deline`The VCS usernames and the names of the users to create, separated by '='. @@ -83,12 +74,7 @@ export class UsersCreateCommand extends Command { printHeader(headerLog, "Create users", "lock") } - async action({ - garden, - log, - opts, - args, - }: CommandParams): Promise> { + async action({ garden, log, opts, args }: CommandParams): Promise> { const addToGroups = (opts["add-to-groups"] || []).map((groupId) => parseInt(groupId, 10)) const fromFile = opts["from-file"] as string | undefined let users: StringMap @@ -122,7 +108,7 @@ export class UsersCreateCommand extends Command { throw new ConfigurationError(noApiMsg("create", "users"), {}) } - const cmdLog = log.info({ status: "active", section: "groups-command", msg: "Creating users..." }) + const cmdLog = log.info({ status: "active", section: "users-command", msg: "Creating users..." }) const usersToCreate = Object.entries(users).map(([vcsUsername, name]) => ({ name, @@ -137,7 +123,7 @@ export class UsersCreateCommand extends Command { let count = 1 const errors: ApiCommandError[] = [] - const results: UserResponse[] = [] + const results: UserResult[] = [] await Bluebird.map( batches, async (userBatch) => { @@ -154,7 +140,7 @@ export class UsersCreateCommand extends Command { } const res = await api.post(`/users/bulk`, { body }) const successes = res.data.filter((d) => d.statusCode === 200).map((d) => d.user) as UserResponse[] - results.push(...successes) + results.push(...successes.map((s) => makeUserFromResponse(s))) const failures = res.data .filter((d) => d.statusCode !== 200) @@ -173,15 +159,13 @@ export class UsersCreateCommand extends Command { { concurrency } ) - handleBulkOperationResult({ + return handleBulkOperationResult({ log, cmdLog, errors, action: "create", resource: "user", - successCount: results.length, + results, }) - - return { result: { errors, results } } } } diff --git a/core/src/commands/enterprise/users/users-delete.ts b/core/src/commands/enterprise/users/users-delete.ts index 8db406b8d3..ea4ee455d9 100644 --- a/core/src/commands/enterprise/users/users-delete.ts +++ b/core/src/commands/enterprise/users/users-delete.ts @@ -12,12 +12,7 @@ import { CommandError, ConfigurationError } from "../../../exceptions" import { printHeader } from "../../../logger/util" import { dedent, deline } from "../../../util/string" import { Command, CommandParams, CommandResult } from "../../base" -import { ApiCommandError, confirmDelete, handleBulkOperationResult, noApiMsg } from "../helpers" - -export interface UsersDeleteCommandResult { - errors: ApiCommandError[] - results: BaseResponse[] -} +import { ApiCommandError, confirmDelete, DeleteResult, handleBulkOperationResult, noApiMsg } from "../helpers" export const usersDeleteArgs = { ids: new StringsParameter({ @@ -44,7 +39,7 @@ export class UsersDeleteCommand extends Command { printHeader(headerLog, "Delete users", "lock") } - async action({ garden, args, log }: CommandParams): Promise> { + async action({ garden, args, log, opts }: CommandParams): Promise> { const usersToDelete = (args.ids || []).map((id) => parseInt(id, 10)) if (usersToDelete.length === 0) { throw new CommandError(`No user IDs provided.`, { @@ -52,7 +47,7 @@ export class UsersDeleteCommand extends Command { }) } - if (!(await confirmDelete("user", usersToDelete.length))) { + if (!opts.yes && !(await confirmDelete("user", usersToDelete.length))) { return {} } @@ -65,13 +60,13 @@ export class UsersDeleteCommand extends Command { let count = 1 const errors: ApiCommandError[] = [] - const results: BaseResponse[] = [] + const results: DeleteResult[] = [] for (const id of usersToDelete) { cmdLog.setState({ msg: `Deleting users... → ${count}/${usersToDelete.length}` }) count++ try { const res = await api.delete(`/users/${id}`) - results.push(res) + results.push({ id, status: res.status }) } catch (err) { errors.push({ identifier: id, @@ -80,15 +75,13 @@ export class UsersDeleteCommand extends Command { } } - handleBulkOperationResult({ + return handleBulkOperationResult({ log, cmdLog, errors, action: "delete", resource: "user", - successCount: results.length, + results, }) - - return { result: { errors, results } } } } diff --git a/core/src/commands/enterprise/users/users-list.ts b/core/src/commands/enterprise/users/users-list.ts index 9ab1d4eda8..d8747f7e3d 100644 --- a/core/src/commands/enterprise/users/users-list.ts +++ b/core/src/commands/enterprise/users/users-list.ts @@ -12,20 +12,11 @@ import { GetAllUsersResponse } from "@garden-io/platform-api-types" import { printHeader } from "../../../logger/util" import { dedent, deline, renderTable } from "../../../util/string" import { Command, CommandParams, CommandResult } from "../../base" -import { applyFilter, getProject, noApiMsg } from "../helpers" +import { applyFilter, getProject, makeUserFromResponse, noApiMsg, UserResult } from "../helpers" import chalk from "chalk" import { sortBy } from "lodash" import { StringsParameter } from "../../../cli/params" -interface User { - id: number - createdAt: string - updatedAt: string - name: string - vcsUsername: string - groups: string[] -} - export const usersListOpts = { "filter-names": new StringsParameter({ help: deline`Filter on user name. Use comma as a separator to filter on multiple names. Accepts glob patterns.`, @@ -55,7 +46,7 @@ export class UsersListCommand extends Command<{}, Opts> { printHeader(headerLog, "List users", "information_desk_person") } - async action({ garden, log, opts }: CommandParams<{}, Opts>): Promise> { + async action({ garden, log, opts }: CommandParams<{}, Opts>): Promise> { const nameFilter = opts["filter-names"] || [] const groupFilter = opts["filter-groups"] || [] @@ -73,20 +64,11 @@ export class UsersListCommand extends Command<{}, Opts> { : "VCS" let page = 0 - let users: User[] = [] + let users: UserResult[] = [] let hasMore = true while (hasMore) { const res = await api.get(`/users?page=${page}`) - users.push( - ...res.data.map((user) => ({ - id: user.id, - name: user.name, - vcsUsername: user.vcsUsername, - createdAt: user.createdAt, - updatedAt: user.updatedAt, - groups: user.groups.map((g) => g.name), - })) - ) + users.push(...res.data.map((user) => makeUserFromResponse(user))) if (res.data.length === 0) { hasMore = false } else { @@ -103,7 +85,12 @@ export class UsersListCommand extends Command<{}, Opts> { const filtered = sortBy(users, "name") .filter((user) => applyFilter(nameFilter, user.name)) - .filter((user) => applyFilter(groupFilter, user.groups)) + .filter((user) => + applyFilter( + groupFilter, + user.groups.map((g) => g.name) + ) + ) if (filtered.length === 0) { log.info("No users found in project that match filters.") @@ -116,7 +103,7 @@ export class UsersListCommand extends Command<{}, Opts> { chalk.cyan.bold(u.name), String(u.id), u.vcsUsername, - u.groups.join(", "), + u.groups.map((g) => g.name).join(", "), new Date(u.createdAt).toUTCString(), ] }) diff --git a/core/src/logger/util.ts b/core/src/logger/util.ts index a2a922fbbe..9535bf8f2a 100644 --- a/core/src/logger/util.ts +++ b/core/src/logger/util.ts @@ -12,7 +12,7 @@ import CircularJSON from "circular-json" import { LogNode, LogLevel } from "./log-node" import { LogEntry, LogEntryParams, EmojiName } from "./log-entry" import { deepMap, deepFilter, safeDumpYaml } from "../util/util" -import { padEnd, isEmpty, isNumber } from "lodash" +import { padEnd, isEmpty } from "lodash" import { dedent } from "../util/string" import hasAnsi from "has-ansi" import { GardenError } from "../exceptions" @@ -227,8 +227,11 @@ export function formatGardenError(error: GardenError) { let out = message || "" // We recursively filter out internal fields (i.e. having names starting with _). - const filteredDetail = deepFilter(detail, (_: any, key: string | number) => { - return !isNumber(key) && !key.startsWith("_") + const filteredDetail = deepFilter(detail, (_val, key: string | number) => { + if (typeof key === "string") { + return !key.startsWith("_") + } + return true }) if (!isEmpty(filteredDetail)) {