Verify all quotas before updating the database

Update quota multi-value in one request is not an atomic operation, so
even if a quota is invalid (e.g., volumes has a non-integer value),
some values may change but the response code is 400.

This patch will make sure the quota limits are of integer type for all
the requested quota keys before updating the database.

Closes-Bug: 1432972
Change-Id: If83af0a80489887098e52b03b9c4012e3ef061b5

diff --git a/cinder/api/contrib/quotas.py b/cinder/api/contrib/quotas.py
index 8ef8ee2e4eebe88071e9af5ba3eb6630f1b8886f..32667cad345b3d060416c63533aa055703039030 100644 (file)
--- 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 9f8b93d90d8b34aab8485bcc8723cd4053c86e95..fa01c1e711eef69da68538a77b4771384d32dc86 100644 (file)
--- 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,