From 9e8b3a2f18d308101c16ac30463375c0f1f280fa Mon Sep 17 00:00:00 2001 From: Robert Cerven Date: Fri, 17 Jun 2022 17:45:03 +0200 Subject: [PATCH] call assert_allowed for every usage of SCM * CLOUDBLD-10221 Signed-off-by: Robert Cerven --- .../plugins/builder_containerbuild.py | 44 ++++++---- tests/test_builder_containerbuild.py | 88 +++++++++++-------- 2 files changed, 78 insertions(+), 54 deletions(-) diff --git a/koji_containerbuild/plugins/builder_containerbuild.py b/koji_containerbuild/plugins/builder_containerbuild.py index 0ff81ca..76acb61 100644 --- a/koji_containerbuild/plugins/builder_containerbuild.py +++ b/koji_containerbuild/plugins/builder_containerbuild.py @@ -745,6 +745,23 @@ def __init__(self, id, method, params, session, options, workdir=None, demux=Tru self.demux = demux self.event_id = None + def _get_scm(self, src, task_info, scratch): + scm = My_SCM(src) + scm_policy_opts = { + 'user_id': task_info['owner'], + 'channel': self.session.getChannel(task_info['channel_id'], + strict=True)['name'], + 'scratch': bool(scratch), + } + scm.assert_allowed( + allowed=self.options.allowed_scms, + session=self.session, + by_config=self.options.allowed_scms_use_config, + by_policy=self.options.allowed_scms_use_policy, + policy_data=scm_policy_opts) + + return scm + def createContainer(self, src=None, target_info=None, arches=None, scratch=None, isolated=None, yum_repourls=None, branch=None, push_url=None, koji_parent_build=None, @@ -760,19 +777,7 @@ def createContainer(self, src=None, target_info=None, arches=None, owner_info = self.session.getUser(this_task['owner']) self.logger.debug("Started by %s", owner_info['name']) - scm = My_SCM(src) - scm_policy_opts = { - 'user_id': this_task['owner'], - 'channel': self.session.getChannel(this_task['channel_id'], - strict=True)['name'], - 'scratch': bool(scratch), - } - scm.assert_allowed( - allowed=self.options.allowed_scms, - session=self.session, - by_config=self.options.allowed_scms_use_config, - by_policy=self.options.allowed_scms_use_policy, - policy_data=scm_policy_opts) + scm = self._get_scm(src, this_task, scratch) git_uri = scm.get_git_uri() component = scm.get_component() @@ -885,11 +890,13 @@ def getArchList(self, build_tag, extra=None): raise koji.BuildError("No matching arches were found") return list(archdict.keys()) - def fetchDockerfile(self, src, build_tag): + def fetchDockerfile(self, src, build_tag, scratch): """ Gets Dockerfile. Roughly corresponds to getSRPM method of build task """ - scm = SCM(src) + this_task = self.session.getTaskInfo(self.id) + scm = self._get_scm(src, this_task, scratch) + scmdir = os.path.join(self.workdir, 'sources') koji.ensuredir(scmdir) @@ -913,9 +920,9 @@ def fetchDockerfile(self, src, build_tag): raise koji.BuildError("Dockerfile file missing: %s" % fn) return fn - def checkLabels(self, src, build_tag, label_overwrites=None): + def checkLabels(self, src, build_tag, scratch, label_overwrites=None): label_overwrites = label_overwrites or {} - dockerfile_path = self.fetchDockerfile(src, build_tag) + dockerfile_path = self.fetchDockerfile(src, build_tag, scratch) labels_wrapper = LabelsWrapper(dockerfile_path, logger_name=self.logger.name, label_overwrites=label_overwrites) @@ -984,7 +991,8 @@ def handler(self, src, target, opts=None): if release_overwrite: label_overwrites = {LABEL_NAME_MAP['RELEASE'][0]: release_overwrite} component, expected_nvr = self.checkLabels(src, label_overwrites=label_overwrites, - build_tag=build_tag) + build_tag=build_tag, + scratch=opts.get('scratch')) # scratch builds do not get imported, and consequently not tagged if not self.opts.get('scratch'): diff --git a/tests/test_builder_containerbuild.py b/tests/test_builder_containerbuild.py index e357b16..2b01af0 100644 --- a/tests/test_builder_containerbuild.py +++ b/tests/test_builder_containerbuild.py @@ -593,7 +593,8 @@ def _mock_git_source(self): """, None, ('fedora-docker', 'fedora-docker-25-3')), )) - def test_checkLabels(self, tmpdir, df_content, missing_labels, return_val): + @pytest.mark.parametrize('scratch', (True, False, None)) + def test_checkLabels(self, tmpdir, df_content, missing_labels, return_val, scratch): cct = builder_containerbuild.BuildContainerTask(id=1, method='buildContainer', params='params', @@ -604,15 +605,16 @@ def test_checkLabels(self, tmpdir, df_content, missing_labels, return_val): dockerfile_content=df_content) (flexmock(cct) .should_receive('fetchDockerfile') + .with_args('src', 'build-tag', scratch) .and_return(folder_info['dockerfile_path'])) if not missing_labels: - check_return = cct.checkLabels('src', 'build-tag') + check_return = cct.checkLabels('src', 'build-tag', scratch) assert check_return == return_val return with pytest.raises(koji.BuildError) as exc_info: - cct.checkLabels('src', 'build-tag') + cct.checkLabels('src', 'build-tag', scratch) err_msg = str(exc_info.value) assert all(label in err_msg for label in missing_labels) @@ -641,9 +643,11 @@ def test_osbs_build(self, tmpdir, pkg_info, failure, orchestrator): workdir='workdir', demux=orchestrator) + build_args = {'git_branch': 'working'} + (flexmock(task) .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') + .with_args(src['src'], 'build-tag', build_args.get('scratch')) .and_return(folders_info['dockerfile_path'])) (flexmock(task) .should_receive('_write_incremental_logs')) @@ -654,7 +658,6 @@ def test_osbs_build(self, tmpdir, pkg_info, failure, orchestrator): (flexmock(task) .should_receive('_write_combined_log')) - build_args = {'git_branch': 'working'} task._osbs = self._mock_osbs(koji_build_id=koji_build_id, src=src, koji_task_id=koji_task_id, @@ -756,7 +759,7 @@ def test_createContainer_failure(self, tmpdir, reason, expected_exc_type): (flexmock(task) .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') + .with_args(src['src'], 'build-tag', None) .and_return(folders_info['dockerfile_path'])) task._osbs = self._mock_osbs(koji_build_id=koji_build_id, @@ -961,8 +964,11 @@ def test_private_branch(self, tmpdir): .and_return({'arches': 'x86_64'})) (session .should_receive('getTaskInfo') - .with_args(koji_task_id, request=True) - .and_return({'owner': 'owner'})) + .with_args(koji_task_id) + .and_return({'owner': 'owner', 'channel_id': 1})) + (session + .should_receive('getChannel') + .and_return({'name': 'default_channel'})) (session .should_receive('getUser') .with_args('owner') @@ -992,7 +998,7 @@ def test_private_branch(self, tmpdir): (flexmock(os.path) .should_receive('exists') .and_return(True)) - task.fetchDockerfile(source, 'build_tag') + task.fetchDockerfile(source, 'build_tag', False) @pytest.mark.parametrize('log_upload_raises', (True, False)) @pytest.mark.parametrize('orchestrator', (True, False)) @@ -1028,10 +1034,6 @@ def test_additional_args(self, tmpdir, log_upload_raises, orchestrator, addition workdir='workdir', demux=orchestrator) - (flexmock(task) - .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') - .and_return(folders_info['dockerfile_path'])) (flexmock(task) .should_receive('_write_incremental_logs')) if orchestrator: @@ -1051,6 +1053,12 @@ def test_additional_args(self, tmpdir, log_upload_raises, orchestrator, addition build_args = deepcopy(additional_args) build_args['git_branch'] = 'working' + + (flexmock(task) + .should_receive('fetchDockerfile') + .with_args(src['src'], 'build-tag', build_args.get('scratch')) + .and_return(folders_info['dockerfile_path'])) + task._osbs = self._mock_osbs(koji_build_id=koji_build_id, src=src, koji_task_id=koji_task_id, @@ -1159,10 +1167,6 @@ def test_flatpak_build(self, tmpdir, isolated, release, koji_parent_build): options=options, workdir='workdir') - (flexmock(task) - .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') - .and_return(folders_info['dockerfile_path'])) (flexmock(task) .should_receive('_write_incremental_logs')) @@ -1183,6 +1187,12 @@ def test_flatpak_build(self, tmpdir, isolated, release, koji_parent_build): orchestrator=True, create_build_args=deepcopy(additional_args), build_not_started=False) + + (flexmock(task) + .should_receive('fetchDockerfile') + .with_args(src['src'], 'build-tag', additional_args.get('scratch')) + .and_return(folders_info['dockerfile_path'])) + task_response = task.handler(src['src'], 'target', opts=additional_args) assert task_response == { 'repositories': ['unique-repo', 'primary-repo'], @@ -1216,10 +1226,6 @@ def test_oversized_tags(self, tmpdir, orchestrator, tag, release, is_oversized): workdir='workdir', demux=orchestrator) - (flexmock(task) - .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') - .and_return(folders_info['dockerfile_path'])) (flexmock(task) .should_receive('_write_incremental_logs')) if orchestrator: @@ -1239,6 +1245,11 @@ def test_oversized_tags(self, tmpdir, orchestrator, tag, release, is_oversized): create_build_args=additional_args.copy(), build_not_started=is_oversized) + (flexmock(task) + .should_receive('fetchDockerfile') + .with_args(src['src'], 'build-tag', additional_args.get('scratch')) + .and_return(folders_info['dockerfile_path'])) + if is_oversized: with pytest.raises(koji.BuildError) as exc_info: task.handler(src['src'], 'target', opts=additional_args) @@ -1315,10 +1326,6 @@ def test_build_nvr_exists(self, tmpdir, orchestrator, build_state, triggered_aft workdir='workdir', demux=orchestrator) - (flexmock(task) - .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') - .and_return(folders_info['dockerfile_path'])) (flexmock(task) .should_receive('_write_incremental_logs')) if orchestrator: @@ -1336,6 +1343,11 @@ def test_build_nvr_exists(self, tmpdir, orchestrator, build_state, triggered_aft if skip_build: additional_args['skip_build'] = True + (flexmock(task) + .should_receive('fetchDockerfile') + .with_args(src['src'], 'build-tag', additional_args.get('scratch')) + .and_return(folders_info['dockerfile_path'])) + task._osbs = self._mock_osbs(koji_build_id=koji_build_id, src=src, koji_task_id=koji_task_id, @@ -1464,17 +1476,18 @@ def test_compose_ids_and_signing_intent(self, tmpdir, additional_args, raises): workdir='workdir', demux=True) + build_args = deepcopy(additional_args) + build_args['git_branch'] = 'working' + (flexmock(task) .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') + .with_args(src['src'], 'build-tag', build_args.get('scratch')) .and_return(folders_info['dockerfile_path'])) (flexmock(task) .should_receive('_write_incremental_logs')) (flexmock(task) .should_receive('_write_demultiplexed_logs')) - build_args = deepcopy(additional_args) - build_args['git_branch'] = 'working' task._osbs = self._mock_osbs(koji_build_id=koji_build_id, src=src, koji_task_id=koji_task_id, @@ -1530,9 +1543,12 @@ def test_arch_override(self, tmpdir, orchestrator, additional_args, raises): workdir='workdir', demux=orchestrator) + build_args = deepcopy(additional_args) + build_args['git_branch'] = 'working' + (flexmock(task) .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') + .with_args(src['src'], 'build-tag', build_args.get('scratch')) .and_return(folders_info['dockerfile_path'])) (flexmock(task) .should_receive('_write_incremental_logs')) @@ -1543,8 +1559,6 @@ def test_arch_override(self, tmpdir, orchestrator, additional_args, raises): (flexmock(task) .should_receive('_write_combined_log')) - build_args = deepcopy(additional_args) - build_args['git_branch'] = 'working' task._osbs = self._mock_osbs(koji_build_id=koji_build_id, src=src, koji_task_id=koji_task_id, @@ -1762,7 +1776,7 @@ def test_schema_validation_valid_options_container(self, build_opts, valid, tmpd (flexmock(task) .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') + .with_args(src['src'], 'build-tag', build_opts.get('scratch')) .and_return(folders_info['dockerfile_path'])) (flexmock(task) @@ -1892,9 +1906,11 @@ def test_raise_OsbsValidationException(self, tmpdir): workdir='workdir', demux=orchestrator) + build_args = {'git_branch': 'working'} + (flexmock(task) .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') + .with_args(src['src'], 'build-tag', build_args.get('scratch')) .and_return(folder_info['dockerfile_path'])) (flexmock(task) .should_receive('_write_incremental_logs')) @@ -1905,7 +1921,6 @@ def test_raise_OsbsValidationException(self, tmpdir): (flexmock(task) .should_receive('_write_combined_log')) - build_args = {'git_branch': 'working'} task._osbs = self._mock_osbs(koji_build_id=koji_build_id, src=src, koji_task_id=koji_task_id, @@ -1943,12 +1958,13 @@ def test_user_warnings(self, tmpdir): workdir=str(tmpdir), demux=True) + build_args = {'git_branch': 'working'} + (flexmock(task) .should_receive('fetchDockerfile') - .with_args(src['src'], 'build-tag') + .with_args(src['src'], 'build-tag', build_args.get('scratch')) .and_return(folders_info['dockerfile_path'])) - build_args = {'git_branch': 'working'} osbs = self._mock_osbs(koji_build_id=koji_build_id, src=src, koji_task_id=koji_task_id,