Skip to content

fix(server-nestjs): use project owner email for sonarqube user creation - #2403

Merged
shikanime merged 2 commits into
mainfrom
wphetsinorath/push-rsstnouuwnwp
Aug 4, 2026
Merged

fix(server-nestjs): use project owner email for sonarqube user creation#2403
shikanime merged 2 commits into
mainfrom
wphetsinorath/push-rsstnouuwnwp

Conversation

@shikanime

@shikanime shikanime commented Jul 31, 2026

Copy link
Copy Markdown
Member

Issues liées

Issues numéro: #2400


Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

@shikanime
shikanime force-pushed the wphetsinorath/push-rsstnouuwnwp branch 2 times, most recently from 20f3e9e to c19af62 Compare July 31, 2026 17:05
@github-actions github-actions Bot added the built label Jul 31, 2026
@shikanime
shikanime force-pushed the wphetsinorath/push-rsstnouuwnwp branch from c19af62 to 21ab391 Compare July 31, 2026 17:10
@shikanime
shikanime enabled auto-merge July 31, 2026 17:10
@shikanime
shikanime requested a review from a team July 31, 2026 17:10
@shikanime shikanime self-assigned this Jul 31, 2026
@shikanime shikanime added this to the 9.24.0 milestone Jul 31, 2026
@shikanime
shikanime force-pushed the wphetsinorath/push-rsstnouuwnwp branch 7 times, most recently from b463a75 to b625ae4 Compare August 3, 2026 09:34
Comment thread apps/server-nestjs/src/modules/gitlab/gitlab-client.service.spec.ts Fixed

@StephaneTrebel StephaneTrebel left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SonarQube: use project.owner.email for user creation instead of
synthetic {slug}@{slug}

GitLab: revoke old mirror tokens on rotation, validate tokens via
API call, restore 1-year token expiry, set ciConfigPath and user
creation hardening (canCreateGroup, forceRandomPassword, projectsLimit)

Vault: group restructure and techRO path divergence are intentional
with built-in roles

E2E: realigned tests with DSO orchestration docs, fix gating

Donc déjà au moins quatre commits atomiques 😅 (en fait j'en compte bien, bien plus)

Et n'hésite pas à découper en MRs stackées, je sens que je vais souvent les revoir, ces 419 lignes. Profitons d'avoir un peu de marge pour améliorer notre process de revue 🙂

Comment thread apps/server-nestjs/src/modules/gitlab/gitlab-client.service.ts Outdated
Comment thread apps/server-nestjs/src/modules/gitlab/gitlab-client.service.spec.ts Outdated
Comment thread apps/server-nestjs/src/modules/gitlab/gitlab.service.ts Outdated
Comment thread apps/server-nestjs/src/modules/gitlab/gitlab.service.ts Outdated
Comment thread apps/server-nestjs/src/modules/vault/vault-client.service.ts
@shikanime
shikanime force-pushed the wphetsinorath/push-rsstnouuwnwp branch 10 times, most recently from bdf27a5 to b5a3e90 Compare August 3, 2026 15:08
@shikanime shikanime changed the title fix(server-nestjs): realign e2e tests with DSO orchestration docs fix(server-nestjs): use project owner email for sonarqube user creation Aug 3, 2026
@shikanime
shikanime force-pushed the wphetsinorath/push-rsstnouuwnwp branch from b5a3e90 to b547d14 Compare August 3, 2026 15:30
@shikanime

Copy link
Copy Markdown
Member Author

Review — PR #2403 (reviewer)

Scope actually reviewed: 9 files, +123/-83, all in sonarqube/ + apps/server-nestjs/src/modules/vault/vault-client.service.ts. Verified locally: vitest run on sonarqube + vault specs → 37/37 pass; diff is internally type-consistent (no dangling callers of the old Promise<…Response> shape). CI green on latest commit.

blocker

  • PR body ≠ diff. The description ("Remove DB_URL gate from 5 e2e specs, standardize describe.runIf, add handleDelete assertions to vault/keycloak/gitlab/nexus/registry, fix gitlab/keycloak handleDelete, add zone.e2e-spec.ts") does not match the pushed diff at all. The real change is: SonarQube search APIs become paginating async generators, user creation uses project.owner.email instead of synthetic {slug}@{slug}, SONAR_PASSWORD is optional in the vault secret, and readTechnReadOnlyCreds is typed VaultSecret<MirrorUserSecret>. Please rewrite the description to reflect the actual diff (or rebase onto the intended commits). As-is it will mislead future readers and the changelog.

warning

  • @StartActiveSpan() on async generators is a no-op for tracing. sonarqube-client.service.ts:273 (searchUsers) and :301 (searchProject) are decorated, but StartActiveSpan only awaits then-able returns (telemetry.decorator.ts:50). An AsyncGenerator is not promise-like, so the span is ended on the first return and the body inside the generator runs outside any span. Not harmful, just dead instrumentation. Either drop the decorator from these two methods or make StartActiveSpan wrap generators. (Established sibling async generators in keycloak/gitlab are not decorated, so this is inconsistent with the codebase.)

nit

  • ensureUser log at sonarqube.service.ts:331 prints email=${project.owner.email} — fine, but if owner is ever absent this throws before the log. Not reachable today (every project has a non-null owner, loaded via projectSelect in project-queries.utils.ts:218), so no action needed; just noting the invariant the new required owner field on ProjectWithDetails now depends on.

Things checked and clean

  • project.owner.email is safe: projectSelect includes owner, and the plugin dispatcher reloads the project (emitProjectEventgetProject) before calling handleUpsert.
  • All 6 callers of searchUsers/searchProject updated to the async-generator contract (service, e2e spec, client spec). No stragglers.
  • vault-client.service.ts SonarqubeUserSecret.SONAR_PASSWORD? + MirrorUserSecret/VaultSecret<T> generic: type-safe, gitlab consumer (gitlab.service.ts:496-502) matches.
  • getAll helper reused from utils/iterable.utils.ts rather than reinvented.

Verdict: REQUEST CHANGES — only for the blocker (description mismatch). The code itself is mergeable; fix the description and this is good to go.

shikanime added a commit that referenced this pull request Aug 4, 2026
Realign apps/server-nestjs/test/*.e2e-spec.ts with DSO orchestration
docs and fix E2E gating. Test-only change; no production source.

Source fixes tracked separately and must merge first:
- #2403 sonarqube user creation uses project.owner.email
- #2405 gitlab token/user hardening (ciConfigPath, canCreateGroup,
  forceRandomPassword, projectsLimit, 1-year expiry, revoke old tokens)
- #2406 vault group restructure and techRO path divergence

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
@shikanime
shikanime force-pushed the wphetsinorath/push-rsstnouuwnwp branch 4 times, most recently from d2b5cc9 to 27b5bf5 Compare August 4, 2026 11:10
@shikanime shikanime mentioned this pull request Aug 4, 2026
Comment thread apps/server-nestjs/src/modules/sonarqube/sonarqube.service.ts
Comment thread README.md
Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
Change-Id: I8776cb553c3a3ef9394ac60aeee526776a6a6964
Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
Change-Id: Ie162865d6cb5ddaa41244b889d7af21b6a6a6964
@shikanime
shikanime force-pushed the wphetsinorath/push-rsstnouuwnwp branch from 27b5bf5 to cc8534a Compare August 4, 2026 13:32
@cloud-pi-native-sonarqube

Copy link
Copy Markdown

@shikanime shikanime moved this to Backlog in Cloud Pi Native Aug 4, 2026
@shikanime shikanime moved this from Backlog to In progress in Cloud Pi Native Aug 4, 2026
@shikanime shikanime moved this from In progress to In review in Cloud Pi Native Aug 4, 2026
@shikanime
shikanime added this pull request to the merge queue Aug 4, 2026
Merged via the queue into main with commit 44d6d27 Aug 4, 2026
66 of 70 checks passed
@shikanime
shikanime deleted the wphetsinorath/push-rsstnouuwnwp branch August 4, 2026 17:20
@github-project-automation github-project-automation Bot moved this from In review to Done in Cloud Pi Native Aug 4, 2026
shikanime added a commit to shikanime/cloud-pi-native-console that referenced this pull request Aug 5, 2026
Realign apps/server-nestjs/test/*.e2e-spec.ts with DSO orchestration
docs and fix E2E gating. Test-only change; no production source.

Source fixes tracked separately and must merge first:
- cloud-pi-native#2403 sonarqube user creation uses project.owner.email
- cloud-pi-native#2405 gitlab token/user hardening (ciConfigPath, canCreateGroup,
  forceRandomPassword, projectsLimit, 1-year expiry, revoke old tokens)
- cloud-pi-native#2406 vault group restructure and techRO path divergence

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
shikanime added a commit to shikanime/cloud-pi-native-console that referenced this pull request Aug 5, 2026
Realign apps/server-nestjs/test/*.e2e-spec.ts with DSO orchestration
docs and fix E2E gating. Test-only change; no production source.

Source fixes tracked separately and must merge first:
- cloud-pi-native#2403 sonarqube user creation uses project.owner.email
- cloud-pi-native#2405 gitlab token/user hardening (ciConfigPath, canCreateGroup,
  forceRandomPassword, projectsLimit, 1-year expiry, revoke old tokens)
- cloud-pi-native#2406 vault group restructure and techRO path divergence

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants