From 0fcf0a7827cf4e8748a382613f9c8d1715c4a1e8 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Alfonso=20Mart=C3=ADnez?= Date: Wed, 28 Jul 2021 09:48:18 +0200 Subject: [PATCH] mgr/dashboard: connect-rgw: adaptation and test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit - Align Dashboard with cephadm: configure credentials using the same logic. - Fix: create a 'dashboard' user per realm (before: only on 1st realm). - Lint fixes, test coverage, method renaming to better reflect behavior and method visibility. Fixes: https://tracker.ceph.com/issues/44605 Signed-off-by: Alfonso Martínez --- src/pybind/mgr/dashboard/module.py | 11 +- .../mgr/dashboard/services/rgw_client.py | 120 ++++++++++-------- src/pybind/mgr/dashboard/tests/__init__.py | 2 + .../mgr/dashboard/tests/test_rgw_client.py | 95 ++++++++++++-- 4 files changed, 160 insertions(+), 68 deletions(-) diff --git a/src/pybind/mgr/dashboard/module.py b/src/pybind/mgr/dashboard/module.py index 512fd4ebcc1dd..62b0c4c922384 100644 --- a/src/pybind/mgr/dashboard/module.py +++ b/src/pybind/mgr/dashboard/module.py @@ -29,7 +29,7 @@ from .controllers import generate_routes, json_error_page from .grafana import push_local_dashboards from .services.auth import AuthManager, AuthManagerTool, JwtManager from .services.exception import dashboard_exception_handler -from .services.rgw_client import configure_rgw_users +from .services.rgw_client import configure_rgw_credentials from .services.sso import SSO_COMMANDS, handle_sso_command from .settings import handle_option_command, options_command_list, options_schema_list from .tools import NotificationQueue, RequestLoggingTool, TaskManager, \ @@ -409,8 +409,13 @@ class Module(MgrModule, CherryPyConfig): @CLIWriteCommand("dashboard connect-rgw") def connect_rgw(self): - configure_rgw_users() - return 0, '', '' + try: + configure_rgw_credentials() + except Exception as error: + logger.exception(error) + return -errno.EINVAL, '', str(error) + + return 0, 'RGW credentials configured', '' def handle_command(self, inbuf, cmd): # pylint: disable=too-many-return-statements diff --git a/src/pybind/mgr/dashboard/services/rgw_client.py b/src/pybind/mgr/dashboard/services/rgw_client.py index fd3ecdb5adeaf..af1f13af6a6a0 100644 --- a/src/pybind/mgr/dashboard/services/rgw_client.py +++ b/src/pybind/mgr/dashboard/services/rgw_client.py @@ -36,13 +36,19 @@ class NoCredentialsException(RequestException): 'the dashboard.') +class RgwAdminException(Exception): + pass + + class RgwDaemon: """Simple representation of a daemon.""" host: str name: str port: int ssl: bool + realm_name: str zonegroup_name: str + zone_name: str def _get_daemons() -> Dict[str, RgwDaemon]: @@ -59,7 +65,9 @@ def _get_daemons() -> Dict[str, RgwDaemon]: if dict_contains_path(daemon_map[key], ['metadata', 'frontend_config#0']): daemon = _determine_rgw_addr(daemon_map[key]) daemon.name = daemon_map[key]['metadata']['id'] + daemon.realm_name = daemon_map[key]['metadata']['realm_name'] daemon.zonegroup_name = daemon_map[key]['metadata']['zonegroup_name'] + daemon.zone_name = daemon_map[key]['metadata']['zone_name'] daemons[daemon.name] = daemon logger.info('Found RGW daemon with configuration: host=%s, port=%d, ssl=%s', daemon.host, daemon.port, str(daemon.ssl)) @@ -185,9 +193,9 @@ def _parse_frontend_config(config) -> Tuple[int, bool]: raise LookupError('Failed to determine RGW port from "{}"'.format(config)) -def radosgw_admin(args: List[str]) -> Tuple[int, str, str]: +def _radosgw_admin(args: List[str]) -> Tuple[int, str, str]: try: - result = subprocess.run( + result = subprocess.run( # pylint: disable=subprocess-run-check [ 'radosgw-admin', '-c', str(mgr.get_ceph_conf_path()), @@ -207,60 +215,62 @@ def radosgw_admin(args: List[str]) -> Tuple[int, str, str]: raise -def configure_rgw_users(): - mgr.log.info('Configuring dashboard RGW credentials') - user = 'dashboard' +def _parse_secrets(user: str, payload: str) -> Tuple[Optional[str], Optional[str]]: + data = json.loads(payload) + for key in data.get('keys', []): + if key.get('user') == user and data.get('system') in ['true', True]: + access_key = key.get('access_key') + secret_key = key.get('secret_key') + return access_key, secret_key + return None, None - def parse_secrets(user: str, out: str) -> Tuple[Optional[str], Optional[str]]: - r = json.loads(out) - for k in r.get('keys', []): - if k.get('user') == user and r.get('system') in ['true', True]: - access_key = k.get('access_key') - secret_key = k.get('secret_key') - return access_key, secret_key - return None, None - - def get_keys(realm: Optional[str]) -> Tuple[str, str]: - cmd = ['user', 'info', '--uid', user] - if realm: - cmd += ['--rgw-realm', realm] - rc, out, _ = radosgw_admin(cmd) - access_key = None - secret_key = None + +def _get_user_keys(user: str, realm: Optional[str] = None) -> Tuple[str, str]: + cmd = ['user', 'info', '--uid', user] + cmd_realm_option = ['--rgw-realm', realm] if realm else [] + if realm: + cmd += cmd_realm_option + rc, out, err = _radosgw_admin(cmd) + access_key = None + secret_key = None + if not rc: + access_key, secret_key = _parse_secrets(user, out) + if not access_key: + rgw_create_user_cmd = [ + 'user', 'create', + '--uid', user, + '--display-name', 'Ceph Dashboard', + '--system', + ] + cmd_realm_option + rc, out, err = _radosgw_admin(rgw_create_user_cmd) if not rc: - access_key, secret_key = parse_secrets(user, out) - if not access_key: - rc, out, err = radosgw_admin([ - 'user', 'create', - '--uid', user, - '--display-name', 'Ceph Dashboard', - '--system', - ]) - if not rc: - access_key, secret_key = parse_secrets(user, out) - if not access_key: - mgr.log.error(f'Unable to create rgw {user} user: {err}') - assert access_key - assert secret_key - return access_key, secret_key - - rc, out, err = radosgw_admin(['realm', 'list']) + access_key, secret_key = _parse_secrets(user, out) + if not access_key: + mgr.log.error(f'Unable to create rgw {user} user: {err}') + assert access_key + assert secret_key + return access_key, secret_key + + +def configure_rgw_credentials(): + mgr.log.info('Configuring dashboard RGW credentials') + user = 'dashboard' + rc, out, err = _radosgw_admin(['realm', 'list']) if rc: - mgr.log.error(f'Unable to list RGW realms: {err}') - return + error = RgwAdminException(f'Unable to list RGW realms: {err}') + mgr.log.exception(error) + raise error j = json.loads(out) realms = j.get('realms', []) - access_key = None - secret_key = None if realms: - als = {} - sls = {} + access_keys = {} + secret_keys = {} for realm in realms: - als[realm], sls[realm] = get_keys(realm) - access_key = json.dumps(als) - secret_key = json.dumps(sls) + access_keys[realm], secret_keys[realm] = _get_user_keys(user, realm) + access_key = json.dumps(access_keys) + secret_key = json.dumps(secret_keys) else: - access_key, secret_key = get_keys(None) + access_key, secret_key = _get_user_keys(user) Settings.RGW_API_ACCESS_KEY = access_key Settings.RGW_API_SECRET_KEY = secret_key @@ -285,8 +295,9 @@ class RgwClient(RestClient): @staticmethod def _get_daemon_connection_info(daemon_name: str) -> dict: try: - access_key = Settings.RGW_API_ACCESS_KEY[daemon_name] - secret_key = Settings.RGW_API_SECRET_KEY[daemon_name] + realm_name = RgwClient._daemons[daemon_name].realm_name + access_key = Settings.RGW_API_ACCESS_KEY[realm_name] + secret_key = Settings.RGW_API_SECRET_KEY[realm_name] except TypeError: # Legacy string values. access_key = Settings.RGW_API_ACCESS_KEY @@ -323,10 +334,11 @@ class RgwClient(RestClient): # The API access key and secret key are mandatory for a minimal configuration. if not (Settings.RGW_API_ACCESS_KEY and Settings.RGW_API_SECRET_KEY): - logger.warning('No credentials found, please consult the ' - 'documentation about how to enable RGW for the ' - 'dashboard.') - raise NoCredentialsException() + try: + configure_rgw_credentials() + except Exception as error: + logger.exception(error) + raise NoCredentialsException() if not daemon_name: # Select default daemon if configured in settings: diff --git a/src/pybind/mgr/dashboard/tests/__init__.py b/src/pybind/mgr/dashboard/tests/__init__.py index 64a71561ff869..f755a69faf8a8 100644 --- a/src/pybind/mgr/dashboard/tests/__init__.py +++ b/src/pybind/mgr/dashboard/tests/__init__.py @@ -277,6 +277,7 @@ class RgwStub(Stub): 'metadata': { 'frontend_config#0': 'beast port=8000', 'id': 'daemon1', + 'realm_name': 'realm1', 'zonegroup_name': 'zonegroup1', 'zone_name': 'zone1' } @@ -286,6 +287,7 @@ class RgwStub(Stub): 'metadata': { 'frontend_config#0': 'civetweb port=8002', 'id': 'daemon2', + 'realm_name': 'realm2', 'zonegroup_name': 'zonegroup2', 'zone_name': 'zone2' } diff --git a/src/pybind/mgr/dashboard/tests/test_rgw_client.py b/src/pybind/mgr/dashboard/tests/test_rgw_client.py index b01076c8b19ef..ff75fcf71c9a8 100644 --- a/src/pybind/mgr/dashboard/tests/test_rgw_client.py +++ b/src/pybind/mgr/dashboard/tests/test_rgw_client.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # pylint: disable=too-many-public-methods +import errno from unittest import TestCase from unittest.mock import Mock, patch @@ -7,12 +8,32 @@ from ..exceptions import DashboardException from ..services.rgw_client import NoCredentialsException, \ NoRgwDaemonsException, RgwClient, _parse_frontend_config from ..settings import Settings -from . import KVStoreMockMixin, RgwStub # pylint: disable=no-name-in-module +from . import CLICommandTestMixin, CmdException, RgwStub # pylint: disable=no-name-in-module @patch('dashboard.services.rgw_client.RgwClient._get_user_id', Mock( return_value='dummy_admin')) -class RgwClientTest(TestCase, KVStoreMockMixin): +class RgwClientTest(TestCase, CLICommandTestMixin): + _dashboard_user_realm1_access_key = 'VUOFXZFK24H81ISTVBTR' + _dashboard_user_realm1_secret_key = '0PGsCvXPGWS3AGgibUZEcd9efLrbbshlUkY3jruR' + _dashboard_user_realm2_access_key = 'OMDR282VYLBC1ZYMYDL0' + _dashboard_user_realm2_secret_key = 'N3thf7jAiwQ90PsPrhC2DIcvCFOsBXtBvPJJMdC3' + _radosgw_admin_result_error = (-errno.EINVAL, '', 'fake error') + _radosgw_admin_result_no_realms = (0, '{}', '') + _radosgw_admin_result_realms = (0, '{"realms": ["realm1", "realm2"]}', '') + _radosgw_admin_result_user_realm1_payload = ( + 0, + '{"keys": [{"user": "dashboard", "access_key": "' + f'{_dashboard_user_realm1_access_key}",' + f'"secret_key": "{_dashboard_user_realm1_secret_key}"}}], "system": "true"}}', + '') + _radosgw_admin_result_user_realm2_payload = ( + 0, + '{"keys": [{"user": "dashboard", "access_key": "' + f'{_dashboard_user_realm2_access_key}",' + f'"secret_key": "{_dashboard_user_realm2_secret_key}"}}], "system": "true"}}', + '') + def setUp(self): RgwStub.get_daemons() self.mock_kv_store() @@ -21,6 +42,67 @@ class RgwClientTest(TestCase, KVStoreMockMixin): 'RGW_API_SECRET_KEY': 'supergeheim', }) + @patch('dashboard.services.rgw_client._radosgw_admin') + def test_connect_rgw(self, radosgw_admin): + radosgw_admin.side_effect = [ + self._radosgw_admin_result_error, + self._radosgw_admin_result_no_realms, + self._radosgw_admin_result_user_realm1_payload + ] + with self.assertRaises(CmdException) as ctx: + self.exec_cmd('connect-rgw') + self.assertEqual(ctx.exception.retcode, -errno.EINVAL) + self.assertIn('Unable to list RGW realms', str(ctx.exception)) + + result = self.exec_cmd('connect-rgw') + self.assertEqual(result, 'RGW credentials configured') + self.assertEqual(Settings.RGW_API_ACCESS_KEY, self._dashboard_user_realm1_access_key) + self.assertEqual(Settings.RGW_API_SECRET_KEY, self._dashboard_user_realm1_secret_key) + + def test_configure_credentials_error(self): + self.CONFIG_KEY_DICT.update({ + 'RGW_API_ACCESS_KEY': '', + 'RGW_API_SECRET_KEY': '', + }) + with self.assertRaises(NoCredentialsException) as cm: + RgwClient.admin_instance() + self.assertIn('No RGW credentials found', str(cm.exception)) + + @patch('dashboard.services.rgw_client._radosgw_admin') + def test_configure_credentials_with_no_realms(self, radosgw_admin): + self.CONFIG_KEY_DICT.update({ + 'RGW_API_ACCESS_KEY': '', + 'RGW_API_SECRET_KEY': '', + }) + radosgw_admin.side_effect = [ + self._radosgw_admin_result_no_realms, + self._radosgw_admin_result_user_realm1_payload + ] + RgwClient.admin_instance() + self.assertEqual(Settings.RGW_API_ACCESS_KEY, self._dashboard_user_realm1_access_key) + self.assertEqual(Settings.RGW_API_SECRET_KEY, self._dashboard_user_realm1_secret_key) + + @patch('dashboard.services.rgw_client._radosgw_admin') + def test_configure_credentials_with_realms(self, radosgw_admin): + self.CONFIG_KEY_DICT.update({ + 'RGW_API_ACCESS_KEY': '', + 'RGW_API_SECRET_KEY': '', + }) + radosgw_admin.side_effect = [ + self._radosgw_admin_result_realms, + self._radosgw_admin_result_user_realm1_payload, + self._radosgw_admin_result_user_realm2_payload + ] + RgwClient.admin_instance() + self.assertEqual(Settings.RGW_API_ACCESS_KEY, { + 'realm1': self._dashboard_user_realm1_access_key, + 'realm2': self._dashboard_user_realm2_access_key + }) + self.assertEqual(Settings.RGW_API_SECRET_KEY, { + 'realm1': self._dashboard_user_realm1_secret_key, + 'realm2': self._dashboard_user_realm2_secret_key + }) + def test_ssl_verify(self): Settings.RGW_API_SSL_VERIFY = True instance = RgwClient.admin_instance() @@ -37,15 +119,6 @@ class RgwClientTest(TestCase, KVStoreMockMixin): RgwClient.admin_instance() self.assertIn('No RGW service is running.', str(cm.exception)) - def test_no_credentials(self): - self.CONFIG_KEY_DICT.update({ - 'RGW_API_ACCESS_KEY': '', - 'RGW_API_SECRET_KEY': '', - }) - with self.assertRaises(NoCredentialsException) as cm: - RgwClient.admin_instance() - self.assertIn('No RGW credentials found', str(cm.exception)) - def test_default_daemon_wrong_settings(self): self.CONFIG_KEY_DICT.update({ 'RGW_API_HOST': '172.20.0.2', -- 2.39.5