From 44e78df1203918d370daa01182e62085e29730a5 Mon Sep 17 00:00:00 2001 From: holger krekel Date: Mon, 11 May 2026 01:57:34 +0200 Subject: [PATCH] cleanup dead code, clarify others --- .github/workflows/ci-no-dns.yaml | 4 +++- .github/workflows/ci.yaml | 2 +- chatmaild/src/chatmaild/config.py | 7 ------ chatmaild/src/chatmaild/tests/test_config.py | 15 ------------- cmdeploy/src/cmdeploy/deployers.py | 2 +- cmdeploy/src/cmdeploy/external/deployer.py | 7 ++---- .../src/cmdeploy/tests/online/test_1_basic.py | 10 ++++++--- cmdeploy/src/cmdeploy/tests/plugin.py | 22 +++++-------------- .../cmdeploy/tests/test_dovecot_deployer.py | 3 +-- 9 files changed, 21 insertions(+), 51 deletions(-) diff --git a/.github/workflows/ci-no-dns.yaml b/.github/workflows/ci-no-dns.yaml index bec903df..5a269365 100644 --- a/.github/workflows/ci-no-dns.yaml +++ b/.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 diff --git a/.github/workflows/ci.yaml b/.github/workflows/ci.yaml index 85c8f0fd..7e5fa781 100644 --- a/.github/workflows/ci.yaml +++ b/.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 diff --git a/chatmaild/src/chatmaild/config.py b/chatmaild/src/chatmaild/config.py index 25e7a5b5..2fbf98b5 100644 --- a/chatmaild/src/chatmaild/config.py +++ b/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}]" diff --git a/chatmaild/src/chatmaild/tests/test_config.py b/chatmaild/src/chatmaild/tests/test_config.py index cd3f1efb..1cb74242 100644 --- a/chatmaild/src/chatmaild/tests/test_config.py +++ b/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"], [ diff --git a/cmdeploy/src/cmdeploy/deployers.py b/cmdeploy/src/cmdeploy/deployers.py index 64b57d81..10bcd14d 100644 --- a/cmdeploy/src/cmdeploy/deployers.py +++ b/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. diff --git a/cmdeploy/src/cmdeploy/external/deployer.py b/cmdeploy/src/cmdeploy/external/deployer.py index 7d087c5b..1cca3299 100644 --- a/cmdeploy/src/cmdeploy/external/deployer.py +++ b/cmdeploy/src/cmdeploy/external/deployer.py @@ -1,4 +1,3 @@ - from pyinfra import host from pyinfra.facts.files import File @@ -21,8 +20,8 @@ class ExternalTlsDeployer(Deployer): def configure(self): # Verify cert and key exist on the remote host using pyinfra facts. for path in (self.cert_path, self.key_path): - if host.get_fact(File, path=path) is None: - raise Exception(f"External TLS file not found on server: {path}") + if host.get_fact(File, path=path) is None: + raise Exception(f"External TLS file not found on server: {path}") self.ensure_systemd_unit( "external/tls-cert-reload.path.j2", @@ -40,5 +39,3 @@ class ExternalTlsDeployer(Deployer): running=True, enabled=True, ) - - diff --git a/cmdeploy/src/cmdeploy/tests/online/test_1_basic.py b/cmdeploy/src/cmdeploy/tests/online/test_1_basic.py index 753cd486..300b19fb 100644 --- a/cmdeploy/src/cmdeploy/tests/online/test_1_basic.py +++ b/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}" + ) diff --git a/cmdeploy/src/cmdeploy/tests/plugin.py b/cmdeploy/src/cmdeploy/tests/plugin.py index 841ed65c..5a4517dd 100644 --- a/cmdeploy/src/cmdeploy/tests/plugin.py +++ b/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: - case "@local": command = [] - case "localhost": command = [] - case _: command = ["ssh", f"root@{self.sshdomain}"] + if self.sshdomain in ("@local", "localhost"): + command = [] + else: + command = ["ssh", f"root@{self.sshdomain}"] [command.append(arg) for arg in getjournal.split()] popen = subprocess.Popen( command, diff --git a/cmdeploy/src/cmdeploy/tests/test_dovecot_deployer.py b/cmdeploy/src/cmdeploy/tests/test_dovecot_deployer.py index f632912e..8c615671 100644 --- a/cmdeploy/src/cmdeploy/tests/test_dovecot_deployer.py +++ b/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"only registered: {registered}" + f"unexpected get_fact({cls.__name__}); only registered: {registered}" ) return facts[cls]