From 9146783f1611d2b8b0f046315707c7c38d9a213c Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Fri, 28 Aug 2026 15:25:57 +0200 Subject: [PATCH 1/2] refactor(server-nestjs): uniformize error type guards in module utils Signed-off-by: William Phetsinorath Change-Id: Iff465b31836b84656a084678aeec082a6a6a6964 --- .../modules/gitlab/gitlab-client.service.ts | 5 ++-- .../src/modules/gitlab/gitlab.utils.ts | 4 ++++ .../src/modules/nexus/nexus-client.service.ts | 23 ++++++++++--------- .../src/modules/nexus/nexus.service.ts | 6 ++--- .../src/modules/nexus/nexus.utils.spec.ts | 15 +++++++++++- .../src/modules/nexus/nexus.utils.ts | 5 ++++ .../src/modules/registry/registry.service.ts | 4 ++-- .../src/modules/vault/vault-client.service.ts | 16 ++++++------- .../src/modules/vault/vault.service.ts | 14 +++++------ .../src/modules/vault/vault.utils.spec.ts | 17 +++++++++++++- .../src/modules/vault/vault.utils.ts | 10 ++++++++ 11 files changed, 83 insertions(+), 36 deletions(-) diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts b/apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts index ed6f71a606..57305c057d 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts @@ -18,7 +18,6 @@ import type { import type { ConfigType } from '@nestjs/config' import { join } from 'node:path' import { defaultBranchName } from '@cpn-console/shared' -import { GitbeakerRequestError } from '@gitbeaker/requester-utils' import { Gitlab as GitlabRest } from '@gitbeaker/rest' import { Inject, Injectable, Logger } from '@nestjs/common' import { gitlabConfigFactory } from '../../config/gitlab.config' @@ -36,7 +35,7 @@ import { TOPIC_SYSTEM_MANAGED, USER_ID_CUSTOM_ATTRIBUTE_KEY, } from './gitlab.constants' -import { generateGitlabCIConfigContent, generateMirrorScriptContent, hasFileContentChanged, hasGitbeakerCause, isGitbeakerNotFound } from './gitlab.utils' +import { generateGitlabCIConfigContent, generateMirrorScriptContent, hasFileContentChanged, hasGitbeakerCause, isGitbeakerNotFound, isGitbeakerUnauthorized } from './gitlab.utils' export const GITLAB_REST_CLIENT = Symbol('GITLAB_REST_CLIENT') @@ -638,7 +637,7 @@ export class GitlabClientService { const self = await client.PersonalAccessTokens.show() return self.active && !self.revoked } catch (error) { - if (error instanceof GitbeakerRequestError && error.cause?.response.status === 401) return false + if (isGitbeakerUnauthorized(error)) return false throw error } } diff --git a/apps/server-nestjs/src/modules/gitlab/gitlab.utils.ts b/apps/server-nestjs/src/modules/gitlab/gitlab.utils.ts index d5699f039c..be748a6f7e 100644 --- a/apps/server-nestjs/src/modules/gitlab/gitlab.utils.ts +++ b/apps/server-nestjs/src/modules/gitlab/gitlab.utils.ts @@ -259,3 +259,7 @@ export function hasGitbeakerCause(error: unknown, pattern: string | RegExp): err : JSON.stringify(error.cause?.description ?? '') return typeof pattern === 'string' ? description.includes(pattern) : pattern.test(description) } + +export function isGitbeakerUnauthorized(error: unknown): error is GitbeakerRequestError { + return error instanceof GitbeakerRequestError && error.cause?.response?.status === 401 +} diff --git a/apps/server-nestjs/src/modules/nexus/nexus-client.service.ts b/apps/server-nestjs/src/modules/nexus/nexus-client.service.ts index 85f9e81d62..d940eb389d 100644 --- a/apps/server-nestjs/src/modules/nexus/nexus-client.service.ts +++ b/apps/server-nestjs/src/modules/nexus/nexus-client.service.ts @@ -1,6 +1,7 @@ import { Inject, Injectable } from '@nestjs/common' import { StartActiveSpan } from '../infrastructure/telemetry/telemetry.decorator' -import { NexusError, NexusHttpClientService } from './nexus-http-client.service' +import { NexusHttpClientService } from './nexus-http-client.service' +import { isNexusNotFound } from './nexus.utils' interface NexusRepositoryStorage { blobStoreName: string @@ -114,7 +115,7 @@ export class NexusClientService { const res = await this.http.fetch(`repositories/maven/hosted/${name}`) return res.data } catch (error) { - if (error instanceof NexusError && error.status === 404) return null + if (isNexusNotFound(error)) return null throw error } } @@ -145,7 +146,7 @@ export class NexusClientService { const res = await this.http.fetch(`repositories/maven/group/${name}`) return res.data } catch (error) { - if (error instanceof NexusError && error.status === 404) return null + if (isNexusNotFound(error)) return null throw error } } @@ -156,7 +157,7 @@ export class NexusClientService { const res = await this.http.fetch(`repositories/npm/hosted/${name}`) return res.data } catch (error) { - if (error instanceof NexusError && error.status === 404) return null + if (isNexusNotFound(error)) return null throw error } } @@ -177,7 +178,7 @@ export class NexusClientService { const res = await this.http.fetch(`repositories/npm/group/${name}`) return res.data } catch (error) { - if (error instanceof NexusError && error.status === 404) return null + if (isNexusNotFound(error)) return null throw error } } @@ -198,7 +199,7 @@ export class NexusClientService { const res = await this.http.fetch(`security/privileges/${name}`) return res.data } catch (error) { - if (error instanceof NexusError && error.status === 404) return null + if (isNexusNotFound(error)) return null throw error } } @@ -218,7 +219,7 @@ export class NexusClientService { try { await this.http.fetch(`security/privileges/${name}`, { method: 'DELETE' }) } catch (error) { - if (error instanceof NexusError && error.status === 404) return + if (isNexusNotFound(error)) return throw error } } @@ -229,7 +230,7 @@ export class NexusClientService { const res = await this.http.fetch(`security/roles/${id}`) return res.data } catch (error) { - if (error instanceof NexusError && error.status === 404) return null + if (isNexusNotFound(error)) return null throw error } } @@ -249,7 +250,7 @@ export class NexusClientService { try { await this.http.fetch(`security/roles/${id}`, { method: 'DELETE' }) } catch (error) { - if (error instanceof NexusError && error.status === 404) return + if (isNexusNotFound(error)) return throw error } } @@ -280,7 +281,7 @@ export class NexusClientService { try { await this.http.fetch(`security/users/${userId}`, { method: 'DELETE' }) } catch (error) { - if (error instanceof NexusError && error.status === 404) return + if (isNexusNotFound(error)) return throw error } } @@ -290,7 +291,7 @@ export class NexusClientService { try { await this.http.fetch(`repositories/${name}`, { method: 'DELETE' }) } catch (error) { - if (error instanceof NexusError && error.status === 404) return + if (isNexusNotFound(error)) return throw error } } diff --git a/apps/server-nestjs/src/modules/nexus/nexus.service.ts b/apps/server-nestjs/src/modules/nexus/nexus.service.ts index 2c9585e7b3..be45fbc556 100644 --- a/apps/server-nestjs/src/modules/nexus/nexus.service.ts +++ b/apps/server-nestjs/src/modules/nexus/nexus.service.ts @@ -14,7 +14,7 @@ import { nexusConfigFactory } from '../../config/nexus.config' import { StartActiveSpan } from '../infrastructure/telemetry/telemetry.decorator' import { capturePluginResult } from '../plugin/plugin.utils' import { VaultClientService } from '../vault/vault-client.service' -import { VaultError } from '../vault/vault-http-client.service' +import { isVaultNotFound } from '../vault/vault.utils' import { NexusClientService } from './nexus-client.service' import { NexusDatastoreService } from './nexus-datastore.service' import { @@ -437,7 +437,7 @@ export class NexusService { try { existingPassword = await this.vault.read(vaultPath).then(res => res.data?.NEXUS_PASSWORD) } catch (error) { - if (error instanceof VaultError && error.kind === 'NotFound') { + if (isVaultNotFound(error)) { existingPassword = undefined } else { throw error @@ -581,7 +581,7 @@ export class NexusService { try { await this.vault.delete(vaultPath) } catch (error) { - if (error instanceof VaultError && error.kind === 'NotFound') return + if (isVaultNotFound(error)) return throw error } } diff --git a/apps/server-nestjs/src/modules/nexus/nexus.utils.spec.ts b/apps/server-nestjs/src/modules/nexus/nexus.utils.spec.ts index 48ccaa36eb..87c951caef 100644 --- a/apps/server-nestjs/src/modules/nexus/nexus.utils.spec.ts +++ b/apps/server-nestjs/src/modules/nexus/nexus.utils.spec.ts @@ -1,8 +1,21 @@ import { describe, expect, it } from 'vitest' -import { generateNexusCredPath } from './nexus.utils' +import { NexusError } from './nexus-http-client.service' +import { generateNexusCredPath, isNexusNotFound } from './nexus.utils' describe('nexus path helpers', () => { it('scopes the NEXUS credentials to the project', () => { expect(generateNexusCredPath('forge', 'my-project')).toBe('forge/my-project/NEXUS') }) }) + +describe('isNexusNotFound', () => { + it('matches a 404 NexusError', () => { + expect(isNexusNotFound(new NexusError('HttpError', 'not found', { status: 404 }))).toBe(true) + }) + + it('rejects a non-404 NexusError and non-Nexus errors', () => { + expect(isNexusNotFound(new NexusError('HttpError', 'conflict', { status: 409 }))).toBe(false) + expect(isNexusNotFound(new Error('boom'))).toBe(false) + expect(isNexusNotFound(null)).toBe(false) + }) +}) diff --git a/apps/server-nestjs/src/modules/nexus/nexus.utils.ts b/apps/server-nestjs/src/modules/nexus/nexus.utils.ts index abc04880d2..4d3ed39661 100644 --- a/apps/server-nestjs/src/modules/nexus/nexus.utils.ts +++ b/apps/server-nestjs/src/modules/nexus/nexus.utils.ts @@ -1,5 +1,6 @@ import type { ProjectWithDetails } from './nexus-datastore.service' import { randomBytes } from 'node:crypto' +import { NexusError } from './nexus-http-client.service' export function getPluginConfig(project: ProjectWithDetails, key: string) { return project.plugins?.find(p => p.key === key)?.value @@ -23,3 +24,7 @@ export function generateMavenHostedRepoName(project: ProjectWithDetails, kind: M export function generateNpmHostedRepoName(project: ProjectWithDetails) { return `${project.slug}-npm` } + +export function isNexusNotFound(error: unknown): error is NexusError { + return error instanceof NexusError && error.status === 404 +} diff --git a/apps/server-nestjs/src/modules/registry/registry.service.ts b/apps/server-nestjs/src/modules/registry/registry.service.ts index 08f17eba7e..0c9aa6bef1 100644 --- a/apps/server-nestjs/src/modules/registry/registry.service.ts +++ b/apps/server-nestjs/src/modules/registry/registry.service.ts @@ -22,7 +22,7 @@ import { find } from '../../utils/iterable.utils' import { StartActiveSpan } from '../infrastructure/telemetry/telemetry.decorator' import { capturePluginResult } from '../plugin/plugin.utils' import { VaultClientService } from '../vault/vault-client.service' -import { VaultError } from '../vault/vault-http-client.service' +import { isVaultNotFound } from '../vault/vault.utils' import { RegistryClientService, roAccess, rwAccess } from './registry-client.service' import { RegistryDatastoreService } from './registry-datastore.service' import { @@ -106,7 +106,7 @@ export class RegistryService { const relativeVaultPath = `REGISTRY/${robotName}` const vaultPath = getProjectVaultPath(project, this.baseConfig.projectsRootDir, relativeVaultPath) const vaultRobotSecret = await this.vault.read(vaultPath).catch((error) => { - if (error instanceof VaultError && error.kind === 'NotFound') return null + if (isVaultNotFound(error)) return null throw error }) 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 a538291036..18fb021913 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 } from './vault.utils' +import { generateGitlabMirrorCredPath, generateSecretGroupPath, generateSonarqubeCredPath, generateTechReadOnlyCredPath, isVaultNotFound } from './vault.utils' export interface VaultSysPoliciesAclUpsertRequest { policy: string @@ -216,7 +216,7 @@ export class VaultClientService { span?.setAttribute('vault.kv.path', vaultCredsPath) this.logger.verbose(`Reading Vault GitLab mirror credentials (projectSlug=${projectSlug}, repoName=${repoName})`) return await this.read>(vaultCredsPath).catch((error) => { - if (error instanceof VaultError && error.kind === 'NotFound') return null + if (isVaultNotFound(error)) return null throw error }) } @@ -241,7 +241,7 @@ export class VaultClientService { span?.setAttribute('vault.kv.path', vaultCredsPath) this.logger.verbose(`Deleting Vault GitLab mirror credentials (projectSlug=${projectSlug}, repoName=${repoName})`) await this.delete(vaultCredsPath).catch((error) => { - if (error instanceof VaultError && error.kind === 'NotFound') return + if (isVaultNotFound(error)) return throw error }) } @@ -253,7 +253,7 @@ export class VaultClientService { span?.setAttribute('project.slug', projectSlug) span?.setAttribute('vault.kv.path', vaultPath) return await this.read(vaultPath).catch((error) => { - if (error instanceof VaultError && error.kind === 'NotFound') return null + if (isVaultNotFound(error)) return null throw error }) } @@ -275,7 +275,7 @@ export class VaultClientService { span?.setAttribute('vault.kv.path', vaultPath) this.logger.verbose(`Reading Vault SonarQube user credentials (projectSlug=${projectSlug})`) return await this.read(vaultPath).catch((error) => { - if (error instanceof VaultError && error.kind === 'NotFound') return null + if (isVaultNotFound(error)) return null throw error }) } @@ -298,7 +298,7 @@ export class VaultClientService { span?.setAttribute('vault.kv.path', vaultPath) this.logger.verbose(`Deleting Vault SonarQube user credentials (projectSlug=${projectSlug})`) await this.delete(vaultPath).catch((error) => { - if (error instanceof VaultError && error.kind === 'NotFound') return + if (isVaultNotFound(error)) return throw error }) } @@ -321,7 +321,7 @@ export class VaultClientService { try { await this.http.fetch(`${kvName}/metadata/${path}`, { method: 'DELETE' }) } catch (error) { - if (error instanceof VaultError && error.kind === 'NotFound') return + if (isVaultNotFound(error)) return throw error } } @@ -339,7 +339,7 @@ export class VaultClientService { } return response.data.keys } catch (error) { - if (error instanceof VaultError && error.kind === 'NotFound') return [] + if (isVaultNotFound(error)) return [] throw error } } diff --git a/apps/server-nestjs/src/modules/vault/vault.service.ts b/apps/server-nestjs/src/modules/vault/vault.service.ts index 5169984e42..a63200cef8 100644 --- a/apps/server-nestjs/src/modules/vault/vault.service.ts +++ b/apps/server-nestjs/src/modules/vault/vault.service.ts @@ -36,7 +36,7 @@ import { PROJECT_SECURITY_GROUP_PATH_SUFFIX_PLUGIN_KEY, SECURITY_GROUP_PATH_PLUGIN_KEY, } from './vault.constants' -import { generateProjectPath } from './vault.utils' +import { generateProjectPath, isVaultBadRequest, isVaultNotFound } from './vault.utils' type ProjectScope = 'admin' | 'devops' | 'developer' | 'readonly' | 'security' @@ -223,7 +223,7 @@ export class VaultService { await this.client.createSysMount(kvName, createBody) this.logger.log(`Created Vault mount ${kvName}`) } catch (error) { - if (error instanceof VaultError && error.kind === 'HttpError' && error.status === 400) { + if (isVaultBadRequest(error)) { await this.client.tuneSysMount(kvName, tuneBody) this.logger.log(`Vault mount ${kvName} already existed, so it was tuned to the expected settings`) return @@ -237,7 +237,7 @@ export class VaultService { await this.client.deleteSysMounts(kvName) this.logger.log(`Deleted Vault mount ${kvName}`) } catch (error) { - if (error instanceof VaultError && error.kind === 'NotFound') { + if (isVaultNotFound(error)) { this.logger.warn(`Vault mount ${kvName} was already missing`) return } @@ -279,7 +279,7 @@ export class VaultService { for (const result of settled) { if (result.status !== 'rejected') continue const error = result.reason - if (error instanceof VaultError && error.kind === 'NotFound') continue + if (isVaultNotFound(error)) continue throw error } } @@ -381,7 +381,7 @@ export class VaultService { for (const result of settled) { if (result.status !== 'rejected') continue const error = result.reason - if (error instanceof VaultError && error.kind === 'NotFound') continue + if (isVaultNotFound(error)) continue throw error } } @@ -423,7 +423,7 @@ export class VaultService { canonical_id: groupResult.data.id, }) } catch (error) { - if (error instanceof VaultError && error.kind === 'HttpError' && error.status === 400) return + if (isVaultBadRequest(error)) return throw error } } @@ -520,7 +520,7 @@ export class VaultService { try { await this.client.delete(fullPath) } catch (error) { - if (error instanceof VaultError && error.kind === 'NotFound') return + if (isVaultNotFound(error)) return throw error } })) diff --git a/apps/server-nestjs/src/modules/vault/vault.utils.spec.ts b/apps/server-nestjs/src/modules/vault/vault.utils.spec.ts index 12e0938b6a..bf10dc54fc 100644 --- a/apps/server-nestjs/src/modules/vault/vault.utils.spec.ts +++ b/apps/server-nestjs/src/modules/vault/vault.utils.spec.ts @@ -1,8 +1,23 @@ import { describe, expect, it } from 'vitest' -import { generateSecretGroupPath } from './vault.utils' +import { VaultError } from './vault-http-client.service' +import { generateSecretGroupPath, isVaultBadRequest, isVaultNotFound } from './vault.utils' describe('vault path helpers', () => { it('scopes a group to the project path', () => { expect(generateSecretGroupPath('forge', 'my-project', 'GITLAB')).toBe('forge/my-project/GITLAB') }) }) + +describe('vault error guards', () => { + it('isVaultNotFound matches a NotFound VaultError', () => { + expect(isVaultNotFound(new VaultError('NotFound', 'missing'))).toBe(true) + expect(isVaultNotFound(new VaultError('HttpError', 'conflict', { status: 409 }))).toBe(false) + expect(isVaultNotFound(new Error('boom'))).toBe(false) + }) + + it('isVaultBadRequest matches a 400 HttpError VaultError', () => { + expect(isVaultBadRequest(new VaultError('HttpError', 'bad request', { status: 400 }))).toBe(true) + expect(isVaultBadRequest(new VaultError('HttpError', 'conflict', { status: 409 }))).toBe(false) + expect(isVaultBadRequest(new VaultError('NotFound', 'missing'))).toBe(false) + }) +}) diff --git a/apps/server-nestjs/src/modules/vault/vault.utils.ts b/apps/server-nestjs/src/modules/vault/vault.utils.ts index 6b86e20d03..c6b1782673 100644 --- a/apps/server-nestjs/src/modules/vault/vault.utils.ts +++ b/apps/server-nestjs/src/modules/vault/vault.utils.ts @@ -1,3 +1,5 @@ +import { VaultError } from './vault-http-client.service' + export function generateProjectPath(projectRootDir: string, projectSlug: string) { return `${projectRootDir}/${projectSlug}` } @@ -17,3 +19,11 @@ export function generateSonarqubeCredPath(projectRootDir: string, projectSlug: s export function generateSecretGroupPath(projectRootDir: string, projectSlug: string, group: string): string { return `${generateProjectPath(projectRootDir, projectSlug)}/${group}` } + +export function isVaultNotFound(error: unknown): error is VaultError { + return error instanceof VaultError && error.kind === 'NotFound' +} + +export function isVaultBadRequest(error: unknown): error is VaultError { + return error instanceof VaultError && error.kind === 'HttpError' && error.status === 400 +} From e61aa06630126d7c768ac66080c0e66819562e0f Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Fri, 28 Aug 2026 12:41:21 +0200 Subject: [PATCH 2/2] 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` +}