fix(server-nestjs): make Keycloak root group creation idempotent

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
     }
   }

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
+}

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

@@ -370,4 +370,22 @@ describe('getOrCreateGroupByPath root resolution (issue #2518)', () => {
 
     expect(result).toMatchObject({ id: 'created-id', path: '/myproject' })
   })
+
+  it('should re-fetch the existing root group when a concurrent creation causes a 409', async () => {
+    // Two initial reads (getGroupByPath then the line-187 re-read) see no root
+    // group; the create then races a concurrent sync and 409s, after which the
+    // existing root group is re-fetched and returned (issue #2618).
+    server.use(
+      http.get(groupsUrl, () => HttpResponse.json([]), { once: true }),
+      http.get(groupsUrl, () => HttpResponse.json([]), { once: true }),
+      http.get(groupsUrl, () => HttpResponse.json([{ id: 'concurrent-root-id', name: 'myproject', path: '/myproject' }])),
+      http.post(groupsUrl, () =>
+        HttpResponse.json({ errorMessage: 'Top level groups must have unique names' }, { status: 409 })),
+      http.get(rootChildrenUrl, () => HttpResponse.json([])),
+    )
+
+    const result = await service.getOrCreateGroupByPath('/myproject')
+
+    expect(result).toMatchObject({ id: 'concurrent-root-id', path: '/myproject' })
+  })
 })

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

@@ -125,11 +125,22 @@ export class KeycloakClientService implements OnModuleInit {
   }
 
   @StartActiveSpan()
-  async createGroup(name: string) {
+  async ensureGroup(name: string) {
     const span = trace.getActiveSpan()
     span?.setAttribute('group.name', name)
     this.logger.debug(`Creating Keycloak group ${name}`)
-    await this.client.groups.create({ name })
+    try {
+      await this.client.groups.create({ name })
+    } catch (err) {
+      // A concurrent reconciliation (e.g. the cron sync and a project upsert)
+      // may have created the root group between the read and the create; treat
+      // the 409 as "already exists" and re-fetch it.
+      if (getErrorResponseStatus(err) !== 409) throw err
+      this.logger.verbose(`Keycloak group ${name} was created concurrently, fetching it`)
+      const existing = await this.getRootGroupByName(name)
+      if (!existing) throw err
+      return existing
+    }
     const created = await this.getRootGroupByName(name)
     if (!created) throw new Error(`Created Keycloak group ${name} but could not fetch it back`)
     return created
@@ -184,7 +195,7 @@ export class KeycloakClientService implements OnModuleInit {
     }
 
     const [rootName, ...rest] = parts
-    let current = await this.getRootGroupByName(rootName) ?? await this.createGroup(rootName)
+    let current = await this.getRootGroupByName(rootName) ?? await this.ensureGroup(rootName)
     for (const name of rest) {
       current = await this.getOrCreateSubGroupByName(current.id, name)
     }

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<NexusMavenHostedRepository>(`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<NexusMavenGroupRepository>(`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<NexusNpmHostedRepository>(`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<NexusNpmGroupRepository>(`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<NexusPrivilege>(`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<NexusRole>(`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
     }
   }

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
     }
   }

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)
+  })
+})

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
+}

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<VaultRobotSecret>(vaultPath).catch((error) => {
-      if (error instanceof VaultError && error.kind === 'NotFound') return null
+      if (isVaultNotFound(error)) return null
       throw error
     })
 

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<Partial<GitlabMirrorSecret>>(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<SonarqubeUserSecret>(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
     }
   }