diff options
| -rw-r--r-- | oslo_policy/policy.py | 10 | ||||
| -rw-r--r-- | oslo_policy/tests/test_policy.py | 34 | ||||
| -rw-r--r-- | releasenotes/notes/fix-bug-1914095-fa71d81c9639ba94.yaml | 8 |
3 files changed, 43 insertions, 9 deletions
diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index 046bec0..903f65b 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -747,6 +747,7 @@ class Enforcer(object): self.rules[default.name] = self.file_rules[ deprecated_rule.name ].check + default._deprecated_rule_handled = True # In this case, the default check string is changing. We need to let # operators know that this is going to change. If they don't want to @@ -767,12 +768,11 @@ class Enforcer(object): default.check = OrCheck([_parser.parse_rule(cs) for cs in [default.check_str, deprecated_rule.check_str]]) + default._deprecated_rule_handled = True if not (self.suppress_deprecation_warnings or self.suppress_default_change_warnings): warnings.warn(deprecated_msg) - default._deprecated_rule_handled = True - def _undefined_check(self, check): '''Check if a RuleCheck references an undefined rule.''' if isinstance(check, RuleCheck): @@ -1101,7 +1101,11 @@ class Enforcer(object): if default.name in self.registered_rules: raise DuplicatePolicyError(default.name) - self.registered_rules[default.name] = default + # NOTE Always make copy of registered rule because policy engine + # update these rules in many places (one example is + # self._handle_deprecated_rule() ). This will avoid any conflict + # in rule object values when running tests in parallel. + self.registered_rules[default.name] = copy.deepcopy(default) def register_defaults(self, defaults): """Registers a list of RuleDefaults. diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index 945b809..513b43d 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -784,6 +784,25 @@ class EnforcerTest(base.PolicyBaseTestCase): policy.RuleDefault(name='owner', check_str='role:owner')) + def test_enforcer_does_not_modify_original_registered_rule(self): + rule_original = policy.RuleDefault( + name='test', + check_str='role:owner',) + rule_original._deprecated_rule_handled = False + self.enforcer.register_default(rule_original) + self.enforcer.registered_rules['test'].check_str = 'role:admin' + self.enforcer.registered_rules['test'].check = 'role:admin' + self.enforcer.registered_rules['test']._deprecated_rule_handled = True + self.assertEqual(self.enforcer.registered_rules['test'].check_str, + 'role:admin') + self.assertEqual(self.enforcer.registered_rules['test'].check, + 'role:admin') + self.assertTrue( + self.enforcer.registered_rules['test']._deprecated_rule_handled) + self.assertEqual(rule_original.check_str, 'role:owner') + self.assertEqual(rule_original.check.__str__(), 'role:owner') + self.assertFalse(rule_original._deprecated_rule_handled) + def test_non_reversible_check(self): self.create_config_file('policy.json', jsonutils.dumps( @@ -1766,21 +1785,24 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): enforcer = policy.Enforcer(self.conf) enforcer.register_defaults([rule]) - + registered_default_rule = enforcer.registered_rules['foo:create_bar'] # Check that rule deprecation handling hasn't been done, yet - self.assertFalse(rule._deprecated_rule_handled) + self.assertFalse(registered_default_rule._deprecated_rule_handled) # Loading the rules will modify the rule check string to logically OR # the new value with the deprecated value enforcer.load_rules() - self.assertTrue(rule._deprecated_rule_handled) + self.assertTrue(registered_default_rule._deprecated_rule_handled) # Make sure the original value is used instead of instantiating new # OrCheck objects whenever we perform subsequent reloads - expected_check = rule.check + expected_check = policy.OrCheck([_parser.parse_rule(cs) for cs in + [rule.check_str, + deprecated_rule.check_str]]) enforcer.load_rules() - self.assertTrue(rule.check is expected_check) - self.assertTrue(rule._deprecated_rule_handled) + self.assertEqual(registered_default_rule.check.__str__(), + expected_check.__str__()) + self.assertTrue(registered_default_rule._deprecated_rule_handled) class DocumentedRuleDefaultTestCase(base.PolicyBaseTestCase): diff --git a/releasenotes/notes/fix-bug-1914095-fa71d81c9639ba94.yaml b/releasenotes/notes/fix-bug-1914095-fa71d81c9639ba94.yaml new file mode 100644 index 0000000..17f2c6f --- /dev/null +++ b/releasenotes/notes/fix-bug-1914095-fa71d81c9639ba94.yaml @@ -0,0 +1,8 @@ +--- +fixes: + - | + This fixes the Bug# 1914095. Policy engine has bug of modifying the + registered rule original object which caused issue when there are + multiple policy objects are processing rules in parallel. + With this fix. policy engine will make copies of all the registered rules + and process accordingly. |