Merge lp:~rvb/maas/bug-1081701-c into lp:~maas-committers/maas/trunk
- bug-1081701-c
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Raphaël Badin | ||||
Approved revision: | no longer in the source branch. | ||||
Merged at revision: | 1376 | ||||
Proposed branch: | lp:~rvb/maas/bug-1081701-c | ||||
Merge into: | lp:~maas-committers/maas/trunk | ||||
Diff against target: |
844 lines (+280/-155) 13 files modified
src/maasserver/api.py (+7/-5) src/maasserver/compose_preseed.py (+9/-6) src/maasserver/fields.py (+1/-25) src/maasserver/preseed.py (+59/-32) src/maasserver/testing/factory.py (+2/-1) src/maasserver/tests/test_api.py (+32/-0) src/maasserver/tests/test_fields.py (+0/-47) src/maasserver/tests/test_forms.py (+0/-8) src/maasserver/tests/test_preseed.py (+49/-17) src/maasserver/utils/__init__.py (+30/-3) src/maasserver/utils/tests/test_utils.py (+70/-9) src/metadataserver/api.py (+5/-1) src/metadataserver/tests/test_api.py (+16/-1) |
||||
To merge this branch: | bzr merge lp:~rvb/maas/bug-1081701-c | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Gavin Panella (community) | Approve | ||
Review via email: mp+135915@code.launchpad.net |
Commit message
When generating a preseed, use the value of the field 'maas_url' from the nodegroup (either the one related to the node for which the preseed is being generated or the one derived from the originating IP) to generate the urls present in the preseed.
Description of the change
This is the part c) of the action plan described in https:/
= Notes =
First off, don't be scared by the size of the diff, there is lots of moving things around.
- I've refactored src/maasserver/
- I've changed absolute_reverse to be able to use an alternative base (i.e., not the default one: DEFAULT_MAAS_URL).
- I split some of the method in preseed.py because the enlistment related methods need the base_url to be passed around and the other preseed-generating methods (those that operate on nodes) can extract that information from node.nodegroup,
- Note that I really want to do "if not base_url:" because base_url can be '' or None.
Drive-by fix:
- I've fixed the raw query inside 'find_nodegroup' to use proper escaping (instead of using string interpolation to build the sql query).
Preview Diff
1 | === modified file 'src/maasserver/api.py' |
2 | --- src/maasserver/api.py 2012-11-23 16:33:06 +0000 |
3 | +++ src/maasserver/api.py 2012-11-23 17:16:21 +0000 |
4 | @@ -172,7 +172,7 @@ |
5 | from maasserver.utils import ( |
6 | absolute_reverse, |
7 | build_absolute_uri, |
8 | - get_origin_ip, |
9 | + find_nodegroup, |
10 | map_enum, |
11 | strip_domain, |
12 | ) |
13 | @@ -400,9 +400,9 @@ |
14 | # If 'nodegroup' is not explicitely specified, get the origin of the |
15 | # request to figure out which nodegroup the new node should be |
16 | # attached to. |
17 | - origin_ip = get_origin_ip(request) |
18 | - if origin_ip is not None: |
19 | - altered_query_data['nodegroup'] = origin_ip |
20 | + nodegroup = find_nodegroup(request) |
21 | + if nodegroup is not None: |
22 | + altered_query_data['nodegroup'] = nodegroup |
23 | |
24 | Form = get_node_create_form(request.user) |
25 | form = Form(altered_query_data) |
26 | @@ -1656,7 +1656,9 @@ |
27 | # 1-1 mapping. |
28 | subarch = pxelinux_subarch |
29 | |
30 | - preseed_url = compose_enlistment_preseed_url() |
31 | + nodegroup = find_nodegroup(request) |
32 | + base_url = nodegroup.maas_url if nodegroup is not None else None |
33 | + preseed_url = compose_enlistment_preseed_url(base_url=base_url) |
34 | hostname = 'maas-enlist' |
35 | domain = Config.objects.get_config('enlistment_domain') |
36 | |
37 | |
38 | === modified file 'src/maasserver/compose_preseed.py' |
39 | --- src/maasserver/compose_preseed.py 2012-10-04 14:00:06 +0000 |
40 | +++ src/maasserver/compose_preseed.py 2012-11-23 17:16:21 +0000 |
41 | @@ -21,7 +21,7 @@ |
42 | import yaml |
43 | |
44 | |
45 | -def compose_cloud_init_preseed(token): |
46 | +def compose_cloud_init_preseed(token, base_url=''): |
47 | """Compose the preseed value for a node in any state but Commissioning.""" |
48 | credentials = urlencode({ |
49 | 'oauth_consumer_key': token.consumer.key, |
50 | @@ -40,7 +40,8 @@ |
51 | # ks_meta, and it gets fed straight into debconf. |
52 | preseed_items = [ |
53 | ('datasources', 'multiselect', 'MAAS'), |
54 | - ('maas-metadata-url', 'string', absolute_reverse('metadata')), |
55 | + ('maas-metadata-url', 'string', absolute_reverse( |
56 | + 'metadata', base_url=base_url)), |
57 | ('maas-metadata-credentials', 'string', credentials), |
58 | ('local-cloud-config', 'string', local_config) |
59 | ] |
60 | @@ -54,12 +55,13 @@ |
61 | for item_name, item_type, item_value in preseed_items) |
62 | |
63 | |
64 | -def compose_commissioning_preseed(token): |
65 | +def compose_commissioning_preseed(token, base_url=''): |
66 | """Compose the preseed value for a Commissioning node.""" |
67 | return "#cloud-config\n%s" % yaml.safe_dump({ |
68 | 'datasource': { |
69 | 'MAAS': { |
70 | - 'metadata_url': absolute_reverse('metadata'), |
71 | + 'metadata_url': absolute_reverse( |
72 | + 'metadata', base_url=base_url), |
73 | 'consumer_key': token.consumer.key, |
74 | 'token_key': token.key, |
75 | 'token_secret': token.secret, |
76 | @@ -86,7 +88,8 @@ |
77 | # Circular import. |
78 | from metadataserver.models import NodeKey |
79 | token = NodeKey.objects.get_token_for_node(node) |
80 | + base_url = node.nodegroup.maas_url |
81 | if node.status == NODE_STATUS.COMMISSIONING: |
82 | - return compose_commissioning_preseed(token) |
83 | + return compose_commissioning_preseed(token, base_url) |
84 | else: |
85 | - return compose_cloud_init_preseed(token) |
86 | + return compose_cloud_init_preseed(token, base_url) |
87 | |
88 | === modified file 'src/maasserver/fields.py' |
89 | --- src/maasserver/fields.py 2012-09-21 09:06:21 +0000 |
90 | +++ src/maasserver/fields.py 2012-11-23 17:16:21 +0000 |
91 | @@ -32,7 +32,6 @@ |
92 | ModelChoiceField, |
93 | RegexField, |
94 | ) |
95 | -from maasserver.utils.orm import get_one |
96 | import psycopg2.extensions |
97 | from south.modelsinspector import add_introspection_rules |
98 | |
99 | @@ -85,25 +84,6 @@ |
100 | else: |
101 | return "%s: %s" % (nodegroup.name, interface.ip) |
102 | |
103 | - def find_nodegroup(self, ip_address): |
104 | - """Find the nodegroup whose subnet contains `ip_address`. |
105 | - |
106 | - The matching nodegroup may have multiple interfaces on the subnet, |
107 | - but there can be only one matching nodegroup. |
108 | - """ |
109 | - # Avoid circular imports. |
110 | - from maasserver.models import NodeGroup |
111 | - |
112 | - return get_one(NodeGroup.objects.raw(""" |
113 | - SELECT * |
114 | - FROM maasserver_nodegroup |
115 | - WHERE id IN ( |
116 | - SELECT nodegroup_id |
117 | - FROM maasserver_nodegroupinterface |
118 | - WHERE (inet '%s' & subnet_mask) = (ip & subnet_mask) |
119 | - ) |
120 | - """ % ip_address)) |
121 | - |
122 | def clean(self, value): |
123 | """Django method: provide expected output for various inputs. |
124 | |
125 | @@ -126,11 +106,7 @@ |
126 | elif isinstance(value, bytes) and '.' not in value: |
127 | nodegroup_id = int(value) |
128 | else: |
129 | - nodegroup = self.find_nodegroup(value) |
130 | - if nodegroup is None: |
131 | - raise ValidationError( |
132 | - "No known subnet contains %s." % value) |
133 | - nodegroup_id = nodegroup.id |
134 | + raise ValidationError("Invalid nodegroup: %s." % value) |
135 | return super(NodeGroupFormField, self).clean(nodegroup_id) |
136 | |
137 | |
138 | |
139 | === modified file 'src/maasserver/preseed.py' |
140 | --- src/maasserver/preseed.py 2012-11-14 10:32:25 +0000 |
141 | +++ src/maasserver/preseed.py 2012-11-23 17:16:21 +0000 |
142 | @@ -38,22 +38,24 @@ |
143 | GENERIC_FILENAME = 'generic' |
144 | |
145 | |
146 | -def get_enlist_preseed(): |
147 | +def get_enlist_preseed(base_url=''): |
148 | """Return the enlistment preseed. |
149 | |
150 | :return: The rendered preseed string. |
151 | :rtype: basestring. |
152 | """ |
153 | - return render_preseed(None, PRESEED_TYPE.ENLIST) |
154 | - |
155 | - |
156 | -def get_enlist_userdata(): |
157 | + return render_enlistment_preseed( |
158 | + PRESEED_TYPE.ENLIST, base_url=base_url) |
159 | + |
160 | + |
161 | +def get_enlist_userdata(base_url=''): |
162 | """Return the enlistment preseed. |
163 | |
164 | :return: The rendered enlistment user-data string. |
165 | :rtype: basestring. |
166 | """ |
167 | - return render_preseed(None, PRESEED_TYPE.ENLIST_USERDATA) |
168 | + return render_enlistment_preseed( |
169 | + PRESEED_TYPE.ENLIST_USERDATA, base_url=base_url) |
170 | |
171 | |
172 | def get_preseed(node): |
173 | @@ -207,9 +209,9 @@ |
174 | return parsed_url.hostname, parsed_url.path |
175 | |
176 | |
177 | -def get_preseed_context(node, release=''): |
178 | - """Return the context dictionary to be used to render preseed templates |
179 | - for this node. |
180 | +def get_preseed_context(release='', base_url=''): |
181 | + """Return the node-independent context dictionary to be used to render |
182 | + preseed templates. |
183 | |
184 | :param node: See `get_preseed_filenames`. |
185 | :param prefix: See `get_preseed_filenames`. |
186 | @@ -222,36 +224,58 @@ |
187 | Config.objects.get_config('main_archive')) |
188 | ports_archive_hostname, ports_archive_directory = get_hostname_and_path( |
189 | Config.objects.get_config('ports_archive')) |
190 | - context = { |
191 | + return { |
192 | 'main_archive_hostname': main_archive_hostname, |
193 | 'main_archive_directory': main_archive_directory, |
194 | 'ports_archive_hostname': ports_archive_hostname, |
195 | 'ports_archive_directory': ports_archive_directory, |
196 | 'release': release, |
197 | 'server_host': server_host, |
198 | - 'server_url': absolute_reverse('nodes_handler'), |
199 | - 'metadata_enlist_url': absolute_reverse('enlist'), |
200 | + 'server_url': absolute_reverse('nodes_handler', base_url=base_url), |
201 | + 'metadata_enlist_url': absolute_reverse('enlist', base_url=base_url), |
202 | 'http_proxy': Config.objects.get_config('http_proxy'), |
203 | } |
204 | - if node is not None: |
205 | - # Create the url and the url-data (POST parameters) used to turn off |
206 | - # PXE booting once the install of the node is finished. |
207 | - node_disable_pxe_url = absolute_reverse( |
208 | - 'metadata-node-by-id', args=['latest', node.system_id]) |
209 | - node_disable_pxe_data = urlencode({'op': 'netboot_off'}) |
210 | - node_context = { |
211 | - 'node': node, |
212 | - 'preseed_data': compose_preseed(node), |
213 | - 'node_disable_pxe_url': node_disable_pxe_url, |
214 | - 'node_disable_pxe_data': node_disable_pxe_data, |
215 | - } |
216 | - context.update(node_context) |
217 | - |
218 | - return context |
219 | + |
220 | + |
221 | +def get_node_preseed_context(node, release=''): |
222 | + """Return the node-dependent context dictionary to be used to render |
223 | + preseed templates. |
224 | + |
225 | + :param node: See `get_preseed_filenames`. |
226 | + :param prefix: See `get_preseed_filenames`. |
227 | + :param release: See `get_preseed_filenames`. |
228 | + :return: The context dictionary. |
229 | + :rtype: dict. |
230 | + """ |
231 | + # Create the url and the url-data (POST parameters) used to turn off |
232 | + # PXE booting once the install of the node is finished. |
233 | + node_disable_pxe_url = absolute_reverse( |
234 | + 'metadata-node-by-id', args=['latest', node.system_id], |
235 | + base_url=node.nodegroup.maas_url) |
236 | + node_disable_pxe_data = urlencode({'op': 'netboot_off'}) |
237 | + return { |
238 | + 'node': node, |
239 | + 'preseed_data': compose_preseed(node), |
240 | + 'node_disable_pxe_url': node_disable_pxe_url, |
241 | + 'node_disable_pxe_data': node_disable_pxe_data, |
242 | + } |
243 | + |
244 | + |
245 | +def render_enlistment_preseed(prefix, release='', base_url=''): |
246 | + """Return the enlistment preseed. |
247 | + |
248 | + :param prefix: See `get_preseed_filenames`. |
249 | + :param release: See `get_preseed_filenames`. |
250 | + :return: The rendered preseed string. |
251 | + :rtype: basestring. |
252 | + """ |
253 | + template = load_preseed_template(None, prefix, release) |
254 | + context = get_preseed_context(release, base_url=base_url) |
255 | + return template.substitute(**context) |
256 | |
257 | |
258 | def render_preseed(node, prefix, release=''): |
259 | - """Find and load a `PreseedTemplate` for the given node. |
260 | + """Return the preseed for the given node. |
261 | |
262 | :param node: See `get_preseed_filenames`. |
263 | :param prefix: See `get_preseed_filenames`. |
264 | @@ -260,23 +284,26 @@ |
265 | :rtype: basestring. |
266 | """ |
267 | template = load_preseed_template(node, prefix, release) |
268 | - context = get_preseed_context(node, release) |
269 | + base_url = node.nodegroup.maas_url |
270 | + context = get_preseed_context(release, base_url=base_url) |
271 | + context.update(get_node_preseed_context(node, release)) |
272 | return template.substitute(**context) |
273 | |
274 | |
275 | -def compose_enlistment_preseed_url(): |
276 | +def compose_enlistment_preseed_url(base_url=''): |
277 | """Compose enlistment preseed URL.""" |
278 | # Always uses the latest version of the metadata API. |
279 | version = 'latest' |
280 | return absolute_reverse( |
281 | 'metadata-enlist-preseed', args=[version], |
282 | - query={'op': 'get_enlist_preseed'}) |
283 | + query={'op': 'get_enlist_preseed'}, base_url=base_url) |
284 | |
285 | |
286 | def compose_preseed_url(node): |
287 | """Compose a metadata URL for `node`'s preseed data.""" |
288 | # Always uses the latest version of the metadata API. |
289 | version = 'latest' |
290 | + base_url = node.nodegroup.maas_url |
291 | return absolute_reverse( |
292 | 'metadata-node-by-id', args=[version, node.system_id], |
293 | - query={'op': 'get_preseed'}) |
294 | + query={'op': 'get_preseed'}, base_url=base_url) |
295 | |
296 | === modified file 'src/maasserver/testing/factory.py' |
297 | --- src/maasserver/testing/factory.py 2012-11-23 14:27:34 +0000 |
298 | +++ src/maasserver/testing/factory.py 2012-11-23 17:16:21 +0000 |
299 | @@ -185,7 +185,7 @@ |
300 | router_ip=None, network=None, subnet_mask=None, |
301 | broadcast_ip=None, ip_range_low=None, |
302 | ip_range_high=None, interface=None, management=None, |
303 | - status=None, **kwargs): |
304 | + status=None, maas_url='', **kwargs): |
305 | """Create a :class:`NodeGroup`. |
306 | |
307 | If network (an instance of IPNetwork) is provided, use it to populate |
308 | @@ -210,6 +210,7 @@ |
309 | ng = NodeGroup.objects.new( |
310 | name=name, uuid=uuid, **interface_settings) |
311 | ng.status = status |
312 | + ng.maas_url = maas_url |
313 | ng.save() |
314 | return ng |
315 | |
316 | |
317 | === modified file 'src/maasserver/tests/test_api.py' |
318 | --- src/maasserver/tests/test_api.py 2012-11-23 14:34:54 +0000 |
319 | +++ src/maasserver/tests/test_api.py 2012-11-23 17:16:21 +0000 |
320 | @@ -3432,6 +3432,21 @@ |
321 | compose_enlistment_preseed_url(), |
322 | json.loads(response.content)["preseed_url"]) |
323 | |
324 | + def test_pxeconfig_enlistment_preseed_url_detects_request_origin(self): |
325 | + self.silence_get_ephemeral_name() |
326 | + ng_url = 'http://%s' % factory.make_name('host') |
327 | + network = IPNetwork("10.1.1/24") |
328 | + ip = factory.getRandomIPInNetwork(network) |
329 | + factory.make_node_group(maas_url=ng_url, network=network) |
330 | + params = self.get_default_params() |
331 | + |
332 | + # Simulate that the request targets ip by setting 'SERVER_NAME'. |
333 | + response = self.client.get( |
334 | + reverse('pxeconfig'), params, SERVER_NAME=ip) |
335 | + self.assertThat( |
336 | + json.loads(response.content)["preseed_url"], |
337 | + StartsWith(ng_url)) |
338 | + |
339 | def test_pxeconfig_has_preseed_url_for_known_node(self): |
340 | params = self.get_mac_params() |
341 | node = MACAddress.objects.get(mac_address=params['mac']).node |
342 | @@ -3440,6 +3455,23 @@ |
343 | compose_preseed_url(node), |
344 | json.loads(response.content)["preseed_url"]) |
345 | |
346 | + def test_preseed_url_for_known_node_uses_nodegroup_maas_url(self): |
347 | + ng_url = 'http://%s' % factory.make_name('host') |
348 | + network = IPNetwork("10.1.1/24") |
349 | + ip = factory.getRandomIPInNetwork(network) |
350 | + nodegroup = factory.make_node_group(maas_url=ng_url, network=network) |
351 | + params = self.get_mac_params() |
352 | + node = MACAddress.objects.get(mac_address=params['mac']).node |
353 | + node.nodegroup = nodegroup |
354 | + node.save() |
355 | + |
356 | + # Simulate that the request targets ip by setting 'SERVER_NAME'. |
357 | + response = self.client.get( |
358 | + reverse('pxeconfig'), params, SERVER_NAME=ip) |
359 | + self.assertThat( |
360 | + json.loads(response.content)["preseed_url"], |
361 | + StartsWith(ng_url)) |
362 | + |
363 | def test_get_boot_purpose_unknown_node(self): |
364 | # A node that's not yet known to MAAS is assumed to be enlisting, |
365 | # which uses a "commissioning" image. |
366 | |
367 | === modified file 'src/maasserver/tests/test_fields.py' |
368 | --- src/maasserver/tests/test_fields.py 2012-10-08 07:35:35 +0000 |
369 | +++ src/maasserver/tests/test_fields.py 2012-11-23 17:16:21 +0000 |
370 | @@ -21,7 +21,6 @@ |
371 | from maasserver.models import ( |
372 | MACAddress, |
373 | NodeGroup, |
374 | - nodegroupinterface, |
375 | NodeGroupInterface, |
376 | ) |
377 | from maasserver.testing.factory import factory |
378 | @@ -33,7 +32,6 @@ |
379 | JSONFieldModel, |
380 | XMLFieldModel, |
381 | ) |
382 | -from netaddr import IPNetwork |
383 | |
384 | |
385 | class TestNodeGroupFormField(TestCase): |
386 | @@ -70,51 +68,6 @@ |
387 | nodegroup, |
388 | NodeGroupFormField().clean("%s" % nodegroup.id)) |
389 | |
390 | - def test_clean_finds_nodegroup_by_network_address(self): |
391 | - nodegroup = factory.make_node_group( |
392 | - network=IPNetwork("192.168.28.1/24")) |
393 | - self.assertEqual( |
394 | - nodegroup, |
395 | - NodeGroupFormField().clean('192.168.28.0')) |
396 | - |
397 | - def test_find_nodegroup_looks_up_nodegroup_by_controller_ip(self): |
398 | - nodegroup = factory.make_node_group() |
399 | - self.assertEqual( |
400 | - nodegroup, |
401 | - NodeGroupFormField().clean(nodegroup.get_managed_interface().ip)) |
402 | - |
403 | - def test_find_nodegroup_accepts_any_ip_in_nodegroup_subnet(self): |
404 | - nodegroup = factory.make_node_group( |
405 | - network=IPNetwork("192.168.41.0/24")) |
406 | - self.assertEqual( |
407 | - nodegroup, |
408 | - NodeGroupFormField().clean('192.168.41.199')) |
409 | - |
410 | - def test_find_nodegroup_reports_if_not_found(self): |
411 | - self.assertRaises( |
412 | - ValidationError, |
413 | - NodeGroupFormField().clean, |
414 | - factory.getRandomIPAddress()) |
415 | - |
416 | - def test_find_nodegroup_reports_if_multiple_matches(self): |
417 | - self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1) |
418 | - factory.make_node_group(network=IPNetwork("10/8")) |
419 | - factory.make_node_group(network=IPNetwork("10.1.1/24")) |
420 | - self.assertRaises( |
421 | - NodeGroup.MultipleObjectsReturned, |
422 | - NodeGroupFormField().clean, '10.1.1.2') |
423 | - |
424 | - def test_find_nodegroup_handles_multiple_matches_on_same_nodegroup(self): |
425 | - self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1) |
426 | - nodegroup = factory.make_node_group(network=IPNetwork("10/8")) |
427 | - NodeGroupInterface.objects.create( |
428 | - nodegroup=nodegroup, ip='10.0.0.2', subnet_mask='255.0.0.0', |
429 | - broadcast_ip='10.0.0.1', interface='eth71') |
430 | - NodeGroupInterface.objects.create( |
431 | - nodegroup=nodegroup, ip='10.0.0.3', subnet_mask='255.0.0.0', |
432 | - broadcast_ip='10.0.0.2', interface='eth72') |
433 | - self.assertEqual(nodegroup, NodeGroupFormField().clean('10.0.0.9')) |
434 | - |
435 | |
436 | class TestMACAddressField(TestCase): |
437 | |
438 | |
439 | === modified file 'src/maasserver/tests/test_forms.py' |
440 | --- src/maasserver/tests/test_forms.py 2012-11-13 10:44:22 +0000 |
441 | +++ src/maasserver/tests/test_forms.py 2012-11-23 17:16:21 +0000 |
442 | @@ -198,14 +198,6 @@ |
443 | NodeGroup.objects.ensure_master(), |
444 | NodeWithMACAddressesForm(self.make_params()).save().nodegroup) |
445 | |
446 | - def test_sets_nodegroup_on_new_node_if_requested(self): |
447 | - nodegroup = factory.make_node_group( |
448 | - network=IPNetwork("192.168.14.0/24"), ip_range_low='192.168.14.2', |
449 | - ip_range_high='192.168.14.254', ip='192.168.14.1') |
450 | - form = NodeWithMACAddressesForm( |
451 | - self.make_params(nodegroup=nodegroup.get_managed_interface().ip)) |
452 | - self.assertEqual(nodegroup, form.save().nodegroup) |
453 | - |
454 | def test_leaves_nodegroup_alone_if_unset_on_existing_node(self): |
455 | # Selecting a node group for a node is only supported on new |
456 | # nodes. You can't change it later. |
457 | |
458 | === modified file 'src/maasserver/tests/test_preseed.py' |
459 | --- src/maasserver/tests/test_preseed.py 2012-11-14 10:32:25 +0000 |
460 | +++ src/maasserver/tests/test_preseed.py 2012-11-23 17:16:21 +0000 |
461 | @@ -30,12 +30,14 @@ |
462 | GENERIC_FILENAME, |
463 | get_enlist_preseed, |
464 | get_hostname_and_path, |
465 | + get_node_preseed_context, |
466 | get_preseed, |
467 | get_preseed_context, |
468 | get_preseed_filenames, |
469 | get_preseed_template, |
470 | load_preseed_template, |
471 | PreseedTemplate, |
472 | + render_enlistment_preseed, |
473 | render_preseed, |
474 | split_subarch, |
475 | TemplateNotFoundError, |
476 | @@ -46,7 +48,10 @@ |
477 | from maastesting.matchers import ContainsAll |
478 | from testtools.matchers import ( |
479 | AllMatch, |
480 | + Contains, |
481 | IsInstance, |
482 | + MatchesAll, |
483 | + Not, |
484 | StartsWith, |
485 | ) |
486 | |
487 | @@ -317,23 +322,6 @@ |
488 | """Tests for `get_preseed_context`.""" |
489 | |
490 | def test_get_preseed_context_contains_keys(self): |
491 | - node = factory.make_node() |
492 | - release = factory.getRandomString() |
493 | - context = get_preseed_context(node, release) |
494 | - self.assertItemsEqual( |
495 | - ['node', 'release', 'metadata_enlist_url', |
496 | - 'server_host', 'server_url', 'preseed_data', |
497 | - 'node_disable_pxe_url', 'node_disable_pxe_data', |
498 | - 'main_archive_hostname', 'main_archive_directory', |
499 | - 'ports_archive_hostname', 'ports_archive_directory', |
500 | - 'http_proxy', |
501 | - ], |
502 | - context) |
503 | - |
504 | - def test_get_preseed_context_if_node_None(self): |
505 | - # If the provided Node is None (when're in the context of an |
506 | - # enlistment preseed) the returned context does not include the |
507 | - # node context. |
508 | release = factory.getRandomString() |
509 | context = get_preseed_context(None, release) |
510 | self.assertItemsEqual( |
511 | @@ -370,6 +358,20 @@ |
512 | )) |
513 | |
514 | |
515 | +class TestNodePreseedContext(TestCase): |
516 | + """Tests for `get_node_preseed_context`.""" |
517 | + |
518 | + def test_get_node_preseed_context_contains_keys(self): |
519 | + node = factory.make_node() |
520 | + release = factory.getRandomString() |
521 | + context = get_node_preseed_context(node, release) |
522 | + self.assertItemsEqual( |
523 | + ['node', 'preseed_data', 'node_disable_pxe_url', |
524 | + 'node_disable_pxe_data', |
525 | + ], |
526 | + context) |
527 | + |
528 | + |
529 | class TestPreseedTemplate(TestCase): |
530 | """Tests for class:`PreseedTemplate`.""" |
531 | |
532 | @@ -399,6 +401,36 @@ |
533 | # error. |
534 | self.assertIsInstance(preseed, str) |
535 | |
536 | + def test_get_preseed_uses_nodegroup_maas_url(self): |
537 | + ng_url = 'http://%s' % factory.make_name('host') |
538 | + ng = factory.make_node_group(maas_url=ng_url) |
539 | + maas_url = 'http://%s' % factory.make_name('host') |
540 | + node = factory.make_node( |
541 | + nodegroup=ng, status=NODE_STATUS.COMMISSIONING) |
542 | + self.patch(settings, 'DEFAULT_MAAS_URL', maas_url) |
543 | + preseed = render_preseed(node, self.preseed, "precise") |
544 | + self.assertThat( |
545 | + preseed, MatchesAll(*[Contains(ng_url), Not(Contains(maas_url))])) |
546 | + |
547 | + |
548 | +class TestRenderEnlistmentPreseed(TestCase): |
549 | + """Tests for `render_enlistment_preseed`.""" |
550 | + |
551 | + def test_render_enlistment_preseed(self): |
552 | + preseed = render_enlistment_preseed(PRESEED_TYPE.ENLIST, "precise") |
553 | + # The test really is that the preseed is rendered without an |
554 | + # error. |
555 | + self.assertIsInstance(preseed, str) |
556 | + |
557 | + def test_get_preseed_uses_nodegroup_maas_url(self): |
558 | + ng_url = 'http://%s' % factory.make_name('host') |
559 | + maas_url = 'http://%s' % factory.make_name('host') |
560 | + self.patch(settings, 'DEFAULT_MAAS_URL', maas_url) |
561 | + preseed = render_enlistment_preseed( |
562 | + PRESEED_TYPE.ENLIST, "precise", base_url=ng_url) |
563 | + self.assertThat( |
564 | + preseed, MatchesAll(*[Contains(ng_url), Not(Contains(maas_url))])) |
565 | + |
566 | |
567 | class TestRenderPreseedArchives(TestCase): |
568 | """Test that the default preseed contains the default mirrors.""" |
569 | |
570 | === modified file 'src/maasserver/utils/__init__.py' |
571 | --- src/maasserver/utils/__init__.py 2012-11-22 13:33:46 +0000 |
572 | +++ src/maasserver/utils/__init__.py 2012-11-23 17:16:21 +0000 |
573 | @@ -13,6 +13,7 @@ |
574 | __all__ = [ |
575 | 'absolute_reverse', |
576 | 'build_absolute_uri', |
577 | + 'find_nodegroup', |
578 | 'get_db_state', |
579 | 'get_origin_ip', |
580 | 'ignore_unused', |
581 | @@ -67,7 +68,7 @@ |
582 | } |
583 | |
584 | |
585 | -def absolute_reverse(view_name, query=None, *args, **kwargs): |
586 | +def absolute_reverse(view_name, query=None, base_url=None, *args, **kwargs): |
587 | """Return the absolute URL (i.e. including the URL scheme specifier and |
588 | the network location of the MAAS server). Internally this method simply |
589 | calls Django's 'reverse' method and prefixes the result of that call with |
590 | @@ -78,11 +79,14 @@ |
591 | :param query: Optional query argument which will be passed down to |
592 | urllib.urlencode. The result of that call will be appended to the |
593 | resulting url. |
594 | + :param base_url: Optional url used as base. If None is provided, then |
595 | + settings.DEFAULT_MAAS_URL will be used. |
596 | :param args: Positional arguments for Django's 'reverse' method. |
597 | :param kwargs: Named arguments for Django's 'reverse' method. |
598 | """ |
599 | - url = urljoin( |
600 | - settings.DEFAULT_MAAS_URL, reverse(view_name, *args, **kwargs)) |
601 | + if not base_url: |
602 | + base_url = settings.DEFAULT_MAAS_URL |
603 | + url = urljoin(base_url, reverse(view_name, *args, **kwargs)) |
604 | if query is not None: |
605 | url += '?%s' % urlencode(query, doseq=True) |
606 | return url |
607 | @@ -118,3 +122,26 @@ |
608 | return socket.gethostbyname(host) |
609 | except socket.error: |
610 | return None |
611 | + |
612 | + |
613 | +def find_nodegroup(request): |
614 | + """Find the nodegroup whose subnet contains the IP Address of the |
615 | + originating host of the request.. |
616 | + |
617 | + The matching nodegroup may have multiple interfaces on the subnet, |
618 | + but there can be only one matching nodegroup. |
619 | + """ |
620 | + # Circular imports. |
621 | + from maasserver.models import NodeGroup |
622 | + ip_address = get_origin_ip(request) |
623 | + if ip_address is not None: |
624 | + return get_one(NodeGroup.objects.raw(""" |
625 | + SELECT * |
626 | + FROM maasserver_nodegroup |
627 | + WHERE id IN ( |
628 | + SELECT nodegroup_id |
629 | + FROM maasserver_nodegroupinterface |
630 | + WHERE (inet %s & subnet_mask) = (ip & subnet_mask) |
631 | + ) |
632 | + """, [ip_address])) |
633 | + return None |
634 | |
635 | === modified file 'src/maasserver/utils/tests/test_utils.py' |
636 | --- src/maasserver/utils/tests/test_utils.py 2012-11-22 13:33:46 +0000 |
637 | +++ src/maasserver/utils/tests/test_utils.py 2012-11-23 17:16:21 +0000 |
638 | @@ -20,11 +20,17 @@ |
639 | from django.http import HttpRequest |
640 | from django.test.client import RequestFactory |
641 | from maasserver.enum import NODE_STATUS_CHOICES |
642 | +from maasserver.models import ( |
643 | + NodeGroup, |
644 | + nodegroupinterface, |
645 | + NodeGroupInterface, |
646 | + ) |
647 | from maasserver.testing.factory import factory |
648 | from maasserver.testing.testcase import TestCase as DjangoTestCase |
649 | from maasserver.utils import ( |
650 | absolute_reverse, |
651 | build_absolute_uri, |
652 | + find_nodegroup, |
653 | get_db_state, |
654 | get_origin_ip, |
655 | map_enum, |
656 | @@ -35,6 +41,7 @@ |
657 | call, |
658 | Mock, |
659 | ) |
660 | +from netaddr import IPNetwork |
661 | |
662 | |
663 | class TestEnum(TestCase): |
664 | @@ -74,13 +81,19 @@ |
665 | |
666 | class TestAbsoluteReverse(DjangoTestCase): |
667 | |
668 | - def test_absolute_reverse_uses_DEFAULT_MAAS_URL(self): |
669 | + def test_absolute_reverse_uses_DEFAULT_MAAS_URL_by_default(self): |
670 | maas_url = 'http://%s' % factory.getRandomString() |
671 | self.patch(settings, 'DEFAULT_MAAS_URL', maas_url) |
672 | absolute_url = absolute_reverse('settings') |
673 | expected_url = settings.DEFAULT_MAAS_URL + reverse('settings') |
674 | self.assertEqual(expected_url, absolute_url) |
675 | |
676 | + def test_absolute_reverse_uses_given_base_url(self): |
677 | + maas_url = 'http://%s' % factory.getRandomString() |
678 | + absolute_url = absolute_reverse('settings', base_url=maas_url) |
679 | + expected_url = maas_url + reverse('settings') |
680 | + self.assertEqual(expected_url, absolute_url) |
681 | + |
682 | def test_absolute_reverse_uses_query_string(self): |
683 | self.patch(settings, 'DEFAULT_MAAS_URL', '') |
684 | parameters = {factory.getRandomString(): factory.getRandomString()} |
685 | @@ -184,26 +197,27 @@ |
686 | self.assertEqual(results, map(strip_domain, inputs)) |
687 | |
688 | |
689 | +def get_request(server_name, server_port='80'): |
690 | + return RequestFactory().post( |
691 | + '/', SERVER_NAME=server_name, SERVER_PORT=server_port) |
692 | + |
693 | + |
694 | class TestGetOriginIP(TestCase): |
695 | |
696 | - def get_request(self, server_name, server_port='80'): |
697 | - return RequestFactory().post( |
698 | - '/', SERVER_NAME=server_name, SERVER_PORT=server_port) |
699 | - |
700 | def test_get_origin_ip_returns_ip(self): |
701 | ip = factory.getRandomIPAddress() |
702 | - request = self.get_request(ip) |
703 | + request = get_request(ip) |
704 | self.assertEqual(ip, get_origin_ip(request)) |
705 | |
706 | def test_get_origin_ip_strips_port(self): |
707 | ip = factory.getRandomIPAddress() |
708 | - request = self.get_request(ip, '8888') |
709 | + request = get_request(ip, '8888') |
710 | self.assertEqual(ip, get_origin_ip(request)) |
711 | |
712 | def test_get_origin_ip_resolves_hostname(self): |
713 | ip = factory.getRandomIPAddress() |
714 | hostname = factory.make_name('hostname') |
715 | - request = self.get_request(hostname) |
716 | + request = get_request(hostname) |
717 | resolver = self.patch(socket, 'gethostbyname', Mock(return_value=ip)) |
718 | self.assertEqual( |
719 | (ip, call(hostname)), |
720 | @@ -211,9 +225,56 @@ |
721 | |
722 | def test_get_origin_ip_returns_None_if_hostname_cannot_get_resolved(self): |
723 | hostname = factory.make_name('hostname') |
724 | - request = self.get_request(hostname) |
725 | + request = get_request(hostname) |
726 | resolver = self.patch( |
727 | socket, 'gethostbyname', Mock(side_effect=socket.error)) |
728 | self.assertEqual( |
729 | (None, call(hostname)), |
730 | (get_origin_ip(request), resolver.call_args)) |
731 | + |
732 | + |
733 | +class TestFindNodegroup(DjangoTestCase): |
734 | + |
735 | + def test_finds_nodegroup_by_network_address(self): |
736 | + nodegroup = factory.make_node_group( |
737 | + network=IPNetwork("192.168.28.1/24")) |
738 | + self.assertEqual( |
739 | + nodegroup, |
740 | + find_nodegroup(get_request('192.168.28.0'))) |
741 | + |
742 | + def test_find_nodegroup_looks_up_nodegroup_by_controller_ip(self): |
743 | + nodegroup = factory.make_node_group() |
744 | + ip = nodegroup.get_managed_interface().ip |
745 | + self.assertEqual( |
746 | + nodegroup, |
747 | + find_nodegroup(get_request(ip))) |
748 | + |
749 | + def test_find_nodegroup_accepts_any_ip_in_nodegroup_subnet(self): |
750 | + nodegroup = factory.make_node_group( |
751 | + network=IPNetwork("192.168.41.0/24")) |
752 | + self.assertEqual( |
753 | + nodegroup, |
754 | + find_nodegroup(get_request('192.168.41.199'))) |
755 | + |
756 | + def test_find_nodegroup_returns_None_if_not_found(self): |
757 | + self.assertIsNone( |
758 | + find_nodegroup(get_request(factory.getRandomIPAddress()))) |
759 | + |
760 | + def test_find_nodegroup_errors_if_multiple_matches(self): |
761 | + self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1) |
762 | + factory.make_node_group(network=IPNetwork("10/8")) |
763 | + factory.make_node_group(network=IPNetwork("10.1.1/24")) |
764 | + self.assertRaises( |
765 | + NodeGroup.MultipleObjectsReturned, |
766 | + find_nodegroup, get_request('10.1.1.2')) |
767 | + |
768 | + def test_find_nodegroup_handles_multiple_matches_on_same_nodegroup(self): |
769 | + self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1) |
770 | + nodegroup = factory.make_node_group(network=IPNetwork("10/8")) |
771 | + NodeGroupInterface.objects.create( |
772 | + nodegroup=nodegroup, ip='10.0.0.2', subnet_mask='255.0.0.0', |
773 | + broadcast_ip='10.0.0.1', interface='eth71') |
774 | + NodeGroupInterface.objects.create( |
775 | + nodegroup=nodegroup, ip='10.0.0.3', subnet_mask='255.0.0.0', |
776 | + broadcast_ip='10.0.0.2', interface='eth72') |
777 | + self.assertEqual(nodegroup, find_nodegroup(get_request('10.0.0.9'))) |
778 | |
779 | === modified file 'src/metadataserver/api.py' |
780 | --- src/metadataserver/api.py 2012-11-22 13:57:18 +0000 |
781 | +++ src/metadataserver/api.py 2012-11-23 17:16:21 +0000 |
782 | @@ -53,6 +53,7 @@ |
783 | get_enlist_userdata, |
784 | get_preseed, |
785 | ) |
786 | +from maasserver.utils import find_nodegroup |
787 | from maasserver.utils.orm import get_one |
788 | from metadataserver.models import ( |
789 | NodeCommissionResult, |
790 | @@ -393,7 +394,10 @@ |
791 | @operation(idempotent=True) |
792 | def get_enlist_preseed(self, request, version=None): |
793 | """Render and return a preseed script for enlistment.""" |
794 | - return HttpResponse(get_enlist_preseed(), mimetype="text/plain") |
795 | + nodegroup = find_nodegroup(request) |
796 | + base_url = nodegroup.maas_url if nodegroup is not None else None |
797 | + return HttpResponse( |
798 | + get_enlist_preseed(base_url=base_url), mimetype="text/plain") |
799 | |
800 | @operation(idempotent=True) |
801 | def get_preseed(self, request, version=None, system_id=None): |
802 | |
803 | === modified file 'src/metadataserver/tests/test_api.py' |
804 | --- src/metadataserver/tests/test_api.py 2012-11-23 12:23:52 +0000 |
805 | +++ src/metadataserver/tests/test_api.py 2012-11-23 17:16:21 +0000 |
806 | @@ -50,7 +50,10 @@ |
807 | NodeUserData, |
808 | ) |
809 | from metadataserver.nodeinituser import get_node_init_user |
810 | +from mock import Mock |
811 | +from netaddr import IPNetwork |
812 | from provisioningserver.enum import POWER_TYPE |
813 | +from testtools.matchers import Contains |
814 | |
815 | |
816 | class TestHelpers(DjangoTestCase): |
817 | @@ -629,7 +632,7 @@ |
818 | 'metadata-enlist-preseed', args=['latest']) |
819 | # Fake the preseed so we're just exercising the view. |
820 | fake_preseed = factory.getRandomString() |
821 | - self.patch(api, "get_enlist_preseed", lambda: fake_preseed) |
822 | + self.patch(api, "get_enlist_preseed", Mock(return_value=fake_preseed)) |
823 | response = self.client.get( |
824 | anon_enlist_preseed_url, {'op': 'get_enlist_preseed'}) |
825 | self.assertEqual( |
826 | @@ -641,6 +644,18 @@ |
827 | response.content), |
828 | response) |
829 | |
830 | + def test_anonymous_get_enlist_preseed_detects_request_origin(self): |
831 | + ng_url = 'http://%s' % factory.make_name('host') |
832 | + network = IPNetwork("10.1.1/24") |
833 | + ip = factory.getRandomIPInNetwork(network) |
834 | + factory.make_node_group(maas_url=ng_url, network=network) |
835 | + anon_enlist_preseed_url = reverse( |
836 | + 'metadata-enlist-preseed', args=['latest']) |
837 | + response = self.client.get( |
838 | + anon_enlist_preseed_url, {'op': 'get_enlist_preseed'}, |
839 | + SERVER_NAME=ip) |
840 | + self.assertThat(response.content, Contains(ng_url)) |
841 | + |
842 | def test_anonymous_get_preseed(self): |
843 | # The preseed for a node can be obtained anonymously. |
844 | node = factory.make_node() |
Looks good.
allenap: test_pxeconfig_ enlistment_ preseed_ url_detects_ request_ origin,
appropriate maas_url… request) /docs.djangopro ject.com/ en/dev/ ref/request-
response/ #django. http.HttpReques t.get_host
why is SERVER_NAME being set to an address in the nodegroup's
network?
rvba: allenap: well, we need to fetch the right nodegroup to get the
rvba: Detect in which nodegroup a node is (it already enlisted) or
will (if enlisting) and use nodegroup.maas_url.
rvba: this exercises this code:
allenap: rvba: Yes, but why SERVER_NAME? That looks like a CGI variable.
rvba: nodegroup = find_nodegroup(
rvba: allenap: ah that… it's just a way to get request.get_host to
return the right host.
rvba: https:/