Skip to content

Commit

Permalink
call assert_allowed for every usage of SCM
Browse files Browse the repository at this point in the history 
* CLOUDBLD-10221

Signed-off-by: Robert Cerven <[email protected]>
  • Loading branch information
@rcerven
rcerven committed Jun 17, 2022
1 parent 42d27c7 commit 9e8b3a2
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 54 deletions.
44 changes: 26 additions & 18 deletions koji_containerbuild/plugins/builder_containerbuild.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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()
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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'):
Expand Down
88 changes: 52 additions & 36 deletions tests/test_builder_containerbuild.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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)
Expand Down Expand Up @@ -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'))
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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')
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -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'))

Expand All @@ -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'],
Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -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:
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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'))
Expand All @@ -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,
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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'))
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 9e8b3a2

Please sign in to comment.