Improve Patrole config options
- Renames "rbac_flag" to "enable_rbac"
- Creates "strict_policy_check" option
- Resolves bug where policy not in policy.json
would pass RBAC test, leading to false test
results
Change-Id: I76429e6cb0ed4cad154a07b7a873fd23209da674
Closes-Bug: #1673626
diff --git a/contrib/post_test_hook.sh b/contrib/post_test_hook.sh
index aeb4013..2eb494f 100644
--- a/contrib/post_test_hook.sh
+++ b/contrib/post_test_hook.sh
@@ -53,10 +53,12 @@
# a "fast' or "slow test" gate
TYPE=$2
-# Set rbac_flag=True under [rbac] section in tempest.conf
-iniset $TEMPEST_CONFIG rbac rbac_flag True
+# Set enable_rbac=True under [rbac] section in tempest.conf
+iniset $TEMPEST_CONFIG rbac enable_rbac True
# Set rbac_test_role=$RBAC_ROLE under [rbac] section in tempest.conf
iniset $TEMPEST_CONFIG rbac rbac_test_role $RBAC_ROLE
+# Set strict_policy_check=False under [rbac] section in tempest.conf
+iniset $TEMPEST_CONFIG rbac strict_policy_check False
# Set additional, necessary CONF values
iniset $TEMPEST_CONFIG auth use_dynamic_credentials True
iniset $TEMPEST_CONFIG auth tempest_roles Member
diff --git a/doc/source/installation.rst b/doc/source/installation.rst
index 00cc57b..96f1c3f 100644
--- a/doc/source/installation.rst
+++ b/doc/source/installation.rst
@@ -48,9 +48,14 @@
#. [rbac] section updates ::
# The role that you want the RBAC tests to use for RBAC testing
- # This needs to be edited to run the test as a different role.
+ # This needs to be edited to run the test as a different role.
rbac_test_role = _member_
# Enables RBAC Tempest tests if set to True. Otherwise, they are
# skipped.
- rbac_flag = True
+ enable_rbac = True
+
+ # If set to true, tests throw a RbacParsingException for policies
+ # not found in the policy.json. Otherwise, they throw a
+ # skipException.
+ strict_policy_check = False
diff --git a/patrole_tempest_plugin/config.py b/patrole_tempest_plugin/config.py
index 1edf877..dfb6cef 100644
--- a/patrole_tempest_plugin/config.py
+++ b/patrole_tempest_plugin/config.py
@@ -23,7 +23,12 @@
default='admin',
help="The current RBAC role against which to run"
" Patrole tests."),
- cfg.BoolOpt('rbac_flag',
+ cfg.BoolOpt('enable_rbac',
default=True,
- help="Enables RBAC tests.")
+ help="Enables RBAC tests."),
+ cfg.BoolOpt('strict_policy_check',
+ default=False,
+ help="If true, throws RbacParsingException for"
+ " policies which don't exist. If false, "
+ "throws skipException.")
]
diff --git a/patrole_tempest_plugin/rbac_auth.py b/patrole_tempest_plugin/rbac_auth.py
index 687c0a8..7281969 100644
--- a/patrole_tempest_plugin/rbac_auth.py
+++ b/patrole_tempest_plugin/rbac_auth.py
@@ -17,10 +17,13 @@
from oslo_log import log as logging
+from tempest import config
+
from patrole_tempest_plugin import rbac_exceptions
from patrole_tempest_plugin import rbac_policy_parser
LOG = logging.getLogger(__name__)
+CONF = config.CONF
class RbacAuthority(object):
@@ -39,5 +42,8 @@
rule_name, role)
return is_allowed
except rbac_exceptions.RbacParsingException as e:
- raise testtools.TestCase.skipException(str(e))
+ if CONF.rbac.strict_policy_check:
+ raise e
+ else:
+ raise testtools.TestCase.skipException(str(e))
return False
diff --git a/patrole_tempest_plugin/rbac_policy_parser.py b/patrole_tempest_plugin/rbac_policy_parser.py
index 62113ac..9d10a36 100644
--- a/patrole_tempest_plugin/rbac_policy_parser.py
+++ b/patrole_tempest_plugin/rbac_policy_parser.py
@@ -104,7 +104,6 @@
access=self._get_access_token(role),
apply_rule=rule_name,
is_admin=is_admin_context)
-
return is_allowed
def _is_admin_context(self, role):
@@ -168,16 +167,11 @@
return result
def _try_rule(self, apply_rule, target, access_data, o):
- try:
- rule = self.rules[apply_rule]
- return rule(target, access_data, o)
- except KeyError as e:
+ if apply_rule not in self.rules:
message = "Policy action: {0} not found in policy file: {1}."\
.format(apply_rule, self.path)
LOG.debug(message)
raise rbac_exceptions.RbacParsingException(message)
- except Exception as e:
- message = "Unknown exception: {0} for policy action: {1} in "\
- "policy file: {2}.".format(e, apply_rule, self.path)
- LOG.debug(message)
- raise rbac_exceptions.RbacParsingException(message)
+ else:
+ rule = self.rules[apply_rule]
+ return rule(target, access_data, o)
diff --git a/patrole_tempest_plugin/tests/api/compute/rbac_base.py b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
index 625b5cf..e5d7d9c 100644
--- a/patrole_tempest_plugin/tests/api/compute/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/compute/rbac_base.py
@@ -29,7 +29,7 @@
@classmethod
def skip_checks(cls):
super(BaseV2ComputeRbacTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
'%s skipped as RBAC flag not enabled' % cls.__name__)
@@ -48,7 +48,7 @@
@classmethod
def skip_checks(cls):
super(BaseV2ComputeAdminRbacTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
'%s skipped as RBAC flag not enabled' % cls.__name__)
diff --git a/patrole_tempest_plugin/tests/api/identity/v2/rbac_base.py b/patrole_tempest_plugin/tests/api/identity/v2/rbac_base.py
index ffee5c0..969631d 100644
--- a/patrole_tempest_plugin/tests/api/identity/v2/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/identity/v2/rbac_base.py
@@ -30,7 +30,7 @@
@classmethod
def skip_checks(cls):
super(BaseIdentityV2AdminRbacTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC Flag not enabled" % cls.__name__)
diff --git a/patrole_tempest_plugin/tests/api/identity/v3/rbac_base.py b/patrole_tempest_plugin/tests/api/identity/v3/rbac_base.py
index 47f6590..127ed3e 100644
--- a/patrole_tempest_plugin/tests/api/identity/v3/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/identity/v3/rbac_base.py
@@ -30,7 +30,7 @@
@classmethod
def skip_checks(cls):
super(BaseIdentityV3RbacAdminTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC Flag not enabled" % cls.__name__)
diff --git a/patrole_tempest_plugin/tests/api/image/rbac_base.py b/patrole_tempest_plugin/tests/api/image/rbac_base.py
index b4fed7d..25521f3 100644
--- a/patrole_tempest_plugin/tests/api/image/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/image/rbac_base.py
@@ -26,7 +26,7 @@
@classmethod
def skip_checks(cls):
super(BaseV1ImageRbacTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC Flag not enabled" % cls.__name__)
@@ -45,7 +45,7 @@
@classmethod
def skip_checks(cls):
super(BaseV2ImageRbacTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC Flag not enabled" % cls.__name__)
diff --git a/patrole_tempest_plugin/tests/api/network/rbac_base.py b/patrole_tempest_plugin/tests/api/network/rbac_base.py
index de5ed88..cad5498 100644
--- a/patrole_tempest_plugin/tests/api/network/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/network/rbac_base.py
@@ -28,7 +28,7 @@
@classmethod
def skip_checks(cls):
super(BaseNetworkRbacTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC Flag not enabled" % cls.__name__)
diff --git a/patrole_tempest_plugin/tests/api/orchestration/rbac_base.py b/patrole_tempest_plugin/tests/api/orchestration/rbac_base.py
index 2cc7978..734007f 100644
--- a/patrole_tempest_plugin/tests/api/orchestration/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/orchestration/rbac_base.py
@@ -26,7 +26,7 @@
@classmethod
def skip_checks(cls):
super(BaseOrchestrationRbacTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC Flag not enabled" % cls.__name__)
diff --git a/patrole_tempest_plugin/tests/api/volume/rbac_base.py b/patrole_tempest_plugin/tests/api/volume/rbac_base.py
index 2a14b19..3c7cd68 100644
--- a/patrole_tempest_plugin/tests/api/volume/rbac_base.py
+++ b/patrole_tempest_plugin/tests/api/volume/rbac_base.py
@@ -26,7 +26,7 @@
@classmethod
def skip_checks(cls):
super(BaseVolumeRbacTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC Flag not enabled" % cls.__name__)
@@ -45,7 +45,7 @@
@classmethod
def skip_checks(cls):
super(BaseVolumeAdminRbacTest, cls).skip_checks()
- if not CONF.rbac.rbac_flag:
+ if not CONF.rbac.enable_rbac:
raise cls.skipException(
"%s skipped as RBAC Flag not enabled" % cls.__name__)
diff --git a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py b/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
index 8583eb5..d5f20f6 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_policy_parser.py
@@ -286,9 +286,8 @@
**{'__getitem__.return_value.side_effect': Exception(
mock.sentinel.error)})
- expected_message = "Unknown exception: {0} for policy action: {1} in "\
- "policy file: {2}.".format(mock.sentinel.error,
- mock.sentinel.rule,
+ expected_message = "Policy action: {0} not found in "\
+ "policy file: {1}.".format(mock.sentinel.rule,
self.custom_policy_file)
e = self.assertRaises(rbac_exceptions.RbacParsingException,
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 705c7e7..e96194b 100644
--- a/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
+++ b/patrole_tempest_plugin/tests/unit/test_rbac_rule_validation.py
@@ -13,7 +13,6 @@
# under the License.
import mock
-import testtools
from patrole_tempest_plugin import rbac_auth
from patrole_tempest_plugin import rbac_exceptions
@@ -171,7 +170,7 @@
self.assertIsNone(wrapper(self.mock_args))
@mock.patch.object(rbac_auth, 'rbac_policy_parser', autospec=True)
- def test_invalid_policy_rule_throws_skip_exception(
+ def test_invalid_policy_rule_throws_parsing_exception(
self, mock_rbac_policy_parser):
mock_rbac_policy_parser.RbacPolicyParser.return_value.allowed.\
side_effect = rbac_exceptions.RbacParsingException
@@ -180,7 +179,7 @@
mock.sentinel.policy_rule)
wrapper = decorator(mock.Mock())
- e = self.assertRaises(testtools.TestCase.skipException, wrapper,
+ e = self.assertRaises(rbac_exceptions.RbacParsingException, wrapper,
self.mock_args)
self.assertEqual('Attempted to test an invalid policy file or action',
str(e))