fix(server-nestjs): make GitLab CI variable provisioning idempotent

apps/server-nestjs/src/modules/gitlab/gitlab-client.service.spec.ts

@@ -1044,51 +1044,100 @@ describe('gitlab-client', () => {
     it('should create a missing group variable', async () => {
       gitlabApi.GroupVariables.show.mockRejectedValueOnce(makeGitbeakerRequestError({ description: '404 Group variable Not Found', status: 404 }))
 
-      await service.setGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'secret', { masked: true, protected: false, variableType: 'env_var' })
+      await service.ensureGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'secret', { masked: true, protected: false, variableType: 'env_var' })
 
       expect(gitlabApi.GroupVariables.create).toHaveBeenCalledWith(groupId, 'SONAR_TOKEN', 'secret', expect.objectContaining({ masked: true, variableType: 'env_var' }))
       expect(gitlabApi.GroupVariables.edit).not.toHaveBeenCalled()
     })
 
     it('should update a group variable when value differs', async () => {
+      gitlabApi.GroupVariables.create.mockRejectedValueOnce(makeGitbeakerRequestError({ description: { key: ['(SONAR_TOKEN) has already been taken'] }, status: 400 }))
       gitlabApi.GroupVariables.show.mockResolvedValueOnce({ key: 'SONAR_TOKEN', value: 'old', variable_type: 'env_var', masked: true, protected: false })
 
-      await service.setGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'new', { masked: true, protected: false, variableType: 'env_var' })
+      await service.ensureGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'new', { masked: true, protected: false, variableType: 'env_var' })
 
       expect(gitlabApi.GroupVariables.edit).toHaveBeenCalledWith(groupId, 'SONAR_TOKEN', 'new', expect.objectContaining({ variableType: 'env_var' }))
-      expect(gitlabApi.GroupVariables.create).not.toHaveBeenCalled()
     })
 
     it('should skip write when group variable matches', async () => {
+      gitlabApi.GroupVariables.create.mockRejectedValueOnce(makeGitbeakerRequestError({ description: { key: ['(SONAR_TOKEN) has already been taken'] }, status: 400 }))
       gitlabApi.GroupVariables.show.mockResolvedValueOnce({ key: 'SONAR_TOKEN', value: 'secret', variable_type: 'env_var', masked: true, protected: false })
 
-      await service.setGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'secret', { masked: true, protected: false, variableType: 'env_var' })
+      await service.ensureGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'secret', { masked: true, protected: false, variableType: 'env_var' })
 
-      expect(gitlabApi.GroupVariables.create).not.toHaveBeenCalled()
+      expect(gitlabApi.GroupVariables.create).toHaveBeenCalledTimes(1)
       expect(gitlabApi.GroupVariables.edit).not.toHaveBeenCalled()
     })
 
     it('should create a missing repo variable', async () => {
       gitlabApi.ProjectVariables.show.mockRejectedValueOnce(makeGitbeakerRequestError({ description: '404 Project variable Not Found', status: 404 }))
 
-      await service.setGitlabRepoVariable(repoId, 'PROJECT_KEY', 'key', { masked: false, protected: false, variableType: 'env_var', environmentScope: '*' })
+      await service.ensureGitlabRepoVariable(repoId, 'PROJECT_KEY', 'key', { masked: false, protected: false, variableType: 'env_var', environmentScope: '*' })
 
       expect(gitlabApi.ProjectVariables.create).toHaveBeenCalledWith(repoId, 'PROJECT_KEY', 'key', expect.objectContaining({ variableType: 'env_var', environmentScope: '*' }))
     })
 
     it('should skip write when repo variable matches', async () => {
+      gitlabApi.ProjectVariables.create.mockRejectedValueOnce(makeGitbeakerRequestError({ description: { key: ['(PROJECT_KEY) has already been taken'] }, status: 400 }))
       gitlabApi.ProjectVariables.show.mockResolvedValueOnce({ key: 'PROJECT_KEY', value: 'key', variable_type: 'env_var', masked: false, protected: false, environment_scope: '*' })
 
-      await service.setGitlabRepoVariable(repoId, 'PROJECT_KEY', 'key', { masked: false, protected: false, variableType: 'env_var', environmentScope: '*' })
+      await service.ensureGitlabRepoVariable(repoId, 'PROJECT_KEY', 'key', { masked: false, protected: false, variableType: 'env_var', environmentScope: '*' })
 
-      expect(gitlabApi.ProjectVariables.create).not.toHaveBeenCalled()
+      expect(gitlabApi.ProjectVariables.create).toHaveBeenCalledTimes(1)
+      expect(gitlabApi.ProjectVariables.edit).not.toHaveBeenCalled()
     })
 
+    it('should tolerate a create collision for a repo variable and no-op when it already matches (race)', async () => {
+      // create() is rejected because the variable already exists; the reload finds it
+      // and the method must not throw (idempotent reconcile, no-op since it matches).
+      gitlabApi.ProjectVariables.create.mockRejectedValueOnce(makeGitbeakerRequestError({ description: { key: ['(PROJECT_KEY) has already been taken'] }, status: 400 }))
+      gitlabApi.ProjectVariables.show.mockResolvedValueOnce({ key: 'PROJECT_KEY', value: 'key', variable_type: 'env_var', masked: false, protected: false, environment_scope: '*' })
+
+      await service.ensureGitlabRepoVariable(repoId, 'PROJECT_KEY', 'key', { masked: false, protected: false, variableType: 'env_var', environmentScope: '*' })
+
+      expect(gitlabApi.ProjectVariables.create).toHaveBeenCalledTimes(1)
+      expect(gitlabApi.ProjectVariables.edit).not.toHaveBeenCalled()
+    })
+
+    it('should tolerate a create collision for a repo variable and edit when the existing value differs (race)', async () => {
+      gitlabApi.ProjectVariables.create.mockRejectedValueOnce(makeGitbeakerRequestError({ description: { key: ['(PROJECT_KEY) has already been taken'] }, status: 400 }))
+      gitlabApi.ProjectVariables.show.mockResolvedValueOnce({ key: 'PROJECT_KEY', value: 'old', variable_type: 'env_var', masked: false, protected: false, environment_scope: '*' })
+
+      await service.ensureGitlabRepoVariable(repoId, 'PROJECT_KEY', 'new', { masked: false, protected: false, variableType: 'env_var', environmentScope: '*' })
+
+      expect(gitlabApi.ProjectVariables.create).toHaveBeenCalledTimes(1)
+      expect(gitlabApi.ProjectVariables.edit).toHaveBeenCalledWith(repoId, 'PROJECT_KEY', 'new', expect.objectContaining({ variableType: 'env_var', environmentScope: '*' }))
+    })
+
+    it('should propagate a non-collision create error for a repo variable', async () => {
+      gitlabApi.ProjectVariables.create.mockRejectedValueOnce(makeGitbeakerRequestError({ description: 'Internal Server Error', status: 500 }))
+
+      await expect(service.ensureGitlabRepoVariable(repoId, 'PROJECT_KEY', 'key', { masked: false, protected: false, variableType: 'env_var', environmentScope: '*' }))
+        .rejects.toThrow(GitbeakerRequestError)
+    })
+
+    it('should tolerate a create collision for a group variable and no-op when it already matches (race)', async () => {
+      gitlabApi.GroupVariables.create.mockRejectedValueOnce(makeGitbeakerRequestError({ description: { key: ['(SONAR_TOKEN) has already been taken'] }, status: 400 }))
+      gitlabApi.GroupVariables.show.mockResolvedValueOnce({ key: 'SONAR_TOKEN', value: 'secret', variable_type: 'env_var', masked: true, protected: false })
+
+      await service.ensureGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'secret', { masked: true, protected: false, variableType: 'env_var' })
+
+      expect(gitlabApi.GroupVariables.create).toHaveBeenCalledTimes(1)
+      expect(gitlabApi.GroupVariables.edit).not.toHaveBeenCalled()
+    })
+
+    it('should propagate a non-collision create error for a group variable', async () => {
+      gitlabApi.GroupVariables.create.mockRejectedValueOnce(makeGitbeakerRequestError({ description: 'Internal Server Error', status: 500 }))
+
+      await expect(service.ensureGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'secret', { masked: true, protected: false, variableType: 'env_var' }))
+        .rejects.toThrow(GitbeakerRequestError)
+    })
     it('should tolerate a non-string cause.description (regression: .includes is not a function)', async () => {
       const error = new GitbeakerRequestError('boom', { cause: { description: 404 as unknown as string, request: new Request('https://gitlab.internal.example/api'), response: new Response(null, { status: 404 }) } })
-      gitlabApi.GroupVariables.show.mockRejectedValueOnce(error)
+      gitlabApi.GroupVariables.create.mockRejectedValueOnce(error)
 
-      await service.setGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'secret', { masked: true, protected: false, variableType: 'env_var' })
+      await expect(service.ensureGitlabGroupVariable(groupId, 'SONAR_TOKEN', 'secret', { masked: true, protected: false, variableType: 'env_var' }))
+        .rejects.toThrow(GitbeakerRequestError)
 
       expect(gitlabApi.GroupVariables.create).toHaveBeenCalledWith(groupId, 'SONAR_TOKEN', 'secret', expect.objectContaining({ masked: true, variableType: 'env_var' }))
     })

apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts

@@ -495,25 +495,32 @@ export class GitlabClientService {
   }
 
   // CI Variables
-  public async setGitlabGroupVariable(
+  public async ensureGitlabGroupVariable(
     groupId: number,
     key: string,
     value: string,
     options: { masked: boolean, protected: boolean, variableType: VariableType },
   ): Promise<void> {
-    const current = await this.client.GroupVariables.show(groupId, key).catch((error) => {
-      if (isGitbeakerNotFound(error)) return undefined
-      throw error
-    })
-    if (!current) {
+    try {
       await this.client.GroupVariables.create(groupId, key, value, {
         variableType: options.variableType,
         masked: options.masked,
         protected: options.protected,
       })
       return
+    } catch (error) {
+      // GitLab enforces (key, environment_scope) uniqueness; a concurrent or prior
+      // reconcile may have created the variable between our read and this create.
+      // Tolerate it like every other ensure* writer: re-read once and reconcile
+      // instead of failing the whole sync.
+      if (!hasGitbeakerCause(error, 'has already been taken')) throw error
+      this.logger.warn(`GitLab group variable already exists (race); reloading (groupId=${groupId}, key=${key})`)
     }
-    if (current.masked === options.masked
+    const current = await this.client.GroupVariables.show(groupId, key).catch((error) => {
+      if (isGitbeakerNotFound(error)) return undefined
+      throw error
+    })
+    if (current?.masked === options.masked
       && current.value === value
       && current.protected === options.protected
       && current.variable_type === options.variableType) {
@@ -527,26 +534,33 @@ export class GitlabClientService {
     })
   }
 
-  public async setGitlabRepoVariable(
+  public async ensureGitlabRepoVariable(
     repoId: number,
     key: string,
     value: string,
     options: { masked: boolean, protected: boolean, variableType: VariableType, environmentScope: string },
   ): Promise<void> {
-    const current = await this.client.ProjectVariables.show(repoId, key, { filter: { environment_scope: options.environmentScope } }).catch((error) => {
-      if (isGitbeakerNotFound(error)) return undefined
-      throw error
-    })
-    if (!current) {
+    try {
       await this.client.ProjectVariables.create(repoId, key, value, {
         variableType: options.variableType,
         masked: options.masked,
         protected: options.protected,
         environmentScope: options.environmentScope,
       })
       return
+    } catch (error) {
+      // GitLab enforces (key, environment_scope) uniqueness; a concurrent or prior
+      // reconcile may have created the variable between our read and this create.
+      // Tolerate it like every other ensure* writer: re-read once and reconcile
+      // instead of failing the whole sync.
+      if (!hasGitbeakerCause(error, 'has already been taken')) throw error
+      this.logger.warn(`GitLab repo variable already exists (race); reloading (repoId=${repoId}, key=${key})`)
     }
-    if (current.masked === options.masked
+    const current = await this.client.ProjectVariables.show(repoId, key, { filter: { environment_scope: options.environmentScope } }).catch((error) => {
+      if (isGitbeakerNotFound(error)) return undefined
+      throw error
+    })
+    if (current?.masked === options.masked
       && current.value === value
       && current.protected === options.protected
       && current.variable_type === options.variableType) {

apps/server-nestjs/src/modules/sonarqube/sonarqube.service.spec.ts

@@ -60,8 +60,8 @@ describe('sonarqubeService', () => {
     config = mockDeep<ConfigType<typeof sonarqubeConfigFactory>>({
     })
     gitlab = mockDeep<GitlabClientService>({
-      setGitlabGroupVariable: vi.fn().mockResolvedValue(undefined),
-      setGitlabRepoVariable: vi.fn().mockResolvedValue(undefined),
+      ensureGitlabGroupVariable: vi.fn().mockResolvedValue(undefined),
+      ensureGitlabRepoVariable: vi.fn().mockResolvedValue(undefined),
       getOrCreateProjectGroup: vi.fn().mockResolvedValue({ id: 42, full_path: 'root', name: 'root' }),
       getOrCreateProjectGroupRepo: vi.fn().mockResolvedValue({ id: 99 }),
     })
@@ -185,10 +185,10 @@ describe('sonarqubeService', () => {
       await service.handleUpsert(project)
 
       const key = generateProjectKey(project.slug, 'repo')
-      expect(gitlab.setGitlabRepoVariable).toHaveBeenCalledWith(repoId, 'PROJECT_KEY', key, expect.objectContaining({ masked: false, variableType: 'env_var', environmentScope: '*' }))
-      expect(gitlab.setGitlabRepoVariable).toHaveBeenCalledWith(repoId, 'PROJECT_NAME', `${project.slug}-repo`, expect.objectContaining({ masked: false, variableType: 'env_var', environmentScope: '*' }))
-      expect(gitlab.setGitlabRepoVariable).toHaveBeenCalledWith(repoId, 'SONAR_PROJECT_PROPERTIES', expect.stringContaining(`sonar.projectKey=${key}`), expect.objectContaining({ masked: false, variableType: 'file', environmentScope: '*' }))
-      expect(gitlab.setGitlabGroupVariable).toHaveBeenCalledWith(groupId, 'SONAR_TOKEN', 'tok', expect.objectContaining({ masked: true, variableType: 'env_var' }))
+      expect(gitlab.ensureGitlabRepoVariable).toHaveBeenCalledWith(repoId, 'PROJECT_KEY', key, expect.objectContaining({ masked: false, variableType: 'env_var', environmentScope: '*' }))
+      expect(gitlab.ensureGitlabRepoVariable).toHaveBeenCalledWith(repoId, 'PROJECT_NAME', `${project.slug}-repo`, expect.objectContaining({ masked: false, variableType: 'env_var', environmentScope: '*' }))
+      expect(gitlab.ensureGitlabRepoVariable).toHaveBeenCalledWith(repoId, 'SONAR_PROJECT_PROPERTIES', expect.stringContaining(`sonar.projectKey=${key}`), expect.objectContaining({ masked: false, variableType: 'file', environmentScope: '*' }))
+      expect(gitlab.ensureGitlabGroupVariable).toHaveBeenCalledWith(groupId, 'SONAR_TOKEN', 'tok', expect.objectContaining({ masked: true, variableType: 'env_var' }))
     })
 
     it('should not recreate user or write vault when both user and secret exist', async () => {

apps/server-nestjs/src/modules/sonarqube/sonarqube.service.ts

@@ -269,7 +269,7 @@ export class SonarqubeService implements OnModuleInit {
 
     // SONAR_TOKEN is shared across every repository of the project; expose it once at the group level.
     if (sonarSecret?.data?.SONAR_TOKEN) {
-      await this.gitlab.setGitlabGroupVariable(
+      await this.gitlab.ensureGitlabGroupVariable(
         gitlabGroup.id,
         'SONAR_TOKEN',
         sonarSecret.data.SONAR_TOKEN,
@@ -322,7 +322,7 @@ export class SonarqubeService implements OnModuleInit {
       variables.push({ key: 'SONAR_TOKEN', value: sonarSecret.data.SONAR_TOKEN, variableType: 'env_var', masked: true })
     }
     await Promise.all(variables.map(async (variable) => {
-      await this.gitlab.setGitlabRepoVariable(repo.id, variable.key, variable.value, {
+      await this.gitlab.ensureGitlabRepoVariable(repo.id, variable.key, variable.value, {
         masked: variable.masked,
         protected: false,
         variableType: variable.variableType,