From 5e3b2dc0dc1e4ba6ea4f281fb17c3705108f9ea0 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Dec 11 2025 16:04:16 +0000 Subject: [PATCH 1/4] recursive task policy data --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index f350f62..85d0ab2 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -675,6 +675,8 @@ def make_task(method, arglist, **opts): req_channel_id = channel_info['id'] else: raise koji.GenericError('Channel %s is disabled.' % opts['channel']) + if opts['parent']: + policy_data.update(policy_data_from_task(opts['parent'], recurse=True)) policy_data.update(policy_data_from_task_args(method, arglist)) ruleset = context.policy.get('channel') @@ -10531,7 +10533,7 @@ def eval_policy(name, data): return ruleset.apply(data) -def policy_data_from_task(task_id): +def policy_data_from_task(task_id, recurse=False): """Calculate policy data from task id :param int task_id: the task id @@ -10540,7 +10542,13 @@ def policy_data_from_task(task_id): """ task = Task(task_id) taskinfo = task.getInfo(strict=True, request=True) - return policy_data_from_task_args(taskinfo['method'], taskinfo['request']) + if recurse and taskinfo['parent']: + # start with the parents data + data = policy_data_from_task(taskinfo['parent'], recurse=recurse) + else: + data = {} + data.update(policy_data_from_task_args(taskinfo['method'], taskinfo['request'])) + return data def policy_data_from_task_args(method, arglist): From d1d3a3f7eec83519754dcc3552848e310d6ac1e8 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Dec 11 2025 21:32:20 +0000 Subject: [PATCH 2/4] update fakepolicy to match --- diff --git a/devtools/fakepolicy b/devtools/fakepolicy index f97a2b7..af59baa 100755 --- a/devtools/fakepolicy +++ b/devtools/fakepolicy @@ -266,6 +266,8 @@ def make_task_params(args, session): data['user_id'] = tinfo['owner'] if args.req_channel: data['req_channel'] = args.req_channel + if tinfo['parent']: + data.update(kojihub.policy_data_from_task(tinfo['parent'], recurse=True)) data.update(kojihub.policy_data_from_task_args(tinfo['method'], taskargs)) # ^ this relies on the mocks to work return args.policy, data From 54e41408d1bfdc807413037dc2e9f8ae330f6a9a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Dec 11 2025 21:32:20 +0000 Subject: [PATCH 3/4] unit tests --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 85d0ab2..08fe575 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -629,12 +629,7 @@ def make_task(method, arglist, **opts): opts['assign'] = get_host(opts['assign'], strict=True)['id'] if 'parent' in opts: # for subtasks, we use some of the parent's options as defaults - query = QueryProcessor( - tables=['task'], - columns=['state', 'owner', 'channel_id', 'priority', 'arch'], - clauses=['id = %(parent)i'], - values={'parent': opts['parent']}) - pdata = query.executeOne() + pdata = _get_task_parent(opts['parent']) if not pdata: raise koji.GenericError("Invalid parent task: %(parent)s" % opts) if pdata['state'] != koji.TASK_STATES['OPEN']: @@ -762,6 +757,16 @@ def make_task(method, arglist, **opts): return task_id +def _get_task_parent(parent_id): + # helper for make_task + query = QueryProcessor( + tables=['task'], + columns=['state', 'owner', 'channel_id', 'priority', 'arch'], + clauses=['id = %(parent)s'], + values={'parent': parent_id}) + return query.executeOne() + + def eventCondition(event, table=None): """return the proper WHERE condition to select data at the time specified by event. """ if not table: diff --git a/tests/test_cli/test_make_task.py b/tests/test_cli/test_make_task.py index 981f043..369b880 100644 --- a/tests/test_cli/test_make_task.py +++ b/tests/test_cli/test_make_task.py @@ -10,7 +10,7 @@ from koji_cli.commands import handle_make_task from . import utils -class TestAddNotification(utils.CliTestCase): +class TestMakeTask(utils.CliTestCase): def setUp(self): self.maxDiff = None self.options = mock.MagicMock() diff --git a/tests/test_hub/test_make_task.py b/tests/test_hub/test_make_task.py index 82bbba7..b337e0e 100644 --- a/tests/test_hub/test_make_task.py +++ b/tests/test_hub/test_make_task.py @@ -2,6 +2,8 @@ import unittest from unittest import mock +import koji +import koji.xmlrpcplus from kojihub import kojihub, kojixmlrpc QP = kojihub.QueryProcessor @@ -47,6 +49,7 @@ class TestMakeTask(unittest.TestCase): self.get_channel_id = mock.patch('kojihub.kojihub.get_channel_id').start() self.currval = mock.patch('kojihub.kojihub.currval').start() self.auto_arch_refuse = mock.patch('kojihub.scheduler.auto_arch_refuse').start() + self.Task = mock.patch('kojihub.kojihub.Task').start() self.set_policy() @@ -130,4 +133,69 @@ class TestMakeTask(unittest.TestCase): for key in expected: self.assertEqual(self.inserts[0].data[key], expected[key]) + @mock.patch('kojihub.kojihub._get_task_parent') + def test_make_task_with_parent(self, _get_task_parent): + self.get_channel.return_value = {'name': 'testing', 'id': 23, 'enabled': True} + self.opts['DefaultChannelCompat'] = True + self.opts['policy']['channel'] = ''' + has req_channel :: req + all :: use bad + ''' + self.set_policy() + # mock parent data + kwargs = {'srpm': 'SRPM', 'build_tag': 100} + arglist = koji.encode_args(**kwargs) + request = koji.xmlrpcplus.dumps(tuple(arglist), methodname='parent_method') + pdata = { + 'state': koji.TASK_STATES['OPEN'], + 'owner': 1, + 'channel_id':23, + 'priority': 20, + 'arch': 'noarch', + 'parent': None, + 'method': 'parent_method', + 'request': request, + } + _get_task_parent.return_value = pdata + self.Task.return_value.getInfo.return_value = pdata + + kojihub.make_task('something', [1, 2, 3], default_channel='testing', parent=5678) + + # in compat mode we expect to hit the "req" policy result + self.get_channel.assert_called_once_with('testing') + self.get_channel_id.assert_not_called() + self.assertEqual(len(self.inserts), 1) + expected = {'state': 0, 'method': 'something', 'parent': 5678, 'arch': 'noarch', + 'channel_id': 23} + for key in expected: + self.assertEqual(self.inserts[0].data[key], expected[key]) + + @mock.patch('kojihub.kojihub._get_task_parent') + def test_make_task_with_invalid_parent(self, _get_task_parent): + _get_task_parent.return_value = None + + with self.assertRaises(koji.GenericError) as ex: + kojihub.make_task('something', [1, 2, 3], default_channel='testing', parent=5678) + + self.assertEqual('Invalid parent task: 5678', str(ex.exception)) + + @mock.patch('kojihub.kojihub._get_task_parent') + def test_make_task_with_nonopen_parent(self, _get_task_parent): + # mock parent data + pdata = { + 'state': koji.TASK_STATES['FAILED'], + 'owner': 1, + 'channel_id':23, + 'priority': 20, + 'arch': 'noarch', + 'parent': None, + } + _get_task_parent.return_value = pdata + + with self.assertRaises(koji.GenericError) as ex: + kojihub.make_task('something', [1, 2, 3], default_channel='testing', parent=5678) + + self.assertEqual('Parent task (id 5678) is not open', str(ex.exception)) + + # the end From 3e944ca30259481887aa89d6df805f8235f64aa2 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 02 2026 16:18:13 +0000 Subject: [PATCH 4/4] avoid specialized helper function --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 08fe575..430daad 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -629,7 +629,7 @@ def make_task(method, arglist, **opts): opts['assign'] = get_host(opts['assign'], strict=True)['id'] if 'parent' in opts: # for subtasks, we use some of the parent's options as defaults - pdata = _get_task_parent(opts['parent']) + pdata = Task(opts['parent']).getInfo(request=False) if not pdata: raise koji.GenericError("Invalid parent task: %(parent)s" % opts) if pdata['state'] != koji.TASK_STATES['OPEN']: @@ -757,16 +757,6 @@ def make_task(method, arglist, **opts): return task_id -def _get_task_parent(parent_id): - # helper for make_task - query = QueryProcessor( - tables=['task'], - columns=['state', 'owner', 'channel_id', 'priority', 'arch'], - clauses=['id = %(parent)s'], - values={'parent': parent_id}) - return query.executeOne() - - def eventCondition(event, table=None): """return the proper WHERE condition to select data at the time specified by event. """ if not table: diff --git a/tests/test_hub/test_make_task.py b/tests/test_hub/test_make_task.py index b337e0e..15c5e38 100644 --- a/tests/test_hub/test_make_task.py +++ b/tests/test_hub/test_make_task.py @@ -133,8 +133,7 @@ class TestMakeTask(unittest.TestCase): for key in expected: self.assertEqual(self.inserts[0].data[key], expected[key]) - @mock.patch('kojihub.kojihub._get_task_parent') - def test_make_task_with_parent(self, _get_task_parent): + def test_make_task_with_parent(self): self.get_channel.return_value = {'name': 'testing', 'id': 23, 'enabled': True} self.opts['DefaultChannelCompat'] = True self.opts['policy']['channel'] = ''' @@ -156,7 +155,6 @@ class TestMakeTask(unittest.TestCase): 'method': 'parent_method', 'request': request, } - _get_task_parent.return_value = pdata self.Task.return_value.getInfo.return_value = pdata kojihub.make_task('something', [1, 2, 3], default_channel='testing', parent=5678) @@ -170,17 +168,15 @@ class TestMakeTask(unittest.TestCase): for key in expected: self.assertEqual(self.inserts[0].data[key], expected[key]) - @mock.patch('kojihub.kojihub._get_task_parent') - def test_make_task_with_invalid_parent(self, _get_task_parent): - _get_task_parent.return_value = None + def test_make_task_with_invalid_parent(self): + self.Task.return_value.getInfo.return_value = None with self.assertRaises(koji.GenericError) as ex: kojihub.make_task('something', [1, 2, 3], default_channel='testing', parent=5678) self.assertEqual('Invalid parent task: 5678', str(ex.exception)) - @mock.patch('kojihub.kojihub._get_task_parent') - def test_make_task_with_nonopen_parent(self, _get_task_parent): + def test_make_task_with_nonopen_parent(self): # mock parent data pdata = { 'state': koji.TASK_STATES['FAILED'], @@ -190,7 +186,7 @@ class TestMakeTask(unittest.TestCase): 'arch': 'noarch', 'parent': None, } - _get_task_parent.return_value = pdata + self.Task.return_value.getInfo.return_value = pdata with self.assertRaises(koji.GenericError) as ex: kojihub.make_task('something', [1, 2, 3], default_channel='testing', parent=5678)