From f8f16bd3f74f3304a74575917ee30ac48747de16 Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Fri, 28 Aug 2026 12:41:21 +0200 Subject: [PATCH] fix(server-nestjs): reuse Vault AppRole secret-id instead of minting on every sync Refs #2622 Co-authored-by: Automata Signed-off-by: William Phetsinorath Change-Id: Ie3d3b7df1e0539c02d6a215ce7eff82a6a6a6964 --- .../src/modules/argocd/argocd.service.spec.ts | 10 ++--- .../src/modules/argocd/argocd.service.ts | 2 +- .../vault/vault-client.service.spec.ts | 44 +++++++++++++++++++ .../src/modules/vault/vault-client.service.ts | 11 ++++- .../src/modules/vault/vault.utils.ts | 4 ++ 5 files changed, 63 insertions(+), 8 deletions(-) diff --git a/apps/server-nestjs/src/modules/argocd/argocd.service.spec.ts b/apps/server-nestjs/src/modules/argocd/argocd.service.spec.ts index bc35b991c5..bd43281c83 100644 --- a/apps/server-nestjs/src/modules/argocd/argocd.service.spec.ts +++ b/apps/server-nestjs/src/modules/argocd/argocd.service.spec.ts @@ -248,7 +248,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) @@ -447,7 +447,7 @@ describe('argoCDService', () => { ), ]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) @@ -538,7 +538,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockResolvedValue(null) @@ -583,7 +583,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) @@ -738,7 +738,7 @@ describe('argoCDService', () => { gitlab.getOrCreateInfraGroupRepoPublicUrl.mockResolvedValue('https://gitlab.internal/infra-repo') gitlab.listFiles.mockResolvedValue([]) vault.getAuthApproleRoleRoleId.mockResolvedValue('role-id') - vault.createAuthApproleRoleSecretId.mockResolvedValue('secret-id') + vault.ensureAuthApproleRoleSecretId.mockResolvedValue('secret-id') gitlab.generateCreateOrUpdateAction.mockImplementation(async (_repoId, _ref, filePath: string, content: string) => { return makeCommitAction({ filePath, content }) }) diff --git a/apps/server-nestjs/src/modules/argocd/argocd.service.ts b/apps/server-nestjs/src/modules/argocd/argocd.service.ts index 17621b0dfb..97b3843db0 100644 --- a/apps/server-nestjs/src/modules/argocd/argocd.service.ts +++ b/apps/server-nestjs/src/modules/argocd/argocd.service.ts @@ -402,7 +402,7 @@ export class ArgoCDService { this.logger.warn(`Couldn't find app role (project=${projectSlug})`) return undefined }) - const secretId = await this.vault.createAuthApproleRoleSecretId(projectSlug).catch(() => { + const secretId = await this.vault.ensureAuthApproleRoleSecretId(projectSlug).catch(() => { this.logger.warn(`Couldn't find secret (project=${projectSlug})`) return undefined }) diff --git a/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts b/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts index 01adf6a96b..bf3a09c4ce 100644 --- a/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts +++ b/apps/server-nestjs/src/modules/vault/vault-client.service.spec.ts @@ -144,4 +144,48 @@ describe('vault', () => { expect(capturedPath).toBe('forge/my-project/GITLAB') }) }) + + describe('ensureAuthApproleRoleSecretId', () => { + it('mints and persists a secret-id on first sync', async () => { + let minted = false + let persisted: unknown + server.use( + http.get(`${vaultUrl}/v1/kv/data/*`, () => { + return HttpResponse.json({}, { status: HttpStatus.NOT_FOUND }) + }), + http.post(`${vaultUrl}/v1/auth/approle/role/*/secret-id`, () => { + minted = true + return HttpResponse.json({ data: { secret_id: 'minted-secret' } }) + }), + http.post(`${vaultUrl}/v1/kv/data/*`, async ({ request }) => { + persisted = await request.json() + return HttpResponse.json({}) + }), + ) + + const secretId = await service.ensureAuthApproleRoleSecretId('my-project') + + expect(secretId).toBe('minted-secret') + expect(minted).toBe(true) + expect(persisted).toEqual({ data: { secret_id: 'minted-secret' } }) + }) + + it('reuses a persisted secret-id on subsequent syncs without minting', async () => { + let minted = false + server.use( + http.get(`${vaultUrl}/v1/kv/data/*`, () => { + return HttpResponse.json({ data: { data: { secret_id: 'persisted-secret' }, metadata: { created_time: '2023-01-01T00:00:00.000Z', version: 1 } } }) + }), + http.post(`${vaultUrl}/v1/auth/approle/role/*/secret-id`, () => { + minted = true + return HttpResponse.json({ data: { secret_id: 'unexpected' } }) + }), + ) + + const secretId = await service.ensureAuthApproleRoleSecretId('my-project') + + expect(secretId).toBe('persisted-secret') + expect(minted).toBe(false) + }) + }) }) diff --git a/apps/server-nestjs/src/modules/vault/vault-client.service.ts b/apps/server-nestjs/src/modules/vault/vault-client.service.ts index 18fb021913..7fa9454e4d 100644 --- a/apps/server-nestjs/src/modules/vault/vault-client.service.ts +++ b/apps/server-nestjs/src/modules/vault/vault-client.service.ts @@ -5,7 +5,7 @@ import { baseConfigFactory } from '../../config/base.config' import { vaultConfigFactory } from '../../config/vault.config' import { StartActiveSpan } from '../infrastructure/telemetry/telemetry.decorator' import { VaultError, VaultHttpClientService } from './vault-http-client.service' -import { generateGitlabMirrorCredPath, generateSecretGroupPath, generateSonarqubeCredPath, generateTechReadOnlyCredPath, isVaultNotFound } from './vault.utils' +import { generateAppRoleSecretIdPath, generateGitlabMirrorCredPath, generateSecretGroupPath, generateSonarqubeCredPath, generateTechReadOnlyCredPath, isVaultNotFound } from './vault.utils' export interface VaultSysPoliciesAclUpsertRequest { policy: string @@ -401,7 +401,13 @@ export class VaultClientService { } @StartActiveSpan() - async createAuthApproleRoleSecretId(roleName: string) { + async ensureAuthApproleRoleSecretId(roleName: string) { + const kvPath = generateAppRoleSecretIdPath(this.baseConfig.projectsRootDir, roleName) + const existing = await this.read<{ secret_id: string }>(kvPath).catch(() => null) + if (existing?.data?.secret_id) { + this.logger.verbose(`Reusing Vault AppRole secret-id for ${roleName}`) + return existing.data.secret_id + } const path = `auth/approle/role/${roleName}/secret-id` this.logger.verbose(`Creating Vault AppRole secret-id for ${roleName}`) const response = await this.http.fetch(path, { method: 'POST' }) @@ -409,6 +415,7 @@ export class VaultClientService { if (!secretId) { throw new VaultError('InvalidResponse', `Vault secret-id not generated for role ${roleName}`, { method: 'POST', path }) } + await this.write({ secret_id: secretId }, kvPath) return secretId } diff --git a/apps/server-nestjs/src/modules/vault/vault.utils.ts b/apps/server-nestjs/src/modules/vault/vault.utils.ts index c6b1782673..72d2700cb6 100644 --- a/apps/server-nestjs/src/modules/vault/vault.utils.ts +++ b/apps/server-nestjs/src/modules/vault/vault.utils.ts @@ -27,3 +27,7 @@ export function isVaultNotFound(error: unknown): error is VaultError { export function isVaultBadRequest(error: unknown): error is VaultError { return error instanceof VaultError && error.kind === 'HttpError' && error.status === 400 } + +export function generateAppRoleSecretIdPath(projectRootDir: string, projectSlug: string) { + return `${generateProjectPath(projectRootDir, projectSlug)}/APPROLE_SECRET_ID` +}