From 16840b6a3f85a070a8f600b09db580c5ccd215a0 Mon Sep 17 00:00:00 2001 From: Stephen Finucane Date: Thu, 10 Dec 2020 21:48:17 +0000 Subject: [PATCH] Add documentation parameters for DeprecatedRule Currently, the way you replace a rule with another rule is by using the 'deprecated_rule' parameter of '(Documented)RuleDefault'. For example: deprecated_rule = policy.DeprecatedRule( name='foo:bar', check_str='role:bazz' ) policy.RuleDefault( name='foo:create_bar', check_str='role:bang', description='Create a bar.', deprecated_rule=deprecated_rule, deprecated_reason='foo:bar has been replaced by foo:create_bar', deprecated_since='N', ) In this instance, we're stating that the 'foo:create_bar' policy replaces the 'foo:bar' policy and we've used (and indeed have to use, to avoid a 'ValueError') the 'deprecated_reason' and 'deprecated_since' parameters on the **new** rule to illustrate why. This is confusing. The new rule clearly isn't the one that's deprecated, so why are we stating the 'deprecated_reason' and 'deprecated_since' there? We can clarify this by instead specifying the reason and timeline on the deprecated rule, like so: deprecated_rule = policy.DeprecatedRule( name='foo:bar', check_str='role:bazz' deprecated_reason='foo:bar has been replaced by foo:create_bar', deprecated_since='N', ) policy.RuleDefault( name='foo:create_bar', check_str='role:bang', description='Create a bar.', deprecated_rule=deprecated_rule, ) Add support for this, with appropriate warnings to nudge people over to the new, improved way of doing things eventually. Change-Id: Ie4809c7749242bd092a2677b7545ef281735d984 Signed-off-by: Stephen Finucane --- oslo_policy/generator.py | 31 +++-- oslo_policy/policy.py | 105 +++++++++------ oslo_policy/tests/test_generator.py | 50 ++++---- oslo_policy/tests/test_policy.py | 121 ++++++++++-------- ...ta-to-DeprecatedRule-79d2e8a3f5d11743.yaml | 12 ++ 5 files changed, 197 insertions(+), 122 deletions(-) create mode 100644 releasenotes/notes/add-deprecated-metadata-to-DeprecatedRule-79d2e8a3f5d11743.yaml diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py index ac1a9920..f2b6027e 100644 --- a/oslo_policy/generator.py +++ b/oslo_policy/generator.py @@ -204,21 +204,32 @@ def _format_rule_default_yaml(default, include_help=True, comment_rule=True, 'reason': _format_help_text(default.deprecated_reason), 'text': text} elif add_deprecated_rules and default.deprecated_rule: + deprecated_reason = ( + default.deprecated_rule.deprecated_reason or + default.deprecated_reason + ) + deprecated_since = ( + default.deprecated_rule.deprecated_since or + default.deprecated_since + ) + # This issues a deprecation warning but aliases the old policy name # with the new policy name for compatibility. deprecated_text = ( '"%(old_name)s":"%(old_check_str)s" has been deprecated ' 'since %(since)s in favor of "%(name)s":"%(check_str)s".' - ) % {'old_name': default.deprecated_rule.name, - 'old_check_str': default.deprecated_rule.check_str, - 'since': default.deprecated_since, - 'name': default.name, - 'check_str': default.check_str, - } - text = ('%(text)s# DEPRECATED\n%(deprecated_text)s\n%(reason)s\n' % - {'text': text, - 'reason': _format_help_text(default.deprecated_reason), - 'deprecated_text': _format_help_text(deprecated_text)}) + ) % { + 'old_name': default.deprecated_rule.name, + 'old_check_str': default.deprecated_rule.check_str, + 'since': deprecated_since, + 'name': default.name, + 'check_str': default.check_str, + } + text = '%(text)s# DEPRECATED\n%(deprecated_text)s\n%(reason)s\n' % { + 'text': text, + 'reason': _format_help_text(deprecated_reason), + 'deprecated_text': _format_help_text(deprecated_text) + } if default.name != default.deprecated_rule.name: text += ('"%(old_name)s": "rule:%(name)s"\n' % diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index f21ebe9f..97c15c2f 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -225,6 +225,7 @@ import collections import copy import logging import os +import typing as ty import warnings from oslo_config import cfg @@ -699,6 +700,10 @@ class Enforcer(object): """ deprecated_rule = default.deprecated_rule + deprecated_reason = ( + deprecated_rule.deprecated_reason or default.deprecated_reason) + deprecated_since = ( + deprecated_rule.deprecated_since or default.deprecated_since) deprecated_msg = ( 'Policy "%(old_name)s":"%(old_check_str)s" was deprecated in ' @@ -708,10 +713,10 @@ class Enforcer(object): 'file and maintain it manually.' % { 'old_name': deprecated_rule.name, 'old_check_str': deprecated_rule.check_str, - 'release': default.deprecated_since, + 'release': deprecated_since, 'name': default.name, 'check_str': default.check_str, - 'reason': default.deprecated_reason + 'reason': deprecated_reason, } ) @@ -1152,21 +1157,20 @@ class RuleDefault(object): :param scope_types: A list containing the intended scopes of the operation being done. - .. versionchanged 1.29 + .. versionchanged:: 1.29 Added *deprecated_rule* parameter. - .. versionchanged 1.29 + .. versionchanged:: 1.29 Added *deprecated_for_removal* parameter. - .. versionchanged 1.29 + .. versionchanged:: 1.29 Added *deprecated_reason* parameter. - .. versionchanged 1.29 + .. versionchanged:: 1.29 Added *deprecated_since* parameter. - .. versionchanged 1.31 + .. versionchanged:: 1.31 Added *scope_types* parameter. - """ def __init__(self, name, check_str, description=None, deprecated_rule=None, deprecated_for_removal=False, @@ -1187,13 +1191,23 @@ class RuleDefault(object): 'deprecated_rule must be a DeprecatedRule object.' ) - if (deprecated_for_removal or deprecated_rule) and ( - deprecated_reason is None or deprecated_since is None): - raise ValueError( - '%(name)s deprecated without deprecated_reason or ' - 'deprecated_since. Both must be supplied if deprecating a ' - 'policy' % {'name': self.name} - ) + # if this rule is being deprecated, we need to provide a deprecation + # reason here, but if this rule is replacing another rule, then the + # deprecation reason belongs on that other rule + if deprecated_for_removal: + if deprecated_reason is None or deprecated_since is None: + raise ValueError( + '%(name)s deprecated without deprecated_reason or ' + 'deprecated_since. Both must be supplied if deprecating a ' + 'policy' % {'name': self.name} + ) + elif deprecated_rule and (deprecated_reason or deprecated_since): + warnings.warn( + f'{name} should not configure deprecated_reason or ' + f'deprecated_since as these should be configured on the ' + f'DeprecatedRule indicated by deprecated_rule. ' + f'This will be an error in a future release', + DeprecationWarning) if scope_types: msg = 'scope_types must be a list of strings.' @@ -1318,6 +1332,8 @@ class DeprecatedRule(object): deprecated_rule = policy.DeprecatedRule( name='foo:create_bar', check_str='role:fizz' + deprecated_reason='role:bang is a better default', + deprecated_since='N', ) policy.DocumentedRuleDefault( @@ -1326,8 +1342,6 @@ class DeprecatedRule(object): 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' ) DeprecatedRule can be used to change the policy name itself. Assume the @@ -1349,6 +1363,8 @@ class DeprecatedRule(object): deprecated_rule = policy.DeprecatedRule( name='foo:post_bar', check_str='role:fizz' + deprecated_reason='foo:create_bar is more consistent', + deprecated_since='N', ) policy.DocumentedRuleDefault( @@ -1357,8 +1373,6 @@ class DeprecatedRule(object): description='Create a bar.', operations=[{'path': '/v1/bars', 'method': 'POST'}], deprecated_rule=deprecated_rule, - deprecated_reason='foo:create_bar is more consistent', - deprecated_since='N' ) Finally, let's use DeprecatedRule to break a policy into more granular @@ -1403,6 +1417,10 @@ class DeprecatedRule(object): deprecated_rule = policy.DeprecatedRule( name='foo:bar', check_str='role:bazz' + deprecated_reason=( + 'foo:bar has been replaced by more granular policies' + ), + deprecated_since='N', ) policy.DocumentedRuleDefault( @@ -1411,8 +1429,6 @@ class DeprecatedRule(object): description='Create a bar.', operations=[{'path': '/v1/bars', 'method': 'POST'}], deprecated_rule=deprecated_rule, - deprecated_reason='foo:create_bar is more granular than foo:bar', - deprecated_since='N' ) policy.DocumentedRuleDefault( name='foo:list_bars', @@ -1420,8 +1436,6 @@ class DeprecatedRule(object): description='List bars.', operations=[{'path': '/v1/bars', 'method': 'GET'}], deprecated_rule=deprecated_rule, - deprecated_reason='foo:list_bars is more granular than foo:bar', - deprecated_since='N' ) policy.DocumentedRuleDefault( name='foo:get_bar', @@ -1429,8 +1443,6 @@ class DeprecatedRule(object): description='Get a bar.', operations=[{'path': '/v1/bars/{bar_id}', 'method': 'GET'}], deprecated_rule=deprecated_rule, - deprecated_reason='foo:get_bar is more granular than foo:bar', - deprecated_since='N' ) policy.DocumentedRuleDefault( name='foo:update_bar', @@ -1438,8 +1450,6 @@ class DeprecatedRule(object): description='Update a bar.', operations=[{'path': '/v1/bars/{bar_id}', 'method': 'PATCH'}], deprecated_rule=deprecated_rule, - deprecated_reason='foo:update_bar is more granular than foo:bar', - deprecated_since='N' ) policy.DocumentedRuleDefault( name='foo:delete_bar', @@ -1447,19 +1457,42 @@ class DeprecatedRule(object): description='Delete a bar.', operations=[{'path': '/v1/bars/{bar_id}', 'method': 'DELETE'}], deprecated_rule=deprecated_rule, - deprecated_reason='foo:delete_bar is more granular than foo:bar', - deprecated_since='N' ) - .. versionchanged 1.29 + :param name: The name of the policy. This is used when referencing it + from another rule or during policy enforcement. + :param check_str: The policy. This is a string defining a policy that + conforms to the policy language outlined at the top of the file. + :param deprecated_reason: indicates why this policy is planned for removal + in a future release. + :param deprecated_since: indicates which release this policy was deprecated + in. Accepts any string, though valid version strings are encouraged. + + .. versionchanged:: 1.29 Added *DeprecatedRule* object. + + .. versionchanged:: 3.4 + Added *deprecated_reason* parameter. + + .. versionchanged:: 3.4 + Added *deprecated_since* parameter. """ - def __init__(self, name, check_str): - """Construct a DeprecatedRule object. - - :param name: the policy name - :param check_str: the value of the policy's check string - """ + def __init__( + self, + name: str, + check_str: str, + *, + deprecated_reason: ty.Optional[str] = None, + deprecated_since: ty.Optional[str] = None, + ): self.name = name self.check_str = check_str + self.deprecated_reason = deprecated_reason + self.deprecated_since = deprecated_since + + if not deprecated_reason or not deprecated_since: + warnings.warn( + f'{name} deprecated without deprecated_reason or ' + f'deprecated_since. This will be an error in a future release', + DeprecationWarning) diff --git a/oslo_policy/tests/test_generator.py b/oslo_policy/tests/test_generator.py index 8df0ec85..5dbb4ed0 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 f27f6b1a..903ad7ab 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -1234,7 +1234,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( @@ -1243,8 +1245,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) @@ -1274,7 +1274,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( @@ -1283,8 +1285,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) @@ -1302,7 +1302,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( @@ -1311,8 +1313,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) @@ -1329,15 +1329,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 ' @@ -1346,7 +1338,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 ' @@ -1420,7 +1420,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( @@ -1429,8 +1431,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 @@ -1442,7 +1442,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( @@ -1451,8 +1453,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'}) @@ -1490,7 +1490,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( @@ -1499,8 +1501,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 @@ -1509,7 +1509,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, @@ -1519,24 +1519,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( @@ -1572,7 +1573,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', @@ -1580,8 +1583,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) @@ -1606,7 +1607,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', @@ -1614,8 +1617,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) @@ -1648,7 +1649,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', @@ -1656,8 +1659,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) @@ -1692,7 +1693,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', @@ -1700,8 +1703,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) @@ -1721,7 +1722,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( @@ -1730,8 +1733,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) @@ -1833,6 +1834,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() diff --git a/releasenotes/notes/add-deprecated-metadata-to-DeprecatedRule-79d2e8a3f5d11743.yaml b/releasenotes/notes/add-deprecated-metadata-to-DeprecatedRule-79d2e8a3f5d11743.yaml new file mode 100644 index 00000000..4d5c4840 --- /dev/null +++ b/releasenotes/notes/add-deprecated-metadata-to-DeprecatedRule-79d2e8a3f5d11743.yaml @@ -0,0 +1,12 @@ +--- +features: + - | + ``DeprecatedRule`` now accepts two new parameters: ``deprecated_reason`` + and ``deprecated_since``. These should be used in place of the equivalent + parameters on the rule that is replacing this rule in order to avoid + confusion. +upgrade: + - | + Users with a ``RuleDefault`` or ``DocumentedRuleDefault`` that have + configured a ``deprecated_rule`` should move the ``deprecated_reason`` + and ``deprecated_since`` parameters to this ``DeprecatedRule``.