diff --git a/apps/server-nestjs/src/modules/registry/registry-client.service.spec.ts b/apps/server-nestjs/src/modules/registry/registry-client.service.spec.ts index 809eae509c..4ceb2ca65e 100644 --- a/apps/server-nestjs/src/modules/registry/registry-client.service.spec.ts +++ b/apps/server-nestjs/src/modules/registry/registry-client.service.spec.ts @@ -1,4 +1,5 @@ import type { ConfigType } from '@nestjs/config' +import type { HarborRepository } from './registry-client.service' import { faker } from '@faker-js/faker' import { HttpStatus } from '@nestjs/common' import { Test } from '@nestjs/testing' @@ -98,4 +99,35 @@ describe('registryService', () => { expect(res).toMatchObject({ status: HttpStatus.OK, data: { project_id: 123 } }) }) + + it('should list repositories with page_size', async () => { + server.use( + http.get(`${harborUrl}/api/v2.0/projects/:projectName/repositories`, async ({ request }) => { + expect(request.url).toContain('page_size=100') + return HttpResponse.json([{ name: 'myproj/repo-a' }]) + }), + ) + + const res: HarborRepository[] = [] + for await (const item of service.getRepositories('myproj')) { + res.push(item) + } + + expect(res).toMatchObject([{ name: 'myproj/repo-a' }]) + }) + + it('should delete a repository by name', async () => { + server.use( + http.delete(`${harborUrl}/api/v2.0/projects/:projectName/repositories/:repositoryName`, async ({ request, params }) => { + expect(request.method).toBe('DELETE') + expect(params.projectName).toBe('myproj') + expect(params.repositoryName).toBe('repo-a') + return new HttpResponse(null, { status: HttpStatus.NO_CONTENT }) + }), + ) + + const res = await service.deleteRepository('myproj', 'repo-a') + + expect(res).toMatchObject({ status: HttpStatus.NO_CONTENT }) + }) }) diff --git a/apps/server-nestjs/src/modules/registry/registry-client.service.ts b/apps/server-nestjs/src/modules/registry/registry-client.service.ts index 356f094bda..1e1ebebad9 100644 --- a/apps/server-nestjs/src/modules/registry/registry-client.service.ts +++ b/apps/server-nestjs/src/modules/registry/registry-client.service.ts @@ -2,6 +2,7 @@ import type { RegistryQuery, RegistryResponse } from './registry-http-client.ser import { HttpStatus, Inject, Injectable } from '@nestjs/common' import { RegistryHttpClientService } from './registry-http-client.service' import { ROBOT_LIST_PAGE_SIZE } from './registry.constants' +import { ensure } from './registry.utils' export const roAccess: HarborAccess[] = [ { resource: 'repository', action: 'pull' }, @@ -60,6 +61,10 @@ export interface HarborGroupMemberRequest { } } +export interface HarborRepository { + name?: string +} + export interface HarborProjectQuota { ref?: { id?: number } hard?: { storage?: number } @@ -141,6 +146,16 @@ export class RegistryClientService { }) } + getRepositories(projectName: string): AsyncGenerator { + return this.paginate(`projects/${encodeURIComponent(projectName)}/repositories`) + } + + async deleteRepository(projectName: string, repositoryName: string) { + return this.http.fetch(`projects/${encodeURIComponent(projectName)}/repositories/${encodeURIComponent(repositoryName)}`, { + method: 'DELETE', + }) + } + async listQuotas(projectId: number) { return this.http.fetch(`quotas?reference_id=${encodeURIComponent(String(projectId))}`, { method: 'GET', @@ -215,21 +230,20 @@ export class RegistryClientService { } async ensureRetention(projectName: string, body: HarborRetentionPolicy) { - const created = await this.createRetention(body) - if (created.status === HttpStatus.CONFLICT) { - const racedId = await this.getRetentionId(projectName) - if (racedId) { - const result = await this.updateRetention(racedId, body) - if (result.status >= HttpStatus.BAD_REQUEST) { - throw new Error(`Harbor retention policy failed (${result.status})`) - } - } - return - } - if (created.status >= HttpStatus.BAD_REQUEST) { - throw new Error(`Harbor retention policy failed (${created.status})`) - } - } + return ensure({ + create: () => this.createRetention(body), + reload: async () => { + const racedId = await this.getRetentionId(projectName) + if (racedId) { + const result = await this.updateRetention(racedId, body) + if (result.status >= HttpStatus.BAD_REQUEST) { + throw new Error(`Harbor retention policy failed (${result.status})`) + } + } + }, + }) + } + async createRetention(body: HarborRetentionPolicy) { return this.http.fetch('retentions', { method: 'POST', @@ -244,7 +258,7 @@ export class RegistryClientService { }) } - private async* paginate(path: string, query: RegistryQuery): AsyncGenerator { + private async* paginate(path: string, query?: RegistryQuery): AsyncGenerator { for (let page = 1; ; page++) { const response = await this.http.fetch(path, { method: 'GET', diff --git a/apps/server-nestjs/src/modules/registry/registry.service.spec.ts b/apps/server-nestjs/src/modules/registry/registry.service.spec.ts index 506cfb7f20..e78347f6e1 100644 --- a/apps/server-nestjs/src/modules/registry/registry.service.spec.ts +++ b/apps/server-nestjs/src/modules/registry/registry.service.spec.ts @@ -42,6 +42,8 @@ describe('registryService', () => { ensureGroupMember: vi.fn(), removeGroupMember: vi.fn().mockResolvedValue(makeNoContent()), deleteProjectByName: vi.fn().mockResolvedValue(makeNoContent()), + getRepositories: vi.fn(async function* () {}), + deleteRepository: vi.fn().mockResolvedValue(makeNoContent()), }) datastore = mockDeep({ getAdminPluginConfig: vi.fn().mockResolvedValue(null), @@ -310,6 +312,33 @@ describe('registryService', () => { expect(client.deleteProjectByName).toHaveBeenCalledWith(project.slug) }) + it('should purge repositories before deleting the project', async () => { + const project = makeProjectWithDetails() + client.getRepositories.mockImplementation(async function* () { + yield { name: `${project.slug}/repo-a` } + yield { name: `${project.slug}/repo-b` } + }) + + await service.handleDelete(project) + + expect(client.deleteRepository).toHaveBeenCalledTimes(2) + expect(client.deleteRepository).toHaveBeenCalledWith(project.slug, 'repo-a') + expect(client.deleteRepository).toHaveBeenCalledWith(project.slug, 'repo-b') + expect(client.deleteProjectByName).toHaveBeenCalledWith(project.slug) + }) + + it('should surface a Harbor rejection as a KO result', async () => { + const project = makeProjectWithDetails() + client.deleteProjectByName.mockResolvedValueOnce({ status: HttpStatus.PRECONDITION_FAILED, data: null }) + + await expect(service.handleDelete(project)).resolves.toEqual({ + harbor: expect.objectContaining({ + status: 'KO', + message: 'Harbor delete project failed (412)', + }), + }) + }) + it('should not delete project when it does not exist', async () => { client.getProjectByName.mockResolvedValueOnce({ status: HttpStatus.NOT_FOUND, data: null }) await service.handleDelete(makeProjectWithDetails()) diff --git a/apps/server-nestjs/src/modules/registry/registry.service.ts b/apps/server-nestjs/src/modules/registry/registry.service.ts index e9311c49fe..f55962209f 100644 --- a/apps/server-nestjs/src/modules/registry/registry.service.ts +++ b/apps/server-nestjs/src/modules/registry/registry.service.ts @@ -283,12 +283,21 @@ export class RegistryService { span?.setAttribute('registry.project.exists', false) return } + await this.deleteRepositories(projectSlug) const deleted = await this.client.deleteProjectByName(projectSlug) if (deleted.status >= 300 && deleted.status !== 404) { throw new Error(`Harbor delete project failed (${deleted.status})`) } } + private async deleteRepositories(projectSlug: string) { + for await (const repository of this.client.getRepositories(projectSlug)) { + const name = repository.name + if (!name) continue + await this.client.deleteRepository(projectSlug, name.split('/').slice(1).join('/')) + } + } + @OnEvent('project.upsert') async handleUpsert(project: ProjectWithDetails): Promise> { return capturePluginResult('harbor', () => this.syncProject(project)) diff --git a/apps/server-nestjs/src/modules/registry/registry.utils.spec.ts b/apps/server-nestjs/src/modules/registry/registry.utils.spec.ts new file mode 100644 index 0000000000..d89ee9af86 --- /dev/null +++ b/apps/server-nestjs/src/modules/registry/registry.utils.spec.ts @@ -0,0 +1,39 @@ +import { HttpStatus } from '@nestjs/common' +import { describe, expect, it, vi } from 'vitest' +import { ensure } from './registry.utils' + +describe('ensure', () => { + it('should resolve when create succeeds', async () => { + const reload = vi.fn() + + await expect(ensure({ create: async () => ({ status: HttpStatus.CREATED, data: null }), reload })).resolves.toBeUndefined() + + expect(reload).not.toHaveBeenCalled() + }) + + it('should reload once on a 409 conflict and never retry create', async () => { + const onCollision = vi.fn() + const create = vi.fn(async () => ({ status: HttpStatus.CONFLICT, data: null })) + const reload = vi.fn(async () => {}) + + await expect(ensure({ create, reload, onCollision })).resolves.toBeUndefined() + + expect(create).toHaveBeenCalledOnce() + expect(onCollision).toHaveBeenCalledOnce() + expect(reload).toHaveBeenCalledOnce() + }) + + it('should surface reload errors', async () => { + const error = new Error('Harbor retention policy failed (400)') + + await expect(ensure({ create: async () => ({ status: HttpStatus.CONFLICT, data: null }), reload: async () => { throw error } })).rejects.toBe(error) + }) + + it('should throw on other >= 400 statuses without reloading', async () => { + const reload = vi.fn() + + await expect(ensure({ create: async () => ({ status: HttpStatus.FORBIDDEN, data: null }), reload })).rejects.toThrow('Harbor request failed (403)') + + expect(reload).not.toHaveBeenCalled() + }) +}) diff --git a/apps/server-nestjs/src/modules/registry/registry.utils.ts b/apps/server-nestjs/src/modules/registry/registry.utils.ts index 8bbe97eb7e..c9c3c3c40a 100644 --- a/apps/server-nestjs/src/modules/registry/registry.utils.ts +++ b/apps/server-nestjs/src/modules/registry/registry.utils.ts @@ -1,5 +1,36 @@ import type { ProjectWithDetails } from './registry-datastore.service' +import type { RegistryResponse } from './registry-http-client.service' import { removeTrailingSlash } from '@cpn-console/shared' +import { HttpStatus } from '@nestjs/common' + +// Whether a Harbor response signals an entity already existing (race collision): +// a 409 conflict on the create call. +export function isRegistryConflict(response: RegistryResponse): boolean { + return response.status === HttpStatus.CONFLICT +} + +// Runs an idempotent write: tries `create`, and on a Harbor race collision +// reloads via `reload` and returns the existing state instead of failing. +// `onCollision` is invoked once when a collision is detected. If the reload +// finds nothing, the error is rethrown so genuine failures are not swallowed. +export async function ensure({ + create, + reload, + onCollision, +}: { + create: () => Promise> + reload: () => Promise + onCollision?: (response: RegistryResponse) => void +}): Promise { + const created = await create() + if (created.status >= HttpStatus.BAD_REQUEST) { + if (isRegistryConflict(created)) { + onCollision?.(created) + return reload() + } + throw new Error(`Harbor request failed (${created.status})`) + } +} export function createProjectSlugCacheKey(projectId: string) { return `registry:project-slug:${projectId}`