diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index 6ba1bcd0..046bec0a 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -698,6 +698,11 @@ class Enforcer(object): DeprecatedRule """ + # Short-circuit the rule deprecation logic if we've already processed + # it for this particular rule. + if default._deprecated_rule_handled: + return + deprecated_rule = default.deprecated_rule deprecated_msg = ( @@ -766,6 +771,8 @@ class Enforcer(object): 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): @@ -1180,6 +1187,7 @@ class RuleDefault(object): self.deprecated_for_removal = deprecated_for_removal self.deprecated_reason = deprecated_reason self.deprecated_since = deprecated_since + self._deprecated_rule_handled = False if self.deprecated_rule: if not isinstance(self.deprecated_rule, DeprecatedRule): diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index f27f6b1a..945b8093 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -1749,6 +1749,39 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): enforcer.enforce('foo:create_bar', {}, {'roles': ['baz']}) ) + def test_deprecation_logic_is_only_performed_once_per_rule(self): + deprecated_rule = policy.DeprecatedRule( + name='foo:create_bar', + check_str='role:fizz' + ) + rule = policy.DocumentedRuleDefault( + name='foo:create_bar', + check_str='role:bang', + description='Create a bar.', + operations=[{'path': '/v1/bars', 'method': 'POST'}], + deprecated_rule=deprecated_rule, + deprecated_reason='"role:bang" is a better default', + deprecated_since='N' + ) + + enforcer = policy.Enforcer(self.conf) + enforcer.register_defaults([rule]) + + # Check that rule deprecation handling hasn't been done, yet + self.assertFalse(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) + + # Make sure the original value is used instead of instantiating new + # OrCheck objects whenever we perform subsequent reloads + expected_check = rule.check + enforcer.load_rules() + self.assertTrue(rule.check is expected_check) + self.assertTrue(rule._deprecated_rule_handled) + class DocumentedRuleDefaultTestCase(base.PolicyBaseTestCase): diff --git a/releasenotes/notes/bug-1913718-f1b46bbff3231d98.yaml b/releasenotes/notes/bug-1913718-f1b46bbff3231d98.yaml new file mode 100644 index 00000000..509d6af2 --- /dev/null +++ b/releasenotes/notes/bug-1913718-f1b46bbff3231d98.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + [`bug 1913718 `_] + The `Enforcer()` object now only processes deprecated rules once at load or + enforcement time, improving performance for users that make extensive use + of policy enforcement.