Cleanup warnings

Zuul has taken to including warnings in the locations that they're
raised in. We have a few of these in recent jobs so go ahead and clean
them up.

Change-Id: Ifcce20159d872ffd1447ca10f126ae2f2162f956
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>
This commit is contained in:
Stephen Finucane 2020-04-07 15:55:17 +01:00
parent ced4d8eae4
commit a0b254efe5
5 changed files with 46 additions and 17 deletions

View File

@ -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,

View File

@ -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'))

View File

@ -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')

View File

@ -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',
@ -1468,6 +1472,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 +1541,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 +1592,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.
@ -1712,37 +1719,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 +1766,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',

View File

@ -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"})