Merge pull request #31266 from rhcs-dashboard/wip-42565-master

mgr/dashboard: fix LazyUUID4 not serializable

Reviewed-by: Kefu Chai <kchai@redhat.com>
Reviewed-by: Patrick Seidensal <pnawracay@suse.com>
Reviewed-by: Volker Theile <vtheile@suse.com>

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)

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

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):
@@ -118,6 +125,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, headers=None):

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.json_body()
+        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.json_body()
+        self.assertGreater(len(data), 0)
+        self.assertIn('traceback', data)
+        self.assertIn('version', data)
+        self.assertIn('request_id', data)

src/pybind/mgr/dashboard/tests/test_rgw_client.py

@@ -8,31 +8,19 @@ except ImportError:
 
 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)

src/pybind/mgr/dashboard/tests/test_tools.py

@@ -117,7 +117,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'})