From 5fe6f8015ac3e532c4cf95201209f49e6b69955f Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Fri, 18 Sep 2015 14:10:26 -0700 Subject: [PATCH] Pecan: fix quota management This patch fixes quota management APIs in the Pecan framework. To this aim: 1) an ad-hoc pair of collection/item controllers are introduced for the quota resource; as the new controllers have been added in a separate module, the neutron.pecan_wsgi.controllers.utils module has been added as well for helpers, routines and classes used by all pecan controllers; 2) the quota API extension is made pecan-aware, meaning that it simply returns a Pecan controller instance rather than deferring the task to the startup process that builds controllers using the home-grown WSGI framework ext manager; 3) the quota resource is now "almost" a standard neutron resource; unfortunately since it does not yet have its own service plugin a special provision is made in the attribute population hook in order to ensure the object is loaded for allowing correct policy enforcement. 4) Functional tests for the quota controller have been added. Closes-Bug: #1505843 Change-Id: I44a1fd73f678e493d5b1163e5f183d9efdc678ac --- neutron/extensions/quotasv2.py | 6 +- neutron/pecan_wsgi/controllers/__init__.py | 16 +++ neutron/pecan_wsgi/controllers/quota.py | 128 ++++++++++++++++++ neutron/pecan_wsgi/controllers/root.py | 86 +++++------- neutron/pecan_wsgi/controllers/utils.py | 43 ++++++ .../pecan_wsgi/hooks/policy_enforcement.py | 20 ++- neutron/pecan_wsgi/startup.py | 24 ++-- .../functional/pecan_wsgi/test_functional.py | 86 ++++++++++++ 8 files changed, 341 insertions(+), 68 deletions(-) create mode 100644 neutron/pecan_wsgi/controllers/quota.py create mode 100644 neutron/pecan_wsgi/controllers/utils.py diff --git a/neutron/extensions/quotasv2.py b/neutron/extensions/quotasv2.py index 00ce987f8..fb0abb4e9 100644 --- a/neutron/extensions/quotasv2.py +++ b/neutron/extensions/quotasv2.py @@ -25,11 +25,11 @@ from neutron.api.v2 import resource from neutron.common import constants as const from neutron.common import exceptions as n_exc from neutron import manager +from neutron.pecan_wsgi import controllers from neutron import quota from neutron.quota import resource_registry from neutron import wsgi - RESOURCE_NAME = 'quota' RESOURCE_COLLECTION = RESOURCE_NAME + "s" QUOTAS = quota.QUOTAS @@ -145,6 +145,10 @@ class Quotasv2(extensions.ExtensionDescriptor): controller, collection_actions={'tenant': 'GET'})] + @classmethod + def get_pecan_controllers(cls): + return ((RESOURCE_COLLECTION, controllers.QuotasController()), ) + def get_extended_resources(self, version): if version == "2.0": return EXTENDED_ATTRIBUTES_2_0 diff --git a/neutron/pecan_wsgi/controllers/__init__.py b/neutron/pecan_wsgi/controllers/__init__.py index e69de29bb..a25c1ad67 100644 --- a/neutron/pecan_wsgi/controllers/__init__.py +++ b/neutron/pecan_wsgi/controllers/__init__.py @@ -0,0 +1,16 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from neutron.pecan_wsgi.controllers import quota + + +QuotasController = quota.QuotasController diff --git a/neutron/pecan_wsgi/controllers/quota.py b/neutron/pecan_wsgi/controllers/quota.py new file mode 100644 index 000000000..d7731024d --- /dev/null +++ b/neutron/pecan_wsgi/controllers/quota.py @@ -0,0 +1,128 @@ +# Copyright (c) 2015 Taturiello Consulting, Meh. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from oslo_config import cfg +from oslo_log import log +from oslo_utils import importutils +from pecan import request +from pecan import response + +from neutron.api.v2 import attributes +from neutron.common import constants +from neutron.common import exceptions as n_exc +from neutron.pecan_wsgi.controllers import utils +from neutron.quota import resource_registry + +LOG = log.getLogger(__name__) +RESOURCE_NAME = "quota" + + +class QuotasController(utils.NeutronPecanController): + + def __init__(self): + self._driver = importutils.import_class( + cfg.CONF.QUOTAS.quota_driver + ) + super(QuotasController, self).__init__( + "%ss" % RESOURCE_NAME, RESOURCE_NAME) + + def _check_admin(self, context, + reason=_("Only admin can view or configure quota")): + if not context.is_admin: + raise n_exc.AdminRequired(reason=reason) + + @utils.expose() + def _lookup(self, tenant_id, *remainder): + return QuotaController(self._driver, tenant_id), remainder + + @utils.expose() + def index(self): + neutron_context = request.context.get('neutron_context') + # FIXME(salv-orlando): There shouldn't be any need to to this eplicit + # check. However some behaviours from the "old" extension have + # been temporarily carried over here + self._check_admin(neutron_context) + # TODO(salv-orlando): proper plurals management + return {self.collection: + self._driver.get_all_quotas( + neutron_context, + resource_registry.get_all_resources())} + + +class QuotaController(utils.NeutronPecanController): + + def __init__(self, _driver, tenant_id): + self._driver = _driver + self._tenant_id = tenant_id + + super(QuotaController, self).__init__( + "%ss" % RESOURCE_NAME, RESOURCE_NAME) + + # Ensure limits for all registered resources are returned + attr_dict = attributes.RESOURCE_ATTRIBUTE_MAP[self.collection] + for quota_resource in resource_registry.get_all_resources().keys(): + attr_dict[quota_resource] = { + 'allow_post': False, + 'allow_put': True, + 'convert_to': attributes.convert_to_int, + 'validate': { + 'type:range': [-1, constants.DB_INTEGER_MAX_VALUE]}, + 'is_visible': True} + + @utils.expose(generic=True) + def index(self): + return get_tenant_quotas(self._tenant_id, self._driver) + + @utils.when(index, method='PUT') + def put(self, *args, **kwargs): + neutron_context = request.context.get('neutron_context') + # For put requests there's always going to be a single element + quota_data = request.context['resources'][0] + for key, value in quota_data.items(): + self._driver.update_quota_limit( + neutron_context, self._tenant_id, key, value) + return get_tenant_quotas(self._tenant_id, self._driver) + + @utils.when(index, method='DELETE') + def delete(self): + neutron_context = request.context.get('neutron_context') + self._driver.delete_tenant_quota(neutron_context, + self._tenant_id) + response.status = 204 + + +def get_tenant_quotas(tenant_id, driver=None): + if not driver: + driver = importutils.import_class(cfg.CONF.QUOTAS.quota_driver) + + neutron_context = request.context.get('neutron_context') + if tenant_id == 'tenant': + # NOTE(salv-orlando): Read the following before the code in order + # to avoid puking. + # There is a weird undocumented behaviour of the Neutron quota API + # as 'tenant' is used as an API action to return the identifier + # of the tenant in the request context. This is used exclusively + # for interaction with python-neutronclient and is a possibly + # unnecessary 'whoami' API endpoint. Pending resolution of this + # API issue, this controller will just treat the magic string + # 'tenant' (and only that string) and return the response expected + # by python-neutronclient + return {'tenant': {'tenant_id': neutron_context.tenant_id}} + tenant_quotas = driver.get_tenant_quotas( + neutron_context, + resource_registry.get_all_resources(), + tenant_id) + tenant_quotas['tenant_id'] = tenant_id + return {RESOURCE_NAME: tenant_quotas} diff --git a/neutron/pecan_wsgi/controllers/root.py b/neutron/pecan_wsgi/controllers/root.py index 977a7e58a..36cc6dc5a 100644 --- a/neutron/pecan_wsgi/controllers/root.py +++ b/neutron/pecan_wsgi/controllers/root.py @@ -22,6 +22,7 @@ from neutron._i18n import _, _LW from neutron.api import extensions from neutron.api.views import versions as versions_view from neutron import manager +from neutron.pecan_wsgi.controllers import utils LOG = log.getLogger(__name__) _VERSION_INFO = {} @@ -36,44 +37,30 @@ def _get_version_info(): return _VERSION_INFO.values() -def expose(*args, **kwargs): - """Helper function so we don't have to specify json for everything.""" - kwargs.setdefault('content_type', 'application/json') - kwargs.setdefault('template', 'json') - return pecan.expose(*args, **kwargs) - - -def when(index, *args, **kwargs): - """Helper function so we don't have to specify json for everything.""" - kwargs.setdefault('content_type', 'application/json') - kwargs.setdefault('template', 'json') - return index.when(*args, **kwargs) - - class RootController(object): - @expose(generic=True) + @utils.expose(generic=True) def index(self): builder = versions_view.get_view_builder(pecan.request) versions = [builder.build(version) for version in _get_version_info()] return dict(versions=versions) - @when(index, method='HEAD') - @when(index, method='POST') - @when(index, method='PATCH') - @when(index, method='PUT') - @when(index, method='DELETE') + @utils.when(index, method='HEAD') + @utils.when(index, method='POST') + @utils.when(index, method='PATCH') + @utils.when(index, method='PUT') + @utils.when(index, method='DELETE') def not_supported(self): pecan.abort(405) class ExtensionsController(object): - @expose() + @utils.expose() def _lookup(self, alias, *remainder): return ExtensionController(alias), remainder - @expose() + @utils.expose() def index(self): ext_mgr = extensions.PluginAwareExtensionManager.get_instance() exts = [extensions.ExtensionController._translate(ext) @@ -93,20 +80,20 @@ class V2Controller(object): extensions = ExtensionsController() - @expose(generic=True) + @utils.expose(generic=True) def index(self): builder = versions_view.get_view_builder(pecan.request) return dict(version=builder.build(self.version_info)) - @when(index, method='HEAD') - @when(index, method='POST') - @when(index, method='PATCH') - @when(index, method='PUT') - @when(index, method='DELETE') + @utils.when(index, method='HEAD') + @utils.when(index, method='POST') + @utils.when(index, method='PATCH') + @utils.when(index, method='PUT') + @utils.when(index, method='DELETE') def not_supported(self): pecan.abort(405) - @expose() + @utils.expose() def _lookup(self, collection, *remainder): controller = manager.NeutronManager.get_controller_for_resource( collection) @@ -131,7 +118,7 @@ class ExtensionController(object): def __init__(self, alias): self.alias = alias - @expose() + @utils.expose() def index(self): ext_mgr = extensions.PluginAwareExtensionManager.get_instance() ext = ext_mgr.extensions.get(self.alias, None) @@ -142,24 +129,15 @@ class ExtensionController(object): return {'extension': extensions.ExtensionController._translate(ext)} -class NeutronPecanController(object): - - def __init__(self, collection, resource): - self.collection = collection - self.resource = resource - self.plugin = manager.NeutronManager.get_plugin_for_resource( - self.resource) - - -class CollectionsController(NeutronPecanController): +class CollectionsController(utils.NeutronPecanController): - @expose() + @utils.expose() def _lookup(self, item, *remainder): # Store resource identifier in request context request.context['resource_id'] = item return ItemController(self.resource, item), remainder - @expose(generic=True) + @utils.expose(generic=True) def index(self, *args, **kwargs): return self.get(*args, **kwargs) @@ -175,14 +153,14 @@ class CollectionsController(NeutronPecanController): neutron_context = request.context['neutron_context'] return {self.collection: lister(neutron_context, filters=filters)} - @when(index, method='HEAD') - @when(index, method='PATCH') - @when(index, method='PUT') - @when(index, method='DELETE') + @utils.when(index, method='HEAD') + @utils.when(index, method='PATCH') + @utils.when(index, method='PUT') + @utils.when(index, method='DELETE') def not_supported(self): pecan.abort(405) - @when(index, method='POST') + @utils.when(index, method='POST') def post(self, *args, **kwargs): # TODO(kevinbenton): emulated bulk! resources = request.context['resources'] @@ -204,13 +182,13 @@ class CollectionsController(NeutronPecanController): return {key: creator(neutron_context, data)} -class ItemController(NeutronPecanController): +class ItemController(utils.NeutronPecanController): def __init__(self, resource, item): super(ItemController, self).__init__(None, resource) self.item = item - @expose(generic=True) + @utils.expose(generic=True) def index(self, *args, **kwargs): return self.get() @@ -219,13 +197,13 @@ class ItemController(NeutronPecanController): neutron_context = request.context['neutron_context'] return {self.resource: getter(neutron_context, self.item)} - @when(index, method='HEAD') - @when(index, method='POST') - @when(index, method='PATCH') + @utils.when(index, method='HEAD') + @utils.when(index, method='POST') + @utils.when(index, method='PATCH') def not_supported(self): pecan.abort(405) - @when(index, method='PUT') + @utils.when(index, method='PUT') def put(self, *args, **kwargs): neutron_context = request.context['neutron_context'] resources = request.context['resources'] @@ -241,7 +219,7 @@ class ItemController(NeutronPecanController): data = {self.resource: resources[0]} return updater(neutron_context, self.item, data) - @when(index, method='DELETE') + @utils.when(index, method='DELETE') def delete(self): # TODO(kevinbenton): setting code could be in a decorator pecan.response.status = 204 diff --git a/neutron/pecan_wsgi/controllers/utils.py b/neutron/pecan_wsgi/controllers/utils.py new file mode 100644 index 000000000..49bd97e58 --- /dev/null +++ b/neutron/pecan_wsgi/controllers/utils.py @@ -0,0 +1,43 @@ +# Copyright (c) 2015 Taturiello Consulting, Meh. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import pecan + +from neutron import manager + +# Utility functions for Pecan controllers. + + +def expose(*args, **kwargs): + """Helper function so we don't have to specify json for everything.""" + kwargs.setdefault('content_type', 'application/json') + kwargs.setdefault('template', 'json') + return pecan.expose(*args, **kwargs) + + +def when(index, *args, **kwargs): + """Helper function so we don't have to specify json for everything.""" + kwargs.setdefault('content_type', 'application/json') + kwargs.setdefault('template', 'json') + return index.when(*args, **kwargs) + + +class NeutronPecanController(object): + + def __init__(self, collection, resource): + self.collection = collection + self.resource = resource + self.plugin = manager.NeutronManager.get_plugin_for_resource( + self.resource) diff --git a/neutron/pecan_wsgi/hooks/policy_enforcement.py b/neutron/pecan_wsgi/hooks/policy_enforcement.py index e505eea48..dcd1800ab 100644 --- a/neutron/pecan_wsgi/hooks/policy_enforcement.py +++ b/neutron/pecan_wsgi/hooks/policy_enforcement.py @@ -24,10 +24,18 @@ import webob from neutron._i18n import _ from neutron.api.v2 import attributes as v2_attributes from neutron.common import constants as const +from neutron.extensions import quotasv2 from neutron import manager +from neutron.pecan_wsgi.controllers import quota from neutron import policy +def _custom_getter(resource, resource_id): + """Helper function to retrieve resources not served by any plugin.""" + if resource == quotasv2.RESOURCE_NAME: + return quota.get_tenant_quotas(resource_id)[quotasv2.RESOURCE_NAME] + + class PolicyHook(hooks.PecanHook): priority = 135 ACTION_MAP = {'POST': 'create', 'PUT': 'update', 'GET': 'get', @@ -39,9 +47,15 @@ class PolicyHook(hooks.PecanHook): if (value.get('required_by_policy') or value.get('primary_key') or 'default' not in value)] plugin = manager.NeutronManager.get_plugin_for_resource(resource) - getter = getattr(plugin, 'get_%s' % resource) - # TODO(kevinbenton): the parent_id logic currently in base.py - return getter(neutron_context, resource_id, fields=field_list) + if plugin: + getter = getattr(plugin, 'get_%s' % resource) + # TODO(kevinbenton): the parent_id logic currently in base.py + return getter(neutron_context, resource_id, fields=field_list) + else: + # Some legit resources, like quota, do not have a plugin yet. + # Retrieving the original object is nevertheless important + # for policy checks. + return _custom_getter(resource, resource_id) def before(self, state): # This hook should be run only for PUT,POST and DELETE methods and for diff --git a/neutron/pecan_wsgi/startup.py b/neutron/pecan_wsgi/startup.py index 4b6dab2f7..e4ea30ed8 100644 --- a/neutron/pecan_wsgi/startup.py +++ b/neutron/pecan_wsgi/startup.py @@ -74,8 +74,8 @@ def initialize_all(): hasattr(ext, 'get_pecan_controllers')] pecan_controllers = {} for ext in pecanized_exts: - LOG.debug("Extension %s is pecan-enabled. Fetching resources " - "and controllers", ext.get_name()) + LOG.info(_LI("Extension %s is pecan-aware. Fetching resources " + "and controllers"), ext.get_name()) controllers = ext.get_pecan_controllers() # controllers is actually a list of pairs where the first element is # the collection name and the second the actual controller @@ -83,24 +83,28 @@ def initialize_all(): pecan_controllers[collection] = coll_controller for collection in attributes.RESOURCE_ATTRIBUTE_MAP: - if collection not in pecan_controllers: - resource = _handle_plurals(collection) + resource = _handle_plurals(collection) + controller = pecan_controllers.get(collection) + if not controller: LOG.debug("Building controller for resource:%s", resource) plugin = _plugin_for_resource(collection) if plugin: manager.NeutronManager.set_plugin_for_resource( resource, plugin) + else: + LOG.warn(_LW("No plugin found for resource:%s. API calls " + "may not be correctly dispatched"), resource) controller = root.CollectionsController(collection, resource) - manager.NeutronManager.set_controller_for_resource( - collection, controller) - LOG.info(_LI("Added controller for resource %(resource)s " - "via URI path segment:%(collection)s"), - {'resource': resource, - 'collection': collection}) else: LOG.debug("There are already controllers for resource:%s", resource) + manager.NeutronManager.set_controller_for_resource( + collection, controller) + LOG.info(_LI("Added controller for resource %(resource)s " + "via URI path segment:%(collection)s"), + {'resource': resource, + 'collection': collection}) # NOTE(salv-orlando): If you are care about code quality, please read below # Hackiness is strong with the piece of code below. It is used for # populating resource plurals and registering resources with the quota diff --git a/neutron/tests/functional/pecan_wsgi/test_functional.py b/neutron/tests/functional/pecan_wsgi/test_functional.py index 2c05b20fb..ee20ec344 100644 --- a/neutron/tests/functional/pecan_wsgi/test_functional.py +++ b/neutron/tests/functional/pecan_wsgi/test_functional.py @@ -462,3 +462,89 @@ class TestRootController(PecanFunctionalTest): def test_head(self): self._test_method_returns_405('head') + + +class TestQuotasController(TestRootController): + """Test quota management API controller.""" + + base_url = '/v2.0/quotas' + default_expected_limits = { + 'network': 10, + 'port': 50, + 'subnet': 10} + + def _verify_limits(self, response, limits): + for resource, limit in limits.items(): + self.assertEqual(limit, response['quota'][resource]) + + def _verify_default_limits(self, response): + self._verify_limits(response, self.default_expected_limits) + + def _verify_after_update(self, response, updated_limits): + expected_limits = self.default_expected_limits.copy() + expected_limits.update(updated_limits) + self._verify_limits(response, expected_limits) + + def test_index_admin(self): + # NOTE(salv-orlando): The quota controller has an hardcoded check for + # admin-ness for this operation, which is supposed to return quotas for + # all tenants. Such check is "vestigial" from the home-grown WSGI and + # shall be removed + response = self.app.get('%s.json' % self.base_url, + headers={'X-Project-Id': 'admin', + 'X-Roles': 'admin'}) + self.assertEqual(200, response.status_int) + + def test_index(self): + response = self.app.get('%s.json' % self.base_url, expect_errors=True) + self.assertEqual(403, response.status_int) + + def test_get_admin(self): + response = self.app.get('%s/foo.json' % self.base_url, + headers={'X-Project-Id': 'admin', + 'X-Roles': 'admin'}) + self.assertEqual(200, response.status_int) + # As quota limits have not been updated, expect default values + json_body = jsonutils.loads(response.body) + self._verify_default_limits(json_body) + + def test_get(self): + # It is not ok to access another tenant's limits + url = '%s/foo.json' % self.base_url + response = self.app.get(url, expect_errors=True) + self.assertEqual(403, response.status_int) + # It is however ok to retrieve your own limits + response = self.app.get(url, headers={'X-Project-Id': 'foo'}) + self.assertEqual(200, response.status_int) + json_body = jsonutils.loads(response.body) + self._verify_default_limits(json_body) + + def test_put_get_delete(self): + # PUT and DELETE actions are in the same test as a meaningful DELETE + # test would require a put anyway + url = '%s/foo.json' % self.base_url + response = self.app.put_json(url, + params={'quota': {'network': 99}}, + headers={'X-Project-Id': 'admin', + 'X-Roles': 'admin'}) + self.assertEqual(200, response.status_int) + json_body = jsonutils.loads(response.body) + self._verify_after_update(json_body, {'network': 99}) + + response = self.app.get(url, headers={'X-Project-Id': 'foo'}) + self.assertEqual(200, response.status_int) + json_body = jsonutils.loads(response.body) + self._verify_after_update(json_body, {'network': 99}) + + response = self.app.delete(url, headers={'X-Project-Id': 'admin', + 'X-Roles': 'admin'}) + self.assertEqual(204, response.status_int) + # As DELETE does not return a body we need another GET + response = self.app.get(url, headers={'X-Project-Id': 'foo'}) + self.assertEqual(200, response.status_int) + json_body = jsonutils.loads(response.body) + self._verify_default_limits(json_body) + + def test_delete(self): + # TODO(salv-orlando) + pass -- 2.45.2