From: ankitagrawal <ankit11.agrawal@nttdata.com>
Date: Tue, 17 Mar 2015 09:08:12 +0000 (-0700)
Subject: Verify all quotas before updating the database
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=0c467970f12cc7212cc43ee210e6386d083b5ef9;p=openstack-build%2Fcinder-build.git

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
(cherry picked from commit 197299985a52535ad995dbd111491b58462f140a)
---

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,