From 2d1242ae69378dae8ce26a51af3d860a2875eba0 Mon Sep 17 00:00:00 2001 From: lisali Date: Fri, 20 May 2016 14:48:02 +0800 Subject: [PATCH] Retype encrypted volumes The patch enables the function to retype a volume to a volume type with different encryptions. The correspoding tempest is added in https://review.openstack.org/#/c/343993/, and it succeeds in master branch. Change-Id: I66d1cfad7c37215cadeca9b7d07cb646fb35b50f Implements: blueprint retype-encrypted-volume --- cinder/db/api.py | 4 - cinder/db/sqlalchemy/api.py | 24 ------ .../unit/api/contrib/test_volume_actions.py | 6 +- cinder/tests/unit/test_volume.py | 34 +++++++-- cinder/tests/unit/test_volume_types.py | 43 +++++++++++ cinder/tests/unit/test_volume_utils.py | 47 ++++++++++++ cinder/volume/api.py | 11 ++- cinder/volume/manager.py | 74 +++++++++++++++---- cinder/volume/utils.py | 17 +++++ cinder/volume/volume_types.py | 21 +++++- ...ype-encrypted-volume-49b66d3e8e65f9a5.yaml | 4 + 11 files changed, 226 insertions(+), 59 deletions(-) create mode 100644 releasenotes/notes/retype-encrypted-volume-49b66d3e8e65f9a5.yaml diff --git a/cinder/db/api.py b/cinder/db/api.py index 43215567d57..e341868130a 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -372,10 +372,6 @@ def volume_has_attachments_filter(): return IMPL.volume_has_attachments_filter() -def volume_has_same_encryption_type(new_vol_type): - return IMPL.volume_has_same_encryption_type(new_vol_type) - - def volume_qos_allows_retype(new_vol_type): return IMPL.volume_qos_allows_retype(new_vol_type) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 59d86966c86..e832ee00f55 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -42,7 +42,6 @@ import six import sqlalchemy from sqlalchemy import MetaData from sqlalchemy import or_, and_, case -from sqlalchemy.orm import aliased from sqlalchemy.orm import joinedload, joinedload_all, undefer_group from sqlalchemy.orm import RelationshipProperty from sqlalchemy.schema import Table @@ -2264,29 +2263,6 @@ def volume_has_attachments_filter(): ~models.VolumeAttachment.deleted)) -def volume_has_same_encryption_type(new_vol_type): - """Filter to check that encryption matches with new volume type. - - They match if both don't have encryption or both have the same Encryption. - """ - # Query for the encryption in the new volume type - encryption_alias = aliased(models.Encryption) - new_enc = sql.select((encryption_alias.encryption_id,)).where(and_( - ~encryption_alias.deleted, - encryption_alias.volume_type_id == new_vol_type)) - - # Query for the encryption in the old volume type - old_enc = sql.select((models.Encryption.encryption_id,)).where(and_( - ~models.Encryption.deleted, - models.Encryption.volume_type_id == models.Volume.volume_type_id)) - - # NOTE(geguileo): This query is optimizable, but at this moment I can't - # figure out how. - return or_(and_(new_enc.as_scalar().is_(None), - old_enc.as_scalar().is_(None)), - new_enc.as_scalar() == old_enc.as_scalar()) - - def volume_qos_allows_retype(new_vol_type): """Filter to check that qos allows retyping the volume to new_vol_type. diff --git a/cinder/tests/unit/api/contrib/test_volume_actions.py b/cinder/tests/unit/api/contrib/test_volume_actions.py index 5e8e7a678ac..caf05625271 100644 --- a/cinder/tests/unit/api/contrib/test_volume_actions.py +++ b/cinder/tests/unit/api/contrib/test_volume_actions.py @@ -739,16 +739,16 @@ class VolumeRetypeActionsTest(test.TestCase): self._retype_volume_encryption('available', 202, False, False, False) def test_retype_volume_orig_no_type_dest_enc(self): - self._retype_volume_encryption('available', 400, False, False) + self._retype_volume_encryption('available', 202, False, False) def test_retype_volume_orig_type_no_enc_dest_no_enc(self): self._retype_volume_encryption('available', 202, True, False, False) def test_retype_volume_orig_type_no_enc_dest_enc(self): - self._retype_volume_encryption('available', 400, True, False) + self._retype_volume_encryption('available', 202, True, False) def test_retype_volume_orig_type_enc_dest_enc(self): - self._retype_volume_encryption('available', 400) + self._retype_volume_encryption('available', 202) def stub_volume_get(self, context, volume_id): diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index ad19ef53ae0..e1006b12446 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -5219,7 +5219,8 @@ class VolumeMigrationTestCase(BaseVolumeTestCase): def _retype_volume_exec(self, driver, mock_notify, snap=False, policy='on-demand', migrate_exc=False, exc=None, diff_equal=False, - replica=False, reserve_vol_type_only=False): + replica=False, reserve_vol_type_only=False, + encryption_changed=False): elevated = context.get_admin_context() project_id = self.context.project_id @@ -5279,7 +5280,10 @@ class VolumeMigrationTestCase(BaseVolumeTestCase): mock.patch.object(db.sqlalchemy.api, 'volume_get') as mock_get: mock_get.return_value = volume _retype.return_value = driver - _diff.return_value = ({}, diff_equal) + returned_diff = {} + if encryption_changed: + returned_diff = {'encryption': 'fake'} + _diff.return_value = (returned_diff, diff_equal) if migrate_exc: _mig.side_effect = KeyError else: @@ -5344,6 +5348,8 @@ class VolumeMigrationTestCase(BaseVolumeTestCase): self.assertEqual(CONF.host, volume.host) self.assertEqual(0, volumes_in_use) mock_notify.assert_not_called() + if encryption_changed: + self.assertTrue(_mig.called) def test_retype_volume_driver_success(self): self._retype_volume_exec(True) @@ -5373,6 +5379,9 @@ class VolumeMigrationTestCase(BaseVolumeTestCase): def test_retype_volume_with_type_only(self): self._retype_volume_exec(True, reserve_vol_type_only=True) + def test_retype_volume_migration_encryption(self): + self._retype_volume_exec(False, encryption_changed=True) + def test_migrate_driver_not_initialized(self): volume = tests_utils.create_volume(self.context, size=0, host=CONF.host) @@ -5995,6 +6004,7 @@ class DriverTestCase(test.TestCase): self.volume.delete_volume(self.context, volume_id) +@ddt.ddt class GenericVolumeDriverTestCase(DriverTestCase): """Test case for VolumeDriver.""" driver_name = "cinder.tests.fake_driver.LoggingVolumeDriver" @@ -6215,7 +6225,12 @@ class GenericVolumeDriverTestCase(DriverTestCase): @mock.patch.object(cinder.volume.manager.VolumeManager, '_detach_volume') @mock.patch.object(volutils, 'copy_volume') @mock.patch.object(volume_rpcapi.VolumeAPI, 'get_capabilities') + @mock.patch.object(cinder.volume.volume_types, + 'volume_types_encryption_changed') + @ddt.data(False, True) def test_copy_volume_data_mgr(self, + encryption_changed, + mock_encryption_changed, mock_get_capabilities, mock_copy, mock_detach, @@ -6228,17 +6243,24 @@ class GenericVolumeDriverTestCase(DriverTestCase): dest_vol = tests_utils.create_volume(self.context, size=1, host=CONF.host) mock_get_connector.return_value = {} + mock_encryption_changed.return_value = encryption_changed self.volume.driver._throttle = mock.MagicMock() attach_expected = [ - mock.call(self.context, dest_vol, {}, remote=False), - mock.call(self.context, src_vol, {}, remote=False)] + mock.call(self.context, dest_vol, {}, + remote=False, + attach_encryptor=encryption_changed), + mock.call(self.context, src_vol, {}, + remote=False, + attach_encryptor=encryption_changed)] detach_expected = [ mock.call(self.context, {'device': {'path': 'bar'}}, - dest_vol, {}, force=False, remote=False), + dest_vol, {}, force=False, remote=False, + attach_encryptor=encryption_changed), mock.call(self.context, {'device': {'path': 'foo'}}, - src_vol, {}, force=False, remote=False)] + src_vol, {}, force=False, remote=False, + attach_encryptor=encryption_changed)] attach_volume_returns = [ {'device': {'path': 'bar'}}, diff --git a/cinder/tests/unit/test_volume_types.py b/cinder/tests/unit/test_volume_types.py index 677c87ac3f1..6ad216b6180 100644 --- a/cinder/tests/unit/test_volume_types.py +++ b/cinder/tests/unit/test_volume_types.py @@ -511,3 +511,46 @@ class VolumeTypeTestCase(test.TestCase): volume_types.create(self.ctxt, "type-test", is_public=False) vtype = volume_types.get_volume_type_by_name(self.ctxt, 'type-test') self.assertIsNotNone(vtype.get('extra_specs', None)) + + @mock.patch('cinder.volume.volume_types.get_volume_type_encryption') + def _exec_volume_types_encryption_changed(self, enc1, enc2, + expected_result, + mock_get_encryption): + def _get_encryption(ctxt, type_id): + if enc1 and enc1['volume_type_id'] == type_id: + return enc1 + if enc2 and enc2['volume_type_id'] == type_id: + return enc2 + return None + + mock_get_encryption.side_effect = _get_encryption + actual_result = volume_types.volume_types_encryption_changed( + self.ctxt, fake.VOLUME_TYPE_ID, fake.VOLUME_TYPE2_ID) + self.assertEqual(expected_result, actual_result) + + def test_volume_types_encryption_changed(self): + enc1 = {'volume_type_id': fake.VOLUME_TYPE_ID, + 'cipher': 'fake', + 'created_at': 'time1', } + enc2 = {'volume_type_id': fake.VOLUME_TYPE2_ID, + 'cipher': 'fake', + 'created_at': 'time2', } + self._exec_volume_types_encryption_changed(enc1, enc2, False) + + def test_volume_types_encryption_changed2(self): + enc1 = {'volume_type_id': fake.VOLUME_TYPE_ID, + 'cipher': 'fake1', + 'created_at': 'time1', } + enc2 = {'volume_type_id': fake.VOLUME_TYPE2_ID, + 'cipher': 'fake2', + 'created_at': 'time1', } + self._exec_volume_types_encryption_changed(enc1, enc2, True) + + def test_volume_types_encryption_changed3(self): + self._exec_volume_types_encryption_changed(None, None, False) + + def test_volume_types_encryption_changed4(self): + enc1 = {'volume_type_id': fake.VOLUME_TYPE_ID, + 'cipher': 'fake1', + 'created_at': 'time1', } + self._exec_volume_types_encryption_changed(enc1, None, True) diff --git a/cinder/tests/unit/test_volume_utils.py b/cinder/tests/unit/test_volume_utils.py index e848838b2b4..f6eb281e6a3 100644 --- a/cinder/tests/unit/test_volume_utils.py +++ b/cinder/tests/unit/test_volume_utils.py @@ -22,10 +22,13 @@ import mock import six from oslo_concurrency import processutils +from oslo_config import cfg from cinder import context +from cinder import db from cinder.db.sqlalchemy import models from cinder import exception +from cinder import keymgr from cinder.objects import fields from cinder import test from cinder.tests.unit.backup import fake_backup @@ -35,6 +38,10 @@ from cinder.tests.unit import fake_volume from cinder import utils from cinder.volume import throttling from cinder.volume import utils as volume_utils +from cinder.volume import volume_types + + +CONF = cfg.CONF class NotifyUsageTestCase(test.TestCase): @@ -873,3 +880,43 @@ class VolumeUtilsTestCase(test.TestCase): self.assertEqual( expected_dict, volume_utils.convert_config_string_to_dict(test_string)) + + @mock.patch('cinder.volume.volume_types.is_encrypted', return_value=False) + def test_create_encryption_key_unencrypted(self, is_encrypted): + result = volume_utils.create_encryption_key(mock.ANY, + mock.ANY, + fake.VOLUME_TYPE_ID) + self.assertIsNone(result) + + @mock.patch('cinder.volume.volume_types.is_encrypted', return_value=True) + @mock.patch('cinder.volume.volume_types.get_volume_type_encryption') + @mock.patch('cinder.keymgr.conf_key_mgr.ConfKeyManager.create_key') + def test_create_encryption_key_encrypted(self, create_key, + get_volume_type_encryption, + is_encryption): + enc_key = {'cipher': 'aes-xts-plain64', + 'key_size': 256, + 'provider': 'p1', + 'control_location': 'front-end', + 'encryption_id': 'uuid1'} + ctxt = context.get_admin_context() + type_ref1 = volume_types.create(ctxt, "type1") + encryption = db.volume_type_encryption_create( + ctxt, type_ref1['id'], enc_key) + get_volume_type_encryption.return_value = encryption + CONF.set_override( + 'api_class', + 'cinder.keymgr.conf_key_mgr.ConfKeyManager', + group='key_manager') + key_manager = keymgr.API() + volume_utils.create_encryption_key(ctxt, + key_manager, + fake.VOLUME_TYPE_ID) + is_encryption.assert_called_once_with(ctxt, + fake.VOLUME_TYPE_ID) + get_volume_type_encryption.assert_called_once_with( + ctxt, + fake.VOLUME_TYPE_ID) + create_key.assert_called_once_with(ctxt, + algorithm='aes', + length=256) diff --git a/cinder/volume/api.py b/cinder/volume/api.py index ecc968bb912..9c0fc4c2735 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -1488,18 +1488,17 @@ class API(base.Base): # We don't support changing QoS at the front-end yet for in-use volumes # TODO(avishay): Call Nova to change QoS setting (libvirt has support # - virDomainSetBlockIoTune() - Nova does not have support yet). - filters = [db.volume_has_same_encryption_type(vol_type_id), - db.volume_qos_allows_retype(vol_type_id)] + filters = [db.volume_qos_allows_retype(vol_type_id)] updates = {'status': 'retyping', 'previous_status': objects.Volume.model.status} if not volume.conditional_update(updates, expected, filters): msg = _('Retype needs volume to be in available or in-use state, ' - 'have same encryption requirements, not be part of an ' - 'active migration or a group, requested type has to be ' - 'different that the one from the volume, and for in-use ' - 'volumes front-end qos specs cannot change.') + 'not be part of an active migration or a consistency ' + 'group, requested type has to be different that the ' + 'one from the volume, and for in-use volumes front-end ' + 'qos specs cannot change.') LOG.error(msg) QUOTAS.rollback(context, reservations + old_reservations, project_id=volume.project_id) diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 3fe6966974c..9adb2bd7ef2 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -59,6 +59,7 @@ from cinder import context from cinder import coordination from cinder import exception from cinder import flow_utils +from cinder import keymgr as key_manager from cinder.i18n import _, _LE, _LI, _LW from cinder.image import cache as image_cache from cinder.image import glance @@ -228,6 +229,7 @@ class VolumeManager(manager.SchedulerDependentManager): requests.packages.urllib3.disable_warnings( requests.packages.urllib3.exceptions.InsecurePlatformWarning) + self.key_manager = key_manager.API(CONF) self.driver = importutils.import_object( volume_driver, configuration=self.configuration, @@ -1570,7 +1572,8 @@ class VolumeManager(manager.SchedulerDependentManager): return {'conn': conn, 'device': vol_handle, 'connector': connector} - def _attach_volume(self, ctxt, volume, properties, remote=False): + def _attach_volume(self, ctxt, volume, properties, remote=False, + attach_encryptor=False): status = volume['status'] if remote: @@ -1586,11 +1589,35 @@ class VolumeManager(manager.SchedulerDependentManager): else: conn = self.initialize_connection(ctxt, volume['id'], properties) - return self._connect_device(conn) + attach_info = self._connect_device(conn) + try: + if attach_encryptor and ( + volume_types.is_encrypted(ctxt, + volume.volume_type_id)): + encryption = self.db.volume_encryption_metadata_get( + ctxt.elevated(), volume.id) + if encryption: + utils.brick_attach_volume_encryptor(ctxt, + attach_info, + encryption) + except Exception: + with excutils.save_and_reraise_exception(): + LOG.error(_LE("Failed to attach volume encryptor" + " %(vol)s."), {'vol': volume['id']}) + self._detach_volume(ctxt, attach_info, volume, properties) + return attach_info def _detach_volume(self, ctxt, attach_info, volume, properties, - force=False, remote=False): + force=False, remote=False, + attach_encryptor=False): connector = attach_info['connector'] + if attach_encryptor and ( + volume_types.is_encrypted(ctxt, + volume.volume_type_id)): + encryption = self.db.volume_encryption_metadata_get( + ctxt.elevated(), volume.id) + if encryption: + utils.brick_detach_volume_encryptor(attach_info, encryption) connector.disconnect_volume(attach_info['conn']['data'], attach_info['device']) @@ -1613,22 +1640,34 @@ class VolumeManager(manager.SchedulerDependentManager): LOG.debug('copy_data_between_volumes %(src)s -> %(dest)s.', {'src': src_vol['name'], 'dest': dest_vol['name']}) - + attach_encryptor = False + # If the encryption method or key is changed, we have to + # copy data through dm-crypt. + if volume_types.volume_types_encryption_changed( + ctxt, + src_vol.volume_type_id, + dest_vol.volume_type_id): + attach_encryptor = True properties = utils.brick_get_connector_properties() dest_remote = remote in ['dest', 'both'] - dest_attach_info = self._attach_volume(ctxt, dest_vol, properties, - remote=dest_remote) + dest_attach_info = self._attach_volume( + ctxt, dest_vol, properties, + remote=dest_remote, + attach_encryptor=attach_encryptor) try: src_remote = remote in ['src', 'both'] - src_attach_info = self._attach_volume(ctxt, src_vol, properties, - remote=src_remote) + src_attach_info = self._attach_volume( + ctxt, src_vol, properties, + remote=src_remote, + attach_encryptor=attach_encryptor) except Exception: with excutils.save_and_reraise_exception(): LOG.error(_LE("Failed to attach source volume for copy.")) self._detach_volume(ctxt, dest_attach_info, dest_vol, - properties, remote=dest_remote) + properties, remote=dest_remote, + attach_encryptor=attach_encryptor) # Check the backend capabilities of migration destination host. rpcapi = volume_rpcapi.VolumeAPI() @@ -1655,11 +1694,13 @@ class VolumeManager(manager.SchedulerDependentManager): try: self._detach_volume(ctxt, dest_attach_info, dest_vol, properties, force=copy_error, - remote=dest_remote) + remote=dest_remote, + attach_encryptor=attach_encryptor) finally: self._detach_volume(ctxt, src_attach_info, src_vol, properties, force=copy_error, - remote=src_remote) + remote=src_remote, + attach_encryptor=attach_encryptor) def _migrate_volume_generic(self, ctxt, volume, host, new_type_id): rpcapi = volume_rpcapi.VolumeAPI() @@ -1669,6 +1710,12 @@ class VolumeManager(manager.SchedulerDependentManager): new_vol_values = {k: volume[k] for k in set(volume.keys()) - skip} if new_type_id: new_vol_values['volume_type_id'] = new_type_id + if volume_types.volume_types_encryption_changed( + ctxt, volume.volume_type_id, new_type_id): + encryption_key_id = vol_utils.create_encryption_key( + ctxt, self.key_manager, new_type_id) + new_vol_values['encryption_key_id'] = encryption_key_id + new_volume = objects.Volume( context=ctxt, host=host['host'], @@ -2133,7 +2180,7 @@ class VolumeManager(manager.SchedulerDependentManager): LOG.info(_LI("Extend volume completed successfully."), resource=volume) - def retype(self, ctxt, volume_id, new_type_id, host, + def retype(self, context, volume_id, new_type_id, host, migration_policy='never', reservations=None, volume=None, old_reservations=None): @@ -2146,8 +2193,6 @@ class VolumeManager(manager.SchedulerDependentManager): QUOTAS.rollback(context, old_reservations) QUOTAS.rollback(context, new_reservations) - context = ctxt.elevated() - # FIXME(dulek): Remove this in v3.0 of RPC API. if volume is None: # For older clients, mimic the old behavior and look up the volume @@ -2221,6 +2266,7 @@ class VolumeManager(manager.SchedulerDependentManager): # We assume that those that support pools do this internally # so we strip off the pools designation if (not retyped and + diff.get('encryption') is None and vol_utils.hosts_are_equivalent(self.driver.host, host['host'])): try: diff --git a/cinder/volume/utils.py b/cinder/volume/utils.py index 2d373c0fe0c..7b0d2379413 100644 --- a/cinder/volume/utils.py +++ b/cinder/volume/utils.py @@ -44,6 +44,7 @@ from cinder import objects from cinder import rpc from cinder import utils from cinder.volume import throttling +from cinder.volume import volume_types CONF = cfg.CONF @@ -797,3 +798,19 @@ def convert_config_string_to_dict(config_string): {'config_string': config_string}) return resultant_dict + + +def create_encryption_key(context, key_manager, volume_type_id): + encryption_key_id = None + if volume_types.is_encrypted(context, volume_type_id): + volume_type_encryption = ( + volume_types.get_volume_type_encryption(context, + volume_type_id)) + cipher = volume_type_encryption.cipher + length = volume_type_encryption.key_size + algorithm = cipher.split('-')[0] if cipher else None + encryption_key_id = key_manager.create_key( + context, + algorithm=algorithm, + length=length) + return encryption_key_id diff --git a/cinder/volume/volume_types.py b/cinder/volume/volume_types.py index be7b69c8fb0..5657aef8f92 100644 --- a/cinder/volume/volume_types.py +++ b/cinder/volume/volume_types.py @@ -33,6 +33,8 @@ from cinder import quota CONF = cfg.CONF LOG = logging.getLogger(__name__) QUOTAS = quota.QUOTAS +ENCRYPTION_IGNORED_FIELDS = ['volume_type_id', 'created_at', 'updated_at', + 'deleted_at'] def create(context, @@ -257,8 +259,7 @@ def volume_types_diff(context, vol_type_id1, vol_type_id2): def _fix_encryption_specs(encryption): if encryption: encryption = dict(encryption) - for param in ['volume_type_id', 'created_at', 'updated_at', - 'deleted_at']: + for param in ENCRYPTION_IGNORED_FIELDS: encryption.pop(param, None) return encryption @@ -313,3 +314,19 @@ def volume_types_diff(context, vol_type_id1, vol_type_id2): all_equal = False return (diff, all_equal) + + +def volume_types_encryption_changed(context, vol_type_id1, vol_type_id2): + """Return whether encryptions of two volume types are same.""" + def _get_encryption(enc): + enc = dict(enc) + for param in ENCRYPTION_IGNORED_FIELDS: + enc.pop(param, None) + return enc + + enc1 = get_volume_type_encryption(context, vol_type_id1) + enc2 = get_volume_type_encryption(context, vol_type_id2) + + enc1_filtered = _get_encryption(enc1) if enc1 else None + enc2_filtered = _get_encryption(enc2) if enc2 else None + return enc1_filtered != enc2_filtered diff --git a/releasenotes/notes/retype-encrypted-volume-49b66d3e8e65f9a5.yaml b/releasenotes/notes/retype-encrypted-volume-49b66d3e8e65f9a5.yaml new file mode 100644 index 00000000000..caec2736496 --- /dev/null +++ b/releasenotes/notes/retype-encrypted-volume-49b66d3e8e65f9a5.yaml @@ -0,0 +1,4 @@ +--- +features: + - Allow to retype volumes with different encryptions, including + changes from unencrypted types to encrypted types and vice-versa.