From 47912f047028db1dc6cd520b657a24874f7a6240 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 28 2018 20:08:57 +0000 Subject: PR#1042: don't fail on missing field in base policy tests Merges #1042 https://pagure.io/koji/pull-request/1042 Fixes: #1038 https://pagure.io/koji/issue/1038 policy tests should handle None/missing values gracefully --- diff --git a/koji/policy.py b/koji/policy.py index 3a7733e..bb0ed00 100644 --- a/koji/policy.py +++ b/koji/policy.py @@ -103,6 +103,8 @@ class BoolTest(BaseSimpleTest): else: # expected when we are subclassed field = self.field + if field not in data: + return False return bool(data[field]) @@ -126,6 +128,8 @@ class MatchTest(BaseSimpleTest): else: # expected when we are subclassed field = self.field + if field not in data: + return False for pattern in args: if fnmatch.fnmatch(data[field], pattern): return True @@ -176,6 +180,8 @@ class CompareTest(BaseSimpleTest): self.value = float(value) def run(self, data): + if self.field not in data: + return False return self.func(data[self.field], self.value) diff --git a/tests/test_hub/test_check_volume_policy.py b/tests/test_hub/test_check_volume_policy.py index 9531d28..f3b6a8b 100644 --- a/tests/test_hub/test_check_volume_policy.py +++ b/tests/test_hub/test_check_volume_policy.py @@ -49,7 +49,7 @@ class TestCheckVolumePolicy(unittest.TestCase): ret = kojihub.check_volume_policy({}) self.assertEqual(ret, None) - with self.assertRaises(KeyError): + with self.assertRaises(koji.GenericError): kojihub.check_volume_policy({}, strict=True) self.lookup_name.assert_not_called() diff --git a/tests/test_lib/test_policy.py b/tests/test_lib/test_policy.py new file mode 100644 index 0000000..d2177df --- /dev/null +++ b/tests/test_lib/test_policy.py @@ -0,0 +1,308 @@ +from __future__ import absolute_import +try: + import unittest2 as unittest +except ImportError: + import unittest + +from nose.tools import raises + +import koji.policy + + +class MyBoolTest(koji.policy.BoolTest): + name = 'bool_check' + field = 'bool_field' + + +class MyMatchTest(koji.policy.MatchTest): + name = 'match_check' + field = 'match_field' + + +class myvarTest(koji.policy.CompareTest): + name = None + field = 'myvar' + allow_float = False + + +class TestBasicTests(unittest.TestCase): + + @raises(NotImplementedError) + def test_base_test(self): + obj = koji.policy.BaseSimpleTest('something') + obj.run({}) + + def test_true_test(self): + obj = koji.policy.TrueTest('something') + self.assertTrue(obj.run({})) + + def test_false_test(self): + obj = koji.policy.FalseTest('something') + self.assertFalse(obj.run({})) + + def test_all_test(self): + obj = koji.policy.AllTest('something') + self.assertTrue(obj.run({})) + + def test_none_test(self): + obj = koji.policy.NoneTest('something') + self.assertFalse(obj.run({})) + + def test_has_test(self): + obj = koji.policy.HasTest('some thing') + self.assertFalse(obj.run({})) + self.assertFalse(obj.run({'blah': 'blah'})) + self.assertTrue(obj.run({'thing': 'blah'})) + self.assertRaises(koji.GenericError, koji.policy.HasTest, 'something') + + def test_bool_test(self): + obj = koji.policy.BoolTest('some thing') + self.assertFalse(obj.run({'thing': None})) + self.assertFalse(obj.run({'thing': []})) + self.assertFalse(obj.run({})) + self.assertTrue(obj.run({'thing': 'yes'})) + + def test_match_test(self): + obj = koji.policy.MatchTest('some thing else') + self.assertFalse(obj.run({'thing': 'elseplus'})) + obj = koji.policy.MatchTest('some thing else*') + self.assertTrue(obj.run({'thing': 'elseplus'})) + self.assertFalse(obj.run({})) + + def test_compare_test(self): + obj = koji.policy.CompareTest('compare thing > 2') + self.assertFalse(obj.run({'thing': 1})) + self.assertFalse(obj.run({'thing': 2})) + self.assertTrue(obj.run({'thing': 3})) + self.assertFalse(obj.run({})) + + obj = koji.policy.CompareTest('compare thing < 1.5') + self.assertFalse(obj.run({'thing': 3.2})) + self.assertTrue(obj.run({'thing': 1.0})) + self.assertFalse(obj.run({})) + + obj = koji.policy.CompareTest('compare thing = 42') + self.assertFalse(obj.run({'thing': 54})) + self.assertTrue(obj.run({'thing': 42})) + self.assertFalse(obj.run({})) + + obj = koji.policy.CompareTest('compare thing != 99') + self.assertFalse(obj.run({'thing': 99})) + self.assertTrue(obj.run({'thing': 100})) + self.assertFalse(obj.run({})) + + obj = koji.policy.CompareTest('compare thing >= 2') + self.assertFalse(obj.run({'thing': 1})) + self.assertTrue(obj.run({'thing': 2})) + self.assertTrue(obj.run({'thing': 3})) + self.assertFalse(obj.run({})) + + obj = koji.policy.CompareTest('compare thing <= 5') + self.assertFalse(obj.run({'thing': 23})) + self.assertTrue(obj.run({'thing': 5})) + self.assertTrue(obj.run({'thing': 0})) + self.assertFalse(obj.run({})) + + @raises(koji.GenericError) + def test_invalid_compare_test(self): + koji.policy.CompareTest('some thing LOL 2') + + +class TestDiscovery(unittest.TestCase): + + def test_find_simple_tests(self): + actual = koji.policy.findSimpleTests(koji.policy.__dict__) + expected = { + 'all': koji.policy.AllTest, + 'bool': koji.policy.BoolTest, + 'compare': koji.policy.CompareTest, + 'false': koji.policy.FalseTest, + 'has': koji.policy.HasTest, + 'match': koji.policy.MatchTest, + 'none': koji.policy.NoneTest, + 'true': koji.policy.TrueTest, + } + self.assertDictEqual(expected, actual) + + +class TestRuleHandling(unittest.TestCase): + + def test_simple_rule_set_instantiation(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + rules = ['true :: allow'] + koji.policy.SimpleRuleSet(rules, tests) + + def test_simple_rule_set_all_actions(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + rules = ['true :: allow'] + obj = koji.policy.SimpleRuleSet(rules, tests) + result = obj.all_actions() + self.assertEquals(result, ['allow']) + + def test_simple_rule_set_apply(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + rules = ['true :: allow'] + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEqual(action, 'allow') + + rules = ['false :: allow'] + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEqual(action, None) + + def test_custom_rules(self): + tests = koji.policy.findSimpleTests([globals(), koji.policy.__dict__]) + + rules = ['bool_check :: True', 'all :: False'] + for val in True, False: + data = {'bool_field' : val} + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEqual(action, str(val)) + + rules = ['match_check foo* :: foo', 'match_check * :: bar'] + data = {'match_field' : 'foo1234'} + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEqual(action, 'foo') + + data = {'match_field' : 'not foo'} + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEqual(action, 'bar') + + data = {'myvar': 37} + rules = ['myvar = 37 :: get back here'] + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEqual(action, 'get back here') + + rules = ['myvar = 2.718281828 :: euler'] + with self.assertRaises(ValueError): + obj = koji.policy.SimpleRuleSet(rules, tests) + + def test_last_rule(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + # no match + rules = ['none :: allow'] + obj = koji.policy.SimpleRuleSet(rules, tests) + self.assertEquals(obj.last_rule(), None) + action = obj.apply(data) + self.assertEquals(obj.last_rule(), '(no match)') + + # simple rule + rules = ['all :: allow'] + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEquals(obj.last_rule(), rules[0]) + + # negate rule + rules = ['none !! allow'] + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEquals(obj.last_rule(), rules[0]) + + # nested rule + policy = ''' +all :: { + all :: { + all :: allow + } +} +''' + rules = policy.splitlines() + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + expected = 'all :: ... all :: ... all :: allow' + self.assertEquals(obj.last_rule(), expected) + + def test_unclosed_brace(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + lines = ['true :: {'] + with self.assertRaises(koji.GenericError): + obj = koji.policy.SimpleRuleSet(lines, tests) + + def test_unmatched_brace(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + lines = ['true :: }'] + with self.assertRaises(koji.GenericError): + obj = koji.policy.SimpleRuleSet(lines, tests) + + def test_no_action(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + lines = ['true && true'] + with self.assertRaises(Exception): + obj = koji.policy.SimpleRuleSet(lines, tests) + + def test_missing_handler(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + lines = ['NOSUCHHANDLER && true :: allow'] + with self.assertRaises(koji.GenericError): + obj = koji.policy.SimpleRuleSet(lines, tests) + + def test_complex_policy(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + policy = ''' +# This is a comment in the test policy + +#^blank line +# commented test && true :: some result + +# First some rules that should never match +false :: ERROR +none :: ERROR + +true !! ERROR +all !! ERROR + +false && true && true :: ERROR +none && true && true :: ERROR + +has NOSUCHFIELD :: ERROR + +# nesting +has DEPTH :: { + match DEPTH 1 :: 1 + all :: { + match DEPTH 2 :: 2 + all :: { + match DEPTH 3 :: 3 + all :: { + match DEPTH 4 :: 4 + all :: END + } + } + } +} +''' + + lines = policy.splitlines() + + for depth in ['1', '2', '3', '4']: + data = {'DEPTH': depth} + obj = koji.policy.SimpleRuleSet(lines, tests) + action = obj.apply(data) + self.assertEqual(action, depth) + + data = {'DEPTH': '99'} + obj = koji.policy.SimpleRuleSet(lines, tests) + action = obj.apply(data) + self.assertEqual(action, 'END') + + actions = set(obj.all_actions()) + self.assertEquals(actions, set(['1', '2', '3', '4', 'ERROR', 'END'])) diff --git a/tests/test_lib_py2only/test_policy.py b/tests/test_lib_py2only/test_policy.py deleted file mode 100644 index 661fda3..0000000 --- a/tests/test_lib_py2only/test_policy.py +++ /dev/null @@ -1,302 +0,0 @@ -from __future__ import absolute_import -try: - import unittest2 as unittest -except ImportError: - import unittest - -from nose.tools import raises - -import koji.policy - - -class MyBoolTest(koji.policy.BoolTest): - name = 'bool_check' - field = 'bool_field' - - -class MyMatchTest(koji.policy.MatchTest): - name = 'match_check' - field = 'match_field' - - -class myvarTest(koji.policy.CompareTest): - name = None - field = 'myvar' - allow_float = False - - -class TestBasicTests(unittest.TestCase): - - @raises(NotImplementedError) - def test_base_test(self): - obj = koji.policy.BaseSimpleTest('something') - obj.run({}) - - def test_true_test(self): - obj = koji.policy.TrueTest('something') - self.assertTrue(obj.run({})) - - def test_false_test(self): - obj = koji.policy.FalseTest('something') - self.assertFalse(obj.run({})) - - def test_all_test(self): - obj = koji.policy.AllTest('something') - self.assertTrue(obj.run({})) - - def test_none_test(self): - obj = koji.policy.NoneTest('something') - self.assertFalse(obj.run({})) - - def test_has_test(self): - obj = koji.policy.HasTest('some thing') - self.assertFalse(obj.run({})) - self.assertFalse(obj.run({'blah': 'blah'})) - self.assertTrue(obj.run({'thing': 'blah'})) - self.assertRaises(koji.GenericError, koji.policy.HasTest, 'something') - - def test_bool_test(self): - obj = koji.policy.BoolTest('some thing') - self.assertFalse(obj.run({'thing': None})) - self.assertFalse(obj.run({'thing': []})) - self.assertTrue(obj.run({'thing': 'yes'})) - - def test_match_test(self): - obj = koji.policy.MatchTest('some thing else') - self.assertFalse(obj.run({'thing': 'elseplus'})) - obj = koji.policy.MatchTest('some thing else*') - self.assertTrue(obj.run({'thing': 'elseplus'})) - - def test_compare_test(self): - obj = koji.policy.CompareTest('compare thing > 2') - self.assertFalse(obj.run({'thing': 1})) - self.assertFalse(obj.run({'thing': 2})) - self.assertTrue(obj.run({'thing': 3})) - - obj = koji.policy.CompareTest('compare thing < 1.5') - self.assertFalse(obj.run({'thing': 3.2})) - self.assertTrue(obj.run({'thing': 1.0})) - - obj = koji.policy.CompareTest('compare thing = 42') - self.assertFalse(obj.run({'thing': 54})) - self.assertTrue(obj.run({'thing': 42})) - - obj = koji.policy.CompareTest('compare thing != 99') - self.assertFalse(obj.run({'thing': 99})) - self.assertTrue(obj.run({'thing': 100})) - - obj = koji.policy.CompareTest('compare thing >= 2') - self.assertFalse(obj.run({'thing': 1})) - self.assertTrue(obj.run({'thing': 2})) - self.assertTrue(obj.run({'thing': 3})) - - obj = koji.policy.CompareTest('compare thing <= 5') - self.assertFalse(obj.run({'thing': 23})) - self.assertTrue(obj.run({'thing': 5})) - self.assertTrue(obj.run({'thing': 0})) - - @raises(koji.GenericError) - def test_invalid_compare_test(self): - koji.policy.CompareTest('some thing LOL 2') - - -class TestDiscovery(unittest.TestCase): - - def test_find_simple_tests(self): - actual = koji.policy.findSimpleTests(koji.policy.__dict__) - expected = { - 'all': koji.policy.AllTest, - 'bool': koji.policy.BoolTest, - 'compare': koji.policy.CompareTest, - 'false': koji.policy.FalseTest, - 'has': koji.policy.HasTest, - 'match': koji.policy.MatchTest, - 'none': koji.policy.NoneTest, - 'true': koji.policy.TrueTest, - } - self.assertDictEqual(expected, actual) - - -class TestRuleHandling(unittest.TestCase): - - def test_simple_rule_set_instantiation(self): - tests = koji.policy.findSimpleTests(koji.policy.__dict__) - rules = ['true :: allow'] - koji.policy.SimpleRuleSet(rules, tests) - - def test_simple_rule_set_all_actions(self): - tests = koji.policy.findSimpleTests(koji.policy.__dict__) - rules = ['true :: allow'] - obj = koji.policy.SimpleRuleSet(rules, tests) - result = obj.all_actions() - self.assertEquals(result, ['allow']) - - def test_simple_rule_set_apply(self): - tests = koji.policy.findSimpleTests(koji.policy.__dict__) - data = {} - - rules = ['true :: allow'] - obj = koji.policy.SimpleRuleSet(rules, tests) - action = obj.apply(data) - self.assertEqual(action, 'allow') - - rules = ['false :: allow'] - obj = koji.policy.SimpleRuleSet(rules, tests) - action = obj.apply(data) - self.assertEqual(action, None) - - def test_custom_rules(self): - tests = koji.policy.findSimpleTests([globals(), koji.policy.__dict__]) - - rules = ['bool_check :: True', 'all :: False'] - for val in True, False: - data = {'bool_field' : val} - obj = koji.policy.SimpleRuleSet(rules, tests) - action = obj.apply(data) - self.assertEqual(action, str(val)) - - rules = ['match_check foo* :: foo', 'match_check * :: bar'] - data = {'match_field' : 'foo1234'} - obj = koji.policy.SimpleRuleSet(rules, tests) - action = obj.apply(data) - self.assertEqual(action, 'foo') - - data = {'match_field' : 'not foo'} - obj = koji.policy.SimpleRuleSet(rules, tests) - action = obj.apply(data) - self.assertEqual(action, 'bar') - - data = {'myvar': 37} - rules = ['myvar = 37 :: get back here'] - obj = koji.policy.SimpleRuleSet(rules, tests) - action = obj.apply(data) - self.assertEqual(action, 'get back here') - - rules = ['myvar = 2.718281828 :: euler'] - with self.assertRaises(ValueError): - obj = koji.policy.SimpleRuleSet(rules, tests) - - def test_last_rule(self): - tests = koji.policy.findSimpleTests(koji.policy.__dict__) - data = {} - - # no match - rules = ['none :: allow'] - obj = koji.policy.SimpleRuleSet(rules, tests) - self.assertEquals(obj.last_rule(), None) - action = obj.apply(data) - self.assertEquals(obj.last_rule(), '(no match)') - - # simple rule - rules = ['all :: allow'] - obj = koji.policy.SimpleRuleSet(rules, tests) - action = obj.apply(data) - self.assertEquals(obj.last_rule(), rules[0]) - - # negate rule - rules = ['none !! allow'] - obj = koji.policy.SimpleRuleSet(rules, tests) - action = obj.apply(data) - self.assertEquals(obj.last_rule(), rules[0]) - - # nested rule - policy = ''' -all :: { - all :: { - all :: allow - } -} -''' - rules = policy.splitlines() - obj = koji.policy.SimpleRuleSet(rules, tests) - action = obj.apply(data) - expected = 'all :: ... all :: ... all :: allow' - self.assertEquals(obj.last_rule(), expected) - - def test_unclosed_brace(self): - tests = koji.policy.findSimpleTests(koji.policy.__dict__) - data = {} - - lines = ['true :: {'] - with self.assertRaises(koji.GenericError): - obj = koji.policy.SimpleRuleSet(lines, tests) - - def test_unmatched_brace(self): - tests = koji.policy.findSimpleTests(koji.policy.__dict__) - data = {} - - lines = ['true :: }'] - with self.assertRaises(koji.GenericError): - obj = koji.policy.SimpleRuleSet(lines, tests) - - def test_no_action(self): - tests = koji.policy.findSimpleTests(koji.policy.__dict__) - data = {} - - lines = ['true && true'] - with self.assertRaises(Exception): - obj = koji.policy.SimpleRuleSet(lines, tests) - - def test_missing_handler(self): - tests = koji.policy.findSimpleTests(koji.policy.__dict__) - data = {} - - lines = ['NOSUCHHANDLER && true :: allow'] - with self.assertRaises(koji.GenericError): - obj = koji.policy.SimpleRuleSet(lines, tests) - - def test_complex_policy(self): - tests = koji.policy.findSimpleTests(koji.policy.__dict__) - data = {} - - policy = ''' -# This is a comment in the test policy - -#^blank line -# commented test && true :: some result - -# First some rules that should never match -false :: ERROR -none :: ERROR - -true !! ERROR -all !! ERROR - -false && true && true :: ERROR -none && true && true :: ERROR - -has NOSUCHFIELD :: ERROR - -# nesting -has DEPTH :: { - match DEPTH 1 :: 1 - all :: { - match DEPTH 2 :: 2 - all :: { - match DEPTH 3 :: 3 - all :: { - match DEPTH 4 :: 4 - all :: END - } - } - } -} -''' - - lines = policy.splitlines() - - for depth in ['1', '2', '3', '4']: - data = {'DEPTH': depth} - obj = koji.policy.SimpleRuleSet(lines, tests) - action = obj.apply(data) - self.assertEqual(action, depth) - - data = {'DEPTH': '99'} - obj = koji.policy.SimpleRuleSet(lines, tests) - action = obj.apply(data) - self.assertEqual(action, 'END') - - actions = set(obj.all_actions()) - self.assertEquals(actions, set(['1', '2', '3', '4', 'ERROR', 'END'])) - -