Projet

Général

Profil

Development #56295

Le code de provisionning tronque les noms des rôles à 70 caractères provoquant des collisions difficiles à voir

Ajouté par Benjamin Dauvergne il y a plus de 2 ans. Mis à jour il y a plus de 2 ans.

Statut:
Fermé
Priorité:
Normal
Assigné à:
Catégorie:
-
Version cible:
-
Début:
20 août 2021
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Coté authentic on a une limite de name à 256 caractères.

Coté brique Django la limite est désormais 150 (et à l'époque de #10481 qui a introduit ça, quasiment une ère différente), c'était 80.

Je n'ai rien contre le fait de tronquer mais il faudrait ajouter un hash à minima pour éviter les collisions.

Je remarque au passage que l'ordre du provisionning n'est pas bon, en cas de provisionning full on doit d'abord supprimer puis créer pour éviter de se marcher sur les pieds lors d'une suppression + renommage qui ne serait pas bien passée du premier coup (mail il y a un ticket de Manu autour de ce code donc je vais attendre avant de lui introduire un problème de rebasage) et qu'il faudrait régler via un provisionning full.


Fichiers

Révisions associées

Révision 739bffb7 (diff)
Ajouté par Benjamin Dauvergne il y a plus de 2 ans

provisionning: add hex digest to truncated role's name (#56295)

Max length of role's name is also augmented to 150 starting with Django
2.2.

Historique

#2

Mis à jour par Frédéric Péters il y a plus de 2 ans

Can happen if uuid and name already exist

Ça fait imaginer que l'unicité (et donc la collision) se joue sur (uuid, name), mais en fait non, l'unicité se joue également sur "name" seul, (déjà en django 1.8),

name = models.CharField(_('name'), max_length=150, unique=True)

Donc oui tout à fait inclure quelques caractères de hash pour faire la différence; et comme on est à 150 caractères depuis django 1.11 on peut en profiter.

#3

Mis à jour par Frédéric Péters il y a plus de 2 ans

Aussi, dans #10481 tu proposais :

Est-ce que j'ouvrirai un autre ticket pour ajouter un champ texte libre avec le nom complet à l'objet hobo.agent.common.models.Role ?

et je répondais bien naïvement que "Non, on ne doit pas avoir de rôle au nom aussi long."

Donc vu comme authentic permet ça, ça arrive, ça pourrait donc bien être au final utile d'avoir le nom long réel dispo.

#4

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

Frédéric Péters a écrit :

Can happen if uuid and name already exist

On peut avoir les deux (d'où ma remarque sur l'ordre suppression/renommage), Group a un index d'unicité sur name et Role qui hérite de Group sur uuid, mais pas sur la paire (ce n'est de toute façon pas souhaitable). On pourrait avoir un role uuid=1234 name=toto supprimé, et un rôle uiid=4567 name=titi renommé en toto, et en provisionning full ça va foirer, on va récupérer uuid=4567 tenter de le renommer en toto et uuid=1234, qui devrait être supprimé, va bloquer l'update.

Donc oui tout à fait inclure quelques caractères de hash pour faire la différence; et comme on est à 150 caractères depuis django 1.11 on peut en profiter.

Ok.

#5

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

Frédéric Péters a écrit :

et je répondais bien naïvement que "Non, on ne doit pas avoir de rôle au nom aussi long."

Donc vu comme authentic permet ça, ça arrive, ça pourrait donc bien être au final utile d'avoir le nom long réel dispo.

Allonger Group.name dans toutes les briques n'est pas évident, je veux bien d'abord faire ce changement et voir plus tard pour allonger les noms éventuellement, on vit déjà avec des ' (....)' sans trop de douleur et de toute façon si on allonge ça se corrigera tout seul lors d'un provisionning full.

#6

Mis à jour par Frédéric Péters il y a plus de 2 ans

Allonger Group.name dans toutes les briques n'est pas évident (...)

Tu parlais d'ajouter une colonne nom complet, ce qui peut se gérer dans hobo seul, pas de modifier la colonne name partout.

#7

Mis à jour par Frédéric Péters il y a plus de 2 ans

(mais c'est un détail de confort pour une situation de nom de rôle bien trop long, ce qui doit rester rare, zappons ça).

#8

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

Dans le doute j'ai quand même fait en sorte que ça n'explose pas en Django 1.11. Concernant l'ajout d'un attribut full_name à Role ça ne poserait effectivement pas de problème, mais il ne sera pas magiquement utilisé par tous les codes qui n'utilisent que le modèle Group (ça se monkeypatch mais pour un problème exceptionnel ça fait beaucoup de bordel pour rien).

#9

Mis à jour par Serghei Mihai il y a plus de 2 ans

  • Statut changé de Solution proposée à Solution validée
#10

Mis à jour par Benjamin Dauvergne il y a plus de 2 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit 739bffb78b6dfd48e084f2433cdb225165483a7a
Author: Benjamin Dauvergne <bdauvergne@entrouvert.com>
Date:   Sun Aug 22 15:10:41 2021 +0200

    provisionning: add hex digest to truncated role's name (#56295)

    Max length of role's name is also augmented to 150 starting with Django
    2.2.
#11

Mis à jour par Frédéric Péters il y a plus de 2 ans

  • Statut changé de Résolu (à déployer) à Solution déployée

Formats disponibles : Atom PDF