From 2ea5ad5594db85b8a77cdd12dadf5ee0479999ac Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Mon, 20 Apr 2026 23:36:55 +0200 Subject: [PATCH 1/3] feat: support multiple catalog index images for Extensions UI [RHIDP-12933] The Extensions UI currently only displays plugins from the primary CATALOG_INDEX_IMAGE. Organizations that maintain plugins across separate registries (e.g., community vs partner catalogs) have no way to surface all of them without rebuilding a single monolithic index. Introduce EXTRA_CATALOG_INDEX_IMAGES so that additional catalog index images can be listed (comma-separated). Each image's catalog entities are extracted to an isolated subdirectory under the extensions volume, making those plugins visible in the Extensions UI without affecting the primary index's dynamic-plugins.default.yaml. Ref: https://issues.redhat.com/browse/RHIDP-12933 Assisted-by: Claude --- docs/dynamic-plugins/installing-plugins.md | 26 + .../install-dynamic-plugins.py | 126 ++++- .../test_install-dynamic-plugins.py | 462 ++++++++++++++++++ 3 files changed, 611 insertions(+), 3 deletions(-) diff --git a/docs/dynamic-plugins/installing-plugins.md b/docs/dynamic-plugins/installing-plugins.md index a32ed388ae..8be7e00f7d 100644 --- a/docs/dynamic-plugins/installing-plugins.md +++ b/docs/dynamic-plugins/installing-plugins.md @@ -85,6 +85,32 @@ The extraction destination is governed by the `CATALOG_ENTITIES_EXTRACT_DIR` env **Note:** If the catalog index image does not contain the `catalog-entities/extensions` directory, a warning will be printed but the extraction of `dynamic-plugins.default.yaml` will still succeed. +### Using extra catalog index images + +In addition to the primary `CATALOG_INDEX_IMAGE`, you can configure additional catalog index images using the `EXTRA_CATALOG_INDEX_IMAGES` environment variable. These extra images provide catalog entities that are made visible in the Extensions UI, but they do **not** contribute `dynamic-plugins.default.yaml` files (only the primary `CATALOG_INDEX_IMAGE` provides default plugin configurations). + +The `EXTRA_CATALOG_INDEX_IMAGES` environment variable accepts a comma-separated list of entries. Each entry can be either a plain image reference or use the `name=` format to choose a simpler sub-directory name: + +``` +# Auto-derived subdirectory names +EXTRA_CATALOG_INDEX_IMAGES=quay.io/rhdh-community/plugin-catalog-index:1.10,quay.io/partner/catalog:latest + +# Explicit subdirectory names +EXTRA_CATALOG_INDEX_IMAGES=community=quay.io/rhdh-community/plugin-catalog-index:1.10,partner=quay.io/partner/catalog:latest + +# Mixed +EXTRA_CATALOG_INDEX_IMAGES=community=quay.io/rhdh-community/plugin-catalog-index:1.10,quay.io/partner/catalog:latest +``` + +Each image's catalog entities are extracted to a separate subdirectory under the catalog entities parent directory: + +- With explicit name: `community=quay.io/rhdh-community/plugin-catalog-index:1.10` will be extracted to `/community/catalog-entities` +- Without name: `quay.io/partner/catalog:latest` will be extracted to `/quay.io_partner_catalog_latest/catalog-entities` (derived by replacing `/`, `:`, and `@` with `_`) + +If multiple entries map to the same subdirectory name, a warning is printed and the later entry overwrites the earlier one. + +**Note:** Extra catalog index images only make plugins visible in the Extensions UI. They do not provide default plugin configurations or enable automatic plugin installation. To install plugins from extra catalog index images, users must add and configure them explicitly in their dynamic plugins configuration file. + ## Installing External Dynamic Plugins RHDH supports external dynamic plugins, which are plugins not included in the core RHDH distribution. These plugins can be installed or uninstalled without rebuilding the RHDH application; only a restart is required to apply the changes. diff --git a/scripts/install-dynamic-plugins/install-dynamic-plugins.py b/scripts/install-dynamic-plugins/install-dynamic-plugins.py index 00100f556f..9e998cf3e3 100755 --- a/scripts/install-dynamic-plugins/install-dynamic-plugins.py +++ b/scripts/install-dynamic-plugins/install-dynamic-plugins.py @@ -40,7 +40,16 @@ Environment Variables: MAX_ENTRY_SIZE: Maximum size of a file in the archive (default: 20MB) SKIP_INTEGRITY_CHECK: Set to "true" to skip integrity check of remote packages - CATALOG_INDEX_IMAGE: OCI image reference for the plugin catalog index (e.g., quay.io/rhdh/plugin-catalog-index:1.9) + CATALOG_INDEX_IMAGE: OCI image reference for the primary plugin catalog index (e.g., quay.io/rhdh/plugin-catalog-index:1.9). + This is the only index from which dynamic-plugins.default.yaml is read. + EXTRA_CATALOG_INDEX_IMAGES: Comma-separated list of additional catalog index image references. + Each entry can be either a plain image reference or 'name=image_ref' to choose the subdirectory name. + Examples: + 'quay.io/rhdh-community/plugin-catalog-index:1.10' (subdirectory auto-derived) + 'community=quay.io/rhdh-community/plugin-catalog-index:1.10' (explicit subdirectory name) + When no name is given, the subdirectory is derived by replacing '/', '@', and ':' with '_'. + These images only provide catalog entities for the Extensions UI; + they do NOT contribute dynamic-plugins.default.yaml files. Configuration: The script expects the `dynamic-plugins.yaml` file to be present in the current directory and to contain the list of plugins to install along with their optional configuration. @@ -1135,6 +1144,111 @@ def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str, ca return default_plugins_file + +def extract_extra_catalog_index(catalog_index_image: str, subdirectory: str, catalog_entities_parent_dir: str) -> None: + """Extract catalog entities from an extra catalog index image. + + Unlike extract_catalog_index(), this does NOT look for dynamic-plugins.default.yaml. + Extra catalog index images only provide catalog entities for the Extensions UI. + + Args: + catalog_index_image: OCI image reference for the extra catalog index + subdirectory: Name of the subdirectory to extract into (e.g., 'community') + catalog_entities_parent_dir: Parent directory for catalog entities extraction + """ + print(f"\n======= Extracting extra catalog index '{subdirectory}' from {catalog_index_image}", flush=True) + + skopeo_path = shutil.which('skopeo') + if skopeo_path is None: + raise InstallException("EXTRA_CATALOG_INDEX_IMAGES is set but skopeo executable not found in PATH. Cannot extract extra catalog index.") + + resolved_image = resolve_image_reference(catalog_index_image) + + with tempfile.TemporaryDirectory() as tmp_dir: + image_url = resolved_image + if not image_url.startswith(DOCKER_PROTOCOL_PREFIX): + image_url = f'{DOCKER_PROTOCOL_PREFIX}{image_url}' + print("\t==> Copying extra catalog index image to local filesystem", flush=True) + local_dir = os.path.join(tmp_dir, 'catalog-index-oci') + + run_command( + [skopeo_path, 'copy', '--override-os=linux', '--override-arch=amd64', image_url, f'dir:{local_dir}'], + f"Failed to download extra catalog index image {resolved_image}" + ) + + manifest_path = os.path.join(local_dir, 'manifest.json') + if not os.path.isfile(manifest_path): + raise InstallException(f"manifest.json not found in extra catalog index image {catalog_index_image}") + + with open(manifest_path, 'r') as f: + manifest = json.load(f) + + catalog_index_temp_dir = os.path.join(tmp_dir, 'extracted') + os.makedirs(catalog_index_temp_dir, exist_ok=True) + + print("\t==> Extracting extra catalog index layers", flush=True) + _extract_catalog_index_layers(manifest, local_dir, catalog_index_temp_dir) + + subdirectory_parent = os.path.join(catalog_entities_parent_dir, subdirectory) + print(f"\t==> Extracting extensions catalog entities to {subdirectory_parent}", flush=True) + + extensions_dir_from_catalog_index = os.path.join(catalog_index_temp_dir, 'catalog-entities', 'extensions') + if not os.path.isdir(extensions_dir_from_catalog_index): + extensions_dir_from_catalog_index = os.path.join(catalog_index_temp_dir, 'catalog-entities', 'marketplace') + + if os.path.isdir(extensions_dir_from_catalog_index): + os.makedirs(subdirectory_parent, exist_ok=True) + catalog_entities_dest = os.path.join(subdirectory_parent, 'catalog-entities') + if os.path.exists(catalog_entities_dest): + shutil.rmtree(catalog_entities_dest, ignore_errors=True, onerror=None) + shutil.copytree(extensions_dir_from_catalog_index, catalog_entities_dest, dirs_exist_ok=True) + print(f"\t==> Successfully extracted extensions catalog entities from extra index image to {subdirectory_parent}", flush=True) + else: + print(f"\t==> WARNING: Extra catalog index image {catalog_index_image} does not have neither 'catalog-entities/extensions/' nor 'catalog-entities/marketplace/' directory", + flush=True) + +def image_ref_to_subdirectory(image_ref: str) -> str: + """Derive a subdirectory name from an image reference by replacing special characters with underscores.""" + return re.sub(r'[/:@]', '_', image_ref) + +def parse_extra_catalog_index_images(extra_images_str: str) -> list[tuple[str, str]]: + """Parse the EXTRA_CATALOG_INDEX_IMAGES environment variable value. + + Supports two formats per entry: + - 'name=image_ref': explicit subdirectory name (e.g., 'community=quay.io/rhdh-community/index:1.10') + - 'image_ref': subdirectory name derived by replacing '/', ':', '@' with '_' + + Args: + extra_images_str: Comma-separated list of entries + + Returns: + List of (subdirectory_name, image_ref) tuples. If multiple images map to the same + subdirectory name, a warning is printed and the last one wins (overwrites). + """ + result = [] + seen_names = {} + for entry in extra_images_str.split(","): + entry = entry.strip() + if not entry: + continue + if "=" in entry: + name, image_ref = entry.split("=", 1) + name = name.strip() + image_ref = image_ref.strip() + else: + image_ref = entry + name = image_ref_to_subdirectory(image_ref) + if not image_ref: + print(f"WARNING: Skipping EXTRA_CATALOG_INDEX_IMAGES entry with empty image reference: '{entry}'", flush=True) + continue + if name in seen_names: + print(f"WARNING: Extra catalog index image '{image_ref}' maps to subdirectory '{name}' " + f"which was already used by '{seen_names[name]}'. The previous extraction will be overwritten.", + flush=True) + seen_names[name] = image_ref + result.append((name, image_ref)) + return result + def pre_merge_oci_disabled_state( include_plugin_lists: list[tuple[str, list[dict]]], main_plugins: list[dict], @@ -1278,11 +1392,17 @@ def main(): # Extract catalog index if CATALOG_INDEX_IMAGE is set catalog_index_image = os.environ.get("CATALOG_INDEX_IMAGE", "") catalog_index_default_file = None + catalog_entities_parent_dir = os.environ.get("CATALOG_ENTITIES_EXTRACT_DIR", os.path.join(tempfile.gettempdir(), "extensions")) if catalog_index_image: - # default to a temporary directory if the env var is not set - catalog_entities_parent_dir = os.environ.get("CATALOG_ENTITIES_EXTRACT_DIR", os.path.join(tempfile.gettempdir(), "extensions")) catalog_index_default_file = extract_catalog_index(catalog_index_image, dynamic_plugins_root, catalog_entities_parent_dir) + # Extract extra catalog index images if EXTRA_CATALOG_INDEX_IMAGES is set + extra_catalog_index_images = os.environ.get("EXTRA_CATALOG_INDEX_IMAGES", "") + if extra_catalog_index_images: + extra_entries = parse_extra_catalog_index_images(extra_catalog_index_images) + for name, image_ref in extra_entries: + extract_extra_catalog_index(image_ref, name, catalog_entities_parent_dir) + skip_integrity_check = os.environ.get("SKIP_INTEGRITY_CHECK", "").lower() == "true" dynamic_plugins_file = 'dynamic-plugins.yaml' diff --git a/scripts/install-dynamic-plugins/test_install-dynamic-plugins.py b/scripts/install-dynamic-plugins/test_install-dynamic-plugins.py index 285884f3e9..3a0b0cb6eb 100644 --- a/scripts/install-dynamic-plugins/test_install-dynamic-plugins.py +++ b/scripts/install-dynamic-plugins/test_install-dynamic-plugins.py @@ -3429,6 +3429,468 @@ def test_mixed_oci_and_npm_only_oci_affected(self, mocker): assert '@backstage/plugin-catalog' in all_plugins +class TestImageRefToSubdirectory: + """Test cases for image_ref_to_subdirectory() function.""" + + def test_replaces_slashes(self): + result = install_dynamic_plugins.image_ref_to_subdirectory("quay.io/rhdh/index") + assert result == "quay.io_rhdh_index" + + def test_replaces_colons(self): + result = install_dynamic_plugins.image_ref_to_subdirectory("quay.io/rhdh/index:1.10") + assert result == "quay.io_rhdh_index_1.10" + + def test_replaces_at_signs(self): + result = install_dynamic_plugins.image_ref_to_subdirectory("quay.io/rhdh/index@sha256:abc123") + assert result == "quay.io_rhdh_index_sha256_abc123" + + def test_all_special_chars(self): + result = install_dynamic_plugins.image_ref_to_subdirectory("registry.example.com:5000/org/image:v1.0") + assert result == "registry.example.com_5000_org_image_v1.0" + + def test_preserves_hyphens_and_dots(self): + result = install_dynamic_plugins.image_ref_to_subdirectory("quay.io/rhdh-community/plugin-catalog-index:1.10") + assert result == "quay.io_rhdh-community_plugin-catalog-index_1.10" + + +class TestParseExtraCatalogIndexImages: + """Test cases for parse_extra_catalog_index_images() function.""" + + def test_single_entry(self): + """Test parsing a single image reference.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + "quay.io/rhdh-community/plugin-catalog-index:1.10" + ) + assert result == [("quay.io_rhdh-community_plugin-catalog-index_1.10", "quay.io/rhdh-community/plugin-catalog-index:1.10")] + + def test_multiple_entries(self): + """Test parsing multiple comma-separated entries.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + "quay.io/rhdh-community/index:1.10,quay.io/partner/catalog:latest" + ) + assert result == [ + ("quay.io_rhdh-community_index_1.10", "quay.io/rhdh-community/index:1.10"), + ("quay.io_partner_catalog_latest", "quay.io/partner/catalog:latest"), + ] + + def test_whitespace_handling(self): + """Test that whitespace around entries is trimmed.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + " quay.io/rhdh-community/index:1.10 , quay.io/partner/catalog:latest " + ) + assert result == [ + ("quay.io_rhdh-community_index_1.10", "quay.io/rhdh-community/index:1.10"), + ("quay.io_partner_catalog_latest", "quay.io/partner/catalog:latest"), + ] + + def test_empty_string(self): + """Test parsing empty string returns empty list.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images("") + assert result == [] + + def test_trailing_comma(self): + """Test that trailing comma is handled gracefully.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + "quay.io/test/image:1.0," + ) + assert result == [("quay.io_test_image_1.0", "quay.io/test/image:1.0")] + + def test_duplicate_subdirectory_warns_and_overwrites(self, capsys): + """Test that duplicate subdirectory names produce a warning but don't error.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + "quay.io/test/image:1.0,quay.io/test/image:1.0" + ) + assert len(result) == 2 + assert result[0][1] == "quay.io/test/image:1.0" + assert result[1][1] == "quay.io/test/image:1.0" + + captured = capsys.readouterr() + assert "WARNING" in captured.out + assert "will be overwritten" in captured.out + + def test_image_ref_with_digest(self): + """Test parsing image reference with digest format.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + "quay.io/rhdh-community/index@sha256:abc123" + ) + assert result == [("quay.io_rhdh-community_index_sha256_abc123", "quay.io/rhdh-community/index@sha256:abc123")] + + def test_explicit_name_format(self): + """Test parsing with explicit name=image_ref format.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + "community=quay.io/rhdh-community/plugin-catalog-index:1.10" + ) + assert result == [("community", "quay.io/rhdh-community/plugin-catalog-index:1.10")] + + def test_mixed_formats(self): + """Test mixing explicit name and auto-derived entries.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + "community=quay.io/rhdh-community/index:1.10,quay.io/partner/catalog:latest" + ) + assert result == [ + ("community", "quay.io/rhdh-community/index:1.10"), + ("quay.io_partner_catalog_latest", "quay.io/partner/catalog:latest"), + ] + + def test_explicit_name_whitespace(self): + """Test that whitespace is trimmed in name=image_ref format.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + " community = quay.io/rhdh-community/index:1.10 " + ) + assert result == [("community", "quay.io/rhdh-community/index:1.10")] + + def test_explicit_name_duplicate_warns(self, capsys): + """Test that duplicate explicit names warn but don't error.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + "community=quay.io/img1:1.0,community=quay.io/img2:2.0" + ) + assert len(result) == 2 + assert result[0] == ("community", "quay.io/img1:1.0") + assert result[1] == ("community", "quay.io/img2:2.0") + + captured = capsys.readouterr() + assert "WARNING" in captured.out + assert "will be overwritten" in captured.out + + def test_explicit_name_empty_image_ref_skipped(self, capsys): + """Test that name= with empty image ref is skipped with a warning.""" + result = install_dynamic_plugins.parse_extra_catalog_index_images( + "community=,quay.io/other/image:1.0" + ) + assert len(result) == 1 + assert result[0][1] == "quay.io/other/image:1.0" + + captured = capsys.readouterr() + assert "WARNING" in captured.out + assert "empty image reference" in captured.out + + +class TestExtractExtraCatalogIndex: + """Test cases for extract_extra_catalog_index() function.""" + + @pytest.fixture + def mock_extra_oci_image(self, tmp_path): + """Create a mock OCI image with catalog entities only (no DPDY file).""" + oci_dir = tmp_path / "extra-oci-image" + oci_dir.mkdir() + + manifest = { + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": { + "mediaType": "application/vnd.oci.image.config.v1+json", + "digest": "sha256:extra123", + "size": 100, + }, + "layers": [ + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "digest": "sha256:extralayer456", + "size": 1000, + } + ], + } + manifest_path = oci_dir / "manifest.json" + manifest_path.write_text(json.dumps(manifest)) + + layer_content_dir = tmp_path / "extra-layer-content" + layer_content_dir.mkdir() + + catalog_entities_dir = layer_content_dir / "catalog-entities" / "extensions" + catalog_entities_dir.mkdir(parents=True) + entity_file = catalog_entities_dir / "community-plugin.yaml" + entity_file.write_text( + "apiVersion: backstage.io/v1alpha1\nkind: Component\nmetadata:\n name: community-plugin" + ) + + layer_tarball = oci_dir / "extralayer456" + with create_test_tarball(layer_tarball) as tar: + tar.add( + str(layer_content_dir / "catalog-entities"), + arcname="catalog-entities", + recursive=True, + ) + + return { + "oci_dir": str(oci_dir), + "manifest_path": str(manifest_path), + "layer_tarball": str(layer_tarball), + } + + @pytest.fixture + def mock_extra_oci_image_marketplace(self, tmp_path): + """Create a mock OCI image with marketplace directory (backward compatibility).""" + oci_dir = tmp_path / "extra-oci-marketplace" + oci_dir.mkdir() + + manifest = { + "schemaVersion": 2, + "layers": [ + { + "digest": "sha256:mktlayer789", + "size": 1000, + } + ], + } + manifest_path = oci_dir / "manifest.json" + manifest_path.write_text(json.dumps(manifest)) + + layer_content_dir = tmp_path / "extra-mkt-layer-content" + layer_content_dir.mkdir() + + catalog_entities_dir = layer_content_dir / "catalog-entities" / "marketplace" + catalog_entities_dir.mkdir(parents=True) + entity_file = catalog_entities_dir / "partner-plugin.yaml" + entity_file.write_text( + "apiVersion: backstage.io/v1alpha1\nkind: Component\nmetadata:\n name: partner-plugin" + ) + + layer_tarball = oci_dir / "mktlayer789" + with create_test_tarball(layer_tarball) as tar: + tar.add( + str(layer_content_dir / "catalog-entities"), + arcname="catalog-entities", + recursive=True, + ) + + return { + "oci_dir": str(oci_dir), + "manifest_path": str(manifest_path), + "layer_tarball": str(layer_tarball), + } + + def test_skopeo_not_found(self, tmp_path, mocker): + """Test that function raises InstallException when skopeo is not available.""" + mocker.patch("shutil.which", return_value=None) + + with pytest.raises( + InstallException, match="skopeo executable not found in PATH" + ): + install_dynamic_plugins.extract_extra_catalog_index( + "quay.io/test/image:latest", + "community", + str(tmp_path / "extensions"), + ) + + def test_skopeo_copy_fails(self, tmp_path, mocker): + """Test that function raises InstallException when skopeo copy fails.""" + import subprocess + + mocker.patch("shutil.which", return_value="/usr/bin/skopeo") + + mock_error = subprocess.CalledProcessError( + returncode=1, + cmd=["/usr/bin/skopeo", "copy", "docker://quay.io/test/image:latest", "dir:/tmp/..."], + ) + mock_error.stderr = "Error: image not found" + mock_error.stdout = "" + mocker.patch("subprocess.run", side_effect=mock_error) + + with pytest.raises(InstallException) as exc_info: + install_dynamic_plugins.extract_extra_catalog_index( + "quay.io/test/image:latest", + "community", + str(tmp_path / "extensions"), + ) + + error_msg = str(exc_info.value) + assert "Failed to download extra catalog index image" in error_msg + + def test_no_manifest(self, tmp_path, mocker): + """Test that function raises InstallException when manifest.json is not found.""" + mocker.patch("shutil.which", return_value="/usr/bin/skopeo") + + mock_result = mocker.Mock() + mock_result.returncode = 0 + mocker.patch("subprocess.run", return_value=mock_result) + + with pytest.raises( + InstallException, match="manifest.json not found in extra catalog index image" + ): + install_dynamic_plugins.extract_extra_catalog_index( + "quay.io/test/image:latest", + "community", + str(tmp_path / "extensions"), + ) + + def test_successful_extraction(self, tmp_path, mocker, mock_extra_oci_image, capsys): + """Test successful extraction of catalog entities to named subdirectory.""" + catalog_entities_parent_dir = tmp_path / "extensions" + + mocker.patch("shutil.which", return_value="/usr/bin/skopeo") + + mock_result = mocker.Mock() + mock_result.returncode = 0 + mock_subprocess_run = create_mock_skopeo_copy( + mock_extra_oci_image["manifest_path"], + mock_extra_oci_image["layer_tarball"], + mock_result, + ) + mocker.patch("subprocess.run", side_effect=mock_subprocess_run) + + install_dynamic_plugins.extract_extra_catalog_index( + "quay.io/rhdh-community/plugin-catalog-index:1.10", + "community", + str(catalog_entities_parent_dir), + ) + + entities_dir = catalog_entities_parent_dir / "community" / "catalog-entities" + assert entities_dir.exists() + entity_file = entities_dir / "community-plugin.yaml" + assert entity_file.exists() + assert "kind: Component" in entity_file.read_text() + + captured = capsys.readouterr() + assert "Successfully extracted extensions catalog entities from extra index image" in captured.out + + def test_marketplace_fallback(self, tmp_path, mocker, mock_extra_oci_image_marketplace, capsys): + """Test that extraction falls back to marketplace directory.""" + catalog_entities_parent_dir = tmp_path / "extensions" + + mocker.patch("shutil.which", return_value="/usr/bin/skopeo") + + mock_result = mocker.Mock() + mock_result.returncode = 0 + mock_subprocess_run = create_mock_skopeo_copy( + mock_extra_oci_image_marketplace["manifest_path"], + mock_extra_oci_image_marketplace["layer_tarball"], + mock_result, + ) + mocker.patch("subprocess.run", side_effect=mock_subprocess_run) + + install_dynamic_plugins.extract_extra_catalog_index( + "quay.io/partner/catalog:latest", + "partner", + str(catalog_entities_parent_dir), + ) + + entities_dir = catalog_entities_parent_dir / "partner" / "catalog-entities" + assert entities_dir.exists() + entity_file = entities_dir / "partner-plugin.yaml" + assert entity_file.exists() + assert "kind: Component" in entity_file.read_text() + + def test_no_catalog_entities_warns(self, tmp_path, mocker, capsys): + """Test that warning is printed when no catalog entities directory exists.""" + oci_dir = tmp_path / "empty-oci" + oci_dir.mkdir() + + manifest = { + "schemaVersion": 2, + "layers": [{"digest": "sha256:empty123", "size": 100}], + } + manifest_path = oci_dir / "manifest.json" + manifest_path.write_text(json.dumps(manifest)) + + layer_content_dir = tmp_path / "empty-layer" + layer_content_dir.mkdir() + readme = layer_content_dir / "README.md" + readme.write_text("# Empty") + + layer_tarball = oci_dir / "empty123" + with create_test_tarball(layer_tarball) as tar: + tar.add(str(readme), arcname="README.md") + + mocker.patch("shutil.which", return_value="/usr/bin/skopeo") + + mock_result = mocker.Mock() + mock_result.returncode = 0 + mock_subprocess_run = create_mock_skopeo_copy(manifest_path, layer_tarball, mock_result) + mocker.patch("subprocess.run", side_effect=mock_subprocess_run) + + install_dynamic_plugins.extract_extra_catalog_index( + "quay.io/test/empty:latest", + "empty", + str(tmp_path / "extensions"), + ) + + captured = capsys.readouterr() + assert "WARNING" in captured.out + assert "does not have neither" in captured.out + + def test_removes_existing_destination(self, tmp_path, mocker, mock_extra_oci_image): + """Test that existing catalog-entities directory is removed before copying.""" + catalog_entities_parent_dir = tmp_path / "extensions" + existing_dir = catalog_entities_parent_dir / "community" / "catalog-entities" + existing_dir.mkdir(parents=True) + old_file = existing_dir / "old-file.yaml" + old_file.write_text("old content") + + mocker.patch("shutil.which", return_value="/usr/bin/skopeo") + + mock_result = mocker.Mock() + mock_result.returncode = 0 + mock_subprocess_run = create_mock_skopeo_copy( + mock_extra_oci_image["manifest_path"], + mock_extra_oci_image["layer_tarball"], + mock_result, + ) + mocker.patch("subprocess.run", side_effect=mock_subprocess_run) + + install_dynamic_plugins.extract_extra_catalog_index( + "quay.io/rhdh-community/index:1.10", + "community", + str(catalog_entities_parent_dir), + ) + + entities_dir = catalog_entities_parent_dir / "community" / "catalog-entities" + assert entities_dir.exists() + assert not old_file.exists() + entity_file = entities_dir / "community-plugin.yaml" + assert entity_file.exists() + + def test_multiple_extra_indexes_different_subdirs(self, tmp_path, mocker, capsys): + """Test that multiple extra indexes extract to separate subdirectories.""" + catalog_entities_parent_dir = tmp_path / "extensions" + + for name, entity_name in [("community", "community-plugin"), ("partner", "partner-plugin")]: + oci_dir = tmp_path / f"oci-{name}" + oci_dir.mkdir() + + digest_hash = f"{name}layer123" + manifest = { + "schemaVersion": 2, + "layers": [{"digest": f"sha256:{digest_hash}", "size": 1000}], + } + (oci_dir / "manifest.json").write_text(json.dumps(manifest)) + + layer_content_dir = tmp_path / f"layer-{name}" + layer_content_dir.mkdir() + entities_dir = layer_content_dir / "catalog-entities" / "extensions" + entities_dir.mkdir(parents=True) + (entities_dir / f"{entity_name}.yaml").write_text( + f"apiVersion: backstage.io/v1alpha1\nkind: Component\nmetadata:\n name: {entity_name}" + ) + + layer_tarball = oci_dir / digest_hash + with create_test_tarball(layer_tarball) as tar: + tar.add(str(layer_content_dir / "catalog-entities"), arcname="catalog-entities", recursive=True) + + mocker.patch("shutil.which", return_value="/usr/bin/skopeo") + + mock_result = mocker.Mock() + mock_result.returncode = 0 + mock_subprocess_run = create_mock_skopeo_copy( + str(oci_dir / "manifest.json"), + str(layer_tarball), + mock_result, + ) + mocker.patch("subprocess.run", side_effect=mock_subprocess_run) + + install_dynamic_plugins.extract_extra_catalog_index( + f"quay.io/test/{name}-index:1.0", + name, + str(catalog_entities_parent_dir), + ) + + community_entities = catalog_entities_parent_dir / "community" / "catalog-entities" + partner_entities = catalog_entities_parent_dir / "partner" / "catalog-entities" + assert community_entities.exists() + assert partner_entities.exists() + assert (community_entities / "community-plugin.yaml").exists() + assert (partner_entities / "partner-plugin.yaml").exists() + + if __name__ == '__main__': pytest.main([__file__, '-v']) From 03e323d08c7417b06a3d18ba934d80ca11f17471 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Tue, 21 Apr 2026 11:52:47 +0200 Subject: [PATCH 2/3] fix: isolate extra catalog index extractions under extra/ subdirectory Prevent an extra catalog index entry like 'catalog-entities=' from overwriting the primary index's catalog entities by extracting all extra images under a dedicated extra/ parent directory. Ref: https://issues.redhat.com/browse/RHIDP-12933 Assisted-by: Claude --- docs/dynamic-plugins/installing-plugins.md | 6 +++--- scripts/install-dynamic-plugins/install-dynamic-plugins.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/dynamic-plugins/installing-plugins.md b/docs/dynamic-plugins/installing-plugins.md index 8be7e00f7d..33c59cd53e 100644 --- a/docs/dynamic-plugins/installing-plugins.md +++ b/docs/dynamic-plugins/installing-plugins.md @@ -102,10 +102,10 @@ EXTRA_CATALOG_INDEX_IMAGES=community=quay.io/rhdh-community/plugin-catalog-index EXTRA_CATALOG_INDEX_IMAGES=community=quay.io/rhdh-community/plugin-catalog-index:1.10,quay.io/partner/catalog:latest ``` -Each image's catalog entities are extracted to a separate subdirectory under the catalog entities parent directory: +Each image's catalog entities are extracted to a separate subdirectory under `/extra/`, keeping them isolated from the primary catalog index entities: -- With explicit name: `community=quay.io/rhdh-community/plugin-catalog-index:1.10` will be extracted to `/community/catalog-entities` -- Without name: `quay.io/partner/catalog:latest` will be extracted to `/quay.io_partner_catalog_latest/catalog-entities` (derived by replacing `/`, `:`, and `@` with `_`) +- With explicit name: `community=quay.io/rhdh-community/plugin-catalog-index:1.10` will be extracted to `/extra/community/catalog-entities` +- Without name: `quay.io/partner/catalog:latest` will be extracted to `/extra/quay.io_partner_catalog_latest/catalog-entities` (derived by replacing `/`, `:`, and `@` with `_`) If multiple entries map to the same subdirectory name, a warning is printed and the later entry overwrites the earlier one. diff --git a/scripts/install-dynamic-plugins/install-dynamic-plugins.py b/scripts/install-dynamic-plugins/install-dynamic-plugins.py index 9e998cf3e3..83ed31556b 100755 --- a/scripts/install-dynamic-plugins/install-dynamic-plugins.py +++ b/scripts/install-dynamic-plugins/install-dynamic-plugins.py @@ -1399,9 +1399,10 @@ def main(): # Extract extra catalog index images if EXTRA_CATALOG_INDEX_IMAGES is set extra_catalog_index_images = os.environ.get("EXTRA_CATALOG_INDEX_IMAGES", "") if extra_catalog_index_images: + extra_parent_dir = os.path.join(catalog_entities_parent_dir, "extra") extra_entries = parse_extra_catalog_index_images(extra_catalog_index_images) for name, image_ref in extra_entries: - extract_extra_catalog_index(image_ref, name, catalog_entities_parent_dir) + extract_extra_catalog_index(image_ref, name, extra_parent_dir) skip_integrity_check = os.environ.get("SKIP_INTEGRITY_CHECK", "").lower() == "true" From 3282663ab7c3fc3a6d4c98e26adf33bc9dc479a8 Mon Sep 17 00:00:00 2001 From: Armel Soro Date: Tue, 21 Apr 2026 12:08:52 +0200 Subject: [PATCH 3/3] fix: move duplicate subdirectory warning next to its extraction header The overwrite warning was printed before the extraction header line, making it look like it related to the previous entry. Move it into extract_extra_catalog_index() so it appears right after the header, clearly associating it with the entry being processed. Ref: https://issues.redhat.com/browse/RHIDP-12933 Assisted-by: Claude --- .../install-dynamic-plugins.py | 20 +++++++++---------- .../test_install-dynamic-plugins.py | 16 ++++----------- 2 files changed, 14 insertions(+), 22 deletions(-) diff --git a/scripts/install-dynamic-plugins/install-dynamic-plugins.py b/scripts/install-dynamic-plugins/install-dynamic-plugins.py index 83ed31556b..366a18cafb 100755 --- a/scripts/install-dynamic-plugins/install-dynamic-plugins.py +++ b/scripts/install-dynamic-plugins/install-dynamic-plugins.py @@ -1145,7 +1145,7 @@ def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str, ca return default_plugins_file -def extract_extra_catalog_index(catalog_index_image: str, subdirectory: str, catalog_entities_parent_dir: str) -> None: +def extract_extra_catalog_index(catalog_index_image: str, subdirectory: str, catalog_entities_parent_dir: str, previously_used_by: str = None) -> None: """Extract catalog entities from an extra catalog index image. Unlike extract_catalog_index(), this does NOT look for dynamic-plugins.default.yaml. @@ -1155,8 +1155,11 @@ def extract_extra_catalog_index(catalog_index_image: str, subdirectory: str, cat catalog_index_image: OCI image reference for the extra catalog index subdirectory: Name of the subdirectory to extract into (e.g., 'community') catalog_entities_parent_dir: Parent directory for catalog entities extraction + previously_used_by: If set, the image ref that previously used this subdirectory (triggers overwrite warning) """ print(f"\n======= Extracting extra catalog index '{subdirectory}' from {catalog_index_image}", flush=True) + if previously_used_by: + print(f"\t==> WARNING: Subdirectory '{subdirectory}' was already used by '{previously_used_by}'. The previous extraction will be overwritten.", flush=True) skopeo_path = shutil.which('skopeo') if skopeo_path is None: @@ -1222,11 +1225,10 @@ def parse_extra_catalog_index_images(extra_images_str: str) -> list[tuple[str, s extra_images_str: Comma-separated list of entries Returns: - List of (subdirectory_name, image_ref) tuples. If multiple images map to the same - subdirectory name, a warning is printed and the last one wins (overwrites). + List of (subdirectory_name, image_ref) tuples in order. Duplicate subdirectory + names are preserved; the caller is responsible for warning and overwriting. """ result = [] - seen_names = {} for entry in extra_images_str.split(","): entry = entry.strip() if not entry: @@ -1241,11 +1243,6 @@ def parse_extra_catalog_index_images(extra_images_str: str) -> list[tuple[str, s if not image_ref: print(f"WARNING: Skipping EXTRA_CATALOG_INDEX_IMAGES entry with empty image reference: '{entry}'", flush=True) continue - if name in seen_names: - print(f"WARNING: Extra catalog index image '{image_ref}' maps to subdirectory '{name}' " - f"which was already used by '{seen_names[name]}'. The previous extraction will be overwritten.", - flush=True) - seen_names[name] = image_ref result.append((name, image_ref)) return result @@ -1401,8 +1398,11 @@ def main(): if extra_catalog_index_images: extra_parent_dir = os.path.join(catalog_entities_parent_dir, "extra") extra_entries = parse_extra_catalog_index_images(extra_catalog_index_images) + seen_names = {} for name, image_ref in extra_entries: - extract_extra_catalog_index(image_ref, name, extra_parent_dir) + previously_used_by = seen_names.get(name) + seen_names[name] = image_ref + extract_extra_catalog_index(image_ref, name, extra_parent_dir, previously_used_by) skip_integrity_check = os.environ.get("SKIP_INTEGRITY_CHECK", "").lower() == "true" diff --git a/scripts/install-dynamic-plugins/test_install-dynamic-plugins.py b/scripts/install-dynamic-plugins/test_install-dynamic-plugins.py index 3a0b0cb6eb..f7fe7626bc 100644 --- a/scripts/install-dynamic-plugins/test_install-dynamic-plugins.py +++ b/scripts/install-dynamic-plugins/test_install-dynamic-plugins.py @@ -3495,8 +3495,8 @@ def test_trailing_comma(self): ) assert result == [("quay.io_test_image_1.0", "quay.io/test/image:1.0")] - def test_duplicate_subdirectory_warns_and_overwrites(self, capsys): - """Test that duplicate subdirectory names produce a warning but don't error.""" + def test_duplicate_subdirectory_returns_all(self): + """Test that duplicate subdirectory names are returned (warning is emitted at extraction time).""" result = install_dynamic_plugins.parse_extra_catalog_index_images( "quay.io/test/image:1.0,quay.io/test/image:1.0" ) @@ -3504,10 +3504,6 @@ def test_duplicate_subdirectory_warns_and_overwrites(self, capsys): assert result[0][1] == "quay.io/test/image:1.0" assert result[1][1] == "quay.io/test/image:1.0" - captured = capsys.readouterr() - assert "WARNING" in captured.out - assert "will be overwritten" in captured.out - def test_image_ref_with_digest(self): """Test parsing image reference with digest format.""" result = install_dynamic_plugins.parse_extra_catalog_index_images( @@ -3539,8 +3535,8 @@ def test_explicit_name_whitespace(self): ) assert result == [("community", "quay.io/rhdh-community/index:1.10")] - def test_explicit_name_duplicate_warns(self, capsys): - """Test that duplicate explicit names warn but don't error.""" + def test_explicit_name_duplicate_returns_all(self): + """Test that duplicate explicit names are returned (warning is emitted at extraction time).""" result = install_dynamic_plugins.parse_extra_catalog_index_images( "community=quay.io/img1:1.0,community=quay.io/img2:2.0" ) @@ -3548,10 +3544,6 @@ def test_explicit_name_duplicate_warns(self, capsys): assert result[0] == ("community", "quay.io/img1:1.0") assert result[1] == ("community", "quay.io/img2:2.0") - captured = capsys.readouterr() - assert "WARNING" in captured.out - assert "will be overwritten" in captured.out - def test_explicit_name_empty_image_ref_skipped(self, capsys): """Test that name= with empty image ref is skipped with a warning.""" result = install_dynamic_plugins.parse_extra_catalog_index_images(