Merge "Validate uuid parameters strictly for create volume API"

cinder/api/v2/volumes.py

@@ -215,6 +215,10 @@ class VolumeController(wsgi.Controller):
 
         source_volid = volume.get('source_volid')
         if source_volid is not None:
+            if not uuidutils.is_uuid_like(source_volid):
+                msg = _("Source volume ID '%s' must be a "
+                        "valid UUID.") % source_volid
+                raise exc.HTTPBadRequest(explanation=msg)
             # Not found exception will be handled at the wsgi level
             kwargs['source_volume'] = \
                 self.volume_api.get_volume(context,
@@ -224,6 +228,10 @@ class VolumeController(wsgi.Controller):
 
         source_replica = volume.get('source_replica')
         if source_replica is not None:
+            if not uuidutils.is_uuid_like(source_replica):
+                msg = _("Source replica ID '%s' must be a "
+                        "valid UUID") % source_replica
+                raise exc.HTTPBadRequest(explanation=msg)
             # Not found exception will be handled at the wsgi level
             src_vol = self.volume_api.get_volume(context,
                                                  source_replica)
@@ -239,6 +247,10 @@ class VolumeController(wsgi.Controller):
         kwargs['consistencygroup'] = None
         consistencygroup_id = volume.get('consistencygroup_id')
         if consistencygroup_id is not None:
+            if not uuidutils.is_uuid_like(consistencygroup_id):
+                msg = _("Consistency group ID '%s' must be a "
+                        "valid UUID.") % consistencygroup_id
+                raise exc.HTTPBadRequest(explanation=msg)
             # Not found exception will be handled at the wsgi level
             kwargs['group'] = self.group_api.get(context, consistencygroup_id)
 

cinder/api/v3/volumes.py

@@ -214,6 +214,10 @@ class VolumeController(volumes_v2.VolumeController):
 
         source_volid = volume.get('source_volid')
         if source_volid is not None:
+            if not uuidutils.is_uuid_like(source_volid):
+                msg = _("Source volume ID '%s' must be a "
+                        "valid UUID.") % source_volid
+                raise exc.HTTPBadRequest(explanation=msg)
             # Not found exception will be handled at the wsgi level
             kwargs['source_volume'] = (
                 self.volume_api.get_volume(context,
@@ -223,6 +227,10 @@ class VolumeController(volumes_v2.VolumeController):
 
         source_replica = volume.get('source_replica')
         if source_replica is not None:
+            if not uuidutils.is_uuid_like(source_replica):
+                msg = _("Source replica ID '%s' must be a "
+                        "valid UUID") % source_replica
+                raise exc.HTTPBadRequest(explanation=msg)
             # Not found exception will be handled at the wsgi level
             src_vol = self.volume_api.get_volume(context,
                                                  source_replica)
@@ -238,6 +246,10 @@ class VolumeController(volumes_v2.VolumeController):
         kwargs['consistencygroup'] = None
         consistencygroup_id = volume.get('consistencygroup_id')
         if consistencygroup_id is not None:
+            if not uuidutils.is_uuid_like(consistencygroup_id):
+                msg = _("Consistency group ID '%s' must be a "
+                        "valid UUID.") % consistencygroup_id
+                raise exc.HTTPBadRequest(explanation=msg)
             # Not found exception will be handled at the wsgi level
             kwargs['group'] = self.group_api.get(context, consistencygroup_id)
 

cinder/tests/unit/api/v2/test_volumes.py

@@ -338,6 +338,21 @@ class VolumeApiTest(test.TestCase):
         get_volume.assert_called_once_with(self.controller.volume_api,
                                            context, source_volid)
 
+    @ddt.data({'source_volid': 1},
+              {'source_volid': []},
+              {'source_replica': 1},
+              {'source_replica': []},
+              {'consistencygroup_id': 1},
+              {'consistencygroup_id': []})
+    def test_volume_creation_fails_with_invalid_uuids(self, updated_uuids):
+        vol = self._vol_in_request_body()
+        vol.update(updated_uuids)
+        body = {"volume": vol}
+        req = fakes.HTTPRequest.blank('/v2/volumes')
+        # Raise 400 for resource requested with invalid uuids.
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
+                          req, body)
+
     @mock.patch.object(volume_api.API, 'get_volume', autospec=True)
     def test_volume_creation_fails_with_invalid_source_replica(self,
                                                                get_volume):

cinder/tests/unit/api/v3/test_volumes.py

@@ -373,6 +373,21 @@ class VolumeApiTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
                           req, body)
 
+    @ddt.data({'source_volid': 1},
+              {'source_volid': []},
+              {'source_replica': 1},
+              {'source_replica': []},
+              {'consistencygroup_id': 1},
+              {'consistencygroup_id': []})
+    def test_volume_creation_fails_with_invalid_uuids(self, updated_uuids):
+        vol = self._vol_in_request_body()
+        vol.update(updated_uuids)
+        body = {"volume": vol}
+        req = fakes.HTTPRequest.blank('/v2/volumes')
+        # Raise 400 for resource requested with invalid uuids.
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.create,
+                          req, body)
+
     @ddt.data({'admin': True, 'version': '3.21'},
               {'admin': False, 'version': '3.21'},
               {'admin': True, 'version': '3.20'},

releasenotes/notes/validate_vol_create_uuids-4f08b4ef201385f6.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    The create volume api will now return 400 error instead of 404/500 if user
+    passes non-uuid values to consistencygroup_id, source_volid and
+    source_replica parameters in the request body.