From 742b4419ed07b8345ad516648382802c9b36db9a Mon Sep 17 00:00:00 2001 From: Robert Cerven Date: Tue, 22 Oct 2024 20:03:48 +0200 Subject: [PATCH] Allow konflux images with oci index, be used in osbs Don't explicitly require parent images to be found it koji and lower waiting time for trying to get koji build * STONEBLD-2896 Signed-off-by: Robert Cerven --- atomic_reactor/config.py | 2 +- atomic_reactor/plugins/check_base_image.py | 5 ++ atomic_reactor/plugins/fetch_sources.py | 8 +-- atomic_reactor/plugins/koji_parent.py | 11 ++-- atomic_reactor/plugins/resolve_composes.py | 11 +++- atomic_reactor/schemas/config.json | 4 +- atomic_reactor/util.py | 34 ++++++++++--- tests/plugins/test_check_base_image.py | 58 +++++++++++++++++----- tests/plugins/test_fetch_sources.py | 16 +++--- tests/plugins/test_koji_parent.py | 19 +++++-- 10 files changed, 129 insertions(+), 39 deletions(-) diff --git a/atomic_reactor/config.py b/atomic_reactor/config.py index dd2df3a15..547444304 100644 --- a/atomic_reactor/config.py +++ b/atomic_reactor/config.py @@ -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) @property def deep_manifest_list_inspection(self): diff --git a/atomic_reactor/plugins/check_base_image.py b/atomic_reactor/plugins/check_base_image.py index 5923a7644..b02f07cc0 100644 --- a/atomic_reactor/plugins/check_base_image.py +++ b/atomic_reactor/plugins/check_base_image.py @@ -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() diff --git a/atomic_reactor/plugins/fetch_sources.py b/atomic_reactor/plugins/fetch_sources.py index b7aea8deb..0ba7d3296 100644 --- a/atomic_reactor/plugins/fetch_sources.py +++ b/atomic_reactor/plugins/fetch_sources.py @@ -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']) + self.log.warning(msg) + continue rpm_hdr = self.session.getRPMHeaders(rpm_id, headers=['SOURCERPM']) if 'SOURCERPM' not in rpm_hdr: diff --git a/atomic_reactor/plugins/koji_parent.py b/atomic_reactor/plugins/koji_parent.py index f5df1b64f..593ede486 100644 --- a/atomic_reactor/plugins/koji_parent.py +++ b/atomic_reactor/plugins/koji_parent.py @@ -23,7 +23,7 @@ import time -DEFAULT_POLL_TIMEOUT = 60 * 10 # 10 minutes +DEFAULT_POLL_TIMEOUT = 60 * 3 # 3 minutes DEFAULT_POLL_INTERVAL = 10 # 10 seconds @@ -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. @@ -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.""" diff --git a/atomic_reactor/plugins/resolve_composes.py b/atomic_reactor/plugins/resolve_composes.py index d2654aa0e..a71205359 100644 --- a/atomic_reactor/plugins/resolve_composes.py +++ b/atomic_reactor/plugins/resolve_composes.py @@ -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: @@ -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'] diff --git a/atomic_reactor/schemas/config.json b/atomic_reactor/schemas/config.json index 472f46d1d..785451db0 100644 --- a/atomic_reactor/schemas/config.json +++ b/atomic_reactor/schemas/config.json @@ -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", diff --git a/atomic_reactor/util.py b/atomic_reactor/util.py index fa21e635d..4c4e0eb7a 100644 --- a/atomic_reactor/util.py +++ b/atomic_reactor/util.py @@ -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 @@ -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. @@ -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() @@ -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 @@ -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 diff --git a/tests/plugins/test_check_base_image.py b/tests/plugins/test_check_base_image.py index 30dd8f2c9..95292a798 100644 --- a/tests/plugins/test_check_base_image.py +++ b/tests/plugins/test_check_base_image.py @@ -343,7 +343,7 @@ 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): @@ -351,16 +351,25 @@ 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)) @@ -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: @@ -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: @@ -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, @@ -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) @@ -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 diff --git a/tests/plugins/test_fetch_sources.py b/tests/plugins/test_fetch_sources.py index 4baf21b4f..e452f0eee 100644 --- a/tests/plugins/test_fetch_sources.py +++ b/tests/plugins/test_fetch_sources.py @@ -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') @@ -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) diff --git a/tests/plugins/test_koji_parent.py b/tests/plugins/test_koji_parent.py index 4ab867029..5f6624f75 100644 --- a/tests/plugins/test_koji_parent.py +++ b/tests/plugins/test_koji_parent.py @@ -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)