diff --git a/deckhand/common/validation_message.py b/deckhand/common/validation_message.py new file mode 100644 index 00000000..6bf41535 --- /dev/null +++ b/deckhand/common/validation_message.py @@ -0,0 +1,62 @@ +# Copyright 2018 AT&T Intellectual Property. All other 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. + + +class ValidationMessage(object): + """ValidationMessage per UCP convention: + https://github.com/att-comdev/ucp-integration/blob/master/docs/source/api-conventions.rst#output-structure # noqa + + Construction of ``ValidationMessage`` message: + + :param string message: Validation failure message. + :param boolean error: True or False, if this is an error message. + :param string name: Identifying name of the validation. + :param string level: The severity of validation result, as "Error", + "Warning", or "Info" + :param string schema: The schema of the document being validated. + :param string doc_name: The name of the document being validated. + :param string diagnostic: Information about what lead to the message, + or details for resolution. + """ + + def __init__(self, + message='Document validation error.', + error=True, + name='Deckhand validation error', + level='Error', + doc_schema='', + doc_name='', + doc_layer='', + diagnostic=''): + level = 'Error' if error else 'Info' + self._output = { + 'message': message, + 'error': error, + 'name': name, + 'documents': [], + 'level': level, + 'kind': self.__class__.__name__ + } + self._output['documents'].append( + dict(schema=doc_schema, name=doc_name, layer=doc_layer)) + if diagnostic: + self._output.update(diagnostic=diagnostic) + + def format_message(self): + """Return ``ValidationMessage`` message. + + :returns: The ``ValidationMessage`` for the Validation API response. + :rtype: dict + """ + return self._output diff --git a/deckhand/control/buckets.py b/deckhand/control/buckets.py index ca4b12a6..4124a393 100644 --- a/deckhand/control/buckets.py +++ b/deckhand/control/buckets.py @@ -14,7 +14,7 @@ import falcon from oslo_log import log as logging -import six +from oslo_utils import excutils from deckhand.control import base as api_base from deckhand.control.views import document as document_view @@ -48,8 +48,8 @@ class BucketsResource(api_base.BaseResource): documents, data_schemas, pre_validate=True) validations = doc_validator.validate_all() except deckhand_errors.InvalidDocumentFormat as e: - LOG.exception(e.format_message()) - raise falcon.HTTPBadRequest(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) for document in documents: if secrets_manager.SecretsManager.requires_encryption(document): @@ -59,11 +59,10 @@ class BucketsResource(api_base.BaseResource): try: documents = self._prepare_secret_documents(documents) - except deckhand_errors.BarbicanException as e: - LOG.error('An unknown exception occurred while trying to store ' - 'a secret in Barbican.') - raise falcon.HTTPInternalServerError( - description=e.format_message()) + except deckhand_errors.BarbicanException: + with excutils.save_and_reraise_exception(): + LOG.error('An unknown exception occurred while trying to store' + ' a secret in Barbican.') created_documents = self._create_revision_documents( bucket_name, documents, validations) @@ -86,8 +85,7 @@ class BucketsResource(api_base.BaseResource): bucket_name, documents, validations=validations) except (deckhand_errors.DuplicateDocumentExists, deckhand_errors.SingletonDocumentConflict) as e: - raise falcon.HTTPConflict(description=e.format_message()) - except Exception as e: - raise falcon.HTTPInternalServerError(description=six.text_type(e)) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) return created_documents diff --git a/deckhand/control/revision_diffing.py b/deckhand/control/revision_diffing.py index 85f48bcd..6a33fbb4 100644 --- a/deckhand/control/revision_diffing.py +++ b/deckhand/control/revision_diffing.py @@ -13,12 +13,16 @@ # limitations under the License. import falcon +from oslo_log import log as logging +from oslo_utils import excutils from deckhand.control import base as api_base from deckhand.db.sqlalchemy import api as db_api from deckhand import errors from deckhand import policy +LOG = logging.getLogger(__name__) + class RevisionDiffingResource(api_base.BaseResource): """API resource for realizing revision diffing.""" @@ -33,8 +37,9 @@ class RevisionDiffingResource(api_base.BaseResource): try: resp_body = db_api.revision_diff( revision_id, comparison_revision_id) - except (errors.RevisionNotFound) as e: - raise falcon.HTTPNotFound(description=e.format_message()) + except errors.RevisionNotFound as e: + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp.status = falcon.HTTP_200 resp.body = resp_body diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index 7c6be839..3cd23373 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -14,9 +14,11 @@ import falcon from oslo_log import log as logging +from oslo_utils import excutils import six from deckhand.common import utils +from deckhand.common.validation_message import ValidationMessage from deckhand.control import base as api_base from deckhand.control import common from deckhand.control.views import document as document_view @@ -118,17 +120,14 @@ class RenderedDocumentsResource(api_base.BaseResource): errors.InvalidDocumentParent, errors.InvalidDocumentReplacement, errors.IndeterminateDocumentParent, + errors.LayeringPolicyNotFound, errors.MissingDocumentKey, errors.SubstitutionSourceDataNotFound, + errors.SubstitutionSourceNotFound, + errors.UnknownSubstitutionError, errors.UnsupportedActionMethod) as e: - raise falcon.HTTPBadRequest(description=e.format_message()) - except (errors.LayeringPolicyNotFound, - errors.SubstitutionSourceNotFound) as e: - raise falcon.HTTPConflict(description=e.format_message()) - except (errors.DeckhandException, - errors.UnknownSubstitutionError) as e: - raise falcon.HTTPInternalServerError( - description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) # Filters to be applied post-rendering, because many documents are # involved in rendering. User filters can only be applied once all @@ -187,12 +186,37 @@ class RenderedDocumentsResource(api_base.BaseResource): try: validations = doc_validator.validate_all() except errors.InvalidDocumentFormat as e: - LOG.error('Failed to post-validate rendered documents.') - LOG.exception(e.format_message()) - raise falcon.HTTPInternalServerError( - description=e.format_message()) + with excutils.save_and_reraise_exception(): + # Post-rendering validation errors likely indicate an internal + # rendering bug, so override the default code to 500. + e.code = 500 + LOG.error('Failed to post-validate rendered documents.') + LOG.exception(e.format_message()) else: - failed_validations = [ - v for v in validations if v['status'] == 'failure'] - if failed_validations: - raise falcon.HTTPBadRequest(description=failed_validations) + error_list = [] + + for validation in validations: + if validation['status'] == 'failure': + error_list.extend([ + ValidationMessage( + error=True, + message=error['message'], + doc_schema=error['schema'], + doc_name=error['name'], + doc_layer=error['layer'], + diagnostic={ + k: v for k, v in error.items() if k in ( + 'schema_path', + 'validation_schema', + 'error_section' + ) + } + ) + for error in validation['errors'] + ]) + + if error_list: + raise errors.InvalidDocumentFormat( + error_list=error_list, + reason='Validation' + ) diff --git a/deckhand/control/revision_tags.py b/deckhand/control/revision_tags.py index 8b5c47db..ab770579 100644 --- a/deckhand/control/revision_tags.py +++ b/deckhand/control/revision_tags.py @@ -14,6 +14,7 @@ import falcon from oslo_log import log as logging +from oslo_utils import excutils from deckhand.control import base as api_base from deckhand.control.views import revision_tag as revision_tag_view @@ -34,10 +35,11 @@ class RevisionTagsResource(api_base.BaseResource): try: resp_tag = db_api.revision_tag_create(revision_id, tag, tag_data) - except (errors.RevisionNotFound, errors.RevisionTagNotFound) as e: - raise falcon.HTTPNotFound(description=e.format_message()) - except errors.RevisionTagBadFormat as e: - raise falcon.HTTPBadRequest(description=e.format_message()) + except (errors.RevisionNotFound, + errors.RevisionTagBadFormat, + errors.errors.RevisionTagNotFound) as e: + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp_body = revision_tag_view.ViewBuilder().show(resp_tag) resp.status = falcon.HTTP_201 @@ -57,7 +59,8 @@ class RevisionTagsResource(api_base.BaseResource): resp_tag = db_api.revision_tag_get(revision_id, tag) except (errors.RevisionNotFound, errors.RevisionTagNotFound) as e: - raise falcon.HTTPNotFound(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp_body = revision_tag_view.ViewBuilder().show(resp_tag) resp.status = falcon.HTTP_200 @@ -69,7 +72,8 @@ class RevisionTagsResource(api_base.BaseResource): try: resp_tags = db_api.revision_tag_get_all(revision_id) except errors.RevisionNotFound as e: - raise falcon.HTTPNotFound(e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp_body = revision_tag_view.ViewBuilder().list(resp_tags) resp.status = falcon.HTTP_200 @@ -89,7 +93,8 @@ class RevisionTagsResource(api_base.BaseResource): db_api.revision_tag_delete(revision_id, tag) except (errors.RevisionNotFound, errors.RevisionTagNotFound) as e: - raise falcon.HTTPNotFound(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp.status = falcon.HTTP_204 @@ -99,6 +104,7 @@ class RevisionTagsResource(api_base.BaseResource): try: db_api.revision_tag_delete_all(revision_id) except errors.RevisionNotFound as e: - raise falcon.HTTPNotFound(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp.status = falcon.HTTP_204 diff --git a/deckhand/control/revisions.py b/deckhand/control/revisions.py index c6ce56b0..4e988b38 100644 --- a/deckhand/control/revisions.py +++ b/deckhand/control/revisions.py @@ -13,6 +13,8 @@ # limitations under the License. import falcon +from oslo_log import log as logging +from oslo_utils import excutils from deckhand.common import utils from deckhand.control import base as api_base @@ -22,6 +24,8 @@ from deckhand.db.sqlalchemy import api as db_api from deckhand import errors from deckhand import policy +LOG = logging.getLogger(__name__) + class RevisionsResource(api_base.BaseResource): """API resource for realizing CRUD operations for revisions.""" @@ -50,7 +54,8 @@ class RevisionsResource(api_base.BaseResource): try: revision = db_api.revision_get(revision_id) except errors.RevisionNotFound as e: - raise falcon.HTTPNotFound(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) revision_resp = self.view_builder.show(revision) resp.status = falcon.HTTP_200 diff --git a/deckhand/control/rollback.py b/deckhand/control/rollback.py index d03e9385..97988b05 100644 --- a/deckhand/control/rollback.py +++ b/deckhand/control/rollback.py @@ -13,6 +13,8 @@ # limitations under the License. import falcon +from oslo_log import log as logging +from oslo_utils import excutils from deckhand.control import base as api_base from deckhand.control.views import revision as revision_view @@ -20,6 +22,8 @@ from deckhand.db.sqlalchemy import api as db_api from deckhand import errors from deckhand import policy +LOG = logging.getLogger(__name__) + class RollbackResource(api_base.BaseResource): """API resource for realizing revision rollback.""" @@ -31,7 +35,8 @@ class RollbackResource(api_base.BaseResource): try: latest_revision = db_api.revision_get_latest() except errors.RevisionNotFound as e: - raise falcon.HTTPNotFound(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) for document in latest_revision['documents']: if document['metadata'].get('storagePolicy') == 'encrypted': @@ -43,7 +48,8 @@ class RollbackResource(api_base.BaseResource): rollback_revision = db_api.revision_rollback( revision_id, latest_revision) except errors.InvalidRollback as e: - raise falcon.HTTPBadRequest(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) revision_resp = self.view_builder.show(rollback_revision) resp.status = falcon.HTTP_201 diff --git a/deckhand/control/validations.py b/deckhand/control/validations.py index 1fa234d6..02ef5ec8 100644 --- a/deckhand/control/validations.py +++ b/deckhand/control/validations.py @@ -14,6 +14,7 @@ import falcon from oslo_log import log as logging +from oslo_utils import excutils from deckhand.control import base as api_base from deckhand.control.views import validation as validation_view @@ -44,7 +45,8 @@ class ValidationsResource(api_base.BaseResource): resp_body = db_api.validation_create( revision_id, validation_name, validation_data) except errors.RevisionNotFound as e: - raise falcon.HTTPNotFound(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp.status = falcon.HTTP_201 resp.append_header('Content-Type', 'application/x-yaml') @@ -77,8 +79,10 @@ class ValidationsResource(api_base.BaseResource): try: entry = db_api.validation_get_entry( revision_id, validation_name, entry_id) - except (errors.RevisionNotFound, errors.ValidationNotFound) as e: - raise falcon.HTTPNotFound(description=e.format_message()) + except (errors.RevisionNotFound, + errors.ValidationNotFound) as e: + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp_body = self.view_builder.show_entry(entry) return resp_body @@ -90,7 +94,8 @@ class ValidationsResource(api_base.BaseResource): entries = db_api.validation_get_all_entries(revision_id, validation_name) except errors.RevisionNotFound as e: - raise falcon.HTTPNotFound(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp_body = self.view_builder.list_entries(entries) return resp_body @@ -100,7 +105,8 @@ class ValidationsResource(api_base.BaseResource): try: validations = db_api.validation_get_all(revision_id) except errors.RevisionNotFound as e: - raise falcon.HTTPNotFound(description=e.format_message()) + with excutils.save_and_reraise_exception(): + LOG.exception(e.format_message()) resp_body = self.view_builder.list(validations) return resp_body diff --git a/deckhand/control/views/document.py b/deckhand/control/views/document.py index ed9d47d7..548b9084 100644 --- a/deckhand/control/views/document.py +++ b/deckhand/control/views/document.py @@ -35,14 +35,14 @@ class ViewBuilder(common.ViewBuilder): attrs = ['id', 'metadata', 'data', 'schema'] for document in documents: - if document['deleted']: + if document.get('deleted'): continue if document['schema'].startswith(types.VALIDATION_POLICY_SCHEMA): continue - resp_obj = {x: document[x] for x in attrs} + resp_obj = {x: document.get(x) for x in attrs} resp_obj.setdefault('status', {}) - resp_obj['status']['bucket'] = document['bucket_name'] - resp_obj['status']['revision'] = document['revision_id'] + resp_obj['status']['bucket'] = document.get('bucket_name') + resp_obj['status']['revision'] = document.get('revision_id') resp_list.append(resp_obj) # Edge case for when all documents are deleted from a bucket. To detect @@ -53,8 +53,8 @@ class ViewBuilder(common.ViewBuilder): # across all the documents in ``documents``. if not resp_list and documents: resp_obj = {'status': {}} - resp_obj['status']['bucket'] = documents[0]['bucket_name'] - resp_obj['status']['revision'] = documents[0]['revision_id'] + resp_obj['status']['bucket'] = documents[0].get('bucket_name') + resp_obj['status']['revision'] = documents[0].get('revision_id') return [resp_obj] return resp_list diff --git a/deckhand/engine/document_validation.py b/deckhand/engine/document_validation.py index 304593dc..e6f607b4 100644 --- a/deckhand/engine/document_validation.py +++ b/deckhand/engine/document_validation.py @@ -25,6 +25,7 @@ import six from deckhand.common import document as document_wrapper from deckhand.common import utils +from deckhand.common.validation_message import ValidationMessage from deckhand.engine.secrets_manager import SecretsSubstitution from deckhand import errors from deckhand import types @@ -107,6 +108,11 @@ class GenericValidator(BaseValidator): __slots__ = ('base_schema') + _diagnostic = ( + 'Ensure that each document has a metadata, schema and data section. ' + 'Each document must pass the schema defined under: ' + 'http://deckhand.readthedocs.io/en/latest/validation.html#base-schema') + def __init__(self): super(GenericValidator, self).__init__() self.base_schema = self._schema_map['v1']['deckhand/Base'] @@ -149,8 +155,16 @@ class GenericValidator(BaseValidator): 'Details: %s', document.schema, document.layer, document.name, error_messages) raise errors.InvalidDocumentFormat( - schema=document.schema, name=document.name, - layer=document.layer, errors=', '.join(error_messages)) + error_list=[ + ValidationMessage(message=message, + doc_schema=document.schema, + doc_name=document.name, + doc_layer=document.layer, + diagnostic=self._diagnostic) + for message in error_messages + ], + reason='Validation' + ) class DataSchemaValidator(GenericValidator): @@ -430,6 +444,7 @@ class DocumentValidation(object): } formatted_results = [] + for result in results: formatted_result = { 'name': types.DECKHAND_SCHEMA_VALIDATION, @@ -459,8 +474,7 @@ class DocumentValidation(object): error_outputs = validator.validate( document, pre_validate=self._pre_validate) if error_outputs: - for error_output in error_outputs: - result['errors'].append(error_output) + result['errors'].extend(error_outputs) if result['errors']: result.setdefault('status', 'failure') @@ -503,5 +517,4 @@ class DocumentValidation(object): result = self._validate_one(document) validation_results.append(result) - validations = self._format_validation_results(validation_results) - return validations + return self._format_validation_results(validation_results) diff --git a/deckhand/engine/layering.py b/deckhand/engine/layering.py index 9aca24fe..bc04c506 100644 --- a/deckhand/engine/layering.py +++ b/deckhand/engine/layering.py @@ -24,6 +24,7 @@ from oslo_utils import excutils from deckhand.common import document as document_wrapper from deckhand.common import utils +from deckhand.common.validation_message import ValidationMessage from deckhand.engine import document_validation from deckhand.engine import secrets_manager from deckhand.engine import utils as engine_utils @@ -343,21 +344,24 @@ class DocumentLayering(object): validator = document_validation.DocumentValidation( documents, pre_validate=True) results = validator.validate_all() - val_errors = [] + + error_list = [] for result in results: - val_errors.extend( - [(e['schema'], e['layer'], e['name'], e['message']) - for e in result['errors']]) - if val_errors: - for error in val_errors: - LOG.error( - 'Document [%s, %s] %s failed with pre-validation error: ' - '%s.', *error) - raise errors.InvalidDocumentFormat( - schema=', '.join(v[0] for v in val_errors), - layer=', '.join(v[1] for v in val_errors), - name=', '.join(v[2] for v in val_errors), - errors=', '.join(v[3] for v in val_errors)) + for e in result['errors']: + LOG.error('Document [%s, %s] %s failed with pre-validation ' + 'error: %s.', e['schema'], e['layer'], e['name'], + e['message']) + error_list.append( + ValidationMessage( + message=e['message'], + doc_schema=e['schema'], + doc_name=e['name'], + doc_layer=e['layer'] + ) + ) + + if error_list: + raise errors.InvalidDocumentFormat(error_list=error_list) def __init__(self, documents, substitution_sources=None, validate=True, fail_on_missing_sub_src=True): @@ -536,8 +540,10 @@ class DocumentLayering(object): if from_child is None: raise errors.MissingDocumentKey( child_schema=child_data.schema, + child_layer=child_data.layer, child_name=child_data.name, parent_schema=overall_data.schema, + parent_layer=overall_data.layer, parent_name=overall_data.name, action=action) diff --git a/deckhand/errors.py b/deckhand/errors.py index f6fb234d..5918d3b5 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -12,7 +12,6 @@ # See the License for the specific language governing permissions and # limitations under the License. -import traceback import yaml import falcon @@ -34,7 +33,7 @@ def format_error_resp(req, resp, status_code=falcon.HTTP_500, message="", - reason="", + reason=None, error_type=None, error_list=None, info_list=None): @@ -63,21 +62,20 @@ def format_error_resp(req, 'error': ``False`` field. """ - if error_type is None: - error_type = 'Unspecified Exception' + error_type = error_type or 'Unspecified Exception' + reason = reason or 'Unspecified' # Since we're handling errors here, if error list is None, set up a default # error item. If we have info items, add them to the message list as well. # In both cases, if the error flag is not set, set it appropriately. - if error_list is None: - error_list = [{'message': 'An error occurred, but was not specified', - 'error': True}] + if not error_list: + error_list = [{'message': message, 'error': True}] else: for error_item in error_list: if 'error' not in error_item: error_item['error'] = True - if info_list is None: + if not info_list: info_list = [] else: for info_item in info_list: @@ -87,7 +85,7 @@ def format_error_resp(req, message_list = error_list + info_list error_response = { - 'kind': 'status', + 'kind': 'Status', 'apiVersion': get_version_from_request(req), 'metadata': {}, 'status': 'Failure', @@ -104,23 +102,35 @@ def format_error_resp(req, 'retry': True if status_code is falcon.HTTP_500 else False } - resp.body = yaml.safe_dump(error_response) + # Don't use yaml.safe_dump to handle unicode correctly. + resp.body = yaml.dump(error_response) resp.status = status_code def default_exception_handler(ex, req, resp, params): - """Catch-all execption handler for standardized output. + """Catch-all exception handler for standardized output. If this is a standard falcon HTTPError, rethrow it for handling by ``default_exception_serializer`` below. """ if isinstance(ex, falcon.HTTPError): - # Allow the falcon http errors to bubble up and get handled. + # Allow the falcon HTTP errors to bubble up and get handled. raise ex + elif isinstance(ex, DeckhandException): + status_code = (getattr(falcon, 'HTTP_%d' % ex.code, falcon.HTTP_500) + if hasattr(ex, 'code') else falcon.HTTP_500) + + format_error_resp( + req, + resp, + status_code=status_code, + message=ex.message, + error_type=ex.__class__.__name__, + error_list=getattr(ex, 'error_list', None), + reason=getattr(ex, 'reason', None) + ) else: # Take care of the uncaught stuff. - exc_string = traceback.format_exc() - LOG.error('Unhanded Exception being handled: \n%s', exc_string) format_error_resp( req, resp, @@ -139,9 +149,9 @@ def default_exception_serializer(req, resp, exception): status_code=exception.status, # TODO(fmontei): Provide an overall error message instead. message=exception.description, - reason=exception.title, error_type=exception.__class__.__name__, - error_list=[{'message': exception.description, 'error': True}] + error_list=getattr(exception, 'error_list', None), + reason=getattr(exception, 'reason', None) ) @@ -164,6 +174,18 @@ class DeckhandException(Exception): message = self.msg_fmt self.message = message + self.reason = kwargs.pop('reason', None) + + error_list = kwargs.pop('error_list', []) + self.error_list = [] + + for error in error_list: + if isinstance(error, str): + error = {'message': error, 'error': True} + else: + error = error.format_message() + self.error_list.append(error) + super(DeckhandException, self).__init__(message) def format_message(self): @@ -175,8 +197,7 @@ class InvalidDocumentFormat(DeckhandException): **Troubleshoot:** """ - msg_fmt = ("The provided document(s) schema=%(schema)s, layer=%(layer)s, " - "name=%(name)s failed schema validation. Errors: %(errors)s") + msg_fmt = ("The provided documents failed schema validation.") code = 400 @@ -206,6 +227,7 @@ class InvalidDocumentParent(DeckhandException): msg_fmt = ("The document parent [%(parent_schema)s] %(parent_name)s is " "invalid for document [%(document_schema)s] %(document_name)s. " "Reason: %(reason)s") + code = 400 class IndeterminateDocumentParent(DeckhandException): diff --git a/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml b/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml index f4cce45a..e2e51a5e 100644 --- a/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml +++ b/deckhand/tests/functional/gabbits/schema-validation/schema-validation-success.yaml @@ -145,37 +145,22 @@ tests: status: 400 response_multidoc_jsonpaths: $.`len`: 1 + $.[0].apiVersion: v1.0 + $.[0].code: 400 Bad Request + $.[0].details.errorCount: 1 + $.[0].details.errorType: InvalidDocumentFormat + $.[0].details.messageList[0].documents: + - layer: site + name: bad + schema: example/Doc/v1 + $.[0].details.messageList[0].error: true + $.[0].details.messageList[0].kind: ValidationMessage + $.[0].details.messageList[0].level: Error + $.[0].details.messageList[0].name: Deckhand validation error + $.[0].kind: Status + $.[0].message: The provided documents failed schema validation. + $.[0].reason: Validation $.[0].status: Failure - $.[0].message: - - errors: - - validation_schema: - "$schema": http://json-schema.org/schema# - properties: - a: - type: string - b: - maximum: 100 - type: integer - minimum: 0 - type: object - required: - - a - - b - additionalProperties: false - error_section: - a: this-one-is-required-and-can-be-different - b: 177 - schema_path: ".properties.b.maximum" - name: bad - schema: example/Doc/v1 - layer: site - path: ".data.b" - message: 177 is greater than the maximum of 100 - name: deckhand-schema-validation - validator: - name: deckhand - version: '1.0' - status: failure - name: add_invalid_document_with_substitutions desc: Add a document that does not follow the schema @@ -216,34 +201,9 @@ tests: status: 400 response_multidoc_jsonpaths: $.`len`: 1 - $.[0].status: Failure - $.[0].message: - - errors: - - name: bad - layer: site - schema: example/Doc/v1 - path: .data.b - schema_path: .properties.b.maximum - error_section: - a: Sanitized to avoid exposing secret. - b: 177 - message: 177 is greater than the maximum of 100 - validation_schema: - $schema: http://json-schema.org/schema# - additionalProperties: False - properties: - a: - type: string - b: - maximum: 100 - minimum: 0 - type: integer - required: - - a - - b - type: object - name: deckhand-schema-validation - validator: - name: deckhand - version: '1.0' - status: failure + $.[0].code: 400 Bad Request + $.[0].details.errorCount: 1 + $.[0].details.errorType: InvalidDocumentFormat + $.[0].details.messageList[0].diagnostic.error_section: + a: 'Sanitized to avoid exposing secret.' + b: 177 diff --git a/deckhand/tests/unit/control/test_errors.py b/deckhand/tests/unit/control/test_errors.py index 195ed4a7..c0dcbfb2 100644 --- a/deckhand/tests/unit/control/test_errors.py +++ b/deckhand/tests/unit/control/test_errors.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import os import yaml import falcon @@ -22,44 +23,41 @@ from deckhand.tests.unit.control import base as test_base class TestErrorFormatting(test_base.BaseControllerTest): - """Test suite for validating error formatting. + """Test suite for validating error formatting.""" - Use mocked exceptions below to guarantee consistent results. - """ - - def test_base_exception_formatting(self): + def test_python_exception_formatting(self): """Verify formatting for an exception class that inherits from :class:`Exception`. """ with mock.patch.object( policy, '_do_enforce_rbac', spec_set=policy._do_enforce_rbac) as m_enforce_rbac: - m_enforce_rbac.side_effect = Exception + m_enforce_rbac.side_effect = Exception('test error') resp = self.app.simulate_put( '/api/v1.0/buckets/test/documents', headers={'Content-Type': 'application/x-yaml'}, body=None) expected = { 'status': 'Failure', - 'kind': 'status', + 'kind': 'Status', 'code': '500 Internal Server Error', 'apiVersion': 'v1.0', - 'reason': '', + 'reason': 'Unspecified', 'retry': True, 'details': { 'errorType': 'Exception', 'errorCount': 1, 'messageList': [ { - 'message': 'An error occurred, but was not specified', + 'message': 'Unhandled Exception raised: test error', 'error': True } ] }, - 'message': 'Unhandled Exception raised: ', + 'message': 'Unhandled Exception raised: test error', 'metadata': {} } - body = yaml.safe_load(resp.text) + body = yaml.load(resp.text) self.assertEqual(500, resp.status_code) self.assertEqual(expected, body) @@ -82,10 +80,10 @@ class TestErrorFormatting(test_base.BaseControllerTest): expected = { 'status': 'Failure', - 'kind': 'status', + 'kind': 'Status', 'code': '403 Forbidden', 'apiVersion': 'v1.0', - 'reason': '403 Forbidden', + 'reason': 'Unspecified', 'retry': False, 'details': { 'errorType': 'HTTPForbidden', @@ -104,3 +102,128 @@ class TestErrorFormatting(test_base.BaseControllerTest): self.assertEqual(403, resp.status_code) self.assertEqual(expected, body) + + +class TestValidationMessageFormatting(test_base.BaseControllerTest): + """Test suite for validating :class:`ValidationMessage` formatting.""" + + def test_put_bucket_validation_message_formatting(self): + """Verify formatting for pre-validation during updating a bucket.""" + rules = {'deckhand:create_cleartext_documents': '@'} + self.policy.set_rules(rules) + + resp = self.app.simulate_put( + '/api/v1.0/buckets/test/documents', + headers={'Content-Type': 'application/x-yaml'}, + body='name: test') + + expected = { + 'status': 'Failure', + 'kind': 'Status', + 'code': '400 Bad Request', + 'apiVersion': 'v1.0', + 'reason': 'Validation', + 'retry': False, + 'details': { + 'errorType': 'InvalidDocumentFormat', + 'errorCount': 2, + 'messageList': [ + { + 'diagnostic': mock.ANY, + 'documents': [{ + 'layer': None, + 'name': None, + 'schema': '' + }], + 'error': True, + 'kind': 'ValidationMessage', + 'level': 'Error', + 'message': mock.ANY, + 'name': 'Deckhand validation error' + }, + { + 'diagnostic': mock.ANY, + 'documents': [{ + 'layer': None, + 'name': None, + 'schema': '' + }], + 'error': True, + 'kind': 'ValidationMessage', + 'level': 'Error', + 'message': mock.ANY, + 'name': 'Deckhand validation error' + } + ] + }, + 'message': 'The provided documents failed schema validation.', + 'metadata': {} + } + body = yaml.safe_load(resp.text) + + self.assertEqual(400, resp.status_code) + self.assertEqual(expected, body) + + def test_rendered_documents_validation_message_formatting(self): + """Verify formatting for post-validation during rendering revision + documents. + """ + rules = {'deckhand:create_cleartext_documents': '@', + 'deckhand:list_cleartext_documents': '@', + 'deckhand:list_encrypted_documents': '@'} + self.policy.set_rules(rules) + + yaml_file = os.path.join(os.getcwd(), 'deckhand', 'tests', 'unit', + 'resources', 'sample_layering_policy.yaml') + with open(yaml_file) as yaml_stream: + payload = yaml_stream.read() + + resp = self.app.simulate_put( + '/api/v1.0/buckets/test/documents', + headers={'Content-Type': 'application/x-yaml'}, + body=payload) + + with mock.patch('deckhand.control.revision_documents.db_api' + '.revision_documents_get', autospec=True) \ + as mock_get_rev_documents: + invalid_document = yaml.safe_load(payload) + invalid_document.pop('metadata') + + mock_get_rev_documents.return_value = [invalid_document] + resp = self.app.simulate_get( + '/api/v1.0/revisions/1/rendered-documents', + headers={'Content-Type': 'application/x-yaml'}) + + expected = { + 'status': 'Failure', + 'kind': 'Status', + 'code': '500 Internal Server Error', + 'apiVersion': 'v1.0', + 'reason': 'Validation', + 'retry': True, + 'details': { + 'errorType': 'InvalidDocumentFormat', + 'errorCount': 1, + 'messageList': [ + { + 'diagnostic': mock.ANY, + 'documents': [{ + 'layer': None, + 'name': None, + 'schema': invalid_document['schema'] + }], + 'error': True, + 'kind': 'ValidationMessage', + 'level': 'Error', + 'message': mock.ANY, + 'name': 'Deckhand validation error' + } + ] + }, + 'message': 'The provided documents failed schema validation.', + 'metadata': {} + } + body = yaml.safe_load(resp.text) + + self.assertEqual(500, resp.status_code) + self.assertEqual(expected, body) diff --git a/deckhand/tests/unit/control/test_middleware.py b/deckhand/tests/unit/control/test_middleware.py index c2c76442..55fa862d 100644 --- a/deckhand/tests/unit/control/test_middleware.py +++ b/deckhand/tests/unit/control/test_middleware.py @@ -121,10 +121,10 @@ class TestYAMLTranslatorNegative(test_base.BaseControllerTest): 'message': "The Content-Type header is required." }] }, - 'kind': 'status', + 'kind': 'Status', 'message': "The Content-Type header is required.", 'metadata': {}, - 'reason': 'Missing header value', + 'reason': 'Unspecified', 'retry': False, 'status': 'Failure' } @@ -153,11 +153,11 @@ class TestYAMLTranslatorNegative(test_base.BaseControllerTest): "content types are: ['application/x-yaml'].") }] }, - 'kind': 'status', + 'kind': 'Status', 'message': ("Unexpected content type: application/json. Expected " "content types are: ['application/x-yaml']."), 'metadata': {}, - 'reason': 'Unsupported media type', + 'reason': 'Unspecified', 'retry': False, 'status': 'Failure' } @@ -188,11 +188,11 @@ class TestYAMLTranslatorNegative(test_base.BaseControllerTest): "content types are: ['application/x-yaml'].") }] }, - 'kind': 'status', + 'kind': 'Status', 'message': ("Unexpected content type: application/yaml. Expected " "content types are: ['application/x-yaml']."), 'metadata': {}, - 'reason': 'Unsupported media type', + 'reason': 'Unspecified', 'retry': False, 'status': 'Failure' } diff --git a/deckhand/tests/unit/control/test_rendered_documents_controller.py b/deckhand/tests/unit/control/test_rendered_documents_controller.py index 13211e9d..d671f799 100644 --- a/deckhand/tests/unit/control/test_rendered_documents_controller.py +++ b/deckhand/tests/unit/control/test_rendered_documents_controller.py @@ -200,8 +200,8 @@ class TestRenderedDocumentsControllerNegative( test_base.BaseControllerTest): def test_rendered_documents_fail_schema_validation(self): - """Validates that when fully rendered documents fail schema validation, - the controller raises a 500 Internal Server Error. + """Validates that when fully rendered documents fail basic schema + validation (sanity-checking), a 500 is raised. """ rules = {'deckhand:list_cleartext_documents': '@', 'deckhand:list_encrypted_documents': '@', @@ -232,6 +232,61 @@ class TestRenderedDocumentsControllerNegative( # schema validation. self.assertEqual(500, resp.status_code) + def test_rendered_documents_fail_post_validation(self): + """Validates that when fully rendered documents fail schema validation, + a 400 is raised. + + For this scenario a DataSchema checks that the relevant document has + a key in its data section, a key which is removed during the rendering + process as the document uses a delete action. This triggers + post-rendering validation failure. + """ + rules = {'deckhand:list_cleartext_documents': '@', + 'deckhand:list_encrypted_documents': '@', + 'deckhand:create_cleartext_documents': '@'} + self.policy.set_rules(rules) + + # Create a document for a bucket. + documents_factory = factories.DocumentFactory(2, [1, 1]) + payload = documents_factory.gen_test({ + "_GLOBAL_DATA_1_": {"data": {"a": "b"}}, + "_SITE_DATA_1_": {"data": {"a": "b"}}, + "_SITE_ACTIONS_1_": { + "actions": [{"method": "delete", "path": "."}] + } + }, site_abstract=False) + + data_schema_factory = factories.DataSchemaFactory() + metadata_name = payload[-1]['schema'] + schema_to_use = { + '$schema': 'http://json-schema.org/schema#', + 'type': 'object', + 'properties': { + 'a': { + 'type': 'string' + } + }, + 'required': ['a'], + 'additionalProperties': False + } + data_schema = data_schema_factory.gen_test( + metadata_name, data=schema_to_use) + payload.append(data_schema) + + resp = self.app.simulate_put( + '/api/v1.0/buckets/mop/documents', + headers={'Content-Type': 'application/x-yaml'}, + body=yaml.safe_dump_all(payload)) + self.assertEqual(200, resp.status_code) + revision_id = list(yaml.safe_load_all(resp.text))[0]['status'][ + 'revision'] + + resp = self.app.simulate_get( + '/api/v1.0/revisions/%s/rendered-documents' % revision_id, + headers={'Content-Type': 'application/x-yaml'}) + + self.assertEqual(400, resp.status_code) + class TestRenderedDocumentsControllerNegativeRBAC( test_base.BaseControllerTest): diff --git a/deckhand/tests/unit/engine/test_document_layering_negative.py b/deckhand/tests/unit/engine/test_document_layering_negative.py index 1fec64fd..0500d27d 100644 --- a/deckhand/tests/unit/engine/test_document_layering_negative.py +++ b/deckhand/tests/unit/engine/test_document_layering_negative.py @@ -277,13 +277,15 @@ class TestDocumentLayeringValidationNegative( layering_policy = copy.deepcopy(lp_template) del layering_policy['data']['layerOrder'] - error_re = ("The provided document\(s\) schema=%s, layer=%s, name=%s " - "failed schema validation. Errors: 'layerOrder' is a " - "required property" % ( - layering_policy['schema'], - layering_policy['metadata']['layeringDefinition'][ - 'layer'], - layering_policy['metadata']['name'])) - self.assertRaisesRegexp( - errors.InvalidDocumentFormat, error_re, self._test_layering, + error_re = r"^'layerOrder' is a required property$" + e = self.assertRaises( + errors.InvalidDocumentFormat, self._test_layering, [layering_policy, document], validate=True) + self.assertRegex(e.error_list[0]['message'], error_re) + self.assertEqual(layering_policy['schema'], + e.error_list[0]['documents'][0]['schema']) + self.assertEqual(layering_policy['metadata']['name'], + e.error_list[0]['documents'][0]['name']) + self.assertEqual(layering_policy['metadata']['layeringDefinition'][ + 'layer'], + e.error_list[0]['documents'][0]['layer']) diff --git a/deckhand/tests/unit/engine/test_document_validation_negative.py b/deckhand/tests/unit/engine/test_document_validation_negative.py index 188533f9..251f74b9 100644 --- a/deckhand/tests/unit/engine/test_document_validation_negative.py +++ b/deckhand/tests/unit/engine/test_document_validation_negative.py @@ -149,21 +149,22 @@ class TestDocumentValidationNegative(test_base.TestDocumentValidationBase): parts = property_to_remove.split('.') missing_property = parts[-1] - expected_err = "'%s' is a required property" % missing_property - self.assertIn(expected_err, e.message) + error_re = r"%s is a required property" % missing_property + self.assertRegex(str(e.error_list).replace("\'", ""), error_re) def test_document_invalid_layering_definition_action(self): document = self._read_data('sample_document') missing_data = self._corrupt_data( document, 'metadata.layeringDefinition.actions.0.method', 'invalid', op='replace') - expected_err = ( - r".+ 'invalid' is not one of \['replace', 'delete', 'merge'\]") + error_re = ( + r".*invalid is not one of \[replace, delete, merge\]") payload = [missing_data] doc_validator = document_validation.DocumentValidation(payload) - self.assertRaisesRegexp(errors.InvalidDocumentFormat, expected_err, - doc_validator.validate_all) + e = self.assertRaises(errors.InvalidDocumentFormat, + doc_validator.validate_all) + self.assertRegex(str(e.error_list[0]).replace("\'", ""), error_re) def test_layering_policy_missing_required_sections(self): properties_to_remove = (