Merge "Add new default roles in os-flavor-access policies"

nova/policies/flavor_access.py

@@ -22,11 +22,28 @@ from nova.policies import base
 BASE_POLICY_NAME = 'os_compute_api:os-flavor-access'
 POLICY_ROOT = 'os_compute_api:os-flavor-access:%s'
 
+# NOTE(gmann): Deprecating this policy explicitly as old defaults
+# admin or owner is not suitable for that which should be admin (Bug#1867840)
+# but changing that will break old deployment so let's keep supporting
+# the old default also and new default can be SYSTEM_READER
+# SYSTEM_READER rule in base class is defined with the deprecated rule of admin
+# not admin or owner which is the main reason that we need to explicitly
+# deprecate this policy here.
+DEPRECATED_FLAVOR_ACCESS_POLICY = policy.DeprecatedRule(
+    BASE_POLICY_NAME,
+    base.RULE_ADMIN_OR_OWNER,
+)
+
+DEPRECATED_REASON = """
+Nova API policies are introducing new default roles with scope_type
+capabilities. Old policies are deprecated and silently going to be ignored
+in nova 23.0.0 release.
+"""
 
 flavor_access_policies = [
     policy.DocumentedRuleDefault(
         name=POLICY_ROOT % 'add_tenant_access',
-        check_str=base.RULE_ADMIN_API,
+        check_str=base.SYSTEM_ADMIN,
         description="Add flavor access to a tenant",
         operations=[
             {
@@ -37,7 +54,7 @@ flavor_access_policies = [
         scope_types=['system']),
     policy.DocumentedRuleDefault(
         name=POLICY_ROOT % 'remove_tenant_access',
-        check_str=base.RULE_ADMIN_API,
+        check_str=base.SYSTEM_ADMIN,
         description="Remove flavor access from a tenant",
         operations=[
             {
@@ -48,7 +65,7 @@ flavor_access_policies = [
         scope_types=['system']),
     policy.DocumentedRuleDefault(
         name=BASE_POLICY_NAME,
-        check_str=base.RULE_ADMIN_OR_OWNER,
+        check_str=base.SYSTEM_READER,
         description="""List flavor access information
 
 Allows access to the full list of tenants that have access
@@ -60,13 +77,10 @@ to a flavor via an os-flavor-access API.
                 'path': '/flavors/{flavor_id}/os-flavor-access'
             },
         ],
-        # NOTE(gmann): This policy is admin_or_owner by default but allowed
-        # for everyone, bug#1867840. There can be multiple project with access
-        # to specific flavorso we cannot say there is single owner of flavor.
-        # Only admin should be able to list the projects having access to any
-        # flavor. We should change this policy defaults to admin only. I am
-        # seeting scope as 'system' only and new defaults can be SYSTEM_ADMIN.
-        scope_types=['system']),
+        scope_types=['system'],
+        deprecated_rule=DEPRECATED_FLAVOR_ACCESS_POLICY,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since='21.0.0'),
 ]
 
 

nova/tests/unit/fake_policy.py

@@ -45,6 +45,7 @@ policy_data = """
     "os_compute_api:extensions": "",
     "os_compute_api:os-flavor-access:remove_tenant_access": "",
     "os_compute_api:os-flavor-access:add_tenant_access": "",
+    "os_compute_api:os-flavor-access": "",
     "os_compute_api:os-flavor-extra-specs:index": "",
     "os_compute_api:os-flavor-extra-specs:show": "",
     "os_compute_api:os-flavor-manage:create": "",

nova/tests/unit/policies/test_flavor_access.py

@@ -15,6 +15,7 @@ import mock
 from oslo_utils.fixture import uuidsentinel as uuids
 
 from nova.api.openstack.compute import flavor_access
+from nova.policies import base as base_policy
 from nova.policies import flavor_access as fa_policy
 from nova.tests.unit.api.openstack import fakes
 from nova.tests.unit import fake_flavor
@@ -64,7 +65,7 @@ class FlavorAccessPolicyTest(base.BasePolicyTest):
 
         # Check that everyone is able to list flavor access
         # information which is nothing but bug#1867840.
-        self.admin_or_owner_authorized_contexts = [
+        self.reader_authorized_contexts = [
             self.legacy_admin_context, self.system_admin_context,
             self.project_admin_context, self.project_member_context,
             self.project_reader_context, self.project_foo_context,
@@ -72,13 +73,13 @@ class FlavorAccessPolicyTest(base.BasePolicyTest):
             self.system_foo_context,
             self.other_project_member_context]
 
-        self.admin_or_owner_unauthorized_contexts = [
+        self.reader_unauthorized_contexts = [
         ]
 
     def test_list_flavor_access_policy(self):
         rule_name = fa_policy.BASE_POLICY_NAME
-        self.common_policy_check(self.admin_or_owner_authorized_contexts,
-                                 self.admin_or_owner_unauthorized_contexts,
+        self.common_policy_check(self.reader_authorized_contexts,
+                                 self.reader_unauthorized_contexts,
                                  rule_name, self.controller_index.index,
                                  self.req, '1')
 
@@ -134,13 +135,59 @@ class FlavorAccessScopeTypePolicyTest(FlavorAccessPolicyTest):
 
         # Check that system user is able to list flavor access
         # information.
-        self.admin_or_owner_authorized_contexts = [
+        self.reader_authorized_contexts = [
             self.system_admin_context,
             self.system_member_context, self.system_reader_context,
             self.system_foo_context]
         # Check that non-system is not able to list flavor access
         # information.
-        self.admin_or_owner_unauthorized_contexts = [
+        self.reader_unauthorized_contexts = [
             self.legacy_admin_context, self.other_project_member_context,
             self.project_admin_context, self.project_member_context,
             self.project_reader_context, self.project_foo_context]
+
+
+class FlavorAccessNoLegacyPolicyTest(FlavorAccessPolicyTest):
+    """Test FlavorAccess APIs policies with system scope enabled,
+    and no more deprecated rules that allow the legacy admin API to
+    access system_redear APIs.
+    """
+    without_deprecated_rules = True
+    rules_without_deprecation = {
+        fa_policy.POLICY_ROOT % "add_tenant_access":
+            base_policy.SYSTEM_ADMIN,
+        fa_policy.POLICY_ROOT % "remove_tenant_access":
+            base_policy.SYSTEM_ADMIN,
+        fa_policy.BASE_POLICY_NAME:
+            base_policy.SYSTEM_READER}
+
+    def setUp(self):
+        super(FlavorAccessNoLegacyPolicyTest, self).setUp()
+        self.flags(enforce_scope=True, group="oslo_policy")
+
+        # Check that system admin is able to add/remove flavor access
+        # to a tenant.
+        self.admin_authorized_contexts = [
+            self.system_admin_context]
+        # Check that non-system-admin is not able to add/remove flavor access
+        # to a tenant.
+        self.admin_unauthorized_contexts = [
+            self.legacy_admin_context, self.system_member_context,
+            self.system_reader_context, self.project_admin_context,
+            self.system_foo_context, self.project_member_context,
+            self.other_project_member_context,
+            self.project_foo_context, self.project_reader_context
+        ]
+
+        # Check that system reader is able to list flavor access
+        # information.
+        self.reader_authorized_contexts = [
+            self.system_admin_context,
+            self.system_member_context, self.system_reader_context]
+        # Check that non-system-reader is not able to list flavor access
+        # information.
+        self.reader_unauthorized_contexts = [
+            self.legacy_admin_context, self.other_project_member_context,
+            self.project_admin_context, self.project_member_context,
+            self.project_reader_context, self.project_foo_context,
+            self.system_foo_context]