mgr/dashboard: API Version changes doesnt appy to pre-defined methods(list, create...

Methods like list(), create(), get() etc doesn't get applied the version.Also for the endpoints that get the version changed, the docs and the request header has still the version v1.0+ in them. So with the version reduced it gives 415 error when trying to make the request. This PR fixes this issue.

Fixes: https://tracker.ceph.com/issues/50855
Signed-off-by: Aashish Sharma <aasharma@redhat.com>
(cherry picked from commit bb52092cd37855a3dfcfae9287e231865e9af3fa)

Conflicts:
    src/pybind/mgr/dashboard/controllers/__init__.py(resolved conflicts due to https://github.com/ceph/ceph/pull/40063)
    src/pybind/mgr/dashboard/tests/test_docs.py(removed unused import DEFAULT_VERSION)

diff --git a/doc/dev/developer_guide/dash-devel.rst b/doc/dev/developer_guide/dash-devel.rst
index 130a6b9a65d40e74368d90f1193222e918a246ed..7f02e36cffa4c76d57edb5e618fea09e2416e236 100644 (file)
--- a/doc/dev/developer_guide/dash-devel.rst
+++ b/doc/dev/developer_guide/dash-devel.rst
@@ -1466,6 +1466,25 @@ same applies to other request types:
 | DELETE       | Yes        | delete         | 204         |
 +--------------+------------+----------------+-------------+
 
+To use a custom endpoint for the above listed methods, you can
+use ``@RESTController.MethodMap``
+
+.. code-block:: python
+
+  import cherrypy
+  from ..tools import ApiController, RESTController
+
+    @RESTController.MethodMap(version='0.1')
+    def create(self):
+      return {"msg": "Hello"}
+
+This decorator supports three parameters to customize the
+endpoint:
+
+* ``resource"``: resource id.
+* ``status=200``: set the HTTP status response code
+* ``version``: version
+
 How to use a custom API endpoint in a RESTController?
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
@@ -1494,7 +1513,7 @@ used. To use a custom endpoint inside a restricted ``RESTController`` use
     def some_post_endpoint(self, **data):
       return {"msg": data}
 
-Both decorators also support four parameters to customize the
+Both decorators also support five parameters to customize the
 endpoint:
 
 * ``method="GET"``: the HTTP method allowed to access this endpoint.
@@ -1503,6 +1522,7 @@ endpoint:
 * ``status=200``: set the HTTP status response code
 * ``query_params=[]``: list of method parameter names that correspond to URL
   query parameters.
+* ``version``: version
 
 How to restrict access to a controller?
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

diff --git a/src/pybind/mgr/dashboard/controllers/__init__.py b/src/pybind/mgr/dashboard/controllers/__init__.py
index 77b5b4747364d4b8f28fc6ac47e57fb3ea26fe3f..7daf2301d351f3b07de90d01bf44495d42ef8166 100644 (file)
--- a/src/pybind/mgr/dashboard/controllers/__init__.py
+++ b/src/pybind/mgr/dashboard/controllers/__init__.py
@@ -788,14 +788,14 @@ class RESTController(BaseController):
     }
 
     _method_mapping = collections.OrderedDict([
-        ('list', {'method': 'GET', 'resource': False, 'status': 200}),
-        ('create', {'method': 'POST', 'resource': False, 'status': 201}),
-        ('bulk_set', {'method': 'PUT', 'resource': False, 'status': 200}),
-        ('bulk_delete', {'method': 'DELETE', 'resource': False, 'status': 204}),
-        ('get', {'method': 'GET', 'resource': True, 'status': 200}),
-        ('delete', {'method': 'DELETE', 'resource': True, 'status': 204}),
-        ('set', {'method': 'PUT', 'resource': True, 'status': 200}),
-        ('singleton_set', {'method': 'PUT', 'resource': False, 'status': 200})
+        ('list', {'method': 'GET', 'resource': False, 'status': 200, 'version': DEFAULT_VERSION}),
+        ('create', {'method': 'POST', 'resource': False, 'status': 201, 'version': DEFAULT_VERSION}),  # noqa E501 #pylint: disable=line-too-long
+        ('bulk_set', {'method': 'PUT', 'resource': False, 'status': 200, 'version': DEFAULT_VERSION}),  # noqa E501 #pylint: disable=line-too-long
+        ('bulk_delete', {'method': 'DELETE', 'resource': False, 'status': 204, 'version': DEFAULT_VERSION}),  # noqa E501 #pylint: disable=line-too-long
+        ('get', {'method': 'GET', 'resource': True, 'status': 200, 'version': DEFAULT_VERSION}),
+        ('delete', {'method': 'DELETE', 'resource': True, 'status': 204, 'version': DEFAULT_VERSION}),  # noqa E501 #pylint: disable=line-too-long
+        ('set', {'method': 'PUT', 'resource': True, 'status': 200, 'version': DEFAULT_VERSION}),
+        ('singleton_set', {'method': 'PUT', 'resource': False, 'status': 200, 'version': DEFAULT_VERSION})  # noqa E501 #pylint: disable=line-too-long
     ])
 
     @classmethod
@@ -840,35 +840,37 @@ class RESTController(BaseController):
 
                 status = meth['status']
                 method = meth['method']
+                if hasattr(func, "__method_map_method__"):
+                    version = func.__method_map_method__['version']
                 if not sec_permissions:
                     permission = cls._permission_map[method]
 
-            elif hasattr(func, "_collection_method_"):
-                if func._collection_method_['path']:
-                    path = func._collection_method_['path']
+            elif hasattr(func, "__collection_method__"):
+                if func.__collection_method__['path']:
+                    path = func.__collection_method__['path']
                 else:
                     path = "/{}".format(func.__name__)
-                status = func._collection_method_['status']
-                method = func._collection_method_['method']
-                query_params = func._collection_method_['query_params']
-                version = func._collection_method_['version']
+                status = func.__collection_method__['status']
+                method = func.__collection_method__['method']
+                query_params = func.__collection_method__['query_params']
+                version = func.__collection_method__['version']
                 if not sec_permissions:
                     permission = cls._permission_map[method]
 
-            elif hasattr(func, "_resource_method_"):
+            elif hasattr(func, "__resource_method__"):
                 if not res_id_params:
                     no_resource_id_params = True
                 else:
                     path_params = ["{{{}}}".format(p) for p in res_id_params]
                     path += "/{}".format("/".join(path_params))
-                    if func._resource_method_['path']:
-                        path += func._resource_method_['path']
+                    if func.__resource_method__['path']:
+                        path += func.__resource_method__['path']
                     else:
                         path += "/{}".format(func.__name__)
-                status = func._resource_method_['status']
-                method = func._resource_method_['method']
-                version = func._resource_method_['version']
-                query_params = func._resource_method_['query_params']
+                status = func.__resource_method__['status']
+                method = func.__resource_method__['method']
+                version = func.__resource_method__['version']
+                query_params = func.__resource_method__['query_params']
                 if not sec_permissions:
                     permission = cls._permission_map[method]
 
@@ -918,7 +920,7 @@ class RESTController(BaseController):
             status = 200
 
         def _wrapper(func):
-            func._resource_method_ = {
+            func.__resource_method__ = {
                 'method': method,
                 'path': path,
                 'status': status,
@@ -928,6 +930,21 @@ class RESTController(BaseController):
             return func
         return _wrapper
 
+    @staticmethod
+    def MethodMap(resource=False, status=None, version=DEFAULT_VERSION):  # noqa: N802
+
+        if status is None:
+            status = 200
+
+        def _wrapper(func):
+            func.__method_map_method__ = {
+                'resource': resource,
+                'status': status,
+                'version': version
+            }
+            return func
+        return _wrapper
+
     @staticmethod
     def Collection(method=None, path=None, status=None, query_params=None,  # noqa: N802
                    version=DEFAULT_VERSION):
@@ -938,7 +955,7 @@ class RESTController(BaseController):
             status = 200
 
         def _wrapper(func):
-            func._collection_method_ = {
+            func.__collection_method__ = {
                 'method': method,
                 'path': path,
                 'status': status,

diff --git a/src/pybind/mgr/dashboard/controllers/docs.py b/src/pybind/mgr/dashboard/controllers/docs.py
index e7ed9742ab9d25e91915283c588dd9b4cdedd2fc..87c46e327bd605e1053457ec172f3587581186da 100644 (file)
--- a/src/pybind/mgr/dashboard/controllers/docs.py
+++ b/src/pybind/mgr/dashboard/controllers/docs.py
@@ -185,7 +185,7 @@ class Docs(BaseController):
         return schema.as_dict()
 
     @classmethod
-    def _gen_responses(cls, method, resp_object=None):
+    def _gen_responses(cls, method, resp_object=None, version=None):
         resp: Dict[str, Dict[str, Union[str, Any]]] = {
             '400': {
                 "description": "Operation exception. Please check the "
@@ -203,26 +203,30 @@ class Docs(BaseController):
                                "response body for the stack trace."
             }
         }
+
+        if not version:
+            version = DEFAULT_VERSION
+
         if method.lower() == 'get':
             resp['200'] = {'description': "OK",
-                           'content': {'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION):
+                           'content': {'application/vnd.ceph.api.v{}+json'.format(version):
                                        {'type': 'object'}}}
         if method.lower() == 'post':
             resp['201'] = {'description': "Resource created.",
-                           'content': {'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION):
+                           'content': {'application/vnd.ceph.api.v{}+json'.format(version):
                                        {'type': 'object'}}}
         if method.lower() == 'put':
             resp['200'] = {'description': "Resource updated.",
-                           'content': {'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION):
+                           'content': {'application/vnd.ceph.api.v{}+json'.format(version):
                                        {'type': 'object'}}}
         if method.lower() == 'delete':
             resp['204'] = {'description': "Resource deleted.",
-                           'content': {'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION):
+                           'content': {'application/vnd.ceph.api.v{}+json'.format(version):
                                        {'type': 'object'}}}
         if method.lower() in ['post', 'put', 'delete']:
             resp['202'] = {'description': "Operation is still executing."
                                           " Please check the task queue.",
-                           'content': {'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION):
+                           'content': {'application/vnd.ceph.api.v{}+json'.format(version):
                                        {'type': 'object'}}}
 
         if resp_object:
@@ -230,7 +234,7 @@ class Docs(BaseController):
                 if status_code in resp:
                     resp[status_code].update({
                         'content': {
-                            'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION): {
+                            'application/vnd.ceph.api.v{}+json'.format(version): {
                                 'schema': cls._gen_schema_for_content(response_body)}}})
 
         return resp
@@ -263,7 +267,7 @@ class Docs(BaseController):
         return parameters
 
     @classmethod
-    def _gen_paths(cls, all_endpoints):
+    def gen_paths(cls, all_endpoints):
         # pylint: disable=R0912
         method_order = ['get', 'post', 'put', 'delete']
         paths = {}
@@ -283,8 +287,19 @@ class Docs(BaseController):
                 func = endpoint.func
 
                 summary = ''
+                version = ''
                 resp = {}
                 p_info = []
+
+                if hasattr(func, 'method_map_method'):
+                    version = func.method_map_method['version']
+
+                elif hasattr(func, 'resource_method'):
+                    version = func.resource_method['version']
+
+                elif hasattr(func, 'collection_method'):
+                    version = func.collection_method['version']
+
                 if hasattr(func, 'doc_info'):
                     if func.doc_info['summary']:
                         summary = func.doc_info['summary']
@@ -304,7 +319,7 @@ class Docs(BaseController):
                     'tags': [cls._get_tag(endpoint)],
                     'description': func.__doc__,
                     'parameters': params,
-                    'responses': cls._gen_responses(method, resp)
+                    'responses': cls._gen_responses(method, resp, version)
                 }
                 if summary:
                     methods[method.lower()]['summary'] = summary
@@ -340,7 +355,7 @@ class Docs(BaseController):
         host = cherrypy.request.base.split('://', 1)[1] if not offline else 'example.com'
         logger.debug("Host: %s", host)
 
-        paths = cls._gen_paths(all_endpoints)
+        paths = cls.gen_paths(all_endpoints)
 
         if not base_url:
             base_url = "/"

diff --git a/src/pybind/mgr/dashboard/tests/test_docs.py b/src/pybind/mgr/dashboard/tests/test_docs.py
index 7f4ea64003e13fc3908003299b7b98f392f04f74..623d5baf6efa16058f191e0703ccdca08419c47f 100644 (file)
--- a/src/pybind/mgr/dashboard/tests/test_docs.py
+++ b/src/pybind/mgr/dashboard/tests/test_docs.py
@@ -30,10 +30,14 @@ class DecoratedController(RESTController):
         },
     )
     @Endpoint(json_response=False)
-    @RESTController.Resource('PUT')
+    @RESTController.Resource('PUT', version='0.1')
     def decorated_func(self, parameter):
         pass
 
+    @RESTController.MethodMap(version='0.1')
+    def list(self):
+        pass
+
 
 # To assure functionality of @EndpointDoc, @GroupDoc
 class DocDecoratorsTest(ControllerTestCase):
@@ -75,7 +79,7 @@ class DocsTest(ControllerTestCase):
         self.assertEqual(Docs()._type_to_str(None), str(SchemaType.OBJECT))
 
     def test_gen_paths(self):
-        outcome = Docs()._gen_paths(False)['/api/doctest//{doctest}/decorated_func']['put']
+        outcome = Docs().gen_paths(False)['/api/doctest//{doctest}/decorated_func']['put']
         self.assertIn('tags', outcome)
         self.assertIn('summary', outcome)
         self.assertIn('parameters', outcome)
@@ -83,7 +87,7 @@ class DocsTest(ControllerTestCase):
 
         expected_response_content = {
             '200': {
-                'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION): {
+                'application/vnd.ceph.api.v0.1+json': {
                     'schema': {'type': 'array',
                                'items': {'type': 'object', 'properties': {
                                    'my_prop': {
@@ -91,7 +95,7 @@ class DocsTest(ControllerTestCase):
                                        'description': '200 property desc.'}}},
                                'required': ['my_prop']}}},
             '202': {
-                'application/vnd.ceph.api.v{}+json'.format(DEFAULT_VERSION): {
+                'application/vnd.ceph.api.v0.1+json': {
                     'schema': {'type': 'object',
                                'properties': {'my_prop': {
                                    'type': 'string',
@@ -104,8 +108,14 @@ class DocsTest(ControllerTestCase):
         # Check that a schema of type 'object' is received in the response.
         self.assertEqual(expected_response_content['202'], outcome['responses']['202']['content'])
 
+    def test_gen_method_paths(self):
+        outcome = Docs().gen_paths(False)['/api/doctest/']['get']
+
+        self.assertEqual({'application/vnd.ceph.api.v0.1+json': {'type': 'object'}},
+                         outcome['responses']['200']['content'])
+
     def test_gen_paths_all(self):
-        paths = Docs()._gen_paths(False)
+        paths = Docs().gen_paths(False)
         for key in paths:
             self.assertTrue(any(base in key.split('/')[1] for base in ['api', 'ui-api']))
 

diff --git a/src/pybind/mgr/dashboard/tests/test_versioning.py b/src/pybind/mgr/dashboard/tests/test_versioning.py
index d051e0550a48819748ff1672b0240fc8c5c6b4c6..666b388bf28844731b3f61c021b2f345be6d532f 100644 (file)
--- a/src/pybind/mgr/dashboard/tests/test_versioning.py
+++ b/src/pybind/mgr/dashboard/tests/test_versioning.py
@@ -11,6 +11,7 @@ from . import ControllerTestCase  # pylint: disable=no-name-in-module
 class VTest(RESTController):
     RESOURCE_ID = "vid"
 
+    @RESTController.MethodMap(version="0.1")
     def list(self):
         return {'version': ""}
 
@@ -31,6 +32,15 @@ class RESTVersioningTest(ControllerTestCase, unittest.TestCase):
     def setup_server(cls):
         cls.setup_controllers([VTest], "/test")
 
+    def test_list(self):
+        for (version, expected_status) in [
+                ("0.1", 200),
+                ("2.0", 415)
+        ]:
+            with self.subTest(version=version):
+                self._get('/test/api/vtest', version=version)
+                self.assertStatus(expected_status)
+
     def test_v1(self):
         for (version, expected_status) in [
                 ("1.0", 200),