From: Ernesto Puerta Date: Wed, 30 Oct 2019 20:50:23 +0000 (+0100) Subject: mgr/dashboard: fix LazyUUID4 not serializable X-Git-Tag: v14.2.8~99^2~2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9c4a79456be66fbaa964c103d2b30f9de400fd77;p=ceph.git mgr/dashboard: fix LazyUUID4 not serializable - Patch older versions of cherrypy (<11.1.0) to include `unique_id` field in Request items. - Add unit tests to verify Debug mode plugin - Fix RGW Client test - Fix Tools test Fixes: https://tracker.ceph.com/issues/42565 Signed-off-by: Ernesto Puerta (cherry picked from commit ae6ab3047b92b82b6aa03dc26dd9b10ebb4264aa) Signed-off-by: Ernesto Puerta Conflicts: - src/pybind/mgr/dashboard/tests/test_plugin_debug.py (json_body to jsonBody, as 38c76db32db4e36a0303dc9321404c9eaad30ecf (#29604) has not been backported yet) --- diff --git a/src/pybind/mgr/dashboard/cherrypy_backports.py b/src/pybind/mgr/dashboard/cherrypy_backports.py index db994b9a7f3..e0449255b56 100644 --- a/src/pybind/mgr/dashboard/cherrypy_backports.py +++ b/src/pybind/mgr/dashboard/cherrypy_backports.py @@ -1,4 +1,36 @@ # -*- coding: utf-8 -*- +""" +Copyright © 2004-2019, CherryPy Team (team@cherrypy.org) + +All rights reserved. + +* * * + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + +* Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + +* Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +* Neither the name of CherryPy nor the names of its + contributors may be used to endorse or promote products derived from + this software without specific prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +""" from distutils.version import StrictVersion @@ -118,8 +150,49 @@ def accept_socket_error_0(v): patch_builtin_ssl_wrap(v, accept_socket_error_0) +def patch_request_unique_id(v): + """ + Older versions of cherrypy don't include request.unique_id field (a lazily + calculated UUID4). + + Monkey-patching is preferred over alternatives as inheritance, as it'd break + type checks (cherrypy/lib/cgtools.py: `isinstance(obj, _cprequest.Request)`) + """ + if v < StrictVersion('11.1.0'): + import uuid + from functools import update_wrapper + from cherrypy._cprequest import Request + + class LazyUUID4(object): + def __str__(self): + """Return UUID4 and keep it for future calls.""" + return str(self.uuid4) + + @property + def uuid4(self): + """Provide unique id on per-request basis using UUID4. + It's evaluated lazily on render. + """ + try: + self._uuid4 + except AttributeError: + # evaluate on first access + self._uuid4 = uuid.uuid4() + + return self._uuid4 + + old_init = Request.__init__ + + def init_with_unique_id(self, *args, **kwargs): + old_init(self, *args, **kwargs) + self.unique_id = LazyUUID4() + + Request.__init__ = update_wrapper(init_with_unique_id, old_init) + + def patch_cherrypy(v): patch_http_connection_init(v) skip_wait_for_occupied_port(v) accept_exceptions_from_builtin_ssl(v) accept_socket_error_0(v) + patch_request_unique_id(v) diff --git a/src/pybind/mgr/dashboard/plugins/debug.py b/src/pybind/mgr/dashboard/plugins/debug.py index f0cbf242a97..f85336f5008 100644 --- a/src/pybind/mgr/dashboard/plugins/debug.py +++ b/src/pybind/mgr/dashboard/plugins/debug.py @@ -3,7 +3,6 @@ from __future__ import absolute_import from enum import Enum import json -import uuid from . import PLUGIN_MANAGER as PM from . import interfaces as I # noqa: E741,N812 @@ -17,7 +16,7 @@ class Actions(Enum): @PM.add_plugin # pylint: disable=too-many-ancestors -class Debug(SP, I.CanCherrypy, I.ConfiguresCherryPy, I.FilterRequest.BeforeHandler): +class Debug(SP, I.CanCherrypy, I.ConfiguresCherryPy): NAME = 'debug' OPTIONS = [ @@ -52,7 +51,7 @@ class Debug(SP, I.CanCherrypy, I.ConfiguresCherryPy, I.FilterRequest.BeforeHandl def custom_error_response(self, status, message, traceback, version): self.response.headers['Content-Type'] = 'application/json' - error_response = dict(status=status, detail=message, request_id=self.request.unique_id) + error_response = dict(status=status, detail=message, request_id=str(self.request.unique_id)) if self.get_option(self.NAME): error_response.update(dict(traceback=traceback, version=version)) @@ -65,9 +64,3 @@ class Debug(SP, I.CanCherrypy, I.ConfiguresCherryPy, I.FilterRequest.BeforeHandl 'environment': 'test_suite' if self.get_option(self.NAME) else 'production', 'error_page.default': self.custom_error_response, }) - - @PM.add_hook - def filter_request_before_handler(self, request): - if not hasattr(request, 'unique_id'): - # Cherrypy v8.9.1 doesn't have this property - request.unique_id = str(uuid.uuid4()) diff --git a/src/pybind/mgr/dashboard/tests/__init__.py b/src/pybind/mgr/dashboard/tests/__init__.py index ee2c4912d38..242beb5238a 100644 --- a/src/pybind/mgr/dashboard/tests/__init__.py +++ b/src/pybind/mgr/dashboard/tests/__init__.py @@ -17,6 +17,13 @@ from ..controllers import json_error_page, generate_controller_routes from ..services.auth import AuthManagerTool from ..services.exception import dashboard_exception_handler +from ..plugins import PLUGIN_MANAGER +from ..plugins import feature_toggles, debug # noqa # pylint: disable=unused-import + + +PLUGIN_MANAGER.hook.init() +PLUGIN_MANAGER.hook.register_commands() + class CmdException(Exception): def __init__(self, retcode, message): @@ -106,6 +113,7 @@ class ControllerTestCase(helper.CPWebCase): 'tools.json_in.on': True, 'tools.json_in.force': False }) + PLUGIN_MANAGER.hook.configure_cherrypy(config=cherrypy.config) super(ControllerTestCase, self).__init__(*args, **kwargs) def _request(self, url, method, data=None): diff --git a/src/pybind/mgr/dashboard/tests/test_plugin_debug.py b/src/pybind/mgr/dashboard/tests/test_plugin_debug.py new file mode 100644 index 00000000000..e6999782d9c --- /dev/null +++ b/src/pybind/mgr/dashboard/tests/test_plugin_debug.py @@ -0,0 +1,38 @@ +# -*- coding: utf-8 -*- +from __future__ import absolute_import + +from . import CLICommandTestMixin, ControllerTestCase + + +class TestPluginDebug(ControllerTestCase, CLICommandTestMixin): + @classmethod + def setup_server(cls): + # pylint: disable=protected-access + cls.setup_controllers([]) + + def setUp(self): + self.mock_kv_store() + + def test_debug_disabled(self): + self.exec_cmd('debug', action='disable') + + self._get('/api/unexisting_controller') + self.assertStatus(404) + + data = self.jsonBody() + self.assertGreater(len(data), 0) + self.assertNotIn('traceback', data) + self.assertNotIn('version', data) + self.assertIn('request_id', data) + + def test_debug_enabled(self): + self.exec_cmd('debug', action='enable') + + self._get('/api/unexisting_controller') + self.assertStatus(404) + + data = self.jsonBody() + self.assertGreater(len(data), 0) + self.assertIn('traceback', data) + self.assertIn('version', data) + self.assertIn('request_id', data) diff --git a/src/pybind/mgr/dashboard/tests/test_rgw_client.py b/src/pybind/mgr/dashboard/tests/test_rgw_client.py index 8bdb01a1b5c..e9e02c6dc37 100644 --- a/src/pybind/mgr/dashboard/tests/test_rgw_client.py +++ b/src/pybind/mgr/dashboard/tests/test_rgw_client.py @@ -3,31 +3,19 @@ import unittest from .. import mgr from ..services.rgw_client import RgwClient +from . import KVStoreMockMixin -class RgwClientTest(unittest.TestCase): - settings = { - 'RGW_API_ACCESS_KEY': 'klausmustermann', - 'RGW_API_SECRET_KEY': 'supergeheim', - 'RGW_API_HOST': 'localhost', - 'RGW_API_USER_ID': 'rgwadmin' - } - - @classmethod - def mock_set_module_option(cls, key, val): - cls.settings[key] = val - - @classmethod - def mock_get_module_option(cls, key, default): - return cls.settings.get(key, default) - - @classmethod - def setUpClass(cls): - mgr.get_module_option.side_effect = cls.mock_get_module_option - mgr.set_module_option.side_effect = cls.mock_set_module_option - +class RgwClientTest(unittest.TestCase, KVStoreMockMixin): def setUp(self): RgwClient._user_instances.clear() # pylint: disable=protected-access + self.mock_kv_store() + self.CONFIG_KEY_DICT.update({ + 'RGW_API_ACCESS_KEY': 'klausmustermann', + 'RGW_API_SECRET_KEY': 'supergeheim', + 'RGW_API_HOST': 'localhost', + 'RGW_API_USER_ID': 'rgwadmin' + }) def test_ssl_verify(self): mgr.set_module_option('RGW_API_SSL_VERIFY', True) diff --git a/src/pybind/mgr/dashboard/tests/test_tools.py b/src/pybind/mgr/dashboard/tests/test_tools.py index 028ec708773..1c2c2e5bd34 100644 --- a/src/pybind/mgr/dashboard/tests/test_tools.py +++ b/src/pybind/mgr/dashboard/tests/test_tools.py @@ -115,7 +115,6 @@ class RESTControllerTest(ControllerTestCase): self.assertIsInstance(body, dict) assert body['detail'] == "The path '/foo' was not found." assert '404' in body['status'] - assert 'traceback' in body def test_args_from_json(self): self._put("/api/fooargs/hello", {'name': 'world'})