Fix follow up comments on policy work

There are few typo or other comments
in policy defaults refresh BP work which
left to do in followup patch.

This is follow up patch to fix comments.
- https://review.opendev.org/#/c/715760/14/nova/tests/unit/policies/test_limits.py@69
- https://review.opendev.org/#/c/717554/3/nova/tests/unit/policies/test_suspend_server.py@95
- https://review.opendev.org/#/c/717176/7/nova/api/openstack/compute/server_groups.py@127
- https://review.opendev.org/#/c/662968/9/nova/tests/unit/policies/base.py@152

Partial implement blueprint policy-defaults-refresh

Change-Id: I88da1494788ff8cfa1b88652dcac7536bc6b2c51

nova/api/openstack/compute/server_groups.py

@@ -123,10 +123,10 @@ class ServerGroupController(wsgi.Controller):
         context = req.environ['nova.context']
         try:
             sg = objects.InstanceGroup.get_by_uuid(context, id)
-            context.can(sg_policies.POLICY_ROOT % 'show',
-                        target={'project_id': sg.project_id})
         except nova.exception.InstanceGroupNotFound as e:
             raise webob.exc.HTTPNotFound(explanation=e.format_message())
+        context.can(sg_policies.POLICY_ROOT % 'show',
+                    target={'project_id': sg.project_id})
         return {'server_group': self._format_server_group(context, sg, req)}
 
     @wsgi.response(204)
@@ -136,10 +136,10 @@ class ServerGroupController(wsgi.Controller):
         context = req.environ['nova.context']
         try:
             sg = objects.InstanceGroup.get_by_uuid(context, id)
-            context.can(sg_policies.POLICY_ROOT % 'delete',
-                        target={'project_id': sg.project_id})
         except nova.exception.InstanceGroupNotFound as e:
             raise webob.exc.HTTPNotFound(explanation=e.format_message())
+        context.can(sg_policies.POLICY_ROOT % 'delete',
+                    target={'project_id': sg.project_id})
         try:
             sg.destroy()
         except nova.exception.InstanceGroupNotFound as e:

nova/tests/unit/policies/base.py

@@ -149,6 +149,11 @@ class BasePolicyTest(test.TestCase):
         def ensure_raises(req, *args, **kwargs):
             exc = self.assertRaises(
                 exception.PolicyNotAuthorized, func, req, *arg, **kwarg)
+            # NOTE(gmann): In case of multi-policy APIs, PolicyNotAuthorized
+            # exception can be raised from either of the policy so checking
+            # the error message, which includes the rule name, can mismatch.
+            # Tests verifying the multi policy can pass rule_name as None
+            # to skip the error message assert.
             if rule_name is not None:
                 self.assertEqual(
                     "Policy doesn't allow %s to be performed." %

nova/tests/unit/policies/test_hypervisors.py

@@ -40,7 +40,7 @@ class HypervisorsPolicyTest(base.BasePolicyTest):
         # perform operations on hypervisors.
         # NOTE(gmann): Until old default rule which is admin_api is
         # deprecated and not removed, project admin and legacy admin
-        # will be able to read the agent data. This make sure that existing
+        # will be able to get hypervisors. This make sure that existing
         # tokens will keep working even we have changed this policy defaults
         # to reader role.
         self.reader_authorized_contexts = [

nova/tests/unit/policies/test_instance_usage_audit_log.py

@@ -38,9 +38,9 @@ class InstanceUsageAuditLogPolicyTest(base.BasePolicyTest):
         # Check that admin is able to get instance usage audit log.
         # NOTE(gmann): Until old default rule which is admin_api is
         # deprecated and not removed, project admin and legacy admin
-        # will be able to read the agent data. This make sure that existing
-        # tokens will keep working even we have changed this policy defaults
-        # to reader role.
+        # will be able to get instance usage audit log. This make sure
+        # that existing tokens will keep working even we have changed
+        # this policy defaults to reader role.
         self.reader_authorized_contexts = [
             self.legacy_admin_context, self.system_admin_context,
             self.project_admin_context, self.system_member_context,

nova/tests/unit/policies/test_limits.py

@@ -66,7 +66,7 @@ class LimitsPolicyTest(base.BasePolicyTest):
         # Check that system reader is able to get other projects limit.
         # NOTE(gmann): Until old default rule which is admin_api is
         # deprecated and not removed, project admin and legacy admin
-        # will be able to read the agent data. This make sure that existing
+        # will be able to get limit. This make sure that existing
         # tokens will keep working even we have changed this policy defaults
         # to reader role.
         self.reader_authorized_contexts = [

nova/tests/unit/policies/test_lock_server.py

@@ -12,14 +12,14 @@
 
 import fixtures
 import mock
-from nova.policies import base as base_policy
-from nova.policies import lock_server as ls_policies
 from oslo_utils.fixture import uuidsentinel as uuids
 from oslo_utils import timeutils
 
 from nova.api.openstack.compute import lock_server
 from nova.compute import vm_states
 from nova import exception
+from nova.policies import base as base_policy
+from nova.policies import lock_server as ls_policies
 from nova.tests.unit.api.openstack import fakes
 from nova.tests.unit import fake_instance
 from nova.tests.unit.policies import base
@@ -49,7 +49,7 @@ class LockServerPolicyTest(base.BasePolicyTest):
         self.mock_get.return_value = self.instance
 
         # Check that admin or and server owner is able to lock/unlock
-        # the sevrer
+        # the server
         self.admin_or_owner_authorized_contexts = [
             self.legacy_admin_context, self.system_admin_context,
             self.project_admin_context, self.project_member_context,
@@ -157,7 +157,7 @@ class LockServerNoLegacyPolicyTest(LockServerScopeTypePolicyTest):
     def setUp(self):
         super(LockServerNoLegacyPolicyTest, self).setUp()
         # Check that system admin or and server owner is able to lock/unlock
-        # the sevrer
+        # the server
         self.admin_or_owner_authorized_contexts = [
             self.system_admin_context,
             self.project_admin_context, self.project_member_context]

nova/tests/unit/policies/test_pause_server.py

@@ -12,13 +12,13 @@
 
 import fixtures
 import mock
-from nova.policies import pause_server as ps_policies
 from oslo_utils.fixture import uuidsentinel as uuids
 from oslo_utils import timeutils
 
 from nova.api.openstack.compute import pause_server
 from nova.compute import vm_states
 from nova import exception
+from nova.policies import pause_server as ps_policies
 from nova.tests.unit.api.openstack import fakes
 from nova.tests.unit import fake_instance
 from nova.tests.unit.policies import base
@@ -48,7 +48,7 @@ class PauseServerPolicyTest(base.BasePolicyTest):
         self.mock_get.return_value = self.instance
 
         # Check that admin or and server owner is able to pause/unpause
-        # the sevrer
+        # the server
         self.admin_or_owner_authorized_contexts = [
             self.legacy_admin_context, self.system_admin_context,
             self.project_admin_context, self.project_member_context,
@@ -95,7 +95,7 @@ class PauseServerPolicyTest(base.BasePolicyTest):
             exc.format_message())
 
     @mock.patch('nova.compute.api.API.pause')
-    def test_pause_sevrer_overridden_policy_pass_with_same_user(
+    def test_pause_server_overridden_policy_pass_with_same_user(
         self, mock_pause):
         rule_name = ps_policies.POLICY_ROOT % 'pause'
         self.policy.set_rules({rule_name: "user_id:%(user_id)s"})
@@ -129,7 +129,7 @@ class PauseServerNoLegacyPolicyTest(PauseServerScopeTypePolicyTest):
     def setUp(self):
         super(PauseServerNoLegacyPolicyTest, self).setUp()
         # Check that system admin or server owner is able to pause/unpause
-        # the sevrer
+        # the server
         self.admin_or_owner_authorized_contexts = [
             self.system_admin_context,
             self.project_admin_context, self.project_member_context]

nova/tests/unit/policies/test_suspend_server.py

@@ -45,7 +45,7 @@ class SuspendServerPolicyTest(base.BasePolicyTest):
         self.mock_get.return_value = self.instance
 
         # Check that admin or and server owner is able to suspend/resume
-        # the sevrer
+        # the server
         self.admin_or_owner_authorized_contexts = [
             self.legacy_admin_context, self.system_admin_context,
             self.project_admin_context, self.project_member_context,
@@ -92,7 +92,7 @@ class SuspendServerPolicyTest(base.BasePolicyTest):
             exc.format_message())
 
     @mock.patch('nova.compute.api.API.suspend')
-    def test_suspend_sevrer_overridden_policy_pass_with_same_user(
+    def test_suspend_server_overridden_policy_pass_with_same_user(
         self, mock_suspend):
         rule_name = policies.POLICY_ROOT % 'suspend'
         self.policy.set_rules({rule_name: "user_id:%(user_id)s"})