Merge "Verify all quotas before updating the database" into stable/kilo

2 files changed, 23 insertions, 0 deletions

diff --git a/cinder/api/contrib/quotas.py b/cinder/api/contrib/quotas.py
index 8ef8ee2e4..32667cad3 100644
--- a/cinder/api/contrib/quotas.py
+++ b/cinder/api/contrib/quotas.py
@@ -107,6 +107,8 @@ class QuotaSetsController(wsgi.Controller):
 
         bad_keys = []
 
+        # NOTE(ankit): Pass #1 - In this loop for body['quota_set'].items(),
+        # we figure out if we have any bad keys.
         for key, value in body['quota_set'].items():
             if (key not in QUOTAS and key not in NON_QUOTA_KEYS):
                 bad_keys.append(key)
@@ -116,11 +118,22 @@ class QuotaSetsController(wsgi.Controller):
             msg = _("Bad key(s) in quota set: %s") % ",".join(bad_keys)
             raise webob.exc.HTTPBadRequest(explanation=msg)
 
+        # NOTE(ankit): Pass #2 - In this loop for body['quota_set'].keys(),
+        # we validate the quota limits to ensure that we can bail out if
+        # any of the items in the set is bad.
+        valid_quotas = {}
         for key in body['quota_set'].keys():
             if key in NON_QUOTA_KEYS:
                 continue
 
             value = self._validate_quota_limit(body['quota_set'][key])
+            valid_quotas[key] = value
+
+        # NOTE(ankit): Pass #3 - At this point we know that all the keys and
+        # values are valid and we can iterate and update them all in one shot
+        # without having to worry about rolling back etc as we have done
+        # the validation up front in the 2 loops above.
+        for key, value in valid_quotas.items():
             try:
                 db.quota_update(context, project_id, key, value)
             except exception.ProjectQuotaNotFound:
diff --git a/cinder/tests/api/contrib/test_quotas.py b/cinder/tests/api/contrib/test_quotas.py
index 9f8b93d90..fa01c1e71 100644
--- a/cinder/tests/api/contrib/test_quotas.py
+++ b/cinder/tests/api/contrib/test_quotas.py
@@ -93,6 +93,16 @@ class QuotaSetsControllerTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           self.req, 'foo', body)
 
+    def test_update_multi_value_with_bad_data(self):
+        orig_quota = self.controller.show(self.req, 'foo')
+        body = make_body(gigabytes=2000, snapshots=15, volumes="should_be_int",
+                         backups=5, tenant_id=None)
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
+                          self.req, 'foo', body)
+        # Verify that quota values are not updated in db
+        new_quota = self.controller.show(self.req, 'foo')
+        self.assertDictMatch(orig_quota, new_quota)
+
     def test_update_bad_quota_limit(self):
         body = {'quota_set': {'gigabytes': -1000}}
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,