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

8 files changed, 24 insertions, 19 deletions

diff --git a/nova/api/openstack/compute/server_groups.py b/nova/api/openstack/compute/server_groups.py
index c1fd2634be..5571cce678 100644
--- a/nova/api/openstack/compute/server_groups.py
+++ b/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:
diff --git a/nova/tests/unit/policies/base.py b/nova/tests/unit/policies/base.py
index 756769a146..61c3dd0b1e 100644
--- a/nova/tests/unit/policies/base.py
+++ b/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." %
diff --git a/nova/tests/unit/policies/test_hypervisors.py b/nova/tests/unit/policies/test_hypervisors.py
index abd5a50da1..4bd054ea26 100644
--- a/nova/tests/unit/policies/test_hypervisors.py
+++ b/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 = [
diff --git a/nova/tests/unit/policies/test_instance_usage_audit_log.py b/nova/tests/unit/policies/test_instance_usage_audit_log.py
index 5611644b76..3ab4e6138f 100644
--- a/nova/tests/unit/policies/test_instance_usage_audit_log.py
+++ b/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,
diff --git a/nova/tests/unit/policies/test_limits.py b/nova/tests/unit/policies/test_limits.py
index 8ff7667936..8760610002 100644
--- a/nova/tests/unit/policies/test_limits.py
+++ b/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 = [
diff --git a/nova/tests/unit/policies/test_lock_server.py b/nova/tests/unit/policies/test_lock_server.py
index 4925214022..21274481b1 100644
--- a/nova/tests/unit/policies/test_lock_server.py
+++ b/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]
diff --git a/nova/tests/unit/policies/test_pause_server.py b/nova/tests/unit/policies/test_pause_server.py
index e279206612..fa2c37d0b6 100644
--- a/nova/tests/unit/policies/test_pause_server.py
+++ b/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]
diff --git a/nova/tests/unit/policies/test_suspend_server.py b/nova/tests/unit/policies/test_suspend_server.py
index 07b073f197..9a65321582 100644
--- a/nova/tests/unit/policies/test_suspend_server.py
+++ b/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"})