Skip to content

chore(nginx-strangler): route /api/v1/clusters to server-nestjs - #2756

Open
shikanime wants to merge 1 commit into
pr/cluster-migrationfrom
pr/nginx-cluster-route
Open

shikanime wants to merge 1 commit into
pr/cluster-migrationfrom
pr/nginx-cluster-route

Conversation

@shikanime

@shikanime shikanime commented Sep 18, 2026 •

Copy link
Copy Markdown
Member

Issues liées

Quel est le comportement actuel ?

The cluster module migration (#2495) lives in server-nestjs; the route flip is cutover config and ships independently so the migration PR stays code-only. The shared ClusterList response type was exported but unwired.

Quel est le nouveau comportement ?

Route /api/v1/clusters traffic to server-nestjs in the strangler nginx config, and retype the cluster controller list route to the contract-derived ClusterList (map-records-to-contract pattern, as in the user-tokens migration).

@github-actions github-actions Bot added the built label Sep 18, 2026
@shikanime shikanime added this to the 9.27.0 milestone Sep 24, 2026
@shikanime
shikanime marked this pull request as ready for review September 24, 2026 08:47
@shikanime
shikanime requested a review from a team as a code owner September 24, 2026 08:47
@shikanime
shikanime force-pushed the pr/nginx-cluster-route branch from 4043955 to adb9434 Compare September 24, 2026 08:57
@shikanime
shikanime changed the base branch from main to pr/cluster-migration September 24, 2026 08:57
@shikanime
shikanime added this pull request to stack #2774 September 24, 2026 08:58
@shikanime shikanime self-assigned this Sep 24, 2026
@shikanime shikanime added the preview Deploy preview app with Argo-cd label Sep 24, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🤖 Hey !

A preview of the application is available at : https://console-pr-2756.dso.cpin-hp.numerique-interieur.fr

Please be patient, deployment may take a few minutes.

StephaneTrebel
StephaneTrebel previously approved these changes Sep 24, 2026
StephaneTrebel
StephaneTrebel previously approved these changes Sep 25, 2026
@shikanime

Copy link
Copy Markdown
Member Author

PR réduite à la bascule nginx.

Le câblage ClusterModule (dont ConfigModule.forFeature(baseConfigFactory), manquant côté service) a été déplacé vers #2495. Les commits de déclenchement CI et le workflow d'alias d'image temporaire sont supprimés.

Diff actuel : apps/nginx-strangler/conf.d/routing.conf uniquement.

@shikanime shikanime removed the preview Deploy preview app with Argo-cd label Sep 28, 2026
@shikanime shikanime moved this from Ready to In review in Cloud Pi Native Sep 29, 2026
@shikanime
shikanime requested review from a team and StephaneTrebel and removed request for StephaneTrebel September 29, 2026 13:07
StephaneTrebel
StephaneTrebel previously approved these changes Sep 30, 2026
@shikanime

Copy link
Copy Markdown
Member Author

Vérifié en local (podman + nginx:alpine, config substituée puis servie telle quelle) :

  • /api/v1/clusters → NESTJS
  • /api/v1/clusters/c-123 → NESTJS
  • /api/v1/clusters/ → NESTJS
  • /api/v1/clusters?x=1 → NESTJS
  • /api/v1/other → LEGACY

Le prefix location /api/v1/clusters capte bien les sous-chemins et la query string, et le fallback /api/ reste sur le legacy. La CI continue de valider la syntaxe via nginx -t.

@shikanime
shikanime force-pushed the pr/nginx-cluster-route branch from 9379951 to 935ac58 Compare October 2, 2026 08:16
@iliesmrf iliesmrf modified the milestones: 9.27.0, 9.28.0 Oct 2, 2026
@shikanime
shikanime force-pushed the pr/nginx-cluster-route branch from 935ac58 to c909074 Compare October 2, 2026 11:56
@shikanime
shikanime force-pushed the pr/nginx-cluster-route branch from c909074 to 4de5fa4 Compare October 2, 2026 12:09

@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.

La bascule ne doit pas être fusionnée avant la correction de la migration parente #2495.

}

# ── Routes migrées vers NestJS ────────────────────────────────────────────────
location /api/v1/clusters {

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.

🔴 Cette bascule exposerait le défaut bloquant de #2495 : les clusterId ne sont pas validés par le contrôleur NestJS, contrairement au contrat et au legacy. Les URL malformées atteindraient Prisma et peuvent répondre 500 au lieu de 400. Attendre la correction et sa couverture HTTP dans #2495 avant de router le trafic ici.

@shikanime
shikanime force-pushed the pr/nginx-cluster-route branch 2 times, most recently from ac7fc8d to 902b407 Compare October 2, 2026 15:09
@shikanime
shikanime force-pushed the pr/nginx-cluster-route branch 2 times, most recently from a28ad2f to cb4fe06 Compare October 7, 2026 15:54

@shikanime shikanime left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Revue du head cb4fe06 (rebasé sur le nouveau head de #2495). Le bloc location /api/v1/clusters vers server-nestjs reprend exactement le gabarit des bascules existantes (proxy_pass, headers Forwarded), préfixe (non =) correct puisque les sous-routes /usage/:id et /:id/environments doivent suivre. Le prérequis du corps — portage des listeners cluster.upsert/cluster.delete dans la même vague — est désormais réel : #2495 head 17a9b3b0 contient bien les @OnEvent dans argocd.service.ts. Format What/Why/References conforme, dépendance #2495 explicite. Rien de bloquant ; un seul nit : le type ClusterList ajouté dans packages/shared est du code mort dans cette PR de bascule.

Résumé des sévérités : 0 bloquant, 0 important, 1 nit.

})

export type ClusterAssociatedEnvironments = ClientInferResponseBody<typeof clusterContract.getClusterEnvironments, 200>
export type ClusterList = ClientInferResponseBody<typeof clusterContract.listClusters, 200>

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

nit — ClusterList n'est importé nulle part sur ce head (vérifié sur l'arbre cb4fe06 : apps/client, apps/server-nestjs, packages — seule la définition existe). Cette PR est présentée comme du « cutover config » ; soit supprimer cet export, soit le déplacer dans #2495 avec son consommateur, soit le justifier dans le corps. Du code mort dans une PR de routage nginx, c'est exactement ce que le corps promet d'éviter (« stays code-only »).

@shikanime
shikanime force-pushed the pr/nginx-cluster-route branch 3 times, most recently from 0b9a4c6 to 38ce05a Compare October 9, 2026 13:08
@shikanime shikanime closed this Oct 9, 2026
@shikanime shikanime reopened this Oct 9, 2026
@shikanime shikanime closed this Oct 9, 2026
@shikanime shikanime reopened this Oct 9, 2026
@shikanime

Copy link
Copy Markdown
Member Author

Superseded by #2847 — rebased directly onto main (nginx config only).

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

Co-authored-by: Automata <automata@shikanime.studio>
Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr>
Change-Id: If05c8547128ca15bfbf4e20dc3d88a236a6a6964

Signed-off-by: Shikanime Deva <22115108+shikanime@users.noreply.github.com>
@cloud-pi-native-sonarqube

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

built technical debt Résoud de la dette technique

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants