Create snapshot throws 500 Internal Error

The server doesn't check whether the parameter "volume_id" is in request body.
So the 500 error has been thrown.

We should catch the KeyError and transfer the KeyError to 400(HTTPBadRequest)
instead of 500.

Change-Id: I8a1dde1fd6ed820b39995af434efacc2a27c9604
Closes-Bug: #1252179

cinder/api/v1/snapshots.py

@@ -167,7 +167,12 @@ def create(self, req, body):
         snapshot = body['snapshot']
         kwargs['metadata'] = snapshot.get('metadata', None)
 
-        volume_id = snapshot['volume_id']
+        try:
+            volume_id = snapshot['volume_id']
+        except KeyError:
+            msg = _("'volume_id' must be specified")
+            raise exc.HTTPBadRequest(explanation=msg)
+
         volume = self.volume_api.get(context, volume_id)
         force = snapshot.get('force', False)
         msg = _("Create snapshot from volume %s")

cinder/api/v2/snapshots.py

@@ -178,7 +178,12 @@ def create(self, req, body):
         snapshot = body['snapshot']
         kwargs['metadata'] = snapshot.get('metadata', None)
 
-        volume_id = snapshot['volume_id']
+        try:
+            volume_id = snapshot['volume_id']
+        except KeyError:
+            msg = _("'volume_id' must be specified")
+            raise exc.HTTPBadRequest(explanation=msg)
+
         volume = self.volume_api.get(context, volume_id)
         force = snapshot.get('force', False)
         msg = _("Create snapshot from volume %s")

cinder/tests/api/v1/test_snapshots.py

@@ -130,6 +130,20 @@ def test_snapshot_create_force(self):
                           req,
                           body)
 
+    def test_snapshot_create_without_volume_id(self):
+        snapshot_name = 'Snapshot Test Name'
+        snapshot_description = 'Snapshot Test Desc'
+        body = {
+            "snapshot": {
+                "force": True,
+                "name": snapshot_name,
+                "description": snapshot_description
+            }
+        }
+        req = fakes.HTTPRequest.blank('/v1/snapshots')
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.create, req, body)
+
     def test_snapshot_update(self):
         self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get)
         self.stubs.Set(volume.api.API, "update_snapshot",

cinder/tests/api/v2/test_snapshots.py

@@ -142,6 +142,20 @@ def test_snapshot_create_force(self):
                           req,
                           body)
 
+    def test_snapshot_create_without_volume_id(self):
+        snapshot_name = 'Snapshot Test Name'
+        snapshot_description = 'Snapshot Test Desc'
+        body = {
+            "snapshot": {
+                "force": True,
+                "name": snapshot_name,
+                "description": snapshot_description
+            }
+        }
+        req = fakes.HTTPRequest.blank('/v2/snapshots')
+        self.assertRaises(webob.exc.HTTPBadRequest,
+                          self.controller.create, req, body)
+
     def test_snapshot_update(self):
         self.stubs.Set(volume.api.API, "get_snapshot", stub_snapshot_get)
         self.stubs.Set(volume.api.API, "update_snapshot",