Add host check on replication actions

Current code has no check for the host field in any of the replication
endpoints (failover_host, freeze, and thaw) so a call to any of these
methods without the required host field will result in a KeyError
exception and a 500 code being returned to the caller.

This patch fixes this by returning expected 400 invalid error, just like
any other action does.

It also adds missing replication tests for contrib/services.py.

Closes-Bug: #1641714
Change-Id: Icf5b1277179dc7e40f53524233d57ea2e399755a

cinder/api/contrib/services.py

@@ -119,6 +119,12 @@ class ServiceController(wsgi.Controller):
     def _failover(self, context, host, backend_id=None):
         return self.volume_api.failover_host(context, host, backend_id)
 
+    def _get_host(self, body):
+        try:
+            return body['host']
+        except (TypeError, KeyError):
+            raise exception.MissingRequired(element='host')
+
     def update(self, req, id, body):
         """Enable/Disable scheduling for a service.
 
@@ -142,23 +148,20 @@ class ServiceController(wsgi.Controller):
             disabled = True
             status = "disabled"
         elif id == "freeze":
-            return self._freeze(context, body['host'])
+            return self._freeze(context, self._get_host(body))
         elif id == "thaw":
-            return self._thaw(context, body['host'])
+            return self._thaw(context, self._get_host(body))
         elif id == "failover_host":
             self._failover(
                 context,
-                body['host'],
+                self._get_host(body),
                 body.get('backend_id', None)
             )
             return webob.Response(status_int=202)
         else:
             raise exception.InvalidInput(reason=_("Unknown action"))
 
-        try:
-            host = body['host']
-        except (TypeError, KeyError):
-            raise exception.MissingRequired(element='host')
+        host = self._get_host(body)
 
         ret_val['disabled'] = disabled
         if id == "disable-log-reason" and ext_loaded:

cinder/tests/unit/api/contrib/test_services.py

@@ -16,6 +16,7 @@
 
 import datetime
 
+import ddt
 from iso8601 import iso8601
 import mock
 import webob.exc
@@ -192,6 +193,7 @@ def fake_utcnow(with_timezone=False):
     return datetime.datetime(2012, 10, 29, 13, 42, 11, tzinfo=tzinfo)
 
 
+@ddt.ddt
 @mock.patch('cinder.db.service_get_all', fake_service_get_all)
 @mock.patch('cinder.db.service_get', fake_service_get)
 @mock.patch('oslo_utils.timeutils.utcnow', fake_utcnow)
@@ -748,3 +750,45 @@ class ServicesTest(test.TestCase):
         self.assertTrue(self.controller._is_valid_as_reason(reason))
         reason = None
         self.assertFalse(self.controller._is_valid_as_reason(reason))
+
+    def test_services_failover_host(self):
+        url = '/v2/%s/os-services/failover_host' % fake.PROJECT_ID
+        req = fakes.HTTPRequest.blank(url)
+        body = {'host': mock.sentinel.host,
+                'backend_id': mock.sentinel.backend_id}
+        with mock.patch.object(self.controller.volume_api, 'failover_host') \
+                as failover_mock:
+            res = self.controller.update(req, 'failover_host', body)
+        failover_mock.assert_called_once_with(req.environ['cinder.context'],
+                                              mock.sentinel.host,
+                                              mock.sentinel.backend_id)
+        self.assertEqual(202, res.status_code)
+
+    def test_services_freeze(self):
+        url = '/v2/%s/os-services/freeze' % fake.PROJECT_ID
+        req = fakes.HTTPRequest.blank(url)
+        body = {'host': mock.sentinel.host}
+        with mock.patch.object(self.controller.volume_api, 'freeze_host') \
+                as freeze_mock:
+            res = self.controller.update(req, 'freeze', body)
+        freeze_mock.assert_called_once_with(req.environ['cinder.context'],
+                                            mock.sentinel.host)
+        self.assertEqual(freeze_mock.return_value, res)
+
+    def test_services_thaw(self):
+        url = '/v2/%s/os-services/thaw' % fake.PROJECT_ID
+        req = fakes.HTTPRequest.blank(url)
+        body = {'host': mock.sentinel.host}
+        with mock.patch.object(self.controller.volume_api, 'thaw_host') \
+                as thaw_mock:
+            res = self.controller.update(req, 'thaw', body)
+        thaw_mock.assert_called_once_with(req.environ['cinder.context'],
+                                          mock.sentinel.host)
+        self.assertEqual(thaw_mock.return_value, res)
+
+    @ddt.data('freeze', 'thaw', 'failover_host')
+    def test_services_replication_calls_no_host(self, method):
+        url = '/v2/%s/os-services/%s' % (fake.PROJECT_ID, method)
+        req = fakes.HTTPRequest.blank(url)
+        self.assertRaises(exception.MissingRequired,
+                          self.controller.update, req, method, {})