Merge "Remove admin namespace throughout Patrole - Nova tests"
diff --git a/patrole_tempest_plugin/rbac_utils.py b/patrole_tempest_plugin/rbac_utils.py
index 4bfa7fe..d952014 100644
--- a/patrole_tempest_plugin/rbac_utils.py
+++ b/patrole_tempest_plugin/rbac_utils.py
@@ -14,7 +14,6 @@
 #    under the License.
 
 import sys
-import testtools
 import time
 
 from oslo_log import log as logging
@@ -137,10 +136,11 @@
         self.switch_role_history.setdefault(key, None)
 
         if self.switch_role_history[key] == toggle_rbac_role:
-            # If the test was skipped, then this is a legitimate use case,
-            # so do not throw an exception.
-            exc_value = sys.exc_info()[1]
-            if not isinstance(exc_value, testtools.TestCase.skipException):
+            # If an exception was thrown, like a skipException or otherwise,
+            # then this is a legitimate reason why `switch_role` was not
+            # called, so only raise an exception if no current exception is
+            # being handled.
+            if sys.exc_info()[0] is None:
                 self.switch_role_history[key] = False
                 error_message = '`toggle_rbac_role` must not be called with '\
                     'the same bool value twice. Make sure that you included '\
diff --git a/patrole_tempest_plugin/tests/api/volume/test_availability_zone_rbac.py b/patrole_tempest_plugin/tests/api/volume/test_availability_zone_rbac.py
deleted file mode 100644
index 95b5949..0000000
--- a/patrole_tempest_plugin/tests/api/volume/test_availability_zone_rbac.py
+++ /dev/null
@@ -1,37 +0,0 @@
-# Copyright 2017 AT&T Corporation.
-# 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 tempest import config
-from tempest.lib import decorators
-
-from patrole_tempest_plugin import rbac_rule_validation
-from patrole_tempest_plugin.tests.api.volume import rbac_base
-
-CONF = config.CONF
-
-
-class AvailabilityZoneRbacTest(rbac_base.BaseVolumeRbacTest):
-
-    @classmethod
-    def setup_clients(cls):
-        super(AvailabilityZoneRbacTest, cls).setup_clients()
-        cls.client = cls.availability_zone_client
-
-    @rbac_rule_validation.action(service="cinder",
-                                 rule="volume:availability_zone_list")
-    @decorators.idempotent_id('8cfd920c-4b6c-402d-b6e2-ede86bedc702')
-    def test_get_availability_zone_list(self):
-        self.rbac_utils.switch_role(self, toggle_rbac_role=True)
-        self.client.list_availability_zones()
diff --git a/patrole_tempest_plugin/tests/api/volume/test_extensions_rbac.py b/patrole_tempest_plugin/tests/api/volume/test_extensions_rbac.py
deleted file mode 100644
index 3304452..0000000
--- a/patrole_tempest_plugin/tests/api/volume/test_extensions_rbac.py
+++ /dev/null
@@ -1,36 +0,0 @@
-# Copyright 2017 AT&T Corporation.
-# 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 tempest import config
-from tempest.lib import decorators
-
-from patrole_tempest_plugin import rbac_rule_validation
-from patrole_tempest_plugin.tests.api.volume import rbac_base
-
-CONF = config.CONF
-
-
-class ExtensionsRbacTest(rbac_base.BaseVolumeRbacTest):
-
-    @rbac_rule_validation.action(service="cinder",
-                                 rule="volume:list_extensions")
-    @decorators.idempotent_id('7f2dcc41-e850-493f-a400-82db4e2b50c0')
-    def test_list_extensions(self):
-        self.rbac_utils.switch_role(self, toggle_rbac_role=True)
-        self.volumes_extension_client.list_extensions()
-
-
-class ExtensionsV3RbacTest(ExtensionsRbacTest):
-    _api_version = 3
diff --git a/patrole_tempest_plugin/tests/api/volume/test_snapshots_metadata_rbac.py b/patrole_tempest_plugin/tests/api/volume/test_snapshots_metadata_rbac.py
index bf48716..81cd854 100644
--- a/patrole_tempest_plugin/tests/api/volume/test_snapshots_metadata_rbac.py
+++ b/patrole_tempest_plugin/tests/api/volume/test_snapshots_metadata_rbac.py
@@ -57,7 +57,7 @@
         self._create_test_snapshot_metadata()
 
     @rbac_rule_validation.action(service="cinder",
-                                 rule="volume:get_volume_image_metadata")
+                                 rule="volume:get_snapshot_metadata")
     @decorators.idempotent_id('f6912bb1-62e6-483d-bcd0-e98c1641f4c3')
     def test_get_snapshot_metadata(self):
         # Create volume and snapshot metadata
diff --git a/patrole_tempest_plugin/tests/api/volume/test_volume_actions_rbac.py b/patrole_tempest_plugin/tests/api/volume/test_volume_actions_rbac.py
index e1f9430..70c73fc 100644
--- a/patrole_tempest_plugin/tests/api/volume/test_volume_actions_rbac.py
+++ b/patrole_tempest_plugin/tests/api/volume/test_volume_actions_rbac.py
@@ -73,7 +73,7 @@
     @test.attr(type="slow")
     @rbac_rule_validation.action(service="cinder", rule="volume:detach")
     @decorators.idempotent_id('5a042f6a-688b-42e6-a02e-fe5c47b89b07')
-    def test_detach_volume_to_instance(self):
+    def test_detach_volume_from_instance(self):
         # Attach the volume
         server = self._create_server()
         self._attach_volume(server)
diff --git a/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud.py b/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py
similarity index 97%
rename from patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud.py
rename to patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py
index 6bc9e4e..b98c39a 100644
--- a/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud.py
+++ b/patrole_tempest_plugin/tests/api/volume/test_volume_basic_crud_rbac.py
@@ -57,7 +57,7 @@
 
     @rbac_rule_validation.action(
         service="cinder",
-        rule="volume_extension:get_volumes_image_metadata")
+        rule="volume_extension:volume_image_metadata")
     @decorators.idempotent_id('3d48ca91-f02b-4616-a69d-4a8b296c8529')
     def test_volume_list_image_metadata(self):
         # Get a list of Volumes
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
index 38b5fea..78d8e66 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -274,6 +274,11 @@
     def test_invalid_policy_rule_throws_parsing_exception(
             self, mock_rbac_policy_parser):
         """Test that invalid policy action causes test to be skipped."""
+        CONF.set_override('strict_policy_check', True, group='rbac',
+                          enforce_type=True)
+        self.addCleanup(CONF.clear_override, 'strict_policy_check',
+                        group='rbac')
+
         mock_rbac_policy_parser.RbacPolicyParser.return_value.allowed.\
             side_effect = rbac_exceptions.RbacParsingException
 
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_utils.py b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
index feccfe5..057ce20 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_utils.py
@@ -195,23 +195,38 @@
                        autospec=True, return_value=False)
     @mock.patch.object(rbac_utils, 'LOG', autospec=True)
     @mock.patch.object(rbac_utils, 'sys', autospec=True)
-    def test_rbac_utils_switch_roles_with_skip_exception(self, mock_sys,
-                                                         mock_log, _):
+    def test_rbac_utils_switch_roles_with_unhandled_exception(self, mock_sys,
+                                                              mock_log, _):
+        """Test whether throwing an unhandled exception doesn't throw error.
+
+        If a skip exception, say, is thrown then this means that switch_role is
+        never called within the test function. But if an unhandled exception
+        or skip exception is thrown, then this should not result in an error
+        being raised.
+        """
         self._mock_list_user_roles_on_project('member_id')
 
-        mock_skip_exception = mock.Mock(spec=testtools.TestCase.skipException)
-        mock_sys.exc_info.return_value = [None, mock_skip_exception]
+        # Skip exception is an example of a legitimate case where `switch_role`
+        # is thrown. AttributeError, on the other hand, is an example of an
+        # unexpected exception being thrown that should be allowed to bubble
+        # up, rather than being obfuscated by `switch_role` error being thrown
+        # instead.
+        unhandled_exceptions = [testtools.TestCase.skipException,
+                                AttributeError]
 
-        # Ordinarily switching to the same role would result in an error,
-        # but because the skipException is thrown before the test finishes,
-        # this is not treated as a failure.
-        self.rbac_utils.switch_role(self.mock_test_obj, False)
-        self.rbac_utils.switch_role(self.mock_test_obj, False)
-        mock_log.error.assert_not_called()
+        for unhandled_exception in unhandled_exceptions:
+            mock_sys.exc_info.return_value = [unhandled_exception]
 
-        self.rbac_utils.switch_role(self.mock_test_obj, True)
-        self.rbac_utils.switch_role(self.mock_test_obj, True)
-        mock_log.error.assert_not_called()
+            # Ordinarily switching to the same role would result in an error,
+            # but because the skipException is thrown before the test finishes,
+            # this is not treated as a failure.
+            self.rbac_utils.switch_role(self.mock_test_obj, False)
+            self.rbac_utils.switch_role(self.mock_test_obj, False)
+            mock_log.error.assert_not_called()
+
+            self.rbac_utils.switch_role(self.mock_test_obj, True)
+            self.rbac_utils.switch_role(self.mock_test_obj, True)
+            mock_log.error.assert_not_called()
 
     @mock.patch.object(rbac_utils.rbac_utils, '_clear_user_roles',
                        autospec=True, return_value=False)