diff --git a/iib/workers/tasks/build.py b/iib/workers/tasks/build.py index 07d9133d9..dbb7ea0c9 100644 --- a/iib/workers/tasks/build.py +++ b/iib/workers/tasks/build.py @@ -48,6 +48,7 @@ get_bundles_from_deprecation_list, get_resolved_bundles, get_resolved_image, + get_images_needing_overwrite_token, podman_pull, request_logger, reset_docker_config, @@ -833,10 +834,19 @@ def handle_add_request( :raises IIBError: if the index image build fails. """ _cleanup() - # Resolve bundles to their digests + # Resolve bundles to their digests. Apply overwrite_from_index_token only to same-namespace + # bundles that are not already covered by worker Docker config credentials. That preserves + # broader template auth (e.g. quay.io/namespace) when present, avoids breaking public pulls + # on other namespaces of the same registry, and still allows the overwrite token to pull + # private same-namespace bundles when no other creds exist. + # Do not stamp from_index auth here — this step does not pull from_index; prepare_request + # and later from_index accessors apply the overwrite token for the index itself. set_request_state(request_id, 'in_progress', 'Resolving the bundles') - with set_registry_token(overwrite_from_index_token, from_index, append=True): + bundles_needing_token = get_images_needing_overwrite_token(from_index, bundles) + + def _resolve_bundles() -> None: + nonlocal resolved_bundles resolved_bundles = get_resolved_bundles(bundles) verify_labels(resolved_bundles) if check_related_images: @@ -846,6 +856,15 @@ def handle_add_request( worker_config.iib_related_image_registry_replacement.get(username), ) + resolved_bundles: List[str] = [] + if bundles_needing_token: + with set_registry_token( + overwrite_from_index_token, bundles_needing_token, append=True + ): + _resolve_bundles() + else: + _resolve_bundles() + # Check if Gating passes for all the bundles if greenwave_config: gate_bundles(resolved_bundles, greenwave_config) @@ -867,7 +886,7 @@ def handle_add_request( from_index_resolved = prebuild_info['from_index_resolved'] Opm.set_opm_version(from_index_resolved) - with set_registry_token(overwrite_from_index_token, from_index_resolved): + with set_registry_token(overwrite_from_index_token, from_index_resolved, append=True): is_fbc = is_image_fbc(from_index_resolved) if from_index else False if is_fbc: # logging requested by stakeholders do not delete diff --git a/iib/workers/tasks/build_fbc_operations.py b/iib/workers/tasks/build_fbc_operations.py index 21079a8a2..befa8fa53 100644 --- a/iib/workers/tasks/build_fbc_operations.py +++ b/iib/workers/tasks/build_fbc_operations.py @@ -18,6 +18,7 @@ from iib.workers.tasks.celery import app from iib.workers.tasks.opm_operations import opm_registry_add_fbc_fragment, Opm from iib.workers.tasks.utils import ( + get_images_needing_overwrite_token, get_resolved_image, prepare_request_for_build, request_logger, @@ -69,12 +70,23 @@ def handle_fbc_operation_request( _cleanup() set_request_state(request_id, 'in_progress', 'Resolving the fbc fragments') - # Resolve all fbc fragments + # Apply overwrite_from_index_token only to same-namespace fragments that are not already + # covered by worker Docker config credentials. from_index is not pulled here; prepare_request + # and later from_index accessors apply the overwrite token for the index itself. + fragments_needing_token = get_images_needing_overwrite_token(from_index, fbc_fragments) resolved_fbc_fragments = [] - for fbc_fragment in fbc_fragments: - with set_registry_token(overwrite_from_index_token, fbc_fragment, append=True): - resolved_fbc_fragment = get_resolved_image(fbc_fragment) - resolved_fbc_fragments.append(resolved_fbc_fragment) + + def _resolve_fbc_fragments() -> None: + for fbc_fragment in fbc_fragments: + resolved_fbc_fragments.append(get_resolved_image(fbc_fragment)) + + if fragments_needing_token: + with set_registry_token( + overwrite_from_index_token, fragments_needing_token, append=True + ): + _resolve_fbc_fragments() + else: + _resolve_fbc_fragments() prebuild_info = prepare_request_for_build( request_id, diff --git a/iib/workers/tasks/opm_operations.py b/iib/workers/tasks/opm_operations.py index 11e5f873f..0ca7ba9e6 100644 --- a/iib/workers/tasks/opm_operations.py +++ b/iib/workers/tasks/opm_operations.py @@ -839,14 +839,32 @@ def opm_registry_add_fbc( ignore_existing=True, ) - _opm_registry_add( - base_dir=base_dir, - index_db=index_db_file, - bundles=bundles, - overwrite_csv=overwrite_csv, - container_tool=container_tool, - graph_update_mode=graph_update_mode, - ) + from iib.workers.tasks.utils import get_images_needing_overwrite_token, set_registry_token + + # opm registry add pulls bundle images; apply overwrite token for same-namespace + # bundles not already covered by worker Docker config (mirrors opm_index_add). + bundles_needing_token = get_images_needing_overwrite_token(from_index, bundles) + if bundles_needing_token: + with set_registry_token( + overwrite_from_index_token, bundles_needing_token, append=True + ): + _opm_registry_add( + base_dir=base_dir, + index_db=index_db_file, + bundles=bundles, + overwrite_csv=overwrite_csv, + container_tool=container_tool, + graph_update_mode=graph_update_mode, + ) + else: + _opm_registry_add( + base_dir=base_dir, + index_db=index_db_file, + bundles=bundles, + overwrite_csv=overwrite_csv, + container_tool=container_tool, + graph_update_mode=graph_update_mode, + ) fbc_dir, _ = opm_migrate(index_db=index_db_file, base_dir=base_dir) # we should keep generating Dockerfile here @@ -1020,22 +1038,42 @@ def opm_registry_add_fbc_fragment( f'Extracting operator packages from {len(fbc_fragments)} fbc fragment(s)', ) + from iib.workers.tasks.utils import get_images_needing_overwrite_token, set_registry_token + # the dir where all the configs from from_index are stored # this will look like /tmp/iib-**/configs - from_index_configs_dir = get_catalog_dir(from_index=from_index, base_dir=temp_dir) + if overwrite_from_index_token: + with set_registry_token(overwrite_from_index_token, from_index, append=True): + from_index_configs_dir = get_catalog_dir(from_index=from_index, base_dir=temp_dir) + else: + from_index_configs_dir = get_catalog_dir(from_index=from_index, base_dir=temp_dir) log.info("The content of from_index configs located at %s", from_index_configs_dir) - # Single pass: Extract all fragment paths and operators + # Single pass: Extract all fragment paths and operators. Re-apply overwrite token for + # same-namespace fragments that are not covered by worker Docker config, matching resolve. fragment_data = [] all_fragment_operators = [] - - for i, fbc_fragment in enumerate(fbc_fragments): - # fragment path will look like /tmp/iib-**/fbc-fragment-{index} - fragment_path, fragment_operators = extract_fbc_fragment( - temp_dir=temp_dir, fbc_fragment=fbc_fragment, fragment_index=i - ) - fragment_data.append((fragment_path, fragment_operators)) - all_fragment_operators.extend(fragment_operators) + fragments_needing_token = get_images_needing_overwrite_token(from_index, fbc_fragments) + + if fragments_needing_token: + with set_registry_token( + overwrite_from_index_token, fragments_needing_token, append=True + ): + for i, fbc_fragment in enumerate(fbc_fragments): + # fragment path will look like /tmp/iib-**/fbc-fragment-{index} + fragment_path, fragment_operators = extract_fbc_fragment( + temp_dir=temp_dir, fbc_fragment=fbc_fragment, fragment_index=i + ) + fragment_data.append((fragment_path, fragment_operators)) + all_fragment_operators.extend(fragment_operators) + else: + for i, fbc_fragment in enumerate(fbc_fragments): + # fragment path will look like /tmp/iib-**/fbc-fragment-{index} + fragment_path, fragment_operators = extract_fbc_fragment( + temp_dir=temp_dir, fbc_fragment=fbc_fragment, fragment_index=i + ) + fragment_data.append((fragment_path, fragment_operators)) + all_fragment_operators.extend(fragment_operators) # Single verification: Check for operators that already exist in the database operators_in_db, index_db_path = verify_operators_exists( @@ -1213,7 +1251,11 @@ def opm_index_add( # The bundles are not resolved since these are stable tags, and references # to a bundle image using a digest fails when using the opm command. - from iib.workers.tasks.utils import run_cmd, set_registry_token + from iib.workers.tasks.utils import ( + get_images_needing_overwrite_token, + run_cmd, + set_registry_token, + ) bundle_str = ','.join(bundles) or '""' cmd = [ @@ -1248,7 +1290,19 @@ def opm_index_add( log.info('Using force to add bundle(s) to index') cmd.extend(['--overwrite-latest']) - with set_registry_token(overwrite_from_index_token, from_index, append=True): + # Authenticate to from_index and to same-namespace bundles that are not already covered + # by worker Docker config (needed when opm pulls those bundles after resolve). + token_images: List[str] = [] + if from_index: + token_images.append(from_index) + for bundle in get_images_needing_overwrite_token(from_index, bundles): + if bundle not in token_images: + token_images.append(bundle) + + if token_images: + with set_registry_token(overwrite_from_index_token, token_images, append=True): + run_cmd(cmd, {'cwd': base_dir}, exc_msg='Failed to add the bundles to the index image') + else: run_cmd(cmd, {'cwd': base_dir}, exc_msg='Failed to add the bundles to the index image') diff --git a/iib/workers/tasks/utils.py b/iib/workers/tasks/utils.py index 6040a5a12..6eec771a9 100644 --- a/iib/workers/tasks/utils.py +++ b/iib/workers/tasks/utils.py @@ -599,40 +599,170 @@ def _docker_auth_key_for_image(container_image: str) -> str: ) +def _docker_auth_keys_covering_image(container_image: str) -> List[str]: + """ + Return docker config ``auths`` keys that can authenticate ``container_image``. + + Keys are ordered from most specific to least specific: + ``registry/namespace/repo``, ``registry/namespace``, then ``registry``. + """ + image_name = ImageName.parse(container_image) + registry = image_name.registry + if not registry: + return [] + + keys: List[str] = [] + if image_name.namespace: + keys.append(f'{registry}/{image_name.namespace}/{image_name.repo}') + keys.append(f'{registry}/{image_name.namespace}') + keys.append(registry) + return keys + + +def _load_docker_config_auths() -> Dict[str, Any]: + """ + Load ``auths`` from the worker Docker config. + + Prefers the active ``~/.docker/config.json`` (often a symlink to the template after + :func:`reset_docker_config`), then falls back to ``iib_docker_config_template``. + """ + conf = get_worker_config() + candidate_paths = [ + os.path.join(os.path.expanduser('~'), '.docker', 'config.json'), + conf.iib_docker_config_template, + ] + for path in candidate_paths: + if not os.path.exists(path): + continue + try: + with open(path, 'r') as f: + return json.load(f).get('auths', {}) or {} + except (OSError, json.JSONDecodeError) as e: + log.warning('Failed to read Docker config auths from %s: %s', path, e) + return {} + + +def docker_config_has_auth_for_image( + container_image: str, auths: Optional[Dict[str, Any]] = None +) -> bool: + """ + Return True if Docker config ``auths`` already covers ``container_image``. + + A covering key is any of registry/namespace/repo, registry/namespace, or registry. + """ + if auths is None: + auths = _load_docker_config_auths() + return any(key in auths for key in _docker_auth_keys_covering_image(container_image)) + + +def docker_config_has_path_auth_for_image( + container_image: str, auths: Optional[Dict[str, Any]] = None +) -> bool: + """ + Return True if Docker config has namespace- or repository-scoped auth for ``container_image``. + + Registry-only keys (for example ``quay.io``) are ignored. Those often exist in the worker + template but do not grant access to a private ``from_index``; treating them as covering + would skip the namespace fallback that makes ``overwrite_from_index_token`` usable. + """ + if auths is None: + auths = _load_docker_config_auths() + keys = _docker_auth_keys_covering_image(container_image) + # keys are [repo, namespace, registry] when namespaced; drop the registry-only key. + path_keys = keys[:-1] if len(keys) > 1 else [] + return any(key in auths for key in path_keys) + + +def get_images_needing_overwrite_token( + from_index: Optional[str], images: List[str] +) -> List[str]: + """ + Return images that should receive ``overwrite_from_index_token`` during resolve/pull. + + An image is included only when: + + * it shares a resolvable registry with ``from_index``, + * it shares a namespace with ``from_index`` when either image has a namespace + (avoids applying a write-only index token to unrelated public repos on the + same registry), and + * the worker Docker config has no namespace- or repository-scoped auth for that image + (registry-only keys such as ``quay.io`` do not count; they often cannot pull private + images and must not suppress the overwrite token) + + This avoids overriding broader template credentials (for example ``quay.io/namespace``) + with a write-only index token, while still allowing the overwrite token to pull private + same-namespace images (bundles or FBC fragments) when no other credentials exist. + """ + if not from_index or not images: + return [] + + from_index_image = ImageName.parse(from_index) + from_index_registry = from_index_image.registry + if not from_index_registry: + return [] + + auths = _load_docker_config_auths() + images_needing_token: List[str] = [] + for image in images: + image_name = ImageName.parse(image) + if image_name.registry != from_index_registry: + continue + # When namespaces are present, require a match so public images under other + # namespaces on the same registry keep anonymous/template auth. + if from_index_image.namespace or image_name.namespace: + if from_index_image.namespace != image_name.namespace: + continue + if docker_config_has_path_auth_for_image(image, auths): + log.debug( + 'Not applying overwrite_from_index_token to %s; Docker config already has ' + 'path-scoped credentials', + image, + ) + continue + images_needing_token.append(image) + + return images_needing_token + + @contextmanager def set_registry_token( - token: Optional[str], container_image: Optional[str], append: bool = False + token: Optional[str], + container_image: Optional[Union[str, List[str]]], + append: bool = False, ) -> Generator: """ - Configure authentication for the image identified by ``container_image``. + Configure authentication for the image(s) identified by ``container_image``. The token is written to ``~/.docker/config.json`` under the most specific ``auths`` key that - container runtimes reliably match for that pull specification: + container runtimes reliably match for each pull specification: - * ``registry/namespace/repo`` when ``container_image`` includes a namespace + * ``registry/namespace/repo`` when the image includes a namespace * ``registry`` when the image has no namespace (for example, ``localhost:5000/myimage:tag``) - If the pull specification does not contain a resolvable registry, :exc:`IIBError` is raised. + If a pull specification does not contain a resolvable registry, :exc:`IIBError` is raised. Broader credentials already present in the Docker configuration, such as registry- or namespace-level entries for the same host, are not modified. Only the scoped ``auths`` key - derived from ``container_image`` is set or overwritten. + derived from each image is set or overwritten. When the worker config has no + namespace/repository-scoped auth for an image (registry-only keys do not count), the + namespace key is also set so private images that are only reachable via + ``overwrite_from_index_token`` can be pulled without stamping registry-level credentials. On exit, the Docker configuration is reset to its pre-request state via :func:`reset_docker_config`. If ``token`` or ``container_image`` is falsy, this context manager does nothing. :param str token: the token in the format of ``username:password`` - :param str container_image: the pull specification of the image to authenticate to. Used to - determine which ``auths`` entry receives ``token``. + :param container_image: the pull specification of the image to authenticate to, or a list of + pull specifications. Each value determines which ``auths`` entry receives ``token``. :param bool append: when ``True``, start from the current ``~/.docker/config.json`` (if it exists) before applying the scoped token. This preserves unrelated ``auths`` entries and is the preferred mode for ``overwrite_from_index`` callers that must override credentials - for a single index image without disturbing other registry configuration. When ``False``, - only the scoped ``auths`` entry and the worker template are merged. + for one or more images without disturbing other registry configuration. When ``False``, + only the scoped ``auths`` entries and the worker template are merged. :return: None :rtype: None - :raises IIBError: if the pull specification does not contain a resolvable registry. + :raises IIBError: if a pull specification does not contain a resolvable registry. """ if not token: log.debug( @@ -643,14 +773,20 @@ def set_registry_token( return if not container_image: - log.debug('Not changing the Docker configuration since no from_index was provided') + log.debug('Not changing the Docker configuration since no container image was provided') + yield + + return + + images = [container_image] if isinstance(container_image, str) else list(container_image) + if not images: + log.debug('Not changing the Docker configuration since no container image was provided') yield return encoded_token = base64.b64encode(token.encode('utf-8')).decode('utf-8') auth_entry = {'auth': encoded_token} - auth_key = _docker_auth_key_for_image(container_image) registry_auths: Dict[str, Any] = {'auths': {}} if append: @@ -665,8 +801,32 @@ def set_registry_token( log.debug('Docker config will be updated') - log.debug('Setting the override token for the image %s', auth_key) - registry_auths['auths'].update({auth_key: auth_entry}) + # Snapshot worker/template auths before we add overwrite keys. Used to decide whether a + # namespace-level fallback is needed when no path-scoped credentials exist yet. + existing_auths = _load_docker_config_auths() + + for image in images: + auth_key = _docker_auth_key_for_image(image) + log.debug('Setting the override token for the image %s', auth_key) + registry_auths.setdefault('auths', {}).update({auth_key: auth_entry}) + + # When from_index (or another image) is only reachable via overwrite_from_index_token + # and docker config has no namespace/repo auth, also set the namespace key. Repo-only + # auth is not always matched reliably; namespace fallback makes the token usable + # without stamping registry-level creds that would override other namespace entries. + # Registry-only template keys (e.g. quay.io) do not count — they often cannot pull a + # private from_index (case: overwrite token is the only usable credential). + if not docker_config_has_path_auth_for_image(image, existing_auths): + image_name = ImageName.parse(image) + if image_name.registry and image_name.namespace: + namespace_key = f'{image_name.registry}/{image_name.namespace}' + if namespace_key != auth_key: + log.debug( + 'No path-scoped Docker auth for %s; also setting overwrite token for %s', + image, + namespace_key, + ) + registry_auths['auths'].update({namespace_key: auth_entry}) with set_registry_auths(registry_auths): yield @@ -1097,7 +1257,7 @@ def get_index_image_info( if not from_index: return result - with set_registry_token(overwrite_from_index_token, from_index): + with set_registry_token(overwrite_from_index_token, from_index, append=True): from_index_resolved = get_resolved_image(from_index) result['arches'] = get_image_arches(from_index_resolved) result['ocp_version'] = ( diff --git a/tests/test_workers/test_tasks/test_build.py b/tests/test_workers/test_tasks/test_build.py index c8e001e4e..addf44d45 100644 --- a/tests/test_workers/test_tasks/test_build.py +++ b/tests/test_workers/test_tasks/test_build.py @@ -857,6 +857,160 @@ def side_effect(*args, base_dir, **kwargs): mock_dep_b.assert_not_called() +@mock.patch('iib.workers.tasks.build._cleanup') +@mock.patch('iib.workers.tasks.build.verify_labels') +@mock.patch('iib.workers.tasks.build.prepare_request_for_build') +@mock.patch('iib.workers.tasks.build._update_index_image_build_state') +@mock.patch('iib.workers.tasks.build.opm_index_add') +@mock.patch('iib.workers.tasks.build._build_image') +@mock.patch('iib.workers.tasks.build._push_image') +@mock.patch('iib.workers.tasks.build._update_index_image_pull_spec') +@mock.patch('iib.workers.tasks.build.set_request_state') +@mock.patch('iib.workers.tasks.build._create_and_push_manifest_list') +@mock.patch('iib.workers.tasks.build.get_resolved_bundles') +@mock.patch('iib.workers.tasks.build._add_label_to_index') +@mock.patch('iib.workers.tasks.build._get_present_bundles') +@mock.patch('iib.workers.tasks.build.add_max_ocp_version_property') +@mock.patch('iib.workers.tasks.build.get_images_needing_overwrite_token') +@mock.patch('iib.workers.tasks.build.set_registry_token') +@mock.patch('iib.workers.tasks.build.is_image_fbc', return_value=False) +@mock.patch('iib.workers.tasks.opm_operations.Opm.set_opm_version') +def test_handle_add_request_skips_overwrite_token_when_bundle_has_covering_auth( + mock_sov, + mock_iifbc, + mock_srt, + mock_gbn, + mock_amovp, + mock_gpb, + mock_alti, + mock_grb, + mock_capml, + mock_srs, + mock_uiips, + mock_pi, + mock_bi, + mock_oia, + mock_uiibs, + mock_prfb, + mock_vl, + mock_cleanup, +): + """When Docker config already covers the bundle, do not apply overwrite token to it.""" + from_index = 'quay.io/ns/comm-pending413:v4.13' + bundle = 'quay.io/ns/ack-controller:1.10.2' + overwrite_from_index_token = 'user:pass' + from_index_resolved = 'quay.io/ns/comm-pending413@sha256:bcdefg' + + mock_prfb.return_value = { + 'arches': {'amd64'}, + 'binary_image': 'binary-image:latest', + 'binary_image_resolved': 'binary-image@sha256:abcdef', + 'from_index_resolved': from_index_resolved, + 'ocp_version': 'v4.13', + 'distribution_scope': 'prod', + } + mock_grb.return_value = ['quay.io/ns/ack-controller@sha256:123'] + mock_gpb.return_value = [], [] + mock_capml.return_value = 'quay.io/namespace/some-image:3' + mock_gbn.return_value = [] # covering auth exists + mock_srt.return_value.__enter__ = mock.Mock(return_value=None) + mock_srt.return_value.__exit__ = mock.Mock(return_value=None) + + build.handle_add_request( + [bundle], + 3, + 'binary-image:latest', + from_index, + None, + None, + None, + False, + False, + overwrite_from_index_token, + ) + + mock_gbn.assert_called_once_with(from_index, [bundle]) + assert mock.call(overwrite_from_index_token, [bundle], append=True) not in mock_srt.call_args_list + assert mock.call(overwrite_from_index_token, [], append=True) not in mock_srt.call_args_list + assert mock.call(overwrite_from_index_token, None, append=True) not in mock_srt.call_args_list + + +@mock.patch('iib.workers.tasks.build._cleanup') +@mock.patch('iib.workers.tasks.build.verify_labels') +@mock.patch('iib.workers.tasks.build.prepare_request_for_build') +@mock.patch('iib.workers.tasks.build._update_index_image_build_state') +@mock.patch('iib.workers.tasks.build.opm_index_add') +@mock.patch('iib.workers.tasks.build._build_image') +@mock.patch('iib.workers.tasks.build._push_image') +@mock.patch('iib.workers.tasks.build._update_index_image_pull_spec') +@mock.patch('iib.workers.tasks.build.set_request_state') +@mock.patch('iib.workers.tasks.build._create_and_push_manifest_list') +@mock.patch('iib.workers.tasks.build.get_resolved_bundles') +@mock.patch('iib.workers.tasks.build._add_label_to_index') +@mock.patch('iib.workers.tasks.build._get_present_bundles') +@mock.patch('iib.workers.tasks.build.add_max_ocp_version_property') +@mock.patch('iib.workers.tasks.build.get_images_needing_overwrite_token') +@mock.patch('iib.workers.tasks.build.set_registry_token') +@mock.patch('iib.workers.tasks.build.is_image_fbc', return_value=False) +@mock.patch('iib.workers.tasks.opm_operations.Opm.set_opm_version') +def test_handle_add_request_applies_overwrite_token_when_bundle_lacks_covering_auth( + mock_sov, + mock_iifbc, + mock_srt, + mock_gbn, + mock_amovp, + mock_gpb, + mock_alti, + mock_grb, + mock_capml, + mock_srs, + mock_uiips, + mock_pi, + mock_bi, + mock_oia, + mock_uiibs, + mock_prfb, + mock_vl, + mock_cleanup, +): + """When no Docker config covers the bundle, apply overwrite token for same-registry pull.""" + from_index = 'quay.io/ns/comm-pending413:v4.13' + bundle = 'quay.io/ns/ack-controller:1.10.2' + overwrite_from_index_token = 'user:pass' + from_index_resolved = 'quay.io/ns/comm-pending413@sha256:bcdefg' + + mock_prfb.return_value = { + 'arches': {'amd64'}, + 'binary_image': 'binary-image:latest', + 'binary_image_resolved': 'binary-image@sha256:abcdef', + 'from_index_resolved': from_index_resolved, + 'ocp_version': 'v4.13', + 'distribution_scope': 'prod', + } + mock_grb.return_value = ['quay.io/ns/ack-controller@sha256:123'] + mock_gpb.return_value = [], [] + mock_capml.return_value = 'quay.io/namespace/some-image:3' + mock_gbn.return_value = [bundle] # no covering auth + mock_srt.return_value.__enter__ = mock.Mock(return_value=None) + mock_srt.return_value.__exit__ = mock.Mock(return_value=None) + + build.handle_add_request( + [bundle], + 3, + 'binary-image:latest', + from_index, + None, + None, + None, + False, + False, + overwrite_from_index_token, + ) + + mock_gbn.assert_called_once_with(from_index, [bundle]) + mock_srt.assert_any_call(overwrite_from_index_token, [bundle], append=True) + + @mock.patch('iib.workers.tasks.build.update_request') @mock.patch('iib.workers.tasks.build._cleanup') @mock.patch('iib.workers.tasks.build.run_cmd') diff --git a/tests/test_workers/test_tasks/test_build_fbc_operations.py b/tests/test_workers/test_tasks/test_build_fbc_operations.py index 54e72b4e0..820efc61e 100644 --- a/tests/test_workers/test_tasks/test_build_fbc_operations.py +++ b/tests/test_workers/test_tasks/test_build_fbc_operations.py @@ -330,6 +330,130 @@ def test_handle_fbc_operation_request_with_overwrite_token( ) +@mock.patch('iib.workers.tasks.build_fbc_operations._update_index_image_pull_spec') +@mock.patch('iib.workers.tasks.build_fbc_operations._create_and_push_manifest_list') +@mock.patch('iib.workers.tasks.build_fbc_operations._push_image') +@mock.patch('iib.workers.tasks.build_fbc_operations._build_image') +@mock.patch('iib.workers.tasks.build_fbc_operations._add_label_to_index') +@mock.patch('iib.workers.tasks.build_fbc_operations.opm_registry_add_fbc_fragment') +@mock.patch('iib.workers.tasks.build_fbc_operations._update_index_image_build_state') +@mock.patch('iib.workers.tasks.build_fbc_operations.prepare_request_for_build') +@mock.patch('iib.workers.tasks.build_fbc_operations.get_images_needing_overwrite_token') +@mock.patch('iib.workers.tasks.build_fbc_operations.set_registry_token') +@mock.patch('iib.workers.tasks.build_fbc_operations.get_resolved_image') +@mock.patch('iib.workers.tasks.build_fbc_operations.set_request_state') +@mock.patch('iib.workers.tasks.build_fbc_operations._cleanup') +@mock.patch('iib.workers.tasks.opm_operations.Opm.set_opm_version') +def test_handle_fbc_operation_skips_overwrite_token_when_fragment_has_covering_auth( + mock_sov, + mock_cleanup, + mock_srs, + mock_gri, + mock_srt, + mock_gin, + mock_prfb, + mock_uiibs, + mock_oraff, + mock_alti, + mock_bi, + mock_pi, + mock_cpml, + mock_uiips, +): + """When Docker config already covers the fragment, do not apply overwrite token.""" + request_id = 10 + from_index = 'quay.io/ns/index:v4.16' + fragment = 'quay.io/ns/fbc-fragment:latest' + overwrite_from_index_token = 'user:password' + + mock_prfb.return_value = { + 'arches': {'amd64'}, + 'binary_image': 'binary-image:latest', + 'binary_image_resolved': 'binary-image@sha256:abcdef', + 'from_index_resolved': 'quay.io/ns/index@sha256:bcdefg', + 'ocp_version': 'v4.16', + 'distribution_scope': 'prod', + } + mock_gri.return_value = 'quay.io/ns/fbc-fragment@sha256:frag1' + mock_gin.return_value = [] # covering auth exists + mock_srt.return_value.__enter__ = mock.Mock(return_value=None) + mock_srt.return_value.__exit__ = mock.Mock(return_value=None) + + build_fbc_operations.handle_fbc_operation_request( + request_id=request_id, + fbc_fragments=[fragment], + from_index=from_index, + binary_image='binary-image:latest', + overwrite_from_index_token=overwrite_from_index_token, + ) + + mock_gin.assert_called_once_with(from_index, [fragment]) + mock_srt.assert_not_called() + mock_gri.assert_called_once_with(fragment) + + +@mock.patch('iib.workers.tasks.build_fbc_operations._update_index_image_pull_spec') +@mock.patch('iib.workers.tasks.build_fbc_operations._create_and_push_manifest_list') +@mock.patch('iib.workers.tasks.build_fbc_operations._push_image') +@mock.patch('iib.workers.tasks.build_fbc_operations._build_image') +@mock.patch('iib.workers.tasks.build_fbc_operations._add_label_to_index') +@mock.patch('iib.workers.tasks.build_fbc_operations.opm_registry_add_fbc_fragment') +@mock.patch('iib.workers.tasks.build_fbc_operations._update_index_image_build_state') +@mock.patch('iib.workers.tasks.build_fbc_operations.prepare_request_for_build') +@mock.patch('iib.workers.tasks.build_fbc_operations.get_images_needing_overwrite_token') +@mock.patch('iib.workers.tasks.build_fbc_operations.set_registry_token') +@mock.patch('iib.workers.tasks.build_fbc_operations.get_resolved_image') +@mock.patch('iib.workers.tasks.build_fbc_operations.set_request_state') +@mock.patch('iib.workers.tasks.build_fbc_operations._cleanup') +@mock.patch('iib.workers.tasks.opm_operations.Opm.set_opm_version') +def test_handle_fbc_operation_applies_overwrite_token_when_fragment_lacks_covering_auth( + mock_sov, + mock_cleanup, + mock_srs, + mock_gri, + mock_srt, + mock_gin, + mock_prfb, + mock_uiibs, + mock_oraff, + mock_alti, + mock_bi, + mock_pi, + mock_cpml, + mock_uiips, +): + """When no Docker config covers the fragment, apply overwrite token for same-registry pull.""" + request_id = 10 + from_index = 'quay.io/ns/index:v4.16' + fragment = 'quay.io/ns/fbc-fragment:latest' + overwrite_from_index_token = 'user:password' + + mock_prfb.return_value = { + 'arches': {'amd64'}, + 'binary_image': 'binary-image:latest', + 'binary_image_resolved': 'binary-image@sha256:abcdef', + 'from_index_resolved': 'quay.io/ns/index@sha256:bcdefg', + 'ocp_version': 'v4.16', + 'distribution_scope': 'prod', + } + mock_gri.return_value = 'quay.io/ns/fbc-fragment@sha256:frag1' + mock_gin.return_value = [fragment] # no covering auth + mock_srt.return_value.__enter__ = mock.Mock(return_value=None) + mock_srt.return_value.__exit__ = mock.Mock(return_value=None) + + build_fbc_operations.handle_fbc_operation_request( + request_id=request_id, + fbc_fragments=[fragment], + from_index=from_index, + binary_image='binary-image:latest', + overwrite_from_index_token=overwrite_from_index_token, + ) + + mock_gin.assert_called_once_with(from_index, [fragment]) + mock_srt.assert_called_once_with(overwrite_from_index_token, [fragment], append=True) + mock_gri.assert_called_once_with(fragment) + + @mock.patch('iib.workers.tasks.build_fbc_operations._update_index_image_pull_spec') @mock.patch('iib.workers.tasks.build_fbc_operations._create_and_push_manifest_list') @mock.patch('iib.workers.tasks.build_fbc_operations._push_image') diff --git a/tests/test_workers/test_tasks/test_opm_operations.py b/tests/test_workers/test_tasks/test_opm_operations.py index 36327859a..be6261ee1 100644 --- a/tests/test_workers/test_tasks/test_opm_operations.py +++ b/tests/test_workers/test_tasks/test_opm_operations.py @@ -462,6 +462,8 @@ def test_opm_registry_add( @pytest.mark.parametrize('overwrite_csv', (True, False)) @pytest.mark.parametrize('container_tool', (None, 'podwoman')) @pytest.mark.parametrize('graph_update_mode', (None, 'semver-skippatch')) +@mock.patch('iib.workers.tasks.utils.get_images_needing_overwrite_token', return_value=[]) +@mock.patch('iib.workers.tasks.utils.set_registry_token') @mock.patch('iib.workers.tasks.opm_operations.create_dockerfile') @mock.patch('iib.workers.tasks.opm_operations.opm_migrate') @mock.patch('iib.workers.tasks.opm_operations._opm_registry_add') @@ -475,6 +477,8 @@ def test_opm_registry_add_fbc( mock_ora, mock_om, mock_ogd, + mock_srt, + mock_gin, from_index, bundles, overwrite_csv, @@ -490,6 +494,8 @@ def test_opm_registry_add_fbc( mock_gid.return_value = index_db_file mock_om.return_value = (fbc_dir, cache_dir) mock_iifbc.return_value = is_fbc + mock_srt.return_value.__enter__ = mock.Mock(return_value=None) + mock_srt.return_value.__exit__ = mock.Mock(return_value=None) opm_operations.opm_registry_add_fbc( base_dir=tmpdir, @@ -499,8 +505,11 @@ def test_opm_registry_add_fbc( graph_update_mode=graph_update_mode, overwrite_csv=overwrite_csv, container_tool=container_tool, + overwrite_from_index_token='user:pass', ) + mock_gin.assert_called_once_with(from_index, bundles) + mock_srt.assert_not_called() mock_ora.assert_called_once_with( base_dir=tmpdir, index_db=index_db_file, @@ -833,9 +842,13 @@ def test_generate_cache_locally_failed( @mock.patch('iib.workers.tasks.opm_operations.get_catalog_dir') @mock.patch('iib.workers.tasks.opm_operations.verify_operators_exists') @mock.patch('iib.workers.tasks.opm_operations.extract_fbc_fragment') +@mock.patch('iib.workers.tasks.utils.get_images_needing_overwrite_token', return_value=[]) +@mock.patch('iib.workers.tasks.utils.set_registry_token') @mock.patch('iib.workers.tasks.opm_operations.set_request_state') def test_opm_registry_add_fbc_fragment( mock_srs, + mock_srt, + mock_gin, mock_eff, mock_voe, mock_gcr, @@ -896,6 +909,8 @@ def test_opm_registry_add_fbc_fragment( 10, tmpdir, from_index, binary_image, [fbc_fragment], None ) + mock_gin.assert_called_once_with(from_index, [fbc_fragment]) + mock_srt.assert_not_called() mock_eff.assert_called_with(temp_dir=tmpdir, fbc_fragment=fbc_fragment, fragment_index=0) mock_voe.assert_called_with( from_index=from_index, @@ -991,11 +1006,13 @@ def test_verify_operator_exists( @pytest.mark.parametrize('overwrite_csv', (True, False)) @pytest.mark.parametrize('container_tool', (None, 'podwoman')) @pytest.mark.parametrize('graph_update_mode', (None, 'semver')) +@mock.patch('iib.workers.tasks.utils.get_images_needing_overwrite_token', return_value=[]) @mock.patch('iib.workers.tasks.utils.set_registry_token') @mock.patch('iib.workers.tasks.utils.run_cmd') def test_opm_index_add( mock_run_cmd, mock_srt, + mock_gin, from_index, bundles, overwrite_csv, @@ -1024,8 +1041,10 @@ def test_opm_index_add( if from_index: assert '--from-index' in opm_args assert from_index in opm_args + mock_gin.assert_called_once_with(from_index, bundles) else: assert '--from-index' not in opm_args + mock_gin.assert_not_called() if overwrite_csv: assert '--overwrite-latest' in opm_args else: @@ -1042,7 +1061,10 @@ def test_opm_index_add( assert '--mode' not in opm_args assert "--enable-alpha" in opm_args - mock_srt.assert_called_once_with('user:pass', from_index, append=True) + if from_index: + mock_srt.assert_called_once_with('user:pass', [from_index], append=True) + else: + mock_srt.assert_not_called() @pytest.mark.parametrize('container_tool', (None, 'podwoman')) diff --git a/tests/test_workers/test_tasks/test_utils.py b/tests/test_workers/test_tasks/test_utils.py index fe600e22e..fcc98990b 100644 --- a/tests/test_workers/test_tasks/test_utils.py +++ b/tests/test_workers/test_tasks/test_utils.py @@ -96,6 +96,7 @@ def test_set_registry_token( '4gU2ltcHNvbgo=' ) }, + 'registry.redhat.io/ns': {'auth': 'dXNlcjpwYXNz'}, 'registry.redhat.io/ns/repo': {'auth': 'dXNlcjpwYXNz'}, } } @@ -103,7 +104,10 @@ def test_set_registry_token( mock_open.assert_called_once_with('/home/iib-worker/.docker/config.json', 'w') assert mock_open.call_count == 1 assert mock_json_dump.call_args[0][0] == { - 'auths': {'registry.redhat.io/ns/repo': {'auth': 'dXNlcjpwYXNz'}} + 'auths': { + 'registry.redhat.io/ns': {'auth': 'dXNlcjpwYXNz'}, + 'registry.redhat.io/ns/repo': {'auth': 'dXNlcjpwYXNz'}, + } } mock_rdc.assert_called_once_with() @@ -304,6 +308,221 @@ def test_set_registry_token_append_overwrites_repo_auth( mock_rdc.assert_called_once_with() +@mock.patch('os.path.expanduser') +@mock.patch('os.path.exists', return_value=True) +@mock.patch('iib.workers.tasks.utils.open') +@mock.patch('iib.workers.tasks.utils.json.dump') +@mock.patch('iib.workers.tasks.utils.reset_docker_config') +def test_set_registry_token_namespace_fallback_when_no_path_auth( + mock_rdc, + mock_json_dump, + mock_open, + mock_exists, + mock_expanduser, +): + """When from_index has no namespace/repo auth, also set namespace key (case 4).""" + mock_expanduser.return_value = '/home/iib-worker' + # Registry-only quay.io must not suppress namespace fallback — it often cannot pull a + # private from_index that is only reachable via overwrite_from_index_token. + mock_open.side_effect = mock.mock_open( + read_data=r'{"auths": {"quay.io": {"auth": "cXVheabcdef"}}}' + ) + + with utils.set_registry_token( + 'user:pass', + 'quay.io/ns/certified-index:v4.14', + append=True, + ): + pass + + assert mock_json_dump.call_args[0][0]['auths'] == { + 'quay.io': {'auth': 'cXVheabcdef'}, + 'quay.io/ns': {'auth': 'dXNlcjpwYXNz'}, + 'quay.io/ns/certified-index': {'auth': 'dXNlcjpwYXNz'}, + } + mock_rdc.assert_called_once_with() + + +@mock.patch('os.path.expanduser') +@mock.patch('os.path.exists', return_value=True) +@mock.patch('iib.workers.tasks.utils.open') +@mock.patch('iib.workers.tasks.utils.json.dump') +@mock.patch('iib.workers.tasks.utils.reset_docker_config') +def test_set_registry_token_no_namespace_fallback_when_namespace_auth_exists( + mock_rdc, + mock_json_dump, + mock_open, + mock_exists, + mock_expanduser, +): + """Namespace template auth must be preserved (case 3) — do not overwrite it.""" + mock_expanduser.return_value = '/home/iib-worker' + mock_open.side_effect = mock.mock_open( + read_data=( + r'{"auths": {"quay.io/ns": {"auth": "bmFtZXNqwertyui"}, ' + r'"quay.io": {"auth": "cXVheabcdef"}}}' + ) + ) + + with utils.set_registry_token( + 'user:pass', + 'quay.io/ns/community-index:v4.13', + append=True, + ): + pass + + assert mock_json_dump.call_args[0][0]['auths'] == { + 'quay.io': {'auth': 'cXVheabcdef'}, + 'quay.io/ns': {'auth': 'bmFtZXNqwertyui'}, + 'quay.io/ns/community-index': {'auth': 'dXNlcjpwYXNz'}, + } + mock_rdc.assert_called_once_with() + + +@mock.patch('os.path.expanduser') +@mock.patch('os.path.exists', return_value=True) +@mock.patch('iib.workers.tasks.utils.open') +@mock.patch('iib.workers.tasks.utils.json.dump') +@mock.patch('iib.workers.tasks.utils.reset_docker_config') +def test_set_registry_token_multiple_images( + mock_rdc, + mock_json_dump, + mock_open, + mock_exists, + mock_expanduser, +): + mock_expanduser.return_value = '/home/iib-worker' + mock_open.side_effect = mock.mock_open( + read_data=r'{"auths": {"quay.io": {"auth": "cXVheabcdef"}}}' + ) + + with utils.set_registry_token( + 'user:pass', + [ + 'quay.io/ns/bundle-a:1.0', + 'quay.io/ns/bundle-b:2.0', + ], + append=True, + ): + pass + + # Registry-only covering does not suppress namespace fallback for either image. + assert mock_json_dump.call_args[0][0]['auths'] == { + 'quay.io': {'auth': 'cXVheabcdef'}, + 'quay.io/ns': {'auth': 'dXNlcjpwYXNz'}, + 'quay.io/ns/bundle-a': {'auth': 'dXNlcjpwYXNz'}, + 'quay.io/ns/bundle-b': {'auth': 'dXNlcjpwYXNz'}, + } + mock_rdc.assert_called_once_with() + + +@pytest.mark.parametrize( + 'container_image, expected_keys', + ( + pytest.param( + 'quay.io/ns/ack-controller:1.10.2', + [ + 'quay.io/ns/ack-controller', + 'quay.io/ns', + 'quay.io', + ], + id='namespaced-repo', + ), + pytest.param( + 'localhost:5000/myimage:tag', + ['localhost:5000'], + id='registry-without-namespace', + ), + ), +) +def test_docker_auth_keys_covering_image(container_image, expected_keys): + assert utils._docker_auth_keys_covering_image(container_image) == expected_keys + + +@pytest.mark.parametrize( + 'auths, container_image, expected', + ( + pytest.param( + {'quay.io/ns': {'auth': 'abc'}}, + 'quay.io/ns/ack-controller:1.10.2', + True, + id='namespace-covers-bundle', + ), + pytest.param( + {'quay.io': {'auth': 'abc'}}, + 'quay.io/ns/ack-controller:1.10.2', + True, + id='registry-covers-bundle', + ), + pytest.param( + {'quay.io/ns/comm-pending413': {'auth': 'abc'}}, + 'quay.io/ns/ack-controller:1.10.2', + False, + id='other-repo-does-not-cover', + ), + pytest.param( + {}, + 'quay.io/ns/ack-controller:1.10.2', + False, + id='empty-auths', + ), + ), +) +def test_docker_config_has_auth_for_image(auths, container_image, expected): + assert utils.docker_config_has_auth_for_image(container_image, auths) is expected + + +@mock.patch('iib.workers.tasks.utils._load_docker_config_auths') +def test_get_images_needing_overwrite_token(mock_load_auths): + from_index = 'quay.io/ns/comm-pending413:v4.13' + covered_image = 'quay.io/ns/ack-controller:1.10.2' + uncovered_image = 'quay.io/ns/other-bundle:1.0' + other_namespace_image = 'quay.io/public/bundle:1.0' + other_registry_image = 'registry.redhat.io/org/bundle:1.0' + + # Repo-level auth covers only one image + mock_load_auths.return_value = { + 'quay.io/ns/ack-controller': {'auth': 'repo_token'}, + } + + assert utils.get_images_needing_overwrite_token( + from_index, + [covered_image, uncovered_image, other_namespace_image, other_registry_image], + ) == [uncovered_image] + + # Namespace-level auth covers all images under quay.io/ns + mock_load_auths.return_value = { + 'quay.io/ns': {'auth': 'namespace_token'}, + } + assert ( + utils.get_images_needing_overwrite_token( + from_index, + [covered_image, uncovered_image, other_namespace_image, other_registry_image], + ) + == [] + ) + + # Registry-only auth does not cover namespaced images (case 3 / overwrite token path) + mock_load_auths.return_value = { + 'quay.io': {'auth': 'registry_token'}, + } + assert utils.get_images_needing_overwrite_token( + from_index, + [covered_image, uncovered_image, other_namespace_image, other_registry_image], + ) == [covered_image, uncovered_image] + + # No covering auth: only same-namespace images need the overwrite token + # (not other namespaces on the same registry — avoids breaking public pulls) + mock_load_auths.return_value = {} + assert utils.get_images_needing_overwrite_token( + from_index, + [covered_image, uncovered_image, other_namespace_image, other_registry_image], + ) == [covered_image, uncovered_image] + + assert utils.get_images_needing_overwrite_token(None, [covered_image]) == [] + assert utils.get_images_needing_overwrite_token(from_index, []) == [] + + @mock.patch('os.remove') def test_set_registry_token_null_token(mock_remove): with utils.set_registry_token(None, 'quay.io/ns/repo:latest'): @@ -320,6 +539,14 @@ def test_set_container_image_null(mock_remove): mock_remove.assert_not_called() +@mock.patch('os.remove') +def test_set_registry_token_empty_image_list(mock_remove): + with utils.set_registry_token('token_username:token_pass', []): + pass + + mock_remove.assert_not_called() + + @mock.patch('iib.workers.tasks.utils.subprocess.run') def test_run_cmd(mock_sub_run): mock_rv = mock.Mock()