fix: get kms values & age key generation order (#1768)

Co-authored-by: jeho <[email protected]>

Author: ferruhcihan

src/cmd/bootstrap.test.ts

@@ -24,35 +24,6 @@ describe('Bootstrapping values', () => {
   }
   const users = [{ id: 'user1', initialPassword: 'existing-password' }, { id: 'user2' }]
   const secrets = { secret: 'true', deep: { nested: 'secret' } }
-  const ageKeys = { publicKey: 'agePublicKey', privateKey: 'agePrivateKey' }
-  const kmsValues = {
-    kms: {
-      sops: {
-        provider: 'azure',
-        azure: {
-          keys: 'key1,key2',
-        },
-      },
-    },
-  }
-  const kmsValuesWithAgeProvider = {
-    kms: {
-      sops: {
-        provider: 'age',
-      },
-    },
-  }
-  const kmsValuesWithAgeFull = {
-    kms: {
-      sops: {
-        provider: 'age',
-        age: {
-          publicKey: 'publicKey',
-          privateKey: 'privateKey',
-        },
-      },
-    },
-  }
   let deps
   beforeEach(() => {
     deps = {
@@ -103,24 +74,64 @@ describe('Bootstrapping values', () => {
     const res = await getStoredClusterSecrets(deps)
     expect(res).toEqual(undefined)
   })
-  it('should not get kms values if those do not exist', async () => {
-    const res = await getKmsValues(values)
-    expect(res).toEqual(undefined)
-  })
-  it('should get kms values if those exist', async () => {
-    const res = await getKmsValues({ ...values, ...kmsValues })
-    expect(res).toEqual(kmsValues)
-  })
-  it('should generate and return new age keys if provider is age and keys are missing', async () => {
-    const kmsValuesDeps = {
-      generateAgeKeys: jest.fn().mockResolvedValue(ageKeys),
+  describe('getKmsValues', () => {
+    let kmsValuesDeps: any
+    const ageKeys = { publicKey: 'agePublicKey', privateKey: 'agePrivateKey' }
+    const values = { someKey: 'someValue' }
+    const kmsValues = {
+      kms: {
+        sops: {
+          provider: 'azure',
+          azure: {
+            keys: 'key1,key2',
+          },
+        },
+      },
     }
-    const res = await getKmsValues({ ...values, ...kmsValuesWithAgeProvider }, kmsValuesDeps)
-    expect(res).toEqual({ kms: { sops: { provider: 'age', age: ageKeys } } })
-  })
-  it('should get kms values if age has public and private key', async () => {
-    const res = await getKmsValues({ ...values, ...kmsValuesWithAgeFull })
-    expect(res).toEqual(kmsValuesWithAgeFull)
+    const kmsValuesWithAgeProvider = {
+      kms: {
+        sops: {
+          provider: 'age',
+        },
+      },
+    }
+    const kmsValuesWithAgeFull = {
+      kms: {
+        sops: {
+          provider: 'age',
+          age: {
+            publicKey: 'publicKey',
+            privateKey: 'privateKey',
+          },
+        },
+      },
+    }
+    beforeEach(() => {
+      kmsValuesDeps = {
+        generateAgeKeys: jest.fn().mockResolvedValue(ageKeys),
+        hfValues: jest.fn(),
+      }
+    })
+    it('should not get kms values if those do not exist', async () => {
+      kmsValuesDeps.hfValues.mockReturnValue(values)
+      const res = await getKmsValues(kmsValuesDeps)
+      expect(res).toBeUndefined()
+    })
+    it('should get kms values if those exist', async () => {
+      kmsValuesDeps.hfValues.mockReturnValue({ ...values, ...kmsValues })
+      const res = await getKmsValues(kmsValuesDeps)
+      expect(res).toEqual(kmsValues)
+    })
+    it('should generate and return new age keys if provider is age and keys are missing', async () => {
+      kmsValuesDeps.hfValues.mockReturnValue({ ...values, ...kmsValuesWithAgeProvider })
+      const res = await getKmsValues(kmsValuesDeps)
+      expect(res).toEqual({ kms: { sops: { provider: 'age', age: ageKeys } } })
+    })
+    it('should get kms values if age has public and private key', async () => {
+      kmsValuesDeps.hfValues.mockReturnValue({ ...values, ...kmsValuesWithAgeFull })
+      const res = await getKmsValues(kmsValuesDeps)
+      expect(res).toEqual(kmsValuesWithAgeFull)
+    })
   })
   it('should set k8sContext and owner if needed', async () => {
     deps.processValues.mockReturnValue(values)

src/cmd/bootstrap.ts

@@ -183,8 +183,9 @@ export const generateAgeKeys = async (deps = { $, terminal }) => {
   }
 }
 
-export const getKmsValues = async (originalValues: any, deps = { generateAgeKeys }) => {
-  const kms = originalValues?.kms
+export const getKmsValues = async (deps = { generateAgeKeys, hfValues }) => {
+  const values = (await deps.hfValues({ defaultValues: true })) as Record<string, any>
+  const kms = values?.kms
   if (!kms) return undefined
   const provider = kms?.sops?.provider
   if (!provider) return {}
@@ -260,13 +261,11 @@ export const processValues = async (
   const { ENV_DIR, VALUES_INPUT } = env
   let originalInput: Record<string, any> | undefined
   let storedSecrets: Record<string, any> | undefined
-  let kmsValues: Record<string, any> | undefined
   if (deps.isChart) {
     d.log(`Loading app values from ${VALUES_INPUT}`)
     const originalValues = (await deps.loadYaml(VALUES_INPUT)) as Record<string, any>
     storedSecrets = (await deps.getStoredClusterSecrets()) || {}
-    kmsValues = (await deps.getKmsValues(originalValues)) || {}
-    originalInput = merge(cloneDeep(storedSecrets || {}), cloneDeep(originalValues), cloneDeep(kmsValues))
+    originalInput = merge(cloneDeep(storedSecrets || {}), cloneDeep(originalValues))
     await deps.writeValues(originalInput)
   } else {
     d.log(`Loading repo values from ${ENV_DIR}`)
@@ -287,8 +286,15 @@ export const processValues = async (
   } else {
     caSecrets = deps.createCustomCA()
   }
+  // get any kms values & generate age keys if needed
+  const kmsValues = (await deps.getKmsValues()) || {}
   // merge existing secrets over newly generated ones to keep them
-  const allSecrets = merge(cloneDeep(caSecrets), cloneDeep(storedSecrets), cloneDeep(generatedSecrets))
+  const allSecrets = merge(
+    cloneDeep(caSecrets),
+    cloneDeep(storedSecrets),
+    cloneDeep(generatedSecrets),
+    cloneDeep(kmsValues),
+  )
   // generate initial passwords for users if they don't have one
   const users = get(originalInput, 'users', [])
   for (const user of users) {