diff --git a/sahara/service/validation.py b/sahara/service/validation.py index 297e2ab4..3422932f 100644 --- a/sahara/service/validation.py +++ b/sahara/service/validation.py @@ -15,7 +15,6 @@ import functools -import jsonschema from oslo_utils import reflection from sahara import exceptions as ex @@ -24,6 +23,21 @@ from sahara.utils import api as u from sahara.utils import api_validator +def _get_path(path): + if path: + path_string = path[0] + for x in path[1:]: + path_string += '[%s]' % str(x) + return path_string + ': ' + return '' + + +def _generate_error(errors): + message = [_get_path(list(e.path)) + e.message for e in errors] + if message: + return ex.SaharaException('\n'.join(message), "VALIDATION_ERROR") + + def validate(schema, *validators): def decorator(func): @functools.wraps(func) @@ -32,13 +46,13 @@ def validate(schema, *validators): try: if schema: validator = api_validator.ApiValidator(schema) - validator.validate(request_data) + errors = validator.iter_errors(request_data) + error = _generate_error(errors) + if error: + return u.bad_request(error) if validators: for validator in validators: validator(**kwargs) - except jsonschema.ValidationError as e: - e.code = "VALIDATION_ERROR" - return u.bad_request(e) except ex.SaharaException as e: return u.bad_request(e) except Exception as e: diff --git a/sahara/tests/unit/service/validation/edp/test_job.py b/sahara/tests/unit/service/validation/edp/test_job.py index f514034b..39520846 100644 --- a/sahara/tests/unit/service/validation/edp/test_job.py +++ b/sahara/tests/unit/service/validation/edp/test_job.py @@ -36,7 +36,7 @@ class TestJobCreateValidation(u.ValidationTestCase): "type": "Jar", }, bad_req_i=(1, "VALIDATION_ERROR", - "'Jar' is not one of " + str(edp.JOB_TYPES_ALL))) + "type: 'Jar' is not one of " + str(edp.JOB_TYPES_ALL))) @mock.patch('sahara.service.edp.api.get_job_binary') def test_check_binaries(self, get_job_binary): diff --git a/sahara/tests/unit/service/validation/edp/test_job_binary.py b/sahara/tests/unit/service/validation/edp/test_job_binary.py index a5eccd6a..d87971e2 100644 --- a/sahara/tests/unit/service/validation/edp/test_job_binary.py +++ b/sahara/tests/unit/service/validation/edp/test_job_binary.py @@ -62,5 +62,5 @@ class TestJobBinaryValidation(u.ValidationTestCase): "url": "internal-db://abacaba", }, bad_req_i=(1, "VALIDATION_ERROR", - "'internal-db://abacaba' is not a " + "url: 'internal-db://abacaba' is not a " "'valid_job_location'")) diff --git a/sahara/tests/unit/service/validation/test_add_tags_validation.py b/sahara/tests/unit/service/validation/test_add_tags_validation.py index 6fd8d596..ebd9447e 100644 --- a/sahara/tests/unit/service/validation/test_add_tags_validation.py +++ b/sahara/tests/unit/service/validation/test_add_tags_validation.py @@ -40,11 +40,11 @@ class TestTagsAddingValidation(u.ValidationTestCase): self._assert_create_object_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', - u"'%s' is not a 'valid_tag'" % tag)) + u"tags[0]: '%s' is not a 'valid_tag'" % tag)) for symb in wrong_symbols: tag = "a%sa" % symb data['tags'] = [tag] self._assert_create_object_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', - u"'%s' is not a 'valid_tag'" % tag)) + u"tags[0]: '%s' is not a 'valid_tag'" % tag)) diff --git a/sahara/tests/unit/service/validation/test_cluster_create_validation.py b/sahara/tests/unit/service/validation/test_cluster_create_validation.py index 2d6fadd2..5ae21014 100644 --- a/sahara/tests/unit/service/validation/test_cluster_create_validation.py +++ b/sahara/tests/unit/service/validation/test_cluster_create_validation.py @@ -114,7 +114,7 @@ class TestClusterCreateValidation(u.ValidationTestCase): 'hadoop_version': "0.1", 'user_keypair_id': '!'}, bad_req_i=(1, 'VALIDATION_ERROR', - "'!' is not a 'valid_keypair_name'") + "user_keypair_id: '!' is not a 'valid_keypair_name'") ) def test_cluster_create_v_image_exists(self): diff --git a/sahara/tests/unit/service/validation/test_cluster_scaling_validation.py b/sahara/tests/unit/service/validation/test_cluster_scaling_validation.py index 6d06d3ce..2834adf8 100644 --- a/sahara/tests/unit/service/validation/test_cluster_scaling_validation.py +++ b/sahara/tests/unit/service/validation/test_cluster_scaling_validation.py @@ -207,7 +207,7 @@ class TestScalingValidation(u.ValidationTestCase): self._assert_cluster_scaling_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', - u"'name' is a required property") + u"resize_node_groups[0]: 'name' is a required property") ) data = { 'resize_node_groups': [ @@ -219,7 +219,8 @@ class TestScalingValidation(u.ValidationTestCase): self._assert_cluster_scaling_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', - u"'count' is a required property") + u"resize_node_groups[0]: 'count' is a required " + u"property") ) @mock.patch("sahara.service.api.OPS") @@ -236,7 +237,7 @@ class TestScalingValidation(u.ValidationTestCase): self._assert_cluster_scaling_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', - "{'node_group_template_id': " + "add_node_groups[0]: {'node_group_template_id': " "'5185a809-6bf7-44ed-9de3-618270550e2c'} " "is not valid under any of the given schemas") ) @@ -252,7 +253,7 @@ class TestScalingValidation(u.ValidationTestCase): self._assert_cluster_scaling_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', - u"{'node_group_template_id': " + u"add_node_groups[0]: {'node_group_template_id': " u"'5185a809-6bf7-44ed-9de3-618270550e2c', " u"'name': 'a'} is not valid under any " u"of the given schemas") @@ -314,7 +315,7 @@ class TestScalingValidation(u.ValidationTestCase): self._assert_cluster_scaling_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', - u"{} is not of type 'array'") + u"resize_node_groups: {} is not of type 'array'") ) data = { 'add_node_groups': {} @@ -322,7 +323,7 @@ class TestScalingValidation(u.ValidationTestCase): self._assert_cluster_scaling_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', - u"{} is not of type 'array'") + u"add_node_groups: {} is not of type 'array'") ) data = { 'resize_node_groups': [], @@ -330,7 +331,7 @@ class TestScalingValidation(u.ValidationTestCase): self._assert_cluster_scaling_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', - u'[] is too short') + u'resize_node_groups: [] is too short') ) @mock.patch("sahara.service.api.OPS") diff --git a/sahara/tests/unit/service/validation/test_cluster_template_create_validation.py b/sahara/tests/unit/service/validation/test_cluster_template_create_validation.py index d55083c6..1c459b56 100644 --- a/sahara/tests/unit/service/validation/test_cluster_template_create_validation.py +++ b/sahara/tests/unit/service/validation/test_cluster_template_create_validation.py @@ -44,7 +44,7 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase): ] }, bad_req_i=(1, 'VALIDATION_ERROR', - "{'name': 'a'} is not valid under " + "node_groups[0]: {'name': 'a'} is not valid under " "any of the given schemas") ) self._assert_create_object_validation( @@ -58,7 +58,7 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase): ] }, bad_req_i=(1, "VALIDATION_ERROR", - "{'name': 'a', 'flavor_id': '42'} " + "node_groups[0]: {'name': 'a', 'flavor_id': '42'} " "is not valid under any of the given schemas") ) @@ -74,7 +74,7 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase): ] }, bad_req_i=(1, "VALIDATION_ERROR", - "{'node_processes': ['namenode'], " + "node_groups[0]: {'node_processes': ['namenode'], " "'name': 'a', " "'flavor_id': '42'} " "is not valid under any of the given schemas") @@ -117,7 +117,8 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase): ] }, bad_req_i=(1, "VALIDATION_ERROR", - "{'node_group_template_id': '', 'name': 'test'} " + "node_groups[0]: {'node_group_template_id': '', " + "'name': 'test'} " "is not valid under any of the given schemas") ) @@ -135,7 +136,7 @@ class TestClusterTemplateCreateValidation(u.ValidationTestCase): ] }, bad_req_i=(1, "VALIDATION_ERROR", - "{'count': 3, " + "node_groups[0]: {'count': 3, " "'node_group_template_id': 'test', " "'name': 'test'} " "is not valid under any of the given schemas") diff --git a/sahara/tests/unit/service/validation/test_ng_template_validation_create.py b/sahara/tests/unit/service/validation/test_ng_template_validation_create.py index 39a88c39..caecd327 100644 --- a/sahara/tests/unit/service/validation/test_ng_template_validation_create.py +++ b/sahara/tests/unit/service/validation/test_ng_template_validation_create.py @@ -70,7 +70,7 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase): 'node_processes': [] }, bad_req_i=(1, 'VALIDATION_ERROR', - u'\[\] is too short') + u'node_processes: \[\] is too short') ) def test_ng_template_create_v_names(self): @@ -186,7 +186,8 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase): 'volumes_per_node': -1 }, bad_req_i=(1, 'VALIDATION_ERROR', - u'-1(.0)? is less than the minimum of 0') + u'volumes_per_node: -1(.0)? is less than the minimum ' + u'of 0') ) self._assert_create_object_validation( data={ @@ -198,7 +199,7 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase): 'volumes_size': 0 }, bad_req_i=(1, 'VALIDATION_ERROR', - u'0(.0)? is less than the minimum of 1') + u'volumes_size: 0(.0)? is less than the minimum of 1') ) def test_ng_template_create_v_types(self): @@ -247,7 +248,7 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase): 'image_id': '12345' }, bad_req_i=(1, 'VALIDATION_ERROR', - "'12345' is not a 'uuid'") + "image_id: '12345' is not a 'uuid'") ) self._assert_create_object_validation( data={ @@ -335,7 +336,8 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase): 'volumes_per_node': -1 }, bad_req_i=(1, 'VALIDATION_ERROR', - u'-1(.0)? is less than the minimum of 0') + u'volumes_per_node: -1(.0)? is less than the minimum ' + u'of 0') ) self._assert_create_object_validation( data={ @@ -347,7 +349,7 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase): 'volumes_size': 0 }, bad_req_i=(1, 'VALIDATION_ERROR', - u'0(.0)? is less than the minimum of 1') + u'volumes_size: 0(.0)? is less than the minimum of 1') ) self._assert_create_object_validation( data={ @@ -373,7 +375,8 @@ class TestNGTemplateCreateValidation(u.ValidationTestCase): } self._assert_create_object_validation( data=data, - bad_req_i=(1, 'VALIDATION_ERROR', "'qwerty' is not a 'posix_path'") + bad_req_i=(1, 'VALIDATION_ERROR', "volume_mount_prefix: 'qwerty' " + "is not a 'posix_path'") ) def test_wrong_floating_ip_pool(self): diff --git a/sahara/tests/unit/service/validation/test_validation.py b/sahara/tests/unit/service/validation/test_validation.py new file mode 100644 index 00000000..8d10c0fa --- /dev/null +++ b/sahara/tests/unit/service/validation/test_validation.py @@ -0,0 +1,49 @@ +# Copyright (c) 2016 Mirantis Inc. +# +# 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 mock + +from sahara import exceptions +from sahara.service import validation +from sahara.tests.unit import base + + +class TestValidation(base.SaharaTestCase): + def test_get_path(self): + self.assertEqual('', validation._get_path([])) + self.assertEqual('param: ', validation._get_path(['param'])) + self.assertEqual( + 'param[0][id]: ', validation._get_path(['param', 0, 'id'])) + + def test_generate_error(self): + # no errors occurred during validation + self.assertIsNone(validation._generate_error([])) + + # one error occurred during validation + err = mock.Mock(message='some error', path=['param']) + generated_err = validation._generate_error([err]) + self.assertIsInstance(generated_err, exceptions.SaharaException) + self.assertEqual('VALIDATION_ERROR', generated_err.code) + self.assertIn('param: some error', generated_err.message) + + # several errors occurred during validation + errors = [mock.Mock(message='first error', path=['param1']), + mock.Mock( + message='second error', path=['param2', 0, 'subparam'])] + generated_err = validation._generate_error(errors) + self.assertIsInstance(generated_err, exceptions.SaharaException) + self.assertEqual('VALIDATION_ERROR', generated_err.code) + self.assertIn('param1: first error\nparam2[0][subparam]: second error', + generated_err.message) diff --git a/sahara/tests/unit/service/validation/utils.py b/sahara/tests/unit/service/validation/utils.py index 12cef144..d58cd3db 100644 --- a/sahara/tests/unit/service/validation/utils.py +++ b/sahara/tests/unit/service/validation/utils.py @@ -317,25 +317,25 @@ class ValidationTestCase(base.SaharaTestCase): self._assert_create_object_validation( data=data, bad_req_i=(1, "VALIDATION_ERROR", - u"None is not of type 'string'") + u"name: None is not of type 'string'") ) data.update({'name': ""}) self._assert_create_object_validation( data=data, bad_req_i=(1, "VALIDATION_ERROR", - u"'' is too short") + u"name: '' is too short") ) data.update({'name': ('a' * 51)}) self._assert_create_object_validation( data=data, bad_req_i=(1, "VALIDATION_ERROR", - u"'%s' is too long" % ('a' * 51)) + u"name: '%s' is too long" % ('a' * 51)) ) data.update({'name': 'a-!'}) self._assert_create_object_validation( data=data, bad_req_i=(1, "VALIDATION_ERROR", - u"'a-!' is not a 'valid_name_hostname'") + u"name: 'a-!' is not a 'valid_name_hostname'") ) def _prop_types_str(self, prop_types): @@ -356,12 +356,13 @@ class ValidationTestCase(base.SaharaTestCase): if isinstance(value, str): value_str = "'%s'" % value_str data.update({p_name: value}) - message = ("%s is not of type %s" % - (value_str, + message = ("%s: %s is not of type %s" % + (p_name, value_str, self._prop_types_str(prop_types))) if "enum" in prop: - message = [message, "%s is not one of %s" % - (value_str, prop["enum"])] + message = [message, "%s: %s is not one of %s" % + (p_name, value_str, + prop["enum"])] self._assert_create_object_validation( data=data, bad_req_i=(1, 'VALIDATION_ERROR', message)