From a4540aae666347c1a7e6f17330232a5d33cc4893 Mon Sep 17 00:00:00 2001 From: Stephen Mulcahy Date: Thu, 28 Mar 2013 15:46:16 +0000 Subject: [PATCH] Enforce exclusive options snapshot-id, source-volid and image-id Fixed checks in api that volume create receives only one of snapshot-id, source-volid or image-id. Fixes bug #1161437 Change-Id: Ibe5ca4bb81e69b0f8e1abe1c2cffe587dc10e3ca --- cinder/api/v1/volumes.py | 3 --- cinder/api/v2/volumes.py | 3 --- cinder/tests/api/v1/test_volumes.py | 18 ------------------ cinder/tests/api/v2/test_volumes.py | 19 ------------------- cinder/tests/test_volume.py | 13 +++++++++++++ cinder/volume/api.py | 9 ++++++--- 6 files changed, 19 insertions(+), 46 deletions(-) diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py index a60f1a4c4..b7a687ee1 100644 --- a/cinder/api/v1/volumes.py +++ b/cinder/api/v1/volumes.py @@ -342,9 +342,6 @@ class VolumeController(wsgi.Controller): image_uuid = None if self.ext_mgr.is_loaded('os-image-create'): image_href = volume.get('imageRef') - if snapshot_id and image_href: - msg = _("Snapshot and image cannot be specified together.") - raise exc.HTTPBadRequest(explanation=msg) if image_href: image_uuid = self._image_uuid_from_href(image_href) kwargs['image_id'] = image_uuid diff --git a/cinder/api/v2/volumes.py b/cinder/api/v2/volumes.py index b88fa10d4..bf46449d4 100644 --- a/cinder/api/v2/volumes.py +++ b/cinder/api/v2/volumes.py @@ -275,9 +275,6 @@ class VolumeController(wsgi.Controller): image_uuid = None if self.ext_mgr.is_loaded('os-image-create'): image_href = volume.get('imageRef') - if snapshot_id and image_href: - msg = _("Snapshot and image cannot be specified together.") - raise exc.HTTPBadRequest(explanation=msg) if image_href: image_uuid = self._image_uuid_from_href(image_href) kwargs['image_id'] = image_uuid diff --git a/cinder/tests/api/v1/test_volumes.py b/cinder/tests/api/v1/test_volumes.py index 3c5e6f99e..b199ca40e 100644 --- a/cinder/tests/api/v1/test_volumes.py +++ b/cinder/tests/api/v1/test_volumes.py @@ -155,24 +155,6 @@ class VolumeApiTest(test.TestCase): res_dict = self.controller.create(req, body) self.assertEqual(res_dict, expected) - def test_volume_create_with_image_id_and_snapshot_id(self): - self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) - self.stubs.Set(volume_api.API, "get_snapshot", stub_snapshot_get) - self.ext_mgr.extensions = {'os-image-create': 'fake'} - vol = {"size": '1', - "display_name": "Volume Test Name", - "display_description": "Volume Test Desc", - "availability_zone": "cinder", - "imageRef": 'c905cedb-7281-47e4-8a62-f26bc5fc4c77', - "source_volid": None, - "snapshot_id": TEST_SNAPSHOT_UUID} - body = {"volume": vol} - req = fakes.HTTPRequest.blank('/v1/volumes') - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, - req, - body) - def test_volume_create_with_image_id_is_integer(self): self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) self.ext_mgr.extensions = {'os-image-create': 'fake'} diff --git a/cinder/tests/api/v2/test_volumes.py b/cinder/tests/api/v2/test_volumes.py index 5d5d1fff5..9b08ab376 100644 --- a/cinder/tests/api/v2/test_volumes.py +++ b/cinder/tests/api/v2/test_volumes.py @@ -166,25 +166,6 @@ class VolumeApiTest(test.TestCase): res_dict = self.controller.create(req, body) self.assertEqual(res_dict, expected) - def test_volume_create_with_image_id_and_snapshot_id(self): - self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) - self.stubs.Set(volume_api.API, "get_snapshot", stub_snapshot_get) - self.ext_mgr.extensions = {'os-image-create': 'fake'} - vol = { - "size": '1', - "name": "Volume Test Name", - "description": "Volume Test Desc", - "availability_zone": "cinder", - "imageRef": 'c905cedb-7281-47e4-8a62-f26bc5fc4c77', - "snapshot_id": TEST_SNAPSHOT_UUID - } - body = {"volume": vol} - req = fakes.HTTPRequest.blank('/v2/volumes') - self.assertRaises(webob.exc.HTTPBadRequest, - self.controller.create, - req, - body) - def test_volume_create_with_image_id_is_integer(self): self.stubs.Set(volume_api.API, "create", stubs.stub_volume_create) self.ext_mgr.extensions = {'os-image-create': 'fake'} diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index 91474f1ce..5a06be538 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -253,6 +253,19 @@ class VolumeTestCase(test.TestCase): self.volume.delete_snapshot(self.context, snapshot_id) self.volume.delete_volume(self.context, volume_src['id']) + def test_create_volume_with_invalid_exclusive_options(self): + """Test volume create with multiple exclusive options fails.""" + volume_api = cinder.volume.api.API() + self.assertRaises(exception.InvalidInput, + volume_api.create, + self.context, + 1, + 'name', + 'description', + snapshot='fake_id', + image_id='fake_id', + source_volume='fake_id') + def test_too_big_volume(self): """Ensure failure if a too large of a volume is requested.""" # FIXME(vish): validation needs to move into the data layer in diff --git a/cinder/volume/api.py b/cinder/volume/api.py index edddd295b..995276748 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -89,9 +89,12 @@ class API(base.Base): image_id=None, volume_type=None, metadata=None, availability_zone=None, source_volume=None): - if ((snapshot is not None) and (source_volume is not None)): - msg = (_("May specify either snapshot, " - "or src volume but not both!")) + exclusive_options = (snapshot, image_id, source_volume) + exclusive_options_set = sum(1 for option in + exclusive_options if option is not None) + if exclusive_options_set > 1: + msg = (_("May specify only one of snapshot, imageRef " + "or source volume")) raise exception.InvalidInput(reason=msg) check_policy(context, 'create') -- 2.45.2