Fix incorrect response returned by the Update Lease API

When updating reservation values, the reservation ID is a required
parameter. However, if the value of "reservation.id" is set to an empty
string, null, or an incorrect UUID, Blazar currently returns a 200 OK
response instead of a 400 Bad Request.

This patch fixes it by raising an InvalidInput exception if an invalid
value is passed for reservation.id.

APIImpact: The response code is changed from 200 OK to 400 Bad Request
if an invalid reservation ID is passed to the Update Lease API.

Change-Id: I83e7c3cc0b6ca79aab2b8bb23ba82fceb96646e2
Closes-Bug: #1793743

blazar/manager/service.py

@@ -412,16 +412,25 @@ class ManagerService(service_utils.RPCServer):
 
             # TODO(frossigneux) rollback if an exception is raised
             reservations = values.get('reservations', [])
-            for reservation in (
-                    db_api.reservation_get_all_by_lease_id(lease_id)):
+            reservations_db = db_api.reservation_get_all_by_lease_id(lease_id)
+            try:
+                invalid_ids = set([r['id'] for r in reservations]).difference(
+                    [r['id'] for r in reservations_db])
+            except KeyError:
+                raise exceptions.MissingParameter(param='reservation ID')
+
+            if invalid_ids:
+                raise common_ex.InvalidInput(
+                    'Please enter valid reservation IDs. Invalid reservation '
+                    'IDs are: %s' % ','.join([str(id) for id in invalid_ids]))
+
+            for reservation in (reservations_db):
                 v = {}
                 v['start_date'] = values['start_date']
                 v['end_date'] = values['end_date']
                 try:
                     v.update([r for r in reservations
                               if r['id'] == reservation['id']].pop())
-                except KeyError:
-                    raise exceptions.MissingParameter(param='reservation ID')
                 except IndexError:
                     pass
                 resource_type = v.get('resource_type',

blazar/tests/manager/test_service.py

@@ -15,6 +15,7 @@
 
 import datetime
 
+import ddt
 import eventlet
 import mock
 from oslo_config import cfg
@@ -84,6 +85,7 @@ class FakeLeaseStatus(object):
         return decorator
 
 
+@ddt.ddt
 class ServiceTestCase(tests.TestCase):
     def setUp(self):
         super(ServiceTestCase, self).setUp()
@@ -904,6 +906,38 @@ class ServiceTestCase(tests.TestCase):
                 manager_ex.MissingParameter, self.manager.update_lease,
                 lease_id=self.lease_id, values=lease_values)
 
+    @ddt.data('', None, '1234', '7085381b-45e0-4e5d-b24a-f965f5e6e5d7')
+    def test_update_reservations_with_invalid_reservation_id(self,
+                                                             reservation_id):
+        lease_values = {
+            'reservations': [
+                {
+                    'disk_gb': 30,
+                    'id': reservation_id,
+                }
+            ]
+        }
+        reservation_get_all = (
+            self.patch(self.db_api, 'reservation_get_all_by_lease_id'))
+        reservation_get_all.return_value = [
+            {
+                'id': u'593e7028-c0d1-4d76-8642-2ffd890b324c',
+                'resource_type': 'virtual:instance',
+            },
+            {
+                'id': u'2eeb784a-2d84-4a89-a201-9d42d61eecb1',
+                'resource_type': 'virtual:instance',
+            }
+        ]
+        target = datetime.datetime(2013, 12, 15)
+        with mock.patch.object(datetime,
+                               'datetime',
+                               mock.Mock(wraps=datetime.datetime)) as patched:
+            patched.utcnow.return_value = target
+        self.assertRaises(
+            exceptions.InvalidInput, self.manager.update_lease,
+            lease_id=self.lease_id, values=lease_values)
+
     def test_update_lease_started_modify_end_date_without_before_end(self):
         def fake_event_get(sort_key, sort_dir, filters):
             if filters['event_type'] == 'start_lease':

lower-constraints.txt

@@ -11,6 +11,7 @@ chardet==3.0.4
 click==6.7
 contextlib2==0.5.5
 coverage==4.0
+ddt==1.0.1
 debtcollector==1.19.0
 decorator==4.2.1
 docutils==0.14

test-requirements.txt

@@ -3,6 +3,7 @@
 # process, which may cause wedges in the gate later.
 hacking!=0.13.0,<0.14,>=0.12.0 # Apache-2.0
 
+ddt>=1.0.1 # MIT
 mock>=2.0.0 # BSD
 fixtures>=3.0.0 # Apache-2.0/BSD
 testrepository>=0.0.18 # Apache-2.0/BSD