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 28f5998e876c..5affb0d7e075 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 824b42b50dbc..90752a03f4d4 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 cb4f1cea7721..f32ddbc699de 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 51bda8e8d62d..7d897e397932 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 b44a3f15b032..83de90f06192 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 d3c54ce6c2b1..33027eff625b 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 7edbfa8f6ada..ac8ebcfd2890 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 e990b5abce92..3ef41fd33127 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 19ad1b85062d..f5f300171c23 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 7517e6d65829..844103883fde 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 a0698542b43a..4677e20dfbf1 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 be5967394d1e..ee94626d79af 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 2c96bab16fd1..e36628224ac2 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 4a525a54da7a..e8b96cd6ba20 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 bb21e0abed86..65df6e63b748 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 dddcac4e2e29..49adbb55c80e 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)