separate between required and recommended entries

cmdeploy/src/cmdeploy/chatmail.zone.j2

@@ -1,3 +1,4 @@
+# Required DNS entries for chatmail servers 
 {% if ipv4 %}
 {{ chatmail_domain }}.                   A     {{ ipv4 }}
 {% endif %}
@@ -5,17 +6,22 @@
 {{ 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 }}
+
+# Recommended DNS entries
+{{ chatmail_domain }}.                   TXT "v=spf1 a:{{ chatmail_domain }} ~all"
+_dmarc.{{ chatmail_domain }}.            TXT "v=DMARC1;p=reject;adkim=s;aspf=s"
+
+{% if acme_account_url %}
+{{ chatmail_domain }}.                   CAA 128 issue "letsencrypt.org;accounturi={{ acme_account_url }}"
+{% endif %}
 _adsp._domainkey.{{ chatmail_domain }}.  TXT "dkim=discardable"
+
+# The following are technically not required for Delta Chat 
+_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 }}.

cmdeploy/src/cmdeploy/cmdeploy.py

@@ -54,7 +54,7 @@ def run_cmd_options(parser):
 def run_cmd(args, out):
     """Deploy chatmail services on the remote server."""
 
-    remote_data = dns.get_initial_remote_data(args, out)
+    remote_data = dns.get_initial_remote_data(args)
     if not dns.check_initial_remote_data(remote_data, print=out.red):
         return 1
 
@@ -86,7 +86,7 @@ def dns_cmd_options(parser):
 
 def dns_cmd(args, out):
     """Check DNS entries and optionally generate dns zone file."""
-    remote_data = dns.get_initial_remote_data(args, out)
+    remote_data = dns.get_initial_remote_data(args)
     if not remote_data:
         return 1
     retcode = dns.show_dns(args, out, remote_data)

cmdeploy/src/cmdeploy/dns.py

@@ -6,7 +6,7 @@ from jinja2 import Template
 from . import remote_funcs
 
 
-def get_initial_remote_data(args, out):
+def get_initial_remote_data(args):
     sshexec = args.get_sshexec()
     mail_domain = args.config.mail_domain
     return sshexec.logged(
@@ -25,6 +25,27 @@ def check_initial_remote_data(remote_data, print=print):
         return remote_data
 
 
+def get_filled_zone_file(remote_data, mail_domain):
+    sts_id = remote_data.get("sts_id")
+    if not sts_id:
+        sts_id = datetime.datetime.now().strftime("%Y%m%d%H%M")
+
+    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=mail_domain,
+    )
+    lines = [x.strip() for x in zonefile.split("\n") if x.strip()]
+    lines.append("")
+    zonefile = "\n".join(lines)
+    return zonefile
+
+
 def show_dns(args, out, remote_data) -> int:
     """Check existing DNS records, optionally write them to zone file
     and return (exitcode, remote_data) tuple."""
@@ -39,27 +60,7 @@ def show_dns(args, out, remote_data) -> int:
         out.red("could not determine dkim_entry, please run 'cmdeploy run'")
         return 1
 
-    sts_id = remote_data.get("sts_id")
-    if not sts_id:
-        sts_id = datetime.datetime.now().strftime("%Y%m%d%H%M")
-
-    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)
-
-    diff_records = sshexec.logged(
-        remote_funcs.check_zonefile, kwargs=dict(zonefile=zonefile)
-    )
+    zonefile = get_filled_zone_file(remote_data, args.config.mail_domain)
 
     if getattr(args, "zonefile", None):
         with open(args.zonefile, "w+") as zf:
@@ -67,11 +68,20 @@ def show_dns(args, out, remote_data) -> int:
         out.green(f"DNS records successfully written to: {args.zonefile}")
         return 0
 
-    if diff_records:
-        out.red("Please set the following DNS entries at your DNS provider:\n")
-        for line in diff_records:
+    required_diff, recommended_diff = sshexec.logged(
+        remote_funcs.check_zonefile, kwargs=dict(zonefile=zonefile)
+    )
+
+    if required_diff:
+        out.red("Please set required DNS entries at your DNS provider:\n")
+        for line in required_diff:
             out(line)
         return 1
-    else:
-        out.green("Great! All your DNS entries are verified and correct.")
+    elif recommended_diff:
+        out("WARNING: these recommended DNS entries are not set:\n")
+        for line in recommended_diff:
+            out(line)
         return 0
+
+    out.green("Great! All your DNS entries are verified and correct.")
+    return 0

cmdeploy/src/cmdeploy/remote_funcs.py

@@ -76,9 +76,16 @@ def query_dns(typ, domain):
 
 def check_zonefile(zonefile):
     """Check expected zone file entries."""
-    diff = []
+    required = True
+    required_diff = []
+    recommended_diff = []
 
     for zf_line in zonefile.splitlines():
+        if "# Recommended" in zf_line:
+            required = False
+            continue
+        if not zf_line.strip() or zf_line.startswith("#"):
+            continue
         print(f"dns-checking {zf_line!r}")
         zf_domain, zf_typ, zf_value = zf_line.split(maxsplit=2)
         zf_domain = zf_domain.rstrip(".")
@@ -86,9 +93,12 @@ def check_zonefile(zonefile):
         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)
+            if required:
+                required_diff.append(zf_line)
+            else:
+                recommended_diff.append(zf_line)
 
-    return diff
+    return required_diff, recommended_diff
 
 
 ## Function Execution server
@@ -119,7 +129,7 @@ def _handle_one_request(cmd):
 if __name__ == "__channelexec__":
     channel = channel  # noqa (channel object gets injected)
 
-    # enable simple "print" debugging for anyone changing this module
+    # enable simple "print" logging for anyone changing this module
     globals()["print"] = lambda x="": channel.send(("log", x))
 
     _run_loop(channel)

cmdeploy/src/cmdeploy/tests/data/zftest.zone

@@ -0,0 +1,17 @@
+# Required DNS entries for chatmail servers
+zftest.testrun.org.                   A     135.181.204.127
+zftest.testrun.org.                   AAAA  2a01:4f9:c012:52f4::1
+zftest.testrun.org.                   MX 10 zftest.testrun.org.
+_mta-sts.zftest.testrun.org.          TXT "v=STSv1; id=202403211706"
+mta-sts.zftest.testrun.org.           CNAME zftest.testrun.org.
+www.zftest.testrun.org.               CNAME zftest.testrun.org.
+opendkim._domainkey.zftest.testrun.org. TXT "v=DKIM1;k=rsa;p=MIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAoYt82CVUyz2ouaqjX2kB+5J80knAyoOU3MGU5aWppmwUwwTvj/oSTSpkc5JMtVTRmKKr8NUDWAL1Yw7dfGqqPHdHfwwjS3BIvDzYx+hzgtz62RnfNgV+/2MAoNpfX7cAFIHdRzEHNtwugc3RDLquqPoupAE3Y2YRw2T5zG5fILh4vwIcJZL5Uq6B92j8wwJqOex" "33n+vm1NKQ9rxo/UsHAmZlJzpooXcG/4igTBxJyJlamVSRR6N7Nul1v//YJb7J6v2o0iPHW6uE0StzKaPPNC2IVosSRFbD9H2oqppltptFSNPlI0E+t0JBWHem6YK7xcugiO3ImMCaaU8g6Jt/wIDAQAB;s=email;t=s"
+# Recommended DNS entries
+_submission._tcp.zftest.testrun.org.  SRV 0 1 587 zftest.testrun.org.
+_submissions._tcp.zftest.testrun.org. SRV 0 1 465 zftest.testrun.org.
+_imap._tcp.zftest.testrun.org.        SRV 0 1 143 zftest.testrun.org.
+_imaps._tcp.zftest.testrun.org.       SRV 0 1 993 zftest.testrun.org.
+zftest.testrun.org.                   CAA 128 issue "letsencrypt.org;accounturi=https://acme-v02.api.letsencrypt.org/acme/acct/1371472956"
+zftest.testrun.org.                   TXT "v=spf1 a:zftest.testrun.org ~all"
+_dmarc.zftest.testrun.org.            TXT "v=DMARC1;p=reject;adkim=s;aspf=s"
+_adsp._domainkey.zftest.testrun.org.  TXT "dkim=discardable"

cmdeploy/src/cmdeploy/tests/plugin.py

@@ -79,6 +79,17 @@ def pytest_report_header():
         return ["-" * len(text), text, "-" * len(text)]
 
 
+@pytest.fixture
+def data(request):
+    datadir = request.fspath.dirpath("data")
+
+    class Data:
+        def get(self, name):
+            return datadir.join(name).read()
+
+    return Data()
+
+
 @pytest.fixture
 def benchmark(request):
     def bench(func, num, name=None, reportfunc=None):

cmdeploy/src/cmdeploy/tests/test_dns.py

@@ -4,24 +4,33 @@ from cmdeploy import remote_funcs
 from cmdeploy.dns import check_initial_remote_data
 
 
-class TestPerformInitialChecks:
-    @pytest.fixture
-    def mockdns(self, monkeypatch):
-        qdict = {
+@pytest.fixture
+def mockdns_base(monkeypatch):
+    qdict = {}
+
+    def query_dns(typ, domain):
+        try:
+            return qdict[typ][domain]
+        except KeyError:
+            return ""
+
+    monkeypatch.setattr(remote_funcs, query_dns.__name__, query_dns)
+    return qdict
+
+
+@pytest.fixture
+def mockdns(mockdns_base):
+    mockdns_base.update(
+        {
             "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()
+        }
+    )
+    return mockdns_base
 
-        def query_dns(typ, domain):
-            try:
-                return qdict[typ][domain]
-            except KeyError:
-                return ""
-
-        monkeypatch.setattr(remote_funcs, query_dns.__name__, query_dns)
-        return qdict
 
+class TestPerformInitialChecks:
     def test_perform_initial_checks_ok1(self, mockdns):
         remote_data = remote_funcs.perform_initial_checks("some.domain")
         assert len(remote_data) == 7
@@ -48,3 +57,30 @@ class TestPerformInitialChecks:
         res = check_initial_remote_data(remote_data, print=l.append)
         assert not res
         assert len(l) == 2
+
+
+def parse_zonefile_into_dict(zonefile, mockdns_base):
+    for zf_line in zonefile.split("\n"):
+        if not zf_line.strip() or zf_line.startswith("#"):
+            continue
+        zf_domain, zf_typ, zf_value = zf_line.split(maxsplit=2)
+        zf_domain = zf_domain.rstrip(".")
+        zf_value = zf_value.strip()
+        mockdns_base.setdefault(zf_typ, {})[zf_domain] = zf_value
+
+
+def test_check_zonefile_all_ok(data, mockdns_base):
+    zonefile = data.get("zftest.zone")
+    parse_zonefile_into_dict(zonefile, mockdns_base)
+    required_diff, recommended_diff = remote_funcs.check_zonefile(zonefile)
+    assert not required_diff and not recommended_diff
+
+
+def test_check_zonefile_recommended_not_set(data, mockdns_base):
+    zonefile = data.get("zftest.zone")
+
+    zonefile_mocked = zonefile.split("# Recommended")[0]
+    parse_zonefile_into_dict(zonefile_mocked, mockdns_base)
+    required_diff, recommended_diff = remote_funcs.check_zonefile(zonefile)
+    assert not required_diff
+    assert len(recommended_diff) == 8