From b2abe32ed02649dd2e21fbc945e8863798419450 Mon Sep 17 00:00:00 2001 From: wanghao Date: Thu, 2 Jul 2015 17:41:17 +0800 Subject: [PATCH] Validate value when user update quota 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 | 26 ++++++++++++- cinder/tests/unit/api/contrib/test_quotas.py | 41 ++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/cinder/api/contrib/quotas.py b/cinder/api/contrib/quotas.py index 6d46de944..1d9955fdb 100644 --- a/cinder/api/contrib/quotas.py +++ b/cinder/api/contrib/quotas.py @@ -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 diff --git a/cinder/tests/unit/api/contrib/test_quotas.py b/cinder/tests/unit/api/contrib/test_quotas.py index 3f0f25ed4..6bdeff262 100644 --- a/cinder/tests/unit/api/contrib/test_quotas.py +++ b/cinder/tests/unit/api/contrib/test_quotas.py @@ -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()) -- 2.45.2