Adds meaningful exceptions for missing attributes
Adds new exception and better explanations for failures due to
missing response body attributes and other unusual circumstances
that may lead to failures during testing.
Closes-Bug: #1699419
Closes-Bug: #1704684
Change-Id: I1c14646dc8d102cd093be09833c23846781e5e73
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 51b9d92..75c4c11 100644
--- a/patrole_tempest_plugin/rbac_rule_validation.py
+++ b/patrole_tempest_plugin/rbac_rule_validation.py
@@ -100,7 +100,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 e733e26..f7fced1 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
@@ -121,10 +121,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