test(server-nestjs): align E2E specs with DSO orchestration

[shikanime]

Realign apps/server-nestjs/test/*.e2e-spec.ts with DSO orchestration.

Test-only change. No production source and no CI/README files in this diff.

Changes

• Centralize e2e timeouts into apps/server-nestjs/test/e2e-timeout.ts, named by the operation they exercise (provision, syncGroups, syncExternal, gitReconcile, provisionHeavy) rather than bare size buckets.
• Assert each resource exists before the on-delete check in the seven plugin e2e suites (gitlab, keycloak, nexus, vault, zone, sonarqube, argocd) so a flaky teardown no longer reads as a green pass.

Verification

• E2E specs are gated behind process.env.E2E; unit CI is unaffected.
• Lint clean on all changed files.
• Registry delete assertion (expect(project.status).toBe(404)) is valid: getProjectByName returns RegistryResponse.status as the raw HTTP code.

Change-Id: Ife034f03d064a633bb02b0521b4b2e3d6a6a6964

[shikanime]

Review — PR #2407 fix(server-nestjs): align GitLab service with legacy resource creation

Verdict: APPROVE (test changes are sound) — but the PR description must be corrected before this is treated as delivered.

I reviewed the actual diff (branch wphetsinorath/push-klzwvkzwmztv, head 9718e9ec7a), not the worktree HEAD. The diff is test-only: 15 files, +233/−82, all under apps/server-nestjs/test/ (plus pre-existing CI/README changes already on the branch).

BLOCKER — description does not match the diff

The PR title, body, and commit message describe production source fixes:

• SonarQube: use project.owner.email for user creation
• GitLab: revoke old mirror tokens, validate via API, 1-year expiry, ciConfigPath, user-creation hardening
• Vault: group restructure + techRO path divergence

None of these are in this diff. I confirmed the corresponding source already exists on the branch (sonarqube-client.service.ts has no owner.email wiring here; handleDelete for gitlab/nexus/registry, makeZoneWithDetails/handleUpsertZone/handleDeleteZone, and getProjectByName are all already present in main/the branch).

These source fixes live in separate open PRs: #2403 (sonarqube email), #2405 (gitlab token/user hardening), #2406 (vault restructure).

→ Either rewrite this PR's title/body/commit to state that #2407 is purely E2E test alignment (and the source work is tracked in #2403/#2405/#2406), or squash the source commits into this branch. As written, a merger expects fixes that never arrive through this PR. This is a docs/process blocker, not a code defect.

NIT — log.e2e-spec.ts:159

limit: 10 → 50 plus switching from positional allLogs[0]/[1] to find(log => log.id === ...) is a real improvement (order-independent). Raising the limit to 50 just to locate two specific rows is a bit blunt but harmless. Keep.

NIT — E2E timeout windows

New delete-lifecycle tests add long hard timeouts (gitlab 72s, argocd 144s, zone 180s). They are correctly gated behind process.env.E2E, so they won't affect CI unit runs — but they are slow-smells typical of live-infra E2E. Acceptable.

NIT — test bootstrap now requires a real .env

Several specs added ConfigModule.forRoot({ envFilePath: getDotenvPaths(), isGlobal: true, load: [baseConfigFactory] }). Correct (baseConfigFactory is now strict), but it means these E2E specs can no longer boot without on-disk env files. Deployment-of-tests note, not a code defect.

What I verified

• All source symbols the new tests call exist on the branch → no missing-symbol compile breakage.
• Registry delete assertion is correct: getProjectByName returns RegistryResponse, whose .status carries the raw HTTP code (registry-http-client.service.ts handleResponse returns { status: response.status, ... }), so expect(project.status).toBe(404) is right. (Note: registry.handleDelete(...) is wrapped by capturePluginResult, so its .harbor?.status is 'OK'/'KO' — but the 404 check is on the direct getProjectByName call, so it's fine.)
• ESLint clean on all 15 changed files (ran against the exact PR-branch tree).
• No security issues in test-only code. No correctness bug in the test logic.

Action before merge

1. Fix the PR title/body/commit to reflect that this is test-only, OR fold in the source commits. The source fixes are real and already reviewed separately in fix(server-nestjs): use project owner email for sonarqube user creation #2403/fix(server-nestjs): harden gitlab token and user handling #2405/fix(server-nestjs): enforce techoRO vault group typing #2406 — they just don't belong to this PR's current diff.
2. Then merge. Tests can't be executed here (no live infra; correctly gated), but lint is clean and the logic lines up with existing source.

[shikanime]

Review: tests sound — description needs trimming (non-blocking)

Verified the actual diff on head 9718e9e: 15 files, all under apps/server-nestjs/test/ — zero source changes. All CI green (lint, unit, build, Sonar, Trivy, CodeQL). Every new test symbol resolves to existing source on the branch. E2E gating relaxation and delete-lifecycle/zone coverage are legitimate.

The production fixes described in the PR body are real but live in sibling PRs, not here:

• #2403 sonarqube: project.owner.email for user creation
• #2405 gitlab: token rotation / validation / user hardening
• #2406 vault: group restructure + techRO path divergence

Request

Trim the PR title/body to reflect that this is test realignment only. As written, it lists source fixes absent from the diff and will mislead readers into thinking #2407 contains the sonar/gitlab/vault changes. Suggested title: test(server-nestjs): realign E2E tests with DSO orchestration docs. Merge #2403/#2405/#2406 alongside this one.

(Note: could not submit an approving review — this is your own PR. Leaving this as a review comment for the record.)

[shikanime]

Review: PR #2407 — align E2E specs with DSO orchestration

Verdict: REQUEST CHANGES (2 blockers, E2E correctness). Unit CI is green and the branch is mergeable, but the new E2E assertions encode behavior that does not hold against current main (and against the in-flight deps as written). These tests run in the E2E pipeline and would fail / block releases.

BLOCKER — Keycloak delete test is wrong against production

apps/server-nestjs/test/keycloak.e2e-spec.ts (new should remove project groups from Keycloak on delete):

await keycloakService.handleDelete(project)
expect(await keycloak.getGroupByPath(`/${testProjectSlug}`)).toBeUndefined()
expect(await keycloak.getGroupByPath(`/${testProjectSlug}/console`)).toBeUndefined()

main handleDelete -> cleanupProject -> purgeOrphanGroups([project]) (apps/server-nestjs/src/modules/keycloak/keycloak.service.ts:194). That only deletes groups whose name is NOT in projectSlugs; the live project's own group (name === slug) is explicitly preserved, and subgroups are never iterated. So after handleDelete the /{slug} and /{slug}/console groups still exist -> both assertions fail. No linked PR (#2403/#2405/#2406) changes Keycloak delete behavior. Either fix handleDelete to actually delete the project's group subtree, or correct the test to match the (intended) production behavior. Note this may belong in a production fix PR, not here.

BLOCKER — Vault delete test depends on unmerged #2406

apps/server-nestjs/test/vault.e2e-spec.ts (new should remove project from Vault on delete):

await expect(vaultClient.getIdentityGroupName(adminGroupName)).rejects.toThrow('Not Found')

On main, getIdentityGroupName (apps/server-nestjs/src/modules/vault/vault-client.service.ts:385) does NOT throw on 404 — http.fetch -> handleResponse returns { status: 404, data: null }. The throw behavior is introduced by #2406 (closed, mergedAt: null, NOT merged). The PR body lists #2406 as a must-merge dependency, but it was closed without merging, so the dependency is broken. Reopen/merge #2406 (or fold its vault-client.service.ts change here) before this assertion can pass.

WARNING — Dependency ordering / PR body inconsistencies

• GitLab delete test (gitlab.e2e-spec.ts) correctly expects group deletion, but only after fix(server-nestjs): harden gitlab token and user handling #2405 merges (it rewrites gitlab.service.ts handleDelete to deleteGroup). fix(server-nestjs): harden gitlab token and user handling #2405 is still OPEN.
• PR body says branch carries stray job-build.yml, workflow-create-or-update-release.yml, README.md — actual diff is 15 test files only; that text is stale. Also the body lists fix(server-nestjs): enforce techoRO vault group typing #2406 as must-merge while fix(server-nestjs): enforce techoRO vault group typing #2406 is closed-unmerged. Please update the description.
• fix(server-nestjs): use project owner email for sonarqube user creation #2403 (sonar user email) is OPEN; the sonar e2e isn't touched here, but note the stated merge-before ordering for fix(server-nestjs): use project owner email for sonarqube user creation #2403/fix(server-nestjs): harden gitlab token and user handling #2405 still applies to the E2E suite as a whole.

NIT

• vi is imported in log.e2e-spec.ts from the diff but appears unused there; lint passed so likely fine, but drop if truly unused.
• Cosmetic renames (*Controller -> *Service, dropped the {} second arg to describe.runIf) are consistent and fine.

What's good

Nexus/Registry delete assertions match main (deleteProject removes maven/npm repos, roles, users; Harbor getProjectByName returns status 404). project-services switch to nexus.activateNpmRepo + NEXUS_CONFIG_KEY_ACTIVATE_NPM_REPO (exists on main) is correct. log/zone ConfigModule wiring matches the existing pattern. All 15 files lint clean.

Gate: do not merge until #2405 lands and the Keycloak + Vault delete tests reflect real production behavior (or their production fixes land first).

[shikanime]

Review: #2407 — test(server-nestjs): align E2E specs

Verdict: APPROVE (reviewer agent)

Re-verified against head 468b4b7cc. All 16 changed files are under apps/server-nestjs/test/ — zero source changes. Every new assertion traces to existing source on the branch (production fixes live in sibling PRs #2403/#2405/#2406). CI green.

Non-blocking: trim the PR title/body to "test realignment only" — as written it lists source fixes absent from this diff and can mislead readers into thinking #2407 contains the sonar/gitlab/vault changes. Merge with #2403/#2405/#2406.

[cloud-pi-native-sonarqube]

Quality Gate passed

Issues

• 0 New Issues
• 0 Fixed Issues
• 0 Accepted Issues

Measures

• 0 Security Hotspots
• 0.00% Coverage (26.00% Estimated after merge)
• 2.72% Duplicated Code (4.20% Estimated after merge)

Project ID: cloud-pi-native_console_AYsa46O31ebDtzs-pYyZ

View in SonarQube

[StephaneTrebel]

Schön ! 👍