Repository navigation
feat(nginx-strangler): route /api/v1/admin/roles to server-nestjs - #2782
Conversation
Vérification de bascule (bout en bout, locale)Test réalisé avec le binaire nginx réel, la configuration
7/7 vérifications OK. CI : 32/32 checks verts. |
a3d642f to
cf607cc
Compare
cf607cc to
6a72d66
Compare
The merge-base changed after approval.
7c4eeb2 to
19b5b08
Compare
6a3a204 to
8c2dd1a
Compare
8c2dd1a to
a560402
Compare
|
Rebase et déblocage du prérequis effectués sur
Les mutations |
a560402 to
8c5749d
Compare
|
Conflit résolu — branche reconstruite sur Résolution : les deux blocs Aucune dépendance git envers #2749 : fichiers disjoints. (Voir #2781 pour le bug NaN qui motive cette bascule.) |
|
🤖 Hey ! A preview of the application is available at : https://console-pr-2782.dso.cpin-hp.numerique-interieur.fr Please be patient, deployment may take a few minutes. |
StephaneTrebel
left a comment
There was a problem hiding this comment.
🔴 La règle de routage corrige bien le chemin de lecture member-counts, et les tests de décompte sont couverts par #2783. Mais cette location route également toutes les mutations admin-role vers NestJS alors que le pont d’événements #2749 est encore en brouillon, en modifications demandées et en retard sur main ; sans ce pont, les synchronisations Keycloak/GitLab ne sont pas garanties. Je demande la livraison et la validation de #2749 avant cette bascule ; le fil inline correspondant est actualisé.
StephaneTrebel
left a comment
There was a problem hiding this comment.
Verdict : changements demandés. La location traite bien la route des décomptes, mais son préfixe bascule aussi toutes les mutations admin-role vers NestJS. Le pont #2749 qui garantit les synchronisations Keycloak/GitLab est encore non validé, en retard sur main et comporte des points bloquants ouverts ; cette bascule ne peut donc pas être livrée seule pour la 9.27.0.
Le point sera traite en parallel, dans l'actuel on utilise les "Reprovionner project" depuis le debut
StephaneTrebel
left a comment
There was a problem hiding this comment.
Verdict : changements demandés. La règle de routage est correcte et sa CI est verte, et le head actualisé de #2749 corrige les deux défauts fonctionnels précédemment signalés. Mais #2749 n’est pas encore rebasée/validée ni fusionnée ; le ledger #2781 exige explicitement cette fusion avant la bascule, qui route aussi les mutations admin-role.
StephaneTrebel
left a comment
There was a problem hiding this comment.
Verdict : changements demandés. Le prérequis #2749 est maintenant rebasé, validé par les checks et approuvé ; il n’est toutefois pas encore fusionné, alors que le ledger #2781 exige qu’il le soit avant la bascule. Cette branche de routage est par ailleurs à 2 commits derrière main (dont la mise à jour contractuelle #2834) : rebase, CI à jour, puis fusion après #2749.
Design: adds the /api/v1/admin/roles location to the migrated-routes block, ahead of the general /api/v1/admin fallback, so role reads and mutations reach the NestJS admin-role module whose member-counts implementation is contract-valid (unlike the frozen legacy handler, see #2781). Resolution carries main's concurrent /api/v1/auth migration verbatim; both blocks kept. Refs #2781 Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I9cd3971dbaa619a997c1fe08d2a3438e6a6a6964
|
StephaneTrebel
left a comment
There was a problem hiding this comment.
Verdict : approuvée. La location route correctement l’ensemble du préfixe admin-role vers NestJS ; le pont prérequis #2749 est maintenant fusionné, et #2783 fournit les tests de décompte. La branche est à jour sur main et les checks, dont Sonar, sont verts. Ledger #2781 : les tests sont livrés ; routage et correctif sont couverts par ce diff, la communication d’équipe reste non vérifiable.

0 New Issues
0 Fixed Issues
0 Accepted Issues
No data about coverage (48.80% Estimated after merge)
Issues liées
Refs #2781
Empilée via GitHub stacks : pile #2751 (
#2749 → #2738 → #2739 → cette PR), base =pr/user-strangler. L'ordre de fusion est porté par la pile (retarget automatique au fil des fusions) ; prérequis de contenu : #2749 (cf. #2723).Quel est le comportement actuel ?
/api/v1/admin/rolesest encore servie parapps/server(aucunelocationdédiée dans le nginx-strangler). Le handler legacycountRolesMembersproduit desNaNdès qu'un utilisateur porte un rôle admin adossé à OIDC : la validation de réponse échoue etGET /api/v1/admin/roles/member-countsrenvoie une 500 (détail et reproduction dans #2781).Quel est le nouveau comportement ?
Ajout de la
location /api/v1/admin/rolesdansapps/nginx-strangler/conf.d/routing.confversserver-nestjs, dontgetAdminRoleMemberCountsexclut déjà les ids OIDC (aucune valeurNaN). Les tests verrouillant le décompte sont portés par #2783 (PR indépendante, applicable surmain).Cette PR introduit-elle un breaking change ?
Non.
Autres informations
nginx -tOK sur la configuration modifiée ; suite unitaireserver-nestjsverte (80 fichiers, 665 tests, sur basemain) ; lint OK ; 0 erreur TypeScript sur les fichiers modifiés.admin-role: le test de décompte vit donc dans test(admin-role): lock member counts against OIDC role ids #2783 (indépendant), et cette couche se limite au routage (+9 lignes).locationpuisnginx -s reload(procédure documentée en tête derouting.conf).