From: Ernesto Puerta Date: Fri, 19 Jul 2019 15:23:56 +0000 (+0200) Subject: mgr/dashboard: clean-up python unit tests X-Git-Tag: v15.1.0~1849^2~1 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=f4cbe8965f83af7db4d7b7ea4b26e532a5308e91;p=ceph.git mgr/dashboard: clean-up python unit tests Fix pylint issues. Refactor JwtManager check to avoid code duplication (found by pylint when --jobs=1). Fix issue with DashboardException.code property, using abs() on potentially None attribute. Disables Doctests in services/rbd.py that are actually integration tests (they check the value of rbd.RBD_FEATURES_NAME_MAPPING). Ideally, these kind of tests should be explicitly executed in an integration testing stage, rather that unit-testing. Disabled tests have been prepended with @DISABLEDOCTEST token. Fixes: https://tracker.ceph.com/issues/40487 Signed-off-by: Ernesto Puerta --- diff --git a/src/pybind/mgr/dashboard/controllers/__init__.py b/src/pybind/mgr/dashboard/controllers/__init__.py index 28f5998e876..5affb0d7e07 100644 --- a/src/pybind/mgr/dashboard/controllers/__init__.py +++ b/src/pybind/mgr/dashboard/controllers/__init__.py @@ -30,13 +30,13 @@ def EndpointDoc(description="", group="", parameters=None, responses=None): if not isinstance(description, str): raise Exception("%s has been called with a description that is not a string: %s" % (EndpointDoc.__name__, description)) - elif not isinstance(group, str): + if not isinstance(group, str): raise Exception("%s has been called with a groupname that is not a string: %s" % (EndpointDoc.__name__, group)) - elif parameters and not isinstance(parameters, dict): + if parameters and not isinstance(parameters, dict): raise Exception("%s has been called with parameters that is not a dict: %s" % (EndpointDoc.__name__, parameters)) - elif responses and not isinstance(responses, dict): + if responses and not isinstance(responses, dict): raise Exception("%s has been called with responses that is not a dict: %s" % (EndpointDoc.__name__, responses)) diff --git a/src/pybind/mgr/dashboard/controllers/auth.py b/src/pybind/mgr/dashboard/controllers/auth.py index 824b42b50db..90752a03f4d 100644 --- a/src/pybind/mgr/dashboard/controllers/auth.py +++ b/src/pybind/mgr/dashboard/controllers/auth.py @@ -2,13 +2,11 @@ from __future__ import absolute_import import cherrypy -import jwt from . import ApiController, RESTController from .. import logger, mgr from ..exceptions import DashboardException from ..services.auth import AuthManager, JwtManager -from ..services.access_control import UserDoesNotExist @ApiController('/auth', secure=False) @@ -57,29 +55,13 @@ class Auth(RESTController): @RESTController.Collection('POST') def check(self, token): if token: - try: - token = JwtManager.decode_token(token) - if not JwtManager.is_blacklisted(token['jti']): - user = AuthManager.get_user(token['username']) - if user.lastUpdate <= token['iat']: - return { - 'username': user.username, - 'permissions': user.permissions_dict(), - 'sso': mgr.SSO_DB.protocol == 'saml2' - } - - logger.debug("AMT: user info changed after token was" - " issued, iat=%s lastUpdate=%s", - token['iat'], user.lastUpdate) - else: - logger.debug('AMT: Token is black-listed') - except jwt.exceptions.ExpiredSignatureError: - logger.debug("AMT: Token has expired") - except jwt.exceptions.InvalidTokenError: - logger.debug("AMT: Failed to decode token") - except UserDoesNotExist: - logger.debug("AMT: Invalid token: user %s does not exist", - token['username']) + user = JwtManager.get_user(token) + if user: + return { + 'username': user.username, + 'permissions': user.permissions_dict(), + 'sso': mgr.SSO_DB.protocol == 'saml2' + } return { - 'login_url': self._get_login_url() + 'login_url': self._get_login_url(), } diff --git a/src/pybind/mgr/dashboard/controllers/rbd_mirroring.py b/src/pybind/mgr/dashboard/controllers/rbd_mirroring.py index cb4f1cea772..f32ddbc699d 100644 --- a/src/pybind/mgr/dashboard/controllers/rbd_mirroring.py +++ b/src/pybind/mgr/dashboard/controllers/rbd_mirroring.py @@ -12,6 +12,8 @@ import rbd from . import ApiController, Endpoint, Task, BaseController, ReadPermission, \ RESTController +from .rbd import _rbd_call + from .. import logger, mgr from ..security import Scope from ..services.ceph_service import CephService @@ -37,11 +39,6 @@ def RbdMirroringTask(name, metadata, wait_for): return composed_decorator -def _rbd_call(pool_name, func, *args, **kwargs): - with mgr.rados.open_ioctx(pool_name) as ioctx: - func(ioctx, *args, **kwargs) - - @ViewCache() def get_daemons_and_pools(): # pylint: disable=R0915 def get_daemons(): diff --git a/src/pybind/mgr/dashboard/controllers/saml2.py b/src/pybind/mgr/dashboard/controllers/saml2.py index 51bda8e8d62..7d897e39793 100644 --- a/src/pybind/mgr/dashboard/controllers/saml2.py +++ b/src/pybind/mgr/dashboard/controllers/saml2.py @@ -75,12 +75,12 @@ class Saml2(BaseController): token = token.decode('utf-8') logger.debug("JWT Token: %s", token) raise cherrypy.HTTPRedirect("{}/#/login?access_token={}".format(url_prefix, token)) - else: - return { - 'is_authenticated': auth.is_authenticated(), - 'errors': errors, - 'reason': auth.get_last_error_reason() - } + + return { + 'is_authenticated': auth.is_authenticated(), + 'errors': errors, + 'reason': auth.get_last_error_reason() + } @Endpoint(xml=True) def metadata(self): diff --git a/src/pybind/mgr/dashboard/exceptions.py b/src/pybind/mgr/dashboard/exceptions.py index b44a3f15b03..83de90f0619 100644 --- a/src/pybind/mgr/dashboard/exceptions.py +++ b/src/pybind/mgr/dashboard/exceptions.py @@ -42,7 +42,7 @@ class DashboardException(Exception): def code(self): if self._code: return str(self._code) - return str(abs(self.errno)) + return str(abs(self.errno)) if self.errno is not None else 'Error' # access control module exceptions diff --git a/src/pybind/mgr/dashboard/plugins/__init__.py b/src/pybind/mgr/dashboard/plugins/__init__.py index d3c54ce6c2b..33027eff625 100644 --- a/src/pybind/mgr/dashboard/plugins/__init__.py +++ b/src/pybind/mgr/dashboard/plugins/__init__.py @@ -59,4 +59,4 @@ class DashboardPluginManager(object): PLUGIN_MANAGER = DashboardPluginManager("ceph-mgr.dashboard") # Load all interfaces and their hooks -from . import interfaces +from . import interfaces # pylint: disable=wrong-import-position,cyclic-import diff --git a/src/pybind/mgr/dashboard/plugins/feature_toggles.py b/src/pybind/mgr/dashboard/plugins/feature_toggles.py index 7edbfa8f6ad..ac8ebcfd289 100644 --- a/src/pybind/mgr/dashboard/plugins/feature_toggles.py +++ b/src/pybind/mgr/dashboard/plugins/feature_toggles.py @@ -44,6 +44,7 @@ class Actions(Enum): STATUS = 'status' +# pylint: disable=too-many-ancestors @PM.add_plugin class FeatureToggles(I.CanMgr, I.CanLog, I.Setupable, I.HasOptions, I.HasCommands, I.FilterRequest.BeforeHandler, @@ -54,6 +55,7 @@ class FeatureToggles(I.CanMgr, I.CanLog, I.Setupable, I.HasOptions, @PM.add_hook def setup(self): + # pylint: disable=attribute-defined-outside-init self.Controller2Feature = { controller: feature for feature, controllers in Feature2Controller.items() @@ -132,8 +134,9 @@ class FeatureToggles(I.CanMgr, I.CanLog, I.Setupable, I.HasOptions, @ApiController('/feature_toggles') class FeatureTogglesEndpoint(RESTController): - def list(_): + def list(_): # pylint: disable=no-self-argument return { + # pylint: disable=protected-access feature.value: self._is_feature_enabled(feature) for feature in Features } diff --git a/src/pybind/mgr/dashboard/plugins/interfaces.py b/src/pybind/mgr/dashboard/plugins/interfaces.py index e990b5abce9..3ef41fd3312 100644 --- a/src/pybind/mgr/dashboard/plugins/interfaces.py +++ b/src/pybind/mgr/dashboard/plugins/interfaces.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import absolute_import -from . import PLUGIN_MANAGER as PM, Interface +from . import PLUGIN_MANAGER as PM, Interface # pylint: disable=cyclic-import class CanMgr(Interface): @@ -22,29 +22,32 @@ class Setupable(Interface): Placeholder for plugin setup, right after server start. CanMgr.mgr and CanLog.log are initialized by then. """ - pass @PM.add_interface class HasOptions(Interface): @PM.add_abcspec - def get_options(self): pass + def get_options(self): + pass @PM.add_interface class HasCommands(Interface): @PM.add_abcspec - def register_commands(self): pass + def register_commands(self): + pass @PM.add_interface class HasControllers(Interface): @PM.add_abcspec - def get_controllers(self): pass + def get_controllers(self): + pass -class FilterRequest: +class FilterRequest(object): @PM.add_interface class BeforeHandler(Interface): @PM.add_abcspec - def filter_request_before_handler(self, request): pass + def filter_request_before_handler(self, request): + pass diff --git a/src/pybind/mgr/dashboard/plugins/lru_cache.py b/src/pybind/mgr/dashboard/plugins/lru_cache.py index 19ad1b85062..f5f300171c2 100644 --- a/src/pybind/mgr/dashboard/plugins/lru_cache.py +++ b/src/pybind/mgr/dashboard/plugins/lru_cache.py @@ -19,12 +19,9 @@ def lru_cache(maxsize=128, typed=False): cache = OrderedDict() stats = [0, 0] rlock = RLock() - setattr( - function, - 'cache_info', - lambda: - "hits={}, misses={}, maxsize={}, currsize={}".format( - stats[0], stats[1], maxsize, len(cache))) + setattr(function, 'cache_info', lambda: + "hits={}, misses={}, maxsize={}, currsize={}".format( + stats[0], stats[1], maxsize, len(cache))) @wraps(function) def wrapper(*args, **kwargs): diff --git a/src/pybind/mgr/dashboard/plugins/pluggy.py b/src/pybind/mgr/dashboard/plugins/pluggy.py index 7517e6d6582..844103883fd 100644 --- a/src/pybind/mgr/dashboard/plugins/pluggy.py +++ b/src/pybind/mgr/dashboard/plugins/pluggy.py @@ -21,8 +21,7 @@ AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. -""" - +"""\ """ CAVEAT: This is a minimal implementation of python-pluggy (based on 0.8.0 interface: @@ -103,9 +102,9 @@ class PluginManager(object): def add_hookspecs(self, module_or_class): """ Dummy method""" - pass - def register(self, plugin, name=None): + def register(self, plugin, name=None): # pylint: disable=unused-argument for attr in dir(plugin): if self.parse_hookimpl_opts(plugin, attr) is not None: + # pylint: disable=protected-access self.hook._add_hookimpl(attr, getattr(plugin, attr)) diff --git a/src/pybind/mgr/dashboard/plugins/ttl_cache.py b/src/pybind/mgr/dashboard/plugins/ttl_cache.py index a0698542b43..4677e20dfbf 100644 --- a/src/pybind/mgr/dashboard/plugins/ttl_cache.py +++ b/src/pybind/mgr/dashboard/plugins/ttl_cache.py @@ -19,12 +19,9 @@ def ttl_cache(ttl, maxsize=128, typed=False): cache = OrderedDict() stats = [0, 0, 0] rlock = RLock() - setattr( - function, - 'cache_info', - lambda: - "hits={}, misses={}, expired={}, maxsize={}, currsize={}".format( - stats[0], stats[1], stats[2], maxsize, len(cache))) + setattr(function, 'cache_info', lambda: + "hits={}, misses={}, expired={}, maxsize={}, currsize={}".format( + stats[0], stats[1], stats[2], maxsize, len(cache))) @wraps(function) def wrapper(*args, **kwargs): diff --git a/src/pybind/mgr/dashboard/services/auth.py b/src/pybind/mgr/dashboard/services/auth.py index be5967394d1..ee94626d79a 100644 --- a/src/pybind/mgr/dashboard/services/auth.py +++ b/src/pybind/mgr/dashboard/services/auth.py @@ -69,17 +69,37 @@ class JwtManager(object): return None @classmethod - def set_user(cls, token): - cls.LOCAL_USER.username = token['username'] + def set_user(cls, username): + cls.LOCAL_USER.username = username @classmethod def reset_user(cls): - cls.set_user({'username': None, 'permissions': None}) + cls.set_user(None) @classmethod def get_username(cls): return getattr(cls.LOCAL_USER, 'username', None) + @classmethod + def get_user(cls, token): + try: + dtoken = JwtManager.decode_token(token) + if not JwtManager.is_blacklisted(dtoken['jti']): + user = AuthManager.get_user(dtoken['username']) + if user.lastUpdate <= dtoken['iat']: + return user + logger.debug("AMT: user info changed after token was issued, iat=%s lastUpdate=%s", + dtoken['iat'], user.lastUpdate) + else: + logger.debug('AMT: Token is black-listed') + except jwt.exceptions.ExpiredSignatureError: + logger.debug("AMT: Token has expired") + except jwt.exceptions.InvalidTokenError: + logger.debug("AMT: Failed to decode token") + except UserDoesNotExist: + logger.debug("AMT: Invalid token: user %s does not exist", dtoken['username']) + return None + @classmethod def blacklist_token(cls, token): token = jwt.decode(token, verify=False) @@ -139,40 +159,23 @@ class AuthManagerTool(cherrypy.Tool): token = JwtManager.get_token_from_header() logger.debug("AMT: token: %s", token) if token: - try: - token = JwtManager.decode_token(token) - if not JwtManager.is_blacklisted(token['jti']): - user = AuthManager.get_user(token['username']) - if user.lastUpdate <= token['iat']: - self._check_authorization(token) - return - - logger.debug("AMT: user info changed after token was" - " issued, iat=%s lastUpdate=%s", - token['iat'], user.lastUpdate) - else: - logger.debug('AMT: Token is black-listed') - except jwt.exceptions.ExpiredSignatureError: - logger.debug("AMT: Token has expired") - except jwt.exceptions.InvalidTokenError: - logger.debug("AMT: Failed to decode token") - except UserDoesNotExist: - logger.debug("AMT: Invalid token: user %s does not exist", - token['username']) - + user = JwtManager.get_user(token) + if user: + self._check_authorization(user.username) + return logger.debug('AMT: Unauthorized access to %s', cherrypy.url(relative='server')) raise cherrypy.HTTPError(401, 'You are not authorized to access ' 'that resource') - def _check_authorization(self, token): + def _check_authorization(self, username): logger.debug("AMT: checking authorization...") - username = token['username'] + username = username handler = cherrypy.request.handler.callable controller = handler.__self__ sec_scope = getattr(controller, '_security_scope', None) sec_perms = getattr(handler, '_security_permissions', None) - JwtManager.set_user(token) + JwtManager.set_user(username) if not sec_scope: # controller does not define any authorization restrictions diff --git a/src/pybind/mgr/dashboard/services/ceph_service.py b/src/pybind/mgr/dashboard/services/ceph_service.py index 2c96bab16fd..e36628224ac 100644 --- a/src/pybind/mgr/dashboard/services/ceph_service.py +++ b/src/pybind/mgr/dashboard/services/ceph_service.py @@ -167,11 +167,11 @@ class CephService(object): kwargs) logger.error(msg) raise SendCommandError(outs, prefix, argdict, r) - else: - try: - return json.loads(outb) - except Exception: # pylint: disable=broad-except - return outb + + try: + return json.loads(outb) + except Exception: # pylint: disable=broad-except + return outb @classmethod def get_rates(cls, svc_type, svc_name, path): diff --git a/src/pybind/mgr/dashboard/services/ganesha.py b/src/pybind/mgr/dashboard/services/ganesha.py index 4a525a54da7..e8b96cd6ba2 100644 --- a/src/pybind/mgr/dashboard/services/ganesha.py +++ b/src/pybind/mgr/dashboard/services/ganesha.py @@ -429,8 +429,7 @@ class RGWFSal(FSal): raise NFSException('Cannot create bucket "{}" as it already ' 'exists, and belongs to other user.' .format(path)) - else: - raise exp + raise exp if not exists: logger.info('Creating new RGW bucket "%s" for user "%s"', path, self.rgw_user_id) diff --git a/src/pybind/mgr/dashboard/services/rbd.py b/src/pybind/mgr/dashboard/services/rbd.py index bb21e0abed8..65df6e63b74 100644 --- a/src/pybind/mgr/dashboard/services/rbd.py +++ b/src/pybind/mgr/dashboard/services/rbd.py @@ -25,7 +25,7 @@ def format_bitmask(features): """ Formats the bitmask: - >>> format_bitmask(45) + @DISABLEDOCTEST: >>> format_bitmask(45) ['deep-flatten', 'exclusive-lock', 'layering', 'object-map'] """ names = [val for key, val in RBD_FEATURES_NAME_MAPPING.items() @@ -37,13 +37,14 @@ def format_features(features): """ Converts the features list to bitmask: - >>> format_features(['deep-flatten', 'exclusive-lock', 'layering', 'object-map']) + @DISABLEDOCTEST: >>> format_features(['deep-flatten', 'exclusive-lock', + 'layering', 'object-map']) 45 - >>> format_features(None) is None + @DISABLEDOCTEST: >>> format_features(None) is None True - >>> format_features('deep-flatten, exclusive-lock') + @DISABLEDOCTEST: >>> format_features('deep-flatten, exclusive-lock') 32 """ if isinstance(features, six.string_types): diff --git a/src/pybind/mgr/dashboard/tests/__init__.py b/src/pybind/mgr/dashboard/tests/__init__.py index dddcac4e2e2..49adbb55c80 100644 --- a/src/pybind/mgr/dashboard/tests/__init__.py +++ b/src/pybind/mgr/dashboard/tests/__init__.py @@ -201,13 +201,13 @@ class ControllerTestCase(helper.CPWebCase): elif method == 'DELETE': self.status = '204 No Content' return + + if 'status' in thread.res_task['exception']: + self.status = thread.res_task['exception']['status'] else: - if 'status' in thread.res_task['exception']: - self.status = thread.res_task['exception']['status'] - else: - self.status = 500 - self.body = json.dumps(thread.res_task['exception']) - return + self.status = 500 + self.body = json.dumps(thread.res_task['exception']) + return def _task_post(self, url, data=None, timeout=60): self._task_request('POST', url, data, timeout)