Merge lp:~mpontillo/maas/migrate-dns-settings-trunk into lp:maas/trunk

Proposed by Mike Pontillo on 2015-07-02
Status: Merged
Approved by: Mike Pontillo on 2015-07-07
Approved revision: 4072
Merged at revision: 4077
Proposed branch: lp:~mpontillo/maas/migrate-dns-settings-trunk
Merge into: lp:maas/trunk
Prerequisite: lp:~mpontillo/maas/remove-iscpy-trunk
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-trunk
Reviewer Review Type Date Requested Status
Raphaël Badin (community) 2015-07-02 Approve on 2015-07-07
Review via email: mp+263610@code.launchpad.net

Commit message

Migrate DNS forwarders and dnssec-validation settings to MAAS database during upgrades.

To post a comment you must log in.
Mike Pontillo (mpontillo) wrote :

During testing, this exception was seen, but it's resolved by executing "sudo apt-get install -f". Need to determine if a proper "apt-get install" rather than "dpkg -i" will fix this problem. If not, need to possibly change when this migration occurs, since it must occur while the database is up, running, and configured.

(pastebin link in case this is unreadable: http://paste.ubuntu.com/11808482/)

Traceback (most recent call last):
  File "/usr/bin/django-admin", line 5, in <module>
    management.execute_from_command_line()
  File "/usr/lib/python2.7/dist-packages/django/core/management/__init__.py", line 399, in execute_from_command_line
    utility.execute()
  File "/usr/lib/python2.7/dist-packages/django/core/management/__init__.py", line 392, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/lib/python2.7/dist-packages/django/core/management/base.py", line 242, in run_from_argv
    self.execute(*args, **options.__dict__)
  File "/usr/lib/python2.7/dist-packages/django/core/management/base.py", line 285, in execute
    output = self.handle(*args, **options)
  File "/usr/lib/python2.7/dist-packages/django/core/management/base.py", line 415, in handle
    return self.handle_noargs(**options)
  File "/usr/lib/python2.7/dist-packages/south/management/commands/syncdb.py", line 90, in handle_noargs
    syncdb.Command().execute(**options)
  File "/usr/lib/python2.7/dist-packages/django/core/management/base.py", line 285, in execute
    output = self.handle(*args, **options)
  File "/usr/lib/python2.7/dist-packages/django/core/management/base.py", line 415, in handle
    return self.handle_noargs(**options)
  File "/usr/lib/python2.7/dist-packages/django/core/management/commands/syncdb.py", line 57, in handle_noargs
    cursor = connection.cursor()
  File "/usr/lib/python2.7/dist-packages/django/db/backends/__init__.py", line 159, in cursor
    cursor = util.CursorWrapper(self._cursor(), self)
  File "/usr/lib/python2.7/dist-packages/django/db/backends/__init__.py", line 129, in _cursor
    self.ensure_connection()
  File "/usr/lib/python2.7/dist-packages/django/db/backends/__init__.py", line 124, in ensure_connection
    self.connect()
  File "/usr/lib/python2.7/dist-packages/django/db/utils.py", line 99, in __exit__
    six.reraise(dj_exc_type, dj_exc_value, traceback)
  File "/usr/lib/python2.7/dist-packages/django/db/backends/__init__.py", line 124, in ensure_connection
    self.connect()
  File "/usr/lib/python2.7/dist-packages/django/db/backends/__init__.py", line 112, in connect
    self.connection = self.get_new_connection(conn_params)
  File "/usr/lib/python2.7/dist-packages/django/db/backends/postgresql_psycopg2/base.py", line 116, in get_new_connection
    return Database.connect(**conn_params)
  File "/usr/lib/python2.7/dist-packages/psycopg2/__init__.py", line 179, in connect
    connection_factory=connection_factory, async=async)
django.db.utils.OperationalError: FATAL: password authentication failed for user "maas"
FATAL: password authentication failed for user "maas"

Christian Reis (kiko) wrote :

Could you add some test code with a few different variations of input to
see how well the importer copes?

Mike Pontillo (mpontillo) wrote :

kiko, see the prerequisite branch for the parser test code:

https://code.launchpad.net/~mpontillo/maas/remove-iscpy-trunk/+merge/263549

In this MP, I'm specifically testing using the named.options.conf in the bug, among other things.

HOWEVER, as you imply, that doesn't necessarily cover negative test cases, such as if the user has invalid forwarders in their configuration. (which this code will happily migrate)

I am *a little* concerned about adding invalid forwarders, except for the fact that this could only happen if the user tries to migrate an already-broken config. For example, if I have:

forwarders {
    something-that-is-not-an-ip-address;
};

In that case, we'll happily migrate the "something that isn't an IP address" to the MAAS configuration.

But again, in this case, the users' DNS config would have already been broken anyway. GIGO.

Mike Pontillo (mpontillo) wrote :

Actually, I'm marking this "Work in progress", because I do need to add additional tests for the migration functionality. Thanks kiko. =)

Raphaël Badin (rvb) wrote :

Looks good. Marking "needs fixing" because you're missing a bunch of tests. I have a bunch of non-blocker remarks as well.

review: Needs Fixing
4063. By Mike Pontillo on 2015-07-07

Merge trunk

4064. By Mike Pontillo on 2015-07-07

Make ISC parsing code order-preserving; address some review comments.

4065. By Mike Pontillo on 2015-07-07

Fix formatting

4066. By Mike Pontillo on 2015-07-07

Preserve ordering in forwarders; add test for order-preserving behavior

4067. By Mike Pontillo on 2015-07-07

Fix formatting

Raphaël Badin (rvb) wrote :

Looks good now, thanks for the changes. As discussed, let's just make sure we test the ordering of the IP addresses and add a test for the ISCException thingy.

review: Approve
4068. By Mike Pontillo on 2015-07-07

Remove ue of set() in edit_named_options, fix tests

4069. By Mike Pontillo on 2015-07-07

Add test case to verify that unmatched brackets throw ISCParseException

4070. By Mike Pontillo on 2015-07-07

Add check for additional parse error corner case

4071. By Mike Pontillo on 2015-07-07

Fix test name, and copy/paste 'typos'

4072. By Mike Pontillo on 2015-07-07

Fix broken tests

Andres Rodriguez (andreserl) wrote :

Also, please let's get the backports for this.
Thanks

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'src/maasserver/management/commands/edit_named_options.py'
--- src/maasserver/management/commands/edit_named_options.py 2015-07-01 17:06:29 +0000
+++ src/maasserver/management/commands/edit_named_options.py 2015-07-07 22:41:15 +0000
@@ -11,6 +11,8 @@
11 print_function,11 print_function,
12 unicode_literals,12 unicode_literals,
13 )13 )
14from collections import OrderedDict
15
1416
15str = None17str = None
1618
@@ -19,16 +21,20 @@
19 'Command',21 'Command',
20 ]22 ]
2123
24from copy import deepcopy
22from datetime import datetime25from datetime import datetime
23from optparse import make_option26from optparse import make_option
24import os27import os
25import shutil28import shutil
29import sys
2630
27from django.core.management.base import (31from django.core.management.base import (
28 BaseCommand,32 BaseCommand,
29 CommandError,33 CommandError,
30)34)
35from maasserver.models import Config
31from maasserver.utils.isc import (36from maasserver.utils.isc import (
37 ISCParseException,
32 make_isc_string,38 make_isc_string,
33 parse_isc_string,39 parse_isc_string,
34)40)
@@ -42,6 +48,24 @@
42 '--config-path', dest='config_path',48 '--config-path', dest='config_path',
43 default="/etc/bind/named.conf.options",49 default="/etc/bind/named.conf.options",
44 help="Specify the configuration file to edit."),50 help="Specify the configuration file to edit."),
51 make_option(
52 '--dry-run', dest='dry_run',
53 default=False, action='store_true',
54 help="Do not edit any configuration; instead, print to stdout the "
55 "actions that would be performed, and/or the new "
56 "configuration that would be written."),
57 make_option(
58 '--force', dest='force',
59 default=False, action='store_true',
60 help="Force the BIND configuration to be written, even if it "
61 "appears as though nothing has changed."),
62 make_option(
63 '--migrate-conflicting-options', default=False,
64 dest='migrate_conflicting_options', action='store_true',
65 help="Causes any options that conflict with MAAS-managed options "
66 "to be deleted from the BIND configuration and moved to the "
67 "MAAS-managed configuration. Requires the MAAS database to "
68 "be configured and running."),
45 )69 )
46 help = (70 help = (
47 "Edit the named.conf.options file so that it includes the "71 "Edit the named.conf.options file so that it includes the "
@@ -67,13 +91,12 @@
67 """91 """
68 try:92 try:
69 config_dict = parse_isc_string(options_file)93 config_dict = parse_isc_string(options_file)
70 except Exception as e:94 except ISCParseException as e:
71 # Yes, it throws bare exceptions :(
72 raise CommandError("Failed to parse %s: %s" % (95 raise CommandError("Failed to parse %s: %s" % (
73 config_path, e.message))96 config_path, e.message))
74 options_block = config_dict.get("options", None)97 options_block = config_dict.get("options", None)
75 if options_block is None:98 if options_block is None:
76 # Something is horribly wrong with the file, bail out rather99 # Something is horribly wrong with the file; bail out rather
77 # than doing anything drastic.100 # than doing anything drastic.
78 raise CommandError(101 raise CommandError(
79 "Can't find options {} block in %s, bailing out without "102 "Can't find options {} block in %s, bailing out without "
@@ -86,24 +109,65 @@
86 options_block['include'] = '"%s%s%s"' % (109 options_block['include'] = '"%s%s%s"' % (
87 dir, os.path.sep, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)110 dir, os.path.sep, MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
88111
89 def remove_forwarders(self, options_block):112 def migrate_forwarders(self, options_block, dry_run, stdout):
90 """Remove existing forwarders from the options block.113 """Remove existing forwarders from the options block.
91114
92 It's a syntax error to have more than one in the combined115 It's a syntax error to have more than one in the combined
93 configuration for named so we just remove whatever was there.116 configuration for named, so we just remove whatever was there.
94 There is no data loss due to the backup file made later.117
118 Migrate any forwarders in the configuration file to the MAAS config.
95 """119 """
96 if 'forwarders' in options_block:120 if 'forwarders' in options_block:
121 bind_forwarders = options_block['forwarders']
122
123 if not dry_run:
124 config, created = Config.objects.get_or_create(
125 name='upstream_dns',
126 defaults={'value': ' '.join(bind_forwarders)})
127 if not created:
128 # A configuration value already exists, so add the
129 # additional values we found in the configuration file to
130 # MAAS.
131 if config.value is None:
132 config.value = ''
133 maas_forwarders = OrderedDict.fromkeys(
134 config.value.split())
135 maas_forwarders.update(bind_forwarders)
136 config.value = ' '.join(maas_forwarders)
137 config.save()
138 else:
139 stdout.write(
140 "// Append to MAAS forwarders: %s\n"
141 % ' '.join(bind_forwarders))
142
97 del options_block['forwarders']143 del options_block['forwarders']
98144
99 def remove_dnssec_validation(self, options_block):145 def migrate_dnssec_validation(self, options_block, dry_run, stdout):
100 """Remove existing dnssec-validation from the options block.146 """Remove existing dnssec-validation from the options block.
101147
102 It's a syntax error to have more than one in the combined148 It's a syntax error to have more than one in the combined
103 configuration for named so we just remove whatever was there.149 configuration for named so we just remove whatever was there.
104 There is no data loss due to the backup file made later.150 There is no data loss due to the backup file made later.
151
152 Migrate this value in the configuration file to the MAAS config.
105 """153 """
106 if 'dnssec-validation' in options_block:154 if 'dnssec-validation' in options_block:
155 dnssec_validation = options_block['dnssec-validation']
156
157 if not dry_run:
158 config, created = Config.objects.get_or_create(
159 name='dnssec_validation',
160 defaults={'value': dnssec_validation})
161 if not created:
162 # Update the MAAS configuration to reflect the new setting
163 # found in the configuration file.
164 config.value = dnssec_validation
165 config.save()
166 else:
167 stdout.write(
168 "// Set MAAS dnssec_validation to: %s\n"
169 % dnssec_validation)
170
107 del options_block['dnssec-validation']171 del options_block['dnssec-validation']
108172
109 def back_up_existing_file(self, config_path):173 def back_up_existing_file(self, config_path):
@@ -115,22 +179,60 @@
115 raise CommandError(179 raise CommandError(
116 "Failed to make a backup of %s, exiting: %s" % (180 "Failed to make a backup of %s, exiting: %s" % (
117 config_path, e.message))181 config_path, e.message))
182 return backup_destination
183
184 def write_new_named_conf_options(self, fd, backup_filename, new_content):
185 fd.write("""\
186//
187// This file is managed by MAAS. Although MAAS attempts to preserve changes
188// made here, it is possible to create conflicts that MAAS can not resolve.
189//
190// DNS settings available in MAAS (for example, forwarders and
191// dnssec-validation) should be managed only in MAAS.
192//
193// The previous configuration file was backed up at:
194// %s
195//
196""" % backup_filename)
197 fd.write(new_content)
198 fd.write("\n")
118199
119 def handle(self, *args, **options):200 def handle(self, *args, **options):
120 """Entry point for BaseCommand."""201 """Entry point for BaseCommand."""
121 # Read stuff in, validate.202 # Read stuff in, validate.
122 config_path = options.get('config_path')203 config_path = options.get('config_path')
204 dry_run = options.get('dry_run')
205 force = options.get('force')
206 stdout = options.get('stdout')
207 if stdout is None:
208 stdout = sys.stdout
209 migrate_conflicting_options = options.get(
210 'migrate_conflicting_options')
211
123 options_file = self.read_file(config_path)212 options_file = self.read_file(config_path)
124 config_dict = self.parse_file(config_path, options_file)213 config_dict = self.parse_file(config_path, options_file)
214 original_config = deepcopy(config_dict)
215
125 options_block = config_dict['options']216 options_block = config_dict['options']
126217
127 # Modify the config.218 # Modify the configuration (if necessary).
128 self.set_up_include_statement(options_block, config_path)219 self.set_up_include_statement(options_block, config_path)
129 self.remove_forwarders(options_block)220
130 self.remove_dnssec_validation(options_block)221 if migrate_conflicting_options:
222 self.migrate_forwarders(options_block, dry_run, stdout)
223 self.migrate_dnssec_validation(options_block, dry_run, stdout)
224
225 # Re-parse the new configuration, so we can detect any changes.
131 new_content = make_isc_string(config_dict)226 new_content = make_isc_string(config_dict)
227 new_config = parse_isc_string(new_content)
132228
133 # Back up and write new file.229 if original_config != new_config or force:
134 self.back_up_existing_file(config_path)230 # The configuration has changed. Back up and write new file.
135 with open(config_path, "wb") as fd:231 if dry_run:
136 fd.write(new_content)232 self.write_new_named_conf_options(
233 stdout, config_path, new_content)
234 else:
235 backup_filename = self.back_up_existing_file(config_path)
236 with open(config_path, "wb") as fd:
237 self.write_new_named_conf_options(
238 fd, backup_filename, new_content)
137239
=== modified file 'src/maasserver/tests/test_commands_edit_named_options.py'
--- src/maasserver/tests/test_commands_edit_named_options.py 2015-05-07 18:14:38 +0000
+++ src/maasserver/tests/test_commands_edit_named_options.py 2015-07-07 22:41:15 +0000
@@ -15,6 +15,7 @@
15__all__ = []15__all__ = []
1616
17from codecs import getwriter17from codecs import getwriter
18from collections import OrderedDict
18from io import BytesIO19from io import BytesIO
19import os20import os
20import shutil21import shutil
@@ -22,11 +23,22 @@
2223
23from django.core.management import call_command24from django.core.management import call_command
24from django.core.management.base import CommandError25from django.core.management.base import CommandError
26from maasserver.management.commands.edit_named_options import (
27 Command as command_module,
28)
29from maasserver.models import Config
25from maasserver.testing.factory import factory30from maasserver.testing.factory import factory
26from maasserver.testing.testcase import MAASServerTestCase31from maasserver.testing.testcase import MAASServerTestCase
32from maasserver.utils import get_one
33from maasserver.utils.isc import (
34 make_isc_string,
35 parse_isc_string,
36 read_isc_file,
37)
27from provisioningserver.dns.config import MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME38from provisioningserver.dns.config import MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME
28from testtools.matchers import (39from testtools.matchers import (
29 Contains,40 Contains,
41 Equals,
30 FileContains,42 FileContains,
31 Not,43 Not,
32)44)
@@ -52,7 +64,27 @@
52OPTIONS_FILE_WITH_FORWARDERS = textwrap.dedent("""\64OPTIONS_FILE_WITH_FORWARDERS = textwrap.dedent("""\
53 options {65 options {
54 directory "/var/cache/bind";66 directory "/var/cache/bind";
55 forwarders { 192.168.1.1; };67 forwarders { 192.168.1.1; 192.168.1.2; };
68 auth-nxdomain no; # conform to RFC1035
69 listen-on-v6 { any; };
70 };
71""")
72
73OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC = textwrap.dedent("""\
74 options {
75 directory "/var/cache/bind";
76 forwarders { 192.168.1.1; 192.168.1.2; };
77 dnssec-validation no;
78 auth-nxdomain no; # conform to RFC1035
79 listen-on-v6 { any; };
80 };
81""")
82
83OPTIONS_FILE_WITH_EXTRA_AND_DUP_FORWARDER = textwrap.dedent("""\
84 options {
85 directory "/var/cache/bind";
86 forwarders { 192.168.1.2; 192.168.1.3; };
87 dnssec-validation no;
56 auth-nxdomain no; # conform to RFC103588 auth-nxdomain no; # conform to RFC1035
57 listen-on-v6 { any; };89 listen-on-v6 { any; };
58 };90 };
@@ -99,18 +131,27 @@
99 self.assertContentFailsWithMessage(131 self.assertContentFailsWithMessage(
100 OPTIONS_FILE, "Failed to make a backup")132 OPTIONS_FILE, "Failed to make a backup")
101133
102 def test_removes_existing_forwarders_config(self):134 def test_does_not_remove_existing_forwarders_config(self):
103 options_file = self.make_file(contents=OPTIONS_FILE_WITH_FORWARDERS)135 options_file = self.make_file(contents=OPTIONS_FILE_WITH_FORWARDERS)
104 call_command(136 call_command(
105 "edit_named_options", config_path=options_file,137 "edit_named_options", config_path=options_file,
106 stdout=self.stdout)138 stdout=self.stdout)
107139
140 options = read_isc_file(options_file)
141 self.assertThat(make_isc_string(options), Contains('forwarders'))
142
143 def test_removes_existing_forwarders_config_if_migrate_set(self):
144 options_file = self.make_file(contents=OPTIONS_FILE_WITH_FORWARDERS)
145 call_command(
146 "edit_named_options", config_path=options_file,
147 migrate_conflicting_options=True, stdout=self.stdout)
148
108 # Check that the file was re-written without forwarders (since149 # Check that the file was re-written without forwarders (since
109 # that's now in the included file).150 # that's now in the included file).
151 options = read_isc_file(options_file)
110 self.assertThat(152 self.assertThat(
111 options_file,153 make_isc_string(options),
112 Not(FileContains(154 Not(Contains('forwarders')))
113 matcher=Contains('forwarders'))))
114155
115 def test_removes_existing_dnssec_validation_config(self):156 def test_removes_existing_dnssec_validation_config(self):
116 options_file = self.make_file(contents=OPTIONS_FILE_WITH_DNSSEC)157 options_file = self.make_file(contents=OPTIONS_FILE_WITH_DNSSEC)
@@ -118,12 +159,23 @@
118 "edit_named_options", config_path=options_file,159 "edit_named_options", config_path=options_file,
119 stdout=self.stdout)160 stdout=self.stdout)
120161
121 # Check that the file was re-written without forwarders (since162 # Check that the file was re-written without dnssec-validation (since
122 # that's now in the included file).163 # that's now in the included file).
123 self.assertThat(164 options = read_isc_file(options_file)
124 options_file,165 self.assertThat(
125 Not(FileContains(166 make_isc_string(options), Contains('dnssec-validation'))
126 matcher=Contains('dnssec-validation'))))167
168 def test_removes_existing_dnssec_validation_config_if_migration_set(self):
169 options_file = self.make_file(contents=OPTIONS_FILE_WITH_DNSSEC)
170 call_command(
171 "edit_named_options", config_path=options_file,
172 migrate_conflicting_options=True, stdout=self.stdout)
173
174 # Check that the file was re-written without dnssec-validation (since
175 # that's now in the included file).
176 options = read_isc_file(options_file)
177 self.assertThat(
178 make_isc_string(options), Not(Contains('dnssec-validation')))
127179
128 def test_normal_operation(self):180 def test_normal_operation(self):
129 options_file = self.make_file(contents=OPTIONS_FILE)181 options_file = self.make_file(contents=OPTIONS_FILE)
@@ -136,11 +188,10 @@
136 MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)188 MAAS_NAMED_CONF_OPTIONS_INSIDE_NAME)
137189
138 # Check that the file was re-written with the include statement.190 # Check that the file was re-written with the include statement.
191 options = read_isc_file(options_file)
139 self.assertThat(192 self.assertThat(
140 options_file,193 make_isc_string(options),
141 FileContains(194 Contains('include "%s";' % expected_path))
142 matcher=Contains(
143 'include "%s";' % expected_path)))
144195
145 # Check that the backup was made.196 # Check that the backup was made.
146 options_file_base = os.path.dirname(options_file)197 options_file_base = os.path.dirname(options_file)
@@ -150,3 +201,106 @@
150 [backup_file] = files201 [backup_file] = files
151 backup_file = os.path.join(options_file_base, backup_file)202 backup_file = os.path.join(options_file_base, backup_file)
152 self.assertThat(backup_file, FileContains(OPTIONS_FILE))203 self.assertThat(backup_file, FileContains(OPTIONS_FILE))
204
205 def test_migrates_bind_config_to_database(self):
206 options_file = self.make_file(
207 contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC)
208 call_command(
209 "edit_named_options", config_path=options_file,
210 migrate_conflicting_options=True, stdout=self.stdout)
211
212 upstream_dns = get_one(Config.objects.filter(name="upstream_dns"))
213 self.assertThat({'192.168.1.1', '192.168.1.2'},
214 Equals(set(upstream_dns.value.split())))
215
216 dnssec_validation = get_one(Config.objects.filter(
217 name="dnssec_validation"))
218 self.assertThat('no', Equals(dnssec_validation.value))
219
220 def test_migrate_combines_with_existing_forwarders(self):
221 options_file = self.make_file(
222 contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC)
223 call_command(
224 "edit_named_options", config_path=options_file,
225 migrate_conflicting_options=True, stdout=self.stdout)
226
227 upstream_dns = get_one(Config.objects.filter(name="upstream_dns"))
228 self.assertThat(OrderedDict.fromkeys(['192.168.1.1', '192.168.1.2']),
229 Equals(OrderedDict.fromkeys(
230 upstream_dns.value.split())))
231
232 dnssec_validation = get_one(Config.objects.filter(
233 name="dnssec_validation"))
234 self.assertThat('no', Equals(dnssec_validation.value))
235
236 options_file = self.make_file(
237 contents=OPTIONS_FILE_WITH_EXTRA_AND_DUP_FORWARDER)
238
239 call_command(
240 "edit_named_options", config_path=options_file,
241 migrate_conflicting_options=True, stdout=self.stdout)
242
243 upstream_dns = get_one(Config.objects.filter(name="upstream_dns"))
244 self.assertThat(
245 OrderedDict.fromkeys(
246 ['192.168.1.1', '192.168.1.2', '192.168.1.3']),
247 Equals(OrderedDict.fromkeys(upstream_dns.value.split())))
248
249 def test_dry_run_migrates_nothing_and_prints_config(self):
250 options_file = self.make_file(
251 contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC)
252 call_command(
253 "edit_named_options", config_path=options_file,
254 migrate_conflicting_options=True, dry_run=True, stdout=self.stdout)
255
256 upstream_dns = get_one(Config.objects.filter(name="upstream_dns"))
257 self.assertIsNone(upstream_dns)
258 dnssec_validation = get_one(Config.objects.filter(
259 name="dnssec_validation"))
260 self.assertIsNone(dnssec_validation)
261
262 # Check that a proper configuration was written to stdout.
263 config = parse_isc_string(self.stdout.getvalue())
264 self.assertIsNotNone(config)
265
266 def test_repeat_migrations_migrate_nothing(self):
267 options_file = self.make_file(
268 contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC)
269 backup_mock = self.patch(command_module, "back_up_existing_file")
270
271 call_command(
272 "edit_named_options", config_path=options_file,
273 migrate_conflicting_options=True, stdout=self.stdout)
274
275 self.assertTrue(backup_mock.called)
276 backup_mock.reset_mock()
277
278 write_mock = self.patch(command_module, "write_new_named_conf_options")
279
280 call_command(
281 "edit_named_options", config_path=options_file,
282 migrate_conflicting_options=True, stdout=self.stdout)
283
284 self.assertFalse(backup_mock.called)
285 self.assertFalse(write_mock.called)
286
287 def test_repeat_forced_migrations_write_file_anyway(self):
288 options_file = self.make_file(
289 contents=OPTIONS_FILE_WITH_FORWARDERS_AND_DNSSEC)
290 backup_mock = self.patch(command_module, "back_up_existing_file")
291
292 call_command(
293 "edit_named_options", config_path=options_file,
294 migrate_conflicting_options=True, stdout=self.stdout)
295
296 self.assertTrue(backup_mock.called)
297 backup_mock.reset_mock()
298
299 write_mock = self.patch(command_module, "write_new_named_conf_options")
300
301 call_command(
302 "edit_named_options", config_path=options_file,
303 migrate_conflicting_options=True, force=True, stdout=self.stdout)
304
305 self.assertTrue(backup_mock.called)
306 self.assertTrue(write_mock.called)
153307
=== modified file 'src/maasserver/utils/isc.py'
--- src/maasserver/utils/isc.py 2015-07-01 17:06:29 +0000
+++ src/maasserver/utils/isc.py 2015-07-07 22:41:15 +0000
@@ -33,18 +33,26 @@
33 print_function,33 print_function,
34 unicode_literals,34 unicode_literals,
35)35)
36from collections import OrderedDict
37
3638
37str = None39str = None
3840
39__metaclass__ = type41__metaclass__ = type
40__all__ = [42__all__ = [
43 'ISCParseException',
41 'make_isc_string',44 'make_isc_string',
42 'parse_isc_string',45 'parse_isc_string',
46 'read_isc_file',
43]47]
4448
45import copy49import copy
4650
4751
52class ISCParseException(Exception):
53 """Thrown when an ISC string cannot be parsed."""
54
55
48def _clip(char_list):56def _clip(char_list):
49 """Clips char_list to individual stanza.57 """Clips char_list to individual stanza.
5058
@@ -64,7 +72,7 @@
64 return index, char_list[:index]72 return index, char_list[:index]
65 elif item == '}':73 elif item == '}':
66 skip -= 174 skip -= 1
67 raise Exception("Invalid brackets.")75 raise ISCParseException("Invalid brackets.")
6876
6977
70def _parse_tokens(char_list):78def _parse_tokens(char_list):
@@ -86,11 +94,11 @@
86 {'10.1.0/32': True, '10.1.1/32': True}}}94 {'10.1.0/32': True, '10.1.1/32': True}}}
87 """95 """
88 index = 096 index = 0
89 dictionary_fragment = {}97 dictionary_fragment = OrderedDict()
90 new_char_list = copy.deepcopy(char_list)98 new_char_list = copy.deepcopy(char_list)
91 if type(new_char_list) == str:99 if type(new_char_list) == str:
92 return new_char_list100 return new_char_list
93 if type(new_char_list) == dict:101 if type(new_char_list) == OrderedDict:
94 return new_char_list102 return new_char_list
95 last_open = None103 last_open = None
96 continuous_line = False104 continuous_line = False
@@ -146,6 +154,8 @@
146 # fine)154 # fine)
147 elif new_char_list[index] not in ['{', ';', '}']:155 elif new_char_list[index] not in ['{', ';', '}']:
148 key = new_char_list[index]156 key = new_char_list[index]
157 if type(dictionary_fragment) == list:
158 raise ISCParseException("Dictionary expected; got a list")
149 dictionary_fragment[key] = ''159 dictionary_fragment[key] = ''
150 index += 1160 index += 1
151 index += 1161 index += 1
@@ -277,7 +287,18 @@
277 new_list[-1] = '%s%s' % (new_list[-1], terminator)287 new_list[-1] = '%s%s' % (new_list[-1], terminator)
278 isc_list.append(288 isc_list.append(
279 '%s { %s }%s' % (option, ' '.join(new_list), terminator))289 '%s { %s }%s' % (option, ' '.join(new_list), terminator))
280 elif type(isc_dict[option]) == dict:290 elif (type(isc_dict[option]) == OrderedDict or
291 type(isc_dict[option]) == dict):
281 isc_list.append('%s { %s }%s' % (292 isc_list.append('%s { %s }%s' % (
282 option, make_isc_string(isc_dict[option]), terminator))293 option, make_isc_string(isc_dict[option]), terminator))
283 return '\n'.join(isc_list)294 return '\n'.join(isc_list)
295
296
297def read_isc_file(isc_file):
298 """Given the specified filename, parses it to create a dictionary.
299
300 :param:isc_file: the filename to read
301 :return:dict: dictionary of ISC file representation
302 """
303 with open(isc_file, "r") as f:
304 return parse_isc_string(f.read())
284305
=== modified file 'src/maasserver/utils/tests/test_isc.py'
--- src/maasserver/utils/tests/test_isc.py 2015-07-03 21:19:59 +0000
+++ src/maasserver/utils/tests/test_isc.py 2015-07-07 22:41:15 +0000
@@ -9,19 +9,23 @@
9 unicode_literals,9 unicode_literals,
10)10)
1111
12
12str = None13str = None
1314
14__metaclass__ = type15__metaclass__ = type
15__all__ = []16__all__ = []
1617
1718from collections import OrderedDict
18from textwrap import dedent19from textwrap import dedent
1920
20from maasserver.utils.isc import (21from maasserver.utils.isc import (
22 ISCParseException,
21 make_isc_string,23 make_isc_string,
22 parse_isc_string,24 parse_isc_string,
25 read_isc_file,
23)26)
24from maastesting.testcase import MAASTestCase27from maastesting.testcase import MAASTestCase
28from testtools import ExpectedException
2529
2630
27class TestParseISCString(MAASTestCase):31class TestParseISCString(MAASTestCase):
@@ -39,10 +43,11 @@
39 """)43 """)
40 options = parse_isc_string(testdata)44 options = parse_isc_string(testdata)
41 self.assertEqual(45 self.assertEqual(
42 {u'options': {u'auth-nxdomain': u'no',46 OrderedDict({u'options': OrderedDict({u'auth-nxdomain': u'no',
43 u'directory': u'"/var/cache/bind"',47 u'directory': u'"/var/cache/bind"',
44 u'dnssec-validation': u'auto',48 u'dnssec-validation': u'auto',
45 u'listen-on-v6': {u'any': True}}}, options)49 u'listen-on-v6': OrderedDict({u'any': True})})}),
50 options)
4651
47 def test_parses_bind_acl(self):52 def test_parses_bind_acl(self):
48 testdata = dedent("""\53 testdata = dedent("""\
@@ -159,6 +164,94 @@
159 config_string = make_isc_string(config)164 config_string = make_isc_string(config)
160 config = parse_isc_string(config_string)165 config = parse_isc_string(config_string)
161 self.assertEqual(166 self.assertEqual(
167 OrderedDict(
168 [(u'acl canonical-int-ns',
169 OrderedDict(
170 [(u'91.189.90.151', True), (u'91.189.89.192', True)])),
171 (u'options', OrderedDict(
172 [(u'directory', u'"/var/cache/bind"'),
173 (u'forwarders', OrderedDict(
174 [(u'91.189.94.2', True)])),
175 (u'dnssec-validation', u'auto'),
176 (u'auth-nxdomain', u'no'),
177 (u'listen-on-v6', OrderedDict([(u'any', True)])),
178 (u'allow-query', OrderedDict([(u'any', True)])),
179 (u'allow-transfer', OrderedDict(
180 [(u'10.222.64.1', True),
181 (u'canonical-int-ns', True)])),
182 (u'notify', u'explicit'),
183 (u'also-notify', OrderedDict(
184 [(u'91.189.90.151', True),
185 (u'91.189.89.192', True)])),
186 (u'allow-query-cache', OrderedDict(
187 [(u'10.222.64.0/18', True)])),
188 (u'recursion', u'yes')])),
189 (u'zone "."', OrderedDict(
190 [(u'type', u'master'),
191 (u'file', u'"/etc/bind/db.special"')]))]),
192 config)
193
194 def test_parser_preserves_order(self):
195 testdata = dedent("""\
196 forwarders {
197 9.9.9.9;
198 8.8.8.8;
199 7.7.7.7;
200 6.6.6.6;
201 5.5.5.5;
202 4.4.4.4;
203 3.3.3.3;
204 2.2.2.2;
205 1.1.1.1;
206 };
207 """)
208 forwarders = parse_isc_string(testdata)
209 self.assertEqual(OrderedDict([(u'forwarders', OrderedDict(
210 [(u'9.9.9.9', True), (u'8.8.8.8', True), (u'7.7.7.7', True),
211 (u'6.6.6.6', True), (u'5.5.5.5', True), (u'4.4.4.4', True),
212 (u'3.3.3.3', True), (u'2.2.2.2', True), (u'1.1.1.1', True)]))]),
213 forwarders)
214
215 def test_parse_unmatched_brackets_throws_iscparseexception(self):
216 with ExpectedException(ISCParseException):
217 parse_isc_string("forwarders {")
218
219 def test_parse_malformed_list_throws_iscparseexception(self):
220 with ExpectedException(ISCParseException):
221 parse_isc_string("forwarders {{}a;;b}")
222
223 def test_read_isc_file(self):
224 testdata = dedent("""\
225 acl canonical-int-ns { 91.189.90.151; 91.189.89.192; };
226
227 options {
228 directory "/var/cache/bind";
229
230 forwarders {
231 91.189.94.2;
232 91.189.94.2;
233 };
234
235 dnssec-validation auto;
236
237 auth-nxdomain no; # conform to RFC1035
238 listen-on-v6 { any; };
239
240 allow-query { any; };
241 allow-transfer { 10.222.64.1; canonical-int-ns; };
242
243 notify explicit;
244 also-notify { 91.189.90.151; 91.189.89.192; };
245
246 allow-query-cache { 10.222.64.0/18; };
247 recursion yes;
248 };
249
250 zone "." { type master; file "/etc/bind/db.special"; };
251 """)
252 testfile = self.make_file(contents=testdata)
253 parsed = read_isc_file(testfile)
254 self.assertEqual(
162 {u'acl canonical-int-ns':255 {u'acl canonical-int-ns':
163 {u'91.189.89.192': True, u'91.189.90.151': True},256 {u'91.189.89.192': True, u'91.189.90.151': True},
164 u'options': {u'allow-query': {u'any': True},257 u'options': {u'allow-query': {u'any': True},
@@ -176,4 +269,4 @@
176 u'recursion': u'yes'},269 u'recursion': u'yes'},
177 u'zone "."':270 u'zone "."':
178 {u'file': u'"/etc/bind/db.special"', u'type': u'master'}},271 {u'file': u'"/etc/bind/db.special"', u'type': u'master'}},
179 config)272 parsed)