From b8c9200ff02e72d6e5d7d5270c202f29d42da4cb Mon Sep 17 00:00:00 2001 From: fabrizio8 Date: Wed, 10 Jun 2020 13:06:06 -0400 Subject: [PATCH] mgr/dashboard: Fix doc endpoints UiApi endpoints were broken because the same base URL was used for both UiApi and Api endpoints. There were two issues with this: - The paths dict keys were based off a string split of path that used base URL length. This caused the wrong keys to be created and the duplicate apiapi seen in the cURL requests generated by OpenAPI. - Fixing the above issue was not enough, as then both Api and UiApi endpoints would have the same base URL. This causes conflicts with doc generation that result in only UiApi endpoints to be generated. Passing only / as the base URL argument for api_json and api_all_json and using the full path as the paths keys solves this issue. Other changes: * base_url variable was removed from controllers/docs.py:_gen_paths and tests/test_docs.py because it was unused after my change. * Added type hints for dicts in controllers/docs.py and controllers/pool.py because they were not passing mypy checks. * test_docs.py unit test added to verify change * Add message which explains that UiApi endpoints are not part of the public api Fixes: https://tracker.ceph.com/issues/45957 Signed-off-by: Fabrizio D'Angelo (cherry picked from commit d1a5300a6e778d8fca5befbc2023df0f5c3ead03) --- .../mgr/dashboard/controllers/cephfs.py | 3 ++- .../mgr/dashboard/controllers/crush_rule.py | 4 +++- src/pybind/mgr/dashboard/controllers/docs.py | 15 ++++++++------- .../controllers/erasure_code_profile.py | 4 +++- src/pybind/mgr/dashboard/controllers/pool.py | 19 ++++++++++--------- src/pybind/mgr/dashboard/tests/test_docs.py | 7 ++++++- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/pybind/mgr/dashboard/controllers/cephfs.py b/src/pybind/mgr/dashboard/controllers/cephfs.py index d482ee6de37cf..55113302e2de1 100644 --- a/src/pybind/mgr/dashboard/controllers/cephfs.py +++ b/src/pybind/mgr/dashboard/controllers/cephfs.py @@ -8,7 +8,7 @@ import os import cherrypy import cephfs -from . import ApiController, RESTController, UiApiController +from . import ApiController, ControllerDoc, RESTController, UiApiController from .. import mgr from ..exceptions import DashboardException from ..security import Scope @@ -467,6 +467,7 @@ class CephFSClients(object): @UiApiController('/cephfs', Scope.CEPHFS) +@ControllerDoc("Dashboard UI helper function; not part of the public API", "CephFSUi") class CephFsUi(CephFS): RESOURCE_ID = 'fs_id' diff --git a/src/pybind/mgr/dashboard/controllers/crush_rule.py b/src/pybind/mgr/dashboard/controllers/crush_rule.py index 6a2ffd8f7709c..3aeef258aa3bb 100644 --- a/src/pybind/mgr/dashboard/controllers/crush_rule.py +++ b/src/pybind/mgr/dashboard/controllers/crush_rule.py @@ -3,7 +3,8 @@ from __future__ import absolute_import from cherrypy import NotFound -from . import ApiController, RESTController, Endpoint, ReadPermission, UiApiController +from . import ApiController, ControllerDoc, RESTController, Endpoint, ReadPermission, \ + UiApiController from ..security import Scope from ..services.ceph_service import CephService from .. import mgr @@ -35,6 +36,7 @@ class CrushRule(RESTController): @UiApiController('/crush_rule', Scope.POOL) +@ControllerDoc("Dashboard UI helper function; not part of the public API", "CrushRuleUi") class CrushRuleUi(CrushRule): @Endpoint() @ReadPermission diff --git a/src/pybind/mgr/dashboard/controllers/docs.py b/src/pybind/mgr/dashboard/controllers/docs.py index 1203db74e7f9a..84de247010c1b 100644 --- a/src/pybind/mgr/dashboard/controllers/docs.py +++ b/src/pybind/mgr/dashboard/controllers/docs.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import +from typing import Any, Dict, Union import logging import cherrypy @@ -31,7 +32,7 @@ class Docs(BaseController): if endpoint.is_api or all_endpoints: list_of_ctrl.add(endpoint.ctrl) - tag_map = {} + tag_map: Dict[str, str] = {} for ctrl in list_of_ctrl: tag_name = ctrl.__name__ tag_descr = "" @@ -183,7 +184,7 @@ class Docs(BaseController): @classmethod def _gen_responses(cls, method, resp_object=None): - resp = { + resp: Dict[str, Dict[str, Union[str, Any]]] = { '400': { "description": "Operation exception. Please check the " "response body for details." @@ -252,7 +253,7 @@ class Docs(BaseController): return parameters @classmethod - def _gen_paths(cls, all_endpoints, base_url): + def _gen_paths(cls, all_endpoints): method_order = ['get', 'post', 'put', 'delete'] paths = {} for path, endpoints in sorted(list(ENDPOINT_MAP.items()), @@ -308,7 +309,7 @@ class Docs(BaseController): methods[method.lower()]['security'] = [{'jwt': []}] if not skip: - paths[path[len(base_url):]] = methods + paths[path] = methods return paths @@ -320,7 +321,7 @@ class Docs(BaseController): host = host[host.index(':')+3:] logger.debug("Host: %s", host) - paths = self._gen_paths(all_endpoints, base_url) + paths = self._gen_paths(all_endpoints) if not base_url: base_url = "/" @@ -363,11 +364,11 @@ class Docs(BaseController): @Endpoint(path="api.json") def api_json(self): - return self._gen_spec(False, "/api") + return self._gen_spec(False, "/") @Endpoint(path="api-all.json") def api_all_json(self): - return self._gen_spec(True, "/api") + return self._gen_spec(True, "/") def _swagger_ui_page(self, all_endpoints=False, token=None): base = cherrypy.request.base diff --git a/src/pybind/mgr/dashboard/controllers/erasure_code_profile.py b/src/pybind/mgr/dashboard/controllers/erasure_code_profile.py index c4cc867220d6c..3c8ba61f9f8ab 100644 --- a/src/pybind/mgr/dashboard/controllers/erasure_code_profile.py +++ b/src/pybind/mgr/dashboard/controllers/erasure_code_profile.py @@ -3,7 +3,8 @@ from __future__ import absolute_import from cherrypy import NotFound -from . import ApiController, RESTController, Endpoint, ReadPermission, UiApiController +from . import ApiController, ControllerDoc, RESTController, Endpoint, ReadPermission, \ + UiApiController from ..security import Scope from ..services.ceph_service import CephService from .. import mgr @@ -36,6 +37,7 @@ class ErasureCodeProfile(RESTController): @UiApiController('/erasure_code_profile', Scope.POOL) +@ControllerDoc("Dashboard UI helper function; not part of the public API", "ErasureCodeProfileUi") class ErasureCodeProfileUi(ErasureCodeProfile): @Endpoint() @ReadPermission diff --git a/src/pybind/mgr/dashboard/controllers/pool.py b/src/pybind/mgr/dashboard/controllers/pool.py index 945a82a177a49..ed4cf15080c67 100644 --- a/src/pybind/mgr/dashboard/controllers/pool.py +++ b/src/pybind/mgr/dashboard/controllers/pool.py @@ -1,10 +1,12 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import +from typing import Any, cast, Dict, Iterable, List, Optional, Union import time import cherrypy -from . import ApiController, RESTController, Endpoint, ReadPermission, Task, UiApiController +from . import ApiController, ControllerDoc, RESTController, Endpoint, ReadPermission, Task, \ + UiApiController from .. import mgr from ..security import Scope from ..services.ceph_service import CephService @@ -27,7 +29,7 @@ class Pool(RESTController): crush_rules = {r['rule_id']: r["rule_name"] for r in mgr.get('osd_map_crush')['rules']} - res = {} + res: Dict[Union[int, str], Union[str, List[Any]]] = {} for attr in attrs: if attr not in pool: continue @@ -60,16 +62,14 @@ class Pool(RESTController): return self._pool_list(attrs, stats) @classmethod - def _get(cls, pool_name, attrs=None, stats=False): - # type: (str, str, bool) -> dict + def _get(cls, pool_name: str, attrs: Optional[str] = None, stats: bool = False) -> dict: pools = cls._pool_list(attrs, stats) pool = [p for p in pools if p['pool_name'] == pool_name] if not pool: raise cherrypy.NotFound('No such pool') return pool[0] - def get(self, pool_name, attrs=None, stats=False): - # type: (str, str, bool) -> dict + def get(self, pool_name: str, attrs: Optional[str] = None, stats: bool = False) -> dict: pool = self._get(pool_name, attrs, stats) pool['configuration'] = RbdConfiguration(pool_name).list() return pool @@ -114,7 +114,7 @@ class Pool(RESTController): yes_i_really_mean_it=True) if update_existing: original_app_metadata = set( - current_pool.get('application_metadata')) + cast(Iterable[Any], current_pool.get('application_metadata'))) else: original_app_metadata = set() @@ -207,6 +207,7 @@ class Pool(RESTController): @UiApiController('/pool', Scope.POOL) +@ControllerDoc("Dashboard UI helper function; not part of the public API", "PoolUi") class PoolUi(Pool): @Endpoint() @ReadPermission @@ -230,8 +231,8 @@ class PoolUi(Pool): if o['name'] == conf_name][0] profiles = CephService.get_erasure_code_profiles() - used_rules = {} - used_profiles = {} + used_rules: Dict[str, List[str]] = {} + used_profiles: Dict[str, List[str]] = {} pool_names = [] for p in self._pool_list(): name = p['pool_name'] diff --git a/src/pybind/mgr/dashboard/tests/test_docs.py b/src/pybind/mgr/dashboard/tests/test_docs.py index 4d6c25761cfe6..a6e03b526711e 100644 --- a/src/pybind/mgr/dashboard/tests/test_docs.py +++ b/src/pybind/mgr/dashboard/tests/test_docs.py @@ -60,12 +60,17 @@ class DocsTest(ControllerTestCase): self.assertEqual(Docs()._type_to_str(str), "string") def test_gen_paths(self): - outcome = Docs()._gen_paths(False, "")['/api/doctest//decorated_func/{parameter}']['get'] + outcome = Docs()._gen_paths(False)['/api/doctest//decorated_func/{parameter}']['get'] self.assertIn('tags', outcome) self.assertIn('summary', outcome) self.assertIn('parameters', outcome) self.assertIn('responses', outcome) + def test_gen_paths_all(self): + paths = Docs()._gen_paths(False) + for key in paths: + self.assertTrue(any(base in key.split('/')[1] for base in ['api', 'ui-api'])) + def test_gen_tags(self): outcome = Docs()._gen_tags(False)[0] self.assertEqual({'description': 'Group description', 'name': 'FooGroup'}, outcome) -- 2.39.5