Merge lp:~lamont/maas/dns-1.9 into lp:maas/1.9

Proposed by LaMont Jones on 2016-01-07
Status: Rejected
Rejected by: LaMont Jones on 2016-04-27
Proposed branch: lp:~lamont/maas/dns-1.9
Merge into: lp:maas/1.9
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-1.9
Reviewer Review Type Date Requested Status
MAAS Committers 2016-01-07 Pending
Review via email: mp+281879@code.launchpad.net

Commit message

Correctly manage reverse DNS: do not manage a /16 because the user added a /22 network.

Description of the change

Correctly manage reverse DNS: do not manage a /16 because the user added a /22 network.

To post a comment you must log in.
Andres Rodriguez (andreserl) wrote :

I thought that we agarres that since this was a change of behavior we were
going to defer this?

On Thursday, January 7, 2016, LaMont Jones <email address hidden>
wrote:

> LaMont Jones has proposed merging lp:~lamont/maas/dns-1.9 into lp:maas/1.9.
>
> Commit message:
> Correctly manage reverse DNS: do not manage a /16 because the user added a
> /22 network.
>
> Requested reviews:
> MAAS Committers (maas-committers)
> Related bugs:
> Bug #1356012 in MAAS: "maas incorrectly overmanages DNS reverse zones"
> https://bugs.launchpad.net/maas/+bug/1356012
> Bug #1531868 in MAAS: "Add network fails for multiple bigger-than-/24
> networks in a single /16"
> https://bugs.launchpad.net/maas/+bug/1531868
>
> For more details, see:
> https://code.launchpad.net/~lamont/maas/dns-1.9/+merge/281879
>
> Correctly manage reverse DNS: do not manage a /16 because the user added a
> /22 network.
> --
> Your team MAAS Committers is requested to review the proposed merge of
> lp:~lamont/maas/dns-1.9 into lp:maas/1.9.
>

--
Andres Rodriguez (RoAkSoAx)
Ubuntu Server Developer
MSc. Telecom & Networking
Systems Engineer

LaMont Jones (lamont) wrote :

That was before a maas 1.8 user reported #1531868, wherein maas tries to reload DNS for changes, and fails utterly to do so, because the named.conf is invalid.

I think that this bugfix warrants reconsideration since it's breaking maas 1.8 users in the field.

Andres Rodriguez (andreserl) wrote :

Ok, let's discuss this next week.

Thanks!

On Thu, Jan 14, 2016 at 2:00 AM, LaMont Jones <email address hidden>
wrote:

> That was before a maas 1.8 user reported #1531868, wherein maas tries to
> reload DNS for changes, and fails utterly to do so, because the named.conf
> is invalid.
>
> I think that this bugfix warrants reconsideration since it's breaking maas
> 1.8 users in the field.
> --
> https://code.launchpad.net/~lamont/maas/dns-1.9/+merge/281879
> Your team MAAS Committers is requested to review the proposed merge of
> lp:~lamont/maas/dns-1.9 into lp:maas/1.9.
>

--
Andres Rodriguez
Engineering Manager, MAAS
Canonical USA, Inc.

lp:~lamont/maas/dns-1.9 updated on 2016-02-17
4520. By LaMont Jones on 2016-02-17

Merge from maas/1.9.

Mike Pontillo (mpontillo) wrote :

Does this still need review, or is it WIP? (I was under the impression that this code has changed significantly in trunk, and would need to be ported back to 1.9 in order for it to be usable there. Is that correct?)

Unmerged revisions

4520. By LaMont Jones on 2016-02-17

Merge from maas/1.9.

4519. By LaMont Jones on 2015-11-28

Cleanup errors introduced in doing review feedback

4518. By LaMont Jones on 2015-11-28

Review feedback

4517. By LaMont Jones on 2015-11-28

For reverse zones, get_GENERATE_directives needs to follow rfc2317.

4516. By LaMont Jones on 2015-11-28

Correctly cast subnet.first to avoid errors with trusty's netaddr.

4515. By LaMont Jones on 2015-11-27

Merge DNS fixes from trunk

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 2016-02-17 21:43:45 +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 2016-02-17 21:43:45 +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 2016-02-17 21:43:45 +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 2016-02-17 21:43:45 +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 2016-02-17 21:43:45 +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 2016-02-17 21:43:45 +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 2016-02-17 21:43:45 +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+ )

Subscribers

People subscribed via source and target branches