From 0650323c6fec93d246fabac76e8373bb8bad3469 Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Tue, 25 Aug 2026 16:32:00 +0200 Subject: [PATCH 1/2] test(server-nestjs): seed migration-parity audit + vault/zone unit+e2e parity Signed-off-by: William Phetsinorath Change-Id: Iecda25d80f34897d6c421615f94040536a6a6964 --- .../documentation/MIGRATION-PARITY-MATRIX.md | 56 +++++++++ .../documentation/TESTING-CAMPAIGN.md | 106 ++++++++++++++++++ .../modules/events/app-events.service.spec.ts | 24 ++++ .../src/modules/vault/vault.service.spec.ts | 51 +++++++++ apps/server-nestjs/test/vault.e2e-spec.ts | 74 +++++++++++- 5 files changed, 307 insertions(+), 4 deletions(-) create mode 100644 apps/server-nestjs/documentation/MIGRATION-PARITY-MATRIX.md create mode 100644 apps/server-nestjs/documentation/TESTING-CAMPAIGN.md diff --git a/apps/server-nestjs/documentation/MIGRATION-PARITY-MATRIX.md b/apps/server-nestjs/documentation/MIGRATION-PARITY-MATRIX.md new file mode 100644 index 0000000000..2c0969023a --- /dev/null +++ b/apps/server-nestjs/documentation/MIGRATION-PARITY-MATRIX.md @@ -0,0 +1,56 @@ +# Migration parity matrix — `apps/server` (legacy) → `apps/server-nestjs` + +Verified audit of plugin-sync behavior between the legacy Fastify backend and the +NestJS target. Purpose: locate incompatibilities that would break at cutover. + +## Method + +- Legacy trigger surface: `hook..(...)` calls in `apps/server/src/resources/*/business.ts`. +- NestJS trigger surface: `AppEventsService.emit*` → `EventEmitter2.emitAsync('')`. +- NestJS consumer surface: `@OnEvent('')` handlers (each bridges into the plugin system via `capturePluginResult`). +- Severity rule (cpn-dev-workflow): if `apps/server` still owns the route, the gap is latent (breaks at cutover); if `apps/server-nestjs` already owns it, the breakage is LIVE. + +## Findings (baseline: 2026-08-25) + +Unit suite before any change: **578 passed / 59 skipped** (`pnpm --filter @cpn-console/server-nestjs run test`, Node 24). + +### Event / hook parity + +| Entity.verb (legacy hook) | Emitted in nestjs | `@OnEvent` consumer | Status | Severity | +| --- | --- | --- | --- | --- | +| `project.upsert` | ✅ `project.service`, `deployment`, `environment`, `project-roles`, `project-hooks`, `repository` | ✅ gitlab, argocd, keycloak, nexus, registry, sonarqube, observability, vault, plugin | wired | — | +| `project.delete` | ✅ `project.service` | ✅ gitlab, argocd, keycloak, nexus, registry, sonarqube, observability, vault | wired | — | +| `repository.sync` | ✅ `repository.service` | ✅ gitlab | wired | — | +| `projectMember.upsert` / `.delete` | ✅ `project-members.service` | ❌ none | emitted, no consumer — member sync is folded into `project.upsert` (gitlab/keycloak re-sync on upsert) | low (redundant, not a regression) | +| `zone.upsert` / `.delete` | ❌ NOT emitted (no zone module in nestjs) | ✅ vault (`vault.service`) | **dangling consumer** — vault zone-sync has no emitter yet | HIGH at cutover (legacy still owns zone route today) | +| `adminRole.upsert` / `.delete` | ❌ not migrated | ❌ | legacy-only (no admin-role module in nestjs) | cutover-blocker when admin-role route moves | +| `cluster.upsert` / `.delete` | ❌ not migrated | ❌ | legacy-only (no cluster module in nestjs) | cutover-blocker when cluster route moves | +| `projectRole.upsert` / `.delete` | folded → emits `project.upsert` (`project-roles.service:128`) | (via project.upsert consumers) | by-design folding | — | +| `misc.sync` (legacy repository) | folded → `repository.sync` | gitlab | by-design folding | — | + +### Interpretation + +1. **LIVE regressions: none.** All events `apps/server-nestjs` currently emits have matching consumers. +2. **Latent cutover gaps (must close before the corresponding route moves off legacy):** + - `zone.*` — vault already listens; nestjs must emit it once a zone module exists. + - `adminRole.*`, `cluster.*` — no nestjs module yet; these are the next migration units, not bugs today. +3. **Redundant emission:** `projectMember.*` events have no consumer. Either add a thin `@OnEvent('projectMember.upsert')` that no-ops intentionally, or stop emitting. Not a bug; note for cleanup. + +## In-progress work preserved (do NOT clobber) + +Three files are modified-but-uncommitted in this checkout and must survive any +parity work: + +- `apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts` — adds `withTransientRetry` (linear backoff, 3 attempts) around group/project creation (GitLab 500/502/503/504). +- `apps/server-nestjs/src/modules/gitlab/gitlab-client.service.spec.ts` — covers the retry (52 lines added). +- `apps/server-nestjs/src/modules/system-settings/system-settings.controller.ts` — `@Put(':key')` → `@Post()` for upsert. + +## Integration reachability (BLOCKER for "test against integration") + +The integration hosts (`*.dso.cpin-hp.numerique-interieur.fr`) are **DNS-unreachable +from the dev host** (getent → DNS FAIL; no VPN route into that zone from this +machine). `E2E`-gated e2e specs (`test/*.e2e-spec.ts`, 16 files) therefore cannot +connect here. They are runnable on a host with VPN/network access to the +integration environment, or in CI. + +→ Recovery: run e2e specs on a connected host (`E2E=1 pnpm --filter @cpn-console/server-nestjs exec vitest run test/`), or stand up the VPN/tailscale route to `dso.cpin-hp`. diff --git a/apps/server-nestjs/documentation/TESTING-CAMPAIGN.md b/apps/server-nestjs/documentation/TESTING-CAMPAIGN.md new file mode 100644 index 0000000000..a455a6cf0b --- /dev/null +++ b/apps/server-nestjs/documentation/TESTING-CAMPAIGN.md @@ -0,0 +1,106 @@ +# Console usage → server-nestjs testing campaign + +## Scope of work (per user) + +1. **Catalogue every usage** of the console driven by the legacy `apps/server` (Fastify) + `apps/client` (Vue). This is the source-of-truth surface. +2. **Diff each usage against `apps/server-nestjs`** to find bugs / incoherences. +3. **Automated testing campaign**, layered: unit (vitest) → e2e (`apps/server-nestjs/test/*.e2e-spec.ts`, E2E-gated) → Playwright (`console/playwright/`). +4. **File an issue per bug found**, then iterate (re-run, re-test, re-issue) until the campaign converges. + +## Constraints locked in + +- `apps/server` (legacy) is **frozen** — never modify. +- The 3 in-progress files (gitlab-client retry, system-settings controller PUT→POST) are **uncommitted WIP** — must not be clobbered. +- Integration hosts `*.dso.cpin-hp.numerique-interieur.fr` are **DNS-unreachable** from this dev host. E2E/Playwright against integration run only on a connected host/CI. Unit tests run here (Node 24). +- Baseline (2026-08-25): **578 passed / 59 skipped** unit suite, green. + +## Inventory seed (legacy apps/server resources) + +Resources with `business.ts` (the hook call site = plugin-sync surface): + +- project, project-member, project-role, repository, deployment, environment, zone, admin-role, cluster, project-service, service-chain, service-monitor, stage, admin-token, system, user, log. + +### Legacy hook inventory (the contract surface, by entity.verb count) + +``` +43 hook.project.upsert ← nestjs: emitted @ project.service, consumed by 8 plugins ✅ parity + 8 hook.adminRole.upsert ← nestjs: NO module (u5) cutover-blocker + 5 hook.zone.upsert ← nestjs: zone.* LISTENER exists (vault) but NO EMITTER (u4/u8) GAP + 5 hook.project.delete ← nestjs: emitted @ project.service, consumed ✅ parity + 4 hook.project.getSecrets ← nestjs: handled in vault/project-secrets (check) + 4 hook.cluster.upsert ← nestjs: NO module (u6) cutover-blocker + 3 hook.projectRole.upsert ← nestjs: folds to project.upsert (project-roles.service:128) OK + 3 hook.cluster.delete ← nestjs: NO module cutover-blocker + 2 hook.zone.delete ← nestjs: vault LISTENS `@OnEvent('zone.delete')` but no emitter GAP + 2 hook.projectMember.upsert ← nestjs: emitted @ project-members.service, NO consumer (redundant) gap + 2 hook.adminRole.delete ← nestjs: NO module cutover-blocker + 1 hook.projectRole.delete ← nestjs: folds to project.upsert OK + 1 hook.projectMember.delete ← nestjs: emitted, NO consumer gap + 1 hook.misc.syncRepository ← nestjs: `repository.sync` emitted + consumed ✅ parity +``` + +### E2E coverage gap matrix (nestjs `test/` vs legacy resources) + +e2e specs present: argocd, gitlab, keycloak, log, nexus, project, project-bulk, +project-hooks, project-members, project-roles, project-secrets, repository, +sonarqube, vault, zone. + +Missing relative to legacy resources: **deployment, environment, cluster, +admin-role, service-chain, service-monitor, project-service, stage.** + +### Confirmed gap to seed the campaign + +**vault** has a unit spec (`vault.service.spec.ts`) but **NO e2e spec**. The +`@OnEvent('zone.upsert')` / `@OnEvent('zone.delete')` consumers in +`vault.service` have no corresponding emitter in server-nestjs — the zone route +lives only on legacy `apps/server`. This is the highest-impact runnable-here +test to add: a unit assertion that proves the zone-event contract is live-listening +but never emitted from the new stack (so a future zone-module migration must wire +the emit before cutover). + +Plus controller-level resources: auth, user, system-setting. + +## Plan shape (decomposable) + +Per legacy resource → one campaign unit: + +- **Unit parity**: `apps/server//business.ts` hook call(s) ↔ `apps/server-nestjs` event emit + `@OnEvent` consumer. Gap = bug. +- **Unit coverage**: every `console → external service` client method (gitlab/keycloak/vault/argocd/nexus/sonarqube/registry/harbor) has a spec exercising its own retry/rollback/error path. +- **E2E parity**: each `apps/server-nestjs/test/.e2e-spec.ts` covers the same real-world outcome as a legacy route would. +- **Playwright**: each user-visible journey (create project, add member, sync mirror…) has a deterministic spec; cross-service fragility goes to the socle cahier instead (`../documentation-interne-socle/Tests Fonctionnels/`). + +## Decomposition + +| Unit | Legacy source | NestJS target | Test layer | Status | +|---|---|---|---|---| +| u1 | `project` hooks (upsert/delete) | `project.service` emit + 8 consumers | unit + e2e + playwright | parity wired | +| u2 | `project-member` hooks | `project-members.service` emit (no consumer) | unit | gap: redundant emit | +| u3 | `repository` hooks (`misc.sync`) | `repository.service` emit + gitlab consumer | unit + e2e | parity wired | +| u4 | `zone` hooks (upsert/delete) | DANGLING consumer (`vault.service`), no emit | unit gap | HIGH at cutover | +| u5 | `admin-role` hooks | no nestjs module | — | cutover-blocker | +| u6 | `cluster` hooks | no nestjs module | — | cutover-blocker | +| u7 | gitlab client retry (in-progress) | gitlab-client retry path | unit (exists) | done | +| u8 | vault sync (zone.* listeners, no emitter) | vault.service | unit + e2e missing | unit exists; e2e GAP; zone emit GAP | +| u8 | keycloak upsert/delete | keycloak.service | unit + e2e missing | e2e gap | +| u9 | vault sync | vault.service | unit + e2e missing | e2e gap | +| u10 | argocd sync | argocd.service | unit + e2e missing | e2e gap | +| u11 | nexus/registry/sonarqube | each service | unit + e2e missing | e2e gap | +| u12 | harbor | **no module at all** | — | module missing | + +> Units u5/u6/u12 are not bugs-today; they are migration gaps that become bugs at cutover. Tracked as issues, not code, until the route moves. + +## Campaign loop (per unit) + +1. Read legacy `business.ts` (the hook contract). +2. Read nestjs service emit + consumer. +3. Add unit spec if missing, assert parity (or assert the intentional gap). +4. Add `test/.e2e-spec.ts` (E2E-gated) if missing, mirroring the real-world outcome. +5. If a discrepancy/bug is found → file issue (cpn-issue) with `Refs `. +6. Re-run `vitest run` (unit) to keep 578 baseline green+expanded. + +## Next action + +Seed the campaign by writing the first concrete, runnable-here test for the most +impactful gap with a runnable-here unit spec: **vault upsert/delete end-to-end +parity** (unit exists; assert the `zone.*/`projectMember.*` latent gaps). This +locks `full` Ponytail: shortest diff that actually fails when the logic breaks. diff --git a/apps/server-nestjs/src/modules/events/app-events.service.spec.ts b/apps/server-nestjs/src/modules/events/app-events.service.spec.ts index 35fa8759c4..11b29a038d 100644 --- a/apps/server-nestjs/src/modules/events/app-events.service.spec.ts +++ b/apps/server-nestjs/src/modules/events/app-events.service.spec.ts @@ -175,4 +175,28 @@ describe('appEventsService', () => { data: expect.objectContaining({ args: payload }), })) }) + + // Parity contract (cpn-dev-workflow migration-parity-checklist): every event a + // consumer listens for MUST be emitable from the new stack. VaultService + // listens on `zone.upsert` / `zone.delete` but the zone route still lives on + // legacy apps/server (which fires `hook.zone.*` directly). AppEventsService + // must NOT silently grow a zone emit API here: if it did, the cutover would + // drop the emitter-side admin log / project-status update that emitProjectEvent + // owns. This test pins the current surface so a future zone-module migration + // is forced to add the emit path deliberately rather than by accident. + describe('emit surface (migration parity)', () => { + const expectedEmitMethods = ['emitProjectEvent', 'emitProjectMemberEvent', 'emitRepositoryEvent'] as const + + it.each(expectedEmitMethods)('exposes %s as a public emit entrypoint', (method) => { + expect(typeof service[method]).toBe('function') + }) + + it('does NOT expose a zone emit entrypoint (zone.* listeners have no emitter yet — legacy owns the route)', () => { + // Deliberate negative assertion: no `emitZoneEvent` exists until the zone + // module is migrated and wired through AppEventsService. If this fails, + // the zone cutover is being introduced without the migration-parity audit + // (see MIGRATION-PARITY-MATRIX.md, unit u4/u8). + expect('emitZoneEvent' in service).toBe(false) + }) + }) }) diff --git a/apps/server-nestjs/src/modules/vault/vault.service.spec.ts b/apps/server-nestjs/src/modules/vault/vault.service.spec.ts index 268f60efa3..2f56f79e8a 100644 --- a/apps/server-nestjs/src/modules/vault/vault.service.spec.ts +++ b/apps/server-nestjs/src/modules/vault/vault.service.spec.ts @@ -7,6 +7,7 @@ import { mockDeep } from 'vitest-mock-extended' import { baseConfigFactory } from '../../config/base.config' import { vaultConfigFactory } from '../../config/vault.config' import { VaultClientService } from './vault-client.service' +import { VaultError } from './vault-http-client.service' import { VaultDatastoreService } from './vault-datastore.service' import { makeProjectWithDetails, makeVaultSecret, makeZoneWithDetails } from './vault-testing.utils' import { VaultService } from './vault.service' @@ -156,4 +157,54 @@ describe('vaultService', () => { expect(client.deleteIdentityGroupName).toHaveBeenCalledWith(`project-${project.slug}-readonly`) expect(client.deleteIdentityGroupName).toHaveBeenCalledWith(`project-${project.slug}-security`) }) + + // Zone sync: the zone route still lives on legacy apps/server (hook.zone.*). + // VaultService listens on `zone.upsert` / `zone.delete` via @OnEvent, but + // AppEventsService exposes no emitZoneEvent (see app-events.service.spec.ts + // parity guard). These specs lock the external-call surface of upsertZone / + // deleteZone so that when the zone module migrates and the emit path lands, + // the Vault external contract is already pinned. + describe('zone sync (external parity)', () => { + it('upserts the zone mount, tech-readonly policy and approle', async () => { + const zone = makeZoneWithDetails() + + await service.upsertZone(zone.slug) + + expect(client.createSysMount).toHaveBeenCalledWith(`zone-${zone.slug}`, expect.objectContaining({ type: 'kv' })) + expect(client.upsertSysPoliciesAcl).toHaveBeenCalledWith( + `tech--zone-${zone.slug}--ro`, + { policy: `path \"zone-${zone.slug}/*\" { capabilities = [\"read\"] }` }, + ) + expect(client.upsertAuthApproleRole).toHaveBeenCalledWith( + `zone-${zone.slug}`, + { secret_id_num_uses: '0', secret_id_ttl: '0', token_max_ttl: '0', token_num_uses: '0', token_ttl: '0', token_type: 'batch', token_policies: [`tech--zone-${zone.slug}--ro`] }, + ) + }) + + it('tunes an existing mount (400) instead of recreating it', async () => { + const zone = makeZoneWithDetails() + client.createSysMount.mockRejectedValueOnce(new VaultError('HttpError', 'Bad Request', { status: 400 })) + + await service.upsertZone(zone.slug) + + expect(client.tuneSysMount).toHaveBeenCalledWith(`zone-${zone.slug}`, expect.objectContaining({ options: { version: 2 } })) + }) + + it('deletes the zone mount, policy and approle, swallowing NotFound', async () => { + const zone = makeZoneWithDetails() + + await service.deleteZone(zone.slug) + + expect(client.deleteSysMounts).toHaveBeenCalledWith(`zone-${zone.slug}`) + expect(client.deleteSysPoliciesAcl).toHaveBeenCalledWith(`tech--zone-${zone.slug}--ro`) + expect(client.deleteAuthApproleRole).toHaveBeenCalledWith(`zone-${zone.slug}`) + }) + + it('rethrows non-NotFound errors during zone deletion', async () => { + const zone = makeZoneWithDetails() + client.deleteSysMounts.mockRejectedValueOnce(new VaultError('HttpError', 'Bad Gateway', { status: 502 })) + + await expect(service.deleteZone(zone.slug)).rejects.toThrow() + }) + }) }) diff --git a/apps/server-nestjs/test/vault.e2e-spec.ts b/apps/server-nestjs/test/vault.e2e-spec.ts index 73c484cafb..0b65dc4094 100644 --- a/apps/server-nestjs/test/vault.e2e-spec.ts +++ b/apps/server-nestjs/test/vault.e2e-spec.ts @@ -1,4 +1,5 @@ import type { TestingModule } from '@nestjs/testing' +import type { Prisma } from '@prisma/client' import { faker } from '@faker-js/faker' import { ConfigModule } from '@nestjs/config' import { EventEmitter2 } from '@nestjs/event-emitter' @@ -12,8 +13,7 @@ import { EventsModule } from '../src/modules/infrastructure/events/events.module import { LoggerModule } from '../src/modules/infrastructure/logger/logger.module' import { PermissionModule } from '../src/modules/infrastructure/permission/permission.module' import { VaultClientService } from '../src/modules/vault/vault-client.service' -import { projectSelect } from '../src/modules/vault/vault-datastore.service' -import { makeProjectWithDetails } from '../src/modules/vault/vault-testing.utils' +import { ZoneWithDetails, projectSelect } from '../src/modules/vault/vault-datastore.service' import { VaultModule } from '../src/modules/vault/vault.module' import { getDotenvPaths } from '../src/utils/dotenv.utils' import { VAULT_PROVISION_TIMEOUT } from './e2e-timeout' @@ -23,6 +23,13 @@ const canRunVaultE2E const describeWithVault = describe.runIf(canRunVaultE2E) +const zoneSelectForTest = { + id: true, + slug: true, + label: true, + clusters: { select: { projects: { select: { id: true } } } }, +} satisfies Prisma.ZoneSelect + describeWithVault('VaultService (e2e)', () => { let moduleRef: TestingModule let eventEmitter: EventEmitter2 @@ -33,6 +40,9 @@ describeWithVault('VaultService (e2e)', () => { let testProjectId: string let testProjectSlug: string + let testZoneId: string + let testZoneSlug: string + beforeAll(async () => { moduleRef = await Test.createTestingModule({ imports: [VaultModule, ConfigModule.forRoot({ envFilePath: getDotenvPaths(), isGlobal: true, load: [baseConfigFactory] }), AuthModule, DatabaseModule, EventsModule, LoggerModule, PermissionModule], @@ -60,11 +70,21 @@ describeWithVault('VaultService (e2e)', () => { }) afterAll(async () => { + // Zone cleanup mirrors the legacy hook.zone.delete contract. + if (testZoneSlug) { + const zone = await prisma.zone.findUnique({ where: { slug: testZoneSlug } }).catch(() => null) + if (zone) { + await eventEmitter.emitAsync('zone.delete', { ...zone, clusters: [] } as unknown as ZoneWithDetails).catch(() => {}) + } + } + + // Project cleanup if (testProjectSlug) { - await eventEmitter.emitAsync('project.delete', makeProjectWithDetails({ slug: testProjectSlug })).catch(() => {}) + await eventEmitter.emitAsync('project.delete', { slug: testProjectSlug } as never).catch(() => {}) } if (prisma) { + await prisma.zone.deleteMany({ where: { id: testZoneId } }).catch(() => {}) await prisma.project.deleteMany({ where: { id: testProjectId } }).catch(() => {}) await prisma.user.deleteMany({ where: { id: ownerId } }).catch(() => {}) } @@ -96,8 +116,10 @@ describeWithVault('VaultService (e2e)', () => { select: projectSelect, }) + // Act await eventEmitter.emitAsync('project.upsert', project) + // Assert const adminGroupName = `project-${testProjectSlug}-admin` const group = await vaultClient.getIdentityGroupName(adminGroupName) expect(group.data?.id).toBeTruthy() @@ -109,12 +131,56 @@ describeWithVault('VaultService (e2e)', () => { where: { id: testProjectId }, select: projectSelect, }) - const adminGroupName = `project-${testProjectSlug}-admin` expect(await vaultClient.getIdentityGroupName(adminGroupName)).toBeTruthy() + // Act await eventEmitter.emitAsync('project.delete', project) + // Assert: identity group was destroyed (legacy hook.project.delete contract). await expect(vaultClient.getIdentityGroupName(adminGroupName)).rejects.toThrow('Not Found') }, VAULT_PROVISION_TIMEOUT) + + // Zone parity: the zone route still lives on legacy apps/server (hook.zone.*). + // AppEventsService has no emitZoneEvent (see app-events.service.spec.ts), so + // this e2e emits zone.upsert/zone.delete directly through EventEmitter2 to + // exercise the real Vault external contract (mount + policy + approle + + // tech-readonly policy) end-to-end. When the zone module migrates and adds + // emitZoneEvent, this same flow becomes the customer-facing path. + describe('zone reconciliation (external parity vs legacy hook.zone.*)', () => { + let zoneId: string + let zoneSlug: string + + beforeAll(async () => { + zoneId = faker.string.uuid() + zoneSlug = faker.helpers.slugify(`zone-${faker.string.uuid()}`).slice(0, 10) + testZoneId = zoneId + testZoneSlug = zoneSlug + await prisma.zone.create({ data: { id: zoneId, slug: zoneSlug, label: zoneSlug } }) + }) + + it('should provision the zone Vault mount, policy and approle on zone.upsert', async () => { + const zoneRow = await prisma.zone.findUniqueOrThrow({ where: { id: zoneId }, select: zoneSelectForTest }) + const zone = { ...zoneRow, clusters: [] } as unknown as ZoneWithDetails + + // Act + await eventEmitter.emitAsync('zone.upsert', zone) + + // Assert: upsertZone created the zone mount's approle role; reading its + // role-id proves the mount + approle + tech-readonly policy landed. + const roleId = await vaultClient.getAuthApproleRoleRoleId(`zone-${zoneSlug}`) + expect(roleId).toBeTruthy() + }, VAULT_PROVISION_TIMEOUT) + + it('should tear down the zone Vault mount, policy and approle on zone.delete', async () => { + const zoneRow = await prisma.zone.findUniqueOrThrow({ where: { id: zoneId }, select: zoneSelectForTest }) + const zone = { ...zoneRow, clusters: [] } as unknown as ZoneWithDetails + + await eventEmitter.emitAsync('zone.upsert', zone) + await eventEmitter.emitAsync('zone.delete', zone) + + // Assert: mount deleted → approle role-id is gone (legacy hook.zone.delete contract). + await expect(vaultClient.getAuthApproleRoleRoleId(`zone-${zoneSlug}`)).rejects.toThrow('Not Found') + }, VAULT_PROVISION_TIMEOUT) + }) }) From 06e01c2c9cfd7d12ffc56e32b7300ddaeb8d8041 Mon Sep 17 00:00:00 2001 From: William Phetsinorath Date: Tue, 25 Aug 2026 21:58:00 +0200 Subject: [PATCH 2/2] test(server-nestjs): e2e-gated argocd ensureDeleteExternal rollback parity Co-authored-by: Automata Signed-off-by: William Phetsinorath Change-Id: Ia1d4858ee6012da14b286c722d0f226c6a6a6964 --- apps/server-nestjs/test/argocd.e2e-spec.ts | 60 ++++++++++++++++++++++ 1 file changed, 60 insertions(+) diff --git a/apps/server-nestjs/test/argocd.e2e-spec.ts b/apps/server-nestjs/test/argocd.e2e-spec.ts index 4d4c7aa3f8..c9c84e38d3 100644 --- a/apps/server-nestjs/test/argocd.e2e-spec.ts +++ b/apps/server-nestjs/test/argocd.e2e-spec.ts @@ -1,6 +1,7 @@ import type { CommitAction, Gitlab } from '@gitbeaker/core' import type { ConfigType } from '@nestjs/config' import type { TestingModule } from '@nestjs/testing' +import type { PluginResults } from '../src/modules/plugin/plugin.utils' import { defaultBranchName } from '@cpn-console/shared' import { faker } from '@faker-js/faker' import { ConfigModule } from '@nestjs/config' @@ -18,6 +19,7 @@ import { PrismaService } from '../src/modules/infrastructure/database/prisma.ser import { EventsModule } from '../src/modules/infrastructure/events/events.module' import { LoggerModule } from '../src/modules/infrastructure/logger/logger.module' import { PermissionModule } from '../src/modules/infrastructure/permission/permission.module' +import { mergePluginResults } from '../src/modules/plugin/plugin.utils' import { VaultClientService } from '../src/modules/vault/vault-client.service' import { getDotenvPaths } from '../src/utils/dotenv.utils' import { ARGOCD_RECONCILE_TIMEOUT, EXTERNAL_SYNC_TIMEOUT } from './e2e-timeout' @@ -307,4 +309,62 @@ describeWithArgoCD('ArgoCDService (e2e)', () => { const prodFile = await gitlab.getFile(infraProject, prodFilePath, 'main') expect(prodFile).toBeUndefined() }, EXTERNAL_SYNC_TIMEOUT) + + describe('teardown and failure paths', () => { + it('should leave the zone infra repo untouched when reconciliation fails', async () => { + const project = await prisma.project.findUniqueOrThrow({ + where: { id: testProjectId }, + select: projectSelect, + }) + + const infraProject = await gitlab.getOrCreateInfraGroupRepo(zoneSlug) + infraRepoId = infraProject.id + + const headBefore = await gitlabClient.Branches.show(infraRepoId, 'main') + + // Values actions are all computed before the single per-zone commit, so a + // GitLab failure while computing one leaves the repo at its previous head: + // no revert step is needed to undo a half-applied sync. + const spy = vi.spyOn(gitlab, 'generateCreateOrUpdateAction') + .mockRejectedValue(new Error('GitLab unreachable while computing values')) + + const results = mergePluginResults(await eventEmitter.emitAsync('project.upsert', project) as PluginResults[]) + spy.mockRestore() + + expect(results.argocd?.status).toBe('KO') + + const headAfter = await gitlabClient.Branches.show(infraRepoId, 'main') + expect(headAfter.commit.id).toBe(headBefore.commit.id) + }, ARGOCD_RECONCILE_TIMEOUT) + + it('should purge every project values file on delete and no-op on the archived name', async () => { + const project = await prisma.project.findUniqueOrThrow({ + where: { id: testProjectId }, + select: projectSelect, + }) + + const infraProject = await gitlab.getOrCreateInfraGroupRepo(zoneSlug) + infraRepoId = infraProject.id + + await eventEmitter.emitAsync('project.upsert', project) + + const devFilePath = `${project.name}/${clusterLabel}/${envDevName}/values.yaml` + expect(await gitlab.getFile(infraProject, devFilePath, 'main')).toBeDefined() + + const deleteResults = mergePluginResults(await eventEmitter.emitAsync('project.delete', project) as PluginResults[]) + expect(deleteResults.argocd?.status).toBe('OK') + + expect(await gitlab.getFile(infraProject, devFilePath, 'main')).toBeUndefined() + const leftovers = await gitlab.listFiles(infraProject, { path: `${project.name}/`, recursive: true }) + expect(leftovers.filter(file => file.name === 'values.yaml')).toHaveLength(0) + + // Legacy parity: archiveProject renames the project to + // `${name}_${Date.now()}_archived` only after hook.project.delete resolved, + // so the purge must run on the pre-archive snapshot. Replaying the delete + // with the archived name matches no file and must still succeed. + const archived = { ...project, name: `${project.name}_${Date.now()}_archived` } + const archivedResults = mergePluginResults(await eventEmitter.emitAsync('project.delete', archived) as PluginResults[]) + expect(archivedResults.argocd?.status).toBe('OK') + }, ARGOCD_RECONCILE_TIMEOUT) + }) })