From 8370a8e0add41604fb6c63753780213b2cd2ba7e Mon Sep 17 00:00:00 2001
From: Tomas Kopecek <tkopecek@redhat.com>
Date: Nov 28 2018 08:01:20 +0000
Subject: don't fail on missing field in base policy tests


Fixes: https://pagure.io/koji/issue/1038

---

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

