Merge "Adds meaningful exceptions for missing attributes"
diff --git a/patrole_tempest_plugin/rbac_exceptions.py b/patrole_tempest_plugin/rbac_exceptions.py
index 5ccb216..5ee65ae 100644
--- a/patrole_tempest_plugin/rbac_exceptions.py
+++ b/patrole_tempest_plugin/rbac_exceptions.py
@@ -16,8 +16,25 @@
from tempest.lib import exceptions
-class RbacActionFailed(exceptions.ClientRestClientException):
- message = "Rbac action failed"
+class RbacConflictingPolicies(exceptions.TempestException):
+ message = ("Conflicting policies preventing this action from being "
+ "performed.")
+
+
+class RbacMalformedResponse(exceptions.TempestException):
+ message = ("The response body is missing the expected %(attribute)s due "
+ "to policy enforcement failure.")
+
+ def __init__(self, empty=False, extra_attr=False, **kwargs):
+ if empty:
+ self.message = ("The response body is empty due to policy "
+ "enforcement failure.")
+ kwargs = {}
+ if extra_attr:
+ self.message = ("The response body contained an unexpected "
+ "attribute due to policy enforcement failure.")
+ kwargs = {}
+ super(RbacMalformedResponse, self).__init__(**kwargs)
class RbacResourceSetupFailed(exceptions.TempestException):
diff --git a/patrole_tempest_plugin/rbac_rule_validation.py b/patrole_tempest_plugin/rbac_rule_validation.py
index da27a20..c7bd38b 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -101,7 +101,9 @@
LOG.error(msg)
raise exceptions.NotFound(
"%s RbacInvalidService was: %s" % (msg, e))
- except (expected_exception, rbac_exceptions.RbacActionFailed) as e:
+ except (expected_exception,
+ rbac_exceptions.RbacConflictingPolicies,
+ rbac_exceptions.RbacMalformedResponse) as e:
if irregular_msg:
LOG.warning(irregular_msg.format(rule, service))
if allowed:
diff --git a/patrole_tempest_plugin/tests/api/compute/test_server_actions_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_server_actions_rbac.py
index cb32eec..0fd5c63 100644
--- a/patrole_tempest_plugin/tests/api/compute/test_server_actions_rbac.py
+++ b/patrole_tempest_plugin/tests/api/compute/test_server_actions_rbac.py
@@ -340,6 +340,5 @@
server = self.servers_client.show_server(self.server_id)['server']
if 'host_status' not in server:
- LOG.info("host_status attribute not returned when role doesn't "
- "have permission to access it.")
- raise rbac_exceptions.RbacActionFailed
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='host_status')
diff --git a/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py
index e942f08..23dd48c 100644
--- a/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py
+++ b/patrole_tempest_plugin/tests/api/compute/test_server_misc_policy_actions_rbac.py
@@ -137,8 +137,8 @@
body = self.servers_client.list_servers(detail=True)['servers']
# If the first server contains "config_drive", then all the others do.
if 'config_drive' not in body[0]:
- raise rbac_exceptions.RbacActionFailed(
- '"config_drive" attribute not found in response body.')
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='config_drive')
@test.requires_ext(extension='os-config-drive', service='compute')
@decorators.idempotent_id('55c62ef7-b72b-4970-acc6-05b0a4316e5d')
@@ -150,8 +150,8 @@
self.rbac_utils.switch_role(self, toggle_rbac_role=True)
body = self.servers_client.show_server(self.server['id'])['server']
if 'config_drive' not in body:
- raise rbac_exceptions.RbacActionFailed(
- '"config_drive" attribute not found in response body.')
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute="config_drive")
@test.requires_ext(extension='os-deferred-delete', service='compute')
@decorators.idempotent_id('189bfed4-1e6d-475c-bb8c-d57e60895391')
@@ -192,14 +192,13 @@
self.server['id'], request_id)['instanceAction']
if 'events' not in instance_action:
- raise rbac_exceptions.RbacActionFailed(
- '"%s" attribute not found in response body.' % 'events')
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='events')
# Microversion 2.51+ returns 'events' always, but not 'traceback'. If
# 'traceback' is also present then policy enforcement passed.
if 'traceback' not in instance_action['events'][0]:
- raise rbac_exceptions.RbacActionFailed(
- '"%s" attribute not found in response body.'
- % 'events.traceback')
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='events.traceback')
@rbac_rule_validation.action(
service="nova",
diff --git a/patrole_tempest_plugin/tests/api/compute/test_server_rbac.py b/patrole_tempest_plugin/tests/api/compute/test_server_rbac.py
index 5539221..10ea801 100644
--- a/patrole_tempest_plugin/tests/api/compute/test_server_rbac.py
+++ b/patrole_tempest_plugin/tests/api/compute/test_server_rbac.py
@@ -184,7 +184,7 @@
# Some other policy may have blocked it.
LOG.info("ServerFault exception caught. Some other policy "
"blocked updating of server")
- raise rbac_exceptions.RbacActionFailed(e)
+ raise rbac_exceptions.RbacConflictingPolicies(e)
class SecurtiyGroupsRbacTest(base.BaseV2ComputeRbacTest):
diff --git a/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py b/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py
index fc50ab5..db1f6e6 100644
--- a/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py
+++ b/patrole_tempest_plugin/tests/api/identity/v2/test_projects_rbac.py
@@ -122,10 +122,8 @@
tenant_ids = [t['id'] for t in tenants]
if admin_tenant_id not in tenant_ids:
- raise rbac_exceptions.RbacActionFailed(
- "The admin tenant id was not returned by the call to "
- "``list_tenants``.")
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute="admin tenant id")
if non_admin_tenant_id in tenant_ids:
- raise rbac_exceptions.RbacActionFailed(
- "The non-admin tenant id was returned by the call to "
- "``list_tenants``.")
+ raise rbac_exceptions.RbacMalformedResponse(
+ extra_attr=True)
diff --git a/patrole_tempest_plugin/tests/api/network/test_networks_multiprovider_rbac.py b/patrole_tempest_plugin/tests/api/network/test_networks_multiprovider_rbac.py
index 01cf0fd..9d2b67a 100644
--- a/patrole_tempest_plugin/tests/api/network/test_networks_multiprovider_rbac.py
+++ b/patrole_tempest_plugin/tests/api/network/test_networks_multiprovider_rbac.py
@@ -95,6 +95,5 @@
if len(response_network) == 0:
LOG.info("NotFound or Forbidden exception are not thrown when "
"role doesn't have access to the endpoint. Instead, "
- "the response will have an empty network body. "
- "This is irregular and should be fixed.")
- raise rbac_exceptions.RbacActionFailed
+ "the response will have an empty network body.")
+ raise rbac_exceptions.RbacMalformedResponse(True)
diff --git a/patrole_tempest_plugin/tests/api/network/test_networks_rbac.py b/patrole_tempest_plugin/tests/api/network/test_networks_rbac.py
index b42be93..148804e 100644
--- a/patrole_tempest_plugin/tests/api/network/test_networks_rbac.py
+++ b/patrole_tempest_plugin/tests/api/network/test_networks_rbac.py
@@ -238,7 +238,7 @@
self.network['id'], **kwargs)['network']
if len(retrieved_network) == 0:
- raise rbac_exceptions.RbacActionFailed
+ raise rbac_exceptions.RbacMalformedResponse(True)
@test.requires_ext(extension='provider', service='network')
@rbac_rule_validation.action(service="neutron",
@@ -257,7 +257,7 @@
self.network['id'], **kwargs)['network']
if len(retrieved_network) == 0:
- raise rbac_exceptions.RbacActionFailed
+ raise rbac_exceptions.RbacMalformedResponse(empty=True)
@test.requires_ext(extension='provider', service='network')
@rbac_rule_validation.action(service="neutron",
@@ -276,7 +276,7 @@
self.network['id'], **kwargs)['network']
if len(retrieved_network) == 0:
- raise rbac_exceptions.RbacActionFailed
+ raise rbac_exceptions.RbacMalformedResponse(empty=True)
key = retrieved_network.get('provider:segmentation_id', "NotFound")
self.assertNotEqual(key, "NotFound")
diff --git a/patrole_tempest_plugin/tests/api/network/test_ports_rbac.py b/patrole_tempest_plugin/tests/api/network/test_ports_rbac.py
index cec860c..888f879 100644
--- a/patrole_tempest_plugin/tests/api/network/test_ports_rbac.py
+++ b/patrole_tempest_plugin/tests/api/network/test_ports_rbac.py
@@ -170,7 +170,8 @@
# Rather than throwing a 403, the field is not present, so raise exc.
if fields[0] not in retrieved_port:
- raise rbac_exceptions.RbacActionFailed
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='binding:vif_type')
@test.requires_ext(extension='binding', service='network')
@rbac_rule_validation.action(service="neutron",
@@ -188,7 +189,8 @@
# Rather than throwing a 403, the field is not present, so raise exc.
if fields[0] not in retrieved_port:
- raise rbac_exceptions.RbacActionFailed
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='binding:vif_details')
@test.requires_ext(extension='binding', service='network')
@rbac_rule_validation.action(service="neutron",
@@ -208,7 +210,8 @@
# Rather than throwing a 403, the field is not present, so raise exc.
if fields[0] not in retrieved_port:
- raise rbac_exceptions.RbacActionFailed
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='binding:host_id')
@test.requires_ext(extension='binding', service='network')
@rbac_rule_validation.action(service="neutron",
@@ -229,7 +232,8 @@
# Rather than throwing a 403, the field is not present, so raise exc.
if fields[0] not in retrieved_port:
- raise rbac_exceptions.RbacActionFailed
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='binding:profile')
@rbac_rule_validation.action(service="neutron",
rule="update_port")
diff --git a/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py b/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py
index 6aec5d1..9ed9eb6 100644
--- a/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py
+++ b/patrole_tempest_plugin/tests/api/network/test_routers_rbac.py
@@ -173,8 +173,8 @@
# Rather than throwing a 403, the field is not present, so raise exc.
if 'distributed' not in retrieved_fields:
- raise rbac_exceptions.RbacActionFailed(
- '"distributed" parameter not present in response body')
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='distributed')
@rbac_rule_validation.action(
service="neutron", rule="update_router")
diff --git a/patrole_tempest_plugin/tests/api/volume/test_groups_rbac.py b/patrole_tempest_plugin/tests/api/volume/test_groups_rbac.py
index 6b07aaa..cdd2261 100644
--- a/patrole_tempest_plugin/tests/api/volume/test_groups_rbac.py
+++ b/patrole_tempest_plugin/tests/api/volume/test_groups_rbac.py
@@ -128,9 +128,8 @@
group_type = self.create_group_type(ignore_notfound=True)
if 'group_specs' not in group_type:
- raise rbac_exceptions.RbacActionFailed(
- 'Policy %s does not return %s in response body.' %
- ('group:access_group_types_specs', 'group_specs'))
+ raise rbac_exceptions.RbacMalformedResponse(
+ attribute='group_specs')
@decorators.idempotent_id('f77f8156-4fc9-4f02-be15-8930f748e10c')
@rbac_rule_validation.action(
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 7ce5548..a9acf1c 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -108,16 +108,17 @@
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_rule_validation_rbac_action_failed_positive(self, mock_policy,
- mock_log):
- """Test RbacActionFailed error is thrown without permission passes.
+ def test_rule_validation_rbac_malformed_response_positive(self,
+ mock_policy,
+ mock_log):
+ """Test RbacMalformedResponse error is thrown without permission passes.
- Positive test case: if RbacActionFailed is thrown and the user is not
- allowed to perform the action, then this is a success.
+ Positive test case: if RbacMalformedResponse is thrown and the user is
+ not allowed to perform the action, then this is a success.
"""
decorator = rbac_rv.action(mock.sentinel.service, mock.sentinel.action)
mock_function = mock.Mock()
- mock_function.side_effect = rbac_exceptions.RbacActionFailed
+ mock_function.side_effect = rbac_exceptions.RbacMalformedResponse
wrapper = decorator(mock_function)
mock_policy.RbacPolicyParser.return_value.allowed.return_value = False
@@ -130,16 +131,65 @@
@mock.patch.object(rbac_rv, 'LOG', autospec=True)
@mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
- def test_rule_validation_rbac_action_failed_negative(self, mock_policy,
- mock_log):
- """Test RbacActionFailed error is thrown with permission fails.
+ def test_rule_validation_rbac_malformed_response_negative(self,
+ mock_policy,
+ mock_log):
+ """Test RbacMalformedResponse error is thrown with permission fails.
- Negative test case: if RbacActionFailed is thrown and the user is
+ Negative test case: if RbacMalformedResponse is thrown and the user is
allowed to perform the action, then this is an expected failure.
"""
decorator = rbac_rv.action(mock.sentinel.service, mock.sentinel.action)
mock_function = mock.Mock()
- mock_function.side_effect = rbac_exceptions.RbacActionFailed
+ mock_function.side_effect = rbac_exceptions.RbacMalformedResponse
+ wrapper = decorator(mock_function)
+
+ mock_policy.RbacPolicyParser.return_value.allowed.return_value = True
+
+ e = self.assertRaises(exceptions.Forbidden, wrapper, self.mock_args)
+ self.assertIn(
+ "Role Member was not allowed to perform sentinel.action.",
+ e.__str__())
+
+ mock_log.error.assert_called_once_with("Role Member was not allowed to"
+ " perform sentinel.action.")
+
+ @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+ @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
+ def test_rule_validation_rbac_conflicting_policies_positive(self,
+ mock_policy,
+ mock_log):
+ """Test RbacConflictingPolicies error is thrown without permission passes.
+
+ Positive test case: if RbacConflictingPolicies is thrown and the user
+ is not allowed to perform the action, then this is a success.
+ """
+ decorator = rbac_rv.action(mock.sentinel.service, mock.sentinel.action)
+ mock_function = mock.Mock()
+ mock_function.side_effect = rbac_exceptions.RbacConflictingPolicies
+ wrapper = decorator(mock_function)
+
+ mock_policy.RbacPolicyParser.return_value.allowed.return_value = False
+
+ result = wrapper(self.mock_args)
+
+ self.assertIsNone(result)
+ mock_log.error.assert_not_called()
+ mock_log.warning.assert_not_called()
+
+ @mock.patch.object(rbac_rv, 'LOG', autospec=True)
+ @mock.patch.object(rbac_rv, 'rbac_policy_parser', autospec=True)
+ def test_rule_validation_rbac_conflicting_policies_negative(self,
+ mock_policy,
+ mock_log):
+ """Test RbacConflictingPolicies error is thrown with permission fails.
+
+ Negative test case: if RbacConflictingPolicies is thrown and the user
+ is allowed to perform the action, then this is an expected failure.
+ """
+ decorator = rbac_rv.action(mock.sentinel.service, mock.sentinel.action)
+ mock_function = mock.Mock()
+ mock_function.side_effect = rbac_exceptions.RbacConflictingPolicies
wrapper = decorator(mock_function)
mock_policy.RbacPolicyParser.return_value.allowed.return_value = True