mgr/dashboard: Unable to edit iSCSI logged-in client

Fixes: https://tracker.ceph.com/issues/46818
Signed-off-by: Ricardo Marques <rimarques@suse.com>
(cherry picked from commit 7ee7fb98d343f6c6c44cac60ce6ca5ab8aeb9021)

diff --git a/src/pybind/mgr/dashboard/controllers/iscsi.py b/src/pybind/mgr/dashboard/controllers/iscsi.py
index d78ef9ce9ac7b1faaae883c92bd43ee756a99344..3eb6fdbb47d89c99f2f81244b3798b0b55d98f4c 100644 (file)
--- a/src/pybind/mgr/dashboard/controllers/iscsi.py
+++ b/src/pybind/mgr/dashboard/controllers/iscsi.py
@@ -1,5 +1,6 @@
 # -*- coding: utf-8 -*-
 # pylint: disable=too-many-branches
+# pylint: disable=too-many-lines
 from __future__ import absolute_import
 
 from copy import deepcopy
@@ -323,6 +324,8 @@ class IscsiTarget(RESTController):
         deleted_portal_names = list(old_portal_names - new_portal_names)
         validate_rest_api(deleted_portal_names)
         IscsiTarget._validate(new_target_iqn, target_controls, portals, disks, groups, settings)
+        IscsiTarget._validate_delete(gateway, target_iqn, config, new_target_iqn, target_controls,
+                                     disks, clients, groups)
         config = IscsiTarget._delete(target_iqn, config, 0, 50, new_target_iqn, target_controls,
                                      portals, disks, clients, groups)
         IscsiTarget._create(new_target_iqn, target_controls, acl_enabled, auth, portals, disks,
@@ -363,13 +366,21 @@ class IscsiTarget(RESTController):
                                                                              group_id)
             TaskManager.current_task().inc_progress(task_progress_inc)
         deleted_clients = []
-        for client_iqn in list(target_config['clients'].keys()):
+        deleted_client_luns = []
+        for client_iqn, client_config in target_config['clients'].items():
             if IscsiTarget._client_deletion_required(target, new_target_iqn, new_target_controls,
                                                      new_clients, client_iqn,
-                                                     new_groups, deleted_groups):
+                                                     deleted_groups):
                 deleted_clients.append(client_iqn)
                 IscsiClient.instance(gateway_name=gateway_name).delete_client(target_iqn,
                                                                               client_iqn)
+            else:
+                for image_id in list(client_config.get('luns', {}).keys()):
+                    if IscsiTarget._client_lun_deletion_required(target, client_iqn, image_id,
+                                                                 new_clients):
+                        deleted_client_luns.append((client_iqn, image_id))
+                        IscsiClient.instance(gateway_name=gateway_name).delete_client_lun(
+                            target_iqn, client_iqn, image_id)
             TaskManager.current_task().inc_progress(task_progress_inc)
         for image_id in target_config['disks']:
             if IscsiTarget._target_lun_deletion_required(target, new_target_iqn,
@@ -380,7 +391,8 @@ class IscsiTarget(RESTController):
                 for client_iqn in not_deleted_clients:
                     client_image_ids = target_config['clients'][client_iqn]['luns'].keys()
                     for client_image_id in client_image_ids:
-                        if image_id == client_image_id:
+                        if image_id == client_image_id and \
+                                (client_iqn, client_image_id) not in deleted_client_luns:
                             IscsiClient.instance(gateway_name=gateway_name).delete_client_lun(
                                 target_iqn, client_iqn, client_image_id)
                 IscsiClient.instance(gateway_name=gateway_name).delete_target_lun(target_iqn,
@@ -423,7 +435,7 @@ class IscsiTarget(RESTController):
         for client_iqn in new_group['members']:
             if IscsiTarget._client_deletion_required(target, new_target_iqn, new_target_controls,
                                                      new_clients, client_iqn,
-                                                     new_groups, []):
+                                                     []):
                 return True
         # Check if any disk inside this group has changed
         for disk in new_group['disks']:
@@ -443,25 +455,32 @@ class IscsiTarget(RESTController):
 
     @staticmethod
     def _client_deletion_required(target, new_target_iqn, new_target_controls,
-                                  new_clients, client_iqn, new_groups, deleted_groups):
+                                  new_clients, client_iqn, deleted_groups):
         if IscsiTarget._target_deletion_required(target, new_target_iqn, new_target_controls):
             return True
-        new_client = deepcopy(IscsiTarget._get_client(new_clients, client_iqn))
+        new_client = IscsiTarget._get_client(new_clients, client_iqn)
         if not new_client:
             return True
-        # Disks inherited from groups must be considered
-        for group in new_groups:
-            if client_iqn in group['members']:
-                new_client['luns'] += group['disks']
-        old_client = IscsiTarget._get_client(target['clients'], client_iqn)
-        if new_client != old_client:
-            return True
         # Check if client belongs to a groups that has been deleted
         for group in target['groups']:
             if group['group_id'] in deleted_groups and client_iqn in group['members']:
                 return True
         return False
 
+    @staticmethod
+    def _client_lun_deletion_required(target, client_iqn, image_id, new_clients):
+        new_client = IscsiTarget._get_client(new_clients, client_iqn)
+        if not new_client:
+            return True
+        new_lun = IscsiTarget._get_disk(new_client.get('luns', []), image_id)
+        if not new_lun:
+            return True
+        old_client = IscsiTarget._get_client(target['clients'], client_iqn)
+        if not old_client:
+            return False
+        old_lun = IscsiTarget._get_disk(old_client.get('luns', []), image_id)
+        return new_lun != old_lun
+
     @staticmethod
     def _get_disk(disks, image_id):
         for disk in disks:
@@ -619,6 +638,33 @@ class IscsiTarget(RESTController):
                                      code='pool_does_not_exist',
                                      component='iscsi')
 
+    @staticmethod
+    def _validate_delete(gateway, target_iqn, config, new_target_iqn=None, new_target_controls=None,
+                         new_disks=None, new_clients=None, new_groups=None):
+        new_target_controls = new_target_controls or {}
+        new_disks = new_disks or []
+        new_clients = new_clients or []
+        new_groups = new_groups or []
+
+        target_config = config['targets'][target_iqn]
+        target = IscsiTarget._config_to_target(target_iqn, config)
+        deleted_groups = []
+        for group_id in list(target_config['groups'].keys()):
+            if IscsiTarget._group_deletion_required(target, new_target_iqn, new_target_controls,
+                                                    new_groups, group_id, new_clients,
+                                                    new_disks):
+                deleted_groups.append(group_id)
+        for client_iqn in list(target_config['clients'].keys()):
+            if IscsiTarget._client_deletion_required(target, new_target_iqn, new_target_controls,
+                                                     new_clients, client_iqn, deleted_groups):
+                client_info = IscsiClient.instance(gateway_name=gateway).get_clientinfo(target_iqn,
+                                                                                        client_iqn)
+                if client_info.get('state', {}).get('LOGGED_IN', []):
+                    raise DashboardException(msg="Client '{}' cannot be deleted until it's logged "
+                                             "out".format(client_iqn),
+                                             code='client_logged_in',
+                                             component='iscsi')
+
     @staticmethod
     def _update_targetauth(config, target_iqn, auth, gateway_name):
         # Target level authentication was introduced in ceph-iscsi config v11
@@ -641,11 +687,11 @@ class IscsiTarget(RESTController):
                                                                              targetauth_action)
 
     @staticmethod
-    def _is_auth_equal(target_auth, auth):
-        return auth['user'] == target_auth['username'] and \
-               auth['password'] == target_auth['password'] and \
-               auth['mutual_user'] == target_auth['mutual_username'] and \
-               auth['mutual_password'] == target_auth['mutual_password']
+    def _is_auth_equal(auth_config, auth):
+        return auth['user'] == auth_config['username'] and \
+            auth['password'] == auth_config['password'] and \
+            auth['mutual_user'] == auth_config['mutual_username'] and \
+            auth['mutual_password'] == auth_config['mutual_password']
 
     @staticmethod
     def _create(target_iqn, target_controls, acl_enabled,
@@ -723,6 +769,9 @@ class IscsiTarget(RESTController):
                 if not target_config or client_iqn not in target_config['clients']:
                     IscsiClient.instance(gateway_name=gateway_name).create_client(target_iqn,
                                                                                   client_iqn)
+                if not target_config or client_iqn not in target_config['clients'] or \
+                        not IscsiTarget._is_auth_equal(target_config['clients'][client_iqn]['auth'],
+                                                       client['auth']):
                     user = client['auth']['user']
                     password = client['auth']['password']
                     m_user = client['auth']['mutual_user']

diff --git a/src/pybind/mgr/dashboard/tests/test_iscsi.py b/src/pybind/mgr/dashboard/tests/test_iscsi.py
index ad6a8760959cca02de17bd2abace5589cc86d868..390f70a96392a7c9c94be5674341dc903b20ff75 100644 (file)
--- a/src/pybind/mgr/dashboard/tests/test_iscsi.py
+++ b/src/pybind/mgr/dashboard/tests/test_iscsi.py
@@ -222,7 +222,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
                     "state": {}
                 }
             })
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_add_bad_client(self, _validate_image_mock):
@@ -263,7 +263,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
         response = copy.deepcopy(iscsi_target_response)
         response['target_iqn'] = target_iqn
         response['clients'][0]['auth']['password'] = 'MyNewPassword'
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_rename_client(self, _validate_image_mock):
@@ -276,7 +276,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
         response = copy.deepcopy(iscsi_target_response)
         response['target_iqn'] = target_iqn
         response['clients'][0]['client_iqn'] = 'iqn.1994-05.com.redhat:rh7-client0'
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_add_disk(self, _validate_image_mock):
@@ -306,7 +306,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
 
             })
         response['clients'][0]['luns'].append({"image": "lun3", "pool": "rbd"})
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_change_disk_image(self, _validate_image_mock):
@@ -321,7 +321,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
         response['target_iqn'] = target_iqn
         response['disks'][0]['image'] = 'lun0'
         response['clients'][0]['luns'][0]['image'] = 'lun0'
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_change_disk_controls(self, _validate_image_mock):
@@ -334,7 +334,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
         response = copy.deepcopy(iscsi_target_response)
         response['target_iqn'] = target_iqn
         response['disks'][0]['controls'] = {"qfull_timeout": 15}
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_rename_target(self, _validate_image_mock):
@@ -346,7 +346,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
         update_request['new_target_iqn'] = new_target_iqn
         response = copy.deepcopy(iscsi_target_response)
         response['target_iqn'] = new_target_iqn
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_rename_group(self, _validate_image_mock):
@@ -359,7 +359,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
         response = copy.deepcopy(iscsi_target_response)
         response['target_iqn'] = target_iqn
         response['groups'][0]['group_id'] = 'mygroup0'
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_add_client_to_group(self, _validate_image_mock):
@@ -397,7 +397,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
                 }
             })
         response['groups'][0]['members'].append('iqn.1994-05.com.redhat:rh7-client3')
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_remove_client_from_group(self, _validate_image_mock):
@@ -410,7 +410,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
         response = copy.deepcopy(iscsi_target_response)
         response['target_iqn'] = target_iqn
         response['groups'][0]['members'].remove('iqn.1994-05.com.redhat:rh7-client2')
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_remove_groups(self, _validate_image_mock):
@@ -423,7 +423,7 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
         response = copy.deepcopy(iscsi_target_response)
         response['target_iqn'] = target_iqn
         response['groups'] = []
-        self._update_iscsi_target(create_request, update_request, response)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
 
     @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
     def test_add_client_to_multiple_groups(self, _validate_image_mock):
@@ -440,11 +440,90 @@ class IscsiTestController(ControllerTestCase, KVStoreMockMixin):
             'component': 'iscsi'
         })
 
-    def _update_iscsi_target(self, create_request, update_request, response):
+    @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
+    def test_remove_client_lun(self, _validate_image_mock):
+        target_iqn = "iqn.2003-01.com.redhat.iscsi-gw:iscsi-igw17"
+        create_request = copy.deepcopy(iscsi_target_request)
+        create_request['target_iqn'] = target_iqn
+        create_request['clients'][0]['luns'] = [
+            {"image": "lun1", "pool": "rbd"},
+            {"image": "lun2", "pool": "rbd"},
+            {"image": "lun3", "pool": "rbd"}
+        ]
+        update_request = copy.deepcopy(create_request)
+        update_request['new_target_iqn'] = target_iqn
+        update_request['clients'][0]['luns'] = [
+            {"image": "lun1", "pool": "rbd"},
+            {"image": "lun3", "pool": "rbd"}
+        ]
+        response = copy.deepcopy(iscsi_target_response)
+        response['target_iqn'] = target_iqn
+        response['clients'][0]['luns'] = [
+            {"image": "lun1", "pool": "rbd"},
+            {"image": "lun3", "pool": "rbd"}
+        ]
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
+
+    @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
+    def test_change_client_auth(self, _validate_image_mock):
+        target_iqn = "iqn.2003-01.com.redhat.iscsi-gw:iscsi-igw18"
+        create_request = copy.deepcopy(iscsi_target_request)
+        create_request['target_iqn'] = target_iqn
+        update_request = copy.deepcopy(create_request)
+        update_request['new_target_iqn'] = target_iqn
+        update_request['clients'][0]['auth']['password'] = 'myiscsipasswordX'
+        response = copy.deepcopy(iscsi_target_response)
+        response['target_iqn'] = target_iqn
+        response['clients'][0]['auth']['password'] = 'myiscsipasswordX'
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
+
+    @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
+    def test_remove_client_logged_in(self, _validate_image_mock):
+        client_info = {
+            'alias': '',
+            'ip_address': [],
+            'state': {'LOGGED_IN': ['node1']}
+        }
+        # pylint: disable=protected-access
+        IscsiClientMock._instance.clientinfo = client_info
+        target_iqn = "iqn.2003-01.com.redhat.iscsi-gw:iscsi-igw19"
+        create_request = copy.deepcopy(iscsi_target_request)
+        create_request['target_iqn'] = target_iqn
+        update_request = copy.deepcopy(create_request)
+        update_request['new_target_iqn'] = target_iqn
+        update_request['clients'].pop(0)
+        response = copy.deepcopy(iscsi_target_response)
+        response['target_iqn'] = target_iqn
+        for client in response['clients']:
+            client['info'] = client_info
+        update_response = {
+            'detail': "Client 'iqn.1994-05.com.redhat:rh7-client' cannot be deleted until it's "
+                      "logged out",
+            'code': 'client_logged_in',
+            'component': 'iscsi'
+        }
+        self._update_iscsi_target(create_request, update_request, 400, update_response, response)
+
+    @mock.patch('dashboard.controllers.iscsi.IscsiTarget._validate_image')
+    def test_remove_client(self, _validate_image_mock):
+        target_iqn = "iqn.2003-01.com.redhat.iscsi-gw:iscsi-igw20"
+        create_request = copy.deepcopy(iscsi_target_request)
+        create_request['target_iqn'] = target_iqn
+        update_request = copy.deepcopy(create_request)
+        update_request['new_target_iqn'] = target_iqn
+        update_request['clients'].pop(0)
+        response = copy.deepcopy(iscsi_target_response)
+        response['target_iqn'] = target_iqn
+        response['clients'].pop(0)
+        self._update_iscsi_target(create_request, update_request, 200, None, response)
+
+    def _update_iscsi_target(self, create_request, update_request, update_response_code,
+                             update_response, response):
         self._task_post('/api/iscsi/target', create_request)
         self.assertStatus(201)
         self._task_put('/api/iscsi/target/{}'.format(create_request['target_iqn']), update_request)
-        self.assertStatus(200)
+        self.assertStatus(update_response_code)
+        self.assertJsonBody(update_response)
         self._get('/api/iscsi/target/{}'.format(update_request['new_target_iqn']))
         self.assertStatus(200)
         self.assertJsonBody(response)
@@ -591,6 +670,11 @@ class IscsiClientMock(object):
             "updated": "",
             "version": 11
         }
+        self.clientinfo = {
+            'alias': '',
+            'ip_address': [],
+            'state': {}
+        }
 
     @classmethod
     def instance(cls, gateway_name=None, service_url=None):
@@ -817,8 +901,4 @@ class IscsiClientMock(object):
 
     def get_clientinfo(self, target_iqn, client_iqn):
         # pylint: disable=unused-argument
-        return {
-            'alias': '',
-            'ip_address': [],
-            'state': {}
-        }
+        return self.clientinfo