Merge lp:~mpontillo/maas/dns-api-ui-consistency--bug-1686864 into lp:~maas-committers/maas/trunk

[Mike Pontillo]

Optionally allow implicit DNS records to be shown using the API.

[MAAS Lander (maas-lander)]

Transitioned to Git.

lp:maas has now moved from Bzr to Git.
Please propose your branches with Launchpad using Git.

git clone https://git.launchpad.net/maas

Unmerged revisions

6026. By Mike Pontillo on 2017-04-28

Don't include redundant DNS data in the extra records shown in the API.

6025. By Mike Pontillo on 2017-04-28

Add a way to get implicit DNS records via the API.

6024. By Mike Pontillo on 2017-04-27

Add a way to see implicit DNS records.

6023. By Mike Pontillo on 2017-04-27

Merge trunk.

=== modified file 'src/maasserver/api/dnsresources.py'
--- src/maasserver/api/dnsresources.py	2017-03-29 22:30:31 +0000
+++ src/maasserver/api/dnsresources.py	2017-04-28 00:50:14 +0000
@@ -2,11 +2,15 @@
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """API handlers: `DNSResource`."""
+from collections import OrderedDict
 
+from django.db.models.query import QuerySet
+from formencode.validators import StringBool
 from maasserver.api.support import (
     admin_method,
     OperationsHandler,
 )
+from maasserver.api.utils import get_optional_param
 from maasserver.enum import NODE_PERMISSION
 from maasserver.exceptions import MAASAPIValidationError
 from maasserver.forms.dnsresource import DNSResourceForm
@@ -27,6 +31,40 @@
 )
 
 
+class DNSResourcesQuerySet(QuerySet):
+
+    def __iter__(self):
+        """Custom iterator which also includes implicit DNS records."""
+        previous_domain = None
+        for record in super().__iter__():
+            # This works because the query always either contains a single
+            # domain, or is sorted by domain.
+            if record.domain != previous_domain:
+                previous_domain = record.domain
+                rrdata = record.domain.render_json_for_related_rrdata(
+                    include_dnsdata=False, as_dict=True)
+                for name, value in rrdata.items():
+                    name = name.split('.')[0]
+                    # Remove redundant info (this is provided in the top
+                    # level 'fqdn' field).
+                    if (self._name_filter is not None and
+                            name != self._name_filter):
+                        continue
+                    items = []
+                    for rr in value:
+                        if (self._rrtype_filter is not None and
+                                rr['rrtype'] != self._rrtype_filter):
+                            continue
+                        del rr['name']
+                        items.append(rr)
+                    if len(items) > 0:
+                        resource = DNSResource(
+                            id=-1, name=name, domain=record.domain)
+                        resource._rrdata = items
+                        yield resource
+            yield record
+
+
 class DNSResourcesHandler(OperationsHandler):
     """Manage dnsresources."""
     api_doc_section_name = "DNSResources"
@@ -44,12 +82,18 @@
         :param name: restrict the listing to entries of the given name.
         :param rrtype: restrict the listing to entries which have
             records of the given rrtype.
+        :param all: if True, also include implicit DNS records created for
+            nodes registered in MAAS.
         """
         data = request.GET
         fqdn = data.get('fqdn', None)
         name = data.get('name', None)
         domainname = data.get('domain', None)
         rrtype = data.get('rrtype', None)
+        if rrtype is not None:
+            rrtype = rrtype.upper()
+        _all = get_optional_param(
+            request.GET, 'all', default=False, validator=StringBool)
         if domainname is None and name is None and fqdn is not None:
             # We need a type for resource separation.  If the user didn't give
             # us a rrtype, then assume it's an address of some sort.
@@ -66,10 +110,22 @@
             query = domain.dnsresource_set.all().order_by('name')
         else:
             query = DNSResource.objects.all().order_by('domain_id', 'name')
+        rrtype_filter = None
+        name_filter = None
         if name is not None:
             query = query.filter(name=name)
+            name_filter = name
         if rrtype is not None:
             query = query.filter(dnsdata__rrtype=rrtype)
+            rrtype_filter = rrtype
+        query = query.prefetch_related('ip_addresses', 'dnsdata_set')
+        query = query.select_related('domain')
+        # Note: This must be done last, otherwise our hacks to show additional
+        # records won't work.
+        if _all is True:
+            query.__class__ = DNSResourcesQuerySet
+            query._name_filter = name_filter
+            query._rrtype_filter = rrtype_filter
         return query
 
     @admin_method
@@ -137,12 +193,19 @@
     @classmethod
     def ip_addresses(cls, dnsresource):
         """Return IPAddresses within the specified dnsresource."""
+        rrdata = getattr(dnsresource, '_rrdata', None)
+        if rrdata is not None:
+            return None
         return dnsresource.ip_addresses.all()
 
     @classmethod
     def resource_records(cls, dnsresource):
         """Other data for this dnsresource."""
-        return dnsresource.dnsdata_set.all().order_by('rrtype')
+        rrdata = getattr(dnsresource, '_rrdata', None)
+        if rrdata is not None:
+            return rrdata
+        else:
+            return dnsresource.dnsdata_set.all().order_by('rrtype')
 
     def read(self, request, id):
         """Read dnsresource.
 
=== modified file 'src/maasserver/models/domain.py'
--- src/maasserver/models/domain.py	2017-04-17 14:11:28 +0000
+++ src/maasserver/models/domain.py	2017-04-28 00:50:14 +0000
@@ -12,6 +12,7 @@
     "validate_domain_name",
     ]
 
+from collections import OrderedDict, defaultdict
 import datetime
 import re
 
@@ -351,7 +352,8 @@
         super(Domain, self).clean(*args, **kwargs)
         self.clean_name()
 
-    def render_json_for_related_rrdata(self, for_list=False):
+    def render_json_for_related_rrdata(
+            self, for_list=False, include_dnsdata=True, as_dict=False):
         """Render a representation of this domain's related non-IP data,
         suitable for converting to JSON.
 
@@ -360,8 +362,13 @@
             DNSData,
             StaticIPAddress,
         )
-        rr_mapping = DNSData.objects.get_hostname_dnsdata_mapping(
-            self, raw_ttl=True)
+        if include_dnsdata is True:
+            rr_mapping = DNSData.objects.get_hostname_dnsdata_mapping(
+                self, raw_ttl=True)
+        else:
+            # Circular imports.
+            from maasserver.models.dnsdata import HostnameRRsetMapping
+            rr_mapping = defaultdict(HostnameRRsetMapping)
         # Smash the IP Addresses in the rrset mapping, so that the far end
         # only needs to worry about one thing.
         ip_mapping = StaticIPAddress.objects.get_hostname_ip_mapping(
@@ -376,16 +383,25 @@
                     rr_mapping[hostname].rrset.add((info.ttl, 'AAAA', ip))
                 else:
                     rr_mapping[hostname].rrset.add((info.ttl, 'A', ip))
-        data = []
+        if as_dict is True:
+            result = OrderedDict()
+        else:
+            result = []
         for hostname, info in rr_mapping.items():
-            data += [{
-                'name': hostname,
-                'system_id': info.system_id,
-                'node_type': info.node_type,
-                'ttl': ttl,
-                'rrtype': rrtype,
-                'rrdata': rrdata
+            data = [{
+                    'name': hostname,
+                    'system_id': info.system_id,
+                    'node_type': info.node_type,
+                    'ttl': ttl,
+                    'rrtype': rrtype,
+                    'rrdata': rrdata
                 }
                 for ttl, rrtype, rrdata in info.rrset
-                ]
-        return data
+            ]
+            if as_dict is True:
+                existing = result.get(hostname, [])
+                existing.extend(data)
+                result[hostname] = existing
+            else:
+                result.extend(data)
+        return result
 
=== modified file 'src/maasserver/models/tests/test_domain.py'
--- src/maasserver/models/tests/test_domain.py	2017-04-05 19:57:09 +0000
+++ src/maasserver/models/tests/test_domain.py	2017-04-28 00:50:14 +0000
@@ -32,7 +32,11 @@
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 from netaddr import IPAddress
-from testtools.matchers import MatchesStructure
+from testtools.matchers import (
+    Equals,
+    HasLength,
+    MatchesStructure,
+)
 from testtools.testcase import ExpectedException
 
 
@@ -372,6 +376,9 @@
                 dnsresource__domain_id=domain.id)
             self.assertEqual("0 0 1688 %s." % target, srvrr.rrdata)
 
+
+class TestRenderRRData(MAASServerTestCase):
+
     def render_rrdata(self, domain, for_list=False):
         rr_map = DNSData.objects.get_hostname_dnsdata_mapping(
             domain, raw_ttl=True)
@@ -414,3 +421,18 @@
         expected = self.render_rrdata(domain, for_list=False)
         actual = domain.render_json_for_related_rrdata(for_list=False)
         self.assertItemsEqual(expected, actual)
+
+    def test_renders_as_dictionary(self):
+        domain = factory.make_Domain()
+        name1 = factory.make_name(prefix='a')
+        name2 = factory.make_name(prefix='b')
+        factory.make_DNSData(name=name1, domain=domain, rrtype='MX')
+        rrdata_list = domain.render_json_for_related_rrdata(as_dict=False)
+        rrdata_dict = domain.render_json_for_related_rrdata(as_dict=True)
+        self.assertThat(rrdata_dict[name1], Equals([rrdata_list[0]]))
+        self.assertThat(rrdata_dict[name1], HasLength(1))
+        factory.make_DNSData(name=name1, domain=domain, rrtype='MX')
+        factory.make_DNSData(name=name2, domain=domain, rrtype='NS')
+        rrdata_dict = domain.render_json_for_related_rrdata(as_dict=True)
+        self.assertThat(rrdata_dict[name1], HasLength(2))
+        self.assertThat(rrdata_dict[name2], HasLength(1))
 
=== modified file 'src/maasserver/testing/factory.py'
--- src/maasserver/testing/factory.py	2017-04-28 00:50:13 +0000
+++ src/maasserver/testing/factory.py	2017-04-28 00:50:14 +0000
@@ -655,7 +655,7 @@
                 else ip
                 for ip in ip_addresses
             ]
-        dnsrr = DNSResource(
+        dnsrr, _ = DNSResource.objects.get_or_create(
             name=name, address_ttl=address_ttl,
             domain=domain)
         dnsrr.save()