diff --git a/deckhand/control/revision_documents.py b/deckhand/control/revision_documents.py index 639ab3f1..839323e7 100644 --- a/deckhand/control/revision_documents.py +++ b/deckhand/control/revision_documents.py @@ -14,6 +14,7 @@ import falcon from oslo_log import log as logging +import six from deckhand.control import base as api_base from deckhand.control import common @@ -58,6 +59,7 @@ class RevisionDocumentsResource(api_base.BaseResource): documents = db_api.revision_get_documents( revision_id, **filters) except errors.RevisionNotFound as e: + LOG.exception(six.text_type(e)) raise falcon.HTTPNotFound(description=e.format_message()) resp.status = falcon.HTTP_200 @@ -95,7 +97,8 @@ class RenderedDocumentsResource(api_base.BaseResource): try: documents = db_api.revision_get_documents( revision_id, **filters) - except (errors.RevisionNotFound) as e: + except errors.RevisionNotFound as e: + LOG.exception(six.text_type(e)) raise falcon.HTTPNotFound(description=e.format_message()) # TODO(fmontei): Currently the only phase of rendering that is @@ -104,7 +107,13 @@ class RenderedDocumentsResource(api_base.BaseResource): # a separate module that handles layering alongside substitution once # layering has been fully integrated into this endpoint. secrets_substitution = secrets_manager.SecretsSubstitution(documents) - rendered_documents = secrets_substitution.substitute_all() + try: + rendered_documents = secrets_substitution.substitute_all() + except errors.DocumentNotFound as e: + LOG.error('Failed to render the documents because a secret ' + 'document could not be found.') + LOG.exception(six.text_type(e)) + raise falcon.HTTPNotFound(description=e.format_message()) resp.status = falcon.HTTP_200 resp.body = self.view_builder.list(rendered_documents) diff --git a/deckhand/errors.py b/deckhand/errors.py index 443cf955..5de31341 100644 --- a/deckhand/errors.py +++ b/deckhand/errors.py @@ -12,6 +12,138 @@ # See the License for the specific language governing permissions and # limitations under the License. +import traceback +import yaml + +import falcon +from oslo_log import log as logging +import six + +LOG = logging.getLogger(__name__) + + +def get_version_from_request(req): + """Attempt to extract the API version string.""" + for part in req.path.split('/'): + if '.' in part and part.startswith('v'): + return part + return 'N/A' + + +def format_error_resp(req, + resp, + status_code=falcon.HTTP_500, + message="", + reason="", + error_type=None, + error_list=None, + info_list=None): + """Generate a error message body and throw a Falcon exception to trigger + an HTTP status. + + :param req: ``falcon`` request object. + :param resp: ``falcon`` response object to update. + :param status_code: ``falcon`` status_code constant. + :param message: Optional error message to include in the body. + This should be the summary level of the error + message, encompassing an overall result. If + no other messages are passed in the error_list, + this message will be repeated in a generated + message for the output message_list. + :param reason: Optional reason code to include in the body + :param error_type: If specified, the error type will be used; + otherwise, this will be set to + 'Unspecified Exception'. + :param error_list: optional list of error dictionaries. Minimally, + the dictionary will contain the 'message' field, + but should also contain 'error': ``True``. + :param info_list: optional list of info message dictionaries. + Minimally, the dictionary needs to contain a + 'message' field, but should also have a + 'error': ``False`` field. + """ + + if error_type is None: + error_type = 'Unspecified Exception' + + # 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}] + else: + for error_item in error_list: + if 'error' not in error_item: + error_item['error'] = True + + if info_list is None: + info_list = [] + else: + for info_item in info_list: + if 'error' not in info_item: + info_item['error'] = False + + message_list = error_list + info_list + + error_response = { + 'kind': 'status', + 'apiVersion': get_version_from_request(req), + 'metadata': {}, + 'status': 'Failure', + 'message': message, + 'reason': reason, + 'details': { + 'errorType': error_type, + 'errorCount': len(error_list), + 'messageList': message_list + }, + 'code': status_code, + # TODO(fmontei): Make this class-specific later. For now, retry + # is set to True only for internal server errors. + 'retry': True if status_code is falcon.HTTP_500 else False + } + + resp.body = yaml.safe_dump(error_response) + resp.status = status_code + + +def default_exception_handler(ex, req, resp, params): + """Catch-all execption 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. + raise ex + 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, + error_type=ex.__class__.__name__, + message="Unhandled Exception raised: %s" % six.text_type(ex) + ) + + +def default_exception_serializer(req, resp, exception): + """Serializes instances of :class:`falcon.HTTPError` into YAML format and + formats the error body so it adheres to the UCP error formatting standard. + """ + format_error_resp( + req, + resp, + 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}] + ) + class DeckhandException(Exception): """Base Deckhand Exception diff --git a/deckhand/service.py b/deckhand/service.py index 028012c1..1ea92b8f 100644 --- a/deckhand/service.py +++ b/deckhand/service.py @@ -27,6 +27,7 @@ from deckhand.control import revisions from deckhand.control import rollback from deckhand.control import validations from deckhand.control import versions +from deckhand import errors CONF = cfg.CONF LOG = log.getLogger(__name__) @@ -61,6 +62,11 @@ def configure_app(app, version=''): app.add_route(os.path.join('/api/%s' % version, path), res) app.add_route('/versions', versions.VersionsResource()) + # Error handlers (FILO handling). + app.add_error_handler(Exception, errors.default_exception_handler) + # Built-in error serializer. + app.set_error_serializer(errors.default_exception_serializer) + return app diff --git a/deckhand/tests/unit/control/test_errors.py b/deckhand/tests/unit/control/test_errors.py new file mode 100644 index 00000000..385fcd1b --- /dev/null +++ b/deckhand/tests/unit/control/test_errors.py @@ -0,0 +1,104 @@ +# Copyright 2017 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. + +import yaml + +import falcon +import mock + +from deckhand import policy +from deckhand.tests.unit.control import base as test_base + + +class TestErrorFormatting(test_base.BaseControllerTest): + """Test suite for validating error formatting. + + Use mocked exceptions below to guarantee consistent results. + """ + + def test_base_exception_formatting(self): + """Verify formatting for an exception class that inherits from + :class:`Exception`. + """ + with mock.patch.object(policy, '_do_enforce_rbac', autospec=True) \ + as m_enforce_rbac: + m_enforce_rbac.side_effect = Exception + resp = self.app.simulate_put( + '/api/v1.0/bucket/test/documents', + headers={'Content-Type': 'application/x-yaml'}, body=None) + + expected = { + 'status': 'Failure', + 'kind': 'status', + 'code': '500 Internal Server Error', + 'apiVersion': 'v1.0', + 'reason': '', + 'retry': True, + 'details': { + 'errorType': 'Exception', + 'errorCount': 1, + 'messageList': [ + { + 'message': 'An error occurred, but was not specified', + 'error': True + } + ] + }, + 'message': 'Unhandled Exception raised: ', + 'metadata': {} + } + body = yaml.safe_load(resp.text) + + self.assertEqual(500, resp.status_code) + self.assertEqual(expected, body) + + def test_falcon_exception_formatting(self): + """Verify formatting for an exception class that inherits from + :class:`falcon.HTTPError`. + """ + expected_msg = ( + 'deckhand:create_cleartext_documents is disallowed by policy') + + with mock.patch.object(policy, '_do_enforce_rbac', autospec=True) \ + as m_enforce_rbac: + m_enforce_rbac.side_effect = falcon.HTTPForbidden( + description=expected_msg) + resp = self.app.simulate_put( + '/api/v1.0/bucket/test/documents', + headers={'Content-Type': 'application/x-yaml'}, body=None) + + expected = { + 'status': 'Failure', + 'kind': 'status', + 'code': '403 Forbidden', + 'apiVersion': 'v1.0', + 'reason': '403 Forbidden', + 'retry': False, + 'details': { + 'errorType': 'HTTPForbidden', + 'errorCount': 1, + 'messageList': [ + { + 'message': expected_msg, + 'error': True + } + ] + }, + 'message': expected_msg, + 'metadata': {} + } + body = yaml.safe_load(resp.text) + + self.assertEqual(403, 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 b60e7e47..51aaf538 100644 --- a/deckhand/tests/unit/control/test_middleware.py +++ b/deckhand/tests/unit/control/test_middleware.py @@ -14,6 +14,8 @@ import yaml +import mock + from deckhand.tests.unit.control import base as test_base @@ -38,8 +40,22 @@ class TestYAMLTranslatorNegative(test_base.BaseControllerTest): self.assertEqual(400, resp.status_code) expected = { - 'description': 'The Content-Type header is required.', - 'title': 'Missing header value' + 'apiVersion': mock.ANY, + 'code': '400 Bad Request', + 'details': { + 'errorCount': 1, + 'errorType': 'HTTPMissingHeader', + 'messageList': [{ + 'error': True, + 'message': "The Content-Type header is required." + }] + }, + 'kind': 'status', + 'message': "The Content-Type header is required.", + 'metadata': {}, + 'reason': 'Missing header value', + 'retry': False, + 'status': 'Failure' } self.assertEqual(expected, yaml.safe_load(resp.content)) @@ -49,10 +65,25 @@ class TestYAMLTranslatorNegative(test_base.BaseControllerTest): self.assertEqual(415, resp.status_code) expected = { - 'description': "Unexpected content type: application/json. " - "Expected content types are: " - "['application/x-yaml'].", - 'title': 'Unsupported media type' + 'apiVersion': mock.ANY, + 'code': '415 Unsupported Media Type', + 'details': { + 'errorCount': 1, + 'errorType': 'HTTPUnsupportedMediaType', + 'messageList': [{ + 'error': True, + 'message': ( + "Unexpected content type: application/json. Expected " + "content types are: ['application/x-yaml'].") + }] + }, + 'kind': 'status', + 'message': ("Unexpected content type: application/json. Expected " + "content types are: ['application/x-yaml']."), + 'metadata': {}, + 'reason': 'Unsupported media type', + 'retry': False, + 'status': 'Failure' } self.assertEqual(expected, yaml.safe_load(resp.content)) @@ -65,9 +96,24 @@ class TestYAMLTranslatorNegative(test_base.BaseControllerTest): self.assertEqual(415, resp.status_code) expected = { - 'description': "Unexpected content type: application/yaml. " - "Expected content types are: " - "['application/x-yaml'].", - 'title': 'Unsupported media type' + 'apiVersion': 'N/A', + 'code': '415 Unsupported Media Type', + 'details': { + 'errorCount': 1, + 'errorType': 'HTTPUnsupportedMediaType', + 'messageList': [{ + 'error': True, + 'message': ( + "Unexpected content type: application/yaml. Expected " + "content types are: ['application/x-yaml'].") + }] + }, + 'kind': 'status', + 'message': ("Unexpected content type: application/yaml. Expected " + "content types are: ['application/x-yaml']."), + 'metadata': {}, + 'reason': 'Unsupported media type', + 'retry': False, + 'status': 'Failure' } self.assertEqual(expected, yaml.safe_load(resp.content))