Start using Inconsistency in CheckLDAPConsistency. refs: #5

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:

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",
             ])

sitechecks/housekeeping.py

+from __future__ import annotations
 from collections import defaultdict
 import logging
 import django_housekeeping as hk

sitechecks/models.py

+from __future__ import annotations
 from django.db import models
 from backend.models import Person
 from process.models import Process