Merge "Add documentation parameters for DeprecatedRule"

2 files changed, 95 insertions, 76 deletions

diff --git a/oslo_policy/tests/test_generator.py b/oslo_policy/tests/test_generator.py
index 8df0ec8..5dbb4ed 100644
--- a/oslo_policy/tests/test_generator.py
+++ b/oslo_policy/tests/test_generator.py
@@ -194,17 +194,17 @@ class GenerateSampleYAMLTestCase(base.PolicyBaseTestCase):
     def test_deprecated_policies_are_aliased_to_new_names(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:post_bar',
-            check_str='role:fizz'
+            check_str='role:fizz',
+            deprecated_reason=(
+                'foo:post_bar is being removed in favor of foo:create_bar'
+            ),
+            deprecated_since='N',
         )
         new_rule = policy.RuleDefault(
             name='foo:create_bar',
             check_str='role:fizz',
             description='Create a bar.',
             deprecated_rule=deprecated_rule,
-            deprecated_reason=(
-                'foo:post_bar is being removed in favor of foo:create_bar'
-            ),
-            deprecated_since='N'
         )
         opts = {'rules': [new_rule]}
 
@@ -240,17 +240,17 @@ class GenerateSampleYAMLTestCase(base.PolicyBaseTestCase):
     def test_deprecated_policies_with_same_name(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
-            check_str='role:old'
+            check_str='role:old',
+            deprecated_reason=(
+                'role:fizz is a more sane default for foo:create_bar'
+            ),
+            deprecated_since='N',
         )
         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]}
 
@@ -606,12 +606,18 @@ class ListRedundantTestCase(base.PolicyBaseTestCase):
         enforcer.register_default(
             policy.RuleDefault('owner', 'project_id:%(project_id)s'))
         # register a new opt
-        deprecated_rule = policy.DeprecatedRule('old_foo', 'role:bar')
+        deprecated_rule = policy.DeprecatedRule(
+            name='old_foo',
+            check_str='role:bar',
+            deprecated_reason='reason',
+            deprecated_since='T'
+        )
         enforcer.register_default(
-            policy.RuleDefault('foo', 'role:foo',
-                               deprecated_rule=deprecated_rule,
-                               deprecated_reason='reason',
-                               deprecated_since='T')
+            policy.RuleDefault(
+                name='foo',
+                check_str='role:foo',
+                deprecated_rule=deprecated_rule,
+            ),
         )
 
         # Mock out stevedore to return the configured enforcer
@@ -656,7 +662,9 @@ class UpgradePolicyTestCase(base.PolicyBaseTestCase):
         self.create_config_file('policy.json', policy_json_contents)
         deprecated_policy = policy.DeprecatedRule(
             name='deprecated_name',
-            check_str='rule:admin'
+            check_str='rule:admin',
+            deprecated_reason='test',
+            deprecated_since='Stein',
         )
         self.new_policy = policy.DocumentedRuleDefault(
             name='new_policy_name',
@@ -664,8 +672,6 @@ class UpgradePolicyTestCase(base.PolicyBaseTestCase):
             description='test_policy',
             operations=[{'path': '/test', 'method': 'GET'}],
             deprecated_rule=deprecated_policy,
-            deprecated_reason='test',
-            deprecated_since='Stein'
         )
         self.extensions = []
         ext = stevedore.extension.Extension(name='test_upgrade',
@@ -848,7 +854,9 @@ class ConvertJsonToYamlTestCase(base.PolicyBaseTestCase):
             'converted_policy.yaml')
         deprecated_policy = policy.DeprecatedRule(
             name='deprecated_rule1_name',
-            check_str='rule:admin'
+            check_str='rule:admin',
+            deprecated_reason='testing',
+            deprecated_since='ussuri',
         )
         self.registered_policy = [
             policy.DocumentedRuleDefault(
@@ -857,9 +865,7 @@ class ConvertJsonToYamlTestCase(base.PolicyBaseTestCase):
                 description='test_rule1',
                 operations=[{'path': '/test', 'method': 'GET'}],
                 deprecated_rule=deprecated_policy,
-                deprecated_reason='testing',
-                deprecated_since='ussuri',
-                scope_types=['system']
+                scope_types=['system'],
             ),
             policy.RuleDefault(
                 name='rule2_name',
diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py
index 513b43d..b97aae3 100644
--- a/oslo_policy/tests/test_policy.py
+++ b/oslo_policy/tests/test_policy.py
@@ -1253,7 +1253,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
     def test_deprecate_a_policy_check_string(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
-            check_str='role:fizz'
+            check_str='role:fizz',
+            deprecated_reason='"role:bang" is a better default',
+            deprecated_since='N'
         )
 
         rule_list = [policy.DocumentedRuleDefault(
@@ -1262,8 +1264,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             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_list)
@@ -1293,7 +1293,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
     def test_deprecate_an_empty_policy_check_string(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
-            check_str=''
+            check_str='',
+            deprecated_reason='because of reasons',
+            deprecated_since='N',
         )
 
         rule_list = [policy.DocumentedRuleDefault(
@@ -1302,8 +1304,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             description='Create a bar.',
             operations=[{'path': '/v1/bars', 'method': 'POST'}],
             deprecated_rule=deprecated_rule,
-            deprecated_reason='because of reasons',
-            deprecated_since='N'
         )]
         enforcer = policy.Enforcer(self.conf)
         enforcer.register_defaults(rule_list)
@@ -1321,7 +1321,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
     def test_deprecate_replace_with_empty_policy_check_string(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
-            check_str='role:fizz'
+            check_str='role:fizz',
+            deprecated_reason='because of reasons',
+            deprecated_since='N',
         )
 
         rule_list = [policy.DocumentedRuleDefault(
@@ -1330,8 +1332,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             description='Create a bar.',
             operations=[{'path': '/v1/bars', 'method': 'POST'}],
             deprecated_rule=deprecated_rule,
-            deprecated_reason='because of reasons',
-            deprecated_since='N'
         )]
         enforcer = policy.Enforcer(self.conf)
         enforcer.register_defaults(rule_list)
@@ -1348,15 +1348,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
     def test_deprecate_a_policy_name(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:bar',
-            check_str='role:baz'
-        )
-
-        rule_list = [policy.DocumentedRuleDefault(
-            name='foo:create_bar',
             check_str='role:baz',
-            description='Create a bar.',
-            operations=[{'path': '/v1/bars/', 'method': 'POST'}],
-            deprecated_rule=deprecated_rule,
             deprecated_reason=(
                 '"foo:bar" is not granular enough. If your deployment has '
                 'overridden "foo:bar", ensure you override the new policies '
@@ -1365,7 +1357,15 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
                 '"foo:bar:update", "foo:bar:list", and "foo:bar:delete", '
                 'which might be backwards incompatible for your deployment'
             ),
-            deprecated_since='N'
+            deprecated_since='N',
+        )
+
+        rule_list = [policy.DocumentedRuleDefault(
+            name='foo:create_bar',
+            check_str='role:baz',
+            description='Create a bar.',
+            operations=[{'path': '/v1/bars/', 'method': 'POST'}],
+            deprecated_rule=deprecated_rule,
         )]
         expected_msg = (
             'Policy "foo:bar":"role:baz" was deprecated in N in favor of '
@@ -1439,7 +1439,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
     def test_deprecate_check_str_suppress_does_not_log_warning(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
-            check_str='role:fizz'
+            check_str='role:fizz',
+            deprecated_reason='"role:bang" is a better default',
+            deprecated_since='N'
         )
 
         rule_list = [policy.DocumentedRuleDefault(
@@ -1448,8 +1450,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             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.suppress_deprecation_warnings = True
@@ -1461,7 +1461,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
     def test_deprecate_name_suppress_does_not_log_warning(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:bar',
-            check_str='role:baz'
+            check_str='role:baz',
+            deprecated_reason='"foo:bar" is not granular enough.',
+            deprecated_since='N',
         )
 
         rule_list = [policy.DocumentedRuleDefault(
@@ -1470,8 +1472,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             description='Create a bar.',
             operations=[{'path': '/v1/bars/', 'method': 'POST'}],
             deprecated_rule=deprecated_rule,
-            deprecated_reason='"foo:bar" is not granular enough.',
-            deprecated_since='N'
         )]
 
         rules = jsonutils.dumps({'foo:bar': 'role:bang'})
@@ -1509,7 +1509,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
     def test_suppress_default_change_warnings_flag_not_log_warning(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
-            check_str='role:fizz'
+            check_str='role:fizz',
+            deprecated_reason='"role:bang" is a better default',
+            deprecated_since='N',
         )
 
         rule_list = [policy.DocumentedRuleDefault(
@@ -1518,8 +1520,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             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.suppress_default_change_warnings = True
@@ -1528,7 +1528,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             enforcer.load_rules()
             mock_warn.assert_not_called()
 
-    def test_deprecated_policy_for_removal_must_include_deprecated_since(self):
+    def test_deprecated_policy_for_removal_must_include_deprecated_meta(self):
         self.assertRaises(
             ValueError,
             policy.DocumentedRuleDefault,
@@ -1538,24 +1538,25 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             operations=[{'path': '/v1/foos/', 'method': 'POST'}],
             deprecated_for_removal=True,
             deprecated_reason='Some reason.'
+            # no deprecated_since
         )
 
-    def test_deprecated_policy_must_include_deprecated_since(self):
+    def test_deprecated_policy_should_not_include_deprecated_meta(self):
         deprecated_rule = policy.DeprecatedRule(
             name='foo:bar',
             check_str='rule:baz'
         )
 
-        self.assertRaises(
-            ValueError,
-            policy.DocumentedRuleDefault,
-            name='foo:bar',
-            check_str='rule:baz',
-            description='Create a foo.',
-            operations=[{'path': '/v1/foos/', 'method': 'POST'}],
-            deprecated_rule=deprecated_rule,
-            deprecated_reason='Some reason.'
-        )
+        with mock.patch('warnings.warn') as mock_warn:
+            policy.DocumentedRuleDefault(
+                name='foo:bar',
+                check_str='rule:baz',
+                description='Create a foo.',
+                operations=[{'path': '/v1/foos/', 'method': 'POST'}],
+                deprecated_rule=deprecated_rule,
+                deprecated_reason='Some reason.'
+            )
+            mock_warn.assert_called_once()
 
     def test_deprecated_rule_requires_deprecated_rule_object(self):
         self.assertRaises(
@@ -1591,7 +1592,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
         # better.
         deprecated_rule = policy.DeprecatedRule(
             name='foo:bar',
-            check_str='role:fizz'
+            check_str='role:fizz',
+            deprecated_reason='"role:bang" is a better default',
+            deprecated_since='N',
         )
         rule_list = [policy.DocumentedRuleDefault(
             name='foo:create_bar',
@@ -1599,8 +1602,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             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'
         )]
         self.enforcer.register_defaults(rule_list)
 
@@ -1625,7 +1626,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
         # better.
         deprecated_rule = policy.DeprecatedRule(
             name='foo:bar',
-            check_str='role:fizz'
+            check_str='role:fizz',
+            deprecated_reason='"role:bang" is a better default',
+            deprecated_since='N',
         )
         rule_list = [policy.DocumentedRuleDefault(
             name='foo:create_bar',
@@ -1633,8 +1636,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             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'
         )]
         self.enforcer.register_defaults(rule_list)
 
@@ -1667,7 +1668,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
         # better.
         deprecated_rule = policy.DeprecatedRule(
             name='foo:bar',
-            check_str='role:fizz'
+            check_str='role:fizz',
+            deprecated_reason='"role:bang" is a better default',
+            deprecated_since='N',
         )
         rule_list = [policy.DocumentedRuleDefault(
             name='foo:create_bar',
@@ -1675,8 +1678,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             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'
         )]
         self.enforcer.register_defaults(rule_list)
 
@@ -1711,7 +1712,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
         # Deprecate the policy name in favor of something better.
         deprecated_rule = policy.DeprecatedRule(
             name='old_rule',
-            check_str='role:bang'
+            check_str='role:bang',
+            deprecated_reason='"old_rule" is a bad name',
+            deprecated_since='N',
         )
         rule_list = [policy.DocumentedRuleDefault(
             name='new_rule',
@@ -1719,8 +1722,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             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)
 
@@ -1740,7 +1741,9 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
                                group='oslo_policy')
         deprecated_rule = policy.DeprecatedRule(
             name='foo:create_bar',
-            check_str='role:fizz'
+            check_str='role:fizz',
+            deprecated_reason='"role:bang" is a better default',
+            deprecated_since='N',
         )
 
         rule_list = [policy.DocumentedRuleDefault(
@@ -1749,8 +1752,6 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
             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_list)
@@ -1888,6 +1889,18 @@ class DocumentedRuleDefaultTestCase(base.PolicyBaseTestCase):
                           operations=invalid_op)
 
 
+class DeprecatedRuleTestCase(base.PolicyBaseTestCase):
+
+    def test_should_include_deprecated_meta(self):
+        with mock.patch('warnings.warn') as mock_warn:
+            policy.DeprecatedRule(
+                name='foo:bar',
+                check_str='rule:baz'
+            )
+
+            mock_warn.assert_called_once()
+
+
 class EnforcerCheckRulesTest(base.PolicyBaseTestCase):
     def setUp(self):
         super(EnforcerCheckRulesTest, self).setUp()