Merge lp:~lamont/maas/dns-2.0 into lp:maas/trunk

Proposed by LaMont Jones on 2015-11-27
Status: Merged
Approved by: LaMont Jones on 2015-11-30
Approved revision: no longer in the source branch.
Merged at revision: 4528
Proposed branch: lp:~lamont/maas/dns-2.0
Merge into: lp:maas/trunk
Diff against target: 917 lines (+389/-151)
7 files modified
etc/maas/templates/dns/named.conf.template (+4/-2)
src/maasserver/dns/config.py (+4/-3)
src/provisioningserver/dns/actions.py (+17/-12)
src/provisioningserver/dns/config.py (+15/-4)
src/provisioningserver/dns/tests/test_actions.py (+4/-4)
src/provisioningserver/dns/tests/test_zoneconfig.py (+179/-52)
src/provisioningserver/dns/zoneconfig.py (+166/-74)
To merge this branch: bzr merge lp:~lamont/maas/dns-2.0
Reviewer Review Type Date Requested Status
Mike Pontillo (community) 2015-11-27 Approve on 2015-11-28
Review via email: mp+278879@code.launchpad.net

Commit message

Correct the management of reverse DNS zones in MAAS. (a /22 means that we create 4 /24 reverse zones, not one /16 reverse zone.)

Description of the change

Correct the management of reverse DNS zones in MAAS. (a /22 means that we create 4 /24 reverse zones, not one /16 reverse zone.)

To post a comment you must log in.
LaMont Jones (lamont) wrote :

general overview: DNSZoneConfigBase.{zone_name,target_path} get replaced with zone_info: a list of 1-128 DNSZoneInfo elements (which contain the subnet and reverse zone name, with target_path being derived as needed.)

The zones that we are authoritative for are limited to only the reverse zones for the subnets we
own.

Mike Pontillo (mpontillo) wrote :

Nice work, LaMont.

I commented on a few minor issues below; in my mind, the only thing keeping this branch from landing is the 'assert', which we should gracefully log and skip instead, rather than crashing regiond.

See comments below.

review: Needs Fixing
Mike Pontillo (mpontillo) wrote :

Looks good. Thanks for the fixes!

review: Approve
Andres Rodriguez (andreserl) wrote :

Hi Guys,

Just keep in mind that fixing this means that the behavior of DNS completely changes, and as such, it is something that's not back portable.

LaMont Jones (lamont) wrote :

> Hi Guys,
>
> Just keep in mind that fixing this means that the behavior of DNS completely
> changes, and as such, it is something that's not back portable.

Acknowledged.

lp:~lamont/maas/dns-2.0 updated on 2015-11-30
4528. By LaMont Jones on 2015-11-30

[r=mpontillo][bug=1356012][author=lamont] Correct the management of reverse DNS zones in MAAS. (a /22 means that we create 4 /24 reverse zones, not one /16 reverse zone.)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'etc/maas/templates/dns/named.conf.template'
2--- etc/maas/templates/dns/named.conf.template 2014-11-14 21:50:42 +0000
3+++ etc/maas/templates/dns/named.conf.template 2015-11-28 05:29:35 +0000
4@@ -2,11 +2,13 @@
5
6 # Zone declarations.
7 {{for zone in zones}}
8-zone "{{zone.zone_name}}" {
9+{{for zoneinfo in zone.zone_info}}
10+zone "{{zoneinfo.zone_name}}" {
11 type master;
12- file "{{zone.target_path}}";
13+ file "{{zoneinfo.target_path}}";
14 };
15 {{endfor}}
16+{{endfor}}
17
18 # Access control for recursive queries. See named.conf.options.inside.maas
19 # for the directives used on this ACL.
20
21=== modified file 'src/maasserver/dns/config.py'
22--- src/maasserver/dns/config.py 2015-09-24 16:22:12 +0000
23+++ src/maasserver/dns/config.py 2015-11-28 05:29:35 +0000
24@@ -50,7 +50,7 @@
25 bind_reconfigure,
26 bind_reload,
27 bind_reload_with_retries,
28- bind_reload_zone,
29+ bind_reload_zones,
30 bind_write_configuration,
31 bind_write_options,
32 bind_write_zones,
33@@ -144,9 +144,10 @@
34
35 serial = next_zone_serial()
36 for zone in ZoneGenerator(clusters, serial):
37- maaslog.info("Generating new DNS zone file for %s", zone.zone_name)
38+ names = [zi.zone_name for zi in zone.zone_info]
39+ maaslog.info("Generating new DNS zone file for %s", " ".join(names))
40 bind_write_zones([zone])
41- bind_reload_zone(zone.zone_name)
42+ bind_reload_zones(names)
43
44
45 def dns_update_zones(clusters):
46
47=== modified file 'src/provisioningserver/dns/actions.py'
48--- src/provisioningserver/dns/actions.py 2015-10-22 00:06:43 +0000
49+++ src/provisioningserver/dns/actions.py 2015-11-28 05:29:35 +0000
50@@ -15,7 +15,7 @@
51 __all__ = [
52 "bind_reconfigure",
53 "bind_reload",
54- "bind_reload_zone",
55+ "bind_reload_zones",
56 "bind_write_configuration",
57 "bind_write_options",
58 "bind_write_zones",
59@@ -87,21 +87,26 @@
60 sleep(interval)
61
62
63-def bind_reload_zone(zone_name):
64+def bind_reload_zones(zone_list):
65 """Ask BIND to reload the zone file for the given zone.
66
67- :param zone_name: The name of the zone to reload.
68+ :param zone_list: A list of zone names to reload, or a single name as a
69+ string.
70 :return: True if success, False otherwise.
71 """
72- try:
73- execute_rndc_command(("reload", zone_name))
74- return True
75- except CalledProcessError as exc:
76- maaslog.error(
77- "Reloading BIND zone %r failed (is it running?): %s",
78- zone_name,
79- exc)
80- return False
81+ ret = True
82+ if not isinstance(zone_list, list):
83+ zone_list = [zone_list]
84+ for name in zone_list:
85+ try:
86+ execute_rndc_command(("reload", name))
87+ except CalledProcessError as exc:
88+ maaslog.error(
89+ "Reloading BIND zone %r failed (is it running?): %s",
90+ name,
91+ exc)
92+ ret = False
93+ return ret
94
95
96 def bind_write_configuration(zones, trusted_networks):
97
98=== modified file 'src/provisioningserver/dns/config.py'
99--- src/provisioningserver/dns/config.py 2015-07-09 22:25:27 +0000
100+++ src/provisioningserver/dns/config.py 2015-11-28 05:29:35 +0000
101@@ -29,6 +29,7 @@
102 import re
103 import sys
104
105+from provisioningserver.logger import get_maas_logger
106 from provisioningserver.utils import locate_config
107 from provisioningserver.utils.fs import atomic_write
108 from provisioningserver.utils.isc import read_isc_file
109@@ -36,6 +37,7 @@
110 import tempita
111
112
113+maaslog = get_maas_logger("dns")
114 NAMED_CONF_OPTIONS = 'named.conf.options'
115 MAAS_NAMED_CONF_NAME = 'named.conf.maas'
116 MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME = 'named.conf.options.inside.maas'
117@@ -319,7 +321,16 @@
118
119 @classmethod
120 def get_include_snippet(cls):
121- target_path = compose_config_path(cls.target_file_name)
122- assert '"' not in target_path, (
123- "DNS config path contains quote: %s." % target_path)
124- return 'include "%s";\n' % target_path
125+ snippet = ""
126+ if isinstance(cls.target_file_name, list):
127+ target_file_names = cls.target_file_name
128+ else:
129+ target_file_names = [cls.target_file_name]
130+ for target_file_name in target_file_names:
131+ target_path = compose_config_path(target_file_name)
132+ if '"' in target_path:
133+ maaslog.error(
134+ "DNS config path contains quote: %s." % target_path)
135+ else:
136+ snippet += 'include "%s";\n' % target_path
137+ return snippet
138
139=== modified file 'src/provisioningserver/dns/tests/test_actions.py'
140--- src/provisioningserver/dns/tests/test_actions.py 2015-10-22 00:06:43 +0000
141+++ src/provisioningserver/dns/tests/test_actions.py 2015-11-28 05:29:35 +0000
142@@ -144,11 +144,11 @@
143
144
145 class TestReloadZone(MAASTestCase):
146- """Tests for :py:func:`actions.bind_reload_zone`."""
147+ """Tests for :py:func:`actions.bind_reload_zones`."""
148
149 def test__executes_rndc_command(self):
150 self.patch_autospec(actions, "execute_rndc_command")
151- self.assertTrue(actions.bind_reload_zone(sentinel.zone))
152+ self.assertTrue(actions.bind_reload_zones(sentinel.zone))
153 self.assertThat(
154 actions.execute_rndc_command,
155 MockCalledOnceWith(("reload", sentinel.zone)))
156@@ -157,7 +157,7 @@
157 erc = self.patch_autospec(actions, "execute_rndc_command")
158 erc.side_effect = factory.make_CalledProcessError()
159 with FakeLogger("maas") as logger:
160- self.assertFalse(actions.bind_reload_zone(sentinel.zone))
161+ self.assertFalse(actions.bind_reload_zones(sentinel.zone))
162 self.assertDocTestMatches(
163 "Reloading BIND zone ... failed (is it running?): "
164 "Command ... returned non-zero exit status ...",
165@@ -166,7 +166,7 @@
166 def test__false_on_subprocess_error(self):
167 erc = self.patch_autospec(actions, "execute_rndc_command")
168 erc.side_effect = factory.make_CalledProcessError()
169- self.assertFalse(actions.bind_reload_zone(sentinel.zone))
170+ self.assertFalse(actions.bind_reload_zones(sentinel.zone))
171
172
173 class TestConfiguration(PservTestCase):
174
175=== modified file 'src/provisioningserver/dns/tests/test_zoneconfig.py'
176--- src/provisioningserver/dns/tests/test_zoneconfig.py 2015-05-07 18:14:38 +0000
177+++ src/provisioningserver/dns/tests/test_zoneconfig.py 2015-11-28 05:29:35 +0000
178@@ -37,6 +37,7 @@
179 from provisioningserver.dns.zoneconfig import (
180 DNSForwardZoneConfig,
181 DNSReverseZoneConfig,
182+ DNSZoneInfo,
183 )
184 from testtools.matchers import (
185 Contains,
186@@ -102,7 +103,7 @@
187 dns_zone_config = DNSForwardZoneConfig(domain)
188 self.assertEqual(
189 os.path.join(get_dns_config_dir(), 'zone.%s' % domain),
190- dns_zone_config.target_path)
191+ dns_zone_config.zone_info[0].target_path)
192
193 def test_get_a_mapping_returns_ipv4_mapping(self):
194 name = factory.make_string()
195@@ -266,7 +267,7 @@
196 factory.make_string(), serial=random.randint(1, 100),
197 dns_ip=factory.make_ipv4_address())
198 dns_zone_config.write_config()
199- filepath = FilePath(dns_zone_config.target_path)
200+ filepath = FilePath(dns_zone_config.zone_info[0].target_path)
201 self.assertTrue(filepath.getPermissions().other.read)
202
203
204@@ -290,34 +291,109 @@
205
206 def test_computes_dns_config_file_paths(self):
207 domain = factory.make_name('zone')
208- reverse_file_name = 'zone.168.192.in-addr.arpa'
209+ reverse_file_name = [
210+ 'zone.%d.168.192.in-addr.arpa' % i for i in range(4)]
211 dns_zone_config = DNSReverseZoneConfig(
212 domain, network=IPNetwork("192.168.0.0/22"))
213+ for i in range(4):
214+ self.assertEqual(
215+ os.path.join(get_dns_config_dir(), reverse_file_name[i]),
216+ dns_zone_config.zone_info[i].target_path)
217+
218+ def test_computes_dns_config_file_paths_for_small_network(self):
219+ domain = factory.make_name('zone')
220+ reverse_file_name = 'zone.192-27.0.168.192.in-addr.arpa'
221+ dns_zone_config = DNSReverseZoneConfig(
222+ domain, network=IPNetwork("192.168.0.192/27"))
223+ self.assertEqual(1, len(dns_zone_config.zone_info))
224 self.assertEqual(
225 os.path.join(get_dns_config_dir(), reverse_file_name),
226- dns_zone_config.target_path)
227+ dns_zone_config.zone_info[0].target_path)
228
229 def test_reverse_zone_file(self):
230 # DNSReverseZoneConfig calculates the reverse zone file name
231 # correctly for IPv4 and IPv6 networks.
232+ # As long as the network size ends on a "nice" boundary (multiple of
233+ # 8 for IPv4, multiple of 4 for IPv6), then there will be one reverse
234+ # zone for the subnet. When it isn't, then there will be 2-128
235+ # individual reverse zones for the subnet.
236+ # A special case is the small subnet (less than 256 hosts for IPv4,
237+ # less than 16 hosts for IPv6), in which case, we follow RFC2317 with
238+ # the modern adjustment of using '-' instead of '/'.
239+ 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"
240 expected = [
241 # IPv4 networks.
242- (IPNetwork('192.168.0.1/22'), '168.192.in-addr.arpa'),
243- (IPNetwork('192.168.0.1/24'), '0.168.192.in-addr.arpa'),
244+ # /22 ==> 4 /24 reverse zones
245+ (
246+ IPNetwork('192.168.0.1/22'),
247+ [DNSZoneInfo(
248+ IPNetwork('192.168.%d.0/24' % i),
249+ '%d.168.192.in-addr.arpa' % i) for i in range(4)]
250+ ),
251+ # /24 ==> 1 reverse zone
252+ (
253+ IPNetwork('192.168.0.1/24'),
254+ [DNSZoneInfo(
255+ IPNetwork('192.168.0.0/24'),
256+ '0.168.192.in-addr.arpa')]
257+ ),
258+ # /29 ==> 1 reverse zones, per RFC2317
259+ (
260+ IPNetwork('192.168.0.241/29'),
261+ [DNSZoneInfo(
262+ IPNetwork('192.168.0.240/29'),
263+ '240-29.0.168.192.in-addr.arpa')]
264+ ),
265 # IPv6 networks.
266- (IPNetwork('3ffe:801::/32'), '1.0.8.0.e.f.f.3.ip6.arpa'),
267- (IPNetwork('2001:db8:0::/48'), '0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa'),
268+ # /32, 48, 56, 64 ==> 1 reverse zones
269+ (
270+ IPNetwork('3ffe:801::/32'),
271+ [DNSZoneInfo(
272+ IPNetwork('3ffe:801::32'),
273+ '1.0.8.0.e.f.f.3.ip6.arpa')]),
274+ (
275+ IPNetwork('2001:db8:0::/48'),
276+ [DNSZoneInfo(
277+ IPNetwork('2001:db8:0::/48'),
278+ '0.0.0.0.8.b.d.0.1.0.0.2.ip6.arpa')]
279+ ),
280 (
281 IPNetwork('2001:ba8:1f1:400::/56'),
282- '4.0.1.f.1.0.8.a.b.0.1.0.0.2.ip6.arpa'
283+ [DNSZoneInfo(
284+ IPNetwork('2001:ba8:1f1:400::/56'),
285+ '4.0.1.f.1.0.8.a.b.0.1.0.0.2.ip6.arpa')],
286 ),
287 (
288 IPNetwork('2610:8:6800:1::/64'),
289- '1.0.0.0.0.0.8.6.8.0.0.0.0.1.6.2.ip6.arpa',
290- ),
291+ [DNSZoneInfo(
292+ IPNetwork('2610:8:6800:1::/64'),
293+ '1.0.0.0.0.0.8.6.8.0.0.0.0.1.6.2.ip6.arpa')],
294+ ),
295+ # /2 with hex digits ==> 4 /4 reverse zones
296+ (
297+ IPNetwork('8000::/2'),
298+ [
299+ DNSZoneInfo(IPNetwork('8000::/4'), '8.ip6.arpa'),
300+ DNSZoneInfo(IPNetwork('9000::/4'), '9.ip6.arpa'),
301+ DNSZoneInfo(IPNetwork('a000::/4'), 'a.ip6.arpa'),
302+ DNSZoneInfo(IPNetwork('b000::/4'), 'b.ip6.arpa'),
303+ ],
304+ ),
305+ # /103 ==> 2 /104 reverse zones
306 (
307 IPNetwork('2001:ba8:1f1:400::/103'),
308- '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',
309+ [DNSZoneInfo(
310+ IPNetwork('2001:ba8:1f1:400:0:0:%d00:0000/104' % i),
311+ zn % i) for i in range(2)],
312+ ),
313+ # /125 ==> 1 reverse zone, based on RFC2317
314+ (
315+ IPNetwork('2001:ba8:1f1:400::/125'),
316+ [DNSZoneInfo(
317+ IPNetwork('2001:ba8:1f1:400::/125'),
318+ "0-125.%s" % (
319+ IPAddress('2001:ba8:1f1:400::').reverse_dns[2:-1]))],
320+
321 ),
322
323 ]
324@@ -325,8 +401,15 @@
325 for network, _ in expected:
326 domain = factory.make_name('zone')
327 dns_zone_config = DNSReverseZoneConfig(domain, network=network)
328- results.append((network, dns_zone_config.zone_name))
329- self.assertEqual(expected, results)
330+ results.append((network, dns_zone_config.zone_info))
331+ # Make sure we have the right number of elements.
332+ self.assertEqual(len(expected), len(results))
333+ # And that the zone names chosen for each element are correct.
334+ for net in range(len(expected)):
335+ for zi in range(len(expected[net][1])):
336+ self.assertItemsEqual(
337+ expected[net][1][zi].zone_name,
338+ results[net][1][zi].zone_name)
339
340 def test_get_ptr_mapping(self):
341 name = factory.make_string()
342@@ -378,11 +461,12 @@
343 factory.make_string(), serial=random.randint(1, 100),
344 network=network)
345 dns_zone_config.write_config()
346- self.assertThat(
347- os.path.join(
348- target_dir, 'zone.%s' % dns_zone_config.zone_name),
349- FileContains(
350- matcher=Contains('IN NS %s.' % dns_zone_config.domain)))
351+ for zone_name in [zi.zone_name for zi in dns_zone_config.zone_info]:
352+ self.assertThat(
353+ os.path.join(
354+ target_dir, 'zone.%s' % zone_name),
355+ FileContains(
356+ matcher=Contains('IN NS %s.' % dns_zone_config.domain)))
357
358 def test_writes_reverse_dns_zone_config(self):
359 target_dir = patch_dns_config_path(self)
360@@ -394,9 +478,41 @@
361 dynamic_ranges=[
362 IPRange(dynamic_network.first, dynamic_network.last)])
363 dns_zone_config.write_config()
364- reverse_file_name = 'zone.168.192.in-addr.arpa'
365- expected_generate_directives = dns_zone_config.get_GENERATE_directives(
366- dynamic_network, domain)
367+ for sub in range(4):
368+ reverse_file_name = 'zone.%d.168.192.in-addr.arpa' % sub
369+ expected_GEN_direct = dns_zone_config.get_GENERATE_directives(
370+ dynamic_network, domain,
371+ DNSZoneInfo(
372+ IPNetwork('192.168.%d.0/24' % sub),
373+ "%d.168.192.in-addr.arpa" % sub))
374+ expected = ContainsAll(
375+ [
376+ 'IN NS %s' % domain
377+ ] +
378+ [
379+ '$GENERATE %s %s IN PTR %s' % (
380+ iterator_values, reverse_dns, hostname)
381+ for iterator_values, reverse_dns, hostname in
382+ expected_GEN_direct
383+ ])
384+ self.assertThat(
385+ os.path.join(target_dir, reverse_file_name),
386+ FileContains(matcher=expected))
387+
388+ def test_writes_reverse_dns_zone_config_for_small_network(self):
389+ target_dir = patch_dns_config_path(self)
390+ domain = factory.make_string()
391+ network = IPNetwork('192.168.0.1/26')
392+ dynamic_network = IPNetwork('192.168.0.1/28')
393+ dns_zone_config = DNSReverseZoneConfig(
394+ domain, serial=random.randint(1, 100), network=network,
395+ dynamic_ranges=[
396+ IPRange(dynamic_network.first, dynamic_network.last)])
397+ dns_zone_config.write_config()
398+ reverse_zone_name = '0-26.0.168.192.in-addr.arpa'
399+ reverse_file_name = 'zone.0-26.0.168.192.in-addr.arpa'
400+ expected_GEN_direct = dns_zone_config.get_GENERATE_directives(
401+ dynamic_network, domain, DNSZoneInfo(network, reverse_zone_name))
402 expected = ContainsAll(
403 [
404 'IN NS %s' % domain
405@@ -405,7 +521,7 @@
406 '$GENERATE %s %s IN PTR %s' % (
407 iterator_values, reverse_dns, hostname)
408 for iterator_values, reverse_dns, hostname in
409- expected_generate_directives
410+ expected_GEN_direct
411 ])
412 self.assertThat(
413 os.path.join(target_dir, reverse_file_name),
414@@ -431,8 +547,9 @@
415 factory.make_string(), serial=random.randint(1, 100),
416 network=factory.make_ipv4_network())
417 dns_zone_config.write_config()
418- filepath = FilePath(dns_zone_config.target_path)
419- self.assertTrue(filepath.getPermissions().other.read)
420+ for tgt in [zi.target_path for zi in dns_zone_config.zone_info]:
421+ filepath = FilePath(tgt)
422+ self.assertTrue(filepath.getPermissions().other.read)
423
424
425 class TestDNSReverseZoneConfig_GetGenerateDirectives(MAASTestCase):
426@@ -451,7 +568,11 @@
427 self.assertItemsEqual(
428 expected_directives,
429 DNSReverseZoneConfig.get_GENERATE_directives(
430- ip_range, domain="domain"))
431+ ip_range,
432+ domain="domain",
433+ zone_info=DNSZoneInfo(
434+ IPNetwork('192.168.0.0/16'),
435+ "168.192.in-addr.arpa")))
436
437 def get_expected_generate_directives(self, network, domain):
438 ip_parts = network.network.format().split('.')
439@@ -481,8 +602,11 @@
440 relevant_ip_parts.reverse()
441 expected_rdns_base = (
442 "%s.%s.in-addr.arpa." % tuple(relevant_ip_parts))
443- expected_rdns_template = "$.%s.%s" % (
444- num + second_octet_offset, expected_rdns_base)
445+ if network.size >= 256:
446+ expected_rdns_template = "$.%s.%s" % (
447+ num + second_octet_offset, expected_rdns_base)
448+ else:
449+ expected_rdns_template = "$"
450 expected_generate_directives.append(
451 (
452 "%s-%s" % (iterator_low, iterator_high),
453@@ -494,22 +618,25 @@
454
455 def test_returns_single_entry_for_slash_24_network(self):
456 network = IPNetwork("%s/24" % factory.make_ipv4_address())
457+ reverse = ".".join(IPAddress(network).reverse_dns.split('.')[1:-1])
458 domain = factory.make_string()
459 expected_generate_directives = self.get_expected_generate_directives(
460 network, domain)
461 directives = DNSReverseZoneConfig.get_GENERATE_directives(
462- network, domain)
463+ network, domain, DNSZoneInfo(network, reverse))
464 self.expectThat(directives, HasLength(1))
465 self.assertItemsEqual(expected_generate_directives, directives)
466
467 def test_returns_single_entry_for_tiny_network(self):
468 network = IPNetwork("%s/28" % factory.make_ipv4_address())
469+ reverse = IPAddress(network).reverse_dns.split('.')
470+ reverse = ".".join(["%s-28" % reverse[0]] + reverse[1:-1])
471 domain = factory.make_string()
472
473 expected_generate_directives = self.get_expected_generate_directives(
474 network, domain)
475 directives = DNSReverseZoneConfig.get_GENERATE_directives(
476- network, domain)
477+ network, domain, DNSZoneInfo(network, reverse))
478 self.expectThat(directives, HasLength(1))
479 self.assertItemsEqual(expected_generate_directives, directives)
480
481@@ -517,31 +644,22 @@
482 ip_range = IPRange('10.0.0.1', '10.0.0.255')
483 domain = factory.make_string()
484 directives = DNSReverseZoneConfig.get_GENERATE_directives(
485- ip_range, domain)
486+ ip_range, domain,
487+ DNSZoneInfo(IPNetwork('10.0.0.0/24'), '0.0.10.in-addr.arpa'))
488 self.expectThat(directives, HasLength(1))
489
490- def test_dtrt_for_larger_networks(self):
491- # For every other network size that we're not explicitly
492- # testing here,
493- # DNSReverseZoneConfig.get_GENERATE_directives() will return
494- # one GENERATE directive for every 255 addresses in the network.
495- for prefixlen in range(23, 17):
496- network = IPNetwork(
497- "%s/%s" % (factory.make_ipv4_address(), prefixlen))
498- domain = factory.make_string()
499- directives = DNSReverseZoneConfig.get_GENERATE_directives(
500- network, domain)
501- self.expectThat(directives, HasLength(network.size / 256))
502-
503- def test_returns_two_entries_for_slash_23_network(self):
504- network = IPNetwork(factory.make_ipv4_network(slash=23))
505+ # generate 2 zones, rather than 1 zone with 2 GENERATEs.
506+ def test_returns_256_entries_for_slash_16_network(self):
507+ network = IPNetwork(factory.make_ipv4_network(slash=16))
508+ reverse = IPAddress(network.first).reverse_dns.split('.')[2:-1]
509+ reverse = ".".join(reverse)
510 domain = factory.make_string()
511
512 expected_generate_directives = self.get_expected_generate_directives(
513 network, domain)
514 directives = DNSReverseZoneConfig.get_GENERATE_directives(
515- network, domain)
516- self.expectThat(directives, HasLength(2))
517+ network, domain, DNSZoneInfo(network, reverse))
518+ self.expectThat(directives, HasLength(256))
519 self.assertItemsEqual(expected_generate_directives, directives)
520
521 def test_ignores_network_larger_than_slash_16(self):
522@@ -549,7 +667,8 @@
523 self.assertEqual(
524 [],
525 DNSReverseZoneConfig.get_GENERATE_directives(
526- network, factory.make_string()))
527+ network, factory.make_string(),
528+ DNSZoneInfo(network, "do not care")))
529
530 def test_ignores_networks_that_span_slash_16s(self):
531 # If the upper and lower bounds of a range span two /16 networks
532@@ -557,7 +676,8 @@
533 # get_GENERATE_directives() will return early
534 ip_range = IPRange('10.0.0.55', '10.1.0.54')
535 directives = DNSReverseZoneConfig.get_GENERATE_directives(
536- ip_range, factory.make_string())
537+ ip_range, factory.make_string(),
538+ DNSZoneInfo(IPNetwork('10.0.0.0/15'), "do not care"))
539 self.assertEqual([], directives)
540
541 def test_sorts_output_by_hostname(self):
542@@ -568,12 +688,19 @@
543 expected_rdns = "$.%s.0.10.in-addr.arpa."
544
545 directives = list(DNSReverseZoneConfig.get_GENERATE_directives(
546- network, domain))
547+ network, domain,
548+ DNSZoneInfo(IPNetwork('10.0.0.0/24'), '0.0.10.in-addr.arpa')))
549 self.expectThat(
550 directives[0], Equals(
551 ("0-255", expected_rdns % "0", expected_hostname % "0")))
552+
553+ expected_hostname = "10-0-%s-$." + domain + "."
554+ expected_rdns = "$.%s.0.10.in-addr.arpa."
555+ directives = list(DNSReverseZoneConfig.get_GENERATE_directives(
556+ network, domain,
557+ DNSZoneInfo(IPNetwork('10.0.1.0/24'), '1.0.10.in-addr.arpa')))
558 self.expectThat(
559- directives[1], Equals(
560+ directives[0], Equals(
561 ("0-255", expected_rdns % "1", expected_hostname % "1")))
562
563
564
565=== modified file 'src/provisioningserver/dns/zoneconfig.py'
566--- src/provisioningserver/dns/zoneconfig.py 2015-05-07 18:14:38 +0000
567+++ src/provisioningserver/dns/zoneconfig.py 2015-11-28 05:29:35 +0000
568@@ -15,13 +15,13 @@
569 __all__ = [
570 'DNSForwardZoneConfig',
571 'DNSReverseZoneConfig',
572+ 'DNSZoneInfo',
573 ]
574
575
576 from abc import ABCMeta
577 from datetime import datetime
578 from itertools import chain
579-import math
580
581 from netaddr import (
582 IPAddress,
583@@ -106,6 +106,24 @@
584 return intersecting_subnets, prefix, rdns_suffix
585
586
587+class DNSZoneInfo:
588+ """Information about a DNS zone"""
589+
590+ def __init__(self, subnetwork, zone_name, target_path=None):
591+ """
592+ :param subnetwork: IPNetwork that this zone (chunk) is for. None
593+ for forward zones.
594+ :param zone_name: Fully-qualified zone name
595+ :param target_path: Optional, can be used to override the target path.
596+ """
597+ self.subnetwork = subnetwork
598+ self.zone_name = zone_name
599+ if target_path is None:
600+ self.target_path = compose_config_path('zone.%s' % zone_name)
601+ else:
602+ self.target_path = target_path
603+
604+
605 class DNSZoneConfigBase:
606 """Base class for zone writers."""
607
608@@ -113,17 +131,17 @@
609
610 template_file_name = 'zone.template'
611
612- def __init__(self, domain, zone_name, serial=None):
613+ def __init__(self, domain, zone_info, serial=None):
614 """
615 :param domain: The domain name of the forward zone.
616- :param zone_name: Fully-qualified zone name.
617+ :param zone_info: list of DNSZoneInfo entries.
618 :param serial: The serial to use in the zone file. This must increment
619 on each change.
620 """
621 self.domain = domain
622- self.zone_name = zone_name
623 self.serial = serial
624- self.target_path = compose_config_path('zone.%s' % self.zone_name)
625+ self.zone_info = zone_info
626+ self.target_base = compose_config_path('zone')
627
628 def make_parameters(self):
629 """Return a dict of the common template parameters."""
630@@ -142,9 +160,12 @@
631 support a resolution of one second, and so this method may set an
632 unexpected modification time in order to maintain that property.
633 """
634- content = render_dns_template(cls.template_file_name, *parameters)
635- with report_missing_config_dir():
636- incremental_write(content, output_file, mode=0644)
637+ if not isinstance(output_file, list):
638+ output_file = [output_file]
639+ for outfile in output_file:
640+ content = render_dns_template(cls.template_file_name, *parameters)
641+ with report_missing_config_dir():
642+ incremental_write(content, outfile, mode=0644)
643
644
645 class DNSForwardZoneConfig(DNSZoneConfigBase):
646@@ -175,7 +196,7 @@
647 self._dynamic_ranges = kwargs.pop('dynamic_ranges', [])
648 self._srv_mapping = kwargs.pop('srv_mapping', [])
649 super(DNSForwardZoneConfig, self).__init__(
650- domain, zone_name=domain, **kwargs)
651+ domain, zone_info=[DNSZoneInfo(None, domain)], **kwargs)
652
653 @classmethod
654 def get_mapping(cls, mapping, domain, dns_ip):
655@@ -255,7 +276,7 @@
656 return []
657
658 generate_directives = set()
659- subnets, prefix, _ = get_details_for_ip_range(dynamic_range)
660+ subnets, prefix, rdns_suffix = get_details_for_ip_range(dynamic_range)
661 for subnet in subnets:
662 iterator = "%d-%d" % (
663 (subnet.first & 0x000000ff),
664@@ -279,27 +300,28 @@
665 def write_config(self):
666 """Write the zone file."""
667 # Create GENERATE directives for IPv4 ranges.
668- generate_directives = list(
669- chain.from_iterable(
670- self.get_GENERATE_directives(dynamic_range)
671- for dynamic_range in self._dynamic_ranges
672- if dynamic_range.version == 4
673- ))
674- self.write_zone_file(
675- self.target_path, self.make_parameters(),
676- {
677- 'mappings': {
678- 'SRV': self.get_srv_mapping(
679- self._srv_mapping),
680- 'A': self.get_A_mapping(
681- self._mapping, self.domain, self._dns_ip),
682- 'AAAA': self.get_AAAA_mapping(
683- self._mapping, self.domain, self._dns_ip),
684- },
685- 'generate_directives': {
686- 'A': generate_directives,
687- }
688- })
689+ for zi in self.zone_info:
690+ generate_directives = list(
691+ chain.from_iterable(
692+ self.get_GENERATE_directives(dynamic_range)
693+ for dynamic_range in self._dynamic_ranges
694+ if dynamic_range.version == 4
695+ ))
696+ self.write_zone_file(
697+ zi.target_path, self.make_parameters(),
698+ {
699+ 'mappings': {
700+ 'SRV': self.get_srv_mapping(
701+ self._srv_mapping),
702+ 'A': self.get_A_mapping(
703+ self._mapping, self.domain, self._dns_ip),
704+ 'AAAA': self.get_AAAA_mapping(
705+ self._mapping, self.domain, self._dns_ip),
706+ },
707+ 'generate_directives': {
708+ 'A': generate_directives,
709+ }
710+ })
711
712
713 class DNSReverseZoneConfig(DNSZoneConfigBase):
714@@ -325,32 +347,88 @@
715 self._mapping = kwargs.pop('mapping', {})
716 self._network = kwargs.pop("network", None)
717 self._dynamic_ranges = kwargs.pop('dynamic_ranges', [])
718- zone_name = self.compose_zone_name(self._network)
719+ zone_info = self.compose_zone_info(self._network)
720 super(DNSReverseZoneConfig, self).__init__(
721- domain, zone_name=zone_name, **kwargs)
722+ domain, zone_info=zone_info, **kwargs)
723
724 @classmethod
725- def compose_zone_name(cls, network):
726- """Return the name of the reverse zone."""
727+ def compose_zone_info(cls, network):
728+ """Return the names of the reverse zones."""
729 # Generate the name of the reverse zone file:
730 # Use netaddr's reverse_dns() to get the reverse IP name
731 # of the first IP address in the network and then drop the first
732 # octets of that name (i.e. drop the octets that will be specified in
733 # the zone file).
734+ # returns a list of (IPNetwork, zone_name, zonefile_path) tuples
735+ info = []
736 first = IPAddress(network.first)
737+ last = IPAddress(network.last)
738 if first.version == 6:
739 # IPv6.
740- # Use float division and ceil to cope with network sizes that
741- # are not divisible by 4.
742- rest_limit = int(math.ceil((128 - network.prefixlen) / 4.))
743+ # 2001:89ab::/19 yields 8.1.0.0.2.ip6.arpa, and the full list
744+ # is 8.1.0.0.2.ip6.arpa, 9.1.0.0.2.ip6.arpa
745+ # The ipv6 reverse dns form is 32 elements of 1 hex digit each.
746+ # How many elements of the reverse DNS name to we throw away?
747+ # Prefixlen of 0-3 gives us 1, 4-7 gives us 2, etc.
748+ # While this seems wrong, we always _add_ a base label back in,
749+ # so it's correct.
750+ rest_limit = (132 - network.prefixlen) / 4
751+ # What is the prefix for each inner subnet (It will be the next
752+ # smaller multiple of 4.) If it's the smallest one, then RFC2317
753+ # tells us that we're adding an extra blob to the front of the
754+ # reverse zone name, and we want the entire prefixlen.
755+ subnet_prefix = (network.prefixlen + 3) / 4 * 4
756+ if subnet_prefix == 128:
757+ subnet_prefix = network.prefixlen
758+ # How big is the step between subnets? Again, special case for
759+ # extra small subnets.
760+ step = 1 << ((128 - network.prefixlen) / 4 * 4)
761+ if step < 16:
762+ step = 16
763+ # Grab the base (hex) and trailing labels for our reverse zone.
764+ split_zone = first.reverse_dns.split('.')
765+ zone_rest = ".".join(split_zone[rest_limit:-1])
766+ base = int(split_zone[rest_limit - 1], 16)
767 else:
768 # IPv4.
769- # Use float division and ceil to cope with splits not done on
770- # octets boundaries.
771- rest_limit = int(math.ceil((32 - network.prefixlen) / 8.))
772- reverse_name = first.reverse_dns.split('.', rest_limit)[-1]
773- # Strip off trailing '.'.
774- return reverse_name[:-1]
775+ # The logic here is the same as for IPv6, but with 8 instead of 4.
776+ rest_limit = (40 - network.prefixlen) / 8
777+ subnet_prefix = (network.prefixlen + 7) / 8 * 8
778+ if subnet_prefix == 32:
779+ subnet_prefix = network.prefixlen
780+ step = 1 << ((32 - network.prefixlen) / 8 * 8)
781+ if step < 256:
782+ step = 256
783+ # Grab the base (decimal) and trailing labels for our reverse
784+ # zone.
785+ split_zone = first.reverse_dns.split('.')
786+ zone_rest = ".".join(split_zone[rest_limit:-1])
787+ base = int(split_zone[rest_limit - 1])
788+ while first <= last:
789+ # Rest_limit has bounds of 1..labelcount+1 (5 or 33).
790+ # If we're stripping any elements, then we just want base.name.
791+ if rest_limit > 1:
792+ if first.version == 6:
793+ new_zone = "%x.%s" % (base, zone_rest)
794+ else:
795+ new_zone = "%d.%s" % (base, zone_rest)
796+ # We didn't actually strip any elemnts, so base goes back with
797+ # the prefixlen attached.
798+ elif first.version == 6:
799+ new_zone = "%x-%d.%s" % (base, network.prefixlen, zone_rest)
800+ else:
801+ new_zone = "%d-%d.%s" % (base, network.prefixlen, zone_rest)
802+ info.append(DNSZoneInfo(
803+ IPNetwork("%s/%d" % (first, subnet_prefix)),
804+ new_zone))
805+ base += 1
806+ try:
807+ first += step
808+ except IndexError:
809+ # IndexError occurs when we go from 255.255.255.255 to
810+ # 0.0.0.0. If we hit that, we're all fine and done.
811+ break
812+ return info
813
814 @classmethod
815 def get_PTR_mapping(cls, mapping, domain, network):
816@@ -366,7 +444,7 @@
817 :param mapping: A hostname:ip-addresses mapping for all known hosts in
818 the reverse zone.
819 :param domain: Zone's domain name.
820- :param network: Zone's network.
821+ :param network: DNS Zone's network.
822 :type network: :class:`netaddr.IPNetwork`
823 """
824 return (
825@@ -380,7 +458,7 @@
826 )
827
828 @classmethod
829- def get_GENERATE_directives(cls, dynamic_range, domain):
830+ def get_GENERATE_directives(cls, dynamic_range, domain, zone_info):
831 """Return the GENERATE directives for the reverse zone of a network."""
832 slash_16 = IPNetwork("%s/16" % IPAddress(dynamic_range.first))
833 if (dynamic_range.size > 256 ** 2 or
834@@ -391,19 +469,29 @@
835 return []
836
837 generate_directives = set()
838+ # The largest subnet returned is a /24.
839 subnets, prefix, rdns_suffix = get_details_for_ip_range(dynamic_range)
840 for subnet in subnets:
841- iterator = "%d-%d" % (
842- (subnet.first & 0x000000ff),
843- (subnet.last & 0x000000ff))
844- hostname = "%s-%d-$" % (
845- prefix.replace('.', '-'),
846- (subnet.first & 0x0000ff00) >> 8)
847- rdns = "$.%d.%s" % (
848- (subnet.first & 0x0000ff00) >> 8,
849- rdns_suffix)
850- generate_directives.add(
851- (iterator, rdns, "%s.%s." % (hostname, domain)))
852+ if (IPAddress(subnet.first) in zone_info.subnetwork):
853+ iterator = "%d-%d" % (
854+ (subnet.first & 0x000000ff),
855+ (subnet.last & 0x000000ff))
856+ hostname = "%s-%d-$" % (
857+ prefix.replace('.', '-'),
858+ (subnet.first & 0x0000ff00) >> 8)
859+ # If we're at least a /24, then fully specify the name,
860+ # rather than trying to figure out how much of the name
861+ # is in the zone name.
862+ if zone_info.subnetwork.prefixlen <= 24:
863+ rdns = "$.%d.%s" % (
864+ (subnet.first & 0x0000ff00) >> 8,
865+ rdns_suffix)
866+ else:
867+ # Let the zone declaration provide the suffix.
868+ # rather than trying to calculate it.
869+ rdns = "$"
870+ generate_directives.add(
871+ (iterator, rdns, "%s.%s." % (hostname, domain)))
872
873 return sorted(
874 generate_directives, key=lambda directive: directive[2])
875@@ -411,21 +499,25 @@
876 def write_config(self):
877 """Write the zone file."""
878 # Create GENERATE directives for IPv4 ranges.
879- generate_directives = list(
880- chain.from_iterable(
881- self.get_GENERATE_directives(dynamic_range, self.domain)
882- for dynamic_range in self._dynamic_ranges
883- if dynamic_range.version == 4
884- ))
885- self.write_zone_file(
886- self.target_path, self.make_parameters(),
887- {
888- 'mappings': {
889- 'PTR': self.get_PTR_mapping(
890- self._mapping, self.domain, self._network),
891- },
892- 'generate_directives': {
893- 'PTR': generate_directives,
894+ for zi in self.zone_info:
895+ generate_directives = list(
896+ chain.from_iterable(
897+ self.get_GENERATE_directives(
898+ dynamic_range,
899+ self.domain,
900+ zi)
901+ for dynamic_range in self._dynamic_ranges
902+ if dynamic_range.version == 4
903+ ))
904+ self.write_zone_file(
905+ zi.target_path, self.make_parameters(),
906+ {
907+ 'mappings': {
908+ 'PTR': self.get_PTR_mapping(
909+ self._mapping, self.domain, zi.subnetwork),
910+ },
911+ 'generate_directives': {
912+ 'PTR': generate_directives,
913+ }
914 }
915- }
916- )
917+ )