From f0936e79f45917fbc32836f271df5b84dddfd318 Mon Sep 17 00:00:00 2001 From: Tulsi Chandwani Date: Wed, 29 Jul 2026 15:56:34 -0500 Subject: [PATCH] Scope overwrite_from_index_token to same-namespace images without path-scoped Docker auth Fixes CLOUDDST-32419 and CLOUDDST-32824 by applying the overwrite token only where worker Docker config cannot already pull the image, and by avoiding blanket token stamping that broke unrelated private fragments and namespace template credentials. Co-authored-by: Cursor --- iib/workers/tasks/build.py | 25 +- iib/workers/tasks/build_fbc_operations.py | 22 +- iib/workers/tasks/opm_operations.py | 94 +++++-- iib/workers/tasks/utils.py | 192 +++++++++++++-- tests/test_workers/test_tasks/test_build.py | 154 ++++++++++++ .../test_tasks/test_build_fbc_operations.py | 124 ++++++++++ .../test_tasks/test_opm_operations.py | 24 +- tests/test_workers/test_tasks/test_utils.py | 229 +++++++++++++++++- 8 files changed, 818 insertions(+), 46 deletions(-) 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()