Improve validation error message

If a sub section of a deploy schema is one of multiple options
python-jsonschema, which used to verify the schema, is unable to
determine which option is intended so cannot produce a useful
error message. As "type" is required by all partitions and volumes
this is be used to determine the correct sub schema, which can then
be validated recursively using python-jsonschema. The validation
error message then gives specific details on each incorrect
parameter.

Change-Id: Icb22b88b40f0fa45a3fe3932d69ba32e4a360edd

bareon/drivers/data/__init__.py

@@ -15,20 +15,15 @@
 
 import json
 
-import jsonschema.validators
-
 from bareon import errors
 
+from validate_schema import validate_schema
+
 
 def validate(schema_path, payload):
     schema = _load_validator_schema(schema_path)
 
-    cls = jsonschema.validators.validator_for(schema)
+    defects = validate_schema(schema, payload)
-    cls.check_schema(schema)
-    schema_validator = cls(schema, format_checker=jsonschema.FormatChecker())
-
-    defects = schema_validator.iter_errors(payload)
-    defects = list(defects)
     if defects:
         raise errors.InputDataSchemaValidationError(defects)
 

bareon/drivers/data/validate_anyof.py

@@ -0,0 +1,77 @@
+#
+# Copyright 2017 Cray Inc.  All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import jsonschema
+import jsonschema.exceptions
+
+import validate_schema
+
+
+class ValidateAnyOf(object):
+
+    def __init__(self, anyof_defect, schema):
+        self.anyof = anyof_defect
+        self.schema = schema
+
+        self.sub_schemas = self._get_sub_schemas()
+        self.defects = []
+
+        if "type" in self.anyof.instance:
+            permitted_types = []
+            validated = False
+
+            for sub_schema in self.sub_schemas:
+                permitted_types.extend(sub_schema
+                                       ['properties']['type']['enum'])
+                if self._verify_type_valid(sub_schema):
+                    self._validate_sub_schema(sub_schema)
+                    validated = True
+
+            if not validated:
+                invalid_type = self.anyof.instance['type']
+                message = " could not be validated, {!r} is not one of {!r}"\
+                    .format(invalid_type, permitted_types)
+                self._raise_validation_error(message)
+        else:
+            message = " could not be validated, u'type' is a required property"
+            self._raise_validation_error(message)
+
+    def _get_sub_schemas(self):
+        """Returns list of sub schemas in anyof defect"""
+        sub_schemas = self.schema
+        path_to_anyof = list(self.anyof.schema_path)
+        for path in path_to_anyof:
+            sub_schemas = sub_schemas[path]
+        return sub_schemas
+
+    def _verify_type_valid(self, sub_schema):
+        """Returns true if type is valid for given schema, false otherwise"""
+        defects = validate_schema.validate_schema(sub_schema
+                                                  ['properties']['type'],
+                                                  self.anyof.instance['type'])
+        return False if defects else True
+
+    def _validate_sub_schema(self, sub_schema):
+        """Performs validation on sub schemas"""
+        for defect in validate_schema.validate_schema(sub_schema,
+                                                      self.anyof.instance):
+            validate_schema.add_path_to_defect_message(self.anyof.path, defect)
+            self.defects.append(defect)
+
+    def _raise_validation_error(self, message):
+        """Adds ValidationError to defects with given message"""
+        defect = jsonschema.exceptions.ValidationError(message)
+        validate_schema.add_path_to_defect_message(self.anyof.path, defect)
+        self.defects.append(defect)

bareon/drivers/data/validate_schema.py

@@ -0,0 +1,41 @@
+#
+# Copyright 2017 Cray Inc.  All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import jsonschema.validators
+
+import validate_anyof
+
+
+def validate_schema(schema, payload):
+    cls = jsonschema.validators.validator_for(schema)
+    cls.check_schema(schema)
+    schema_validator = cls(schema, format_checker=jsonschema.FormatChecker())
+
+    defects = []
+    for defect in schema_validator.iter_errors(payload):
+        if defect.validator == "anyOf":
+            anyof_defects = validate_anyof.ValidateAnyOf(defect,
+                                                         schema).defects
+            defects.extend(anyof_defects)
+        else:
+            add_path_to_defect_message(defect.path, defect)
+            defects.append(defect)
+    return defects
+
+
+def add_path_to_defect_message(path, defect):
+    if path:
+        path_string = ':'.join((str(x) for x in list(path)))
+        defect.message = '{}:{}'.format(path_string, defect.message)

bareon/errors.py

@@ -55,17 +55,15 @@ class InputDataSchemaValidationError(WrongInputDataError):
     def __init__(self, defects):
         human_readable_defects = []
         for idx, d in enumerate(defects):
-            path = list(d.path)
-            path = '/'.join((str(x) for x in path))
             human_readable_defects.append(
-                '{:>2} (/{}): {}'.format('#{}'.format(idx), path, d.message))
+                '[ERROR{:>2}] {}'.format(' {}'.format(idx + 1), d.message))
 
         indent = ' ' * 4
         separator = '\n{}'.format(indent)
         message = 'Invalid input data:\n{}{}'.format(
             indent, separator.join(human_readable_defects))
 
-        super(WrongInputDataError, self).__init__(message, defects)
+        super(WrongInputDataError, self).__init__(message)
         self.defects = defects
 
 

bareon/tests/test_validate_schema.py

@@ -0,0 +1,158 @@
+#
+# Copyright 2017 Cray Inc.  All Rights Reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+import pkg_resources
+import unittest
+
+import bareon.drivers.data
+
+from bareon import errors
+
+
+class TestValidateSchema(unittest.TestCase):
+    def setUp(self):
+        self.validation_path = pkg_resources.resource_filename(
+            'bareon.drivers.data.json_schemes', 'ironic.json')
+        self.default_message = "Invalid input data:\n"
+        self.deploy_schema = {
+            "images": [
+                {
+                    "name": "centos",
+                    "image_pull_url": "centos-7.1.1503.raw",
+                    "target": "/"
+                }
+            ],
+            "image_deploy_flags": {
+                "rsync_flags": "-a -A -X --timeout 300"
+            },
+            "partitions": [
+                {
+                    "id": {
+                        "type": "name",
+                        "value": "vda"
+                    },
+                    "size": "15000 MB",
+                    "type": "disk",
+                    "volumes": [
+                        {
+                            "file_system": "ext4",
+                            "mount": "/",
+                            "size": "10000 MB",
+                            "type": "partition"
+                        },
+                        {
+                            "size": "remaining",
+                            "type": "pv",
+                            "vg": "volume_group"
+                        }
+                    ]
+                },
+                {
+                    "id": "volume_group",
+                    "type": "vg",
+                    "volumes": [
+                        {
+                            "file_system": "ext3",
+                            "mount": "/home",
+                            "name": "home",
+                            "size": "3000 MB",
+                            "type": "lv"
+                        },
+                        {
+                            "file_system": "ext3",
+                            "mount": "/var",
+                            "name": "var",
+                            "size": "remaining",
+                            "type": "lv"
+                        }
+
+                    ]
+                }
+            ],
+            "partitions_policy": "clean"
+        }
+
+    def test_working(self):
+        bareon.drivers.data.validate(self.validation_path,
+                                     self.deploy_schema)
+
+    def test_partitions_size_missing(self):
+        del self.deploy_schema["partitions"][0]["size"]
+        err_message = (
+            self.default_message +
+            "    [ERROR 1] partitions:0:u'size' is a required property")
+        with self.assertRaises(errors.InputDataSchemaValidationError) as err:
+            bareon.drivers.data.validate(self.validation_path,
+                                         self.deploy_schema)
+        self.assertEqual(str(err.exception), err_message)
+
+    def test_partitions_size_not_string(self):
+        self.deploy_schema["partitions"][0]["size"] = []
+        err_message = (
+            self.default_message +
+            "    [ERROR 1] partitions:0:size:[] is not of type u'string'")
+        with self.assertRaises(errors.InputDataSchemaValidationError) as err:
+            bareon.drivers.data.validate(self.validation_path,
+                                         self.deploy_schema)
+        self.assertEqual(str(err.exception), err_message)
+
+    def test_partitions_type_missing(self):
+        del self.deploy_schema["partitions"][0]["type"]
+        err_message = (
+            self.default_message +
+            "    [ERROR 1] partitions:0: could not be validated, "
+            "u'type' is a required property")
+        with self.assertRaises(errors.InputDataSchemaValidationError) as err:
+            bareon.drivers.data.validate(self.validation_path,
+                                         self.deploy_schema)
+        self.assertEqual(str(err.exception), err_message)
+
+    def test_partitions_type_not_valid(self):
+        self.deploy_schema["partitions"][0]["type"] = "invalid"
+        err_message = (
+            self.default_message +
+            "    [ERROR 1] partitions:0: could not be validated, "
+            "'invalid' is not one of [u'disk', u'vg']")
+        with self.assertRaises(errors.InputDataSchemaValidationError) as err:
+            bareon.drivers.data.validate(self.validation_path,
+                                         self.deploy_schema)
+        self.assertEqual(str(err.exception), err_message)
+
+    def test_volumes_type_missing(self):
+        del self.deploy_schema["partitions"][0]["volumes"][0]["type"]
+        err_message = (
+            self.default_message +
+            "    [ERROR 1] partitions:0:volumes:0: could not be validated, "
+            "u'type' is a required property")
+        with self.assertRaises(errors.InputDataSchemaValidationError) as err:
+            bareon.drivers.data.validate(self.validation_path,
+                                         self.deploy_schema)
+        self.assertEqual(str(err.exception), err_message)
+
+    def test_volumes_type_not_valid(self):
+        self.deploy_schema["partitions"][0]["volumes"][0]["type"] = "invalid"
+        err_message = (
+            self.default_message +
+            "    [ERROR 1] partitions:0:volumes:0: could not be validated, "
+            "'invalid' is not one of [u'pv', u'raid', u'partition', u'boot', "
+            "u'lvm_meta_pool']")
+        with self.assertRaises(errors.InputDataSchemaValidationError) as err:
+            bareon.drivers.data.validate(self.validation_path,
+                                         self.deploy_schema)
+        self.assertEqual(str(err.exception), err_message)
+
+
+if __name__ == '__main__':
+    unittest.main()