refactor: healthcheck in service by shikanime · Pull Request #2382 · cloud-pi-native/console

shikanime · 2026-07-28T13:31:37Z

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

Issues liées

Issues numéro: #2290

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:58Z

🤖 Hey !

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

github-actions · 2026-07-28T14:05:53Z

🤖 Hey !

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

github-actions · 2026-07-28T15:21:33Z

🤖 Hey !

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

github-actions · 2026-07-28T15:25:54Z

🤖 Hey !

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

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

🤖 Hey !

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

shikanime · 2026-07-28T15:37:18Z

Review: PR #2382 — refactor: healthcheck in service

Verdict: REQUEST CHANGES (1 runtime blocker CI cannot catch + 1 style regression).

Summary of changes

Migrates the OpenCds healthcheck out of its own OpenCdsModule into ServiceChainModule (renamed OpenCdsHealthService → ServiceChainHealthService), and moves the /api/v1/healthz orchestration from the controller into a new HealthzService. The controller now delegates to HealthzService.check(), which injects every *HealthService (most as @Optional()) and pushes them all as checks.

The refactor intent is sound and the mechanical rename/delete is clean (no dangling references to OpenCdsModule/OpenCdsHealthService remain). But two issues block merge.

🔴 Critical (must fix)

1. ServiceChainModule cannot resolve HealthIndicatorService — app will fail to bootstrap
apps/server-nestjs/src/modules/service-chain/service-chain.module.ts now declares ServiceChainHealthService as a provider, and that service injects HealthIndicatorService from @nestjs/terminus (service-chain-health.service.ts:9). The deleted OpenCdsModule previously imported TerminusModule, which provided that token. ServiceChainModule (and AuthModule, EventsModule, UserPermissionModule, ConfigurationModule that it imports) do not import or re-export TerminusModule — DatabaseModule imports it but does not export it.
NestJS resolves a provider's dependencies only from the module's own imports or transitively-exported modules. None of ServiceChainModule's import graph re-exports TerminusModule, so HealthIndicatorService is not available → Nest can't resolve dependencies of the ServiceChainHealthService (?) + HealthIndicatorService at bootstrap.
This is exactly the failure class that green CI misses: build does not resolve the DI graph, and unit tests construct services with isolated mocks so they never hit module wiring.
Fix: add TerminusModule to ServiceChainModule.imports.

🟡 Major (should fix)

2. CRLF line endings introduced on the moved file
apps/server-nestjs/src/modules/service-chain/service-chain-health.service.ts is committed with 30 CRLF line endings (every line), while the file did not have them on origin/main (opencds-health.service.ts = 0 CRLF) and every sibling in service-chain/ is LF-only. This was introduced by the rename/rewrite in this PR. The repo ESLint config has no linebreak-style rule (confirmed), so lint CI stays green, but it's a genuine style regression for a repo that is otherwise strictly LF. Renormalize to LF.

🟢 Minor (nice to have)

HealthzService is no longer config-gated: every check is always pushed and each *HealthService reports 'Not configured' via indicator.down() when its URL is absent. Since HealthCheckService.check() treats any down as overall 503, a deployment without (e.g.) ARGOCD_URL now reports the whole /healthz endpoint red instead of the old behavior where unconfigured optional services were skipped. That may be acceptable for your k8s liveness model, but it is a silent behavioral change from the prior controller (which gated on config.*Url). Worth a conscious decision: either keep down (documented) or gate in HealthzService like before.
ServiceChainHealthService still keys on openCdsUrl/openCdsApiToken from ConfigurationService despite being renamed "ServiceChain" — rename is cosmetic; the health probe is still purely OpenCds. Not a bug, just naming drift.

Testing

No test added for HealthzService (none existed for the prior controller either, so not a regression). The DI blocker above would be caught by a single Test.createTestingModule({ imports: [ServiceChainModule] }) bootstrap test — consider adding one.

Approval decision

REQUEST CHANGES — the TerminusModule import is a bootstrap blocker in production even though all CI is green.

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

🤖 Hey !

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

github-actions · 2026-07-28T16:53:50Z

🤖 Hey !

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

github-actions · 2026-07-28T16:58:07Z

🤖 Hey !

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

github-actions · 2026-07-28T17:31:16Z

🤖 Hey !

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

False positive

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

cloud-pi-native-sonarqube · 2026-07-29T14:20:00Z

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

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch from 2afaa82 to 70282a9 Compare July 28, 2026 13:55

shikanime closed this Jul 28, 2026

shikanime reopened this Jul 28, 2026

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

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch 3 times, most recently from 654729f to ede984c Compare July 28, 2026 15:10

shikanime enabled auto-merge July 28, 2026 15:11

shikanime commented Jul 28, 2026

View reviewed changes

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

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch from ede984c to 21ee6f5 Compare July 28, 2026 16:07

shikanime requested a review from a team July 28, 2026 16:08

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch 2 times, most recently from 3948370 to 8390f7e Compare July 28, 2026 16:49

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch from 8390f7e to 979c65e 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

KepoParis previously requested changes Jul 29, 2026

View reviewed changes

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch from 979c65e to 83d6dee Compare July 29, 2026 09:02

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch 5 times, most recently from 833d742 to 88589a0 Compare July 29, 2026 12:53

StephaneTrebel previously approved these changes Jul 29, 2026

View reviewed changes

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch from 88589a0 to a985be5 Compare July 29, 2026 12:59

StephaneTrebel removed the request for review from KepoParis July 29, 2026 13:10

shikanime added this pull request to the merge queue Jul 29, 2026

github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 29, 2026

shikanime added this pull request to the merge queue Jul 29, 2026

shikanime removed this pull request from the merge queue due to a manual request Jul 29, 2026

shikanime dismissed StephaneTrebel’s stale review via d01ef87 July 29, 2026 13:47

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch from a985be5 to d01ef87 Compare July 29, 2026 13:47

shikanime enabled auto-merge July 29, 2026 13:47

shikanime requested review from KepoParis and StephaneTrebel July 29, 2026 13:48

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

View reviewed changes

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

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch 2 times, most recently from b680260 to a43637b Compare July 29, 2026 14:05

StephaneTrebel approved these changes Jul 29, 2026

View reviewed changes

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch from a43637b to 8d8c7f9 Compare July 29, 2026 14:13

refactor: healthcheck in service

3cc2587

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

shikanime force-pushed the shikanime/push-nyztnkrkysqz branch from 8d8c7f9 to 3cc2587 Compare July 29, 2026 14:13

shikanime added this pull request to the merge queue Jul 29, 2026

Merged via the queue into main with commit f9f2e25 Jul 29, 2026
32 of 33 checks passed

shikanime deleted the shikanime/push-nyztnkrkysqz branch July 29, 2026 14:41

Uh oh!

Conversation

shikanime commented Jul 28, 2026

Issues liées

Quel est le comportement actuel ?

Quel est le nouveau comportement ?

Cette PR introduit-elle un breaking change ?

Autres informations

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 #2382 — refactor: healthcheck in service

Summary of changes

🔴 Critical (must fix)

🟡 Major (should fix)

🟢 Minor (nice to have)

Testing

Approval decision

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!

Uh oh!

github-actions Bot commented Jul 28, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

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

Quality Gate passed

Issues

Measures

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants