Merge lp:~blake-rouse/maas/osystem-preseed-prefix into lp:maas

Proposed by Blake Rouse
Status: Merged
Approved by: Blake Rouse
Approved revision: 2302
Merged at revision: 2386
Proposed branch: lp:~blake-rouse/maas/osystem-preseed-prefix
Merge into: lp:maas
Prerequisite: lp:~blake-rouse/maas/osystem-preseed-cleanup
Diff against target: 314 lines (+66/-40)
4 files modified
src/maasserver/preseed.py (+34/-20)
src/maasserver/templates/maasserver/node_preseed.html (+1/-1)
src/maasserver/tests/test_preseed.py (+26/-17)
src/maasserver/views/tests/test_nodes.py (+5/-2)
To merge this branch: bzr merge lp:~blake-rouse/maas/osystem-preseed-prefix
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve
Review via email: mp+218139@code.launchpad.net

Commit message

Adds osystem to the preseed template filename, fixes html escaping generated preseed in node view.

Description of the change

Allows osystem to be used in the filename of a preseed template. This is used to support multiple operating systems. Example is Windows uses a different preseed.

Fixes an issue were the preseed view output is not html escaped. This was not an issue for the current preseeds, but for other operating systems this is an issue. Example is Windows uses XML for its preseed.

To post a comment you must log in.
2300. By Blake Rouse

Merge osystem-preseed-cleanup.

2301. By Blake Rouse

Merge osystem-preseed-cleanup.

Revision history for this message
Julian Edwards (julian-edwards) wrote :

Nice branch! Small, focused and easy to follow :) It's good to land with a couple of nits to fix as below:

[1]

174 -{{ preseed }}
175 +{{ preseed|force_escape }}

Was there a reason to use force_escape here? Django's docs say to use just "escape" unless you want immediate escaping, which I don't think is the case?

https://docs.djangoproject.com/en/dev/ref/templates/builtins/#std:templatefilter-escape

[2]

The tests for escaping could be improved if they did a more explicit test, such as looking for &lt;mytag&gt; when the preseed is <mytag>.

review: Approve
2302. By Blake Rouse

Merge trunk.

Revision history for this message
Blake Rouse (blake-rouse) wrote :

Using escape fails to escape the output. Unknown why, but using force_escape works correctly.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src/maasserver/preseed.py'
2--- src/maasserver/preseed.py 2014-05-29 18:20:58 +0000
3+++ src/maasserver/preseed.py 2014-05-29 18:52:46 +0000
4@@ -176,14 +176,16 @@
5 if node.status == NODE_STATUS.COMMISSIONING:
6 return render_preseed(
7 node, PRESEED_TYPE.COMMISSIONING,
8+ osystem=Config.objects.get_config('commissioning_osystem'),
9 release=Config.objects.get_config('commissioning_distro_series'))
10 else:
11 return render_preseed(
12 node, get_preseed_type_for(node),
13- release=node.get_distro_series())
14-
15-
16-def get_preseed_filenames(node, prefix='', release='', default=False):
17+ osystem=node.get_osystem(), release=node.get_distro_series())
18+
19+
20+def get_preseed_filenames(node, prefix='', osystem='', release='',
21+ default=False):
22 """List possible preseed template filenames for the given node.
23
24 :param node: The node to return template preseed filenames for.
25@@ -192,7 +194,9 @@
26 a prefix in the template filenames). Usually one of {'', 'enlist',
27 'commissioning'}.
28 :type prefix: unicode
29- :param release: The Ubuntu release to be used.
30+ :param osystem: The operating system to be used.
31+ :type osystem: unicode
32+ :param release: The os release to be used.
33 :type release: unicode
34 :param default: Should we return the default ('generic') template as a
35 last resort template?
36@@ -200,10 +204,11 @@
37
38 Returns a list of possible preseed template filenames using the following
39 lookup order:
40- {prefix}_{node_architecture}_{node_subarchitecture}_{release}_{node_name}
41- {prefix}_{node_architecture}_{node_subarchitecture}_{release}
42- {prefix}_{node_architecture}_{node_subarchitecture}
43- {prefix}_{node_architecture}
44+ {prefix}_{osystem}_{node_arch}_{node_subarch}_{release}_{node_name}
45+ {prefix}_{osystem}_{node_arch}_{node_subarch}_{release}
46+ {prefix}_{osystem}_{node_arch}_{node_subarch}
47+ {prefix}_{osystem}_{node_arch}
48+ {prefix}_{osystem}
49 {prefix}
50 'generic'
51 """
52@@ -211,6 +216,8 @@
53 # Add prefix.
54 if prefix != '':
55 elements.append(prefix)
56+ # Add osystem
57+ elements.append(osystem)
58 # Add architecture/sub-architecture.
59 if node is not None:
60 arch = split_subarch(node.architecture)
61@@ -284,11 +291,12 @@
62 self.name = name
63
64
65-def load_preseed_template(node, prefix, release=''):
66+def load_preseed_template(node, prefix, osystem='', release=''):
67 """Find and load a `PreseedTemplate` for the given node.
68
69 :param node: See `get_preseed_filenames`.
70 :param prefix: See `get_preseed_filenames`.
71+ :param osystem: See `get_preseed_filenames`.
72 :param release: See `get_preseed_filenames`.
73 """
74
75@@ -298,7 +306,8 @@
76 It is defined to preserve the context (node, name, release, default)
77 since this will be called (by Tempita) called out of scope.
78 """
79- filenames = list(get_preseed_filenames(node, name, release, default))
80+ filenames = list(get_preseed_filenames(
81+ node, name, osystem, release, default))
82 filepath, content = get_preseed_template(filenames)
83 if filepath is None:
84 raise TemplateNotFoundError(name)
85@@ -353,10 +362,11 @@
86 return interfaces[0].ip
87
88
89-def get_preseed_context(release='', nodegroup=None):
90+def get_preseed_context(osystem='', release='', nodegroup=None):
91 """Return the node-independent context dictionary to be used to render
92 preseed templates.
93
94+ :param osystem: See `get_preseed_filenames`.
95 :param release: See `get_preseed_filenames`.
96 :param nodegroup: The nodegroup used to generate the preseed.
97 :return: The context dictionary.
98@@ -376,6 +386,7 @@
99 'main_archive_directory': main_archive_directory,
100 'ports_archive_hostname': ports_archive_hostname,
101 'ports_archive_directory': ports_archive_directory,
102+ 'osystem': osystem,
103 'release': release,
104 'server_host': server_host,
105 'server_url': absolute_reverse('nodes_handler', base_url=base_url),
106@@ -384,11 +395,12 @@
107 }
108
109
110-def get_node_preseed_context(node, release=''):
111+def get_node_preseed_context(node, osystem='', release=''):
112 """Return the node-dependent context dictionary to be used to render
113 preseed templates.
114
115 :param node: See `get_preseed_filenames`.
116+ :param osystem: See `get_preseed_filenames`.
117 :param release: See `get_preseed_filenames`.
118 :return: The context dictionary.
119 :rtype: dict.
120@@ -412,36 +424,38 @@
121 }
122
123
124-def render_enlistment_preseed(prefix, release='', nodegroup=None):
125+def render_enlistment_preseed(prefix, osystem='', release='', nodegroup=None):
126 """Return the enlistment preseed.
127
128 :param prefix: See `get_preseed_filenames`.
129+ :param osystem: See `get_preseed_filenames`.
130 :param release: See `get_preseed_filenames`.
131 :param nodegroup: The nodegroup used to generate the preseed.
132 :return: The rendered preseed string.
133 :rtype: unicode.
134 """
135- template = load_preseed_template(None, prefix, release)
136- context = get_preseed_context(release, nodegroup=nodegroup)
137+ template = load_preseed_template(None, prefix, osystem, release)
138+ context = get_preseed_context(osystem, release, nodegroup=nodegroup)
139 # Render the snippets in the main template.
140 snippets = get_snippet_context()
141 snippets.update(context)
142 return template.substitute(**snippets)
143
144
145-def render_preseed(node, prefix, release=''):
146+def render_preseed(node, prefix, osystem='', release=''):
147 """Return the preseed for the given node.
148
149 :param node: See `get_preseed_filenames`.
150 :param prefix: See `get_preseed_filenames`.
151+ :param osystem: See `get_preseed_filenames`.
152 :param release: See `get_preseed_filenames`.
153 :return: The rendered preseed string.
154 :rtype: unicode.
155 """
156- template = load_preseed_template(node, prefix, release)
157+ template = load_preseed_template(node, prefix, osystem, release)
158 nodegroup = node.nodegroup
159- context = get_preseed_context(release, nodegroup=nodegroup)
160- context.update(get_node_preseed_context(node, release))
161+ context = get_preseed_context(osystem, release, nodegroup=nodegroup)
162+ context.update(get_node_preseed_context(node, osystem, release))
163 return template.substitute(**context)
164
165
166
167=== modified file 'src/maasserver/templates/maasserver/node_preseed.html'
168--- src/maasserver/templates/maasserver/node_preseed.html 2013-09-13 15:52:14 +0000
169+++ src/maasserver/templates/maasserver/node_preseed.html 2014-05-29 18:52:46 +0000
170@@ -18,7 +18,7 @@
171 This node is commissioning. This is the commissioning preseed.
172 {% endif %}
173 <pre>
174-{{ preseed }}
175+{{ preseed|force_escape }}
176 </pre>
177 </div>
178 {% endblock %}
179
180=== modified file 'src/maasserver/tests/test_preseed.py'
181--- src/maasserver/tests/test_preseed.py 2014-05-29 18:20:58 +0000
182+++ src/maasserver/tests/test_preseed.py 2014-05-29 18:52:46 +0000
183@@ -101,43 +101,51 @@
184 def test_get_preseed_filenames_returns_filenames(self):
185 hostname = factory.getRandomString()
186 prefix = factory.getRandomString()
187+ osystem = factory.getRandomString()
188 release = factory.getRandomString()
189 node = factory.make_node(hostname=hostname)
190 arch, subarch = node.architecture.split('/')
191 self.assertSequenceEqual(
192 [
193- '%s_%s_%s_%s_%s' % (prefix, arch, subarch, release, hostname),
194- '%s_%s_%s_%s' % (prefix, arch, subarch, release),
195- '%s_%s_%s' % (prefix, arch, subarch),
196- '%s_%s' % (prefix, arch),
197+ '%s_%s_%s_%s_%s_%s' % (
198+ prefix, osystem, arch, subarch, release, hostname),
199+ '%s_%s_%s_%s_%s' % (prefix, osystem, arch, subarch, release),
200+ '%s_%s_%s_%s' % (prefix, osystem, arch, subarch),
201+ '%s_%s_%s' % (prefix, osystem, arch),
202+ '%s_%s' % (prefix, osystem),
203 '%s' % prefix,
204 'generic',
205 ],
206- list(get_preseed_filenames(node, prefix, release, default=True)))
207+ list(get_preseed_filenames(
208+ node, prefix, osystem, release, default=True)))
209
210 def test_get_preseed_filenames_if_node_is_None(self):
211+ osystem = factory.getRandomString()
212 release = factory.getRandomString()
213 prefix = factory.getRandomString()
214 self.assertSequenceEqual(
215 [
216- '%s_%s' % (prefix, release),
217+ '%s_%s_%s' % (prefix, osystem, release),
218+ '%s_%s' % (prefix, osystem),
219 '%s' % prefix,
220 ],
221- list(get_preseed_filenames(None, prefix, release)))
222+ list(get_preseed_filenames(None, prefix, osystem, release)))
223
224 def test_get_preseed_filenames_supports_empty_prefix(self):
225 hostname = factory.getRandomString()
226+ osystem = factory.getRandomString()
227 release = factory.getRandomString()
228 node = factory.make_node(hostname=hostname)
229 arch, subarch = node.architecture.split('/')
230 self.assertSequenceEqual(
231 [
232- '%s_%s_%s_%s' % (arch, subarch, release, hostname),
233- '%s_%s_%s' % (arch, subarch, release),
234- '%s_%s' % (arch, subarch),
235- '%s' % arch,
236+ '%s_%s_%s_%s_%s' % (osystem, arch, subarch, release, hostname),
237+ '%s_%s_%s_%s' % (osystem, arch, subarch, release),
238+ '%s_%s_%s' % (osystem, arch, subarch),
239+ '%s_%s' % (osystem, arch),
240+ '%s' % osystem,
241 ],
242- list(get_preseed_filenames(node, '', release)))
243+ list(get_preseed_filenames(node, '', osystem, release)))
244
245 def test_get_preseed_filenames_returns_list_without_default(self):
246 # If default=False is passed to get_preseed_filenames, the
247@@ -280,18 +288,19 @@
248 # At the top of the lookup hierarchy is a template specific to this
249 # node. It will be used first if it's present.
250 prefix = factory.getRandomString()
251+ osystem = factory.getRandomString()
252 release = factory.getRandomString()
253 # Create the generic and 'prefix' templates. They will be ignored
254 # due to the presence of a more specific template.
255 self.create_template(self.location, GENERIC_FILENAME)
256 self.create_template(self.location, prefix)
257 node = factory.make_node(hostname=factory.getRandomString())
258- node_template_name = "%s_%s_%s_%s" % (
259- prefix, node.architecture.replace('/', '_'),
260+ node_template_name = "%s_%s_%s_%s_%s" % (
261+ prefix, osystem, node.architecture.replace('/', '_'),
262 release, node.hostname)
263 # Create the node-specific template.
264 content = self.create_template(self.location, node_template_name)
265- template = load_preseed_template(node, prefix, release)
266+ template = load_preseed_template(node, prefix, osystem, release)
267 self.assertEqual(content, template.substitute())
268
269 def test_load_preseed_template_with_inherits(self):
270@@ -448,8 +457,8 @@
271 nodegroup = factory.make_node_group(maas_url=factory.getRandomString())
272 context = get_preseed_context(release, nodegroup)
273 self.assertItemsEqual(
274- ['release', 'metadata_enlist_url', 'server_host', 'server_url',
275- 'main_archive_hostname', 'main_archive_directory',
276+ ['osystem', 'release', 'metadata_enlist_url', 'server_host',
277+ 'server_url', 'main_archive_hostname', 'main_archive_directory',
278 'ports_archive_hostname', 'ports_archive_directory',
279 'http_proxy'],
280 context)
281
282=== modified file 'src/maasserver/views/tests/test_nodes.py'
283--- src/maasserver/views/tests/test_nodes.py 2014-04-25 18:03:38 +0000
284+++ src/maasserver/views/tests/test_nodes.py 2014-05-29 18:52:46 +0000
285@@ -27,6 +27,7 @@
286
287 from django.conf import settings
288 from django.core.urlresolvers import reverse
289+from django.utils import html
290 from lxml.etree import XPath
291 from lxml.html import fromstring
292 import maasserver.api
293@@ -1437,7 +1438,8 @@
294 node = factory.make_node(owner=self.logged_in_user)
295 node_preseed_link = reverse('node-preseed-view', args=[node.system_id])
296 response = self.client.get(node_preseed_link)
297- self.assertIn(get_preseed(node), response.content)
298+ escaped = html.escape(get_preseed(node))
299+ self.assertIn(escaped, response.content)
300
301 def test_preseedview_node_catches_template_error(self):
302 self.client_log_in()
303@@ -1456,9 +1458,10 @@
304 )
305 node_preseed_link = reverse('node-preseed-view', args=[node.system_id])
306 response = self.client.get(node_preseed_link)
307+ escaped = html.escape(get_preseed(node))
308 self.assertThat(
309 response.content,
310- ContainsAll([get_preseed(node), "This node is commissioning."]))
311+ ContainsAll([escaped, "This node is commissioning."]))
312
313 def test_preseedview_node_displays_link_to_view_node(self):
314 self.client_log_in()