Projet

Général

Profil

Development #54078

mode verbeux pour la commande sync-ldap-users

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

Statut:
Fermé
Priorité:
Normal
Assigné à:
Catégorie:
-
Version cible:
-
Début:
18 mai 2021
Echéance:
% réalisé:

0%

Temps estimé:
Patch proposed:
Oui
Planning:
Non

Description

Des gens exécutent

authentic2-multitenant-manage tenant_command sync-ldap-users -d iile-auth.guichet-citoyen.be

et ça n'affiche rien et "rien se passe" et j'aimerais pouvoir leur dire qu'ajouter -v 2 leur produira des informations utiles.


Fichiers

Révisions associées

Révision 50f9c714 (diff)
Ajouté par Valentin Deniaud il y a plus de 2 ans

ldap: add useful output to sync-ldap-users command (#54078)

Historique

#2

Mis à jour par Valentin Deniaud il y a presque 3 ans

Passer -v 2 affiche déjà des trucs,

        if verbosity > 1:
            print('Updated users :')
        for user in LDAPBackend.get_users():
            if getattr(user, '_changed', False) and verbosity > 1:
                print(' -', user.uuid, user.get_username(), user.get_full_name())

Et également le logger.warning(u'unable to synchronize with LDAP servers %s') dans LDAPBackend.get_users(). Tu avais vu ça ? Il faudrait quoi en plus ?

#3

Mis à jour par Frédéric Péters il y a presque 3 ans

Oui je connais et non ça n'affiche rien d'utile; c'est même pire que tout ça affiche "Updated users :" et ça rend la main. Et quand il y a des utilisateurs ils sont tous affichés d'un coup après un long silence parce que le temps est passé dans LDAPBackend.get_users().

Ce qui aurait été utile pour prendre le cas du ticket lié, en mode très verbeux, ça aurait pu être :

Synchronising users from realm "xxx" (server: ldap://...)
Binding with XXX account: success
Searching for (&(uid=*)(memberOf=CN=Domain Users, CN=Users, DC=xxx, DC=xx))
No results.

i.e. être clair sur la requête effectuée et son résultat. (de manière très utile il y aurait aussi affichage de la commande ldapsearch permettant de reproduire la requête, mais là ça commence à optimiser sur une utilisation ligne de commande, ce qui sera moins pertinent quand la configuration se fera via l'UI).

#4

Mis à jour par Valentin Deniaud il y a plus de 2 ans

  • Assigné à mis à Valentin Deniaud
#5

Mis à jour par Valentin Deniaud il y a plus de 2 ans

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

Ce qui aurait été utile pour prendre le cas du ticket lié, en mode très verbeux

Choix de plutôt avoir cette sortie être le mode normal, le mode verbeux affiche la liste des utilisateurs mis à jour, et possibilité de ne rien afficher en passant -v0.

#6

Mis à jour par Valentin Deniaud il y a plus de 2 ans

  • Statut changé de Solution proposée à En cours

Je me bats avec le système de log qui fuit entre les tests et je reviens

#8

Mis à jour par Paul Marillonnet il y a plus de 2 ans

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

Top, il y a tout plein d’infos.

Peut-être juste différencier la ligne de log pour le binding simple et le binding SASL ? (Je ne crois pas que SASL soit encore utilisé dans nos raccordements mais sait-on jamais.)

#9

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

Je ne capte pas bien comment les infos de logs vont s'afficher à l'écran. (j'aurais imaginé quelque chose qui modifie la config de logging pour assurer un log vers la console, genre).

#10

Mis à jour par Valentin Deniaud il y a plus de 2 ans

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

Je ne capte pas bien comment les infos de logs vont s'afficher à l'écran. (j'aurais imaginé quelque chose qui modifie la config de logging pour assurer un log vers la console, genre).

Bien vu je me suis laissé porté par la config de logging de devinst qui fait ça. Du coup,

+        root_logger.addHandler(logging.StreamHandler())

et ça me semble bon, en testant avec la config Debian.

#11

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

Pour moi il y a beaucoup trop de truc au niveau info, plutôt tout mettre en debug. Ça concerne surtout get_connection(); dans get_users() vu que ça n'est appelé que par la commande ça me dérange moins.

#12

Mis à jour par Valentin Deniaud il y a plus de 2 ans

Benjamin Dauvergne a écrit :

plutôt tout mettre en debug

La sortie est trop polluée, ça donne genre

got config <énorme dictionnaire>
Synchronising users from realm "ldap" 
Binding anonymously: success
Connected to server ldapi://%2Ftmp%2Fslapd-serverqda1981_%2Fsocket
Searching for (|(mail=*)(uid=*))
retrieved attributes for 'uid=mïchu0,o=ôrga': {'dn': 'uid=mïchu0,o=ôrga', 'uid': ['mïchu0'], 'sn': ['Michu'], 'givenname': ['Étienne'], 'l': ['locality0'], 'mail': ['etienne.michu@example.net']}
lookup using external_id ['uid']: 'm%C3%AFchu0'
lookup using external_id ['dn:noquote']: 'uid=mïchu0,o=ôrga'
groups for dn 'uid=mïchu0,o=ôrga': set()
roles for dn 'uid=mïchu0,o=ôrga': set()

pour un seul utilisateur.

La seule solution me paraît être d'inventer un niveau genre logging.INFO - 1 qui empêche ces messages de tomber dans les logs de niveau INFO ?

#13

Mis à jour par Valentin Deniaud il y a plus de 2 ans

Valentin Deniaud a écrit :

La seule solution me paraît être d'inventer un niveau genre logging.INFO - 1 qui empêche ces messages de tomber dans les logs de niveau INFO ?

Qui ne dit mot consent, voici ce patch.

#14

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

  • Statut changé de Solution proposée à En cours

Valentin Deniaud a écrit :

Valentin Deniaud a écrit :

La seule solution me paraît être d'inventer un niveau genre logging.INFO - 1 qui empêche ces messages de tomber dans les logs de niveau INFO ?

Qui ne dit mot consent, voici ce patch.

13 jours on est large, j'ai des tickets sans commentaire pendant des mois ou des années :/

J'aurai plutôt été pour d'abord supprimer des trucs et améliorer les messages de log avant d'inventer des niveaux :
  • got config : c'est déjà du DEBUG, mais je virerai
  • Synchronising users from realm "ldap" : ok INFO
  • Binding anonymously: success : debug si succès, error sinon
  • Connected to server ldapi://%2Ftmp%2Fslapd-serverqda1981_%2Fsocket à merge avec le précédent, ça donne la même info on bind et on connecte dans le même objectif, et donc DEBUG, ça pourrait donner "Binding to server ldaps://... (anonymously/with identity xyz)" sinon Connection error / Binding error
  • Searching for (|(mail=*)(uid=*)) : DEBUG
  • Server returned : INFO mais pareil à joindre au précédent, "Search for ... returned xxx users"
  • retrieved attributes : debug, c'est déjà le cas
  • look up ... / groups for : c'est déjà debug
  • Updated (qui n'est pas dans ton commentaire mais dans le code) : INFO, mais faudrait faire la différence updated/created qui n'est pas faire actuellement et retourner les attributs et rôles mis à jour en cas de mise à jour, faudrait étendre _changed pour un dico plutôt qu'un simple booléen)

Aussi il faudrait unformiser le préfix [%(ldap_url)s] qu'on a dans une partie des logs, sinon ça fait bizarre.

j'aurai dit -v 1 : error, -v 2 warning/info, -v3 debug, en -v 2 on devrait avoir une sortie comme ça et rien d'autre :

2021-09-14 12:07:01 [ldaps://.....] Synchronizing users (realm=...)
2021-09-14 12:07:01 [ldaps://.....] Binded (anonymously/with user xxx/by certificate yyy)
2021-09-14 12:07:01 [ldaps://.....] Created user bdauvergne@entrouvert.com(<uuid>) from uid=bdauvergne,ou=users,... (first_name=Benjamin,last_name=Dauvergne,roles=Toto,Titi)
2021-09-14 12:07:01 [ldaps://.....] Updated user fpeters@entrouvert.com(<uuid>) from uid=bdauvergne,ou=users,... (first_name=-Frederic,+Frédéric,last_name=-Peters,+Péters,roles=+Toto,-Titi)
2021-09-14 12:07:01 [ldaps://.....] Synchronizing done, retrieved N users

#15

Mis à jour par Valentin Deniaud il y a plus de 2 ans

J'ai l'impression que le sujet du ticket se perd, l'idée c'est vraiment d'avoir une sortie lisible et utile à la commande, pour faire ça j'ai choisi d'utiliser et de faire remonter les logs mais c'était peut-être une mauvaise piste vu toutes les modifs que ça semble devoir amener. Je fais encore une tentative cependant.

Benjamin Dauvergne a écrit :

Aussi il faudrait unformiser le préfix [%(ldap_url)s] qu'on a dans une partie des logs, sinon ça fait bizarre.

Ça ne concerne pas la sortie lisible qui nous intéresse, ça irait d'ailleurs dans le mauvais sens, je laisse pour un autre ticket.

  • Binding anonymously: success : debug si succès, error sinon
  • Connected to server ldapi://%2Ftmp%2Fslapd-serverqda1981_%2Fsocket à merge avec le précédent, ça donne la même info on bind et on connecte dans le même objectif, et donc DEBUG, ça pourrait donner "Binding to server ldaps://... (anonymously/with identity xyz)" sinon Connection error / Binding error

OK mais tu le mets plus bas dans la sortie niveau INFO (donc je combine mais laisse niveau INFO).

  • Searching for (|(mail=*)(uid=*)) : DEBUG
  • Server returned : INFO mais pareil à joindre au précédent, "Search for ... returned xxx users"

OK mais comme le filtre peut être long et moche ça diminue la lisibilité de l'info du nombre d'utilisateurs retournés.

  • Updated (qui n'est pas dans ton commentaire mais dans le code) : INFO, mais faudrait faire la différence updated/created

J'ai peur que ça fasse beaucoup mais soit, quelqu'un ouvrira un autre ticket pour râler.

retourner les attributs et rôles mis à jour en cas de mise à jour, faudrait étendre _changed pour un dico plutôt qu'un simple booléen)

Je laisse pour un autre ticket.

j'aurai dit -v 1 : error, -v 2 warning/info, -v3 debug, en -v 2 on devrait avoir une sortie comme ça et rien d'autre :

OK, même si j'aimais bien l'idée qu'il y ait une sortie par défaut.

#16

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

Valentin Deniaud a écrit :

J'ai l'impression que le sujet du ticket se perd, l'idée c'est vraiment d'avoir une sortie lisible et utile à la commande, pour faire ça j'ai choisi d'utiliser et de faire remonter les logs mais c'était peut-être une mauvaise piste vu toutes les modifs que ça semble devoir amener. Je fais encore une tentative cependant.

Benjamin Dauvergne a écrit :

Aussi il faudrait unformiser le préfix [%(ldap_url)s] qu'on a dans une partie des logs, sinon ça fait bizarre.

Ça ne concerne pas la sortie lisible qui nous intéresse, ça irait d'ailleurs dans le mauvais sens, je laisse pour un autre ticket.

Ok, j'aime bien l'idée d'avoir les timestamps ça donne une idée du temps que tout ça prend ou de quand ça arrive pour croiser avec des logs LDAPs.

  • Binding anonymously: success : debug si succès, error sinon
  • Connected to server ldapi://%2Ftmp%2Fslapd-serverqda1981_%2Fsocket à merge avec le précédent, ça donne la même info on bind et on connecte dans le même objectif, et donc DEBUG, ça pourrait donner "Binding to server ldaps://... (anonymously/with identity xyz)" sinon Connection error / Binding error

OK mais tu le mets plus bas dans la sortie niveau INFO (donc je combine mais laisse niveau INFO).

Oui, désolé j'ai parlé des logs existants au fil de l'eau mais quand on les combine ça me parait justifier de les remonter au niveau du plus haut. Pour moi tout ça c'est pour le debug, personne n'en a besoin en général.

  • Searching for (|(mail=*)(uid=*)) : DEBUG
  • Server returned : INFO mais pareil à joindre au précédent, "Search for ... returned xxx users"

OK mais comme le filtre peut être long et moche ça diminue la lisibilité de l'info du nombre d'utilisateurs retournés.

Tu peux le mettre au début alors si tu penses que c'est une information plus importante.

  • Updated (qui n'est pas dans ton commentaire mais dans le code) : INFO, mais faudrait faire la différence updated/created

J'ai peur que ça fasse beaucoup mais soit, quelqu'un ouvrira un autre ticket pour râler.

Dans 99% des cas ça n'affichera rien puisque tout est déjà synchronisé.

j'aurai dit -v 1 : error, -v 2 warning/info, -v3 debug, en -v 2 on devrait avoir une sortie comme ça et rien d'autre :

OK, même si j'aimais bien l'idée qu'il y ait une sortie par défaut.

C'est une commande lancé en cron, s'il y a une sortie par défaut même sur un fonctionnement normal ça va faire des mails ou alors il faudra forcer -v 0 dans les cron, un changement en plus; ça ne me parait pas justifié, quelqu'un qui investigue pensera à faire -v2 (s'il a l'habitude des commandes django).

#17

Mis à jour par Valentin Deniaud il y a plus de 2 ans

Voilà en ajoutant un timestamp (pas visible par la capture dans les tests mais marche sans problème dans un shell)

#18

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

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

Ok.

#19

Mis à jour par Valentin Deniaud il y a plus de 2 ans

  • Statut changé de Solution validée à Résolu (à déployer)
commit f597d8cf069155cd58239bec5c891fc7ce9ed39b
Author: Valentin Deniaud <vdeniaud@entrouvert.com>
Date:   Thu Aug 26 18:09:44 2021 +0200

    ldap: add useful output to sync-ldap-users command (#54078)
#20

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