[fix] Refactor DNS conf management for domains (#299)

* Add an helper that build a dict describing the dns conf
* Synchronize the dyndns dns conf with the one from domain.py
* [mod] try to make code more lisible
* [mod] try to make code a bit more lisible
* [mod/fix] try to simplify and clean the code (and remove what looks like a debug return)
* [fix] First delete records, then add the new records

src/yunohost/domain.py

@@ -198,67 +198,26 @@ def domain_dns_conf(domain, ttl=None):
         ttl -- Time to live
 
     """
+
     ttl = 3600 if ttl is None else ttl
-    ip4 = ip6 = None
 
-    # A/AAAA records
+    dns_conf = _build_dns_conf(domain, ttl)
-    ip4 = get_public_ip()
-    result = (
-        "@ {ttl} IN A {ip4}\n"
-        "* {ttl} IN A {ip4}\n"
-    ).format(ttl=ttl, ip4=ip4)
 
-    try:
+    result = ""
-        ip6 = get_public_ip(6)
-    except:
-        pass
-    else:
-        result += (
-            "@ {ttl} IN AAAA {ip6}\n"
-            "* {ttl} IN AAAA {ip6}\n"
-        ).format(ttl=ttl, ip6=ip6)
 
-    # Jabber/XMPP
+    result += "# Basic ipv4/ipv6 records"
-    result += ("\n"
+    for record in dns_conf["basic"]:
-        "_xmpp-client._tcp {ttl} IN SRV 0 5 5222 {domain}.\n"
+        result += "\n{name} {ttl} IN {type} {value}".format(**record)
-        "_xmpp-server._tcp {ttl} IN SRV 0 5 5269 {domain}.\n"
-        "muc {ttl} IN CNAME @\n"
-        "pubsub {ttl} IN CNAME @\n"
-        "vjud {ttl} IN CNAME @\n"
-               ).format(ttl=ttl, domain=domain)
 
-    # Email
+    result += "\n\n"
-    result += ('\n'
+    result += "# XMPP"
-        '@ {ttl} IN MX 10 {domain}.\n'
+    for record in dns_conf["xmpp"]:
-        '@ {ttl} IN TXT "v=spf1 a mx ip4:{ip4}'
+        result += "\n{name} {ttl} IN {type} {value}".format(**record)
-               ).format(ttl=ttl, domain=domain, ip4=ip4)
-    if ip6 is not None:
-        result += ' ip6:{ip6}'.format(ip6=ip6)
-    result += ' -all"'
 
-    # DKIM
+    result += "\n\n"
-    try:
+    result += "# Mail"
-        with open('/etc/dkim/{domain}.mail.txt'.format(domain=domain)) as f:
+    for record in dns_conf["mail"]:
-            dkim_content = f.read()
+        result += "\n{name} {ttl} IN {type} {value}".format(**record)
-    except IOError:
-        pass
-    else:
-        dkim = re.match((
-            r'^(?P<host>[a-z_\-\.]+)[\s]+([0-9]+[\s]+)?IN[\s]+TXT[\s]+[^"]*'
-            '(?=.*(;[\s]*|")v=(?P<v>[^";]+))'
-            '(?=.*(;[\s]*|")k=(?P<k>[^";]+))'
-            '(?=.*(;[\s]*|")p=(?P<p>[^";]+))'), dkim_content, re.M | re.S
-        )
-        if dkim:
-            result += '\n{host}. {ttl} IN TXT "v={v}; k={k}; p={p}"'.format(
-                host='{0}.{1}'.format(dkim.group('host'), domain), ttl=ttl,
-                v=dkim.group('v'), k=dkim.group('k'), p=dkim.group('p')
-            )
-
-            # If DKIM is set, add dummy DMARC support
-            result += '\n_dmarc {ttl} IN TXT "v=DMARC1; p=none"'.format(
-                ttl=ttl
-            )
 
     return result
 
@@ -322,6 +281,7 @@ def get_public_ip(protocol=4):
         url = 'https://ip6.yunohost.org'
     else:
         raise ValueError("invalid protocol version")
+
     try:
         return urlopen(url).read().strip()
     except IOError:
@@ -357,3 +317,123 @@ def _normalize_domain_path(domain, path):
     path = "/" + path.strip("/")
 
     return domain, path
+
+
+def _build_dns_conf(domain, ttl=3600):
+    """
+    Internal function that will returns a data structure containing the needed
+    information to generate/adapt the dns configuration
+
+    The returned datastructure will have the following form:
+    {
+        "basic": [
+            # if ipv4 available
+            {"type": "A", "name": "@", "value": "123.123.123.123", "ttl": 3600},
+            {"type": "A", "name": "*", "value": "123.123.123.123", "ttl": 3600},
+            # if ipv6 available
+            {"type": "AAAA", "name": "@", "value": "valid-ipv6", "ttl": 3600},
+            {"type": "AAAA", "name": "*", "value": "valid-ipv6", "ttl": 3600},
+        ],
+        "xmpp": [
+            {"type": "SRV", "name": "_xmpp-client._tcp", "value": "0 5 5222 domain.tld.", "ttl": 3600},
+            {"type": "SRV", "name": "_xmpp-server._tcp", "value": "0 5 5269 domain.tld.", "ttl": 3600},
+            {"type": "CNAME", "name": "muc", "value": "@", "ttl": 3600},
+            {"type": "CNAME", "name": "pubsub", "value": "@", "ttl": 3600},
+            {"type": "CNAME", "name": "vjud", "value": "@", "ttl": 3600}
+        ],
+        "mail": [
+            {"type": "MX", "name": "@", "value": "10 domain.tld.", "ttl": 3600},
+            {"type": "TXT", "name": "@", "value": "\"v=spf1 a mx ip4:123.123.123.123 ipv6:valid-ipv6 -all\"", "ttl": 3600 },
+            {"type": "TXT", "name": "mail._domainkey", "value": "\"v=DKIM1; k=rsa; p=some-super-long-key\"", "ttl": 3600},
+            {"type": "TXT", "name": "_dmarc", "value": "\"v=DMARC1; p=none\"", "ttl": 3600}
+        ],
+    }
+    """
+
+    try:
+        ipv4 = get_public_ip()
+    except:
+        ipv4 = None
+
+    try:
+        ipv6 = get_public_ip(6)
+    except:
+        ipv6 = None
+
+    basic = []
+
+    # Basic ipv4/ipv6 records
+    if ipv4:
+        basic += [
+            ["@", ttl, "A", ipv4],
+            ["*", ttl, "A", ipv4],
+        ]
+
+    if ipv6:
+        basic += [
+            ["@", ttl, "AAAA", ipv6],
+            ["*", ttl, "AAAA", ipv6],
+        ]
+
+    # XMPP
+    xmpp = [
+        ["_xmpp-client._tcp", ttl, "SRV", "0 5 5222 %s." % domain],
+        ["_xmpp-server._tcp", ttl, "SRV", "0 5 5269 %s." % domain],
+        ["muc", ttl, "CNAME", "@"],
+        ["pubsub", ttl, "CNAME", "@"],
+        ["vjud", ttl, "CNAME", "@"],
+    ]
+
+    # SPF record
+    spf_record = '"v=spf1 a mx'
+    if ipv4:
+        spf_record += ' ip4:{ip4}'.format(ip4=ipv4)
+    if ipv6:
+        spf_record += ' ip6:{ip6}'.format(ip6=ipv6)
+    spf_record += ' -all"'
+
+    # Email
+    mail = [
+        ["@", ttl, "MX", "10 %s." % domain],
+        ["@", ttl, "TXT", spf_record],
+    ]
+
+    # DKIM/DMARC record
+    dkim_host, dkim_publickey = _get_DKIM(domain)
+
+    if dkim_host:
+        mail += [
+            [dkim_host, ttl, "TXT", dkim_publickey],
+            ["_dmarc", ttl, "TXT", '"v=DMARC1; p=none"'],
+        ]
+
+    return {
+        "basic": [{"name": name, "ttl": ttl, "type": type_, "value": value} for name, ttl, type_, value in basic],
+        "xmpp": [{"name": name, "ttl": ttl, "type": type_, "value": value} for name, ttl, type_, value in xmpp],
+        "mail": [{"name": name, "ttl": ttl, "type": type_, "value": value} for name, ttl, type_, value in mail],
+    }
+
+
+def _get_DKIM(domain):
+    DKIM_file = '/etc/dkim/{domain}.mail.txt'.format(domain=domain)
+
+    if not os.path.isfile(DKIM_file):
+        return (None, None)
+
+    with open(DKIM_file) as f:
+        dkim_content = f.read()
+
+    dkim = re.match((
+        r'^(?P<host>[a-z_\-\.]+)[\s]+([0-9]+[\s]+)?IN[\s]+TXT[\s]+[^"]*'
+        '(?=.*(;[\s]*|")v=(?P<v>[^";]+))'
+        '(?=.*(;[\s]*|")k=(?P<k>[^";]+))'
+        '(?=.*(;[\s]*|")p=(?P<p>[^";]+))'), dkim_content, re.M | re.S
+    )
+
+    if not dkim:
+        return (None, None)
+
+    return (
+        dkim.group('host'),
+        '"v={v}; k={k}; p={p}"'.format(v=dkim.group('v'), k=dkim.group('k'), p=dkim.group('p'))
+    )

src/yunohost/dyndns.py

@@ -35,7 +35,7 @@ import subprocess
 from moulinette.core import MoulinetteError
 from moulinette.utils.log import getActionLogger
 
-from yunohost.domain import get_public_ip, _get_maindomain
+from yunohost.domain import get_public_ip, _get_maindomain, _build_dns_conf
 
 logger = getActionLogger('yunohost.dyndns')
 
@@ -168,7 +168,10 @@ def dyndns_update(dyn_host="dyndns.yunohost.org", domain=None, key=None,
     except IOError:
         old_ipv6 = '0000:0000:0000:0000:0000:0000:0000:0000'
 
-    if old_ip != ipv4 or old_ipv6 != ipv6:
+    # no need to update
+    if old_ip == ipv4 and old_ipv6 == ipv6:
+        return
+
     if domain is None:
         # Retrieve the first registered domain
         for path in glob.iglob('/etc/yunohost/dyndns/K*.private'):
@@ -176,6 +179,7 @@ def dyndns_update(dyn_host="dyndns.yunohost.org", domain=None, key=None,
             if not match:
                 continue
             _domain = match.group('domain')
+
             try:
                 # Check if domain is registered
                 if requests.get('https://{0}/test/{1}'.format(
@@ -192,67 +196,67 @@ def dyndns_update(dyn_host="dyndns.yunohost.org", domain=None, key=None,
                                   m18n.n('dyndns_no_domain_registered'))
 
     if key is None:
-            keys = glob.glob(
+        keys = glob.glob('/etc/yunohost/dyndns/K{0}.+*.private'.format(domain))
-                '/etc/yunohost/dyndns/K{0}.+*.private'.format(domain))
+
-            if len(keys) > 0:
+        if not keys:
+            raise MoulinetteError(errno.EIO, m18n.n('dyndns_key_not_found'))
+
         key = keys[0]
-        if not key:
-            raise MoulinetteError(errno.EIO,
-                                  m18n.n('dyndns_key_not_found'))
 
     host = domain.split('.')[1:]
     host = '.'.join(host)
+
     lines = [
         'server %s' % dyn_host,
         'zone %s' % host,
-            'update delete %s. A' % domain,
-            'update delete %s. AAAA' % domain,
-            'update delete %s. MX' % domain,
-            'update delete %s. TXT' % domain,
-            'update delete pubsub.%s. A' % domain,
-            'update delete pubsub.%s. AAAA' % domain,
-            'update delete muc.%s. A' % domain,
-            'update delete muc.%s. AAAA' % domain,
-            'update delete vjud.%s. A' % domain,
-            'update delete vjud.%s. AAAA' % domain,
-            'update delete _xmpp-client._tcp.%s. SRV' % domain,
-            'update delete _xmpp-server._tcp.%s. SRV' % domain,
-            'update add %s. 1800 A %s' % (domain, ipv4),
-            'update add %s. 14400 MX 5 %s.' % (domain, domain),
-            'update add %s. 14400 TXT "v=spf1 a mx -all"' % domain,
-            'update add pubsub.%s. 1800 A %s' % (domain, ipv4),
-            'update add muc.%s. 1800 A %s' % (domain, ipv4),
-            'update add vjud.%s. 1800 A %s' % (domain, ipv4),
-            'update add _xmpp-client._tcp.%s. 14400 SRV 0 5 5222 %s.' % (domain, domain),
-            'update add _xmpp-server._tcp.%s. 14400 SRV 0 5 5269 %s.' % (domain, domain)
-        ]
-        if ipv6 is not None:
-            lines += [
-                'update add %s. 1800 AAAA %s' % (domain, ipv6),
-                'update add pubsub.%s. 1800 AAAA %s' % (domain, ipv6),
-                'update add muc.%s. 1800 AAAA %s' % (domain, ipv6),
-                'update add vjud.%s. 1800 AAAA %s' % (domain, ipv6),
     ]
+
+    dns_conf = _build_dns_conf(domain)
+
+    # Delete the old records for all domain/subdomains
+
+    # every dns_conf.values() is a list of :
+    # [{"name": "...", "ttl": "...", "type": "...", "value": "..."}]
+    for records in dns_conf.values():
+        for record in records:
+            action = "update delete {name}.{domain}.".format(domain=domain, **record)
+            action = action.replace(" @.", " ")
+            lines.append(action)
+
+    # Add the new records for all domain/subdomains
+
+    for records in dns_conf.values():
+        for record in records:
+            # (For some reason) here we want the format with everytime the
+            # entire, full domain shown explicitly, not just "muc" or "@", it
+            # should be muc.the.domain.tld. or the.domain.tld
+            if record["value"] == "@":
+                record["value"] = domain
+
+            action = "update add {name}.{domain}. {ttl} {type} {value}".format(domain=domain, **record)
+            action = action.replace(" @.", " ")
+            lines.append(action)
+
     lines += [
         'show',
         'send'
     ]
-        with open('/etc/yunohost/dyndns/zone', 'w') as zone:
-            for line in lines:
-                zone.write(line + '\n')
 
-        if os.system('/usr/bin/nsupdate -k %s /etc/yunohost/dyndns/zone' % key) == 0:
+    with open('/etc/yunohost/dyndns/zone', 'w') as zone:
+        zone.write('\n'.join(lines))
+
+    if os.system('/usr/bin/nsupdate -k %s /etc/yunohost/dyndns/zone' % key) != 0:
+        os.system('rm -f /etc/yunohost/dyndns/old_ip')
+        os.system('rm -f /etc/yunohost/dyndns/old_ipv6')
+        raise MoulinetteError(errno.EPERM,
+                              m18n.n('dyndns_ip_update_failed'))
+
     logger.success(m18n.n('dyndns_ip_updated'))
     with open('/etc/yunohost/dyndns/old_ip', 'w') as f:
         f.write(ipv4)
     if ipv6 is not None:
         with open('/etc/yunohost/dyndns/old_ipv6', 'w') as f:
             f.write(ipv6)
-        else:
-            os.system('rm -f /etc/yunohost/dyndns/old_ip')
-            os.system('rm -f /etc/yunohost/dyndns/old_ipv6')
-            raise MoulinetteError(errno.EPERM,
-                                  m18n.n('dyndns_ip_update_failed'))
 
 
 def dyndns_installcron():