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

diff --git a/cinder/api/v1/volumes.py b/cinder/api/v1/volumes.py
index a60f1a4c4c64cf28daa854fd19ab7eae772cd7e2..b7a687ee1f7f5b0be022e5b465ece09ef534665d 100644 (file)
--- 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 b88fa10d424ede30a5c0458f63d1e8425c53b89b..bf46449d44274feee3e0187e806a422a74a9f0fe 100644 (file)
--- 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 3c5e6f99ea08e73727a92b53e72e66a4d407a3a9..b199ca40e84489036e69c71d426163119d5c14d1 100644 (file)
--- 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 5d5d1fff54c4730ed6dddcac1160230a6b17ca3a..9b08ab376a2ecd1e76930fd9380ec595a9e019f6 100644 (file)
--- 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 91474f1ce8b5df49503b7650613bb63b9c6c8f35..5a06be538fef95a950fa5a62ab51a1693ea00860 100644 (file)
--- 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 edddd295b3ede1741d1ea25a91e64da572f4852b..995276748cb4e8d15da9398164032e9e5be38588 100644 (file)
--- 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')