Merge ~cgrabowski/maas:fix_overlapping_subnets_bind_misconfigure into maas:master
- Git
- lp:~cgrabowski/maas
- fix_overlapping_subnets_bind_misconfigure
- Merge into master
Status: | Merged |
---|---|
Approved by: | Christian Grabowski |
Approved revision: | 40348f8e30e9ffebe508d36d3d64ce712b60c00a |
Merge reported by: | MAAS Lander |
Merged at revision: | not available |
Proposed branch: | ~cgrabowski/maas:fix_overlapping_subnets_bind_misconfigure |
Merge into: | maas:master |
Diff against target: |
1493 lines (+876/-360) 5 files modified
src/maasserver/dns/tests/test_zonegenerator.py (+570/-55) src/maasserver/dns/zonegenerator.py (+259/-96) src/maastesting/noseplug.py (+3/-1) src/provisioningserver/dns/tests/test_zoneconfig.py (+26/-152) src/provisioningserver/dns/zoneconfig.py (+18/-56) |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
MAAS Lander | Approve | ||
Jack Lloyd-Walters | Approve | ||
Review via email: mp+457267@code.launchpad.net |
Commit message
fix: overlapping subnets no longer cause BIND to fail configuring
Description of the change
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b fix_overlapping
STATUS: FAILED
LOG: http://
COMMIT: 8bff38ac17e03b0
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b fix_overlapping
STATUS: FAILED
LOG: http://
COMMIT: 9fb26758287ea50
Christian Grabowski (cgrabowski) wrote : | # |
jenkins: !test
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b fix_overlapping
STATUS: FAILED
LOG: http://
COMMIT: 9fb26758287ea50
Christian Grabowski (cgrabowski) wrote : | # |
jenkins: !test
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b fix_overlapping
STATUS: SUCCESS
COMMIT: 9fb26758287ea50
Jack Lloyd-Walters (lloydwaltersj) wrote : | # |
Looks good, a few inline nits about changing to f-strings, but nothing that affects functionality.
Jack Lloyd-Walters (lloydwaltersj) : | # |
Christian Grabowski (cgrabowski) : | # |
- d68d1b0... by Christian Grabowski
-
fix nits
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b fix_overlapping
STATUS: FAILED
LOG: http://
COMMIT: 2fc97d3aa94813e
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b fix_overlapping
STATUS: FAILED
LOG: http://
COMMIT: d68d1b0b3897a05
- 40348f8... by Christian Grabowski
-
fix flaky test
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b fix_overlapping
STATUS: FAILED
LOG: http://
COMMIT: 654834eb173923c
MAAS Lander (maas-lander) wrote : | # |
UNIT TESTS
-b fix_overlapping
STATUS: SUCCESS
COMMIT: 40348f8e30e9ffe
Preview Diff
1 | diff --git a/src/maasserver/dns/tests/test_zonegenerator.py b/src/maasserver/dns/tests/test_zonegenerator.py |
2 | index d213c1a..1555ae6 100644 |
3 | --- a/src/maasserver/dns/tests/test_zonegenerator.py |
4 | +++ b/src/maasserver/dns/tests/test_zonegenerator.py |
5 | @@ -1,6 +1,7 @@ |
6 | # Copyright 2014-2022 Canonical Ltd. This software is licensed under the |
7 | # GNU Affero General Public License version 3 (see the file LICENSE). |
8 | |
9 | +import os |
10 | import random |
11 | import socket |
12 | from unittest.mock import ANY, call, Mock |
13 | @@ -37,7 +38,10 @@ from maasserver.enum import IPADDRESS_TYPE, NODE_STATUS, RDNS_MODE |
14 | from maasserver.exceptions import UnresolvableHost |
15 | from maasserver.models import Config, Domain, Subnet |
16 | from maasserver.models.dnsdata import HostnameRRsetMapping |
17 | -from maasserver.models.staticipaddress import HostnameIPMapping |
18 | +from maasserver.models.staticipaddress import ( |
19 | + HostnameIPMapping, |
20 | + StaticIPAddress, |
21 | +) |
22 | from maasserver.testing.config import RegionConfigurationFixture |
23 | from maasserver.testing.factory import factory |
24 | from maasserver.testing.testcase import ( |
25 | @@ -49,6 +53,7 @@ from maastesting.factory import factory as maastesting_factory |
26 | from maastesting.fakemethod import FakeMethod |
27 | from maastesting.matchers import MockAnyCall, MockCalledOnceWith, MockNotCalled |
28 | from provisioningserver.dns.config import DynamicDNSUpdate |
29 | +from provisioningserver.dns.testing import patch_zone_file_config_path |
30 | from provisioningserver.dns.zoneconfig import ( |
31 | DNSForwardZoneConfig, |
32 | DNSReverseZoneConfig, |
33 | @@ -414,7 +419,7 @@ class TestZoneGenerator(MAASServerTestCase): |
34 | ], # purposely out of order to assert subnets are being sorted |
35 | serial=random.randint(0, 65535), |
36 | ).as_list() |
37 | - self.assertEqual(len(zones), 7) |
38 | + self.assertEqual(len(zones), 13) # 5 /24s and 8 from the /21 |
39 | expected_domains = [ |
40 | "overlap", |
41 | "36.232.10.in-addr.arpa", |
42 | @@ -728,14 +733,15 @@ class TestZoneGenerator(MAASServerTestCase): |
43 | def test_supernet_inherits_rfc2317_net(self): |
44 | domain = Domain.objects.get_default_domain() |
45 | subnet1 = factory.make_Subnet(host_bits=2) |
46 | - net = IPNetwork(subnet1.cidr) |
47 | - if net.version == 6: |
48 | + net1 = IPNetwork(subnet1.cidr) |
49 | + if net1.version == 6: |
50 | prefixlen = random.randint(121, 124) |
51 | else: |
52 | prefixlen = random.randint(22, 24) |
53 | - parent = IPNetwork("%s/%d" % (net.network, prefixlen)) |
54 | - parent = IPNetwork("%s/%d" % (parent.network, prefixlen)) |
55 | + parent = IPNetwork(f"{net1.network}/{prefixlen:d}") |
56 | + parent = IPNetwork(f"{parent.network}/{prefixlen:d}") |
57 | subnet2 = factory.make_Subnet(cidr=parent) |
58 | + net2 = IPNetwork(subnet2.cidr) |
59 | node = factory.make_Node_with_Interface_on_Subnet( |
60 | subnet=subnet1, |
61 | vlan=subnet1.vlan, |
62 | @@ -746,56 +752,51 @@ class TestZoneGenerator(MAASServerTestCase): |
63 | factory.make_StaticIPAddress(interface=boot_iface, subnet=subnet1) |
64 | default_ttl = random.randint(10, 300) |
65 | Config.objects.set_config("default_dns_ttl", default_ttl) |
66 | + serial = random.randint(0, 65535) |
67 | zones = ZoneGenerator( |
68 | domain, |
69 | [subnet1, subnet2], |
70 | default_ttl=default_ttl, |
71 | - serial=random.randint(0, 65535), |
72 | + serial=serial, |
73 | ).as_list() |
74 | - self.assertThat( |
75 | - zones, |
76 | - MatchesSetwise( |
77 | - forward_zone(domain.name), |
78 | - reverse_zone(domain.name, subnet1.cidr), |
79 | - reverse_zone(domain.name, subnet2.cidr), |
80 | + expected = [ |
81 | + DNSForwardZoneConfig( |
82 | + domain.name, |
83 | + serial=serial, |
84 | + default_ttl=default_ttl, |
85 | ), |
86 | - ) |
87 | - self.assertEqual(set(), zones[1]._rfc2317_ranges) |
88 | - self.assertEqual({net}, zones[2]._rfc2317_ranges) |
89 | - |
90 | - def test_two_managed_interfaces_yields_one_forward_two_reverse_zones(self): |
91 | - default_domain = Domain.objects.get_default_domain().name |
92 | - domain = factory.make_Domain() |
93 | - subnet1 = factory.make_Subnet() |
94 | - subnet2 = factory.make_Subnet() |
95 | - expected_zones = [ |
96 | - forward_zone(domain.name), |
97 | - reverse_zone(default_domain, subnet1.cidr), |
98 | - reverse_zone(default_domain, subnet2.cidr), |
99 | ] |
100 | - subnets = Subnet.objects.all() |
101 | - |
102 | - expected_zones = ( |
103 | - [forward_zone(domain.name)] |
104 | - + [ |
105 | - reverse_zone(default_domain, subnet.get_ipnetwork()) |
106 | - for subnet in subnets |
107 | - ] |
108 | - + [ |
109 | - reverse_zone( |
110 | - default_domain, |
111 | - self.rfc2317_network(subnet.get_ipnetwork()), |
112 | + for net in [net1, net2]: |
113 | + if ( |
114 | + net.version == 6 and net.prefixlen < 124 |
115 | + ) or net.prefixlen < 24: |
116 | + for network in ZoneGenerator._split_large_subnet(net2): |
117 | + expected.append( |
118 | + DNSReverseZoneConfig(domain.name, network=network) |
119 | + ) |
120 | + elif net.version == 6 and net.prefixlen > 124: |
121 | + expected.append( |
122 | + DNSReverseZoneConfig( |
123 | + domain.name, network=IPNetwork(f"{net.network}/124") |
124 | + ) |
125 | ) |
126 | - for subnet in subnets |
127 | - if self.rfc2317_network(subnet.get_ipnetwork()) is not None |
128 | - ] |
129 | - ) |
130 | - self.assertThat( |
131 | - ZoneGenerator( |
132 | - domain, [subnet1, subnet2], serial=random.randint(0, 65535) |
133 | - ).as_list(), |
134 | - MatchesSetwise(*expected_zones), |
135 | + expected.append(DNSReverseZoneConfig(domain.name, network=net)) |
136 | + elif net.version == 4 and net.prefixlen > 24: |
137 | + expected.append( |
138 | + DNSReverseZoneConfig( |
139 | + domain.name, network=IPNetwork(f"{net.network}/24") |
140 | + ) |
141 | + ) |
142 | + expected.append(DNSReverseZoneConfig(domain.name, network=net)) |
143 | + else: |
144 | + expected.append(DNSReverseZoneConfig(domain.name, network=net)) |
145 | + self.assertCountEqual( |
146 | + set([(zone.domain, zone._network) for zone in zones]), |
147 | + set((e.domain, e._network) for e in expected), |
148 | + f"{subnet1} {subnet2}", |
149 | ) |
150 | + self.assertEqual(set(), zones[1]._rfc2317_ranges) |
151 | + self.assertEqual({net1}, zones[2]._rfc2317_ranges) |
152 | |
153 | def test_with_many_yields_many_zones(self): |
154 | # This demonstrates ZoneGenerator in all-singing all-dancing mode. |
155 | @@ -806,18 +807,29 @@ class TestZoneGenerator(MAASServerTestCase): |
156 | subnets = Subnet.objects.all() |
157 | expected_zones = set() |
158 | for domain in domains: |
159 | - expected_zones.add(forward_zone(domain.name)) |
160 | + expected_zones.add(DNSForwardZoneConfig(domain.name)) |
161 | for subnet in subnets: |
162 | - expected_zones.add(reverse_zone(default_domain.name, subnet.cidr)) |
163 | - rfc2317_net = self.rfc2317_network(subnet.get_ipnetwork()) |
164 | - if rfc2317_net is not None: |
165 | + networks = ZoneGenerator._split_large_subnet( |
166 | + IPNetwork(subnet.cidr) |
167 | + ) |
168 | + for network in networks: |
169 | expected_zones.add( |
170 | - reverse_zone(default_domain.name, rfc2317_net.cidr) |
171 | + DNSReverseZoneConfig(default_domain.name, network=network) |
172 | ) |
173 | + if rfc2317_net := self.rfc2317_network(network): |
174 | + expected_zones.add( |
175 | + DNSReverseZoneConfig( |
176 | + default_domain.name, |
177 | + network=IPNetwork(rfc2317_net.cidr), |
178 | + ) |
179 | + ) |
180 | actual_zones = ZoneGenerator( |
181 | domains, subnets, serial=random.randint(0, 65535) |
182 | ).as_list() |
183 | - self.assertThat(actual_zones, MatchesSetwise(*expected_zones)) |
184 | + self.assertCountEqual( |
185 | + [(zone.domain, zone._network) for zone in actual_zones], |
186 | + [(zone.domain, zone._network) for zone in expected_zones], |
187 | + ) |
188 | |
189 | def test_zone_generator_handles_rdns_mode_equal_enabled(self): |
190 | Domain.objects.get_or_create(name="one") |
191 | @@ -898,6 +910,359 @@ class TestZoneGenerator(MAASServerTestCase): |
192 | ), |
193 | ) |
194 | |
195 | + def test_configs_are_merged_when_overlapping(self): |
196 | + self.patch(warn_loopback) |
197 | + default_domain = Domain.objects.get_default_domain() |
198 | + subnet1 = factory.make_Subnet(cidr="10.0.1.0/24") |
199 | + subnet2 = factory.make_Subnet(cidr="10.0.0.0/21") |
200 | + subnet1_ips = [ |
201 | + factory.make_StaticIPAddress( |
202 | + ip=factory.pick_ip_in_Subnet(subnet1), subnet=subnet1 |
203 | + ) |
204 | + for _ in range(3) |
205 | + ] |
206 | + subnet2_ips = [ |
207 | + factory.make_StaticIPAddress( |
208 | + ip=factory.pick_ip_in_Subnet(subnet2), subnet=subnet2 |
209 | + ) |
210 | + for _ in range(3) |
211 | + ] |
212 | + subnet1_records = [ |
213 | + factory.make_DNSResource(domain=default_domain, ip_addresses=[ip]) |
214 | + for ip in subnet1_ips |
215 | + ] |
216 | + subnet2_records = [ |
217 | + factory.make_DNSResource(domain=default_domain, ip_addresses=[ip]) |
218 | + for ip in subnet2_ips |
219 | + ] |
220 | + serial = random.randint(0, 65535) |
221 | + dynamic_updates = [ |
222 | + DynamicDNSUpdate( |
223 | + operation="INSERT", |
224 | + name=record.name, |
225 | + zone=default_domain.name, |
226 | + rectype="A", |
227 | + ttl=record.address_ttl, |
228 | + answer=ip.ip, |
229 | + ) |
230 | + for record in subnet1_records + subnet2_records |
231 | + for ip in record.ip_addresses.all() |
232 | + ] |
233 | + zones = ZoneGenerator( |
234 | + [default_domain], |
235 | + [subnet1, subnet2], |
236 | + serial=serial, |
237 | + dynamic_updates=dynamic_updates, |
238 | + ).as_list() |
239 | + |
240 | + def _generate_mapping_for_network(network, records): |
241 | + mapping = {} |
242 | + for record in records: |
243 | + if ip_set := set( |
244 | + ip.ip |
245 | + for ip in record.ip_addresses.all() |
246 | + if IPAddress(ip.ip) in network |
247 | + ): |
248 | + mapping[ |
249 | + f"{record.name}.{default_domain.name}" |
250 | + ] = HostnameIPMapping( |
251 | + None, |
252 | + record.address_ttl, |
253 | + ip_set, |
254 | + None, |
255 | + 1, |
256 | + None, |
257 | + ) |
258 | + return mapping |
259 | + |
260 | + expected = [ |
261 | + DNSForwardZoneConfig( |
262 | + default_domain.name, |
263 | + mapping={ |
264 | + record.name: HostnameIPMapping( |
265 | + None, |
266 | + record.address_ttl, |
267 | + set(record.ip_addresses.all()), |
268 | + None, |
269 | + 1, |
270 | + None, |
271 | + ) |
272 | + for record in subnet1_records + subnet2_records |
273 | + }, |
274 | + dynamic_updates=dynamic_updates, |
275 | + ), |
276 | + DNSReverseZoneConfig( |
277 | + default_domain.name, |
278 | + network=IPNetwork(subnet1.cidr), |
279 | + mapping=_generate_mapping_for_network( |
280 | + IPNetwork(subnet1.cidr), subnet1_records + subnet2_records |
281 | + ), |
282 | + dynamic_updates=[ |
283 | + DynamicDNSUpdate.as_reverse_record_update( |
284 | + update, IPNetwork(subnet1.cidr) |
285 | + ) |
286 | + for update in dynamic_updates |
287 | + if update.answer_as_ip in IPNetwork(subnet1.cidr) |
288 | + ], |
289 | + ), |
290 | + DNSReverseZoneConfig( |
291 | + default_domain.name, |
292 | + network=IPNetwork("10.0.0.0/24"), |
293 | + mapping=_generate_mapping_for_network( |
294 | + IPNetwork("10.0.0.0/24"), subnet2_records |
295 | + ), |
296 | + dynamic_updates=[ |
297 | + DynamicDNSUpdate.as_reverse_record_update( |
298 | + update, IPNetwork("10.0.0.0/24") |
299 | + ) |
300 | + for update in dynamic_updates |
301 | + if update.answer_as_ip in IPNetwork("10.0.0.0/24") |
302 | + ], |
303 | + ), |
304 | + DNSReverseZoneConfig( |
305 | + default_domain.name, |
306 | + network=IPNetwork("10.0.2.0/24"), |
307 | + mapping=_generate_mapping_for_network( |
308 | + IPNetwork("10.0.2.0/24"), subnet2_records |
309 | + ), |
310 | + dynamic_updates=[ |
311 | + DynamicDNSUpdate.as_reverse_record_update( |
312 | + update, IPNetwork("10.0.2.0/24") |
313 | + ) |
314 | + for update in dynamic_updates |
315 | + if update.answer_as_ip in IPNetwork("10.0.2.0/24") |
316 | + ], |
317 | + ), |
318 | + DNSReverseZoneConfig( |
319 | + default_domain.name, |
320 | + network=IPNetwork("10.0.3.0/24"), |
321 | + mapping=_generate_mapping_for_network( |
322 | + IPNetwork("10.0.3.0/24"), subnet2_records |
323 | + ), |
324 | + dynamic_updates=[ |
325 | + DynamicDNSUpdate.as_reverse_record_update( |
326 | + update, IPNetwork("10.0.3.0/24") |
327 | + ) |
328 | + for update in dynamic_updates |
329 | + if update.answer_as_ip in IPNetwork("10.0.3.0/24") |
330 | + ], |
331 | + ), |
332 | + DNSReverseZoneConfig( |
333 | + default_domain.name, |
334 | + network=IPNetwork("10.0.4.0/24"), |
335 | + mapping=_generate_mapping_for_network( |
336 | + IPNetwork("10.0.4.0/24"), subnet2_records |
337 | + ), |
338 | + dynamic_updates=[ |
339 | + DynamicDNSUpdate.as_reverse_record_update( |
340 | + update, IPNetwork("10.0.4.0/24") |
341 | + ) |
342 | + for update in dynamic_updates |
343 | + if update.answer_as_ip in IPNetwork("10.0.4.0/24") |
344 | + ], |
345 | + ), |
346 | + DNSReverseZoneConfig( |
347 | + default_domain.name, |
348 | + network=IPNetwork("10.0.5.0/24"), |
349 | + mapping=_generate_mapping_for_network( |
350 | + IPNetwork("10.0.5.0/24"), subnet2_records |
351 | + ), |
352 | + dynamic_updates=[ |
353 | + DynamicDNSUpdate.as_reverse_record_update( |
354 | + update, IPNetwork("10.0.5.0/24") |
355 | + ) |
356 | + for update in dynamic_updates |
357 | + if update.answer_as_ip in IPNetwork("10.0.5.0/24") |
358 | + ], |
359 | + ), |
360 | + DNSReverseZoneConfig( |
361 | + default_domain.name, |
362 | + network=IPNetwork("10.0.6.0/24"), |
363 | + mapping=_generate_mapping_for_network( |
364 | + IPNetwork("10.0.6.0/24"), subnet2_records |
365 | + ), |
366 | + dynamic_updates=[ |
367 | + DynamicDNSUpdate.as_reverse_record_update( |
368 | + update, IPNetwork("10.0.6.0/24") |
369 | + ) |
370 | + for update in dynamic_updates |
371 | + if update.answer_as_ip in IPNetwork("10.0.6.0/24") |
372 | + ], |
373 | + ), |
374 | + DNSReverseZoneConfig( |
375 | + default_domain.name, |
376 | + network=IPNetwork("10.0.7.0/24"), |
377 | + mapping=_generate_mapping_for_network( |
378 | + IPNetwork("10.0.7.0/24"), subnet2_records |
379 | + ), |
380 | + dynamic_updates=[ |
381 | + DynamicDNSUpdate.as_reverse_record_update( |
382 | + update, IPNetwork("10.0.7.0/24") |
383 | + ) |
384 | + for update in dynamic_updates |
385 | + if update.answer_as_ip in IPNetwork("10.0.7.0/24") |
386 | + ], |
387 | + ), |
388 | + ] |
389 | + |
390 | + for i, zone in enumerate(zones): |
391 | + self.assertEqual(zone.domain, expected[i].domain) |
392 | + self.assertEqual(zone._network, expected[i]._network) |
393 | + self.assertCountEqual( |
394 | + zone._mapping, |
395 | + expected[i]._mapping, |
396 | + ) |
397 | + self.assertCountEqual( |
398 | + zone._dynamic_updates, expected[i]._dynamic_updates |
399 | + ) |
400 | + if isinstance(zone, DNSReverseZoneConfig): |
401 | + self.assertCountEqual( |
402 | + zone._dynamic_ranges, expected[i]._dynamic_ranges |
403 | + ) |
404 | + self.assertCountEqual( |
405 | + zone._rfc2317_ranges, expected[i]._rfc2317_ranges |
406 | + ) |
407 | + |
408 | + def test_configs_are_merged_when_glue_overlaps(self): |
409 | + self.patch(warn_loopback) |
410 | + default_domain = Domain.objects.get_default_domain() |
411 | + subnet1 = factory.make_Subnet(cidr="10.0.1.0/24") |
412 | + subnet2 = factory.make_Subnet(cidr="10.0.1.0/26") |
413 | + subnet1_ips = [ |
414 | + factory.make_StaticIPAddress( |
415 | + ip=f"10.0.1.{253 + i}", |
416 | + subnet=subnet1, # avoid allocation collision |
417 | + ) |
418 | + for i in range(3) |
419 | + ] |
420 | + subnet2_ips = [ |
421 | + factory.make_StaticIPAddress( |
422 | + ip=factory.pick_ip_in_Subnet(subnet2), subnet=subnet2 |
423 | + ) |
424 | + for _ in range(3) |
425 | + ] |
426 | + subnet1_records = [ |
427 | + factory.make_DNSResource(domain=default_domain, ip_addresses=[ip]) |
428 | + for ip in subnet1_ips |
429 | + ] |
430 | + subnet2_records = [ |
431 | + factory.make_DNSResource(domain=default_domain, ip_addresses=[ip]) |
432 | + for ip in subnet2_ips |
433 | + ] |
434 | + serial = random.randint(0, 65535) |
435 | + dynamic_updates = [ |
436 | + DynamicDNSUpdate( |
437 | + operation="INSERT", |
438 | + name=record.name, |
439 | + zone=default_domain.name, |
440 | + rectype="A", |
441 | + ttl=record.address_ttl, |
442 | + answer=ip.ip, |
443 | + ) |
444 | + for record in subnet1_records + subnet2_records |
445 | + for ip in record.ip_addresses.all() |
446 | + ] |
447 | + zones = ZoneGenerator( |
448 | + [default_domain], |
449 | + [subnet1, subnet2], |
450 | + serial=serial, |
451 | + dynamic_updates=dynamic_updates, |
452 | + ).as_list() |
453 | + |
454 | + def _generate_mapping_for_network(network, other_network, records): |
455 | + mapping = {} |
456 | + for record in records: |
457 | + ip_set = set( |
458 | + ip.ip |
459 | + for ip in record.ip_addresses.all() |
460 | + if IPAddress(ip.ip) in network |
461 | + and ( |
462 | + IPAddress(ip.ip) not in other_network |
463 | + or other_network.prefixlen < network.prefixlen |
464 | + ) |
465 | + ) |
466 | + if len(ip_set) > 0: |
467 | + mapping[ |
468 | + f"{record.name}.{default_domain.name}" |
469 | + ] = HostnameIPMapping( |
470 | + None, |
471 | + record.address_ttl, |
472 | + ip_set, |
473 | + None, |
474 | + 1, |
475 | + None, |
476 | + ) |
477 | + return mapping |
478 | + |
479 | + expected = [ |
480 | + DNSForwardZoneConfig( |
481 | + default_domain.name, |
482 | + mapping={ |
483 | + record.name: HostnameIPMapping( |
484 | + None, |
485 | + record.address_ttl, |
486 | + set(ip.ip for ip in record.ip_addresses.all()), |
487 | + None, |
488 | + 1, |
489 | + None, |
490 | + ) |
491 | + for record in subnet1_records + subnet2_records |
492 | + }, |
493 | + dynamic_updates=dynamic_updates, |
494 | + ), |
495 | + DNSReverseZoneConfig( |
496 | + default_domain.name, |
497 | + network=IPNetwork(subnet2.cidr), |
498 | + mapping=_generate_mapping_for_network( |
499 | + IPNetwork(subnet2.cidr), |
500 | + IPNetwork(subnet1.cidr), |
501 | + subnet1_records + subnet2_records, |
502 | + ), |
503 | + dynamic_updates=[ |
504 | + DynamicDNSUpdate.as_reverse_record_update( |
505 | + update, IPNetwork(subnet2.cidr) |
506 | + ) |
507 | + for update in dynamic_updates |
508 | + if update.answer_as_ip in IPNetwork(subnet2.cidr) |
509 | + ], |
510 | + ), |
511 | + DNSReverseZoneConfig( |
512 | + default_domain.name, |
513 | + network=IPNetwork(subnet1.cidr), |
514 | + mapping=_generate_mapping_for_network( |
515 | + IPNetwork(subnet1.cidr), |
516 | + IPNetwork(subnet2.cidr), |
517 | + subnet1_records + subnet2_records, |
518 | + ), |
519 | + dynamic_updates=[ |
520 | + DynamicDNSUpdate.as_reverse_record_update( |
521 | + update, IPNetwork(subnet1.cidr) |
522 | + ) |
523 | + for update in dynamic_updates |
524 | + if update.answer_as_ip in IPNetwork(subnet1.cidr) |
525 | + ], |
526 | + rfc2317_ranges=set([IPNetwork(subnet2.cidr)]), |
527 | + ), |
528 | + ] |
529 | + |
530 | + for i, zone in enumerate(zones): |
531 | + self.assertEqual(zone.domain, expected[i].domain) |
532 | + self.assertEqual(zone._network, expected[i]._network) |
533 | + self.assertCountEqual( |
534 | + zone._mapping, |
535 | + expected[i]._mapping, |
536 | + ) |
537 | + self.assertCountEqual( |
538 | + zone._dynamic_updates, expected[i]._dynamic_updates |
539 | + ) |
540 | + if isinstance(zone, DNSReverseZoneConfig): |
541 | + self.assertCountEqual( |
542 | + zone._dynamic_ranges, expected[i]._dynamic_ranges |
543 | + ) |
544 | + self.assertCountEqual( |
545 | + zone._rfc2317_ranges, expected[i]._rfc2317_ranges |
546 | + ) |
547 | + |
548 | |
549 | class TestZoneGeneratorTTL(MAASTransactionServerTestCase): |
550 | """Tests for TTL in :class:ZoneGenerator`.""" |
551 | @@ -1006,7 +1371,22 @@ class TestZoneGeneratorTTL(MAASTransactionServerTestCase): |
552 | serial=random.randint(0, 65535), |
553 | ).as_list() |
554 | self.assertEqual(expected_forward, zones[0]._mapping) |
555 | - self.assertEqual(expected_reverse, zones[1]._mapping) |
556 | + for zone in zones[1:]: |
557 | + if ip_set := set( |
558 | + ip |
559 | + for ip in expected_reverse[node.fqdn].ips |
560 | + if IPAddress(ip) in zone._network |
561 | + ): |
562 | + expected_rev = { |
563 | + node.fqdn: HostnameIPMapping( |
564 | + node.system_id, |
565 | + node.address_ttl, |
566 | + ip_set, |
567 | + node.node_type, |
568 | + dnsrr.id, |
569 | + ) |
570 | + } |
571 | + self.assertEqual(expected_rev, zone._mapping) |
572 | |
573 | @transactional |
574 | def test_dnsresource_address_overrides_domain(self): |
575 | @@ -1058,7 +1438,22 @@ class TestZoneGeneratorTTL(MAASTransactionServerTestCase): |
576 | serial=random.randint(0, 65535), |
577 | ).as_list() |
578 | self.assertEqual(expected_forward, zones[0]._mapping) |
579 | - self.assertEqual(expected_reverse, zones[1]._mapping) |
580 | + |
581 | + for zone in zones[1:]: |
582 | + expected = {} |
583 | + for expected_label, expected_mapping in expected_reverse.items(): |
584 | + if ip_set := set( |
585 | + ip for ip in expected_mapping.ips if ip in zone._network |
586 | + ): |
587 | + expected[expected_label] = HostnameIPMapping( |
588 | + system_id=expected_mapping.system_id, |
589 | + ttl=expected_mapping.ttl, |
590 | + ips=ip_set, |
591 | + node_type=expected_mapping.node_type, |
592 | + dnsresource_id=expected_mapping.dnsresource_id, |
593 | + user_id=expected_mapping.user_id, |
594 | + ) |
595 | + self.assertEqual(expected, zone._mapping) |
596 | |
597 | @transactional |
598 | def test_dnsdata_inherits_global(self): |
599 | @@ -1167,3 +1562,123 @@ class TestZoneGeneratorTTL(MAASTransactionServerTestCase): |
600 | [zone_config] = ZoneGenerator(domains=[domain], subnets=[], serial=123) |
601 | self.assertEqual(domain.name, zone_config.domain) |
602 | self.assertEqual(42, zone_config.default_ttl) |
603 | + |
604 | + |
605 | +class TestZoneGeneratorEndToEnd(MAASServerTestCase): |
606 | + def _find_most_specific_subnet( |
607 | + self, ip: StaticIPAddress, subnets: list[Subnet] |
608 | + ): |
609 | + networks = [] |
610 | + for subnet in subnets: |
611 | + net = IPNetwork(subnet.cidr) |
612 | + if net.prefixlen < 24: |
613 | + networks += ZoneGenerator._split_large_subnet(net) |
614 | + else: |
615 | + networks.append(net) |
616 | + sorted_nets = sorted(networks, key=lambda net: -1 * net.prefixlen) |
617 | + for net in sorted_nets: |
618 | + if IPAddress(ip.ip) in net: |
619 | + return net |
620 | + |
621 | + def test_ZoneGenerator_generates_config_for_zone_files(self): |
622 | + config_path = patch_zone_file_config_path(self) |
623 | + default_domain = Domain.objects.get_default_domain() |
624 | + domain = factory.make_Domain() |
625 | + subnet1 = factory.make_Subnet(cidr="10.0.1.0/24") |
626 | + subnet2 = factory.make_Subnet(cidr="10.0.0.0/22") |
627 | + subnet3 = factory.make_Subnet(cidr="10.0.1.0/27") |
628 | + subnet1_ips = [ |
629 | + factory.make_StaticIPAddress( |
630 | + ip=factory.pick_ip_in_Subnet(subnet1), subnet=subnet1 |
631 | + ) |
632 | + for _ in range(3) |
633 | + ] |
634 | + subnet2_ips = [ |
635 | + factory.make_StaticIPAddress( |
636 | + ip=factory.pick_ip_in_Subnet( |
637 | + subnet2, but_not=list(subnet1.get_ipranges_in_use()) |
638 | + ), |
639 | + subnet=subnet2, |
640 | + ) |
641 | + for _ in range(3) |
642 | + ] |
643 | + subnet3_ips = [ |
644 | + factory.make_StaticIPAddress( |
645 | + ip=factory.pick_ip_in_Subnet( |
646 | + subnet3, |
647 | + but_not=list(subnet1.get_ipranges_in_use()) |
648 | + + list(subnet2.get_ipranges_in_use()), |
649 | + ), |
650 | + subnet=subnet3, |
651 | + ) |
652 | + for _ in range(3) |
653 | + ] |
654 | + subnet1_records = [ |
655 | + factory.make_DNSResource( |
656 | + domain=random.choice((default_domain, domain)), |
657 | + ip_addresses=[ip], |
658 | + ) |
659 | + for ip in subnet1_ips |
660 | + ] |
661 | + subnet2_records = [ |
662 | + factory.make_DNSResource( |
663 | + domain=random.choice((default_domain, domain)), |
664 | + ip_addresses=[ip], |
665 | + ) |
666 | + for ip in subnet2_ips |
667 | + ] |
668 | + subnet3_records = [ |
669 | + factory.make_DNSResource( |
670 | + domain=random.choice((default_domain, domain)), |
671 | + ip_addresses=[ip], |
672 | + ) |
673 | + for ip in subnet3_ips |
674 | + ] |
675 | + all_records = subnet1_records + subnet2_records + subnet3_records |
676 | + zones = ZoneGenerator( |
677 | + [default_domain, domain], |
678 | + [subnet1, subnet2, subnet3], |
679 | + serial=random.randint(0, 65535), |
680 | + ).as_list() |
681 | + for zone in zones: |
682 | + zone.write_config() |
683 | + |
684 | + # check forward zones |
685 | + with open( |
686 | + os.path.join(config_path, f"zone.{default_domain.name}"), "r" |
687 | + ) as zf: |
688 | + default_domain_contents = zf.read() |
689 | + |
690 | + with open(os.path.join(config_path, f"zone.{domain.name}"), "r") as zf: |
691 | + domain_contents = zf.read() |
692 | + |
693 | + for record in all_records: |
694 | + if record.domain == default_domain: |
695 | + contents = default_domain_contents |
696 | + else: |
697 | + contents = domain_contents |
698 | + |
699 | + self.assertIn( |
700 | + f"{record.name} 30 IN A {record.ip_addresses.first().ip}", |
701 | + contents, |
702 | + ) |
703 | + |
704 | + # check reverse zones |
705 | + for record in all_records: |
706 | + ip = record.ip_addresses.first() |
707 | + subnet = self._find_most_specific_subnet( |
708 | + ip, [subnet1, subnet2, subnet3] |
709 | + ) |
710 | + rev_subnet = ".".join(str(subnet.network).split(".")[2::-1]) |
711 | + if subnet.prefixlen > 24: |
712 | + rev_subnet = f"{str(subnet.network).split('.')[-1]}-{subnet.prefixlen}.{rev_subnet}" |
713 | + with open( |
714 | + os.path.join(config_path, f"zone.{rev_subnet}.in-addr.arpa"), |
715 | + "r", |
716 | + ) as zf: |
717 | + contents = zf.read() |
718 | + self.assertIn( |
719 | + f"{ip.ip.split('.')[-1]} 30 IN PTR {record.fqdn}", |
720 | + contents, |
721 | + f"{subnet} {ip.ip}", |
722 | + ) |
723 | diff --git a/src/maasserver/dns/zonegenerator.py b/src/maasserver/dns/zonegenerator.py |
724 | index b6fd406..8272e5c 100644 |
725 | --- a/src/maasserver/dns/zonegenerator.py |
726 | +++ b/src/maasserver/dns/zonegenerator.py |
727 | @@ -18,7 +18,11 @@ from maasserver.models.config import Config |
728 | from maasserver.models.dnsdata import DNSData, HostnameRRsetMapping |
729 | from maasserver.models.dnsresource import separate_fqdn |
730 | from maasserver.models.domain import Domain |
731 | -from maasserver.models.staticipaddress import StaticIPAddress |
732 | +from maasserver.models.iprange import IPRange |
733 | +from maasserver.models.staticipaddress import ( |
734 | + HostnameIPMapping, |
735 | + StaticIPAddress, |
736 | +) |
737 | from maasserver.models.subnet import Subnet |
738 | from maasserver.server_address import get_maas_facing_server_addresses |
739 | from provisioningserver.dns.config import DynamicDNSUpdate |
740 | @@ -226,6 +230,7 @@ class ZoneGenerator: |
741 | if self._dynamic_updates is None: |
742 | self._dynamic_updates = [] |
743 | self.force_config_write = force_config_write # some data changed that nsupdate cannot update if true |
744 | + self._existing_subnet_cfgs = {} |
745 | |
746 | @staticmethod |
747 | def _get_mappings(): |
748 | @@ -357,18 +362,93 @@ class ZoneGenerator: |
749 | ) |
750 | |
751 | @staticmethod |
752 | - def _gen_reverse_zones( |
753 | - subnets, |
754 | - serial, |
755 | - ns_host_name, |
756 | - mappings, |
757 | - default_ttl, |
758 | - dynamic_updates, |
759 | - force_config_write, |
760 | + def _split_large_subnet(network: IPNetwork) -> list[IPNetwork]: |
761 | + # Generate the name of the reverse zone file: |
762 | + # Use netaddr's reverse_dns() to get the reverse IP name |
763 | + # of the first IP address in the network and then drop the first |
764 | + # octets of that name (i.e. drop the octets that will be specified in |
765 | + # the zone file). |
766 | + # returns a list of (IPNetwork, zone_name, zonefile_path) tuples |
767 | + new_networks = [] |
768 | + first = IPAddress(network.first) |
769 | + last = IPAddress(network.last) |
770 | + if first.version == 6: |
771 | + # IPv6. |
772 | + # 2001:89ab::/19 yields 8.1.0.0.2.ip6.arpa, and the full list |
773 | + # is 8.1.0.0.2.ip6.arpa, 9.1.0.0.2.ip6.arpa |
774 | + # The ipv6 reverse dns form is 32 elements of 1 hex digit each. |
775 | + # How many elements of the reverse DNS name to we throw away? |
776 | + # Prefixlen of 0-3 gives us 1, 4-7 gives us 2, etc. |
777 | + # While this seems wrong, we always _add_ a base label back in, |
778 | + # so it's correct. |
779 | + rest_limit = (132 - network.prefixlen) // 4 |
780 | + # What is the prefix for each inner subnet (It will be the next |
781 | + # smaller multiple of 4.) If it's the smallest one, then RFC2317 |
782 | + # tells us that we're adding an extra blob to the front of the |
783 | + # reverse zone name, and we want the entire prefixlen. |
784 | + subnet_prefix = (network.prefixlen + 3) // 4 * 4 |
785 | + if subnet_prefix == 128: |
786 | + subnet_prefix = network.prefixlen |
787 | + # How big is the step between subnets? Again, special case for |
788 | + # extra small subnets. |
789 | + step = 1 << ((128 - network.prefixlen) // 4 * 4) |
790 | + if step < 16: |
791 | + step = 16 |
792 | + # Grab the base (hex) and trailing labels for our reverse zone. |
793 | + split_zone = first.reverse_dns.split(".") |
794 | + base = int(split_zone[rest_limit - 1], 16) |
795 | + else: |
796 | + # IPv4. |
797 | + # The logic here is the same as for IPv6, but with 8 instead of 4. |
798 | + rest_limit = (40 - network.prefixlen) // 8 |
799 | + subnet_prefix = (network.prefixlen + 7) // 8 * 8 |
800 | + if subnet_prefix == 32: |
801 | + subnet_prefix = network.prefixlen |
802 | + step = 1 << ((32 - network.prefixlen) // 8 * 8) |
803 | + if step < 256: |
804 | + step = 256 |
805 | + # Grab the base (decimal) and trailing labels for our reverse |
806 | + # zone. |
807 | + split_zone = first.reverse_dns.split(".") |
808 | + base = int(split_zone[rest_limit - 1]) |
809 | + |
810 | + while first <= last: |
811 | + if first > last: |
812 | + # if the excluding subnet pushes the base IP beyond the bounds of the generating subnet, we've reached the end and return early |
813 | + return new_networks |
814 | + |
815 | + new_networks.append(IPNetwork(f"{first}/{subnet_prefix:d}")) |
816 | + base += 1 |
817 | + try: |
818 | + first += step |
819 | + except IndexError: |
820 | + # IndexError occurs when we go from 255.255.255.255 to |
821 | + # 0.0.0.0. If we hit that, we're all fine and done. |
822 | + break |
823 | + return new_networks |
824 | + |
825 | + @staticmethod |
826 | + def _filter_mapping_for_network( |
827 | + network: IPNetwork, mappings: dict[str, HostnameIPMapping] |
828 | ): |
829 | - """Generator of reverse zones, sorted by network.""" |
830 | + net_mappings = {} |
831 | + for k, v in mappings.items(): |
832 | + if ips_in_net := set( |
833 | + ip for ip in v.ips if IPAddress(ip) in network |
834 | + ): |
835 | + net_mappings[k] = HostnameIPMapping( |
836 | + v.system_id, |
837 | + v.ttl, |
838 | + ips_in_net, |
839 | + v.node_type, |
840 | + v.dnsresource_id, |
841 | + v.user_id, |
842 | + ) |
843 | |
844 | - subnets = set(subnets) |
845 | + return net_mappings |
846 | + |
847 | + @staticmethod |
848 | + def _generate_glue_nets(subnets: list[Subnet]): |
849 | # Generate the list of parent networks for rfc2317 glue. Note that we |
850 | # need to handle the case where we are controlling both the small net |
851 | # and a bigger network containing the /24, not just a /24 network. |
852 | @@ -393,6 +473,82 @@ class ZoneGenerator: |
853 | ) |
854 | rfc2317_glue.setdefault(basenet, set()).add(network) |
855 | |
856 | + return rfc2317_glue |
857 | + |
858 | + @staticmethod |
859 | + def _find_glue_nets( |
860 | + network: IPNetwork, rfc2317_glue: defaultdict[str, set[IPNetwork]] |
861 | + ): |
862 | + # Use the default_domain as the name for the NS host in the reverse |
863 | + # zones. If this network is actually a parent rfc2317 glue |
864 | + # network, then we need to generate the glue records. |
865 | + # We need to detect the need for glue in our networks that are |
866 | + # big. |
867 | + if ( |
868 | + network.version == 6 and network.prefixlen < 124 |
869 | + ) or network.prefixlen < 24: |
870 | + glue = set() |
871 | + # This is the reason for needing the subnets sorted in |
872 | + # increasing order of size. |
873 | + for net in rfc2317_glue.copy().keys(): |
874 | + if net in network: |
875 | + glue.update(rfc2317_glue[net]) |
876 | + del rfc2317_glue[net] |
877 | + elif network in rfc2317_glue: |
878 | + glue = rfc2317_glue[network] |
879 | + del rfc2317_glue[network] |
880 | + else: |
881 | + glue = set() |
882 | + return glue |
883 | + |
884 | + @staticmethod |
885 | + def _merge_into_existing_network( |
886 | + network: IPNetwork, |
887 | + existing: dict[IPNetwork, DNSReverseZoneConfig], |
888 | + mapping: dict[str, HostnameIPMapping], |
889 | + dynamic_ranges: list[IPRange] | None = [], |
890 | + dynamic_updates: list[DynamicDNSUpdate] | None = [], |
891 | + glue: set[IPNetwork] | None = set(), |
892 | + is_glue_net: bool = False, |
893 | + ): |
894 | + # since all dynamic updates are passed and we then filter for those belonging |
895 | + # in the network, the existing config already has all updates and we do not need |
896 | + # to merge them, just add them if they haven't already |
897 | + if not existing[network]._dynamic_updates: |
898 | + existing[network]._dynamic_updates = dynamic_updates |
899 | + existing[network]._rfc2317_ranges = existing[ |
900 | + network |
901 | + ]._rfc2317_ranges.union(glue) |
902 | + for k, v in mapping.items(): |
903 | + if k in existing[network]._mapping: |
904 | + existing[network]._mapping[k].ips.union(v.ips) |
905 | + else: |
906 | + existing[network]._mapping[k] = v |
907 | + existing[network]._dynamic_ranges += dynamic_ranges |
908 | + for glue_net in glue.union(existing[network]._rfc2317_ranges): |
909 | + for k, v in existing[network]._mapping.copy().items(): |
910 | + if ip_set := set(ip for ip in v.ips if ip not in glue_net): |
911 | + existing[network]._mapping[k].ips = ip_set |
912 | + else: |
913 | + del existing[network]._mapping[k] |
914 | + |
915 | + @staticmethod |
916 | + def _gen_reverse_zones( |
917 | + subnets, |
918 | + serial, |
919 | + ns_host_name, |
920 | + mappings, |
921 | + default_ttl, |
922 | + dynamic_updates, |
923 | + force_config_write, |
924 | + existing_subnet_cfgs={}, |
925 | + ): |
926 | + """Generator of reverse zones, sorted by network.""" |
927 | + |
928 | + subnets = set(subnets) |
929 | + |
930 | + rfc2317_glue = ZoneGenerator._generate_glue_nets(subnets) |
931 | + |
932 | # Since get_hostname_ip_mapping(Subnet) ignores Subnet.id, so we can |
933 | # just do it once and be happy. LP#1600259 |
934 | if len(subnets): |
935 | @@ -412,7 +568,7 @@ class ZoneGenerator: |
936 | key=lambda subnet: IPNetwork(subnet.cidr).prefixlen, |
937 | reverse=True, |
938 | ): |
939 | - network = IPNetwork(subnet.cidr) |
940 | + base_network = IPNetwork(subnet.cidr) |
941 | if subnet.rdns_mode == RDNS_MODE.DISABLED: |
942 | # If we are not doing reverse dns for this subnet, then just |
943 | # skip to the next subnet. |
944 | @@ -421,103 +577,109 @@ class ZoneGenerator: |
945 | ) |
946 | continue |
947 | |
948 | + networks = ZoneGenerator._split_large_subnet(base_network) |
949 | + |
950 | # 1. Figure out the dynamic ranges. |
951 | dynamic_ranges = [ |
952 | ip_range.netaddr_iprange |
953 | for ip_range in subnet.get_dynamic_ranges() |
954 | ] |
955 | |
956 | - # 2. Start with the map of all of the nodes, including all |
957 | - # DNSResource-associated addresses. We will prune this to just |
958 | - # entries for the subnet when we actually generate the zonefile. |
959 | - # If we get here, then we have subnets, so we noticed that above |
960 | - # and created mappings['reverse']. LP#1600259 |
961 | - mapping = mappings["reverse"] |
962 | - |
963 | - # Use the default_domain as the name for the NS host in the reverse |
964 | - # zones. If this network is actually a parent rfc2317 glue |
965 | - # network, then we need to generate the glue records. |
966 | - # We need to detect the need for glue in our networks that are |
967 | - # big. |
968 | - if ( |
969 | - network.version == 6 and network.prefixlen < 124 |
970 | - ) or network.prefixlen < 24: |
971 | - glue = set() |
972 | - # This is the reason for needing the subnets sorted in |
973 | - # increasing order of size. |
974 | - for net in rfc2317_glue.copy().keys(): |
975 | - if net in network: |
976 | - glue.update(rfc2317_glue[net]) |
977 | - del rfc2317_glue[net] |
978 | - elif network in rfc2317_glue: |
979 | - glue = rfc2317_glue[network] |
980 | - del rfc2317_glue[network] |
981 | - else: |
982 | - glue = set() |
983 | + for network in networks: |
984 | + # 2. Start with the map of all of the nodes, including all |
985 | + # DNSResource-associated addresses. We will prune this to just |
986 | + # entries for the subnet when we actually generate the zonefile. |
987 | + # If we get here, then we have subnets, so we noticed that above |
988 | + # and created mappings['reverse']. LP#1600259 |
989 | + mapping = ZoneGenerator._filter_mapping_for_network( |
990 | + network, mappings["reverse"] |
991 | + ) |
992 | |
993 | - domain_updates = [ |
994 | - DynamicDNSUpdate.as_reverse_record_update(update, network) |
995 | - for update in dynamic_updates |
996 | - if update.answer |
997 | - and update.answer_is_ip |
998 | - and (update.answer_as_ip in network) |
999 | - ] |
1000 | + glue = ZoneGenerator._find_glue_nets(network, rfc2317_glue) |
1001 | + domain_updates = [ |
1002 | + DynamicDNSUpdate.as_reverse_record_update(update, network) |
1003 | + for update in dynamic_updates |
1004 | + if update.answer |
1005 | + and update.answer_is_ip |
1006 | + and (update.answer_as_ip in network) |
1007 | + ] |
1008 | + |
1009 | + if network in existing_subnet_cfgs: |
1010 | + ZoneGenerator._merge_into_existing_network( |
1011 | + network, |
1012 | + existing_subnet_cfgs, |
1013 | + mapping, |
1014 | + dynamic_ranges=dynamic_ranges, |
1015 | + dynamic_updates=domain_updates, |
1016 | + glue=glue, |
1017 | + ) |
1018 | + else: |
1019 | + existing_subnet_cfgs[network] = DNSReverseZoneConfig( |
1020 | + ns_host_name, |
1021 | + serial=serial, |
1022 | + default_ttl=default_ttl, |
1023 | + ns_host_name=ns_host_name, |
1024 | + mapping=mapping, |
1025 | + network=network, |
1026 | + dynamic_ranges=dynamic_ranges, |
1027 | + rfc2317_ranges=glue, |
1028 | + dynamic_updates=domain_updates, |
1029 | + force_config_write=force_config_write, |
1030 | + ) |
1031 | |
1032 | - yield DNSReverseZoneConfig( |
1033 | - ns_host_name, |
1034 | - serial=serial, |
1035 | - default_ttl=default_ttl, |
1036 | - ns_host_name=ns_host_name, |
1037 | - mapping=mapping, |
1038 | - network=network, |
1039 | - dynamic_ranges=dynamic_ranges, |
1040 | - rfc2317_ranges=glue, |
1041 | - exclude={ |
1042 | - IPNetwork(s.cidr) for s in subnets if s is not subnet |
1043 | - }, |
1044 | - dynamic_updates=domain_updates, |
1045 | - force_config_write=force_config_write, |
1046 | - ) |
1047 | - # Now provide any remaining rfc2317 glue networks. |
1048 | - for network, ranges in rfc2317_glue.items(): |
1049 | - exclude_set = { |
1050 | - IPNetwork(s.cidr) |
1051 | - for s in subnets |
1052 | - if network in IPNetwork(s.cidr) |
1053 | - } |
1054 | - domain_updates = [] |
1055 | - for update in dynamic_updates: |
1056 | - glue_update = True |
1057 | - for exclude_net in exclude_set: |
1058 | + yield existing_subnet_cfgs[network] |
1059 | + |
1060 | + # Now provide any remaining rfc2317 glue networks. |
1061 | + for network, ranges in rfc2317_glue.items(): |
1062 | + exclude_set = { |
1063 | + IPNetwork(s.cidr) |
1064 | + for s in subnets |
1065 | + if network in IPNetwork(s.cidr) |
1066 | + } |
1067 | + domain_updates = [] |
1068 | + for update in dynamic_updates: |
1069 | + glue_update = True |
1070 | + for exclude_net in exclude_set: |
1071 | + if ( |
1072 | + update.answer |
1073 | + and update.answer_is_ip |
1074 | + and update.answer_as_ip in exclude_net |
1075 | + ): |
1076 | + glue_update = False |
1077 | + break |
1078 | if ( |
1079 | - update.answer |
1080 | + glue_update |
1081 | + and update.answer |
1082 | and update.answer_is_ip |
1083 | - and update.answer_as_ip in exclude_net |
1084 | + and update.answer_as_ip in network |
1085 | ): |
1086 | - glue_update = False |
1087 | - break |
1088 | - if ( |
1089 | - glue_update |
1090 | - and update.answer |
1091 | - and update.answer_is_ip |
1092 | - and update.answer_as_ip in network |
1093 | - ): |
1094 | - domain_updates.append( |
1095 | - DynamicDNSUpdate.as_reverse_record_update( |
1096 | - update, network |
1097 | + domain_updates.append( |
1098 | + DynamicDNSUpdate.as_reverse_record_update( |
1099 | + update, network |
1100 | + ) |
1101 | ) |
1102 | + |
1103 | + if network in existing_subnet_cfgs: |
1104 | + ZoneGenerator._merge_into_existing_network( |
1105 | + network, |
1106 | + existing_subnet_cfgs, |
1107 | + mapping, |
1108 | + dynamic_updates=domain_updates, |
1109 | + glue=ranges, |
1110 | + is_glue_net=True, |
1111 | ) |
1112 | - yield DNSReverseZoneConfig( |
1113 | - ns_host_name, |
1114 | - serial=serial, |
1115 | - default_ttl=default_ttl, |
1116 | - network=network, |
1117 | - ns_host_name=ns_host_name, |
1118 | - rfc2317_ranges=ranges, |
1119 | - exclude=exclude_set, |
1120 | - dynamic_updates=domain_updates, |
1121 | - force_config_write=force_config_write, |
1122 | - ) |
1123 | + else: |
1124 | + existing_subnet_cfgs[network] = DNSReverseZoneConfig( |
1125 | + ns_host_name, |
1126 | + serial=serial, |
1127 | + default_ttl=default_ttl, |
1128 | + network=network, |
1129 | + ns_host_name=ns_host_name, |
1130 | + rfc2317_ranges=ranges, |
1131 | + dynamic_updates=domain_updates, |
1132 | + force_config_write=force_config_write, |
1133 | + ) |
1134 | + yield existing_subnet_cfgs[network] |
1135 | |
1136 | def __iter__(self): |
1137 | """Iterate over zone configs. |
1138 | @@ -553,6 +715,7 @@ class ZoneGenerator: |
1139 | default_ttl, |
1140 | self._dynamic_updates, |
1141 | self.force_config_write, |
1142 | + existing_subnet_cfgs=self._existing_subnet_cfgs, |
1143 | ), |
1144 | ) |
1145 | |
1146 | diff --git a/src/maastesting/noseplug.py b/src/maastesting/noseplug.py |
1147 | index 02949ec..ff21c32 100644 |
1148 | --- a/src/maastesting/noseplug.py |
1149 | +++ b/src/maastesting/noseplug.py |
1150 | @@ -474,7 +474,9 @@ class CleanTestToolsFailure(Plugin): |
1151 | ec, ev, tb = err |
1152 | if ec is not _StringException: |
1153 | return err |
1154 | - return Exception, Exception(*ev.args), tb |
1155 | + if hasattr(ev, "args"): |
1156 | + return Exception, Exception(*ev.args), tb |
1157 | + return Exception, Exception(ev), tb |
1158 | |
1159 | formatError = formatFailure |
1160 | |
1161 | diff --git a/src/provisioningserver/dns/tests/test_zoneconfig.py b/src/provisioningserver/dns/tests/test_zoneconfig.py |
1162 | index 1433493..06e6c87 100644 |
1163 | --- a/src/provisioningserver/dns/tests/test_zoneconfig.py |
1164 | +++ b/src/provisioningserver/dns/tests/test_zoneconfig.py |
1165 | @@ -521,17 +521,16 @@ class TestDNSReverseZoneConfig(MAASTestCase): |
1166 | |
1167 | def test_computes_zone_file_config_file_paths(self): |
1168 | domain = factory.make_name("zone") |
1169 | - reverse_file_name = [ |
1170 | - "zone.%d.168.192.in-addr.arpa" % i for i in range(4) |
1171 | - ] |
1172 | + # in order to merge changes, maasserver.dns.zone_generator.ZoneGenerator will split large subnets |
1173 | + # meaning there's a 1:1 zonefile and DNSReverseZoneConfig |
1174 | + reverse_file_name = "zone.0.168.192.in-addr.arpa" |
1175 | dns_zone_config = DNSReverseZoneConfig( |
1176 | - domain, network=IPNetwork("192.168.0.0/22") |
1177 | + domain, network=IPNetwork("192.168.0.0/24") |
1178 | + ) |
1179 | + self.assertEqual( |
1180 | + os.path.join(get_zone_file_config_dir(), reverse_file_name), |
1181 | + dns_zone_config.zone_info[0].target_path, |
1182 | ) |
1183 | - for i in range(4): |
1184 | - self.assertEqual( |
1185 | - os.path.join(get_zone_file_config_dir(), reverse_file_name[i]), |
1186 | - dns_zone_config.zone_info[i].target_path, |
1187 | - ) |
1188 | |
1189 | def test_computes_zone_file_config_file_paths_for_small_network(self): |
1190 | domain = factory.make_name("zone") |
1191 | @@ -555,20 +554,8 @@ class TestDNSReverseZoneConfig(MAASTestCase): |
1192 | # A special case is the small subnet (less than 256 hosts for IPv4, |
1193 | # less than 16 hosts for IPv6), in which case, we follow RFC2317 with |
1194 | # the modern adjustment of using '-' instead of '/'. |
1195 | - zn = "%d.0.0.0.0.0.0.0.0.0.0.0.4.0.1.f.1.0.8.a.b.0.1.0.0.2.ip6.arpa" |
1196 | expected = [ |
1197 | # IPv4 networks. |
1198 | - # /22 ==> 4 /24 reverse zones |
1199 | - ( |
1200 | - IPNetwork("192.168.0.1/22"), |
1201 | - [ |
1202 | - DomainInfo( |
1203 | - IPNetwork("192.168.%d.0/24" % i), |
1204 | - "%d.168.192.in-addr.arpa" % i, |
1205 | - ) |
1206 | - for i in range(4) |
1207 | - ], |
1208 | - ), |
1209 | # /24 ==> 1 reverse zone |
1210 | ( |
1211 | IPNetwork("192.168.0.1/24"), |
1212 | @@ -625,27 +612,6 @@ class TestDNSReverseZoneConfig(MAASTestCase): |
1213 | ) |
1214 | ], |
1215 | ), |
1216 | - # /2 with hex digits ==> 4 /4 reverse zones |
1217 | - ( |
1218 | - IPNetwork("8000::/2"), |
1219 | - [ |
1220 | - DomainInfo(IPNetwork("8000::/4"), "8.ip6.arpa"), |
1221 | - DomainInfo(IPNetwork("9000::/4"), "9.ip6.arpa"), |
1222 | - DomainInfo(IPNetwork("a000::/4"), "a.ip6.arpa"), |
1223 | - DomainInfo(IPNetwork("b000::/4"), "b.ip6.arpa"), |
1224 | - ], |
1225 | - ), |
1226 | - # /103 ==> 2 /104 reverse zones |
1227 | - ( |
1228 | - IPNetwork("2001:ba8:1f1:400::/103"), |
1229 | - [ |
1230 | - DomainInfo( |
1231 | - IPNetwork("2001:ba8:1f1:400:0:0:%d00:0000/104" % i), |
1232 | - zn % i, |
1233 | - ) |
1234 | - for i in range(2) |
1235 | - ], |
1236 | - ), |
1237 | # /125 ==> 1 reverse zone, based on RFC2317 |
1238 | ( |
1239 | IPNetwork("2001:ba8:1f1:400::/125"), |
1240 | @@ -818,7 +784,7 @@ class TestDNSReverseZoneConfig(MAASTestCase): |
1241 | target_dir = patch_zone_file_config_path(self) |
1242 | domain = factory.make_string() |
1243 | ns_host_name = factory.make_name("ns") |
1244 | - network = IPNetwork("192.168.0.1/22") |
1245 | + network = IPNetwork("192.168.0.1/24") |
1246 | dynamic_network = IPNetwork("192.168.0.1/28") |
1247 | dns_zone_config = DNSReverseZoneConfig( |
1248 | domain, |
1249 | @@ -830,25 +796,24 @@ class TestDNSReverseZoneConfig(MAASTestCase): |
1250 | ], |
1251 | ) |
1252 | dns_zone_config.write_config() |
1253 | - for sub in range(4): |
1254 | - reverse_file_name = f"zone.{sub}.168.192.in-addr.arpa" |
1255 | - expected_GEN_direct = dns_zone_config.get_GENERATE_directives( |
1256 | - dynamic_network, |
1257 | - domain, |
1258 | - DomainInfo( |
1259 | - IPNetwork(f"192.168.{sub}.0/24"), |
1260 | - f"{sub}.168.192.in-addr.arpa", |
1261 | - ), |
1262 | - ) |
1263 | - with open(os.path.join(target_dir, reverse_file_name), "r") as fh: |
1264 | - contents = fh.read() |
1265 | - needles = [f"30 IN NS {ns_host_name}"] + [ |
1266 | - f"$GENERATE {iterator_values} {reverse_dns} IN PTR {hostname}" |
1267 | - for iterator_values, reverse_dns, hostname in expected_GEN_direct |
1268 | - ] |
1269 | + reverse_file_name = "zone.0.168.192.in-addr.arpa" |
1270 | + expected_GEN_direct = dns_zone_config.get_GENERATE_directives( |
1271 | + dynamic_network, |
1272 | + domain, |
1273 | + DomainInfo( |
1274 | + IPNetwork("192.168.0.0/24"), |
1275 | + "0.168.192.in-addr.arpa", |
1276 | + ), |
1277 | + ) |
1278 | + with open(os.path.join(target_dir, reverse_file_name), "r") as fh: |
1279 | + contents = fh.read() |
1280 | + needles = [f"30 IN NS {ns_host_name}"] + [ |
1281 | + f"$GENERATE {iterator_values} {reverse_dns} IN PTR {hostname}" |
1282 | + for iterator_values, reverse_dns, hostname in expected_GEN_direct |
1283 | + ] |
1284 | |
1285 | - for needle in needles: |
1286 | - self.assertIn(needle, contents) |
1287 | + for needle in needles: |
1288 | + self.assertIn(needle, contents) |
1289 | |
1290 | def test_writes_reverse_dns_zone_config_for_small_network(self): |
1291 | target_dir = patch_zone_file_config_path(self) |
1292 | @@ -1106,97 +1071,6 @@ class TestDNSReverseZoneConfig(MAASTestCase): |
1293 | ], |
1294 | ) |
1295 | |
1296 | - def test_dynamic_updates_are_only_sent_for_specific_domain_info(self): |
1297 | - patch_zone_file_config_path(self) |
1298 | - domain = factory.make_string() |
1299 | - network = IPNetwork("10.246.64.0/21") |
1300 | - subnetwork1 = IPNetwork("10.246.64.0/24") |
1301 | - subnetwork2 = IPNetwork("10.246.65.0/24") |
1302 | - ip1 = factory.pick_ip_in_network(subnetwork1) |
1303 | - ip2 = factory.pick_ip_in_network(subnetwork2) |
1304 | - hostname1 = f"{factory.make_string()}.{domain}" |
1305 | - hostname2 = f"{factory.make_string()}.{domain}" |
1306 | - fwd_updates = [ |
1307 | - DynamicDNSUpdate( |
1308 | - operation="INSERT", |
1309 | - zone=domain, |
1310 | - name=hostname1, |
1311 | - rectype="A", |
1312 | - answer=ip1, |
1313 | - ), |
1314 | - DynamicDNSUpdate( |
1315 | - operation="INSERT", |
1316 | - zone=domain, |
1317 | - name=hostname2, |
1318 | - rectype="A", |
1319 | - answer=ip2, |
1320 | - ), |
1321 | - ] |
1322 | - rev_updates = [ |
1323 | - DynamicDNSUpdate.as_reverse_record_update(update, network) |
1324 | - for update in fwd_updates |
1325 | - ] |
1326 | - # gets changed to a /24 and any other space in the original |
1327 | - # subnet is split into a separate zone for a given /24 |
1328 | - zone = DNSReverseZoneConfig( |
1329 | - domain, |
1330 | - serial=random.randint(1, 100), |
1331 | - network=network, |
1332 | - dynamic_updates=rev_updates, |
1333 | - ) |
1334 | - |
1335 | - run_command = self.patch(actions, "run_command") |
1336 | - zone.write_config() |
1337 | - zone.write_config() |
1338 | - |
1339 | - expected_stdin1 = "\n".join( |
1340 | - [ |
1341 | - "server localhost", |
1342 | - "zone 64.246.10.in-addr.arpa", |
1343 | - f"update add {IPAddress(ip1).reverse_dns} {zone.default_ttl} PTR {hostname1}", |
1344 | - f"update add 64.246.10.in-addr.arpa {zone.default_ttl} SOA 64.246.10.in-addr.arpa. nobody.example.com. {zone.serial} 600 1800 604800 {zone.default_ttl}", |
1345 | - "send\n", |
1346 | - ] |
1347 | - ) |
1348 | - |
1349 | - expected_stdin2 = "\n".join( |
1350 | - [ |
1351 | - "server localhost", |
1352 | - "zone 65.246.10.in-addr.arpa", |
1353 | - f"update add {IPAddress(ip2).reverse_dns} {zone.default_ttl} PTR {hostname2}", |
1354 | - f"update add 65.246.10.in-addr.arpa {zone.default_ttl} SOA 65.246.10.in-addr.arpa. nobody.example.com. {zone.serial} 600 1800 604800 {zone.default_ttl}", |
1355 | - "send\n", |
1356 | - ] |
1357 | - ) |
1358 | - |
1359 | - expected_stdin3 = "\n".join( |
1360 | - [ |
1361 | - "server localhost", |
1362 | - "zone 71.246.10.in-addr.arpa", |
1363 | - f"update add 71.246.10.in-addr.arpa {zone.default_ttl} SOA 71.246.10.in-addr.arpa. nobody.example.com. {zone.serial} 600 1800 604800 {zone.default_ttl}", |
1364 | - "send\n", |
1365 | - ] |
1366 | - ) |
1367 | - |
1368 | - run_command.assert_any_call( |
1369 | - "nsupdate", |
1370 | - "-k", |
1371 | - get_nsupdate_key_path(), |
1372 | - stdin=expected_stdin1.encode("ascii"), |
1373 | - ) |
1374 | - run_command.assert_any_call( |
1375 | - "nsupdate", |
1376 | - "-k", |
1377 | - get_nsupdate_key_path(), |
1378 | - stdin=expected_stdin2.encode("ascii"), |
1379 | - ) |
1380 | - run_command.assert_any_call( |
1381 | - "nsupdate", |
1382 | - "-k", |
1383 | - get_nsupdate_key_path(), |
1384 | - stdin=expected_stdin3.encode("ascii"), |
1385 | - ) |
1386 | - |
1387 | |
1388 | class TestDNSReverseZoneConfig_GetGenerateDirectives(MAASTestCase): |
1389 | """Tests for `DNSReverseZoneConfig.get_GENERATE_directives()`.""" |
1390 | diff --git a/src/provisioningserver/dns/zoneconfig.py b/src/provisioningserver/dns/zoneconfig.py |
1391 | index 2e37e19..cf8a8ef 100644 |
1392 | --- a/src/provisioningserver/dns/zoneconfig.py |
1393 | +++ b/src/provisioningserver/dns/zoneconfig.py |
1394 | @@ -413,30 +413,11 @@ class DNSReverseZoneConfig(DomainConfigBase): |
1395 | self._network = kwargs.pop("network", None) |
1396 | self._dynamic_ranges = kwargs.pop("dynamic_ranges", []) |
1397 | self._rfc2317_ranges = kwargs.pop("rfc2317_ranges", []) |
1398 | - self._exclude = kwargs.pop("exclude", set()) |
1399 | - zone_info = self.compose_zone_info( |
1400 | - self._network, exclude=self._exclude |
1401 | - ) |
1402 | + zone_info = self.compose_zone_info(self._network) |
1403 | super().__init__(domain, zone_info=zone_info, **kwargs) |
1404 | |
1405 | @classmethod |
1406 | - def _skip_if_overlaps(cls, first, base, step, network, exclude): |
1407 | - for other_network in exclude: |
1408 | - if ( |
1409 | - first in other_network |
1410 | - and network.prefixlen < other_network.prefixlen |
1411 | - ): # allow the more specific overlapping subnet to create the zone config |
1412 | - try: |
1413 | - base += 1 |
1414 | - first += step |
1415 | - except IndexError: |
1416 | - # IndexError occurs when we go from 255.255.255.255 to |
1417 | - # 0.0.0.0. If we hit that, we're all fine and done. |
1418 | - break |
1419 | - return (first, base) |
1420 | - |
1421 | - @classmethod |
1422 | - def compose_zone_info(cls, network, exclude=()): |
1423 | + def compose_zone_info(cls, network): |
1424 | """Return the names of the reverse zones.""" |
1425 | # Generate the name of the reverse zone file: |
1426 | # Use netaddr's reverse_dns() to get the reverse IP name |
1427 | @@ -444,9 +425,7 @@ class DNSReverseZoneConfig(DomainConfigBase): |
1428 | # octets of that name (i.e. drop the octets that will be specified in |
1429 | # the zone file). |
1430 | # returns a list of (IPNetwork, zone_name, zonefile_path) tuples |
1431 | - info = [] |
1432 | first = IPAddress(network.first) |
1433 | - last = IPAddress(network.last) |
1434 | if first.version == 6: |
1435 | # IPv6. |
1436 | # 2001:89ab::/19 yields 8.1.0.0.2.ip6.arpa, and the full list |
1437 | @@ -488,40 +467,23 @@ class DNSReverseZoneConfig(DomainConfigBase): |
1438 | split_zone = first.reverse_dns.split(".") |
1439 | zone_rest = ".".join(split_zone[rest_limit:-1]) |
1440 | base = int(split_zone[rest_limit - 1]) |
1441 | - while first <= last: |
1442 | - (first, base) = cls._skip_if_overlaps( |
1443 | - first, base, step, network, exclude |
1444 | - ) |
1445 | - if first > last: |
1446 | - # if the excluding subnet pushes the base IP beyond the bounds of the generating subnet, we've reached the end and return early |
1447 | - return info |
1448 | - |
1449 | - # Rest_limit has bounds of 1..labelcount+1 (5 or 33). |
1450 | - # If we're stripping any elements, then we just want base.name. |
1451 | - if rest_limit > 1: |
1452 | - if first.version == 6: |
1453 | - new_zone = f"{base:x}.{zone_rest}" |
1454 | - else: |
1455 | - new_zone = "%d.%s" % (base, zone_rest) |
1456 | - # We didn't actually strip any elemnts, so base goes back with |
1457 | - # the prefixlen attached. |
1458 | - elif first.version == 6: |
1459 | - new_zone = "%x-%d.%s" % (base, network.prefixlen, zone_rest) |
1460 | + |
1461 | + # Rest_limit has bounds of 1..labelcount+1 (5 or 33). |
1462 | + # If we're stripping any elements, then we just want base.name. |
1463 | + if rest_limit > 1: |
1464 | + if first.version == 6: |
1465 | + new_zone = f"{base:x}.{zone_rest}" |
1466 | else: |
1467 | - new_zone = "%d-%d.%s" % (base, network.prefixlen, zone_rest) |
1468 | - info.append( |
1469 | - DomainInfo( |
1470 | - IPNetwork("%s/%d" % (first, subnet_prefix)), new_zone |
1471 | - ) |
1472 | - ) |
1473 | - base += 1 |
1474 | - try: |
1475 | - first += step |
1476 | - except IndexError: |
1477 | - # IndexError occurs when we go from 255.255.255.255 to |
1478 | - # 0.0.0.0. If we hit that, we're all fine and done. |
1479 | - break |
1480 | - return info |
1481 | + new_zone = f"{base:d}.{zone_rest}" |
1482 | + # We didn't actually strip any elemnts, so base goes back with |
1483 | + # the prefixlen attached. |
1484 | + elif first.version == 6: |
1485 | + new_zone = f"{base:x}-{network.prefixlen:d}.{zone_rest}" |
1486 | + else: |
1487 | + new_zone = f"{base:d}-{network.prefixlen:d}.{zone_rest}" |
1488 | + return [ |
1489 | + DomainInfo(IPNetwork(f"{first}/{subnet_prefix:d}"), new_zone), |
1490 | + ] |
1491 | |
1492 | @classmethod |
1493 | def get_PTR_mapping(cls, mapping, network): |
UNIT TESTS _subnets_ bind_misconfigu re lp:~cgrabowski/maas/+git/maas into -b master lp:~maas-committers/maas
-b fix_overlapping
STATUS: FAILED maas-ci. internal: 8080/job/ maas-tester/ 4217/console 944ec179a77f072 3727071874
LOG: http://
COMMIT: 8498fd5425c23a1