diff --git a/AGENTS.md b/AGENTS.md index a410b1006..609fed7f9 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -57,3 +57,4 @@ Key modules: - **Never edit existing Alembic migrations** — generate a new revision instead. - **API ↔ Worker task signatures must match** — renaming args silently breaks in-flight requests. - **Avoid making changes to the old worker architecture** - Always ask before making changes to the following functions - `handle_add_request`, `handle_rm_request`, `handle_merge_request`, `handle_create_empty_index_request`, `handle_fbc_operation_request`, `handle_regenerate_bundle_request` +- **Divergent-tag requests never merge their MR** — they reuse the base OCP branch's Konflux Component and must stay throw-away; the guard is `overwrite_from_index and not sources.is_divergent`. Never source their index.db from ORAS. diff --git a/docker/containerized/README.md b/docker/containerized/README.md index 375a1465e..26b564960 100644 --- a/docker/containerized/README.md +++ b/docker/containerized/README.md @@ -253,6 +253,39 @@ The worker configuration is in `docker/containerized/worker_config.py`. This fil - Includes the containerized task modules - Validates required configuration on startup +## Git Branch and Tag Semantics + +The Git branch used for a request's catalog is keyed on the **image tag**, not a fixed OCP-version mapping. For the existing fleet this is a no-op, since prod index tags already equal the OCP version (e.g. `v4.19` → branch `v4.19`). + +### Onboarding a Non-OCP Tag + +To onboard a tag that isn't an OCP version (e.g. a custom or pre-release tag), a maintainer must: + +1. Create a Git branch in the catalog repository named exactly after the tag. +2. Provision a Konflux Component for that branch, so pushes to it trigger a PipelineRun. + +Once both exist, requests against that tag build and push normally, and `overwrite_from_index` is honored like any other branch. + +### Divergent Tags (No Matching Branch) + +If a request targets a tag with no corresponding Git branch — for example a timestamped, point-in-time tag cut from an existing index — IIB treats it as a **divergent tag**: + +- Configs and `index.db` are extracted directly from the index image (unprivileged), not sourced from ORAS. Only the FBC configs and the hidden `index.db` are extracted; if the image carries no hidden `index.db`, the request fails ("no index.db found, onboard the image to build") — there is no labeled-db or empty-db fallback. +- The build reuses the base OCP branch's existing Konflux Component via a throw-away merge request. +- That MR is **never merged** — it is always closed after the pipeline completes (or on failure), regardless of outcome. +- `overwrite_from_index` is **rejected** for divergent-tag requests; there is no direct-push path. + +This lets IIB build and validate a one-off tag without requiring per-tag branch/Component provisioning. + +## Index DB Artifact and ImageStream Tag Naming + +Cached `index.db` artifact tags (ORAS) and ImageStream tags are keyed on the index image's content (manifest) digest — `idb-` — resolved via `skopeo inspect`, not on its pullspec. Because the key is the content itself, it is both namespace-safe and promotion-safe: + +- **Namespace-safe:** two images that share a repository name in different registry namespaces (e.g. `quay.io/redhat/my-index:v4.17` vs `quay.io/redhat-pending/my-index:v4.17`) have different content and therefore different digests, so they never collide on the same cache tag. +- **Promotion-safe:** the same image content addressed by different pullspecs after a release or mirror (e.g. `quay.io/my-namespace/iib-pub:v4.17` → `registry.access.redhat.com/some-namespace/operator-index:v4.17`) preserves its manifest digest, so both pullspecs resolve to the *same* cache entry and share one `index.db`. + +Cache entries written under the previous pullspec-derived naming scheme are orphaned by this change — they are not migrated in place. On the normal path IIB never falls back to extracting `index.db` from the image: if the digest-keyed artifact is missing, the request fails with a "no index.db found for the image, onboard the image to build" error, and the image must be onboarded (which populates the artifact) before it can be built. Orphaned entries are cleaned up by the existing cache-pruning process rather than any code path in this workflow. + ## Differences from Traditional Workflow | Aspect | Traditional Workflow | Containerized Workflow | diff --git a/iib/exceptions.py b/iib/exceptions.py index 72cdd28b4..c52454aaf 100644 --- a/iib/exceptions.py +++ b/iib/exceptions.py @@ -13,6 +13,15 @@ class IIBError(BaseException): """An error was encountered in IIB.""" +class FileNotFoundInImageError(IIBError): + """A requested path was not present in a container image. + + Subclasses IIBError so existing ``except IIBError`` handlers still catch it, + while letting callers distinguish a genuinely absent path from a real + extraction failure (registry, OCI parsing, layer, or tar error). + """ + + class ValidationError(BaseException): """Denote invalid input.""" diff --git a/iib/workers/config.py b/iib/workers/config.py index 2b8bdd3db..7ffec7435 100644 --- a/iib/workers/config.py +++ b/iib/workers/config.py @@ -71,7 +71,7 @@ class Config(object): iib_index_db_oras_auth_path: str = os.path.join( os.path.expanduser('~'), '.docker', 'oras', 'config.json' ) - iib_index_db_artifact_tag_template: str = '{image_name}-{tag}' + iib_index_db_artifact_tag_template: str = 'idb-{digest}' iib_index_db_artifact_template: str = '{registry}/index-db:{tag}' # Whether to use OpenShift ImageStream cache for index.db artifacts # Requires OpenShift cluster with ImageStream configured diff --git a/iib/workers/tasks/build_containerized_add.py b/iib/workers/tasks/build_containerized_add.py index cc614c9ec..e3dc7c2d8 100644 --- a/iib/workers/tasks/build_containerized_add.py +++ b/iib/workers/tasks/build_containerized_add.py @@ -20,7 +20,7 @@ ) from iib.workers.tasks.celery import app from iib.workers.tasks.containerized_utils import ( - prepare_git_repository_for_build, + prepare_build_sources, fetch_and_verify_index_db_artifact, write_build_metadata, git_commit_and_create_mr, @@ -148,11 +148,10 @@ def handle_containerized_add_request( distribution_scope = prebuild_info['distribution_scope'] index_to_gitlab_push_map = index_to_gitlab_push_map or {} - # Variables mr_details, last_commit_sha and original_index_db_digest - # needs to be assigned; otherwise cleanup_on_failure() fails when an exception is raised. + # Variables mr_details and last_commit_sha need to be assigned; otherwise + # cleanup_on_failure() fails when an exception is raised. mr_details: Optional[Dict[str, str]] = None last_commit_sha: Optional[str] = None - original_index_db_digest: Optional[str] = None Opm.set_opm_version(from_index_resolved) @@ -160,26 +159,30 @@ def handle_containerized_add_request( present_bundles: List[BundleImage] = [] present_bundles_pull_spec: List[str] = [] with tempfile.TemporaryDirectory(prefix=f'iib-{request_id}-') as temp_dir: - branch = prebuild_info['ocp_version'] - - # Set up and clone Git repository - ( - index_git_repo, - local_git_repo_path, - localized_git_catalog_path, - ) = prepare_git_repository_for_build( + sources = prepare_build_sources( request_id=request_id, from_index=str(from_index), + from_index_resolved=from_index_resolved, temp_dir=temp_dir, - branch=branch, + ocp_version=prebuild_info['ocp_version'], index_to_gitlab_push_map=index_to_gitlab_push_map, + overwrite_from_index=overwrite_from_index, ) - - # Pull index.db artifact (uses ImageStream cache if configured, otherwise pulls directly) - artifact_index_db_file = fetch_and_verify_index_db_artifact( - from_index=str(from_index), - temp_dir=temp_dir, - ) + index_git_repo = sources.index_git_repo + local_git_repo_path = sources.local_git_repo_path + localized_git_catalog_path = sources.localized_git_catalog_path + branch = sources.target_branch + + # Divergent path already has index.db extracted from the image; the normal + # path pulls it from ORAS. NEVER fall back to ORAS on the divergent path — + # that would read the base OCP branch's index.db. + if sources.index_db_path is not None: + artifact_index_db_file = sources.index_db_path + else: + artifact_index_db_file = fetch_and_verify_index_db_artifact( + from_index=str(from_index), + temp_dir=temp_dir, + ) msg = 'Checking if bundles are already present in index image' log.info(msg) @@ -330,12 +333,13 @@ def handle_containerized_add_request( # Push updated index.db before merging the MR so that on failure both # git and the index.db artifact remain consistent (MR stays open, - # cleanup_on_failure closes it and rolls back the artifact). - original_index_db_digest = push_index_db_artifact( + # cleanup_on_failure closes it; the content-addressed artifact needs no rollback). + push_index_db_artifact( request_id=request_id, from_index=str(from_index), index_db_path=artifact_index_db_file, operators=operators, + output_image=image_url, overwrite_from_index=overwrite_from_index, request_type='add', ) @@ -343,7 +347,7 @@ def handle_containerized_add_request( # Merge or close the MR as the final step so that all side effects # (replication, metadata, index.db push) have succeeded before git # is advanced. This prevents git/index.db divergence on partial failure. - if overwrite_from_index: + if overwrite_from_index and not sources.is_divergent: merge_mr_after_build(mr_details, index_git_repo) # Prevent cleanup_on_failure from trying to close an already-merged MR mr_details = None @@ -364,7 +368,6 @@ def handle_containerized_add_request( request_id=request_id, from_index=str(from_index), index_repo_map=index_to_gitlab_push_map or {}, - original_index_db_digest=original_index_db_digest, reason=f"error: {e}", ) raise IIBError(f"Failed to add bundles: {e}") diff --git a/iib/workers/tasks/build_containerized_create_empty_index.py b/iib/workers/tasks/build_containerized_create_empty_index.py index ed6bfc348..203db2aa8 100644 --- a/iib/workers/tasks/build_containerized_create_empty_index.py +++ b/iib/workers/tasks/build_containerized_create_empty_index.py @@ -34,7 +34,6 @@ opm_validate, ) from iib.workers.tasks.oras_utils import ( - _get_artifact_combined_tag, _get_name_and_tag_from_pullspec, get_oras_artifact, ) @@ -176,7 +175,6 @@ def handle_containerized_create_empty_index_request( index_git_repo: Optional[str] = None last_commit_sha: Optional[str] = None output_pull_spec: Optional[str] = None - original_index_db_digest: Optional[str] = None with tempfile.TemporaryDirectory(prefix=f'iib-{request_id}-') as temp_dir: branch = ocp_version @@ -199,11 +197,16 @@ def handle_containerized_create_empty_index_request( conf = get_worker_config() empty_tag = conf.get('iib_empty_index_db_tag', 'empty') - # Construct the pullspec for the empty index.db artifact + # Construct the pullspec for the empty index.db artifact. + # This tag intentionally does NOT use the content-digest key that + # _get_artifact_combined_tag derives for real indexes: the empty index.db + # is a shared, content-free seed artifact keyed only by image name + + # "empty", so it is deliberately reusable across indexes rather than tied + # to any single image's manifest digest. image_name, _ = _get_name_and_tag_from_pullspec(from_index) empty_artifact_ref = conf['iib_index_db_artifact_template'].format( registry=conf['iib_index_db_artifact_registry'], - tag=_get_artifact_combined_tag(image_name, empty_tag), + tag=f"{image_name}-{empty_tag}", ) log.info('Fetching empty index.db from %s', empty_artifact_ref) @@ -344,12 +347,13 @@ def handle_containerized_create_empty_index_request( # Push the empty index.db with request ID tag # Since overwrite_from_index is False, this will only push with request_id tag - # and will not overwrite the v4.x tag - original_index_db_digest = push_index_db_artifact( + # and will not overwrite the current artifact + push_index_db_artifact( request_id=request_id, from_index=from_index, index_db_path=str(index_db_path), operators=[], # Empty list since we're creating an empty index + output_image=image_url, overwrite_from_index=False, # Always False for create_empty_index request_type='create_empty_index', ) @@ -372,7 +376,6 @@ def handle_containerized_create_empty_index_request( request_id=request_id, from_index=from_index, index_repo_map=index_to_gitlab_push_map or {}, - original_index_db_digest=original_index_db_digest, reason=f"error: {e}", ) raise IIBError(f"Failed to create empty index: {e}") diff --git a/iib/workers/tasks/build_containerized_fbc_operations.py b/iib/workers/tasks/build_containerized_fbc_operations.py index d28e1118d..66df83df3 100644 --- a/iib/workers/tasks/build_containerized_fbc_operations.py +++ b/iib/workers/tasks/build_containerized_fbc_operations.py @@ -18,7 +18,7 @@ fetch_and_verify_index_db_artifact, git_commit_and_create_mr, monitor_pipeline_and_extract_image, - prepare_git_repository_for_build, + prepare_build_sources, push_index_db_artifact, replicate_image_to_tagged_destinations, write_build_metadata, @@ -112,11 +112,10 @@ def handle_containerized_fbc_operation_request( distribution_scope = prebuild_info['distribution_scope'] index_to_gitlab_push_map = index_to_gitlab_push_map or {} - # Variables mr_details, last_commit_sha and original_index_db_digest - # needs to be assigned; otherwise cleanup_on_failure() fails when an exception is raised. + # Variables mr_details and last_commit_sha need to be assigned; otherwise + # cleanup_on_failure() fails when an exception is raised. mr_details: Optional[Dict[str, str]] = None last_commit_sha: Optional[str] = None - original_index_db_digest: Optional[str] = None Opm.set_opm_version(from_index_resolved) @@ -131,26 +130,30 @@ def handle_containerized_fbc_operation_request( _update_index_image_build_state(request_id, prebuild_info) with tempfile.TemporaryDirectory(prefix=f'iib-{request_id}-') as temp_dir: - branch = prebuild_info['ocp_version'] - - # Set up and clone Git repository - ( - index_git_repo, - local_git_repo_path, - localized_git_catalog_path, - ) = prepare_git_repository_for_build( + sources = prepare_build_sources( request_id=request_id, from_index=from_index, + from_index_resolved=from_index_resolved, temp_dir=temp_dir, - branch=branch, + ocp_version=prebuild_info['ocp_version'], index_to_gitlab_push_map=index_to_gitlab_push_map, + overwrite_from_index=overwrite_from_index, ) + index_git_repo = sources.index_git_repo + local_git_repo_path = sources.local_git_repo_path + localized_git_catalog_path = sources.localized_git_catalog_path + branch = sources.target_branch - # Pull index.db artifact (uses ImageStream cache if configured, otherwise pulls directly) - artifact_index_db_file = fetch_and_verify_index_db_artifact( - from_index=from_index, - temp_dir=temp_dir, - ) + # Divergent path already has index.db extracted from the image; the normal + # path pulls it from ORAS. NEVER fall back to ORAS on the divergent path — + # that would read the base OCP branch's index.db. + if sources.index_db_path is not None: + artifact_index_db_file = sources.index_db_path + else: + artifact_index_db_file = fetch_and_verify_index_db_artifact( + from_index=from_index, + temp_dir=temp_dir, + ) set_request_state(request_id, 'in_progress', 'Adding fbc fragment') ( @@ -232,12 +235,13 @@ def handle_containerized_fbc_operation_request( # Push updated index.db before merging the MR so that on failure both # git and the index.db artifact remain consistent (MR stays open, - # cleanup_on_failure closes it and rolls back the artifact). - original_index_db_digest = push_index_db_artifact( + # cleanup_on_failure closes it; the content-addressed artifact needs no rollback). + push_index_db_artifact( request_id=request_id, from_index=from_index, index_db_path=index_db_path, operators=operators_in_db, + output_image=image_url, overwrite_from_index=overwrite_from_index, request_type='fbc_operations', ) @@ -245,7 +249,7 @@ def handle_containerized_fbc_operation_request( # Merge or close the MR as the final step so that all side effects # (replication, metadata, index.db push) have succeeded before git # is advanced. This prevents git/index.db divergence on partial failure. - if overwrite_from_index: + if overwrite_from_index and not sources.is_divergent: merge_mr_after_build(mr_details, index_git_repo) # Prevent cleanup_on_failure from trying to close an already-merged MR mr_details = None @@ -267,7 +271,6 @@ def handle_containerized_fbc_operation_request( request_id=request_id, from_index=from_index, index_repo_map=index_to_gitlab_push_map or {}, - original_index_db_digest=original_index_db_digest, reason=f"error: {e}", ) raise IIBError(f"Failed to add FBC fragment: {e}") diff --git a/iib/workers/tasks/build_containerized_merge.py b/iib/workers/tasks/build_containerized_merge.py index 299123b8e..d6a61a8e5 100644 --- a/iib/workers/tasks/build_containerized_merge.py +++ b/iib/workers/tasks/build_containerized_merge.py @@ -159,7 +159,6 @@ def handle_containerized_merge_request( index_git_repo: Optional[str] = None last_commit_sha: Optional[str] = None output_pull_spec: Optional[str] = None - original_index_db_digest: Optional[str] = None with tempfile.TemporaryDirectory(prefix=f'iib-{request_id}-') as temp_dir: # Setup and clone Git repository @@ -371,12 +370,13 @@ def handle_containerized_merge_request( # Push updated index.db before merging the MR so that on failure both # git and the index.db artifact remain consistent (MR stays open, - # cleanup_on_failure closes it and rolls back the artifact). - original_index_db_digest = push_index_db_artifact( + # cleanup_on_failure closes it; the content-addressed artifact needs no rollback). + push_index_db_artifact( request_id=request_id, from_index=effective_index_image, index_db_path=source_index_db_path, operators=operators_in_db, + output_image=image_url, overwrite_from_index=overwrite_target_index, request_type='merge', ) @@ -407,7 +407,6 @@ def handle_containerized_merge_request( request_id=request_id, from_index=effective_index_image, index_repo_map={}, - original_index_db_digest=original_index_db_digest, reason=f"error: {e}", ) # Reset Docker config for the next request. This is a fail safe. diff --git a/iib/workers/tasks/build_containerized_regenerate_bundle.py b/iib/workers/tasks/build_containerized_regenerate_bundle.py index 6db7d81cf..1f4b80dbd 100644 --- a/iib/workers/tasks/build_containerized_regenerate_bundle.py +++ b/iib/workers/tasks/build_containerized_regenerate_bundle.py @@ -274,7 +274,6 @@ def handle_containerized_regenerate_bundle_request( request_id=request_id, from_index='', # No from_index for bundle regeneration index_repo_map={}, - original_index_db_digest=None, # No index.db for bundle regeneration reason=f"error: {e}", ) raise IIBError(f"Failed to regenerate bundle: {e}") diff --git a/iib/workers/tasks/build_containerized_rm.py b/iib/workers/tasks/build_containerized_rm.py index 1d5185953..029dcec6d 100644 --- a/iib/workers/tasks/build_containerized_rm.py +++ b/iib/workers/tasks/build_containerized_rm.py @@ -20,7 +20,7 @@ fetch_and_verify_index_db_artifact, git_commit_and_create_mr, monitor_pipeline_and_extract_image, - prepare_git_repository_for_build, + prepare_build_sources, push_index_db_artifact, replicate_image_to_tagged_destinations, write_build_metadata, @@ -131,29 +131,32 @@ def handle_containerized_rm_request( operators_in_db: Set[str] = set() last_commit_sha: Optional[str] = None output_pull_spec: Optional[str] = None - original_index_db_digest: Optional[str] = None with tempfile.TemporaryDirectory(prefix=f'iib-{request_id}-') as temp_dir: - branch = ocp_version - - # Set up and clone Git repository - ( - index_git_repo, - local_git_repo_path, - localized_git_catalog_path, - ) = prepare_git_repository_for_build( + sources = prepare_build_sources( request_id=request_id, from_index=from_index, + from_index_resolved=from_index_resolved, temp_dir=temp_dir, - branch=branch, + ocp_version=ocp_version, index_to_gitlab_push_map=index_to_gitlab_push_map or {}, + overwrite_from_index=overwrite_from_index, ) - - # Pull index.db artifact (uses ImageStream cache if configured, otherwise pulls directly) - index_db_path = fetch_and_verify_index_db_artifact( - from_index=from_index, - temp_dir=temp_dir, - ) + index_git_repo = sources.index_git_repo + local_git_repo_path = sources.local_git_repo_path + localized_git_catalog_path = sources.localized_git_catalog_path + branch = sources.target_branch + + # Divergent path already has index.db extracted from the image; the normal + # path pulls it from ORAS. NEVER fall back to ORAS on the divergent path — + # that would read the base OCP branch's index.db. + if sources.index_db_path is not None: + index_db_path = sources.index_db_path + else: + index_db_path = fetch_and_verify_index_db_artifact( + from_index=from_index, + temp_dir=temp_dir, + ) # Remove operators from /configs set_request_state(request_id, 'in_progress', 'Removing operators from catalog') @@ -286,12 +289,13 @@ def handle_containerized_rm_request( # Push updated index.db before merging the MR so that on failure both # git and the index.db artifact remain consistent (MR stays open, - # cleanup_on_failure closes it and rolls back the artifact). - original_index_db_digest = push_index_db_artifact( + # cleanup_on_failure closes it; the content-addressed artifact needs no rollback). + push_index_db_artifact( request_id=request_id, from_index=from_index, index_db_path=index_db_path, operators=operators, + output_image=image_url, overwrite_from_index=overwrite_from_index, request_type='rm', ) @@ -299,7 +303,7 @@ def handle_containerized_rm_request( # Merge or close the MR as the final step so that all side effects # (replication, metadata, index.db push) have succeeded before git # is advanced. This prevents git/index.db divergence on partial failure. - if overwrite_from_index: + if overwrite_from_index and not sources.is_divergent: merge_mr_after_build(mr_details, index_git_repo) # Prevent cleanup_on_failure from trying to close an already-merged MR mr_details = None @@ -322,7 +326,6 @@ def handle_containerized_rm_request( request_id=request_id, from_index=from_index, index_repo_map=index_to_gitlab_push_map or {}, - original_index_db_digest=original_index_db_digest, reason=f"error: {e}", ) raise IIBError(f"Failed to remove operators: {e}") diff --git a/iib/workers/tasks/containerized_utils.py b/iib/workers/tasks/containerized_utils.py index 160f491c7..c506fa93a 100644 --- a/iib/workers/tasks/containerized_utils.py +++ b/iib/workers/tasks/containerized_utils.py @@ -2,15 +2,16 @@ """This file contains utility functions for containerized IIB operations.""" import json import logging +import posixpath import queue import shutil -import tarfile import tempfile import threading +from dataclasses import dataclass from pathlib import Path from typing import Dict, List, Optional, Tuple, Union -from iib.exceptions import IIBError +from iib.exceptions import IIBError, FileNotFoundInImageError from iib.workers.api_utils import set_request_state from iib.workers.config import get_worker_config from iib.workers.tasks.iib_static_types import BundleImage @@ -22,6 +23,7 @@ get_git_token, get_last_commit_sha, merge_mr, + remote_branch_exists, resolve_git_url, revert_last_commit, ) @@ -31,9 +33,8 @@ wait_for_pipeline_completion, ) from iib.workers.tasks.oras_utils import ( - _get_artifact_combined_tag, - _get_name_and_tag_from_pullspec, - get_image_digest, + _get_index_digest, + get_index_tag, get_indexdb_artifact_pullspec, get_imagestream_artifact_pullspec, get_oras_artifact, @@ -41,107 +42,219 @@ refresh_indexdb_cache_for_image, verify_indexdb_cache_for_image, ) -from iib.workers.tasks.utils import run_cmd, skopeo_inspect +from iib.workers.tasks.utils import get_image_label, skopeo_inspect log = logging.getLogger(__name__) def extract_files_from_image_non_privileged(image: str, src_path: str, dest_path: str) -> None: """ - Extract files from container image without podman/docker runtime. - - This function uses skopeo to download the image as OCI layout, then extracts - the requested path from the image layers. This approach works in non-privileged - environments without container runtime access. + Extract a file or directory from a container image, unprivileged. + + Delegates to ``oc image extract``, which correctly applies OCI layer and + whiteout semantics and handles the absolute symlinks (e.g. /etc/alternatives, + /etc/crypto-policies) and read-only cross-layer overwrites (e.g. /etc/machine-id) + that a hand-rolled ``tarfile`` reconstruction of the whole root filesystem + chokes on. It works without a container runtime, so it is safe in the + unprivileged API/worker containers. ``oc`` is already installed in the worker + base image (see docker/Dockerfile-base-image). + + ``oc image extract`` distinguishes files from directories by argument shape and + exits 0 while extracting nothing when the path is absent, so this function + probes both shapes and treats "nothing extracted" as a missing path: + + * directory: ``--path=/*:`` unpacks the directory's contents into + ``dest_path`` (matching the previous copytree-of-contents behaviour); + * file: ``--path=:`` places the file at ``/``, which + is then copied to ``dest_path`` (which may have a different basename). + + A glob on a file path matches nothing, so trying the directory form first is + safe; the file form is only reached (a second ``oc`` invocation) when the + directory form yields nothing. + + ``oc image extract`` only unpacks file entries: an empty directory (a directory + with no files under it, e.g. an empty index's ``/configs``) contributes no + entries and is indistinguishable here from an absent path — both raise + ``FileNotFoundInImageError``. Callers that can legitimately expect an empty + directory (because they hold another signal, such as the image's configs + label) must catch this and treat it as empty rather than missing. :param str image: the pull specification of the container image - :param str src_path: the full path within the container image to copy from - :param str dest_path: the full path on the local host to copy into - :raises IIBError: if the extraction fails or src_path is not found + :param str src_path: the absolute path within the container image to copy from + :param str dest_path: the path on the local host to copy into + :raises FileNotFoundInImageError: if src_path is absent (or an empty directory) + :raises IIBError: if src_path is not absolute or ``oc image extract`` fails """ - # Create temporary directory for OCI layout + from iib.workers.tasks.utils import run_cmd + + if not src_path.startswith('/'): + raise IIBError(f'src_path must be an absolute image path, got {src_path!r}') + + normalized = src_path.rstrip('/') + if not normalized: + raise IIBError(f'src_path must name a path under /, got {src_path!r}') + with tempfile.TemporaryDirectory(prefix='iib-extract-') as temp_dir: temp_path = Path(temp_dir) - oci_dir = temp_path / 'oci' - oci_dir.mkdir(parents=True, exist_ok=True) - # Download image as OCI layout using skopeo - log.info('Downloading image %s as OCI layout', image) - _skopeo_copy( - source=f'docker://{image}', - destination=f'oci:{oci_dir}', - copy_all=False, - exc_msg=f'Failed to download image {image} as OCI layout', + # 1) Directory form: unpack the directory's children into a staging dir. + dir_staging = temp_path / 'dir' + dir_staging.mkdir() + log.info('Extracting %s from image %s to %s', src_path, image, dest_path) + run_cmd( + [ + 'oc', + 'image', + 'extract', + '--confirm', + f'--path={normalized}/*:{dir_staging.as_posix()}', + image, + ], + exc_msg=f'Failed to extract {src_path} from image {image}', ) - - # Read OCI index to find the manifest - index_path = oci_dir / 'index.json' - if not index_path.exists(): - raise IIBError(f'OCI index.json not found at {index_path}') - - with open(index_path, 'r') as f: - index = json.load(f) - - # Get the manifest digest from the index - manifests = index.get('manifests', []) - if not manifests: - raise IIBError(f'No manifests found in OCI index for image {image}') - - manifest_digest = manifests[0]['digest'].replace('sha256:', '') - manifest_path = oci_dir / 'blobs' / 'sha256' / manifest_digest - - # Read manifest to get layer information - with open(manifest_path, 'r') as f: - manifest = json.load(f) - - layers = manifest.get('layers', []) - if not layers: - raise IIBError(f'No layers found in manifest for image {image}') - - # Create extraction directory to build the filesystem - extract_dir = temp_path / 'rootfs' - extract_dir.mkdir(parents=True, exist_ok=True) - - # Extract each layer in order to build the complete filesystem - log.info('Extracting %d layers from image %s', len(layers), image) - for layer in layers: - layer_digest = layer['digest'].replace('sha256:', '') - layer_path = oci_dir / 'blobs' / 'sha256' / layer_digest - - if not layer_path.exists(): - raise IIBError(f'Layer blob not found at {layer_path}') - - # Extract layer tar.gz to build filesystem - try: - with tarfile.open(layer_path, 'r:gz') as tar: - # Extract all members safely with path traversal protection - tar.extractall(path=extract_dir, filter='data') - except Exception as e: - raise IIBError(f'Failed to extract layer {layer_digest}: {e}') - - # Normalize src_path (remove leading slash for filesystem access) - normalized_src = src_path.lstrip('/') - source_full_path = extract_dir / normalized_src - - # Verify the requested path exists in the extracted filesystem - if not source_full_path.exists(): - raise IIBError( - f'Path {src_path} not found in image {image}. ' - f'Looked for {source_full_path} in extracted filesystem.' + if any(dir_staging.iterdir()): + dest = Path(dest_path) + dest.mkdir(parents=True, exist_ok=True) + # Copy the directory's contents into dest_path, preserving any nested + # symlinks verbatim (symlinks=True) rather than resolving them against + # the host root during the copy. + shutil.copytree(dir_staging, dest, dirs_exist_ok=True, symlinks=True) + log.info( + 'Extracted %s from image %s to %s as a directory (glob form)', + src_path, + image, + dest_path, ) + return + log.debug( + 'Directory-glob form matched nothing for %s in image %s; trying no-glob form', + src_path, + image, + ) - # Copy the requested path to destination - dest = Path(dest_path) - log.info('Copying %s from image to %s', src_path, dest_path) - if source_full_path.is_dir(): - # If source is a directory, copy its contents - shutil.copytree(source_full_path, dest, dirs_exist_ok=True) - else: - # If source is a file, copy the file + # 2) File form: the path is a single file, placed at /. + file_staging = temp_path / 'file' + file_staging.mkdir() + run_cmd( + [ + 'oc', + 'image', + 'extract', + '--confirm', + f'--path={normalized}:{file_staging.as_posix()}', + image, + ], + exc_msg=f'Failed to extract {src_path} from image {image}', + ) + extracted_file = file_staging / posixpath.basename(normalized) + if extracted_file.is_file(): + dest = Path(dest_path) dest.parent.mkdir(parents=True, exist_ok=True) - shutil.copy2(source_full_path, dest) + shutil.copy2(extracted_file, dest) + log.info( + 'Extracted %s from image %s to %s as a file (no-glob form)', + src_path, + image, + dest_path, + ) + return + + # 3) Neither shape produced output. Because 'oc image extract' unpacks only + # file entries, this means src_path is either absent or an empty directory + # (the two are indistinguishable here). Raise the specific + # FileNotFoundInImageError (a subclass of IIBError) so callers can tell this + # apart from an 'oc' failure above and, where they have another signal (e.g. + # a configs label), treat an empty directory as empty rather than missing. + log.error( + 'Nothing extracted for %s in image %s: neither the directory-glob nor the ' + 'no-glob form of "oc image extract" produced output (path is absent or an ' + 'empty directory)', + src_path, + image, + ) + raise FileNotFoundInImageError(f'Path {src_path} not found in image {image}.') + + +def extract_catalog_and_db_from_image(from_index_resolved: str, temp_dir: str) -> Tuple[str, str]: + """ + Extract FBC configs and index.db from an index image, unprivileged. + + Used on the divergent-tag path where no git branch / ORAS artifact exists yet. + The image is the source of truth for its own content. + + The caller must pass the digest-resolved pullspec (``from_index_resolved``), + not the mutable tag, so the extracted content matches the image the request + already inspected during prebuild (OPM version, build metadata) and so the + repeated image reads here cannot disagree with each other. + + Only the FBC configs and the hidden index.db are extracted. The hidden db is + the sole source of truth for the SQLite index; there is no fallback to the + labeled database path and no synthesised empty db. An image that carries no + hidden index.db has not been onboarded to the containerized build flow and + the request is failed so the image can be onboarded first. + + The configs label is the signal that the image declares an FBC root, so a + declared-but-empty configs directory (an empty index, whose ``/configs`` holds + no files) is treated as an empty catalog rather than a missing path — the + divergent add/rm operation then populates it. + + :param str from_index_resolved: The digest-resolved from_index image pullspec. + :param str temp_dir: Base temp directory for extraction. + :return: Tuple of (configs_dir_path, index_db_path). + :rtype: Tuple[str, str] + :raises IIBError: If the image has no FBC configs label, if it carries no + hidden index.db, or if the hidden-db extraction fails for any other + reason (e.g. a registry, OCI parsing, layer, or tar error). + """ + configs_label = get_image_label( + from_index_resolved, 'operators.operatorframework.io.index.configs.v1' + ) + if not configs_label: + raise IIBError(f"Index image {from_index_resolved} does not contain a file-based catalog.") + + configs_dir = str(Path(temp_dir) / 'extracted_configs') + log.info( + 'Extracting FBC configs from %s (label path %s) to %s', + from_index_resolved, + configs_label, + configs_dir, + ) + try: + extract_files_from_image_non_privileged(from_index_resolved, configs_label, configs_dir) + except FileNotFoundInImageError: + # The image declares a configs label but nothing is stored under it. 'oc + # image extract' cannot represent an empty directory, so this is an empty + # FBC catalog (an empty index), not a broken image. Use an empty configs + # directory; the handler's add/rm operation writes the operators into it. + log.info( + 'Configs path %s in %s is empty; using an empty catalog directory at %s', + configs_label, + from_index_resolved, + configs_dir, + ) + Path(configs_dir).mkdir(parents=True, exist_ok=True) + + index_db_path = str(Path(temp_dir) / 'extracted_index.db') + conf = get_worker_config() + hidden_db_path = conf['hidden_index_db_path'] + + try: + # The hidden db is the only source of truth for the SQLite index. + log.info( + 'Extracting hidden index.db from %s (image path %s) to %s', + from_index_resolved, + hidden_db_path, + index_db_path, + ) + extract_files_from_image_non_privileged(from_index_resolved, hidden_db_path, index_db_path) + except FileNotFoundInImageError: + raise IIBError( + f"No index.db found in image {from_index_resolved} at hidden path " + f"{hidden_db_path}. Onboard the image to build." + ) - log.info('Successfully extracted %s from image %s to %s', src_path, image, dest_path) + log.info('Extracted FBC configs to %s and index.db to %s', configs_dir, index_db_path) + return configs_dir, index_db_path class ValidateBundlesThread(threading.Thread): @@ -267,12 +380,18 @@ def pull_index_db_artifact(from_index: str, temp_dir: str) -> str: # Pull directly from Quay — either cache is disabled, stale, or unavailable artifact_ref = get_indexdb_artifact_pullspec(from_index) - artifact_dir = get_oras_artifact( - artifact_ref, - temp_dir, - ) - - return artifact_dir + log.info('Pulling index.db artifact %s into %s', artifact_ref, temp_dir) + try: + return get_oras_artifact( + artifact_ref, + temp_dir, + ) + except IIBError: + log.error('index.db artifact %s not found for image %s', artifact_ref, from_index) + raise IIBError( + f"No index.db found for the image {from_index} (artifact {artifact_ref}). " + "Onboard the image to build." + ) def write_build_metadata( @@ -348,72 +467,68 @@ def push_index_db_artifact( from_index: str, index_db_path: str, operators: List[str], + output_image: str, overwrite_from_index: bool = False, request_type: str = 'rm', -) -> Optional[str]: +) -> None: """ - Push updated index.db artifact to registry with appropriate tags. + Push the updated index.db artifact, keyed on the built OUTPUT image digest. - This function pushes the index.db file to the artifact registry with a request-specific - tag and optionally to the v4.x tag if overwrite_from_index is True. It captures - the original digest of the v4.x tag before overwriting for potential rollback. + A per-request tag is always pushed for traceability. For overwrite requests + (which advance the from_index tag to the output image) the current artifact + ``idb-`` is also pushed (warm-push), so the next request that + resolves the tag to that digest finds it cached. Content keys are never + overwritten in place, so no original-digest rollback is required. :param int request_id: The IIB request ID :param str from_index: The from_index pullspec :param str index_db_path: Path to the index.db file to push :param List[str] operators: List of operators involved in the operation + :param str output_image: The built output image pullspec, used to derive the content key :param bool overwrite_from_index: Whether to overwrite the from_index :param str request_type: Type of request (e.g., 'rm', 'add') - :return: Original digest of v4.x tag if captured, None otherwise - :rtype: Optional[str] - """ - original_index_db_digest = None - - if index_db_path and Path(index_db_path).exists(): - # Get directory and filename separately to push only the filename - # This ensures ORAS extracts the file as just "index.db" without - # directory structure - index_db_file = Path(index_db_path) - index_db_dir = str(index_db_file.parent) - index_db_filename = index_db_file.name - log.info('Pushing from directory: %s, filename: %s', index_db_dir, index_db_filename) - - # Push with request_id tag irrespective of overwrite_from_index - set_request_state(request_id, 'in_progress', 'Pushing updated index database') - image_name, tag = _get_name_and_tag_from_pullspec(from_index) - conf = get_worker_config() - request_artifact_ref = conf['iib_index_db_artifact_template'].format( + """ + if not (index_db_path and Path(index_db_path).exists()): + return + + index_db_file = Path(index_db_path) + index_db_dir = str(index_db_file.parent) + index_db_filename = index_db_file.name + log.info('Pushing from directory: %s, filename: %s', index_db_dir, index_db_filename) + + set_request_state(request_id, 'in_progress', 'Pushing updated index database') + conf = get_worker_config() + output_tag = f'idb-{_get_index_digest(output_image)}' + + request_artifact_ref = conf['iib_index_db_artifact_template'].format( + registry=conf['iib_index_db_artifact_registry'], + tag=f'{output_tag}-{request_id}', + ) + artifact_refs = [request_artifact_ref] + if overwrite_from_index: + current_artifact_ref = conf['iib_index_db_artifact_template'].format( registry=conf['iib_index_db_artifact_registry'], - tag=f"{_get_artifact_combined_tag(image_name, tag)}-{request_id}", + tag=output_tag, ) - artifact_refs = [request_artifact_ref] - if overwrite_from_index: - # Get the current digest of v4.x tag before overwriting it - # This allows us to restore it if anything fails after the push - v4x_artifact_ref = get_indexdb_artifact_pullspec(from_index) - log.info('Capturing original digest of %s for potential rollback', v4x_artifact_ref) - original_index_db_digest = get_image_digest(v4x_artifact_ref) - log.info('Original index.db digest: %s', original_index_db_digest) - artifact_refs.append(v4x_artifact_ref) - - # Build annotations - only include operators if not empty - annotations = { - 'request_id': str(request_id), - 'request_type': request_type, - } - if operators: - annotations['operators'] = ','.join(operators) - - for artifact_ref in artifact_refs: - push_oras_artifact( - artifact_ref=artifact_ref, - local_path=index_db_filename, - cwd=index_db_dir, - annotations=annotations.copy(), - ) - log.info('Pushed %s to registry', artifact_ref) + artifact_refs.append(current_artifact_ref) - return original_index_db_digest + annotations = { + 'request_id': str(request_id), + 'request_type': request_type, + 'from_index': from_index, + 'output_image': output_image, + } + if operators: + annotations['operators'] = ','.join(operators) + + for artifact_ref in artifact_refs: + push_oras_artifact( + artifact_ref=artifact_ref, + local_path=index_db_filename, + cwd=index_db_dir, + annotations=annotations.copy(), + ) + log.info('Pushed %s to registry', artifact_ref) def cleanup_on_failure( @@ -424,16 +539,15 @@ def cleanup_on_failure( request_id: int, from_index: str, index_repo_map: Dict[str, str], - original_index_db_digest: Optional[str] = None, reason: str = "error", ) -> None: """ - Clean up Git changes and index.db artifacts on failure. + Clean up Git changes on failure. If a merge request was created, it will be closed (since the commit is only in a feature branch). If changes were pushed directly to the main branch, the commit - will be reverted. If the index.db artifact was pushed to the v4.x tag, it will be - restored to the original digest. + will be reverted. Content-addressed index.db artifacts are immutable, so there is + nothing to roll back on the artifact side. :param Optional[Dict[str, str]] mr_details: Details of the merge request if one was created :param Optional[str] last_commit_sha: The SHA of the last commit @@ -442,7 +556,6 @@ def cleanup_on_failure( :param int request_id: The IIB request ID :param str from_index: The from_index pullspec :param Dict[str, str] index_repo_map: Mapping of index images to Git repositories - :param Optional[str] original_index_db_digest: Original digest of index.db before overwrite :param str reason: Reason for the cleanup (used in log messages) """ if mr_details and index_git_repo: @@ -470,30 +583,120 @@ def cleanup_on_failure( else: log.error("Neither MR nor commit to revert. No cleanup needed for %s", reason) - # Restore index.db artifact to original digest if it was overwritten - if original_index_db_digest: - log.info("Restoring index.db artifact to original digest due to %s", reason) - try: - # Get the v4.x artifact reference - v4x_artifact_ref = get_indexdb_artifact_pullspec(from_index) - - # Extract registry and repository from the pullspec - # Format: quay.io/namespace/repo:tag -> we need quay.io/namespace/repo - artifact_name = v4x_artifact_ref.rsplit(':', 1)[0] - - # Use oras copy to restore the old digest to v4.x tag - # This is a registry-to-registry copy, no download needed - source_ref = f'{artifact_name}@{original_index_db_digest}' - log.info("Restoring %s from %s", v4x_artifact_ref, source_ref) - - run_cmd( - ['oras', 'copy', source_ref, v4x_artifact_ref], - exc_msg=f'Failed to restore index.db artifact ' - f'from {source_ref} to {v4x_artifact_ref}', - ) - log.info("Successfully restored index.db artifact to original digest") - except Exception as restore_error: - log.error("Failed to restore index.db artifact: %s", restore_error) + +@dataclass +class BuildSources: + """Resolved inputs for a containerized build.""" + + index_git_repo: str + local_git_repo_path: str + localized_git_catalog_path: str + index_db_path: Optional[str] # None => pull from ORAS; set => use extracted db + target_branch: str + is_divergent: bool + + +def prepare_build_sources( + request_id: int, + from_index: str, + from_index_resolved: str, + temp_dir: str, + ocp_version: str, + index_to_gitlab_push_map: Dict[str, str], + overwrite_from_index: bool, +) -> BuildSources: + """ + Resolve git repo + branch and decide the normal vs divergent build path. + + Normal path: a branch named after the image tag exists -> build against it + (overwrite allowed). Divergent path: no branch for the tag -> reject + overwrite, reuse the base OCP branch's Konflux Component, and seed content + by extracting configs+index.db from the image. + + Branch selection keys on the mutable tag (``from_index``), but divergent + content is extracted from the digest-resolved pullspec + (``from_index_resolved``) so it matches the image the request already + inspected during prebuild and cannot drift if the tag moves mid-request. + + :param int request_id: The IIB request ID + :param str from_index: The from_index pullspec (tag form, used for branch selection) + :param str from_index_resolved: The digest-resolved from_index pullspec (content source) + :param str temp_dir: Temporary directory to clone into / extract to + :param str ocp_version: Base OCP version branch, e.g. "v4.19" + :param Dict[str, str] index_to_gitlab_push_map: Mapping of index images to Git repositories + :param bool overwrite_from_index: Whether the request wants to overwrite from_index + :return: The resolved build sources + :rtype: BuildSources + :raises IIBError: if the git mapping is missing, overwrite is requested on + the divergent path, or the base OCP branch is not onboarded. + """ + index_git_repo = resolve_git_url(from_index=from_index, index_repo_map=index_to_gitlab_push_map) + if not index_git_repo: + raise IIBError( + f"Git repository mapping not found for from_index: {from_index}. " + "index_to_gitlab_push_map is required." + ) + token_name, git_token = get_git_token(index_git_repo) + + tag = get_index_tag(from_index) + set_request_state(request_id, 'in_progress', 'Cloning Git repository') + + if remote_branch_exists(index_git_repo, tag, token_name, git_token): + # Normal path — build against the tag branch, pull index.db from ORAS. + target_branch = tag + local_git_repo_path = Path(temp_dir) / 'git' / target_branch + local_git_repo_path.mkdir(parents=True, exist_ok=True) + clone_git_repo( + index_git_repo, target_branch, token_name, git_token, str(local_git_repo_path) + ) + catalog_path = local_git_repo_path / 'configs' + if not catalog_path.exists(): + raise IIBError(f"Catalogs directory not found in {local_git_repo_path}") + return BuildSources( + index_git_repo=index_git_repo, + local_git_repo_path=str(local_git_repo_path), + localized_git_catalog_path=str(catalog_path), + index_db_path=None, + target_branch=target_branch, + is_divergent=False, + ) + + # Divergent path. + if overwrite_from_index: + raise IIBError( + f"Cannot overwrite tag '{tag}' of {from_index}: no onboarded branch/Component " + f"exists for it. Onboard a '{tag}' branch to enable overwrite." + ) + + target_branch = ocp_version + if not remote_branch_exists(index_git_repo, target_branch, token_name, git_token): + raise IIBError( + f"Base OCP branch '{target_branch}' is not onboarded for {index_git_repo}. " + "Onboard the index before building divergent tags." + ) + + local_git_repo_path = Path(temp_dir) / 'git' / target_branch + local_git_repo_path.mkdir(parents=True, exist_ok=True) + clone_git_repo(index_git_repo, target_branch, token_name, git_token, str(local_git_repo_path)) + + # Content from the image (source of truth), scaffolding from the OCP branch. + # Extract from the digest-resolved pullspec, not the mutable tag. + extracted_configs, extracted_db = extract_catalog_and_db_from_image( + from_index_resolved, temp_dir + ) + catalog_path = local_git_repo_path / 'configs' + if catalog_path.exists(): + shutil.rmtree(catalog_path) + shutil.copytree(extracted_configs, catalog_path) + + return BuildSources( + index_git_repo=index_git_repo, + local_git_repo_path=str(local_git_repo_path), + localized_git_catalog_path=str(catalog_path), + index_db_path=extracted_db, + target_branch=target_branch, + is_divergent=True, + ) def prepare_git_repository_for_build( diff --git a/iib/workers/tasks/git_utils.py b/iib/workers/tasks/git_utils.py index cd98ac50b..fa71b6112 100644 --- a/iib/workers/tasks/git_utils.py +++ b/iib/workers/tasks/git_utils.py @@ -165,6 +165,46 @@ def validate_git_remote_branch(repo_url: str, branch: str) -> None: raise IIBError(f"Remote branch '{branch}' not found for repo {repo_url}") +def remote_branch_exists( + repo_url: str, + branch: str, + token_name: Optional[str] = None, + token: Optional[str] = None, +) -> bool: + """ + Return True if the given branch exists on the remote. + + This distinguishes a reachable remote with no matching branch (returns + False) from a failed command such as a network or auth error (raises), so a + transient failure is never silently interpreted as a missing branch. A + silent false negative would misroute a request onto the divergent build path. + + When ``token_name`` and ``token`` are provided, authentication is injected + into the remote URL the same way :func:`clone_git_repo` does, so the check + works against private repositories. + + :param str repo_url: The git repository URL. + :param str branch: The branch name to check. + :param str token_name: Optional name of the Git repository token. + :param str token: Optional value of the Git repository token. + :rtype: bool + :raises IIBError: If the ``git ls-remote`` command fails (e.g. network or auth error). + """ + ls_remote_url = repo_url + if token_name and token: + base_url = repo_url.replace('https://', '') + ls_remote_url = f"https://{token_name}:{token}@{base_url}" + + # strict=True (default) so a command failure raises IIBError instead of + # returning empty output that would be misread as "branch absent". The + # exc_msg deliberately references repo_url, not the token-injected URL. + remote_branch_status = run_cmd( + ["git", "ls-remote", "--heads", ls_remote_url, branch], + exc_msg=f"Error checking for remote branch '{branch}' in repo {repo_url}", + ) + return bool(remote_branch_status.strip()) + + def commit_and_push( request_id: int, local_repo_path: str, diff --git a/iib/workers/tasks/oras_utils.py b/iib/workers/tasks/oras_utils.py index 358aefc4b..d3d4c11d9 100644 --- a/iib/workers/tasks/oras_utils.py +++ b/iib/workers/tasks/oras_utils.py @@ -58,21 +58,43 @@ def _get_name_and_tag_from_pullspec(image_pullspec: str) -> Tuple[str, str]: return index_name, tag -def _get_artifact_combined_tag(image_name: str, tag: str) -> str: +def _get_index_digest(pullspec: str) -> str: """ - Generate a combined artifact tag for the given image name and tag. + Resolve a pullspec to its bare hex manifest digest (sha256), for artifact tags. - This function generates a unique combined tag for an image by using a template - string defined in the worker configuration and replacing placeholders with the - provided image name and tag. + :param str pullspec: The full index image pullspec. + :return: The 64-char hex digest without the ``sha256:`` prefix. + :rtype: str + """ + return get_image_digest(pullspec).split(':', 1)[-1] - :param str image_name: The name of the image. - :param str tag: The version or identifier tag to be combined. - :return: A formatted string representing the combined artifact tag. + +def get_index_tag(from_index: str) -> str: + """ + Return only the tag portion of a full index image pullspec. + + :param str from_index: The full index image pullspec (registry/namespace/repo:tag). + :return: The tag portion of the pullspec. + :rtype: str + """ + _, tag = _get_name_and_tag_from_pullspec(from_index) + return tag + + +def _get_artifact_combined_tag(from_index: str) -> str: + """ + Generate the content-addressed artifact/ImageStream tag for an index image. + + Keyed on the image's manifest digest ALONE (not name/tag/pullspec) so that + identical content addressed by different pullspecs (e.g. a released mirror) + shares one artifact, while different content never collides. + + :param str from_index: The full index image pullspec (registry/namespace/repo:tag). + :return: Combined tag, e.g. "idb-<64-hex-sha256>". :rtype: str """ return get_worker_config()['iib_index_db_artifact_tag_template'].format( - image_name=image_name, tag=tag + digest=_get_index_digest(from_index) ) @@ -87,11 +109,10 @@ def get_indexdb_artifact_pullspec(from_index: str) -> str: :rtype: str """ conf = get_worker_config() - image_name, tag = _get_name_and_tag_from_pullspec(from_index) return conf['iib_index_db_artifact_template'].format( registry=conf['iib_index_db_artifact_registry'], - tag=_get_artifact_combined_tag(image_name, tag), + tag=_get_artifact_combined_tag(from_index), ) @@ -285,16 +306,15 @@ def verify_indexdb_cache_for_image(index_image_pullspec: str) -> bool: """ Verify the synchronization state of the index database cache for a given container image. - This function extracts the image name and tag from the specified image - pullspec, generates an artifact combined tag, and verifies whether the - database cache for the image is synchronized. + This function generates the digest-based artifact combined tag for the + specified image pullspec and verifies whether the database cache for the + image is synchronized. :param str index_image_pullspec: The pull specification string of the container image. :return: The result of the cache synchronization verification process. :rtype: str """ - index_name, tag = _get_name_and_tag_from_pullspec(index_image_pullspec) - return verify_indexdb_cache_sync(_get_artifact_combined_tag(index_name, tag)) + return verify_indexdb_cache_sync(_get_artifact_combined_tag(index_image_pullspec)) def refresh_indexdb_cache( @@ -335,15 +355,12 @@ def refresh_indexdb_cache_for_image(index_image_pullspec: str) -> None: """ Refresh the cached data for an index database, associating it with the given image pullspec. - This function extracts the name and tag from the specified image pullspec, - and refreshes the associated index database cache. + This function generates the digest-based artifact combined tag for the + specified image pullspec, and refreshes the associated index database cache. :param str index_image_pullspec: The pull specification of the index image to cache. - :return: A formatted string combining the index name and tag. - :rtype: str """ - index_name, tag = _get_name_and_tag_from_pullspec(index_image_pullspec) - refresh_indexdb_cache(_get_artifact_combined_tag(index_name, tag)) + refresh_indexdb_cache(_get_artifact_combined_tag(index_image_pullspec)) def get_imagestream_artifact_pullspec(from_index: str) -> str: @@ -358,8 +375,7 @@ def get_imagestream_artifact_pullspec(from_index: str) -> str: :rtype: str """ conf = get_worker_config() - image_name, tag = _get_name_and_tag_from_pullspec(from_index) - combined_tag = _get_artifact_combined_tag(image_name, tag) + combined_tag = _get_artifact_combined_tag(from_index) # ImageStream pullspec format: # image-registry.openshift-image-registry.svc:5000/{namespace}/index-db:{combined_tag} diff --git a/tests/test_workers/test_tasks/test_build_containerized_add.py b/tests/test_workers/test_tasks/test_build_containerized_add.py index 9ba81a39f..585441ada 100644 --- a/tests/test_workers/test_tasks/test_build_containerized_add.py +++ b/tests/test_workers/test_tasks/test_build_containerized_add.py @@ -5,7 +5,7 @@ import pytest from iib.exceptions import IIBError -from iib.workers.tasks import build_containerized_add +from iib.workers.tasks import build_containerized_add, containerized_utils @pytest.mark.parametrize('check_related_images', (True, False)) @@ -51,7 +51,7 @@ @mock.patch('iib.workers.tasks.build_containerized_add._get_missing_bundles') @mock.patch('iib.workers.tasks.build_containerized_add._get_present_bundles') @mock.patch('iib.workers.tasks.build_containerized_add.fetch_and_verify_index_db_artifact') -@mock.patch('iib.workers.tasks.build_containerized_add.prepare_git_repository_for_build') +@mock.patch('iib.workers.tasks.build_containerized_add.prepare_build_sources') @mock.patch('iib.workers.tasks.build_containerized_add.tempfile.TemporaryDirectory') @mock.patch('iib.workers.tasks.build_containerized_add._update_index_image_build_state') @mock.patch('iib.workers.tasks.build_containerized_add.Opm') @@ -71,7 +71,7 @@ def test_handle_containerized_add_request( mock_opm, mock_update_build_state, mock_td, - mock_prepare_git, + mock_prepare_sources, mock_fetch_index_db, mock_get_present, mock_get_missing, @@ -128,10 +128,13 @@ def test_handle_containerized_add_request( local_git_repo_path = Path(tmpdir) / 'git_repo' localized_git_catalog_path = Path(local_git_repo_path) / "configs" local_git_repo_path.mkdir(parents=True) - mock_prepare_git.return_value = ( - index_git_repo, - local_git_repo_path, - localized_git_catalog_path, + mock_prepare_sources.return_value = containerized_utils.BuildSources( + index_git_repo=index_git_repo, + local_git_repo_path=local_git_repo_path, + localized_git_catalog_path=localized_git_catalog_path, + index_db_path=None, + target_branch='v4.12', + is_divergent=False, ) mock_fetch_index_db.return_value = index_db_path @@ -168,7 +171,7 @@ def test_handle_containerized_add_request( mock_replicate.return_value = output_pull_specs # Mock final artifact push - mock_push_index_db.return_value = 'sha256:index_db_digest' + mock_push_index_db.return_value = None # Call the function if with_deprecations: @@ -207,12 +210,14 @@ def test_handle_containerized_add_request( mock_prepare_req.assert_called_once() # Verify git preparation - mock_prepare_git.assert_called_once_with( + mock_prepare_sources.assert_called_once_with( request_id=request_id, from_index=str(from_index), + from_index_resolved='from-index@sha256:abcdef', temp_dir=temp_dir_path, - branch='v4.12', + ocp_version='v4.12', index_to_gitlab_push_map={}, + overwrite_from_index=False, ) # Verify bundle checks @@ -279,6 +284,7 @@ def test_handle_containerized_add_request( ) mock_push_index_db.assert_called_once() + assert mock_push_index_db.call_args.kwargs['output_image'] == image_url mock_cleanup_mr.assert_called_once() mock_cleanup_failure.assert_not_called() @@ -307,7 +313,7 @@ def test_handle_containerized_add_request( @mock.patch('iib.workers.tasks.build_containerized_add._get_missing_bundles') @mock.patch('iib.workers.tasks.build_containerized_add._get_present_bundles') @mock.patch('iib.workers.tasks.build_containerized_add.fetch_and_verify_index_db_artifact') -@mock.patch('iib.workers.tasks.build_containerized_add.prepare_git_repository_for_build') +@mock.patch('iib.workers.tasks.build_containerized_add.prepare_build_sources') @mock.patch('iib.workers.tasks.build_containerized_add.tempfile.TemporaryDirectory') @mock.patch('iib.workers.tasks.build_containerized_add._update_index_image_build_state') @mock.patch('iib.workers.tasks.build_containerized_add.Opm') @@ -327,7 +333,7 @@ def test_handle_containerized_add_request_failure( mock_opm, mock_update_build_state, mock_td, - mock_prepare_git, + mock_prepare_sources, mock_fetch_index_db, mock_get_present, mock_get_missing, @@ -371,7 +377,14 @@ def test_handle_containerized_add_request_failure( mock_prepare_req.return_value = prebuild_info # Mock git repo preparation - mock_prepare_git.return_value = (mock.Mock(), '/tmp/repo', '/tmp/repo/catalog') + mock_prepare_sources.return_value = containerized_utils.BuildSources( + index_git_repo=mock.Mock(), + local_git_repo_path='/tmp/repo', + localized_git_catalog_path='/tmp/repo/catalog', + index_db_path=None, + target_branch='v4.12', + is_divergent=False, + ) # Mock TD mock_td.return_value.__enter__.return_value = '/tmp/iib-test' @@ -428,7 +441,7 @@ def test_handle_containerized_add_request_failure( @mock.patch('iib.workers.tasks.build_containerized_add._get_missing_bundles') @mock.patch('iib.workers.tasks.build_containerized_add._get_present_bundles') @mock.patch('iib.workers.tasks.build_containerized_add.fetch_and_verify_index_db_artifact') -@mock.patch('iib.workers.tasks.build_containerized_add.prepare_git_repository_for_build') +@mock.patch('iib.workers.tasks.build_containerized_add.prepare_build_sources') @mock.patch('iib.workers.tasks.build_containerized_add.tempfile.TemporaryDirectory') @mock.patch('iib.workers.tasks.build_containerized_add._update_index_image_build_state') @mock.patch('iib.workers.tasks.build_containerized_add.Opm') @@ -450,7 +463,7 @@ def test_handle_containerized_add_request_overwrite( mock_opm, mock_update_build_state, mock_td, - mock_prepare_git, + mock_prepare_sources, mock_fetch_index_db, mock_get_present, mock_get_missing, @@ -502,10 +515,13 @@ def test_handle_containerized_add_request_overwrite( local_git_repo_path = Path(tmpdir) / 'git_repo' localized_git_catalog_path = Path(local_git_repo_path) / "configs" local_git_repo_path.mkdir(parents=True) - mock_prepare_git.return_value = ( - index_git_repo, - local_git_repo_path, - localized_git_catalog_path, + mock_prepare_sources.return_value = containerized_utils.BuildSources( + index_git_repo=index_git_repo, + local_git_repo_path=local_git_repo_path, + localized_git_catalog_path=localized_git_catalog_path, + index_db_path=None, + target_branch='v4.12', + is_divergent=False, ) mock_fetch_index_db.return_value = index_db_path @@ -527,7 +543,7 @@ def test_handle_containerized_add_request_overwrite( output_pull_specs = ['registry.example.com/final-image:456'] mock_replicate.return_value = output_pull_specs - mock_push_index_db.return_value = 'sha256:index_db_digest' + mock_push_index_db.return_value = None build_containerized_add.handle_containerized_add_request( bundles=bundles, @@ -546,4 +562,161 @@ def test_handle_containerized_add_request_overwrite( # Verify the handler completed successfully mock_push_index_db.assert_called_once() + assert mock_push_index_db.call_args.kwargs['output_image'] == image_url + mock_cleanup_failure.assert_not_called() + + +@mock.patch('iib.workers.tasks.build_containerized_add.shutil.copytree') +@mock.patch('iib.workers.tasks.build_containerized_add.Path.mkdir') +@mock.patch('iib.workers.tasks.build_containerized_add.cleanup_on_failure') +@mock.patch('iib.workers.tasks.build_containerized_add.set_request_state') +@mock.patch('iib.workers.tasks.build_containerized_add.cleanup_merge_request_if_exists') +@mock.patch('iib.workers.tasks.build_containerized_add.push_index_db_artifact') +@mock.patch('iib.workers.tasks.build_containerized_add._update_index_image_pull_spec') +@mock.patch('iib.workers.tasks.build_containerized_add.replicate_image_to_tagged_destinations') +@mock.patch('iib.workers.tasks.build_containerized_add.monitor_pipeline_and_extract_image') +@mock.patch('iib.workers.tasks.build_containerized_add.git_commit_and_create_mr') +@mock.patch('iib.workers.tasks.build_containerized_add.write_build_metadata') +@mock.patch('iib.workers.tasks.build_containerized_add.chmod_recursively') +@mock.patch('iib.workers.tasks.build_containerized_add.merge_catalogs_dirs') +@mock.patch( + 'iib.workers.tasks.build_containerized_add.remove_deprecated_operators_from_git_catalog' +) +@mock.patch('iib.workers.tasks.build_containerized_add.Path.is_dir') +@mock.patch('iib.workers.tasks.build_containerized_add.opm_migrate') +@mock.patch('iib.workers.tasks.build_containerized_add.deprecate_bundles_db') +@mock.patch('iib.workers.tasks.build_containerized_add.get_bundles_from_deprecation_list') +@mock.patch('iib.workers.tasks.build_containerized_add._opm_registry_add') +@mock.patch('iib.workers.tasks.build_containerized_add._get_missing_bundles') +@mock.patch('iib.workers.tasks.build_containerized_add._get_present_bundles') +@mock.patch('iib.workers.tasks.build_containerized_add.fetch_and_verify_index_db_artifact') +@mock.patch('iib.workers.tasks.build_containerized_add.prepare_build_sources') +@mock.patch('iib.workers.tasks.build_containerized_add.tempfile.TemporaryDirectory') +@mock.patch('iib.workers.tasks.build_containerized_add._update_index_image_build_state') +@mock.patch('iib.workers.tasks.build_containerized_add.Opm') +@mock.patch('iib.workers.tasks.build_containerized_add.prepare_request_for_build') +@mock.patch('iib.workers.tasks.build_containerized_add.inspect_related_images') +@mock.patch('iib.workers.tasks.build_containerized_add.verify_labels') +@mock.patch('iib.workers.tasks.build_containerized_add.get_resolved_bundles') +@mock.patch('iib.workers.tasks.build_containerized_add.set_registry_token') +@mock.patch('iib.workers.tasks.build_containerized_add.reset_docker_config') +@mock.patch('iib.workers.tasks.build_containerized_add.merge_mr_after_build') +def test_add_divergent_never_merges( + mock_merge_mr, + mock_reset_docker, + mock_set_token, + mock_get_resolved, + mock_verify_labels, + mock_inspect, + mock_prepare_req, + mock_opm, + mock_update_build_state, + mock_td, + mock_prepare_sources, + mock_fetch_index_db, + mock_get_present, + mock_get_missing, + mock_opm_add, + mock_get_deprecations, + mock_deprecate, + mock_opm_migrate, + mock_path_isdir, + mock_remove_deprecated, + mock_merge, + mock_chmod, + mock_write_meta, + mock_git_commit, + mock_monitor, + mock_replicate, + mock_update_pull_spec, + mock_push_index_db, + mock_cleanup_mr, + mock_set_state, + mock_cleanup_failure, + mock_makedirs, + mock_copytree, + tmpdir, +): + """Divergent path must never fall back to ORAS and must never merge the MR.""" + bundles = ['some-bundle:latest'] + request_id = 789 + binary_image = 'binary-image:latest' + resolved_bundles = ['some-bundle@sha256:123456'] + index_db_path = '/tmp/divergent/index.db' + temp_dir_path = '/tmp/iib-789-temp' + from_index = 'index:latest' + + mock_get_resolved.return_value = resolved_bundles + mock_td.return_value.__enter__.return_value = temp_dir_path + + prebuild_info = { + 'from_index_resolved': 'from-index@sha256:abcdef', + 'binary_image_resolved': 'binary-image@sha256:fedcba', + 'arches': {'amd64'}, + 'bundle_mapping': {'some-operator': resolved_bundles}, + 'ocp_version': 'v4.12', + 'distribution_scope': 'prod', + 'binary_image': binary_image, + } + mock_prepare_req.return_value = prebuild_info + + index_git_repo = mock.Mock() + local_git_repo_path = Path(tmpdir) / 'git_repo' + localized_git_catalog_path = Path(local_git_repo_path) / "configs" + local_git_repo_path.mkdir(parents=True) + mock_prepare_sources.return_value = containerized_utils.BuildSources( + index_git_repo=index_git_repo, + local_git_repo_path=local_git_repo_path, + localized_git_catalog_path=localized_git_catalog_path, + index_db_path=index_db_path, + target_branch='v4.14', + is_divergent=True, + ) + + mock_path_isdir.return_value = True + mock_get_present.return_value = ([], []) + mock_get_missing.return_value = resolved_bundles + mock_get_deprecations.return_value = [] + + catalog_from_db = '/tmp/from_db' + mock_opm_migrate.return_value = (catalog_from_db, None) + + mr_details = {'mr_id': 1, 'mr_url': 'https://gitlab.com/mr/1', 'source_branch': 'iib-789-v4.14'} + mock_git_commit.return_value = (mr_details, 'commit_sha_789') + + image_url = 'registry.example.com/output-image:tag' + mock_monitor.return_value = image_url + + output_pull_specs = ['registry.example.com/final-image:789'] + mock_replicate.return_value = output_pull_specs + + mock_push_index_db.return_value = None + + build_containerized_add.handle_containerized_add_request( + bundles=bundles, + request_id=request_id, + binary_image=binary_image, + from_index=from_index, + overwrite_from_index=True, + overwrite_from_index_token="user:pass", + ) + + # Divergent path uses the extracted index.db, never ORAS. + mock_fetch_index_db.assert_not_called() + assert mock_push_index_db.call_args.kwargs['output_image'] == image_url + + # overwrite_from_index=True here: the divergent BuildSources bypasses Task 4's + # entry-point overwrite rejection (mocked directly), so the ONLY thing that can + # prevent a merge is the handler-level `and not sources.is_divergent` guard. If + # that guard were removed, this MR would be merged and this assertion would fail. + mock_merge_mr.assert_not_called() + mock_cleanup_mr.assert_called_once() + + mock_opm_add.assert_called_once_with( + base_dir=temp_dir_path, + index_db=index_db_path, + bundles=resolved_bundles, + overwrite_csv=False, + graph_update_mode=None, + ) mock_cleanup_failure.assert_not_called() diff --git a/tests/test_workers/test_tasks/test_build_containerized_create_empty_index.py b/tests/test_workers/test_tasks/test_build_containerized_create_empty_index.py index 7f5424871..860282e25 100644 --- a/tests/test_workers/test_tasks/test_build_containerized_create_empty_index.py +++ b/tests/test_workers/test_tasks/test_build_containerized_create_empty_index.py @@ -18,10 +18,8 @@ @mock.patch('iib.workers.tasks.containerized_utils.set_request_state') @mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') @mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') -@mock.patch('iib.workers.tasks.containerized_utils.get_image_digest') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') @mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@mock.patch('iib.workers.tasks.containerized_utils._get_artifact_combined_tag') -@mock.patch('iib.workers.tasks.containerized_utils._get_name_and_tag_from_pullspec') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @mock.patch('iib.workers.tasks.containerized_utils.wait_for_pipeline_completion') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -69,8 +67,6 @@ def test_handle_containerized_create_empty_index_primary_path( mock_fpr, mock_wfpc, mock_gpiu, - mock_gntfp, - mock_gact, mock_giap, mock_gid, mock_poa, @@ -136,10 +132,8 @@ def test_handle_containerized_create_empty_index_primary_path( mock_gpiu.return_value = 'quay.io/konflux/image@sha256:built' # Mock ORAS push related functions - mock_gntfp.return_value = ('index-image', 'v4.14') - mock_gact.return_value = 'index-image-v4.14' mock_giap.return_value = 'registry.io/index-db:v4.14' - mock_gid.return_value = 'sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abc' + mock_gid.return_value = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abc0' # Mock worker config for utils mock_gwc_utils.return_value = { @@ -236,10 +230,8 @@ def test_handle_containerized_create_empty_index_primary_path( @mock.patch('iib.workers.tasks.containerized_utils.set_request_state') @mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') @mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') -@mock.patch('iib.workers.tasks.containerized_utils.get_image_digest') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') @mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@mock.patch('iib.workers.tasks.containerized_utils._get_artifact_combined_tag') -@mock.patch('iib.workers.tasks.containerized_utils._get_name_and_tag_from_pullspec') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @mock.patch('iib.workers.tasks.containerized_utils.wait_for_pipeline_completion') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -295,8 +287,6 @@ def test_handle_containerized_create_empty_index_fallback( mock_fpr, mock_wfpc, mock_gpiu, - mock_gntfp, - mock_gact, mock_giap, mock_gid, mock_poa, @@ -382,10 +372,8 @@ def test_handle_containerized_create_empty_index_fallback( mock_gpiu.return_value = 'quay.io/konflux/image@sha256:fallback' # Mock ORAS push related functions - mock_gntfp.return_value = ('index-image', 'v4.14') - mock_gact.return_value = 'index-image-v4.14' mock_giap.return_value = 'registry.io/index-db:v4.14' - mock_gid.return_value = 'sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abc' + mock_gid.return_value = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abc0' # Mock worker config for utils mock_gwc_utils.return_value = { @@ -599,10 +587,8 @@ def test_handle_containerized_create_empty_index_missing_git_mapping( @mock.patch('iib.workers.tasks.containerized_utils.set_request_state') @mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') @mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') -@mock.patch('iib.workers.tasks.containerized_utils.get_image_digest') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') @mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@mock.patch('iib.workers.tasks.containerized_utils._get_artifact_combined_tag') -@mock.patch('iib.workers.tasks.containerized_utils._get_name_and_tag_from_pullspec') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @mock.patch('iib.workers.tasks.containerized_utils.wait_for_pipeline_completion') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -658,8 +644,6 @@ def test_handle_containerized_create_empty_index_unexpected_opm_error( mock_fpr, mock_wfpc, mock_gpiu, - mock_gntfp, - mock_gact, mock_giap, mock_gid, mock_poa, @@ -720,10 +704,8 @@ def test_handle_containerized_create_empty_index_unexpected_opm_error( mock_gpiu.return_value = 'quay.io/konflux/image@sha256:built' # Mock ORAS push related functions - mock_gntfp.return_value = ('index-image', 'v4.14') - mock_gact.return_value = 'index-image-v4.14' mock_giap.return_value = 'registry.io/index-db:v4.14' - mock_gid.return_value = 'sha256:abc' + mock_gid.return_value = 'abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234567' mock_ritd.return_value = ['registry.io/iib-build:5'] mock_gwc_utils.return_value = { diff --git a/tests/test_workers/test_tasks/test_build_containerized_fbc_operations.py b/tests/test_workers/test_tasks/test_build_containerized_fbc_operations.py index dccb7feb9..8e55a50f6 100644 --- a/tests/test_workers/test_tasks/test_build_containerized_fbc_operations.py +++ b/tests/test_workers/test_tasks/test_build_containerized_fbc_operations.py @@ -3,10 +3,11 @@ import pytest from iib.exceptions import IIBError -from iib.workers.tasks import build_containerized_fbc_operations +from iib.workers.tasks import build_containerized_fbc_operations, containerized_utils from iib.workers.tasks.utils import RequestConfigFBCOperation +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations._update_index_image_pull_spec') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations.push_index_db_artifact') @@ -60,10 +61,11 @@ def test_handle_containerized_fbc_operation_request( mock_pida_push, mock_cof, mock_uiips, + mock_rbe, ): """Test containerized FBC operation with single fragment.""" request_id = 10 - from_index = 'from-index:latest' + from_index = 'quay.io/iib/from-index:latest' binary_image = 'binary-image:latest' binary_image_config = {'prod': {'v4.5': 'some_image'}} fbc_fragments = ['fbc-fragment:latest'] @@ -176,6 +178,7 @@ def test_handle_containerized_fbc_operation_request( assert mock_srs.call_args[0][1] == 'complete' +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations._update_index_image_pull_spec') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations.push_index_db_artifact') @@ -229,10 +232,11 @@ def test_handle_containerized_fbc_operation_request_multiple_fragments( mock_pida_push, mock_cof, mock_uiips, + mock_rbe, ): """Test containerized FBC operation with multiple fragments.""" request_id = 10 - from_index = 'from-index:latest' + from_index = 'quay.io/iib/from-index:latest' binary_image = 'binary-image:latest' binary_image_config = {'prod': {'v4.5': 'some_image'}} fbc_fragments = ['fbc-fragment1:latest', 'fbc-fragment2:latest'] @@ -291,6 +295,7 @@ def test_handle_containerized_fbc_operation_request_multiple_fragments( ] +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations._update_index_image_pull_spec') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations.push_index_db_artifact') @@ -346,6 +351,7 @@ def test_handle_containerized_fbc_operation_request_with_overwrite( mock_pida_push, mock_cof, mock_uiips, + mock_rbe, ): """Test containerized FBC operation with overwrite_from_index=True.""" request_id = 10 @@ -399,6 +405,7 @@ def test_handle_containerized_fbc_operation_request_with_overwrite( from_index='quay.io/iib/from-index:latest', index_db_path='/tmp/d', operators=['op1'], + output_image='reg/img', overwrite_from_index=True, request_type='fbc_operations', ) @@ -418,6 +425,7 @@ def test_handle_containerized_fbc_operation_request_with_overwrite( ) +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations._update_index_image_pull_spec') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_fbc_operations.push_index_db_artifact') @@ -469,6 +477,7 @@ def test_handle_containerized_fbc_operation_request_failure( mock_pida_push, mock_cof, mock_uiips, + mock_rbe, ): """Test containerized FBC operation failure handling.""" request_id = 10 @@ -514,3 +523,122 @@ def test_handle_containerized_fbc_operation_request_failure( args, kwargs = mock_cof.call_args assert kwargs['request_id'] == request_id assert "error: Download failed" in kwargs['reason'] + + +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations._update_index_image_pull_spec') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.cleanup_on_failure') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.cleanup_merge_request_if_exists') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.merge_mr_after_build') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.push_index_db_artifact') +@mock.patch( + 'iib.workers.tasks.build_containerized_fbc_operations.replicate_image_to_tagged_destinations' +) +@mock.patch( + 'iib.workers.tasks.build_containerized_fbc_operations.monitor_pipeline_and_extract_image' +) +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.git_commit_and_create_mr') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.write_build_metadata') +@mock.patch( + 'iib.workers.tasks.build_containerized_fbc_operations.opm_registry_add_fbc_fragment_containerized' +) +@mock.patch( + 'iib.workers.tasks.build_containerized_fbc_operations.fetch_and_verify_index_db_artifact' +) +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.prepare_build_sources') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations._update_index_image_build_state') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.Opm.set_opm_version') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.prepare_request_for_build') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.get_resolved_image') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.set_request_state') +@mock.patch('iib.workers.tasks.build_containerized_fbc_operations.reset_docker_config') +def test_fbc_operations_divergent_never_merges( + mock_rdc, + mock_srs, + mock_gri, + mock_prfb, + mock_sov, + mock_uiibs, + mock_prepare_sources, + mock_fetch_index_db, + mock_oraff, + mock_wbm, + mock_git_commit, + mock_monitor, + mock_replicate, + mock_push_index_db, + mock_merge_mr, + mock_cleanup_mr, + mock_cof, + mock_uiips, + tmp_path, +): + """Divergent path must never fall back to ORAS and must never merge the MR.""" + request_id = 999 + from_index = 'quay.io/iib/from-index:v4.99' + binary_image = 'binary-image:latest' + fbc_fragments = ['quay.io/iib/fbc-fragment1:latest'] + + mock_prfb.return_value = { + 'arches': {'amd64'}, + 'binary_image_resolved': 'binary@sha256:123', + 'from_index_resolved': 'index@sha256:456', + 'ocp_version': 'v4.14', + 'distribution_scope': 'prod', + } + mock_gri.return_value = 'fbc@sha256:789' + + index_db_path = str(tmp_path / 'index.db') + index_git_repo = 'https://gitlab.com/repo/x.git' + local_git_repo_path = str(tmp_path / 'git_repo') + localized_git_catalog_path = str(tmp_path / 'git_repo' / 'configs') + mock_prepare_sources.return_value = containerized_utils.BuildSources( + index_git_repo=index_git_repo, + local_git_repo_path=local_git_repo_path, + localized_git_catalog_path=localized_git_catalog_path, + index_db_path=index_db_path, + target_branch='v4.14', + is_divergent=True, + ) + + mock_oraff.return_value = ('/tmp/updated_catalog_path', index_db_path, ['op1']) + + mr_details = { + 'mr_id': '1', + 'mr_url': 'https://gitlab.com/mr/1', + 'source_branch': 'iib-request-999-v4.14', + } + mock_git_commit.return_value = (mr_details, 'commit_sha_999') + mock_monitor.return_value = 'registry/output-image:sha256-12345' + mock_replicate.return_value = ['registry.example.com/final-image:999'] + mock_push_index_db.return_value = None + + overwrite_token = 'user:token' + build_containerized_fbc_operations.handle_containerized_fbc_operation_request( + request_id=request_id, + fbc_fragments=fbc_fragments, + from_index=from_index, + binary_image=binary_image, + overwrite_from_index=True, + overwrite_from_index_token=overwrite_token, + index_to_gitlab_push_map={'quay.io/iib/from-index': index_git_repo}, + ) + + # Divergent path uses the extracted index.db, never ORAS. + mock_fetch_index_db.assert_not_called() + + # overwrite_from_index=True here: the divergent BuildSources bypasses Task 4's + # entry-point overwrite rejection (mocked directly), so the ONLY thing that can + # prevent a merge is the handler-level `and not sources.is_divergent` guard. If + # that guard were removed, this MR would be merged and this assertion would fail. + mock_merge_mr.assert_not_called() + mock_cleanup_mr.assert_called_once() + + mock_oraff.assert_called_once_with( + request_id=request_id, + temp_dir=mock.ANY, + from_index_configs_dir=localized_git_catalog_path, + fbc_fragments=['fbc@sha256:789'], + overwrite_from_index_token=overwrite_token, + index_db_path=index_db_path, + ) + mock_cof.assert_not_called() diff --git a/tests/test_workers/test_tasks/test_build_containerized_rm.py b/tests/test_workers/test_tasks/test_build_containerized_rm.py index 76eef493a..ea405686c 100644 --- a/tests/test_workers/test_tasks/test_build_containerized_rm.py +++ b/tests/test_workers/test_tasks/test_build_containerized_rm.py @@ -4,10 +4,11 @@ from unittest import mock from iib.exceptions import IIBError -from iib.workers.tasks import build_containerized_rm +from iib.workers.tasks import build_containerized_rm, containerized_utils from iib.workers.tasks.utils import RequestConfigAddRm +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_rm._update_index_image_pull_spec') @@ -15,10 +16,8 @@ @mock.patch('iib.workers.tasks.containerized_utils.set_request_state') @mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') @mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') -@mock.patch('iib.workers.tasks.containerized_utils.get_image_digest') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') @mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@mock.patch('iib.workers.tasks.containerized_utils._get_artifact_combined_tag') -@mock.patch('iib.workers.tasks.containerized_utils._get_name_and_tag_from_pullspec') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @mock.patch('iib.workers.tasks.containerized_utils.wait_for_pipeline_completion') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -72,8 +71,6 @@ def test_handle_containerized_rm_request_success_with_overwrite( mock_fpr, mock_wfpc, mock_gpiu, - mock_gntfp, - mock_gact, mock_giap, mock_gid, mock_poa, @@ -83,6 +80,7 @@ def test_handle_containerized_rm_request_success_with_overwrite( mock_uiips, mock_cof, mock_rdc, + mock_rbe, ): """Test successful operator removal with overwrite_from_index.""" # Setup @@ -140,10 +138,8 @@ def test_handle_containerized_rm_request_success_with_overwrite( mock_gpiu.return_value = 'quay.io/konflux/built-image@sha256:xyz789' # Mock ORAS push related functions - mock_gntfp.return_value = ('index-image', 'v4.14') - mock_gact.return_value = 'index-image-v4.14' mock_giap.return_value = 'registry.io/index-db:v4.14' - mock_gid.return_value = 'sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abc' + mock_gid.return_value = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abc0' # Mock worker config mock_gwc.return_value = { @@ -207,9 +203,12 @@ def test_handle_containerized_rm_request_success_with_overwrite( # Verify image was copied assert mock_sc.call_count >= 1 - # Verify index.db was pushed (2 times: request_id tag + v4.x tag) + # Verify index.db was pushed (2 times: request_id tag + current-artifact tag) assert mock_poa.call_count == 2 + # Verify the content key was resolved from the built output image + mock_gid.assert_called_once_with('quay.io/konflux/built-image@sha256:xyz789') + # Verify final state final_call = mock_srs.call_args_list[-1] assert final_call[0][0] == request_id @@ -220,6 +219,7 @@ def test_handle_containerized_rm_request_success_with_overwrite( assert mock_rdc.call_count >= 1 +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_rm._update_index_image_pull_spec') @@ -227,9 +227,8 @@ def test_handle_containerized_rm_request_success_with_overwrite( @mock.patch('iib.workers.tasks.containerized_utils.set_request_state') @mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') @mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') @mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@mock.patch('iib.workers.tasks.containerized_utils._get_artifact_combined_tag') -@mock.patch('iib.workers.tasks.containerized_utils._get_name_and_tag_from_pullspec') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @mock.patch('iib.workers.tasks.containerized_utils.wait_for_pipeline_completion') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -281,8 +280,7 @@ def test_handle_containerized_rm_request_with_mr( mock_fpr, mock_wfpc, mock_gpiu, - mock_gntfp, - mock_gact, + mock_gid, mock_giap, mock_poa, mock_gwc, @@ -291,6 +289,7 @@ def test_handle_containerized_rm_request_with_mr( mock_uiips, mock_cof, mock_rdc, + mock_rbe, ): """Test operator removal without overwrite creates and closes MR.""" # Setup @@ -341,9 +340,8 @@ def test_handle_containerized_rm_request_with_mr( mock_gpiu.return_value = 'quay.io/konflux/image@sha256:built' # Mock ORAS push related functions (only request_id tag, no overwrite) - mock_gntfp.return_value = ('index-image', 'v4.14') - mock_gact.return_value = 'index-image-v4.14' mock_giap.return_value = 'registry.io/index-db:v4.14' + mock_gid.return_value = 'abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234567' # Mock config mock_gwc.return_value = { @@ -383,6 +381,7 @@ def test_handle_containerized_rm_request_with_mr( ({'op1', 'op2'}, True), # Multiple operators in DB ], ) +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_rm._update_index_image_pull_spec') @@ -390,10 +389,8 @@ def test_handle_containerized_rm_request_with_mr( @mock.patch('iib.workers.tasks.containerized_utils.set_request_state') @mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') @mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') -@mock.patch('iib.workers.tasks.containerized_utils.get_image_digest') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') @mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@mock.patch('iib.workers.tasks.containerized_utils._get_artifact_combined_tag') -@mock.patch('iib.workers.tasks.containerized_utils._get_name_and_tag_from_pullspec') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @mock.patch('iib.workers.tasks.containerized_utils.wait_for_pipeline_completion') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -445,8 +442,6 @@ def test_handle_containerized_rm_conditional_opm_rm( mock_fpr, mock_wfpc, mock_gpiu, - mock_gntfp, - mock_gact, mock_giap, mock_gid, mock_poa, @@ -456,6 +451,7 @@ def test_handle_containerized_rm_conditional_opm_rm( mock_uiips, mock_cof, mock_rdc, + mock_rbe, operators_in_db, should_call_opm_rm, ): @@ -504,10 +500,8 @@ def test_handle_containerized_rm_conditional_opm_rm( mock_gpiu.return_value = 'image@sha' # Mock ORAS push related functions (conditionally used based on operators_in_db) - mock_gntfp.return_value = ('index', 'v4.14') - mock_gact.return_value = 'index-v4.14' mock_giap.return_value = 'reg/index-db:v4.14' - mock_gid.return_value = 'sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abc' + mock_gid.return_value = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789abc0' mock_gwc.return_value = { 'iib_registry': 'reg', @@ -589,6 +583,7 @@ def test_handle_containerized_rm_missing_git_mapping( ) +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.containerized_utils.clone_git_repo') @@ -614,6 +609,7 @@ def test_handle_containerized_rm_missing_configs_dir( mock_cgr, mock_cof, mock_rdc, + mock_rbe, ): """Test that missing configs directory raises error.""" request_id = 5 @@ -650,6 +646,7 @@ def test_handle_containerized_rm_missing_configs_dir( mock_cof.assert_not_called() +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.containerized_utils.pull_index_db_artifact') @@ -681,6 +678,7 @@ def test_handle_containerized_rm_missing_index_db( mock_pida, mock_cof, mock_rdc, + mock_rbe, ): """Test that missing index.db file raises error.""" request_id = 6 @@ -717,6 +715,7 @@ def test_handle_containerized_rm_missing_index_db( mock_cof.assert_not_called() +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @@ -772,6 +771,7 @@ def test_handle_containerized_rm_pipeline_failure( mock_gpiu, mock_cof, mock_rdc, + mock_rbe, ): """Test that pipeline failure triggers cleanup.""" request_id = 7 @@ -828,6 +828,7 @@ def test_handle_containerized_rm_pipeline_failure( assert 'Pipeline not found' in cleanup_call[1]['reason'] +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_rm._update_index_image_pull_spec') @@ -835,10 +836,8 @@ def test_handle_containerized_rm_pipeline_failure( @mock.patch('iib.workers.tasks.containerized_utils.set_request_state') @mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') @mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') -@mock.patch('iib.workers.tasks.containerized_utils.get_image_digest') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') @mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@mock.patch('iib.workers.tasks.containerized_utils._get_artifact_combined_tag') -@mock.patch('iib.workers.tasks.containerized_utils._get_name_and_tag_from_pullspec') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @mock.patch('iib.workers.tasks.containerized_utils.wait_for_pipeline_completion') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -890,8 +889,6 @@ def test_handle_containerized_rm_with_index_db_push( mock_fpr, mock_wfpc, mock_gpiu, - mock_gntfp, - mock_gact, mock_giap, mock_gid, mock_poa, @@ -901,6 +898,7 @@ def test_handle_containerized_rm_with_index_db_push( mock_uiips, mock_cof, mock_rdc, + mock_rbe, ): """Test that index.db is pushed when operators exist in DB and overwrite token is provided.""" request_id = 8 @@ -944,10 +942,8 @@ def test_handle_containerized_rm_with_index_db_push( mock_gpiu.return_value = 'image@sha' # Mock ORAS push related functions - mock_gntfp.return_value = ('index-image', 'v4.14') - mock_gact.return_value = 'index-image-v4.14' mock_giap.return_value = 'registry.io/index-db:v4.14' - mock_gid.return_value = 'sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789ab' + mock_gid.return_value = '0123456789abcdef0123456789abcdef0123456789abcdef0123456789ab00' mock_gwc.return_value = { 'iib_registry': 'registry.io', @@ -969,10 +965,10 @@ def test_handle_containerized_rm_with_index_db_push( # Verify MR was merged after build (overwrite flow) mock_merge_mr.assert_called_once() - # Verify index.db was pushed (2 times: request_id tag + v4.x tag) + # Verify index.db was pushed (2 times: request_id tag + current-artifact tag) assert mock_poa.call_count == 2 - # Verify original digest was captured + # Verify the content key was resolved from the built output image mock_gid.assert_called_once() # Verify annotations were added @@ -990,16 +986,15 @@ def test_handle_containerized_rm_with_index_db_push( (['latest', 'v4.14'], 3), # request_id + latest + v4.14 ], ) +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_rm._update_index_image_pull_spec') @mock.patch('iib.workers.tasks.containerized_utils._skopeo_copy') @mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') @mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') -@mock.patch('iib.workers.tasks.containerized_utils.get_image_digest') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') @mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@mock.patch('iib.workers.tasks.containerized_utils._get_artifact_combined_tag') -@mock.patch('iib.workers.tasks.containerized_utils._get_name_and_tag_from_pullspec') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @mock.patch('iib.workers.tasks.containerized_utils.wait_for_pipeline_completion') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -1051,8 +1046,6 @@ def test_handle_containerized_rm_with_build_tags( mock_fpr, mock_wfpc, mock_gpiu, - mock_gntfp, - mock_gact, mock_giap, mock_gid, mock_poa, @@ -1061,6 +1054,7 @@ def test_handle_containerized_rm_with_build_tags( mock_uiips, mock_cof, mock_rdc, + mock_rbe, build_tags, expected_tag_count, ): @@ -1100,10 +1094,8 @@ def test_handle_containerized_rm_with_build_tags( # Mock ORAS-related functions # (needed because push_index_db_artifact now called even with empty operators) - mock_gntfp.return_value = ('index', 'v4.14') - mock_gact.return_value = 'index-v4.14' mock_giap.return_value = 'registry.io/index-db:v4.14' - mock_gid.return_value = 'sha256:abcdef' + mock_gid.return_value = 'abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234567' mock_gwc.return_value = { 'iib_registry': 'registry.io', @@ -1131,6 +1123,7 @@ def test_handle_containerized_rm_with_build_tags( assert mock_poa.call_count == 2 # request_id tag + v4.x tag (overwrite_from_index=True) +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.build_containerized_rm._update_index_image_pull_spec') @@ -1138,9 +1131,8 @@ def test_handle_containerized_rm_with_build_tags( @mock.patch('iib.workers.tasks.containerized_utils.set_request_state') @mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') @mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') @mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@mock.patch('iib.workers.tasks.containerized_utils._get_artifact_combined_tag') -@mock.patch('iib.workers.tasks.containerized_utils._get_name_and_tag_from_pullspec') @mock.patch('iib.workers.tasks.containerized_utils.get_pipelinerun_image_url') @mock.patch('iib.workers.tasks.containerized_utils.wait_for_pipeline_completion') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -1192,8 +1184,7 @@ def test_handle_containerized_rm_close_mr_failure_logged( mock_fpr, mock_wfpc, mock_gpiu, - mock_gntfp, - mock_gact, + mock_gid, mock_giap, mock_poa, mock_gwc, @@ -1202,6 +1193,7 @@ def test_handle_containerized_rm_close_mr_failure_logged( mock_uiips, mock_cof, mock_rdc, + mock_rbe, ): """Test that MR close failure is logged but doesn't fail the request.""" request_id = 10 @@ -1238,9 +1230,8 @@ def test_handle_containerized_rm_close_mr_failure_logged( mock_gpiu.return_value = 'quay.io/konflux/image@sha256:built' # Mock ORAS push related functions - mock_gntfp.return_value = ('index-image', 'v4.14') - mock_gact.return_value = 'index-image-v4.14' mock_giap.return_value = 'registry.io/index-db:v4.14' + mock_gid.return_value = 'abcdef0123456789abcdef0123456789abcdef0123456789abcdef01234568' mock_gwc.return_value = { 'iib_registry': 'registry.io', @@ -1268,6 +1259,7 @@ def test_handle_containerized_rm_close_mr_failure_logged( assert final_call[0][1] == 'complete' +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.containerized_utils.find_pipelinerun') @@ -1319,6 +1311,7 @@ def test_handle_containerized_rm_pipelinerun_missing_name( mock_fpr, mock_cof, mock_rdc, + mock_rbe, ): """Test error when pipelinerun metadata doesn't contain name.""" request_id = 11 @@ -1372,6 +1365,7 @@ def test_handle_containerized_rm_pipelinerun_missing_name( mock_cof.assert_called_once() +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists') @mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') @mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') @mock.patch('iib.workers.tasks.containerized_utils._skopeo_copy') @@ -1433,6 +1427,7 @@ def test_handle_containerized_rm_missing_output_pull_spec( mock_sc, mock_cof, mock_rdc, + mock_rbe, ): """Test error when output_pull_spec is not set (defensive check).""" request_id = 12 @@ -1490,3 +1485,132 @@ def test_handle_containerized_rm_missing_output_pull_spec( # Verify cleanup was called mock_cof.assert_called_once() + + +@mock.patch('iib.workers.tasks.build_containerized_rm.reset_docker_config') +@mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_on_failure') +@mock.patch('iib.workers.tasks.build_containerized_rm._update_index_image_pull_spec') +@mock.patch('iib.workers.tasks.build_containerized_rm.cleanup_merge_request_if_exists') +@mock.patch('iib.workers.tasks.build_containerized_rm.merge_mr_after_build') +@mock.patch('iib.workers.tasks.build_containerized_rm.push_index_db_artifact') +@mock.patch('iib.workers.tasks.build_containerized_rm.replicate_image_to_tagged_destinations') +@mock.patch('iib.workers.tasks.build_containerized_rm.monitor_pipeline_and_extract_image') +@mock.patch('iib.workers.tasks.build_containerized_rm.git_commit_and_create_mr') +@mock.patch('iib.workers.tasks.build_containerized_rm.write_build_metadata') +@mock.patch('iib.workers.tasks.build_containerized_rm.opm_validate') +@mock.patch('iib.workers.tasks.build_containerized_rm.merge_catalogs_dirs') +@mock.patch('iib.workers.tasks.build_containerized_rm.opm_registry_rm_fbc') +@mock.patch('iib.workers.tasks.build_containerized_rm.verify_operators_exists') +@mock.patch('iib.workers.tasks.build_containerized_rm.fetch_and_verify_index_db_artifact') +@mock.patch('iib.workers.tasks.build_containerized_rm.prepare_build_sources') +@mock.patch('iib.workers.tasks.build_containerized_rm.remove_operator_deprecations') +@mock.patch('iib.workers.tasks.build_containerized_rm.os.path.exists') +@mock.patch('iib.workers.tasks.build_containerized_rm.os.rename') +@mock.patch('iib.workers.tasks.build_containerized_rm.shutil.rmtree') +@mock.patch('iib.workers.tasks.build_containerized_rm.shutil.copytree') +@mock.patch('iib.workers.tasks.build_containerized_rm.tempfile.TemporaryDirectory') +@mock.patch('iib.workers.tasks.build_containerized_rm._update_index_image_build_state') +@mock.patch('iib.workers.tasks.build_containerized_rm.Opm') +@mock.patch('iib.workers.tasks.build_containerized_rm.prepare_request_for_build') +@mock.patch('iib.workers.tasks.build_containerized_rm.set_request_state') +def test_rm_divergent_never_merges( + mock_srs, + mock_prfb, + mock_opm, + mock_uiibs, + mock_tempdir, + mock_copytree, + mock_rmtree, + mock_rename, + mock_os_exists, + mock_remove_deprecations, + mock_prepare_sources, + mock_fetch_index_db, + mock_voe, + mock_orrf, + mock_mcd, + mock_ov, + mock_wbm, + mock_git_commit, + mock_monitor, + mock_replicate, + mock_push_index_db, + mock_merge_mr, + mock_cleanup_mr, + mock_update_pull_spec, + mock_cof, + mock_rdc, + tmp_path, +): + """Divergent path must never fall back to ORAS and must never merge the MR.""" + request_id = 789 + operators = ['operator1'] + from_index = 'quay.io/namespace/index-image:v4.99' + binary_image = 'registry.io/binary:latest' + + temp_dir = '/tmp/iib-789-test' + mock_tempdir.return_value.__enter__.return_value = temp_dir + + mock_prfb.return_value = { + 'arches': {'amd64'}, + 'binary_image_resolved': 'registry.io/binary@sha256:abc', + 'from_index_resolved': 'quay.io/namespace/index-image@sha256:def', + 'ocp_version': 'v4.14', + 'distribution_scope': 'prod', + } + mock_opm.opm_version = 'v1.28.0' + mock_os_exists.return_value = True + + index_db_path = str(tmp_path / 'index.db') + index_git_repo = 'https://gitlab.com/repo/x.git' + local_git_repo_path = str(tmp_path / 'git_repo') + localized_git_catalog_path = str(tmp_path / 'git_repo' / 'configs') + mock_prepare_sources.return_value = containerized_utils.BuildSources( + index_git_repo=index_git_repo, + local_git_repo_path=local_git_repo_path, + localized_git_catalog_path=localized_git_catalog_path, + index_db_path=index_db_path, + target_branch='v4.14', + is_divergent=True, + ) + + mock_voe.return_value = ({'operator1'}, index_db_path) + fbc_dir = os.path.join(temp_dir, 'fbc') + mock_orrf.return_value = (fbc_dir, None) + + mr_details = { + 'mr_id': '1', + 'mr_url': 'https://gitlab.com/mr/1', + 'source_branch': 'iib-request-789-v4.14', + } + mock_git_commit.return_value = (mr_details, 'commit_sha_789') + mock_monitor.return_value = 'quay.io/konflux/built-image@sha256:xyz789' + mock_replicate.return_value = ['registry.example.com/final-image:789'] + mock_push_index_db.return_value = None + + build_containerized_rm.handle_containerized_rm_request( + operators=operators, + request_id=request_id, + from_index=from_index, + binary_image=binary_image, + overwrite_from_index=True, + overwrite_from_index_token='user:token', + index_to_gitlab_push_map={'quay.io/namespace/index-image': index_git_repo}, + ) + + # Divergent path uses the extracted index.db, never ORAS. + mock_fetch_index_db.assert_not_called() + + # overwrite_from_index=True here: the divergent BuildSources bypasses Task 4's + # entry-point overwrite rejection (mocked directly), so the ONLY thing that can + # prevent a merge is the handler-level `and not sources.is_divergent` guard. If + # that guard were removed, this MR would be merged and this assertion would fail. + mock_merge_mr.assert_not_called() + mock_cleanup_mr.assert_called_once() + + mock_orrf.assert_called_once() + assert mock_orrf.call_args.kwargs['index_db_path'] == index_db_path + mock_cof.assert_not_called() + + expected_output_image = 'quay.io/konflux/built-image@sha256:xyz789' + assert mock_push_index_db.call_args.kwargs['output_image'] == expected_output_image diff --git a/tests/test_workers/test_tasks/test_containerized_utils.py b/tests/test_workers/test_tasks/test_containerized_utils.py index bb3908599..88cdacd8d 100644 --- a/tests/test_workers/test_tasks/test_containerized_utils.py +++ b/tests/test_workers/test_tasks/test_containerized_utils.py @@ -1,15 +1,19 @@ # SPDX-License-Identifier: GPL-3.0-or-later +import inspect import json import os -import tarfile +from unittest import mock from unittest.mock import patch import pytest -from iib.exceptions import IIBError +from iib.exceptions import IIBError, FileNotFoundInImageError +from iib.workers.tasks import containerized_utils as cu from iib.workers.tasks.containerized_utils import ( + extract_catalog_and_db_from_image, extract_files_from_image_non_privileged, pull_index_db_artifact, + push_index_db_artifact, write_build_metadata, cleanup_on_failure, validate_bundles_in_parallel, @@ -284,6 +288,83 @@ def test_pull_index_db_artifact_refresh_cache_fails_falls_back_to_quay( ) +@mock.patch('iib.workers.tasks.containerized_utils.get_oras_artifact') +@mock.patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') +@mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') +def test_pull_raises_when_artifact_missing(m_gwc, m_ref, m_pull): + """When the digest-keyed artifact is missing in Quay, the request must fail. + + The normal path never falls back to extracting index.db from the image; an + un-onboarded image has no artifact and the request is failed so it can be + onboarded first. + """ + m_gwc.return_value = {'iib_use_imagestream_cache': False} + m_ref.return_value = 'quay.io/iib/index-db:idb-x' + m_pull.side_effect = IIBError('not found') # Quay miss + with pytest.raises(IIBError, match='No index.db found for the image'): + pull_index_db_artifact('quay.io/ns/foo:v4.17', '/tmp/req') + + +@mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') +@mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') +@mock.patch('iib.workers.tasks.containerized_utils.set_request_state') +@mock.patch('pathlib.Path.exists', return_value=True) +def test_push_keys_current_artifact_on_output_digest( + m_exists, m_state, m_gwc, m_digest, m_push, tmp_path +): + m_gwc.return_value = { + 'iib_index_db_artifact_registry': 'quay.io/iib', + 'iib_index_db_artifact_template': '{registry}/index-db:{tag}', + } + # digest resolved from the OUTPUT image, not from_index + m_digest.return_value = 'f' * 64 + db = tmp_path / 'index.db' + db.write_text('x') + result = push_index_db_artifact( + request_id=42, + from_index='quay.io/ns/foo:v4.17', + index_db_path=str(db), + operators=['op1'], + output_image='quay.io/ns/foo@sha256:' + 'f' * 64, + overwrite_from_index=True, + request_type='add', + ) + assert result is None + m_digest.assert_called_with('quay.io/ns/foo@sha256:' + 'f' * 64) + pushed_refs = {c.kwargs['artifact_ref'] for c in m_push.call_args_list} + assert 'quay.io/iib/index-db:idb-' + 'f' * 64 in pushed_refs # warm-push (overwrite) + assert 'quay.io/iib/index-db:idb-' + 'f' * 64 + '-42' in pushed_refs # per-request tag + + +@mock.patch('iib.workers.tasks.containerized_utils.push_oras_artifact') +@mock.patch('iib.workers.tasks.containerized_utils._get_index_digest') +@mock.patch('iib.workers.tasks.containerized_utils.get_worker_config') +@mock.patch('iib.workers.tasks.containerized_utils.set_request_state') +@mock.patch('pathlib.Path.exists', return_value=True) +def test_push_throwaway_skips_current_artifact( + m_exists, m_state, m_gwc, m_digest, m_push, tmp_path +): + m_gwc.return_value = { + 'iib_index_db_artifact_registry': 'quay.io/iib', + 'iib_index_db_artifact_template': '{registry}/index-db:{tag}', + } + m_digest.return_value = 'a' * 64 + db = tmp_path / 'index.db' + db.write_text('x') + push_index_db_artifact( + request_id=7, + from_index='quay.io/ns/foo:v4.17', + index_db_path=str(db), + operators=[], + output_image='quay.io/ns/foo@sha256:' + 'a' * 64, + overwrite_from_index=False, + request_type='add', + ) + pushed_refs = {c.kwargs['artifact_ref'] for c in m_push.call_args_list} + assert pushed_refs == {'quay.io/iib/index-db:idb-' + 'a' * 64 + '-7'} # only per-request tag + + @patch('iib.workers.tasks.containerized_utils.log') def test_write_build_metadata_creates_expected_json(mock_log, tmp_path): """write_build_metadata should create JSON file with expected content.""" @@ -469,100 +550,10 @@ def test_cleanup_on_failure_no_mr_no_commit(mock_log): ) -@patch('iib.workers.tasks.containerized_utils.run_cmd') -@patch('iib.workers.tasks.containerized_utils.get_indexdb_artifact_pullspec') -@patch('iib.workers.tasks.containerized_utils.log') -def test_cleanup_on_failure_restores_index_db_artifact( - mock_log, mock_get_indexdb_artifact_pullspec, mock_run_cmd -): - """If original_index_db_digest is provided, oras copy should be invoked correctly.""" - mr_details = None - last_commit_sha = None - index_git_repo = None - overwrite_from_index = False - request_id = 1 - from_index = 'quay.io/ns/index:v4.19' - index_repo_map = {} - original_digest = 'sha256:0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef' - - v4x_artifact_ref = 'quay.io/ns/index-indexdb:v4.19' - mock_get_indexdb_artifact_pullspec.return_value = v4x_artifact_ref - - cleanup_on_failure( - mr_details=mr_details, - last_commit_sha=last_commit_sha, - index_git_repo=index_git_repo, - overwrite_from_index=overwrite_from_index, - request_id=request_id, - from_index=from_index, - index_repo_map=index_repo_map, - original_index_db_digest=original_digest, - ) - - mock_log.info.assert_any_call( - "Restoring index.db artifact to original digest due to %s", "error" - ) - - artifact_name = v4x_artifact_ref.rsplit(':', 1)[0] - expected_source_ref = f'{artifact_name}@{original_digest}' - - mock_run_cmd.assert_called_once_with( - ['oras', 'copy', expected_source_ref, v4x_artifact_ref], - exc_msg=( - f'Failed to restore index.db artifact from {expected_source_ref} ' - f'to {v4x_artifact_ref}' - ), - ) - mock_log.info.assert_any_call("Successfully restored index.db artifact to original digest") - - -@patch('iib.workers.tasks.containerized_utils.run_cmd') -@patch('iib.workers.tasks.oras_utils.get_indexdb_artifact_pullspec') -@patch('iib.workers.tasks.containerized_utils.log') -def test_cleanup_on_failure_restore_failure_is_logged( - mock_log, mock_get_indexdb_artifact_pullspec, mock_run_cmd -): - """If restoring the artifact fails, error should be logged.""" - mock_get_indexdb_artifact_pullspec.return_value = 'quay.io/ns/index-indexdb:v4.19' - mock_run_cmd.side_effect = RuntimeError("oras copy failed") - - cleanup_on_failure( - mr_details=None, - last_commit_sha=None, - index_git_repo=None, - overwrite_from_index=False, - request_id=1, - from_index='quay.io/ns/index:v4.19', - index_repo_map={}, - original_index_db_digest='sha256:0123456789abcdef0123456789abcdef0123456789abcde', - ) - - mock_run_cmd.assert_called_once() - mock_log.error.assert_any_call( - "Failed to restore index.db artifact: %s", mock_run_cmd.side_effect - ) - - -@patch('iib.workers.tasks.containerized_utils.log') -@patch('iib.workers.tasks.oras_utils.get_indexdb_artifact_pullspec') -@patch('iib.workers.tasks.utils.run_cmd') -def test_cleanup_on_failure_no_restore_when_no_original_digest( - mock_run_cmd, mock_get_indexdb_artifact_pullspec, mock_log -): - """If original_index_db_digest is not provided, restoration must not be attempted.""" - cleanup_on_failure( - mr_details=None, - last_commit_sha=None, - index_git_repo=None, - overwrite_from_index=False, - request_id=1, - from_index='quay.io/ns/index:v4.19', - index_repo_map={}, - original_index_db_digest=None, - ) - - mock_get_indexdb_artifact_pullspec.assert_not_called() - mock_run_cmd.assert_not_called() +def test_cleanup_on_failure_has_no_rollback_param(): + """Content keys are immutable: cleanup_on_failure no longer restores artifacts.""" + params = inspect.signature(cleanup_on_failure).parameters + assert 'original_index_db_digest' not in params @patch('iib.workers.tasks.containerized_utils.skopeo_inspect') @@ -1087,267 +1078,119 @@ def test_wait_for_bundle_validation_threads_failure_raises_error_string( mock_log.error.assert_called() -# Tests for extract_files_from_image_non_privileged -@patch('iib.workers.tasks.containerized_utils.log') -@patch('iib.workers.tasks.containerized_utils._skopeo_copy') -def test_extract_files_from_image_non_privileged_success_directory( - mock_skopeo_copy, mock_log, tmpdir -): - """Test successful extraction of a directory from container image.""" - import os - import tarfile +# Tests for extract_files_from_image_non_privileged (backed by 'oc image extract') +def _parse_oc_extract_path(cmd): + """Return (src, dst, is_dir_form) parsed from an 'oc image extract' argv.""" + assert cmd[:4] == ['oc', 'image', 'extract', '--confirm'] + path_arg = next(a for a in cmd if a.startswith('--path=')).removeprefix('--path=') + src, dst = path_arg.rsplit(':', 1) + if src.endswith('/*'): + return src[: -len('/*')], dst, True + return src, dst, False + - # Setup destination directory +@patch('iib.workers.tasks.utils.run_cmd') +def test_extract_files_from_image_non_privileged_success_directory(mock_run_cmd, tmpdir): + """A directory is extracted via the '/*' glob form into dest_path.""" + + def fake_oc(cmd, *args, **kwargs): + src, dst, is_dir = _parse_oc_extract_path(cmd) + if is_dir and src == '/manifests': + # oc unpacks the directory's children into dst. + os.makedirs(dst, exist_ok=True) + with open(os.path.join(dst, 'test_manifest.yaml'), 'w') as f: + f.write('test: data') + + mock_run_cmd.side_effect = fake_oc dest_dir = tmpdir.join('dest') - # Mock skopeo_copy to create proper OCI layout - def mock_copy(source, destination, copy_all, exc_msg): - # Extract OCI directory path from destination (format: oci:/path/to/oci) - oci_path = destination.replace('oci:', '') - - # Create OCI layout structure - os.makedirs(oci_path, exist_ok=True) - blobs_dir = os.path.join(oci_path, 'blobs', 'sha256') - os.makedirs(blobs_dir, exist_ok=True) - - # Create index.json - index_json = { - 'manifests': [ - { - 'digest': 'sha256:abc123', - 'mediaType': 'application/vnd.oci.image.manifest.v1+json', - } - ] - } - with open(os.path.join(oci_path, 'index.json'), 'w') as f: - json.dump(index_json, f) - - # Create manifest - manifest_json = { - 'layers': [ - { - 'digest': 'sha256:layer1', - 'mediaType': 'application/vnd.oci.image.layer.v1.tar+gzip', - } - ] - } - with open(os.path.join(blobs_dir, 'abc123'), 'w') as f: - json.dump(manifest_json, f) - - # Create layer tar.gz with /manifests directory - layer_path = os.path.join(blobs_dir, 'layer1') - with tarfile.open(layer_path, 'w:gz') as tar: - # Create a temporary test file - test_file = tmpdir.join('temp_test_manifest.yaml') - test_file.write('test: data') - # Add it to the tar with the path we expect in the image - tar.add(str(test_file), arcname='manifests/test_manifest.yaml') - - mock_skopeo_copy.side_effect = mock_copy - - # Call the function under test extract_files_from_image_non_privileged('quay.io/ns/test:v1', '/manifests', str(dest_dir)) - # Verify the extraction succeeded - assert dest_dir.check(dir=True) - extracted_file = dest_dir.join('test_manifest.yaml') - assert extracted_file.check(file=True) - assert extracted_file.read() == 'test: data' - - # Verify skopeo was called - mock_skopeo_copy.assert_called_once() - call_args = mock_skopeo_copy.call_args - assert call_args[1]['source'] == 'docker://quay.io/ns/test:v1' - assert 'oci:' in call_args[1]['destination'] - assert call_args[1]['copy_all'] is False - - -@patch('iib.workers.tasks.containerized_utils.log') -@patch('iib.workers.tasks.containerized_utils._skopeo_copy') -def test_extract_files_from_image_non_privileged_missing_index(mock_skopeo_copy, mock_log, tmpdir): - """Test extraction fails when OCI index.json is missing.""" - # Mock skopeo_copy to create OCI dir without index.json - def mock_copy(source, destination, copy_all, exc_msg): - # Extract OCI directory path from destination - oci_path = destination.replace('oci:', '') - os.makedirs(oci_path, exist_ok=True) - # Don't create index.json to simulate error - - mock_skopeo_copy.side_effect = mock_copy - - with pytest.raises(IIBError, match='OCI index.json not found'): - extract_files_from_image_non_privileged( - 'quay.io/ns/test:v1', '/manifests', str(tmpdir.join('dest')) - ) - - -@patch('iib.workers.tasks.containerized_utils.log') -@patch('iib.workers.tasks.containerized_utils._skopeo_copy') -def test_extract_files_from_image_non_privileged_no_manifests(mock_skopeo_copy, mock_log, tmpdir): - """Test extraction fails when no manifests in OCI index.""" - - def mock_copy(source, destination, copy_all, exc_msg): - oci_path = destination.replace('oci:', '') - os.makedirs(oci_path, exist_ok=True) - # Create index.json with empty manifests - index_json = {'manifests': []} - with open(os.path.join(oci_path, 'index.json'), 'w') as f: - json.dump(index_json, f) - - mock_skopeo_copy.side_effect = mock_copy - - with pytest.raises(IIBError, match='No manifests found in OCI index'): - extract_files_from_image_non_privileged( - 'quay.io/ns/test:v1', '/manifests', str(tmpdir.join('dest')) - ) - - -@patch('iib.workers.tasks.containerized_utils.log') -@patch('iib.workers.tasks.containerized_utils._skopeo_copy') -def test_extract_files_from_image_non_privileged_no_layers(mock_skopeo_copy, mock_log, tmpdir): - """Test extraction fails when no layers in manifest.""" - - def mock_copy(source, destination, copy_all, exc_msg): - oci_path = destination.replace('oci:', '') - blobs_dir = os.path.join(oci_path, 'blobs', 'sha256') - os.makedirs(blobs_dir, exist_ok=True) - - # Create index.json - index_json = {'manifests': [{'digest': 'sha256:abc123'}]} - with open(os.path.join(oci_path, 'index.json'), 'w') as f: - json.dump(index_json, f) - - # Create manifest with no layers - manifest_json = {'layers': []} - with open(os.path.join(blobs_dir, 'abc123'), 'w') as f: - json.dump(manifest_json, f) - - mock_skopeo_copy.side_effect = mock_copy - - with pytest.raises(IIBError, match='No layers found in manifest'): - extract_files_from_image_non_privileged( - 'quay.io/ns/test:v1', '/manifests', str(tmpdir.join('dest')) - ) + extracted = dest_dir.join('test_manifest.yaml') + assert extracted.check(file=True) + assert extracted.read() == 'test: data' + # Directory form is tried first and is sufficient (single oc invocation). + mock_run_cmd.assert_called_once() + src, _, is_dir = _parse_oc_extract_path(mock_run_cmd.call_args[0][0]) + assert (src, is_dir) == ('/manifests', True) -@patch('iib.workers.tasks.containerized_utils.log') -@patch('iib.workers.tasks.containerized_utils._skopeo_copy') -def test_extract_files_from_image_non_privileged_missing_layer_blob( - mock_skopeo_copy, mock_log, tmpdir -): - """Test extraction fails when layer blob file is missing.""" +@patch('iib.workers.tasks.utils.run_cmd') +def test_extract_files_from_image_non_privileged_success_file(mock_run_cmd, tmpdir): + """A single file is extracted via the no-glob form and copied to dest_path. + + dest_path may have a different basename than the source file (e.g. the hidden + 'do.not.edit.db' is copied to 'extracted_index.db'). + """ + + def fake_oc(cmd, *args, **kwargs): + src, dst, is_dir = _parse_oc_extract_path(cmd) + # Directory form finds nothing for a file path; file form places the file + # at /. + if not is_dir and src == '/var/lib/iib/_hidden/do.not.edit.db': + os.makedirs(dst, exist_ok=True) + with open(os.path.join(dst, 'do.not.edit.db'), 'wb') as f: + f.write(b'SQLite format 3\x00') + + mock_run_cmd.side_effect = fake_oc + dest_file = tmpdir.join('extracted_index.db') + + extract_files_from_image_non_privileged( + 'quay.io/ns/test:v1', '/var/lib/iib/_hidden/do.not.edit.db', str(dest_file) + ) - def mock_copy(source, destination, copy_all, exc_msg): - oci_path = destination.replace('oci:', '') - blobs_dir = os.path.join(oci_path, 'blobs', 'sha256') - os.makedirs(blobs_dir, exist_ok=True) + assert dest_file.check(file=True) + assert dest_file.read_binary() == b'SQLite format 3\x00' + # Directory form first (empty), then file form. + assert mock_run_cmd.call_count == 2 + forms = [_parse_oc_extract_path(c[0][0])[2] for c in mock_run_cmd.call_args_list] + assert forms == [True, False] - # Create index.json - index_json = {'manifests': [{'digest': 'sha256:abc123'}]} - with open(os.path.join(oci_path, 'index.json'), 'w') as f: - json.dump(index_json, f) - # Create manifest with layer reference - manifest_json = {'layers': [{'digest': 'sha256:missing_layer'}]} - with open(os.path.join(blobs_dir, 'abc123'), 'w') as f: - json.dump(manifest_json, f) - # Don't create the layer blob file +@patch('iib.workers.tasks.utils.run_cmd') +def test_extract_files_from_image_non_privileged_path_not_found(mock_run_cmd, tmpdir): + """When neither form extracts anything, FileNotFoundInImageError is raised. - mock_skopeo_copy.side_effect = mock_copy + 'oc image extract' unpacks only file entries, so an absent path and an empty + directory both extract nothing and are indistinguishable at this layer. + """ + # oc exits 0 and extracts nothing when the path is absent: run_cmd is a no-op. + mock_run_cmd.return_value = '' - with pytest.raises(IIBError, match='Layer blob not found'): + with pytest.raises(FileNotFoundInImageError, match='Path /manifests not found in image'): extract_files_from_image_non_privileged( 'quay.io/ns/test:v1', '/manifests', str(tmpdir.join('dest')) ) + assert mock_run_cmd.call_count == 2 -@patch('iib.workers.tasks.containerized_utils.log') -@patch('iib.workers.tasks.containerized_utils._skopeo_copy') -def test_extract_files_from_image_non_privileged_path_not_found(mock_skopeo_copy, mock_log, tmpdir): - """Test extraction fails when requested path doesn't exist in image.""" - - def mock_copy(source, destination, copy_all, exc_msg): - oci_path = destination.replace('oci:', '') - blobs_dir = os.path.join(oci_path, 'blobs', 'sha256') - os.makedirs(blobs_dir, exist_ok=True) - - # Create index.json - index_json = {'manifests': [{'digest': 'sha256:abc123'}]} - with open(os.path.join(oci_path, 'index.json'), 'w') as f: - json.dump(index_json, f) - - # Create manifest - manifest_json = {'layers': [{'digest': 'sha256:layer1'}]} - with open(os.path.join(blobs_dir, 'abc123'), 'w') as f: - json.dump(manifest_json, f) - - # Create empty layer tar.gz (no content) - layer_path = os.path.join(blobs_dir, 'layer1') - with tarfile.open(layer_path, 'w:gz'): - pass # Empty tar - - mock_skopeo_copy.side_effect = mock_copy +@patch('iib.workers.tasks.utils.run_cmd') +def test_extract_files_from_image_non_privileged_oc_failure(mock_run_cmd, tmpdir): + """A failure from 'oc image extract' propagates as IIBError.""" + mock_run_cmd.side_effect = IIBError('Failed to extract /manifests from image') - with pytest.raises(IIBError, match='Path /manifests not found in image'): + with pytest.raises(IIBError, match='Failed to extract /manifests from image'): extract_files_from_image_non_privileged( 'quay.io/ns/test:v1', '/manifests', str(tmpdir.join('dest')) ) -@patch('iib.workers.tasks.containerized_utils.log') -@patch('iib.workers.tasks.containerized_utils._skopeo_copy') -def test_extract_files_from_image_non_privileged_invalid_layer_tarball( - mock_skopeo_copy, mock_log, tmpdir -): - """Test extraction fails when layer tarball is corrupted.""" - - def mock_copy(source, destination, copy_all, exc_msg): - oci_path = destination.replace('oci:', '') - blobs_dir = os.path.join(oci_path, 'blobs', 'sha256') - os.makedirs(blobs_dir, exist_ok=True) - - # Create index.json - index_json = {'manifests': [{'digest': 'sha256:abc123'}]} - with open(os.path.join(oci_path, 'index.json'), 'w') as f: - json.dump(index_json, f) - - # Create manifest - manifest_json = {'layers': [{'digest': 'sha256:corrupted_layer'}]} - with open(os.path.join(blobs_dir, 'abc123'), 'w') as f: - json.dump(manifest_json, f) - - # Create corrupted layer (not a valid tar.gz) - layer_path = os.path.join(blobs_dir, 'corrupted_layer') - with open(layer_path, 'w') as f: - f.write('not a valid tar.gz file') - - mock_skopeo_copy.side_effect = mock_copy - - with pytest.raises(IIBError, match='Failed to extract layer'): +@patch('iib.workers.tasks.utils.run_cmd') +def test_extract_files_from_image_non_privileged_relative_src_rejected(mock_run_cmd, tmpdir): + """A non-absolute src_path is rejected before invoking oc.""" + with pytest.raises(IIBError, match='must be an absolute image path'): extract_files_from_image_non_privileged( - 'quay.io/ns/test:v1', '/manifests', str(tmpdir.join('dest')) + 'quay.io/ns/test:v1', 'manifests', str(tmpdir.join('dest')) ) + mock_run_cmd.assert_not_called() -@patch('iib.workers.tasks.containerized_utils.log') -@patch('iib.workers.tasks.containerized_utils._skopeo_copy') -def test_extract_files_from_image_non_privileged_skopeo_copy_failure( - mock_skopeo_copy, mock_log, tmpdir -): - """Test extraction fails when skopeo copy fails.""" - mock_skopeo_copy.side_effect = IIBError('Failed to download image') - - with pytest.raises(IIBError, match='Failed to download image'): - extract_files_from_image_non_privileged( - 'quay.io/ns/test:v1', '/manifests', str(tmpdir.join('dest')) - ) - - mock_skopeo_copy.assert_called_once() - # Verify the call was made with correct parameters - call_args = mock_skopeo_copy.call_args - assert call_args[1]['source'] == 'docker://quay.io/ns/test:v1' - assert 'oci:' in call_args[1]['destination'] - assert call_args[1]['copy_all'] is False +@patch('iib.workers.tasks.utils.run_cmd') +def test_extract_files_from_image_non_privileged_root_src_rejected(mock_run_cmd, tmpdir): + """A src_path of '/' names no path under root and is rejected.""" + with pytest.raises(IIBError, match='must name a path under /'): + extract_files_from_image_non_privileged('quay.io/ns/test:v1', '/', str(tmpdir.join('dest'))) + mock_run_cmd.assert_not_called() @patch('iib.workers.tasks.containerized_utils.get_last_commit_sha') @@ -1413,3 +1256,192 @@ def test_merge_mr_after_build_failure_closes_mr(mock_merge_mr, mock_close_mr): merge_mr_after_build(mr_details, 'https://gitlab.example.com/project') mock_close_mr.assert_called_once_with(mr_details, 'https://gitlab.example.com/project') + + +@patch('iib.workers.tasks.containerized_utils.get_image_label') +@patch('iib.workers.tasks.containerized_utils.extract_files_from_image_non_privileged') +def test_extract_catalog_and_db_prefers_hidden_db(mock_extract, mock_label, tmp_path): + """When a hidden index.db exists, it is preferred over the labeled db.""" + + def label_side_effect(image, label): + return { + 'operators.operatorframework.io.index.configs.v1': '/configs', + 'operators.operatorframework.io.index.database.v1': '/database/index.db', + }[label] + + mock_label.side_effect = label_side_effect + + configs_dir, index_db = extract_catalog_and_db_from_image( + 'quay.io/redhat/my-index:test', str(tmp_path) + ) + + assert configs_dir.endswith('configs') + assert index_db.endswith('index.db') + # Two extractions: configs dir and the hidden db file. + assert mock_extract.call_count == 2 + + +@patch('iib.workers.tasks.containerized_utils.get_image_label') +@patch('iib.workers.tasks.containerized_utils.extract_files_from_image_non_privileged') +def test_extract_catalog_and_db_raises_when_no_hidden_db(mock_extract, mock_label, tmp_path): + """When the hidden db is absent, the request fails; there is no labeled-db or empty-db fallback. + + An image with no hidden index.db has not been onboarded to the containerized + build flow, so the request must fail rather than degrade to a labeled db or a + synthesised empty db. + """ + mock_label.side_effect = lambda image, label: { + 'operators.operatorframework.io.index.configs.v1': '/configs', + 'operators.operatorframework.io.index.database.v1': '/database/index.db', + }[label] + # First call (configs) ok; second call (hidden db) raises FileNotFoundInImageError. + mock_extract.side_effect = [None, FileNotFoundInImageError('no hidden db')] + + with pytest.raises(IIBError, match='No index.db found in image'): + extract_catalog_and_db_from_image('quay.io/redhat/my-index:test', str(tmp_path)) + + # Only two extraction attempts: configs dir and the failed hidden db lookup. + # The labeled database.v1 path is never read. + assert mock_extract.call_count == 2 + + +@patch('iib.workers.tasks.containerized_utils.get_image_label') +@patch('iib.workers.tasks.containerized_utils.extract_files_from_image_non_privileged') +def test_extract_catalog_and_db_empty_configs_uses_empty_dir(mock_extract, mock_label, tmp_path): + """A declared-but-empty /configs (empty index) yields an empty catalog, not a failure. + + 'oc image extract' cannot represent an empty directory, so extracting an empty + index's /configs raises FileNotFoundInImageError. Because the image declares a + configs label, this is treated as an empty catalog directory; the hidden db is + still extracted normally. + """ + mock_label.side_effect = lambda image, label: { + 'operators.operatorframework.io.index.configs.v1': '/configs', + }.get(label, '') + # First call (configs) reports nothing under the declared path; second call + # (hidden db) succeeds. + mock_extract.side_effect = [FileNotFoundInImageError('empty /configs'), None] + + configs_dir, index_db = extract_catalog_and_db_from_image( + 'quay.io/redhat/empty-index:test', str(tmp_path) + ) + + assert configs_dir.endswith('extracted_configs') + assert os.path.isdir(configs_dir) + assert os.listdir(configs_dir) == [] # empty catalog + assert index_db.endswith('index.db') + assert mock_extract.call_count == 2 + + +@patch('iib.workers.tasks.containerized_utils.get_image_label') +@patch('iib.workers.tasks.containerized_utils.extract_files_from_image_non_privileged') +def test_extract_catalog_and_db_propagates_real_extraction_error( + mock_extract, mock_label, tmp_path +): + """A genuine extraction failure (not a missing path) must propagate, not degrade. + + Only FileNotFoundInImageError signals an absent hidden db; a plain IIBError + (registry/OCI/layer/tar failure) must not be silently swallowed as "no hidden + db", which would build an image from an incomplete index.db. + """ + mock_label.side_effect = lambda image, label: { + 'operators.operatorframework.io.index.configs.v1': '/configs', + 'operators.operatorframework.io.index.database.v1': '/database/index.db', + }[label] + # First call (configs) ok; second call (hidden db) raises a real error. + mock_extract.side_effect = [None, IIBError('registry unreachable')] + + with pytest.raises(IIBError, match='registry unreachable'): + extract_catalog_and_db_from_image('quay.io/redhat/my-index:test', str(tmp_path)) + + +@patch('iib.workers.tasks.containerized_utils.get_image_label') +def test_extract_catalog_and_db_raises_without_configs_label(mock_label): + """If the image has no FBC configs label, an IIBError is raised.""" + mock_label.return_value = '' + + with pytest.raises(IIBError, match='does not contain a file-based catalog'): + extract_catalog_and_db_from_image('quay.io/redhat/my-index:test', '/tmp/does-not-matter') + + +@mock.patch('iib.workers.tasks.containerized_utils.set_request_state') +@mock.patch('iib.workers.tasks.containerized_utils.clone_git_repo') +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists', return_value=True) +@mock.patch('iib.workers.tasks.containerized_utils.get_git_token', return_value=('n', 't')) +@mock.patch( + 'iib.workers.tasks.containerized_utils.resolve_git_url', return_value='https://gitlab/x.git' +) +def test_prepare_build_sources_normal( + mock_url, mock_tok, mock_exists, mock_clone, mock_state, tmp_path +): + (tmp_path / 'git' / 'v4.14' / 'configs').mkdir(parents=True) + src = cu.prepare_build_sources( + request_id=1, + from_index='quay.io/redhat/my-index:v4.14', + from_index_resolved='quay.io/redhat/my-index@sha256:deadbeef', + temp_dir=str(tmp_path), + ocp_version='v4.14', + index_to_gitlab_push_map={'quay.io/redhat/my-index': 'https://gitlab/x.git'}, + overwrite_from_index=True, + ) + assert src.is_divergent is False + assert src.target_branch == 'v4.14' + assert src.index_db_path is None # normal path pulls from ORAS + + +@mock.patch('iib.workers.tasks.containerized_utils.set_request_state') +@mock.patch('iib.workers.tasks.containerized_utils.get_git_token', return_value=('n', 't')) +@mock.patch('iib.workers.tasks.containerized_utils.remote_branch_exists', return_value=False) +@mock.patch( + 'iib.workers.tasks.containerized_utils.resolve_git_url', return_value='https://gitlab/x.git' +) +def test_prepare_build_sources_divergent_rejects_overwrite( + mock_url, mock_exists, mock_tok, mock_state, tmp_path +): + with pytest.raises(IIBError, match='overwrite'): + cu.prepare_build_sources( + request_id=1, + from_index='quay.io/redhat/my-index:test', + from_index_resolved='quay.io/redhat/my-index@sha256:deadbeef', + temp_dir=str(tmp_path), + ocp_version='v4.14', + index_to_gitlab_push_map={'quay.io/redhat/my-index': 'https://gitlab/x.git'}, + overwrite_from_index=True, + ) + + +@mock.patch('iib.workers.tasks.containerized_utils.set_request_state') +@mock.patch('iib.workers.tasks.containerized_utils.extract_catalog_and_db_from_image') +@mock.patch('iib.workers.tasks.containerized_utils.clone_git_repo') +@mock.patch('iib.workers.tasks.containerized_utils.get_git_token', return_value=('n', 't')) +@mock.patch( + 'iib.workers.tasks.containerized_utils.resolve_git_url', return_value='https://gitlab/x.git' +) +def test_prepare_build_sources_divergent_extracts( + mock_url, mock_tok, mock_clone, mock_extract, mock_state, tmp_path +): + # tag branch missing, ocp branch present + with mock.patch( + 'iib.workers.tasks.containerized_utils.remote_branch_exists', + side_effect=[False, True], + ): + cfg = tmp_path / 'ex_configs' + cfg.mkdir() + (cfg / 'op').mkdir() + mock_extract.return_value = (str(cfg), str(tmp_path / 'ex.db')) + src = cu.prepare_build_sources( + request_id=1, + from_index='quay.io/redhat/my-index:test', + from_index_resolved='quay.io/redhat/my-index@sha256:deadbeef', + temp_dir=str(tmp_path), + ocp_version='v4.14', + index_to_gitlab_push_map={'quay.io/redhat/my-index': 'https://gitlab/x.git'}, + overwrite_from_index=False, + ) + assert src.is_divergent is True + assert src.target_branch == 'v4.14' + assert src.index_db_path == str(tmp_path / 'ex.db') + # Divergent extraction must read the resolved digest, not the mutable tag, so + # the index.db matches the exact image the request resolved to. + mock_extract.assert_called_once() + assert mock_extract.call_args.args[0] == 'quay.io/redhat/my-index@sha256:deadbeef' diff --git a/tests/test_workers/test_tasks/test_git_utils.py b/tests/test_workers/test_tasks/test_git_utils.py index f2e6fc939..ee04b347a 100644 --- a/tests/test_workers/test_tasks/test_git_utils.py +++ b/tests/test_workers/test_tasks/test_git_utils.py @@ -1273,3 +1273,44 @@ def test_merge_gitlab_mr_retries_on_transient_failure(mock_extract, mock_session assert result == 'abc123' assert mock_session.put.call_count == 2 mock_sleep.assert_called_once() + + +@mock.patch('iib.workers.tasks.git_utils.run_cmd') +def test_remote_branch_exists_true(mock_run): + mock_run.return_value = 'abc123\trefs/heads/v4.14\n' + assert git_utils.remote_branch_exists('https://gitlab/x.git', 'v4.14') is True + + +@mock.patch('iib.workers.tasks.git_utils.run_cmd') +def test_remote_branch_exists_false(mock_run): + mock_run.return_value = '' + assert git_utils.remote_branch_exists('https://gitlab/x.git', 'test') is False + + +@mock.patch('iib.workers.tasks.git_utils.run_cmd') +def test_remote_branch_exists_injects_token(mock_run): + mock_run.return_value = 'abc123\trefs/heads/v4.14\n' + + assert ( + git_utils.remote_branch_exists( + 'https://gitlab/x.git', 'v4.14', token_name='user', token='secret' + ) + is True + ) + + args, kwargs = mock_run.call_args + ls_remote_cmd = args[0] + # Token is injected into the URL passed to git, but the user-facing exc_msg + # references the token-free repo_url so the secret is not leaked in errors. + assert ls_remote_cmd[-2] == 'https://user:secret@gitlab/x.git' + assert 'secret' not in kwargs['exc_msg'] + + +@mock.patch('iib.workers.tasks.git_utils.run_cmd') +def test_remote_branch_exists_propagates_command_failure(mock_run): + # A command failure (network/auth error) must raise, not be silently read as + # "branch absent", which would misroute the request onto the divergent path. + mock_run.side_effect = IIBError('git ls-remote failed') + + with pytest.raises(IIBError, match='git ls-remote failed'): + git_utils.remote_branch_exists('https://gitlab/x.git', 'v4.14') diff --git a/tests/test_workers/test_tasks/test_oras_utils.py b/tests/test_workers/test_tasks/test_oras_utils.py index 88337c391..5fac8c09a 100644 --- a/tests/test_workers/test_tasks/test_oras_utils.py +++ b/tests/test_workers/test_tasks/test_oras_utils.py @@ -13,9 +13,50 @@ verify_indexdb_cache_sync, get_image_stream_digest, refresh_indexdb_cache, + _get_index_digest, + _get_artifact_combined_tag, + get_indexdb_artifact_pullspec, + get_index_tag, ) +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_get_index_digest_strips_algo_prefix(mock_digest): + mock_digest.return_value = 'sha256:' + 'a' * 64 + assert _get_index_digest('quay.io/ns/repo:v4.17') == 'a' * 64 + mock_digest.assert_called_once_with('quay.io/ns/repo:v4.17') + + +@mock.patch('iib.workers.tasks.oras_utils.get_worker_config') +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_combined_tag_is_digest_only(mock_digest, mock_gwc): + mock_gwc.return_value = {'iib_index_db_artifact_tag_template': 'idb-{digest}'} + mock_digest.return_value = 'sha256:' + 'b' * 64 + assert _get_artifact_combined_tag('quay.io/my-ns/iib-pub:v4.17') == 'idb-' + 'b' * 64 + + +@mock.patch('iib.workers.tasks.oras_utils.get_worker_config') +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_same_content_different_pullspec_same_tag(mock_digest, mock_gwc): + """Problem 1b: promotion — identical digest under two pullspecs => identical tag.""" + mock_gwc.return_value = {'iib_index_db_artifact_tag_template': 'idb-{digest}'} + mock_digest.return_value = 'sha256:' + 'c' * 64 + staging = _get_artifact_combined_tag('quay.io/my-ns/iib-pub:v4.17') + released = _get_artifact_combined_tag('registry.access.redhat.com/some-ns/operator-index:v4.17') + assert staging == released == 'idb-' + 'c' * 64 + + +@mock.patch('iib.workers.tasks.oras_utils.get_worker_config') +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_different_content_different_tag(mock_digest, mock_gwc): + """Problem 1: different images (different digests) => different tags.""" + mock_gwc.return_value = {'iib_index_db_artifact_tag_template': 'idb-{digest}'} + mock_digest.side_effect = ['sha256:' + 'd' * 64, 'sha256:' + 'e' * 64] + a = _get_artifact_combined_tag('quay.io/redhat/foo:v4.17') + b = _get_artifact_combined_tag('quay.io/redhat-pending/foo:v4.17') + assert a != b + + @pytest.fixture() def registry_auths(): return {'auths': {'quay.io': {'auth': 'dXNlcjpwYXNz'}}} # base64 encoded user:pass @@ -653,171 +694,163 @@ def test_get_name_and_tag_from_pullspec_invalid(invalid_pullspec, expected_error @pytest.mark.parametrize( - "image_name,tag,expected_tag", + "pullspec,digest,expected_tag", [ - ("iib-pub-pending", "v4.17", "iib-pub-pending-v4.17"), - ("my-image", "latest", "my-image-latest"), - ("test-index", "v1.0.0", "test-index-v1.0.0"), + ("quay.io/ns/iib-pub-pending:v4.17", "a" * 64, "idb-" + "a" * 64), + ("quay.io/ns/my-image:latest", "b" * 64, "idb-" + "b" * 64), + ("quay.io/ns/test-index:v1.0.0", "c" * 64, "idb-" + "c" * 64), ], ) @mock.patch('iib.workers.tasks.oras_utils.get_worker_config') -def test_get_artifact_combined_tag(mock_gwc, image_name, tag, expected_tag): +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_get_artifact_combined_tag(mock_digest, mock_gwc, pullspec, digest, expected_tag): """Test generating combined artifact tags.""" - from iib.workers.tasks.oras_utils import _get_artifact_combined_tag - - mock_gwc.return_value = {'iib_index_db_artifact_tag_template': '{image_name}-{tag}'} + mock_gwc.return_value = {'iib_index_db_artifact_tag_template': 'idb-{digest}'} + mock_digest.return_value = f'sha256:{digest}' - result = _get_artifact_combined_tag(image_name, tag) + result = _get_artifact_combined_tag(pullspec) assert result == expected_tag @pytest.mark.parametrize( - "from_index,expected_pullspec", + "from_index,digest", [ - ( - "registry.example.com/namespace/iib-pub-pending:v4.17", - "test-artifact-registry/index-db:iib-pub-pending-v4.17", - ), - ( - "quay.io/namespace/my-image:latest", - "test-artifact-registry/index-db:my-image-latest", - ), - ( - "registry.io/org/repo/index-image:v1.0.0", - "test-artifact-registry/index-db:index-image-v1.0.0", - ), - ( - "registry.example.com/namespace/iib-pub-pending:v4.17@sha256:abc123", - "test-artifact-registry/index-db:iib-pub-pending-v4.17", - ), + ("registry.example.com/namespace/iib-pub-pending:v4.17", "a" * 64), + ("quay.io/namespace/my-image:latest", "b" * 64), + ("registry.io/org/repo/index-image:v1.0.0", "c" * 64), + ("registry.example.com/namespace/iib-pub-pending:v4.17@sha256:abc123", "d" * 64), ], ) @mock.patch('iib.workers.tasks.oras_utils.get_worker_config') -def test_get_indexdb_artifact_pullspec(mock_gwc, from_index, expected_pullspec): +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_get_indexdb_artifact_pullspec(mock_digest, mock_gwc, from_index, digest): """Test constructing index DB artifact pullspecs.""" - from iib.workers.tasks.oras_utils import get_indexdb_artifact_pullspec - mock_gwc.return_value = { 'iib_index_db_artifact_registry': 'test-artifact-registry', 'iib_index_db_artifact_template': '{registry}/index-db:{tag}', - 'iib_index_db_artifact_tag_template': '{image_name}-{tag}', + 'iib_index_db_artifact_tag_template': 'idb-{digest}', } + mock_digest.return_value = f'sha256:{digest}' result = get_indexdb_artifact_pullspec(from_index) - assert result == expected_pullspec + assert result == f'test-artifact-registry/index-db:idb-{digest}' + mock_digest.assert_called_once_with(from_index) @mock.patch('iib.workers.tasks.oras_utils.get_worker_config') -def test_get_indexdb_artifact_pullspec_invalid(mock_gwc): - """Test _get_indexdb_artifact_pullspec with invalid pullspec.""" - from iib.workers.tasks.oras_utils import get_indexdb_artifact_pullspec - +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_get_indexdb_artifact_pullspec_digest_resolution_failure(mock_digest, mock_gwc): + """Test get_indexdb_artifact_pullspec propagates digest resolution failures.""" mock_gwc.return_value = { 'iib_index_db_artifact_registry': 'test-artifact-registry', 'iib_index_db_artifact_template': '{registry}/index-db:{tag}', - 'iib_index_db_artifact_tag_template': '{image_name}-{tag}', + 'iib_index_db_artifact_tag_template': 'idb-{digest}', } + mock_digest.side_effect = IIBError('Failed to inspect image') - with pytest.raises(IIBError, match="Missing tag"): - get_indexdb_artifact_pullspec("registry.example.com/namespace/image") + with pytest.raises(IIBError, match="Failed to inspect image"): + get_indexdb_artifact_pullspec("registry.example.com/namespace/image:v1.0.0") @mock.patch('iib.workers.tasks.oras_utils.verify_indexdb_cache_sync') +@mock.patch('iib.workers.tasks.oras_utils.get_worker_config') +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') @pytest.mark.parametrize( - "pullspec,expected_combined_tag,sync_result", + "pullspec,digest,sync_result", [ - ( - "registry.example.com/namespace/iib-pub-pending:v4.17", - "iib-pub-pending-v4.17", - True, - ), - ( - "quay.io/namespace/my-image:latest", - "my-image-latest", - False, - ), - ( - "registry.io/org/repo/index-image:v1.0.0@sha256:abc123", - "index-image-v1.0.0", - True, - ), + ("registry.example.com/namespace/iib-pub-pending:v4.17", "a" * 64, True), + ("quay.io/namespace/my-image:latest", "b" * 64, False), + ("registry.io/org/repo/index-image:v1.0.0@sha256:abc123", "c" * 64, True), ], ) def test_verify_indexdb_cache_for_image( - mock_verify_sync, pullspec, expected_combined_tag, sync_result + mock_digest, mock_gwc, mock_verify_sync, pullspec, digest, sync_result ): """Test verify_indexdb_cache_for_image with various pullspecs.""" from iib.workers.tasks.oras_utils import verify_indexdb_cache_for_image + mock_gwc.return_value = {'iib_index_db_artifact_tag_template': 'idb-{digest}'} + mock_digest.return_value = f'sha256:{digest}' mock_verify_sync.return_value = sync_result result = verify_indexdb_cache_for_image(pullspec) assert result == sync_result - mock_verify_sync.assert_called_once_with(expected_combined_tag) + mock_verify_sync.assert_called_once_with(f'idb-{digest}') @mock.patch('iib.workers.tasks.oras_utils.verify_indexdb_cache_sync') -def test_verify_indexdb_cache_for_image_invalid_pullspec(mock_verify_sync): - """Test verify_indexdb_cache_for_image with invalid pullspec.""" +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_verify_indexdb_cache_for_image_digest_resolution_failure(mock_digest, mock_verify_sync): + """Test verify_indexdb_cache_for_image propagates digest resolution failures.""" from iib.workers.tasks.oras_utils import verify_indexdb_cache_for_image - with pytest.raises(IIBError, match="Missing tag"): - verify_indexdb_cache_for_image("registry.example.com/namespace/image") + mock_digest.side_effect = IIBError('Failed to inspect image') + + with pytest.raises(IIBError, match="Failed to inspect image"): + verify_indexdb_cache_for_image("registry.example.com/namespace/image:v1.0.0") mock_verify_sync.assert_not_called() @mock.patch('iib.workers.tasks.oras_utils.refresh_indexdb_cache') +@mock.patch('iib.workers.tasks.oras_utils.get_worker_config') +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') @pytest.mark.parametrize( - "pullspec,expected_combined_tag", + "pullspec,digest", [ - ( - "registry.example.com/namespace/iib-pub-pending:v4.17", - "iib-pub-pending-v4.17", - ), - ( - "quay.io/namespace/my-image:latest", - "my-image-latest", - ), - ( - "registry.io/org/repo/index-image:v1.0.0", - "index-image-v1.0.0", - ), - ( - "registry.example.com/namespace/iib-pub-pending:v4.17@sha256:abc123", - "iib-pub-pending-v4.17", - ), + ("registry.example.com/namespace/iib-pub-pending:v4.17", "a" * 64), + ("quay.io/namespace/my-image:latest", "b" * 64), + ("registry.io/org/repo/index-image:v1.0.0", "c" * 64), + ("registry.example.com/namespace/iib-pub-pending:v4.17@sha256:abc123", "d" * 64), ], ) -def test_refresh_indexdb_cache_for_image(mock_refresh_cache, pullspec, expected_combined_tag): +def test_refresh_indexdb_cache_for_image( + mock_digest, mock_gwc, mock_refresh_cache, pullspec, digest +): """Test refresh_indexdb_cache_for_image with various pullspecs.""" from iib.workers.tasks.oras_utils import refresh_indexdb_cache_for_image + mock_gwc.return_value = {'iib_index_db_artifact_tag_template': 'idb-{digest}'} + mock_digest.return_value = f'sha256:{digest}' + refresh_indexdb_cache_for_image(pullspec) - mock_refresh_cache.assert_called_once_with(expected_combined_tag) + mock_refresh_cache.assert_called_once_with(f'idb-{digest}') @mock.patch('iib.workers.tasks.oras_utils.refresh_indexdb_cache') -def test_refresh_indexdb_cache_for_image_invalid_pullspec(mock_refresh_cache): - """Test refresh_indexdb_cache_for_image with invalid pullspec.""" +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_refresh_indexdb_cache_for_image_digest_resolution_failure(mock_digest, mock_refresh_cache): + """Test refresh_indexdb_cache_for_image propagates digest resolution failures.""" from iib.workers.tasks.oras_utils import refresh_indexdb_cache_for_image - with pytest.raises(IIBError, match="Missing tag"): - refresh_indexdb_cache_for_image("registry.example.com/namespace/image") + mock_digest.side_effect = IIBError('Failed to inspect image') + + with pytest.raises(IIBError, match="Failed to inspect image"): + refresh_indexdb_cache_for_image("registry.example.com/namespace/image:v1.0.0") mock_refresh_cache.assert_not_called() @mock.patch('iib.workers.tasks.oras_utils.refresh_indexdb_cache') -def test_refresh_indexdb_cache_for_image_propagates_exception(mock_refresh_cache): +@mock.patch('iib.workers.tasks.oras_utils.get_worker_config') +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_refresh_indexdb_cache_for_image_propagates_exception( + mock_digest, mock_gwc, mock_refresh_cache +): """Test if refresh_indexdb_cache_for_image propagates exceptions from refresh_indexdb_cache.""" from iib.workers.tasks.oras_utils import refresh_indexdb_cache_for_image + mock_gwc.return_value = {'iib_index_db_artifact_tag_template': 'idb-{digest}'} + mock_digest.return_value = 'sha256:' + 'e' * 64 mock_refresh_cache.side_effect = IIBError('Refresh failed') with pytest.raises(IIBError, match='Refresh failed'): refresh_indexdb_cache_for_image("registry.example.com/namespace/image:v1.0.0") + + +def test_get_index_tag(): + assert get_index_tag('quay.io/redhat/my-index:v4.17') == 'v4.17' diff --git a/tests/test_workers/test_tasks/test_utils.py b/tests/test_workers/test_tasks/test_utils.py index cad086d23..9b97bf8fb 100644 --- a/tests/test_workers/test_tasks/test_utils.py +++ b/tests/test_workers/test_tasks/test_utils.py @@ -1874,135 +1874,99 @@ def test_change_dir_invalid_directory_does_not_change_cwd(tmp_path): @pytest.mark.parametrize( - "from_index,expected_pullspec", + "from_index,digest", [ - ( - "registry.example.com/namespace/iib-pub-pending:v4.17", - "test-artifact-registry/index-db:iib-pub-pending-v4.17", - ), - ( - "quay.io/namespace/my-image:latest", - "test-artifact-registry/index-db:my-image-latest", - ), - ( - "registry.io/org/repo/index-image:v1.0.0", - "test-artifact-registry/index-db:index-image-v1.0.0", - ), - ( - "registry.example.com/namespace/iib-pub-pending:v4.17@sha256:abc123", - "test-artifact-registry/index-db:iib-pub-pending-v4.17", - ), + ("registry.example.com/namespace/iib-pub-pending:v4.17", "a" * 64), + ("quay.io/namespace/my-image:latest", "b" * 64), + ("registry.io/org/repo/index-image:v1.0.0", "c" * 64), + ("registry.example.com/namespace/iib-pub-pending:v4.17@sha256:abc123", "d" * 64), ], ) @mock.patch('iib.workers.tasks.oras_utils.get_worker_config') -def test_get_indexdb_artifact_pullspec(mock_gwc, from_index, expected_pullspec): +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_get_indexdb_artifact_pullspec(mock_digest, mock_gwc, from_index, digest): """Test constructing index DB artifact pullspecs.""" from iib.workers.tasks.oras_utils import get_indexdb_artifact_pullspec mock_gwc.return_value = { 'iib_index_db_artifact_registry': 'test-artifact-registry', 'iib_index_db_artifact_template': '{registry}/index-db:{tag}', - 'iib_index_db_artifact_tag_template': '{image_name}-{tag}', + 'iib_index_db_artifact_tag_template': 'idb-{digest}', } + mock_digest.return_value = f'sha256:{digest}' result = get_indexdb_artifact_pullspec(from_index) - assert result == expected_pullspec + assert result == f'test-artifact-registry/index-db:idb-{digest}' -@mock.patch('iib.workers.tasks.oras_utils.get_worker_config') -def test_get_indexdb_artifact_pullspec_invalid(mock_gwc): - """Test _get_indexdb_artifact_pullspec with invalid pullspec.""" +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_get_indexdb_artifact_pullspec_invalid(mock_digest): + """Test get_indexdb_artifact_pullspec propagates digest resolution failures.""" from iib.workers.tasks.oras_utils import get_indexdb_artifact_pullspec - mock_gwc.return_value = { - 'iib_index_db_artifact_registry': 'test-artifact-registry', - 'iib_index_db_artifact_template': '{registry}/index-db:{tag}', - 'iib_index_db_artifact_tag_template': '{image_name}-{tag}', - } + mock_digest.side_effect = IIBError('Failed to inspect image') - with pytest.raises(IIBError, match="Missing tag"): + with pytest.raises(IIBError, match="Failed to inspect image"): get_indexdb_artifact_pullspec("registry.example.com/namespace/image") @pytest.mark.parametrize( - "from_index,expected_pullspec", + "from_index,digest", [ - ( - "registry.example.com/namespace/iib-pub-pending:v4.17", - "test-imagestream-registry/index-db:iib-pub-pending-v4.17", - ), - ( - "quay.io/namespace/my-image:latest", - "test-imagestream-registry/index-db:my-image-latest", - ), - ( - "registry.io/org/repo/index-image:v1.0.0", - "test-imagestream-registry/index-db:index-image-v1.0.0", - ), - ( - "registry.example.com/namespace/iib-pub-pending:v4.17@sha256:abc123", - "test-imagestream-registry/index-db:iib-pub-pending-v4.17", - ), + ("registry.example.com/namespace/iib-pub-pending:v4.17", "a" * 64), + ("quay.io/namespace/my-image:latest", "b" * 64), + ("registry.io/org/repo/index-image:v1.0.0", "c" * 64), + ("registry.example.com/namespace/iib-pub-pending:v4.17@sha256:abc123", "d" * 64), ], ) @mock.patch('iib.workers.tasks.oras_utils.get_worker_config') -def test_get_imagestream_artifact_pullspec(mock_gwc, from_index, expected_pullspec): +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_get_imagestream_artifact_pullspec(mock_digest, mock_gwc, from_index, digest): """Test constructing ImageStream artifact pullspecs.""" mock_gwc.return_value = { 'iib_index_db_imagestream_registry': 'test-imagestream-registry', 'iib_index_db_artifact_template': '{registry}/index-db:{tag}', - 'iib_index_db_artifact_tag_template': '{image_name}-{tag}', + 'iib_index_db_artifact_tag_template': 'idb-{digest}', } + mock_digest.return_value = f'sha256:{digest}' result = get_imagestream_artifact_pullspec(from_index) - assert result == expected_pullspec + assert result == f'test-imagestream-registry/index-db:idb-{digest}' -@mock.patch('iib.workers.tasks.oras_utils.get_worker_config') -def test_get_imagestream_artifact_pullspec_invalid(mock_gwc): - """Test get_imagestream_artifact_pullspec with invalid pullspec.""" - mock_gwc.return_value = { - 'iib_index_db_imagestream_registry': 'test-imagestream-registry', - 'iib_index_db_artifact_template': '{registry}/index-db:{tag}', - 'iib_index_db_artifact_tag_template': '{image_name}-{tag}', - } +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') +def test_get_imagestream_artifact_pullspec_invalid(mock_digest): + """Test get_imagestream_artifact_pullspec propagates digest resolution failures.""" + mock_digest.side_effect = IIBError('Failed to inspect image') - with pytest.raises(IIBError, match="Missing tag"): + with pytest.raises(IIBError, match="Failed to inspect image"): get_imagestream_artifact_pullspec("registry.example.com/namespace/image") @mock.patch('iib.workers.tasks.oras_utils.verify_indexdb_cache_sync') +@mock.patch('iib.workers.tasks.oras_utils.get_worker_config') +@mock.patch('iib.workers.tasks.oras_utils.get_image_digest') @pytest.mark.parametrize( - "pullspec,expected_combined_tag,sync_result", + "pullspec,digest,sync_result", [ - ( - "registry.example.com/namespace/iib-pub-pending:v4.17", - "iib-pub-pending-v4.17", - True, - ), - ( - "quay.io/namespace/my-image:latest", - "my-image-latest", - False, - ), - ( - "registry.io/org/repo/index-image:v1.0.0@sha256:abc123", - "index-image-v1.0.0", - True, - ), + ("registry.example.com/namespace/iib-pub-pending:v4.17", "a" * 64, True), + ("quay.io/namespace/my-image:latest", "b" * 64, False), + ("registry.io/org/repo/index-image:v1.0.0@sha256:abc123", "c" * 64, True), ], ) def test_verify_indexdb_cache_for_image( - mock_verify_sync, pullspec, expected_combined_tag, sync_result + mock_digest, mock_gwc, mock_verify_sync, pullspec, digest, sync_result ): """Test verify_indexdb_cache_for_image with various pullspecs.""" from iib.workers.tasks.oras_utils import verify_indexdb_cache_for_image + mock_gwc.return_value = {'iib_index_db_artifact_tag_template': 'idb-{digest}'} + mock_digest.return_value = f'sha256:{digest}' mock_verify_sync.return_value = sync_result result = verify_indexdb_cache_for_image(pullspec) assert result == sync_result - mock_verify_sync.assert_called_once_with(expected_combined_tag) + mock_verify_sync.assert_called_once_with(f'idb-{digest}')