From 63385d3c4dcfb725222ffd2a854f421581596fa8 Mon Sep 17 00:00:00 2001 From: Brianna Poulos Date: Mon, 18 Nov 2013 18:04:15 -0500 Subject: [PATCH] Add update support to volume type encryption This patch adds update support to the Volume Type Encryption API Extension. It allows the updating of volume type encryption for a given volume type as long as no volumes are currently in use with that volume type. Change-Id: If5f9ef792626e7dfaf82365bb6838b964f8238ae Implements: blueprint encrypt-cinder-volumes https://blueprints.launchpad.net/nova/+spec/encrypt-cinder-volumes --- cinder/api/contrib/volume_type_encryption.py | 36 ++- cinder/db/api.py | 14 +- cinder/db/sqlalchemy/api.py | 31 ++- cinder/exception.py | 4 + .../contrib/test_volume_type_encryption.py | 255 +++++++++++------- cinder/tests/test_db_api.py | 46 +++- cinder/tests/test_volume.py | 8 +- cinder/tests/test_volume_types.py | 13 +- 8 files changed, 280 insertions(+), 127 deletions(-) diff --git a/cinder/api/contrib/volume_type_encryption.py b/cinder/api/contrib/volume_type_encryption.py index 12107cb95..8b59e30e4 100644 --- a/cinder/api/contrib/volume_type_encryption.py +++ b/cinder/api/contrib/volume_type_encryption.py @@ -125,14 +125,46 @@ class VolumeTypeEncryptionController(wsgi.Controller): self._check_encryption_input(encryption_specs) - db.volume_type_encryption_update_or_create(context, type_id, - encryption_specs) + db.volume_type_encryption_create(context, type_id, encryption_specs) notifier_info = dict(type_id=type_id, specs=encryption_specs) notifier_api.notify(context, 'volumeTypeEncryption', 'volume_type_encryption.create', notifier_api.INFO, notifier_info) return body + @wsgi.serializers(xml=VolumeTypeEncryptionTemplate) + def update(self, req, type_id, id, body=None): + """Update encryption specs for a given volume type.""" + context = req.environ['cinder.context'] + authorize(context) + + if not body: + expl = _('Request body empty.') + raise webob.exc.HTTPBadRequest(explanation=expl) + if not self.is_valid_body(body, 'encryption'): + expl = _('Update body is not valid. It must contain "encryption."') + raise webob.exc.HTTPBadRequest(explanation=expl) + if len(body) > 1: + expl = _('Request body contains too many items.') + raise webob.exc.HTTPBadRequest(explanation=expl) + + self._check_type(context, type_id) + + if self._encrypted_type_in_use(context, type_id): + expl = _('Cannot update encryption specs. Volume type in use.') + raise webob.exc.HTTPBadRequest(explanation=expl) + + encryption_specs = body['encryption'] + self._check_encryption_input(encryption_specs, create=False) + + db.volume_type_encryption_update(context, type_id, encryption_specs) + notifier_info = dict(type_id=type_id, id=id) + notifier_api.notify(context, 'volumeTypeEncryption', + 'volume_type_encryption.update', + notifier_api.INFO, notifier_info) + + return body + @wsgi.serializers(xml=VolumeTypeEncryptionTemplate) def show(self, req, type_id, id): """Return a single encryption item.""" diff --git a/cinder/db/api.py b/cinder/db/api.py index a59d5409c..77a33f607 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -452,12 +452,14 @@ def volume_type_encryption_delete(context, volume_type_id): return IMPL.volume_type_encryption_delete(context, volume_type_id) -# TODO(joel-coffman): split into two functions -- update and create -def volume_type_encryption_update_or_create(context, volume_type_id, - encryption_specs): - return IMPL.volume_type_encryption_update_or_create(context, - volume_type_id, - encryption_specs) +def volume_type_encryption_create(context, volume_type_id, encryption_specs): + return IMPL.volume_type_encryption_create(context, volume_type_id, + encryption_specs) + + +def volume_type_encryption_update(context, volume_type_id, encryption_specs): + return IMPL.volume_type_encryption_update(context, volume_type_id, + encryption_specs) def volume_type_encryption_volume_get(context, volume_type_id, session=None): diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 35f03fc2a..41b947913 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -2244,22 +2244,35 @@ def volume_type_encryption_delete(context, volume_type_id): @require_admin_context -def volume_type_encryption_update_or_create(context, volume_type_id, - values): +def volume_type_encryption_create(context, volume_type_id, values): session = get_session() - encryption = volume_type_encryption_get(context, volume_type_id, - session) - - if not encryption: + with session.begin(): encryption = models.Encryption() if 'volume_type_id' not in values: values['volume_type_id'] = volume_type_id - encryption.update(values) - encryption.save(session=session) + encryption.update(values) + encryption.save(session=session) + + return encryption + + +@require_admin_context +def volume_type_encryption_update(context, volume_type_id, values): + session = get_session() + with session.begin(): + encryption = volume_type_encryption_get(context, volume_type_id, + session) + + if not encryption: + raise exception.VolumeTypeEncryptionNotFound(type_id= + volume_type_id) + + encryption.update(values) + encryption.save(session=session) - return encryption + return encryption def volume_type_encryption_volume_get(context, volume_type_id, session=None): diff --git a/cinder/exception.py b/cinder/exception.py index dfbb05246..36e96344f 100644 --- a/cinder/exception.py +++ b/cinder/exception.py @@ -369,6 +369,10 @@ class VolumeTypeEncryptionExists(Invalid): message = _("Volume type encryption for type %(type_id)s already exists.") +class VolumeTypeEncryptionNotFound(NotFound): + message = _("Volume type encryption for type %(type_id)s does not exist.") + + class MalformedRequestBody(CinderException): message = _("Malformed message body: %(reason)s") diff --git a/cinder/tests/api/contrib/test_volume_type_encryption.py b/cinder/tests/api/contrib/test_volume_type_encryption.py index fc6bacc50..8cd7b50bf 100644 --- a/cinder/tests/api/contrib/test_volume_type_encryption.py +++ b/cinder/tests/api/contrib/test_volume_type_encryption.py @@ -24,10 +24,6 @@ from cinder import test from cinder.tests.api import fakes -def return_volume_type_encryption_db(context, volume_type_id, session): - return stub_volume_type_encryption() - - def return_volume_type_encryption(context, volume_type_id): return stub_volume_type_encryption() @@ -49,6 +45,11 @@ def volume_type_encryption_get(context, volume_type_id): class VolumeTypeEncryptionTest(test.TestCase): + _default_volume_type = { + 'id': 'fake_type_id', + 'name': 'fake_type', + } + def setUp(self): super(VolumeTypeEncryptionTest, self).setUp() self.flags(host='fake', @@ -76,15 +77,22 @@ class VolumeTypeEncryptionTest(test.TestCase): return req.get_response(fakes.wsgi_app(fake_auth_context=ctxt)) + def _create_type_and_encryption(self, volume_type, body=None): + if body is None: + body = {"encryption": stub_volume_type_encryption()} + + db.volume_type_create(context.get_admin_context(), volume_type) + + return self._get_response(volume_type, req_method='POST', + req_body=json.dumps(body), + req_headers='application/json') + def test_index(self): self.stubs.Set(db, 'volume_type_encryption_get', return_volume_type_encryption) - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } - db.volume_type_create(context.get_admin_context(), volume_type) + volume_type = self._default_volume_type + self._create_type_and_encryption(volume_type) res = self._get_response(volume_type) self.assertEqual(200, res.status_code) @@ -96,11 +104,7 @@ class VolumeTypeEncryptionTest(test.TestCase): db.volume_type_destroy(context.get_admin_context(), volume_type['id']) def test_index_invalid_type(self): - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } - + volume_type = self._default_volume_type res = self._get_response(volume_type) self.assertEqual(404, res.status_code) res_dict = json.loads(res.body) @@ -115,15 +119,8 @@ class VolumeTypeEncryptionTest(test.TestCase): self.assertEqual(expected, res_dict) def test_show_key_size(self): - self.stubs.Set(db, 'volume_type_encryption_get', - return_volume_type_encryption) - - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } - db.volume_type_create(context.get_admin_context(), volume_type) - + volume_type = self._default_volume_type + self._create_type_and_encryption(volume_type) res = self._get_response(volume_type, url='/v2/fake/types/%s/encryption/key_size') res_dict = json.loads(res.body) @@ -134,14 +131,8 @@ class VolumeTypeEncryptionTest(test.TestCase): db.volume_type_destroy(context.get_admin_context(), volume_type['id']) def test_show_provider(self): - self.stubs.Set(db, 'volume_type_encryption_get', - return_volume_type_encryption) - - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } - db.volume_type_create(context.get_admin_context(), volume_type) + volume_type = self._default_volume_type + self._create_type_and_encryption(volume_type) res = self._get_response(volume_type, url='/v2/fake/types/%s/encryption/provider') @@ -152,14 +143,8 @@ class VolumeTypeEncryptionTest(test.TestCase): db.volume_type_destroy(context.get_admin_context(), volume_type['id']) def test_show_item_not_found(self): - self.stubs.Set(db, 'volume_type_encryption_get', - return_volume_type_encryption) - - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } - db.volume_type_create(context.get_admin_context(), volume_type) + volume_type = self._default_volume_type + self._create_type_and_encryption(volume_type) res = self._get_response(volume_type, url='/v2/fake/types/%s/encryption/fake') @@ -176,10 +161,7 @@ class VolumeTypeEncryptionTest(test.TestCase): db.volume_type_destroy(context.get_admin_context(), volume_type['id']) def _create(self, cipher, control_location, key_size, provider): - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } + volume_type = self._default_volume_type db.volume_type_create(context.get_admin_context(), volume_type) body = {"encryption": {'cipher': cipher, @@ -230,10 +212,7 @@ class VolumeTypeEncryptionTest(test.TestCase): self._create('fake_cipher', 'front-end', 128, 'fake_encryptor') def test_create_xml(self): - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } + volume_type = self._default_volume_type db.volume_type_create(context.get_admin_context(), volume_type) ctxt = context.RequestContext('fake', 'fake', is_admin=True) @@ -252,17 +231,10 @@ class VolumeTypeEncryptionTest(test.TestCase): db.volume_type_destroy(context.get_admin_context(), volume_type['id']) def test_create_invalid_volume_type(self): - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } - - body = {"encryption": {'cipher': 'cipher', - 'control_location': 'front-end', - 'key_size': 128, - 'provider': 'fake_provider', - 'volume_type_id': 'volume_type'}} + volume_type = self._default_volume_type + body = {"encryption": stub_volume_type_encryption()} + # Attempt to create encryption without first creating type res = self._get_response(volume_type, req_method='POST', req_body=json.dumps(body), req_headers='application/json') @@ -281,20 +253,9 @@ class VolumeTypeEncryptionTest(test.TestCase): self.assertEqual(expected, res_dict) def test_create_encryption_type_exists(self): - self.stubs.Set(db, 'volume_type_encryption_get', - return_volume_type_encryption) - - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } - db.volume_type_create(context.get_admin_context(), volume_type) - - body = {"encryption": {'cipher': 'cipher', - 'control_location': 'front-end', - 'key_size': 128, - 'provider': 'fake_provider', - 'volume_type_id': volume_type['id']}} + volume_type = self._default_volume_type + body = {"encryption": stub_volume_type_encryption()} + self._create_type_and_encryption(volume_type, body) # Try to create encryption specs for a volume type # that already has them. @@ -315,10 +276,7 @@ class VolumeTypeEncryptionTest(test.TestCase): def test_create_volume_exists(self): # Create the volume type and a volume with the volume type. - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } + volume_type = self._default_volume_type db.volume_type_create(context.get_admin_context(), volume_type) db.volume_create(context.get_admin_context(), {'id': 'fake_id', @@ -355,10 +313,8 @@ class VolumeTypeEncryptionTest(test.TestCase): def _encryption_create_bad_body(self, body, msg='Create body is not valid.'): - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } + + volume_type = self._default_volume_type db.volume_type_create(context.get_admin_context(), volume_type) res = self._get_response(volume_type, req_method='POST', req_body=json.dumps(body), @@ -409,16 +365,11 @@ class VolumeTypeEncryptionTest(test.TestCase): self._encryption_create_bad_body(body=body, msg=msg) def test_delete(self): - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } + volume_type = self._default_volume_type db.volume_type_create(context.get_admin_context(), volume_type) # Test that before create, there's nothing with a get - res = self._get_response(volume_type, req_method='GET', - req_headers='application/json', - url='/v2/fake/types/%s/encryption') + res = self._get_response(volume_type) self.assertEqual(200, res.status_code) res_dict = json.loads(res.body) self.assertEqual({}, res_dict) @@ -459,10 +410,7 @@ class VolumeTypeEncryptionTest(test.TestCase): def test_delete_with_volume_in_use(self): # Create the volume type - volume_type = { - 'id': 'fake_type_id', - 'name': 'fake_type', - } + volume_type = self._default_volume_type db.volume_type_create(context.get_admin_context(), volume_type) body = {"encryption": {'cipher': 'cipher', @@ -534,3 +482,130 @@ class VolumeTypeEncryptionTest(test.TestCase): self.assertEqual({}, res_dict) db.volume_type_destroy(context.get_admin_context(), volume_type['id']) + + def test_update_item(self): + volume_type = self._default_volume_type + + # Create Encryption Specs + create_body = {"encryption": {'cipher': 'cipher', + 'control_location': 'front-end', + 'key_size': 128, + 'provider': 'fake_provider', + 'volume_type_id': volume_type['id']}} + self._create_type_and_encryption(volume_type, create_body) + + # Update Encryption Specs + update_body = {"encryption": {'key_size': 512, + 'provider': 'fake_provider2'}} + + res = self.\ + _get_response(volume_type, req_method='PUT', + req_body=json.dumps(update_body), + req_headers='application/json', + url='/v2/fake/types/%s/encryption/fake_type_id') + + res_dict = json.loads(res.body) + self.assertEqual(512, res_dict['encryption']['key_size']) + self.assertEqual('fake_provider2', res_dict['encryption']['provider']) + + # Get Encryption Specs + res = self._get_response(volume_type) + res_dict = json.loads(res.body) + + # Confirm Encryption Specs + self.assertEqual(512, res_dict['key_size']) + self.assertEqual('fake_provider2', res_dict['provider']) + + db.volume_type_destroy(context.get_admin_context(), volume_type['id']) + + def _encryption_update_bad_body(self, update_body, msg): + + # Create Volume Type and Encryption + volume_type = self._default_volume_type + res = self._create_type_and_encryption(volume_type) + + # Update Encryption + res = self.\ + _get_response(volume_type, req_method='PUT', + req_body=json.dumps(update_body), + req_headers='application/json', + url='/v2/fake/types/%s/encryption/fake_type_id') + + res_dict = json.loads(res.body) + + expected = { + 'badRequest': { + 'code': 400, + 'message': (msg) + } + } + + # Confirm Failure + self.assertEqual(expected, res_dict) + db.volume_type_destroy(context.get_admin_context(), volume_type['id']) + + def test_update_too_many_items(self): + update_body = {"encryption": {'key_size': 512}, + "encryption2": {'key_size': 256}} + msg = 'Request body contains too many items.' + self._encryption_update_bad_body(update_body, msg) + + def test_update_key_size_non_integer(self): + update_body = {"encryption": {'key_size': 'abc'}} + msg = 'Invalid input received: key_size must be an integer' + self._encryption_update_bad_body(update_body, msg) + + def test_update_item_invalid_body(self): + update_body = {"key_size": "value1"} + msg = 'Update body is not valid. It must contain "encryption."' + self._encryption_update_bad_body(update_body, msg) + + def _encryption_empty_update(self, update_body): + msg = 'Request body empty.' + self._encryption_update_bad_body(update_body, msg) + + def test_update_no_body(self): + self._encryption_empty_update(update_body=None) + + def test_update_empty_body(self): + self._encryption_empty_update(update_body={}) + + def test_update_with_volume_in_use(self): + # Create the volume type and encryption + volume_type = self._default_volume_type + self._create_type_and_encryption(volume_type) + + # Create a volume with the volume type + db.volume_create(context.get_admin_context(), + {'id': 'fake_id', + 'display_description': 'Test Desc', + 'size': 20, + 'status': 'creating', + 'instance_uuid': None, + 'host': 'dummy', + 'volume_type_id': volume_type['id']}) + + # Get the Encryption + res = self._get_response(volume_type) + self.assertEqual(200, res.status_code) + res_dict = json.loads(res.body) + self.assertEqual(volume_type['id'], res_dict['volume_type_id']) + + # Update, and test that there is an error since volumes exist + update_body = {"encryption": {'key_size': 512}} + + res = self.\ + _get_response(volume_type, req_method='PUT', + req_body=json.dumps(update_body), + req_headers='application/json', + url='/v2/fake/types/%s/encryption/fake_type_id') + self.assertEqual(400, res.status_code) + res_dict = json.loads(res.body) + expected = { + 'badRequest': { + 'code': 400, + 'message': 'Cannot update encryption specs. ' + 'Volume type in use.' + } + } + self.assertEqual(expected, res_dict) diff --git a/cinder/tests/test_db_api.py b/cinder/tests/test_db_api.py index a46e9ecc0..cb1cd7a5c 100644 --- a/cinder/tests/test_db_api.py +++ b/cinder/tests/test_db_api.py @@ -565,11 +565,11 @@ class DBAPIEncryptionTestCase(BaseTest): def setUp(self): super(DBAPIEncryptionTestCase, self).setUp() self.created = \ - [db.volume_type_encryption_update_or_create(self.ctxt, 'fake_type', - values) + [db.volume_type_encryption_create(self.ctxt, + values['volume_type_id'], values) for values in self._get_values()] - def _get_values(self, one=False): + def _get_values(self, one=False, updated=False): base_values = { 'cipher': 'fake_cipher', 'key_size': 256, @@ -577,21 +577,43 @@ class DBAPIEncryptionTestCase(BaseTest): 'volume_type_id': 'fake_type', 'control_location': 'front-end', } + updated_values = { + 'cipher': 'fake_updated_cipher', + 'key_size': 512, + 'provider': 'fake_updated_provider', + 'volume_type_id': 'fake_type', + 'control_location': 'front-end', + } + if one: return base_values + if updated: + values = updated_values + else: + values = base_values + def compose(val, step): if isinstance(val, str): step = str(step) return val + step - return [dict([(k, compose(v, i)) for k, v in base_values.items()]) + return [dict([(k, compose(v, i)) for k, v in values.items()]) for i in range(1, 4)] - def test_volume_type_encryption_update_or_create(self): + def test_volume_type_encryption_create(self): values = self._get_values() for i, encryption in enumerate(self.created): - self._assertEqualObjects(values[i], encryption, + self._assertEqualObjects(values[i], encryption, self._ignored_keys) + + def test_volume_type_encryption_update(self): + update_values = self._get_values(updated=True) + self.updated = \ + [db.volume_type_encryption_update(self.ctxt, + values['volume_type_id'], values) + for values in update_values] + for i, encryption in enumerate(self.updated): + self._assertEqualObjects(update_values[i], encryption, self._ignored_keys) def test_volume_type_encryption_get(self): @@ -602,6 +624,13 @@ class DBAPIEncryptionTestCase(BaseTest): self._assertEqualObjects(encryption, encryption_get, self._ignored_keys) + def test_volume_type_update_with_no_create(self): + self.assertRaises(exception.VolumeTypeEncryptionNotFound, + db.volume_type_encryption_update, + self.ctxt, + 'fake_no_create_type', + {'cipher': 'fake_updated_cipher'}) + def test_volume_type_encryption_delete(self): values = { 'cipher': 'fake_cipher', @@ -611,9 +640,8 @@ class DBAPIEncryptionTestCase(BaseTest): 'control_location': 'front-end', } - encryption = db.volume_type_encryption_update_or_create(self.ctxt, - 'fake_type', - values) + encryption = db.volume_type_encryption_create(self.ctxt, 'fake_type', + values) self._assertEqualObjects(values, encryption, self._ignored_keys) db.volume_type_encryption_delete(self.ctxt, diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 476a9f3ff..80acdabb6 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -400,7 +400,7 @@ class VolumeTestCase(BaseVolumeTestCase): db.volume_type_create(ctxt, {'id': '61298380-0c12-11e3-bfd6-4b48424183be', 'name': 'LUKS'}) - db.volume_type_encryption_update_or_create( + db.volume_type_encryption_create( ctxt, '61298380-0c12-11e3-bfd6-4b48424183be', {'control_location': 'front-end', 'provider': ENCRYPTION_PROVIDER}) @@ -425,7 +425,7 @@ class VolumeTestCase(BaseVolumeTestCase): db.volume_type_create(ctxt, {'id': '61298380-0c12-11e3-bfd6-4b48424183be', 'name': 'LUKS'}) - db.volume_type_encryption_update_or_create( + db.volume_type_encryption_create( ctxt, '61298380-0c12-11e3-bfd6-4b48424183be', {'control_location': 'front-end', 'provider': ENCRYPTION_PROVIDER}) @@ -777,7 +777,7 @@ class VolumeTestCase(BaseVolumeTestCase): db.volume_type_create(ctxt, {'id': '61298380-0c12-11e3-bfd6-4b48424183be', 'name': 'LUKS'}) - db.volume_type_encryption_update_or_create( + db.volume_type_encryption_create( ctxt, '61298380-0c12-11e3-bfd6-4b48424183be', {'control_location': 'front-end', 'provider': ENCRYPTION_PROVIDER}) @@ -831,7 +831,7 @@ class VolumeTestCase(BaseVolumeTestCase): db.volume_type_create(ctxt, {'id': '61298380-0c12-11e3-bfd6-4b48424183be', 'name': 'LUKS'}) - db.volume_type_encryption_update_or_create( + db.volume_type_encryption_create( ctxt, '61298380-0c12-11e3-bfd6-4b48424183be', {'control_location': 'front-end', 'provider': ENCRYPTION_PROVIDER}) diff --git a/cinder/tests/test_volume_types.py b/cinder/tests/test_volume_types.py index 02b2f0785..02ca94b91 100644 --- a/cinder/tests/test_volume_types.py +++ b/cinder/tests/test_volume_types.py @@ -211,9 +211,8 @@ class VolumeTypeTestCase(test.TestCase): 'control_location': 'front-end', 'provider': 'fake_provider', } - db_api.volume_type_encryption_update_or_create(self.ctxt, - volume_type_id, - encryption) + db_api.volume_type_encryption_create(self.ctxt, volume_type_id, + encryption) self.assertTrue(volume_types.is_encrypted(self.ctxt, volume_type_id)) def test_get_volume_type_qos_specs(self): @@ -292,10 +291,10 @@ class VolumeTypeTestCase(test.TestCase): 'control_location': 'front-end'} enc_keyvals2 = {'cipher': 'c1', 'key_size': 128, 'provider': 'p1', 'control_location': 'front-end'} - db.volume_type_encryption_update_or_create(self.ctxt, type_ref1['id'], - enc_keyvals1) - db.volume_type_encryption_update_or_create(self.ctxt, type_ref2['id'], - enc_keyvals2) + db.volume_type_encryption_create(self.ctxt, type_ref1['id'], + enc_keyvals1) + db.volume_type_encryption_create(self.ctxt, type_ref2['id'], + enc_keyvals2) diff, same = volume_types.volume_types_diff(self.ctxt, type_ref1['id'], type_ref2['id']) self.assertEqual(same, False) -- 2.45.2