diff options
| -rw-r--r-- | .zuul.yaml | 2 | ||||
| -rw-r--r-- | oslo_policy/generator.py | 17 | ||||
| -rw-r--r-- | oslo_policy/opts.py | 11 | ||||
| -rw-r--r-- | oslo_policy/policy.py | 15 | ||||
| -rw-r--r-- | oslo_policy/tests/test_generator.py | 6 | ||||
| -rw-r--r-- | oslo_policy/tests/test_parser.py | 9 | ||||
| -rw-r--r-- | oslo_policy/tests/test_policy.py | 82 | ||||
| -rw-r--r-- | oslo_policy/tests/test_shell.py | 4 | ||||
| -rw-r--r-- | releasenotes/notes/enforce_new_defaults-6ae17d8b8d166a2c.yaml | 11 | ||||
| -rw-r--r-- | releasenotes/source/index.rst | 1 | ||||
| -rw-r--r-- | releasenotes/source/ussuri.rst | 6 |
11 files changed, 144 insertions, 20 deletions
@@ -3,7 +3,7 @@ - check-requirements - lib-forward-testing-python3 - openstack-lower-constraints-jobs - - openstack-python3-ussuri-jobs + - openstack-python3-victoria-jobs - periodic-stable-jobs - publish-openstack-docs-pti - release-notes-jobs-python3 diff --git a/oslo_policy/generator.py b/oslo_policy/generator.py index fd9f159..48a9972 100644 --- a/oslo_policy/generator.py +++ b/oslo_policy/generator.py @@ -277,6 +277,9 @@ def _generate_sample(namespaces, output_file=None, output_format='yaml', ',\n '.join(sections_text), '\n}\n')) + if output_file != sys.stdout: + output_file.close() + def _generate_policy(namespace, output_file=None): """Generate a policy file showing what will be used. @@ -304,6 +307,9 @@ def _generate_policy(namespace, output_file=None): for section in _sort_and_format_by_section(policies, include_help=False): output_file.write(section) + if output_file != sys.stdout: + output_file.close() + def _list_redundant(namespace): """Generate a list of configured policies which match defaults. @@ -396,12 +402,11 @@ def upgrade_policy(args=None, conf=None): _upgrade_policies(policies, default_policies) if conf.output_file: - if conf.format == 'yaml': - yaml.safe_dump(policies, open(conf.output_file, 'w'), - default_flow_style=False) - elif conf.format == 'json': - jsonutils.dump(policies, open(conf.output_file, 'w'), - indent=4) + with open(conf.output_file, 'w') as fh: + if conf.format == 'yaml': + yaml.safe_dump(policies, fh, default_flow_style=False) + elif conf.format == 'json': + jsonutils.dump(policies, fh, indent=4) else: if conf.format == 'yaml': sys.stdout.write(yaml.safe_dump(policies, diff --git a/oslo_policy/opts.py b/oslo_policy/opts.py index e0e1dd6..5eb93d7 100644 --- a/oslo_policy/opts.py +++ b/oslo_policy/opts.py @@ -34,6 +34,17 @@ _options = [ 'will be raised. If ``False``, a message will be ' 'logged informing operators that policies are being ' 'invoked with mismatching scope.')), + cfg.BoolOpt('enforce_new_defaults', + default=False, + help=_('This option controls whether or not to use old ' + 'deprecated defaults when evaluating policies. If ' + '``True``, the old deprecated defaults are not going ' + 'to be evaluated. This means if any existing token is ' + 'allowed for old defaults but is disallowed for new ' + 'defaults, it will be disallowed. It is encouraged to ' + 'enable this flag along with the ``enforce_scope`` ' + 'flag so that you can get the benefits of new defaults ' + 'and ``scope_type`` together')), cfg.StrOpt('policy_file', default='policy.json', help=_('The relative or absolute path of a file that maps ' diff --git a/oslo_policy/policy.py b/oslo_policy/policy.py index c1dca53..406ed35 100644 --- a/oslo_policy/policy.py +++ b/oslo_policy/policy.py @@ -503,6 +503,12 @@ class Enforcer(object): self._policy_dir_mtimes = {} self._file_cache = {} self._informed_no_policy_file = False + # NOTE(gmann): This flag will suppress the warning for + # policies changing their default check_str that have + # not been overridden by operators. This does not affect the + # warning for policy changed their name or deprecated + # for removal. + self.suppress_default_change_warnings = False # FOR TESTING ONLY self.suppress_deprecation_warnings = False @@ -540,6 +546,7 @@ class Enforcer(object): self.registered_rules = {} self.file_rules = {} self._informed_no_policy_file = False + self.suppress_default_change_warnings = False self.suppress_deprecation_warnings = False def load_rules(self, force_reload=False): @@ -696,13 +703,17 @@ class Enforcer(object): # 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 + # If the enforce_new_defaults flag is True, do not add OrCheck to the + # old check_str and enforce only the new defaults. + if (not self.conf.oslo_policy.enforce_new_defaults + and deprecated_rule.check_str != default.check_str and default.name not in self.file_rules): default.check = OrCheck([_parser.parse_rule(cs) for cs in [default.check_str, deprecated_rule.check_str]]) - if not self.suppress_deprecation_warnings: + if not (self.suppress_deprecation_warnings + or self.suppress_default_change_warnings): warnings.warn(deprecated_msg) def _undefined_check(self, check): diff --git a/oslo_policy/tests/test_generator.py b/oslo_policy/tests/test_generator.py index 33d420e..86003fe 100644 --- a/oslo_policy/tests/test_generator.py +++ b/oslo_policy/tests/test_generator.py @@ -674,7 +674,8 @@ class UpgradePolicyTestCase(base.PolicyBaseTestCase): with mock.patch('sys.argv', testargs): generator.upgrade_policy(conf=self.local_conf) new_file = self.get_config_file_fullname('new_policy.json') - new_policy = jsonutils.loads(open(new_file, 'r').read()) + with open(new_file, 'r') as fh: + new_policy = jsonutils.loads(fh.read()) self.assertIsNotNone(new_policy.get('new_policy_name')) self.assertIsNone(new_policy.get('deprecated_name')) @@ -693,7 +694,8 @@ class UpgradePolicyTestCase(base.PolicyBaseTestCase): with mock.patch('sys.argv', testargs): generator.upgrade_policy(conf=self.local_conf) new_file = self.get_config_file_fullname('new_policy.yaml') - new_policy = yaml.safe_load(open(new_file, 'r')) + with open(new_file, 'r') as fh: + new_policy = yaml.safe_load(fh) self.assertIsNotNone(new_policy.get('new_policy_name')) self.assertIsNone(new_policy.get('deprecated_name')) diff --git a/oslo_policy/tests/test_parser.py b/oslo_policy/tests/test_parser.py index edfd475..f1ac36b 100644 --- a/oslo_policy/tests/test_parser.py +++ b/oslo_policy/tests/test_parser.py @@ -34,16 +34,20 @@ class ParseCheckTestCase(test_base.BaseTestCase): self.assertIsInstance(result, _checks.TrueCheck) - def test_bad_rule(self): + @mock.patch.object(_parser, 'LOG') + def test_bad_rule(self, mock_log): result = _parser._parse_check('foobar') self.assertIsInstance(result, _checks.FalseCheck) + mock_log.exception.assert_called_once() @mock.patch.object(_checks, 'registered_checks', {}) - def test_no_handler(self): + @mock.patch.object(_parser, 'LOG') + def test_no_handler(self, mock_log): result = _parser._parse_check('no:handler') self.assertIsInstance(result, _checks.FalseCheck) + mock_log.error.assert_called() @mock.patch.object(_checks, 'registered_checks', { 'spam': mock.Mock(return_value='spam_check'), @@ -375,6 +379,7 @@ class ParseTextRuleTestCase(test_base.BaseTestCase): mock_shift.assert_has_calls( [mock.call('tok1', 'val1'), mock.call('tok2', 'val2')]) + @mock.patch.object(_parser, 'LOG', new=mock.Mock()) @mock.patch.object(_parser, '_parse_tokenize', return_value=[]) def test_fail(self, mock_parse_tokenize): result = _parser._parse_text_rule('test rule') diff --git a/oslo_policy/tests/test_policy.py b/oslo_policy/tests/test_policy.py index f3f75b0..b67504b 100644 --- a/oslo_policy/tests/test_policy.py +++ b/oslo_policy/tests/test_policy.py @@ -797,6 +797,7 @@ class EnforcerTest(base.PolicyBaseTestCase): for k, v in expected_creds.items(): self.assertEqual(expected_creds[k], creds[k]) + @mock.patch('warnings.warn', new=mock.Mock()) def test_map_context_attributes_populated_system(self): request_context = context.RequestContext(system_scope='all') expected_creds = request_context.to_policy_values() @@ -1161,6 +1162,7 @@ class RuleDefaultTestCase(base.PolicyBaseTestCase): class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): + @mock.patch('warnings.warn', new=mock.Mock()) def test_deprecate_a_policy_check_string(self): deprecated_rule = policy.DeprecatedRule( name='foo:create_bar', @@ -1200,6 +1202,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): enforcer.enforce('foo:create_bar', {}, {'roles': ['baz']}) ) + @mock.patch('warnings.warn', new=mock.Mock()) def test_deprecate_an_empty_policy_check_string(self): deprecated_rule = policy.DeprecatedRule( name='foo:create_bar', @@ -1227,6 +1230,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): enforcer.enforce('foo:create_bar', {}, {'roles': ['fizz']}, do_raise=True) + @mock.patch('warnings.warn', new=mock.Mock()) def test_deprecate_replace_with_empty_policy_check_string(self): deprecated_rule = policy.DeprecatedRule( name='foo:create_bar', @@ -1415,6 +1419,28 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): enforcer.load_rules() mock_warn.assert_not_called() + def test_suppress_default_change_warnings_flag_not_log_warning(self): + deprecated_rule = policy.DeprecatedRule( + name='foo:create_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' + )] + enforcer = policy.Enforcer(self.conf) + enforcer.suppress_default_change_warnings = True + 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, @@ -1468,6 +1494,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): deprecated_since='N' ) + @mock.patch('warnings.warn', new=mock.Mock()) def test_override_deprecated_policy_with_old_name(self): # Simulate an operator overriding a policy rules = jsonutils.dumps({'foo:bar': 'role:bazz'}) @@ -1536,6 +1563,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bazz']}) ) + @mock.patch('warnings.warn', new=mock.Mock()) 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 @@ -1586,6 +1614,7 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): self.enforcer.enforce('foo:create_bar', {}, {'roles': ['bazz']}) ) + @mock.patch('warnings.warn', new=mock.Mock()) def test_override_deprecated_policy_with_new_rule(self): # Simulate an operator overriding a deprecated policy with a reference # to the new policy, as done by the sample policy generator. @@ -1619,6 +1648,39 @@ class DocumentedRuleDefaultDeprecationTestCase(base.PolicyBaseTestCase): # Verify that we didn't overwrite the new rule. self.assertEqual('bang', self.enforcer.rules['new_rule'].match) + def test_enforce_new_defaults_no_old_check_string(self): + self.conf.set_override('enforce_new_defaults', True, + group='oslo_policy') + deprecated_rule = policy.DeprecatedRule( + name='foo:create_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' + )] + 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() + self.assertTrue( + enforcer.enforce('foo:create_bar', {}, {'roles': ['bang']}) + ) + self.assertFalse( + enforcer.enforce('foo:create_bar', {}, {'roles': ['fizz']}) + ) + self.assertFalse( + enforcer.enforce('foo:create_bar', {}, {'roles': ['baz']}) + ) + class DocumentedRuleDefaultTestCase(base.PolicyBaseTestCase): @@ -1712,37 +1774,46 @@ class EnforcerCheckRulesTest(base.PolicyBaseTestCase): self.enforcer.load_rules(True) self.assertTrue(self.enforcer.check_rules(raise_on_violation=True)) - def test_undefined_rule(self): + @mock.patch.object(policy, 'LOG') + def test_undefined_rule(self, mock_log): rules = jsonutils.dumps({'foo': 'rule:bar'}) self.create_config_file('policy.json', rules) self.enforcer.load_rules(True) self.assertFalse(self.enforcer.check_rules()) + mock_log.warning.assert_called() - def test_undefined_rule_raises(self): + @mock.patch.object(policy, 'LOG') + def test_undefined_rule_raises(self, mock_log): rules = jsonutils.dumps({'foo': 'rule:bar'}) self.create_config_file('policy.json', rules) self.enforcer.load_rules(True) self.assertRaises(policy.InvalidDefinitionError, self.enforcer.check_rules, raise_on_violation=True) + mock_log.warning.assert_called() - def test_cyclical_rules(self): + @mock.patch.object(policy, 'LOG') + def test_cyclical_rules(self, mock_log): rules = jsonutils.dumps({'foo': 'rule:bar', 'bar': 'rule:foo'}) self.create_config_file('policy.json', rules) self.enforcer.load_rules(True) self.assertFalse(self.enforcer.check_rules()) + mock_log.warning.assert_called() - def test_cyclical_rules_raises(self): + @mock.patch.object(policy, 'LOG') + def test_cyclical_rules_raises(self, mock_log): rules = jsonutils.dumps({'foo': 'rule:bar', 'bar': 'rule:foo'}) self.create_config_file('policy.json', rules) self.enforcer.load_rules(True) self.assertRaises(policy.InvalidDefinitionError, self.enforcer.check_rules, raise_on_violation=True) + mock_log.warning.assert_called() - def test_complex_cyclical_rules_false(self): + @mock.patch.object(policy, 'LOG') + def test_complex_cyclical_rules_false(self, mock_log): rules = jsonutils.dumps({'foo': 'rule:bar', 'bar': 'rule:baz and role:admin', 'baz': 'rule:foo or role:user'}) @@ -1750,6 +1821,7 @@ class EnforcerCheckRulesTest(base.PolicyBaseTestCase): self.enforcer.load_rules(True) self.assertFalse(self.enforcer.check_rules()) + mock_log.warning.assert_called() def test_complex_cyclical_rules_true(self): rules = jsonutils.dumps({'foo': 'rule:bar or rule:baz', diff --git a/oslo_policy/tests/test_shell.py b/oslo_policy/tests/test_shell.py index 0eb6aa6..b27347c 100644 --- a/oslo_policy/tests/test_shell.py +++ b/oslo_policy/tests/test_shell.py @@ -229,7 +229,7 @@ passed: sampleservice:sample_rule2 self.create_config_file( "target.json", jsonutils.dumps(target)) - target_file = open(self.get_config_file_fullname('target.json'), 'r') - target_from_file = target_file.read() + with open(self.get_config_file_fullname('target.json'), 'r') as fh: + target_from_file = fh.read() result = shell.flatten(jsonutils.loads(target_from_file)) self.assertEqual(result, {"target.secret.project_id": "1234"}) diff --git a/releasenotes/notes/enforce_new_defaults-6ae17d8b8d166a2c.yaml b/releasenotes/notes/enforce_new_defaults-6ae17d8b8d166a2c.yaml new file mode 100644 index 0000000..4a537de --- /dev/null +++ b/releasenotes/notes/enforce_new_defaults-6ae17d8b8d166a2c.yaml @@ -0,0 +1,11 @@ +features: + - | + A new configuration option ``enforce_new_defaults`` has been + added to the ``[oslo_policy]`` group to control whether or not to + use the old deprecated defaults. If ``True``, the old deprecated + defaults are not going to be evaluated which means if any existing + token is allowed for old defaults but disallowed for new defaults + it will be disallowed. It is encouraged to enable this flag along + with the ``enforce_scope`` flag so that you can get the benefits of + new defaults and ``scope_type`` together. This way operators can switch + to new defaults without overwriting the rules in the policy file. diff --git a/releasenotes/source/index.rst b/releasenotes/source/index.rst index 4d7b133..2ca8034 100644 --- a/releasenotes/source/index.rst +++ b/releasenotes/source/index.rst @@ -6,6 +6,7 @@ :maxdepth: 1 unreleased + ussuri train stein rocky diff --git a/releasenotes/source/ussuri.rst b/releasenotes/source/ussuri.rst new file mode 100644 index 0000000..e21e50e --- /dev/null +++ b/releasenotes/source/ussuri.rst @@ -0,0 +1,6 @@ +=========================== +Ussuri Series Release Notes +=========================== + +.. release-notes:: + :branch: stable/ussuri |