diff -Nru cinder-21.1.0/api-ref/source/v2/parameters.yaml cinder-21.3.1/api-ref/source/v2/parameters.yaml --- cinder-21.1.0/api-ref/source/v2/parameters.yaml 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/api-ref/source/v2/parameters.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -1081,15 +1081,6 @@ in: body required: true type: string -multiattach_req: - description: | - To enable this volume to attach to more than one - server, set this value to ``true``. Default is ``false``. - Note that support for multiattach volumes depends on the volume - type being used. - in: body - required: false - type: boolean multiattach_resp: description: | If true, this volume can attach to more than one diff -Nru cinder-21.1.0/api-ref/source/v2/volumes-v2-volumes.inc cinder-21.3.1/api-ref/source/v2/volumes-v2-volumes.inc --- cinder-21.1.0/api-ref/source/v2/volumes-v2-volumes.inc 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/api-ref/source/v2/volumes-v2-volumes.inc 2023-08-14 16:54:18.000000000 +0000 @@ -183,7 +183,6 @@ - size: size - description: description_9 - imageRef: imageRef - - multiattach: multiattach_req - availability_zone: availability_zone - source_volid: source_volid - name: volume_name_optional diff -Nru cinder-21.1.0/api-ref/source/v3/attachments.inc cinder-21.3.1/api-ref/source/v3/attachments.inc --- cinder-21.1.0/api-ref/source/v3/attachments.inc 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/api-ref/source/v3/attachments.inc 2023-08-14 16:54:18.000000000 +0000 @@ -41,6 +41,20 @@ Deletes an attachment. +For security reasons (see bug `#2004555 +`_) the Block Storage API rejects +REST API calls manually made from users with a 409 status code if there is a +Nova instance currently using the attachment, which happens when all the +following conditions are met: + +- Attachment has an instance uuid +- VM exists in Nova +- Instance has the volume attached +- Attached volume in instance is using the attachment + +Calls coming from other OpenStack services (like the Compute Service) are +always accepted. + Available starting in the 3.27 microversion. Response codes @@ -54,6 +68,7 @@ - 400 - 404 + - 409 Request diff -Nru cinder-21.1.0/api-ref/source/v3/parameters.yaml cinder-21.3.1/api-ref/source/v3/parameters.yaml --- cinder-21.1.0/api-ref/source/v3/parameters.yaml 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/api-ref/source/v3/parameters.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -1915,15 +1915,6 @@ in: body required: true type: string -multiattach_req: - description: | - To enable this volume to attach to more than one - server, set this value to ``true``. Default is ``false``. - Note that support for multiattach volumes depends on the volume - type being used. See :ref:`valid boolean values ` - in: body - required: false - type: boolean multiattach_resp: description: | If true, this volume can attach to more than one diff -Nru cinder-21.1.0/api-ref/source/v3/volumes-v3-volumes-actions.inc cinder-21.3.1/api-ref/source/v3/volumes-v3-volumes-actions.inc --- cinder-21.1.0/api-ref/source/v3/volumes-v3-volumes-actions.inc 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/api-ref/source/v3/volumes-v3-volumes-actions.inc 2023-08-14 16:54:18.000000000 +0000 @@ -337,6 +337,21 @@ - Volume status must be ``in-use``. +For security reasons (see bug `#2004555 +`_), regardless of the policy +defaults, the Block Storage API rejects REST API calls manually made from +users with a 409 status code if completing the request could pose a risk, which +happens if all of these happen: + +- The request comes from a user +- There's an instance uuid in provided attachment or in the volume's attachment +- VM exists in Nova +- Instance has the volume attached +- Attached volume in instance is using the attachment + +Calls coming from other OpenStack services (like the Compute Service) are +always accepted. + Response codes -------------- @@ -344,6 +359,9 @@ - 202 +.. rest_status_code:: error ../status.yaml + + - 409 Request ------- @@ -415,6 +433,21 @@ through the ``volume_extension:volume_admin_actions:force_detach`` rule in the policy configuration file. +For security reasons (see bug `#2004555 +`_), regardless of the policy +defaults, the Block Storage API rejects REST API calls manually made from +users with a 409 status code if completing the request could pose a risk, which +happens if all of these happen: + +- The request comes from a user +- There's an instance uuid in provided attachment or in the volume's attachment +- VM exists in Nova +- Instance has the volume attached +- Attached volume in instance is using the attachment + +Calls coming from other OpenStack services (like the Compute Service) are +always accepted. + Response codes -------------- @@ -422,6 +455,9 @@ - 202 +.. rest_status_code:: error ../status.yaml + + - 409 Request ------- @@ -883,6 +919,22 @@ - Volume status must be ``in-use``. +For security reasons (see bug `#2004555 +`_), regardless of the policy +defaults, the Block Storage API rejects REST API calls manually made from +users with a 409 status code if completing the request could pose a risk, which +happens if all of these happen: + +- The request comes from a user +- There's an instance uuid in the volume's attachment +- VM exists in Nova +- Instance has the volume attached +- Attached volume in instance is using the attachment + +Calls coming from other OpenStack services (like the Compute Service) are +always accepted. + + Response codes -------------- @@ -890,6 +942,9 @@ - 202 +.. rest_status_code:: error ../status.yaml + + - 409 Request ------- diff -Nru cinder-21.1.0/api-ref/source/v3/volumes-v3-volumes.inc cinder-21.3.1/api-ref/source/v3/volumes-v3-volumes.inc --- cinder-21.1.0/api-ref/source/v3/volumes-v3-volumes.inc 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/api-ref/source/v3/volumes-v3-volumes.inc 2023-08-14 16:54:18.000000000 +0000 @@ -218,7 +218,6 @@ - availability_zone: availability_zone - source_volid: source_volid - description: description_vol - - multiattach: multiattach_req - snapshot_id: snapshot_id - backup_id: backup_id - name: volume_name_optional diff -Nru cinder-21.1.0/cinder/api/schemas/volumes.py cinder-21.3.1/cinder/api/schemas/volumes.py --- cinder-21.1.0/cinder/api/schemas/volumes.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/api/schemas/volumes.py 2023-08-14 16:54:18.000000000 +0000 @@ -49,6 +49,12 @@ 'consistencygroup_id': parameter_types.optional_uuid, 'size': parameter_types.volume_size_allows_null, 'availability_zone': parameter_types.availability_zone, + # The functionality to create a multiattach volume by the + # multiattach parameter is removed. + # We accept the parameter but raise a BadRequest stating the + # "new way" of creating multiattach volumes i.e. with a + # multiattach volume type so users using the "old way" + # have ease of moving into the new functionality. 'multiattach': parameter_types.optional_boolean, 'image_id': {'type': ['string', 'null'], 'minLength': 0, 'maxLength': 255}, diff -Nru cinder-21.1.0/cinder/api/v2/volumes.py cinder-21.3.1/cinder/api/v2/volumes.py --- cinder-21.1.0/cinder/api/v2/volumes.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/api/v2/volumes.py 2023-08-14 16:54:18.000000000 +0000 @@ -19,7 +19,6 @@ from oslo_config import cfg from oslo_log import log as logging -from oslo_log import versionutils from oslo_utils import uuidutils import webob from webob import exc @@ -261,14 +260,6 @@ kwargs['availability_zone'] = volume.get('availability_zone', None) kwargs['scheduler_hints'] = volume.get('scheduler_hints', None) - kwargs['multiattach'] = utils.get_bool_param('multiattach', volume) - - if kwargs.get('multiattach', False): - msg = ("The option 'multiattach' " - "is deprecated and will be removed in a future " - "release. The default behavior going forward will " - "be to specify multiattach enabled volume types.") - versionutils.report_deprecated_feature(LOG, msg) try: new_volume = self.volume_api.create( diff -Nru cinder-21.1.0/cinder/api/v3/volumes.py cinder-21.3.1/cinder/api/v3/volumes.py --- cinder-21.1.0/cinder/api/v3/volumes.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/api/v3/volumes.py 2023-08-14 16:54:18.000000000 +0000 @@ -15,7 +15,6 @@ from http import HTTPStatus from oslo_log import log as logging -from oslo_log import versionutils from oslo_utils import timeutils import webob from webob import exc @@ -387,15 +386,14 @@ kwargs['availability_zone'] = volume.get('availability_zone', None) kwargs['scheduler_hints'] = volume.get('scheduler_hints', None) - multiattach = volume.get('multiattach', False) - kwargs['multiattach'] = multiattach - + multiattach = utils.get_bool_param('multiattach', volume) if multiattach: - msg = ("The option 'multiattach' " - "is deprecated and will be removed in a future " - "release. The default behavior going forward will " - "be to specify multiattach enabled volume types.") - versionutils.report_deprecated_feature(LOG, msg) + msg = _("multiattach parameter has been removed. The default " + "behavior is to use multiattach enabled volume types. " + "Contact your administrator to create a multiattach " + "enabled volume type and use it to create multiattach " + "volumes.") + raise exc.HTTPBadRequest(explanation=msg) try: new_volume = self.volume_api.create( context, size, volume.get('display_name'), diff -Nru cinder-21.1.0/cinder/backup/manager.py cinder-21.3.1/cinder/backup/manager.py --- cinder-21.1.0/cinder/backup/manager.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/backup/manager.py 2023-08-14 16:54:18.000000000 +0000 @@ -416,10 +416,15 @@ snapshot.status = fields.SnapshotStatus.AVAILABLE snapshot.save() else: - self.db.volume_update( - context, volume_id, - {'status': previous_status, - 'previous_status': 'error_backing-up'}) + try: + self.db.volume_update( + context, volume_id, + {'status': previous_status, + 'previous_status': 'error_backing-up'}) + except exception.VolumeNotFound: + # If the volume was deleted we cannot update its + # status but we still want to set the backup to error. + pass volume_utils.update_backup_error(backup, str(err)) def _start_backup(self, context, backup, volume): diff -Nru cinder-21.1.0/cinder/compute/nova.py cinder-21.3.1/cinder/compute/nova.py --- cinder-21.1.0/cinder/compute/nova.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/compute/nova.py 2023-08-14 16:54:18.000000000 +0000 @@ -133,6 +133,7 @@ class API(base.Base): """API for interacting with novaclient.""" + NotFound = nova_exceptions.NotFound def __init__(self): self.message_api = message_api.API() @@ -237,3 +238,9 @@ resource_uuid=volume_id, detail=message_field.Detail.REIMAGE_VOLUME_FAILED) return result + + @staticmethod + def get_server_volume(context, server_id, volume_id): + # Use microversion that includes attachment_id + nova = novaclient(context, api_version='2.89') + return nova.volumes.get_server_volume(server_id, volume_id) diff -Nru cinder-21.1.0/cinder/exception.py cinder-21.3.1/cinder/exception.py --- cinder-21.1.0/cinder/exception.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/exception.py 2023-08-14 16:54:18.000000000 +0000 @@ -1092,3 +1092,10 @@ "Driver initiator data for initiator '%(initiator)s' and backend " "'%(namespace)s' with key '%(key)s' already exists." ) + + +class ConflictNovaUsingAttachment(CinderException): + message = _("Detach volume from instance %(instance_id)s using the " + "Compute API") + code = 409 + safe = True diff -Nru cinder-21.1.0/cinder/image/image_utils.py cinder-21.3.1/cinder/image/image_utils.py --- cinder-21.1.0/cinder/image/image_utils.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/image/image_utils.py 2023-08-14 16:54:18.000000000 +0000 @@ -89,6 +89,24 @@ 'when an image is converted to raw format as it is written ' 'to a volume. If this list is empty, no VMDK images are ' 'allowed.'), + cfg.ListOpt('reserved_image_namespaces', + help='List of reserved image namespaces that should be ' + 'filtered out when uploading a volume as an image back ' + 'to Glance. When a volume is created from an image, ' + 'Cinder stores the image properties as volume ' + 'image metadata, and if the volume is later uploaded as ' + 'an image, Cinder will add these properties when it ' + 'creates the image in Glance. This can cause problems ' + 'for image metadata that are in namespaces that glance ' + 'reserves for itself, or when properties (such as an ' + 'image signature) cannot apply to the new image, or when ' + 'an operator has configured glance property protections ' + 'to make some image properties read-only. Cinder will ' + '*always* filter out image metadata in the namespaces ' + '`os_glance` and `img_signature`; this configuration ' + 'option allows operators to specify *additional* ' + 'namespaces to be excluded.', + default=[]), ] CONF = cfg.CONF @@ -113,6 +131,8 @@ COMPRESSIBLE_IMAGE_FORMATS = ('qcow2',) +GLANCE_RESERVED_NAMESPACES = ["os_glance", "img_signature"] + def validate_stores_id(context: context.RequestContext, image_service_store_id: str) -> None: @@ -1284,3 +1304,39 @@ if not self.temporary_images.get(user): return None return self.temporary_images[user].get(image_id) + + +def _filter_out_metadata(metadata, filter_keys): + new_metadata = {} + for k, v in metadata.items(): + if any(k.startswith(filter_key) + for filter_key in filter_keys): + continue + new_metadata[k] = v + return new_metadata + + +def filter_out_reserved_namespaces_metadata( + metadata: Optional[dict[str, str]]) -> dict[str, str]: + + reserved_name_spaces = GLANCE_RESERVED_NAMESPACES.copy() + if CONF.reserved_image_namespaces: + for image_namespace in CONF.reserved_image_namespaces: + if image_namespace not in reserved_name_spaces: + reserved_name_spaces.append(image_namespace) + + if not metadata: + LOG.debug("No metadata to be filtered.") + return {} + + new_metadata = _filter_out_metadata(metadata, reserved_name_spaces) + # NOTE(ganso): handle adjustment of metadata structure performed by + # the cinder.volume.api.API._merge_volume_image_meta() method + if 'properties' in new_metadata: + new_metadata['properties'] = _filter_out_metadata( + metadata['properties'], reserved_name_spaces) + + LOG.debug("The metadata set [%s] was filtered using the reserved name " + "spaces [%s], and the result is [%s].", metadata, + reserved_name_spaces, new_metadata) + return new_metadata diff -Nru cinder-21.1.0/cinder/scheduler/filter_scheduler.py cinder-21.3.1/cinder/scheduler/filter_scheduler.py --- cinder-21.1.0/cinder/scheduler/filter_scheduler.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/scheduler/filter_scheduler.py 2023-08-14 16:54:18.000000000 +0000 @@ -335,19 +335,6 @@ self.populate_filter_properties(request_spec, filter_properties) - # If multiattach is enabled on a volume, we need to add - # multiattach to extra specs, so that the capability - # filtering is enabled. - multiattach = request_spec['volume_properties'].get('multiattach', - False) - if multiattach and 'multiattach' not in resource_type.get( - 'extra_specs', {}): - if 'extra_specs' not in resource_type: - resource_type['extra_specs'] = {} - - resource_type['extra_specs'].update( - multiattach=' True') - # Revert volume consumed capacity if it's a rescheduled request retry = filter_properties.get('retry', {}) if retry.get('backends', []): diff -Nru cinder-21.1.0/cinder/tests/unit/api/contrib/test_admin_actions.py cinder-21.3.1/cinder/tests/unit/api/contrib/test_admin_actions.py --- cinder-21.1.0/cinder/tests/unit/api/contrib/test_admin_actions.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/api/contrib/test_admin_actions.py 2023-08-14 16:54:18.000000000 +0000 @@ -1037,6 +1037,8 @@ super(AdminActionsAttachDetachTest, self).setUp() # start service to handle rpc messages for attach requests self.svc = self.start_service('volume', host='test') + self.mock_deletion_allowed = self.mock_object( + volume_api.API, 'attachment_deletion_allowed', return_value=None) def tearDown(self): self.svc.stop() @@ -1092,6 +1094,16 @@ admin_metadata = volume.admin_metadata self.assertEqual(1, len(admin_metadata)) self.assertEqual('False', admin_metadata['readonly']) + # One call is for the terminate_connection and the other is for the + # detach + self.assertEqual(2, self.mock_deletion_allowed.call_count) + self.mock_deletion_allowed.assert_has_calls( + [mock.call(self.ctx, None, mock.ANY), + mock.call(self.ctx, attachment['id'], mock.ANY)]) + for i in (0, 1): + self.assertIsInstance( + self.mock_deletion_allowed.call_args_list[i][0][2], + objects.Volume) def test_force_detach_host_attached_volume(self): # current status is available @@ -1143,6 +1155,16 @@ admin_metadata = volume['admin_metadata'] self.assertEqual(1, len(admin_metadata)) self.assertEqual('False', admin_metadata['readonly']) + # One call is for the terminate_connection and the other is for the + # detach + self.assertEqual(2, self.mock_deletion_allowed.call_count) + self.mock_deletion_allowed.assert_has_calls( + [mock.call(self.ctx, None, mock.ANY), + mock.call(self.ctx, attachment['id'], mock.ANY)]) + for i in (0, 1): + self.assertIsInstance( + self.mock_deletion_allowed.call_args_list[i][0][2], + objects.Volume) def test_volume_force_detach_raises_remote_error(self): # current status is available @@ -1186,6 +1208,10 @@ resp = req.get_response(app()) self.assertEqual(HTTPStatus.BAD_REQUEST, resp.status_int) + self.mock_deletion_allowed.assert_called_once_with( + self.ctx, None, volume) + self.mock_deletion_allowed.reset_mock() + # test for VolumeBackendAPIException volume_remote_error = ( messaging.RemoteError(exc_type='VolumeBackendAPIException')) @@ -1205,6 +1231,8 @@ self.assertRaises(messaging.RemoteError, req.get_response, app()) + self.mock_deletion_allowed.assert_called_once_with( + self.ctx, None, volume) def test_volume_force_detach_raises_db_error(self): # In case of DB error 500 error code is returned to user @@ -1250,6 +1278,8 @@ self.assertRaises(messaging.RemoteError, req.get_response, app()) + self.mock_deletion_allowed.assert_called_once_with( + self.ctx, None, volume) def test_volume_force_detach_missing_connector(self): # current status is available @@ -1290,6 +1320,8 @@ # make request resp = req.get_response(app()) self.assertEqual(HTTPStatus.ACCEPTED, resp.status_int) + self.mock_deletion_allowed.assert_called_once_with( + self.ctx, None, volume) def test_attach_in_used_volume_by_instance(self): """Test that attaching to an in-use volume fails.""" diff -Nru cinder-21.1.0/cinder/tests/unit/api/v2/test_volumes.py cinder-21.3.1/cinder/tests/unit/api/v2/test_volumes.py --- cinder-21.1.0/cinder/tests/unit/api/v2/test_volumes.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/api/v2/test_volumes.py 2023-08-14 16:54:18.000000000 +0000 @@ -158,8 +158,7 @@ consistencygroup_id=None, volume_type=None, image_ref=None, - image_id=None, - multiattach=False): + image_id=None): vol = {"size": size, "name": name, "description": description, @@ -168,7 +167,6 @@ "source_volid": source_volid, "consistencygroup_id": consistencygroup_id, "volume_type": volume_type, - "multiattach": multiattach, } if image_id is not None: @@ -240,7 +238,6 @@ 'consistencygroup': None, 'availability_zone': availability_zone, 'scheduler_hints': None, - 'multiattach': False, } @mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full', @@ -570,37 +567,6 @@ req, body=body) - def test_volume_create_with_invalid_multiattach(self): - vol = self._vol_in_request_body(multiattach="InvalidBool") - body = {"volume": vol} - req = fakes.HTTPRequest.blank('/v3/volumes') - - self.assertRaises(exception.ValidationError, - self.controller.create, - req, - body=body) - - @mock.patch.object(volume_api.API, 'create', autospec=True) - @mock.patch.object(volume_api.API, 'get', autospec=True) - @mock.patch.object(db.sqlalchemy.api, '_volume_type_get_full', - autospec=True) - def test_volume_create_with_valid_multiattach(self, - volume_type_get, - get, create): - create.side_effect = v2_fakes.fake_volume_api_create - get.side_effect = v2_fakes.fake_volume_get - volume_type_get.side_effect = v2_fakes.fake_volume_type_get - - vol = self._vol_in_request_body(multiattach=True) - body = {"volume": vol} - - ex = self._expected_vol_from_controller(multiattach=True) - - req = fakes.HTTPRequest.blank('/v3/volumes') - res_dict = self.controller.create(req, body=body) - - self.assertEqual(ex, res_dict) - @ddt.data({'a' * 256: 'a'}, {'a': 'a' * 256}, {'': 'a'}, diff -Nru cinder-21.1.0/cinder/tests/unit/api/v3/test_attachments.py cinder-21.3.1/cinder/tests/unit/api/v3/test_attachments.py --- cinder-21.1.0/cinder/tests/unit/api/v3/test_attachments.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/api/v3/test_attachments.py 2023-08-14 16:54:18.000000000 +0000 @@ -159,6 +159,8 @@ @ddt.data('reserved', 'attached') @mock.patch.object(volume_rpcapi.VolumeAPI, 'attachment_delete') def test_delete_attachment(self, status, mock_delete): + self.patch('cinder.volume.api.API.attachment_deletion_allowed', + return_value=None) volume1 = self._create_volume(display_name='fake_volume_1', project_id=fake.PROJECT_ID) attachment = self._create_attachment( diff -Nru cinder-21.1.0/cinder/tests/unit/api/v3/test_volumes.py cinder-21.3.1/cinder/tests/unit/api/v3/test_volumes.py --- cinder-21.1.0/cinder/tests/unit/api/v3/test_volumes.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/api/v3/test_volumes.py 2023-08-14 16:54:18.000000000 +0000 @@ -619,7 +619,6 @@ 'consistencygroup': None, 'availability_zone': availability_zone, 'scheduler_hints': None, - 'multiattach': False, 'group': test_group, } @@ -1189,3 +1188,25 @@ volumes = res_dict['volumes'] self.assertEqual(1, len(volumes)) self.assertEqual(vols[1].id, volumes[0]['id']) + + def test_create_volume_with_multiattach_param(self): + """Tests creating a volume with multiattach=True but no multiattach + + volume type. + + This test verifies that providing the multiattach parameter will error + out the request since it is removed and the recommended way is to + create a multiattach volume using a multiattach volume type. + """ + req = fakes.HTTPRequest.blank('/v3/volumes') + + body = {'volume': { + 'name': 'test name', + 'description': 'test desc', + 'size': 1, + 'multiattach': True}} + exc = self.assertRaises(webob.exc.HTTPBadRequest, + self.controller.create, + req, body=body) + self.assertIn("multiattach parameter has been removed", + exc.explanation) diff -Nru cinder-21.1.0/cinder/tests/unit/attachments/test_attachments_api.py cinder-21.3.1/cinder/tests/unit/attachments/test_attachments_api.py --- cinder-21.1.0/cinder/tests/unit/attachments/test_attachments_api.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/attachments/test_attachments_api.py 2023-08-14 16:54:18.000000000 +0000 @@ -12,12 +12,14 @@ from unittest import mock +from cinder.compute import nova from cinder import context from cinder import db from cinder import exception from cinder import objects from cinder.tests.unit.api.v2 import fakes as v2_fakes from cinder.tests.unit import fake_constants as fake +from cinder.tests.unit import fake_volume from cinder.tests.unit import test from cinder.tests.unit import utils as tests_utils from cinder.volume import api as volume_api @@ -78,10 +80,13 @@ attachment.id) self.assertEqual(connection_info, new_attachment.connection_info) + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete') def test_attachment_delete_reserved(self, - mock_rpc_attachment_delete): + mock_rpc_attachment_delete, + mock_allowed): """Test attachment_delete with reserved.""" + mock_allowed.return_value = None volume_params = {'status': 'available'} vref = tests_utils.create_volume(self.context, **volume_params) @@ -94,18 +99,22 @@ self.assertEqual(vref.id, aref.volume_id) self.volume_api.attachment_delete(self.context, aobj) + mock_allowed.assert_called_once_with(self.context, aobj) # Since it's just reserved and never finalized, we should never make an # rpc call mock_rpc_attachment_delete.assert_not_called() + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete') @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update') def test_attachment_create_update_and_delete( self, mock_rpc_attachment_update, - mock_rpc_attachment_delete): + mock_rpc_attachment_delete, + mock_allowed): """Test attachment_delete.""" + mock_allowed.return_value = None volume_params = {'status': 'available'} connection_info = {'fake_key': 'fake_value', 'fake_key2': ['fake_value1', 'fake_value2']} @@ -142,6 +151,7 @@ self.volume_api.attachment_delete(self.context, aref) + mock_allowed.assert_called_once_with(self.context, aref) mock_rpc_attachment_delete.assert_called_once_with(self.context, aref.id, mock.ANY) @@ -173,10 +183,13 @@ vref.id) self.assertEqual(2, len(vref.volume_attachment)) + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update') def test_attachment_create_reserve_delete( self, - mock_rpc_attachment_update): + mock_rpc_attachment_update, + mock_allowed): + mock_allowed.return_value = None volume_params = {'status': 'available'} connector = { "initiator": "iqn.1993-08.org.debian:01:cad181614cec", @@ -211,12 +224,15 @@ # attachments reserve self.volume_api.attachment_delete(self.context, aref) + mock_allowed.assert_called_once_with(self.context, aref) vref = objects.Volume.get_by_id(self.context, vref.id) self.assertEqual('reserved', vref.status) - def test_reserve_reserve_delete(self): + @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') + def test_reserve_reserve_delete(self, mock_allowed): """Test that we keep reserved status across multiple reserves.""" + mock_allowed.return_value = None volume_params = {'status': 'available'} vref = tests_utils.create_volume(self.context, **volume_params) @@ -235,6 +251,7 @@ self.assertEqual('reserved', vref.status) self.volume_api.attachment_delete(self.context, aref) + mock_allowed.assert_called_once_with(self.context, aref) vref = objects.Volume.get_by_id(self.context, vref.id) self.assertEqual('reserved', vref.status) @@ -344,3 +361,169 @@ self.context, vref, fake.UUID1) + + def _get_attachment(self, with_instance_id=True): + volume = fake_volume.fake_volume_obj(self.context, id=fake.VOLUME_ID) + volume.volume_attachment = objects.VolumeAttachmentList() + attachment = fake_volume.volume_attachment_ovo( + self.context, + volume_id=fake.VOLUME_ID, + instance_uuid=fake.INSTANCE_ID if with_instance_id else None, + connection_info='{"a": 1}') + attachment.volume = volume + return attachment + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_service_call(self, mock_get_server): + """Service calls are never redirected.""" + self.context.service_roles = ['reader', 'service'] + attachment = self._get_attachment() + self.volume_api.attachment_deletion_allowed(self.context, attachment) + mock_get_server.assert_not_called() + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_service_call_different_service_name( + self, mock_get_server): + """Service calls are never redirected and role can be different. + + In this test we support 2 different service roles, the standard service + and a custom one called captain_awesome, and passing the custom one + works as expected. + """ + self.override_config('service_token_roles', + ['service', 'captain_awesome'], + group='keystone_authtoken') + + self.context.service_roles = ['reader', 'captain_awesome'] + attachment = self._get_attachment() + self.volume_api.attachment_deletion_allowed(self.context, attachment) + mock_get_server.assert_not_called() + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_no_instance(self, mock_get_server): + """Attachments with no instance id are never redirected.""" + attachment = self._get_attachment(with_instance_id=False) + self.volume_api.attachment_deletion_allowed(self.context, attachment) + mock_get_server.assert_not_called() + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_no_conn_info(self, mock_get_server): + """Attachments with no connection information are never redirected.""" + attachment = self._get_attachment(with_instance_id=False) + attachment.connection_info = None + self.volume_api.attachment_deletion_allowed(self.context, attachment) + + mock_get_server.assert_not_called() + + def test_attachment_deletion_allowed_no_attachment(self): + """For users don't allow operation with no attachment reference.""" + self.assertRaises(exception.ConflictNovaUsingAttachment, + self.volume_api.attachment_deletion_allowed, + self.context, None) + + @mock.patch('cinder.objects.VolumeAttachment.get_by_id', + side_effect=exception.VolumeAttachmentNotFound()) + def test_attachment_deletion_allowed_attachment_id_not_found(self, + mock_get): + """For users don't allow if attachment cannot be found.""" + attachment = self._get_attachment(with_instance_id=False) + attachment.connection_info = None + self.assertRaises(exception.ConflictNovaUsingAttachment, + self.volume_api.attachment_deletion_allowed, + self.context, fake.ATTACHMENT_ID) + mock_get.assert_called_once_with(self.context, fake.ATTACHMENT_ID) + + def test_attachment_deletion_allowed_volume_no_attachments(self): + """For users allow if volume has no attachments.""" + volume = tests_utils.create_volume(self.context) + self.volume_api.attachment_deletion_allowed(self.context, None, volume) + + def test_attachment_deletion_allowed_multiple_attachment(self): + """For users don't allow if volume has multiple attachments.""" + attachment = self._get_attachment() + volume = attachment.volume + volume.volume_attachment = objects.VolumeAttachmentList( + objects=[attachment, attachment]) + self.assertRaises(exception.ConflictNovaUsingAttachment, + self.volume_api.attachment_deletion_allowed, + self.context, None, volume) + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_vm_not_found(self, mock_get_server): + """Don't reject if instance doesn't exist""" + mock_get_server.side_effect = nova.API.NotFound(404) + attachment = self._get_attachment() + self.volume_api.attachment_deletion_allowed(self.context, attachment) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + fake.VOLUME_ID) + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_attachment_from_volume( + self, mock_get_server): + """Don't reject if instance doesn't exist""" + mock_get_server.side_effect = nova.API.NotFound(404) + attachment = self._get_attachment() + volume = attachment.volume + volume.volume_attachment = objects.VolumeAttachmentList( + objects=[attachment]) + self.volume_api.attachment_deletion_allowed(self.context, None, volume) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + volume.id) + + @mock.patch('cinder.objects.VolumeAttachment.get_by_id') + def test_attachment_deletion_allowed_mismatched_volume_and_attach_id( + self, mock_get_attatchment): + """Reject if volume and attachment don't match.""" + attachment = self._get_attachment() + volume = attachment.volume + volume.volume_attachment = objects.VolumeAttachmentList( + objects=[attachment]) + attachment2 = self._get_attachment() + attachment2.volume_id = attachment.volume.id = fake.VOLUME2_ID + self.assertRaises(exception.InvalidInput, + self.volume_api.attachment_deletion_allowed, + self.context, attachment2.id, volume) + mock_get_attatchment.assert_called_once_with(self.context, + attachment2.id) + + @mock.patch('cinder.objects.VolumeAttachment.get_by_id') + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_not_found_attachment_id( + self, mock_get_server, mock_get_attachment): + """Don't reject if instance doesn't exist""" + mock_get_server.side_effect = nova.API.NotFound(404) + mock_get_attachment.return_value = self._get_attachment() + + self.volume_api.attachment_deletion_allowed(self.context, + fake.ATTACHMENT_ID) + + mock_get_attachment.assert_called_once_with(self.context, + fake.ATTACHMENT_ID) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + fake.VOLUME_ID) + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_mismatch_id(self, mock_get_server): + """Don't reject if attachment id on nova doesn't match""" + mock_get_server.return_value.attachment_id = fake.ATTACHMENT2_ID + attachment = self._get_attachment() + self.volume_api.attachment_deletion_allowed(self.context, attachment) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + fake.VOLUME_ID) + + @mock.patch('cinder.compute.nova.API.get_server_volume') + def test_attachment_deletion_allowed_user_call_fails(self, + mock_get_server): + """Fail user calls""" + attachment = self._get_attachment() + mock_get_server.return_value.attachment_id = attachment.id + self.assertRaises(exception.ConflictNovaUsingAttachment, + self.volume_api.attachment_deletion_allowed, + self.context, attachment) + + mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, + fake.VOLUME_ID) diff -Nru cinder-21.1.0/cinder/tests/unit/backup/test_backup.py cinder-21.3.1/cinder/tests/unit/backup/test_backup.py --- cinder-21.1.0/cinder/tests/unit/backup/test_backup.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/backup/test_backup.py 2023-08-14 16:54:18.000000000 +0000 @@ -675,6 +675,26 @@ @mock.patch('cinder.backup.manager.BackupManager._start_backup', side_effect=FakeBackupException(str(uuid.uuid4()))) + @mock.patch.object(db, 'volume_update') + def test_create_backup_aborted_volume_not_found(self, vol_up_mock, + start_backup_mock): + """Test error handling when backup fails and volume does not exist.""" + vol_id = self._create_volume_db_entry(size=1) + backup = self._create_backup_db_entry(volume_id=vol_id) + + vol_up_mock.side_effect = exception.VolumeNotFound(volume_id=vol_id) + + self.assertRaises(FakeBackupException, + self.backup_mgr.create_backup, + self.ctxt, + backup) + backup.refresh() + self.assertEqual(fields.BackupStatus.ERROR, backup.status) + self.assertTrue(start_backup_mock.called) + self.assertTrue(vol_up_mock.called) + + @mock.patch('cinder.backup.manager.BackupManager._start_backup', + side_effect=FakeBackupException(str(uuid.uuid4()))) def test_create_backup_with_snapshot_error(self, mock_start_backup): """Test error handling when error occurs during backup creation.""" vol_id = self._create_volume_db_entry(size=1) diff -Nru cinder-21.1.0/cinder/tests/unit/policies/test_attachments.py cinder-21.3.1/cinder/tests/unit/policies/test_attachments.py --- cinder-21.1.0/cinder/tests/unit/policies/test_attachments.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/policies/test_attachments.py 2023-08-14 16:54:18.000000000 +0000 @@ -62,6 +62,9 @@ self.api_path = '/v3/%s/attachments' % (self.project_id) self.api_version = mv.NEW_ATTACH + self.mock_is_service = self.patch( + 'cinder.volume.api.API.is_service_request', + return_value=True) def _initialize_connection(self, volume, connector): return {'data': connector} diff -Nru cinder-21.1.0/cinder/tests/unit/policies/test_volume_actions.py cinder-21.3.1/cinder/tests/unit/policies/test_volume_actions.py --- cinder-21.1.0/cinder/tests/unit/policies/test_volume_actions.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/policies/test_volume_actions.py 2023-08-14 16:54:18.000000000 +0000 @@ -89,6 +89,9 @@ self._initialize_connection) self.api_path = '/v3/%s/volumes' % (self.project_id) self.api_version = mv.BASE_VERSION + self.mock_is_service = self.patch( + 'cinder.volume.api.API.is_service_request', + return_value=True) def _initialize_connection(self, volume, connector): return {'data': connector} @@ -946,6 +949,7 @@ self.assertEqual(HTTPStatus.ACCEPTED, response.status_int) body = {"os-detach": {}} + # Detach for user call succeeds because the volume has no attachments response = self._get_request_response(admin_context, path, 'POST', body=body) self.assertEqual(HTTPStatus.ACCEPTED, response.status_int) @@ -966,6 +970,7 @@ body=body) self.assertEqual(HTTPStatus.ACCEPTED, response.status_int) + # Succeeds for a user call because there are no attachments body = {"os-detach": {}} response = self._get_request_response(user_context, path, 'POST', body=body) @@ -1062,6 +1067,7 @@ 'terminate_connection') def test_admin_can_initialize_terminate_conn(self, mock_t, mock_i): admin_context = self.admin_context + admin_context.service_roles = ['service'] volume = self._create_fake_volume(admin_context) path = '/v3/%(project_id)s/volumes/%(volume_id)s/action' % { @@ -1084,6 +1090,7 @@ 'terminate_connection') def test_owner_can_initialize_terminate_conn(self, mock_t, mock_i): user_context = self.user_context + user_context.service_roles = ['service'] volume = self._create_fake_volume(user_context) path = '/v3/%(project_id)s/volumes/%(volume_id)s/action' % { diff -Nru cinder-21.1.0/cinder/tests/unit/test_image_utils.py cinder-21.3.1/cinder/tests/unit/test_image_utils.py --- cinder-21.1.0/cinder/tests/unit/test_image_utils.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/test_image_utils.py 2023-08-14 16:54:18.000000000 +0000 @@ -2455,3 +2455,135 @@ image_utils.convert_image(src, dest, out_fmt) mock_check.assert_called_once_with(src, None, None, None, True) + + +@ddt.ddt +class TestFilterReservedNamespaces(test.TestCase): + + def setUp(self): + super(TestFilterReservedNamespaces, self).setUp() + self.mock_object(image_utils, 'LOG', side_effect=image_utils.LOG) + + def test_filter_out_reserved_namespaces_metadata_with_empty_metadata(self): + metadata_for_test = None + method_return = image_utils.filter_out_reserved_namespaces_metadata( + metadata_for_test) + + self.assertEqual({}, method_return) + + image_utils.LOG.debug.assert_has_calls( + [mock.call("No metadata to be filtered.")] + ) + + @ddt.data( # remove default keys + ({"some_key": 13, "other_key": "test", + "os_glance_key": "this should be removed", + "os_glance_key2": "this should also be removed"}, + None, + []), + # remove nothing + ({"some_key": 13, "other_key": "test"}, + None, + []), + # custom config empty + ({"some_key": 13, "other_key": "test", + "os_glance_key": "this should be removed", + "os_glance_key2": "this should also be removed"}, + [], + []), + # custom config + ({"some_key": 13, "other_key": "test", + "os_glance_key": "this should be removed", + "os_glance_key2": "this should also be removed", + "custom_key": "this should be removed", + "another_custom_key": "this should also be removed"}, + ['custom_key', 'another_custom_key'], + ['custom_key', 'another_custom_key'])) + @ddt.unpack + def test_filter_out_reserved_namespaces_metadata( + self, metadata_for_test, config, keys_to_pop): + hardcoded_keys = ['os_glance', "img_signature"] + + keys_to_pop = hardcoded_keys + keys_to_pop + + if config: + self.override_config('reserved_image_namespaces', config) + + expected_result = {"some_key": 13, "other_key": "test"} + + method_return = image_utils.filter_out_reserved_namespaces_metadata( + metadata_for_test) + + self.assertEqual(expected_result, method_return) + + image_utils.LOG.debug.assert_has_calls([ + mock.call("The metadata set [%s] was filtered using the reserved " + "name spaces [%s], and the result is [%s].", + metadata_for_test, keys_to_pop, expected_result) + ]) + + @ddt.data( # remove default keys + ({"some_key": 13, "other_key": "test", + "os_glance_key": "this should be removed", + "os_glance_key2": "this should also be removed", + "properties": {"os_glance_key3": "this should be removed", + "os_glance_key4": "this should also be removed", + "another_key": "foobar"} + }, + None, + []), + # remove nothing + ({"some_key": 13, "other_key": "test", + "properties": {"another_key": "foobar"}}, + None, + []), + # custom config empty + ({"some_key": 13, "other_key": "test", + "os_glance_key": "this should be removed", + "os_glance_key2": "this should also be removed", + "properties": {"os_glance_key3": "this should be removed", + "os_glance_key4": "this should also be removed", + "another_key": "foobar"} + }, + [], + []), + # custom config + ({"some_key": 13, "other_key": "test", + "os_glance_key": "this should be removed", + "os_glance_key2": "this should also be removed", + "properties": {"os_glance_key3": "this should be removed", + "os_glance_key4": "this should also be removed", + "custom_key": "this should be removed", + "another_custom_key": "this should also be removed", + "another_key": "foobar"}, + }, + ['custom_key', 'another_custom_key'], + ['custom_key', 'another_custom_key'])) + @ddt.unpack + def test_filter_out_reserved_namespaces_metadata_properties( + self, metadata_for_test, config, keys_to_pop): + hardcoded_keys = ['os_glance', "img_signature"] + + keys_to_pop = hardcoded_keys + keys_to_pop + + if config: + self.override_config('reserved_image_namespaces', config) + + expected_result = { + "some_key": 13, + "other_key": "test", + "properties": { + "another_key": "foobar" + } + } + + method_return = image_utils.filter_out_reserved_namespaces_metadata( + metadata_for_test) + + self.assertEqual(expected_result, method_return) + + image_utils.LOG.debug.assert_has_calls([ + mock.call("The metadata set [%s] was filtered using the reserved " + "name spaces [%s], and the result is [%s].", + metadata_for_test, keys_to_pop, expected_result) + ]) diff -Nru cinder-21.1.0/cinder/tests/unit/volume/drivers/dell_emc/powerstore/test_volume_create_from_source.py cinder-21.3.1/cinder/tests/unit/volume/drivers/dell_emc/powerstore/test_volume_create_from_source.py --- cinder-21.1.0/cinder/tests/unit/volume/drivers/dell_emc/powerstore/test_volume_create_from_source.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/volume/drivers/dell_emc/powerstore/test_volume_create_from_source.py 2023-08-14 16:54:18.000000000 +0000 @@ -19,6 +19,7 @@ from cinder.tests.unit import fake_snapshot from cinder.tests.unit import fake_volume from cinder.tests.unit.volume.drivers.dell_emc import powerstore +from cinder.volume.drivers.dell_emc.powerstore import client class TestVolumeCreateFromSource(powerstore.TestPowerStoreDriver): @@ -114,3 +115,26 @@ self.source_volume ) self.assertIn("Failed to extend PowerStore volume", error.msg) + + @mock.patch("requests.request") + def test_create_snapshot_limit_reached(self, mock_create_snap_limit): + snapshot_limit_reached_response = { + 'errorCode': 0, + 'message': 'messages', + 'messages': [ + { + 'code': client.TOO_MANY_SNAPS_ERROR, + } + ] + } + mock_create_snap_limit.return_value = powerstore.MockResponse( + snapshot_limit_reached_response, rc=400) + error = self.assertRaises( + exception.SnapshotLimitReached, + self.driver.adapter.create_volume_from_source, + self.volume, + self.source_volume + ) + self.assertIn( + "Exceeded the configured limit of 32 snapshots per volume.", + error.msg) diff -Nru cinder-21.1.0/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py cinder-21.3.1/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py --- cinder-21.1.0/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_base.py 2023-08-14 16:54:18.000000000 +0000 @@ -16,7 +16,6 @@ """Unit tests for the NetApp NFS storage driver.""" import copy import os -import threading import time from unittest import mock @@ -620,36 +619,6 @@ os.path.exists.assert_called_once_with( 'dir/' + fake.CLONE_DESTINATION_NAME) - def test__spawn_clean_cache_job_clean_job_setup(self): - self.driver.cleaning = True - mock_debug_log = self.mock_object(nfs_base.LOG, 'debug') - self.mock_object(utils, 'synchronized', return_value=lambda f: f) - - retval = self.driver._spawn_clean_cache_job() - - self.assertIsNone(retval) - self.assertEqual(1, mock_debug_log.call_count) - - def test__spawn_clean_cache_job_new_clean_job(self): - - class FakeTimer(object): - def start(self): - pass - - fake_timer = FakeTimer() - self.mock_object(utils, 'synchronized', return_value=lambda f: f) - self.mock_object(fake_timer, 'start') - self.mock_object(nfs_base.LOG, 'debug') - self.mock_object(self.driver, '_clean_image_cache') - self.mock_object(threading, 'Timer', return_value=fake_timer) - - retval = self.driver._spawn_clean_cache_job() - - self.assertIsNone(retval) - threading.Timer.assert_called_once_with( - 0, self.driver._clean_image_cache) - fake_timer.start.assert_called_once_with() - def test_cleanup_volume_on_failure(self): path = '%s/%s' % (fake.NFS_SHARE, fake.NFS_VOLUME['name']) mock_local_path = self.mock_object(self.driver, 'local_path') @@ -1077,13 +1046,22 @@ self.driver, '_delete_snapshots_marked_for_deletion') mock_call_ems_logging = self.mock_object( self.driver, '_handle_ems_logging') + mock_call_clean_image_cache = self.mock_object( + self.driver, '_clean_image_cache') + + # image cache cleanup task can be configured with custom timeout + cache_cleanup_interval = loopingcalls.ONE_HOUR + self.driver.configuration.netapp_nfs_image_cache_cleanup_interval = ( + cache_cleanup_interval) self.driver._add_looping_tasks() mock_add_task.assert_has_calls([ mock.call(mock_call_snap_cleanup, loopingcalls.ONE_MINUTE, loopingcalls.ONE_MINUTE), - mock.call(mock_call_ems_logging, loopingcalls.ONE_HOUR)]) + mock.call(mock_call_ems_logging, loopingcalls.ONE_HOUR), + mock.call(mock_call_clean_image_cache, cache_cleanup_interval) + ]) def test__clone_from_cache(self): image_id = 'fake_image_id' diff -Nru cinder-21.1.0/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py cinder-21.3.1/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py --- cinder-21.1.0/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/volume/drivers/netapp/dataontap/test_nfs_cmode.py 2023-08-14 16:54:18.000000000 +0000 @@ -137,7 +137,6 @@ mock_debug_log = self.mock_object(nfs_cmode.LOG, 'debug') self.mock_object(self.driver, 'get_filter_function') self.mock_object(self.driver, 'get_goodness_function') - self.mock_object(self.driver, '_spawn_clean_cache_job') self.driver.zapi_client = mock.Mock() self.mock_object(self.driver, '_get_pool_stats', return_value={}) expected_stats = { @@ -153,7 +152,6 @@ retval = self.driver._update_volume_stats() self.assertIsNone(retval) - self.assertTrue(self.driver._spawn_clean_cache_job.called) self.assertEqual(1, mock_debug_log.call_count) self.assertEqual(expected_stats, self.driver._stats) diff -Nru cinder-21.1.0/cinder/tests/unit/volume/drivers/test_infinidat.py cinder-21.3.1/cinder/tests/unit/volume/drivers/test_infinidat.py --- cinder-21.1.0/cinder/tests/unit/volume/drivers/test_infinidat.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/volume/drivers/test_infinidat.py 2023-08-14 16:54:18.000000000 +0000 @@ -39,6 +39,7 @@ TEST_IP_ADDRESS3 = '3.3.3.3' TEST_IP_ADDRESS4 = '4.4.4.4' TEST_INITIATOR_IQN = 'iqn.2012-07.org.initiator:01' +TEST_INITIATOR_IQN2 = 'iqn.2012-07.org.initiator:02' TEST_TARGET_IQN = 'iqn.2012-07.org.target:01' TEST_ISCSI_TCP_PORT1 = 3261 TEST_ISCSI_TCP_PORT2 = 3262 @@ -57,15 +58,26 @@ TEST_SNAPSHOT_SOURCE_ID = 67890 TEST_SNAPSHOT_METADATA = {'cinder_id': fake.SNAPSHOT_ID} -test_volume = mock.Mock(id=fake.VOLUME_ID, size=1, - volume_type_id=fake.VOLUME_TYPE_ID) +test_volume = mock.Mock(id=fake.VOLUME_ID, name_id=fake.VOLUME_ID, size=1, + volume_type_id=fake.VOLUME_TYPE_ID, + multiattach=False, volume_attachment=None) +test_volume2 = mock.Mock(id=fake.VOLUME2_ID, name_id=fake.VOLUME2_ID, size=1, + volume_type_id=fake.VOLUME_TYPE_ID, + multiattach=False, volume_attachment=None) test_snapshot = mock.Mock(id=fake.SNAPSHOT_ID, volume=test_volume, volume_id=test_volume.id) -test_clone = mock.Mock(id=fake.VOLUME4_ID, size=1) +test_clone = mock.Mock(id=fake.VOLUME4_ID, size=1, multiattach=False, + volume_attachment=None) test_group = mock.Mock(id=fake.GROUP_ID) test_snapgroup = mock.Mock(id=fake.GROUP_SNAPSHOT_ID, group=test_group) test_connector = dict(wwpns=[TEST_WWN_1], initiator=TEST_INITIATOR_IQN) +test_connector2 = dict(wwpns=[TEST_WWN_2], + initiator=TEST_INITIATOR_IQN2) +test_connector3 = dict(wwpns=None, initiator=None) +test_attachment1 = mock.Mock(connector=test_connector) +test_attachment2 = mock.Mock(connector=test_connector2) +test_attachment3 = mock.Mock(connector=None) def skip_driver_setup(func): @@ -104,6 +116,7 @@ # mock external library dependencies infinisdk = self.patch("cinder.volume.drivers.infinidat.infinisdk") capacity = self.patch("cinder.volume.drivers.infinidat.capacity") + self._log = self.patch("cinder.volume.drivers.infinidat.LOG") self._iqn = self.patch("cinder.volume.drivers.infinidat.iqn") self._wwn = self.patch("cinder.volume.drivers.infinidat.wwn") self._wwn.WWN = mock.Mock @@ -258,15 +271,41 @@ self.driver.initialize_connection(test_volume, test_connector) self._validate_host_metadata() + @ddt.data({'connector': None, 'multiattach': True, + 'attachment': [test_attachment1, test_attachment1]}, + {'connector': test_connector3, 'multiattach': True, + 'attachment': [test_attachment1, test_attachment1]}, + {'connector': test_connector, 'multiattach': False, + 'attachment': [test_attachment1]}, + {'connector': test_connector, 'multiattach': True, + 'attachment': None}, + {'connector': test_connector, 'multiattach': True, + 'attachment': [test_attachment2, test_attachment3]}) + @ddt.unpack + def test__is_volume_multiattached_negative(self, connector, + multiattach, attachment): + volume = copy.deepcopy(test_volume) + volume.multiattach = multiattach + volume.volume_attachment = attachment + self.assertFalse(self.driver._is_volume_multiattached(volume, + connector)) + def test_terminate_connection(self): - self.driver.terminate_connection(test_volume, test_connector) + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) def test_terminate_connection_delete_host(self): self._mock_host.get_luns.return_value = [object()] - self.driver.terminate_connection(test_volume, test_connector) + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) self.assertEqual(0, self._mock_host.safe_delete.call_count) self._mock_host.get_luns.return_value = [] - self.driver.terminate_connection(test_volume, test_connector) + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) self.assertEqual(1, self._mock_host.safe_delete.call_count) def test_terminate_connection_volume_doesnt_exist(self): @@ -614,8 +653,10 @@ mock_mapping = mock.Mock() mock_mapping.get_host.return_value = mock_infinidat_host self._mock_volume.get_logical_units.return_value = [mock_mapping] + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1, test_attachment2] # connector is None - force detach - detach all mappings - self.driver.terminate_connection(test_volume, None) + self.assertTrue(self.driver.terminate_connection(volume, None)) # make sure we actually detached the host mapping self._mock_host.unmap_volume.assert_called_once() self._mock_host.safe_delete.assert_called_once() @@ -858,7 +899,85 @@ def test_unmanage_snapshot(self): self.driver.unmanage_snapshot(test_snapshot) + def test_terminate_connection_no_attachment_connector(self): + volume = copy.deepcopy(test_volume) + volume.multiattach = True + volume.volume_attachment = [test_attachment3] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) + + def test_terminate_connection_no_host(self): + self._system.hosts.safe_get.return_value = None + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) + + def test_terminate_connection_no_mapping(self): + self._mock_host.unmap_volume.side_effect = KeyError + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) + + def test_update_migrated_volume_new_volume_not_found(self): + self._system.volumes.safe_get.side_effect = [ + None, self._mock_volume] + self.assertRaises(exception.VolumeNotFound, + self.driver.update_migrated_volume, + None, test_volume, test_volume2, + 'available') + + @mock.patch('cinder.volume.drivers.infinidat.InfiniboxVolumeDriver.' + '_set_cinder_object_metadata') + def test_update_migrated_volume_volume_not_found(self, set_metadata): + self._system.volumes.safe_get.side_effect = [ + self._mock_new_volume, None] + update = self.driver.update_migrated_volume(None, + test_volume, + test_volume2, + 'available') + expected = {'_name_id': None, 'provider_location': None} + self.assertEqual(expected, update) + set_metadata.assert_called_once_with(self._mock_new_volume, + test_volume) + @mock.patch('cinder.volume.drivers.infinidat.InfiniboxVolumeDriver.' + '_set_cinder_object_metadata') + def test_update_migrated_new_volume_rename_error(self, set_metadata): + self._system.volumes.safe_get.side_effect = [ + self._mock_new_volume, None] + self._mock_new_volume.update_name.side_effect = [ + FakeInfinisdkException] + update = self.driver.update_migrated_volume(None, + test_volume, + test_volume2, + 'available') + expected = {'_name_id': test_volume2.name_id, + 'provider_location': None} + self.assertEqual(expected, update) + set_metadata.assert_called_once_with(self._mock_new_volume, + test_volume) + + @mock.patch('cinder.volume.drivers.infinidat.InfiniboxVolumeDriver.' + '_set_cinder_object_metadata') + def test_update_migrated(self, set_metadata): + self._system.volumes.safe_get.side_effect = [ + self._mock_new_volume, self._mock_volume] + self._mock_new_volume.update_name.side_effect = None + update = self.driver.update_migrated_volume(None, + test_volume, + test_volume2, + 'available') + expected = {'_name_id': test_volume2.name_id, + 'provider_location': None} + self.assertEqual(expected, update) + set_metadata.assert_called_once_with(self._mock_new_volume, + test_volume) + self.assertEqual(0, self._log.error.call_count) + + +@ddt.ddt class InfiniboxDriverTestCaseFC(InfiniboxDriverTestCaseBase): def test_initialize_connection_multiple_wwpns(self): connector = {'wwpns': [TEST_WWN_1, TEST_WWN_2]} @@ -872,7 +991,27 @@ self.assertRaises(exception.InvalidConnectorException, self.driver.validate_connector, iscsi_connector) + @ddt.data({'connector': test_connector, + 'attachment': [test_attachment1, test_attachment1]}, + {'connector': test_connector2, + 'attachment': [test_attachment2, test_attachment2]}) + @ddt.unpack + def test__is_volume_multiattached_positive(self, connector, attachment): + volume = copy.deepcopy(test_volume) + volume.multiattach = True + volume.volume_attachment = attachment + self.assertTrue(self.driver._is_volume_multiattached(volume, + connector)) + def test_terminate_connection_multiattached_volume(self): + volume = copy.deepcopy(test_volume) + volume.multiattach = True + volume.volume_attachment = [test_attachment1, test_attachment1] + self.assertTrue(self.driver.terminate_connection(volume, + test_connector)) + + +@ddt.ddt class InfiniboxDriverTestCaseISCSI(InfiniboxDriverTestCaseBase): def setUp(self): super(InfiniboxDriverTestCaseISCSI, self).setUp() @@ -1055,8 +1194,23 @@ } self.assertEqual(expected, result) + @ddt.data({'connector': test_connector, + 'attachment': [test_attachment1, test_attachment1]}, + {'connector': test_connector2, + 'attachment': [test_attachment2, test_attachment2]}) + @ddt.unpack + def test__is_volume_multiattached_positive(self, connector, attachment): + volume = copy.deepcopy(test_volume) + volume.multiattach = True + volume.volume_attachment = attachment + self.assertTrue(self.driver._is_volume_multiattached(volume, + connector)) + def test_terminate_connection(self): - self.driver.terminate_connection(test_volume, test_connector) + volume = copy.deepcopy(test_volume) + volume.volume_attachment = [test_attachment1] + self.assertFalse(self.driver.terminate_connection(volume, + test_connector)) def test_validate_connector(self): fc_connector = {'wwpns': [TEST_WWN_1, TEST_WWN_2]} diff -Nru cinder-21.1.0/cinder/tests/unit/volume/drivers/test_pure.py cinder-21.3.1/cinder/tests/unit/volume/drivers/test_pure.py --- cinder-21.1.0/cinder/tests/unit/volume/drivers/test_pure.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/volume/drivers/test_pure.py 2023-08-14 16:54:18.000000000 +0000 @@ -747,6 +747,8 @@ super(PureBaseSharedDriverTestCase, self).setUp() self.driver = pure.PureBaseVolumeDriver(configuration=self.mock_config) self.driver._array = self.array + self.mock_object(self.driver, '_get_current_array', + return_value=self.array) self.driver._replication_pod_name = 'cinder-pod' self.driver._replication_pg_name = 'cinder-group' self.purestorage_module.FlashArray.side_effect = None @@ -820,6 +822,24 @@ return group_snap, group_snap_name +class PureBaseVolumeDriverGetCurrentArrayTestCase(PureDriverTestCase): + def setUp(self): + super(PureBaseVolumeDriverGetCurrentArrayTestCase, self).setUp() + self.driver = pure.PureBaseVolumeDriver(configuration=self.mock_config) + self.driver._array = self.array + self.driver._replication_pod_name = 'cinder-pod' + self.driver._replication_pg_name = 'cinder-group' + self.purestorage_module.FlashArray.side_effect = None + + def test_get_current_array(self): + self.driver._is_active_cluster_enabled = True + self.array.array_id = '47966b2d-a1ed-4144-8cae-6332794562b8' + self.array.get_pod.return_value = CINDER_POD + self.driver._active_cluster_target_arrays = [self.array] + self.driver._get_current_array() + self.array.get_pod.assert_called_with('cinder-pod') + + @ddt.ddt(testNameFormat=ddt.TestNameFormat.INDEX_ONLY) class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): def _setup_mocks_for_replication(self): @@ -983,6 +1003,15 @@ retention = mock.MagicMock() mock_generate_replication_retention.return_value = retention self._setup_mocks_for_replication() + self.mock_config.safe_get.return_value = [ + { + "backend_id": "foo", + "managed_backend_name": None, + "san_ip": "1.2.3.4", + "api_token": "abc123", + "type": "sync", + } + ] self.async_array2.get.return_value = GET_ARRAY_SECONDARY self.array.get.return_value = GET_ARRAY_PRIMARY self.purestorage_module.FlashArray.side_effect = [self.array, @@ -990,12 +1019,9 @@ self.driver._storage_protocol = 'iSCSI' self.driver.do_setup(None) self.assertEqual(self.array, self.driver._array) - self.assertEqual(1, len(self.driver._replication_target_arrays)) - self.assertEqual(self.async_array2, - self.driver._replication_target_arrays[0]) calls = [ mock.call(self.array, [self.async_array2], 'cinder-group', - REPLICATION_INTERVAL_IN_SEC, retention) + 3600, retention) ] mock_setup_repl_pgroups.assert_has_calls(calls) @@ -1028,11 +1054,6 @@ self.driver._storage_protocol = 'iSCSI' self.driver.do_setup(None) self.assertEqual(self.array, self.driver._array) - - mock_setup_repl_pgroups.assert_has_calls([ - mock.call(self.array, [mock_sync_target], 'cinder-group', - REPLICATION_INTERVAL_IN_SEC, retention), - ]) mock_setup_pods.assert_has_calls([ mock.call(self.array, [mock_sync_target], 'cinder-pod') ]) @@ -1748,6 +1769,7 @@ model_update = self.driver.create_consistencygroup(None, cgroup) expected_name = "consisgroup-" + cgroup.id + "-cinder" + self.driver._get_current_array.assert_called() self.array.create_pgroup.assert_called_with(expected_name) self.assertEqual({'status': 'available'}, model_update) @@ -3403,6 +3425,8 @@ self.mock_config.use_chap_auth = False self.driver = pure.PureISCSIDriver(configuration=self.mock_config) self.driver._array = self.array + self.mock_object(self.driver, '_get_current_array', + return_value=self.array) self.driver._storage_protocol = 'iSCSI' self.mock_utils = mock.Mock() self.driver.driver_utils = self.mock_utils @@ -3905,6 +3929,8 @@ self.driver = pure.PureFCDriver(configuration=self.mock_config) self.driver._storage_protocol = "FC" self.driver._array = self.array + self.mock_object(self.driver, '_get_current_array', + return_value=self.array) self.driver._lookup_service = mock.Mock() def test_get_host(self): @@ -4445,6 +4471,8 @@ super(PureNVMEDriverTestCase, self).setUp() self.driver = pure.PureNVMEDriver(configuration=self.mock_config) self.driver._array = self.array + self.mock_object(self.driver, '_get_current_array', + return_value=self.array) self.driver._storage_protocol = 'NVMe-RoCE' self.mock_utils = mock.Mock() self.driver.transport_type = "rdma" diff -Nru cinder-21.1.0/cinder/tests/unit/volume/drivers/test_rbd.py cinder-21.3.1/cinder/tests/unit/volume/drivers/test_rbd.py --- cinder-21.1.0/cinder/tests/unit/volume/drivers/test_rbd.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/volume/drivers/test_rbd.py 2023-08-14 16:54:18.000000000 +0000 @@ -19,6 +19,7 @@ import os import tempfile import time +import types from unittest import mock from unittest.mock import call import uuid @@ -176,14 +177,14 @@ @ddt.ddt class RBDTestCase(test.TestCase): - @staticmethod - def _make_configuration(conf_in=None): + @classmethod + def _make_configuration(cls, conf_in=None): cfg = mock.Mock(spec=conf.Configuration) cfg.image_conversion_dir = None cfg.rbd_cluster_name = 'nondefault' cfg.rbd_pool = 'rbd' cfg.rbd_ceph_conf = '/etc/ceph/my_ceph.conf' - cfg.rbd_secret_uuid = None + cfg.rbd_secret_uuid = '5fe62cc7-0392-4a32-8466-081ce0ea970f' cfg.rbd_user = 'cinder' cfg.volume_backend_name = None cfg.volume_dd_blocksize = '1M' @@ -193,6 +194,10 @@ cfg.backup_use_temp_snapshot = False cfg.enable_deferred_deletion = False + # Because the mocked conf doesn't actually have an underlying oslo conf + # it doesn't have the set_default method, so we use a fake one. + cfg.set_default = types.MethodType(cls._set_default, cfg) + if conf_in is not None: for k in conf_in: setattr(cfg, k, conf_in[k]) @@ -200,6 +205,12 @@ return cfg @staticmethod + def _set_default(cfg, name, value, group=None): + # Ignore the group for now + if not getattr(cfg, name): + setattr(cfg, name, value) + + @staticmethod def _make_drv(conf_in): cfg = RBDTestCase._make_configuration(conf_in) @@ -337,8 +348,11 @@ def test_do_setup_replication_disabled(self): with mock.patch.object(self.driver.configuration, 'safe_get', - return_value=None): + return_value=None), \ + mock.patch.object(self.driver, + '_set_default_secret_uuid') as mock_secret: self.driver.do_setup(self.context) + mock_secret.assert_called_once_with() self.assertFalse(self.driver._is_replication_enabled) self.assertEqual([], self.driver._replication_targets) self.assertEqual([], self.driver._target_names) @@ -348,6 +362,36 @@ 'secret_uuid': self.cfg.rbd_secret_uuid}, self.driver._active_config) + @ddt.data('', None) + @mock.patch.object(driver.RBDDriver, '_get_fsid') + def test__set_default_secret_uuid_missing(self, secret_uuid, mock_fsid): + # Clear the current values + self.cfg.rbd_secret_uuid = secret_uuid + self.driver._active_config['secret_uuid'] = secret_uuid + # Fake fsid value returned by the cluster + fsid = str(uuid.uuid4()) + mock_fsid.return_value = fsid + + self.driver._set_default_secret_uuid() + + mock_fsid.assert_called_once_with() + self.assertEqual(fsid, self.driver._active_config['secret_uuid']) + self.assertEqual(fsid, self.cfg.rbd_secret_uuid) + + @mock.patch.object(driver.RBDDriver, '_get_fsid') + def test__set_default_secret_uuid_present(self, mock_fsid): + # Set secret_uuid like _get_target_config does on do_setup + secret_uuid = self.cfg.rbd_secret_uuid + self.driver._active_config['secret_uuid'] = secret_uuid + # Fake fsid value returned by the cluster (should not be callled) + mock_fsid.return_value = str(uuid.uuid4()) + self.driver._set_default_secret_uuid() + mock_fsid.assert_not_called() + # Values must not have changed + self.assertEqual(secret_uuid, + self.driver._active_config['secret_uuid']) + self.assertEqual(secret_uuid, self.cfg.rbd_secret_uuid) + def test_do_setup_replication(self): cfg = [{'backend_id': 'secondary-backend', 'conf': 'foo', @@ -1925,6 +1969,51 @@ ]) self.assertEqual((free_capacity, total_capacity), result) + @ddt.data( + # Normal case, no quota and dynamic total + {'free_capacity': 27.0, 'total_capacity': 28.44}, + # No quota and static total + {'dynamic_total': False, + 'free_capacity': 27.0, 'total_capacity': 59.96}, + # Quota and dynamic total + {'quota_max_bytes': 3221225472, 'max_avail': 1073741824, + 'free_capacity': 1, 'total_capacity': 2.44}, + # Quota and static total + {'quota_max_bytes': 3221225472, 'max_avail': 1073741824, + 'dynamic_total': False, + 'free_capacity': 1, 'total_capacity': 3.00}, + # Quota and dynamic total when free would be negative + {'quota_max_bytes': 1073741824, + 'free_capacity': 0, 'total_capacity': 1.44}, + ) + @ddt.unpack + @common_mocks + def test_get_pool_nautilus(self, free_capacity, total_capacity, + max_avail=28987613184, quota_max_bytes=0, + dynamic_total=True): + client = self.mock_client.return_value + client.__enter__.return_value = client + client.cluster.mon_command.side_effect = [ + (0, '{"stats":{"total_bytes":64385286144,' + '"total_used_bytes":3289628672,"total_avail_bytes":61095657472},' + '"pools":[{"name":"rbd","id":2,"stats":{"kb_used":1510197,' + '"stored":1546440971,"bytes_used":4639322913,"max_avail":%s,' + '"objects":412}},{"name":"volumes","id":3,"stats":{"kb_used":0,' + '"bytes_used":0,"max_avail":28987613184,"objects":0}}]}\n' % + max_avail, ''), + (0, '{"pool_name":"volumes","pool_id":4,"quota_max_objects":0,' + '"quota_max_bytes":%s}\n' % quota_max_bytes, ''), + ] + with mock.patch.object(self.driver.configuration, 'safe_get', + return_value=dynamic_total): + result = self.driver._get_pool_stats() + client.cluster.mon_command.assert_has_calls([ + mock.call('{"prefix":"df", "format":"json"}', b''), + mock.call('{"prefix":"osd pool get-quota", "pool": "rbd",' + ' "format":"json"}', b''), + ]) + self.assertEqual((free_capacity, total_capacity), result) + @common_mocks def test_get_pool_bytes(self): """Test for mon_commands returning bytes instead of strings.""" @@ -1983,7 +2072,8 @@ self.driver._active_config = {'name': 'secondary_id', 'user': 'foo', - 'conf': 'bar'} + 'conf': 'bar', + 'secret_uuid': self.cfg.rbd_secret_uuid} expected = { 'driver_volume_type': 'rbd', 'data': { diff -Nru cinder-21.1.0/cinder/tests/unit/volume/test_connection.py cinder-21.3.1/cinder/tests/unit/volume/test_connection.py --- cinder-21.1.0/cinder/tests/unit/volume/test_connection.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/volume/test_connection.py 2023-08-14 16:54:18.000000000 +0000 @@ -1334,7 +1334,8 @@ self.context, volume, None, None, None, None) - def test_volume_detach_in_maintenance(self): + @mock.patch('cinder.volume.api.API.attachment_deletion_allowed') + def test_volume_detach_in_maintenance(self, mock_attachment_deletion): """Test detach the volume in maintenance.""" test_meta1 = {'fake_key1': 'fake_value1', 'fake_key2': 'fake_value2'} volume = tests_utils.create_volume(self.context, metadata=test_meta1, @@ -1345,3 +1346,5 @@ volume_api.detach, self.context, volume, None) + mock_attachment_deletion.assert_called_once_with(self.context, + None, volume) diff -Nru cinder-21.1.0/cinder/tests/unit/volume/test_volume.py cinder-21.3.1/cinder/tests/unit/volume/test_volume.py --- cinder-21.1.0/cinder/tests/unit/volume/test_volume.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/tests/unit/volume/test_volume.py 2023-08-14 16:54:18.000000000 +0000 @@ -776,19 +776,6 @@ self.assertEqual(foo['id'], vol['volume_type_id']) self.assertTrue(vol['multiattach']) - def test_create_volume_with_multiattach_flag(self): - """Tests creating a volume with multiattach=True but no special type. - - This tests the pre 3.50 microversion behavior of being able to create - a volume with the multiattach request parameter regardless of a - multiattach-capable volume type. - """ - volume_api = cinder.volume.api.API() - volume = volume_api.create( - self.context, 1, 'name', 'description', multiattach=True, - volume_type=self.vol_type) - self.assertTrue(volume.multiattach) - def _fail_multiattach_policy_authorize(self, policy): if policy == vol_policy.MULTIATTACH_POLICY: raise exception.PolicyNotAuthorized(action='Test') @@ -813,16 +800,6 @@ 1, 'admin-vol', 'description', volume_type=foo) - def test_create_volume_with_multiattach_flag_not_authorized(self): - """Test policy unauthorized create with multiattach flag.""" - volume_api = cinder.volume.api.API() - - with mock.patch.object(self.context, 'authorize') as mock_auth: - mock_auth.side_effect = self._fail_multiattach_policy_authorize - self.assertRaises(exception.PolicyNotAuthorized, - volume_api.create, self.context, 1, 'name', - 'description', multiattach=True) - @mock.patch.object(key_manager, 'API', fake_keymgr.fake_api) def test_create_volume_with_encrypted_volume_type_multiattach(self): ctxt = context.get_admin_context() diff -Nru cinder-21.1.0/cinder/volume/api.py cinder-21.3.1/cinder/volume/api.py --- cinder-21.1.0/cinder/volume/api.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/volume/api.py 2023-08-14 16:54:18.000000000 +0000 @@ -34,6 +34,7 @@ from cinder.api import common from cinder.common import constants +from cinder import compute from cinder import context from cinder import coordination from cinder import db @@ -43,6 +44,7 @@ from cinder.i18n import _ from cinder.image import cache as image_cache from cinder.image import glance +from cinder.image import image_utils from cinder.message import api as message_api from cinder.message import message_field from cinder import objects @@ -229,7 +231,6 @@ source_replica=None, consistencygroup: Optional[objects.ConsistencyGroup] = None, cgsnapshot: Optional[objects.CGSnapshot] = None, - multiattach: Optional[bool] = False, source_cg=None, group: Optional[objects.Group] = None, group_snapshot=None, @@ -338,7 +339,6 @@ 'optional_args': {'is_quota_committed': False}, 'consistencygroup': consistencygroup, 'cgsnapshot': cgsnapshot, - 'raw_multiattach': multiattach, 'group': group, 'group_snapshot': group_snapshot, 'source_group': source_group, @@ -863,11 +863,14 @@ attachment_id: str) -> None: context.authorize(vol_action_policy.DETACH_POLICY, target_obj=volume) + self.attachment_deletion_allowed(context, attachment_id, volume) + if volume['status'] == 'maintenance': LOG.info('Unable to detach volume, ' 'because it is in maintenance.', resource=volume) msg = _("The volume cannot be detached in maintenance mode.") raise exception.InvalidVolume(reason=msg) + detach_results = self.volume_rpcapi.detach_volume(context, volume, attachment_id) LOG.info("Detach volume completed successfully.", @@ -894,6 +897,19 @@ resource=volume) return init_results + @staticmethod + def is_service_request(ctxt: 'context.RequestContext') -> bool: + """Check if a request is coming from a service + + A request is coming from a service if it has a service token and the + service user has one of the roles configured in the + `service_token_roles` configuration option in the + `[keystone_authtoken]` section (defaults to `service`). + """ + roles = ctxt.service_roles + service_roles = set(CONF.keystone_authtoken.service_token_roles) + return bool(roles and service_roles.intersection(roles)) + def terminate_connection(self, context: context.RequestContext, volume: objects.Volume, @@ -901,6 +917,8 @@ force: bool = False) -> None: context.authorize(vol_action_policy.TERMINATE_POLICY, target_obj=volume) + self.attachment_deletion_allowed(context, None, volume) + self.volume_rpcapi.terminate_connection(context, volume, connector, @@ -1483,6 +1501,9 @@ try: self._merge_volume_image_meta(context, volume, metadata) + metadata = image_utils.filter_out_reserved_namespaces_metadata( + metadata) + recv_metadata = self.image_service.create(context, metadata) # NOTE(ZhengMa): Check if allow image compression before image @@ -2519,11 +2540,90 @@ attachment_ref.save() return attachment_ref + def attachment_deletion_allowed(self, + ctxt: context.RequestContext, + attachment_or_attachment_id, + volume=None): + """Check if deleting an attachment is allowed (Bug #2004555) + + Allowed is based on the REST API policy, the status of the attachment, + where it is used, and who is making the request. + + Deleting an attachment on the Cinder side while leaving the volume + connected to the nova host results in leftover devices that can lead to + data leaks/corruption. + + OS-Brick may have code to detect it, but in some cases it is detected + after it has already been exposed, so it's better to prevent users from + being able to intentionally triggering the issue. + """ + # It's ok to delete an attachment if the request comes from a service + if self.is_service_request(ctxt): + return + + if not attachment_or_attachment_id and volume: + if not volume.volume_attachment: + return + if len(volume.volume_attachment) == 1: + attachment_or_attachment_id = volume.volume_attachment[0] + + if isinstance(attachment_or_attachment_id, str): + try: + attachment = objects.VolumeAttachment.get_by_id( + ctxt, attachment_or_attachment_id) + except exception.VolumeAttachmentNotFound: + attachment = None + else: + attachment = attachment_or_attachment_id + + if attachment: + if volume: + if volume.id != attachment.volume_id: + raise exception.InvalidInput( + reason='Mismatched volume and attachment') + + server_id = attachment.instance_uuid + # It's ok to delete if it's not connected to a vm. + if not server_id or not attachment.connection_info: + return + + volume = volume or attachment.volume + nova = compute.API() + LOG.info('Attachment connected to vm %s, checking data on nova', + server_id) + # If nova is down the client raises 503 and we report that + try: + nova_volume = nova.get_server_volume(ctxt, server_id, + volume.id) + except nova.NotFound: + LOG.warning('Instance or volume not found on Nova, deleting ' + 'attachment locally, which may leave leftover ' + 'devices on Nova compute') + return + + if nova_volume.attachment_id != attachment.id: + LOG.warning('Mismatch! Nova has different attachment id (%s) ' + 'for the volume, deleting attachment locally. ' + 'May leave leftover devices in a compute node', + nova_volume.attachment_id) + return + else: + server_id = '' + + LOG.error('Detected user call to delete in-use attachment. Call must ' + 'come from the nova service and nova must be configured to ' + 'send the service token. Bug #2004555') + raise exception.ConflictNovaUsingAttachment(instance_id=server_id) + def attachment_delete(self, ctxt: context.RequestContext, attachment) -> objects.VolumeAttachmentList: + # Check if policy allows user to delete attachment ctxt.authorize(attachment_policy.DELETE_POLICY, target_obj=attachment) + + self.attachment_deletion_allowed(ctxt, attachment) + volume = attachment.volume if attachment.attach_status == fields.VolumeAttachStatus.RESERVED: diff -Nru cinder-21.1.0/cinder/volume/drivers/dell_emc/powerstore/client.py cinder-21.3.1/cinder/volume/drivers/dell_emc/powerstore/client.py --- cinder-21.1.0/cinder/volume/drivers/dell_emc/powerstore/client.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/volume/drivers/dell_emc/powerstore/client.py 2023-08-14 16:54:18.000000000 +0000 @@ -31,6 +31,8 @@ LOG = logging.getLogger(__name__) VOLUME_NOT_MAPPED_ERROR = "0xE0A08001000F" SESSION_ALREADY_FAILED_OVER_ERROR = "0xE0201005000C" +TOO_MANY_SNAPS_ERROR = "0xE0A040010003" +MAX_SNAPS_IN_VTREE = 32 class PowerStoreClient(object): @@ -271,6 +273,10 @@ "entity": entity, "entity_id": entity_id, }) LOG.error(msg) + if ("messages" in response and + response["messages"][0]["code"] == TOO_MANY_SNAPS_ERROR): + raise exception.SnapshotLimitReached( + set_limit=MAX_SNAPS_IN_VTREE) raise exception.VolumeBackendAPIException(data=msg) return response["id"] diff -Nru cinder-21.1.0/cinder/volume/drivers/infinidat.py cinder-21.3.1/cinder/volume/drivers/infinidat.py --- cinder-21.1.0/cinder/volume/drivers/infinidat.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/volume/drivers/infinidat.py 2023-08-14 16:54:18.000000000 +0000 @@ -125,10 +125,12 @@ 1.8 - added revert to snapshot 1.9 - added manage/unmanage/manageable-list volume/snapshot 1.10 - added support for TLS/SSL communication + 1.11 - fixed generic volume migration + 1.12 - fixed volume multi-attach """ - VERSION = '1.10' + VERSION = '1.12' # ThirdPartySystems wiki page CI_WIKI_NAME = "INFINIDAT_CI" @@ -202,8 +204,17 @@ 'in the connector.', {'data': required}) raise exception.InvalidConnectorException(missing=required) - def _make_volume_name(self, cinder_volume): - return 'openstack-vol-%s' % cinder_volume.id + def _make_volume_name(self, cinder_volume, migration=False): + """Return the Infinidat volume name. + + Use Cinder volume id in case of volume migration + and use Cinder volume name_id for all other cases. + """ + if migration: + key = cinder_volume.id + else: + key = cinder_volume.name_id + return 'openstack-vol-%s' % key def _make_snapshot_name(self, cinder_snapshot): return 'openstack-snap-%s' % cinder_snapshot.id @@ -468,6 +479,32 @@ ports = [iqn.IQN(connector['initiator'])] return ports + def _is_volume_multiattached(self, volume, connector): + """Returns whether the volume is multiattached. + + Check if there are multiple attachments to the volume + from the same connector. Terminate connection only for + the last attachment from the corresponding host. + """ + if not (connector and volume.multiattach and + volume.volume_attachment): + return False + keys = ['system uuid'] + if self._protocol == constants.FC: + keys.append('wwpns') + else: + keys.append('initiator') + for key in keys: + if not (key in connector and connector[key]): + continue + if sum(1 for attachment in volume.volume_attachment if + attachment.connector and key in attachment.connector and + attachment.connector[key] == connector[key]) > 1: + LOG.debug('Volume %s is multiattached to %s %s', + volume.name_id, key, connector[key]) + return True + return False + @infinisdk_to_cinder_exceptions @coordination.synchronized('infinidat-{self.management_address}-lock') def initialize_connection(self, volume, connector): @@ -481,6 +518,8 @@ @coordination.synchronized('infinidat-{self.management_address}-lock') def terminate_connection(self, volume, connector, **kwargs): """Unmap an InfiniBox volume from the host""" + if self._is_volume_multiattached(volume, connector): + return True infinidat_volume = self._get_infinidat_volume(volume) if self._protocol == constants.FC: volume_type = 'fibre_channel' @@ -511,11 +550,11 @@ target_wwpns)) result_data = dict(target_wwn=target_wwpns, initiator_target_map=target_map) - conn_info = dict(driver_volume_type=volume_type, - data=result_data) if self._protocol == constants.FC: + conn_info = dict(driver_volume_type=volume_type, + data=result_data) fczm_utils.remove_fc_zone(conn_info) - return conn_info + return volume.volume_attachment and len(volume.volume_attachment) > 1 @infinisdk_to_cinder_exceptions def get_volume_stats(self, refresh=False): @@ -655,7 +694,8 @@ # we need a cinder-volume-like object to map the clone by name # (which is derived from the cinder id) but the clone is internal # so there is no such object. mock one - clone = mock.Mock(id=str(volume.id) + '-internal') + clone = mock.Mock(name_id=str(volume.name_id) + '-internal', + multiattach=False, volume_attachment=[]) try: infinidat_volume = self._create_volume(volume) try: @@ -1194,3 +1234,41 @@ """ infinidat_snapshot = self._get_infinidat_snapshot(snapshot) infinidat_snapshot.clear_metadata() + + @infinisdk_to_cinder_exceptions + def update_migrated_volume(self, ctxt, volume, new_volume, + original_volume_status): + """Return model update from Infinidat for migrated volume. + + This method should rename the back-end volume name(id) on the + destination host back to its original name(id) on the source host. + + :param ctxt: The context used to run the method update_migrated_volume + :param volume: The original volume that was migrated to this backend + :param new_volume: The migration volume object that was created on + this backend as part of the migration process + :param original_volume_status: The status of the original volume + :returns: model_update to update DB with any needed changes + """ + model_update = {'_name_id': new_volume.name_id, + 'provider_location': None} + new_volume_name = self._make_volume_name(new_volume, migration=True) + new_infinidat_volume = self._get_infinidat_volume(new_volume) + self._set_cinder_object_metadata(new_infinidat_volume, volume) + volume_name = self._make_volume_name(volume, migration=True) + try: + infinidat_volume = self._get_infinidat_volume(volume) + except exception.VolumeNotFound: + LOG.debug('Source volume %s not found', volume_name) + else: + volume_pool = infinidat_volume.get_pool_name() + LOG.debug('Found source volume %s in pool %s', + volume_name, volume_pool) + return model_update + try: + new_infinidat_volume.update_name(volume_name) + except infinisdk.core.exceptions.InfiniSDKException as error: + LOG.error('Failed to rename destination volume %s -> %s: %s', + new_volume_name, volume_name, error) + return model_update + return {'_name_id': None, 'provider_location': None} diff -Nru cinder-21.1.0/cinder/volume/drivers/netapp/dataontap/nfs_base.py cinder-21.3.1/cinder/volume/drivers/netapp/dataontap/nfs_base.py --- cinder-21.1.0/cinder/volume/drivers/netapp/dataontap/nfs_base.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/volume/drivers/netapp/dataontap/nfs_base.py 2023-08-14 16:54:18.000000000 +0000 @@ -25,7 +25,6 @@ import math import os import re -import threading import time from oslo_concurrency import processutils @@ -116,6 +115,13 @@ self._handle_ems_logging, loopingcalls.ONE_HOUR) + # Add the task that periodically cleanup old expired internal + # image caching. + self.loopingcalls.add_task( + self._clean_image_cache, + self.configuration.netapp_nfs_image_cache_cleanup_interval + ) + def _delete_snapshots_marked_for_deletion(self): snapshots = self.zapi_client.get_snapshots_marked_for_deletion() for snapshot in snapshots: @@ -547,49 +553,32 @@ os.utime(src_path, None) _do_clone() - @utils.synchronized('clean_cache') - def _spawn_clean_cache_job(self): - """Spawns a clean task if not running.""" - if getattr(self, 'cleaning', None): - LOG.debug('Image cache cleaning in progress. Returning... ') - return - else: - # Set cleaning to True - self.cleaning = True - t = threading.Timer(0, self._clean_image_cache) - t.start() - def _clean_image_cache(self): """Clean the image cache files in cache of space crunch.""" - try: - LOG.debug('Image cache cleaning in progress.') - thres_size_perc_start = ( - self.configuration.thres_avl_size_perc_start) - thres_size_perc_stop = self.configuration.thres_avl_size_perc_stop - for share in getattr(self, '_mounted_shares', []): - try: - total_size, total_avl = self._get_capacity_info(share) - avl_percent = int((float(total_avl) / total_size) * 100) - if avl_percent <= thres_size_perc_start: - LOG.info('Cleaning cache for share %s.', share) - eligible_files = self._find_old_cache_files(share) - threshold_size = int( - (thres_size_perc_stop * total_size) / 100) - bytes_to_free = int(threshold_size - total_avl) - LOG.debug('Files to be queued for deletion %s', - eligible_files) - self._delete_files_till_bytes_free( - eligible_files, share, bytes_to_free) - else: - continue - except Exception as e: - LOG.warning('Exception during cache cleaning' - ' %(share)s. Message - %(ex)s', - {'share': share, 'ex': e}) + LOG.debug('Image cache cleaning in progress.') + thres_size_perc_start = ( + self.configuration.thres_avl_size_perc_start) + thres_size_perc_stop = self.configuration.thres_avl_size_perc_stop + for share in self._mounted_shares: + try: + total_size, total_avl = self._get_capacity_info(share) + avl_percent = int((float(total_avl) / total_size) * 100) + if avl_percent <= thres_size_perc_start: + LOG.info('Cleaning cache for share %s.', share) + eligible_files = self._find_old_cache_files(share) + threshold_size = int( + (thres_size_perc_stop * total_size) / 100) + bytes_to_free = int(threshold_size - total_avl) + LOG.debug('Files to be queued for deletion %s', + eligible_files) + self._delete_files_till_bytes_free( + eligible_files, share, bytes_to_free) + else: continue - finally: - LOG.debug('Image cache cleaning done.') - self.cleaning = False + except Exception as e: + LOG.warning('Exception during cache cleaning' + ' %(share)s. Message - %(ex)s', + {'share': share, 'ex': e}) def _shortlist_del_eligible_files(self, share, old_files): """Prepares list of eligible files to be deleted from cache.""" diff -Nru cinder-21.1.0/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py cinder-21.3.1/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py --- cinder-21.1.0/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/volume/drivers/netapp/dataontap/nfs_cmode.py 2023-08-14 16:54:18.000000000 +0000 @@ -337,7 +337,6 @@ # Used for service state report data['replication_enabled'] = self.replication_enabled - self._spawn_clean_cache_job() self._stats = data def _get_pool_stats(self, filter_function=None, goodness_function=None): diff -Nru cinder-21.1.0/cinder/volume/drivers/netapp/options.py cinder-21.3.1/cinder/volume/drivers/netapp/options.py --- cinder-21.1.0/cinder/volume/drivers/netapp/options.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/volume/drivers/netapp/options.py 2023-08-14 16:54:18.000000000 +0000 @@ -122,6 +122,11 @@ 'provisioning of block storage volumes should occur.')), ] netapp_img_cache_opts = [ + cfg.IntOpt('netapp_nfs_image_cache_cleanup_interval', + default=600, + min=60, + help=('Sets time in seconds between NFS image cache ' + 'cleanup tasks.')), cfg.IntOpt('thres_avl_size_perc_start', default=20, help=('If the percentage of available space for an NFS share ' diff -Nru cinder-21.1.0/cinder/volume/drivers/pure.py cinder-21.3.1/cinder/volume/drivers/pure.py --- cinder-21.1.0/cinder/volume/drivers/pure.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/volume/drivers/pure.py 2023-08-14 16:54:18.000000000 +0000 @@ -145,6 +145,7 @@ ERR_MSG_ALREADY_BELONGS = "already belongs to" ERR_MSG_EXISTING_CONNECTIONS = "cannot be deleted due to existing connections" ERR_MSG_ALREADY_IN_USE = "already in use" +ERR_MSG_ARRAY_LIMIT = "limit reached" EXTRA_SPECS_REPL_ENABLED = "replication_enabled" EXTRA_SPECS_REPL_TYPE = "replication_type" @@ -285,27 +286,32 @@ uniform = strutils.bool_from_string( replication_device.get("uniform", False)) - target_array = self._get_flasharray( - san_ip, - api_token, - verify_https=verify_https, - ssl_cert_path=ssl_cert_path - ) + try: + target_array = self._get_flasharray( + san_ip, + api_token, + verify_https=verify_https, + ssl_cert_path=ssl_cert_path + ) - target_array_info = target_array.get() - target_array.array_name = target_array_info["array_name"] - target_array.array_id = target_array_info["id"] - target_array.replication_type = repl_type - target_array.backend_id = backend_id - target_array.uniform = uniform - - LOG.info("Added secondary array: backend_id='%s', name='%s'," - " id='%s', type='%s', uniform='%s'", - target_array.backend_id, - target_array.array_name, - target_array.array_id, - target_array.replication_type, - target_array.uniform) + target_array_info = target_array.get() + target_array.array_name = target_array_info["array_name"] + target_array.array_id = target_array_info["id"] + target_array.replication_type = repl_type + target_array.backend_id = backend_id + target_array.uniform = uniform + + LOG.info("Added secondary array: backend_id='%s'," + " name='%s', id='%s', type='%s', uniform='%s'", + target_array.backend_id, + target_array.array_name, + target_array.array_id, + target_array.replication_type, + target_array.uniform) + except purestorage.PureError as err: + LOG.warning("Failed to set up secondary array with " + "message: %(msg)s", {"msg": err.reason}) + continue self._replication_target_arrays.append(target_array) if repl_type == REPLICATION_TYPE_SYNC: @@ -365,38 +371,44 @@ # Raises PureDriverException if unable to connect and PureHTTPError # if unable to authenticate. - self._array = self._get_flasharray( - self.configuration.san_ip, - api_token=self.configuration.pure_api_token, - verify_https=self.configuration.driver_ssl_cert_verify, - ssl_cert_path=self.configuration.driver_ssl_cert_path - ) - - array_info = self._array.get() - if version.parse(array_info["version"]) < version.parse( - '5.3.0' - ): - msg = _("FlashArray Purity version less than 5.3.0 unsupported." - " Please upgrade your backend to a supported version.") - raise PureDriverException(msg) + try: + self._array = self._get_flasharray( + self.configuration.san_ip, + api_token=self.configuration.pure_api_token, + verify_https=self.configuration.driver_ssl_cert_verify, + ssl_cert_path=self.configuration.driver_ssl_cert_path + ) - self._array.array_name = array_info["array_name"] - self._array.array_id = array_info["id"] - self._array.replication_type = None - self._array.backend_id = self._backend_name - self._array.preferred = True - self._array.uniform = True - - LOG.info("Primary array: backend_id='%s', name='%s', id='%s'", - self.configuration.config_group, - self._array.array_name, - self._array.array_id) + array_info = self._array.get() + if version.parse(array_info["version"]) < version.parse( + '5.3.0' + ): + msg = _("FlashArray Purity version less than 5.3.0 " + "unsupported. Please upgrade your backend to " + "a supported version.") + raise PureDriverException(msg) + + self._array.array_name = array_info["array_name"] + self._array.array_id = array_info["id"] + self._array.replication_type = None + self._array.backend_id = self._backend_name + self._array.preferred = True + self._array.uniform = True + + LOG.info("Primary array: backend_id='%s', name='%s', id='%s'", + self.configuration.config_group, + self._array.array_name, + self._array.array_id) + except purestorage.PureError as err: + LOG.warning("self.do_setup failed to set up primary array with" + " message: %(msg)s", {"msg": err.reason}) self.do_setup_replication() # If we have failed over at some point we need to adjust our current # array based on the one that we have failed over to - if (self._active_backend_id is not None and + if (self._array is not None and + self._active_backend_id is not None and self._active_backend_id != self._array.backend_id): for secondary_array in self._replication_target_arrays: if secondary_array.backend_id == self._active_backend_id: @@ -415,7 +427,7 @@ # Only set this up on sync rep arrays self._setup_replicated_pods( - self._get_current_array(), + self._get_current_array(True), self._active_cluster_target_arrays, self._replication_pod_name ) @@ -423,7 +435,7 @@ # Even if the array is configured for sync rep set it # up to handle async too self._setup_replicated_pgroups( - self._get_current_array(), + self._get_current_array(True), self._replication_target_arrays, self._replication_pg_name, self._replication_interval, @@ -551,7 +563,7 @@ except purestorage.PureError as err: with excutils.save_and_reraise_exception(): LOG.error("Failed to add volume %s to pgroup, removing volume", - err) + err.reason) array.destroy_volume(purity_vol_name) array.eradicate_volume(purity_vol_name) @@ -2198,8 +2210,9 @@ except purestorage.PureHTTPError as err: with excutils.save_and_reraise_exception() as ctxt: if err.code == 400 and ( - ERR_MSG_ALREADY_EXISTS - in err.text): + ERR_MSG_ALREADY_EXISTS in err.text + or ERR_MSG_ARRAY_LIMIT in err.text + ): ctxt.reraise = False LOG.info("Skipping add array %(target_array)s to pod" " %(pod_name)s since it's already added.", @@ -2527,7 +2540,29 @@ wwn = '3624a9370' + volume_info['serial'] return wwn.lower() - def _get_current_array(self): + def _get_current_array(self, init=False): + if not init and self._is_active_cluster_enabled: + for target_array in self._active_cluster_target_arrays: + try: + LOG.info("Checking target array %s...", + target_array.array_name) + status_ok = False + pod_info = target_array.get_pod(self._replication_pod_name) + for pod_array in pod_info['arrays']: + if pod_array['array_id'] == target_array.array_id: + if pod_array['status'] == 'online': + status_ok = True + break + if not status_ok: + LOG.warning("Target array is offline. Volume " + "replication in unknown state. Check " + "replication links and array state.") + except purestorage.PureError as err: + LOG.warning("self.get_pod failed with" + " message: %(msg)s", {"msg": err}) + raise purestorage.PureError('No functional arrays ' + 'available') + return self._array def _set_current_array(self, array): @@ -2813,7 +2848,13 @@ target_luns = [] target_wwns = [] for array in target_arrays: - connection = self._connect(array, pure_vol_name, connector) + try: + connection = self._connect(array, pure_vol_name, connector) + except purestorage.PureError as err: + # Swallow any exception, just warn and continue + LOG.warning("self._connect failed with" + " message: %(msg)s", {"msg": err.reason}) + continue array_wwns = self._get_array_wwns(array) for wwn in array_wwns: target_wwns.append(wwn) diff -Nru cinder-21.1.0/cinder/volume/drivers/rbd.py cinder-21.3.1/cinder/volume/drivers/rbd.py --- cinder-21.1.0/cinder/volume/drivers/rbd.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/volume/drivers/rbd.py 2023-08-14 16:54:18.000000000 +0000 @@ -81,7 +81,7 @@ 'dependency from volume to snapshot'), cfg.StrOpt('rbd_secret_uuid', help='The libvirt uuid of the secret for the rbd_user ' - 'volumes'), + 'volumes. Defaults to the cluster FSID.'), cfg.IntOpt('rbd_max_clone_depth', default=5, help='Maximum number of nested volume clones that are ' @@ -424,6 +424,17 @@ """Performs initialization steps that could raise exceptions.""" self._do_setup_replication() self._active_config = self._get_target_config(self._active_backend_id) + self._set_default_secret_uuid() + + def _set_default_secret_uuid(self): + # Set secret_uuid to the cluster FSID if missing, should only happen + # with the primary/default configuration + if not self._active_config['secret_uuid']: + # self._active_config must be set before this call + fsid = self._get_fsid() + self._active_config['secret_uuid'] = fsid + LOG.info('Secret UUID defaulting to cluster FSID: %s', fsid) + self.configuration.set_default('rbd_secret_uuid', fsid) def _do_setup_replication(self) -> None: replication_devices = self.configuration.safe_get( @@ -711,12 +722,15 @@ total_capacity: float free_capacity: float + + # In Nautilus bytes_used was renamed to stored + bytes_used = pool_stats.get('stored', pool_stats['bytes_used']) quota_outbuf = encodeutils.safe_decode(quota_outbuf) bytes_quota = json.loads(quota_outbuf)['quota_max_bytes'] # With quota the total is the quota limit and free is quota - used if bytes_quota: total_capacity = bytes_quota - free_capacity = max(min(total_capacity - pool_stats['bytes_used'], + free_capacity = max(min(total_capacity - bytes_used, pool_stats['max_avail']), 0) # Without quota free is pools max available and total is global size @@ -726,7 +740,7 @@ # If we want dynamic total capacity (default behavior) if self.configuration.safe_get('report_dynamic_total_capacity'): - total_capacity = free_capacity + pool_stats['bytes_used'] + total_capacity = free_capacity + bytes_used free_capacity = round((float(free_capacity) / units.Gi), 2) total_capacity = round((float(total_capacity) / units.Gi), 2) @@ -1719,6 +1733,7 @@ self._active_backend_id = secondary_id self._active_config = remote + self._set_default_secret_uuid() LOG.info('RBD driver failover completion completed.') def failover_host(self, diff -Nru cinder-21.1.0/cinder/volume/flows/api/create_volume.py cinder-21.3.1/cinder/volume/flows/api/create_volume.py --- cinder-21.1.0/cinder/volume/flows/api/create_volume.py 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/cinder/volume/flows/api/create_volume.py 2023-08-14 16:54:18.000000000 +0000 @@ -444,8 +444,7 @@ cgsnapshot, group, group_snapshot, - backup: Optional[dict], - multiattach: bool = False) -> dict[str, Any]: + backup: Optional[dict]) -> dict[str, Any]: utils.check_exclusive_options(snapshot=snapshot, imageRef=image_id, @@ -493,11 +492,7 @@ volume_type = objects.VolumeType.get_by_name_or_id( context, volume_type_id) extra_specs = volume_type.get('extra_specs', {}) - # NOTE(tommylikehu): Although the parameter `multiattach` from - # create volume API is deprecated now, we still need to consider - # it when multiattach is not enabled in volume type. - multiattach = (extra_specs.get( - 'multiattach', '') == ' True' or multiattach) + multiattach = (extra_specs.get('multiattach', '') == ' True') if multiattach and encryption_key_id: msg = _('Multiattach cannot be used with encrypted volumes.') raise exception.InvalidVolume(reason=msg) @@ -914,8 +909,7 @@ availability_zones, rebind={'size': 'raw_size', 'availability_zone': 'raw_availability_zone', - 'volume_type': 'raw_volume_type', - 'multiattach': 'raw_multiattach'})) + 'volume_type': 'raw_volume_type'})) api_flow.add(QuotaReserveTask(), EntryCreateTask(), QuotaCommitTask()) diff -Nru cinder-21.1.0/debian/changelog cinder-21.3.1/debian/changelog --- cinder-21.1.0/debian/changelog 2023-05-12 10:38:22.000000000 +0000 +++ cinder-21.3.1/debian/changelog 2024-06-21 07:19:56.000000000 +0000 @@ -1,3 +1,18 @@ +cinder (2:21.3.1-1+deb12u1) bookworm-security; urgency=high + + * New upstream release. + * Drop CVE-2023-2088_Reject_unsafe_delete_attachment_calls.patch applied + upstream. + * Blacklist RBDISCSITestCase.test_unsupported_client_version(). + * Add add-params-thin_provisioning-equal-one.patch. + * CVE-2024-32498: Arbitrary file access through custom QCOW2 external data. + Add upstream patch (Closes: #1074763): + - cve-2024-32498-cinder-stable-2023.1.patch + * Build-depends on qemu-utils. + * Correctly calls manage_glance_api_servers() in config script. + + -- Thomas Goirand Fri, 21 Jun 2024 09:19:56 +0200 + cinder (2:21.1.0-3) unstable; urgency=medium * CVE-2023-2088: Unauthorized volume access through deleted volume diff -Nru cinder-21.1.0/debian/cinder-common.config.in cinder-21.3.1/debian/cinder-common.config.in --- cinder-21.1.0/debian/cinder-common.config.in 2023-05-12 10:38:22.000000000 +0000 +++ cinder-21.3.1/debian/cinder-common.config.in 2024-06-21 07:19:56.000000000 +0000 @@ -58,6 +58,7 @@ fi fi fi +manage_glance_api_servers db_input high cinder/volume_group || true db_go manage_cinder_my_ip diff -Nru cinder-21.1.0/debian/control cinder-21.3.1/debian/control --- cinder-21.1.0/debian/control 2023-05-12 10:38:22.000000000 +0000 +++ cinder-21.3.1/debian/control 2024-06-21 07:19:56.000000000 +0000 @@ -98,6 +98,7 @@ python3-tz, python3-webob, python3-zstd, + qemu-utils, subunit, Standards-Version: 4.4.1 Vcs-Browser: https://salsa.debian.org/openstack-team/services/cinder diff -Nru cinder-21.1.0/debian/patches/add-params-thin_provisioning-equal-one.patch cinder-21.3.1/debian/patches/add-params-thin_provisioning-equal-one.patch --- cinder-21.1.0/debian/patches/add-params-thin_provisioning-equal-one.patch 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/debian/patches/add-params-thin_provisioning-equal-one.patch 2024-06-21 07:19:56.000000000 +0000 @@ -0,0 +1,17 @@ +Description: Add "params thin_provisioning=1" in tgt export + Add 'params thin_provisioning=1' in iscsi volume export + to allow fstrim to work with the lvm backend. +Author: Thomas Goirand +Forwarded: https://review.opendev.org/c/openstack/cinder/+/900513 +Last-Update: 2023-12-04 + +--- cinder-23.0.0.orig/cinder/volume/targets/tgt.py ++++ cinder-23.0.0/cinder/volume/targets/tgt.py +@@ -42,6 +42,7 @@ class TgtAdm(iscsi.ISCSITarget): + %(chap_auth)s + %(target_flags)s + write-cache %(write_cache)s ++ params thin_provisioning=1 + + """) + diff -Nru cinder-21.1.0/debian/patches/CVE-2023-2088_Reject_unsafe_delete_attachment_calls.patch cinder-21.3.1/debian/patches/CVE-2023-2088_Reject_unsafe_delete_attachment_calls.patch --- cinder-21.1.0/debian/patches/CVE-2023-2088_Reject_unsafe_delete_attachment_calls.patch 2023-05-12 10:38:22.000000000 +0000 +++ cinder-21.3.1/debian/patches/CVE-2023-2088_Reject_unsafe_delete_attachment_calls.patch 1970-01-01 00:00:00.000000000 +0000 @@ -1,986 +0,0 @@ -From cb4682fb836912225c5da1536108a0d05fd5c46e Mon Sep 17 00:00:00 2001 -From: Gorka Eguileor -Date: Thu, 16 Feb 2023 15:57:15 +0100 -Subject: [PATCH] Reject unsafe delete attachment calls - -Due to how the Linux SCSI kernel driver works there are some storage -systems, such as iSCSI with shared targets, where a normal user can -access other projects' volume data connected to the same compute host -using the attachments REST API. - -This affects both single and multi-pathed connections. - -To prevent users from doing this, unintentionally or maliciously, -cinder-api will now reject some delete attachment requests that are -deemed unsafe. - -Cinder will process the delete attachment request normally in the -following cases: - -- The request comes from an OpenStack service that is sending the - service token that has one of the roles in `service_token_roles`. -- Attachment doesn't have an instance_uuid value -- The instance for the attachment doesn't exist in Nova -- According to Nova the volume is not connected to the instance -- Nova is not using this attachment record - -There are 3 operations in the actions REST API endpoint that can be used -for an attack: - -- `os-terminate_connection`: Terminate volume attachment available at -- `os-detach`: Detach a volume -- `os-force_detach`: Force detach a volume - -In this endpoint we just won't allow anything that is not coming from a -service. This should not be a problem because: - -- Cinder backup doesn't use the REST API but RPC calls via RabbitMQ -- Glance doesn't use this interface - -Checking whether it's a service or not is done at the cinder-api level -by checking that the service user that made the call has at least one of -the roles in the `service_token_roles` configuration. These roles are -retrieved from keystone by the keystone middleware using the value of -the "X-Service-Token" header. - -If Cinder is configured with `service_token_roles_required = true` and -an attacker provides non-service valid credentials the service will -return a 401 error, otherwise it'll return 409 as if a normal user had -made the call without the service token. - -Closes-Bug: #2004555 -Change-Id: I612905a1bf4a1706cce913c0d8a6df7a240d599a -(cherry picked from commit 6df1839bdf288107c600b3e53dff7593a6d4c161) -Conflicts: - cinder/exception.py -(cherry picked from commit dd6010a9f7bf8cbe0189992f0848515321781747) ---- - -Index: cinder/api-ref/source/v3/attachments.inc -=================================================================== ---- cinder.orig/api-ref/source/v3/attachments.inc -+++ cinder/api-ref/source/v3/attachments.inc -@@ -41,6 +41,20 @@ Delete attachment - - Deletes an attachment. - -+For security reasons (see bug `#2004555 -+`_) the Block Storage API rejects -+REST API calls manually made from users with a 409 status code if there is a -+Nova instance currently using the attachment, which happens when all the -+following conditions are met: -+ -+- Attachment has an instance uuid -+- VM exists in Nova -+- Instance has the volume attached -+- Attached volume in instance is using the attachment -+ -+Calls coming from other OpenStack services (like the Compute Service) are -+always accepted. -+ - Available starting in the 3.27 microversion. - - Response codes -@@ -54,6 +68,7 @@ Response codes - - - 400 - - 404 -+ - 409 - - - Request -Index: cinder/api-ref/source/v3/volumes-v3-volumes-actions.inc -=================================================================== ---- cinder.orig/api-ref/source/v3/volumes-v3-volumes-actions.inc -+++ cinder/api-ref/source/v3/volumes-v3-volumes-actions.inc -@@ -337,6 +337,21 @@ Preconditions - - - Volume status must be ``in-use``. - -+For security reasons (see bug `#2004555 -+`_), regardless of the policy -+defaults, the Block Storage API rejects REST API calls manually made from -+users with a 409 status code if completing the request could pose a risk, which -+happens if all of these happen: -+ -+- The request comes from a user -+- There's an instance uuid in provided attachment or in the volume's attachment -+- VM exists in Nova -+- Instance has the volume attached -+- Attached volume in instance is using the attachment -+ -+Calls coming from other OpenStack services (like the Compute Service) are -+always accepted. -+ - Response codes - -------------- - -@@ -344,6 +359,9 @@ Response codes - - - 202 - -+.. rest_status_code:: error ../status.yaml -+ -+ - 409 - - Request - ------- -@@ -415,6 +433,21 @@ perform this operation. Cloud providers - through the ``volume_extension:volume_admin_actions:force_detach`` rule in - the policy configuration file. - -+For security reasons (see bug `#2004555 -+`_), regardless of the policy -+defaults, the Block Storage API rejects REST API calls manually made from -+users with a 409 status code if completing the request could pose a risk, which -+happens if all of these happen: -+ -+- The request comes from a user -+- There's an instance uuid in provided attachment or in the volume's attachment -+- VM exists in Nova -+- Instance has the volume attached -+- Attached volume in instance is using the attachment -+ -+Calls coming from other OpenStack services (like the Compute Service) are -+always accepted. -+ - Response codes - -------------- - -@@ -422,6 +455,9 @@ Response codes - - - 202 - -+.. rest_status_code:: error ../status.yaml -+ -+ - 409 - - Request - ------- -@@ -883,6 +919,22 @@ Preconditions - - - Volume status must be ``in-use``. - -+For security reasons (see bug `#2004555 -+`_), regardless of the policy -+defaults, the Block Storage API rejects REST API calls manually made from -+users with a 409 status code if completing the request could pose a risk, which -+happens if all of these happen: -+ -+- The request comes from a user -+- There's an instance uuid in the volume's attachment -+- VM exists in Nova -+- Instance has the volume attached -+- Attached volume in instance is using the attachment -+ -+Calls coming from other OpenStack services (like the Compute Service) are -+always accepted. -+ -+ - Response codes - -------------- - -@@ -890,6 +942,9 @@ Response codes - - - 202 - -+.. rest_status_code:: error ../status.yaml -+ -+ - 409 - - Request - ------- -Index: cinder/cinder/compute/nova.py -=================================================================== ---- cinder.orig/cinder/compute/nova.py -+++ cinder/cinder/compute/nova.py -@@ -133,6 +133,7 @@ def novaclient(context, privileged_user= - - class API(base.Base): - """API for interacting with novaclient.""" -+ NotFound = nova_exceptions.NotFound - - def __init__(self): - self.message_api = message_api.API() -@@ -237,3 +238,9 @@ class API(base.Base): - resource_uuid=volume_id, - detail=message_field.Detail.REIMAGE_VOLUME_FAILED) - return result -+ -+ @staticmethod -+ def get_server_volume(context, server_id, volume_id): -+ # Use microversion that includes attachment_id -+ nova = novaclient(context, api_version='2.89') -+ return nova.volumes.get_server_volume(server_id, volume_id) -Index: cinder/cinder/exception.py -=================================================================== ---- cinder.orig/cinder/exception.py -+++ cinder/cinder/exception.py -@@ -1092,3 +1092,10 @@ class DriverInitiatorDataExists(Duplicat - "Driver initiator data for initiator '%(initiator)s' and backend " - "'%(namespace)s' with key '%(key)s' already exists." - ) -+ -+ -+class ConflictNovaUsingAttachment(CinderException): -+ message = _("Detach volume from instance %(instance_id)s using the " -+ "Compute API") -+ code = 409 -+ safe = True -Index: cinder/cinder/tests/unit/api/contrib/test_admin_actions.py -=================================================================== ---- cinder.orig/cinder/tests/unit/api/contrib/test_admin_actions.py -+++ cinder/cinder/tests/unit/api/contrib/test_admin_actions.py -@@ -1037,6 +1037,8 @@ class AdminActionsAttachDetachTest(BaseA - super(AdminActionsAttachDetachTest, self).setUp() - # start service to handle rpc messages for attach requests - self.svc = self.start_service('volume', host='test') -+ self.mock_deletion_allowed = self.mock_object( -+ volume_api.API, 'attachment_deletion_allowed', return_value=None) - - def tearDown(self): - self.svc.stop() -@@ -1092,6 +1094,16 @@ class AdminActionsAttachDetachTest(BaseA - admin_metadata = volume.admin_metadata - self.assertEqual(1, len(admin_metadata)) - self.assertEqual('False', admin_metadata['readonly']) -+ # One call is for the terminate_connection and the other is for the -+ # detach -+ self.assertEqual(2, self.mock_deletion_allowed.call_count) -+ self.mock_deletion_allowed.assert_has_calls( -+ [mock.call(self.ctx, None, mock.ANY), -+ mock.call(self.ctx, attachment['id'], mock.ANY)]) -+ for i in (0, 1): -+ self.assertIsInstance( -+ self.mock_deletion_allowed.call_args_list[i][0][2], -+ objects.Volume) - - def test_force_detach_host_attached_volume(self): - # current status is available -@@ -1143,6 +1155,16 @@ class AdminActionsAttachDetachTest(BaseA - admin_metadata = volume['admin_metadata'] - self.assertEqual(1, len(admin_metadata)) - self.assertEqual('False', admin_metadata['readonly']) -+ # One call is for the terminate_connection and the other is for the -+ # detach -+ self.assertEqual(2, self.mock_deletion_allowed.call_count) -+ self.mock_deletion_allowed.assert_has_calls( -+ [mock.call(self.ctx, None, mock.ANY), -+ mock.call(self.ctx, attachment['id'], mock.ANY)]) -+ for i in (0, 1): -+ self.assertIsInstance( -+ self.mock_deletion_allowed.call_args_list[i][0][2], -+ objects.Volume) - - def test_volume_force_detach_raises_remote_error(self): - # current status is available -@@ -1186,6 +1208,10 @@ class AdminActionsAttachDetachTest(BaseA - resp = req.get_response(app()) - self.assertEqual(HTTPStatus.BAD_REQUEST, resp.status_int) - -+ self.mock_deletion_allowed.assert_called_once_with( -+ self.ctx, None, volume) -+ self.mock_deletion_allowed.reset_mock() -+ - # test for VolumeBackendAPIException - volume_remote_error = ( - messaging.RemoteError(exc_type='VolumeBackendAPIException')) -@@ -1205,6 +1231,8 @@ class AdminActionsAttachDetachTest(BaseA - self.assertRaises(messaging.RemoteError, - req.get_response, - app()) -+ self.mock_deletion_allowed.assert_called_once_with( -+ self.ctx, None, volume) - - def test_volume_force_detach_raises_db_error(self): - # In case of DB error 500 error code is returned to user -@@ -1250,6 +1278,8 @@ class AdminActionsAttachDetachTest(BaseA - self.assertRaises(messaging.RemoteError, - req.get_response, - app()) -+ self.mock_deletion_allowed.assert_called_once_with( -+ self.ctx, None, volume) - - def test_volume_force_detach_missing_connector(self): - # current status is available -@@ -1290,6 +1320,8 @@ class AdminActionsAttachDetachTest(BaseA - # make request - resp = req.get_response(app()) - self.assertEqual(HTTPStatus.ACCEPTED, resp.status_int) -+ self.mock_deletion_allowed.assert_called_once_with( -+ self.ctx, None, volume) - - def test_attach_in_used_volume_by_instance(self): - """Test that attaching to an in-use volume fails.""" -Index: cinder/cinder/tests/unit/api/v3/test_attachments.py -=================================================================== ---- cinder.orig/cinder/tests/unit/api/v3/test_attachments.py -+++ cinder/cinder/tests/unit/api/v3/test_attachments.py -@@ -159,6 +159,8 @@ class AttachmentsAPITestCase(test.TestCa - @ddt.data('reserved', 'attached') - @mock.patch.object(volume_rpcapi.VolumeAPI, 'attachment_delete') - def test_delete_attachment(self, status, mock_delete): -+ self.patch('cinder.volume.api.API.attachment_deletion_allowed', -+ return_value=None) - volume1 = self._create_volume(display_name='fake_volume_1', - project_id=fake.PROJECT_ID) - attachment = self._create_attachment( -Index: cinder/cinder/tests/unit/attachments/test_attachments_api.py -=================================================================== ---- cinder.orig/cinder/tests/unit/attachments/test_attachments_api.py -+++ cinder/cinder/tests/unit/attachments/test_attachments_api.py -@@ -12,12 +12,14 @@ - - from unittest import mock - -+from cinder.compute import nova - from cinder import context - from cinder import db - from cinder import exception - from cinder import objects - from cinder.tests.unit.api.v2 import fakes as v2_fakes - from cinder.tests.unit import fake_constants as fake -+from cinder.tests.unit import fake_volume - from cinder.tests.unit import test - from cinder.tests.unit import utils as tests_utils - from cinder.volume import api as volume_api -@@ -78,10 +80,13 @@ class AttachmentManagerTestCase(test.Tes - attachment.id) - self.assertEqual(connection_info, new_attachment.connection_info) - -+ @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') - @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete') - def test_attachment_delete_reserved(self, -- mock_rpc_attachment_delete): -+ mock_rpc_attachment_delete, -+ mock_allowed): - """Test attachment_delete with reserved.""" -+ mock_allowed.return_value = None - volume_params = {'status': 'available'} - - vref = tests_utils.create_volume(self.context, **volume_params) -@@ -94,18 +99,22 @@ class AttachmentManagerTestCase(test.Tes - self.assertEqual(vref.id, aref.volume_id) - self.volume_api.attachment_delete(self.context, - aobj) -+ mock_allowed.assert_called_once_with(self.context, aobj) - - # Since it's just reserved and never finalized, we should never make an - # rpc call - mock_rpc_attachment_delete.assert_not_called() - -+ @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') - @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_delete') - @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update') - def test_attachment_create_update_and_delete( - self, - mock_rpc_attachment_update, -- mock_rpc_attachment_delete): -+ mock_rpc_attachment_delete, -+ mock_allowed): - """Test attachment_delete.""" -+ mock_allowed.return_value = None - volume_params = {'status': 'available'} - connection_info = {'fake_key': 'fake_value', - 'fake_key2': ['fake_value1', 'fake_value2']} -@@ -142,6 +151,7 @@ class AttachmentManagerTestCase(test.Tes - self.volume_api.attachment_delete(self.context, - aref) - -+ mock_allowed.assert_called_once_with(self.context, aref) - mock_rpc_attachment_delete.assert_called_once_with(self.context, - aref.id, - mock.ANY) -@@ -173,10 +183,13 @@ class AttachmentManagerTestCase(test.Tes - vref.id) - self.assertEqual(2, len(vref.volume_attachment)) - -+ @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') - @mock.patch('cinder.volume.rpcapi.VolumeAPI.attachment_update') - def test_attachment_create_reserve_delete( - self, -- mock_rpc_attachment_update): -+ mock_rpc_attachment_update, -+ mock_allowed): -+ mock_allowed.return_value = None - volume_params = {'status': 'available'} - connector = { - "initiator": "iqn.1993-08.org.debian:01:cad181614cec", -@@ -211,12 +224,15 @@ class AttachmentManagerTestCase(test.Tes - # attachments reserve - self.volume_api.attachment_delete(self.context, - aref) -+ mock_allowed.assert_called_once_with(self.context, aref) - vref = objects.Volume.get_by_id(self.context, - vref.id) - self.assertEqual('reserved', vref.status) - -- def test_reserve_reserve_delete(self): -+ @mock.patch.object(volume_api.API, 'attachment_deletion_allowed') -+ def test_reserve_reserve_delete(self, mock_allowed): - """Test that we keep reserved status across multiple reserves.""" -+ mock_allowed.return_value = None - volume_params = {'status': 'available'} - - vref = tests_utils.create_volume(self.context, **volume_params) -@@ -235,6 +251,7 @@ class AttachmentManagerTestCase(test.Tes - self.assertEqual('reserved', vref.status) - self.volume_api.attachment_delete(self.context, - aref) -+ mock_allowed.assert_called_once_with(self.context, aref) - vref = objects.Volume.get_by_id(self.context, - vref.id) - self.assertEqual('reserved', vref.status) -@@ -344,3 +361,169 @@ class AttachmentManagerTestCase(test.Tes - self.context, - vref, - fake.UUID1) -+ -+ def _get_attachment(self, with_instance_id=True): -+ volume = fake_volume.fake_volume_obj(self.context, id=fake.VOLUME_ID) -+ volume.volume_attachment = objects.VolumeAttachmentList() -+ attachment = fake_volume.volume_attachment_ovo( -+ self.context, -+ volume_id=fake.VOLUME_ID, -+ instance_uuid=fake.INSTANCE_ID if with_instance_id else None, -+ connection_info='{"a": 1}') -+ attachment.volume = volume -+ return attachment -+ -+ @mock.patch('cinder.compute.nova.API.get_server_volume') -+ def test_attachment_deletion_allowed_service_call(self, mock_get_server): -+ """Service calls are never redirected.""" -+ self.context.service_roles = ['reader', 'service'] -+ attachment = self._get_attachment() -+ self.volume_api.attachment_deletion_allowed(self.context, attachment) -+ mock_get_server.assert_not_called() -+ -+ @mock.patch('cinder.compute.nova.API.get_server_volume') -+ def test_attachment_deletion_allowed_service_call_different_service_name( -+ self, mock_get_server): -+ """Service calls are never redirected and role can be different. -+ -+ In this test we support 2 different service roles, the standard service -+ and a custom one called captain_awesome, and passing the custom one -+ works as expected. -+ """ -+ self.override_config('service_token_roles', -+ ['service', 'captain_awesome'], -+ group='keystone_authtoken') -+ -+ self.context.service_roles = ['reader', 'captain_awesome'] -+ attachment = self._get_attachment() -+ self.volume_api.attachment_deletion_allowed(self.context, attachment) -+ mock_get_server.assert_not_called() -+ -+ @mock.patch('cinder.compute.nova.API.get_server_volume') -+ def test_attachment_deletion_allowed_no_instance(self, mock_get_server): -+ """Attachments with no instance id are never redirected.""" -+ attachment = self._get_attachment(with_instance_id=False) -+ self.volume_api.attachment_deletion_allowed(self.context, attachment) -+ mock_get_server.assert_not_called() -+ -+ @mock.patch('cinder.compute.nova.API.get_server_volume') -+ def test_attachment_deletion_allowed_no_conn_info(self, mock_get_server): -+ """Attachments with no connection information are never redirected.""" -+ attachment = self._get_attachment(with_instance_id=False) -+ attachment.connection_info = None -+ self.volume_api.attachment_deletion_allowed(self.context, attachment) -+ -+ mock_get_server.assert_not_called() -+ -+ def test_attachment_deletion_allowed_no_attachment(self): -+ """For users don't allow operation with no attachment reference.""" -+ self.assertRaises(exception.ConflictNovaUsingAttachment, -+ self.volume_api.attachment_deletion_allowed, -+ self.context, None) -+ -+ @mock.patch('cinder.objects.VolumeAttachment.get_by_id', -+ side_effect=exception.VolumeAttachmentNotFound()) -+ def test_attachment_deletion_allowed_attachment_id_not_found(self, -+ mock_get): -+ """For users don't allow if attachment cannot be found.""" -+ attachment = self._get_attachment(with_instance_id=False) -+ attachment.connection_info = None -+ self.assertRaises(exception.ConflictNovaUsingAttachment, -+ self.volume_api.attachment_deletion_allowed, -+ self.context, fake.ATTACHMENT_ID) -+ mock_get.assert_called_once_with(self.context, fake.ATTACHMENT_ID) -+ -+ def test_attachment_deletion_allowed_volume_no_attachments(self): -+ """For users allow if volume has no attachments.""" -+ volume = tests_utils.create_volume(self.context) -+ self.volume_api.attachment_deletion_allowed(self.context, None, volume) -+ -+ def test_attachment_deletion_allowed_multiple_attachment(self): -+ """For users don't allow if volume has multiple attachments.""" -+ attachment = self._get_attachment() -+ volume = attachment.volume -+ volume.volume_attachment = objects.VolumeAttachmentList( -+ objects=[attachment, attachment]) -+ self.assertRaises(exception.ConflictNovaUsingAttachment, -+ self.volume_api.attachment_deletion_allowed, -+ self.context, None, volume) -+ -+ @mock.patch('cinder.compute.nova.API.get_server_volume') -+ def test_attachment_deletion_allowed_vm_not_found(self, mock_get_server): -+ """Don't reject if instance doesn't exist""" -+ mock_get_server.side_effect = nova.API.NotFound(404) -+ attachment = self._get_attachment() -+ self.volume_api.attachment_deletion_allowed(self.context, attachment) -+ -+ mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, -+ fake.VOLUME_ID) -+ -+ @mock.patch('cinder.compute.nova.API.get_server_volume') -+ def test_attachment_deletion_allowed_attachment_from_volume( -+ self, mock_get_server): -+ """Don't reject if instance doesn't exist""" -+ mock_get_server.side_effect = nova.API.NotFound(404) -+ attachment = self._get_attachment() -+ volume = attachment.volume -+ volume.volume_attachment = objects.VolumeAttachmentList( -+ objects=[attachment]) -+ self.volume_api.attachment_deletion_allowed(self.context, None, volume) -+ -+ mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, -+ volume.id) -+ -+ @mock.patch('cinder.objects.VolumeAttachment.get_by_id') -+ def test_attachment_deletion_allowed_mismatched_volume_and_attach_id( -+ self, mock_get_attatchment): -+ """Reject if volume and attachment don't match.""" -+ attachment = self._get_attachment() -+ volume = attachment.volume -+ volume.volume_attachment = objects.VolumeAttachmentList( -+ objects=[attachment]) -+ attachment2 = self._get_attachment() -+ attachment2.volume_id = attachment.volume.id = fake.VOLUME2_ID -+ self.assertRaises(exception.InvalidInput, -+ self.volume_api.attachment_deletion_allowed, -+ self.context, attachment2.id, volume) -+ mock_get_attatchment.assert_called_once_with(self.context, -+ attachment2.id) -+ -+ @mock.patch('cinder.objects.VolumeAttachment.get_by_id') -+ @mock.patch('cinder.compute.nova.API.get_server_volume') -+ def test_attachment_deletion_allowed_not_found_attachment_id( -+ self, mock_get_server, mock_get_attachment): -+ """Don't reject if instance doesn't exist""" -+ mock_get_server.side_effect = nova.API.NotFound(404) -+ mock_get_attachment.return_value = self._get_attachment() -+ -+ self.volume_api.attachment_deletion_allowed(self.context, -+ fake.ATTACHMENT_ID) -+ -+ mock_get_attachment.assert_called_once_with(self.context, -+ fake.ATTACHMENT_ID) -+ -+ mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, -+ fake.VOLUME_ID) -+ -+ @mock.patch('cinder.compute.nova.API.get_server_volume') -+ def test_attachment_deletion_allowed_mismatch_id(self, mock_get_server): -+ """Don't reject if attachment id on nova doesn't match""" -+ mock_get_server.return_value.attachment_id = fake.ATTACHMENT2_ID -+ attachment = self._get_attachment() -+ self.volume_api.attachment_deletion_allowed(self.context, attachment) -+ -+ mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, -+ fake.VOLUME_ID) -+ -+ @mock.patch('cinder.compute.nova.API.get_server_volume') -+ def test_attachment_deletion_allowed_user_call_fails(self, -+ mock_get_server): -+ """Fail user calls""" -+ attachment = self._get_attachment() -+ mock_get_server.return_value.attachment_id = attachment.id -+ self.assertRaises(exception.ConflictNovaUsingAttachment, -+ self.volume_api.attachment_deletion_allowed, -+ self.context, attachment) -+ -+ mock_get_server.assert_called_once_with(self.context, fake.INSTANCE_ID, -+ fake.VOLUME_ID) -Index: cinder/cinder/tests/unit/policies/test_attachments.py -=================================================================== ---- cinder.orig/cinder/tests/unit/policies/test_attachments.py -+++ cinder/cinder/tests/unit/policies/test_attachments.py -@@ -62,6 +62,9 @@ class AttachmentsPolicyTest(base.BasePol - - self.api_path = '/v3/%s/attachments' % (self.project_id) - self.api_version = mv.NEW_ATTACH -+ self.mock_is_service = self.patch( -+ 'cinder.volume.api.API.is_service_request', -+ return_value=True) - - def _initialize_connection(self, volume, connector): - return {'data': connector} -Index: cinder/cinder/tests/unit/policies/test_volume_actions.py -=================================================================== ---- cinder.orig/cinder/tests/unit/policies/test_volume_actions.py -+++ cinder/cinder/tests/unit/policies/test_volume_actions.py -@@ -89,6 +89,9 @@ class VolumeActionsPolicyTest(base.BaseP - self._initialize_connection) - self.api_path = '/v3/%s/volumes' % (self.project_id) - self.api_version = mv.BASE_VERSION -+ self.mock_is_service = self.patch( -+ 'cinder.volume.api.API.is_service_request', -+ return_value=True) - - def _initialize_connection(self, volume, connector): - return {'data': connector} -@@ -946,6 +949,7 @@ class VolumeProtectionTests(test_base.Ci - self.assertEqual(HTTPStatus.ACCEPTED, response.status_int) - - body = {"os-detach": {}} -+ # Detach for user call succeeds because the volume has no attachments - response = self._get_request_response(admin_context, path, 'POST', - body=body) - self.assertEqual(HTTPStatus.ACCEPTED, response.status_int) -@@ -966,6 +970,7 @@ class VolumeProtectionTests(test_base.Ci - body=body) - self.assertEqual(HTTPStatus.ACCEPTED, response.status_int) - -+ # Succeeds for a user call because there are no attachments - body = {"os-detach": {}} - response = self._get_request_response(user_context, path, 'POST', - body=body) -@@ -1062,6 +1067,7 @@ class VolumeProtectionTests(test_base.Ci - 'terminate_connection') - def test_admin_can_initialize_terminate_conn(self, mock_t, mock_i): - admin_context = self.admin_context -+ admin_context.service_roles = ['service'] - - volume = self._create_fake_volume(admin_context) - path = '/v3/%(project_id)s/volumes/%(volume_id)s/action' % { -@@ -1084,6 +1090,7 @@ class VolumeProtectionTests(test_base.Ci - 'terminate_connection') - def test_owner_can_initialize_terminate_conn(self, mock_t, mock_i): - user_context = self.user_context -+ user_context.service_roles = ['service'] - - volume = self._create_fake_volume(user_context) - path = '/v3/%(project_id)s/volumes/%(volume_id)s/action' % { -Index: cinder/cinder/tests/unit/volume/test_connection.py -=================================================================== ---- cinder.orig/cinder/tests/unit/volume/test_connection.py -+++ cinder/cinder/tests/unit/volume/test_connection.py -@@ -1334,7 +1334,8 @@ class VolumeAttachDetachTestCase(base.Ba - self.context, - volume, None, None, None, None) - -- def test_volume_detach_in_maintenance(self): -+ @mock.patch('cinder.volume.api.API.attachment_deletion_allowed') -+ def test_volume_detach_in_maintenance(self, mock_attachment_deletion): - """Test detach the volume in maintenance.""" - test_meta1 = {'fake_key1': 'fake_value1', 'fake_key2': 'fake_value2'} - volume = tests_utils.create_volume(self.context, metadata=test_meta1, -@@ -1345,3 +1346,5 @@ class VolumeAttachDetachTestCase(base.Ba - volume_api.detach, - self.context, - volume, None) -+ mock_attachment_deletion.assert_called_once_with(self.context, -+ None, volume) -Index: cinder/cinder/volume/api.py -=================================================================== ---- cinder.orig/cinder/volume/api.py -+++ cinder/cinder/volume/api.py -@@ -34,6 +34,7 @@ import webob - - from cinder.api import common - from cinder.common import constants -+from cinder import compute - from cinder import context - from cinder import coordination - from cinder import db -@@ -863,11 +864,14 @@ class API(base.Base): - attachment_id: str) -> None: - context.authorize(vol_action_policy.DETACH_POLICY, - target_obj=volume) -+ self.attachment_deletion_allowed(context, attachment_id, volume) -+ - if volume['status'] == 'maintenance': - LOG.info('Unable to detach volume, ' - 'because it is in maintenance.', resource=volume) - msg = _("The volume cannot be detached in maintenance mode.") - raise exception.InvalidVolume(reason=msg) -+ - detach_results = self.volume_rpcapi.detach_volume(context, volume, - attachment_id) - LOG.info("Detach volume completed successfully.", -@@ -894,6 +898,19 @@ class API(base.Base): - resource=volume) - return init_results - -+ @staticmethod -+ def is_service_request(ctxt: 'context.RequestContext') -> bool: -+ """Check if a request is coming from a service -+ -+ A request is coming from a service if it has a service token and the -+ service user has one of the roles configured in the -+ `service_token_roles` configuration option in the -+ `[keystone_authtoken]` section (defaults to `service`). -+ """ -+ roles = ctxt.service_roles -+ service_roles = set(CONF.keystone_authtoken.service_token_roles) -+ return bool(roles and service_roles.intersection(roles)) -+ - def terminate_connection(self, - context: context.RequestContext, - volume: objects.Volume, -@@ -901,6 +918,8 @@ class API(base.Base): - force: bool = False) -> None: - context.authorize(vol_action_policy.TERMINATE_POLICY, - target_obj=volume) -+ self.attachment_deletion_allowed(context, None, volume) -+ - self.volume_rpcapi.terminate_connection(context, - volume, - connector, -@@ -2519,11 +2538,90 @@ class API(base.Base): - attachment_ref.save() - return attachment_ref - -+ def attachment_deletion_allowed(self, -+ ctxt: context.RequestContext, -+ attachment_or_attachment_id, -+ volume=None): -+ """Check if deleting an attachment is allowed (Bug #2004555) -+ -+ Allowed is based on the REST API policy, the status of the attachment, -+ where it is used, and who is making the request. -+ -+ Deleting an attachment on the Cinder side while leaving the volume -+ connected to the nova host results in leftover devices that can lead to -+ data leaks/corruption. -+ -+ OS-Brick may have code to detect it, but in some cases it is detected -+ after it has already been exposed, so it's better to prevent users from -+ being able to intentionally triggering the issue. -+ """ -+ # It's ok to delete an attachment if the request comes from a service -+ if self.is_service_request(ctxt): -+ return -+ -+ if not attachment_or_attachment_id and volume: -+ if not volume.volume_attachment: -+ return -+ if len(volume.volume_attachment) == 1: -+ attachment_or_attachment_id = volume.volume_attachment[0] -+ -+ if isinstance(attachment_or_attachment_id, str): -+ try: -+ attachment = objects.VolumeAttachment.get_by_id( -+ ctxt, attachment_or_attachment_id) -+ except exception.VolumeAttachmentNotFound: -+ attachment = None -+ else: -+ attachment = attachment_or_attachment_id -+ -+ if attachment: -+ if volume: -+ if volume.id != attachment.volume_id: -+ raise exception.InvalidInput( -+ reason='Mismatched volume and attachment') -+ -+ server_id = attachment.instance_uuid -+ # It's ok to delete if it's not connected to a vm. -+ if not server_id or not attachment.connection_info: -+ return -+ -+ volume = volume or attachment.volume -+ nova = compute.API() -+ LOG.info('Attachment connected to vm %s, checking data on nova', -+ server_id) -+ # If nova is down the client raises 503 and we report that -+ try: -+ nova_volume = nova.get_server_volume(ctxt, server_id, -+ volume.id) -+ except nova.NotFound: -+ LOG.warning('Instance or volume not found on Nova, deleting ' -+ 'attachment locally, which may leave leftover ' -+ 'devices on Nova compute') -+ return -+ -+ if nova_volume.attachment_id != attachment.id: -+ LOG.warning('Mismatch! Nova has different attachment id (%s) ' -+ 'for the volume, deleting attachment locally. ' -+ 'May leave leftover devices in a compute node', -+ nova_volume.attachment_id) -+ return -+ else: -+ server_id = '' -+ -+ LOG.error('Detected user call to delete in-use attachment. Call must ' -+ 'come from the nova service and nova must be configured to ' -+ 'send the service token. Bug #2004555') -+ raise exception.ConflictNovaUsingAttachment(instance_id=server_id) -+ - def attachment_delete(self, - ctxt: context.RequestContext, - attachment) -> objects.VolumeAttachmentList: -+ # Check if policy allows user to delete attachment - ctxt.authorize(attachment_policy.DELETE_POLICY, - target_obj=attachment) -+ -+ self.attachment_deletion_allowed(ctxt, attachment) -+ - volume = attachment.volume - - if attachment.attach_status == fields.VolumeAttachStatus.RESERVED: -Index: cinder/doc/source/configuration/block-storage/service-token.rst -=================================================================== ---- cinder.orig/doc/source/configuration/block-storage/service-token.rst -+++ cinder/doc/source/configuration/block-storage/service-token.rst -@@ -1,6 +1,6 @@ --========================================================= --Using service tokens to prevent long-running job failures --========================================================= -+==================== -+Using service tokens -+==================== - - When a user initiates a request whose processing involves multiple services - (for example, a boot-from-volume request to the Compute Service will require -@@ -8,20 +8,32 @@ processing by the Block Storage Service, - Image Service), the user's token is handed from service to service. This - ensures that the requestor is tracked correctly for audit purposes and also - guarantees that the requestor has the appropriate permissions to do what needs --to be done by the other services. If the chain of operations takes a long --time, however, the user's token may expire before the action is completed, --leading to the failure of the user's original request. -- --One way to deal with this is to set a long token life in Keystone, and this may --be what you are currently doing. But this can be problematic for installations --whose security policies prefer short user token lives. Beginning with the --Queens release, an alternative solution is available. You have the ability to --configure some services (particularly Nova and Cinder) to send a "service --token" along with the user's token. When properly configured, the Identity --Service will validate an expired user token *when it is accompanied by a valid --service token*. Thus if the user's token expires somewhere during a long --running chain of operations among various OpenStack services, the operations --can continue. -+to be done by the other services. -+ -+There are several instances where we want to differentiate between a request -+coming from the user to one coming from another OpenStack service on behalf of -+the user: -+ -+- **For security reasons** There are some operations in the Block Storage -+ service, required for normal operations, that could be exploited by a -+ malicious user to gain access to resources belonging to other users. By -+ differentiating when the request comes directly from a user and when from -+ another OpenStack service the Cinder service can protect the deployment. -+ -+- To prevent long-running job failures: If the chain of operations takes a long -+ time, the user's token may expire before the action is completed, leading to -+ the failure of the user's original request. -+ -+ One way to deal with this is to set a long token life in Keystone, and this -+ may be what you are currently doing. But this can be problematic for -+ installations whose security policies prefer short user token lives. -+ Beginning with the Queens release, an alternative solution is available. You -+ have the ability to configure some services (particularly Nova and Cinder) to -+ send a "service token" along with the user's token. When properly -+ configured, the Identity Service will validate an expired user token *when it -+ is accompanied by a valid service token*. Thus if the user's token expires -+ somewhere during a long running chain of operations among various OpenStack -+ services, the operations can continue. - - .. note:: - There's nothing special about a service token. It's a regular token -Index: cinder/doc/source/configuration/index.rst -=================================================================== ---- cinder.orig/doc/source/configuration/index.rst -+++ cinder/doc/source/configuration/index.rst -@@ -6,6 +6,7 @@ Cinder Service Configuration - :maxdepth: 1 - - block-storage/block-storage-overview.rst -+ block-storage/service-token.rst - block-storage/volume-drivers.rst - block-storage/backup-drivers.rst - block-storage/schedulers.rst -@@ -15,10 +16,16 @@ Cinder Service Configuration - block-storage/policy-config-HOWTO.rst - block-storage/fc-zoning.rst - block-storage/volume-encryption.rst -- block-storage/service-token.rst - block-storage/config-options.rst - block-storage/samples/index.rst - -+.. warning:: -+ -+ For security reasons **Service Tokens must to be configured** in OpenStack -+ for Cinder to operate securely. Pay close attention to the :doc:`specific -+ section describing it: `. See -+ https://bugs.launchpad.net/nova/+bug/2004555 for details. -+ - .. note:: - - The examples of common configurations for shared -Index: cinder/doc/source/install/index.rst -=================================================================== ---- cinder.orig/doc/source/install/index.rst -+++ cinder/doc/source/install/index.rst -@@ -35,6 +35,13 @@ Adding Cinder to your OpenStack Environm - - The following links describe how to install the Cinder Block Storage Service: - -+.. warning:: -+ -+ For security reasons **Service Tokens must to be configured** in OpenStack -+ for Cinder to operate securely. Pay close attention to the :doc:`specific -+ section describing it: <../configuration/block-storage/service-token>`. See -+ https://bugs.launchpad.net/nova/+bug/2004555 for details. -+ - .. toctree:: - - get-started-block-storage -Index: cinder/releasenotes/notes/redirect-detach-nova-4b7b7902d7d182e0.yaml -=================================================================== ---- /dev/null -+++ cinder/releasenotes/notes/redirect-detach-nova-4b7b7902d7d182e0.yaml -@@ -0,0 +1,42 @@ -+--- -+critical: -+ - | -+ Detaching volumes will fail if Nova is not `configured to send service -+ tokens `_, -+ please read the upgrade section for more information. (`Bug #2004555 -+ `_). -+upgrade: -+ - | -+ Nova must be `configured to send service tokens -+ `_ -+ **and** cinder must be configured to recognize at least one of the roles -+ that the nova service user has been assigned in keystone. By default, -+ cinder will recognize the ``service`` role, so if the nova service user -+ is assigned a differently named role in your cloud, you must adjust your -+ cinder configuration file (``service_token_roles`` configuration option -+ in the ``keystone_authtoken`` section). If nova and cinder are not -+ configured correctly in this regard, detaching volumes will no longer -+ work (`Bug #2004555 `_). -+security: -+ - | -+ As part of the fix for `Bug #2004555 -+ `_, cinder now rejects -+ user attachment delete requests for attachments that are being used by nova -+ instances to ensure that no leftover devices are produced on the compute -+ nodes which could be used to access another project's volumes. Terminate -+ connection, detach, and force detach volume actions are not allowed for -+ users. -+fixes: -+ - | -+ `Bug #2004555 `_: Fixed -+ issue where a user manually deleting an attachment, calling terminate -+ connection, detach, or force detach, for a volume that is still used by a -+ nova instance resulted in leftover devices on the compute node. These -+ operations will now fail. -+issues: -+ - | -+ For security reasons (`Bug #2004555 -+ `_) manually deleting an -+ attachment, manually doing the ``os-terminate_connection`` ``os-detach`` or -+ ``os-force_detach`` actions will no longer be allowed unless the request is -+ coming from another OpenStack service on behalf of a user. diff -Nru cinder-21.1.0/debian/patches/cve-2024-32498-cinder-stable-2023.1.patch cinder-21.3.1/debian/patches/cve-2024-32498-cinder-stable-2023.1.patch --- cinder-21.1.0/debian/patches/cve-2024-32498-cinder-stable-2023.1.patch 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/debian/patches/cve-2024-32498-cinder-stable-2023.1.patch 2024-06-21 07:19:56.000000000 +0000 @@ -0,0 +1,2294 @@ +From 96461c2b75a7a7e58b55a1ff6068625096be8064 Mon Sep 17 00:00:00 2001 +From: Brian Rosmaita +Date: Wed, 26 Jun 2024 14:09:30 -0400 +Subject: [PATCH] CVE-2024-32498: Check for external qcow2 data file + +Adds code to image_utils to check for a qcow2 external data +file, a recent feature of qemu which we do not support and +which can be used maliciously. + +Advice from the qemu-img community is that it is dangerous +to call qemu-img info on untrusted files, so we copy over +the format_inspector module from Glance. This performs basic +analysis on the image data file so we can detect problematic +images before we call qemu-img info to get all the image +attributes. It is expected that this code will eventually be +added to oslo so it can be consumed by Glance, Cinder, and +Nova. + +Because cinder itself may create qcow2 format images with a +backing file in nfs-based backends, the glance format_inspector +has been modified to optionally allow such files. Since we are +monkeying with the format_inspector code, we also copy over +its unit tests to prevent regressions and to add tests for the +changed code. + +Includes an additional fix to prevent an issue where a user +could mount a raw volume and write a qcow2 header with a larger +virtual size on it. On reattaching the volume it would have the +new larger virtual size avaialable without actually changing +the size value in cinder. While we cannot prevent this we can +prevent the user from using this volume again, which makes this +exploit pointless. + +Co-authored-by: Dan Smith +Co-authored-by: Felix Huettner + +Change-Id: I65857288b797cde573e7443ac6e7e6f57fedde01 +Closes-bug: #2059809 +(cherry picked from commit b207be3278f3dab6cfe945bd3d307db168f6f6c7) +(cherry picked from commit fe0940994ca93344c4d4a9923471ada6fb3a38ef) +(cherry picked from commit c7fbbba552ea8080965fccbabb4f54855651e7b9) +--- + cinder/image/format_inspector.py | 938 ++++++++++++++++++ + cinder/image/image_utils.py | 88 +- + .../tests/unit/image/test_format_inspector.py | 515 ++++++++++ + cinder/tests/unit/test_image_utils.py | 272 ++++- + cinder/tests/unit/volume/drivers/test_nfs.py | 37 +- + .../tests/unit/volume/drivers/test_quobyte.py | 76 +- + .../unit/volume/drivers/test_remotefs.py | 3 +- + cinder/volume/drivers/nfs.py | 10 + + cinder/volume/drivers/remotefs.py | 3 +- + ...allow-qcow2-datafile-abc4e6d8be766710.yaml | 19 + + 10 files changed, 1903 insertions(+), 58 deletions(-) + create mode 100644 cinder/image/format_inspector.py + create mode 100644 cinder/tests/unit/image/test_format_inspector.py + create mode 100644 releasenotes/notes/bug-2059809-disallow-qcow2-datafile-abc4e6d8be766710.yaml + +Index: cinder/cinder/image/format_inspector.py +=================================================================== +--- /dev/null ++++ cinder/cinder/image/format_inspector.py +@@ -0,0 +1,938 @@ ++# Copyright 2020 Red Hat, Inc ++# All Rights Reserved. ++# ++# Licensed under the Apache License, Version 2.0 (the "License"); you may ++# not use this file except in compliance with the License. You may obtain ++# a copy of the License at ++# ++# http://www.apache.org/licenses/LICENSE-2.0 ++# ++# Unless required by applicable law or agreed to in writing, software ++# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT ++# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the ++# License for the specific language governing permissions and limitations ++# under the License. ++ ++""" ++This is a python implementation of virtual disk format inspection routines ++gathered from various public specification documents, as well as qemu disk ++driver code. It attempts to store and parse the minimum amount of data ++required, and in a streaming-friendly manner to collect metadata about ++complex-format images. ++""" ++ ++import struct ++ ++from oslo_log import log as logging ++ ++LOG = logging.getLogger(__name__) ++ ++ ++def chunked_reader(fileobj, chunk_size=512): ++ while True: ++ chunk = fileobj.read(chunk_size) ++ if not chunk: ++ break ++ yield chunk ++ ++ ++class CaptureRegion(object): ++ """Represents a region of a file we want to capture. ++ ++ A region of a file we want to capture requires a byte offset into ++ the file and a length. This is expected to be used by a data ++ processing loop, calling capture() with the most recently-read ++ chunk. This class handles the task of grabbing the desired region ++ of data across potentially multiple fractional and unaligned reads. ++ ++ :param offset: Byte offset into the file starting the region ++ :param length: The length of the region ++ """ ++ def __init__(self, offset, length): ++ self.offset = offset ++ self.length = length ++ self.data = b'' ++ ++ @property ++ def complete(self): ++ """Returns True when we have captured the desired data.""" ++ return self.length == len(self.data) ++ ++ def capture(self, chunk, current_position): ++ """Process a chunk of data. ++ ++ This should be called for each chunk in the read loop, at least ++ until complete returns True. ++ ++ :param chunk: A chunk of bytes in the file ++ :param current_position: The position of the file processed by the ++ read loop so far. Note that this will be ++ the position in the file *after* the chunk ++ being presented. ++ """ ++ read_start = current_position - len(chunk) ++ if (read_start <= self.offset <= current_position or ++ self.offset <= read_start <= (self.offset + self.length)): ++ if read_start < self.offset: ++ lead_gap = self.offset - read_start ++ else: ++ lead_gap = 0 ++ self.data += chunk[lead_gap:] ++ self.data = self.data[:self.length] ++ ++ ++class ImageFormatError(Exception): ++ """An unrecoverable image format error that aborts the process.""" ++ pass ++ ++ ++class TraceDisabled(object): ++ """A logger-like thing that swallows tracing when we do not want it.""" ++ def debug(self, *a, **k): ++ pass ++ ++ info = debug ++ warning = debug ++ error = debug ++ ++ ++class FileInspector(object): ++ """A stream-based disk image inspector. ++ ++ This base class works on raw images and is subclassed for more ++ complex types. It is to be presented with the file to be examined ++ one chunk at a time, during read processing and will only store ++ as much data as necessary to determine required attributes of ++ the file. ++ """ ++ ++ def __init__(self, tracing=False): ++ self._total_count = 0 ++ ++ # NOTE(danms): The logging in here is extremely verbose for a reason, ++ # but should never really be enabled at that level at runtime. To ++ # retain all that work and assist in future debug, we have a separate ++ # debug flag that can be passed from a manual tool to turn it on. ++ if tracing: ++ self._log = logging.getLogger(str(self)) ++ else: ++ self._log = TraceDisabled() ++ self._capture_regions = {} ++ ++ def _capture(self, chunk, only=None): ++ for name, region in self._capture_regions.items(): ++ if only and name not in only: ++ continue ++ if not region.complete: ++ region.capture(chunk, self._total_count) ++ ++ def eat_chunk(self, chunk): ++ """Call this to present chunks of the file to the inspector.""" ++ pre_regions = set(self._capture_regions.keys()) ++ ++ # Increment our position-in-file counter ++ self._total_count += len(chunk) ++ ++ # Run through the regions we know of to see if they want this ++ # data ++ self._capture(chunk) ++ ++ # Let the format do some post-read processing of the stream ++ self.post_process() ++ ++ # Check to see if the post-read processing added new regions ++ # which may require the current chunk. ++ new_regions = set(self._capture_regions.keys()) - pre_regions ++ if new_regions: ++ self._capture(chunk, only=new_regions) ++ ++ def post_process(self): ++ """Post-read hook to process what has been read so far. ++ ++ This will be called after each chunk is read and potentially captured ++ by the defined regions. If any regions are defined by this call, ++ those regions will be presented with the current chunk in case it ++ is within one of the new regions. ++ """ ++ pass ++ ++ def region(self, name): ++ """Get a CaptureRegion by name.""" ++ return self._capture_regions[name] ++ ++ def new_region(self, name, region): ++ """Add a new CaptureRegion by name.""" ++ if self.has_region(name): ++ # This is a bug, we tried to add the same region twice ++ raise ImageFormatError('Inspector re-added region %s' % name) ++ self._capture_regions[name] = region ++ ++ def has_region(self, name): ++ """Returns True if named region has been defined.""" ++ return name in self._capture_regions ++ ++ @property ++ def format_match(self): ++ """Returns True if the file appears to be the expected format.""" ++ return True ++ ++ @property ++ def virtual_size(self): ++ """Returns the virtual size of the disk image, or zero if unknown.""" ++ return self._total_count ++ ++ @property ++ def actual_size(self): ++ """Returns the total size of the file. ++ ++ This is usually smaller than virtual_size. NOTE: this will only be ++ accurate if the entire file is read and processed. ++ """ ++ return self._total_count ++ ++ @property ++ def complete(self): ++ """Returns True if we have all the information needed.""" ++ return all(r.complete for r in self._capture_regions.values()) ++ ++ def __str__(self): ++ """The string name of this file format.""" ++ return 'raw' ++ ++ @property ++ def context_info(self): ++ """Return info on amount of data held in memory for auditing. ++ ++ This is a dict of region:sizeinbytes items that the inspector ++ uses to examine the file. ++ """ ++ return {name: len(region.data) for name, region in ++ self._capture_regions.items()} ++ ++ @classmethod ++ def from_file(cls, filename): ++ """Read as much of a file as necessary to complete inspection. ++ ++ NOTE: Because we only read as much of the file as necessary, the ++ actual_size property will not reflect the size of the file, but the ++ amount of data we read before we satisfied the inspector. ++ ++ Raises ImageFormatError if we cannot parse the file. ++ """ ++ inspector = cls() ++ with open(filename, 'rb') as f: ++ for chunk in chunked_reader(f): ++ inspector.eat_chunk(chunk) ++ if inspector.complete: ++ # No need to eat any more data ++ break ++ if not inspector.complete or not inspector.format_match: ++ raise ImageFormatError('File is not in requested format') ++ return inspector ++ ++ def safety_check(self): ++ """Perform some checks to determine if this file is safe. ++ ++ Returns True if safe, False otherwise. It may raise ImageFormatError ++ if safety cannot be guaranteed because of parsing or other errors. ++ """ ++ return True ++ ++ ++# The qcow2 format consists of a big-endian 72-byte header, of which ++# only a small portion has information we care about: ++# ++# Dec Hex Name ++# 0 0x00 Magic 4-bytes 'QFI\xfb' ++# 4 0x04 Version (uint32_t, should always be 2 for modern files) ++# . . . ++# 8 0x08 Backing file offset (uint64_t) ++# 24 0x18 Size in bytes (unint64_t) ++# . . . ++# 72 0x48 Incompatible features bitfield (6 bytes) ++# ++# https://gitlab.com/qemu-project/qemu/-/blob/master/docs/interop/qcow2.txt ++class QcowInspector(FileInspector): ++ """QEMU QCOW2 Format ++ ++ This should only require about 32 bytes of the beginning of the file ++ to determine the virtual size, and 104 bytes to perform the safety check. ++ """ ++ ++ BF_OFFSET = 0x08 ++ BF_OFFSET_LEN = 8 ++ I_FEATURES = 0x48 ++ I_FEATURES_LEN = 8 ++ I_FEATURES_DATAFILE_BIT = 3 ++ I_FEATURES_MAX_BIT = 4 ++ ++ def __init__(self, *a, **k): ++ super(QcowInspector, self).__init__(*a, **k) ++ self.new_region('header', CaptureRegion(0, 512)) ++ ++ def _qcow_header_data(self): ++ magic, version, bf_offset, bf_sz, cluster_bits, size = ( ++ struct.unpack('>4sIQIIQ', self.region('header').data[:32])) ++ return magic, size ++ ++ @property ++ def has_header(self): ++ return self.region('header').complete ++ ++ @property ++ def virtual_size(self): ++ if not self.region('header').complete: ++ return 0 ++ if not self.format_match: ++ return 0 ++ magic, size = self._qcow_header_data() ++ return size ++ ++ @property ++ def format_match(self): ++ if not self.region('header').complete: ++ return False ++ magic, size = self._qcow_header_data() ++ return magic == b'QFI\xFB' ++ ++ @property ++ def has_backing_file(self): ++ if not self.region('header').complete: ++ return None ++ if not self.format_match: ++ return False ++ bf_offset_bytes = self.region('header').data[ ++ self.BF_OFFSET:self.BF_OFFSET + self.BF_OFFSET_LEN] ++ # nonzero means "has a backing file" ++ bf_offset, = struct.unpack('>Q', bf_offset_bytes) ++ return bf_offset != 0 ++ ++ @property ++ def has_unknown_features(self): ++ if not self.region('header').complete: ++ return None ++ if not self.format_match: ++ return False ++ i_features = self.region('header').data[ ++ self.I_FEATURES:self.I_FEATURES + self.I_FEATURES_LEN] ++ ++ # This is the maximum byte number we should expect any bits to be set ++ max_byte = self.I_FEATURES_MAX_BIT // 8 ++ ++ # The flag bytes are in big-endian ordering, so if we process ++ # them in index-order, they're reversed ++ for i, byte_num in enumerate(reversed(range(self.I_FEATURES_LEN))): ++ if byte_num == max_byte: ++ # If we're in the max-allowed byte, allow any bits less than ++ # the maximum-known feature flag bit to be set ++ allow_mask = ((1 << self.I_FEATURES_MAX_BIT) - 1) ++ elif byte_num > max_byte: ++ # If we're above the byte with the maximum known feature flag ++ # bit, then we expect all zeroes ++ allow_mask = 0x0 ++ else: ++ # Any earlier-than-the-maximum byte can have any of the flag ++ # bits set ++ allow_mask = 0xFF ++ ++ if i_features[i] & ~allow_mask: ++ LOG.warning('Found unknown feature bit in byte %i: %s/%s', ++ byte_num, bin(i_features[byte_num] & ~allow_mask), ++ bin(allow_mask)) ++ return True ++ ++ return False ++ ++ @property ++ def has_data_file(self): ++ if not self.region('header').complete: ++ return None ++ if not self.format_match: ++ return False ++ i_features = self.region('header').data[ ++ self.I_FEATURES:self.I_FEATURES + self.I_FEATURES_LEN] ++ ++ # First byte of bitfield, which is i_features[7] ++ byte = self.I_FEATURES_LEN - 1 - self.I_FEATURES_DATAFILE_BIT // 8 ++ # Third bit of bitfield, which is 0x04 ++ bit = 1 << (self.I_FEATURES_DATAFILE_BIT - 1 % 8) ++ return bool(i_features[byte] & bit) ++ ++ def __str__(self): ++ return 'qcow2' ++ ++ def safety_check(self): ++ return (not self.has_backing_file and ++ not self.has_data_file and ++ not self.has_unknown_features) ++ ++ def safety_check_allow_backing_file(self): ++ return (not self.has_data_file and ++ not self.has_unknown_features) ++ ++ ++class QEDInspector(FileInspector): ++ def __init__(self, tracing=False): ++ super().__init__(tracing) ++ self.new_region('header', CaptureRegion(0, 512)) ++ ++ @property ++ def format_match(self): ++ if not self.region('header').complete: ++ return False ++ return self.region('header').data.startswith(b'QED\x00') ++ ++ def safety_check(self): ++ # QED format is not supported by anyone, but we want to detect it ++ # and mark it as just always unsafe. ++ return False ++ ++ ++# The VHD (or VPC as QEMU calls it) format consists of a big-endian ++# 512-byte "footer" at the beginning of the file with various ++# information, most of which does not matter to us: ++# ++# Dec Hex Name ++# 0 0x00 Magic string (8-bytes, always 'conectix') ++# 40 0x28 Disk size (uint64_t) ++# ++# https://github.com/qemu/qemu/blob/master/block/vpc.c ++class VHDInspector(FileInspector): ++ """Connectix/MS VPC VHD Format ++ ++ This should only require about 512 bytes of the beginning of the file ++ to determine the virtual size. ++ """ ++ def __init__(self, *a, **k): ++ super(VHDInspector, self).__init__(*a, **k) ++ self.new_region('header', CaptureRegion(0, 512)) ++ ++ @property ++ def format_match(self): ++ return self.region('header').data.startswith(b'conectix') ++ ++ @property ++ def virtual_size(self): ++ if not self.region('header').complete: ++ return 0 ++ ++ if not self.format_match: ++ return 0 ++ ++ return struct.unpack('>Q', self.region('header').data[40:48])[0] ++ ++ def __str__(self): ++ return 'vhd' ++ ++ ++# The VHDX format consists of a complex dynamic little-endian ++# structure with multiple regions of metadata and data, linked by ++# offsets with in the file (and within regions), identified by MSFT ++# GUID strings. The header is a 320KiB structure, only a few pieces of ++# which we actually need to capture and interpret: ++# ++# Dec Hex Name ++# 0 0x00000 Identity (Technically 9-bytes, padded to 64KiB, the first ++# 8 bytes of which are 'vhdxfile') ++# 196608 0x30000 The Region table (64KiB of a 32-byte header, followed ++# by up to 2047 36-byte region table entry structures) ++# ++# The region table header includes two items we need to read and parse, ++# which are: ++# ++# 196608 0x30000 4-byte signature ('regi') ++# 196616 0x30008 Entry count (uint32-t) ++# ++# The region table entries follow the region table header immediately ++# and are identified by a 16-byte GUID, and provide an offset of the ++# start of that region. We care about the "metadata region", identified ++# by the METAREGION class variable. The region table entry is (offsets ++# from the beginning of the entry, since it could be in multiple places): ++# ++# 0 0x00000 16-byte MSFT GUID ++# 16 0x00010 Offset of the actual metadata region (uint64_t) ++# ++# When we find the METAREGION table entry, we need to grab that offset ++# and start examining the region structure at that point. That ++# consists of a metadata table of structures, which point to places in ++# the data in an unstructured space that follows. The header is ++# (offsets relative to the region start): ++# ++# 0 0x00000 8-byte signature ('metadata') ++# . . . ++# 16 0x00010 2-byte entry count (up to 2047 entries max) ++# ++# This header is followed by the specified number of metadata entry ++# structures, identified by GUID: ++# ++# 0 0x00000 16-byte MSFT GUID ++# 16 0x00010 4-byte offset (uint32_t, relative to the beginning of ++# the metadata region) ++# ++# We need to find the "Virtual Disk Size" metadata item, identified by ++# the GUID in the VIRTUAL_DISK_SIZE class variable, grab the offset, ++# add it to the offset of the metadata region, and examine that 8-byte ++# chunk of data that follows. ++# ++# The "Virtual Disk Size" is a naked uint64_t which contains the size ++# of the virtual disk, and is our ultimate target here. ++# ++# https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-vhdx/83e061f8-f6e2-4de1-91bd-5d518a43d477 ++class VHDXInspector(FileInspector): ++ """MS VHDX Format ++ ++ This requires some complex parsing of the stream. The first 256KiB ++ of the image is stored to get the header and region information, ++ and then we capture the first metadata region to read those ++ records, find the location of the virtual size data and parse ++ it. This needs to store the metadata table entries up until the ++ VDS record, which may consist of up to 2047 32-byte entries at ++ max. Finally, it must store a chunk of data at the offset of the ++ actual VDS uint64. ++ ++ """ ++ METAREGION = '8B7CA206-4790-4B9A-B8FE-575F050F886E' ++ VIRTUAL_DISK_SIZE = '2FA54224-CD1B-4876-B211-5DBED83BF4B8' ++ VHDX_METADATA_TABLE_MAX_SIZE = 32 * 2048 # From qemu ++ ++ def __init__(self, *a, **k): ++ super(VHDXInspector, self).__init__(*a, **k) ++ self.new_region('ident', CaptureRegion(0, 32)) ++ self.new_region('header', CaptureRegion(192 * 1024, 64 * 1024)) ++ ++ def post_process(self): ++ # After reading a chunk, we may have the following conditions: ++ # ++ # 1. We may have just completed the header region, and if so, ++ # we need to immediately read and calculate the location of ++ # the metadata region, as it may be starting in the same ++ # read we just did. ++ # 2. We may have just completed the metadata region, and if so, ++ # we need to immediately calculate the location of the ++ # "virtual disk size" record, as it may be starting in the ++ # same read we just did. ++ if self.region('header').complete and not self.has_region('metadata'): ++ region = self._find_meta_region() ++ if region: ++ self.new_region('metadata', region) ++ elif self.has_region('metadata') and not self.has_region('vds'): ++ region = self._find_meta_entry(self.VIRTUAL_DISK_SIZE) ++ if region: ++ self.new_region('vds', region) ++ ++ @property ++ def format_match(self): ++ return self.region('ident').data.startswith(b'vhdxfile') ++ ++ @staticmethod ++ def _guid(buf): ++ """Format a MSFT GUID from the 16-byte input buffer.""" ++ guid_format = '= 2048: ++ raise ImageFormatError('Region count is %i (limit 2047)' % count) ++ ++ # Process the regions until we find the metadata one; grab the ++ # offset and return ++ self._log.debug('Region entry first is %x', region_entry_first) ++ self._log.debug('Region entries %i', count) ++ meta_offset = 0 ++ for i in range(0, count): ++ entry_start = region_entry_first + (i * 32) ++ entry_end = entry_start + 32 ++ entry = self.region('header').data[entry_start:entry_end] ++ self._log.debug('Entry offset is %x', entry_start) ++ ++ # GUID is the first 16 bytes ++ guid = self._guid(entry[:16]) ++ if guid == self.METAREGION: ++ # This entry is the metadata region entry ++ meta_offset, meta_len, meta_req = struct.unpack( ++ '= 2048: ++ raise ImageFormatError( ++ 'Metadata item count is %i (limit 2047)' % count) ++ ++ for i in range(0, count): ++ entry_offset = 32 + (i * 32) ++ guid = self._guid(meta_buffer[entry_offset:entry_offset + 16]) ++ if guid == desired_guid: ++ # Found the item we are looking for by id. ++ # Stop our region from capturing ++ item_offset, item_length, _reserved = struct.unpack( ++ ' imageutils.QemuImgInfo: ++def qemu_img_info( ++ path: str, ++ run_as_root: bool = True, ++ force_share: bool = False, ++ allow_qcow2_backing_file: bool = False) -> imageutils.QemuImgInfo: + """Return an object containing the parsed output from qemu-img info.""" +- cmd = ['env', 'LC_ALL=C', 'qemu-img', 'info', '--output=json'] ++ ++ inspector = format_inspector.detect_file_format(path) ++ format_name = str(inspector) ++ safe = inspector.safety_check() ++ if not safe and format_name == 'qcow2' and allow_qcow2_backing_file: ++ safe = inspector.safety_check_allow_backing_file() ++ if not safe: ++ LOG.warning('Image/Volume %s failed safety check', path) ++ # NOTE(danms): This is the same exception as would be raised ++ # by qemu_img_info() if the disk format was unreadable or ++ # otherwise unsuitable. ++ raise exception.Invalid( ++ reason=_('Image/Volume failed safety check')) ++ ++ cmd = ['env', 'LC_ALL=C', 'qemu-img', 'info', ++ '-f', format_name, '--output=json'] + if force_share: + if qemu_img_supports_force_share(): + cmd.append('--force-share') +@@ -180,8 +198,32 @@ def qemu_img_info(path: str, + prlimit=QEMU_IMG_LIMITS) + info = imageutils.QemuImgInfo(out, format='json') + ++ # FIXME: figure out a more elegant way to do this ++ if info.file_format == 'raw': ++ # The format_inspector will detect a luks image as 'raw', and then when ++ # we call qemu-img info -f raw above, we don't get any of the luks ++ # format-specific info (some of which is used in the create_volume ++ # flow). So we need to check if this is really a luks container. ++ # (We didn't have to do this in the past because we called ++ # qemu-img info without -f.) ++ cmd = ['env', 'LC_ALL=C', 'qemu-img', 'info', ++ '-f', 'luks', '--output=json'] ++ if force_share: ++ cmd.append('--force-share') ++ cmd.append(path) ++ if os.name == 'nt': ++ cmd = cmd[2:] ++ try: ++ out, _err = utils.execute(*cmd, run_as_root=run_as_root, ++ prlimit=QEMU_IMG_LIMITS) ++ info = imageutils.QemuImgInfo(out, format='json') ++ except processutils.ProcessExecutionError: ++ # we'll just use the info object we already got earlier ++ pass ++ + # From Cinder's point of view, any 'luks' formatted images +- # should be treated as 'raw'. ++ # should be treated as 'raw'. (This changes the file_format, but ++ # not any of the format-specific information.) + if info.file_format == 'luks': + info.file_format = 'raw' + +@@ -687,6 +729,35 @@ def get_qemu_data(image_id: str, + return data + + ++def check_qcow2_image(image_id: str, data: imageutils.QemuImgInfo) -> None: ++ """Check some rules about qcow2 images. ++ ++ Does not check for a backing_file, because cinder has some legitimate ++ use cases for qcow2 backing files. ++ ++ Makes sure the image: ++ ++ - does not have a data_file ++ ++ :param image_id: the image id ++ :param data: an imageutils.QemuImgInfo object ++ :raises ImageUnacceptable: when the image fails the check ++ """ ++ try: ++ data_file = data.format_specific['data'].get('data-file') ++ except (KeyError, TypeError): ++ LOG.debug('Unexpected response from qemu-img info when processing ' ++ 'image %s: missing format-specific info for a qcow2 image', ++ image_id) ++ msg = _('Cannot determine format-specific information') ++ raise exception.ImageUnacceptable(image_id=image_id, reason=msg) ++ if data_file: ++ LOG.warning("Refusing to process qcow2 file with data-file '%s'", ++ data_file) ++ msg = _('A qcow2 format image is not allowed to have a data file') ++ raise exception.ImageUnacceptable(image_id=image_id, reason=msg) ++ ++ + def check_vmdk_image(image_id: str, data: imageutils.QemuImgInfo) -> None: + """Check some rules about VMDK images. + +@@ -777,6 +848,8 @@ def check_image_format(source: str, + + if data.file_format == 'vmdk': + check_vmdk_image(image_id, data) ++ if data.file_format == 'qcow2': ++ check_qcow2_image(image_id, data) + + + def fetch_verify_image(context: context.RequestContext, +@@ -819,6 +892,11 @@ def fetch_verify_image(context: context. + if fmt == 'vmdk': + check_vmdk_image(image_id, data) + ++ # Bug #2059809: a qcow2 can have a data file that's similar ++ # to a backing file and is also unacceptable ++ if fmt == 'qcow2': ++ check_qcow2_image(image_id, data) ++ + + def fetch_to_vhd(context: context.RequestContext, + image_service: glance.GlanceImageService, +Index: cinder/cinder/tests/unit/image/test_format_inspector.py +=================================================================== +--- /dev/null ++++ cinder/cinder/tests/unit/image/test_format_inspector.py +@@ -0,0 +1,515 @@ ++# Copyright 2020 Red Hat, Inc ++# All Rights Reserved. ++# ++# Licensed under the Apache License, Version 2.0 (the "License"); you may ++# not use this file except in compliance with the License. You may obtain ++# a copy of the License at ++# ++# http://www.apache.org/licenses/LICENSE-2.0 ++# ++# Unless required by applicable law or agreed to in writing, software ++# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT ++# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the ++# License for the specific language governing permissions and limitations ++# under the License. ++ ++import io ++import os ++import re ++import struct ++import subprocess ++import tempfile ++from unittest import mock ++ ++from oslo_utils import units ++ ++from cinder.image import format_inspector ++from cinder.tests.unit import test ++ ++ ++def get_size_from_qemu_img(filename): ++ output = subprocess.check_output('qemu-img info "%s"' % filename, ++ shell=True) ++ for line in output.split(b'\n'): ++ m = re.search(b'^virtual size: .* .([0-9]+) bytes', line.strip()) ++ if m: ++ return int(m.group(1)) ++ ++ raise Exception('Could not find virtual size with qemu-img') ++ ++ ++class TestFormatInspectors(test.TestCase): ++ def setUp(self): ++ super(TestFormatInspectors, self).setUp() ++ self._created_files = [] ++ ++ def tearDown(self): ++ super(TestFormatInspectors, self).tearDown() ++ for fn in self._created_files: ++ try: ++ os.remove(fn) ++ except Exception: ++ pass ++ ++ def _create_img(self, fmt, size, subformat=None, options=None, ++ backing_file=None): ++ if fmt == 'vhd': ++ # QEMU calls the vhd format vpc ++ fmt = 'vpc' ++ ++ if options is None: ++ options = {} ++ opt = '' ++ prefix = 'glance-unittest-formatinspector-' ++ ++ if subformat: ++ options['subformat'] = subformat ++ prefix += subformat + '-' ++ ++ if options: ++ opt += '-o ' + ','.join('%s=%s' % (k, v) ++ for k, v in options.items()) ++ ++ if backing_file is not None: ++ opt += ' -b %s -F raw' % backing_file ++ ++ fn = tempfile.mktemp(prefix=prefix, ++ suffix='.%s' % fmt) ++ self._created_files.append(fn) ++ subprocess.check_output( ++ 'qemu-img create -f %s %s %s %i' % (fmt, opt, fn, size), ++ shell=True) ++ return fn ++ ++ def _create_allocated_vmdk(self, size_mb, subformat=None): ++ # We need a "big" VMDK file to exercise some parts of the code of the ++ # format_inspector. A way to create one is to first create an empty ++ # file, and then to convert it with the -S 0 option. ++ ++ if subformat is None: ++ # Matches qemu-img default, see `qemu-img convert -O vmdk -o help` ++ subformat = 'monolithicSparse' ++ ++ prefix = 'glance-unittest-formatinspector-%s-' % subformat ++ fn = tempfile.mktemp(prefix=prefix, suffix='.vmdk') ++ self._created_files.append(fn) ++ raw = tempfile.mktemp(prefix=prefix, suffix='.raw') ++ self._created_files.append(raw) ++ ++ # Create a file with pseudo-random data, otherwise it will get ++ # compressed in the streamOptimized format ++ subprocess.check_output( ++ 'dd if=/dev/urandom of=%s bs=1M count=%i' % (raw, size_mb), ++ shell=True) ++ ++ # Convert it to VMDK ++ subprocess.check_output( ++ 'qemu-img convert -f raw -O vmdk -o subformat=%s -S 0 %s %s' % ( ++ subformat, raw, fn), ++ shell=True) ++ return fn ++ ++ def _test_format_at_block_size(self, format_name, img, block_size): ++ fmt = format_inspector.get_inspector(format_name)() ++ self.assertIsNotNone(fmt, ++ 'Did not get format inspector for %s' % ( ++ format_name)) ++ wrapper = format_inspector.InfoWrapper(open(img, 'rb'), fmt) ++ ++ while True: ++ chunk = wrapper.read(block_size) ++ if not chunk: ++ break ++ ++ wrapper.close() ++ return fmt ++ ++ def _test_format_at_image_size(self, format_name, image_size, ++ subformat=None): ++ img = self._create_img(format_name, image_size, subformat=subformat) ++ ++ # Some formats have internal alignment restrictions making this not ++ # always exactly like image_size, so get the real value for comparison ++ virtual_size = get_size_from_qemu_img(img) ++ ++ # Read the format in various sizes, some of which will read whole ++ # sections in a single read, others will be completely unaligned, etc. ++ for block_size in (64 * units.Ki, 512, 17, 1 * units.Mi): ++ fmt = self._test_format_at_block_size(format_name, img, block_size) ++ self.assertTrue(fmt.format_match, ++ 'Failed to match %s at size %i block %i' % ( ++ format_name, image_size, block_size)) ++ self.assertEqual(virtual_size, fmt.virtual_size, ++ ('Failed to calculate size for %s at size %i ' ++ 'block %i') % (format_name, image_size, ++ block_size)) ++ memory = sum(fmt.context_info.values()) ++ self.assertLess(memory, 512 * units.Ki, ++ 'Format used more than 512KiB of memory: %s' % ( ++ fmt.context_info)) ++ ++ def _test_format(self, format_name, subformat=None): ++ # Try a few different image sizes, including some odd and very small ++ # sizes ++ for image_size in (512, 513, 2057, 7): ++ self._test_format_at_image_size(format_name, image_size * units.Mi, ++ subformat=subformat) ++ ++ def test_qcow2(self): ++ self._test_format('qcow2') ++ ++ def test_vhd(self): ++ self._test_format('vhd') ++ ++ def test_vhdx(self): ++ self._test_format('vhdx') ++ ++ def test_vmdk(self): ++ self._test_format('vmdk') ++ ++ def test_vmdk_stream_optimized(self): ++ self._test_format('vmdk', 'streamOptimized') ++ ++ def test_from_file_reads_minimum(self): ++ img = self._create_img('qcow2', 10 * units.Mi) ++ file_size = os.stat(img).st_size ++ fmt = format_inspector.QcowInspector.from_file(img) ++ # We know everything we need from the first 512 bytes of a QCOW image, ++ # so make sure that we did not read the whole thing when we inspect ++ # a local file. ++ self.assertLess(fmt.actual_size, file_size) ++ ++ def test_qed_always_unsafe(self): ++ img = self._create_img('qed', 10 * units.Mi) ++ fmt = format_inspector.get_inspector('qed').from_file(img) ++ self.assertTrue(fmt.format_match) ++ self.assertFalse(fmt.safety_check()) ++ ++ def _test_vmdk_bad_descriptor_offset(self, subformat=None): ++ format_name = 'vmdk' ++ image_size = 10 * units.Mi ++ descriptorOffsetAddr = 0x1c ++ BAD_ADDRESS = 0x400 ++ img = self._create_img(format_name, image_size, subformat=subformat) ++ ++ # Corrupt the header ++ fd = open(img, 'r+b') ++ fd.seek(descriptorOffsetAddr) ++ fd.write(struct.pack('`_ for details. ++fixes: ++ - | ++ `Bug #2059809 `_: ++ Fixed issue where a qcow2 format image with an external data file ++ could expose host information. Such an image is now rejected with ++ an ``ImageUnacceptable`` error if it is used to create a volume. ++ Given that qcow2 external data files were never supported by ++ Cinder, the only use for such an image previously was to attempt ++ to steal host information, and hence this change should have no ++ impact on users. diff -Nru cinder-21.1.0/debian/patches/series cinder-21.3.1/debian/patches/series --- cinder-21.1.0/debian/patches/series 2023-05-12 10:38:22.000000000 +0000 +++ cinder-21.3.1/debian/patches/series 2024-06-21 07:19:56.000000000 +0000 @@ -2,4 +2,5 @@ do-not-use-urllib3-from-botocore.patch remove-test_backup_s3.patch py3.11-test_rbd_iscsi_Make_tests_compatible_with_python.patch -CVE-2023-2088_Reject_unsafe_delete_attachment_calls.patch +add-params-thin_provisioning-equal-one.patch +cve-2024-32498-cinder-stable-2023.1.patch diff -Nru cinder-21.1.0/debian/rules cinder-21.3.1/debian/rules --- cinder-21.1.0/debian/rules 2023-05-12 10:38:22.000000000 +0000 +++ cinder-21.3.1/debian/rules 2024-06-21 07:19:56.000000000 +0000 @@ -46,7 +46,7 @@ ifeq (,$(findstring nocheck, $(DEB_BUILD_OPTIONS))) # See https://bugs.launchpad.net/cinder/+bug/2000436 - pkgos-dh_auto_test --no-py2 'cinder\.tests\.unit\.(?!(volume\.drivers\.test_nfs.*)|volume\.drivers\.test_rbd\.RBDTestCase\.test_delete_volume_not_found|.*backup\.drivers\.test_backup_ceph\.BackupCephTestCase.*|.*volume\.drivers\.hpe\.test_hpe3par.*)' + pkgos-dh_auto_test --no-py2 'cinder\.tests\.unit\.(?!(volume\.drivers\.test_nfs.*)|volume\.drivers\.test_rbd\.RBDTestCase\.test_delete_volume_not_found|.*backup\.drivers\.test_backup_ceph\.BackupCephTestCase.*|.*volume\.drivers\.hpe\.test_hpe3par.*|volume\.drivers\.ceph\.test_rbd_iscsi\.RBDISCSITestCase\.test_unsupported_client_version)' endif mkdir -p $(CURDIR)/debian/cinder-common/usr/share/cinder-common diff -Nru cinder-21.1.0/doc/source/configuration/block-storage/drivers/dell-emc-unity-driver.rst cinder-21.3.1/doc/source/configuration/block-storage/drivers/dell-emc-unity-driver.rst --- cinder-21.1.0/doc/source/configuration/block-storage/drivers/dell-emc-unity-driver.rst 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/doc/source/configuration/block-storage/drivers/dell-emc-unity-driver.rst 2023-08-14 16:54:18.000000000 +0000 @@ -472,7 +472,7 @@ The way could be different depending on the type of replications - sync or async. Refer to `Unity Replication White Paper -`_ +`_ for more detail. 2. Add `replication_device` to storage backend settings in `cinder.conf`, then @@ -529,7 +529,7 @@ The way could be different depending on the type of replications - sync or async. Refer to `Unity Replication White Paper -`_ +`_ for more detail. 2. Add `replication_device` to storage backend settings in `cinder.conf`, then diff -Nru cinder-21.1.0/doc/source/configuration/block-storage/drivers/infinidat-volume-driver.rst cinder-21.3.1/doc/source/configuration/block-storage/drivers/infinidat-volume-driver.rst --- cinder-21.1.0/doc/source/configuration/block-storage/drivers/infinidat-volume-driver.rst 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/doc/source/configuration/block-storage/drivers/infinidat-volume-driver.rst 2023-08-14 16:54:18.000000000 +0000 @@ -25,6 +25,7 @@ * Revert a volume to a snapshot. * Manage and unmanage volumes and snapshots. * List manageable volumes and snapshots. +* Attach a volume to multiple instances at once (multi-attach). External package installation ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff -Nru cinder-21.1.0/doc/source/configuration/block-storage/service-token.rst cinder-21.3.1/doc/source/configuration/block-storage/service-token.rst --- cinder-21.1.0/doc/source/configuration/block-storage/service-token.rst 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/doc/source/configuration/block-storage/service-token.rst 2023-08-14 16:54:18.000000000 +0000 @@ -1,6 +1,6 @@ -========================================================= -Using service tokens to prevent long-running job failures -========================================================= +==================== +Using service tokens +==================== When a user initiates a request whose processing involves multiple services (for example, a boot-from-volume request to the Compute Service will require @@ -8,20 +8,32 @@ Image Service), the user's token is handed from service to service. This ensures that the requestor is tracked correctly for audit purposes and also guarantees that the requestor has the appropriate permissions to do what needs -to be done by the other services. If the chain of operations takes a long -time, however, the user's token may expire before the action is completed, -leading to the failure of the user's original request. - -One way to deal with this is to set a long token life in Keystone, and this may -be what you are currently doing. But this can be problematic for installations -whose security policies prefer short user token lives. Beginning with the -Queens release, an alternative solution is available. You have the ability to -configure some services (particularly Nova and Cinder) to send a "service -token" along with the user's token. When properly configured, the Identity -Service will validate an expired user token *when it is accompanied by a valid -service token*. Thus if the user's token expires somewhere during a long -running chain of operations among various OpenStack services, the operations -can continue. +to be done by the other services. + +There are several instances where we want to differentiate between a request +coming from the user to one coming from another OpenStack service on behalf of +the user: + +- **For security reasons** There are some operations in the Block Storage + service, required for normal operations, that could be exploited by a + malicious user to gain access to resources belonging to other users. By + differentiating when the request comes directly from a user and when from + another OpenStack service the Cinder service can protect the deployment. + +- To prevent long-running job failures: If the chain of operations takes a long + time, the user's token may expire before the action is completed, leading to + the failure of the user's original request. + + One way to deal with this is to set a long token life in Keystone, and this + may be what you are currently doing. But this can be problematic for + installations whose security policies prefer short user token lives. + Beginning with the Queens release, an alternative solution is available. You + have the ability to configure some services (particularly Nova and Cinder) to + send a "service token" along with the user's token. When properly + configured, the Identity Service will validate an expired user token *when it + is accompanied by a valid service token*. Thus if the user's token expires + somewhere during a long running chain of operations among various OpenStack + services, the operations can continue. .. note:: There's nothing special about a service token. It's a regular token diff -Nru cinder-21.1.0/doc/source/configuration/index.rst cinder-21.3.1/doc/source/configuration/index.rst --- cinder-21.1.0/doc/source/configuration/index.rst 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/doc/source/configuration/index.rst 2023-08-14 16:54:18.000000000 +0000 @@ -6,6 +6,7 @@ :maxdepth: 1 block-storage/block-storage-overview.rst + block-storage/service-token.rst block-storage/volume-drivers.rst block-storage/backup-drivers.rst block-storage/schedulers.rst @@ -15,10 +16,16 @@ block-storage/policy-config-HOWTO.rst block-storage/fc-zoning.rst block-storage/volume-encryption.rst - block-storage/service-token.rst block-storage/config-options.rst block-storage/samples/index.rst +.. warning:: + + For security reasons **Service Tokens must to be configured** in OpenStack + for Cinder to operate securely. Pay close attention to the :doc:`specific + section describing it: `. See + https://bugs.launchpad.net/nova/+bug/2004555 for details. + .. note:: The examples of common configurations for shared diff -Nru cinder-21.1.0/doc/source/install/index.rst cinder-21.3.1/doc/source/install/index.rst --- cinder-21.1.0/doc/source/install/index.rst 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/doc/source/install/index.rst 2023-08-14 16:54:18.000000000 +0000 @@ -35,6 +35,13 @@ The following links describe how to install the Cinder Block Storage Service: +.. warning:: + + For security reasons **Service Tokens must to be configured** in OpenStack + for Cinder to operate securely. Pay close attention to the :doc:`specific + section describing it: <../configuration/block-storage/service-token>`. See + https://bugs.launchpad.net/nova/+bug/2004555 for details. + .. toctree:: get-started-block-storage diff -Nru cinder-21.1.0/releasenotes/notes/backup-volumenotfound-set-to-error-fa47b3631093a702.yaml cinder-21.3.1/releasenotes/notes/backup-volumenotfound-set-to-error-fa47b3631093a702.yaml --- cinder-21.1.0/releasenotes/notes/backup-volumenotfound-set-to-error-fa47b3631093a702.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/backup-volumenotfound-set-to-error-fa47b3631093a702.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,5 @@ +--- +fixes: + - | + `Bug #1996049 `_: Fixed bug + where backup was not set to error on failure when volume did not exist. diff -Nru cinder-21.1.0/releasenotes/notes/bug1945500-e4df056b8be2e0ef.yaml cinder-21.3.1/releasenotes/notes/bug1945500-e4df056b8be2e0ef.yaml --- cinder-21.1.0/releasenotes/notes/bug1945500-e4df056b8be2e0ef.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/bug1945500-e4df056b8be2e0ef.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,8 @@ +--- +fixes: + - | + `Bug #1945500 `_: The + original attempt at fixing this bug did not account for differences in + how glance and cinder store image metadata, and as a result some image + properties were not filtered out. This new improved fix addresses those + differences and makes the filtering more thorough. diff -Nru cinder-21.1.0/releasenotes/notes/bug-1962824-ff0cac0d35021f84.yaml cinder-21.3.1/releasenotes/notes/bug-1962824-ff0cac0d35021f84.yaml --- cinder-21.1.0/releasenotes/notes/bug-1962824-ff0cac0d35021f84.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/bug-1962824-ff0cac0d35021f84.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,9 @@ +--- +fixes: + - | + PowerStore driver `bug #1962824 + `_: Fixed Cinder + volume caching mechanism for the driver. Now the driver + correctly raises ``exception.SnapshotLimitReached`` when maximum + snapshots are created for a given volume and the volume cache is + invalidated to allow a new row of fast volume clones. diff -Nru cinder-21.1.0/releasenotes/notes/bug-1982350-infinidat-fix-multi-attach-19f62d182b675e59.yaml cinder-21.3.1/releasenotes/notes/bug-1982350-infinidat-fix-multi-attach-19f62d182b675e59.yaml --- cinder-21.1.0/releasenotes/notes/bug-1982350-infinidat-fix-multi-attach-19f62d182b675e59.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/bug-1982350-infinidat-fix-multi-attach-19f62d182b675e59.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,9 @@ +--- +fixes: + - | + Infinidat Driver `bug #1982350 + `_: + Fixed Infinidat driver multi-attach feature. + Added a check if there are multiple attachments to the volume + from the same connector and terminate connection only for the + last attachment from the corresponding host. diff -Nru cinder-21.1.0/releasenotes/notes/bug-1982405-infinidat-fix-generic-volume-migration-da33a6fe980ac4eb.yaml cinder-21.3.1/releasenotes/notes/bug-1982405-infinidat-fix-generic-volume-migration-da33a6fe980ac4eb.yaml --- cinder-21.1.0/releasenotes/notes/bug-1982405-infinidat-fix-generic-volume-migration-da33a6fe980ac4eb.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/bug-1982405-infinidat-fix-generic-volume-migration-da33a6fe980ac4eb.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,7 @@ +--- +fixes: + - | + Infinidat Driver `bug #1982405 + `_: + Fixed Infinidat driver to allow generic volume migration + between two storage pools within the same cluster. diff -Nru cinder-21.1.0/releasenotes/notes/bug-2008017-netapp-fix-native-threads-04d8f58f4c29b03d.yaml cinder-21.3.1/releasenotes/notes/bug-2008017-netapp-fix-native-threads-04d8f58f4c29b03d.yaml --- cinder-21.1.0/releasenotes/notes/bug-2008017-netapp-fix-native-threads-04d8f58f4c29b03d.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/bug-2008017-netapp-fix-native-threads-04d8f58f4c29b03d.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,6 @@ +--- +fixes: + - | + `Bug #2008017 `_: Fixed + NetApp NFS driver to never spawn a native thread avoid thread starvation + and other related issues. diff -Nru cinder-21.1.0/releasenotes/notes/fix-reserved-image-properties-9519ddc080e7ed1a.yaml cinder-21.3.1/releasenotes/notes/fix-reserved-image-properties-9519ddc080e7ed1a.yaml --- cinder-21.1.0/releasenotes/notes/fix-reserved-image-properties-9519ddc080e7ed1a.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/fix-reserved-image-properties-9519ddc080e7ed1a.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,28 @@ +--- +upgrade: + - | + We introduced a new config parameter, ``reserved_image_namespaces``, + that allows operators to set the image properties to filter out from + volume image metadata by namespace when uploading a volume to Glance. + These properties, if not filtered out, cause failures when uploading + images back to Glance. The error will happen on Glance side when the + reserved namespaces are used. This option is also useful when an operator + wants to use the Glance property protections feature to make some image + properties read-only. +fixes: + - | + `Bug #1945500 `_: Fixed + an error when uploading to Glance a previously downloaded glance image + when glance multistore is enabled. Glance reserves image properties + in the namespace 'os_glance' for its own use and will not allow + images to be created with these properties. Additionally, there are image + properties, such as those associated with image signature verification, + that are stored in a volume's image metadata, which should not be added + to a new image when a volume is being uploaded as an image. Thus Cinder + will no longer include any volume image metadata in the namespaces + ``os_glance`` and ``img_signature`` when it creates an image in Glance. + Furthermore, because the Glance property protections feature allows an + operator to configure specific image properties as read-only, this fix + adds a configuration option, ``reserved_image_namespaces``, that allows an + operator to exclude additional image properties by namespace (the + ``os_glance`` and ``img_signature`` namespaces are *always* excluded). diff -Nru cinder-21.1.0/releasenotes/notes/pure-validate-replica-arrays-a76630cab9435770.yaml cinder-21.3.1/releasenotes/notes/pure-validate-replica-arrays-a76630cab9435770.yaml --- cinder-21.1.0/releasenotes/notes/pure-validate-replica-arrays-a76630cab9435770.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/pure-validate-replica-arrays-a76630cab9435770.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,6 @@ +--- +fixes: + - | + Pure Storage FlashArray driver `bug #1969784 + `_: Fixed array failover + incorrectly handles loss of an array due to network issue diff -Nru cinder-21.1.0/releasenotes/notes/rbd-rbd_secret_uuid-fsid-95daee128f59c8e4.yaml cinder-21.3.1/releasenotes/notes/rbd-rbd_secret_uuid-fsid-95daee128f59c8e4.yaml --- cinder-21.1.0/releasenotes/notes/rbd-rbd_secret_uuid-fsid-95daee128f59c8e4.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/rbd-rbd_secret_uuid-fsid-95daee128f59c8e4.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,5 @@ +--- +features: + - | + RBD driver: Sets the Ceph cluster FSID as the default value for the + ``rbd_secret_uuid`` configuration option. diff -Nru cinder-21.1.0/releasenotes/notes/rbd-total_capacity-60f10b45e3a8c8ea.yaml cinder-21.3.1/releasenotes/notes/rbd-total_capacity-60f10b45e3a8c8ea.yaml --- cinder-21.1.0/releasenotes/notes/rbd-total_capacity-60f10b45e3a8c8ea.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/rbd-total_capacity-60f10b45e3a8c8ea.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,8 @@ +--- +fixes: + - | + RBD driver `bug #1960206 + `_: Fixed + ``total_capacity`` reported by the driver to the scheduler on Ceph clusters + that have renamed the ``bytes_used`` field to ``stored``. (e.g., `Nautilus + `_). diff -Nru cinder-21.1.0/releasenotes/notes/redirect-detach-nova-4b7b7902d7d182e0.yaml cinder-21.3.1/releasenotes/notes/redirect-detach-nova-4b7b7902d7d182e0.yaml --- cinder-21.1.0/releasenotes/notes/redirect-detach-nova-4b7b7902d7d182e0.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/redirect-detach-nova-4b7b7902d7d182e0.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,42 @@ +--- +critical: + - | + Detaching volumes will fail if Nova is not `configured to send service + tokens `_, + please read the upgrade section for more information. (`Bug #2004555 + `_). +upgrade: + - | + Nova must be `configured to send service tokens + `_ + **and** cinder must be configured to recognize at least one of the roles + that the nova service user has been assigned in keystone. By default, + cinder will recognize the ``service`` role, so if the nova service user + is assigned a differently named role in your cloud, you must adjust your + cinder configuration file (``service_token_roles`` configuration option + in the ``keystone_authtoken`` section). If nova and cinder are not + configured correctly in this regard, detaching volumes will no longer + work (`Bug #2004555 `_). +security: + - | + As part of the fix for `Bug #2004555 + `_, cinder now rejects + user attachment delete requests for attachments that are being used by nova + instances to ensure that no leftover devices are produced on the compute + nodes which could be used to access another project's volumes. Terminate + connection, detach, and force detach volume actions are not allowed for + users. +fixes: + - | + `Bug #2004555 `_: Fixed + issue where a user manually deleting an attachment, calling terminate + connection, detach, or force detach, for a volume that is still used by a + nova instance resulted in leftover devices on the compute node. These + operations will now fail. +issues: + - | + For security reasons (`Bug #2004555 + `_) manually deleting an + attachment, manually doing the ``os-terminate_connection`` ``os-detach`` or + ``os-force_detach`` actions will no longer be allowed unless the request is + coming from another OpenStack service on behalf of a user. diff -Nru cinder-21.1.0/releasenotes/notes/remove-multiattach-request-param-4444e02533f919da.yaml cinder-21.3.1/releasenotes/notes/remove-multiattach-request-param-4444e02533f919da.yaml --- cinder-21.1.0/releasenotes/notes/remove-multiattach-request-param-4444e02533f919da.yaml 1970-01-01 00:00:00.000000000 +0000 +++ cinder-21.3.1/releasenotes/notes/remove-multiattach-request-param-4444e02533f919da.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -0,0 +1,20 @@ +--- +fixes: + - | + `Bug #2008259 `_: + Fixed the volume create functionality where non-admin users were + able to create multiattach volumes by providing the `multiattach` + parameter in the request body. Now we can only create multiattach + volumes using a multiattach volume type, which is also the + recommended way. +other: + - | + Removed the ability to create multiattach volumes by specifying + `multiattach` parameter in the request body of a volume create + operation. This functionality is unsafe, can lead to data loss, + and has been deprecated since the Queens release. + The recommended method for creating a multiattach volume is to + use a volume type that supports multiattach. By default, volume + types can only be created by the operator. Users who have a need + for multiattach volumes should contact their operator if a suitable + volume type is not available. diff -Nru cinder-21.1.0/tox.ini cinder-21.3.1/tox.ini --- cinder-21.1.0/tox.ini 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/tox.ini 2023-08-14 16:54:18.000000000 +0000 @@ -1,6 +1,8 @@ [tox] minversion = 3.18.0 -requires = virtualenv>=20.4.2 +requires = + virtualenv>=20.4.2 + tox<4 skipsdist = True # python runtimes: https://governance.openstack.org/tc/reference/project-testing-interface.html#tested-runtimes envlist = py3,compliance,pep8 diff -Nru cinder-21.1.0/.zuul.yaml cinder-21.3.1/.zuul.yaml --- cinder-21.1.0/.zuul.yaml 2023-01-24 23:07:10.000000000 +0000 +++ cinder-21.3.1/.zuul.yaml 2023-08-14 16:54:18.000000000 +0000 @@ -1,4 +1,6 @@ - project: + vars: + ensure_tox_version: '<4' templates: - openstack-python3-zed-jobs - openstack-python3-zed-jobs-arm64