add DNS tests, make remote ssh-exec errors show locally, cleanup ssh-bootstrap

- don't try to guess IP addresses but insist on A and AAAA records
- try to allow ipv4 or ipv6 only zones
- move chatmail.zone generation to jinja so we can have conditionals

cmdeploy/src/cmdeploy/chatmail.zone.f

@@ -1,15 +0,0 @@
-{chatmail_domain}.                   A     {ipv4}
-{chatmail_domain}.                   AAAA  {ipv6}
-{chatmail_domain}.                   MX 10 {chatmail_domain}.
-_submission._tcp.{chatmail_domain}.  SRV 0 1 587 {chatmail_domain}.
-_submissions._tcp.{chatmail_domain}. SRV 0 1 465 {chatmail_domain}.
-_imap._tcp.{chatmail_domain}.        SRV 0 1 143 {chatmail_domain}.
-_imaps._tcp.{chatmail_domain}.       SRV 0 1 993 {chatmail_domain}.
-{chatmail_domain}.                   CAA 128 issue "letsencrypt.org;accounturi={acme_account_url}"
-{chatmail_domain}.                   TXT "v=spf1 a:{chatmail_domain} ~all"
-_dmarc.{chatmail_domain}.            TXT "v=DMARC1;p=reject;adkim=s;aspf=s"
-_mta-sts.{chatmail_domain}.          TXT "v=STSv1; id={sts_id}"
-mta-sts.{chatmail_domain}.           CNAME {chatmail_domain}.
-www.{chatmail_domain}.               CNAME {chatmail_domain}.
-{dkim_entry}
-_adsp._domainkey.{chatmail_domain}.  TXT "dkim=discardable"

cmdeploy/src/cmdeploy/chatmail.zone.j2

@@ -0,0 +1,21 @@
+{% if ipv4 %}
+{{ chatmail_domain }}.                   A     {{ ipv4 }}
+{% endif %}
+{% if ipv6 %}
+{{ chatmail_domain }}.                   AAAA  {{ ipv6 }}
+{% endif %}
+{{ chatmail_domain }}.                   MX 10 {{ chatmail_domain }}.
+_submission._tcp.{{ chatmail_domain }}.  SRV 0 1 587 {{ chatmail_domain }}.
+_submissions._tcp.{{ chatmail_domain }}. SRV 0 1 465 {{ chatmail_domain }}.
+_imap._tcp.{{ chatmail_domain }}.        SRV 0 1 143 {{ chatmail_domain }}.
+_imaps._tcp.{{ chatmail_domain }}.       SRV 0 1 993 {{ chatmail_domain }}.
+{% if acme_account_url %}
+{{ chatmail_domain }}.                   CAA 128 issue "letsencrypt.org;accounturi={{ acme_account_url }}"
+{% endif %}
+{{ chatmail_domain }}.                   TXT "v=spf1 a:{{ chatmail_domain }} ~all"
+_dmarc.{{ chatmail_domain }}.            TXT "v=DMARC1;p=reject;adkim=s;aspf=s"
+_mta-sts.{{ chatmail_domain }}.          TXT "v=STSv1; id={{ sts_id }}"
+mta-sts.{{ chatmail_domain }}.           CNAME {{ chatmail_domain }}.
+www.{{ chatmail_domain }}.               CNAME {{ chatmail_domain }}.
+{{ dkim_entry }}
+_adsp._domainkey.{{ chatmail_domain }}.  TXT "dkim=discardable"

cmdeploy/src/cmdeploy/cmdeploy.py

@@ -15,8 +15,7 @@ from pathlib import Path
 from chatmaild.config import read_config, write_initial_config
 from termcolor import colored
 
-from . import remote_funcs
-from .dns import show_dns
+from . import dns, remote_funcs
 from .sshexec import SSHExec
 
 #
@@ -54,7 +53,10 @@ def run_cmd_options(parser):
 
 def run_cmd(args, out):
     """Deploy chatmail services on the remote server."""
-    retcode, remote_data = show_dns(args, out)
+
+    remote_data = dns.get_initial_remote_data(args, out)
+    if not dns.check_initial_remote_data(remote_data, print=out.red):
+        return 1
 
     env = os.environ.copy()
     env["CHATMAIL_INI"] = args.inipath
@@ -62,12 +64,12 @@ def run_cmd(args, out):
     pyinf = "pyinfra --dry" if args.dry_run else "pyinfra"
     cmd = f"{pyinf} --ssh-user root {args.config.mail_domain} {deploy_path}"
 
-    out.check_call(cmd, env=env)
+    retcode = out.check_call(cmd, env=env)
     if retcode == 0:
-        out.green("Deploy completed, call `cmdeploy test` next.")
+        out.green("Deploy completed, call `cmdeploy dns` next.")
     elif not remote_data["acme_account_url"]:
         out.red("Deploy completed but letsencrypt not configured")
-        out.red("Run 'cmdeploy dns' or 'cmdeploy run' again")
+        out.red("Run 'cmdeploy run' again")
         retcode = 0
     else:
         out.red("Deploy failed")
@@ -84,7 +86,10 @@ def dns_cmd_options(parser):
 
 def dns_cmd(args, out):
     """Check DNS entries and optionally generate dns zone file."""
-    retcode, remote_data = show_dns(args, out)
+    remote_data = dns.get_initial_remote_data(args, out)
+    if not remote_data:
+        return 1
+    retcode = dns.show_dns(args, out, remote_data)
     return retcode
 
 
@@ -278,9 +283,14 @@ def main(args=None):
     if not hasattr(args, "func"):
         return parser.parse_args(["-h"])
 
-    def get_sshexec(log=None):
-        print(f"[ssh] login to {args.config.mail_domain}")
-        return SSHExec(args.config.mail_domain, remote_funcs, log=log)
+    ssh_cache = []
+
+    def get_sshexec():
+        if not ssh_cache:
+            print(f"[ssh] login to {args.config.mail_domain}")
+            ssh = SSHExec(args.config.mail_domain, remote_funcs, verbose=args.verbose)
+            ssh_cache.append(ssh)
+        return ssh_cache[0]
 
     args.get_sshexec = get_sshexec
 
@@ -301,7 +311,6 @@ def main(args=None):
         if res is None:
             res = 0
         return res
-
     except KeyboardInterrupt:
         out.red("KeyboardInterrupt")
         sys.exit(130)

cmdeploy/src/cmdeploy/dns.py

@@ -1,59 +1,77 @@
 import datetime
 import importlib
-import sys
+
+from jinja2 import Template
 
 from . import remote_funcs
 
 
-def show_dns(args, out) -> int:
+def get_initial_remote_data(args, out):
+    sshexec = args.get_sshexec()
+    mail_domain = args.config.mail_domain
+    return sshexec.logged(
+        call=remote_funcs.perform_initial_checks, kwargs=dict(mail_domain=mail_domain)
+    )
+
+
+def check_initial_remote_data(remote_data, print=print):
+    mail_domain = remote_data["mail_domain"]
+    if not remote_data["A"] and not remote_data["AAAA"]:
+        print("Missing A and/or AAAA DNS records for {mail_domain}!")
+    elif not remote_data["MTA_STS"]:
+        print("Missing MTA-STS CNAME record:")
+        print(f"mta-sts.{mail_domain}.   CNAME  {mail_domain}")
+    else:
+        return remote_data
+
+
+def show_dns(args, out, remote_data) -> int:
     """Check existing DNS records, optionally write them to zone file
     and return (exitcode, remote_data) tuple."""
-    template = importlib.resources.files(__package__).joinpath("chatmail.zone.f")
-    mail_domain = args.config.mail_domain
 
-    def log_progress(data):
-        sys.stdout.write(".")
-        sys.stdout.flush()
+    sshexec = args.get_sshexec()
 
-    sshexec = args.get_sshexec(log=print if args.verbose else log_progress)
-    print("Checking DNS entries ", end="\n" if args.verbose else "")
+    if not remote_data["acme_account_url"]:
+        out.red("could not get letsencrypt account url, please run 'cmdeploy run'")
+        return 1
 
-    remote_data = sshexec(remote_funcs.perform_initial_checks, mail_domain=mail_domain)
+    if not remote_data["dkim_entry"]:
+        out.red("could not determine dkim_entry, please run 'cmdeploy run'")
+        return 1
 
-    assert remote_data["ipv4"] or remote_data["ipv6"]
+    sts_id = remote_data.get("sts_id")
+    if not sts_id:
+        sts_id = datetime.datetime.now().strftime("%Y%m%d%H%M")
 
-    with open(template, "r") as f:
-        zonefile = f.read().format(
-            acme_account_url=remote_data["acme_account_url"],
-            dkim_entry=remote_data["dkim_entry"],
-            ipv6=remote_data["ipv6"],
-            ipv4=remote_data["ipv4"],
-            sts_id=datetime.datetime.now().strftime("%Y%m%d%H%M"),
-            chatmail_domain=args.config.mail_domain,
-        )
+    template = importlib.resources.files(__package__).joinpath("chatmail.zone.j2")
+    content = template.read_text()
+    zonefile = Template(content).render(
+        acme_account_url=remote_data.get("acme_account_url"),
+        dkim_entry=remote_data["dkim_entry"],
+        ipv4=remote_data["A"],
+        ipv6=remote_data["AAAA"],
+        sts_id=sts_id,
+        chatmail_domain=args.config.mail_domain,
+    )
+    lines = [x.strip() for x in zonefile.split("\n") if x.strip()]
+    lines.append("")
+    zonefile = "\n".join(lines)
 
-    to_print = sshexec(remote_funcs.check_zonefile, zonefile=zonefile)
-    if not args.verbose:
-        print()
+    diff_records = sshexec.logged(
+        remote_funcs.check_zonefile, kwargs=dict(zonefile=zonefile)
+    )
 
     if getattr(args, "zonefile", None):
         with open(args.zonefile, "w+") as zf:
             zf.write(zonefile)
         out.green(f"DNS records successfully written to: {args.zonefile}")
-        return 0, remote_data
+        return 0
 
-    if to_print:
-        to_print.insert(
-            0, "You should configure the following entries at your DNS provider:\n"
-        )
-        to_print.append(
-            "\nIf you already configured the DNS entries, "
-            "wait a bit until the DNS entries propagate to the Internet."
-        )
-        out.red("\n".join(to_print))
-        exit_code = 1
+    if diff_records:
+        out.red("Please set the following DNS entries at your DNS provider:\n")
+        for line in diff_records:
+            out(line)
+        return 1
     else:
         out.green("Great! All your DNS entries are verified and correct.")
-        exit_code = 0
-
-    return exit_code, remote_data
+        return 0

cmdeploy/src/cmdeploy/remote_funcs.py

@@ -1,21 +1,22 @@
 """
-Functions to be executed on an ssh-connected host.
+Pure python functions which execute remotely in a system Python interpreter.
 
-All functions of this module need to work with Python builtin types
-and standard library dependencies only.
+All functions of this module
 
-When a remote function executes remotely, it runs in a system python interpreter
-without any installed dependencies.
+- need to get and and return Python builtin data types only,
 
+- can only use standard library dependencies,
+
+- can freely call each other.
 """
 
 import re
-import socket
+import traceback
 from subprocess import CalledProcessError, check_output
 
 
 def shell(command, fail_ok=False):
-    log(f"$ {command}")
+    print(f"$ {command}")
     try:
         return check_output(command, shell=True).decode().rstrip()
     except CalledProcessError:
@@ -30,76 +31,95 @@ def get_systemd_running():
 
 
 def perform_initial_checks(mail_domain):
-    res = {}
+    """Collecting initial DNS zone content."""
+    assert mail_domain
+    A = query_dns("A", mail_domain)
+    AAAA = query_dns("AAAA", mail_domain)
+    MTA_STS = query_dns("CNAME", f"mta-sts.{mail_domain}")
+
+    res = dict(mail_domain=mail_domain, A=A, AAAA=AAAA, MTA_STS=MTA_STS)
+    if not MTA_STS or (not A and not AAAA):
+        return res
 
     res["acme_account_url"] = shell("acmetool account-url", fail_ok=True)
     if not shell("dig", fail_ok=True):
         shell("apt-get install -y dnsutils")
     shell(f"unbound-control flush_zone {mail_domain}", fail_ok=True)
-
     res["dkim_entry"] = get_dkim_entry(mail_domain, dkim_selector="opendkim")
-    res["ipv4"] = get_ip_address(socket.AF_INET)
-    res["ipv6"] = get_ip_address(socket.AF_INET6)
+
+    # parse out sts-id if exists, example: "v=STSv1; id=2090123"
+    parts = query_dns("TXT", f"_mta-sts.{mail_domain}").split("id=")
+    res["sts_id"] = parts[1].rstrip('"') if len(parts) == 2 else ""
     return res
 
 
 def get_dkim_entry(mail_domain, dkim_selector):
-    dkim_pubkey = shell(
-        f"openssl rsa -in /etc/dkimkeys/{dkim_selector}.private "
-        "-pubout 2>/dev/null | awk '/-/{next}{printf(\"%s\",$0)}'"
-    )
+    try:
+        dkim_pubkey = shell(
+            f"openssl rsa -in /etc/dkimkeys/{dkim_selector}.private "
+            "-pubout 2>/dev/null | awk '/-/{next}{printf(\"%s\",$0)}'"
+        )
+    except CalledProcessError:
+        return
     dkim_value_raw = f"v=DKIM1;k=rsa;p={dkim_pubkey};s=email;t=s"
     dkim_value = '" "'.join(re.findall(".{1,255}", dkim_value_raw))
     return f'{dkim_selector}._domainkey.{mail_domain}. TXT "{dkim_value}"'
 
 
-def get_ip_address(typ):
-    sock = socket.socket(typ, socket.SOCK_DGRAM)
-    sock.settimeout(0)
-    sock.connect(("notifications.delta.chat", 1))
-    return sock.getsockname()[0]
-
-
 def query_dns(typ, domain):
     res = shell(f"dig -r -q {domain} -t {typ} +short")
-    return set(filter(None, res.split("\n")))
+    print(res)
+    if res:
+        return res.split("\n")[0]
+    return ""
 
 
 def check_zonefile(zonefile):
+    """Check expected zone file entries."""
     diff = []
 
     for zf_line in zonefile.splitlines():
+        print(f"dns-checking {zf_line!r}")
         zf_domain, zf_typ, zf_value = zf_line.split(maxsplit=2)
         zf_domain = zf_domain.rstrip(".")
         zf_value = zf_value.strip()
-        query_values = query_dns(zf_typ, zf_domain)
-        if zf_value in query_values:
-            continue
-
-        if zf_typ == "CAA" and zf_value.endswith('accounturi="'):
-            # this is an initial run where acmetool did not work yet
-            continue
-
-        if query_values and zf_typ == "TXT" and zf_domain.startswith("_mta-sts."):
-            (query_value,) = query_values
-            if query_value.split("id=")[0] == zf_value.split("id=")[0]:
-                continue
-
-        assert zf_typ in ("A", "AAAA", "CNAME", "CAA", "SRV", "MX", "TXT"), zf_line
-        diff.append(zf_line)
+        query_value = query_dns(zf_typ, zf_domain)
+        if zf_value != query_value:
+            assert zf_typ in ("A", "AAAA", "CNAME", "CAA", "SRV", "MX", "TXT"), zf_line
+            diff.append(zf_line)
 
     return diff
 
 
+## Function Execution server
+
+
+def _run_loop(cmd_channel):
+    while 1:
+        cmd = cmd_channel.receive()
+        if cmd is None:
+            break
+
+        cmd_channel.send(_handle_one_request(cmd))
+
+
+def _handle_one_request(cmd):
+    func_name, kwargs = cmd
+    try:
+        res = globals()[func_name](**kwargs)
+        return ("finish", res)
+    except:
+        data = traceback.format_exc()
+        return ("error", data)
+
+
 # check if this module is executed remotely
 # and setup a simple serialized function-execution loop
 
 if __name__ == "__channelexec__":
+    channel = channel  # noqa (channel object gets injected)
 
-    def log(item):
-        channel.send(("log", item))  # noqa
+    # enable simple "print" debugging for anyone changing this module
+    globals()["print"] = lambda x="": channel.send(("log", x))
 
-    while 1:
-        func_name, kwargs = channel.receive()  # noqa
-        res = globals()[func_name](**kwargs)  # noqa
-        channel.send(("finish", res))  # noqa
+    _run_loop(channel)

cmdeploy/src/cmdeploy/sshexec.py

@@ -1,20 +1,48 @@
+import sys
+
 import execnet
 
 
+class FuncError(Exception):
+    pass
+
+
 class SSHExec:
     RemoteError = execnet.RemoteError
+    FuncError = FuncError
 
-    def __init__(self, host, remote_funcs, log=None, python="python3", timeout=60):
+    def __init__(self, host, remote_funcs, verbose=False, python="python3", timeout=60):
         self.gateway = execnet.makegateway(f"ssh=root@{host}//python={python}")
         self._remote_cmdloop_channel = self.gateway.remote_exec(remote_funcs)
-        self.log = log
         self.timeout = timeout
+        self.verbose = verbose
 
-    def __call__(self, func, **kwargs):
-        self._remote_cmdloop_channel.send((func.__name__, kwargs))
+    def __call__(self, call, kwargs=None, log_callback=None):
+        if kwargs is None:
+            kwargs = {}
+        self._remote_cmdloop_channel.send((call.__name__, kwargs))
         while 1:
             code, data = self._remote_cmdloop_channel.receive(timeout=self.timeout)
-            if code == "log" and self.log:
-                self.log(data)
+            if log_callback is not None and code == "log":
+                log_callback(data)
             elif code == "finish":
                 return data
+            elif code == "error":
+                raise self.FuncError(data)
+
+    def logged(self, call, kwargs):
+        def log_progress(data):
+            sys.stdout.write(".")
+            sys.stdout.flush()
+
+        title = call.__doc__
+        if not title:
+            title = call.__name__
+        if self.verbose:
+            print("[ssh] " + title)
+            return self(call, kwargs, log_callback=print)
+        else:
+            print(title, end="")
+            res = self(call, kwargs, log_callback=log_progress)
+            print()
+            return res

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

@@ -7,18 +7,50 @@ from cmdeploy.sshexec import SSHExec
 
 
 class TestSSHExecutor:
-    @pytest.fixture
+    @pytest.fixture(scope="class")
     def sshexec(self, sshdomain):
         return SSHExec(sshdomain, remote_funcs)
 
     def test_ls(self, sshexec):
-        out = sshexec(remote_funcs.shell, command="ls")
-        out2 = sshexec(remote_funcs.shell, command="ls")
+        out = sshexec(call=remote_funcs.shell, kwargs=dict(command="ls"))
+        out2 = sshexec(call=remote_funcs.shell, kwargs=dict(command="ls"))
         assert out == out2
 
     def test_perform_initial(self, sshexec, maildomain):
-        res = sshexec(remote_funcs.perform_initial_checks, mail_domain=maildomain)
-        assert res["ipv4"] or res["ipv6"]
+        res = sshexec(
+            remote_funcs.perform_initial_checks, kwargs=dict(mail_domain=maildomain)
+        )
+        assert res["A"] or res["AAAA"]
+
+    def test_logged(self, sshexec, maildomain, capsys):
+        sshexec.logged(
+            remote_funcs.perform_initial_checks, kwargs=dict(mail_domain=maildomain)
+        )
+        out, err = capsys.readouterr()
+        assert out.startswith("Collecting")
+        assert out.endswith("....\n")
+        assert out.count("\n") == 1
+
+        sshexec.verbose = True
+        sshexec.logged(
+            remote_funcs.perform_initial_checks, kwargs=dict(mail_domain=maildomain)
+        )
+        out, err = capsys.readouterr()
+        lines = out.split("\n")
+        assert len(lines) > 4
+        assert remote_funcs.perform_initial_checks.__doc__ in lines[0]
+
+    def test_exception(self, sshexec, capsys):
+        try:
+            sshexec.logged(
+                remote_funcs.perform_initial_checks,
+                kwargs=dict(mail_domain=None),
+            )
+        except sshexec.FuncError as e:
+            assert "remote_funcs.py" in str(e)
+            assert "AssertionError" in str(e)
+        else:
+            pytest.fail("didn't raise exception")
 
 
 def test_remote(remote, imap_or_smtp):

cmdeploy/src/cmdeploy/tests/plugin.py

@@ -35,7 +35,7 @@ def pytest_runtest_setup(item):
             pytest.skip("skipping slow test, use --slow to run")
 
 
-@pytest.fixture
+@pytest.fixture(scope="session")
 def chatmail_config(pytestconfig):
     current = basedir = Path().resolve()
     while 1:
@@ -49,12 +49,12 @@ def chatmail_config(pytestconfig):
     pytest.skip(f"no chatmail.ini file found in {basedir} or parent dirs")
 
 
-@pytest.fixture
+@pytest.fixture(scope="session")
 def maildomain(chatmail_config):
     return chatmail_config.mail_domain
 
 
-@pytest.fixture
+@pytest.fixture(scope="session")
 def sshdomain(maildomain):
     return os.environ.get("CHATMAIL_SSH", maildomain)
 

cmdeploy/src/cmdeploy/tests/test_dns.py

@@ -0,0 +1,50 @@
+import pytest
+
+from cmdeploy import remote_funcs
+from cmdeploy.dns import check_initial_remote_data
+
+
+class TestPerformInitialChecks:
+    @pytest.fixture
+    def mockdns(self, monkeypatch):
+        qdict = {
+            "A": {"some.domain": "1.1.1.1"},
+            "AAAA": {"some.domain": "fde5:cd7a:9e1c:3240:5a99:936f:cdac:53ae"},
+            "CNAME": {"mta-sts.some.domain": "some.domain"},
+        }.copy()
+
+        def query_dns(typ, domain):
+            try:
+                return qdict[typ][domain]
+            except KeyError:
+                return ""
+
+        monkeypatch.setattr(remote_funcs, query_dns.__name__, query_dns)
+        return qdict
+
+    def test_perform_initial_checks_ok1(self, mockdns):
+        remote_data = remote_funcs.perform_initial_checks("some.domain")
+        assert len(remote_data) == 7
+
+    @pytest.mark.parametrize("drop", ["A", "AAAA"])
+    def test_perform_initial_checks_with_one_of_A_AAAA(self, mockdns, drop):
+        del mockdns[drop]
+        remote_data = remote_funcs.perform_initial_checks("some.domain")
+        assert len(remote_data) == 7
+        assert not remote_data[drop]
+
+        l = []
+        res = check_initial_remote_data(remote_data, print=l.append)
+        assert res
+        assert not l
+
+    def test_perform_initial_checks_no_mta_sts(self, mockdns):
+        del mockdns["CNAME"]
+        remote_data = remote_funcs.perform_initial_checks("some.domain")
+        assert len(remote_data) == 4
+        assert not remote_data["MTA_STS"]
+
+        l = []
+        res = check_initial_remote_data(remote_data, print=l.append)
+        assert not res
+        assert len(l) == 2