cloud-init

Merge ~smoser/cloud-init:cleanup/jsonschema-follow-on into cloud-init:master

  1. Git
  2. lp:~smoser/cloud-init
  3. cleanup/jsonschema-follow-on
  4. Merge into master
Proposed by Scott Moser
Status: Merged
Approved by: Scott Moser
Approved revision: b52429168c86040f7b7a9cc868b2b568f35ff83b
Merge reported by: Scott Moser
Merged at revision: 5037252ca5dc54c6d978b23dba063ac5fabc98fa
Proposed branch: ~smoser/cloud-init:cleanup/jsonschema-follow-on
Merge into: cloud-init:master
Diff against target: 383 lines (+104/-76)
5 files modified
cloudinit/config/tests/test_snap.py (+15/-26)
cloudinit/config/tests/test_ubuntu_advantage.py (+28/-2)
cloudinit/tests/helpers.py (+19/-0)
tests/unittests/test_handler/test_handler_bootcmd.py (+19/-23)
tests/unittests/test_handler/test_handler_runcmd.py (+23/-25)
Reviewer Review Type Date Requested Status
Chad Smith Approve
Server Team CI bot continuous-integration Approve
Review via email: mp+343609@code.launchpad.net

Commit message

schema: in validation, raise ImportError if strict but no jsonschema.

validate_cloudconfig_schema with strict=True would not actually validate
if there was no jsonschema available. That seems kind of strange.
The change here is to make it raise an exception if strict was passed in.
And then to fix the one test that needed a skipIfJsonSchema wrapper.

Description of the change

see commit message

To post a comment you must log in.
Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:ffb65aadca33bb507eee8f44325e2c58d6928212
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1030/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1030/rebuild

review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

I like the idea; and the fix is needed. I just have a question about whether we can find a way to generalize that validate_or_fail method and avoid duplication.

Revision history for this message
Scott Moser (smoser) wrote :

I agree... I was tempted to do that. I'll go ahead and use the mixin you suggesetd.

~smoser/cloud-init:cleanup/jsonschema-follow-on updated
c1711b2... by Scott Moser

add assertValidSchema and use it

Revision history for this message
Server Team CI bot (server-team-bot) wrote :

PASSED: Continuous integration, rev:b52429168c86040f7b7a9cc868b2b568f35ff83b
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1038/
Executed test runs:
    SUCCESS: Checkout
    SUCCESS: Unit & Style Tests
    SUCCESS: Ubuntu LTS: Build
    SUCCESS: Ubuntu LTS: Integration
    SUCCESS: MAAS Compatability Testing
    IN_PROGRESS: Declarative: Post Actions

Click here to trigger a rebuild:
https://jenkins.ubuntu.com/server/job/cloud-init-ci/1038/rebuild

review: Approve (continuous-integration)
Revision history for this message
Chad Smith (chad.smith) wrote :

Good changeset and simplification magic based on top-level key.

Revision history for this message
Chad Smith (chad.smith) :
review: Approve
~smoser/cloud-init:cleanup/jsonschema-follow-on updated
fe118bb... by Scott Moser

fix pylint

Revision history for this message
Scott Moser (smoser) wrote :

An upstream commit landed for this bug.

To view that commit see the following URL:
https://git.launchpad.net/cloud-init/commit/?id=5037252c

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1diff --git a/cloudinit/config/tests/test_snap.py b/cloudinit/config/tests/test_snap.py
2index 492d2d4..34c80f1 100644
3--- a/cloudinit/config/tests/test_snap.py
4+++ b/cloudinit/config/tests/test_snap.py
5@@ -9,7 +9,7 @@ from cloudinit.config.cc_snap import (
6 from cloudinit.config.schema import validate_cloudconfig_schema
7 from cloudinit import util
8 from cloudinit.tests.helpers import (
9- CiTestCase, mock, wrap_and_call, skipUnlessJsonSchema)
10+ CiTestCase, SchemaTestCaseMixin, mock, wrap_and_call, skipUnlessJsonSchema)
11
12
13 SYSTEM_USER_ASSERTION = """\
14@@ -245,9 +245,10 @@ class TestRunCommands(CiTestCase):
15
16
17 @skipUnlessJsonSchema()
18-class TestSchema(CiTestCase):
19+class TestSchema(CiTestCase, SchemaTestCaseMixin):
20
21 with_logs = True
22+ schema = schema
23
24 def test_schema_warns_on_snap_not_as_dict(self):
25 """If the snap configuration is not a dict, emit a warning."""
26@@ -342,39 +343,27 @@ class TestSchema(CiTestCase):
27
28 def test_duplicates_are_fine_array_array(self):
29 """Duplicated commands array/array entries are allowed."""
30- byebye = ["echo", "bye"]
31- try:
32- cfg = {'snap': {'commands': [byebye, byebye]}}
33- validate_cloudconfig_schema(cfg, schema, strict=True)
34- except schema.SchemaValidationError as e:
35- self.fail("command entries can be duplicate.")
36+ self.assertSchemaValid(
37+ {'commands': [["echo", "bye"], ["echo" "bye"]]},
38+ "command entries can be duplicate.")
39
40 def test_duplicates_are_fine_array_string(self):
41 """Duplicated commands array/string entries are allowed."""
42- byebye = "echo bye"
43- try:
44- cfg = {'snap': {'commands': [byebye, byebye]}}
45- validate_cloudconfig_schema(cfg, schema, strict=True)
46- except schema.SchemaValidationError as e:
47- self.fail("command entries can be duplicate.")
48+ self.assertSchemaValid(
49+ {'commands': ["echo bye", "echo bye"]},
50+ "command entries can be duplicate.")
51
52 def test_duplicates_are_fine_dict_array(self):
53 """Duplicated commands dict/array entries are allowed."""
54- byebye = ["echo", "bye"]
55- try:
56- cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
57- validate_cloudconfig_schema(cfg, schema, strict=True)
58- except schema.SchemaValidationError as e:
59- self.fail("command entries can be duplicate.")
60+ self.assertSchemaValid(
61+ {'commands': {'00': ["echo", "bye"], '01': ["echo", "bye"]}},
62+ "command entries can be duplicate.")
63
64 def test_duplicates_are_fine_dict_string(self):
65 """Duplicated commands dict/string entries are allowed."""
66- byebye = "echo bye"
67- try:
68- cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
69- validate_cloudconfig_schema(cfg, schema, strict=True)
70- except schema.SchemaValidationError as e:
71- self.fail("command entries can be duplicate.")
72+ self.assertSchemaValid(
73+ {'commands': {'00': "echo bye", '01': "echo bye"}},
74+ "command entries can be duplicate.")
75
76
77 class TestHandle(CiTestCase):
78diff --git a/cloudinit/config/tests/test_ubuntu_advantage.py b/cloudinit/config/tests/test_ubuntu_advantage.py
79index f2a59fa..f1beeff 100644
80--- a/cloudinit/config/tests/test_ubuntu_advantage.py
81+++ b/cloudinit/config/tests/test_ubuntu_advantage.py
82@@ -7,7 +7,8 @@ from cloudinit.config.cc_ubuntu_advantage import (
83 handle, maybe_install_ua_tools, run_commands, schema)
84 from cloudinit.config.schema import validate_cloudconfig_schema
85 from cloudinit import util
86-from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJsonSchema
87+from cloudinit.tests.helpers import (
88+ CiTestCase, mock, SchemaTestCaseMixin, skipUnlessJsonSchema)
89
90
91 # Module path used in mocks
92@@ -105,9 +106,10 @@ class TestRunCommands(CiTestCase):
93
94
95 @skipUnlessJsonSchema()
96-class TestSchema(CiTestCase):
97+class TestSchema(CiTestCase, SchemaTestCaseMixin):
98
99 with_logs = True
100+ schema = schema
101
102 def test_schema_warns_on_ubuntu_advantage_not_as_dict(self):
103 """If ubuntu-advantage configuration is not a dict, emit a warning."""
104@@ -169,6 +171,30 @@ class TestSchema(CiTestCase):
105 {'ubuntu-advantage': {'commands': {'01': 'also valid'}}}, schema)
106 self.assertEqual('', self.logs.getvalue())
107
108+ def test_duplicates_are_fine_array_array(self):
109+ """Duplicated commands array/array entries are allowed."""
110+ self.assertSchemaValid(
111+ {'commands': [["echo", "bye"], ["echo" "bye"]]},
112+ "command entries can be duplicate.")
113+
114+ def test_duplicates_are_fine_array_string(self):
115+ """Duplicated commands array/string entries are allowed."""
116+ self.assertSchemaValid(
117+ {'commands': ["echo bye", "echo bye"]},
118+ "command entries can be duplicate.")
119+
120+ def test_duplicates_are_fine_dict_array(self):
121+ """Duplicated commands dict/array entries are allowed."""
122+ self.assertSchemaValid(
123+ {'commands': {'00': ["echo", "bye"], '01': ["echo", "bye"]}},
124+ "command entries can be duplicate.")
125+
126+ def test_duplicates_are_fine_dict_string(self):
127+ """Duplicated commands dict/string entries are allowed."""
128+ self.assertSchemaValid(
129+ {'commands': {'00': "echo bye", '01': "echo bye"}},
130+ "command entries can be duplicate.")
131+
132
133 class TestHandle(CiTestCase):
134
135diff --git a/cloudinit/tests/helpers.py b/cloudinit/tests/helpers.py
136index 5aada6e..4999f1f 100644
137--- a/cloudinit/tests/helpers.py
138+++ b/cloudinit/tests/helpers.py
139@@ -24,6 +24,8 @@ try:
140 except ImportError:
141 from ConfigParser import ConfigParser
142
143+from cloudinit.config.schema import (
144+ SchemaValidationError, validate_cloudconfig_schema)
145 from cloudinit import helpers as ch
146 from cloudinit import util
147
148@@ -312,6 +314,23 @@ class HttprettyTestCase(CiTestCase):
149 super(HttprettyTestCase, self).tearDown()
150
151
152+class SchemaTestCaseMixin(unittest2.TestCase):
153+
154+ def assertSchemaValid(self, cfg, msg="Valid Schema failed validation."):
155+ """Assert the config is valid per self.schema.
156+
157+ If there is only one top level key in the schema properties, then
158+ the cfg will be put under that key."""
159+ props = list(self.schema.get('properties'))
160+ # put cfg under top level key if there is only one in the schema
161+ if len(props) == 1:
162+ cfg = {props[0]: cfg}
163+ try:
164+ validate_cloudconfig_schema(cfg, self.schema, strict=True)
165+ except SchemaValidationError:
166+ self.fail(msg)
167+
168+
169 def populate_dir(path, files):
170 if not os.path.exists(path):
171 os.makedirs(path)
172diff --git a/tests/unittests/test_handler/test_handler_bootcmd.py b/tests/unittests/test_handler/test_handler_bootcmd.py
173index c3abedd..b137526 100644
174--- a/tests/unittests/test_handler/test_handler_bootcmd.py
175+++ b/tests/unittests/test_handler/test_handler_bootcmd.py
176@@ -1,9 +1,10 @@
177 # This file is part of cloud-init. See LICENSE file for license information.
178
179-from cloudinit.config import cc_bootcmd, schema
180+from cloudinit.config.cc_bootcmd import handle, schema
181 from cloudinit.sources import DataSourceNone
182 from cloudinit import (distros, helpers, cloud, util)
183-from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJsonSchema
184+from cloudinit.tests.helpers import (
185+ CiTestCase, mock, SchemaTestCaseMixin, skipUnlessJsonSchema)
186
187 import logging
188 import tempfile
189@@ -50,7 +51,7 @@ class TestBootcmd(CiTestCase):
190 """When the provided config doesn't contain bootcmd, skip it."""
191 cfg = {}
192 mycloud = self._get_cloud('ubuntu')
193- cc_bootcmd.handle('notimportant', cfg, mycloud, LOG, None)
194+ handle('notimportant', cfg, mycloud, LOG, None)
195 self.assertIn(
196 "Skipping module named notimportant, no 'bootcmd' key",
197 self.logs.getvalue())
198@@ -60,7 +61,7 @@ class TestBootcmd(CiTestCase):
199 invalid_config = {'bootcmd': 1}
200 cc = self._get_cloud('ubuntu')
201 with self.assertRaises(TypeError) as context_manager:
202- cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, [])
203+ handle('cc_bootcmd', invalid_config, cc, LOG, [])
204 self.assertIn('Failed to shellify bootcmd', self.logs.getvalue())
205 self.assertEqual(
206 "Input to shellify was type 'int'. Expected list or tuple.",
207@@ -76,7 +77,7 @@ class TestBootcmd(CiTestCase):
208 invalid_config = {'bootcmd': 1}
209 cc = self._get_cloud('ubuntu')
210 with self.assertRaises(TypeError):
211- cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, [])
212+ handle('cc_bootcmd', invalid_config, cc, LOG, [])
213 self.assertIn(
214 'Invalid config:\nbootcmd: 1 is not of type \'array\'',
215 self.logs.getvalue())
216@@ -93,7 +94,7 @@ class TestBootcmd(CiTestCase):
217 'bootcmd': ['ls /', 20, ['wget', 'http://stuff/blah'], {'a': 'n'}]}
218 cc = self._get_cloud('ubuntu')
219 with self.assertRaises(TypeError) as context_manager:
220- cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, [])
221+ handle('cc_bootcmd', invalid_config, cc, LOG, [])
222 expected_warnings = [
223 'bootcmd.1: 20 is not valid under any of the given schemas',
224 'bootcmd.3: {\'a\': \'n\'} is not valid under any of the given'
225@@ -117,7 +118,7 @@ class TestBootcmd(CiTestCase):
226 'echo {0} $INSTANCE_ID > {1}'.format(my_id, out_file)]}
227
228 with mock.patch(self._etmpfile_path, FakeExtendedTempFile):
229- cc_bootcmd.handle('cc_bootcmd', valid_config, cc, LOG, [])
230+ handle('cc_bootcmd', valid_config, cc, LOG, [])
231 self.assertEqual(my_id + ' iid-datasource-none\n',
232 util.load_file(out_file))
233
234@@ -128,7 +129,7 @@ class TestBootcmd(CiTestCase):
235
236 with mock.patch(self._etmpfile_path, FakeExtendedTempFile):
237 with self.assertRaises(util.ProcessExecutionError) as ctxt_manager:
238- cc_bootcmd.handle('does-not-matter', valid_config, cc, LOG, [])
239+ handle('does-not-matter', valid_config, cc, LOG, [])
240 self.assertIn(
241 'Unexpected error while running command.\n'
242 "Command: ['/bin/sh',",
243@@ -138,26 +139,21 @@ class TestBootcmd(CiTestCase):
244 self.logs.getvalue())
245
246
247-class TestSchema(CiTestCase):
248+@skipUnlessJsonSchema()
249+class TestSchema(CiTestCase, SchemaTestCaseMixin):
250 """Directly test schema rather than through handle."""
251
252+ schema = schema
253+
254 def test_duplicates_are_fine_array_array(self):
255- """Duplicated array entries are allowed."""
256- try:
257- byebye = ["echo", "bye"]
258- schema.validate_cloudconfig_schema(
259- {'bootcmd': [byebye, byebye]}, cc_bootcmd.schema, strict=True)
260- except schema.SchemaValidationError as e:
261- self.fail("runcmd entries as list are allowed to be duplicates.")
262+ """Duplicated commands array/array entries are allowed."""
263+ self.assertSchemaValid(
264+ ["byebye", "byebye"], 'command entries can be duplicate')
265
266 def test_duplicates_are_fine_array_string(self):
267- """Duplicated array entries entries are allowed in values of array."""
268- try:
269- byebye = "echo bye"
270- schema.validate_cloudconfig_schema(
271- {'bootcmd': [byebye, byebye]}, cc_bootcmd.schema, strict=True)
272- except schema.SchemaValidationError as e:
273- self.fail("runcmd entries are allowed to be duplicates.")
274+ """Duplicated commands array/string entries are allowed."""
275+ self.assertSchemaValid(
276+ ["echo bye", "echo bye"], "command entries can be duplicate.")
277
278
279 # vi: ts=4 expandtab
280diff --git a/tests/unittests/test_handler/test_handler_runcmd.py b/tests/unittests/test_handler/test_handler_runcmd.py
281index ee1981d..9ce334a 100644
282--- a/tests/unittests/test_handler/test_handler_runcmd.py
283+++ b/tests/unittests/test_handler/test_handler_runcmd.py
284@@ -1,10 +1,11 @@
285 # This file is part of cloud-init. See LICENSE file for license information.
286
287-from cloudinit.config import cc_runcmd, schema
288+from cloudinit.config.cc_runcmd import handle, schema
289 from cloudinit.sources import DataSourceNone
290 from cloudinit import (distros, helpers, cloud, util)
291 from cloudinit.tests.helpers import (
292- CiTestCase, FilesystemMockingTestCase, skipUnlessJsonSchema)
293+ CiTestCase, FilesystemMockingTestCase, SchemaTestCaseMixin,
294+ skipUnlessJsonSchema)
295
296 import logging
297 import os
298@@ -35,7 +36,7 @@ class TestRuncmd(FilesystemMockingTestCase):
299 """When the provided config doesn't contain runcmd, skip it."""
300 cfg = {}
301 mycloud = self._get_cloud('ubuntu')
302- cc_runcmd.handle('notimportant', cfg, mycloud, LOG, None)
303+ handle('notimportant', cfg, mycloud, LOG, None)
304 self.assertIn(
305 "Skipping module named notimportant, no 'runcmd' key",
306 self.logs.getvalue())
307@@ -44,7 +45,7 @@ class TestRuncmd(FilesystemMockingTestCase):
308 """Commands which can't be converted to shell will raise errors."""
309 invalid_config = {'runcmd': 1}
310 cc = self._get_cloud('ubuntu')
311- cc_runcmd.handle('cc_runcmd', invalid_config, cc, LOG, [])
312+ handle('cc_runcmd', invalid_config, cc, LOG, [])
313 self.assertIn(
314 'Failed to shellify 1 into file'
315 ' /var/lib/cloud/instances/iid-datasource-none/scripts/runcmd',
316@@ -59,7 +60,7 @@ class TestRuncmd(FilesystemMockingTestCase):
317 """
318 invalid_config = {'runcmd': 1}
319 cc = self._get_cloud('ubuntu')
320- cc_runcmd.handle('cc_runcmd', invalid_config, cc, LOG, [])
321+ handle('cc_runcmd', invalid_config, cc, LOG, [])
322 self.assertIn(
323 'Invalid config:\nruncmd: 1 is not of type \'array\'',
324 self.logs.getvalue())
325@@ -75,7 +76,7 @@ class TestRuncmd(FilesystemMockingTestCase):
326 invalid_config = {
327 'runcmd': ['ls /', 20, ['wget', 'http://stuff/blah'], {'a': 'n'}]}
328 cc = self._get_cloud('ubuntu')
329- cc_runcmd.handle('cc_runcmd', invalid_config, cc, LOG, [])
330+ handle('cc_runcmd', invalid_config, cc, LOG, [])
331 expected_warnings = [
332 'runcmd.1: 20 is not valid under any of the given schemas',
333 'runcmd.3: {\'a\': \'n\'} is not valid under any of the given'
334@@ -90,7 +91,7 @@ class TestRuncmd(FilesystemMockingTestCase):
335 """Valid runcmd schema is written to a runcmd shell script."""
336 valid_config = {'runcmd': [['ls', '/']]}
337 cc = self._get_cloud('ubuntu')
338- cc_runcmd.handle('cc_runcmd', valid_config, cc, LOG, [])
339+ handle('cc_runcmd', valid_config, cc, LOG, [])
340 runcmd_file = os.path.join(
341 self.new_root,
342 'var/lib/cloud/instances/iid-datasource-none/scripts/runcmd')
343@@ -99,25 +100,22 @@ class TestRuncmd(FilesystemMockingTestCase):
344 self.assertEqual(0o700, stat.S_IMODE(file_stat.st_mode))
345
346
347-class TestSchema(CiTestCase):
348+@skipUnlessJsonSchema()
349+class TestSchema(CiTestCase, SchemaTestCaseMixin):
350 """Directly test schema rather than through handle."""
351
352- def test_duplicates_are_fine_array(self):
353- """Duplicated array entries are allowed."""
354- try:
355- byebye = ["echo", "bye"]
356- schema.validate_cloudconfig_schema(
357- {'runcmd': [byebye, byebye]}, cc_runcmd.schema, strict=True)
358- except schema.SchemaValidationError as e:
359- self.fail("runcmd entries as list are allowed to be duplicates.")
360-
361- def test_duplicates_are_fine_string(self):
362- """Duplicated string entries are allowed."""
363- try:
364- byebye = "echo bye"
365- schema.validate_cloudconfig_schema(
366- {'runcmd': [byebye, byebye]}, cc_runcmd.schema, strict=True)
367- except schema.SchemaValidationError as e:
368- self.fail("runcmd entries are allowed to be duplicates.")
369+ schema = schema
370+
371+ def test_duplicates_are_fine_array_array(self):
372+ """Duplicated commands array/array entries are allowed."""
373+ self.assertSchemaValid(
374+ [["echo", "bye"], ["echo", "bye"]],
375+ "command entries can be duplicate.")
376+
377+ def test_duplicates_are_fine_array_string(self):
378+ """Duplicated commands array/string entries are allowed."""
379+ self.assertSchemaValid(
380+ ["echo bye", "echo bye"],
381+ "command entries can be duplicate.")
382
383 # vi: ts=4 expandtab

Subscribers

People subscribed via source and target branches