Merge lp:~mpontillo/maas/migrate-dns-settings-1.8 into lp:maas/1.8
- migrate-dns-settings-1.8
- Merge into 1.8
Proposed by
Mike Pontillo
Status: | Merged |
---|---|
Approved by: | Mike Pontillo |
Approved revision: | no longer in the source branch. |
Merged at revision: | 4018 |
Proposed branch: | lp:~mpontillo/maas/migrate-dns-settings-1.8 |
Merge into: | lp:maas/1.8 |
Diff against target: |
689 lines (+409/-39) 4 files modified
src/maasserver/management/commands/edit_named_options.py (+116/-14) src/maasserver/tests/test_commands_edit_named_options.py (+169/-15) src/maasserver/utils/isc.py (+25/-4) src/maasserver/utils/tests/test_isc.py (+99/-6) |
To merge this branch: | bzr merge lp:~mpontillo/maas/migrate-dns-settings-1.8 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Mike Pontillo (community) | Approve | ||
Andres Rodriguez (community) | Approve | ||
Review via email: mp+264086@code.launchpad.net |
This proposal supersedes a proposal from 2015-07-08.
Commit message
Backport DNS migration fixes (Merge revision 4077 from trunk)
Description of the change
To post a comment you must log in.
Revision history for this message
Mike Pontillo (mpontillo) wrote : | # |
I'll add my own +1; this was a trivial merge from trunk.
review:
Approve
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'src/maasserver/management/commands/edit_named_options.py' |
2 | --- src/maasserver/management/commands/edit_named_options.py 2015-07-01 17:14:28 +0000 |
3 | +++ src/maasserver/management/commands/edit_named_options.py 2015-07-08 00:02:38 +0000 |
4 | @@ -11,6 +11,8 @@ |
5 | print_function, |
6 | unicode_literals, |
7 | ) |
8 | +from collections import OrderedDict |
9 | + |
10 | |
11 | str = None |
12 | |
13 | @@ -19,16 +21,20 @@ |
14 | 'Command', |
15 | ] |
16 | |
17 | +from copy import deepcopy |
18 | from datetime import datetime |
19 | from optparse import make_option |
20 | import os |
21 | import shutil |
22 | +import sys |
23 | |
24 | from django.core.management.base import ( |
25 | BaseCommand, |
26 | CommandError, |
27 | ) |
28 | +from maasserver.models import Config |
29 | from maasserver.utils.isc import ( |
30 | + ISCParseException, |
31 | make_isc_string, |
32 | parse_isc_string, |
33 | ) |
34 | @@ -42,6 +48,24 @@ |
35 | '--config-path', dest='config_path', |
36 | default="/etc/bind/named.conf.options", |
37 | help="Specify the configuration file to edit."), |
38 | + make_option( |
39 | + '--dry-run', dest='dry_run', |
40 | + default=False, action='store_true', |
41 | + help="Do not edit any configuration; instead, print to stdout the " |
42 | + "actions that would be performed, and/or the new " |
43 | + "configuration that would be written."), |
44 | + make_option( |
45 | + '--force', dest='force', |
46 | + default=False, action='store_true', |
47 | + help="Force the BIND configuration to be written, even if it " |
48 | + "appears as though nothing has changed."), |
49 | + make_option( |
50 | + '--migrate-conflicting-options', default=False, |
51 | + dest='migrate_conflicting_options', action='store_true', |
52 | + help="Causes any options that conflict with MAAS-managed options " |
53 | + "to be deleted from the BIND configuration and moved to the " |
54 | + "MAAS-managed configuration. Requires the MAAS database to " |
55 | + "be configured and running."), |
56 | ) |
57 | help = ( |
58 | "Edit the named.conf.options file so that it includes the " |
59 | @@ -67,13 +91,12 @@ |
60 | """ |
61 | try: |
62 | config_dict = parse_isc_string(options_file) |
63 | - except Exception as e: |
64 | - # Yes, it throws bare exceptions :( |
65 | + except ISCParseException as e: |
66 | raise CommandError("Failed to parse %s: %s" % ( |
67 | config_path, e.message)) |
68 | options_block = config_dict.get("options", None) |
69 | if options_block is None: |
70 | - # Something is horribly wrong with the file, bail out rather |
71 | + # Something is horribly wrong with the file; bail out rather |
72 | # than doing anything drastic. |
73 | raise CommandError( |
74 | "Can't find options {} block in %s, bailing out without " |
75 | @@ -86,24 +109,65 @@ |
76 | options_block['include'] = '"%s%s%s"' % ( |
77 | dir, os.path.sep, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME) |
78 | |
79 | - def remove_forwarders(self, options_block): |
80 | + def migrate_forwarders(self, options_block, dry_run, stdout): |
81 | """Remove existing forwarders from the options block. |
82 | |
83 | It's a syntax error to have more than one in the combined |
84 | - configuration for named so we just remove whatever was there. |
85 | - There is no data loss due to the backup file made later. |
86 | + configuration for named, so we just remove whatever was there. |
87 | + |
88 | + Migrate any forwarders in the configuration file to the MAAS config. |
89 | """ |
90 | if 'forwarders' in options_block: |
91 | + bind_forwarders = options_block['forwarders'] |
92 | + |
93 | + if not dry_run: |
94 | + config, created = Config.objects.get_or_create( |
95 | + name='upstream_dns', |
96 | + defaults={'value': ' '.join(bind_forwarders)}) |
97 | + if not created: |
98 | + # A configuration value already exists, so add the |
99 | + # additional values we found in the configuration file to |
100 | + # MAAS. |
101 | + if config.value is None: |
102 | + config.value = '' |
103 | + maas_forwarders = OrderedDict.fromkeys( |
104 | + config.value.split()) |
105 | + maas_forwarders.update(bind_forwarders) |
106 | + config.value = ' '.join(maas_forwarders) |
107 | + config.save() |
108 | + else: |
109 | + stdout.write( |
110 | + "// Append to MAAS forwarders: %s\n" |
111 | + % ' '.join(bind_forwarders)) |
112 | + |
113 | del options_block['forwarders'] |
114 | |
115 | - def remove_dnssec_validation(self, options_block): |
116 | + def migrate_dnssec_validation(self, options_block, dry_run, stdout): |
117 | """Remove existing dnssec-validation from the options block. |
118 | |
119 | It's a syntax error to have more than one in the combined |
120 | configuration for named so we just remove whatever was there. |
121 | There is no data loss due to the backup file made later. |
122 | + |
123 | + Migrate this value in the configuration file to the MAAS config. |
124 | """ |
125 | if 'dnssec-validation' in options_block: |
126 | + dnssec_validation = options_block['dnssec-validation'] |
127 | + |
128 | + if not dry_run: |
129 | + config, created = Config.objects.get_or_create( |
130 | + name='dnssec_validation', |
131 | + defaults={'value': dnssec_validation}) |
132 | + if not created: |
133 | + # Update the MAAS configuration to reflect the new setting |
134 | + # found in the configuration file. |
135 | + config.value = dnssec_validation |
136 | + config.save() |
137 | + else: |
138 | + stdout.write( |
139 | + "// Set MAAS dnssec_validation to: %s\n" |
140 | + % dnssec_validation) |
141 | + |
142 | del options_block['dnssec-validation'] |
143 | |
144 | def back_up_existing_file(self, config_path): |
145 | @@ -115,22 +179,60 @@ |
146 | raise CommandError( |
147 | "Failed to make a backup of %s, exiting: %s" % ( |
148 | config_path, e.message)) |
149 | + return backup_destination |
150 | + |
151 | + def write_new_named_conf_options(self, fd, backup_filename, new_content): |
152 | + fd.write("""\ |
153 | +// |
154 | +// This file is managed by MAAS. Although MAAS attempts to preserve changes |
155 | +// made here, it is possible to create conflicts that MAAS can not resolve. |
156 | +// |
157 | +// DNS settings available in MAAS (for example, forwarders and |
158 | +// dnssec-validation) should be managed only in MAAS. |
159 | +// |
160 | +// The previous configuration file was backed up at: |
161 | +// %s |
162 | +// |
163 | +""" % backup_filename) |
164 | + fd.write(new_content) |
165 | + fd.write("\n") |
166 | |
167 | def handle(self, *args, **options): |
168 | """Entry point for BaseCommand.""" |
169 | # Read stuff in, validate. |
170 | config_path = options.get('config_path') |
171 | + dry_run = options.get('dry_run') |
172 | + force = options.get('force') |
173 | + stdout = options.get('stdout') |
174 | + if stdout is None: |
175 | + stdout = sys.stdout |
176 | + migrate_conflicting_options = options.get( |
177 | + 'migrate_conflicting_options') |
178 | + |
179 | options_file = self.read_file(config_path) |
180 | config_dict = self.parse_file(config_path, options_file) |
181 | + original_config = deepcopy(config_dict) |
182 | + |
183 | options_block = config_dict['options'] |
184 | |
185 | - # Modify the config. |
186 | + # Modify the configuration (if necessary). |
187 | self.set_up_include_statement(options_block, config_path) |
188 | - self.remove_forwarders(options_block) |
189 | - self.remove_dnssec_validation(options_block) |
190 | + |
191 | + if migrate_conflicting_options: |
192 | + self.migrate_forwarders(options_block, dry_run, stdout) |
193 | + self.migrate_dnssec_validation(options_block, dry_run, stdout) |
194 | + |
195 | + # Re-parse the new configuration, so we can detect any changes. |
196 | new_content = make_isc_string(config_dict) |
197 | + new_config = parse_isc_string(new_content) |
198 | |
199 | - # Back up and write new file. |
200 | - self.back_up_existing_file(config_path) |
201 | - with open(config_path, "wb") as fd: |
202 | - fd.write(new_content) |
203 | + if original_config != new_config or force: |
204 | + # The configuration has changed. Back up and write new file. |
205 | + if dry_run: |
206 | + self.write_new_named_conf_options( |
207 | + stdout, config_path, new_content) |
208 | + else: |
209 | + backup_filename = self.back_up_existing_file(config_path) |
210 | + with open(config_path, "wb") as fd: |
211 | + self.write_new_named_conf_options( |
212 | + fd, backup_filename, new_content) |
213 | |
214 | === modified file 'src/maasserver/tests/test_commands_edit_named_options.py' |
215 | --- src/maasserver/tests/test_commands_edit_named_options.py 2015-05-07 18:14:38 +0000 |
216 | +++ src/maasserver/tests/test_commands_edit_named_options.py 2015-07-08 00:02:38 +0000 |
217 | @@ -15,6 +15,7 @@ |
218 | __all__ = [] |
219 | |
220 | from codecs import getwriter |
221 | +from collections import OrderedDict |
222 | from io import BytesIO |
223 | import os |
224 | import shutil |
225 | @@ -22,11 +23,22 @@ |
226 | |
227 | from django.core.management import call_command |
228 | from django.core.management.base import CommandError |
229 | +from maasserver.management.commands.edit_named_options import ( |
230 | + Command as command_module, |
231 | +) |
232 | +from maasserver.models import Config |
233 | from maasserver.testing.factory import factory |
234 | from maasserver.testing.testcase import MAASServerTestCase |
235 | +from maasserver.utils import get_one |
236 | +from maasserver.utils.isc import ( |
237 | + make_isc_string, |
238 | + parse_isc_string, |
239 | + read_isc_file, |
240 | +) |
241 | from provisioningserver.dns.config import MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME |
242 | from testtools.matchers import ( |
243 | Contains, |
244 | + Equals, |
245 | FileContains, |
246 | Not, |
247 | ) |
248 | @@ -52,7 +64,27 @@ |
249 | OPTIONS_FILE_WITH_FORWARDERS = textwrap.dedent("""\ |
250 | options { |
251 | directory "/var/cache/bind"; |
252 | - forwarders { 192.168.1.1; }; |
253 | + forwarders { 192.168.1.1; 192.168.1.2; }; |
254 | + auth-nxdomain no; # conform to RFC1035 |
255 | + listen-on-v6 { any; }; |
256 | + }; |
257 | +""") |
258 | + |
259 | +OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC = textwrap.dedent("""\ |
260 | + options { |
261 | + directory "/var/cache/bind"; |
262 | + forwarders { 192.168.1.1; 192.168.1.2; }; |
263 | + dnssec-validation no; |
264 | + auth-nxdomain no; # conform to RFC1035 |
265 | + listen-on-v6 { any; }; |
266 | + }; |
267 | +""") |
268 | + |
269 | +OPTIONS_FILE_WITH_EXTRA_AND_DUP_FORWARDER = textwrap.dedent("""\ |
270 | + options { |
271 | + directory "/var/cache/bind"; |
272 | + forwarders { 192.168.1.2; 192.168.1.3; }; |
273 | + dnssec-validation no; |
274 | auth-nxdomain no; # conform to RFC1035 |
275 | listen-on-v6 { any; }; |
276 | }; |
277 | @@ -99,18 +131,27 @@ |
278 | self.assertContentFailsWithMessage( |
279 | OPTIONS_FILE, "Failed to make a backup") |
280 | |
281 | - def test_removes_existing_forwarders_config(self): |
282 | + def test_does_not_remove_existing_forwarders_config(self): |
283 | options_file = self.make_file(contents=OPTIONS_FILE_WITH_FORWARDERS) |
284 | call_command( |
285 | "edit_named_options", config_path=options_file, |
286 | stdout=self.stdout) |
287 | |
288 | + options = read_isc_file(options_file) |
289 | + self.assertThat(make_isc_string(options), Contains('forwarders')) |
290 | + |
291 | + def test_removes_existing_forwarders_config_if_migrate_set(self): |
292 | + options_file = self.make_file(contents=OPTIONS_FILE_WITH_FORWARDERS) |
293 | + call_command( |
294 | + "edit_named_options", config_path=options_file, |
295 | + migrate_conflicting_options=True, stdout=self.stdout) |
296 | + |
297 | # Check that the file was re-written without forwarders (since |
298 | # that's now in the included file). |
299 | + options = read_isc_file(options_file) |
300 | self.assertThat( |
301 | - options_file, |
302 | - Not(FileContains( |
303 | - matcher=Contains('forwarders')))) |
304 | + make_isc_string(options), |
305 | + Not(Contains('forwarders'))) |
306 | |
307 | def test_removes_existing_dnssec_validation_config(self): |
308 | options_file = self.make_file(contents=OPTIONS_FILE_WITH_DNSSEC) |
309 | @@ -118,12 +159,23 @@ |
310 | "edit_named_options", config_path=options_file, |
311 | stdout=self.stdout) |
312 | |
313 | - # Check that the file was re-written without forwarders (since |
314 | - # that's now in the included file). |
315 | - self.assertThat( |
316 | - options_file, |
317 | - Not(FileContains( |
318 | - matcher=Contains('dnssec-validation')))) |
319 | + # Check that the file was re-written without dnssec-validation (since |
320 | + # that's now in the included file). |
321 | + options = read_isc_file(options_file) |
322 | + self.assertThat( |
323 | + make_isc_string(options), Contains('dnssec-validation')) |
324 | + |
325 | + def test_removes_existing_dnssec_validation_config_if_migration_set(self): |
326 | + options_file = self.make_file(contents=OPTIONS_FILE_WITH_DNSSEC) |
327 | + call_command( |
328 | + "edit_named_options", config_path=options_file, |
329 | + migrate_conflicting_options=True, stdout=self.stdout) |
330 | + |
331 | + # Check that the file was re-written without dnssec-validation (since |
332 | + # that's now in the included file). |
333 | + options = read_isc_file(options_file) |
334 | + self.assertThat( |
335 | + make_isc_string(options), Not(Contains('dnssec-validation'))) |
336 | |
337 | def test_normal_operation(self): |
338 | options_file = self.make_file(contents=OPTIONS_FILE) |
339 | @@ -136,11 +188,10 @@ |
340 | MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME) |
341 | |
342 | # Check that the file was re-written with the include statement. |
343 | + options = read_isc_file(options_file) |
344 | self.assertThat( |
345 | - options_file, |
346 | - FileContains( |
347 | - matcher=Contains( |
348 | - 'include "%s";' % expected_path))) |
349 | + make_isc_string(options), |
350 | + Contains('include "%s";' % expected_path)) |
351 | |
352 | # Check that the backup was made. |
353 | options_file_base = os.path.dirname(options_file) |
354 | @@ -150,3 +201,106 @@ |
355 | [backup_file] = files |
356 | backup_file = os.path.join(options_file_base, backup_file) |
357 | self.assertThat(backup_file, FileContains(OPTIONS_FILE)) |
358 | + |
359 | + def test_migrates_bind_config_to_database(self): |
360 | + options_file = self.make_file( |
361 | + contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC) |
362 | + call_command( |
363 | + "edit_named_options", config_path=options_file, |
364 | + migrate_conflicting_options=True, stdout=self.stdout) |
365 | + |
366 | + upstream_dns = get_one(Config.objects.filter(name="upstream_dns")) |
367 | + self.assertThat({'192.168.1.1', '192.168.1.2'}, |
368 | + Equals(set(upstream_dns.value.split()))) |
369 | + |
370 | + dnssec_validation = get_one(Config.objects.filter( |
371 | + name="dnssec_validation")) |
372 | + self.assertThat('no', Equals(dnssec_validation.value)) |
373 | + |
374 | + def test_migrate_combines_with_existing_forwarders(self): |
375 | + options_file = self.make_file( |
376 | + contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC) |
377 | + call_command( |
378 | + "edit_named_options", config_path=options_file, |
379 | + migrate_conflicting_options=True, stdout=self.stdout) |
380 | + |
381 | + upstream_dns = get_one(Config.objects.filter(name="upstream_dns")) |
382 | + self.assertThat(OrderedDict.fromkeys(['192.168.1.1', '192.168.1.2']), |
383 | + Equals(OrderedDict.fromkeys( |
384 | + upstream_dns.value.split()))) |
385 | + |
386 | + dnssec_validation = get_one(Config.objects.filter( |
387 | + name="dnssec_validation")) |
388 | + self.assertThat('no', Equals(dnssec_validation.value)) |
389 | + |
390 | + options_file = self.make_file( |
391 | + contents=OPTIONS_FILE_WITH_EXTRA_AND_DUP_FORWARDER) |
392 | + |
393 | + call_command( |
394 | + "edit_named_options", config_path=options_file, |
395 | + migrate_conflicting_options=True, stdout=self.stdout) |
396 | + |
397 | + upstream_dns = get_one(Config.objects.filter(name="upstream_dns")) |
398 | + self.assertThat( |
399 | + OrderedDict.fromkeys( |
400 | + ['192.168.1.1', '192.168.1.2', '192.168.1.3']), |
401 | + Equals(OrderedDict.fromkeys(upstream_dns.value.split()))) |
402 | + |
403 | + def test_dry_run_migrates_nothing_and_prints_config(self): |
404 | + options_file = self.make_file( |
405 | + contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC) |
406 | + call_command( |
407 | + "edit_named_options", config_path=options_file, |
408 | + migrate_conflicting_options=True, dry_run=True, stdout=self.stdout) |
409 | + |
410 | + upstream_dns = get_one(Config.objects.filter(name="upstream_dns")) |
411 | + self.assertIsNone(upstream_dns) |
412 | + dnssec_validation = get_one(Config.objects.filter( |
413 | + name="dnssec_validation")) |
414 | + self.assertIsNone(dnssec_validation) |
415 | + |
416 | + # Check that a proper configuration was written to stdout. |
417 | + config = parse_isc_string(self.stdout.getvalue()) |
418 | + self.assertIsNotNone(config) |
419 | + |
420 | + def test_repeat_migrations_migrate_nothing(self): |
421 | + options_file = self.make_file( |
422 | + contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC) |
423 | + backup_mock = self.patch(command_module, "back_up_existing_file") |
424 | + |
425 | + call_command( |
426 | + "edit_named_options", config_path=options_file, |
427 | + migrate_conflicting_options=True, stdout=self.stdout) |
428 | + |
429 | + self.assertTrue(backup_mock.called) |
430 | + backup_mock.reset_mock() |
431 | + |
432 | + write_mock = self.patch(command_module, "write_new_named_conf_options") |
433 | + |
434 | + call_command( |
435 | + "edit_named_options", config_path=options_file, |
436 | + migrate_conflicting_options=True, stdout=self.stdout) |
437 | + |
438 | + self.assertFalse(backup_mock.called) |
439 | + self.assertFalse(write_mock.called) |
440 | + |
441 | + def test_repeat_forced_migrations_write_file_anyway(self): |
442 | + options_file = self.make_file( |
443 | + contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC) |
444 | + backup_mock = self.patch(command_module, "back_up_existing_file") |
445 | + |
446 | + call_command( |
447 | + "edit_named_options", config_path=options_file, |
448 | + migrate_conflicting_options=True, stdout=self.stdout) |
449 | + |
450 | + self.assertTrue(backup_mock.called) |
451 | + backup_mock.reset_mock() |
452 | + |
453 | + write_mock = self.patch(command_module, "write_new_named_conf_options") |
454 | + |
455 | + call_command( |
456 | + "edit_named_options", config_path=options_file, |
457 | + migrate_conflicting_options=True, force=True, stdout=self.stdout) |
458 | + |
459 | + self.assertTrue(backup_mock.called) |
460 | + self.assertTrue(write_mock.called) |
461 | |
462 | === modified file 'src/maasserver/utils/isc.py' |
463 | --- src/maasserver/utils/isc.py 2015-07-01 17:14:28 +0000 |
464 | +++ src/maasserver/utils/isc.py 2015-07-08 00:02:38 +0000 |
465 | @@ -33,18 +33,26 @@ |
466 | print_function, |
467 | unicode_literals, |
468 | ) |
469 | +from collections import OrderedDict |
470 | + |
471 | |
472 | str = None |
473 | |
474 | __metaclass__ = type |
475 | __all__ = [ |
476 | + 'ISCParseException', |
477 | 'make_isc_string', |
478 | 'parse_isc_string', |
479 | + 'read_isc_file', |
480 | ] |
481 | |
482 | import copy |
483 | |
484 | |
485 | +class ISCParseException(Exception): |
486 | + """Thrown when an ISC string cannot be parsed.""" |
487 | + |
488 | + |
489 | def _clip(char_list): |
490 | """Clips char_list to individual stanza. |
491 | |
492 | @@ -64,7 +72,7 @@ |
493 | return index, char_list[:index] |
494 | elif item == '}': |
495 | skip -= 1 |
496 | - raise Exception("Invalid brackets.") |
497 | + raise ISCParseException("Invalid brackets.") |
498 | |
499 | |
500 | def _parse_tokens(char_list): |
501 | @@ -86,11 +94,11 @@ |
502 | {'10.1.0/32': True, '10.1.1/32': True}}} |
503 | """ |
504 | index = 0 |
505 | - dictionary_fragment = {} |
506 | + dictionary_fragment = OrderedDict() |
507 | new_char_list = copy.deepcopy(char_list) |
508 | if type(new_char_list) == str: |
509 | return new_char_list |
510 | - if type(new_char_list) == dict: |
511 | + if type(new_char_list) == OrderedDict: |
512 | return new_char_list |
513 | last_open = None |
514 | continuous_line = False |
515 | @@ -146,6 +154,8 @@ |
516 | # fine) |
517 | elif new_char_list[index] not in ['{', ';', '}']: |
518 | key = new_char_list[index] |
519 | + if type(dictionary_fragment) == list: |
520 | + raise ISCParseException("Dictionary expected; got a list") |
521 | dictionary_fragment[key] = '' |
522 | index += 1 |
523 | index += 1 |
524 | @@ -277,7 +287,18 @@ |
525 | new_list[-1] = '%s%s' % (new_list[-1], terminator) |
526 | isc_list.append( |
527 | '%s { %s }%s' % (option, ' '.join(new_list), terminator)) |
528 | - elif type(isc_dict[option]) == dict: |
529 | + elif (type(isc_dict[option]) == OrderedDict or |
530 | + type(isc_dict[option]) == dict): |
531 | isc_list.append('%s { %s }%s' % ( |
532 | option, make_isc_string(isc_dict[option]), terminator)) |
533 | return '\n'.join(isc_list) |
534 | + |
535 | + |
536 | +def read_isc_file(isc_file): |
537 | + """Given the specified filename, parses it to create a dictionary. |
538 | + |
539 | + :param:isc_file: the filename to read |
540 | + :return:dict: dictionary of ISC file representation |
541 | + """ |
542 | + with open(isc_file, "r") as f: |
543 | + return parse_isc_string(f.read()) |
544 | |
545 | === modified file 'src/maasserver/utils/tests/test_isc.py' |
546 | --- src/maasserver/utils/tests/test_isc.py 2015-07-04 09:39:50 +0000 |
547 | +++ src/maasserver/utils/tests/test_isc.py 2015-07-08 00:02:38 +0000 |
548 | @@ -9,19 +9,23 @@ |
549 | unicode_literals, |
550 | ) |
551 | |
552 | + |
553 | str = None |
554 | |
555 | __metaclass__ = type |
556 | __all__ = [] |
557 | |
558 | - |
559 | +from collections import OrderedDict |
560 | from textwrap import dedent |
561 | |
562 | from maasserver.utils.isc import ( |
563 | + ISCParseException, |
564 | make_isc_string, |
565 | parse_isc_string, |
566 | + read_isc_file, |
567 | ) |
568 | from maastesting.testcase import MAASTestCase |
569 | +from testtools import ExpectedException |
570 | |
571 | |
572 | class TestParseISCString(MAASTestCase): |
573 | @@ -39,10 +43,11 @@ |
574 | """) |
575 | options = parse_isc_string(testdata) |
576 | self.assertEqual( |
577 | - {u'options': {u'auth-nxdomain': u'no', |
578 | - u'directory': u'"/var/cache/bind"', |
579 | - u'dnssec-validation': u'auto', |
580 | - u'listen-on-v6': {u'any': True}}}, options) |
581 | + OrderedDict({u'options': OrderedDict({u'auth-nxdomain': u'no', |
582 | + u'directory': u'"/var/cache/bind"', |
583 | + u'dnssec-validation': u'auto', |
584 | + u'listen-on-v6': OrderedDict({u'any': True})})}), |
585 | + options) |
586 | |
587 | def test_parses_bind_acl(self): |
588 | testdata = dedent("""\ |
589 | @@ -159,6 +164,94 @@ |
590 | config_string = make_isc_string(config) |
591 | config = parse_isc_string(config_string) |
592 | self.assertEqual( |
593 | + OrderedDict( |
594 | + [(u'acl canonical-int-ns', |
595 | + OrderedDict( |
596 | + [(u'91.189.90.151', True), (u'91.189.89.192', True)])), |
597 | + (u'options', OrderedDict( |
598 | + [(u'directory', u'"/var/cache/bind"'), |
599 | + (u'forwarders', OrderedDict( |
600 | + [(u'91.189.94.2', True)])), |
601 | + (u'dnssec-validation', u'auto'), |
602 | + (u'auth-nxdomain', u'no'), |
603 | + (u'listen-on-v6', OrderedDict([(u'any', True)])), |
604 | + (u'allow-query', OrderedDict([(u'any', True)])), |
605 | + (u'allow-transfer', OrderedDict( |
606 | + [(u'10.222.64.1', True), |
607 | + (u'canonical-int-ns', True)])), |
608 | + (u'notify', u'explicit'), |
609 | + (u'also-notify', OrderedDict( |
610 | + [(u'91.189.90.151', True), |
611 | + (u'91.189.89.192', True)])), |
612 | + (u'allow-query-cache', OrderedDict( |
613 | + [(u'10.222.64.0/18', True)])), |
614 | + (u'recursion', u'yes')])), |
615 | + (u'zone "."', OrderedDict( |
616 | + [(u'type', u'master'), |
617 | + (u'file', u'"/etc/bind/db.special"')]))]), |
618 | + config) |
619 | + |
620 | + def test_parser_preserves_order(self): |
621 | + testdata = dedent("""\ |
622 | + forwarders { |
623 | + 9.9.9.9; |
624 | + 8.8.8.8; |
625 | + 7.7.7.7; |
626 | + 6.6.6.6; |
627 | + 5.5.5.5; |
628 | + 4.4.4.4; |
629 | + 3.3.3.3; |
630 | + 2.2.2.2; |
631 | + 1.1.1.1; |
632 | + }; |
633 | + """) |
634 | + forwarders = parse_isc_string(testdata) |
635 | + self.assertEqual(OrderedDict([(u'forwarders', OrderedDict( |
636 | + [(u'9.9.9.9', True), (u'8.8.8.8', True), (u'7.7.7.7', True), |
637 | + (u'6.6.6.6', True), (u'5.5.5.5', True), (u'4.4.4.4', True), |
638 | + (u'3.3.3.3', True), (u'2.2.2.2', True), (u'1.1.1.1', True)]))]), |
639 | + forwarders) |
640 | + |
641 | + def test_parse_unmatched_brackets_throws_iscparseexception(self): |
642 | + with ExpectedException(ISCParseException): |
643 | + parse_isc_string("forwarders {") |
644 | + |
645 | + def test_parse_malformed_list_throws_iscparseexception(self): |
646 | + with ExpectedException(ISCParseException): |
647 | + parse_isc_string("forwarders {{}a;;b}") |
648 | + |
649 | + def test_read_isc_file(self): |
650 | + testdata = dedent("""\ |
651 | + acl canonical-int-ns { 91.189.90.151; 91.189.89.192; }; |
652 | + |
653 | + options { |
654 | + directory "/var/cache/bind"; |
655 | + |
656 | + forwarders { |
657 | + 91.189.94.2; |
658 | + 91.189.94.2; |
659 | + }; |
660 | + |
661 | + dnssec-validation auto; |
662 | + |
663 | + auth-nxdomain no; # conform to RFC1035 |
664 | + listen-on-v6 { any; }; |
665 | + |
666 | + allow-query { any; }; |
667 | + allow-transfer { 10.222.64.1; canonical-int-ns; }; |
668 | + |
669 | + notify explicit; |
670 | + also-notify { 91.189.90.151; 91.189.89.192; }; |
671 | + |
672 | + allow-query-cache { 10.222.64.0/18; }; |
673 | + recursion yes; |
674 | + }; |
675 | + |
676 | + zone "." { type master; file "/etc/bind/db.special"; }; |
677 | + """) |
678 | + testfile = self.make_file(contents=testdata) |
679 | + parsed = read_isc_file(testfile) |
680 | + self.assertEqual( |
681 | {u'acl canonical-int-ns': |
682 | {u'91.189.89.192': True, u'91.189.90.151': True}, |
683 | u'options': {u'allow-query': {u'any': True}, |
684 | @@ -176,4 +269,4 @@ |
685 | u'recursion': u'yes'}, |
686 | u'zone "."': |
687 | {u'file': u'"/etc/bind/db.special"', u'type': u'master'}}, |
688 | - config) |
689 | + parsed) |
lgtm! After a quick look!