]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Migrate volume should check para "host" in request
authorhuangtianhua <huangtianhua@huawei.com>
Fri, 22 Nov 2013 07:22:07 +0000 (15:22 +0800)
committerAvishay Traeger <avishay@il.ibm.com>
Sun, 24 Nov 2013 08:20:46 +0000 (10:20 +0200)
The server doesn't check whether the parameter "host" 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.

Closes-Bug: #1253904
Change-Id: I3fb07113816a87f284b47e32bacd57f78a32676c

cinder/api/contrib/admin_actions.py
cinder/tests/api/contrib/test_admin_actions.py

index 6ccae0a1efee0e1cff27cc39e13f09c59516a2e6..57e06b7d34e350c73658da0118960e62405c1a75 100644 (file)
@@ -148,7 +148,10 @@ class VolumeAdminController(AdminController):
         except exception.NotFound:
             raise exc.HTTPNotFound()
         params = body['os-migrate_volume']
-        host = params['host']
+        try:
+            host = params['host']
+        except KeyError:
+            raise exc.HTTPBadRequest("Must specify 'host'")
         force_host_copy = params.get('force_host_copy', False)
         if isinstance(force_host_copy, basestring):
             try:
index 8ee821477a2bb80e5d71dd504b464dfa7ec613a1..d4dad0bfc0a221003d27d3b114037741816ecf28 100644 (file)
@@ -584,6 +584,23 @@ class AdminActionsTest(test.TestCase):
         volume = self._migrate_volume_prep()
         self._migrate_volume_exec(ctx, volume, host, expected_status)
 
+    def test_migrate_volume_without_host_parameter(self):
+        expected_status = 400
+        host = 'test3'
+        ctx = context.RequestContext('admin', 'fake', True)
+        volume = self._migrate_volume_prep()
+        # build request to migrate without host
+        req = webob.Request.blank('/v2/fake/volumes/%s/action' % volume['id'])
+        req.method = 'POST'
+        req.headers['content-type'] = 'application/json'
+        body = {'os-migrate_volume': {'host': host,
+                                      'force_host_copy': False}}
+        req.body = jsonutils.dumps(body)
+        req.environ['cinder.context'] = ctx
+        resp = req.get_response(app())
+        # verify status
+        self.assertEqual(resp.status_int, expected_status)
+
     def test_migrate_volume_host_no_exist(self):
         expected_status = 400
         host = 'test3'