Merge "Add expected errors decorator for more resiliency"

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)

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

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
 
 

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)

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.',
+            'apiVersion': mock.ANY,
-            'title': 'Missing header value'
+            '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. "
+            'apiVersion': mock.ANY,
-                           "Expected content types are: "
+            'code': '415 Unsupported Media Type',
-                           "['application/x-yaml'].",
+            'details': {
-            'title': 'Unsupported media type'
+                '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. "
+            'apiVersion': 'N/A',
-                           "Expected content types are: "
+            'code': '415 Unsupported Media Type',
-                           "['application/x-yaml'].",
+            'details': {
-            'title': 'Unsupported media type'
+                '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))