From 28b071f5611f571bff399ef3c4c092dfa973dcd1 Mon Sep 17 00:00:00 2001 From: Graham Hayes Date: Mon, 13 Jul 2015 19:55:00 +0100 Subject: [PATCH] Fixup v2 API Validation * Added list type checking to recordset.records * Added list type checking to domain.masters * Added extra debug output * Catch all Exception clause in objects.adaptors.base.parse() Change-Id: I900d418c16f016e0bd4e3f30cec393734b2feca4 Closes-Bug: #1473212 --- designate/objects/adapters/api_v2/domain.py | 20 +++++- .../objects/adapters/api_v2/recordset.py | 19 +++++- designate/objects/adapters/base.py | 65 +++++++++++++++---- designate/objects/base.py | 7 ++ designate/objects/recordset.py | 45 +++++++++++++ 5 files changed, 140 insertions(+), 16 deletions(-) diff --git a/designate/objects/adapters/api_v2/domain.py b/designate/objects/adapters/api_v2/domain.py index 60ee0fff5..5158ff13e 100644 --- a/designate/objects/adapters/api_v2/domain.py +++ b/designate/objects/adapters/api_v2/domain.py @@ -15,6 +15,8 @@ from oslo_log import log as logging from designate.objects.adapters.api_v2 import base from designate import objects +from designate import exceptions + LOG = logging.getLogger(__name__) @@ -66,8 +68,22 @@ class DomainAPIv2Adapter(base.APIv2Adapter): # https://bugs.launchpad.net/designate/+bug/1432842 is fixed if 'masters' in values: - object.set_masters(values.get('masters')) - del values['masters'] + if isinstance(values['masters'], list): + object.set_masters(values.get('masters')) + del values['masters'] + else: + errors = objects.ValidationErrorList() + e = objects.ValidationError() + e.path = ['masters'] + e.validator = 'type' + e.validator_value = ["list"] + e.message = ("'%(data)s' is not a valid list of masters" + % {'data': values['masters']}) + # Add it to the list for later + errors.append(e) + raise exceptions.InvalidObject( + "Provided object does not match " + "schema", errors=errors, object=cls.ADAPTER_OBJECT()) return super(DomainAPIv2Adapter, cls)._parse_object( values, object, *args, **kwargs) diff --git a/designate/objects/adapters/api_v2/recordset.py b/designate/objects/adapters/api_v2/recordset.py index 04a3f4a98..33d4d055c 100644 --- a/designate/objects/adapters/api_v2/recordset.py +++ b/designate/objects/adapters/api_v2/recordset.py @@ -73,8 +73,23 @@ class RecordSetAPIv2Adapter(base.APIv2Adapter): # Get new list of Records new_records = set() if 'records' in new_recordset: - for record in new_recordset['records']: - new_records.add(record) + if isinstance(new_recordset['records'], list): + for record in new_recordset['records']: + new_records.add(record) + else: + errors = objects.ValidationErrorList() + e = objects.ValidationError() + e.path = ['records'] + e.validator = 'type' + e.validator_value = ["list"] + e.message = ("'%(data)s' is not a valid list of records" + % {'data': new_recordset['records']}) + # Add it to the list for later + errors.append(e) + raise exceptions.InvalidObject( + "Provided object does not match " + "schema", errors=errors, object=cls.ADAPTER_OBJECT()) + # Get differences of Records records_to_add = new_records.difference(original_records) records_to_rm = original_records.difference(new_records) diff --git a/designate/objects/adapters/base.py b/designate/objects/adapters/base.py index 145ead6f1..06663025f 100644 --- a/designate/objects/adapters/base.py +++ b/designate/objects/adapters/base.py @@ -17,6 +17,8 @@ import six from designate import objects from designate import exceptions +from designate.i18n import _LE +from designate.i18n import _LI LOG = logging.getLogger(__name__) @@ -139,18 +141,57 @@ class DesignateAdapter(object): @classmethod def parse(cls, format_, values, output_object, *args, **kwargs): - if isinstance(output_object, objects.ListObjectMixin): - # type_ = 'list' - return cls.get_object_adapter( - format_, - output_object)._parse_list( - values, output_object, *args, **kwargs) - else: - # type_ = 'object' - return cls.get_object_adapter( - format_, - output_object)._parse_object( - values, output_object, *args, **kwargs) + LOG.debug("Creating %s object with values %r" % + (output_object.obj_name(), values)) + + try: + if isinstance(output_object, objects.ListObjectMixin): + # type_ = 'list' + return cls.get_object_adapter( + format_, + output_object)._parse_list( + values, output_object, *args, **kwargs) + else: + # type_ = 'object' + return cls.get_object_adapter( + format_, + output_object)._parse_object( + values, output_object, *args, **kwargs) + + except TypeError as e: + LOG.exception(_LE("TypeError creating %(name)s with values" + " %(values)r") % + {"name": output_object.obj_name(), "values": values}) + + error_message = str.format( + 'Provided object does not match schema. ' + 'Got a TypeError with message %s' % six.text_type(e)) + raise exceptions.InvalidObject(error_message) + + except AttributeError as e: + LOG.exception(_LE("AttributeError creating %(name)s " + "with values %(values)r") % + {"name": output_object.obj_name(), "values": values}) + error_message = str.format( + 'Provided object is not valid. ' + 'Got an AttributeError with message %s' % six.text_type(e)) + raise exceptions.InvalidObject(error_message) + + except exceptions.InvalidObject: + LOG.info(_LI("InvalidObject creating %(name)s with " + "values %(values)r") % + {"name": output_object.obj_name(), "values": values}) + raise + + except Exception as e: + LOG.exception(_LE("Exception creating %(name)s with " + "values %(values)r") % + {"name": output_object.obj_name(), "values": values}) + error_message = str.format( + 'Provided object is not valid. ' + 'Got a %s error with message %s' % + (type(e).__name__, six.text_type(e))) + raise exceptions.InvalidObject(error_message) @classmethod def _parse_object(cls, values, output_object, *args, **kwargs): diff --git a/designate/objects/base.py b/designate/objects/base.py index 74b284f14..1a23eca4b 100644 --- a/designate/objects/base.py +++ b/designate/objects/base.py @@ -306,6 +306,13 @@ class DesignateObject(object): errors.append(ValidationError.from_js_error(error)) if len(errors) > 0: + LOG.debug( + "Error Validating '%(name)s' object with values: " + "%(values)r", { + 'name': self.obj_name(), + 'values': values, + } + ) raise exceptions.InvalidObject( "Provided object does not match " "schema", errors=errors, object=self) diff --git a/designate/objects/recordset.py b/designate/objects/recordset.py index 1b6bbc7d6..7fa6438d7 100644 --- a/designate/objects/recordset.py +++ b/designate/objects/recordset.py @@ -12,9 +12,12 @@ # 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 logging from copy import deepcopy +import six + from designate import exceptions from designate import utils from designate.objects import base @@ -129,6 +132,11 @@ class RecordSet(base.DictObjectMixin, base.PersistentObjectMixin, def validate(self): + LOG.debug("Validating '%(name)s' object with values: %(values)r", { + 'name': self.obj_name(), + 'values': self.to_dict(), + }) + errors = ValidationErrorList() # Get the right classes (e.g. A for Recordsets with type: 'A') @@ -184,6 +192,36 @@ class RecordSet(base.DictObjectMixin, base.PersistentObjectMixin, # Add it to the list for later errors.append(e) error_indexes.append(i) + + except TypeError as e: + e = ValidationError() + e.path = ['records', i] + e.validator = 'format' + e.validator_value = [self.type] + e.message = ("'%(data)s' is not a '%(type)s' Record" + % {'data': record.data, 'type': self.type}) + # Add it to the list for later + errors.append(e) + error_indexes.append(i) + + except AttributeError as e: + e = ValidationError() + e.path = ['records', i] + e.validator = 'format' + e.validator_value = [self.type] + e.message = ("'%(data)s' is not a '%(type)s' Record" + % {'data': record.data, 'type': self.type}) + # Add it to the list for later + errors.append(e) + error_indexes.append(i) + + except Exception as e: + error_message = str.format( + 'Provided object is not valid. ' + 'Got a %s error with message %s' % + (type(e).__name__, six.text_type(e))) + raise exceptions.InvalidObject(error_message) + else: # Seems to have loaded right - add it to be validated by # JSONSchema @@ -215,6 +253,13 @@ class RecordSet(base.DictObjectMixin, base.PersistentObjectMixin, # If JSONSchema passes, but we found parsing errors, # raise an exception if len(errors) > 0: + LOG.debug( + "Error Validating '%(name)s' object with values: " + "%(values)r", { + 'name': self.obj_name(), + 'values': self.to_dict(), + } + ) raise exceptions.InvalidObject( "Provided object does not match " "schema", errors=errors, object=self)