← Back to team overview

cloud-init-dev team mailing list archive

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

 

Scott Moser has proposed merging ~smoser/cloud-init:cleanup/jsonschema-follow-on into cloud-init:master.

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.

Requested reviews:
  cloud-init commiters (cloud-init-dev)

For more details, see:
https://code.launchpad.net/~smoser/cloud-init/+git/cloud-init/+merge/343609

see commit message
-- 
Your team cloud-init commiters is requested to review the proposed merge of ~smoser/cloud-init:cleanup/jsonschema-follow-on into cloud-init:master.
diff --git a/cloudinit/config/schema.py b/cloudinit/config/schema.py
index ca7d0d5..49ada0b 100644
--- a/cloudinit/config/schema.py
+++ b/cloudinit/config/schema.py
@@ -74,6 +74,8 @@ def validate_cloudconfig_schema(config, schema, strict=False):
     try:
         from jsonschema import Draft4Validator, FormatChecker
     except ImportError:
+        if strict:
+            raise
         logging.debug(
             'Ignoring schema validation. python-jsonschema is not present')
         return
diff --git a/cloudinit/config/tests/test_snap.py b/cloudinit/config/tests/test_snap.py
index 492d2d4..cb0e5bb 100644
--- a/cloudinit/config/tests/test_snap.py
+++ b/cloudinit/config/tests/test_snap.py
@@ -6,7 +6,8 @@ from six import StringIO
 from cloudinit.config.cc_snap import (
     ASSERTIONS_FILE, add_assertions, handle, maybe_install_squashfuse,
     run_commands, schema)
-from cloudinit.config.schema import validate_cloudconfig_schema
+from cloudinit.config.schema import (
+    SchemaValidationError, validate_cloudconfig_schema)
 from cloudinit import util
 from cloudinit.tests.helpers import (
     CiTestCase, mock, wrap_and_call, skipUnlessJsonSchema)
@@ -340,41 +341,36 @@ class TestSchema(CiTestCase):
             {'snap': {'assertions': {'01': 'also valid'}}}, schema)
         self.assertEqual('', self.logs.getvalue())
 
+    def _validate_or_fail(self, cfg, msg):
+        try:
+            validate_cloudconfig_schema(
+                {'snap': cfg}, schema, strict=True)
+        except SchemaValidationError as e:
+            self.fail(msg)
+
     def test_duplicates_are_fine_array_array(self):
         """Duplicated commands array/array entries are allowed."""
-        byebye = ["echo", "bye"]
-        try:
-            cfg = {'snap': {'commands': [byebye, byebye]}}
-            validate_cloudconfig_schema(cfg, schema, strict=True)
-        except schema.SchemaValidationError as e:
-            self.fail("command entries can be duplicate.")
+        self._validate_or_fail(
+            {'commands': [["echo", "bye"], ["echo", "bye"]]},
+            "command entries can be duplicate.")
 
     def test_duplicates_are_fine_array_string(self):
         """Duplicated commands array/string entries are allowed."""
-        byebye = "echo bye"
-        try:
-            cfg = {'snap': {'commands': [byebye, byebye]}}
-            validate_cloudconfig_schema(cfg, schema, strict=True)
-        except schema.SchemaValidationError as e:
-            self.fail("command entries can be duplicate.")
+        self._validate_or_fail(
+            {'commands': ["echo bye", "echo bye"]},
+            "command entries can be duplicate.")
 
     def test_duplicates_are_fine_dict_array(self):
         """Duplicated commands dict/array entries are allowed."""
-        byebye = ["echo", "bye"]
-        try:
-            cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
-            validate_cloudconfig_schema(cfg, schema, strict=True)
-        except schema.SchemaValidationError as e:
-            self.fail("command entries can be duplicate.")
+        self._validate_or_fail(
+            {'commands': {'00': ["echo", "bye"], '01': ["echo", "bye"]}},
+            "command entries can be duplicate.")
 
     def test_duplicates_are_fine_dict_string(self):
         """Duplicated commands dict/string entries are allowed."""
-        byebye = "echo bye"
-        try:
-            cfg = {'snap': {'commands': {'00': byebye, '01': byebye}}}
-            validate_cloudconfig_schema(cfg, schema, strict=True)
-        except schema.SchemaValidationError as e:
-            self.fail("command entries can be duplicate.")
+        self._validate_or_fail(
+            {'commands': {'00': "echo bye", '01': "echo bye"}},
+            "command entries can be duplicate.")
 
 
 class TestHandle(CiTestCase):
diff --git a/cloudinit/config/tests/test_ubuntu_advantage.py b/cloudinit/config/tests/test_ubuntu_advantage.py
index f2a59fa..2c1a014 100644
--- a/cloudinit/config/tests/test_ubuntu_advantage.py
+++ b/cloudinit/config/tests/test_ubuntu_advantage.py
@@ -5,7 +5,8 @@ from six import StringIO
 
 from cloudinit.config.cc_ubuntu_advantage import (
     handle, maybe_install_ua_tools, run_commands, schema)
-from cloudinit.config.schema import validate_cloudconfig_schema
+from cloudinit.config.schema import (
+    SchemaValidationError, validate_cloudconfig_schema)
 from cloudinit import util
 from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJsonSchema
 
@@ -169,6 +170,37 @@ class TestSchema(CiTestCase):
             {'ubuntu-advantage': {'commands': {'01': 'also valid'}}}, schema)
         self.assertEqual('', self.logs.getvalue())
 
+    def _validate_or_fail(self, cfg, msg):
+        try:
+            validate_cloudconfig_schema(
+                {'ubuntu-advantage': cfg}, schema, strict=True)
+        except SchemaValidationError as e:
+            self.fail(msg)
+
+    def test_duplicates_are_fine_array_array(self):
+        """Duplicated commands array/array entries are allowed."""
+        self._validate_or_fail(
+            {'commands': [["echo", "bye"], ["echo", "bye"]]},
+            "command entries can be duplicate.")
+
+    def test_duplicates_are_fine_array_string(self):
+        """Duplicated commands array/string entries are allowed."""
+        self._validate_or_fail(
+            {'commands': ["echo bye", "echo bye"]},
+            "command entries can be duplicate.")
+
+    def test_duplicates_are_fine_dict_array(self):
+        """Duplicated commands dict/array entries are allowed."""
+        self._validate_or_fail(
+            {'commands': {'00': ["echo", "bye"], '01': ["echo", "bye"]}},
+            "command entries can be duplicate.")
+
+    def test_duplicates_are_fine_dict_string(self):
+        """Duplicated commands dict/string entries are allowed."""
+        self._validate_or_fail(
+            {'commands': {'00': "echo bye", '01': "echo bye"}},
+            "command entries can be duplicate.")
+
 
 class TestHandle(CiTestCase):
 
diff --git a/tests/unittests/test_handler/test_handler_bootcmd.py b/tests/unittests/test_handler/test_handler_bootcmd.py
index c3abedd..9d0d1b8 100644
--- a/tests/unittests/test_handler/test_handler_bootcmd.py
+++ b/tests/unittests/test_handler/test_handler_bootcmd.py
@@ -1,7 +1,9 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
-from cloudinit.config import cc_bootcmd, schema
+from cloudinit.config.cc_bootcmd import handle, schema
 from cloudinit.sources import DataSourceNone
+from cloudinit.config.schema import (
+    SchemaValidationError, validate_cloudconfig_schema)
 from cloudinit import (distros, helpers, cloud, util)
 from cloudinit.tests.helpers import CiTestCase, mock, skipUnlessJsonSchema
 
@@ -50,7 +52,7 @@ class TestBootcmd(CiTestCase):
         """When the provided config doesn't contain bootcmd, skip it."""
         cfg = {}
         mycloud = self._get_cloud('ubuntu')
-        cc_bootcmd.handle('notimportant', cfg, mycloud, LOG, None)
+        handle('notimportant', cfg, mycloud, LOG, None)
         self.assertIn(
             "Skipping module named notimportant, no 'bootcmd' key",
             self.logs.getvalue())
@@ -60,7 +62,7 @@ class TestBootcmd(CiTestCase):
         invalid_config = {'bootcmd': 1}
         cc = self._get_cloud('ubuntu')
         with self.assertRaises(TypeError) as context_manager:
-            cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, [])
+            handle('cc_bootcmd', invalid_config, cc, LOG, [])
         self.assertIn('Failed to shellify bootcmd', self.logs.getvalue())
         self.assertEqual(
             "Input to shellify was type 'int'. Expected list or tuple.",
@@ -76,7 +78,7 @@ class TestBootcmd(CiTestCase):
         invalid_config = {'bootcmd': 1}
         cc = self._get_cloud('ubuntu')
         with self.assertRaises(TypeError):
-            cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, [])
+            handle('cc_bootcmd', invalid_config, cc, LOG, [])
         self.assertIn(
             'Invalid config:\nbootcmd: 1 is not of type \'array\'',
             self.logs.getvalue())
@@ -93,7 +95,7 @@ class TestBootcmd(CiTestCase):
             'bootcmd': ['ls /', 20, ['wget', 'http://stuff/blah'], {'a': 'n'}]}
         cc = self._get_cloud('ubuntu')
         with self.assertRaises(TypeError) as context_manager:
-            cc_bootcmd.handle('cc_bootcmd', invalid_config, cc, LOG, [])
+            handle('cc_bootcmd', invalid_config, cc, LOG, [])
         expected_warnings = [
             'bootcmd.1: 20 is not valid under any of the given schemas',
             'bootcmd.3: {\'a\': \'n\'} is not valid under any of the given'
@@ -117,7 +119,7 @@ class TestBootcmd(CiTestCase):
             'echo {0} $INSTANCE_ID > {1}'.format(my_id, out_file)]}
 
         with mock.patch(self._etmpfile_path, FakeExtendedTempFile):
-            cc_bootcmd.handle('cc_bootcmd', valid_config, cc, LOG, [])
+            handle('cc_bootcmd', valid_config, cc, LOG, [])
         self.assertEqual(my_id + ' iid-datasource-none\n',
                          util.load_file(out_file))
 
@@ -128,7 +130,7 @@ class TestBootcmd(CiTestCase):
 
         with mock.patch(self._etmpfile_path, FakeExtendedTempFile):
             with self.assertRaises(util.ProcessExecutionError) as ctxt_manager:
-                cc_bootcmd.handle('does-not-matter', valid_config, cc, LOG, [])
+                handle('does-not-matter', valid_config, cc, LOG, [])
         self.assertIn(
             'Unexpected error while running command.\n'
             "Command: ['/bin/sh',",
@@ -138,26 +140,28 @@ class TestBootcmd(CiTestCase):
             self.logs.getvalue())
 
 
+@skipUnlessJsonSchema()
 class TestSchema(CiTestCase):
     """Directly test schema rather than through handle."""
 
-    def test_duplicates_are_fine_array_array(self):
-        """Duplicated array entries are allowed."""
+    def _validate_or_fail(self, cfg, msg):
         try:
-            byebye = ["echo", "bye"]
-            schema.validate_cloudconfig_schema(
-                {'bootcmd': [byebye, byebye]}, cc_bootcmd.schema, strict=True)
-        except schema.SchemaValidationError as e:
-            self.fail("runcmd entries as list are allowed to be duplicates.")
+            validate_cloudconfig_schema(
+                {'bootcmd': cfg}, schema, strict=True)
+        except SchemaValidationError as e:
+            self.fail(msg)
+
+    def test_duplicates_are_fine_array_array(self):
+        """Duplicated commands array/array entries are allowed."""
+        self._validate_or_fail(
+            [["echo", "bye"], ["echo", "bye"]],
+            "command entries can be duplicate.")
 
     def test_duplicates_are_fine_array_string(self):
-        """Duplicated array entries entries are allowed in values of array."""
-        try:
-            byebye = "echo bye"
-            schema.validate_cloudconfig_schema(
-                {'bootcmd': [byebye, byebye]}, cc_bootcmd.schema, strict=True)
-        except schema.SchemaValidationError as e:
-            self.fail("runcmd entries are allowed to be duplicates.")
+        """Duplicated commands array/string entries are allowed."""
+        self._validate_or_fail(
+            ["echo bye", "echo bye"],
+            "command entries can be duplicate.")
 
 
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_handler/test_handler_runcmd.py b/tests/unittests/test_handler/test_handler_runcmd.py
index ee1981d..ae7aad8 100644
--- a/tests/unittests/test_handler/test_handler_runcmd.py
+++ b/tests/unittests/test_handler/test_handler_runcmd.py
@@ -1,7 +1,9 @@
 # This file is part of cloud-init. See LICENSE file for license information.
 
-from cloudinit.config import cc_runcmd, schema
+from cloudinit.config.cc_runcmd import handle, schema
 from cloudinit.sources import DataSourceNone
+from cloudinit.config.schema import (
+    SchemaValidationError, validate_cloudconfig_schema)
 from cloudinit import (distros, helpers, cloud, util)
 from cloudinit.tests.helpers import (
     CiTestCase, FilesystemMockingTestCase, skipUnlessJsonSchema)
@@ -35,7 +37,7 @@ class TestRuncmd(FilesystemMockingTestCase):
         """When the provided config doesn't contain runcmd, skip it."""
         cfg = {}
         mycloud = self._get_cloud('ubuntu')
-        cc_runcmd.handle('notimportant', cfg, mycloud, LOG, None)
+        handle('notimportant', cfg, mycloud, LOG, None)
         self.assertIn(
             "Skipping module named notimportant, no 'runcmd' key",
             self.logs.getvalue())
@@ -44,7 +46,7 @@ class TestRuncmd(FilesystemMockingTestCase):
         """Commands which can't be converted to shell will raise errors."""
         invalid_config = {'runcmd': 1}
         cc = self._get_cloud('ubuntu')
-        cc_runcmd.handle('cc_runcmd', invalid_config, cc, LOG, [])
+        handle('cc_runcmd', invalid_config, cc, LOG, [])
         self.assertIn(
             'Failed to shellify 1 into file'
             ' /var/lib/cloud/instances/iid-datasource-none/scripts/runcmd',
@@ -59,7 +61,7 @@ class TestRuncmd(FilesystemMockingTestCase):
         """
         invalid_config = {'runcmd': 1}
         cc = self._get_cloud('ubuntu')
-        cc_runcmd.handle('cc_runcmd', invalid_config, cc, LOG, [])
+        handle('cc_runcmd', invalid_config, cc, LOG, [])
         self.assertIn(
             'Invalid config:\nruncmd: 1 is not of type \'array\'',
             self.logs.getvalue())
@@ -75,7 +77,7 @@ class TestRuncmd(FilesystemMockingTestCase):
         invalid_config = {
             'runcmd': ['ls /', 20, ['wget', 'http://stuff/blah'], {'a': 'n'}]}
         cc = self._get_cloud('ubuntu')
-        cc_runcmd.handle('cc_runcmd', invalid_config, cc, LOG, [])
+        handle('cc_runcmd', invalid_config, cc, LOG, [])
         expected_warnings = [
             'runcmd.1: 20 is not valid under any of the given schemas',
             'runcmd.3: {\'a\': \'n\'} is not valid under any of the given'
@@ -90,7 +92,7 @@ class TestRuncmd(FilesystemMockingTestCase):
         """Valid runcmd schema is written to a runcmd shell script."""
         valid_config = {'runcmd': [['ls', '/']]}
         cc = self._get_cloud('ubuntu')
-        cc_runcmd.handle('cc_runcmd', valid_config, cc, LOG, [])
+        handle('cc_runcmd', valid_config, cc, LOG, [])
         runcmd_file = os.path.join(
             self.new_root,
             'var/lib/cloud/instances/iid-datasource-none/scripts/runcmd')
@@ -99,25 +101,27 @@ class TestRuncmd(FilesystemMockingTestCase):
         self.assertEqual(0o700, stat.S_IMODE(file_stat.st_mode))
 
 
+@skipUnlessJsonSchema()
 class TestSchema(CiTestCase):
     """Directly test schema rather than through handle."""
 
-    def test_duplicates_are_fine_array(self):
-        """Duplicated array entries are allowed."""
+    def _validate_or_fail(self, cfg, msg):
         try:
-            byebye = ["echo", "bye"]
-            schema.validate_cloudconfig_schema(
-                {'runcmd': [byebye, byebye]}, cc_runcmd.schema, strict=True)
-        except schema.SchemaValidationError as e:
-            self.fail("runcmd entries as list are allowed to be duplicates.")
-
-    def test_duplicates_are_fine_string(self):
-        """Duplicated string entries are allowed."""
-        try:
-            byebye = "echo bye"
-            schema.validate_cloudconfig_schema(
-                {'runcmd': [byebye, byebye]}, cc_runcmd.schema, strict=True)
-        except schema.SchemaValidationError as e:
-            self.fail("runcmd entries are allowed to be duplicates.")
+            validate_cloudconfig_schema(
+                {'runcmd': cfg}, schema, strict=True)
+        except SchemaValidationError as e:
+            self.fail(msg)
+
+    def test_duplicates_are_fine_array_array(self):
+        """Duplicated commands array/array entries are allowed."""
+        self._validate_or_fail(
+            [["echo", "bye"], ["echo", "bye"]],
+            "command entries can be duplicate.")
+
+    def test_duplicates_are_fine_array_string(self):
+        """Duplicated commands array/string entries are allowed."""
+        self._validate_or_fail(
+            ["echo bye", "echo bye"],
+            "command entries can be duplicate.")
 
 # vi: ts=4 expandtab
diff --git a/tests/unittests/test_handler/test_schema.py b/tests/unittests/test_handler/test_schema.py
index ac41f12..6791c56 100644
--- a/tests/unittests/test_handler/test_schema.py
+++ b/tests/unittests/test_handler/test_schema.py
@@ -351,6 +351,7 @@ class MainTest(CiTestCase):
         self.assertIn('\nNTP\n---\n', m_stdout.getvalue())
         self.assertIn('\nRuncmd\n------\n', m_stdout.getvalue())
 
+    @skipUnlessJsonSchema()
     def test_main_validates_config_file(self):
         """When --config-file parameter is provided, main validates schema."""
         myyaml = self.tmp_path('my.yaml')

Follow ups