fix(server-nestjs): make Keycloak root group creation idempotent - #2623
Open
shikanime wants to merge 2 commits into
Open
fix(server-nestjs): make Keycloak root group creation idempotent#2623shikanime wants to merge 2 commits into
shikanime wants to merge 2 commits into
Conversation
shikanime
force-pushed
the
fix/keycloak-root-group-idempotent
branch
from
August 28, 2026 11:39
bceb1cd to
dd18554
Compare
Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: Iff465b31836b84656a084678aeec082a6a6a6964
… race) Refs #2618 Co-authored-by: Automata <automata@shikanime.studio> Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I5944dfc5a50f280ef836ce9e9a136f366a6a6964
shikanime
force-pushed
the
fix/keycloak-root-group-idempotent
branch
from
August 28, 2026 14:20
dd18554 to
0a0efb2
Compare
Member
Author
Updated reviewThe fast-path read at line 187 is intentional — when the root group exists but the full path does not (e.g. I initially flagged the read as redundant, and it IS technically redundant for single-part paths ( The PR is correct as-is. |
shikanime
commented
Aug 28, 2026
Comment on lines
-187
to
+198
| let current = await this.getRootGroupByName(rootName) ?? await this.createGroup(rootName) | ||
| let current = await this.getRootGroupByName(rootName) ?? await this.ensureGroup(rootName) |
Member
Author
There was a problem hiding this comment.
getRootGroupByName is probably not needed, since ensureGroup also do that
shikanime
marked this pull request as ready for review
August 28, 2026 15:39
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issues liées
#2618 (fermer délibérément après la fusion)
Quel est le comportement actuel ?
La création du groupe racine Keycloak n'est pas idempotente face à une course concurrente.
getOrCreateGroupByPathlit d'abord le groupe racine, puis appellecreateGroupsi celui-ci est absent. Or deux réconciliations simultanées (la synchronisation cron et unproject.upsert) peuvent toutes deux passer la lecture avant qu'aucune n'ait créé le groupe : la seconde échoue alors avec une erreur HTTP 409, car le groupe existe désormais.createGroupne tolérait pas ce 409 et remontait l'erreur au lieu de récupérer le groupe déjà créé.Comportement attendu
Un 409 sur la création du groupe racine doit être traité comme « le groupe existe déjà » : le groupe existant est re-consulté via
getRootGroupByNamepuis renvoyé, exactement de la même manière que le chemin des sous-groupes (getOrCreateSubGroupByName) le fait déjà. Aucun autre comportement ne change.Changements
client.groups.createdanscreateGroupavec une tolérance au 409 : en cas de 409, re-consulter le groupe racine existant et le renvoyer.