Replaced check_mx management command by a periodic task

2026-10-17 05:41:25 +02:00 · 2026-01-12 16:43:50 +01:00
parent 4ca4f9ab7f
commit 4b1b9b2042
7 changed files with 127 additions and 174 deletions
@@ -223,6 +223,7 @@ class AdminGlobalParametersSerializer(serializers.Serializer):
    enable_dkim_checks = serializers.BooleanField(default=True)
    enable_dmarc_checks = serializers.BooleanField(default=True)
    enable_autoconfig_checks = serializers.BooleanField(default=True)
+    enable_dns_notifications = serializers.BooleanField(default=True)
    custom_dns_server = serializers.IPAddressField(allow_blank=True, allow_null=True)
    enable_dnsbl_checks = serializers.BooleanField(default=True)
    dkim_keys_storage_dir = serializers.CharField(allow_blank=True, allow_null=True)
@@ -110,6 +110,18 @@ GLOBAL_PARAMETERS_STRUCT = collections.OrderedDict(
                                ),
                            },
                        ),
+                        (
+                            "enable_dns_notifications",
+                            {
+                                "label": gettext_lazy(
+                                    "Notify domain admins about DNS issues"
+                                ),
+                                "help_text": gettext_lazy(
+                                    "Send a notification by email to domain administrators "
+                                    "when DNS issues are encountered"
+                                ),
+                            },
+                        ),
                        (
                            "custom_dns_server",
                            {
@@ -1,16 +1,10 @@
-"""Management command to check defined domains."""
-
 import ipaddress
-
-import gevent
-from gevent import socket
+import socket

 from django.conf import settings
 from django.core.mail import EmailMessage
-from django.core.management.base import BaseCommand
 from django.template.loader import render_to_string
 from django.utils import timezone
-from django.utils.encoding import smart_str
 from django.utils.functional import cached_property
 from django.utils.translation import gettext as _

@@ -19,10 +13,7 @@ from modoboa.dnstools import models as dns_models
 from modoboa.parameters import tools as param_tools


-class CheckMXRecords(BaseCommand):
-    """Command class."""
-
-    help = "Check defined domains."  # NOQA:A003
+class DNSChecker:

    @cached_property
    def providers(self):
@@ -36,52 +27,24 @@ class CheckMXRecords(BaseCommand):
        """Return sender address for notifications."""
        return param_tools.get_global_parameter("sender_address", app="core")

+    @cached_property
+    def config(self) -> dict:
+        return dict(param_tools.get_global_parameters("admin"))
+
    @cached_property
    def valid_mxs(self):
        """Return valid MXs set in admin."""
-        valid_mxs = param_tools.get_global_parameter("valid_mxs")
+        valid_mxs = self.config["valid_mxs"]
        return [
-            ipaddress.ip_network(smart_str(v.strip()))
-            for v in valid_mxs.split()
-            if v.strip()
+            ipaddress.ip_network(str(v.strip())) for v in valid_mxs.split() if v.strip()
        ]

-    def add_arguments(self, parser):
-        """Add extra arguments to command."""
-        parser.add_argument(
-            "--no-dnsbl", action="store_true", default=False, help="Skip DNSBL queries."
-        )
-        parser.add_argument(
-            "--email",
-            type=str,
-            action="append",
-            default=[],
-            help="One or more email to notify",
-        )
-        parser.add_argument(
-            "--skip-admin-emails",
-            action="store_true",
-            default=False,
-            help="Skip domain's admins email notification.",
-        )
-        parser.add_argument(
-            "--domain",
-            type=str,
-            action="append",
-            default=[],
-            help="Domain name or id to update.",
-        )
-        parser.add_argument(
-            "--timeout", type=int, default=3, help="Timeout used for queries."
-        )
-        parser.add_argument("--ttl", type=int, default=7200, help="TTL for dns query.")
-
    def query_dnsbl(self, mx_list, provider):
        """Check given IP against given DNSBL provider."""
        results = {}
        for ip, mxs in mx_list.items():
            try:
-                ip = ipaddress.ip_address(smart_str(ip))
+                ip = ipaddress.ip_address(str(ip))
            except ValueError:
                continue
            else:
@@ -99,9 +62,9 @@ class CheckMXRecords(BaseCommand):
                result = False
            for mx in mxs:
                results[mx] = result
-        return provider, results
+        return results

-    def store_dnsbl_result(self, domain, provider, results, **options):
+    def store_dnsbl_result(self, domain, provider, results):
        """Store DNSBL provider results for domain.

        Return a list of alerts.
@@ -142,23 +105,24 @@ class CheckMXRecords(BaseCommand):
        models.DNSBLResult.objects.bulk_create(to_create)
        return alerts

-    def send_alert_notifications(self, domain, alerts, subject, tpl, **options):
+    def send_alert_notifications(
+        self, domain: models.Domain, alerts: list, subject: str, tpl: str
+    ) -> None:
        """Send email notifications about given alerts."""
-        emails = list(options["email"])
-        if not options["skip_admin_emails"]:
-            emails.extend(
-                domain.admins.exclude(mailbox__isnull=True).values_list(
-                    "email", flat=True
-                )
-            )
+        if not self.config["enable_dns_notifications"]:
+            return
+        emails = domain.admins.exclude(mailbox__isnull=True).values_list(
+            "email", flat=True
+        )
        if not len(emails):
            return
        content = render_to_string(tpl, {"domain": domain, "alerts": alerts})
        msg = EmailMessage(subject, content.strip(), self.sender, emails)
        msg.send()

-    def check_valid_mx(self, domain, mx_list, **options):
-        """Check that domain's MX record exist.
+    def check_valid_mx(self, domain: models.Domain, mx_list: list) -> None:
+        """
+        Check that domain's MX record exist.

        If `valid_mx` is provided, retrieved MX records must be
        contained in it.
@@ -212,26 +176,25 @@ class CheckMXRecords(BaseCommand):
            return
        subject = _("[modoboa] MX issue(s) for domain {}").format(domain.name)
        tpl = "admin/notifications/domain_invalid_mx.html"
-        self.send_alert_notifications(domain, alerts, subject, tpl, **options)
+        self.send_alert_notifications(domain, alerts, subject, tpl)
+
+    def run(self, domain: models.Domain, ttl: int = 7200):
+        # Remove deprecated records first
+        domain.dnsblresult_set.exclude(provider__in=self.providers).delete()

-    def check_domain(self, domain, timeout=3, ttl=7200, **options):
-        """Check specified domain."""
        mx_list = list(models.MXRecord.objects.get_or_create_for_domain(domain, ttl))

-        if param_tools.get_global_parameter("enable_mx_checks"):
-            self.check_valid_mx(domain, mx_list, **options)
+        if self.config["enable_mx_checks"]:
+            self.check_valid_mx(domain, mx_list)

-        if param_tools.get_global_parameter("enable_spf_checks"):
+        if self.config["enable_spf_checks"]:
            dns_models.DNSRecord.objects.get_or_create_for_domain(domain, "spf", ttl)
-        condition = (
-            param_tools.get_global_parameter("enable_dkim_checks")
-            and domain.dkim_public_key
-        )
+        condition = self.config["enable_dkim_checks"] and domain.dkim_public_key
        if condition:
            dns_models.DNSRecord.objects.get_or_create_for_domain(domain, "dkim", ttl)
-        if param_tools.get_global_parameter("enable_dmarc_checks"):
+        if self.config["enable_dmarc_checks"]:
            dns_models.DNSRecord.objects.get_or_create_for_domain(domain, "dmarc", ttl)
-        if param_tools.get_global_parameter("enable_autoconfig_checks"):
+        if self.config["enable_autoconfig_checks"]:
            dns_models.DNSRecord.objects.get_or_create_for_domain(
                domain, "autoconfig", ttl
            )
@@ -239,11 +202,8 @@ class CheckMXRecords(BaseCommand):
                domain, "autodiscover", ttl
            )

-        condition = (
-            not param_tools.get_global_parameter("enable_dnsbl_checks")
-            or options["no_dnsbl"] is True
-        )
-        if condition or not mx_list:
+        condition = not self.config["enable_dnsbl_checks"] or not mx_list
+        if condition:
            return

        mx_by_ip = {}
@@ -253,44 +213,14 @@ class CheckMXRecords(BaseCommand):
            elif mx not in mx_by_ip[mx.address]:
                mx_by_ip[mx.address].append(mx)

-        jobs = [
-            gevent.spawn(self.query_dnsbl, mx_by_ip, provider)
-            for provider in self.providers
-        ]
-        gevent.joinall(jobs, timeout)
        alerts = []
-        for job in jobs:
-            if not job.successful():
-                continue
-            provider, results = job.value
-            alerts += self.store_dnsbl_result(domain, provider, results, **options)
+        for provider in self.providers:
+            results = self.query_dnsbl(mx_by_ip, provider)
+            alerts += self.store_dnsbl_result(domain, provider, results)
+
        if not alerts:
            return
+
        subject = _("[modoboa] DNSBL issue(s) for domain {}").format(domain.name)
        tpl = "admin/notifications/domain_in_dnsbl.html"
-        self.send_alert_notifications(domain, alerts, subject, tpl, **options)
-
-    def handle(self, *args, **options):
-        """Command entry point."""
-        # Remove deprecated records first
-        models.DNSBLResult.objects.exclude(provider__in=self.providers).delete()
-
-        if options["domain"]:
-            domains = []
-            for domain in options["domain"]:
-                try:
-                    if domain.isdigit():
-                        domains.append(models.Domain.objects.get(pk=domain))
-                    else:
-                        domains.append(models.Domain.objects.get(name=domain))
-                except models.Domain.DoesNotExist:
-                    pass
-        else:
-            domains = models.Domain.objects.filter(enabled=True, enable_dns_checks=True)
-
-        options.pop("domain")
-
-        for domain in domains:
-            if domain.uses_a_reserved_tld:
-                continue
-            self.check_domain(domain, **options)
+        self.send_alert_notifications(domain, alerts, subject, tpl)
@@ -4,8 +4,14 @@ import logging
 import os
 import shutil

+from django.db.models import F
+from django.utils import timezone
+
+import django_rq
+
+from modoboa.admin import models
 from modoboa.admin.app_settings import load_admin_settings
-from modoboa.admin.models.mailbox import MailboxOperation
+from modoboa.admin.dns_checker import DNSChecker
 from modoboa.lib.sysutils import exec_cmd
 from modoboa.parameters import tools as param_tools

@@ -56,8 +62,25 @@ def handle_mailbox_operations():
    load_admin_settings()
    if not param_tools.get_global_parameter("handle_mailboxes"):
        return
-    for ope in MailboxOperation.objects.all():
+    for ope in models.MailboxOperation.objects.all():
        if ope.type == "rename":
            rename_mailbox(ope)
        elif ope.type == "delete":
            delete_mailbox(ope)
+
+
+def launch_domain_dns_checks(domain_id: int):
+    domain = models.Domain.objects.get(id=domain_id)
+    DNSChecker().run(domain)
+
+
+def handle_dns_checks():
+    """Launch DNS checks for every possible domain."""
+    minute = timezone.now().minute
+    queue = django_rq.get_queue("modoboa")
+    for domain in models.Domain.objects.annotate(slot=F("id") % 60).filter(
+        enable_dns_checks=True, slot=minute
+    ):
+        if domain.uses_a_reserved_tld:
+            continue
+        queue.enqueue(launch_domain_dns_checks, domain.id)
@@ -3,12 +3,17 @@
 from unittest import mock

 import dns.resolver
+from rq import SimpleWorker
 from testfixtures import LogCapture

-from django.core import mail, management
+from django.core import mail
 from django.test import override_settings
 from django.utils.translation import gettext as _

+import django_rq
+
+from modoboa.admin import jobs
+from modoboa.admin.dns_checker import DNSChecker
 from modoboa.core import models as core_models, factories as core_factories
 from modoboa.lib.tests import ModoTestCase
 from . import utils
@@ -44,26 +49,26 @@ class MXTestCase(ModoTestCase):
        cls.localconfig.save()
        models.MXRecord.objects.all().delete()

-    @mock.patch("gevent.socket.gethostbyname")
+    @mock.patch("socket.gethostbyname")
    @mock.patch("socket.getaddrinfo")
    @mock.patch.object(dns.resolver.Resolver, "resolve")
-    def test_management_command(
-        self, mock_query, mock_getaddrinfo, mock_g_gethostbyname
-    ):
+    def test_dns_checker(self, mock_query, mock_getaddrinfo, mock_g_gethostbyname):
        """Check that command works fine."""
        mock_query.side_effect = utils.mock_dns_query_result
        mock_getaddrinfo.side_effect = utils.mock_ip_query_result
        mock_g_gethostbyname.return_value = "1.2.3.4"
+        self.set_global_parameter("enable_dnsbl_checks", False)
+
        self.assertEqual(models.MXRecord.objects.count(), 0)
        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx", "--no-dnsbl", "--ttl=0")
+            DNSChecker().run(self.domain, ttl=0)
        self.assertTrue(models.MXRecord.objects.filter(domain=self.domain).exists())

        # we passed a ttl to 0. this will reset the cache this time
        qs = models.MXRecord.objects.filter(domain=self.domain)
        id_ = qs[0].id
        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx", "--no-dnsbl", "--ttl=7200")
+            DNSChecker().run(self.domain)
        qs = models.MXRecord.objects.filter(domain=self.domain)
        self.assertNotEqual(id_, qs[0].id)
        id_ = qs[0].id
@@ -71,40 +76,11 @@ class MXTestCase(ModoTestCase):
        # assume that mxrecords ids are the same. means that we taking care of
        # ttl
        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx", "--no-dnsbl")
+            DNSChecker().run(self.domain)
        qs = models.MXRecord.objects.filter(domain=self.domain)
        self.assertEqual(id_, qs[0].id)

-    @mock.patch("gevent.socket.gethostbyname")
-    @mock.patch("socket.getaddrinfo")
-    @mock.patch.object(dns.resolver.Resolver, "resolve")
-    def test_single_domain_update(
-        self, mock_query, mock_getaddrinfo, mock_g_gethostbyname
-    ):
-        """Update only one domain."""
-        mock_query.side_effect = utils.mock_dns_query_result
-        mock_getaddrinfo.side_effect = utils.mock_ip_query_result
-        mock_g_gethostbyname.return_value = "1.2.3.4"
-        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx", "--domain", self.domain.name)
-        self.assertTrue(models.MXRecord.objects.filter(domain=self.domain).exists())
-        self.assertFalse(
-            models.MXRecord.objects.filter(domain=self.bad_domain).exists()
-        )
-
-        with LogCapture("modoboa.dns"):
-            management.call_command(
-                "modo", "check_mx", "--domain", str(self.bad_domain.pk)
-            )
-        self.assertFalse(
-            models.MXRecord.objects.filter(domain=self.bad_domain).exists()
-        )
-        self.assertEqual(len(mail.outbox), 1)
-
-        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx", "--domain", "toto.com")
-
-    @mock.patch("gevent.socket.gethostbyname")
+    @mock.patch("socket.gethostbyname")
    @mock.patch("socket.getaddrinfo")
    @mock.patch.object(dns.resolver.Resolver, "resolve")
    def test_invalid_mx(self, mock_query, mock_getaddrinfo, mock_g_gethostbyname):
@@ -112,6 +88,8 @@ class MXTestCase(ModoTestCase):
        mock_query.side_effect = utils.mock_dns_query_result
        mock_getaddrinfo.side_effect = utils.mock_ip_query_result
        mock_g_gethostbyname.return_value = "1.2.3.4"
+        self.set_global_parameter("enable_dnsbl_checks", False)
+
        domain = factories.DomainFactory(name="invalid-mx.com")
        # Add domain admin with mailbox
        mb = factories.MailboxFactory(
@@ -122,14 +100,10 @@ class MXTestCase(ModoTestCase):
        )
        domain.add_admin(mb.user)
        with LogCapture("modoboa.dns"):
-            management.call_command(
-                "modo", "check_mx", "--domain", domain.name, "--no-dnsbl"
-            )
+            DNSChecker().run(domain)
        self.assertEqual(domain.alarms.count(), 1)
        with LogCapture("modoboa.dns"):
-            management.call_command(
-                "modo", "check_mx", "--domain", domain.name, "--no-dnsbl"
-            )
+            DNSChecker().run(domain)
        self.assertEqual(domain.alarms.count(), 1)
        self.assertEqual(len(mail.outbox), 1)

@@ -226,6 +200,15 @@ class DNSBLTestCase(ModoTestCase):
        """Create some data."""
        super().setUpTestData()
        cls.domain = factories.DomainFactory(name="modoboa.org")
+        # Add domain admin with mailbox
+        mb = factories.MailboxFactory(
+            address="admin",
+            domain=cls.domain,
+            user__username="admin@modoboa.org",
+            user__groups=("DomainAdmins",),
+        )
+        cls.domain.add_admin(mb.user)
+
        factories.DomainFactory(name="does-not-exist.example.com")
        cls.domain2 = factories.DomainFactory(
            name="test.localhost"
@@ -235,19 +218,21 @@ class DNSBLTestCase(ModoTestCase):
        )
        models.DNSBLResult.objects.all().delete()

-    @mock.patch("gevent.socket.gethostbyname")
+    @mock.patch("socket.gethostbyname")
    @mock.patch("socket.getaddrinfo")
    @mock.patch.object(dns.resolver.Resolver, "resolve")
-    def test_management_command(
-        self, mock_query, mock_getaddrinfo, mock_g_gethostbyname
-    ):
+    def test_job(self, mock_query, mock_getaddrinfo, mock_g_gethostbyname):
        """Check that command works fine."""
        mock_query.side_effect = utils.mock_dns_query_result
        mock_getaddrinfo.side_effect = utils.mock_ip_query_result
        mock_g_gethostbyname.return_value = "1.2.3.4"
        self.assertEqual(models.DNSBLResult.objects.count(), 0)
+
+        queue = django_rq.get_queue("modoboa")
+        worker = SimpleWorker([queue], connection=queue.connection)
        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx")
+            jobs.handle_dns_checks()
+            worker.work(burst=True)
        self.assertTrue(models.DNSBLResult.objects.filter(domain=self.domain).exists())
        self.assertFalse(
            models.DNSBLResult.objects.filter(domain=self.domain3).exists()
@@ -255,7 +240,7 @@ class DNSBLTestCase(ModoTestCase):
        self.assertFalse(self.domain.uses_a_reserved_tld)
        self.assertTrue(self.domain2.uses_a_reserved_tld)

-    @mock.patch("gevent.socket.gethostbyname")
+    @mock.patch("socket.gethostbyname")
    @mock.patch("socket.getaddrinfo")
    @mock.patch.object(dns.resolver.Resolver, "resolve")
    def test_notifications(self, mock_query, mock_getaddrinfo, mock_g_gethostbyname):
@@ -264,10 +249,10 @@ class DNSBLTestCase(ModoTestCase):
        mock_getaddrinfo.side_effect = utils.mock_ip_query_result
        mock_g_gethostbyname.return_value = "127.0.0.4"
        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx", "--email", "user@example.test")
-        self.assertEqual(len(mail.outbox), 2)
+            DNSChecker().run(self.domain)
+        self.assertEqual(len(mail.outbox), 1)

-    @mock.patch("gevent.socket.gethostbyname")
+    @mock.patch("socket.gethostbyname")
    @mock.patch("socket.getaddrinfo")
    @mock.patch.object(dns.resolver.Resolver, "resolve")
    def test_notifications_wrong_dnsbl_response(
@@ -278,10 +263,10 @@ class DNSBLTestCase(ModoTestCase):
        mock_getaddrinfo.side_effect = utils.mock_ip_query_result
        mock_g_gethostbyname.return_value = "127.255.255.254"  # <--Spamhaus response when querying from an open resolver
        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx", "--email", "user@example.test")
+            DNSChecker().run(self.domain)
        self.assertEqual(len(mail.outbox), 1)

-    @mock.patch("gevent.socket.gethostbyname")
+    @mock.patch("socket.gethostbyname")
    @mock.patch("socket.getaddrinfo")
    @mock.patch.object(dns.resolver.Resolver, "resolve")
    def test_management_command_no_dnsbl(
@@ -291,9 +276,10 @@ class DNSBLTestCase(ModoTestCase):
        mock_query.side_effect = utils.mock_dns_query_result
        mock_getaddrinfo.side_effect = utils.mock_ip_query_result
        mock_g_gethostbyname.return_value = "1.2.3.4"
+        self.set_global_parameter("enable_dnsbl_checks", False)
        self.assertEqual(models.DNSBLResult.objects.count(), 0)
        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx", "--no-dnsbl")
+            DNSChecker().run(self.domain)
        self.assertFalse(models.DNSBLResult.objects.filter(domain=self.domain).exists())


@@ -306,7 +292,7 @@ class DNSChecksTestCase(ModoTestCase):
        super().setUpTestData()
        cls.domain = factories.DomainFactory(name="dns-checks.com")

-    @mock.patch("gevent.socket.gethostbyname")
+    @mock.patch("socket.gethostbyname")
    @mock.patch("socket.getaddrinfo")
    @mock.patch.object(dns.resolver.Resolver, "resolve")
    def test_management_command(
@@ -316,13 +302,14 @@ class DNSChecksTestCase(ModoTestCase):
        mock_query.side_effect = utils.mock_dns_query_result
        mock_getaddrinfo.side_effect = utils.mock_ip_query_result
        mock_g_gethostbyname.return_value = "1.2.3.4"
+        self.set_global_parameter("enable_dnsbl_checks", False)

        self.domain.enable_dkim = True
        self.domain.dkim_public_key = "XXXXX"
        self.domain.save(update_fields=["enable_dkim", "dkim_public_key"])

        with LogCapture("modoboa.dns"):
-            management.call_command("modo", "check_mx", "--no-dnsbl", "--ttl=0")
+            DNSChecker().run(self.domain, ttl=0)

        self.assertIsNot(self.domain.spf_record, None)
        self.assertIsNot(self.domain.dkim_record, None)
@@ -48,7 +48,6 @@ dependencies = [
 	"dnspython==2.8.0",
 	"feedparser==6.0.12",
 	"fido2==2.0.0",
-	"gevent==25.9.1",
 	"progressbar33==2.4",
 	"python-dateutil",
 	"cryptography",
@@ -23,6 +23,7 @@ register(
 )

 register(admin_jobs.handle_mailbox_operations, queue_name="dovecot", cron="* * * * *")
+register(admin_jobs.handle_dns_checks, queue_name="modoboa", cron="* * * * *")

 register(maillog_jobs.logparser, queue_name="modoboa", cron="*/5 * * * *")
 register(maillog_jobs.update_statistics, queue_name="modoboa", cron="0 * * * *")
@@ -30,5 +31,5 @@ register(maillog_jobs.update_statistics, queue_name="modoboa", cron="0 * * * *")
 register(calendars_jobs.generate_rights, queue_name="modoboa", cron="*/2 * * * *")

 if "modoboa.amavis" in settings.MODOBOA_APPS:
-    register(amavis_jobs.qcleanup, queue="modoboa", cron="0 0 * * *")
-    register(amavis_jobs.amnotify, queue="modoboa", cron="0 12 * * *")
+    register(amavis_jobs.qcleanup, queue_name="modoboa", cron="0 0 * * *")
+    register(amavis_jobs.amnotify, queue_name="modoboa", cron="0 12 * * *")