From c15ffc2ee22c85813f5c36324d3a3097fff45d48 Mon Sep 17 00:00:00 2001 From: Ghanshyam Mann Date: Sun, 29 Mar 2020 00:18:52 -0500 Subject: [PATCH] Combine the limits policies in single place limits and used_limits extensions were megred in - I76e02214e958a55b6de8033243b46b259949e5ac But policy were left in separate file. limits policy is in policies/limits which is general policy to get the limit of project. used_limit is in polocies/used_limit which is enforced in view builder for gettting the limit of other project. This commit: - move used_limit in policies/limit file - move the used_limit policy enforcement from view buidler to limit API controller. - adjust the tests due to above changes. Partial implement blueprint policy-defaults-refresh Change-Id: Iefe41cc95cd967b368588dea5ff195bb4af3eca7 --- nova/api/openstack/compute/limits.py | 10 ++++- nova/api/openstack/compute/views/limits.py | 13 ------ nova/policies/__init__.py | 2 - nova/policies/limits.py | 19 +++++++- nova/policies/used_limits.py | 45 ------------------- .../unit/api/openstack/compute/test_limits.py | 40 ++++++----------- 6 files changed, 41 insertions(+), 88 deletions(-) delete mode 100644 nova/policies/used_limits.py diff --git a/nova/api/openstack/compute/limits.py b/nova/api/openstack/compute/limits.py index 9d28ffe70c72..a6e9dd41907b 100644 --- a/nova/api/openstack/compute/limits.py +++ b/nova/api/openstack/compute/limits.py @@ -75,7 +75,15 @@ class LimitsController(wsgi.Controller): """Return all global limit information.""" context = req.environ['nova.context'] context.can(limits_policies.BASE_POLICY_NAME) - project_id = req.params.get('tenant_id', context.project_id) + project_id = context.project_id + if 'tenant_id' in req.GET: + project_id = req.GET.get('tenant_id') + target = { + 'project_id': project_id, + 'user_id': context.user_id + } + context.can(limits_policies.USED_LIMIT_POLICY_NAME, target) + quotas = QUOTAS.get_project_quotas(context, project_id, usages=True) builder = limits_views.ViewBuilder() diff --git a/nova/api/openstack/compute/views/limits.py b/nova/api/openstack/compute/views/limits.py index f58f3191f510..60870cbe4e34 100644 --- a/nova/api/openstack/compute/views/limits.py +++ b/nova/api/openstack/compute/views/limits.py @@ -13,8 +13,6 @@ # License for the specific language governing permissions and limitations # under the License. -from nova.policies import used_limits as ul_policies - class ViewBuilder(object): """OpenStack API base limits view builder.""" @@ -79,7 +77,6 @@ class ViewBuilder(object): return limits def _build_used_limits(self, request, quotas, filtered_limits): - self._check_requested_project_scope(request) quota_map = { 'totalRAMUsed': 'ram', 'totalCoresUsed': 'cores', @@ -94,13 +91,3 @@ class ViewBuilder(object): used_limits[display_name] = quotas[key]['in_use'] return used_limits - - def _check_requested_project_scope(self, request): - if 'tenant_id' in request.GET: - context = request.environ['nova.context'] - tenant_id = request.GET.get('tenant_id') - target = { - 'project_id': tenant_id, - 'user_id': context.user_id - } - context.can(ul_policies.BASE_POLICY_NAME, target) diff --git a/nova/policies/__init__.py b/nova/policies/__init__.py index 5857abc58485..91362d26064f 100644 --- a/nova/policies/__init__.py +++ b/nova/policies/__init__.py @@ -66,7 +66,6 @@ from nova.policies import shelve from nova.policies import simple_tenant_usage from nova.policies import suspend_server from nova.policies import tenant_networks -from nova.policies import used_limits from nova.policies import volumes from nova.policies import volumes_attachments @@ -126,7 +125,6 @@ def list_rules(): simple_tenant_usage.list_rules(), suspend_server.list_rules(), tenant_networks.list_rules(), - used_limits.list_rules(), volumes.list_rules(), volumes_attachments.list_rules() ) diff --git a/nova/policies/limits.py b/nova/policies/limits.py index a9ee51f0a22c..cbdf01e623cb 100644 --- a/nova/policies/limits.py +++ b/nova/policies/limits.py @@ -19,13 +19,30 @@ from nova.policies import base BASE_POLICY_NAME = 'os_compute_api:limits' +USED_LIMIT_POLICY_NAME = 'os_compute_api:os-used-limits' limits_policies = [ policy.DocumentedRuleDefault( BASE_POLICY_NAME, base.RULE_ANY, - "Show rate and absolute limits for the project", + "Show rate and absolute limits for the current user project", + [ + { + 'method': 'GET', + 'path': '/limits' + } + ]), + # TODO(aunnam): Remove this rule after we separate the scope check from + # policies, as this is only checking the scope. + policy.DocumentedRuleDefault( + USED_LIMIT_POLICY_NAME, + base.RULE_ADMIN_API, + """Show rate and absolute limits for the project. + +This policy only checks if the user has access to the requested +project limits. And this check is performed only after the check +os_compute_api:limits passes""", [ { 'method': 'GET', diff --git a/nova/policies/used_limits.py b/nova/policies/used_limits.py deleted file mode 100644 index 80cf4d30168b..000000000000 --- a/nova/policies/used_limits.py +++ /dev/null @@ -1,45 +0,0 @@ -# Copyright 2016 Cloudbase Solutions Srl -# 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_policy import policy - -from nova.policies import base - - -BASE_POLICY_NAME = 'os_compute_api:os-used-limits' - - -used_limits_policies = [ - # TODO(aunnam): Remove this rule after we separate the scope check from - # policies, as this is only checking the scope. - policy.DocumentedRuleDefault( - BASE_POLICY_NAME, - base.RULE_ADMIN_API, - """Show rate and absolute limits for the project. - -This policy only checks if the user has access to the requested -project limits. And this check is performed only after the check -os_compute_api:limits passes""", - [ - { - 'method': 'GET', - 'path': '/limits' - } - ]), -] - - -def list_rules(): - return used_limits_policies diff --git a/nova/tests/unit/api/openstack/compute/test_limits.py b/nova/tests/unit/api/openstack/compute/test_limits.py index 7bff0d18230d..9405f740645f 100644 --- a/nova/tests/unit/api/openstack/compute/test_limits.py +++ b/nova/tests/unit/api/openstack/compute/test_limits.py @@ -28,7 +28,7 @@ from nova.api.openstack.compute import views from nova.api.openstack import wsgi import nova.context from nova import exception -from nova.policies import used_limits as ul_policies +from nova.policies import limits as l_policies from nova import quota from nova import test from nova.tests.unit.api.openstack import fakes @@ -214,7 +214,7 @@ class LimitsControllerTestV21(BaseLimitTestSuite): return_value={}) as mock_get_quotas: fake_req.get_response(self.controller) self.assertEqual(2, self.mock_can.call_count) - self.mock_can.assert_called_with(ul_policies.BASE_POLICY_NAME, + self.mock_can.assert_called_with(l_policies.USED_LIMIT_POLICY_NAME, target) mock_get_quotas.assert_called_once_with(context, tenant_id, usages=True) @@ -349,9 +349,6 @@ class LimitsViewBuilderTest(test.NoDBTestCase): self.view_builder = views.limits.ViewBuilder() self.req = fakes.HTTPRequest.blank('/?tenant_id=None') self.rate_limits = [] - patcher = self.mock_can = mock.patch('nova.context.RequestContext.can') - self.mock_can = patcher.start() - self.addCleanup(patcher.stop) self.absolute_limits = {"metadata_items": {'limit': 1, 'in_use': 1}, "injected_files": {'limit': 5, 'in_use': 1}, "injected_file_content_bytes": @@ -376,27 +373,6 @@ class LimitsViewBuilderTest(test.NoDBTestCase): output = self.view_builder.build(self.req, quotas) self.assertThat(output, matchers.DictMatches(expected_limits)) - def test_non_admin_cannot_fetch_used_limits_for_any_other_project(self): - project_id = "123456" - user_id = "A1234" - tenant_id = "abcd" - target = { - "project_id": tenant_id, - "user_id": user_id - } - req = fakes.HTTPRequest.blank('/?tenant_id=%s' % tenant_id) - context = nova.context.RequestContext(user_id, project_id) - req.environ["nova.context"] = context - - self.mock_can.side_effect = exception.PolicyNotAuthorized( - action="os_compute_api:os-used-limits") - self.assertRaises(exception.PolicyNotAuthorized, - self.view_builder.build, - req, self.absolute_limits) - - self.mock_can.assert_called_with(ul_policies.BASE_POLICY_NAME, - target) - class LimitsPolicyEnforcementV21(test.NoDBTestCase): @@ -415,6 +391,18 @@ class LimitsPolicyEnforcementV21(test.NoDBTestCase): "Policy doesn't allow %s to be performed." % rule_name, exc.format_message()) + def test_non_admin_cannot_fetch_used_limits_for_any_other_project(self): + project_id = "123456" + user_id = "A1234" + tenant_id = "abcd" + req = fakes.HTTPRequest.blank('/?tenant_id=%s' % tenant_id) + context = nova.context.RequestContext(user_id, project_id) + req.environ["nova.context"] = context + + self.assertRaises(exception.PolicyNotAuthorized, + self.controller.index, + req) + class LimitsControllerTestV236(BaseLimitTestSuite):