diff --git a/cinder/tests/unit/volume/drivers/lightos/test_lightos_storage.py b/cinder/tests/unit/volume/drivers/lightos/test_lightos_storage.py index 3f4130bfdc5..0d0ab8644c4 100644 --- a/cinder/tests/unit/volume/drivers/lightos/test_lightos_storage.py +++ b/cinder/tests/unit/volume/drivers/lightos/test_lightos_storage.py @@ -63,6 +63,7 @@ FAKE_LIGHTOS_CLUSTER_INFO: Dict[str, str] = { } FAKE_CLIENT_HOSTNQN = "hostnqn1" +FAKE_HOST_IPS = ['10.10.0.1'] VOLUME_BACKEND_NAME = "lightos_backend" RESERVED_PERCENTAGE = 30 DEVICE_SCAN_ATTEMPTS_DEFAULT = 5 @@ -85,6 +86,7 @@ class InitiatorConnectorFactoryMocker: class InitialConnectorMock: nqn = FAKE_CLIENT_HOSTNQN found_discovery_client = True + host_ips = FAKE_HOST_IPS def get_hostnqn(self): return self.__class__.nqn @@ -92,15 +94,18 @@ class InitialConnectorMock: def find_dsc(self): return self.__class__.found_discovery_client + def get_host_ips(self): + return self.__class__.host_ips + def get_connector_properties(self, root): return dict(nqn=self.__class__.nqn, - found_dsc=self.__class__.found_discovery_client) + found_dsc=self.__class__.found_discovery_client, + host_ips=self.__class__.host_ips) def get_connector_properties(): connector = InitialConnectorMock() - return dict(nqn=connector.get_hostnqn(), - found_dsc=connector.find_dsc()) + return connector.get_connector_properties(None) def get_vol_etag(volume): @@ -172,6 +177,9 @@ class DBMock(object): volume["size"] = kwargs["size"] if kwargs.get("acl", None): volume["acl"] = {'values': kwargs.get('acl')} + + if kwargs.get("ip_acl", None): + volume["IPAcl"] = {'values': kwargs.get('ip_acl')} volume["ETag"] = get_vol_etag(volume) return httpstatus.OK, volume @@ -202,7 +210,7 @@ class DBMock(object): return httpstatus.OK, vol def update_volume(self, **kwargs): - assert ("project_name" in kwargs and kwargs["project_name"]), \ + assert "project_name" in kwargs and kwargs["project_name"], \ "project_name must be provided" def create_snapshot(self, snapshot) -> Tuple[int, Dict]: @@ -275,6 +283,7 @@ class LightOSStorageVolumeDriverTest(test.TestCase): configuration.lightos_default_compression_enabled = ( DEFAULT_COMPRESSION) configuration.lightos_default_num_replicas = 3 + configuration.lightos_use_ipacl = True configuration.num_volume_device_scan_tries = ( DEVICE_SCAN_ATTEMPTS_DEFAULT) configuration.lightos_api_service_timeout = LIGHTOS_API_SERVICE_TIMEOUT @@ -314,6 +323,10 @@ class LightOSStorageVolumeDriverTest(test.TestCase): return (httpstatus.OK, FAKE_LIGHTOS_CLUSTER_INFO) elif cmd == "create_volume": project_name = kwargs["project_name"] + ipacl = ( + {'values': ['ALLOW_NONE']} + if self.driver.configuration.lightos_use_ipacl + else {'values': ['ALLOW_ANY']}) volume = { "project_name": project_name, "name": kwargs["name"], @@ -322,6 +335,7 @@ class LightOSStorageVolumeDriverTest(test.TestCase): "compression": kwargs["compression"], "src_snapshot_name": kwargs["src_snapshot_name"], "acl": {'values': kwargs.get('acl')}, + "IPAcl": ipacl, "state": "Available", } volume["ETag"] = get_vol_etag(volume) @@ -396,6 +410,20 @@ class LightOSStorageVolumeDriverTest(test.TestCase): self.driver.delete_volume(volume) db.volume_destroy(self.ctxt, volume.id) + def test_create_volume_ipacl_off(self): + """Test that lightos_client succeed. ipacl false""" + self.driver.configuration.lightos_use_ipacl = False + self.driver.do_setup(None) + + vol_type = test_utils.create_volume_type(self.ctxt, self, + name='my_vol_type') + volume = test_utils.create_volume(self.ctxt, size=4, + volume_type_id=vol_type.id) + + self.driver.create_volume(volume) + self.driver.delete_volume(volume) + db.volume_destroy(self.ctxt, volume.id) + def test_create_volume_same_volume_twice_succeed(self): """Test succeed to create an exiting volume.""" self.driver.do_setup(None) @@ -418,6 +446,10 @@ class LightOSStorageVolumeDriverTest(test.TestCase): def send_cmd_mock(cmd, **kwargs): if cmd == "create_volume": project_name = kwargs["project_name"] + ipacl = ( + {'values': ['ALLOW_NONE']} + if self.driver.configuration.lightos_use_ipacl + else {'values': ['ALLOW_ANY']}) volume = { "project_name": project_name, "name": kwargs["name"], @@ -426,6 +458,7 @@ class LightOSStorageVolumeDriverTest(test.TestCase): "compression": kwargs["compression"], "src_snapshot_name": kwargs["src_snapshot_name"], "acl": {'values': kwargs.get('acl')}, + "IPAcl": ipacl, "state": vol_state, } volume["ETag"] = get_vol_etag(volume) @@ -462,6 +495,10 @@ class LightOSStorageVolumeDriverTest(test.TestCase): def send_cmd_mock(cmd, **kwargs): if cmd == "create_volume": project_name = kwargs["project_name"] + ipacl = ( + {'values': ['ALLOW_NONE']} + if self.driver.configuration.lightos_use_ipacl + else {'values': ['ALLOW_ANY']}) volume = { "project_name": project_name, "name": kwargs["name"], @@ -470,6 +507,7 @@ class LightOSStorageVolumeDriverTest(test.TestCase): "compression": kwargs["compression"], "src_snapshot_name": kwargs["src_snapshot_name"], "acl": {'values': kwargs.get('acl')}, + "IPAcl": ipacl, "state": "Migrating", } volume["ETag"] = get_vol_etag(volume) @@ -641,6 +679,7 @@ class LightOSStorageVolumeDriverTest(test.TestCase): def test_initialize_connection(self): InitialConnectorMock.nqn = "hostnqn1" InitialConnectorMock.found_discovery_client = True + InitialConnectorMock.host_ips = FAKE_HOST_IPS self.driver.do_setup(None) vol_type = test_utils.create_volume_type(self.ctxt, self, name='my_vol_type') @@ -657,6 +696,37 @@ class LightOSStorageVolumeDriverTest(test.TestCase): self.assertEqual( self.db.data['projects']['default']['volumes'][0]['UUID'], connection_props['data']['uuid']) + self.assertEqual( + self.db.data['projects']['default']['volumes'][0]['IPAcl'], + {'values': FAKE_HOST_IPS}) + + self.driver.delete_volume(volume) + db.volume_destroy(self.ctxt, volume.id) + + def test_initialize_connection_ipacl_disabled(self): + self.driver.configuration.lightos_use_ipacl = False + InitialConnectorMock.nqn = "hostnqn1" + InitialConnectorMock.found_discovery_client = True + self.driver.do_setup(None) + vol_type = test_utils.create_volume_type(self.ctxt, self, + name='my_vol_type') + volume = test_utils.create_volume(self.ctxt, size=4, + volume_type_id=vol_type.id) + self.driver.create_volume(volume) + + connection_props = \ + self.driver.initialize_connection(volume, + get_connector_properties()) + self.assertIn('driver_volume_type', connection_props) + self.assertEqual('lightos', connection_props['driver_volume_type']) + self.assertEqual(FAKE_LIGHTOS_CLUSTER_INFO['subsystemNQN'], + connection_props['data']['subsysnqn']) + self.assertEqual( + self.db.data['projects']['default']['volumes'][0]['UUID'], + connection_props['data']['uuid']) + self.assertEqual( + self.db.data['projects']['default']['volumes'][0]['IPAcl'], + {'values': ['ALLOW_ANY']}) self.driver.delete_volume(volume) db.volume_destroy(self.ctxt, volume.id) @@ -668,6 +738,10 @@ class LightOSStorageVolumeDriverTest(test.TestCase): def send_cmd_mock(cmd, **kwargs): if cmd == "create_volume": project_name = kwargs["project_name"] + ipacl = ( + {'values': ['ALLOW_NONE']} + if self.driver.configuration.lightos_use_ipacl + else {'values': ['ALLOW_ANY']}) volume = { "project_name": project_name, "name": kwargs["name"], @@ -676,6 +750,7 @@ class LightOSStorageVolumeDriverTest(test.TestCase): "compression": kwargs["compression"], "src_snapshot_name": kwargs["src_snapshot_name"], "acl": {'values': kwargs.get('acl')}, + "IPAcl": ipacl, "state": "Migrating", } volume["ETag"] = get_vol_etag(volume) diff --git a/cinder/volume/drivers/lightos.py b/cinder/volume/drivers/lightos.py index d71d06c8ef9..03e7a69b901 100644 --- a/cinder/volume/drivers/lightos.py +++ b/cinder/volume/drivers/lightos.py @@ -25,6 +25,7 @@ from urllib.parse import urlparse from oslo_config import cfg from oslo_log import log as logging from oslo_utils import importutils +from oslo_utils import netutils from oslo_utils import units import requests import urllib3 @@ -77,7 +78,16 @@ lightos_opts = [ cfg.IntOpt('lightos_api_service_timeout', default=30, help='The default amount of time (in seconds) to wait for' - ' an API endpoint response.') + ' an API endpoint response.'), + cfg.BoolOpt('lightos_use_ipacl', + default=True, + help='IPACL work in conjunction with the standard NVME ACL.' + ' A host must be in both the IPACL and the ACL of a volume to' + ' access that volume. Cinder always sets the volume`s ACL.' + ' If lightos_use_ipacl is set to True, Cinder will also add' + ' the host`s IP addresses to a volume IPACL. If set to' + ' False, any IP address may access the volume. The default' + ' is True.'), ] CONF = cfg.CONF @@ -152,6 +162,9 @@ class LightOSConnection(object): 'acl': { 'values': kwargs.get('acl'), }, + 'IPAcl': { + 'values': kwargs.get('ip_acl'), + }, 'sourceSnapshotUUID': kwargs.get( 'src_snapshot_uuid'), 'sourceSnapshotName': kwargs.get( @@ -170,6 +183,9 @@ class LightOSConnection(object): 'acl': { 'values': kwargs.get('acl'), }, + 'IPAcl': { + 'values': kwargs.get('ip_acl'), + }, }), 'extend_volume': ('PUT', @@ -598,6 +614,7 @@ class LightOSVolumeDriver(driver.VolumeDriver): src_snapshot_lightos_name=None): """Create a new LightOS volume for this openstack volume.""" (compression, num_replicas, _) = self._get_volume_specs(os_volume) + vol_ipAcl = ['ALLOW_NONE'] if self.use_ip_acl() else ['ALLOW_ANY'] return self.cluster.send_cmd( cmd='create_volume', project_name=project_name, @@ -607,7 +624,8 @@ class LightOSVolumeDriver(driver.VolumeDriver): n_replicas=num_replicas, compression=compression, src_snapshot_name=src_snapshot_lightos_name, - acl=['ALLOW_NONE'] + acl=['ALLOW_NONE'], + ip_acl=vol_ipAcl ) def _get_lightos_uuid(self, project_name, volume): @@ -1034,17 +1052,22 @@ class LightOSVolumeDriver(driver.VolumeDriver): return server_properties - def set_volume_acl(self, project_name, lightos_uuid, acl, etag): + def set_volume_acl(self, project_name, lightos_uuid, acl, ip_acl, etag): return self.cluster.send_cmd( cmd='update_volume', project_name=project_name, timeout=self.logical_op_timeout, volume_uuid=lightos_uuid, acl=acl, + ip_acl=ip_acl, etag=etag ) - def __add_volume_acl(self, project_name, lightos_volname, acl_to_add): + def use_ip_acl(self): + return self.configuration.lightos_use_ipacl + + def __add_volume_acl(self, project_name, lightos_volname, acl_to_add, + host_ips): (status, data) = self._get_lightos_volume(project_name, self.logical_op_timeout, vol_name=lightos_volname) @@ -1063,7 +1086,13 @@ class LightOSVolumeDriver(driver.VolumeDriver): LOG.warning('Got LightOS volume without ACL?! data: %s', data) return False + ip_acl = data.get('IPAcl') + if self.use_ip_acl() and not ip_acl: + LOG.warning('Got LightOS volume without IP ACL?! data: %s', data) + return False + acl = acl.get('values', []) + ip_acl = ip_acl.get('values', []) # remove ALLOW_NONE and add our acl_to_add if not already there if 'ALLOW_NONE' in acl: @@ -1071,15 +1100,53 @@ class LightOSVolumeDriver(driver.VolumeDriver): if acl_to_add not in acl: acl.append(acl_to_add) + if 'ALLOW_NONE' in ip_acl: + ip_acl.remove('ALLOW_NONE') + + if self.use_ip_acl(): + ip_acl = list(set(ip_acl).union(set(host_ips))) + else: + ip_acl = ['ALLOW_ANY'] + + # The max (16) elemenets are allowed in IPACL. + # if elements are more than 16 then remove + # less-frequently used IPv6 address(s), and IPv4 if needed. + + ipv4addrs = [addr for addr in ip_acl if netutils.is_valid_ipv4(addr)] + ipv6addrs = [addr for addr in ip_acl if netutils.is_valid_ipv6(addr)] + + IpAcl_size = 16 + if len(ipv4addrs) > IpAcl_size: + LOG.warning( + 'IPv4 address(es) are more than maximum (%s)' + ' allowed in IP-ACL of volume, therefore reducing' + ' IPv4 address(es) written to IP-ACL of volume %s' + ' of project %s', IpAcl_size, lightos_volname, + project_name) + + ip_acl = ipv4addrs[0: IpAcl_size] + + elif len(ip_acl) > IpAcl_size: + LOG.warning( + 'Combined IPv4 and IPv6 address(es) are more than' + ' maximum (%s) allowed in IP-ACL of volume, therefore' + ' reducing IPv6 address(es) written to IP-ACL of' + ' volume %s of project %s', IpAcl_size, + lightos_volname, project_name) + + ipv6addrs_count = IpAcl_size - len(ipv4addrs) + ip_acl = ipv4addrs + (ipv6addrs[0: ipv6addrs_count]) + return self.set_volume_acl( project_name, lightos_uuid, acl, + ip_acl, etag=data.get( 'ETag', '')) - def add_volume_acl(self, project_name, volume, acl_to_add): + def add_volume_acl(self, project_name, volume, acl_to_add, host_ips): LOG.debug( 'add_volume_acl got volume %s project %s acl %s', volume, @@ -1090,13 +1157,15 @@ class LightOSVolumeDriver(driver.VolumeDriver): self.__add_volume_acl, project_name, lightos_volname, - acl_to_add) + acl_to_add, + host_ips) def __remove_volume_acl( self, project_name, lightos_volname, - acl_to_remove): + acl_to_remove, + host_ips): (status, data) = self._get_lightos_volume(project_name, self.logical_op_timeout, vol_name=lightos_volname) @@ -1138,19 +1207,41 @@ class LightOSVolumeDriver(driver.VolumeDriver): if not acl: acl.append('ALLOW_NONE') + ip_acl = data.get('IPAcl') + if self.use_ip_acl() and not ip_acl: + LOG.warning('Got LightOS volume without IP ACL?! data: %s', data) + return False + + ip_acl = ip_acl.get('values') + if self.use_ip_acl() and not ip_acl: + LOG.warning( + 'Got LightOS volume without IP ACL values?! data: %s', data) + return False + + for ip in host_ips: + try: + ip_acl.remove(ip) + except ValueError: + LOG.warning( + 'Could not find matching ip %s in ip-acl of volume %s ', + ip, lightos_volname) + + if not ip_acl: + ip_acl.append('ALLOW_NONE') + return self.set_volume_acl( project_name, lightos_uuid, acl, - etag=data.get( - 'ETag', - '')) + ip_acl, + etag=data.get('ETag', '')) def __overwrite_volume_acl( self, project_name, lightos_volname, - acl): + acl, + host_ips): status, data = self._get_lightos_volume(project_name, self.logical_op_timeout, vol_name=lightos_volname) @@ -1170,11 +1261,12 @@ class LightOSVolumeDriver(driver.VolumeDriver): project_name, lightos_uuid, acl, + host_ips, etag=data.get( 'ETag', '')) - def remove_volume_acl(self, project_name, volume, acl_to_remove): + def remove_volume_acl(self, project_name, volume, acl_to_remove, host_ips): lightos_volname = self._lightos_volname(volume) LOG.debug('remove_volume_acl volume %s project %s acl %s', volume, project_name, acl_to_remove) @@ -1182,7 +1274,8 @@ class LightOSVolumeDriver(driver.VolumeDriver): self.__remove_volume_acl, project_name, lightos_volname, - acl_to_remove) + acl_to_remove, + host_ips) def remove_all_volume_acls(self, project_name, volume): lightos_volname = self._lightos_volname(volume) @@ -1192,9 +1285,11 @@ class LightOSVolumeDriver(driver.VolumeDriver): self.__overwrite_volume_acl, project_name, lightos_volname, + ['ALLOW_NONE'], ['ALLOW_NONE']) - def update_volume_acl(self, func, project_name, lightos_volname, acl): + def update_volume_acl(self, func, project_name, lightos_volname, acl, + host_ips): # loop because lightos api is async end = time.time() + self.logical_op_timeout first_iteration = True @@ -1202,7 +1297,7 @@ class LightOSVolumeDriver(driver.VolumeDriver): if not first_iteration: time.sleep(1) first_iteration = False - res = func(project_name, lightos_volname, acl) + res = func(project_name, lightos_volname, acl, host_ips) if not isinstance(res, tuple): LOG.debug('Update_volume: func %s(%s project %s) failed', func, lightos_volname, project_name) @@ -1410,6 +1505,10 @@ class LightOSVolumeDriver(driver.VolumeDriver): def initialize_connection(self, volume, connector): hostnqn = connector.get('nqn') found_dsc = connector.get('found_dsc') + host_ips = connector.get('host_ips', []) + LOG.info('Current host hostNQN is %s and IP(s) are %s', + hostnqn, + host_ips) LOG.debug( 'initialize_connection: connector hostnqn is %s found_dsc %s', hostnqn, @@ -1424,9 +1523,14 @@ class LightOSVolumeDriver(driver.VolumeDriver): 'client, aborting' % (connector)) raise exception.VolumeBackendAPIException(message=_(msg)) + if not host_ips: + msg = 'Connector (%s) did not find host IPs, aborting' % ( + connector) + raise exception.VolumeBackendAPIException(message=_(msg)) + lightos_volname = self._lightos_volname(volume) project_name = self._get_lightos_project_name(volume) - success = self.add_volume_acl(project_name, volume, hostnqn) + success = self.add_volume_acl(project_name, volume, hostnqn, host_ips) if not success or not self._wait_for_volume_acl( project_name, lightos_volname, hostnqn, True): msg = ('Could not add ACL for hostnqn %s LightOS volume' @@ -1439,6 +1543,7 @@ class LightOSVolumeDriver(driver.VolumeDriver): def terminate_connection(self, volume, connector, **kwargs): force = 'force' in kwargs hostnqn = connector.get('nqn') if connector else None + host_ips = connector.get('host_ips', []) if connector else [] LOG.debug( 'terminate_connection: force %s kwargs %s hostnqn %s', force, @@ -1462,7 +1567,8 @@ class LightOSVolumeDriver(driver.VolumeDriver): lightos_volname = self._lightos_volname(volume) project_name = self._get_lightos_project_name(volume) - success = self.remove_volume_acl(project_name, volume, hostnqn) + success = self.remove_volume_acl(project_name, volume, hostnqn, + host_ips) if not success or not self._wait_for_volume_acl( project_name, lightos_volname, hostnqn, False): LOG.warning( diff --git a/releasenotes/notes/lightbits-volume-ipacl-23da3aa469689817.yaml b/releasenotes/notes/lightbits-volume-ipacl-23da3aa469689817.yaml new file mode 100644 index 00000000000..de29631e230 --- /dev/null +++ b/releasenotes/notes/lightbits-volume-ipacl-23da3aa469689817.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Lightbits driver: Added a new configuration option + ``lightos_use_ipacl``, defaulting to true. When set to true, the + Cinder driver will restrict access to each volume to the IP + addresses of the host machine that the volume is attached to.