refactor(secrets): payload creation for secrets update command (#6106)

* fix(secrets): payload creation for `secrets update` command * test: fix test assertions
garden-io · May 30, 2024 · 5fb1d94 · 5fb1d94
1 parent 974de64
commit 5fb1d94
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 27 deletions.
diff --git a/core/src/commands/cloud/secrets/secrets-update.ts b/core/src/commands/cloud/secrets/secrets-update.ts
@@ -19,7 +19,13 @@ import type { Log } from "../../../logger/log-entry.js"
 import type { SecretResult } from "./secret-helpers.js"
 import { makeSecretFromResponse } from "./secret-helpers.js"
 import { getEnvironmentByNameOrThrow } from "./secret-helpers.js"
-import type { BulkCreateSecretRequest, BulkUpdateSecretRequest, CloudApi, Secret } from "../../../cloud/api.js"
+import type {
+  BulkCreateSecretRequest,
+  BulkUpdateSecretRequest,
+  CloudApi,
+  Secret,
+  SingleUpdateSecretRequest,
+} from "../../../cloud/api.js"
 
 export const secretsUpdateArgs = {
   secretNamesOrIds: new StringsParameter({
@@ -182,7 +188,7 @@ async function prepareSecretsRequests(params: {
   const allSecrets = await api.fetchAllSecrets(projectId, log)
 
   let secretsToCreate: Secret[]
-  let secretsToUpdate: Array<UpdateSecretBody>
+  let secretsToUpdate: SingleUpdateSecretRequest[]
   if (updateById) {
     if (upsert) {
       log.warn(`Updating secrets by IDs. Flag --upsert has no effect when it's used with --update-by-id.`)
@@ -191,8 +197,17 @@ async function prepareSecretsRequests(params: {
     const inputSecretDict = fromPairs(inputSecrets.map((s) => [s.name, s.value]))
     // update secrets by ids
     secretsToUpdate = sortBy(allSecrets, "name")
-      .filter((secret) => !!inputSecretDict[secret.id])
-      .map((secret) => ({ ...secret, newValue: inputSecrets[secret.id] }))
+      .filter((existingSecret) => !!inputSecretDict[existingSecret.id])
+      .map((existingSecret) => {
+        const updateSecretsPayload: SingleUpdateSecretRequest = {
+          id: existingSecret.id,
+          environmentId: existingSecret.environment?.id,
+          userId: existingSecret.user?.id,
+          name: existingSecret.name,
+          value: inputSecretDict[existingSecret.id],
+        }
+        return updateSecretsPayload
+      })
     secretsToCreate = []
   } else {
     // update secrets by name
@@ -223,8 +238,6 @@ async function prepareSecretsRequests(params: {
   }
 }
 
-export type UpdateSecretBody = CloudApiSecretResult & { newValue: string }
-
 export async function getSecretsToUpdateByName({
   allSecrets,
   environmentName,
@@ -237,7 +250,7 @@ export async function getSecretsToUpdateByName({
   userId?: string
   inputSecrets: Secret[]
   log: Log
-}): Promise<Array<UpdateSecretBody>> {
+}): Promise<SingleUpdateSecretRequest[]> {
   const inputSecretDict = fromPairs(inputSecrets.map((s) => [s.name, s.value]))
 
   const filteredSecrets = sortBy(allSecrets, "name")
@@ -273,10 +286,19 @@ export async function getSecretsToUpdateByName({
     })
   }
 
-  return filteredSecrets.map((secret) => ({ ...secret, newValue: inputSecretDict[secret.name] }))
+  return filteredSecrets.map((existingSecret) => {
+    const updateSecretsPayload: SingleUpdateSecretRequest = {
+      id: existingSecret.id,
+      environmentId: existingSecret.environment?.id,
+      userId: existingSecret.user?.id,
+      name: existingSecret.name,
+      value: inputSecretDict[existingSecret.name],
+    }
+    return updateSecretsPayload
+  })
 }
 
-export function getSecretsToCreate(inputSecrets: Secret[], secretsToUpdate: Array<UpdateSecretBody>): Secret[] {
+export function getSecretsToCreate(inputSecrets: Secret[], secretsToUpdate: SingleUpdateSecretRequest[]): Secret[] {
   const secretToUpdateIds = new Set(secretsToUpdate.map((secret) => secret.name))
   return inputSecrets.filter((inputSecret) => !secretToUpdateIds.has(inputSecret.name))
 }
diff --git a/core/test/unit/src/commands/cloud/secrets/secrets-update.ts b/core/test/unit/src/commands/cloud/secrets/secrets-update.ts
@@ -15,7 +15,7 @@ import {
 } from "../../../../../../src/commands/cloud/secrets/secrets-update.js"
 import { deline } from "../../../../../../src/util/string.js"
 import { expectError, getDataDir, makeTestGarden } from "../../../../../helpers.js"
-import type { Secret } from "../../../../../../src/cloud/api.js"
+import type { Secret, SingleUpdateSecretRequest } from "../../../../../../src/cloud/api.js"
 
 describe("SecretsUpdateCommand", () => {
   const projectRoot = getDataDir("test-project-b")
@@ -93,21 +93,23 @@ describe("SecretsUpdateCommand", () => {
     const garden = await makeTestGarden(projectRoot)
     const log = garden.log
     const inputSecrets: Secret[] = [{ name: "secret2", value: "foo" }]
-    const actual = await getSecretsToUpdateByName({
+    const actualSecret = await getSecretsToUpdateByName({
       allSecrets,
       environmentName: undefined,
       userId: undefined,
       inputSecrets,
       log,
     })
 
-    const expectedSecret = allSecrets.find((a) => a.id === "2")
-    expect(actual).to.eql([
-      {
-        ...expectedSecret,
-        newValue: "foo",
-      },
-    ])
+    const matchingSecret = allSecrets.find((a) => a.id === "2")!
+    const expectedSecret: SingleUpdateSecretRequest = {
+      id: matchingSecret.id,
+      environmentId: matchingSecret.environment?.id,
+      userId: matchingSecret.user?.id,
+      name: matchingSecret.name,
+      value: "foo",
+    }
+    expect(actualSecret).to.eql([expectedSecret])
   })
 
   it(`should throw an error when multiple secrets of same name are found, and user and env scopes are not set`, async () => {
@@ -138,22 +140,23 @@ describe("SecretsUpdateCommand", () => {
     const log = garden.log
     const inputSecrets: Secret[] = [{ name: "secret1", value: "foo" }]
 
-    const actual = await getSecretsToUpdateByName({
+    const actualSecret = await getSecretsToUpdateByName({
       allSecrets,
       environmentName: "env1",
       userId: "u1",
       inputSecrets,
       log,
     })
 
-    const expectedSecret = allSecrets.find((a) => a.id === "4")
-
-    expect(actual).to.eql([
-      {
-        ...expectedSecret,
-        newValue: "foo",
-      },
-    ])
+    const matchingSecret = allSecrets.find((a) => a.id === "4")!
+    const expectedSecret: SingleUpdateSecretRequest = {
+      id: matchingSecret.id,
+      environmentId: matchingSecret.environment?.id,
+      userId: matchingSecret.user?.id,
+      name: matchingSecret.name,
+      value: "foo",
+    }
+    expect(actualSecret).to.eql([expectedSecret])
   })
 
   it(`should get correct difference between new secrets and existing secrets for upsert`, async () => {