Add new default roles in remaining servers policies

This adds new defaults roles in remaining servers
as PROJECT_ADMIN with TODO to move the to SYSTEM_ADMIN.

Also add tests to simulates the future where we drop the deprecation
fall back in the policy by overriding the rules with a version where
there are no deprecated rule options. Operators can do the same by
adding overrides in their policy files that match the default but
stop the rule deprecation fallback from happening.

Partial implement blueprint policy-defaults-refresh

Change-Id: I72b48eb78fde29083bc23ddb25459f6c41590c37

6 files changed, 55 insertions, 53 deletions

diff --git a/nova/policies/base.py b/nova/policies/base.py
index c54848e3b5..32b18291e0 100644
--- a/nova/policies/base.py
+++ b/nova/policies/base.py
@@ -47,6 +47,7 @@ in nova 23.0.0 release.
 # oslo.policy's RuleDefault objects.
 SYSTEM_ADMIN = 'rule:system_admin_api'
 SYSTEM_READER = 'rule:system_reader_api'
+PROJECT_ADMIN = 'rule:project_admin_api'
 PROJECT_MEMBER = 'rule:project_member_api'
 PROJECT_READER = 'rule:project_reader_api'
 PROJECT_MEMBER_OR_SYSTEM_ADMIN = 'rule:system_admin_or_owner'
@@ -113,6 +114,13 @@ rules = [
         deprecated_reason=DEPRECATED_REASON,
         deprecated_since='21.0.0'),
     policy.RuleDefault(
+        "project_admin_api",
+        "role:admin and project_id:%(project_id)s",
+        "Default rule for Project level admin APIs.",
+        deprecated_rule=DEPRECATED_ADMIN_POLICY,
+        deprecated_reason=DEPRECATED_REASON,
+        deprecated_since='21.0.0'),
+    policy.RuleDefault(
         "project_member_api",
         "role:member and project_id:%(project_id)s",
         "Default rule for Project level non admin APIs.",
diff --git a/nova/policies/servers.py b/nova/policies/servers.py
index 0b138aab9e..811030bdb7 100644
--- a/nova/policies/servers.py
+++ b/nova/policies/servers.py
@@ -177,13 +177,17 @@ allow everyone.
         scope_types=['project']),
     policy.DocumentedRuleDefault(
         name=SERVERS % 'create:forced_host',
-        # TODO(gmann): Do we need PROJECT_ADMIN for this?
-        # PROJECT_ADMIN is not used in policies yet and this
-        # can be first one. This policy is checked after 'create' policy
-        # which is PROJECT_MEMBER so making this as SYSTEM_ADMIN
-        # does not make sense as system scoped role cannot
-        # pass the 'create' policy. opinion ?
-        check_str=base.RULE_ADMIN_API,
+        # TODO(gmann): We need to make it SYSTEM_ADMIN.
+        # PROJECT_ADMIN is added for now because create server
+        # policy is project scoped and there is no way to
+        # pass the project_id in request body for system scoped
+        # roles so that create server for other project with force host.
+        # To achieve that, we need to update the create server API to
+        # accept the project_id for whom the server needs to be created
+        # and then change the scope of this policy to system-only
+        # Because that is API change it needs to be done with new
+        # microversion.
+        check_str=base.PROJECT_ADMIN,
         description="""
 Create a server on the specified host and/or node.
 
@@ -252,7 +256,17 @@ validated by the scheduler filters unlike the
         scope_types=['project']),
     policy.DocumentedRuleDefault(
         name=ZERO_DISK_FLAVOR,
-        check_str=base.RULE_ADMIN_API,
+        # TODO(gmann): We need to make it SYSTEM_ADMIN.
+        # PROJECT_ADMIN is added for now because create server
+        # policy is project scoped and there is no way to
+        # pass the project_id in request body for system scoped
+        # roles so that create server for other project with zero disk flavor.
+        # To achieve that, we need to update the create server API to
+        # accept the project_id for whom the server needs to be created
+        # and then change the scope of this policy to system-only
+        # Because that is API change it needs to be done with new
+        # microversion.
+        check_str=base.PROJECT_ADMIN,
         description="""
 This rule controls the compute API validation behavior of creating a server
 with a flavor that has 0 disk, indicating the server should be volume-backed.
@@ -277,7 +291,17 @@ https://bugs.launchpad.net/nova/+bug/1739646 for details.
         scope_types=['system', 'project']),
     policy.DocumentedRuleDefault(
         name=NETWORK_ATTACH_EXTERNAL,
-        check_str='is_admin:True',
+        # TODO(gmann): We need to make it SYSTEM_ADMIN.
+        # PROJECT_ADMIN is added for now because create server
+        # policy is project scoped and there is no way to
+        # pass the project_id in request body for system scoped
+        # roles so that create server for other project or attach the
+        # external network. To achieve that, we need to update the
+        # create server API to accept the project_id for whom the
+        # server needs to be created and then change the scope of this
+        # policy to system-only Because that is API change it needs to
+        # be done with new microversion.
+        check_str=base.PROJECT_ADMIN,
         description="Attach an unshared external network to a server",
         operations=[
             # Create a server with a requested network or port.
diff --git a/nova/tests/unit/fake_policy.py b/nova/tests/unit/fake_policy.py
index a33aa5b87a..1af4fe2cf0 100644
--- a/nova/tests/unit/fake_policy.py
+++ b/nova/tests/unit/fake_policy.py
@@ -17,9 +17,11 @@ policy_data = """
 {
     "context_is_admin": "role:admin or role:administrator",
 
+    "network:attach_external_network": "",
     "os_compute_api:servers:create": "",
     "os_compute_api:servers:create:attach_volume": "",
     "os_compute_api:servers:create:attach_network": "",
+    "os_compute_api:servers:create:forced_host": "",
     "os_compute_api:servers:create:trusted_certs": "",
     "os_compute_api:servers:create_image": "",
     "os_compute_api:servers:create_image:allow_volume_backed": "",
diff --git a/nova/tests/unit/network/test_neutron.py b/nova/tests/unit/network/test_neutron.py
index 92547abdc4..3e6dc6417b 100644
--- a/nova/tests/unit/network/test_neutron.py
+++ b/nova/tests/unit/network/test_neutron.py
@@ -1461,6 +1461,8 @@ class TestAPI(TestAPIBase):
         """Only one network is available, it's external, and the client
            is unauthorized to use it.
         """
+        rules = {'network:attach_external_network': 'is_admin:True'}
+        policy.set_rules(oslo_policy.Rules.from_dict(rules))
         mocked_client = mock.create_autospec(client.Client)
         mock_get_client.return_value = mocked_client
         self.instance = fake_instance.fake_instance_obj(self.context,
@@ -7025,6 +7027,8 @@ class TestAllocateForInstance(test.NoDBTestCase):
             requested_networks, ordered_networks)
 
     def test_validate_requested_network_ids_raises_forbidden(self):
+        rules = {'network:attach_external_network': 'is_admin:True'}
+        policy.set_rules(oslo_policy.Rules.from_dict(rules))
         self._assert_validate_requested_network_ids_raises(
             exception.ExternalNetworkAttachForbidden,
             [{'id': "net1", 'router:external': True, 'shared': False}])
diff --git a/nova/tests/unit/policies/test_servers.py b/nova/tests/unit/policies/test_servers.py
index 2e261956db..6d62245e39 100644
--- a/nova/tests/unit/policies/test_servers.py
+++ b/nova/tests/unit/policies/test_servers.py
@@ -49,6 +49,7 @@ class ServersPolicyTest(base.BasePolicyTest):
         self.rule_attach_network = policies.SERVERS % 'create:attach_network'
         self.rule_attach_volume = policies.SERVERS % 'create:attach_volume'
         self.rule_requested_destination = policies.REQUESTED_DESTINATION
+        self.rule_forced_host = policies.SERVERS % 'create:forced_host'
 
         self.req = fakes.HTTPRequest.blank('')
         user_id = self.req.environ['nova.context'].user_id
@@ -422,10 +423,9 @@ class ServersPolicyTest(base.BasePolicyTest):
         self.policy.set_rules({rule: "@"}, overwrite=False)
         mock_create.return_value = ([self.instance], '')
         mock_az.return_value = ('test', 'host', None)
-        rule_name = policies.SERVERS % 'create:forced_host'
-        self.common_policy_check(self.admin_authorized_contexts,
-                                 self.admin_unauthorized_contexts,
-                                 rule_name,
+        self.common_policy_check(self.project_admin_authorized_contexts,
+                                 self.project_admin_unauthorized_contexts,
+                                 self.rule_forced_host,
                                  self.controller.create,
                                  self.req, body=self.body)
 
@@ -1325,6 +1325,7 @@ class ServersScopeTypePolicyTest(ServersPolicyTest):
         self.rule_attach_network = None
         self.rule_attach_volume = None
         self.rule_requested_destination = None
+        self.rule_forced_host = None
 
         # Check that system admin is able to create server with host request
         # and get server extended attributes or host status.
@@ -1385,29 +1386,6 @@ class ServersScopeTypePolicyTest(ServersPolicyTest):
             self.other_project_reader_context
         ]
 
-    @mock.patch('nova.compute.api.API.create')
-    @mock.patch('nova.compute.api.API.parse_availability_zone')
-    def test_create_forced_host_server_policy(self, mock_az, mock_create):
-        # TODO(gmann): Uncomment this once we figure out the right defaults
-        # for 'create:forced_host'
-        # The 'create' policy is checked before 'create:forced_host' so
-        # we need to allow 'create' policy for everyone. Also skip the
-        # error message assertion because for
-        # system scoped unauth context 'create' policy fail and for project
-        # scoped unauth context 'create:forced_host' fail.
-
-        # rule = policies.SERVERS % 'create'
-        # self.policy.set_rules({rule: "@"})
-        # mock_create.return_value = ([self.instance], '')
-        # mock_az.return_value = ('test', 'host', None)
-        # rule_name = policies.SERVERS % 'create:forced_host'
-        # self.common_policy_check(self.admin_authorized_contexts,
-        #                         self.admin_unauthorized_contexts,
-        #                         None,
-        #                         self.controller.create,
-        #                         self.req, body=self.body)
-        pass
-
 
 class ServersNoLegacyPolicyTest(ServersScopeTypePolicyTest):
     """Test Servers APIs policies with system scope enabled,
@@ -1461,21 +1439,6 @@ class ServersNoLegacyPolicyTest(ServersScopeTypePolicyTest):
             self.project_foo_context,
             self.system_foo_context
         ]
-        # Check that sustem reader is able to list the server
-        # for all projects.
-        # self.system_reader_authorized_contexts = [
-        #    self.system_admin_context,
-        #    self.project_admin_context, self.system_member_context,
-        #    self.system_reader_context]
-        # Check that non-system reader is not able to list the server
-        # for all projects.
-        # self.system_reader_unauthorized_contexts = [
-        #    self.legacy_admin_context, self.system_foo_context,
-        #    self.project_member_context,
-        #    self.project_reader_context, self.project_foo_context,
-        #    self.other_project_member_context,
-        #    self.other_project_reader_context
-        # ]
         # Check if project member can create the server.
         self.project_member_authorized_contexts = [
             self.legacy_admin_context, self.project_admin_context,
diff --git a/nova/tests/unit/test_policy.py b/nova/tests/unit/test_policy.py
index ee4a4b87bc..6f41b6343a 100644
--- a/nova/tests/unit/test_policy.py
+++ b/nova/tests/unit/test_policy.py
@@ -528,8 +528,9 @@ class RealRolePolicyTestCase(test.NoDBTestCase):
         special_rules = ('admin_api', 'admin_or_owner', 'context_is_admin',
                          'os_compute_api:os-quota-class-sets:show',
                          'system_admin_api', 'system_reader_api',
-                         'project_member_api', 'project_reader_api',
-                         'system_admin_or_owner', 'system_or_project_reader')
+                         'project_admin_api', 'project_member_api',
+                         'project_reader_api', 'system_admin_or_owner',
+                         'system_or_project_reader')
         result = set(rules.keys()) - set(self.admin_only_rules +
             self.admin_or_owner_rules +
             self.allow_all_rules + self.system_reader_rules +