From 9641e5ca0d758376fb7f7f5156056d5206d71046 Mon Sep 17 00:00:00 2001
From: Lance Bragstad <lbragstad@gmail.com>
Date: Thu, 25 Jul 2019 16:28:01 +0000
Subject: [PATCH] Only alias when policy names change

Previously, oslo.policy would generate policy files with aliased names
in the event the name was changing for backwards compatibility. This
isn't needed if the name isn't changing and only the check string is
changing.

This patch adds a conditional to the generator logic that only aliases
the old name to the new name if the name is changing. Otherwise, it
only outputs comments about the deprecation.

Co-Authored-By: Ben Nemec <bnemec@redhat.com>
Change-Id: I89ff60354e4751a5096832023441d2e6166db92a
---
 oslo_policy/generator.py            | 19 +++++++++----
 oslo_policy/tests/test_generator.py | 43 +++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 6 deletions(-)

diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py
index 1ea768ea..bd753896 100644
--- a/oslo_policy/generator.py
+++ b/oslo_policy/generator.py
@@ -193,12 +193,19 @@ def _format_rule_default_yaml(default, include_help=True):
              'check_str': default.check_str,
              'reason': default.deprecated_reason}
 
-        text = (
-            '%(text)s%(deprecated_text)s\n"%(old_name)s": "rule:%(name)s"\n'
-        ) % {'text': text,
-             'deprecated_text': _format_help_text(deprecated_text),
-             'old_name': default.deprecated_rule.name,
-             'name': default.name}
+        if default.name != default.deprecated_rule.name:
+            text = (
+                '%(text)s%(deprecated_text)s\n"%(old_name)s": "rule:%(name)s"'
+                '\n'
+            ) % {'text': text,
+                 'deprecated_text': _format_help_text(deprecated_text),
+                 'old_name': default.deprecated_rule.name,
+                 'name': default.name}
+        else:
+            text = (
+                '%(text)s%(deprecated_text)s\n'
+            ) % {'text': text,
+                 'deprecated_text': _format_help_text(deprecated_text)}
 
     return text
 
diff --git a/oslo_policy/tests/test_generator.py b/oslo_policy/tests/test_generator.py
index ab0940aa..18c54065 100644
--- a/oslo_policy/tests/test_generator.py
+++ b/oslo_policy/tests/test_generator.py
@@ -227,6 +227,49 @@ class GenerateSampleYAMLTestCase(base.PolicyBaseTestCase):
 # favor of "foo:create_bar":"role:fizz". foo:post_bar is being removed
 # in favor of foo:create_bar
 "foo:post_bar": "rule:foo:create_bar"
+'''
+        stdout = self._capture_stdout()
+        with mock.patch('stevedore.named.NamedExtensionManager',
+                        return_value=test_mgr) as mock_ext_mgr:
+            generator._generate_sample(['rules'], output_file=None)
+            mock_ext_mgr.assert_called_once_with(
+                'oslo.policy.policies', names=['rules'],
+                on_load_failure_callback=generator.on_load_failure_callback,
+                invoke_on_load=True
+            )
+        self.assertEqual(expected, stdout.getvalue())
+
+    def test_deprecated_policies_with_same_name(self):
+        deprecated_rule = policy.DeprecatedRule(
+            name='foo:create_bar',
+            check_str='role:old'
+        )
+        new_rule = policy.RuleDefault(
+            name='foo:create_bar',
+            check_str='role:fizz',
+            description='Create a bar.',
+            deprecated_rule=deprecated_rule,
+            deprecated_reason=(
+                'role:fizz is a more sane default for foo:create_bar'
+            ),
+            deprecated_since='N'
+        )
+        opts = {'rules': [new_rule]}
+
+        extensions = []
+        for name, opts in opts.items():
+            ext = stevedore.extension.Extension(name=name, entry_point=None,
+                                                plugin=None, obj=opts)
+            extensions.append(ext)
+        test_mgr = stevedore.named.NamedExtensionManager.make_test_instance(
+            extensions=extensions, namespace=['rules'])
+
+        expected = '''# Create a bar.
+#"foo:create_bar": "role:fizz"
+
+# DEPRECATED "foo:create_bar":"role:old" has been deprecated since N
+# in favor of "foo:create_bar":"role:fizz". role:fizz is a more sane
+# default for foo:create_bar
 '''
         stdout = self._capture_stdout()
         with mock.patch('stevedore.named.NamedExtensionManager',