Merge "Validate physical host reservation parameters"

blazar/manager/exceptions.py

@@ -129,11 +129,21 @@ class NotEnoughHostsAvailable(exceptions.BlazarException):
     msg_fmt = _("Not enough hosts available")
 
 
+class MalformedParameter(exceptions.BlazarException):
+    code = 400
+    msg_fmt = _("Malformed parameter %(param)s")
+
+
 class MalformedRequirements(exceptions.BlazarException):
     code = 400
     msg_fmt = _("Malformed requirements %(rqrms)s")
 
 
+class MissingParameter(exceptions.BlazarException):
+    code = 400
+    msg_fmt = _("Missing parameter %(param)s")
+
+
 class InvalidState(exceptions.BlazarException):
     code = 409
     msg_fmt = _("Invalid State %(state)s for %(id)s")

blazar/plugins/oshosts/host_plugin.py

@@ -17,6 +17,7 @@
 import datetime
 
 from oslo_config import cfg
+from oslo_utils import strutils
 
 from blazar.db import api as db_api
 from blazar.db import exceptions as db_ex
@@ -67,8 +68,9 @@ class PhysicalHostPlugin(base.BasePlugin, nova.NovaClientWrapper):
 
     def reserve_resource(self, reservation_id, values):
         """Create reservation."""
-        min_hosts = values.get('min')
+        min_hosts = self._convert_int_param(values.get('min'), 'min')
-        max_hosts = values.get('max')
+        max_hosts = self._convert_int_param(values.get('max'), 'max')
+
         if 0 <= min_hosts and min_hosts <= max_hosts:
             count_range = str(min_hosts) + '-' + str(max_hosts)
         else:
@@ -372,3 +374,13 @@ class PhysicalHostPlugin(base.BasePlugin, nova.NovaClientWrapper):
             return all_host_ids[:int(max_host)]
         else:
             return []
+
+    def _convert_int_param(self, param, name):
+        """Checks that the parameter is present and can be converted to int."""
+        if param is None:
+            raise manager_ex.MissingParameter(param=name)
+        if strutils.is_int_like(param):
+            param = int(param)
+        else:
+            raise manager_ex.MalformedParameter(param=name)
+        return param

blazar/tests/plugins/test_physical_host_plugin.py

@@ -361,6 +361,39 @@ class PhysicalHostPluginTestCase(tests.TestCase):
         ]
         host_allocation_create.assert_has_calls(calls)
 
+    def test_create_reservation_with_missing_param(self):
+        values = {
+            'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c',
+            'max': u'2',
+            'hypervisor_properties': '["=", "$memory_mb", "256"]',
+            'resource_properties': '',
+            'start_date': datetime.datetime(2017, 3, 1, 20, 00),
+            'end_date': datetime.datetime(2017, 3, 2, 20, 00),
+            'resource_type': plugin.RESOURCE_TYPE,
+        }
+        self.assertRaises(
+            manager_exceptions.MissingParameter,
+            self.fake_phys_plugin.reserve_resource,
+            u'441c1476-9f8f-4700-9f30-cd9b6fef3509',
+            values)
+
+    def test_create_reservation_with_invalid_param(self):
+        values = {
+            'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c',
+            'min': u'2',
+            'max': u'three',
+            'hypervisor_properties': '["=", "$memory_mb", "256"]',
+            'resource_properties': '',
+            'start_date': datetime.datetime(2017, 3, 1, 20, 00),
+            'end_date': datetime.datetime(2017, 3, 2, 20, 00),
+            'resource_type': plugin.RESOURCE_TYPE,
+        }
+        self.assertRaises(
+            manager_exceptions.MalformedParameter,
+            self.fake_phys_plugin.reserve_resource,
+            u'441c1476-9f8f-4700-9f30-cd9b6fef3509',
+            values)
+
     def test_create_reservation_with_invalid_range(self):
         values = {
             'lease_id': u'018c1b43-e69e-4aef-a543-09681539cf4c',

contrib/tempest/tempest/scenario/test_host_reservation.py

@@ -68,6 +68,49 @@ class TestHostReservationScenario(rrs.ResourceReservationScenarioTest):
 
         return body
 
+    def get_lease_body_missing_param(self, lease_name, host_name):
+        current_time = datetime.datetime.utcnow()
+        end_time = current_time + datetime.timedelta(hours=1)
+        body = {
+            "start_date": current_time.strftime('%Y-%m-%d %H:%M'),
+            "end_date": end_time.strftime('%Y-%m-%d %H:%M'),
+            "name": lease_name,
+            "events": [],
+            }
+        body["reservations"] = [
+            {
+                "hypervisor_properties": ('["==", "$hypervisor_hostname", "'
+                                          '%s"]' % host_name),
+                "min": '1',
+                "resource_type": 'physical:host',
+                "resource_properties": ''
+                }
+            ]
+
+        return body
+
+    def get_invalid_lease_body(self, lease_name, host_name):
+        current_time = datetime.datetime.utcnow()
+        end_time = current_time + datetime.timedelta(hours=1)
+        body = {
+            "start_date": current_time.strftime('%Y-%m-%d %H:%M'),
+            "end_date": end_time.strftime('%Y-%m-%d %H:%M'),
+            "name": lease_name,
+            "events": [],
+            }
+        body["reservations"] = [
+            {
+                "hypervisor_properties": ('["==", "$hypervisor_hostname", "'
+                                          '%s"]' % host_name),
+                "max": 'foo',
+                "min": 'bar',
+                "resource_type": 'physical:host',
+                "resource_properties": ''
+                }
+            ]
+
+        return body
+
     def fetch_aggregate_by_name(self, name):
         aggregates = self.aggr_client.list_aggregates()['aggregates']
         try:
@@ -101,6 +144,17 @@ class TestHostReservationScenario(rrs.ResourceReservationScenarioTest):
         freepool = self.fetch_aggregate_by_name('freepool')
         self.assertTrue(host['host'] in freepool['hosts'])
 
+        # try creating a new lease with a missing parameter
+        body = self.get_lease_body_missing_param('scenario-1-missing-param',
+                                                 host['host'])
+        self.assertRaises(exceptions.BadRequest,
+                          self.reservation_client.create_lease, body)
+
+        # try creating a new lease with an invalid request
+        body = self.get_invalid_lease_body('scenario-1-invalid', host['host'])
+        self.assertRaises(exceptions.BadRequest,
+                          self.reservation_client.create_lease, body)
+
         # create new lease and start reservation immediatly
         body = self.get_lease_body('scenario-1', host['host'])
         lease = self.reservation_client.create_lease(body)['lease']