From 20742f4f1cfb7f408635db783ec29b249a533b5d Mon Sep 17 00:00:00 2001 From: Vladimir Vagaytsev Date: Wed, 22 May 2024 14:30:05 +0200 Subject: [PATCH] improvement(cli): more detailed logging in `cloud secret` commands (#6065) * refactor: inline redundant helpers * improvement(cloud): warn on input source precedence for users and secrets * chore: unwrap unnecessary if-statement * chore: add comments * refactor: init both secret collections in the same place Reverse refactoring. * refactor: rename local var for more clarity * refactor: isolate complex secrets-splitting logic in a function * improvement: warn on simultaneous usage of mutual-exclusive flags --- core/src/commands/cloud/helpers.ts | 17 ++- .../commands/cloud/secrets/secret-helpers.ts | 16 --- .../commands/cloud/secrets/secrets-create.ts | 18 ++-- .../commands/cloud/secrets/secrets-update.ts | 102 ++++++++++++------ core/src/commands/cloud/users/user-helpers.ts | 16 --- core/src/commands/cloud/users/users-create.ts | 17 +-- 6 files changed, 105 insertions(+), 81 deletions(-) diff --git a/core/src/commands/cloud/helpers.ts b/core/src/commands/cloud/helpers.ts index 38986d7a0b..d46fefb8c9 100644 --- a/core/src/commands/cloud/helpers.ts +++ b/core/src/commands/cloud/helpers.ts @@ -138,15 +138,23 @@ export async function readInputKeyValueResources({ resourceFilePath, resourcesFromArgs, resourceName, + log, }: { resourceFilePath: string | undefined resourcesFromArgs: string[] | undefined resourceName: string + log: Log }): Promise { - // TODO: --from-file takes implicit precedence over args. - // Document this or allow both, or throw an error if both sources are defined. + // File source (by naming convention for args/opts, it's defined via --from-file option) + // always takes precedence over the positional arguments. if (resourceFilePath) { try { + if (resourcesFromArgs && resourcesFromArgs.length > 0) { + log.warn( + `Reading ${resourceName}s from file ${resourceFilePath}. Positional arguments will be ignored: ${resourcesFromArgs.join(" ")}.` + ) + } + const dotEnvFileContent = await readFile(resourceFilePath) return dotenv.parse(dotEnvFileContent) } catch (err) { @@ -154,7 +162,10 @@ export async function readInputKeyValueResources({ message: `Unable to read ${resourceName}(s) from file at path ${resourceFilePath}: ${err}`, }) } - } else if (resourcesFromArgs) { + } + + // Get input resources from positional arguments in no input file defined. + if (resourcesFromArgs) { return resourcesFromArgs.reduce((acc, keyValPair) => { try { const resourceEntry = dotenv.parse(keyValPair) diff --git a/core/src/commands/cloud/secrets/secret-helpers.ts b/core/src/commands/cloud/secrets/secret-helpers.ts index 30a1ee58b9..1e95f44cd2 100644 --- a/core/src/commands/cloud/secrets/secret-helpers.ts +++ b/core/src/commands/cloud/secrets/secret-helpers.ts @@ -7,8 +7,6 @@ */ import type { ListSecretsResponse, SecretResult as SecretResultApi } from "@garden-io/platform-api-types" -import type { StringMap } from "../../../config/common.js" -import { readInputKeyValueResources } from "../helpers.js" import type { CloudApi, CloudEnvironment, CloudProject } from "../../../cloud/api.js" import type { Log } from "../../../logger/log-entry.js" import queryString from "query-string" @@ -79,20 +77,6 @@ export function makeSecretFromResponse(res: SecretResultApi): SecretResult { return secret } -export async function readInputSecrets({ - secretsFilePath, - secretsFromArgs, -}: { - secretsFilePath: string | undefined - secretsFromArgs: string[] | undefined -}): Promise { - return await readInputKeyValueResources({ - resourceFilePath: secretsFilePath, - resourcesFromArgs: secretsFromArgs, - resourceName: "secret", - }) -} - const secretsPageLimit = 100 export async function fetchAllSecrets(api: CloudApi, projectId: string, log: Log): Promise { diff --git a/core/src/commands/cloud/secrets/secrets-create.ts b/core/src/commands/cloud/secrets/secrets-create.ts index 385cfbf0a7..2676b258d3 100644 --- a/core/src/commands/cloud/secrets/secrets-create.ts +++ b/core/src/commands/cloud/secrets/secrets-create.ts @@ -6,19 +6,17 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -import { CommandError, ConfigurationError, CloudApiError, GardenError } from "../../../exceptions.js" +import { CloudApiError, CommandError, ConfigurationError, GardenError } from "../../../exceptions.js" import type { CreateSecretResponse } from "@garden-io/platform-api-types" import { printHeader } from "../../../logger/util.js" import type { CommandParams, CommandResult } from "../../base.js" import { Command } from "../../base.js" import type { ApiCommandError } from "../helpers.js" -import { handleBulkOperationResult, noApiMsg } from "../helpers.js" +import { handleBulkOperationResult, noApiMsg, readInputKeyValueResources } from "../helpers.js" import { dedent, deline } from "../../../util/string.js" import { PathParameter, StringParameter, StringsParameter } from "../../../cli/params.js" import type { SecretResult } from "./secret-helpers.js" -import { getEnvironmentByNameOrThrow } from "./secret-helpers.js" -import { readInputSecrets } from "./secret-helpers.js" -import { makeSecretFromResponse } from "./secret-helpers.js" +import { getEnvironmentByNameOrThrow, makeSecretFromResponse } from "./secret-helpers.js" import { enumerate } from "../../../util/enumerate.js" export const secretsCreateArgs = { @@ -88,7 +86,14 @@ export class SecretsCreateCommand extends Command { }) } - const secrets = await readInputSecrets({ secretsFilePath, secretsFromArgs: args.secrets }) + const cmdLog = log.createLog({ name: "secrets-command" }) + + const secrets = await readInputKeyValueResources({ + resourceFilePath: secretsFilePath, + resourcesFromArgs: args.secrets, + resourceName: "secret", + log: cmdLog, + }) const api = garden.cloudApi if (!api) { @@ -113,7 +118,6 @@ export class SecretsCreateCommand extends Command { } const secretsToCreate = Object.entries(secrets) - const cmdLog = log.createLog({ name: "secrets-command" }) cmdLog.info("Creating secrets...") const errors: ApiCommandError[] = [] diff --git a/core/src/commands/cloud/secrets/secrets-update.ts b/core/src/commands/cloud/secrets/secrets-update.ts index 7f500b9c43..1cf2352753 100644 --- a/core/src/commands/cloud/secrets/secrets-update.ts +++ b/core/src/commands/cloud/secrets/secrets-update.ts @@ -16,14 +16,12 @@ import { dedent, deline } from "../../../util/string.js" import type { CommandParams, CommandResult } from "../../base.js" import { Command } from "../../base.js" import type { ApiCommandError } from "../helpers.js" -import { handleBulkOperationResult, noApiMsg } from "../helpers.js" +import { handleBulkOperationResult, noApiMsg, readInputKeyValueResources } from "../helpers.js" import type { Log } from "../../../logger/log-entry.js" import type { SecretResult } from "./secret-helpers.js" -import { getEnvironmentByNameOrThrow } from "./secret-helpers.js" -import { fetchAllSecrets } from "./secret-helpers.js" -import { readInputSecrets } from "./secret-helpers.js" -import { makeSecretFromResponse } from "./secret-helpers.js" +import { fetchAllSecrets, getEnvironmentByNameOrThrow, makeSecretFromResponse } from "./secret-helpers.js" import { enumerate } from "../../../util/enumerate.js" +import type { CloudApi, CloudProject } from "../../../cloud/api.js" export const secretsUpdateArgs = { secretNamesOrIds: new StringsParameter({ @@ -94,7 +92,7 @@ export class SecretsUpdateCommand extends Command { const userId = opts["scope-to-user-id"] as string | undefined const secretsFilePath = opts["from-file"] as string | undefined const updateById = opts["update-by-id"] as boolean | undefined - const isUpsert = opts["upsert"] as boolean | undefined + const upsert = opts["upsert"] as boolean | undefined if (!updateById && userId !== undefined && !envName) { throw new CommandError({ @@ -102,7 +100,14 @@ export class SecretsUpdateCommand extends Command { }) } - const secretsToUpdateArgs = await readInputSecrets({ secretsFilePath, secretsFromArgs: args.secretNamesOrIds }) + const cmdLog = log.createLog({ name: "secrets-command" }) + + const inputSecrets = await readInputKeyValueResources({ + resourceFilePath: secretsFilePath, + resourcesFromArgs: args.secretNamesOrIds, + resourceName: "secret", + log: cmdLog, + }) const api = garden.cloudApi if (!api) { @@ -116,31 +121,16 @@ export class SecretsUpdateCommand extends Command { const environmentId: string | undefined = getEnvironmentByNameOrThrow({ envName, project })?.id - const allSecrets: SecretResult[] = await fetchAllSecrets(api, project.id, log) - - let secretsToUpdate: Array - if (updateById) { - // update secrets by ids - secretsToUpdate = sortBy(allSecrets, "name") - .filter((secret) => Object.keys(secretsToUpdateArgs).includes(secret.id)) - .map((secret) => ({ ...secret, newValue: secretsToUpdateArgs[secret.id] })) - } else { - // update secrets by name - secretsToUpdate = await getSecretsToUpdateByName({ - allSecrets, - envName, - userId, - secretsToUpdateArgs, - log, - }) - } - - let secretsToCreate: [string, string][] = [] - if (isUpsert && !updateById) { - // if --upsert is set, check the diff between secrets to update and command args to find out - // secrets that do not exist yet and can be created - secretsToCreate = getSecretsToCreate(secretsToUpdateArgs, secretsToUpdate) - } + const { secretsToCreate, secretsToUpdate } = await splitSecretsByExistence({ + api, + envName, + inputSecrets, + log, + project, + updateById, + upsert, + userId, + }) if (secretsToUpdate.length === 0 && secretsToCreate.length === 0) { throw new CommandError({ @@ -148,7 +138,6 @@ export class SecretsUpdateCommand extends Command { }) } - const cmdLog = log.createLog({ name: "secrets-command" }) if (secretsToUpdate?.length > 0) { cmdLog.info(`${secretsToUpdate.length} existing secret(s) to be updated.`) } @@ -209,6 +198,53 @@ export class SecretsUpdateCommand extends Command { } } +async function splitSecretsByExistence(params: { + api: CloudApi + envName: string | undefined + log: Log + inputSecrets: StringMap + project: CloudProject + updateById: boolean | undefined + upsert: boolean | undefined + userId: string | undefined +}): Promise<{ secretsToCreate: Array<[string, string]>; secretsToUpdate: Array }> { + const { api, envName, inputSecrets, log, project, updateById, upsert, userId } = params + + const allSecrets: SecretResult[] = await fetchAllSecrets(api, project.id, log) + + let secretsToCreate: [string, string][] + let secretsToUpdate: Array + if (updateById) { + if (upsert) { + log.warn(`Updating secrets by IDs. Flag --upsert has no effect when it's used with --update-by-id.`) + } + + // update secrets by ids + secretsToUpdate = sortBy(allSecrets, "name") + .filter((secret) => Object.keys(inputSecrets).includes(secret.id)) + .map((secret) => ({ ...secret, newValue: inputSecrets[secret.id] })) + secretsToCreate = [] + } else { + // update secrets by name + secretsToUpdate = await getSecretsToUpdateByName({ + allSecrets, + envName, + userId, + secretsToUpdateArgs: inputSecrets, + log, + }) + if (upsert) { + // if --upsert is set, check the diff between secrets to update and command args to find out + // secrets that do not exist yet and can be created + secretsToCreate = getSecretsToCreate(inputSecrets, secretsToUpdate) + } else { + secretsToCreate = [] + } + } + + return { secretsToCreate, secretsToUpdate } +} + export type UpdateSecretBody = SecretResult & { newValue: string } export async function getSecretsToUpdateByName({ diff --git a/core/src/commands/cloud/users/user-helpers.ts b/core/src/commands/cloud/users/user-helpers.ts index fc908b30fa..b254b5dc0b 100644 --- a/core/src/commands/cloud/users/user-helpers.ts +++ b/core/src/commands/cloud/users/user-helpers.ts @@ -7,8 +7,6 @@ */ import type { UserResult as UserResultApi } from "@garden-io/platform-api-types" -import type { StringMap } from "../../../config/common.js" -import { readInputKeyValueResources } from "../helpers.js" export interface UserResult { id: string @@ -32,17 +30,3 @@ export function makeUserFromResponse(user: UserResultApi): UserResult { groups: user.groups.map((g) => ({ id: g.id, name: g.name })), } } - -export async function readInputUsers({ - usersFilePath, - usersFromArgs, -}: { - usersFilePath: string | undefined - usersFromArgs: string[] | undefined -}): Promise { - return await readInputKeyValueResources({ - resourceFilePath: usersFilePath, - resourcesFromArgs: usersFromArgs, - resourceName: "user", - }) -} diff --git a/core/src/commands/cloud/users/users-create.ts b/core/src/commands/cloud/users/users-create.ts index c40a954114..1f210fec09 100644 --- a/core/src/commands/cloud/users/users-create.ts +++ b/core/src/commands/cloud/users/users-create.ts @@ -16,13 +16,12 @@ import { printHeader } from "../../../logger/util.js" import type { CommandParams, CommandResult } from "../../base.js" import { Command } from "../../base.js" import type { ApiCommandError } from "../helpers.js" -import { handleBulkOperationResult, noApiMsg } from "../helpers.js" +import { handleBulkOperationResult, noApiMsg, readInputKeyValueResources } from "../helpers.js" import { dedent, deline } from "../../../util/string.js" import { PathParameter, StringsParameter } from "../../../cli/params.js" import { chunk } from "lodash-es" import pMap from "p-map" import type { UserResult } from "./user-helpers.js" -import { readInputUsers } from "./user-helpers.js" import { makeUserFromResponse } from "./user-helpers.js" // This is the limit set by the API. @@ -84,21 +83,27 @@ export class UsersCreateCommand extends Command { const addToGroups: string[] = opts["add-to-groups"] || [] const usersFilePath = opts["from-file"] as string | undefined - const users = await readInputUsers({ usersFilePath, usersFromArgs: args.users }) + const cmdLog = log.createLog({ name: "users-command" }) + + const users = await readInputKeyValueResources({ + resourceFilePath: usersFilePath, + resourcesFromArgs: args.users, + resourceName: "user", + log: cmdLog, + }) const api = garden.cloudApi if (!api) { throw new ConfigurationError({ message: noApiMsg("create", "users") }) } - const cmdLog = log.createLog({ name: "users-command" }) - cmdLog.info("Creating users...") - const usersToCreate = Object.entries(users).map(([vcsUsername, name]) => ({ name, vcsUsername, serviceAccount: false, })) + cmdLog.info("Creating users...") + const batches = chunk(usersToCreate, MAX_USERS_PER_REQUEST) // This pretty arbitrary, but the bulk action can create 100 users at a time // so the queue shouldn't ever get very long.