From d37c0e9c2eba9e1c551d1f57e9d9bb5a02b92d7f Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 27 2026 22:30:38 +0000 Subject: [PATCH 1/5] support multi-action policies and negated tests Based partially on work by cobrien from 2009. This extends the existing policy handling code as follows * Supports multi match rules * Supports negated tests * Supports break N action * Supports stop action * Backwards compatible * adds unit tests for new functionality * requires no fixes to existing unit tests --- diff --git a/koji/policy.py b/koji/policy.py index 2e6706f..3341860 100644 --- a/koji/policy.py +++ b/koji/policy.py @@ -46,6 +46,22 @@ class BaseSimpleTest(object): return self.str +class NegatedTest(object): + + def __init__(self, test): + self.test = test + + @property + def name(self): + return "!%s" % self.test.name + + def run(self, data): + return not self.test.run(data) + + def __str__(self): + return "! %s" % self.test + + # The following tests are generic enough that we can place them here class TrueTest(BaseSimpleTest): @@ -257,21 +273,53 @@ class CompareTest(BaseSimpleTest): return self.func(data[self.field], self.value) +class BaseAction(object): + '''Abstract base class for actions''' + + +class PolicyAction(BaseAction): + '''Normal actions from policies''' + + def __init__(self, text, name): + self.text = text + self.name = name + + def __str__(self): + return self.text + + +class BreakAction(BaseAction): + '''A flow control action. Breaks out of nested rule sets''' + + def __init__(self, depth=1): + self.depth = depth + + def __str__(self): + return 'break %i' % self.depth + + +class StopAction(BaseAction): + '''A flow control action. Stops policy execution''' + + def __str__(self): + return 'stop' + + class SimpleRuleSet(object): def __init__(self, rules, tests): self.tests = tests self.rules = self.parse_rules(rules) - self.lastrule = None - self.lastaction = None + self.ruleset = self.rules # alias for backwards compatibility self.logger = logging.getLogger('koji.policy') + self.checker = None def parse_rules(self, lines): """Parse rules into a ruleset data structure At the top level, the structure is a set of rules [rule1, rule2, ...] - Each rule is a pair + Each rule is a list [tests, negate, action ] Tests is a list of test handlers: [handler1, handler2, ...] @@ -286,8 +334,7 @@ class SimpleRuleSet(object): [[test1, test2, test3], negate [[[test1, test2], negate, "action"]]]]]] """ - cursor = [] - self.ruleset = cursor + rules = cursor = [] stack = [] for line in lines: rule = self.parse_line(line) @@ -310,6 +357,7 @@ class SimpleRuleSet(object): if stack: # unclosed { raise koji.GenericError("nesting error in rule set") + return rules def parse_line(self, line): """Parse line as a rule @@ -346,22 +394,62 @@ class SimpleRuleSet(object): if pos == -1: raise Exception("bad policy line: %s" % line) negate = True - tests = line[:pos] + tests = self.parse_tests(line[:pos]) action = line[pos + 2:] - tests = [self.get_test_handler(x) for x in tests.split('&&')] - action = action.strip() + action = self.parse_action(action.strip()) # just return action = { for nested rules return tests, negate, action - def get_test_handler(self, str): - name = str.split(None, 1)[0] + def parse_tests(self, s): + """Given the tests portion of a policy line, return list of tests""" + return [self.get_test_handler(x) for x in s.split('&&')] + + def parse_action(self, action): + if action in ['{', '}']: + # these are handled in parse_rules + return action + name = action.split(None, 1)[0] + if name == 'break': + args = action.split()[1:] + if not args: + return BreakAction() + elif len(args) > 1: + raise koji.GenericError('Invalid break action: %s' % action) + else: + try: + depth = int(args[0]) + except ValueError: + raise koji.GenericError('Invalid break action: %s' % action) + return BreakAction(depth) + elif name == 'stop': + args = action.split()[1:] + if args: + raise koji.GenericError('Invalid stop action: %s' % action) + return StopAction() + else: + return PolicyAction(action, name) + + def get_test_handler(self, test): + negate = False + try: + parts = test.split(None, 1) + name = parts[0] + if name == '!': + negate = True + test = parts[1] + name = test.split(None, 1)[0] + except IndexError: + raise koji.GenericError("missing/invalid test: %r" % test) try: - return self.tests[name](str) + handler = self.tests[name](test) + if negate: + handler = NegatedTest(handler) + return handler except KeyError: raise koji.GenericError("missing test handler: %s" % name) def all_actions(self): - """report a list of all actions in the ruleset + """report a list of all possible actions in the ruleset (only the first word of the action is considered) """ @@ -369,56 +457,124 @@ class SimpleRuleSet(object): for tests, negate, action in rules: if isinstance(action, list): _recurse(action, index) - else: - name = action.split(None, 1)[0] - index[name] = 1 + elif isinstance(action, PolicyAction): + index[action.name] = 1 + # ignore other special actions like break index = {} - _recurse(self.ruleset, index) + _recurse(self.rules, index) return to_list(index.keys()) - def _apply(self, rules, data, top=False): + def apply(self, data, multi=False): + self.checker = RuleChecker(self, data) + return self.checker.apply(multi=multi) + + def last_rule(self): + # wrapper for backwards compatibility + if self.checker: + return self.checker.last_rule() + else: + return None + + +class RuleChecker(object): + + def __init__(self, ruleset, data): + self.ruleset = ruleset + self.data = data + self.logger = logging.getLogger('koji.policy') + self.lastrule = None + self.lastaction = None + self.lastrun = None + + def apply(self, multi=False): + # backwards compatible interface + self.run(multi=multi) + if multi: + return [r['action'].text for r in self.lastrun['results']] + elif self.lastaction: + return self.lastaction.text + else: + return None + + def run(self, multi=True): + self.logger.debug("policy start") + self.lastaction = None + self.lastrule = [] + results = [] + self.lastrun = {'multi': multi, 'results': results} + for action, trace in self._apply(self.ruleset.rules): + self.lastaction = action + self.lastrule = trace + results.append({'action': action, 'trace': trace}) + if not multi: + break + self.logger.debug("policy done") + return self.lastrun + + def _apply(self, rules, trace=[]): + """Apply rules recursively, yielding matching actions""" for tests, negate, action in rules: + self.logger.debug("Rule: %s", rule_str(tests, negate, action)) - if top: - self.lastrule = [] - value = False + value = True + # the parser does not accept rules with no tests, so tests cannot be empty for test in tests: - check = test.run(data) + check = test.run(self.data) self.logger.debug(" %s -> %s", test, check) if not check: + value = False break - else: - # all tests in current rule passed - value = True + if negate: value = not value + if value: - self.lastrule.append([tests, negate]) + next_trace = list(trace) + next_trace.append((tests, negate)) if isinstance(action, list): self.logger.debug(" matched: entering subrule") # action is a list of subrules - ret = self._apply(action, data) - if ret is not None: - return ret - # if ret is None, then none of the subrules matched, - # so we keep going + for result in self._apply(rules=action, trace=next_trace): + if isinstance(result, BreakAction): + if result.depth > 1 and trace: + self.logger.debug(" passing break up the line") + yield BreakAction(result.depth - 1) + return + elif isinstance(result, StopAction): + if trace: + yield result + return + else: + yield result + elif isinstance(action, BreakAction): + self.logger.debug(" rule matched: %s", action) + if action.depth > 1 and trace: + self.logger.debug(" passing break up the line") + # also tell our parent to break + yield BreakAction(action.depth - 1) + self.logger.debug(" break: skipping rest of level") + return + elif isinstance(action, StopAction): + self.logger.debug(" rule matched: %s", action) + if trace: + yield StopAction() + return else: self.logger.debug(" rule matched: %s", action) - return action - return None - - def apply(self, data): - self.logger.debug("policy start") - self.lastrule = [] - self.lastaction = self._apply(self.ruleset, data, top=True) - self.logger.debug("policy done") - return self.lastaction + yield (action, next_trace) def last_rule(self): - if self.lastrule is None: + # backwards compatible trace + if not self.lastrun: return None + elif not self.lastrun['results']: + return "(no match)" + result = self.lastrun['results'][-1] + return self.pretty_trace(result) + + def pretty_trace(self, result): ret = [] - for (tests, negate) in self.lastrule: + for (tests, negate) in result['trace']: line = '&&'.join([str(t) for t in tests]) if negate: line += '!! ' @@ -426,10 +582,10 @@ class SimpleRuleSet(object): line += ':: ' ret.append(line) ret = '... '.join(ret) - if self.lastaction is None: + if result['action'] is None: ret += "(no match)" else: - ret += self.lastaction + ret += result['action'].text return ret diff --git a/tests/test_lib/test_multipolicy.py b/tests/test_lib/test_multipolicy.py new file mode 100644 index 0000000..6956208 --- /dev/null +++ b/tests/test_lib/test_multipolicy.py @@ -0,0 +1,125 @@ + +from __future__ import absolute_import +from __future__ import print_function +import unittest + +from koji.policy import BaseSimpleTest, AllTest, SimpleRuleSet, RuleChecker + + +ruleset = """ + +mammal && goes moo :: cow +mammal && goes meow :: cat +mammal && flies :: { + goes moo :: flying cow + goes moo !! bat +} + +mammal !! { + flies :: { + goes squish :: bug + goes squawk chirp :: bird + } + flies !! { + goes splish splash :: fish + goes squish :: creepy crawly bug + all :: some other thing + } +} + +""" + + +class PolicyTestMammal(BaseSimpleTest): + + name = "mammal" + + def run(self, data): + critter = data['critter'] + return critter.mammal + + +class PolicyTestFlies(BaseSimpleTest): + + name = "flies" + + def run(self, data): + critter = data['critter'] + return critter.flies + + +class PolicyTestGoes(BaseSimpleTest): + + name = "goes" + + def __init__(self, args): + BaseSimpleTest.__init__(self, args) + self.noises = args.strip().split()[1:] + + def run(self, data): + critter = data['critter'] + return critter.goes in self.noises + + +class critter(object): + def __init__(self, mammal=True, goes="moo", flies=False): + self.mammal = mammal + self.goes = goes + self.flies = flies + + +class TestBasicTests(unittest.TestCase): + + def setUp(self): + from koji.policy import findSimpleTests + tests = findSimpleTests(globals()) + self.rules = SimpleRuleSet(ruleset.splitlines(), tests) + + test_params = [ + # [result, critter kwds] + [['cow'], + {}], # default critter + [['cat'], + {'mammal': True, 'goes': 'meow', 'flies': False}], + [['cow', 'flying cow'], + {'mammal': True, 'goes': 'moo', 'flies': True}], + [['fish', 'some other thing'], + {'mammal': False, 'goes': 'splash', 'flies': False}], + [['bug'], + {'mammal': False, 'goes': 'squish', 'flies': True}], + [['bird'], + {'mammal': False, 'goes': "chirp", 'flies': True}], + [['bat'], + {'mammal': True, 'goes': "squish", 'flies': True}], + [['some other thing'], + {'mammal': False, 'goes': "thud", 'flies': False}], + [[], # no matching rules for dog + {'mammal': True, 'goes': 'woof', 'flies': False}], + ] + + def test_basic_multipolicy(self): + for expected, kwds in self.test_params: + data = {'critter': critter(**kwds)} + checker = RuleChecker(self.rules, data) + + first = checker.apply(multi=False) + self.assertEqual(checker.lastrun['multi'], False) + + results = checker.apply(multi=True) + + print("all matches:") + for result in checker.lastrun['results']: + print(" ", checker.pretty_trace(result)) + + # check that single result matches first result of multi run + self.assertEqual(checker.lastrun['multi'], True) + if first: + self.assertEqual(first, results[0]) + else: + self.assertEqual([], results) + + self.assertEqual(results, expected) + + +# +# The end. diff --git a/tests/test_lib/test_policy.py b/tests/test_lib/test_policy.py index 4ef0e7a..8f3c3d8 100644 --- a/tests/test_lib/test_policy.py +++ b/tests/test_lib/test_policy.py @@ -232,6 +232,12 @@ class TestRuleHandling(unittest.TestCase): action = obj.apply(data) self.assertEqual(obj.last_rule(), rules[0]) + # negate test + rules = ['! false :: allow'] + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEqual(obj.last_rule(), rules[0]) + # nested rule policy = ''' all :: { @@ -270,6 +276,22 @@ all :: { with self.assertRaises(Exception): obj = koji.policy.SimpleRuleSet(lines, tests) + def test_no_tests(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + lines = [':: allow'] + with self.assertRaises(koji.GenericError): + obj = koji.policy.SimpleRuleSet(lines, tests) + + def test_blank_test(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + lines = ['true && && true :: allow'] + with self.assertRaises(koji.GenericError): + obj = koji.policy.SimpleRuleSet(lines, tests) + def test_missing_handler(self): tests = koji.policy.findSimpleTests(koji.policy.__dict__) data = {} @@ -278,6 +300,153 @@ all :: { with self.assertRaises(koji.GenericError): obj = koji.policy.SimpleRuleSet(lines, tests) + def test_negated_tests(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + rules = ['true && ! false && true :: allow'] + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEqual(action, 'allow') + + rules = ['! true && true && true :: allow'] + obj = koji.policy.SimpleRuleSet(rules, tests) + action = obj.apply(data) + self.assertEqual(action, None) + + def test_break(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + policy = ''' + true :: { + true :: break + true :: ERROR + } + true :: OK + ''' + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + action = obj.apply(data) + self.assertEqual(action, 'OK') + + policy = ''' + true :: { + true :: { + true :: break + true :: ERROR + } + true :: OK + } + true :: ERROR + ''' + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + action = obj.apply(data) + self.assertEqual(action, 'OK') + + policy = ''' + true :: break + true :: ERROR + ''' + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + action = obj.apply(data) + self.assertEqual(action, None) + + def test_break_n(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + policy = ''' + true :: { + true :: { + true :: { + true :: { + true :: break 3 + true :: ERROR + } + true :: ERROR + } + true :: ERROR + } + true :: OK + } + true :: ERROR + ''' + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + action = obj.apply(data) + self.assertEqual(action, 'OK') + + def test_stop(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + policy = ''' + true :: { + true :: { + true :: { + true :: stop + true :: ERROR + } + true :: ERROR + } + true :: ERROR + } + true :: ERROR + ''' + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + action = obj.apply(data) + self.assertEqual(action, None) + action = obj.apply(data, multi=True) + self.assertEqual(action, []) + + policy = ''' + true :: { + true :: hello + true :: world + true :: stop + true :: putting + } + true :: computers + true :: into + true :: everything + ''' + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + action = obj.apply(data, multi=True) + self.assertEqual(action, ['hello', 'world']) + + policy = ''' + true :: OK + false :: ERROR + true :: stop + true :: ERROR + true :: ERROR + ''' + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + action = obj.apply(data, multi=True) + self.assertEqual(action, ['OK']) + + def test_break_invalid(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + # not a number + policy = '''true :: break NOTANUMBER''' + with self.assertRaises(koji.GenericError): + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + + # too many args + policy = '''true :: break 1 2''' + with self.assertRaises(koji.GenericError): + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + + def test_stop_invalid(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + + policy = '''true :: stop 1''' + with self.assertRaises(koji.GenericError): + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + + policy = '''true :: stop that bus!''' + with self.assertRaises(koji.GenericError): + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + def test_complex_policy(self): tests = koji.policy.findSimpleTests(koji.policy.__dict__) data = {} @@ -295,6 +464,9 @@ none :: ERROR true !! ERROR all !! ERROR +! true :: ERROR +! false !! ERROR + false && true && true :: ERROR none && true && true :: ERROR From c6ad3a4cf560e05df1e6ae05540fe787f95f3e06 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 27 2026 22:30:38 +0000 Subject: [PATCH 2/5] add flag action and flagged test --- diff --git a/koji/policy.py b/koji/policy.py index 3341860..8bbcadd 100644 --- a/koji/policy.py +++ b/koji/policy.py @@ -273,6 +273,23 @@ class CompareTest(BaseSimpleTest): return self.func(data[self.field], self.value) +class FlaggedTest(BaseSimpleTest): + '''True if flag has been set by a flag action. False otherwise''' + + name = 'flagged' + + def __init__(self, str): + self.str = str + self.key = self.get_key(self.str.split()[1]) + + @staticmethod + def get_key(name): + return '__policyflag__%s' % name + + def run(self, data): + return bool(data.get(self.key)) + + class BaseAction(object): '''Abstract base class for actions''' @@ -305,6 +322,20 @@ class StopAction(BaseAction): return 'stop' +class FlagAction(BaseAction): + '''Set a named flag + + Named flags can be checked with the flag test + ''' + + def __init__(self, name): + self.name = name + self.key = FlaggedTest.get_key(name) + + def __str__(self): + return 'flag %s' % self.name + + class SimpleRuleSet(object): def __init__(self, rules, tests): @@ -426,6 +457,9 @@ class SimpleRuleSet(object): if args: raise koji.GenericError('Invalid stop action: %s' % action) return StopAction() + elif name == 'flag': + flagname = action.split()[1] + return FlagAction(flagname) else: return PolicyAction(action, name) @@ -559,6 +593,8 @@ class RuleChecker(object): if trace: yield StopAction() return + elif isinstance(action, FlagAction): + self.data[action.key] = True else: self.logger.debug(" rule matched: %s", action) yield (action, next_trace) From e6a7c474c97767fd04b2a32116cc02191771f747 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 27 2026 22:30:38 +0000 Subject: [PATCH 3/5] unit test for flag action --- diff --git a/tests/test_lib/test_policy.py b/tests/test_lib/test_policy.py index 8f3c3d8..954adec 100644 --- a/tests/test_lib/test_policy.py +++ b/tests/test_lib/test_policy.py @@ -146,6 +146,7 @@ class TestDiscovery(unittest.TestCase): 'none': koji.policy.NoneTest, 'target': koji.policy.TargetTest, 'true': koji.policy.TrueTest, + 'flagged': koji.policy.FlaggedTest, } self.assertDictEqual(expected, actual) @@ -447,6 +448,21 @@ all :: { with self.assertRaises(koji.GenericError): obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + def test_flag_action(self): + tests = koji.policy.findSimpleTests(koji.policy.__dict__) + data = {} + + policy = ''' + true :: flag foo + false :: flag bar + flagged bar :: BAD + flagged foo :: OK + flagged baz :: BAD + ''' + obj = koji.policy.SimpleRuleSet(policy.splitlines(), tests) + action = obj.apply(data) + self.assertEqual(action, 'OK') + def test_complex_policy(self): tests = koji.policy.findSimpleTests(koji.policy.__dict__) data = {} From bafd830418e599c2c96168d2fe9c78055bc860f4 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 27 2026 22:30:38 +0000 Subject: [PATCH 4/5] fix rule_str() --- diff --git a/koji/policy.py b/koji/policy.py index 8bbcadd..a48951c 100644 --- a/koji/policy.py +++ b/koji/policy.py @@ -634,7 +634,7 @@ def rule_str(tests, negate, action): if isinstance(action, list): line += '{ ...' else: - line += action + line += str(action) return line From 59584b4de7927306ae2aeeb6fa68b4bbf6891cfd Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 27 2026 22:30:38 +0000 Subject: [PATCH 5/5] note actions in debug output --- diff --git a/koji/policy.py b/koji/policy.py index a48951c..e79d682 100644 --- a/koji/policy.py +++ b/koji/policy.py @@ -542,6 +542,11 @@ class RuleChecker(object): results.append({'action': action, 'trace': trace}) if not multi: break + if not results: + self.logger.debug(" (no action)") + else: + for r in results: + self.logger.debug(" action: %s", r['action']) self.logger.debug("policy done") return self.lastrun