From d3352ff422db6ba6a5e7bd4f7220af0d97efd0ac Mon Sep 17 00:00:00 2001 From: Chris Dent Date: Tue, 20 Feb 2018 10:31:49 +0000 Subject: [PATCH] Identify the keystone service when raising 503 When the keystonemiddleware is used directly in the WSGI stack of an application, the 503 that is raised when the keystone service errors or cannot be reached needs to identify that keystone is the service that has failed, otherwise it appears to the client that it is the service they are trying to access is down, which is misleading. This addresses the problem in the most straightforward way possible: the exception that causes the 503 is given a message including the word "Keystone". The call method in BaseAuthTokenTestCase gains an expected_body_string kwarg. If not None, the response body (as a six.text_type) is compared with the value. Change-Id: Idf211e7bc99139744af232f5ea3ecb4be41551ca Closes-Bug: #1747655 Closes-Bug: #1749797 --- keystonemiddleware/auth_token/__init__.py | 3 ++- keystonemiddleware/tests/unit/auth_token/base.py | 6 +++++- .../tests/unit/auth_token/test_auth_token_middleware.py | 4 +++- releasenotes/notes/bug-1747655-6e563d9317bb0f13.yaml | 9 +++++++++ 4 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 releasenotes/notes/bug-1747655-6e563d9317bb0f13.yaml diff --git a/keystonemiddleware/auth_token/__init__.py b/keystonemiddleware/auth_token/__init__.py index 67668185..48140b0c 100644 --- a/keystonemiddleware/auth_token/__init__.py +++ b/keystonemiddleware/auth_token/__init__.py @@ -778,7 +778,8 @@ class AuthProtocol(BaseAuthProtocol): ksm_exceptions.RevocationListError, ksm_exceptions.ServiceError) as e: self.log.critical('Unable to validate token: %s', e) - raise webob.exc.HTTPServiceUnavailable() + raise webob.exc.HTTPServiceUnavailable( + 'The Keystone service is temporarily unavailable.') except ksm_exceptions.InvalidToken: self.log.debug('Token validation failure.', exc_info=True) if token_hashes: diff --git a/keystonemiddleware/tests/unit/auth_token/base.py b/keystonemiddleware/tests/unit/auth_token/base.py index 775ddc32..23b86736 100644 --- a/keystonemiddleware/tests/unit/auth_token/base.py +++ b/keystonemiddleware/tests/unit/auth_token/base.py @@ -15,6 +15,7 @@ from oslo_config import cfg from oslo_config import fixture as cfg_fixture from oslo_log import log as logging from requests_mock.contrib import fixture as rm_fixture +import six from six.moves import http_client import webob.dec @@ -48,7 +49,8 @@ class BaseAuthTokenTestCase(utils.MiddlewareTestCase): return auth_token.AuthProtocol(_do_cb, opts) def call(self, middleware, method='GET', path='/', headers=None, - expected_status=http_client.OK): + expected_status=http_client.OK, + expected_body_string=None): req = webob.Request.blank(path) req.method = method @@ -57,5 +59,7 @@ class BaseAuthTokenTestCase(utils.MiddlewareTestCase): resp = req.get_response(middleware) self.assertEqual(expected_status, resp.status_int) + if expected_body_string: + self.assertIn(expected_body_string, six.text_type(resp.body)) resp.request = req return resp diff --git a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py index 5c59d145..efcd7efd 100644 --- a/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py +++ b/keystonemiddleware/tests/unit/auth_token/test_auth_token_middleware.py @@ -1071,13 +1071,15 @@ class CommonAuthTokenMiddlewareTest(object): def test_http_request_max_retries(self): times_retry = 10 + body_string = 'The Keystone service is temporarily unavailable.' conf = {'http_request_max_retries': '%s' % times_retry} self.set_middleware(conf=conf) with mock.patch('time.sleep') as mock_obj: self.call_middleware(headers={'X-Auth-Token': ERROR_TOKEN}, - expected_status=503) + expected_status=503, + expected_body_string=body_string) self.assertEqual(mock_obj.call_count, times_retry) diff --git a/releasenotes/notes/bug-1747655-6e563d9317bb0f13.yaml b/releasenotes/notes/bug-1747655-6e563d9317bb0f13.yaml new file mode 100644 index 00000000..65f4c5aa --- /dev/null +++ b/releasenotes/notes/bug-1747655-6e563d9317bb0f13.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + [`bug/1747655 `_] + When keystone is temporarily unavailable, keystonemiddleware correctly + sends a 503 response to the HTTP client but was not identifying which + service was down, leading to confusion on whether it was keystone or the + service using keystonemiddleware that was unavailable. This change + identifies keystone in the error response.