Merge "Combine the limits policies in single place"

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()

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)

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()
     )

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',

nova/policies/used_limits.py

@@ -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

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):