Merge ~mpontillo/maas:fix-dnsresource-synthetic-records-for-zero-record-case--bug-1791383 into maas:master

Proposed by Mike Pontillo on 2018-09-07
Status: Merged
Approved by: Mike Pontillo on 2018-09-08
Approved revision: 900145961bbb448a430186fd29c99b9705bf7985
Merge reported by: MAAS Lander
Merged at revision: not available
Proposed branch: ~mpontillo/maas:fix-dnsresource-synthetic-records-for-zero-record-case--bug-1791383
Merge into: maas:master
Diff against target: 133 lines (+60/-25)
2 files modified
src/maasserver/api/dnsresources.py (+35/-25)
src/maasserver/api/tests/test_dnsresources.py (+25/-0)
Reviewer Review Type Date Requested Status
MAAS Lander Approve on 2018-09-08
Andres Rodriguez (community) 2018-09-07 Approve on 2018-09-07
Review via email: mp+354524@code.launchpad.net

Commit message

LP: #1791383 - Fix dnsresources read all=true when no concrete records exist.

To post a comment you must log in.
9001459... by Mike Pontillo on 2018-09-07

Format imports.

Andres Rodriguez (andreserl) wrote :

Lgtm!!!

review: Approve
MAAS Lander (maas-lander) wrote :

UNIT TESTS
-b fix-dnsresource-synthetic-records-for-zero-record-case--bug-1791383 lp:~mpontillo/maas/+git/maas into -b master lp:~maas-committers/maas

STATUS: SUCCESS
COMMIT: 900145961bbb448a430186fd29c99b9705bf7985

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/src/maasserver/api/dnsresources.py b/src/maasserver/api/dnsresources.py
2index bd8e994..7a8ae00 100644
3--- a/src/maasserver/api/dnsresources.py
4+++ b/src/maasserver/api/dnsresources.py
5@@ -34,41 +34,50 @@ class DNSResourcesQuerySet(QuerySet):
6
7 def __iter__(self):
8 """Custom iterator which also includes implicit DNS records."""
9- previous_domain = None
10+ domain = None
11+ if self._domain_filter is not None:
12+ unvisited_domains = set()
13+ unvisited_domains.add(self._domain_filter)
14+ else:
15+ unvisited_domains = set(Domain.objects.all())
16 for record in super().__iter__():
17 # This works because the query always either contains a single
18 # domain, or is sorted by domain.
19- if record.domain != previous_domain:
20- previous_domain = record.domain
21- rrdata = record.domain.render_json_for_related_rrdata(
22- include_dnsdata=False, as_dict=True,
23- user=self._user_filter)
24- for name, value in rrdata.items():
25- name = name.split('.')[0]
26- # Remove redundant info (this is provided in the top
27- # level 'fqdn' field).
28- if (self._name_filter is not None and
29- name != self._name_filter):
30- continue
31- items = []
32- for rr in value:
33- if (self._rrtype_filter is not None and
34- rr['rrtype'] != self._rrtype_filter):
35- continue
36- del rr['name']
37- items.append(rr)
38- if len(items) > 0:
39- resource = DNSResource(
40- id=-1, name=name, domain=record.domain)
41- resource._rrdata = items
42- yield resource
43+ if record.domain != domain:
44+ domain = record.domain
45+ unvisited_domains.remove(domain)
46+ yield from self._generate_synthetic_rrdata(domain)
47 yield record
48+ for domain in unvisited_domains:
49+ yield from self._generate_synthetic_rrdata(domain)
50+
51+ def _generate_synthetic_rrdata(self, domain):
52+ rrdata = domain.render_json_for_related_rrdata(
53+ include_dnsdata=False, as_dict=True, user=self._user_filter)
54+ for name, value in rrdata.items():
55+ name = name.split('.')[0]
56+ # Remove redundant info (this is provided in the top
57+ # level 'fqdn' field).
58+ if (self._name_filter is not None and name != self._name_filter):
59+ continue
60+ items = []
61+ for rr in value:
62+ if (self._rrtype_filter is not None and
63+ rr['rrtype'] != self._rrtype_filter):
64+ continue
65+ del rr['name']
66+ items.append(rr)
67+ if len(items) > 0:
68+ resource = DNSResource(id=-1, name=name, domain=domain)
69+ resource._rrdata = items
70+ yield resource
71
72
73 def get_dnsresource_queryset(
74 all_records: bool, domainname: str=None, name: str=None,
75 rrtype: str=None, user=None):
76 # If the domain is a name, make it an id.
77+ domain = None
78 if domainname is not None:
79 if domainname.isdigit():
80 domain = Domain.objects.get_domain_or_404(
81@@ -96,6 +105,7 @@ def get_dnsresource_queryset(
82 query.__class__ = DNSResourcesQuerySet
83 query._name_filter = name_filter
84 query._rrtype_filter = rrtype_filter
85+ query._domain_filter = domain
86 # Note: the _user_filter should be set to None we want to display
87 # all records, even for non-superusers.
88 query._user_filter = user
89diff --git a/src/maasserver/api/tests/test_dnsresources.py b/src/maasserver/api/tests/test_dnsresources.py
90index ed3aa02..6c92513 100644
91--- a/src/maasserver/api/tests/test_dnsresources.py
92+++ b/src/maasserver/api/tests/test_dnsresources.py
93@@ -11,8 +11,10 @@ import random
94
95 from django.conf import settings
96 from maasserver.api.dnsresources import get_dnsresource_queryset
97+from maasserver.enum import NODE_STATUS
98 from maasserver.models.dnsdata import DNSData
99 from maasserver.models.dnsresource import DNSResource
100+from maasserver.models.domain import Domain
101 from maasserver.testing.api import APITestCase
102 from maasserver.testing.factory import factory
103 from maasserver.utils.django_urls import reverse
104@@ -121,6 +123,29 @@ class TestDNSResourcesAPI(APITestCase.ForUser):
105 ]
106 self.assertItemsEqual(expected_ids, result_ids)
107
108+ def test_read_all_with_only_implicit_records(self):
109+ self.become_admin()
110+ domain = Domain.objects.get_default_domain()
111+ uri = get_dnsresources_uri()
112+ machine = factory.make_Node_with_Interface_on_Subnet(
113+ status=NODE_STATUS.DEPLOYED, domain=domain)
114+ factory.make_StaticIPAddress(interface=machine.boot_interface)
115+ response = self.client.get(
116+ uri,
117+ {
118+ 'all': 'true',
119+ }
120+ )
121+ self.assertEqual(
122+ http.client.OK, response.status_code, response.content)
123+ expected_ids = [-1]
124+ result_ids = [
125+ dnsresource["id"]
126+ for dnsresource in json.loads(
127+ response.content.decode(settings.DEFAULT_CHARSET))
128+ ]
129+ self.assertItemsEqual(expected_ids, result_ids)
130+
131 def test_read_with_name(self):
132 for _ in range(3):
133 factory.make_DNSResource()

Subscribers

People subscribed via source and target branches

to all changes: