fix(observability): restore legacy dual-bucket Grafana RBAC sync - #2688
Merged
Conversation
shikanime
force-pushed
the
fix/observability-dual-bucket
branch
from
September 8, 2026 09:06
b6fbf16 to
ca0cc82
Compare
5 tasks
shikanime
force-pushed
the
fix/observability-dual-bucket
branch
from
September 8, 2026 09:11
ca0cc82 to
b67e78b
Compare
shikanime
marked this pull request as ready for review
September 8, 2026 09:17
Contributor
|
🤖 Hey ! A preview of the application is available at : https://console-pr-2688.dso.cpin-hp.numerique-interieur.fr Please be patient, deployment may take a few minutes. |
shikanime
force-pushed
the
fix/observability-dual-bucket
branch
3 times, most recently
from
September 8, 2026 10:14
d83efec to
1950b3f
Compare
shikanime
marked this pull request as draft
September 8, 2026 10:51
shikanime
force-pushed
the
fix/observability-dual-bucket
branch
from
September 8, 2026 11:47
1950b3f to
08f3ece
Compare
shikanime
commented
Sep 8, 2026
shikanime
left a comment
shikanime
left a comment
Member
Author
There was a problem hiding this comment.
Verdict : Approuvé (auto-revue — les deux ajustements ci-dessous sont appliqués dans le commit qui suit)
Le remplissage par bucket restaure fidèlement la sémantique legacy, les specs unitaires verrouillent la parité dans les deux sens, et le spec e2e rejoue la chaîne complète contre Keycloak réel. L'extraction de addUserToBucket améliore nettement la lisibilité de getListPerms sans changer le comportement.
shikanime
force-pushed
the
fix/observability-dual-bucket
branch
2 times, most recently
from
September 8, 2026 12:09
c773596 to
1997bf4
Compare
shikanime
marked this pull request as ready for review
September 8, 2026 12:11
shikanime
force-pushed
the
fix/observability-dual-bucket
branch
from
September 8, 2026 12:15
1997bf4 to
4f30572
Compare
Member
Author
|
Contributor
|
🤖 Hey ! A preview of the application is available at : https://console-pr-2688.dso.cpin-hp.numerique-interieur.fr Please be patient, deployment may take a few minutes. |
shikanime
enabled auto-merge
September 8, 2026 12:50
shikanime
marked this pull request as draft
September 8, 2026 13:06
auto-merge was automatically disabled
September 8, 2026 13:06
Pull request was converted to draft
getListPerms filled only one stage bucket pair (prod if any prod env exists, else hors-prod), where the legacy plugin filled both. Since reconcileGroupMembership removes members absent from the desired list, the first project.upsert under server-nestjs wiped every membership of the opposite pair — and Grafana maps org roles from exactly those Keycloak groups, so users deterministically lost access. Restore per-stage bucketing: a user lands in prod when a prod env exists AND in hors-prod when a real (named) non-prod stage exists (RBAC doc §3: 'Les deux peuvent coexister'). Fix the spec assertions that codified the single-bucket contract and add a reconcile regression guard. Extends the observability service spec with an E2E-gated suite that drives the production event chain (event bus -> AppEventsService -> ObservabilityService -> getListPerms -> real Keycloak from the dev stack) and asserts final group membership against real Postgres + Keycloak, regression-locking the dual-bucket fill end to end. Refs #2686 Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: Ic8c8e2f352d164737f7a8ea3635e3d1b6a6a6964
shikanime
force-pushed
the
fix/observability-dual-bucket
branch
from
September 8, 2026 13:12
4f30572 to
5e1c760
Compare
shikanime
marked this pull request as ready for review
September 8, 2026 13:13
|
0 New Issues
0 Fixed Issues
0 Accepted Issues
shikanime
enabled auto-merge
September 8, 2026 13:20
shikanime
added a commit
that referenced
this pull request
Sep 8, 2026
Each e2e spec keeps only its describe body; the E2E gate, its describe.runIf alias (and, for observability, the shared Grafana subgroup constants) move to a sibling <name>.utils.ts. The vitest include globs only match *.e2e-spec.ts, so utils files stay out of both unit and gated runs. Stacked on fix/observability-dual-bucket (PR #2688). Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I602e5529ac2cea360ce6025d4f06e0b9efe359e9
StephaneTrebel
approved these changes
Sep 8, 2026
shikanime
added a commit
that referenced
this pull request
Sep 9, 2026
Each e2e spec keeps only its describe body; the E2E gate, its describe.runIf alias (and, for observability, the shared Grafana subgroup constants) move to a sibling <name>.utils.ts. The vitest include globs only match *.e2e-spec.ts, so utils files stay out of both unit and gated runs. Stacked on fix/observability-dual-bucket (PR #2688). Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I602e5529ac2cea360ce6025d4f06e0b9efe359e9
shikanime
added a commit
that referenced
this pull request
Sep 9, 2026
Each e2e spec keeps only its describe body; the E2E gate, its describe.runIf alias (and, for observability, the shared Grafana subgroup constants) move to a sibling <name>.utils.ts. The vitest include globs only match *.e2e-spec.ts, so utils files stay out of both unit and gated runs. Stacked on fix/observability-dual-bucket (PR #2688). Signed-off-by: William Phetsinorath <william.phetsinorath-open@interieur.gouv.fr> Change-Id: I602e5529ac2cea360ce6025d4f06e0b9efe359e9
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
2 participants
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
Issues numéro: Refs #2686
Quel est le comportement actuel ?
Depuis la migration du plugin observability vers server-nestjs (#2418),
getListPerms(apps/server-nestjs/src/modules/observability/observability.utils.ts) range chaque utilisateur dans une seule paire de groupes Keycloak Grafana : prod si au moins un environnement prod existe, hors-prod sinon. Le plugin historique (console-plugin-observability) rangeait chaque utilisateur dans les deux paires. CommereconcileGroupMembershipest une sync diff-and-remove, le premierproject.upsertsous server-nestjs a retiré tous les membres du groupe opposé — et Grafana associe ses rôles org à partir de ces groupes : perte d'accès déterministe.Quel est le nouveau comportement ?
Bucketing par stage restauré (parité legacy, doc RBAC §3 : « Les deux peuvent coexister ») : un utilisateur apparaît dans la paire prod si un environnement prod existe et dans la paire hors-prod si un environnement non-prod existe. Les assertions du spec unitaire qui codefaient la sémantique mono-bucket sont corrigées, et un test de non-régression garantit que
reconcileGroupMembershipne retire personne quand les deux stages existent. Un spec e2e (test/observability-sync.e2e-spec.ts) rejoue la chaîne complète — événementproject.upsert→AppEventsService→ObservabilityService→getListPerms→ Keycloak en mémoire — et vérifie l'état final des groupes, pas les appels.Cette PR introduit-elle un breaking change ?
Non. Le correctif restaure le comportement documenté avant migration. Remédiation des instances affectées : rejouer
project.upsertpar projet concerné — la sync ré-ajoute les adhésions détruites (idempotent).Autres informations
grafana/*confirmés par la doc RBAC) : 🐛 [BUG] - Perte d'accès Grafana : getListPerms ne remplit plus qu'une seule paire de groupes Keycloak (prod XOR hors-prod) #2686 (commentaires)tsc --noEmità parité avec main, eslint propre sur les fichiers modifiés. Le remplissage par bucket est extrait dansaddUserToBucket(complexité cognitive SonarQube degetListPerms: 21 → sous le seuil de 15), comportement inchangé