From 240c6913ab5e4d2412a41d3de38b784cbf7e5e1b Mon Sep 17 00:00:00 2001 From: Enrico Zini Date: Tue, 21 Apr 2020 22:08:51 +0200 Subject: [PATCH] Start using Inconsistency in CheckLDAPConsistency. refs: #5 --- dsa/housekeeping.py | 80 ++++++++++++++++++++++------------ dsa/tests/test_housekeeping.py | 39 ++++++++--------- sitechecks/housekeeping.py | 1 + sitechecks/models.py | 1 + 4 files changed, 71 insertions(+), 50 deletions(-) diff --git a/dsa/housekeeping.py b/dsa/housekeeping.py index d7f5ec7..93b50b5 100644 --- a/dsa/housekeeping.py +++ b/dsa/housekeeping.py @@ -6,6 +6,7 @@ from django.utils.timezone import now from backend.housekeeping import MakeLink, Housekeeper from . import udldap from backend import const +from sitechecks.models import Inconsistency import backend.models as bmodels import process.models as pmodels import backend.ops as bops @@ -140,8 +141,10 @@ class CheckLDAPConsistency(hk.Task): fpr = entry.single("keyFingerPrint") if fpr: - log.warn("%s: %s has fingerprint %s and gid %s in LDAP, but is not in our db", - self.IDENTIFIER, entry.uid, fpr, entry.single("gidNumber")) + Inconsistency.objects.found( + self.IDENTIFIER, + f"{entry.uid} has fingerprint {fpr} and gid {entry.single('gidNumber')} in LDAP," + " but is not in our db") else: ldap_fields_args = { "cn": entry.single("cn"), @@ -199,9 +202,12 @@ class CheckLDAPConsistency(hk.Task): elif not entry.is_dd: # mom040267 has a locked account (see the FD comments on the site) if person.ldap_fields.uid != "mom040267": - log.warn("%s: %s has accountStatus '%s' (comment: %s) but in our db the state is %s", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment"), const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f" (comment: {entry.single('accountComment')})" + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]}", + person=person) if entry.is_dd: # if person.status not in (const.STATUS_REMOVED_DD, const.STATUS_EMERITUS_DD): @@ -210,26 +216,36 @@ class CheckLDAPConsistency(hk.Task): if person.status_changed > self.email_forwarding_cutoff: if dsa_status == "retiring" and person.status != const.STATUS_EMERITUS_DD: - log.warn("%s: %s has accountStatus '%s' (comment: %s) but in our db the state is %s [retiring]", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment"), const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f" (comment: {entry.single('accountComment')})" + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]} [retiring]", + person=person) if dsa_status == "inactive" and person.status != const.STATUS_REMOVED_DD: - log.warn("%s: %s has accountStatus '%s' (comment: %s) but in our db the state is %s [inactive]", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment"), const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f" (comment: {entry.single('accountComment')})" + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]} [inactive]", + person=person) if dsa_status == "locked": parsed = entry.single("accountStatus").split() if len(parsed) == 1: - log.warn("%s: %s has accountStatus '%s' (comment: %s) locked with no date", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment")) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f"(comment: {entry.single('accountComment')}) locked with no date", + person=person) else: if dsa_status_date is not None and dsa_status_date < self.email_forwarding_cutoff.date(): - log.warn("%s: %s has accountStatus '%s' (comment: %s) locked for longer than 6 months", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment")) + Inconsistency.objects.found( + self.IDENTIFIER, + f"person has accountStatus '{entry.single('accountStatus')}'" + f" (comment: {entry.single('accountComment')}) locked for longer than 6 months", + person=person) def run_main(self, stage): # Prefetch people and index them by uid @@ -253,15 +269,19 @@ class CheckLDAPConsistency(hk.Task): try: dsa_status_date = datetime.datetime.strptime(parsed[1], "%Y-%m-%d").date() except ValueError: - log.warn("%s: %s has accountStatus '%s' (comment: %s) with unparsable date", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus"), - entry.single("accountComment")) + Inconsistency.objects.found( + self.IDENTIFIER, + "person has accountStatus '{entry.single('accountStatus')}'" + " (comment: {entry.single('accountComment')}) with unparsable date", + person=person) dsa_status_date = None else: dsa_status_date = None if dsa_status not in ("retiring", "inactive", "memorial", "locked", "renamed"): - log.warn("%s: %s has unknown accountStatus %s", - self.IDENTIFIER, self.hk.link(person), entry.single("accountStatus")) + Inconsistency.objects.found( + self.IDENTIFIER, + "person has unknown accountStatus {entry.single('accountStatus')}", + person=person) else: self.check_account_status(person, entry, dsa_status, dsa_status_date) else: @@ -269,14 +289,18 @@ class CheckLDAPConsistency(hk.Task): # DDs we don't expect if entry.is_dd: if person.status not in (const.STATUS_DD_U, const.STATUS_DD_NU): - log.warn("%s: %s has supplementaryGid 'Debian', but in our db the state is %s", - self.IDENTIFIER, self.hk.link(person), - const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + "person has supplementaryGid 'Debian'," + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]}", + person=person) else: if person.status in (const.STATUS_DD_U, const.STATUS_DD_NU): - log.warn("%s: %s has no supplementaryGid 'Debian', but in our db the state is %s", - self.IDENTIFIER, self.hk.link(person), - const.ALL_STATUS_DESCS[person.status]) + Inconsistency.objects.found( + self.IDENTIFIER, + "person has no supplementaryGid 'Debian'," + f" but in our db the state is {const.ALL_STATUS_DESCS[person.status]}", + person=person) email = entry.single("emailForward") if email != person.ldap_fields.email: diff --git a/dsa/tests/test_housekeeping.py b/dsa/tests/test_housekeeping.py index b591a70..2862cc9 100644 --- a/dsa/tests/test_housekeeping.py +++ b/dsa/tests/test_housekeeping.py @@ -16,6 +16,12 @@ class MockHousekeeper: def link(self, person): return str(person.ldap_fields.uid or person.pk) + def run(self, cls, stage: str = "main"): + task = cls(self) + task.IDENTIFIER = f"test.{cls.__name__}" + getattr(task, f"run_{stage}")(None) + return task + class MockEntry: def __init__(self, uid, **attrs): @@ -41,8 +47,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None) ] with LogCapture() as lc: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) lc.check(("dsa.housekeeping", "WARNING", "None: newdd: created to mirror a removed DD account from LDAP")) p = Person.objects.get(ldap_fields__uid="newdd") @@ -65,8 +70,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): sn="tsn", emailForward="test@example.org", accountStatus=None) ] with LogCapture() as lc: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) lc.check(("dsa.housekeeping", "WARNING", "None: newguest: created to mirror a removed guest account from LDAP")) @@ -91,8 +95,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): accountStatus=None) ] with LogCapture() as lc: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) lc.check(("dsa.housekeeping", "WARNING", "None: newdd has fingerprint 66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB and gid 800 in LDAP," " but is not in our db")) @@ -107,8 +110,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): mn="tmn", sn="tsn", emailForward="test@example.org", accountStatus=None) ] with LogCapture() as lc: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) lc.check( ("dsa.housekeeping", "INFO", "None: dd_u changing email_ldap from dd_u@example.org to test@example.org" " (source: LDAP)"), @@ -138,8 +140,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): keyFingerPrint="66B4DFB68CB24EBBD8650BC4F4B4B0CC797EBFAB", accountStatus=None) ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:" "None: dm has supplementaryGid 'Debian', but in our db the state is Debian Maintainer", @@ -157,8 +158,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): accountStatus="inactive 2018-03-20"), ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:" "None: dd_u has accountStatus 'inactive 2018-03-20' (comment: None)" @@ -178,8 +178,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): accountStatus="inactive 2018-03-20", accountComment="RT#1234"), ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:None: dd_u has accountStatus 'inactive 2018-03-20' " '(comment: RT#1234) but in our db the state is Debian Developer, uploading [inactive]']) @@ -197,8 +196,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): emailForward="dd_u@example.org", accountStatus="retiring 2018-03-20"), ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:None: dd_u has accountStatus 'retiring 2018-03-20' " '(comment: None) but in our db the state is Debian Developer, uploading ' @@ -209,8 +207,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): process.approved_time = now() process.save() with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "INFO:dsa.housekeeping:None: dd_u closed from dsa: retiring 2018-03-20", ]) @@ -234,8 +231,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): emailForward="test@example.org", accountStatus="retiring 2018-03-20"), ] with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "WARNING:dsa.housekeeping:None: dd_u has accountStatus 'retiring 2018-03-20' " '(comment: None) but in our db the state is Debian Developer, uploading ' @@ -248,8 +244,7 @@ class TestCheckLDAPConsistency(ProcessFixtureMixin, TestCase): process.approved_time = now() process.save() with self.assertLogs() as log: - task = CheckLDAPConsistency(self.hk) - task.run_main(None) + self.hk.run(CheckLDAPConsistency) self.assertEqual(log.output, [ "INFO:dsa.housekeeping:None: dd_u closed from dsa: retiring 2018-03-20", ]) diff --git a/sitechecks/housekeeping.py b/sitechecks/housekeeping.py index 80dfc52..33b308c 100644 --- a/sitechecks/housekeeping.py +++ b/sitechecks/housekeeping.py @@ -1,3 +1,4 @@ +from __future__ import annotations from collections import defaultdict import logging import django_housekeeping as hk diff --git a/sitechecks/models.py b/sitechecks/models.py index ac449c1..9af63a8 100644 --- a/sitechecks/models.py +++ b/sitechecks/models.py @@ -1,3 +1,4 @@ +from __future__ import annotations from django.db import models from backend.models import Person from process.models import Process -- GitLab