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