fixup! feat(api): Support updating multiple subdomains at once using dyndns API

Author: peterthomassen

api/desecapi/models/domains.py

@@ -40,17 +40,6 @@ def filter_qname(self, qname: str, **kwargs) -> models.query.QuerySet:
             dotted_qname=Value(f".{qname}", output_field=CharField()),
         ).filter(dotted_qname__endswith=F("dotted_name"), **kwargs)
 
-    def filter_qnames(self, qnames: list[str], **kwargs) -> models.query.QuerySet:
-        if not qnames:
-            return self.none()
-
-        qsets = [
-            self.filter_qname(qname, **kwargs).order_by("-name_length")[:1]
-            for qname in qnames
-        ]
-
-        return qsets[0].union(*qsets[1:], all=True)
-
 
 class Domain(ExportModelOperationsMixin("Domain"), models.Model):
     @staticmethod

api/desecapi/tests/test_dyndns12update.py

@@ -161,6 +161,20 @@ def test_ddclient_dyndns2_v4_invalid(self):
         self.assertStatus(response, status.HTTP_400_BAD_REQUEST)
         self.assertIn("malformed", str(response.data))
 
+    def test_ddclient_dyndns2_v4_valid_priority(self):
+        # /nic/update?system=dyndns&hostname=foobar.dedyn.io&myip=10.2.3.4asdf
+        params = {
+            "domain_name": self.my_domain.name,
+            "system": "dyndns",
+            "hostname": self.my_domain.name,
+            "myip": "invalid",
+            "ip": "10.4.2.1",
+        }
+        response = self.client.get(self.reverse("v1:dyndns12update"), params)
+        self.assertStatus(response, status.HTTP_200_OK)
+        self.assertEqual(response.data, "good")
+        self.assertIP(ipv4="10.4.2.1")
+
     def test_ddclient_dyndns2_v4_invalid_or_foreign_domain(self):
         # /nic/update?system=dyndns&hostname=<...>&myip=10.2.3.4
         for name in [

api/desecapi/views/dyndns.py

@@ -82,14 +82,20 @@ def _find_action(self, param_keys, separator) -> UpdateAction:
         # Check URL parameters
         for param_key in param_keys:
             try:
-                params = {
-                    param.strip()
-                    for param in self.request.query_params[param_key].split(",")
-                    if separator in param or param.strip() in ("", "preserve")
-                }
+                params = set(
+                    filter(
+                        lambda param: separator in param or param in ("", "preserve"),
+                        map(str.strip, self.request.query_params[param_key].split(",")),
+                    )
+                )
             except KeyError:
                 continue
-            if len(params) > 1:
+            if not params:
+                continue
+
+            try:
+                (param,) = params  # unpacks if params has exactly one element
+            except ValueError:  # more than one element
                 if params & {"", "preserve"}:
                     raise ValidationError(
                         detail=f'IP parameter "{param_key}" cannot have addresses and "preserve" at the same time.',
@@ -100,23 +106,23 @@ def _find_action(self, param_keys, separator) -> UpdateAction:
                         detail=f'IP parameter "{param_key}" cannot use subnet notation with multiple addresses.',
                         code="multiple-subnet",
                     )
-            if params:
-                params = list(params)
-                if len(params) == 1 and "/" in params[0]:
-                    try:
-                        subnet = ip_network(params[0], strict=False)
-                        return UpdateWithSubnet(subnet=subnet)
-                    except ValueError as e:
-                        raise ValidationError(
-                            detail=f'IP parameter "{param_key}" is an invalid subnet: {e}',
-                            code="invalid-subnet",
-                        )
-                if params == ["preserve"]:
-                    return PreserveIPs()
-                elif params == [""]:
-                    return SetIPs(ips=[])
-                else:
-                    return SetIPs(ips=params)
+            else:  # one element
+                match param:
+                    case "":
+                        return SetIPs(ips=[])
+                    case "preserve":
+                        return PreserveIPs()
+                    case str(x) if "/" in x:
+                        try:
+                            subnet = ip_network(param, strict=False)
+                            return UpdateWithSubnet(subnet=subnet)
+                        except ValueError as e:
+                            raise ValidationError(
+                                detail=f'IP parameter "{param_key}" is an invalid subnet: {e}',
+                                code="invalid-subnet",
+                            )
+
+            return SetIPs(ips=list(params))
 
         # Check remote IP address
         client_ip = self.request.META.get("REMOTE_ADDR")
@@ -184,9 +190,15 @@ def qnames(self) -> set[str]:
 
     @cached_property
     def domain(self) -> Domain:
-        domains = Domain.objects.filter_qnames(
-            self.qnames, owner=self.request.user
-        ).all()
+        qname_qs = (
+            Domain.objects.filter_qname(qname, owner=self.request.user)
+            for qname in self.qnames
+        )
+        domains = (
+            Domain.objects.none()
+            .union(*(qs.order_by("-name_length")[:1] for qs in qname_qs), all=True)
+            .all()
+        )
 
         if len(domains) != len(self.qnames):
             metrics.get("desecapi_dynDNS12_domain_not_found").inc()
@@ -224,52 +236,46 @@ def _get_records(records: list[RR], action: UpdateAction) -> list[str] | None:
         Determines the final list of IP address records based on the given action.
 
         Args:
-            records (list): A list of Record objects for a single domain.
+            records (list): A list of RR objects (for one RRset).
             action (UpdateAction): The action to perform.
 
         Returns:
             list or None: A list of IP address strings, or None if the records should be preserved.
         """
-        if isinstance(action, SetIPs):
-            return action.ips
-        elif isinstance(action, UpdateWithSubnet):
-            return replace_ip_subnet(records, action.subnet)
-        elif isinstance(action, PreserveIPs):
-            return None
+        match action:
+            case SetIPs():
+                return action.ips
+            case UpdateWithSubnet():
+                return replace_ip_subnet(records, action.subnet)
+            case PreserveIPs():
+                return None
 
     def get(self, request, *args, **kwargs) -> Response:
-        instances = self.get_queryset().all()
+        instances = self.get_queryset()
 
-        grouped_records = defaultdict(list)
+        subname_records = defaultdict(list)
         for rrset in instances:
-            grouped_records[rrset.subname].extend(rrset.records.all())
+            subname_records[rrset.subname].extend(rrset.records.all())
 
         actions = {
             "A": self._find_action(["myip", "myipv4", "ip"], separator="."),
             "AAAA": self._find_action(["myipv6", "ipv6", "myip", "ip"], separator=":"),
         }
 
-        data = []
-        for subname in self.subnames:
-            subname_records = grouped_records.get(subname, [])
-
-            data += [
-                {
-                    "type": type_,
-                    "subname": subname,
-                    "ttl": 60,
-                    "records": records,
-                }
-                for type_, action in actions.items()
-                if (records := self._get_records(subname_records, action)) is not None
-            ]
-
-        serializer = self.get_serializer(
-            instances,
-            data=data,
-            many=True,
-            partial=True,
-        )
+        data = [
+            {
+                "type": type_,
+                "subname": subname,
+                "ttl": 60,
+                "records": records,
+            }
+            for subname in self.subnames
+            for type_, action in actions.items()
+            if (records := self._get_records(subname_records[subname], action))
+            is not None
+        ]
+
+        serializer = self.get_serializer(instances, data=data, many=True, partial=True)
         try:
             serializer.is_valid(raise_exception=True)
         except ValidationError as e:

docs/dyndns/configure.rst

@@ -189,8 +189,8 @@ in the same request.
 When using ddclient, you can simply specify multiple (sub)domains in your
 ``ddclient.conf`` by separating them with a comma. For example, you can set
 ``domain.org,sub.domain.org&myipv6=preserve`` as the domain to update. In this
-case the IPv4 address of both ``domain.org`` and ``sub.domain.org`` while will
-be updated while preserving the (different) IPv6 addresses.
+case the IPv4 address of both ``domain.org`` and ``sub.domain.org`` will be
+updated while preserving any (different) IPv6 addresses.
 
 If you try to update several subdomains by issuing multiple update requests,
 your update requests may be refused (see :ref:`rate-limits`).

docs/dyndns/update-api.rst

@@ -73,27 +73,27 @@ the case, we suggest looking for another client.
 Determine Hostname
 ******************
 To update your IP address in the DNS, our servers need to determine the
-hostname you want to update.  To determine the hostname, we try the following
+hostname(s) you want to update.  To determine them, we try the following
 steps until there is a match:
 
 - ``hostname`` query string parameter, unless it is set to ``YES`` (this
-  sometimes happens with dynDNS update clients). This parameter can also be a
-  comma-separated list of hostnames to update multiple domains in a single
-  request. All updates are performed in a single, atomic transaction.
+  sometimes happens with dynDNS update clients).
 
-- ``host_id`` query string parameter. This can also be a comma-separated list of
-  hostnames.
+- ``host_id`` query string parameter.
 
-- The username as provided in the HTTP Basic Authorization header. This can also
-  be a comma-separated list of hostnames.
+- The username as provided in the HTTP Basic Authorization header.
 
-- The username as provided in the ``username`` query string parameter. This can
-  also be a comma-separated list of hostnames. All hostnames must belong to the
-  same domain.
+- The username as provided in the ``username`` query string parameter.
 
 - After successful authentication (no matter how), the only hostname that is
   associated with your user account (if not ambiguous).
 
+You can either specify a single hostname, or a comma-separated list of hostnames
+in order to update multiple subdomains in a single request. This works with any
+of the above parameters; however, all hostnames must belong to the same domain.
+The resulting updates are performed atomically (that is, they are either all
+applied or they all fail).
+
 If we cannot determine a hostname to update, the API returns a status code of
 ``400 Bad Request`` (if no hostname was given but multiple domains exist in
 the account) or ``404 Not Found`` (if the specified domain was not found).
@@ -225,5 +225,5 @@ or option 2::
 
 Update multiple domains simultaneously::
 
-  curl "https://update.dedyn.io/?hostname=<your domain>,<your sub domain>&myip=1.2.3.4" \
+  curl "https://update.dedyn.io/?hostname=<your domain>,<your subdomain>&myip=1.2.3.4" \
     --header "Authorization: Token <your token secret>"