From: Aashish Sharma Date: Wed, 19 May 2021 05:57:13 +0000 (+0530) Subject: mgr/dashboard: API Version changes doesnt appy to pre-defined methods(list, create... X-Git-Tag: v17.1.0~1790^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=bb52092cd37855a3dfcfae9287e231865e9af3fa;p=ceph.git mgr/dashboard: API Version changes doesnt appy to pre-defined methods(list, create etc.) 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 --- diff --git a/doc/dev/developer_guide/dash-devel.rst b/doc/dev/developer_guide/dash-devel.rst index 4cd81a1dcb53..5702806b7d72 100644 --- 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 ee8d3bd7bb49..babfbb4296d3 100755 --- a/src/pybind/mgr/dashboard/controllers/__init__.py +++ b/src/pybind/mgr/dashboard/controllers/__init__.py @@ -812,14 +812,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 @@ -858,10 +858,10 @@ class RESTController(BaseController): cls._update_endpoint_params_method_map( func, res_id_params, endpoint_params) - elif hasattr(func, "_collection_method_"): + elif hasattr(func, "collection_method"): cls._update_endpoint_params_collection_map(func, endpoint_params) - elif hasattr(func, "_resource_method_"): + elif hasattr(func, "resource_method"): cls._update_endpoint_params_resource_method( res_id_params, endpoint_params, func) @@ -900,27 +900,27 @@ class RESTController(BaseController): else: path_params = ["{{{}}}".format(p) for p in res_id_params] endpoint_params['path'] += "/{}".format("/".join(path_params)) - if func._resource_method_['path']: - endpoint_params['path'] += func._resource_method_['path'] + if func.resource_method['path']: + endpoint_params['path'] += func.resource_method['path'] else: endpoint_params['path'] += "/{}".format(func.__name__) - endpoint_params['status'] = func._resource_method_['status'] - endpoint_params['method'] = func._resource_method_['method'] - endpoint_params['version'] = func._resource_method_['version'] - endpoint_params['query_params'] = func._resource_method_['query_params'] + endpoint_params['status'] = func.resource_method['status'] + endpoint_params['method'] = func.resource_method['method'] + endpoint_params['version'] = func.resource_method['version'] + endpoint_params['query_params'] = func.resource_method['query_params'] if not endpoint_params['sec_permissions']: endpoint_params['permission'] = cls._permission_map[endpoint_params['method']] @classmethod def _update_endpoint_params_collection_map(cls, func, endpoint_params): - if func._collection_method_['path']: - endpoint_params['path'] = func._collection_method_['path'] + if func.collection_method['path']: + endpoint_params['path'] = func.collection_method['path'] else: endpoint_params['path'] = "/{}".format(func.__name__) - endpoint_params['status'] = func._collection_method_['status'] - endpoint_params['method'] = func._collection_method_['method'] - endpoint_params['query_params'] = func._collection_method_['query_params'] - endpoint_params['version'] = func._collection_method_['version'] + endpoint_params['status'] = func.collection_method['status'] + endpoint_params['method'] = func.collection_method['method'] + endpoint_params['query_params'] = func.collection_method['query_params'] + endpoint_params['version'] = func.collection_method['version'] if not endpoint_params['sec_permissions']: endpoint_params['permission'] = cls._permission_map[endpoint_params['method']] @@ -937,6 +937,8 @@ class RESTController(BaseController): endpoint_params['status'] = meth['status'] endpoint_params['method'] = meth['method'] + if hasattr(func, "method_map_method"): + endpoint_params['version'] = func.method_map_method['version'] if not endpoint_params['sec_permissions']: endpoint_params['permission'] = cls._permission_map[endpoint_params['method']] @@ -959,7 +961,7 @@ class RESTController(BaseController): status = 200 def _wrapper(func): - func._resource_method_ = { + func.resource_method = { 'method': method, 'path': path, 'status': status, @@ -969,6 +971,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): @@ -979,7 +996,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 295a36ad8559..368dbeb42404 100644 --- 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 84cfddd629ec..63d5f1731a15 100644 --- a/src/pybind/mgr/dashboard/tests/test_docs.py +++ b/src/pybind/mgr/dashboard/tests/test_docs.py @@ -3,7 +3,6 @@ from __future__ import absolute_import import unittest -from .. import DEFAULT_VERSION from ..api.doc import SchemaType from ..controllers import ApiController, ControllerDoc, Endpoint, EndpointDoc, RESTController from ..controllers.docs import Docs @@ -32,10 +31,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): @@ -77,7 +80,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) @@ -85,7 +88,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': { @@ -93,7 +96,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', @@ -106,8 +109,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 d051e0550a48..666b388bf288 100644 --- 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),