airship
/
deckhand
Code Issues Proposed changes

Add expected errors decorator for more resiliency 

To increase resiliency, add Deckhand error handling hooks to
format unknown errors into something more useful for debugging.
Also override exception formatting to be consistent with UCP
error formatting standard.

Most of this logic is borrowed from Shipyard for consistency.

Also includes basic unit tests to validate error formatting.

Change-Id: If7f8c3bf6b6ada7697611a0bef7bf8f635fc0b7f
This commit is contained in:
Felipe Monteiro 2017-10-24 15:47:30 +01:00
parent 0e0d96ef71
commit 514338c3bf
5 changed files with 309 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
deckhand/control/revision_documents.py
View File

@ -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)

132
deckhand/errors.py
View File

@ -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

6
deckhand/service.py
View File

@ -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

104
deckhand/tests/unit/control/test_errors.py Normal file
View File

@ -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)

66
deckhand/tests/unit/control/test_middleware.py
View File

@ -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))