Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow konflux images with oci index, be used in osbs #2122

Merged
merged 1 commit into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion atomic_reactor/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,7 @@ def hide_files(self):
@property
def skip_koji_check_for_base_image(self):
return self._get_value(ReactorConfigKeys.SKIP_KOJI_CHECK_FOR_BASE_IMAGE_KEY,
fallback=False)
fallback=True)
MartinBasti marked this conversation as resolved.
Show resolved Hide resolved

@property
def deep_manifest_list_inspection(self):
Expand Down
5 changes: 5 additions & 0 deletions atomic_reactor/plugins/check_base_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,11 @@ def _get_manifest_list(self, image: ImageName) -> requests.Response:
reg_client = self._get_registry_client(image.registry)

manifest_list = reg_client.get_manifest_list(image)

if not manifest_list:
self.log.info('manifest list not found trying to get image index')
manifest_list = reg_client.get_manifest_index(image)

if '@sha256:' in str(image) and not manifest_list:
# we want to adjust the tag only for manifest list fetching
image = image.copy()
Expand Down
8 changes: 5 additions & 3 deletions atomic_reactor/plugins/fetch_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -683,9 +683,11 @@ def get_srpm_urls(self, sigkeys=None, insecure=False):
self.log.debug('Resolving SRPM for RPM ID: %s', rpm_id)

if rpm['external_repo_name'] != 'INTERNAL':
msg = ('RPM comes from an external repo (RPM ID: {}). '
'External RPMs are currently not supported.').format(rpm_id)
raise RuntimeError(msg)
msg = ('RPM comes from an external repo (RPM ID: {}; NVR: {}). '
'External RPMs are currently not supported, '
'skipping').format(rpm_id, rpm['nvr'])
MartinBasti marked this conversation as resolved.
Show resolved Hide resolved
self.log.warning(msg)
continue

rpm_hdr = self.session.getRPMHeaders(rpm_id, headers=['SOURCERPM'])
if 'SOURCERPM' not in rpm_hdr:
Expand Down
11 changes: 8 additions & 3 deletions atomic_reactor/plugins/koji_parent.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import time


DEFAULT_POLL_TIMEOUT = 60 * 10 # 10 minutes
DEFAULT_POLL_TIMEOUT = 60 * 3 # 3 minutes
MartinBasti marked this conversation as resolved.
Show resolved Hide resolved
DEFAULT_POLL_INTERVAL = 10 # 10 seconds


Expand Down Expand Up @@ -273,7 +273,7 @@ def detect_parent_image_nvr(self, image_name, inspect_data=None):

return '-'.join(label_values)

def wait_for_parent_image_build(self, nvr: str) -> Dict[str, Any]:
def wait_for_parent_image_build(self, nvr: str) -> Optional[Dict[str, Any]]:
"""
Given image NVR, wait for the build that produced it to show up in koji.
If it doesn't within the timeout, raise an error.
Expand All @@ -295,7 +295,12 @@ def wait_for_parent_image_build(self, nvr: str) -> Dict[str, Any]:
exc_msg = ('Parent image Koji build {} state is {}, not COMPLETE.')
raise KojiParentBuildMissing(exc_msg.format(nvr, build_state))
time.sleep(self.poll_interval)
raise KojiParentBuildMissing('Parent image Koji build NOT found for {}!'.format(nvr))

if not self.workflow.conf.skip_koji_check_for_base_image:
raise KojiParentBuildMissing('Parent image Koji build NOT found for {}!'.format(nvr))

self.log.warning("Parent image Koji build NOT found for %s!", nvr)
return None

def make_result(self) -> Optional[Dict[str, Any]]:
"""Construct the result dict to be preserved in the build metadata."""
Expand Down
11 changes: 9 additions & 2 deletions atomic_reactor/plugins/resolve_composes.py
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ def adjust_for_inherit(self):
if not self.plugin_result:
return

build_info = self.plugin_result[BASE_IMAGE_KOJI_BUILD]
build_info = self.plugin_result.get(BASE_IMAGE_KOJI_BUILD)
if not build_info:
self.log.warning('Parent koji build does not exist can not inherit from the parent')
return
parent_repourls = []

try:
Expand Down Expand Up @@ -218,7 +221,11 @@ def adjust_signing_intent_from_parent(self):
PLUGIN_KOJI_PARENT_KEY)
return

build_info = self.plugin_result[BASE_IMAGE_KOJI_BUILD]
build_info = self.plugin_result.get(BASE_IMAGE_KOJI_BUILD)
if not build_info:
self.log.warning('Parent koji build does not exist can not adjust '
'signing intent from the parent')
return

try:
parent_signing_intent_name = build_info['extra']['image']['odcs']['signing_intent']
Expand Down
4 changes: 2 additions & 2 deletions atomic_reactor/schemas/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -497,9 +497,9 @@
"type": "string"
},
"skip_koji_check_for_base_image": {
"description": "Allow base image without a related koji build (insecure) when there are no NVR labels set on the base image. If the NVR labels are set on the base image, the check is performed regardless",
"description": "Allow base image without a related koji build (insecure)",
"type": "boolean",
"default": false
"default": true
},
"deep_manifest_list_inspection": {
"description": "If manifest list digest check fails, inspect the manifest list contents",
Expand Down
34 changes: 28 additions & 6 deletions atomic_reactor/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -762,6 +762,17 @@ def get_manifest_list(self, image: ImageName) -> Optional[requests.Response]:
response, _ = self.get_manifest(image, version)
return response

def get_manifest_index(self, image: ImageName) -> Optional[requests.Response]:
"""Return manifest index for image.

:param image: ImageName, the remote image to inspect

:return: response, or None, with manifest list
"""
version = 'oci_index'
response, _ = self.get_manifest(image, version)
return response

def get_manifest_list_digest(self, image):
"""Return manifest list digest for image

Expand All @@ -775,7 +786,7 @@ def get_manifest_list_digest(self, image):
return 'sha256:{}'.format(digest_dict['sha256sum'])

def get_all_manifests(
self, image: ImageName, versions: Sequence[str] = ('v1', 'v2', 'v2_list')
self, image: ImageName, versions: Sequence[str] = ('v1', 'v2', 'v2_list', 'oci_index')
) -> Dict[str, requests.Response]:
"""Return manifest digests for image.

Expand Down Expand Up @@ -815,6 +826,11 @@ def get_inspect_for_image(
blob_config, config_digest = self._config_and_id_from_manifest_list(
image, manifest_list, arch
)
elif 'oci_index' in all_man_digests:
manifest_list = all_man_digests['oci_index'].json()
blob_config, config_digest = self._config_and_id_from_manifest_list(
image, manifest_list, arch
)
# get config for v2 digest
elif 'v2' in all_man_digests:
v2_json = all_man_digests['v2'].json()
Expand Down Expand Up @@ -889,11 +905,17 @@ def _config_and_id_from_manifest_list(
)
manifest = arch_manifests[0]

if manifest["mediaType"] != MEDIA_TYPE_DOCKER_V2_SCHEMA2:
raise RuntimeError(f"Image {image}: v2 schema 1 in manifest list")
if (manifest["mediaType"] != MEDIA_TYPE_DOCKER_V2_SCHEMA2 and
manifest["mediaType"] != MEDIA_TYPE_OCI_V1):
raise RuntimeError(f"Image {image}: v2 schema 1 in manifest list, "
f"oci in image index is missing")

image_digest = manifest["digest"]

v2_digest = manifest["digest"]
return self.get_config_and_id_from_registry(image, v2_digest, version='v2')
if manifest["mediaType"] == MEDIA_TYPE_DOCKER_V2_SCHEMA2:
return self.get_config_and_id_from_registry(image, image_digest, version='v2')
else:
return self.get_config_and_id_from_registry(image, image_digest, version='oci')

def get_config_and_id_from_registry(self, image, digest: str, version='v2') -> Tuple[Dict, str]:
"""Return image config by digest
Expand Down Expand Up @@ -1115,7 +1137,7 @@ def get_manifest_list(image, registry, insecure=False, dockercfg_path=None):


def get_all_manifests(image, registry, insecure=False, dockercfg_path=None,
versions=('v1', 'v2', 'v2_list')):
versions=('v1', 'v2', 'v2_list', 'oci_index')):
"""Return manifest digests for image.

:param image: ImageName, the remote image to inspect
Expand Down
58 changes: 46 additions & 12 deletions tests/plugins/test_check_base_image.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,24 +343,33 @@ def workflow_callback(workflow):
workflow_callback=workflow_callback)
assert log_message in caplog.text

@pytest.mark.parametrize('has_manifest_list', (True, False))
@pytest.mark.parametrize('has_manifest_list', ("list", "index", None))
@pytest.mark.parametrize('has_v2s2_manifest', (True, False))
def test_single_platform_build(self, caplog, workflow, has_manifest_list, has_v2s2_manifest):

class StubResponse(object):
content = b'stubContent'

def workflow_callback(workflow):
if has_manifest_list:
workflow = self.prepare(workflow, mock_get_manifest_list=True)
if has_manifest_list == "list":
workflow = self.prepare(workflow, mock_get_manifest_list=True,
mock_get_manifest_index=False)
elif has_manifest_list == "index":
workflow = self.prepare(workflow, mock_get_manifest_list=False,
mock_get_manifest_index=True)
else:
workflow = self.prepare(workflow, mock_get_manifest_list=False)
workflow = self.prepare(workflow, mock_get_manifest_list=False,
mock_get_manifest_index=False)
workflow.data.plugins_results[PLUGIN_CHECK_AND_SET_PLATFORMS_KEY] = {'x86_64'}
if not has_manifest_list:

if has_manifest_list is None:
resp = {'v2': StubResponse()} if has_v2s2_manifest else {}
(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_manifest_list')
.and_return(None))
(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_manifest_index')
.and_return(None))
(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_all_manifests')
.and_return(resp))
Expand Down Expand Up @@ -399,6 +408,9 @@ def workflow_callback(workflow):
(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_manifest_list')
.and_return(None))
(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_manifest_index')
.and_return(None))
return workflow

with pytest.raises(PluginFailedException) as exc_info:
Expand Down Expand Up @@ -462,6 +474,11 @@ def workflow_callback(workflow):
.with_args(manifest_image)
.and_return(None)
.once())
(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_manifest_index')
.with_args(manifest_image)
.and_return(None)
.once())
return workflow

with pytest.raises(PluginFailedException) as exc_info:
Expand Down Expand Up @@ -509,6 +526,11 @@ def workflow_callback(workflow):
.with_args(manifest_image_original)
.and_return(None)
.times(2))
(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_manifest_index')
.with_args(manifest_image_original)
.and_return(None)
.times(2))
docker_tag = '{}-{}'.format(version, release)
manifest_tag = '{}/{}:{}'. \
format(SOURCE_REGISTRY,
Expand Down Expand Up @@ -594,7 +616,7 @@ def workflow_callback(workflow):
builder_digests_dict = test_vals['workflow'].data.parent_images_digests
assert builder_digests_dict == test_vals['expected_digest']

def prepare(self, workflow, mock_get_manifest_list=False):
def prepare(self, workflow, mock_get_manifest_list=False, mock_get_manifest_index=False):
# Setup expected platforms
env = (
MockEnv(workflow)
Expand All @@ -607,17 +629,29 @@ def prepare(self, workflow, mock_get_manifest_list=False):
)
)

manifest_list = {
'manifests': [
{'platform': {'architecture': 'amd64'}, 'digest': 'sha256:123456'},
{'platform': {'architecture': 'ppc64le'}, 'digest': 'sha256:654321'},
]
}

if mock_get_manifest_list:
# Setup multi-arch manifest list
manifest_list = {
'manifests': [
{'platform': {'architecture': 'amd64'}, 'digest': 'sha256:123456'},
{'platform': {'architecture': 'ppc64le'}, 'digest': 'sha256:654321'},
]
}
(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_manifest_list')
.and_return(flexmock(json=lambda: manifest_list,
content=json.dumps(manifest_list).encode('utf-8'))))

if mock_get_manifest_index:
# Setup multi-arch oci index
(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_manifest_index')
.and_return(flexmock(json=lambda: manifest_list,
content=json.dumps(manifest_list).encode('utf-8'))))

(flexmock(atomic_reactor.util.RegistryClient)
.should_receive('get_manifest_list')
.and_return(None))

return env.workflow
16 changes: 10 additions & 6 deletions tests/plugins/test_fetch_sources.py
Original file line number Diff line number Diff line change
Expand Up @@ -1205,7 +1205,9 @@ def test_lookaside_cache(self, caplog, requests_mock, koji_session, workflow, so
assert log_msg in caplog.text

@pytest.mark.parametrize('reason', ['external', 'other'])
def test_missing_srpm_header(self, koji_session, workflow, source_dir, reason):
def test_missing_srpm_header(self, caplog, requests_mock, koji_session, workflow,
source_dir, reason):
mock_koji_manifest_download(source_dir, requests_mock)
(flexmock(koji_session)
.should_receive('listArchives')
.with_args(object, type='image')
Expand All @@ -1225,14 +1227,16 @@ def test_missing_srpm_header(self, koji_session, workflow, source_dir, reason):
.and_return({}))

runner = mock_env(workflow, source_dir, koji_build_nvr='foobar-1-1')
with pytest.raises(PluginFailedException) as exc_info:
runner.run()

if reason == 'external':
assert 'RPM comes from an external repo' in str(exc_info.value)
else:
if reason != 'external':
with pytest.raises(PluginFailedException) as exc_info:
runner.run()
assert 'Missing SOURCERPM header' in str(exc_info.value)

else:
runner.run()
assert 'RPM comes from an external repo' in caplog.text

def test_no_srpms_and_remote_sources(self, koji_session, workflow, source_dir):
set_no_remote_source_in_koji_build(koji_session)
(flexmock(koji_session)
Expand Down
19 changes: 15 additions & 4 deletions tests/plugins/test_koji_parent.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,15 +141,26 @@ def test_koji_ssl_certs_used(self, tmpdir, workflow, koji_session): # noqa
plugin_args = {'koji_ssl_certs_dir': str(tmpdir)}
self.run_plugin_with_args(workflow, plugin_args)

def test_koji_build_not_found(self, workflow, koji_session): # noqa
@pytest.mark.parametrize('external', [True, False])
def test_koji_build_not_found(self, caplog, workflow, koji_session, external): # noqa
(flexmock(koji_session)
.should_receive('getBuild')
.with_args(KOJI_BUILD_NVR)
.and_return(None))

with pytest.raises(PluginFailedException) as exc_info:
self.run_plugin_with_args(workflow, {'poll_timeout': 0.01})
assert 'KojiParentBuildMissing' in str(exc_info.value)
if external:
result = {
PARENT_IMAGES_KOJI_BUILDS: {
ImageName.parse('base').to_str(): None,
}
}
self.run_plugin_with_args(workflow, expect_result=result,
external_base=external)
assert 'Parent image Koji build NOT found for' in caplog.text
else:
with pytest.raises(PluginFailedException) as exc_info:
self.run_plugin_with_args(workflow, {'poll_timeout': 0.01}, external_base=external)
assert 'KojiParentBuildMissing' in str(exc_info.value)

def test_koji_build_deleted(self, workflow, koji_session): # noqa
(flexmock(koji_session)
Expand Down