From b1c01b2878411b5edc25672c3eca6b333e4afe90 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 11 2020 19:26:46 +0000 Subject: [PATCH 1/5] split off "policy data from task" logic --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 74f8670..51876d5 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -563,46 +563,7 @@ def make_task(method, arglist, **opts): if 'channel' in opts: policy_data['req_channel'] = opts['channel'] req_channel_id = get_channel_id(opts['channel'], strict=True) - params = {} - try: - params = koji.tasks.parse_task_params(method, arglist) - except TypeError: - logger.warning("%s is not a standard koji task", method) - except koji.ParameterError: - logger.warning("Cannot parse parameters: %s of %s task", arglist, method) - except Exception: - logger.warning("Unexcepted error occurs when parsing parameters: %s of %s task", - arglist, method, exc_info=True) - if params: - # parameters that indicate source for build - for k in ('src', 'spec_url', 'url'): - if method == 'newRepo': - # newRepo has a 'src' parameter that means something else - break - if k in params: - policy_data['source'] = params.get(k) - break - # parameters that indicate build target - target = None - hastarget = False - for k in ('target', 'build_target', 'target_info'): - if k in params: - target = params.get(k) - hastarget = True - break - if hastarget: - if isinstance(target, dict): - if 'name' not in target: - logger.warning("Bad build target parameter: %r", target) - target = None - else: - target = target.get('name') - if target is None: - policy_data['target'] = None - else: - policy_data['target'] = get_build_target(target, strict=True)['name'] - t_opts = params.get('opts', {}) - policy_data['scratch'] = t_opts.get('scratch', False) + policy_data.update(policy_data_from_task_args(method, arglist)) ruleset = context.policy.get('channel') result = ruleset.apply(policy_data) @@ -9842,6 +9803,71 @@ def check_policy(name, data, default='deny', strict=False, force=False): raise koji.ActionNotAllowed(err_str) +def policy_data_from_task(task_id): + """Calculate policy data from task id + + :param int task_id: the task id + + :returns: dict with policy data + """ + task = Task(task_id) + taskinfo = task.getInfo(strict=True, request=True) + return policy_data_from_task_args(taskinfo['method'], taskinfo['request']) + + +def policy_data_from_task_args(method, arglist): + """Calculate policy data from task arguments + + :param str method: task method + :param list arglist: raw task params + + :returns: dict with policy data + """ + params = {} + policy_data = {} + try: + params = koji.tasks.parse_task_params(method, arglist) + except TypeError: + logger.warning("%s is not a standard koji task", method) + except koji.ParameterError: + logger.warning("Cannot parse parameters: %s of %s task", arglist, method) + except Exception: + logger.warning("Unexcepted error occurs when parsing parameters: %s of %s task", + arglist, method, exc_info=True) + if params: + # parameters that indicate source for build + for k in ('src', 'spec_url', 'url'): + if method == 'newRepo': + # newRepo has a 'src' parameter that means something else + break + if k in params: + policy_data['source'] = params.get(k) + break + # parameters that indicate build target + target = None + hastarget = False + for k in ('target', 'build_target', 'target_info'): + if k in params: + target = params.get(k) + hastarget = True + break + if hastarget: + if isinstance(target, dict): + if 'name' not in target: + logger.warning("Bad build target parameter: %r", target) + target = None + else: + target = target.get('name') + if target is None: + policy_data['target'] = None + else: + policy_data['target'] = get_build_target(target, strict=True)['name'] + t_opts = params.get('opts', {}) + policy_data['scratch'] = t_opts.get('scratch', False) + + return policy_data + + def assert_policy(name, data, default='deny', force=False): """Enforce the named policy From 2c41d8b581c52426a05cb33e8f55eac77699d630 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 11 2020 19:26:46 +0000 Subject: [PATCH 2/5] use policy_data_from_task in complete*Build calls --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 51876d5..329cf75 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5886,6 +5886,8 @@ def import_build(srpm, rpms, brmap=None, task_id=None, build_id=None, logs=None) 'import': True, 'import_type': 'rpm', } + if task_id is not None: + policy_data.update(policy_data_from_task(task_id)) vol = check_volume_policy(policy_data, strict=False, default='DEFAULT') build['volume_id'] = vol['id'] build['volume_name'] = vol['name'] @@ -13795,6 +13797,7 @@ class HostExports(object): 'import': True, 'import_type': 'maven', } + policy_data.update(policy_data_from_task(task_id)) vol = check_volume_policy(policy_data, strict=False, default='DEFAULT') if vol['id'] != build_info['volume_id']: build_info['volume_id'] = vol['id'] @@ -13876,6 +13879,7 @@ class HostExports(object): 'import': True, 'import_type': 'maven', } + policy_data.update(policy_data_from_task(task_id)) vol = check_volume_policy(policy_data, strict=False, default='DEFAULT') if vol['id'] != build_info['volume_id']: build_info['volume_id'] = vol['id'] @@ -14050,6 +14054,7 @@ class HostExports(object): 'import': True, 'import_type': 'win', } + policy_data.update(policy_data_from_task(task_id)) vol = check_volume_policy(policy_data, strict=False, default='DEFAULT') if vol['id'] != build_info['volume_id']: build_info['volume_id'] = vol['id'] From 1bc60cf327cda3d031148d8a76dea241ff7c15a0 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 11 2020 19:26:46 +0000 Subject: [PATCH 3/5] fix handling of indirectionimage in policy_data_from_task_args() --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 329cf75..b18bac5 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -9836,36 +9836,43 @@ def policy_data_from_task_args(method, arglist): except Exception: logger.warning("Unexcepted error occurs when parsing parameters: %s of %s task", arglist, method, exc_info=True) - if params: - # parameters that indicate source for build - for k in ('src', 'spec_url', 'url'): - if method == 'newRepo': - # newRepo has a 'src' parameter that means something else - break - if k in params: - policy_data['source'] = params.get(k) - break - # parameters that indicate build target - target = None - hastarget = False - for k in ('target', 'build_target', 'target_info'): - if k in params: - target = params.get(k) - hastarget = True - break - if hastarget: - if isinstance(target, dict): - if 'name' not in target: - logger.warning("Bad build target parameter: %r", target) - target = None - else: - target = target.get('name') - if target is None: - policy_data['target'] = None + if not params: + return {} + + if method == 'indirectionimage': + # this handler buries all its arguments in a single 'opts' parameter + opts = params.get('opts') or {} + params = dict(**opts) + params['opts'] = opts + # parameters that indicate source for build + for k in ('src', 'spec_url', 'url'): + if method == 'newRepo': + # newRepo has a 'src' parameter that means something else + break + if k in params: + policy_data['source'] = params.get(k) + break + # parameters that indicate build target + target = None + hastarget = False + for k in ('target', 'build_target', 'target_info'): + if k in params: + target = params.get(k) + hastarget = True + break + if hastarget: + if isinstance(target, dict): + if 'name' not in target: + logger.warning("Bad build target parameter: %r", target) + target = None else: - policy_data['target'] = get_build_target(target, strict=True)['name'] - t_opts = params.get('opts', {}) - policy_data['scratch'] = t_opts.get('scratch', False) + target = target.get('name') + if target is None: + policy_data['target'] = None + else: + policy_data['target'] = get_build_target(target, strict=True)['name'] + t_opts = params.get('opts', {}) + policy_data['scratch'] = t_opts.get('scratch', False) return policy_data From 3f41852358da35e3ba88c487f6b1a87c064b4cb9 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 11 2020 19:26:52 +0000 Subject: [PATCH 4/5] policy_get_build_tags: use target data if available Fixes https://pagure.io/koji/issue/2305 --- diff --git a/hub/kojihub.py b/hub/kojihub.py index b18bac5..e461fa2 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -9327,13 +9327,19 @@ def policy_get_cgs(data): def policy_get_build_tags(data): - # pull cg info out - # note that br_id will be None if a component had no buildroot if 'build_tag' in data: return [get_tag(data['build_tag'], strict=True)['name']] elif 'build_tags' in data: return [get_tag(t, strict=True)['name'] for t in data['build_tags']] - # otherise look at buildroots + + # see if we have a target + target = data.get('target') + if target: + target = get_build_target(target, strict=False) + if target: + return [target['build_tag_name']] + + # otherwise look at buildroots tags = set() for br_id in policy_get_brs(data): if br_id is None: From e17cd013a748fbe31ec393340c2b74951a6ab724 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jun 11 2020 19:27:20 +0000 Subject: [PATCH 5/5] also consider task data in apply_volume_policy() --- diff --git a/hub/kojihub.py b/hub/kojihub.py index e461fa2..8f123f3 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -5652,6 +5652,9 @@ def apply_volume_policy(build, strict=False): volume we be retained. """ policy_data = {'build': build} + task_id = build['task_id'] + if task_id: + policy_data.update(policy_data_from_task(task_id)) volume = check_volume_policy(policy_data, strict=strict) if volume is None: # just leave the build where it is