Merge lp:~mpontillo/maas/dns-template-changes-trunk into lp:maas/trunk

Proposed by Mike Pontillo on 2015-07-09
Status: Merged
Approved by: Mike Pontillo on 2015-07-09
Approved revision: 4081
Merged at revision: 4082
Proposed branch: lp:~mpontillo/maas/dns-template-changes-trunk
Merge into: lp:maas/trunk
Diff against target: 351 lines (+176/-6)
8 files modified
etc/maas/templates/dns/named.conf.options.inside.maas.template (+7/-0)
src/maasserver/management/commands/edit_named_options.py (+1/-1)
src/maasserver/tests/test_commands_edit_named_options.py (+2/-2)
src/provisioningserver/dns/config.py (+38/-0)
src/provisioningserver/dns/testing.py (+2/-0)
src/provisioningserver/dns/tests/test_config.py (+117/-0)
src/provisioningserver/utils/isc.py (+3/-1)
src/provisioningserver/utils/tests/test_isc.py (+6/-2)
To merge this branch: bzr merge lp:~mpontillo/maas/dns-template-changes-trunk
Reviewer Review Type Date Requested Status
Raphaël Badin (community) Approve on 2015-07-09
Andres Rodriguez (community) 2015-07-09 Approve on 2015-07-09
Review via email: mp+264216@code.launchpad.net

Commit message

Allow users to define allow-* parameters in named.conf.options; prevent writing them to named.conf.options.inside.maas in that case.

Description of the change

 * Add code to parse /etc/bind/named.conf.options before writing out the template and check if we're about to write duplicate options to the configuration. If so, suppress those options from being written from within the template.

 * Move ISC parsing utilities to provisioningserver so they can be used within the DNS writing code (which seems like it doesn't belong in provisioningserver in the first place, but moving that seemed more risky than moving a single utility, and the ISC parsing utilities might one day be used on the cluster.)

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

lgtm! Although, I'd like someone else to review as well.

review: Approve
Raphaël Badin (rvb) wrote :

Looks good. I've got a question and a bunch of remarks but nothing major.

review: Approve
Mike Pontillo (mpontillo) wrote :

Thanks for the review. I went back and re-did this with the "bzr mv" command, and addressed your other concerns.

MAAS Lander (maas-lander) wrote :
Download full text (81.3 KiB)

The attempt to merge lp:~mpontillo/maas/dns-template-changes-trunk into lp:maas failed. Below is the output from the failed tests.

Ign http://security.ubuntu.com trusty-security InRelease
Get:1 http://security.ubuntu.com trusty-security Release.gpg [933 B]
Get:2 http://security.ubuntu.com trusty-security Release [63.5 kB]
Ign http://nova.clouds.archive.ubuntu.com trusty InRelease
Ign http://nova.clouds.archive.ubuntu.com trusty-updates InRelease
Get:3 http://security.ubuntu.com trusty-security/main Sources [87.9 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty Release.gpg
Get:4 http://nova.clouds.archive.ubuntu.com trusty-updates Release.gpg [933 B]
Get:5 http://security.ubuntu.com trusty-security/universe Sources [28.1 kB]
Get:6 http://security.ubuntu.com trusty-security/main amd64 Packages [309 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty Release
Get:7 http://nova.clouds.archive.ubuntu.com trusty-updates Release [63.5 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty/main Sources
Get:8 http://security.ubuntu.com trusty-security/universe amd64 Packages [111 kB]
Hit http://security.ubuntu.com trusty-security/main Translation-en
Hit http://security.ubuntu.com trusty-security/universe Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Sources
Hit http://nova.clouds.archive.ubuntu.com trusty/main amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/universe amd64 Packages
Hit http://nova.clouds.archive.ubuntu.com trusty/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en
Get:9 http://nova.clouds.archive.ubuntu.com trusty-updates/main Sources [213 kB]
Get:10 http://nova.clouds.archive.ubuntu.com trusty-updates/universe Sources [125 kB]
Get:11 http://nova.clouds.archive.ubuntu.com trusty-updates/main amd64 Packages [569 kB]
Get:12 http://nova.clouds.archive.ubuntu.com trusty-updates/universe amd64 Packages [295 kB]
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/main Translation-en
Hit http://nova.clouds.archive.ubuntu.com trusty-updates/universe Translation-en
Ign http://nova.clouds.archive.ubuntu.com trusty/main Translation-en_US
Ign http://nova.clouds.archive.ubuntu.com trusty/universe Translation-en_US
Fetched 1,867 kB in 3s (579 kB/s)
Reading package lists...
sudo DEBIAN_FRONTEND=noninteractive apt-get -y \
     --no-install-recommends install apache2 authbind bind9 bind9utils build-essential bzr-builddeb chromium-browser chromium-chromedriver curl daemontools debhelper dh-apport dh-systemd distro-info dnsutils firefox freeipmi-tools git gjs ipython isc-dhcp-common libjs-angularjs libjs-jquery libjs-jquery-hotkeys libjs-yui3-full libjs-yui3-min libpq-dev make nodejs-legacy npm pep8 phantomjs postgresql pyflakes python-apt python-bson python-bzrlib python-convoy python-coverage python-crochet python-cssselect python-curtin python-dev python-distro-info python-django python-django-piston python-django-south python-djorm-ext-pgarray python-docutils python-extras python-fixtures python-flake8 python-formencode python-hivex python-httplib2 python-jinja2 python-jsonschema python-lockfile python-lxml python-mock python-netaddr python-netifaces python-n...

4081. By Mike Pontillo on 2015-07-09

Format imports

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.options.inside.maas.template'
2--- etc/maas/templates/dns/named.conf.options.inside.maas.template 2015-04-24 19:49:30 +0000
3+++ etc/maas/templates/dns/named.conf.options.inside.maas.template 2015-07-09 22:46:20 +0000
4@@ -8,6 +8,13 @@
5
6 dnssec-validation {{dnssec_validation}};
7
8+
9+{{if not upstream_allow_query}}
10 allow-query { any; };
11+{{endif}}
12+{{if not upstream_allow_recursion}}
13 allow-recursion { trusted; };
14+{{endif}}
15+{{if not upstream_allow_query_cache}}
16 allow-query-cache { trusted; };
17+{{endif}}
18
19=== modified file 'src/maasserver/management/commands/edit_named_options.py'
20--- src/maasserver/management/commands/edit_named_options.py 2015-07-07 20:11:36 +0000
21+++ src/maasserver/management/commands/edit_named_options.py 2015-07-09 22:46:20 +0000
22@@ -33,7 +33,7 @@
23 CommandError,
24 )
25 from maasserver.models import Config
26-from maasserver.utils.isc import (
27+from provisioningserver.utils.isc import (
28 ISCParseException,
29 make_isc_string,
30 parse_isc_string,
31
32=== modified file 'src/maasserver/tests/test_commands_edit_named_options.py'
33--- src/maasserver/tests/test_commands_edit_named_options.py 2015-07-07 22:40:48 +0000
34+++ src/maasserver/tests/test_commands_edit_named_options.py 2015-07-09 22:46:20 +0000
35@@ -30,12 +30,12 @@
36 from maasserver.testing.factory import factory
37 from maasserver.testing.testcase import MAASServerTestCase
38 from maasserver.utils import get_one
39-from maasserver.utils.isc import (
40+from provisioningserver.dns.config import MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME
41+from provisioningserver.utils.isc import (
42 make_isc_string,
43 parse_isc_string,
44 read_isc_file,
45 )
46-from provisioningserver.dns.config import MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME
47 from testtools.matchers import (
48 Contains,
49 Equals,
50
51=== modified file 'src/provisioningserver/dns/config.py'
52--- src/provisioningserver/dns/config.py 2015-05-07 18:14:38 +0000
53+++ src/provisioningserver/dns/config.py 2015-07-09 22:46:20 +0000
54@@ -31,10 +31,12 @@
55
56 from provisioningserver.utils import locate_config
57 from provisioningserver.utils.fs import atomic_write
58+from provisioningserver.utils.isc import read_isc_file
59 from provisioningserver.utils.shell import call_and_check
60 import tempita
61
62
63+NAMED_CONF_OPTIONS = 'named.conf.options'
64 MAAS_NAMED_CONF_NAME = 'named.conf.maas'
65 MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME = 'named.conf.options.inside.maas'
66 MAAS_NAMED_RNDC_CONF_NAME = 'named.conf.rndc.maas'
67@@ -53,6 +55,18 @@
68 return setting
69
70
71+def get_bind_config_dir():
72+ """Location of bind configuration files."""
73+ setting = os.getenv(
74+ "MAAS_BIND_CONFIG_DIR",
75+ locate_config(os.path.pardir, "bind"))
76+ if isinstance(setting, bytes):
77+ fsenc = sys.getfilesystemencoding()
78+ return setting.decode(fsenc)
79+ else:
80+ return setting
81+
82+
83 def get_dns_rndc_port():
84 """RNDC port to be configured by MAAS to communicate with BIND."""
85 setting = os.getenv("MAAS_DNS_RNDC_PORT", "954")
86@@ -193,6 +207,25 @@
87 # template that uses this value.
88 kwargs.setdefault("upstream_dns")
89 kwargs.setdefault("dnssec_validation", "auto")
90+
91+ # Parse the options file and make sure MAAS doesn't define any options
92+ # that the user has already customized.
93+ allow_user_override_options = [
94+ "allow-query",
95+ "allow-recursion",
96+ "allow-query-cache",
97+ ]
98+
99+ try:
100+ parsed_options = read_isc_file(
101+ compose_bind_config_path(NAMED_CONF_OPTIONS))
102+ except IOError:
103+ parsed_options = {}
104+
105+ options = parsed_options.get('options', {})
106+ for option in allow_user_override_options:
107+ kwargs['upstream_' + option.replace('-', '_')] = option in options
108+
109 try:
110 rendered = template.substitute(kwargs)
111 except NameError as error:
112@@ -207,6 +240,11 @@
113 return os.path.join(get_dns_config_dir(), filename)
114
115
116+def compose_bind_config_path(filename):
117+ """Return the full path for a DNS config or zone file."""
118+ return os.path.join(get_bind_config_dir(), filename)
119+
120+
121 def render_dns_template(template_name, *parameters):
122 """Generate contents for a DNS configuration or zone file.
123
124
125=== modified file 'src/provisioningserver/dns/testing.py'
126--- src/provisioningserver/dns/testing.py 2015-05-11 09:51:49 +0000
127+++ src/provisioningserver/dns/testing.py 2015-07-09 22:46:20 +0000
128@@ -32,6 +32,8 @@
129 config_dir = config_dir.encode(fsenc)
130 testcase.useFixture(
131 EnvironmentVariable(b"MAAS_DNS_CONFIG_DIR", config_dir))
132+ testcase.useFixture(
133+ EnvironmentVariable(b"MAAS_BIND_CONFIG_DIR", config_dir))
134 return config_dir.decode(fsenc)
135
136
137
138=== modified file 'src/provisioningserver/dns/tests/test_config.py'
139--- src/provisioningserver/dns/tests/test_config.py 2015-05-07 18:14:38 +0000
140+++ src/provisioningserver/dns/tests/test_config.py 2015-07-09 22:46:20 +0000
141@@ -39,6 +39,7 @@
142 MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME,
143 MAAS_NAMED_RNDC_CONF_NAME,
144 MAAS_RNDC_CONF_NAME,
145+ NAMED_CONF_OPTIONS,
146 render_dns_template,
147 report_missing_config_dir,
148 set_up_options_conf,
149@@ -54,12 +55,16 @@
150 DNSReverseZoneConfig,
151 )
152 from provisioningserver.utils import locate_config
153+from provisioningserver.utils.isc import read_isc_file
154 from testtools.matchers import (
155+ AllMatch,
156 Contains,
157 ContainsAll,
158 EndsWith,
159+ Equals,
160 FileContains,
161 FileExists,
162+ Is,
163 IsInstance,
164 MatchesAll,
165 Not,
166@@ -70,6 +75,47 @@
167 from twisted.python.filepath import FilePath
168
169
170+NAMED_CONF_OPTIONS_CONTENTS = dedent("""\
171+ options {
172+ forwarders {
173+ 8.8.8.8;
174+ 8.8.4.4;
175+ };
176+ dnssec-validation auto;
177+ allow-query { any; };
178+ allow-recursion { trusted; };
179+ allow-query-cache { trusted; };
180+ auth-nxdomain no;
181+ listen-on-v6 { any; };
182+ };
183+ """)
184+
185+NAMED_CONF_OPTIONS_WITH_ALLOW_QUERY_CONTENTS = dedent("""\
186+ options {
187+ forwarders {
188+ 8.8.8.8;
189+ 8.8.4.4;
190+ };
191+ dnssec-validation auto;
192+ allow-query { any; };
193+ auth-nxdomain no;
194+ listen-on-v6 { any; };
195+ };
196+ """)
197+
198+NAMED_CONF_OPTIONS_NO_ALLOW_CONTENTS = dedent("""\
199+ options {
200+ forwarders {
201+ 8.8.8.8;
202+ 8.8.4.4;
203+ };
204+ dnssec-validation auto;
205+ auth-nxdomain no;
206+ listen-on-v6 { any; };
207+ };
208+ """)
209+
210+
211 class TestHelpers(MAASTestCase):
212
213 def test_get_dns_config_dir_defaults_to_etc_bind_maas(self):
214@@ -90,6 +136,24 @@
215 IsInstance(unicode),
216 ))
217
218+ def test_get_bind_config_dir_defaults_to_etc_bind_maas(self):
219+ self.useFixture(EnvironmentVariable("MAAS_BIND_CONFIG_DIR"))
220+ self.assertThat(
221+ config.get_bind_config_dir(), MatchesAll(
222+ SamePath(locate_config("../bind")),
223+ IsInstance(unicode),
224+ ))
225+
226+ def test_get_bind_config_dir_checks_environ_first(self):
227+ directory = self.make_dir()
228+ self.useFixture(EnvironmentVariable(
229+ "MAAS_BIND_CONFIG_DIR", directory.encode("ascii")))
230+ self.assertThat(
231+ config.get_bind_config_dir(), MatchesAll(
232+ SamePath(directory),
233+ IsInstance(unicode),
234+ ))
235+
236 def test_get_dns_root_port_defaults_to_954(self):
237 self.useFixture(EnvironmentVariable("MAAS_DNS_RNDC_PORT"))
238 self.assertEqual(954, config.get_dns_rndc_port())
239@@ -142,6 +206,59 @@
240 FileContains(matcher=Contains(address))
241 for address in fake_dns)))
242
243+ def test_set_up_options_conf_write_config_assumes_no_overrides(self):
244+ dns_conf_dir = patch_dns_config_path(self)
245+ set_up_options_conf()
246+ target_file = os.path.join(
247+ dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
248+ target = read_isc_file(target_file)
249+ self.assertThat([
250+ target['allow-query']['any'],
251+ target['allow-recursion']['trusted'],
252+ target['allow-query-cache']['trusted'],
253+ ], AllMatch(Equals(True)))
254+
255+ def test_set_up_options_conf_write_config_allows_overrides(self):
256+ dns_conf_dir = patch_dns_config_path(self)
257+ factory.make_file(
258+ location=dns_conf_dir, name=NAMED_CONF_OPTIONS,
259+ contents=NAMED_CONF_OPTIONS_CONTENTS)
260+ set_up_options_conf()
261+ target_file = os.path.join(
262+ dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
263+ target = read_isc_file(target_file)
264+ self.assertThat([
265+ target.get('allow-query'),
266+ target.get('allow-recursion'),
267+ target.get('allow-query-cache'),
268+ ], AllMatch(Is(None)))
269+
270+ def test_set_up_options_conf_write_config_allows_zero_overrides(self):
271+ dns_conf_dir = patch_dns_config_path(self)
272+ factory.make_file(
273+ location=dns_conf_dir, name=NAMED_CONF_OPTIONS,
274+ contents=NAMED_CONF_OPTIONS_NO_ALLOW_CONTENTS)
275+ set_up_options_conf()
276+ target_file = os.path.join(
277+ dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
278+ target = read_isc_file(target_file)
279+ self.assertThat([
280+ target['allow-query']['any'],
281+ target['allow-recursion']['trusted'],
282+ target['allow-query-cache']['trusted'],
283+ ], AllMatch(Equals(True)))
284+
285+ def test_set_up_options_conf_write_config_allows_single_override(self):
286+ dns_conf_dir = patch_dns_config_path(self)
287+ factory.make_file(
288+ location=dns_conf_dir, name=NAMED_CONF_OPTIONS,
289+ contents=NAMED_CONF_OPTIONS_WITH_ALLOW_QUERY_CONTENTS)
290+ set_up_options_conf()
291+ target_file = os.path.join(
292+ dns_conf_dir, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
293+ target = read_isc_file(target_file)
294+ self.assertIsNone(target.get('allow-query'))
295+
296 def test_set_up_options_conf_handles_no_upstream_dns(self):
297 dns_conf_dir = patch_dns_config_path(self)
298 set_up_options_conf()
299
300=== renamed file 'src/maasserver/utils/isc.py' => 'src/provisioningserver/utils/isc.py'
301--- src/maasserver/utils/isc.py 2015-07-07 20:50:41 +0000
302+++ src/provisioningserver/utils/isc.py 2015-07-09 22:46:20 +0000
303@@ -144,6 +144,8 @@
304 # If there are more than 1 'keywords' at new_char_list[index]
305 # ex - "recursion no;"
306 elif len(new_char_list[index].split()) >= 2:
307+ if type(dictionary_fragment) == list:
308+ raise ISCParseException("Syntax error")
309 dictionary_fragment[
310 new_char_list[index].split()[0]] = (
311 ' '.join(new_char_list[index].split()[1:]))
312@@ -155,7 +157,7 @@
313 elif new_char_list[index] not in ['{', ';', '}']:
314 key = new_char_list[index]
315 if type(dictionary_fragment) == list:
316- raise ISCParseException("Dictionary expected; got a list")
317+ raise ISCParseException("Syntax error")
318 dictionary_fragment[key] = ''
319 index += 1
320 index += 1
321
322=== renamed file 'src/maasserver/utils/tests/test_isc.py' => 'src/provisioningserver/utils/tests/test_isc.py'
323--- src/maasserver/utils/tests/test_isc.py 2015-07-07 20:50:41 +0000
324+++ src/provisioningserver/utils/tests/test_isc.py 2015-07-09 22:46:20 +0000
325@@ -18,13 +18,13 @@
326 from collections import OrderedDict
327 from textwrap import dedent
328
329-from maasserver.utils.isc import (
330+from maastesting.testcase import MAASTestCase
331+from provisioningserver.utils.isc import (
332 ISCParseException,
333 make_isc_string,
334 parse_isc_string,
335 read_isc_file,
336 )
337-from maastesting.testcase import MAASTestCase
338 from testtools import ExpectedException
339
340
341@@ -220,6 +220,10 @@
342 with ExpectedException(ISCParseException):
343 parse_isc_string("forwarders {{}a;;b}")
344
345+ def test_parse_forgotten_semicolons_throw_iscparseexception(self):
346+ with ExpectedException(ISCParseException):
347+ parse_isc_string("a { b; } { c; } d e;")
348+
349 def test_read_isc_file(self):
350 testdata = dedent("""\
351 acl canonical-int-ns { 91.189.90.151; 91.189.89.192; };