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

diff --git a/cinder/api/v1/snapshots.py b/cinder/api/v1/snapshots.py
index d2922dc5abe78074850d93e68d6fe2b8a5038fb4..08d87afbf6ee81d8a70e995ab0a9c4aaf4408b3b 100644 (file)
--- a/cinder/api/v1/snapshots.py
+++ b/cinder/api/v1/snapshots.py
@@ -167,7 +167,12 @@ class SnapshotsController(wsgi.Controller):
         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")

diff --git a/cinder/api/v2/snapshots.py b/cinder/api/v2/snapshots.py
index f6ac6da76a94f055368dfb2b678b01182244e399..24703317a8c6daf153edf89155339ec94413b458 100644 (file)
--- a/cinder/api/v2/snapshots.py
+++ b/cinder/api/v2/snapshots.py
@@ -178,7 +178,12 @@ class SnapshotsController(wsgi.Controller):
         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")

diff --git a/cinder/tests/api/v1/test_snapshots.py b/cinder/tests/api/v1/test_snapshots.py
index 831a0db1ba48a11b438f270c741a4148ac930739..c5b0b8c2871103f0c0afe98c4c83629515843865 100644 (file)
--- a/cinder/tests/api/v1/test_snapshots.py
+++ b/cinder/tests/api/v1/test_snapshots.py
@@ -130,6 +130,20 @@ class SnapshotApiTest(test.TestCase):
                           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",

diff --git a/cinder/tests/api/v2/test_snapshots.py b/cinder/tests/api/v2/test_snapshots.py
index 0264a313507f8e6e5b48f966e49f9e2deabf890b..43fa3a4e197cb72fa37f25aebe30914b5b7fdb7e 100644 (file)
--- a/cinder/tests/api/v2/test_snapshots.py
+++ b/cinder/tests/api/v2/test_snapshots.py
@@ -142,6 +142,20 @@ class SnapshotApiTest(test.TestCase):
                           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",