Merge "Fix reference cycle caused by deprecated sample override" into stable/train2.3.3

2 files changed, 40 insertions, 1 deletions

diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py
index 0a1173c..3429ea4 100644
--- a/oslo_policy/policy.py
+++ b/oslo_policy/policy.py
@@ -673,8 +673,14 @@ class Enforcer(object):
             # the default deprecated policy, override the new policy's default
             # with the old check string. This should prevents unwanted exposure
             # to APIs on upgrade.
+            # There's one exception to this: When we generate a sample policy,
+            # we set the deprecated rule name to reference the new rule. If we
+            # see that the deprecated override rule is just the new rule, then
+            # we shouldn't mess with it.
             if (self.file_rules[deprecated_rule.name].check
-                    != _parser.parse_rule(deprecated_rule.check_str)):
+                    != _parser.parse_rule(deprecated_rule.check_str) and
+                    str(self.file_rules[deprecated_rule.name].check)
+                    != 'rule:%s' % default.name):
                 if default.name not in self.file_rules.keys():
                     self.rules[default.name] = self.file_rules[
                         deprecated_rule.name
diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py
index ba4896b..88d363a 100644
--- a/oslo_policy/tests/test_policy.py
+++ b/oslo_policy/tests/test_policy.py
@@ -1522,6 +1522,39 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bazz']})
         )
 
+    def test_override_deprecated_policy_with_new_rule(self):
+        # Simulate an operator overriding a deprecated policy with a reference
+        # to the new policy, as done by the sample policy generator.
+        rules = jsonutils.dumps({'old_rule': 'rule:new_rule'})
+        self.create_config_file('policy.json', rules)
+
+        # Deprecate the policy name in favor of something better.
+        deprecated_rule = policy.DeprecatedRule(
+            name='old_rule',
+            check_str='role:bang'
+        )
+        rule_list = [policy.DocumentedRuleDefault(
+            name='new_rule',
+            check_str='role:bang',
+            description='Replacement for old_rule.',
+            operations=[{'path': '/v1/bars', 'method': 'POST'}],
+            deprecated_rule=deprecated_rule,
+            deprecated_reason='"old_rule" is a bad name',
+            deprecated_since='N'
+        )]
+        self.enforcer.register_defaults(rule_list)
+
+        # Make sure the override supplied by the operator using the old policy
+        # name is used in favor of the old or new default.
+        self.assertFalse(
+            self.enforcer.enforce('new_rule', {}, {'roles': ['fizz']})
+        )
+        self.assertTrue(
+            self.enforcer.enforce('new_rule', {}, {'roles': ['bang']})
+        )
+        # Verify that we didn't overwrite the new rule.
+        self.assertEqual('bang', self.enforcer.rules['new_rule'].match)
+
 
 class DocumentedRuleDefaultTestCase(base.PolicyBaseTestCase):