Merge "Make upgrades more robust with policy overrides"
This commit is contained in:
Zuul
Gerrit Code Review
commit
0af23a37bc
|
|
@ -575,66 +575,11 @@ class Enforcer(object):
|
|||
force_reload, False)
|
||||
|
||||
for default in self.registered_rules.values():
|
||||
if default.deprecated_rule:
|
||||
deprecated_msg = (
|
||||
'Policy "%(old_name)s":"%(old_check_str)s" was '
|
||||
'deprecated in %(release)s in favor of "%(name)s":'
|
||||
'"%(check_str)s". Reason: %(reason)s. Either ensure '
|
||||
'your deployment is ready for the new default or '
|
||||
'copy/paste the deprecated policy into your policy '
|
||||
'file and maintain it manually.' % {
|
||||
'old_name': default.deprecated_rule.name,
|
||||
'old_check_str': default.deprecated_rule.check_str,
|
||||
'release': default.deprecated_since,
|
||||
'name': default.name,
|
||||
'check_str': default.check_str,
|
||||
'reason': default.deprecated_reason
|
||||
}
|
||||
)
|
||||
if default.deprecated_rule.name != default.name and (
|
||||
default.deprecated_rule.name in self.rules):
|
||||
# Print a warning because the actual policy name is
|
||||
# changing. If deployers are relying on an override for
|
||||
# foo:bar and it's getting renamed to foo:create_bar
|
||||
# then they need to be able to see that before they
|
||||
# roll out the next release.
|
||||
warnings.warn(deprecated_msg)
|
||||
if (default.deprecated_rule.check_str !=
|
||||
default.check_str and default.name not in
|
||||
self.rules):
|
||||
# In this case, the default check_str is changing. We
|
||||
# need to let operators know that this is going to
|
||||
# change. If they don't want to override it, they are
|
||||
# going to have to make sure the right infrastructure
|
||||
# exists before they upgrade. This overrides the new
|
||||
# check with an OrCheck that combines the new and old
|
||||
# check_str attributes from the new and deprecated
|
||||
# policies. This will make it so that deployments don't
|
||||
# break on upgrade, but they receive log messages
|
||||
# telling them stuff is going to change if they don't
|
||||
# maintain the policy manually or add infrastructure to
|
||||
# their deployment to support the new policy.
|
||||
default.check = _parser.parse_rule(
|
||||
default.check_str + ' or ' +
|
||||
default.deprecated_rule.check_str
|
||||
)
|
||||
warnings.warn(deprecated_msg)
|
||||
if default.deprecated_for_removal and (
|
||||
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.
|
||||
warnings.warn(
|
||||
'Policy "%(policy)s":"%(check_str)s" was '
|
||||
'deprecated for removal in %(release)s. Reason: '
|
||||
'%(reason)s. Its value may be silently ignored in '
|
||||
'the future.' % {
|
||||
'policy': default.name,
|
||||
'check_str': default.check_str,
|
||||
'release': default.deprecated_since,
|
||||
'reason': default.deprecated_reason
|
||||
}
|
||||
)
|
||||
if default.deprecated_for_removal:
|
||||
self._emit_deprecated_for_removal_warning(default)
|
||||
elif default.deprecated_rule:
|
||||
self._handle_deprecated_rule(default)
|
||||
|
||||
if default.name not in self.rules:
|
||||
self.rules[default.name] = default.check
|
||||
|
||||
|
|
@ -668,6 +613,87 @@ class Enforcer(object):
|
|||
|
||||
return not violation
|
||||
|
||||
def _emit_deprecated_for_removal_warning(self, default):
|
||||
# If the policy is being removed completely, we need to let operators
|
||||
# know that the policy is going to be silently ignored in the future
|
||||
# and they can remove it from their overrides since it isn't being
|
||||
# replaced by another policy.
|
||||
if default.name in self.file_rules:
|
||||
warnings.warn(
|
||||
'Policy "%(policy)s":"%(check_str)s" was deprecated for '
|
||||
'removal in %(release)s. Reason: %(reason)s. Its value may be '
|
||||
'silently ignored in the future.' % {
|
||||
'policy': default.name,
|
||||
'check_str': default.check_str,
|
||||
'release': default.deprecated_since,
|
||||
'reason': default.deprecated_reason
|
||||
}
|
||||
)
|
||||
|
||||
def _handle_deprecated_rule(self, default):
|
||||
"""Handle cases where a policy rule has been deprecated.
|
||||
|
||||
:param default: an instance of RuleDefault that contains an instance of
|
||||
DeprecatedRule
|
||||
"""
|
||||
|
||||
deprecated_rule = default.deprecated_rule
|
||||
|
||||
deprecated_msg = (
|
||||
'Policy "%(old_name)s":"%(old_check_str)s" was deprecated in '
|
||||
'%(release)s in favor of "%(name)s":"%(check_str)s". Reason: '
|
||||
'%(reason)s. Either ensure your deployment is ready for the new '
|
||||
'default or copy/paste the deprecated policy into your policy '
|
||||
'file and maintain it manually.' % {
|
||||
'old_name': deprecated_rule.name,
|
||||
'old_check_str': deprecated_rule.check_str,
|
||||
'release': default.deprecated_since,
|
||||
'name': default.name,
|
||||
'check_str': default.check_str,
|
||||
'reason': default.deprecated_reason
|
||||
}
|
||||
)
|
||||
|
||||
# Print a warning because the actual policy name is changing. If
|
||||
# operators are relying on an override for foo:bar and it's getting
|
||||
# renamed to foo:create_bar then they need to be able to see that
|
||||
# before they roll out the next release. If the policy name is in
|
||||
# self.file_rules, we know that it's being overridden.
|
||||
if deprecated_rule.name != default.name and (
|
||||
deprecated_rule.name in self.file_rules):
|
||||
|
||||
warnings.warn(deprecated_msg)
|
||||
|
||||
# If the deprecated policy is being overridden and doesn't match
|
||||
# the default deprecated policy, override the new policy's default
|
||||
# with the old check string. This should prevents unwanted exposure
|
||||
# to APIs on upgrade.
|
||||
if (self.file_rules[deprecated_rule.name].check
|
||||
!= _parser.parse_rule(deprecated_rule.check_str)):
|
||||
if default.name not in self.file_rules.keys():
|
||||
self.rules[default.name] = self.file_rules[
|
||||
deprecated_rule.name
|
||||
].check
|
||||
|
||||
# In this case, the default check string is changing. We need to let
|
||||
# operators know that this is going to change. If they don't want to
|
||||
# override it, they are going to have to make sure the right
|
||||
# infrastructure exists before they upgrade. This overrides the new
|
||||
# check with an OrCheck that combines the new and old check_str
|
||||
# attributes from the new and deprecated policies. This will make it so
|
||||
# that deployments don't break on upgrade, but they receive log
|
||||
# messages telling them stuff is going to change if they don't maintain
|
||||
# the policy manually or add infrastructure to their deployment to
|
||||
# support the new policy.
|
||||
if (deprecated_rule.check_str != default.check_str
|
||||
and default.name not in self.file_rules):
|
||||
|
||||
default.check = _parser.parse_rule(
|
||||
default.check_str + ' or ' +
|
||||
deprecated_rule.check_str
|
||||
)
|
||||
warnings.warn(deprecated_msg)
|
||||
|
||||
def _undefined_check(self, check):
|
||||
'''Check if a RuleCheck references an undefined rule.'''
|
||||
if isinstance(check, RuleCheck):
|
||||
|
|
|
|||
|
|
@ -1235,6 +1235,124 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase):
|
|||
deprecated_since='N'
|
||||
)
|
||||
|
||||
def test_override_deprecated_policy_with_old_name(self):
|
||||
# Simulate an operator overriding a policy
|
||||
rules = jsonutils.dumps({'foo:bar': 'role:bazz'})
|
||||
self.create_config_file('policy.json', rules)
|
||||
|
||||
# Deprecate the policy name and check string in favor of something
|
||||
# better.
|
||||
deprecated_rule = policy.DeprecatedRule(
|
||||
name='foo:bar',
|
||||
check_str='role:fizz'
|
||||
)
|
||||
rule_list = [policy.DocumentedRuleDefault(
|
||||
name='foo:create_bar',
|
||||
check_str='role:bang',
|
||||
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)
|
||||
|
||||
# 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('foo:create_bar', {}, {'roles': ['fizz']})
|
||||
)
|
||||
self.assertFalse(
|
||||
self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bang']})
|
||||
)
|
||||
self.assertTrue(
|
||||
self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bazz']})
|
||||
)
|
||||
|
||||
def test_override_deprecated_policy_with_new_name(self):
|
||||
# Simulate an operator overriding a policy using the new policy name
|
||||
rules = jsonutils.dumps({'foo:create_bar': 'role:bazz'})
|
||||
self.create_config_file('policy.json', rules)
|
||||
|
||||
# Deprecate the policy name and check string in favor of something
|
||||
# better.
|
||||
deprecated_rule = policy.DeprecatedRule(
|
||||
name='foo:bar',
|
||||
check_str='role:fizz'
|
||||
)
|
||||
rule_list = [policy.DocumentedRuleDefault(
|
||||
name='foo:create_bar',
|
||||
check_str='role:bang',
|
||||
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)
|
||||
|
||||
# Make sure the override supplied by the operator is being used in
|
||||
# place of either default value.
|
||||
self.assertFalse(
|
||||
self.enforcer.enforce('foo:create_bar', {}, {'roles': ['fizz']})
|
||||
)
|
||||
self.assertFalse(
|
||||
self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bang']})
|
||||
)
|
||||
self.assertTrue(
|
||||
self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bazz']})
|
||||
)
|
||||
|
||||
def test_override_both_new_and_old_policy(self):
|
||||
# Simulate an operator overriding a policy using both the the new and
|
||||
# old policy names. The following doesn't make a whole lot of sense
|
||||
# because the overrides are conflicting, but we want to make sure that
|
||||
# oslo.policy uses foo:create_bar instead of foo:bar.
|
||||
rules_dict = {
|
||||
'foo:create_bar': 'role:bazz',
|
||||
'foo:bar': 'role:wee'
|
||||
}
|
||||
rules = jsonutils.dumps(rules_dict)
|
||||
self.create_config_file('policy.json', rules)
|
||||
|
||||
# Deprecate the policy name and check string in favor of something
|
||||
# better.
|
||||
deprecated_rule = policy.DeprecatedRule(
|
||||
name='foo:bar',
|
||||
check_str='role:fizz'
|
||||
)
|
||||
rule_list = [policy.DocumentedRuleDefault(
|
||||
name='foo:create_bar',
|
||||
check_str='role:bang',
|
||||
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)
|
||||
|
||||
# The default check string for the old policy name foo:bar should fail
|
||||
self.assertFalse(
|
||||
self.enforcer.enforce('foo:create_bar', {}, {'roles': ['fizz']})
|
||||
)
|
||||
|
||||
# The default check string for the new policy name foo:create_bar
|
||||
# should fail
|
||||
self.assertFalse(
|
||||
self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bang']})
|
||||
)
|
||||
|
||||
# The override for the old policy name foo:bar should fail
|
||||
self.assertFalse(
|
||||
self.enforcer.enforce('foo:create_bar', {}, {'roles': ['wee']})
|
||||
)
|
||||
|
||||
# The override for foo:create_bar should pass
|
||||
self.assertTrue(
|
||||
self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bazz']})
|
||||
)
|
||||
|
||||
|
||||
class DocumentedRuleDefaultTestCase(base.PolicyBaseTestCase):
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue