From 3bdbbb95a05b26e84e09a550d0bde3facf3a76d4 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alfonso=20Mart=C3=ADnez?= Date: Mon, 3 May 2021 18:46:08 +0200 Subject: [PATCH] mgr/dashboard: fix set-ssl-certificate{,-key} commands MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit - Now create-self-signed-cert command relies on set-ssl-certificate{,-key} commands. - Simplify the command testing and increase the test coverage. Fixes: https://tracker.ceph.com/issues/50519 Signed-off-by: Alfonso Martínez (cherry picked from commit fb20990bca55ffe1712a905c3a9741e8df004e74) --- src/pybind/mgr/dashboard/module.py | 68 ++++++++++++---------- src/pybind/mgr/dashboard/tests/__init__.py | 51 ++++++++-------- src/pybind/mgr/dashboard/tests/test_ssl.py | 28 +++++++++ src/pybind/mgr/dashboard/tests/test_sso.py | 12 +--- 4 files changed, 96 insertions(+), 63 deletions(-) create mode 100644 src/pybind/mgr/dashboard/tests/test_ssl.py diff --git a/src/pybind/mgr/dashboard/module.py b/src/pybind/mgr/dashboard/module.py index 69b158f696a20..c8e5ba4020a12 100644 --- a/src/pybind/mgr/dashboard/module.py +++ b/src/pybind/mgr/dashboard/module.py @@ -14,9 +14,16 @@ import sys import tempfile import threading import time -from typing import Optional +from typing import TYPE_CHECKING, Optional -from mgr_module import CLIWriteCommand, MgrModule, MgrStandbyModule, Option, _get_localized_key +if TYPE_CHECKING: + if sys.version_info >= (3, 8): + from typing import Literal + else: + from typing_extensions import Literal + +from mgr_module import CLIWriteCommand, HandleCommandResult, MgrModule, \ + MgrStandbyModule, Option, _get_localized_key from mgr_util import ServerConfigException, create_self_signed_cert, \ get_default_addr, verify_tls_files @@ -214,6 +221,10 @@ class CherryPyConfig(object): return uri +if TYPE_CHECKING: + SslConfigKey = Literal['crt', 'key'] + + class Module(MgrModule, CherryPyConfig): """ dashboard module entrypoint @@ -357,31 +368,36 @@ class Module(MgrModule, CherryPyConfig): logger.info('Stopping engine...') self.shutdown_event.set() - @CLIWriteCommand("dashboard set-ssl-certificate") - def set_ssl_certificate(self, - mgr_id: Optional[str] = None, - inbuf: Optional[bytes] = None): + def _set_ssl_item(self, item_label: str, item_key: 'SslConfigKey' = 'crt', + mgr_id: Optional[str] = None, inbuf: Optional[str] = None): if inbuf is None: - return -errno.EINVAL, '',\ - 'Please specify the certificate file with "-i" option' + return -errno.EINVAL, '', f'Please specify the {item_label} with "-i" option' + if mgr_id is not None: - self.set_store(_get_localized_key(mgr_id, 'crt'), inbuf.decode()) + self.set_store(_get_localized_key(mgr_id, item_key), inbuf) else: - self.set_store('crt', inbuf.decode()) - return 0, 'SSL certificate updated', '' + self.set_store(item_key, inbuf) + return 0, f'SSL {item_label} updated', '' + + @CLIWriteCommand("dashboard set-ssl-certificate") + def set_ssl_certificate(self, mgr_id: Optional[str] = None, inbuf: Optional[str] = None): + return self._set_ssl_item('certificate', 'crt', mgr_id, inbuf) @CLIWriteCommand("dashboard set-ssl-certificate-key") - def set_ssl_certificate_key(self, - mgr_id: Optional[str] = None, - inbuf: Optional[bytes] = None): - if inbuf is None: - return -errno.EINVAL, '',\ - 'Please specify the certificate key file with "-i" option' - if mgr_id is not None: - self.set_store(_get_localized_key(mgr_id, 'key'), inbuf.decode()) - else: - self.set_store('key', inbuf.decode()) - return 0, 'SSL certificate key updated', '' + def set_ssl_certificate_key(self, mgr_id: Optional[str] = None, inbuf: Optional[str] = None): + return self._set_ssl_item('certificate key', 'key', mgr_id, inbuf) + + @CLIWriteCommand("dashboard create-self-signed-cert") + def set_mgr_created_self_signed_cert(self): + cert, pkey = create_self_signed_cert('IT', 'ceph-dashboard') + result = HandleCommandResult(*self.set_ssl_certificate(inbuf=cert)) + if result.retval != 0: + return result + + result = HandleCommandResult(*self.set_ssl_certificate_key(inbuf=pkey)) + if result.retval != 0: + return result + return 0, 'Self-signed certificate created', '' def handle_command(self, inbuf, cmd): # pylint: disable=too-many-return-statements @@ -397,9 +413,6 @@ class Module(MgrModule, CherryPyConfig): if cmd['prefix'] == 'dashboard get-jwt-token-ttl': ttl = self.get_module_option('jwt_token_ttl', JwtManager.JWT_TOKEN_TTL) return 0, str(ttl), '' - if cmd['prefix'] == 'dashboard create-self-signed-cert': - self.create_self_signed_cert() - return 0, 'Self-signed certificate created', '' if cmd['prefix'] == 'dashboard grafana dashboards update': push_local_dashboards() return 0, 'Grafana dashboards updated', '' @@ -407,11 +420,6 @@ class Module(MgrModule, CherryPyConfig): return (-errno.EINVAL, '', 'Command not found \'{0}\'' .format(cmd['prefix'])) - def create_self_signed_cert(self): - cert, pkey = create_self_signed_cert('IT', 'ceph-dashboard') - self.set_store('crt', cert) - self.set_store('key', pkey) - def notify(self, notify_type, notify_id): NotificationQueue.new_notification(notify_type, notify_id) diff --git a/src/pybind/mgr/dashboard/tests/__init__.py b/src/pybind/mgr/dashboard/tests/__init__.py index 72d5204b6588d..46c6b8043cc8b 100644 --- a/src/pybind/mgr/dashboard/tests/__init__.py +++ b/src/pybind/mgr/dashboard/tests/__init__.py @@ -11,11 +11,12 @@ from unittest.mock import Mock import cherrypy from cherrypy._cptools import HandlerWrapperTool from cherrypy.test import helper -from mgr_module import CLICommand +from mgr_module import HandleCommandResult from pyfakefs import fake_filesystem from .. import DEFAULT_VERSION, mgr from ..controllers import generate_controller_routes, json_error_page +from ..module import Module from ..plugins import PLUGIN_MANAGER, debug, feature_toggles # noqa from ..services.auth import AuthManagerTool from ..services.exception import dashboard_exception_handler @@ -28,34 +29,22 @@ PLUGIN_MANAGER.hook.register_commands() logger = logging.getLogger('tests') +class ModuleTestClass(Module): + """Dashboard module subclass for testing the module methods.""" + + def __init__(self) -> None: + pass + + def _unconfigure_logging(self) -> None: + pass + + class CmdException(Exception): def __init__(self, retcode, message): super(CmdException, self).__init__(message) self.retcode = retcode -def exec_dashboard_cmd(command_handler, cmd, **kwargs): - inbuf = kwargs['inbuf'] if 'inbuf' in kwargs else None - cmd_dict = {'prefix': 'dashboard {}'.format(cmd)} - cmd_dict.update(kwargs) - if cmd_dict['prefix'] not in CLICommand.COMMANDS: - ret, out, err = command_handler(cmd_dict) - if ret < 0: - raise CmdException(ret, err) - try: - return json.loads(out) - except ValueError: - return out - - ret, out, err = CLICommand.COMMANDS[cmd_dict['prefix']].call(mgr, cmd_dict, inbuf) - if ret < 0: - raise CmdException(ret, err) - try: - return json.loads(out) - except ValueError: - return out - - class KVStoreMockMixin(object): CONFIG_KEY_DICT = {} @@ -81,10 +70,24 @@ class KVStoreMockMixin(object): return cls.CONFIG_KEY_DICT.get(key, None) +# pylint: disable=protected-access class CLICommandTestMixin(KVStoreMockMixin): + _dashboard_module = ModuleTestClass() + @classmethod def exec_cmd(cls, cmd, **kwargs): - return exec_dashboard_cmd(None, cmd, **kwargs) + inbuf = kwargs['inbuf'] if 'inbuf' in kwargs else None + cmd_dict = {'prefix': 'dashboard {}'.format(cmd)} + cmd_dict.update(kwargs) + + result = HandleCommandResult(*cls._dashboard_module._handle_command(inbuf, cmd_dict)) + + if result.retval < 0: + raise CmdException(result.retval, result.stderr) + try: + return json.loads(result.stdout) + except ValueError: + return result.stdout class FakeFsMixin(object): diff --git a/src/pybind/mgr/dashboard/tests/test_ssl.py b/src/pybind/mgr/dashboard/tests/test_ssl.py new file mode 100644 index 0000000000000..840f2b8c9b07c --- /dev/null +++ b/src/pybind/mgr/dashboard/tests/test_ssl.py @@ -0,0 +1,28 @@ +import errno +import unittest + +from ..tests import CLICommandTestMixin, CmdException + + +class SslTest(unittest.TestCase, CLICommandTestMixin): + + def test_ssl_certificate_and_key(self): + with self.assertRaises(CmdException) as ctx: + self.exec_cmd('set-ssl-certificate', inbuf='', mgr_id='x') + self.assertEqual(ctx.exception.retcode, -errno.EINVAL) + self.assertEqual(str(ctx.exception), 'Please specify the certificate with "-i" option') + + result = self.exec_cmd('set-ssl-certificate', inbuf='content', mgr_id='x') + self.assertEqual(result, 'SSL certificate updated') + + with self.assertRaises(CmdException) as ctx: + self.exec_cmd('set-ssl-certificate-key', inbuf='', mgr_id='x') + self.assertEqual(ctx.exception.retcode, -errno.EINVAL) + self.assertEqual(str(ctx.exception), 'Please specify the certificate key with "-i" option') + + result = self.exec_cmd('set-ssl-certificate-key', inbuf='content', mgr_id='x') + self.assertEqual(result, 'SSL certificate key updated') + + def test_set_mgr_created_self_signed_cert(self): + result = self.exec_cmd('create-self-signed-cert') + self.assertEqual(result, 'Self-signed certificate created') diff --git a/src/pybind/mgr/dashboard/tests/test_sso.py b/src/pybind/mgr/dashboard/tests/test_sso.py index ee97fb20fddf4..ab565137a18f9 100644 --- a/src/pybind/mgr/dashboard/tests/test_sso.py +++ b/src/pybind/mgr/dashboard/tests/test_sso.py @@ -5,13 +5,11 @@ from __future__ import absolute_import import errno import unittest -from ..services.sso import handle_sso_command, load_sso_db -from . import CmdException # pylint: disable=no-name-in-module -from . import KVStoreMockMixin # pylint: disable=no-name-in-module -from . import exec_dashboard_cmd # pylint: disable=no-name-in-module +from ..services.sso import load_sso_db +from . import CLICommandTestMixin, CmdException # pylint: disable=no-name-in-module -class AccessControlTest(unittest.TestCase, KVStoreMockMixin): +class AccessControlTest(unittest.TestCase, CLICommandTestMixin): IDP_METADATA = '''