openstack
/
oslo.policy
Code Issues Proposed changes

Only log deprecation warnings when they are overridden 

The policy deprecation logic use to log a warning for operators that
a policy was being removed regardless of the policy being overridden
in a policy file somewhere. This can be somewhat noisy especially if
there isn't anything for the operator to do since they haven't
overridden the default.

This commit changes the check to see if the deprecated policy is
in the file_rules instead of just the registered rules. This means
that operators should only see a deprecated for removal warning
iff they are providing an override.

Change-Id: Ia82516e9a13f6d04be2428b2a03883272be93329
This commit is contained in:
Lance Bragstad 2018-01-05 20:41:31 +00:00
parent 8835af6aa5
commit 4f68708a49
2 changed files with 23 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
oslo_policy/policy.py
View File

@ -608,7 +608,7 @@ class Enforcer(object):
)
warnings.warn(deprecated_msg)
if default.deprecated_for_removal and (
default.name in self.rules):
default.name in self.file_rules):
# If a policy is going to be removed altogether, then we
# need to make sure we let operators know so they can clean
# up their policy files, if they are overriding it.

23
oslo_policy/tests/test_policy.py
View File

@ -956,7 +956,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
enforcer.load_rules(True)
mock_warn.assert_called_once_with(expected_msg)
def test_deprecate_a_policy_for_removal(self):
def test_deprecate_a_policy_for_removal_logs_warning_when_overridden(self):
rule_list = [policy.DocumentedRuleDefault(
name='foo:bar',
check_str='role:baz',
@ -982,6 +982,27 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
enforcer.load_rules()
mock_warn.assert_called_once_with(expected_msg)
def test_deprecate_a_policy_for_removal_does_not_log_warning(self):
# We should only log a warning for operators if they are supplying an
# override for a policy that is deprecated for removal.
rule_list = [policy.DocumentedRuleDefault(
name='foo:bar',
check_str='role:baz',
description='Create a foo.',
operations=[{'path': '/v1/foos/', 'method': 'POST'}],
deprecated_for_removal=True,
deprecated_reason=(
'"foo:bar" is no longer a policy used by the service'
),
deprecated_since='N'
)]
enforcer = policy.Enforcer(self.conf)
enforcer.register_defaults(rule_list)
with mock.patch('warnings.warn') as mock_warn:
enforcer.load_rules()
mock_warn.assert_not_called()
def test_deprecated_policy_for_removal_must_include_deprecated_since(self):
self.assertRaises(
ValueError,