]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
mgr/dashboard: connect-rgw: adaptation and test coverage
authorAlfonso Martínez <almartin@redhat.com>
Wed, 28 Jul 2021 07:48:18 +0000 (09:48 +0200)
committerAlfonso Martínez <almartin@redhat.com>
Tue, 10 Aug 2021 12:06:03 +0000 (14:06 +0200)
- 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 <almartin@redhat.com>
src/pybind/mgr/dashboard/module.py
src/pybind/mgr/dashboard/services/rgw_client.py
src/pybind/mgr/dashboard/tests/__init__.py
src/pybind/mgr/dashboard/tests/test_rgw_client.py

index 512fd4ebcc1ddcad3116f499fd1c5330f21e5d3c..62b0c4c922384d759287e3b46df0fa168dd77e68 100644 (file)
@@ -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
index fd3ecdb5adeafabafec62759fcae4d95d2ad53db..af1f13af6a6a069eea4093b7273b93906ee763e0 100644 (file)
@@ -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:
index 64a71561ff8693ff279950680dbabcaaa18aa362..f755a69faf8a8e2e44a0d59627c3da9156203d98 100644 (file)
@@ -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'
                 }
index b01076c8b19efe0ec06dbeb8a606ef48366e4427..ff75fcf71c9a8a1af248c2ed34f75ced59e3302e 100644 (file)
@@ -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',