diff --git a/apps/server-nestjs/src/modules/nexus/nexus-client.service.spec.ts b/apps/server-nestjs/src/modules/nexus/nexus-client.service.spec.ts index a47f2bc7a5..cb1ce89136 100644 --- a/apps/server-nestjs/src/modules/nexus/nexus-client.service.spec.ts +++ b/apps/server-nestjs/src/modules/nexus/nexus-client.service.spec.ts @@ -77,4 +77,61 @@ describe('nexusClientService', () => { await service.updateSecurityUsersChangePassword('u1', 'pw123') }) + + it('should re-fetch the existing role when ensureSecurityRoles hits a 409', async () => { + const role = { id: 'proj-role-id', name: 'proj-role-id', description: 'desc', privileges: ['nx-app'] } + server.use( + http.post(`${nexusUrl}/service/rest/v1/security/roles`, () => + HttpResponse.json({ errorMessage: 'Role already exists' }, { status: HttpStatus.CONFLICT })), + http.get(`${nexusUrl}/service/rest/v1/security/roles/:id`, () => HttpResponse.json(role)), + ) + + await expect(service.ensureSecurityRoles(role)).resolves.toEqual(role) + }) + + it('should rethrow non-collision errors from ensureSecurityRoles without re-fetching', async () => { + let fetches = 0 + server.use( + http.post(`${nexusUrl}/service/rest/v1/security/roles`, () => { + fetches++ + return HttpResponse.json({ errorMessage: 'Internal error' }, { status: HttpStatus.INTERNAL_SERVER_ERROR }) + }), + ) + + await expect(service.ensureSecurityRoles({ id: 'r', name: 'r', description: 'desc', privileges: [] })) + .rejects.toThrow('responded 500') + expect(fetches).toBe(1) + }) + + it('should re-fetch the existing repository when ensureRepositoriesMavenHosted hits a 400 already-exists', async () => { + const repo = { + name: 'proj-hosted', + online: true, + storage: { blobStoreName: 'default', strictContentTypeValidation: true, writePolicy: 'ALLOW' }, + component: { proprietaryComponents: true }, + maven: { versionPolicy: 'MIXED', layoutPolicy: 'STRICT', contentDisposition: 'ATTACHMENT' }, + } + server.use( + http.post(`${nexusUrl}/service/rest/v1/repositories/maven/hosted`, () => + new HttpResponse(null, { status: HttpStatus.BAD_REQUEST, statusText: 'Repository already exists' })), + http.get(`${nexusUrl}/service/rest/v1/repositories/maven/hosted/:name`, () => HttpResponse.json(repo)), + ) + + await expect(service.ensureRepositoriesMavenHosted(repo)).resolves.toEqual(repo) + }) + + it('should rethrow a 400 without an already-exists message from ensureRepositoriesMavenHosted', async () => { + server.use( + http.post(`${nexusUrl}/service/rest/v1/repositories/maven/hosted`, () => + new HttpResponse(null, { status: HttpStatus.BAD_REQUEST, statusText: 'Bad Request' })), + ) + + await expect(service.ensureRepositoriesMavenHosted({ + name: 'proj-hosted', + online: true, + storage: { blobStoreName: 'default', strictContentTypeValidation: true, writePolicy: 'ALLOW' }, + component: { proprietaryComponents: true }, + maven: { versionPolicy: 'MIXED', layoutPolicy: 'STRICT', contentDisposition: 'ATTACHMENT' }, + })).rejects.toThrow('responded 400') + }) }) 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 d940eb389d..17046c08dc 100644 --- a/apps/server-nestjs/src/modules/nexus/nexus-client.service.ts +++ b/apps/server-nestjs/src/modules/nexus/nexus-client.service.ts @@ -1,7 +1,7 @@ import { Inject, Injectable } from '@nestjs/common' import { StartActiveSpan } from '../infrastructure/telemetry/telemetry.decorator' import { NexusHttpClientService } from './nexus-http-client.service' -import { isNexusNotFound } from './nexus.utils' +import { ensure, isNexusNotFound } from './nexus.utils' interface NexusRepositoryStorage { blobStoreName: string @@ -121,8 +121,14 @@ export class NexusClientService { } @StartActiveSpan() - async createRepositoriesMavenHosted(body: NexusMavenHostedRepositoryUpsertRequest) { - await this.http.fetch('repositories/maven/hosted', { method: 'POST', body }) + async ensureRepositoriesMavenHosted(body: NexusMavenHostedRepositoryUpsertRequest): Promise { + return ensure({ + create: async () => { + await this.http.fetch('repositories/maven/hosted', { method: 'POST', body }) + return undefined + }, + reload: async () => await this.getRepositoriesMavenHosted(body.name) ?? undefined, + }) } @StartActiveSpan() @@ -131,8 +137,14 @@ export class NexusClientService { } @StartActiveSpan() - async createRepositoriesMavenGroup(body: NexusMavenGroupRepositoryUpsertRequest) { - await this.http.fetch('repositories/maven/group', { method: 'POST', body }) + async ensureRepositoriesMavenGroup(body: NexusMavenGroupRepositoryUpsertRequest): Promise { + return ensure({ + create: async () => { + await this.http.fetch('repositories/maven/group', { method: 'POST', body }) + return undefined + }, + reload: async () => await this.getRepositoriesMavenGroup(body.name) ?? undefined, + }) } @StartActiveSpan() @@ -163,8 +175,14 @@ export class NexusClientService { } @StartActiveSpan() - async createRepositoriesNpmHosted(body: NexusNpmHostedRepositoryUpsertRequest) { - await this.http.fetch('repositories/npm/hosted', { method: 'POST', body }) + async ensureRepositoriesNpmHosted(body: NexusNpmHostedRepositoryUpsertRequest): Promise { + return ensure({ + create: async () => { + await this.http.fetch('repositories/npm/hosted', { method: 'POST', body }) + return undefined + }, + reload: async () => await this.getRepositoriesNpmHosted(body.name) ?? undefined, + }) } @StartActiveSpan() @@ -184,8 +202,14 @@ export class NexusClientService { } @StartActiveSpan() - async postRepositoriesNpmGroup(body: NexusNpmGroupRepositoryUpsertRequest) { - await this.http.fetch('repositories/npm/group', { method: 'POST', body }) + async ensureRepositoriesNpmGroup(body: NexusNpmGroupRepositoryUpsertRequest): Promise { + return ensure({ + create: async () => { + await this.http.fetch('repositories/npm/group', { method: 'POST', body }) + return undefined + }, + reload: async () => await this.getRepositoriesNpmGroup(body.name) ?? undefined, + }) } @StartActiveSpan() @@ -205,8 +229,14 @@ export class NexusClientService { } @StartActiveSpan() - async createSecurityPrivilegesRepositoryView(body: NexusRepositoryViewPrivilegeUpsertRequest) { - await this.http.fetch('security/privileges/repository-view', { method: 'POST', body }) + async ensureSecurityPrivilegesRepositoryView(body: NexusRepositoryViewPrivilegeUpsertRequest): Promise { + return ensure({ + create: async () => { + await this.http.fetch('security/privileges/repository-view', { method: 'POST', body }) + return undefined + }, + reload: async () => await this.getSecurityPrivileges(body.name) ?? undefined, + }) } @StartActiveSpan() @@ -236,8 +266,14 @@ export class NexusClientService { } @StartActiveSpan() - async createSecurityRoles(body: NexusRoleCreateRequest) { - await this.http.fetch('security/roles', { method: 'POST', body }) + async ensureSecurityRoles(body: NexusRoleCreateRequest): Promise { + return ensure({ + create: async () => { + await this.http.fetch('security/roles', { method: 'POST', body }) + return undefined + }, + reload: async () => await this.getSecurityRoles(body.id) ?? undefined, + }) } @StartActiveSpan() @@ -272,8 +308,17 @@ export class NexusClientService { } @StartActiveSpan() - async createSecurityUsers(body: NexusUserCreateRequest) { - await this.http.fetch('security/users', { method: 'POST', body }) + async ensureSecurityUsers(body: NexusUserCreateRequest): Promise<{ userId: string } | undefined> { + return ensure({ + create: async () => { + await this.http.fetch('security/users', { method: 'POST', body }) + return undefined + }, + reload: async () => { + const users = await this.getSecurityUsers(body.userId) + return users.find(user => user.userId === body.userId) + }, + }) } @StartActiveSpan() diff --git a/apps/server-nestjs/src/modules/nexus/nexus.service.spec.ts b/apps/server-nestjs/src/modules/nexus/nexus.service.spec.ts index 7bb63a5097..62f7196221 100644 --- a/apps/server-nestjs/src/modules/nexus/nexus.service.spec.ts +++ b/apps/server-nestjs/src/modules/nexus/nexus.service.spec.ts @@ -84,7 +84,7 @@ describe('nexusService', () => { await service.handleUpsert(project) - expect(client.createRepositoriesMavenHosted).toHaveBeenCalled() + expect(client.ensureRepositoriesMavenHosted).toHaveBeenCalled() expect(client.deleteRepositoriesByName).toHaveBeenCalled() expect(vault.write).toHaveBeenCalledWith( expect.objectContaining({ @@ -113,7 +113,7 @@ describe('nexusService', () => { await service.handleCron() - expect(client.createSecurityUsers).toHaveBeenCalledTimes(2) + expect(client.ensureSecurityUsers).toHaveBeenCalledTimes(2) }) it('reuses existing vault password at the new path and does not rotate', async () => { @@ -134,7 +134,7 @@ describe('nexusService', () => { await service.handleUpsert(project) expect(client.updateSecurityUsersChangePassword).not.toHaveBeenCalled() - expect(client.createSecurityUsers).not.toHaveBeenCalled() + expect(client.ensureSecurityUsers).not.toHaveBeenCalled() expect(vault.write).toHaveBeenCalledWith(expect.objectContaining({ NEXUS_USERNAME: project.slug, NEXUS_PASSWORD: 'existing', @@ -162,7 +162,7 @@ describe('nexusService', () => { await service.handleUpsert(project) expect(client.updateSecurityUsersChangePassword).toHaveBeenCalledWith(project.slug, expect.any(String)) - expect(client.createSecurityUsers).not.toHaveBeenCalled() + expect(client.ensureSecurityUsers).not.toHaveBeenCalled() expect(vault.write).toHaveBeenCalledWith( expect.objectContaining({ NEXUS_USERNAME: project.slug, @@ -196,13 +196,13 @@ describe('nexusService', () => { }) datastore.getAllProjects.mockResolvedValue([project, staleProject]) - client.createSecurityRoles.mockImplementation(async (body) => { + client.ensureSecurityRoles.mockImplementation(async (body) => { if (body.id.startsWith('console-')) throw new Error('Request failed: POST security/roles responded 400 Bad Request') }) await expect(service.handleUpsert(project)).resolves.not.toThrow() - expect(client.createSecurityRoles).toHaveBeenCalledWith(expect.objectContaining({ id: 'console-admin' })) + expect(client.ensureSecurityRoles).toHaveBeenCalledWith(expect.objectContaining({ id: 'console-admin' })) }) it('dedupes project group roles by role id and keeps the highest privileges', async () => { @@ -223,7 +223,7 @@ describe('nexusService', () => { await service.handleUpsert(project) - expect(client.createSecurityRoles).toHaveBeenCalledWith(expect.objectContaining({ + expect(client.ensureSecurityRoles).toHaveBeenCalledWith(expect.objectContaining({ id: `${project.slug}-console-devops`, privileges: expect.arrayContaining([`${project.slug}-privilege-group`]), })) diff --git a/apps/server-nestjs/src/modules/nexus/nexus.service.ts b/apps/server-nestjs/src/modules/nexus/nexus.service.ts index be45fbc556..18f66cc728 100644 --- a/apps/server-nestjs/src/modules/nexus/nexus.service.ts +++ b/apps/server-nestjs/src/modules/nexus/nexus.service.ts @@ -170,7 +170,7 @@ export class NexusService { private async upsertPrivilege(body: NexusPrivilege) { const existing = await this.client.getSecurityPrivileges(body.name) if (!existing) { - await this.client.createSecurityPrivilegesRepositoryView(body) + await this.client.ensureSecurityPrivilegesRepositoryView(body) return } await this.client.updateSecurityPrivilegesRepositoryView(body.name, body) @@ -194,7 +194,7 @@ export class NexusService { }, } if (!existing) { - await this.client.createRepositoriesMavenHosted(body) + await this.client.ensureRepositoriesMavenHosted(body) return } await this.client.updateRepositoriesMavenHosted(repoName, body) @@ -213,7 +213,7 @@ export class NexusService { component: { proprietaryComponents: true }, } if (!existing) { - await this.client.createRepositoriesNpmHosted(body) + await this.client.ensureRepositoriesNpmHosted(body) return } await this.client.updateRepositoriesNpmHosted(repoName, body) @@ -233,7 +233,7 @@ export class NexusService { }, } if (!existing) { - await this.client.postRepositoriesNpmGroup(body) + await this.client.ensureRepositoriesNpmGroup(body) return } await this.client.putRepositoriesNpmGroup(repoName, body) @@ -317,7 +317,7 @@ export class NexusService { }, } if (!existing) { - await this.client.createRepositoriesMavenGroup(body) + await this.client.ensureRepositoriesMavenGroup(body) return } await this.client.updateRepositoriesMavenGroup(repoName, body) @@ -416,7 +416,7 @@ export class NexusService { const roleId = `${project.slug}-ID` const role = await this.client.getSecurityRoles(roleId) if (!role) { - await this.client.createSecurityRoles({ + await this.client.ensureSecurityRoles({ id: roleId, name: `${project.slug}-role`, description: 'desc', @@ -452,7 +452,7 @@ export class NexusService { await this.client.updateSecurityUsersChangePassword(project.slug, ensuredPassword) } } else { - await this.client.createSecurityUsers({ + await this.client.ensureSecurityUsers({ userId: project.slug, firstName: project.owner.firstName, lastName: project.owner.lastName, @@ -472,7 +472,7 @@ export class NexusService { private async ensureSecurityRole(id: string, privileges: string[]) { const role = await this.client.getSecurityRoles(id) if (!role) { - await this.client.createSecurityRoles({ + await this.client.ensureSecurityRoles({ id, name: id, description: 'desc', 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 87c951caef..164c8b0d96 100644 --- a/apps/server-nestjs/src/modules/nexus/nexus.utils.spec.ts +++ b/apps/server-nestjs/src/modules/nexus/nexus.utils.spec.ts @@ -1,6 +1,6 @@ -import { describe, expect, it } from 'vitest' +import { describe, expect, it, vi } from 'vitest' import { NexusError } from './nexus-http-client.service' -import { generateNexusCredPath, isNexusNotFound } from './nexus.utils' +import { ensure, generateNexusCredPath, isNexusAlreadyExists, isNexusNotFound } from './nexus.utils' describe('nexus path helpers', () => { it('scopes the NEXUS credentials to the project', () => { @@ -19,3 +19,54 @@ describe('isNexusNotFound', () => { expect(isNexusNotFound(null)).toBe(false) }) }) + +describe('isNexusAlreadyExists', () => { + it('matches a 409 or an already/exists message', () => { + expect(isNexusAlreadyExists(new NexusError('HttpError', 'conflict', { status: 409 }))).toBe(true) + expect(isNexusAlreadyExists(new NexusError('HttpError', 'Repository already exists', { status: 400 }))).toBe(true) + }) + + it('rejects other errors', () => { + expect(isNexusAlreadyExists(new NexusError('HttpError', 'bad request', { status: 400 }))).toBe(false) + expect(isNexusAlreadyExists(new Error('already exists'))).toBe(false) + expect(isNexusAlreadyExists(null)).toBe(false) + }) +}) + +describe('ensure', () => { + it('returns the created value when create succeeds', async () => { + const reload = vi.fn() + + await expect(ensure({ create: async () => 'created', reload })).resolves.toBe('created') + + expect(reload).not.toHaveBeenCalled() + }) + + it('reloads once on a collision and never retries create', async () => { + const error = new NexusError('HttpError', 'already exists', { status: 409 }) + const create = vi.fn(async () => { throw error }) + const onCollision = vi.fn() + const reload = vi.fn(async () => 'existing') + + await expect(ensure({ create, reload, onCollision })).resolves.toBe('existing') + + expect(create).toHaveBeenCalledOnce() + expect(onCollision).toHaveBeenCalledWith(error) + expect(reload).toHaveBeenCalledOnce() + }) + + it('rethrows the original error when a collision finds nothing on reload', async () => { + const error = new NexusError('HttpError', 'already exists', { status: 409 }) + + await expect(ensure({ create: async () => { throw error }, reload: async () => undefined })).rejects.toBe(error) + }) + + it('rethrows non-collision errors without reloading', async () => { + const error = new NexusError('HttpError', 'forbidden', { status: 403 }) + const reload = vi.fn() + + await expect(ensure({ create: async () => { throw error }, reload })).rejects.toBe(error) + + expect(reload).not.toHaveBeenCalled() + }) +}) diff --git a/apps/server-nestjs/src/modules/nexus/nexus.utils.ts b/apps/server-nestjs/src/modules/nexus/nexus.utils.ts index 4d3ed39661..eae9a956e0 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 { HttpStatus } from '@nestjs/common' import { NexusError } from './nexus-http-client.service' export function getPluginConfig(project: ProjectWithDetails, key: string) { @@ -28,3 +29,38 @@ export function generateNpmHostedRepoName(project: ProjectWithDetails) { export function isNexusNotFound(error: unknown): error is NexusError { return error instanceof NexusError && error.status === 404 } + +// Whether a Nexus error signals an entity already existing (race collision): +// a 409 conflict, or a 4xx whose message mentions "already"/"exists" +// (Nexus reports some collisions as a generic Bad Request). +export function isNexusAlreadyExists(error: unknown): error is NexusError { + if (!(error instanceof NexusError)) return false + if (error.status === HttpStatus.CONFLICT) return true + return error.status !== undefined && error.status >= 400 && error.status < 500 && /already|exists/i.test(error.message) +} + +// Runs an idempotent write: tries `create`, and on a Nexus race collision +// reloads via `reload` and returns the existing entity instead of failing. +// `onCollision` is invoked once when a collision is detected. If the reload +// finds nothing, the original error is rethrown so genuine failures are not +// swallowed. +export async function ensure({ + create, + reload, + onCollision, +}: { + create: () => Promise + reload: () => Promise + onCollision?: (error: unknown) => void +}): Promise { + try { + return await create() + } catch (error) { + if (isNexusAlreadyExists(error)) { + onCollision?.(error) + const existing = await reload() + if (existing) return existing + } + throw error + } +}