From e1bd5149c335d603014f01e4e663cf4fb096bd2c Mon Sep 17 00:00:00 2001 From: He Jie Xu Date: Tue, 5 Feb 2013 19:05:53 +0800 Subject: [PATCH] Wrap quota controller with resource.Resource Fixes bug 1116137 Currently the format of error message returned by quota extension was different with quantum other resource. Other resource will return as json(eg, '{"QuantumError": "error message"}'). But quota extension only return messages without any format. 'quantum.api.v2.resource.Resource' provider error messages processing. So wrap quota controller with it. By the way, fix some small stuff: * Use specific exception 'QuotaTenantNotFound' instead of generic exception. * Correct error message. * Use attribute mapping checking the request body. Change-Id: I71261198aa79e9ed8e0ae672de32552abdbf45c1 --- quantum/common/exceptions.py | 4 + quantum/extensions/quotasv2.py | 75 ++++----- ...ta_per_tenant_ext.py => test_quota_ext.py} | 155 ++++++++++-------- 3 files changed, 123 insertions(+), 111 deletions(-) rename quantum/tests/unit/{test_quota_per_tenant_ext.py => test_quota_ext.py} (73%) diff --git a/quantum/common/exceptions.py b/quantum/common/exceptions.py index 7015de21c3..a2997bca69 100644 --- a/quantum/common/exceptions.py +++ b/quantum/common/exceptions.py @@ -228,6 +228,10 @@ class OverQuota(Conflict): message = _("Quota exceeded for resources: %(overs)s") +class QuotaMissingTenant(BadRequest): + message = _("Tenant-id was missing from Quota request") + + class InvalidQuotaValue(Conflict): message = _("Change would make usage less than 0 for the following " "resources: %(unders)s") diff --git a/quantum/extensions/quotasv2.py b/quantum/extensions/quotasv2.py index 41a95018ea..1f9c634b98 100644 --- a/quantum/extensions/quotasv2.py +++ b/quantum/extensions/quotasv2.py @@ -19,8 +19,10 @@ from oslo.config import cfg import webob from quantum.api import extensions +from quantum.api.v2.attributes import convert_to_int from quantum.api.v2 import base -from quantum.common import exceptions +from quantum.api.v2 import resource +from quantum.common import exceptions as q_exc from quantum.manager import QuantumManager from quantum.openstack.common import importutils from quantum import quota @@ -49,21 +51,10 @@ class QuotaSetsController(wsgi.Controller): attr_dict = EXTENDED_ATTRIBUTES_2_0[RESOURCE_COLLECTION] attr_dict[quota_resource] = {'allow_post': False, 'allow_put': True, - 'convert_to': int, + 'convert_to': convert_to_int, 'is_visible': True} self._update_extended_attributes = False - def _get_body(self, request): - body = self._deserialize(request.body, - request.best_match_content_type()) - if self._update_extended_attributes: - self._update_attributes() - - attr_info = EXTENDED_ATTRIBUTES_2_0[RESOURCE_COLLECTION] - req_body = base.Controller.prepare_request_body( - request.context, body, False, self._resource_name, attr_info) - return req_body - def _get_quotas(self, request, tenant_id): return self._driver.get_tenant_quotas( request.context, QUOTAS.resources, tenant_id) @@ -73,8 +64,7 @@ class QuotaSetsController(wsgi.Controller): def index(self, request): context = request.context - if not context.is_admin: - raise webob.exc.HTTPForbidden() + self._check_admin(context) return {self._resource_name + "s": self._driver.get_all_quotas(context, QUOTAS.resources)} @@ -82,44 +72,35 @@ class QuotaSetsController(wsgi.Controller): """Retrieve the tenant info in context.""" context = request.context if not context.tenant_id: - raise webob.exc.HTTPBadRequest('invalid tenant') + raise q_exc.QuotaMissingTenant() return {'tenant': {'tenant_id': context.tenant_id}} def show(self, request, id): - context = request.context - tenant_id = id - if not tenant_id: - raise webob.exc.HTTPBadRequest('invalid tenant') - if (tenant_id != context.tenant_id and - not context.is_admin): - raise webob.exc.HTTPForbidden() - return {self._resource_name: - self._get_quotas(request, tenant_id)} + if id != request.context.tenant_id: + self._check_admin(request.context, + reason=_("Non-admin is not authorised " + "to access quotas for another tenant")) + return {self._resource_name: self._get_quotas(request, id)} - def _check_modification_delete_privilege(self, context, tenant_id): - if not tenant_id: - raise webob.exc.HTTPBadRequest('invalid tenant') + def _check_admin(self, context, + reason=_("Only admin can view or configure quota")): if not context.is_admin: - raise webob.exc.HTTPForbidden() - return tenant_id + raise q_exc.AdminRequired(reason=reason) def delete(self, request, id): - tenant_id = self._check_modification_delete_privilege(request.context, - id) - self._driver.delete_tenant_quota(request.context, tenant_id) + self._check_admin(request.context) + self._driver.delete_tenant_quota(request.context, id) - def update(self, request, id): - tenant_id = self._check_modification_delete_privilege(request.context, - id) - req_body = self._get_body(request) - for key in req_body[self._resource_name].keys(): - if key in QUOTAS.resources: - value = int(req_body[self._resource_name][key]) - self._driver.update_quota_limit(request.context, - tenant_id, - key, - value) - return {self._resource_name: self._get_quotas(request, tenant_id)} + def update(self, request, id, body=None): + self._check_admin(request.context) + if self._update_extended_attributes: + self._update_attributes() + body = base.Controller.prepare_request_body( + request.context, body, False, self._resource_name, + EXTENDED_ATTRIBUTES_2_0[RESOURCE_COLLECTION]) + for key, value in body[self._resource_name].iteritems(): + self._driver.update_quota_limit(request.context, id, key, value) + return {self._resource_name: self._get_quotas(request, id)} class Quotasv2(extensions.ExtensionDescriptor): @@ -151,7 +132,9 @@ class Quotasv2(extensions.ExtensionDescriptor): @classmethod def get_resources(cls): """ Returns Ext Resources """ - controller = QuotaSetsController(QuantumManager.get_plugin()) + controller = resource.Resource( + QuotaSetsController(QuantumManager.get_plugin()), + faults=base.FAULT_MAP) return [extensions.ResourceExtension( Quotasv2.get_alias(), controller, diff --git a/quantum/tests/unit/test_quota_per_tenant_ext.py b/quantum/tests/unit/test_quota_ext.py similarity index 73% rename from quantum/tests/unit/test_quota_per_tenant_ext.py rename to quantum/tests/unit/test_quota_ext.py index 0006562e29..73cfc74f97 100644 --- a/quantum/tests/unit/test_quota_per_tenant_ext.py +++ b/quantum/tests/unit/test_quota_ext.py @@ -12,7 +12,6 @@ from quantum.db import api as db from quantum import manager from quantum.plugins.linuxbridge.db import l2network_db_v2 from quantum import quota -from quantum.tests import base from quantum.tests.unit import test_api_v2 from quantum.tests.unit import test_extensions from quantum.tests.unit import testlib_api @@ -24,7 +23,6 @@ _get_path = test_api_v2._get_path class QuotaExtensionTestCase(testlib_api.WebTestCase): - fmt = 'json' def setUp(self): super(QuotaExtensionTestCase, self).setUp() @@ -47,10 +45,6 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): # Update the plugin and extensions path cfg.CONF.set_override('core_plugin', TARGET_PLUGIN) - cfg.CONF.set_override( - 'quota_driver', - 'quantum.db.quota_db.DbQuotaDriver', - group='QUOTAS') cfg.CONF.set_override( 'quota_items', ['network', 'subnet', 'port', 'extra1'], @@ -68,7 +62,6 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): app = config.load_paste_app('extensions_test_app') ext_middleware = extensions.ExtensionMiddleware(app, ext_mgr=ext_mgr) self.api = webtest.TestApp(ext_middleware) - super(QuotaExtensionTestCase, self).setUp() def tearDown(self): self._plugin_patcher.stop() @@ -82,6 +75,17 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): attributes.RESOURCE_ATTRIBUTE_MAP = self.saved_attr_map super(QuotaExtensionTestCase, self).tearDown() + +class QuotaExtensionDbTestCase(QuotaExtensionTestCase): + fmt = 'json' + + def setUp(self): + cfg.CONF.set_override( + 'quota_driver', + 'quantum.db.quota_db.DbQuotaDriver', + group='QUOTAS') + super(QuotaExtensionDbTestCase, self).setUp() + def test_quotas_loaded_right(self): res = self.api.get(_get_path('quotas', fmt=self.fmt)) quota = self.deserialize(res) @@ -106,8 +110,12 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): res = self.api.get(_get_path('quotas', id=tenant_id, fmt=self.fmt), extra_environ=env) self.assertEqual(200, res.status_int) + quota = self.deserialize(res) + self.assertEqual(10, quota['quota']['network']) + self.assertEqual(10, quota['quota']['subnet']) + self.assertEqual(50, quota['quota']['port']) - def test_show_quotas_without_admin_forbidden(self): + def test_show_quotas_without_admin_forbidden_returns_403(self): tenant_id = 'tenant_id1' env = {'quantum.context': context.Context('', tenant_id + '2', is_admin=False)} @@ -115,7 +123,37 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): extra_environ=env, expect_errors=True) self.assertEqual(403, res.status_int) - def test_update_quotas_without_admin_forbidden(self): + def test_show_quotas_with_owner_tenant(self): + tenant_id = 'tenant_id1' + env = {'quantum.context': context.Context('', tenant_id, + is_admin=False)} + res = self.api.get(_get_path('quotas', id=tenant_id, fmt=self.fmt), + extra_environ=env) + self.assertEqual(200, res.status_int) + quota = self.deserialize(res) + self.assertEqual(10, quota['quota']['network']) + self.assertEqual(10, quota['quota']['subnet']) + self.assertEqual(50, quota['quota']['port']) + + def test_list_quotas_with_admin(self): + tenant_id = 'tenant_id1' + env = {'quantum.context': context.Context('', tenant_id, + is_admin=True)} + res = self.api.get(_get_path('quotas', fmt=self.fmt), + extra_environ=env) + self.assertEqual(200, res.status_int) + quota = self.deserialize(res) + self.assertEqual([], quota['quotas']) + + def test_list_quotas_without_admin_forbidden_returns_403(self): + tenant_id = 'tenant_id1' + env = {'quantum.context': context.Context('', tenant_id, + is_admin=False)} + res = self.api.get(_get_path('quotas', fmt=self.fmt), + extra_environ=env, expect_errors=True) + self.assertEqual(403, res.status_int) + + def test_update_quotas_without_admin_forbidden_returns_403(self): tenant_id = 'tenant_id1' env = {'quantum.context': context.Context('', tenant_id, is_admin=False)} @@ -125,6 +163,26 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): expect_errors=True) self.assertEqual(403, res.status_int) + def test_update_quotas_with_non_integer_returns_400(self): + tenant_id = 'tenant_id1' + env = {'quantum.context': context.Context('', tenant_id, + is_admin=True)} + quotas = {'quota': {'network': 'abc'}} + res = self.api.put(_get_path('quotas', id=tenant_id, fmt=self.fmt), + self.serialize(quotas), extra_environ=env, + expect_errors=True) + self.assertEqual(400, res.status_int) + + def test_update_quotas_with_non_support_resource_returns_400(self): + tenant_id = 'tenant_id1' + env = {'quantum.context': context.Context('', tenant_id, + is_admin=True)} + quotas = {'quota': {'abc': 100}} + res = self.api.put(_get_path('quotas', id=tenant_id, fmt=self.fmt), + self.serialize(quotas), extra_environ=env, + expect_errors=True) + self.assertEqual(400, res.status_int) + def test_update_quotas_with_admin(self): tenant_id = 'tenant_id1' env = {'quantum.context': context.Context('', tenant_id + '2', @@ -138,6 +196,8 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): extra_environ=env2) quota = self.deserialize(res) self.assertEqual(100, quota['quota']['network']) + self.assertEqual(10, quota['quota']['subnet']) + self.assertEqual(50, quota['quota']['port']) def test_update_attributes(self): tenant_id = 'tenant_id1' @@ -161,7 +221,7 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): extra_environ=env) self.assertEqual(204, res.status_int) - def test_delete_quotas_without_admin_forbidden(self): + def test_delete_quotas_without_admin_forbidden_returns_403(self): tenant_id = 'tenant_id1' env = {'quantum.context': context.Context('', tenant_id, is_admin=False)} @@ -169,7 +229,7 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): extra_environ=env, expect_errors=True) self.assertEqual(403, res.status_int) - def test_quotas_loaded_bad(self): + def test_quotas_loaded_bad_returns_404(self): try: res = self.api.get(_get_path('quotas'), expect_errors=True) self.assertEqual(404, res.status_int) @@ -210,66 +270,31 @@ class QuotaExtensionTestCase(testlib_api.WebTestCase): tenant_id, network=-1) + def test_quotas_get_tenant_from_request_context(self): + tenant_id = 'tenant_id1' + env = {'quantum.context': context.Context('', tenant_id, + is_admin=True)} + res = self.api.get(_get_path('quotas/tenant', fmt=self.fmt), + extra_environ=env) + self.assertEqual(200, res.status_int) + quota = self.deserialize(res) + self.assertEqual(quota['tenant']['tenant_id'], tenant_id) -class QuotaExtensionTestCaseXML(QuotaExtensionTestCase): + def test_quotas_get_tenant_from_empty_request_context_returns_400(self): + env = {'quantum.context': context.Context('', '', + is_admin=True)} + res = self.api.get(_get_path('quotas/tenant', fmt=self.fmt), + extra_environ=env, expect_errors=True) + self.assertEqual(400, res.status_int) + + +class QuotaExtensionDbTestCaseXML(QuotaExtensionDbTestCase): fmt = 'xml' -class QuotaExtensionCfgTestCase(testlib_api.WebTestCase): +class QuotaExtensionCfgTestCase(QuotaExtensionTestCase): fmt = 'json' - def setUp(self): - super(QuotaExtensionCfgTestCase, self).setUp() - db._ENGINE = None - db._MAKER = None - # Ensure 'stale' patched copies of the plugin are never returned - manager.QuantumManager._instance = None - - # Ensure existing ExtensionManager is not used - extensions.PluginAwareExtensionManager._instance = None - - # Save the global RESOURCE_ATTRIBUTE_MAP - self.saved_attr_map = {} - for resource, attrs in attributes.RESOURCE_ATTRIBUTE_MAP.iteritems(): - self.saved_attr_map[resource] = attrs.copy() - - # Create the default configurations - args = ['--config-file', test_extensions.etcdir('quantum.conf.test')] - config.parse(args=args) - - # Update the plugin and extensions path - cfg.CONF.set_override('core_plugin', TARGET_PLUGIN) - cfg.CONF.set_override( - 'quota_items', - ['network', 'subnet', 'port', 'extra1'], - group='QUOTAS') - quota.QUOTAS = quota.QuotaEngine() - quota.register_resources_from_config() - self._plugin_patcher = mock.patch(TARGET_PLUGIN, autospec=True) - self.plugin = self._plugin_patcher.start() - self.plugin.return_value.supported_extension_aliases = ['quotas'] - # QUOTAS will regester the items in conf when starting - # extra1 here is added later, so have to do it manually - quota.QUOTAS.register_resource_by_name('extra1') - ext_mgr = extensions.PluginAwareExtensionManager.get_instance() - l2network_db_v2.initialize() - app = config.load_paste_app('extensions_test_app') - ext_middleware = extensions.ExtensionMiddleware(app, ext_mgr=ext_mgr) - self.api = webtest.TestApp(ext_middleware) - super(QuotaExtensionCfgTestCase, self).setUp() - - def tearDown(self): - self._plugin_patcher.stop() - self.api = None - self.plugin = None - db._ENGINE = None - db._MAKER = None - cfg.CONF.reset() - - # Restore the global RESOURCE_ATTRIBUTE_MAP - attributes.RESOURCE_ATTRIBUTE_MAP = self.saved_attr_map - super(QuotaExtensionCfgTestCase, self).tearDown() - def test_quotas_default_values(self): tenant_id = 'tenant_id1' env = {'quantum.context': context.Context('', tenant_id)}