refactor: conditional module activation by shikanime · Pull Request #2381 · cloud-pi-native/console

shikanime · 2026-07-28T13:28:18Z

Signed-off-by: William Phetsinorath william.phetsinorath-open@interieur.gouv.fr
Change-Id: I6f501bd1633ceb14edaa9bf2a324ea026a6a6964

Issues liées

Issues numéro: #2380

Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

github-actions · 2026-07-28T14:03:49Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T15:22:32Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T15:26:07Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T15:27:12Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T15:29:16Z

🤖 Hey !

The security scan report for the current pull request is available here.

shikanime · 2026-07-28T15:32:01Z

Review: PR #2381 — conditional module activation

Production refactor is sound: builds, full suite green (407 passed, e2e skipped), all new specs pass, DI resolution is correct. One blocker on test design, two nits, one info item. (Posting as a comment since GitHub won't let me request-changes on my own PR.)

WARNING — plugin.module.spec.ts tests the harness, not the module

apps/server-nestjs/src/modules/plugin/plugin.module.spec.ts wraps PluginModule in ConditionalModule.registerWhen(PluginModule, 'USE_X') and asserts PluginService is absent when USE_X=false. That only proves @nestjs/config's registerWhen gates the whole module — it never exercises plugin.module.ts's internal conditional imports (registerWhen(ArgoCDModule,'USE_ARGOCD'), etc.). In production main.module.ts imports PluginModule directly (unconditional), so this scenario never occurs.

Worse, the USE_KEYCLOAK (L34) and USE_VAULT (L58) cases assert gates that do not exist in plugin.module.ts — KeycloakModule and VaultModule are imported unconditionally there. Those two tests pass only because of the harness wrap and actively mislead a future reader into thinking PluginModule is gated by those flags.

Fix: load PluginModule directly (drop the harness registerWhen wrapper) and assert the downstream services' presence/absence, e.g. module.get(ArgoCDService) present when USE_ARGOCD=true / throws when false — mirroring the correct pattern already in argocd.module.spec.ts. Remove the USE_KEYCLOAK/USE_VAULT cases (or add the real gates to plugin.module.ts if gating was intended).

NIT — CRLF line endings

apps/server-nestjs/src/modules/service-chain/service-chain-health.service.ts is committed with CRLF (confirmed: blob i/crlf, 30 CR bytes) while the rest of the repo is LF. No prettier/eslint gate enforces it here, but normalize to LF for consistency.

NIT — redundant ConfigModule.forRoot()

healthz.module.ts:18 calls ConfigModule.forRoot(). ConfigModule is global and ConfigurationModule already calls forRoot({...}) with options. The explicit import/forRoot in healthz is redundant — drop it and rely on the global instance.

INFO — health probe key rename (possible minor breaking change)

healthz.controller previously reported the OpenCds probe under key 'opencds'; HealthzService.check() now passes 'serviceChain'. Confirm no external monitor/alert keys on 'opencds' in the health payload before merging.

Verified OK

@nestjs/config@4.0.3 provides ConditionalModule — version fine.
All XxxHealthService providers are exported by their modules; @Optional() DI in HealthzService is correct (probe omitted when its module is not loaded).
No dangling refs to deleted OpenCdsModule/OpenCdsHealthService.
No security issues; controller auth posture unchanged (was already unguarded).

github-actions · 2026-07-28T16:18:00Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T16:49:12Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T16:55:30Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T17:03:29Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T17:46:20Z

🤖 Hey !

The security scan report for the current pull request is available here.

github-actions · 2026-07-28T17:54:24Z

🤖 Hey !

The security scan report for the current pull request is available here.

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I6f501bd1633ceb14edaa9bf2a324ea026a6a6964

cloud-pi-native-sonarqube · 2026-07-30T14:23:49Z

Quality Gate passed

Issues

Measures

Project ID: cloud-pi-native_console_AYsa46O31ebDtzs-pYyZ

View in SonarQube

shikanime self-assigned this Jul 28, 2026

shikanime added the tech Technical issue label Jul 28, 2026

shikanime added this to the 9.23.0 milestone Jul 28, 2026

github-code-quality Bot found potential problems Jul 28, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/main.module.ts Fixed

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch 2 times, most recently from 546e3d4 to a5e314e Compare July 28, 2026 13:55

github-actions Bot added the built label Jul 28, 2026

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch 4 times, most recently from 5e2c7bd to 477e838 Compare July 28, 2026 15:11

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch from 477e838 to cd5ac06 Compare July 28, 2026 16:07

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch 2 times, most recently from a14484a to 6100723 Compare July 28, 2026 16:49

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch from 6100723 to bf84e68 Compare July 28, 2026 16:56

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch from bf84e68 to 6bb5e00 Compare July 28, 2026 17:21

github-code-quality Bot found potential problems Jul 28, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/modules/service-chain/service-chain.module.ts Fixed

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch from 6bb5e00 to 80f347b Compare July 28, 2026 17:47

StephaneTrebel reviewed Jul 29, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/modules/healthz/healthz.service.spec.ts

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch from 471ddc1 to d5319ac Compare July 29, 2026 15:29

StephaneTrebel self-requested a review July 29, 2026 15:30

StephaneTrebel previously approved these changes Jul 29, 2026

View reviewed changes

shikanime dismissed StephaneTrebel’s stale review via ae07f22 July 29, 2026 15:36

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch 6 times, most recently from 5d44868 to c568328 Compare July 29, 2026 16:35

KepoParis previously approved these changes Jul 29, 2026

View reviewed changes

shikanime dismissed KepoParis’s stale review via c474296 July 29, 2026 16:42

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch 2 times, most recently from c474296 to 13d3d55 Compare July 29, 2026 16:44

shikanime requested review from KepoParis and StephaneTrebel July 29, 2026 16:45

shikanime force-pushed the shikanime/push-yztnkuuslsmy branch 4 times, most recently from ba391c5 to 5e9fe49 Compare July 29, 2026 17:00

KepoParis previously approved these changes Jul 29, 2026

View reviewed changes

StephaneTrebel requested changes Jul 30, 2026

View reviewed changes

Comment thread apps/server-nestjs/.env.docker-example Outdated

Comment thread apps/server-nestjs/.env-example Outdated

Comment thread apps/server-nestjs/src/main.module.ts Outdated

shikanime commented Jul 30, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/modules/healthz/healthz.module.ts

github-code-quality Bot found potential problems Jul 30, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/main.module.ts Fixed

github-code-quality Bot found potential problems Jul 30, 2026

View reviewed changes

Comment thread apps/server-nestjs/src/modules/plugin/plugin.module.spec.ts Fixed

refactor: conditional module activation

3312fe9

Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I6f501bd1633ceb14edaa9bf2a324ea026a6a6964

StephaneTrebel approved these changes Jul 30, 2026

View reviewed changes

shikanime mentioned this pull request Jul 30, 2026

chore: add nestjs linter #2395

Closed

Uh oh!

Conversation

shikanime commented Jul 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issues liées

Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

Uh oh!

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

shikanime commented Jul 28, 2026

Review: PR #2381 — conditional module activation

WARNING — plugin.module.spec.ts tests the harness, not the module

NIT — CRLF line endings

NIT — redundant ConfigModule.forRoot()

INFO — health probe key rename (possible minor breaking change)

Verified OK

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cloud-pi-native-sonarqube Bot commented Jul 30, 2026

Quality Gate passed

Issues

Measures

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

shikanime commented Jul 28, 2026 •

edited

Loading