cleanup dead code, clarify others

.github/workflows/ci-no-dns.yaml

@@ -9,6 +9,8 @@ on:
   pull_request:
     branches: [ "main" ]
 
+permissions: {}
+
 # Newest push wins: Prevents multiple runs from clashing and wasting runner efforts
 concurrency:
   group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
@@ -18,7 +20,7 @@ concurrency:
 jobs:
   no-dns:
     name: LXC deploy and test
-    uses: chatmail/cmlxc/.github/workflows/lxc-test.yml@v0.14.1
+    uses: chatmail/cmlxc/.github/workflows/lxc-test.yml@v0.14.5
     with:
       cmlxc_commands: |
         cmlxc init 

.github/workflows/ci.yaml

@@ -57,7 +57,7 @@ jobs:
 
   lxc-test:
     name: LXC deploy and test
-    uses: chatmail/cmlxc/.github/workflows/lxc-test.yml@v0.14.1
+    uses: chatmail/cmlxc/.github/workflows/lxc-test.yml@v0.14.5
     with:
       cmlxc_commands: |
         cmlxc init 

chatmaild/src/chatmaild/config.py

@@ -200,13 +200,6 @@ def is_valid_ipv4(address: str) -> bool:
         return False
 
 
-def format_arpa_address(address: str) -> str:
-    if is_valid_ipv4(address):
-        return ipaddress.IPv4Address(address).reverse_pointer
-    DomainValidator().validate_domain_re(address)
-    return address
-
-
 def format_mail_domain(raw_domain: str) -> str:
     if is_valid_ipv4(raw_domain):
         return f"[{raw_domain}]"

chatmaild/src/chatmaild/tests/test_config.py

@@ -3,7 +3,6 @@ from contextlib import nullcontext as does_not_raise
 import pytest
 
 from chatmaild.config import (
-    format_arpa_address,
     format_mail_domain,
     is_valid_ipv4,
     parse_size_mb,
@@ -165,20 +164,6 @@ def test_is_valid_ipv4(input, result):
     assert result == is_valid_ipv4(input)
 
 
-@pytest.mark.parametrize(
-    ["input", "result", "exception"],
-    [
-        ("example.org", "example.org", does_not_raise()),
-        ("1.3.3.7", "7.3.3.1.in-addr.arpa", does_not_raise()),
-        ("fe::1", None, pytest.raises(ValueError)),
-        ("12394142", None, pytest.raises(ValueError)),
-    ],
-)
-def test_format_arpa_address(input, result, exception):
-    with exception:
-        assert result == format_arpa_address(input)
-
-
 @pytest.mark.parametrize(
     ["input", "result", "exception"],
     [

cmdeploy/src/cmdeploy/deployers.py

@@ -540,7 +540,7 @@ def deploy_chatmail(config_path: Path, disable_mail: bool, website_only: bool) -
         WebsiteDeployer(config),
         ChatmailVenvDeployer(config),
         MtastsDeployer(),
-        OpendkimDeployer(config.mail_domain),
+        *([] if config.ipv4_relay else [OpendkimDeployer(bare_host)]),
         # Dovecot should be started before Postfix
         # because it creates authentication socket
         # required by Postfix.

cmdeploy/src/cmdeploy/external/deployer.py

@@ -1,4 +1,3 @@
-
 from pyinfra import host
 from pyinfra.facts.files import File
 
@@ -40,5 +39,3 @@ class ExternalTlsDeployer(Deployer):
             running=True,
             enabled=True,
         )
-
-

cmdeploy/src/cmdeploy/tests/online/test_1_basic.py

@@ -5,10 +5,10 @@ import subprocess
 import time
 
 import pytest
+from chatmaild.config import is_valid_ipv4
 
 from cmdeploy import remote
 from cmdeploy.cmdeploy import get_sshexec
-from chatmaild.config import is_valid_ipv4
 
 
 class TestSSHExecutor:
@@ -64,8 +64,10 @@ class TestSSHExecutor:
         else:
             pytest.fail("didn't raise exception")
 
-    def test_opendkim_restarted(self, sshexec):
+    def test_opendkim_restarted(self, sshexec, maildomain):
         """check that opendkim is not running for longer than a day."""
+        if is_valid_ipv4(maildomain):
+            pytest.skip(f"{maildomain} is an IPv4 relay, opendkim is not installed")
         cmd = "systemctl show opendkim --timestamp=utc --property=ActiveEnterTimestamp"
         out = sshexec(call=remote.rshell.shell, kwargs=dict(command=cmd))
         datestring = out.split("=")[1]
@@ -293,4 +295,6 @@ def test_nginx_access_log_only_defined_once(sshdomain):
         kwargs=dict(command="nginx -T 2>/dev/null"),
     )
     access_logs = [l for l in conf.splitlines() if l.strip().startswith("access_log")]
-    assert len(access_logs) == 1, f"expected 1 access_log, found {len(access_logs)}: {access_logs}"
+    assert len(access_logs) == 1, (
+        f"expected 1 access_log, found {len(access_logs)}: {access_logs}"
+    )

cmdeploy/src/cmdeploy/tests/plugin.py

@@ -1,5 +1,4 @@
 import imaplib
-import ipaddress
 import itertools
 import os
 import random
@@ -10,20 +9,11 @@ import time
 from pathlib import Path
 
 import pytest
-from chatmaild.config import read_config, format_mail_domain, is_valid_ipv4
+from chatmaild.config import format_mail_domain, is_valid_ipv4, read_config
-
 
 conftestdir = Path(__file__).parent
 
 
-def _is_ip(domain):
-    try:
-        ipaddress.ip_address(domain)
-        return True
-    except ValueError:
-        return False
-
-
 def pytest_configure(config):
     config._benchresults = {}
     config.addinivalue_line(
@@ -349,7 +339,7 @@ class ChatmailACFactory:
             account = self.dc.add_account()
             domain_deliverable = format_mail_domain(domain)
             addr, password = self.gencreds(domain_deliverable)
-            if _is_ip(domain):
+            if is_valid_ipv4(domain):
                 # Use DCLOGIN scheme with explicit server hosts,
                 # matching how madmail presents its addresses to users.
                 qr = (
@@ -423,10 +413,10 @@ class Remote:
     def iter_output(self, logcmd="", ready=None):
         getjournal = "journalctl -f" if not logcmd else logcmd
         print(self.sshdomain)
-        match self.sshdomain:
+        if self.sshdomain in ("@local", "localhost"):
-            case "@local": command = []
+            command = []
-            case "localhost": command = []
+        else:
-            case _: command = ["ssh", f"root@{self.sshdomain}"]
+            command = ["ssh", f"root@{self.sshdomain}"]
         [command.append(arg) for arg in getjournal.split()]
         popen = subprocess.Popen(
             command,

cmdeploy/src/cmdeploy/tests/test_dovecot_deployer.py

@@ -23,8 +23,7 @@ def make_host(*fact_pairs):
         if cls not in facts:
             registered = ", ".join(c.__name__ for c in facts)
             raise LookupError(
-                f"unexpected get_fact({cls.__name__}); "
+                f"unexpected get_fact({cls.__name__}); only registered: {registered}"
-                f"only registered: {registered}"
             )
         return facts[cls]