From 5657bb5a6e0a354e43eed0332f20bbf08cd140bf Mon Sep 17 00:00:00 2001 From: Eric Harney Date: Fri, 24 Mar 2017 14:48:12 -0400 Subject: [PATCH] Catch Castellan errors in create volume If encryption types are setup in an invalid way, Barbican will throw a 400 error which results in Cinder producing a 500 error here. For example, setting key size = 2 produces: "4xx Client error: Bad Request: Provided field value is not supported" from Castellan/Barbican. Catch this and produce a generic HTTP 400 instead. Closes-Bug: #1675895 Change-Id: Icd64d7e54d61a415b1be3b438625e880a7259416 --- .../volume/flows/test_create_volume_flow.py | 54 +++++++++++++++++++ cinder/volume/flows/api/create_volume.py | 15 ++++-- 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/cinder/tests/unit/volume/flows/test_create_volume_flow.py b/cinder/tests/unit/volume/flows/test_create_volume_flow.py index bb451b0239a..24633e09b6d 100644 --- a/cinder/tests/unit/volume/flows/test_create_volume_flow.py +++ b/cinder/tests/unit/volume/flows/test_create_volume_flow.py @@ -19,6 +19,7 @@ import sys import ddt import mock +from castellan.common import exception as castellan_exc from castellan.tests.unit.key_manager import mock_key_manager from oslo_utils import imageutils @@ -347,6 +348,59 @@ class CreateVolumeFlowTestCase(test.TestCase): 'replication_status': 'disabled'} self.assertEqual(expected_result, result) + @mock.patch('cinder.volume.volume_types.is_encrypted', + return_value=True) + @mock.patch('cinder.volume.volume_types.get_volume_type_encryption', + return_value=mock.Mock(cipher='my-cipher-2000')) + @mock.patch('cinder.volume.volume_types.get_volume_type_qos_specs', + return_value={'qos_specs': None}) + @mock.patch('cinder.volume.flows.api.create_volume.' + 'ExtractVolumeRequestTask._get_volume_type_id', + return_value=1) + def test_get_encryption_key_id_castellan_error( + self, + mock_get_type_id, + mock_get_qos, + mock_get_volume_type_encryption, + mock_is_encrypted): + + fake_image_service = fake_image.FakeImageService() + image_id = 99 + image_meta = {'id': image_id, + 'status': 'active', + 'size': 1} + fake_image_service.create(self.ctxt, image_meta) + fake_key_manager = mock_key_manager.MockKeyManager() + volume_type = 'type1' + + with mock.patch.object(fake_key_manager, 'create_key', + side_effect=castellan_exc.KeyManagerError): + with mock.patch.object(fake_key_manager, 'get', + return_value=fakes.ENCRYPTION_KEY_ID): + + task = create_volume.ExtractVolumeRequestTask( + fake_image_service, + {'nova'}) + + self.assertRaises(exception.Invalid, + task.execute, + self.ctxt, + size=1, + snapshot=None, + image_id=image_id, + source_volume=None, + availability_zone='nova', + volume_type=volume_type, + metadata=None, + key_manager=fake_key_manager, + source_replica=None, + consistencygroup=None, + cgsnapshot=None, + group=None) + + mock_is_encrypted.assert_called_once_with(self.ctxt, 1) + mock_get_volume_type_encryption.assert_called_once_with(self.ctxt, 1) + @mock.patch('cinder.volume.volume_types.is_encrypted') @mock.patch('cinder.volume.volume_types.get_volume_type_qos_specs') @mock.patch('cinder.volume.flows.api.create_volume.' diff --git a/cinder/volume/flows/api/create_volume.py b/cinder/volume/flows/api/create_volume.py index 15777fbded4..3dfdb965b95 100644 --- a/cinder/volume/flows/api/create_volume.py +++ b/cinder/volume/flows/api/create_volume.py @@ -11,6 +11,7 @@ # under the License. +from castellan.common import exception as castellan_exc from oslo_config import cfg from oslo_log import log as logging from oslo_utils import timeutils @@ -392,9 +393,17 @@ class ExtractVolumeRequestTask(flow_utils.CinderTask): # hyphenated format (aes-xts-plain64). The algorithm needs # to be parsed out to pass to the key manager (aes). algorithm = cipher.split('-')[0] if cipher else None - encryption_key_id = key_manager.create_key(context, - algorithm=algorithm, - length=length) + try: + encryption_key_id = key_manager.create_key( + context, + algorithm=algorithm, + length=length) + except castellan_exc.KeyManagerError: + # The messaging back to the client here is + # purposefully terse, so we don't leak any sensitive + # details. + LOG.exception("Key manager error") + raise exception.Invalid(message="Key manager error") return encryption_key_id