diff --git a/watcher/api/hooks.py b/watcher/api/hooks.py index c76ad2d6e..db0bf78c0 100644 --- a/watcher/api/hooks.py +++ b/watcher/api/hooks.py @@ -18,6 +18,7 @@ from oslo_config import cfg from oslo_utils import importutils from pecan import hooks +from six.moves import http_client from watcher.common import context @@ -95,18 +96,20 @@ class NoExceptionTracebackHook(hooks.PecanHook): return # Do nothing if there is no error. - if 200 <= state.response.status_int < 400: + # Status codes in the range 200 (OK) to 399 (400 = BAD_REQUEST) are not + # an error. + if (http_client.OK <= state.response.status_int < + http_client.BAD_REQUEST): return json_body = state.response.json - # Do not remove traceback when server in debug mode (except 'Server' - # errors when 'debuginfo' will be used for traces). - if cfg.CONF.debug and json_body.get('faultcode') != 'Server': + # Do not remove traceback when traceback config is set + if cfg.CONF.debug: return faultstring = json_body.get('faultstring') traceback_marker = 'Traceback (most recent call last):' - if faultstring and (traceback_marker in faultstring): + if faultstring and traceback_marker in faultstring: # Cut-off traceback. faultstring = faultstring.split(traceback_marker, 1)[0] # Remove trailing newlines and spaces if any. diff --git a/watcher/api/middleware/parsable_error.py b/watcher/api/middleware/parsable_error.py index 4c94a50e6..4acfe10ee 100644 --- a/watcher/api/middleware/parsable_error.py +++ b/watcher/api/middleware/parsable_error.py @@ -27,14 +27,14 @@ from oslo_serialization import jsonutils import six import webob -from watcher._i18n import _ -from watcher._i18n import _LE +from watcher._i18n import _, _LE LOG = log.getLogger(__name__) class ParsableErrorMiddleware(object): """Replace error body with something the client can parse.""" + def __init__(self, app): self.app = app @@ -59,8 +59,7 @@ class ParsableErrorMiddleware(object): # compute the length. headers = [(h, v) for (h, v) in headers - if h not in ('Content-Length', 'Content-Type') - ] + if h not in ('Content-Length', 'Content-Type')] # Save the headers in case we need to modify them. state['headers'] = headers return start_response(status, headers, exc_info) @@ -68,24 +67,27 @@ class ParsableErrorMiddleware(object): app_iter = self.app(environ, replacement_start_response) if (state['status_code'] // 100) not in (2, 3): req = webob.Request(environ) - if (req.accept.best_match(['application/json', 'application/xml'] - ) == 'application/xml'): + if ( + req.accept.best_match( + ['application/json', + 'application/xml']) == 'application/xml' + ): try: # simple check xml is valid - body = [et.ElementTree.tostring( - et.ElementTree.Element('error_message', - text='\n'.join(app_iter)))] + body = [ + et.ElementTree.tostring( + et.ElementTree.Element( + 'error_message', text='\n'.join(app_iter)))] except et.ElementTree.ParseError as err: LOG.error(_LE('Error parsing HTTP response: %s'), err) - body = [et.ElementTree.tostring( - et.ElementTree.Element('error_message', - text=state['status_code']))] + body = ['%s' + '' % state['status_code']] state['headers'].append(('Content-Type', 'application/xml')) else: if six.PY3: app_iter = [i.decode('utf-8') for i in app_iter] body = [jsonutils.dumps( - {'error_message': '\n'.join(app_iter)})] + {'error_message': '\n'.join(app_iter)})] if six.PY3: body = [item.encode('utf-8') for item in body] state['headers'].append(('Content-Type', 'application/json')) diff --git a/watcher/tests/api/test_hooks.py b/watcher/tests/api/test_hooks.py index b4701648e..c1d692d61 100644 --- a/watcher/tests/api/test_hooks.py +++ b/watcher/tests/api/test_hooks.py @@ -1,17 +1,18 @@ -# Copyright 2014 -# The Cloudscaling Group, Inc. +# -*- encoding: utf-8 -*- # -# 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 +# 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 +# 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. +# 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. + +"""Tests for the Pecan API hooks.""" from __future__ import unicode_literals @@ -19,64 +20,100 @@ import mock from oslo_config import cfg import oslo_messaging as messaging from oslo_serialization import jsonutils +import six +from six.moves import http_client from watcher.api.controllers import root from watcher.api import hooks -from watcher.common import context as watcher_context -from watcher.tests.api import base as api_base -from watcher.tests import base -from watcher.tests import fakes +from watcher.common import context +from watcher.tests.api import base -class TestContextHook(base.BaseTestCase): - - def setUp(self): - super(TestContextHook, self).setUp() - self.app = fakes.FakeApp() - - def test_context_hook_before_method(self): - state = mock.Mock(request=fakes.FakePecanRequest()) - hook = hooks.ContextHook() - hook.before(state) - ctx = state.request.context - self.assertIsInstance(ctx, watcher_context.RequestContext) - self.assertEqual(ctx.auth_token, - fakes.fakeAuthTokenHeaders['X-Auth-Token']) - self.assertEqual(ctx.project_id, - fakes.fakeAuthTokenHeaders['X-Project-Id']) - self.assertEqual(ctx.user_id, - fakes.fakeAuthTokenHeaders['X-User-Id']) - self.assertEqual(ctx.auth_url, - fakes.fakeAuthTokenHeaders['X-Auth-Url']) - self.assertEqual(ctx.domain_name, - fakes.fakeAuthTokenHeaders['X-User-Domain-Name']) - self.assertEqual(ctx.domain_id, - fakes.fakeAuthTokenHeaders['X-User-Domain-Id']) - self.assertIsNone(ctx.auth_token_info) - - def test_context_hook_before_method_auth_info(self): - state = mock.Mock(request=fakes.FakePecanRequest()) - state.request.environ['keystone.token_info'] = 'assert_this' - hook = hooks.ContextHook() - hook.before(state) - ctx = state.request.context - self.assertIsInstance(ctx, watcher_context.RequestContext) - self.assertEqual(fakes.fakeAuthTokenHeaders['X-Auth-Token'], - ctx.auth_token) - self.assertEqual('assert_this', ctx.auth_token_info) +class FakeRequest(object): + def __init__(self, headers, context, environ): + self.headers = headers + self.context = context + self.environ = environ or {} + self.version = (1, 0) + self.host_url = 'http://127.0.0.1:6385' -class TestNoExceptionTracebackHook(api_base.FunctionalTest): +class FakeRequestState(object): + def __init__(self, headers=None, context=None, environ=None): + self.request = FakeRequest(headers, context, environ) + self.response = FakeRequest(headers, context, environ) - TRACE = [ - 'Traceback (most recent call last):', - ' File "/opt/stack/watcher/watcher/openstack/common/rpc/amqp.py",' - ' line 434, in _process_data\\n **args)', - ' File "/opt/stack/watcher/watcher/openstack/common/rpc/' - 'dispatcher.py", line 172, in dispatch\\n result =' - ' getattr(proxyobj, method)(context, **kwargs)'] + def set_context(self): + headers = self.request.headers + creds = { + 'user': headers.get('X-User') or headers.get('X-User-Id'), + 'domain_id': headers.get('X-User-Domain-Id'), + 'domain_name': headers.get('X-User-Domain-Name'), + 'auth_token': headers.get('X-Auth-Token'), + 'roles': headers.get('X-Roles', '').split(','), + } + is_admin = ('admin' in creds['roles'] or + 'administrator' in creds['roles']) + is_public_api = self.request.environ.get('is_public_api', False) + + self.request.context = context.RequestContext( + is_admin=is_admin, is_public_api=is_public_api, **creds) + + +def fake_headers(admin=False): + headers = { + 'X-Auth-Token': '8d9f235ca7464dd7ba46f81515797ea0', + 'X-Domain-Id': 'None', + 'X-Domain-Name': 'None', + 'X-Project-Domain-Id': 'default', + 'X-Project-Domain-Name': 'Default', + 'X-Role': '_member_,admin', + 'X-Roles': '_member_,admin', + # 'X-Tenant': 'foo', + # 'X-Tenant-Id': 'b4efa69d4ffa4973863f2eefc094f7f8', + # 'X-Tenant-Name': 'foo', + 'X-User': 'foo', + 'X-User-Domain-Id': 'default', + 'X-User-Domain-Name': 'Default', + 'X-User-Id': '604ab2a197c442c2a84aba66708a9e1e', + 'X-User-Name': 'foo', + } + if admin: + headers.update({ + 'X-Project-Name': 'admin', + 'X-Role': '_member_,admin', + 'X-Roles': '_member_,admin', + 'X-Tenant': 'admin', + # 'X-Tenant-Name': 'admin', + # 'X-Tenant': 'admin' + 'X-Tenant-Name': 'admin', + 'X-Tenant-Id': 'c2a3a69d456a412376efdd9dac38', + 'X-Project-Name': 'admin', + 'X-Project-Id': 'c2a3a69d456a412376efdd9dac38', + }) + else: + headers.update({ + 'X-Role': '_member_', + 'X-Roles': '_member_', + 'X-Tenant': 'foo', + 'X-Tenant-Name': 'foo', + 'X-Tenant-Id': 'b4efa69d,4ffa4973863f2eefc094f7f8', + 'X-Project-Name': 'foo', + 'X-Project-Id': 'b4efa69d4ffa4973863f2eefc094f7f8', + }) + return headers + + +class TestNoExceptionTracebackHook(base.FunctionalTest): + + TRACE = ['Traceback (most recent call last):', + ' File "/opt/stack/watcher/watcher/common/rpc/amqp.py",' + ' line 434, in _process_data\\n **args)', + ' File "/opt/stack/watcher/watcher/common/rpc/' + 'dispatcher.py", line 172, in dispatch\\n result =' + ' getattr(proxyobj, method)(ctxt, **kwargs)'] MSG_WITHOUT_TRACE = "Test exception message." - MSG_WITH_TRACE = "{0}\n{1}".format(MSG_WITHOUT_TRACE, "\n".join(TRACE)) + MSG_WITH_TRACE = MSG_WITHOUT_TRACE + "\n" + "\n".join(TRACE) def setUp(self): super(TestNoExceptionTracebackHook, self).setUp() @@ -96,7 +133,7 @@ class TestNoExceptionTracebackHook(api_base.FunctionalTest): def test_hook_remote_error_success(self): test_exc_type = 'TestException' self.root_convert_mock.side_effect = messaging.rpc.RemoteError( - test_exc_type, self.MSG_WITHOUT_TRACE, "\n".join(self.TRACE)) + test_exc_type, self.MSG_WITHOUT_TRACE, self.TRACE) response = self.get_json('/', path_prefix='', expect_errors=True) @@ -106,12 +143,13 @@ class TestNoExceptionTracebackHook(api_base.FunctionalTest): # rare thing (happens due to wrong deserialization settings etc.) # we don't care about this garbage. expected_msg = ("Remote error: %s %s" - % (test_exc_type, self.MSG_WITHOUT_TRACE)) + % (test_exc_type, self.MSG_WITHOUT_TRACE) + + ("\n[u'" if six.PY2 else "\n['")) actual_msg = jsonutils.loads( response.json['error_message'])['faultstring'] self.assertEqual(expected_msg, actual_msg) - def test_hook_without_traceback(self): + def _test_hook_without_traceback(self): msg = "Error message without traceback \n but \n multiline" self.root_convert_mock.side_effect = Exception(msg) @@ -121,24 +159,118 @@ class TestNoExceptionTracebackHook(api_base.FunctionalTest): response.json['error_message'])['faultstring'] self.assertEqual(msg, actual_msg) - def test_hook_server_debug_on_serverfault(self): + def test_hook_without_traceback(self): + self._test_hook_without_traceback() + + def test_hook_without_traceback_debug(self): cfg.CONF.set_override('debug', True, enforce_type=True) + self._test_hook_without_traceback() + + def test_hook_without_traceback_debug_tracebacks(self): + cfg.CONF.set_override('debug', True, enforce_type=True) + self._test_hook_without_traceback() + + def _test_hook_on_serverfault(self): self.root_convert_mock.side_effect = Exception(self.MSG_WITH_TRACE) response = self.get_json('/', path_prefix='', expect_errors=True) actual_msg = jsonutils.loads( response.json['error_message'])['faultstring'] - self.assertEqual(self.MSG_WITHOUT_TRACE, actual_msg) + return actual_msg - def test_hook_server_debug_on_clientfault(self): + def test_hook_on_serverfault(self): + cfg.CONF.set_override('debug', False, enforce_type=True) + msg = self._test_hook_on_serverfault() + self.assertEqual(self.MSG_WITHOUT_TRACE, msg) + + def test_hook_on_serverfault_debug(self): cfg.CONF.set_override('debug', True, enforce_type=True) + msg = self._test_hook_on_serverfault() + self.assertEqual(self.MSG_WITH_TRACE, msg) + + def _test_hook_on_clientfault(self): client_error = Exception(self.MSG_WITH_TRACE) - client_error.code = 400 + client_error.code = http_client.BAD_REQUEST self.root_convert_mock.side_effect = client_error response = self.get_json('/', path_prefix='', expect_errors=True) actual_msg = jsonutils.loads( response.json['error_message'])['faultstring'] - self.assertEqual(self.MSG_WITH_TRACE, actual_msg) + return actual_msg + + def test_hook_on_clientfault(self): + msg = self._test_hook_on_clientfault() + self.assertEqual(self.MSG_WITHOUT_TRACE, msg) + + def test_hook_on_clientfault_debug_tracebacks(self): + cfg.CONF.set_override('debug', True, enforce_type=True) + msg = self._test_hook_on_clientfault() + self.assertEqual(self.MSG_WITH_TRACE, msg) + + +class TestContextHook(base.FunctionalTest): + @mock.patch.object(context, 'RequestContext') + def test_context_hook_not_admin(self, mock_ctx): + cfg.CONF.set_override( + 'auth_type', 'password', group='watcher_clients_auth') + headers = fake_headers(admin=False) + reqstate = FakeRequestState(headers=headers) + context_hook = hooks.ContextHook() + context_hook.before(reqstate) + mock_ctx.assert_called_with( + auth_token=headers['X-Auth-Token'], + user=headers['X-User'], + user_id=headers['X-User-Id'], + domain_id=headers['X-User-Domain-Id'], + domain_name=headers['X-User-Domain-Name'], + auth_url=cfg.CONF.keystone_authtoken.auth_uri, + project=headers['X-Project-Name'], + project_id=headers['X-Project-Id'], + show_deleted=None, + auth_token_info=self.token_info, + roles=headers['X-Roles'].split(',')) + + @mock.patch.object(context, 'RequestContext') + def test_context_hook_admin(self, mock_ctx): + cfg.CONF.set_override( + 'auth_type', 'password', group='watcher_clients_auth') + headers = fake_headers(admin=True) + reqstate = FakeRequestState(headers=headers) + context_hook = hooks.ContextHook() + context_hook.before(reqstate) + mock_ctx.assert_called_with( + auth_token=headers['X-Auth-Token'], + user=headers['X-User'], + user_id=headers['X-User-Id'], + domain_id=headers['X-User-Domain-Id'], + domain_name=headers['X-User-Domain-Name'], + auth_url=cfg.CONF.keystone_authtoken.auth_uri, + project=headers['X-Project-Name'], + project_id=headers['X-Project-Id'], + show_deleted=None, + auth_token_info=self.token_info, + roles=headers['X-Roles'].split(',')) + + @mock.patch.object(context, 'RequestContext') + def test_context_hook_public_api(self, mock_ctx): + cfg.CONF.set_override( + 'auth_type', 'password', group='watcher_clients_auth') + headers = fake_headers(admin=True) + env = {'is_public_api': True} + reqstate = FakeRequestState(headers=headers, environ=env) + context_hook = hooks.ContextHook() + context_hook.before(reqstate) + mock_ctx.assert_called_with( + auth_token=headers['X-Auth-Token'], + user=headers['X-User'], + user_id=headers['X-User-Id'], + domain_id=headers['X-User-Domain-Id'], + domain_name=headers['X-User-Domain-Name'], + auth_url=cfg.CONF.keystone_authtoken.auth_uri, + project=headers['X-Project-Name'], + project_id=headers['X-Project-Id'], + show_deleted=None, + auth_token_info=self.token_info, + roles=headers['X-Roles'].split(',')) diff --git a/watcher/tests/base.py b/watcher/tests/base.py index a2f45aa7c..b882c2687 100644 --- a/watcher/tests/base.py +++ b/watcher/tests/base.py @@ -66,7 +66,7 @@ class TestCase(BaseTestCase): app_config_path = os.path.join(os.path.dirname(__file__), 'config.py') self.app = testing.load_test_app(app_config_path) - token_info = { + self.token_info = { 'token': { 'project': { 'id': 'fake_project' @@ -77,14 +77,16 @@ class TestCase(BaseTestCase): } } self.context = watcher_context.RequestContext( - auth_token_info=token_info, + auth_token_info=self.token_info, project_id='fake_project', user_id='fake_user') + self.policy = self.useFixture(policy_fixture.PolicyFixture()) + def make_context(*args, **kwargs): # If context hasn't been constructed with token_info if not kwargs.get('auth_token_info'): - kwargs['auth_token_info'] = copy.deepcopy(token_info) + kwargs['auth_token_info'] = copy.deepcopy(self.token_info) if not kwargs.get('project_id'): kwargs['project_id'] = 'fake_project' if not kwargs.get('user_id'):