From 45eccc4e25934fbc97051843160afa632640a393 Mon Sep 17 00:00:00 2001 From: Benjamin Michael Cooper Date: Fri, 25 Aug 2017 13:10:15 +0100 Subject: [PATCH] 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 | 11 +- bareon/drivers/data/validate_anyof.py | 77 ++++++++++++ bareon/drivers/data/validate_schema.py | 41 +++++++ bareon/errors.py | 6 +- bareon/tests/test_validate_schema.py | 158 +++++++++++++++++++++++++ 5 files changed, 281 insertions(+), 12 deletions(-) create mode 100644 bareon/drivers/data/validate_anyof.py create mode 100644 bareon/drivers/data/validate_schema.py create mode 100644 bareon/tests/test_validate_schema.py diff --git a/bareon/drivers/data/__init__.py b/bareon/drivers/data/__init__.py index 5572807..71443d5 100644 --- a/bareon/drivers/data/__init__.py +++ b/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) - cls.check_schema(schema) - schema_validator = cls(schema, format_checker=jsonschema.FormatChecker()) - - defects = schema_validator.iter_errors(payload) - defects = list(defects) + defects = validate_schema(schema, payload) if defects: raise errors.InputDataSchemaValidationError(defects) diff --git a/bareon/drivers/data/validate_anyof.py b/bareon/drivers/data/validate_anyof.py new file mode 100644 index 0000000..edb6044 --- /dev/null +++ b/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) diff --git a/bareon/drivers/data/validate_schema.py b/bareon/drivers/data/validate_schema.py new file mode 100644 index 0000000..c00179d --- /dev/null +++ b/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) diff --git a/bareon/errors.py b/bareon/errors.py index 6bd2e19..b4c0d76 100644 --- a/bareon/errors.py +++ b/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 diff --git a/bareon/tests/test_validate_schema.py b/bareon/tests/test_validate_schema.py new file mode 100644 index 0000000..f2edcd2 --- /dev/null +++ b/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()