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
garden-io · May 22, 2024 · 20742f4 · 20742f4
1 parent 6247676
commit 20742f4
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 81 deletions.
diff --git a/core/src/commands/cloud/helpers.ts b/core/src/commands/cloud/helpers.ts
@@ -138,23 +138,34 @@ export async function readInputKeyValueResources({
   resourceFilePath,
   resourcesFromArgs,
   resourceName,
+  log,
 }: {
   resourceFilePath: string | undefined
   resourcesFromArgs: string[] | undefined
   resourceName: string
+  log: Log
 }): Promise<StringMap> {
-  // 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) {
       throw new CommandError({
         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
@@ -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<StringMap> {
-  return await readInputKeyValueResources({
-    resourceFilePath: secretsFilePath,
-    resourcesFromArgs: secretsFromArgs,
-    resourceName: "secret",
-  })
-}
-
 const secretsPageLimit = 100
 
 export async function fetchAllSecrets(api: CloudApi, projectId: string, log: Log): Promise<SecretResult[]> {

diff --git 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<Args, Opts> {
       })
     }
 
-    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<Args, Opts> {
     }
 
     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
@@ -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,15 +92,22 @@ export class SecretsUpdateCommand extends Command<Args, Opts> {
     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({
         message: `Got user ID but not environment name. Secrets scoped to users must be scoped to environments as well.`,
       })
     }
 
-    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,39 +121,23 @@ export class SecretsUpdateCommand extends Command<Args, Opts> {
 
     const environmentId: string | undefined = getEnvironmentByNameOrThrow({ envName, project })?.id
 
-    const allSecrets: SecretResult[] = await fetchAllSecrets(api, project.id, log)
-
-    let secretsToUpdate: Array<UpdateSecretBody>
-    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({
         message: `No secrets to be updated or created.`,
       })
     }
 
-    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<Args, Opts> {
   }
 }
 
+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<UpdateSecretBody> }> {
+  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<UpdateSecretBody>
+  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
@@ -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<StringMap> {
-  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
@@ -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<Args, Opts> {
     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.