]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Validate value when user update quota
authorwanghao <wanghao749@huawei.com>
Thu, 2 Jul 2015 09:41:17 +0000 (17:41 +0800)
committerwanghao <wanghao749@huawei.com>
Fri, 21 Aug 2015 02:08:31 +0000 (10:08 +0800)
Cinder currently doesn't check the existing resource
when user lowers the quota sets. It should have an optional
action that doesn't allow to update the quota of resources
lower than current usage.

This patch adds code to validate the update value of quota and
ensure the value can't be lower than existing resource.

APIImpact
Add an optional boolean argument 'skip_validation' with default value True.
{
  "quota_set": {xxxx},
  "skip_validation": 'True'/'False'
}
If skip_validation=False and the update value is lower than existing resource,
API will return 400 Bad Request.

Change-Id: I24428ce53e5d10e77235f372bb6d0b70ea672412
Closes-Bug: #1304234

cinder/api/contrib/quotas.py
cinder/tests/unit/api/contrib/test_quotas.py

index 6d46de94497151b90d1f0a9f52f140a5673a59a8..1d9955fdbe0341c169657a8e562412158519e6e7 100644 (file)
@@ -24,6 +24,7 @@ from cinder.db.sqlalchemy import api as sqlalchemy_api
 from cinder import exception
 from cinder.i18n import _
 from cinder import quota
+from cinder import utils
 
 
 QUOTAS = quota.QUOTAS
@@ -56,6 +57,15 @@ class QuotaSetsController(wsgi.Controller):
 
         return dict(quota_set=quota_set)
 
+    def _validate_existing_resource(self, key, value, quota_values):
+        if key == 'per_volume_gigabytes':
+            return
+        v = quota_values.get(key, {})
+        if value < (v.get('in_use', 0) + v.get('reserved', 0)):
+            msg = _("Quota %s limit must be equal or greater than existing "
+                    "resources.") % key
+            raise webob.exc.HTTPBadRequest(explanation=msg)
+
     def _get_quotas(self, context, id, usages=False, parent_project_id=None):
         values = QUOTAS.get_project_quotas(context, id, usages=usages,
                                            parent_project_id=parent_project_id)
@@ -93,6 +103,14 @@ class QuotaSetsController(wsgi.Controller):
         project_id = id
         self.assert_valid_body(body, 'quota_set')
 
+        # Get the optional argument 'skip_validation' from body,
+        # if skip_validation is False, then validate existing resource.
+        skip_flag = body.get('skip_validation', True)
+        if not utils.is_valid_boolstr(skip_flag):
+            msg = _("Invalid value '%s' for skip_validation.") % skip_flag
+            raise exception.InvalidParameterValue(err=msg)
+        skip_flag = strutils.bool_from_string(skip_flag)
+
         bad_keys = []
 
         # NOTE(ankit): Pass #1 - In this loop for body['quota_set'].items(),
@@ -108,7 +126,10 @@ class QuotaSetsController(wsgi.Controller):
 
         # 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.
+        # any of the items in the set is bad. Meanwhile we validate value
+        # to ensure that the value can't be lower than number of existing
+        # resources.
+        quota_values = QUOTAS.get_project_quotas(context, project_id)
         valid_quotas = {}
         for key in body['quota_set'].keys():
             if key in NON_QUOTA_KEYS:
@@ -118,6 +139,9 @@ class QuotaSetsController(wsgi.Controller):
                 body['quota_set'][key], key, min_value=-1,
                 max_value=db.MAX_INT)
 
+            if not skip_flag:
+                self._validate_existing_resource(key, value, quota_values)
+
         # 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
index 3f0f25ed4116792b3dd99945c5f550f17e0a2a9b..6bdeff262bde382a84cf89f162aca0854a01647e 100644 (file)
@@ -29,6 +29,7 @@ from cinder.api.contrib import quotas
 from cinder import context
 from cinder import db
 from cinder import test
+from cinder.tests.unit import test_db_api
 
 
 def make_body(root=True, gigabytes=1000, snapshots=10,
@@ -140,6 +141,8 @@ class QuotaSetsControllerTest(test.TestCase):
 
     def test_update_no_admin(self):
         self.req.environ['cinder.context'].is_admin = False
+        self.req.environ['cinder.context'].project_id = 'foo'
+        self.req.environ['cinder.context'].user_id = 'foo_user'
         self.assertRaises(webob.exc.HTTPForbidden, self.controller.update,
                           self.req, 'foo', make_body(tenant_id=None))
 
@@ -153,6 +156,44 @@ class QuotaSetsControllerTest(test.TestCase):
         self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
                           self.req, 'foo', body)
 
+    def _commit_quota_reservation(self):
+        # Create simple quota and quota usage.
+        ctxt = context.get_admin_context()
+        res = test_db_api._quota_reserve(ctxt, 'foo')
+        db.reservation_commit(ctxt, res, 'foo')
+        expected = {'project_id': 'foo',
+                    'volumes': {'reserved': 0, 'in_use': 1},
+                    'gigabytes': {'reserved': 0, 'in_use': 2},
+                    }
+        self.assertEqual(expected,
+                         db.quota_usage_get_all_by_project(ctxt, 'foo'))
+
+    def test_update_lower_than_existing_resources_when_skip_false(self):
+        self._commit_quota_reservation()
+        body = {'quota_set': {'volumes': 0},
+                'skip_validation': 'false'}
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
+                          self.req, 'foo', body)
+        body = {'quota_set': {'gigabytes': 1},
+                'skip_validation': 'false'}
+        self.assertRaises(webob.exc.HTTPBadRequest, self.controller.update,
+                          self.req, 'foo', body)
+
+    def test_update_lower_than_existing_resources_when_skip_true(self):
+        self._commit_quota_reservation()
+        body = {'quota_set': {'volumes': 0},
+                'skip_validation': 'true'}
+        result = self.controller.update(self.req, 'foo', body)
+        self.assertEqual(body['quota_set']['volumes'],
+                         result['quota_set']['volumes'])
+
+    def test_update_lower_than_existing_resources_without_skip_argument(self):
+        self._commit_quota_reservation()
+        body = {'quota_set': {'volumes': 0}}
+        result = self.controller.update(self.req, 'foo', body)
+        self.assertEqual(body['quota_set']['volumes'],
+                         result['quota_set']['volumes'])
+
     def test_delete(self):
         result_show = self.controller.show(self.req, 'foo')
         self.assertDictMatch(result_show, make_body())