From dd727943d060e9e32ad7a4390337560b8ced1529 Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Tue, 28 Nov 2017 14:13:21 -0500 Subject: [PATCH] Disallow unmanaging encrypted volumes Unmanaging encrypted volumes is problematic because unmanage assumes that you will be able to manage the volume again for later use, but, we have no mechanism currently to keep track of the encryption key which would be required for using an encrypted volume again. While this may work out ok when using the conf_key manager, this patch does not distinguish between conf_key and barbican deployments. * The Ocata backport skips the async error message for this event due to refactoring of the messages system, to minimize risk. Closes-Bug: #1731518 Change-Id: I7506fa36962404c80f1cc9c6370693728e5393a7 (cherry picked from commit de584713d993e3fb3f68cf2ee8ce37186a9276d5) Conflicts: cinder/volume/api.py (cherry picked from commit 30ca90ffcc5be2cf2854a97cf55d54f98046cda5) Conflicts: cinder/message/message_field.py cinder/tests/unit/api/contrib/test_volume_unmanage.py cinder/tests/unit/volume/test_volume.py --- cinder/tests/unit/api/contrib/test_volume_unmanage.py | 8 ++++++++ cinder/volume/api.py | 5 +++++ cinder/volume/manager.py | 5 +++++ 3 files changed, 18 insertions(+) diff --git a/cinder/tests/unit/api/contrib/test_volume_unmanage.py b/cinder/tests/unit/api/contrib/test_volume_unmanage.py index 3ec2e621ecd..1382948ba62 100644 --- a/cinder/tests/unit/api/contrib/test_volume_unmanage.py +++ b/cinder/tests/unit/api/contrib/test_volume_unmanage.py @@ -94,3 +94,11 @@ class VolumeUnmanageTest(test.TestCase): self.assertEqual(400, res.status_int, res) db.volume_destroy(self.ctxt, vol.id) db.snapshot_destroy(self.ctxt, snap.id) + + def test_unmanage_encrypted_volume_denied(self): + vol = utils.create_volume( + self.ctxt, + encryption_key_id='7a98391f-6619-46af-bd00-5862a3f7f1bd') + res = self._get_resp(vol.id) + self.assertEqual(400, res.status_int, res) + db.volume_destroy(self.ctxt, vol.id) diff --git a/cinder/volume/api.py b/cinder/volume/api.py index f359b5a93b4..92d67f3c31c 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -399,6 +399,11 @@ class API(base.Base): if not unmanage_only: volume.assert_not_frozen() + if unmanage_only and volume.encryption_key_id is not None: + msg = _("Unmanaging encrypted volumes is not supported.") + e = exception.Invalid(reason=msg) + raise e + # Build required conditions for conditional update expected = { 'attach_status': db.Not(fields.VolumeAttachStatus.ATTACHED), diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 5c45ba7efce..98700171b32 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -721,6 +721,11 @@ class VolumeManager(manager.CleanableManager, raise exception.VolumeAttached(volume_id=volume.id) self._check_is_our_resource(volume) + if unmanage_only and volume.encryption_key_id is not None: + raise exception.Invalid( + reason=_("Unmanaging encrypted volumes is not " + "supported.")) + if unmanage_only and cascade: # This could be done, but is ruled out for now just # for simplicity.