From 39e9fd54a41061272736e59fcdba3893308ad42a Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sun, 23 Nov 2025 03:47:11 +0000 Subject: [PATCH 01/22] feat: add catalog index OCI artifact support to install-dynamic-plugins.py Enable install-dynamic-plugins.py to consume plugin catalog index as an OCI artifact. When CATALOG_INDEX_IMAGE is set, the script uses skopeo to pull the catalog index image, extracts its layers to a temporary directory, and reads dynamic-plugins.default.yaml from the artifact. This file is prepended to the includes list, replacing the embedded default file to avoid duplicates. The implementation supports tar auto-detection (r:*) for different compression formats, fixes an OCI plugin directory creation bug, and ensures cleanup of temporary files after processing. Fully backwards compatible when CATALOG_INDEX_IMAGE is not set. --- docker/install-dynamic-plugins.py | 138 +++++++++++++++++++++++++++++- 1 file changed, 135 insertions(+), 3 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index 244b48e914..fd3affdc47 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -40,6 +40,7 @@ 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) 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. @@ -458,7 +459,7 @@ def get_plugin_tar(self, image: str) -> str: return self.image_to_tarball[image] def extract_plugin(self, tar_file: str, plugin_path: str) -> None: - with tarfile.open(tar_file, 'r:gz') as tar: # NOSONAR + with tarfile.open(tar_file, 'r:*') as tar: # NOSONAR # extract only the files in specified directory filesToExtract = [] for member in tar.getmembers(): @@ -542,7 +543,9 @@ def install(self, plugin: dict, plugin_path_by_hash: dict) -> str: plugin_path = self.downloader.download(package) # Save digest for future comparison - digest_file_path = os.path.join(self.destination, plugin_path, 'dynamic-plugin-image.hash') + plugin_directory = os.path.join(self.destination, plugin_path) + os.makedirs(plugin_directory, exist_ok=True) # Ensure directory exists + digest_file_path = os.path.join(plugin_directory, 'dynamic-plugin-image.hash') with open(digest_file_path, 'w') as f: f.write(self.downloader.digest(package)) @@ -604,7 +607,7 @@ def _extract_npm_package(self, archive: str) -> str: os.mkdir(directory) print('\t==> Extracting package archive', archive, flush=True) - with tarfile.open(archive, 'r:gz') as tar: + with tarfile.open(archive, 'r:*') as tar: for member in tar.getmembers(): if member.isreg(): if not member.name.startswith('package/'): @@ -789,6 +792,112 @@ def wait_for_lock_release(lock_file_path): time.sleep(1) print("======= Lock released.") +def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> str: + """ + Extract the catalog index OCI image to the specified mount path. + + Args: + catalog_index_image: OCI image reference (e.g., quay.io/rhdh/plugin-catalog-index:1.9) + catalog_index_mount: Path where the catalog index should be extracted + + Returns: + Path to the extracted dynamic-plugins.default.yaml if found, otherwise None + """ + if not catalog_index_image: + print("======= No CATALOG_INDEX_IMAGE specified, skipping catalog index extraction", flush=True) + return None + + print(f"\n======= Extracting catalog index from {catalog_index_image}", flush=True) + + # Check if skopeo is available + skopeo_path = shutil.which('skopeo') + if skopeo_path is None: + print("WARNING: skopeo executable not found in PATH, skipping catalog index extraction", flush=True) + return None + + try: + # Create a temporary directory inside the catalog index mount for extraction + catalog_index_temp_dir = os.path.join(catalog_index_mount, '.catalog-index-temp') + os.makedirs(catalog_index_temp_dir, exist_ok=True) + + # Create temporary directory for downloading the image + with tempfile.TemporaryDirectory() as tmp_dir: + # Convert image reference to docker:// format for skopeo + if not catalog_index_image.startswith('docker://'): + image_url = f'docker://{catalog_index_image}' + else: + image_url = catalog_index_image + + print(f"\t==> Copying catalog index image to local filesystem", flush=True) + local_dir = os.path.join(tmp_dir, 'catalog-index-oci') + + # Download the OCI image using skopeo + result = subprocess.run( + [skopeo_path, 'copy', image_url, f'dir:{local_dir}'], + capture_output=True, + text=True + ) + + if result.returncode != 0: + print(f"WARNING: Failed to download catalog index image: {result.stderr}", flush=True) + return None + + # Read the manifest to get the layers + manifest_path = os.path.join(local_dir, 'manifest.json') + if not os.path.isfile(manifest_path): + print("WARNING: manifest.json not found in catalog index image", flush=True) + return None + + with open(manifest_path, 'r') as f: + manifest = json.load(f) + + # Extract all layers + print(f"\t==> Extracting catalog index layers", flush=True) + max_entry_size = int(os.environ.get('MAX_ENTRY_SIZE', 20000000)) + + for layer in manifest.get('layers', []): + layer_digest = layer.get('digest', '') + if not layer_digest: + continue + + # Get the layer file + (_sha, filename) = layer_digest.split(':') + layer_file = os.path.join(local_dir, filename) + + if not os.path.isfile(layer_file): + print(f"\t==> WARNING: Layer file {filename} not found", flush=True) + continue + + # Extract the layer + print(f"\t==> Extracting layer {filename}", flush=True) + with tarfile.open(layer_file, 'r:*') as tar: + for member in tar.getmembers(): + # Security checks + if member.size > max_entry_size: + print(f"\t==> WARNING: Skipping large file {member.name} in catalog index", flush=True) + continue + + if member.islnk() or member.issym(): + realpath = os.path.realpath(os.path.join(catalog_index_temp_dir, *os.path.split(member.linkname))) + if not realpath.startswith(catalog_index_temp_dir): + print(f"\t==> WARNING: Skipping link outside archive: {member.name}", flush=True) + continue + + tar.extract(member, path=catalog_index_temp_dir, filter='tar') + + # Check if dynamic-plugins.default.yaml exists + default_plugins_file = os.path.join(catalog_index_temp_dir, 'dynamic-plugins.default.yaml') + if os.path.isfile(default_plugins_file): + print(f"\t==> Successfully extracted catalog index with dynamic-plugins.default.yaml", flush=True) + return default_plugins_file + else: + print(f"\t==> Catalog index extracted but dynamic-plugins.default.yaml not found", flush=True) + return None + + except Exception as e: + print(f"WARNING: Error extracting catalog index: {e}", flush=True) + return None + def main(): dynamicPluginsRoot = sys.argv[1] @@ -798,6 +907,12 @@ def main(): signal.signal(signal.SIGTERM, lambda signum, frame: sys.exit(0)) create_lock(lock_file_path) + # Extract catalog index if CATALOG_INDEX_IMAGE is set + catalog_index_image = os.environ.get("CATALOG_INDEX_IMAGE", "") + catalog_index_default_file = None + if catalog_index_image: + catalog_index_default_file = extract_catalog_index(catalog_index_image, dynamicPluginsRoot) + skipIntegrityCheck = os.environ.get("SKIP_INTEGRITY_CHECK", "").lower() == "true" dynamicPluginsFile = 'dynamic-plugins.yaml' @@ -843,6 +958,17 @@ def main(): if not isinstance(includes, list): raise InstallException(f"content of the \'includes\' field must be a list in {dynamicPluginsFile}") + # Prepend catalog index default file to includes if it was extracted + if catalog_index_default_file and os.path.isfile(catalog_index_default_file): + print(f"\n======= Prepending catalog index default plugins file: {catalog_index_default_file}", flush=True) + includes.insert(0, catalog_index_default_file) + + # Remove the embedded default file from includes to avoid duplicates + embedded_default = 'dynamic-plugins.default.yaml' + if embedded_default in includes: + print(f"\t==> Removing embedded default file from includes (replaced by catalog index)", flush=True) + includes.remove(embedded_default) + for include in includes: if not isinstance(include, str): raise InstallException(f"content of the \'includes\' field must be a list of strings in {dynamicPluginsFile}") @@ -920,5 +1046,11 @@ def main(): print('\n======= Removing previously installed dynamic plugin', plugin_path_by_hash[hash_value], flush=True) shutil.rmtree(plugin_directory, ignore_errors=True, onerror=None) + # Clean up temporary catalog index directory if it exists + catalog_index_temp_dir = os.path.join(dynamicPluginsRoot, '.catalog-index-temp') + if os.path.exists(catalog_index_temp_dir): + print(f'\n======= Cleaning up temporary catalog index directory', flush=True) + shutil.rmtree(catalog_index_temp_dir, ignore_errors=True, onerror=None) + if __name__ == '__main__': main() From dd2917e8f6db4aa447a6e00b90fc798f105a9a2c Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Wed, 26 Nov 2025 11:39:01 +0000 Subject: [PATCH 02/22] Add a defensive file existence check in the extract_catalog_index() function to handle cases where layer files referenced in the OCI manifest are missing from the downloaded image. Instead of failing with an unhandled exception, the function should print a warning and continues processing other layers. Add unit tests to validate the catalog index extraction features including success cases, missing file handling, and large file skipping. Lastly add minimal documentation Signed-off-by: Fortune Ndlovu --- docker/install-dynamic-plugins.py | 243 +++-- docker/test_install-dynamic-plugins.py | 1011 +++++++++++++------- docs/dynamic-plugins/installing-plugins.md | 46 + 3 files changed, 808 insertions(+), 492 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index fd3affdc47..2a957889e4 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -140,11 +140,11 @@ def __init__(self, plugin: dict, dynamicPluginsFile: str, allPlugins: dict): self.plugin = plugin self.dynamicPluginsFile = dynamicPluginsFile self.allPlugins = allPlugins - + def parse_plugin_key(self, package: str) -> str: """Parses the package and returns the plugin key. Must be implemented by subclasses.""" return package - + def add_new_plugin(self, pluginKey: str): """Adds a new plugin to the allPlugins dict.""" self.allPlugins[pluginKey] = self.plugin @@ -157,7 +157,7 @@ def merge_plugin(self, level: int): if not isinstance(pluginKey, str): raise InstallException(f"content of the \'package\' field must be a string in {self.dynamicPluginsFile}") pluginKey = self.parse_plugin_key(pluginKey) - + if pluginKey not in self.allPlugins: print(f'\n======= Adding new dynamic plugin configuration for {pluginKey}', flush=True) # Keep track of the level of the plugin modification to know when dupe conflicts occur in `includes` and main config files @@ -166,11 +166,11 @@ def merge_plugin(self, level: int): else: # Override the included plugins with fields in the main plugins list print('\n======= Overriding dynamic plugin configuration', pluginKey, flush=True) - + # Check for duplicate plugin configurations defined at the same level (level = 0 for `includes` and 1 for the main config file) if self.allPlugins[pluginKey].get("last_modified_level") == level: raise InstallException(f"Duplicate plugin configuration for {self.plugin['package']} found in {self.dynamicPluginsFile}.") - + self.allPlugins[pluginKey]["last_modified_level"] = level self.override_plugin(pluginKey) @@ -240,10 +240,10 @@ class NPMPackageMerger(PackageMerger): r'$' ) ] - + def __init__(self, plugin: dict, dynamicPluginsFile: str, allPlugins: dict): super().__init__(plugin, dynamicPluginsFile, allPlugins) - + def parse_plugin_key(self, package: str) -> str: """ Parses NPM package specification and returns a version-stripped plugin key. @@ -256,15 +256,15 @@ def parse_plugin_key(self, package: str) -> str: - Local paths: ./path -> ./path (unchanged) - Tarballs: kept as-is since there is no standard format for them """ - + # Local packages don't need version stripping if package.startswith('./'): return package - + # Tarballs are kept as-is since there is no standard format for them if package.endswith('.tgz'): return package - + # remove @version from NPM aliases: alias@npm:package[@version] alias_match = re.match(self.NPM_ALIAS_PATTERN, package) if alias_match: @@ -275,12 +275,12 @@ def parse_plugin_key(self, package: str) -> str: # Recursively parse the npm package part to strip its version npm_key = self._strip_npm_package_version(package_scope + npm_package) return f"{alias_name}@npm:{npm_key}" - + # Check for git URLs for git_pattern in self.GIT_URL_PATTERNS: git_match = re.match(git_pattern, package) - + if git_match: # Remove the #ref part if present return package.split('#')[0] @@ -294,31 +294,31 @@ def _strip_npm_package_version(self, package: str) -> str: scope = npm_match.group(1) or '' pkg_name = npm_match.group(2) return f"{scope}{pkg_name}" - + # If no pattern matches, return as-is (could be tarball URL or other format) return package class PluginInstaller: """Base class for plugin installers with common functionality.""" - + def __init__(self, destination: str, skip_integrity_check: bool = False): self.destination = destination self.skip_integrity_check = skip_integrity_check - + def should_skip_installation(self, plugin: dict, plugin_path_by_hash: dict) -> tuple[bool, str]: """Check if plugin installation should be skipped based on pull policy and current state.""" plugin_hash = plugin['hash'] pull_policy = plugin.get('pullPolicy', PullPolicy.IF_NOT_PRESENT) force_download = plugin.get('forceDownload', False) - + if plugin_hash not in plugin_path_by_hash: return False, "not_installed" - + if pull_policy == PullPolicy.ALWAYS or force_download: return False, "force_download" - + return True, "already_installed" - + def install(self, plugin: dict, plugin_path_by_hash: dict) -> str: """Install a plugin and return the plugin path. Must be implemented by subclasses.""" raise NotImplementedError() @@ -347,11 +347,11 @@ def parse_plugin_key(self, package: str) -> tuple[str, str, bool]: pluginKey: plugin key generated from the OCI package name version: detected tag or digest of the plugin inheritVersion: boolean indicating if the `{{inherit}}` tag is used - """ + """ match = re.match(self.EXPECTED_OCI_PATTERN, package) if not match: raise InstallException(f"oci package \'{package}\' is not in the expected format \'oci://:!\' or \'oci://@sha:!\' in {self.dynamicPluginsFile} where is one of {RECOGNIZED_ALGORITHMS}") - + # Strip away the version (tag or digest) from the package string, resulting in oci://:! # This helps ensure keys used to identify OCI plugins are independent of the version of the plugin registry = match.group(1) @@ -359,13 +359,13 @@ def parse_plugin_key(self, package: str) -> tuple[str, str, bool]: digest_version = match.group(3) version = tag_version if tag_version else digest_version - - path = match.group(4) - + + path = match.group(4) + # {{inherit}} tag indicates that the version should be inherited from the included configuration. Must NOT have a SHA digest included. inheritVersion = (tag_version == "{{inherit}}" and digest_version == None) pluginKey = f"{registry}:!{path}" - + return pluginKey, version, inheritVersion def add_new_plugin(self, version: str, inheritVersion: bool, pluginKey: str): """ @@ -383,26 +383,26 @@ def override_plugin(self, version: str, inheritVersion: bool, pluginKey: str): If `inheritVersion` is True, the version of the existing plugin config will be ignored. """ if inheritVersion is not True: - self.allPlugins[pluginKey]['package'] = self.plugin['package'] # Override package since no version inheritance - + self.allPlugins[pluginKey]['package'] = self.plugin['package'] # Override package since no version inheritance + if self.allPlugins[pluginKey]['version'] != version: print(f"INFO: Overriding version for {pluginKey} from `{self.allPlugins[pluginKey]['version']}` to `{version}`") - + self.allPlugins[pluginKey]["version"] = version - + for key in self.plugin: if key == 'package': continue if key == "version": continue self.allPlugins[pluginKey][key] = self.plugin[key] - + def merge_plugin(self, level: int): package = self.plugin['package'] if not isinstance(package, str): raise InstallException(f"content of the \'package\' field must be a string in {self.dynamicPluginsFile}") pluginKey, version, inheritVersion = self.parse_plugin_key(package) - + # If package does not already exist, add it if pluginKey not in self.allPlugins: print(f'\n======= Adding new dynamic plugin configuration for version `{version}` of {pluginKey}', flush=True) @@ -412,16 +412,16 @@ def merge_plugin(self, level: int): else: # Override the included plugins with fields in the main plugins list print('\n======= Overriding dynamic plugin configuration', pluginKey, flush=True) - + # Check for duplicate plugin configurations defined at the same level (level = 0 for `includes` and 1 for the main config file) if self.allPlugins[pluginKey].get("last_modified_level") == level: raise InstallException(f"Duplicate plugin configuration for {self.plugin['package']} found in {self.dynamicPluginsFile}.") - + self.allPlugins[pluginKey]["last_modified_level"] = level self.override_plugin(version, inheritVersion, pluginKey) class OciDownloader: """Helper class for downloading and extracting plugins from OCI container images.""" - + def __init__(self, destination: str): self._skopeo = shutil.which('skopeo') if self._skopeo is None: @@ -488,7 +488,7 @@ def download(self, package: str) -> str: shutil.rmtree(plugin_directory, ignore_errors=True, onerror=None) self.extract_plugin(tar_file=tar_file, plugin_path=plugin_path) return plugin_path - + def digest(self, package: str) -> str: (image, _) = package.split('!') image_url = image.replace('oci://', 'docker://') @@ -500,39 +500,39 @@ def digest(self, package: str) -> str: class OciPluginInstaller(PluginInstaller): """Handles OCI container-based plugin installation using skopeo.""" - + def __init__(self, destination: str, skip_integrity_check: bool = False): super().__init__(destination, skip_integrity_check) self.downloader = OciDownloader(destination) - + def should_skip_installation(self, plugin: dict, plugin_path_by_hash: dict) -> tuple[bool, str]: """OCI packages have special digest-based checking for ALWAYS pull policy.""" package = plugin['package'] plugin_hash = plugin['hash'] pull_policy = plugin.get('pullPolicy', PullPolicy.ALWAYS if ':latest!' in package else PullPolicy.IF_NOT_PRESENT) - + if plugin_hash not in plugin_path_by_hash: return False, "not_installed" - + if pull_policy == PullPolicy.IF_NOT_PRESENT: return True, "already_installed" - + if pull_policy == PullPolicy.ALWAYS: # Check if digest has changed installed_path = plugin_path_by_hash[plugin_hash] digest_file_path = os.path.join(self.destination, installed_path, 'dynamic-plugin-image.hash') - + local_digest = None if os.path.isfile(digest_file_path): with open(digest_file_path, 'r') as f: local_digest = f.read().strip() - + remote_digest = self.downloader.digest(package) if remote_digest == local_digest: return True, "digest_unchanged" - + return False, "force_download" - + def install(self, plugin: dict, plugin_path_by_hash: dict) -> str: """Install an OCI plugin package.""" package = plugin['package'] @@ -541,112 +541,112 @@ def install(self, plugin: dict, plugin_path_by_hash: dict) -> str: try: plugin_path = self.downloader.download(package) - + # Save digest for future comparison plugin_directory = os.path.join(self.destination, plugin_path) os.makedirs(plugin_directory, exist_ok=True) # Ensure directory exists digest_file_path = os.path.join(plugin_directory, 'dynamic-plugin-image.hash') with open(digest_file_path, 'w') as f: f.write(self.downloader.digest(package)) - + # Clean up duplicate hashes for key in [k for k, v in plugin_path_by_hash.items() if v == plugin_path]: plugin_path_by_hash.pop(key) - + return plugin_path - + except Exception as e: raise InstallException(f"Error while installing OCI plugin {package}: {e}") class NpmPluginInstaller(PluginInstaller): """Handles NPM and local package installation using npm pack.""" - + def __init__(self, destination: str, skip_integrity_check: bool = False): super().__init__(destination, skip_integrity_check) self.max_entry_size = int(os.environ.get('MAX_ENTRY_SIZE', 20000000)) - + def install(self, plugin: dict, plugin_path_by_hash: dict) -> str: """Install an NPM or local plugin package.""" package = plugin['package'] package_is_local = package.startswith('./') - + if package_is_local: package = os.path.join(os.getcwd(), package[2:]) - + # Verify integrity requirements if not package_is_local and not self.skip_integrity_check and 'integrity' not in plugin: raise InstallException(f"No integrity hash provided for Package {package}") - + # Download package print('\t==> Grabbing package archive through `npm pack`', flush=True) result = subprocess.run(['npm', 'pack', package], capture_output=True, cwd=self.destination) if result.returncode != 0: raise InstallException(f'Error while installing plugin {package} with \'npm pack\' : {result.stderr.decode("utf-8")}') - + archive = os.path.join(self.destination, result.stdout.decode('utf-8').strip()) - + # Verify integrity for remote packages if not (package_is_local or self.skip_integrity_check): print('\t==> Verifying package integrity', flush=True) verify_package_integrity(plugin, archive, self.destination) - + # Extract package plugin_path = self._extract_npm_package(archive) - + return plugin_path - + def _extract_npm_package(self, archive: str) -> str: """Extract NPM package archive with security protections.""" directory = archive.replace('.tgz', '') directory_realpath = os.path.realpath(directory) plugin_path = os.path.basename(directory_realpath) - + if os.path.exists(directory): print('\t==> Removing previous plugin directory', directory, flush=True) shutil.rmtree(directory, ignore_errors=True) os.mkdir(directory) - + print('\t==> Extracting package archive', archive, flush=True) with tarfile.open(archive, 'r:*') as tar: for member in tar.getmembers(): if member.isreg(): if not member.name.startswith('package/'): raise InstallException(f"NPM package archive does not start with 'package/' as it should: {member.name}") - + if member.size > self.max_entry_size: raise InstallException(f'Zip bomb detected in {member.name}') - + member.name = member.name.removeprefix('package/') tar.extract(member, path=directory, filter='tar') - + elif member.isdir(): print('\t\tSkipping directory entry', member.name, flush=True) - + elif member.islnk() or member.issym(): if not member.linkpath.startswith('package/'): raise InstallException(f'NPM package archive contains a link outside of the archive: {member.name} -> {member.linkpath}') - + member.name = member.name.removeprefix('package/') member.linkpath = member.linkpath.removeprefix('package/') - + realpath = os.path.realpath(os.path.join(directory, *os.path.split(member.linkname))) if not realpath.startswith(directory_realpath): raise InstallException(f'NPM package archive contains a link outside of the archive: {member.name} -> {member.linkpath}') - + tar.extract(member, path=directory, filter='tar') - + else: type_mapping = { tarfile.CHRTYPE: "character device", - tarfile.BLKTYPE: "block device", + tarfile.BLKTYPE: "block device", tarfile.FIFOTYPE: "FIFO" } type_str = type_mapping.get(member.type, "unknown") raise InstallException(f'NPM package archive contains a non regular file: {member.name} - {type_str}') - + print('\t==> Removing package archive', archive, flush=True) os.remove(archive) - + return plugin_path def create_plugin_installer(package: str, destination: str, skip_integrity_check: bool = False) -> PluginInstaller: @@ -659,15 +659,15 @@ def create_plugin_installer(package: str, destination: str, skip_integrity_check def install_plugin(plugin: dict, plugin_path_by_hash: dict, destination: str, skip_integrity_check: bool = False) -> tuple[str, dict]: """Install a single plugin and handle configuration merging.""" package = plugin['package'] - + # Check if plugin is disabled if plugin.get('disabled', False): print(f'\n======= Skipping disabled dynamic plugin {package}', flush=True) return None, {} - + # Create appropriate installer installer = create_plugin_installer(package, destination, skip_integrity_check) - + # Check if installation should be skipped should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) if should_skip: @@ -676,18 +676,18 @@ def install_plugin(plugin: dict, plugin_path_by_hash: dict, destination: str, sk if plugin['hash'] in plugin_path_by_hash: plugin_path_by_hash.pop(plugin['hash']) return None, plugin.get('pluginConfig', {}) - + # Install the plugin print(f'\n======= Installing dynamic plugin {package}', flush=True) plugin_path = installer.install(plugin, plugin_path_by_hash) - + # Create hash file for tracking hash_file_path = os.path.join(destination, plugin_path, 'dynamic-plugin-config.hash') with open(hash_file_path, 'w') as f: f.write(plugin['hash']) - + print(f'\t==> Successfully installed dynamic plugin {package}', flush=True) - + return plugin_path, plugin.get('pluginConfig', {}) RECOGNIZED_ALGORITHMS = ( @@ -703,9 +703,9 @@ def get_local_package_info(package_path: str) -> dict: abs_package_path = os.path.join(os.getcwd(), package_path[2:]) else: abs_package_path = package_path - + package_json_path = os.path.join(abs_package_path, 'package.json') - + if not os.path.isfile(package_json_path): # If no package.json, fall back to directory modification time if os.path.isdir(abs_package_path): @@ -713,25 +713,25 @@ def get_local_package_info(package_path: str) -> dict: return {'_directory_mtime': mtime} else: return {'_not_found': True} - + with open(package_json_path, 'r') as f: package_json = json.load(f) - + # Extract relevant fields that indicate package changes info = {} info['_package_json'] = package_json - + # Also include package.json modification time as additional change detection info['_package_json_mtime'] = os.path.getmtime(package_json_path) - + # Include package-lock.json or yarn.lock modification time if present for lock_file in ['package-lock.json', 'yarn.lock']: lock_path = os.path.join(abs_package_path, lock_file) if os.path.isfile(lock_path): info[f'_{lock_file}_mtime'] = os.path.getmtime(lock_path) - + return info - + except (json.JSONDecodeError, OSError, IOError) as e: # If we can't read the package info, include the error in hash # This ensures we'll try to reinstall if there are permission issues, etc. @@ -793,82 +793,60 @@ def wait_for_lock_release(lock_file_path): print("======= Lock released.") def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> str: - """ - Extract the catalog index OCI image to the specified mount path. - - Args: - catalog_index_image: OCI image reference (e.g., quay.io/rhdh/plugin-catalog-index:1.9) - catalog_index_mount: Path where the catalog index should be extracted - - Returns: - Path to the extracted dynamic-plugins.default.yaml if found, otherwise None - """ + """Extract the catalog index OCI image and return the path to dynamic-plugins.default.yaml if found.""" if not catalog_index_image: print("======= No CATALOG_INDEX_IMAGE specified, skipping catalog index extraction", flush=True) return None - - print(f"\n======= Extracting catalog index from {catalog_index_image}", flush=True) - - # Check if skopeo is available - skopeo_path = shutil.which('skopeo') - if skopeo_path is None: - print("WARNING: skopeo executable not found in PATH, skipping catalog index extraction", flush=True) - return None - try: - # Create a temporary directory inside the catalog index mount for extraction + print(f"\n======= Extracting catalog index from {catalog_index_image}", flush=True) + skopeo_path = shutil.which('skopeo') + if skopeo_path is None: + print("WARNING: skopeo executable not found in PATH, skipping catalog index extraction", flush=True) + return None + catalog_index_temp_dir = os.path.join(catalog_index_mount, '.catalog-index-temp') os.makedirs(catalog_index_temp_dir, exist_ok=True) - - # Create temporary directory for downloading the image + with tempfile.TemporaryDirectory() as tmp_dir: - # Convert image reference to docker:// format for skopeo if not catalog_index_image.startswith('docker://'): image_url = f'docker://{catalog_index_image}' else: image_url = catalog_index_image - print(f"\t==> Copying catalog index image to local filesystem", flush=True) local_dir = os.path.join(tmp_dir, 'catalog-index-oci') - + # Download the OCI image using skopeo result = subprocess.run( [skopeo_path, 'copy', image_url, f'dir:{local_dir}'], capture_output=True, text=True ) - if result.returncode != 0: print(f"WARNING: Failed to download catalog index image: {result.stderr}", flush=True) return None - - # Read the manifest to get the layers + manifest_path = os.path.join(local_dir, 'manifest.json') if not os.path.isfile(manifest_path): print("WARNING: manifest.json not found in catalog index image", flush=True) return None - + with open(manifest_path, 'r') as f: manifest = json.load(f) - - # Extract all layers + print(f"\t==> Extracting catalog index layers", flush=True) max_entry_size = int(os.environ.get('MAX_ENTRY_SIZE', 20000000)) - + for layer in manifest.get('layers', []): layer_digest = layer.get('digest', '') if not layer_digest: continue - - # Get the layer file + (_sha, filename) = layer_digest.split(':') layer_file = os.path.join(local_dir, filename) - if not os.path.isfile(layer_file): print(f"\t==> WARNING: Layer file {filename} not found", flush=True) continue - - # Extract the layer + print(f"\t==> Extracting layer {filename}", flush=True) with tarfile.open(layer_file, 'r:*') as tar: for member in tar.getmembers(): @@ -876,16 +854,13 @@ def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> if member.size > max_entry_size: print(f"\t==> WARNING: Skipping large file {member.name} in catalog index", flush=True) continue - if member.islnk() or member.issym(): realpath = os.path.realpath(os.path.join(catalog_index_temp_dir, *os.path.split(member.linkname))) if not realpath.startswith(catalog_index_temp_dir): print(f"\t==> WARNING: Skipping link outside archive: {member.name}", flush=True) continue - tar.extract(member, path=catalog_index_temp_dir, filter='tar') - - # Check if dynamic-plugins.default.yaml exists + default_plugins_file = os.path.join(catalog_index_temp_dir, 'dynamic-plugins.default.yaml') if os.path.isfile(default_plugins_file): print(f"\t==> Successfully extracted catalog index with dynamic-plugins.default.yaml", flush=True) @@ -893,7 +868,7 @@ def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> else: print(f"\t==> Catalog index extracted but dynamic-plugins.default.yaml not found", flush=True) return None - + except Exception as e: print(f"WARNING: Error extracting catalog index: {e}", flush=True) return None @@ -962,7 +937,7 @@ def main(): if catalog_index_default_file and os.path.isfile(catalog_index_default_file): print(f"\n======= Prepending catalog index default plugins file: {catalog_index_default_file}", flush=True) includes.insert(0, catalog_index_default_file) - + # Remove the embedded default file from includes to avoid duplicates embedded_default = 'dynamic-plugins.default.yaml' if embedded_default in includes: @@ -1002,7 +977,7 @@ def main(): for plugin in plugins: mergePlugin(plugin, allPlugins, dynamicPluginsFile, level=1) - + # add a hash for each plugin configuration to detect changes and check if version field is set for OCI packages for plugin in allPlugins.values(): hash_dict = copy.deepcopy(plugin) @@ -1010,12 +985,12 @@ def main(): hash_dict.pop('pluginConfig', None) # Don't track the internal version field used to track version inheritance hash_dict.pop('version', None) - + package = plugin['package'] if package.startswith('./'): local_info = get_local_package_info(package) hash_dict['_local_package_info'] = local_info - + hash = hashlib.sha256(json.dumps(hash_dict, sort_keys=True).encode('utf-8')).hexdigest() plugin['hash'] = hash @@ -1029,11 +1004,11 @@ def main(): with open(hash_file_path, 'r') as hash_file: hash_value = hash_file.read().strip() plugin_path_by_hash[hash_value] = dir_name - + # iterate through the list of plugins for plugin in allPlugins.values(): _, plugin_config = install_plugin(plugin, plugin_path_by_hash, dynamicPluginsRoot, skipIntegrityCheck) - + # Merge plugin configuration if provided if plugin_config: globalConfig = maybeMergeConfig(plugin_config, globalConfig) diff --git a/docker/test_install-dynamic-plugins.py b/docker/test_install-dynamic-plugins.py index fde7be4b66..fe2fd51d2a 100644 --- a/docker/test_install-dynamic-plugins.py +++ b/docker/test_install-dynamic-plugins.py @@ -20,6 +20,7 @@ - OciPackageMerger.parse_plugin_key() - Parsing OCI package formats - NPMPackageMerger.merge_plugin() - Plugin config merging and override logic - OciPackageMerger.merge_plugin() - OCI plugin merging with version inheritance +- extract_catalog_index() - Catalog index OCI image extraction and validation Installation: To install test dependencies: @@ -28,10 +29,11 @@ Running tests: Run all tests: $ pytest test_install-dynamic-plugins.py -v - + Run specific test class: $ pytest test_install-dynamic-plugins.py::TestNPMPackageMergerParsePluginKey -v - + $ pytest test_install-dynamic-plugins.py::TestExtractCatalogIndex -v + Run with coverage: $ pytest test_install-dynamic-plugins.py --cov -v """ @@ -60,13 +62,13 @@ class TestNPMPackageMergerParsePluginKey: """Test cases for NPMPackageMerger.parse_plugin_key() method.""" - + @pytest.fixture def npm_merger(self): """Create an NPMPackageMerger instance for testing.""" plugin = {'package': 'test-package'} return NPMPackageMerger(plugin, 'test-file.yaml', {}) - + @pytest.mark.parametrize("input_package,expected_output", [ # Standard NPM packages with version stripping ('@npmcli/arborist@latest', '@npmcli/arborist'), @@ -75,18 +77,18 @@ def npm_merger(self): ('package-name@^1.0.0', 'package-name'), ('package-name@~2.1.0', 'package-name'), ('package-name@1.x', 'package-name'), - + # Packages without version (unchanged) ('package-name', 'package-name'), ('@scope/package', '@scope/package'), - + # NPM aliases with version stripping ('semver:@npm:semver@7.2.2', 'semver:@npm:semver'), ('my-alias@npm:@npmcli/semver-with-patch', 'my-alias@npm:@npmcli/semver-with-patch'), ('semver:@npm:@npmcli/semver-with-patch@1.0.0', 'semver:@npm:@npmcli/semver-with-patch'), ('alias@npm:package@1.0.0', 'alias@npm:package'), ('alias@npm:@scope/package@2.0.0', 'alias@npm:@scope/package'), - + # Git URLs with ref stripping ('npm/cli#c12ea07', 'npm/cli'), ('user/repo#main', 'user/repo'), @@ -97,11 +99,11 @@ def npm_merger(self): ('git+ssh://git@github.com/user/repo.git#tag', 'git+ssh://git@github.com/user/repo.git'), ('git://github.com/user/repo#commit', 'git://github.com/user/repo'), ('https://github.com/user/repo.git#v1.0.0', 'https://github.com/user/repo.git'), - + # Local paths (unchanged) ('./my-local-plugin', './my-local-plugin'), ('./path/to/plugin', './path/to/plugin'), - + # Tarballs (unchanged) ('package.tgz', 'package.tgz'), ('my-package-1.0.0.tgz', 'my-package-1.0.0.tgz'), @@ -115,13 +117,13 @@ def test_parse_plugin_key_success_cases(self, npm_merger, input_package, expecte class TestOciPackageMergerParsePluginKey: """Test cases for OciPackageMerger.parse_plugin_key() method.""" - + @pytest.fixture def oci_merger(self): """Create an OciPackageMerger instance for testing.""" plugin = {'package': 'oci://example.com:v1.0!plugin'} return OciPackageMerger(plugin, 'test-file.yaml', {}) - + @pytest.mark.parametrize("input_package,expected_key,expected_version,expected_inherit", [ # Tag-based packages ( @@ -148,7 +150,7 @@ def oci_merger(self): 'v2.0.0', False ), - + # Digest-based packages with different algorithms ( 'oci://quay.io/user/plugin@sha256:abc123def456!plugin', @@ -168,7 +170,7 @@ def oci_merger(self): 'blake3:1234567890abcdef', False ), - + # Inherit version pattern ( 'oci://quay.io/user/plugin:{{inherit}}!plugin', @@ -188,36 +190,36 @@ def test_parse_plugin_key_success_cases( ): """Test that parse_plugin_key correctly parses valid OCI package formats.""" plugin_key, version, inherit_version = oci_merger.parse_plugin_key(input_package) - + assert plugin_key == expected_key, f"Expected key {expected_key}, got {plugin_key}" assert version == expected_version, f"Expected version {expected_version}, got {version}" assert inherit_version == expected_inherit, f"Expected inherit {expected_inherit}, got {inherit_version}" - + @pytest.mark.parametrize("invalid_package,error_substring", [ # Missing ! separator ('oci://registry.io/plugin:v1.0', 'not in the expected format'), - + # Missing tag/digest ('oci://registry.io/plugin!path', 'not in the expected format'), - + # Invalid format - no tag or digest before ! ('oci://registry.io!path', 'not in the expected format'), - + # Invalid digest algorithm (md5 not in RECOGNIZED_ALGORITHMS) ('oci://registry.io/plugin@md5:abc123!plugin', 'not in the expected format'), - + # Invalid format - multiple @ symbols ('oci://registry.io/plugin@@sha256:abc!plugin', 'not in the expected format'), - + # Invalid format - multiple : symbols in tag ('oci://registry.io/plugin:v1:v2!plugin', 'not in the expected format'), - + # Empty tag ('oci://registry.io/plugin:!plugin', 'not in the expected format'), - + # Empty path after ! ('oci://registry.io/plugin:v1.0!', 'not in the expected format'), - + # No oci:// prefix (but this should fail the regex) ('registry.io/plugin:v1.0!plugin', 'not in the expected format'), ]) @@ -225,37 +227,37 @@ def test_parse_plugin_key_error_cases(self, oci_merger, invalid_package, error_s """Test that parse_plugin_key raises InstallException for invalid OCI package formats.""" with pytest.raises(InstallException) as exc_info: oci_merger.parse_plugin_key(invalid_package) - + assert error_substring in str(exc_info.value), \ f"Expected error message to contain '{error_substring}', got: {str(exc_info.value)}" - + def test_parse_plugin_key_complex_digest(self, oci_merger): """Test parsing OCI package with complex digest value.""" # Note: The pattern allows any value after @ including special strings like {{inherit}} # though this would be semantically incorrect for digest format input_pkg = 'oci://registry.io/plugin@sha256:abc123def456789!plugin' plugin_key, version, inherit = oci_merger.parse_plugin_key(input_pkg) - + assert plugin_key == 'oci://registry.io/plugin:!plugin' assert version == 'sha256:abc123def456789' assert inherit is False - + def test_parse_plugin_key_strips_version_from_key(self, oci_merger): """Test that the plugin key does not contain version information.""" input_pkg = 'oci://quay.io/user/plugin:v1.0.0!my-plugin' plugin_key, version, _ = oci_merger.parse_plugin_key(input_pkg) - + # The key should not contain the version assert ':v1.0.0' not in plugin_key assert plugin_key == 'oci://quay.io/user/plugin:!my-plugin' # But the version should be returned separately assert version == 'v1.0.0' - + def test_parse_plugin_key_with_nested_path(self, oci_merger): """Test parsing OCI package with nested path after !.""" input_pkg = 'oci://registry.io/plugin:v1.0!path/to/nested/plugin' plugin_key, version, inherit = oci_merger.parse_plugin_key(input_pkg) - + assert plugin_key == 'oci://registry.io/plugin:!path/to/nested/plugin' assert version == 'v1.0' assert inherit is False @@ -263,32 +265,32 @@ def test_parse_plugin_key_with_nested_path(self, oci_merger): class TestEdgeCases: """Test edge cases and boundary conditions.""" - + def test_npm_merger_empty_string(self): """Test NPM merger with empty package string.""" plugin = {'package': ''} merger = NPMPackageMerger(plugin, 'test.yaml', {}) result = merger.parse_plugin_key('') assert result == '' - + def test_npm_merger_special_characters_in_package(self): """Test NPM packages with special characters.""" plugin = {'package': 'test'} merger = NPMPackageMerger(plugin, 'test.yaml', {}) - + # Package name with underscores and hyphens result = merger.parse_plugin_key('my_special-package@1.0.0') assert result == 'my_special-package' - + def test_oci_merger_long_digest(self): """Test OCI package with realistic long SHA256 digest.""" plugin = {'package': 'oci://example.com:v1!plugin'} merger = OciPackageMerger(plugin, 'test.yaml', {}) - + long_digest = 'sha256:' + 'a' * 64 input_pkg = f'oci://quay.io/user/plugin@{long_digest}!plugin' plugin_key, version, inherit = merger.parse_plugin_key(input_pkg) - + assert plugin_key == 'oci://quay.io/user/plugin:!plugin' assert version == long_digest assert inherit is False @@ -296,45 +298,45 @@ def test_oci_merger_long_digest(self): class TestNPMPackageMergerMergePlugin: """Test cases for NPMPackageMerger.merge_plugin() method.""" - + def test_add_new_plugin_level_0(self): """Test adding a new plugin at level 0.""" all_plugins = {} plugin = {'package': 'test-package@1.0.0', 'disabled': False} merger = NPMPackageMerger(plugin, 'test-file.yaml', all_plugins) - + merger.merge_plugin(level=0) - + # Check plugin was added assert 'test-package' in all_plugins assert all_plugins['test-package']['package'] == 'test-package@1.0.0' assert all_plugins['test-package']['disabled'] is False assert all_plugins['test-package']['last_modified_level'] == 0 - + def test_override_plugin_level_0_to_1(self): """Test overriding a plugin from level 0 to level 1.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'test-package@1.0.0', 'disabled': False} merger1 = NPMPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 plugin2 = {'package': 'test-package@2.0.0', 'disabled': True} merger2 = NPMPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check override succeeded assert all_plugins['test-package']['disabled'] is True assert all_plugins['test-package']['last_modified_level'] == 1 # Package field should be overridden assert all_plugins['test-package']['package'] == 'test-package@2.0.0' - + def test_override_multiple_config_fields(self): """Test overriding multiple plugin config fields.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = { 'package': '@scope/plugin@1.0.0', @@ -344,7 +346,7 @@ def test_override_multiple_config_fields(self): } merger1 = NPMPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 plugin2 = { 'package': '@scope/plugin@2.0.0', @@ -355,7 +357,7 @@ def test_override_multiple_config_fields(self): } merger2 = NPMPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check all fields were updated except package assert all_plugins['@scope/plugin']['disabled'] is True assert all_plugins['@scope/plugin']['pullPolicy'] == 'Always' @@ -363,74 +365,74 @@ def test_override_multiple_config_fields(self): assert all_plugins['@scope/plugin']['integrity'] == 'sha256-abc123' # Package field not overridden assert all_plugins['@scope/plugin']['package'] == '@scope/plugin@2.0.0' - + def test_duplicate_plugin_same_level_0_raises_error(self): """Test that duplicate plugin at same level 0 raises InstallException.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'duplicate-package@1.0.0'} merger1 = NPMPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Try to add same plugin again at level 0 plugin2 = {'package': 'duplicate-package@2.0.0'} merger2 = NPMPackageMerger(plugin2, 'included-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger2.merge_plugin(level=0) - + assert 'Duplicate plugin configuration' in str(exc_info.value) assert 'duplicate-package@2.0.0' in str(exc_info.value) - + def test_duplicate_plugin_same_level_1_raises_error(self): """Test that duplicate plugin at same level 1 raises InstallException.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'test-package@1.0.0'} merger1 = NPMPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 plugin2 = {'package': 'test-package@2.0.0'} merger2 = NPMPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Try to add same plugin again at level 1 plugin3 = {'package': 'test-package@3.0.0'} merger3 = NPMPackageMerger(plugin3, 'main-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger3.merge_plugin(level=1) - + assert 'Duplicate plugin configuration' in str(exc_info.value) - + def test_invalid_package_field_type_raises_error(self): """Test that non-string package field raises InstallException.""" all_plugins = {} plugin = {'package': 123} merger = NPMPackageMerger(plugin, 'test-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger.merge_plugin(level=0) - + assert 'must be a string' in str(exc_info.value) - + def test_version_stripping_in_plugin_key(self): """Test that version is stripped from plugin key.""" all_plugins = {} - + # Add plugin with version plugin1 = {'package': 'my-plugin@1.0.0'} merger1 = NPMPackageMerger(plugin1, 'test-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override with different version plugin2 = {'package': 'my-plugin@2.0.0', 'disabled': True} merger2 = NPMPackageMerger(plugin2, 'test-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Both should map to same key assert 'my-plugin' in all_plugins assert all_plugins['my-plugin']['disabled'] is True @@ -438,73 +440,73 @@ def test_version_stripping_in_plugin_key(self): class TestOciPackageMergerMergePlugin: """Test cases for OciPackageMerger.merge_plugin() method.""" - + def test_add_new_plugin_with_tag(self): """Test adding a new OCI plugin with tag.""" all_plugins = {} plugin = {'package': 'oci://registry.io/plugin:v1.0!path'} merger = OciPackageMerger(plugin, 'test-file.yaml', all_plugins) - + merger.merge_plugin(level=0) - + plugin_key = 'oci://registry.io/plugin:!path' assert plugin_key in all_plugins assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin:v1.0!path' assert all_plugins[plugin_key]['version'] == 'v1.0' assert all_plugins[plugin_key]['last_modified_level'] == 0 - + def test_add_new_plugin_with_digest(self): """Test adding a new OCI plugin with digest.""" all_plugins = {} plugin = {'package': 'oci://registry.io/plugin@sha256:abc123!path'} merger = OciPackageMerger(plugin, 'test-file.yaml', all_plugins) - + merger.merge_plugin(level=0) - + plugin_key = 'oci://registry.io/plugin:!path' assert plugin_key in all_plugins assert all_plugins[plugin_key]['version'] == 'sha256:abc123' - + def test_override_plugin_version(self, capsys): """Test overriding OCI plugin version from level 0 to 1.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 with new version plugin2 = {'package': 'oci://registry.io/plugin:v2.0!path'} merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check version was updated plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'v2.0' assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin:v2.0!path' assert all_plugins[plugin_key]['last_modified_level'] == 1 - + # Check that override message was printed captured = capsys.readouterr() assert 'Overriding version' in captured.out assert 'v1.0' in captured.out assert 'v2.0' in captured.out - + def test_use_inherit_to_preserve_version(self): """Test using {{inherit}} to preserve existing version.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 with {{inherit}} plugin2 = {'package': 'oci://registry.io/plugin:{{inherit}}!path', 'disabled': True} merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check version was preserved plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'v1.0' @@ -512,11 +514,11 @@ def test_use_inherit_to_preserve_version(self): assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin:v1.0!path' # But other config should be updated assert all_plugins[plugin_key]['disabled'] is True - + def test_override_config_with_version_inheritance(self): """Test overriding plugin config while preserving version with {{inherit}}.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = { 'package': 'oci://registry.io/plugin:v1.0!path', @@ -524,7 +526,7 @@ def test_override_config_with_version_inheritance(self): } merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override config at level 1 with {{inherit}} plugin2 = { 'package': 'oci://registry.io/plugin:{{inherit}}!path', @@ -532,16 +534,16 @@ def test_override_config_with_version_inheritance(self): } merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check version preserved and config updated plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'v1.0' assert all_plugins[plugin_key]['pluginConfig'] == {'key2': 'value2'} - + def test_override_config_without_version_inheritance(self): """Test overriding both version and config.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = { 'package': 'oci://registry.io/plugin:v1.0!path', @@ -549,7 +551,7 @@ def test_override_config_without_version_inheritance(self): } merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override both at level 1 plugin2 = { 'package': 'oci://registry.io/plugin:v2.0!path', @@ -557,111 +559,111 @@ def test_override_config_without_version_inheritance(self): } merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check both were updated plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'v2.0' assert all_plugins[plugin_key]['pluginConfig'] == {'key2': 'value2'} assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin:v2.0!path' - + def test_override_from_tag_to_digest(self): """Test overriding from tag to digest.""" all_plugins = {} - + # Add plugin with tag at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override with digest at level 1 plugin2 = {'package': 'oci://registry.io/plugin@sha256:abc123def456!path'} merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check version updated to digest format plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'sha256:abc123def456' assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin@sha256:abc123def456!path' - + def test_new_plugin_with_inherit_raises_error(self): """Test that using {{inherit}} on a new plugin raises InstallException.""" all_plugins = {} plugin = {'package': 'oci://registry.io/plugin:{{inherit}}!path'} merger = OciPackageMerger(plugin, 'test-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger.merge_plugin(level=0) - + assert '{{inherit}}' in str(exc_info.value) assert 'no resolved tag or digest' in str(exc_info.value) - + def test_duplicate_oci_plugin_same_level_0_raises_error(self): """Test that duplicate OCI plugin at same level 0 raises InstallException.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Try to add same plugin again at level 0 plugin2 = {'package': 'oci://registry.io/plugin:v2.0!path'} merger2 = OciPackageMerger(plugin2, 'included-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger2.merge_plugin(level=0) - + assert 'Duplicate plugin configuration' in str(exc_info.value) - + def test_duplicate_oci_plugin_same_level_1_raises_error(self): """Test that duplicate OCI plugin at same level 1 raises InstallException.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 plugin2 = {'package': 'oci://registry.io/plugin:v2.0!path'} merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Try to add same plugin again at level 1 plugin3 = {'package': 'oci://registry.io/plugin:v3.0!path'} merger3 = OciPackageMerger(plugin3, 'main-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger3.merge_plugin(level=1) - + assert 'Duplicate plugin configuration' in str(exc_info.value) - + def test_invalid_package_field_type_raises_error(self): """Test that non-string package field raises InstallException.""" all_plugins = {} plugin = {'package': ['not', 'a', 'string']} merger = OciPackageMerger(plugin, 'test-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger.merge_plugin(level=0) - + assert 'must be a string' in str(exc_info.value) class TestPluginInstallerShouldSkipInstallation: """Test cases for PluginInstaller.should_skip_installation() method.""" - + def test_plugin_not_installed_returns_false(self, tmp_path): """Test that plugin not in hash dict returns False.""" plugin = {'hash': 'abc123', 'package': 'test-pkg'} plugin_path_by_hash = {} # Empty - nothing installed installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "not_installed" - + def test_plugin_installed_if_not_present_skips(self, tmp_path): """Test that installed plugin with IF_NOT_PRESENT policy skips.""" plugin = { @@ -671,12 +673,12 @@ def test_plugin_installed_if_not_present_skips(self, tmp_path): } plugin_path_by_hash = {'abc123': 'test-pkg-1.0.0'} installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is True assert reason == "already_installed" - + def test_plugin_installed_always_policy_forces_download(self, tmp_path): """Test that ALWAYS policy forces download.""" plugin = { @@ -686,12 +688,12 @@ def test_plugin_installed_always_policy_forces_download(self, tmp_path): } plugin_path_by_hash = {'abc123': 'test-pkg-1.0.0'} installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "force_download" - + def test_plugin_installed_force_download_flag(self, tmp_path): """Test that forceDownload flag forces download.""" plugin = { @@ -701,27 +703,27 @@ def test_plugin_installed_force_download_flag(self, tmp_path): } plugin_path_by_hash = {'abc123': 'test-pkg-1.0.0'} installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "force_download" - + def test_default_pull_policy_if_not_present(self, tmp_path): """Test that default pull policy is IF_NOT_PRESENT.""" plugin = {'hash': 'abc123', 'package': 'test-pkg'} # No pullPolicy plugin_path_by_hash = {'abc123': 'test-pkg-1.0.0'} installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is True assert reason == "already_installed" class TestOciPluginInstallerShouldSkipInstallation: """Test cases for OciPluginInstaller.should_skip_installation() method.""" - + def test_plugin_not_installed_returns_false(self, tmp_path, mocker): """Test that plugin not in hash dict returns False.""" plugin = { @@ -729,17 +731,17 @@ def test_plugin_not_installed_returns_false(self, tmp_path, mocker): 'package': 'oci://registry.io/plugin:latest!path' } plugin_path_by_hash = {} - + # Mock OciDownloader mock_downloader = mocker.MagicMock() installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "not_installed" - + def test_always_policy_unchanged_digest_skips(self, tmp_path, mocker): """Test that ALWAYS policy with unchanged digest skips download.""" plugin_path = 'plugin-dir' @@ -749,24 +751,24 @@ def test_always_policy_unchanged_digest_skips(self, tmp_path, mocker): 'pullPolicy': 'Always' } plugin_path_by_hash = {'abc123': plugin_path} - + # Create digest file with matching digest digest_file = tmp_path / plugin_path / 'dynamic-plugin-image.hash' digest_file.parent.mkdir(parents=True) digest_file.write_text('matching_digest') - + # Mock downloader to return same digest mock_downloader = mocker.MagicMock() mock_downloader.digest.return_value = 'matching_digest' - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is True assert reason == "digest_unchanged" - + def test_always_policy_changed_digest_forces_download(self, tmp_path, mocker): """Test that ALWAYS policy with changed digest forces download.""" plugin_path = 'plugin-dir' @@ -776,21 +778,21 @@ def test_always_policy_changed_digest_forces_download(self, tmp_path, mocker): 'pullPolicy': 'Always' } plugin_path_by_hash = {'abc123': plugin_path} - + # Create digest file with old digest digest_file = tmp_path / plugin_path / 'dynamic-plugin-image.hash' digest_file.parent.mkdir(parents=True) digest_file.write_text('old_digest') - + # Mock downloader to return different digest mock_downloader = mocker.MagicMock() mock_downloader.digest.return_value = 'new_digest' - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "force_download" def test_if_not_present_policy_skips(self, tmp_path, mocker): @@ -802,28 +804,28 @@ def test_if_not_present_policy_skips(self, tmp_path, mocker): 'pullPolicy': 'IfNotPresent' } plugin_path_by_hash = {'abc123': plugin_path} - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is True assert reason == "already_installed" class TestNpmPluginInstallerInstall: """Test cases for NpmPluginInstaller.install() method and verify_package_integrity() (mocked).""" - + def test_missing_integrity_remote_package_raises_exception(self, tmp_path): """Test that missing integrity for remote package raises exception.""" plugin = {'package': 'test-package@1.0.0'} # No integrity plugin_path_by_hash = {} - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path), skip_integrity_check=False) - + with pytest.raises(InstallException) as exc_info: installer.install(plugin, plugin_path_by_hash) - + assert 'No integrity hash provided' in str(exc_info.value) - + def test_invalid_integrity_hash_type_raises_exception(self, tmp_path, mocker): """Test that invalid integrity hash type raises exception.""" plugin = {'package': 'test-package@1.0.0', 'integrity': 1234567890} @@ -843,7 +845,7 @@ def test_invalid_integrity_hash_format_raises_exception(self, tmp_path, mocker): def test_invalid_integrity_algorithm_raises_exception(self, tmp_path, mocker): """Test that unrecognized integrity algorithm raises exception.""" plugin = {'package': 'test-package@1.0.0', 'integrity': 'invalidalgo-1234567890abcdef'} - + with pytest.raises(InstallException) as exc_info: install_dynamic_plugins.verify_package_integrity(plugin, "dummy-archive.tgz", str(tmp_path)) assert 'is not supported' in str(exc_info.value) @@ -851,7 +853,7 @@ def test_invalid_integrity_algorithm_raises_exception(self, tmp_path, mocker): def test_invalid_integrity_hash_base64_encoding_raises_exception(self, tmp_path, mocker): """Test invalid base64 encoding in hash triggers exception.""" plugin = {'package': 'test-package@1.0.0', 'integrity': 'sha256-not@base64!'} - + with pytest.raises(InstallException) as exc_info: install_dynamic_plugins.verify_package_integrity(plugin, "dummy-archive.tgz", str(tmp_path)) assert 'is not a valid base64 encoding' in str(exc_info.value) @@ -869,53 +871,53 @@ def test_skip_integrity_check_flag_works(self, tmp_path, mocker): """Test that skip_integrity_check flag bypasses integrity check.""" plugin = {'package': 'test-package@1.0.0'} # No integrity plugin_path_by_hash = {} - + # Mock npm pack mock_result = mocker.MagicMock() mock_result.returncode = 0 mock_result.stdout = b'test-package-1.0.0.tgz' mocker.patch('subprocess.run', return_value=mock_result) - + # Mock tarball extraction mock_tarfile = mocker.patch('tarfile.open') mock_tar = mocker.MagicMock() mock_tar.getmembers.return_value = [] mock_tarfile.return_value.__enter__.return_value = mock_tar - + # Mock file operations mocker.patch('os.path.exists', return_value=False) mocker.patch('os.mkdir') mocker.patch('os.remove') - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path), skip_integrity_check=True) plugin_path = installer.install(plugin, plugin_path_by_hash) - + assert plugin_path == 'test-package-1.0.0' @pytest.mark.integration class TestNpmPluginInstallerIntegration: """Integration tests with real file operations.""" - + @pytest.mark.integration def test_verify_package_integrity_with_real_tarball(self, tmp_path): """Test integrity verification with actual openssl commands.""" import tarfile import subprocess import shutil - + # Skip if openssl not available if not shutil.which('openssl'): pytest.skip("openssl not available") - + # Create a real test tarball test_dir = tmp_path / "test-package" test_dir.mkdir() (test_dir / "index.js").write_text("console.log('test');") - + tarball_path = tmp_path / "test-package.tgz" with tarfile.open(tarball_path, "w:gz") as tar: tar.add(test_dir, arcname="package") - + # Calculate actual integrity hash using openssl cat_process = subprocess.Popen(["cat", str(tarball_path)], stdout=subprocess.PIPE) openssl_dgst = subprocess.Popen( @@ -930,32 +932,32 @@ def test_verify_package_integrity_with_real_tarball(self, tmp_path): ) integrity_hash, _ = openssl_b64.communicate() integrity_hash = integrity_hash.decode('utf-8').strip() - + # Create plugin with real integrity plugin = { 'package': 'test-package', 'integrity': f'sha256-{integrity_hash}' } - + # Test verification succeeds with correct hash install_dynamic_plugins.verify_package_integrity(plugin, str(tarball_path), str(tmp_path)) - + # Test verification fails with wrong hash (valid base64 but wrong hash) plugin_wrong = { 'package': 'test-package', 'integrity': 'sha256-YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXowMTIzNDU2' } - + with pytest.raises(InstallException) as exc_info: install_dynamic_plugins.verify_package_integrity(plugin_wrong, str(tarball_path), str(tmp_path)) - + assert 'does not match' in str(exc_info.value) - + @pytest.mark.integration def test_extract_npm_package_with_real_tarball(self, tmp_path): """Test tarball extraction with real tar file.""" import tarfile - + # Create a realistic NPM package structure package_dir = tmp_path / "source" / "package" package_dir.mkdir(parents=True) @@ -963,55 +965,55 @@ def test_extract_npm_package_with_real_tarball(self, tmp_path): (package_dir / "index.js").write_text("module.exports = {};") (package_dir / "lib").mkdir() (package_dir / "lib" / "helper.js").write_text("exports.helper = () => {};") - + # Create tarball following NPM format (with 'package/' prefix) tarball_path = tmp_path / "test-package-1.0.0.tgz" with tarfile.open(tarball_path, "w:gz") as tar: tar.add(package_dir, arcname="package") - + # Test extraction installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) plugin_path = installer._extract_npm_package(str(tarball_path)) - + # Verify extracted files extracted_dir = tmp_path / "test-package-1.0.0" assert extracted_dir.exists() assert (extracted_dir / "package.json").exists() assert (extracted_dir / "index.js").exists() assert (extracted_dir / "lib" / "helper.js").exists() - + # Verify tarball was removed assert not tarball_path.exists() - + @pytest.mark.integration def test_zip_bomb_protection_real_tarball(self, tmp_path): """Test that extraction rejects tarballs with oversized files.""" import tarfile - + # Create a tarball with a file exceeding MAX_ENTRY_SIZE large_content = b"x" * 25_000_000 # 25MB (exceeds default 20MB) - + package_dir = tmp_path / "source" / "package" package_dir.mkdir(parents=True) (package_dir / "huge-file.bin").write_bytes(large_content) - + tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: tar.add(package_dir / "huge-file.bin", arcname="package/huge-file.bin") - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'Zip bomb' in str(exc_info.value) - + @pytest.mark.integration def test_path_traversal_protection_real_tarball(self, tmp_path): """Test that extraction rejects tarballs with without package/ prefix.""" import tarfile import io - + # Create tarball with path traversal attempt tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1019,20 +1021,20 @@ def test_path_traversal_protection_real_tarball(self, tmp_path): info = tarfile.TarInfo(name="test") info.size = 10 tar.addfile(info, io.BytesIO(b"malicious!")) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'does not start with' in str(exc_info.value) - + @pytest.mark.integration def test_symlink_with_invalid_linkpath_prefix(self, tmp_path): """Test that extraction rejects symlinks with linkpath not starting with 'package/'.""" import tarfile import io - + # Create tarball with a symlink that has invalid linkpath prefix tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1040,27 +1042,27 @@ def test_symlink_with_invalid_linkpath_prefix(self, tmp_path): info = tarfile.TarInfo(name="package/index.js") info.size = 10 tar.addfile(info, io.BytesIO(b"console.log")) - + # Add a symlink with linkpath not starting with 'package/' link_info = tarfile.TarInfo(name="package/malicious-link") link_info.type = tarfile.SYMTYPE link_info.linkname = "../../../etc/passwd" # Does not start with 'package/' tar.addfile(link_info) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'contains a link outside of the archive' in str(exc_info.value) assert 'malicious-link' in str(exc_info.value) - + @pytest.mark.integration def test_symlink_resolving_outside_directory(self, tmp_path): """Test that extraction rejects symlinks that resolve outside the target directory.""" import tarfile import io - + # Create tarball with a symlink that resolves outside the extraction directory tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1068,27 +1070,27 @@ def test_symlink_resolving_outside_directory(self, tmp_path): info = tarfile.TarInfo(name="package/index.js") info.size = 10 tar.addfile(info, io.BytesIO(b"console.log")) - + # Add a symlink with proper prefix but resolves outside # Using relative path traversal that starts with package/ but goes outside link_info = tarfile.TarInfo(name="package/subdir/malicious-link") link_info.type = tarfile.SYMTYPE link_info.linkname = "package/../../../etc/passwd" # Starts with 'package/' but resolves outside tar.addfile(link_info) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'contains a link outside of the archive' in str(exc_info.value) - + @pytest.mark.integration def test_hardlink_resolving_outside_directory(self, tmp_path): """Test that extraction rejects hardlinks that resolve outside the target directory.""" import tarfile import io - + # Create tarball with a hardlink that resolves outside the extraction directory tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1096,26 +1098,26 @@ def test_hardlink_resolving_outside_directory(self, tmp_path): info = tarfile.TarInfo(name="package/index.js") info.size = 10 tar.addfile(info, io.BytesIO(b"console.log")) - + # Add a hardlink with proper prefix but resolves outside link_info = tarfile.TarInfo(name="package/subdir/malicious-hardlink") link_info.type = tarfile.LNKTYPE link_info.linkname = "package/../../../etc/passwd" # Starts with 'package/' but resolves outside tar.addfile(link_info) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'contains a link outside of the archive' in str(exc_info.value) - + @pytest.mark.integration def test_valid_symlink_extraction(self, tmp_path): """Test that valid symlinks within the package are extracted correctly.""" import tarfile import io - + # Create tarball with valid internal symlinks tarball_path = tmp_path / "valid-package.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1124,41 +1126,41 @@ def test_valid_symlink_extraction(self, tmp_path): content = b"module.exports = { helper: () => {} };" info.size = len(content) tar.addfile(info, io.BytesIO(content)) - + # Add a valid symlink pointing to the file within package/ link_info = tarfile.TarInfo(name="package/index.js") link_info.type = tarfile.SYMTYPE link_info.linkname = "package/lib/helper.js" tar.addfile(link_info) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) plugin_path = installer._extract_npm_package(str(tarball_path)) - + # Verify extraction succeeded extracted_dir = tmp_path / plugin_path assert extracted_dir.exists() assert (extracted_dir / "lib" / "helper.js").exists() assert (extracted_dir / "index.js").exists() assert (extracted_dir / "index.js").is_symlink() - + @pytest.mark.integration def test_install_real_npm_package(self, tmp_path): """Integration test with actual npm pack on a real package.""" import shutil - + # Only run if npm is available if not shutil.which('npm'): pytest.skip("npm not available") - + plugin = { 'package': 'semver@7.0.0', # Small, stable package 'integrity': 'sha512-+GB6zVA9LWh6zovYQLALHwv5rb2PHGlJi3lfiqIHxR0uuwCgefcOJc59v9fv1w8GbStwxuuqqAjI9NMAOOgq1A==' } plugin_path_by_hash = {} - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path), skip_integrity_check=False) plugin_path = installer.install(plugin, plugin_path_by_hash) - + # Verify plugin was installed installed_dir = tmp_path / plugin_path assert installed_dir.exists() @@ -1166,80 +1168,80 @@ def test_install_real_npm_package(self, tmp_path): class TestOciDownloader: """Test cases for OciDownloader class.""" - + def test_skopeo_command_execution(self, tmp_path, mocker): """Test that skopeo commands are executed correctly.""" # Mock shutil.which to return a fake skopeo path mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Mock subprocess.run mock_run = mocker.patch('subprocess.run') mock_run.return_value.returncode = 0 mock_run.return_value.stdout = b'output' - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) result = downloader.skopeo(['inspect', 'docker://example.com/image:latest']) - + # Verify skopeo was called with correct arguments mock_run.assert_called_once() call_args = mock_run.call_args[0][0] assert call_args[0] == '/usr/bin/skopeo' assert call_args[1] == 'inspect' assert result == b'output' - + def test_skopeo_not_found_raises_exception(self, tmp_path, mocker): """Test that missing skopeo raises InstallException.""" mocker.patch('shutil.which', return_value=None) - + with pytest.raises(InstallException) as exc_info: install_dynamic_plugins.OciDownloader(str(tmp_path)) - + assert 'skopeo executable not found' in str(exc_info.value) - + def test_get_plugin_tar_caches_downloads(self, tmp_path, mocker): """Test that get_plugin_tar caches downloaded images.""" mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Mock skopeo copy mock_run = mocker.patch('subprocess.run') mock_run.return_value.returncode = 0 - + # Create fake manifest manifest_data = { 'layers': [{'digest': 'sha256:abc123'}] } - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) - + # Mock the manifest file read mocker.patch('builtins.open', mocker.mock_open(read_data=json.dumps(manifest_data))) mocker.patch('os.path.join', side_effect=lambda *args: '/'.join(args)) - + image = 'oci://registry.io/plugin:v1.0' - + # First call should execute skopeo tar_path1 = downloader.get_plugin_tar(image) - + # Second call should return cached result tar_path2 = downloader.get_plugin_tar(image) - + # Should return same path assert tar_path1 == tar_path2 - + # Verify image is cached assert image in downloader.image_to_tarball - + def test_extract_plugin_with_valid_path(self, tmp_path, mocker): """Test extracting a plugin from a tar file.""" import tarfile import io - + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Create a real test tarball with plugin files plugin_path = "internal-backstage-plugin-test" tarball_path = tmp_path / "test.tar.gz" - + with tarfile.open(tarball_path, "w:gz") as tar: # Add plugin files for filename in ["package.json", "index.js"]: @@ -1247,45 +1249,45 @@ def test_extract_plugin_with_valid_path(self, tmp_path, mocker): content = b'{"name": "test"}' if filename.endswith('.json') else b'console.log("test");' info.size = len(content) tar.addfile(info, io.BytesIO(content)) - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) downloader.extract_plugin(str(tarball_path), plugin_path) - + # Verify files were extracted extracted_dir = tmp_path / plugin_path assert extracted_dir.exists() assert (extracted_dir / "package.json").exists() assert (extracted_dir / "index.js").exists() - + def test_extract_plugin_rejects_oversized_files(self, tmp_path, mocker): """Test that extract_plugin rejects files larger than max_entry_size.""" import tarfile import io - + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + plugin_path = "plugin" tarball_path = tmp_path / "malicious.tar.gz" - + # Create tarball with oversized file (needs actual content matching size) large_content = b"x" * 25_000_000 # 25MB, exceeds default 20MB - + with tarfile.open(tarball_path, "w:gz") as tar: info = tarfile.TarInfo(name=f"{plugin_path}/huge.bin") info.size = len(large_content) tar.addfile(info, io.BytesIO(large_content)) - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: downloader.extract_plugin(str(tarball_path), plugin_path) - + assert 'Zip bomb' in str(exc_info.value) - + def test_download_removes_previous_installation(self, tmp_path, mocker): """Test that download removes previous plugin directory.""" mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Create existing plugin directory with old content plugin_path = "internal-backstage-plugin-test" existing_dir = tmp_path / plugin_path @@ -1295,67 +1297,67 @@ def test_download_removes_previous_installation(self, tmp_path, mocker): old_subdir = existing_dir / "old-subdir" old_subdir.mkdir() (old_subdir / "old-nested.txt").write_text("old nested content") - + # Verify old content exists before assert existing_dir.exists() assert old_file.exists() assert old_subdir.exists() - + # Mock get_plugin_tar and extract_plugin to simulate extraction downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) mocker.patch.object(downloader, 'get_plugin_tar', return_value='/fake/tar/path') - + def mock_extract(tar_file, plugin_path): # Simulate extraction by creating new files plugin_dir = tmp_path / plugin_path plugin_dir.mkdir(parents=True, exist_ok=True) (plugin_dir / "package.json").write_text('{"name": "new-plugin"}') (plugin_dir / "index.js").write_text("console.log('new');") - + mocker.patch.object(downloader, 'extract_plugin', side_effect=mock_extract) - + package = f'oci://registry.io/plugin:v1.0!{plugin_path}' result = downloader.download(package) - + # Verify extraction was called downloader.extract_plugin.assert_called_once() assert result == plugin_path - + # Verify old content was removed assert not old_file.exists(), "Old file should have been removed" assert not old_subdir.exists(), "Old subdirectory should have been removed" - + # Verify new content exists new_dir = tmp_path / plugin_path assert new_dir.exists(), "New plugin directory should exist" assert (new_dir / "package.json").exists(), "New package.json should exist" assert (new_dir / "index.js").exists(), "New index.js should exist" - + # Verify old content is definitely gone assert not (new_dir / "old-file.txt").exists(), "Old file should not exist in new installation" assert not (new_dir / "old-subdir").exists(), "Old subdirectory should not exist in new installation" - + def test_digest_returns_image_digest(self, tmp_path, mocker): """Test that digest() returns the correct digest from remote image.""" mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Mock skopeo inspect output inspect_output = { 'Digest': 'sha256:abc123def456789' } - + mock_run = mocker.patch('subprocess.run') mock_run.return_value.returncode = 0 mock_run.return_value.stdout = json.dumps(inspect_output).encode('utf-8') - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) package = 'oci://registry.io/plugin:v1.0!path' - + digest = downloader.digest(package) - + # Should return just the hash part assert digest == 'abc123def456789' - + # Verify skopeo inspect was called mock_run.assert_called_once() call_args = mock_run.call_args[0][0] @@ -1365,7 +1367,7 @@ def test_digest_returns_image_digest(self, tmp_path, mocker): class TestOciPluginInstallerInstall: """Test cases for OciPluginInstaller.install() method.""" - + def test_install_creates_digest_file(self, tmp_path, mocker): """Test that install creates a digest file for tracking.""" plugin_path = "test-plugin" @@ -1373,41 +1375,41 @@ def test_install_creates_digest_file(self, tmp_path, mocker): 'package': f'oci://registry.io/plugin:v1.0!{plugin_path}', 'version': 'v1.0' } - + # Mock the downloader mock_downloader = mocker.MagicMock() mock_downloader.download.return_value = plugin_path mock_downloader.digest.return_value = 'abc123digest' - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + # Create the plugin directory that download would create plugin_dir = tmp_path / plugin_path plugin_dir.mkdir() - + result = installer.install(plugin, {}) - + # Verify digest file was created digest_file = plugin_dir / 'dynamic-plugin-image.hash' assert digest_file.exists() assert digest_file.read_text() == 'abc123digest' assert result == plugin_path - + def test_install_missing_version_raises_exception(self, tmp_path, mocker): """Test that install raises exception when version is not set.""" plugin = { 'package': 'oci://registry.io/plugin:v1.0!path', 'version': None } - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer.install(plugin, {}) - + assert 'Tag or Digest is not set' in str(exc_info.value) - + def test_install_cleans_up_duplicate_hashes(self, tmp_path, mocker): """Test that install removes duplicate hash entries.""" plugin_path = "test-plugin" @@ -1416,48 +1418,48 @@ def test_install_cleans_up_duplicate_hashes(self, tmp_path, mocker): 'version': 'v1.0', 'hash': 'newhash' } - + plugin_path_by_hash = { 'oldhash': plugin_path, 'anotherhash': plugin_path } - + # Mock the downloader mock_downloader = mocker.MagicMock() mock_downloader.download.return_value = plugin_path mock_downloader.digest.return_value = 'digest123' - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + # Create plugin directory plugin_dir = tmp_path / plugin_path plugin_dir.mkdir() - + result = installer.install(plugin, plugin_path_by_hash) - + # Verify old hashes were removed assert 'oldhash' not in plugin_path_by_hash assert 'anotherhash' not in plugin_path_by_hash assert result == plugin_path - + def test_install_handles_download_errors(self, tmp_path, mocker): """Test that install properly handles download errors.""" plugin = { 'package': 'oci://registry.io/plugin:v1.0!path', 'version': 'v1.0' } - + # Mock downloader to raise an exception mock_downloader = mocker.MagicMock() mock_downloader.download.side_effect = Exception("Network error") - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + with pytest.raises(InstallException) as exc_info: installer.install(plugin, {}) - + assert 'Error while installing OCI plugin' in str(exc_info.value) assert 'Network error' in str(exc_info.value) @@ -1465,118 +1467,118 @@ def test_install_handles_download_errors(self, tmp_path, mocker): @pytest.mark.integration class TestOciIntegration: """Integration tests with real OCI images.""" - + @pytest.mark.integration def test_download_real_oci_image(self, tmp_path): """Test downloading and extracting a real OCI image.""" import shutil - + # Skip if skopeo not available if not shutil.which('skopeo'): pytest.skip("skopeo not available") - + package = 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-simple-chat' - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) plugin_path = downloader.download(package) - + # Verify plugin was extracted plugin_dir = tmp_path / plugin_path assert plugin_dir.exists() assert (plugin_dir / "package.json").exists() - + # Verify we can read package.json package_json = json.loads((plugin_dir / "package.json").read_text()) assert 'name' in package_json - + @pytest.mark.integration def test_get_digest_from_real_image(self, tmp_path): """Test getting digest from a real OCI image.""" import shutil - + if not shutil.which('skopeo'): pytest.skip("skopeo not available") - + package = 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-simple-chat' - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) digest = downloader.digest(package) - + # Digest should be a hex string assert isinstance(digest, str) assert len(digest) > 0 - + @pytest.mark.integration def test_install_oci_plugin_creates_hash_file(self, tmp_path): """Test full installation of OCI plugin with hash file creation.""" import shutil - + if not shutil.which('skopeo'): pytest.skip("skopeo not available") - + plugin_path_name = 'internal-backstage-plugin-simple-chat' plugin = { 'package': f'oci://quay.io/gashcrumb/example-root-http-middleware:latest!{plugin_path_name}', 'version': 'latest' } - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) plugin_path = installer.install(plugin, {}) - + # Verify installation plugin_dir = tmp_path / plugin_path assert plugin_dir.exists() assert (plugin_dir / "package.json").exists() - + # Verify digest hash file was created hash_file = plugin_dir / 'dynamic-plugin-image.hash' assert hash_file.exists() digest = hash_file.read_text().strip() assert len(digest) > 0 - + @pytest.mark.integration def test_download_multiple_plugins_from_same_image(self, tmp_path): """Test downloading multiple plugins from the same OCI image.""" import shutil - + if not shutil.which('skopeo'): pytest.skip("skopeo not available") - + # Two plugins from the same image packages = [ 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-simple-chat', 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-middleware-header-example-dynamic' ] - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) - + plugin_paths = [] for package in packages: plugin_path = downloader.download(package) plugin_paths.append(plugin_path) - + # Verify plugin was extracted plugin_dir = tmp_path / plugin_path assert plugin_dir.exists() assert (plugin_dir / "package.json").exists() - + # Verify both plugins were extracted assert len(plugin_paths) == 2 assert plugin_paths[0] != plugin_paths[1] - + @pytest.mark.integration def test_oci_plugin_with_inherit_version(self, tmp_path): """Test that inherit version pattern works in plugin merge.""" # This tests the version inheritance at the merge level all_plugins = {} - + # First add a plugin with explicit version plugin1 = { 'package': 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-simple-chat-backend-dynamic' } merger1 = install_dynamic_plugins.OciPackageMerger(plugin1, 'test.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Then override with {{inherit}} plugin2 = { 'package': 'oci://quay.io/gashcrumb/example-root-http-middleware:{{inherit}}!internal-backstage-plugin-simple-chat-backend-dynamic', @@ -1584,7 +1586,7 @@ def test_oci_plugin_with_inherit_version(self, tmp_path): } merger2 = install_dynamic_plugins.OciPackageMerger(plugin2, 'test.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Version should be inherited from plugin1 plugin_key = 'oci://quay.io/gashcrumb/example-root-http-middleware:!internal-backstage-plugin-simple-chat-backend-dynamic' assert plugin_key in all_plugins @@ -1594,13 +1596,13 @@ def test_oci_plugin_with_inherit_version(self, tmp_path): class TestGetLocalPackageInfo: """Test cases for get_local_package_info() function.""" - + def test_package_with_valid_package_json(self, tmp_path): """Test getting info from a package with valid package.json.""" # Create a package directory with package.json package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json = { "name": "test-package", "version": "1.0.0", @@ -1608,10 +1610,10 @@ def test_package_with_valid_package_json(self, tmp_path): } package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps(package_json)) - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify the info contains expected fields assert '_package_json' in info assert info['_package_json'] == package_json @@ -1620,184 +1622,477 @@ def test_package_with_valid_package_json(self, tmp_path): # Should not have lock file mtimes assert '_package-lock.json_mtime' not in info assert '_yarn.lock_mtime' not in info - + def test_package_with_relative_path(self, tmp_path, monkeypatch): """Test getting info from a package using relative path (./).""" # Create a package directory package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json = {"name": "test-package", "version": "2.0.0"} (package_dir / "package.json").write_text(json.dumps(package_json)) - + # Change to tmp_path directory and use relative path monkeypatch.chdir(tmp_path) - + # Get package info with relative path info = install_dynamic_plugins.get_local_package_info('./test-package') - + # Verify the info is correct assert info['_package_json'] == package_json assert '_package_json_mtime' in info - + def test_package_with_package_lock_json(self, tmp_path): """Test getting info from a package with package-lock.json.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps({"name": "test", "version": "1.0.0"})) - + package_lock_path = package_dir / "package-lock.json" package_lock_path.write_text(json.dumps({"lockfileVersion": 2})) - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify lock file mtime is included assert '_package-lock.json_mtime' in info assert info['_package-lock.json_mtime'] == package_lock_path.stat().st_mtime assert '_yarn.lock_mtime' not in info - + def test_package_with_yarn_lock(self, tmp_path): """Test getting info from a package with yarn.lock.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps({"name": "test", "version": "1.0.0"})) - + yarn_lock_path = package_dir / "yarn.lock" yarn_lock_path.write_text("# yarn lockfile v1") - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify lock file mtime is included assert '_yarn.lock_mtime' in info assert info['_yarn.lock_mtime'] == yarn_lock_path.stat().st_mtime assert '_package-lock.json_mtime' not in info - + def test_package_with_both_lock_files(self, tmp_path): """Test getting info from a package with both package-lock.json and yarn.lock.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps({"name": "test", "version": "1.0.0"})) - + package_lock_path = package_dir / "package-lock.json" package_lock_path.write_text(json.dumps({"lockfileVersion": 2})) - + yarn_lock_path = package_dir / "yarn.lock" yarn_lock_path.write_text("# yarn lockfile v1") - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify both lock file mtimes are included assert '_package-lock.json_mtime' in info assert '_yarn.lock_mtime' in info assert info['_package-lock.json_mtime'] == package_lock_path.stat().st_mtime assert info['_yarn.lock_mtime'] == yarn_lock_path.stat().st_mtime - + def test_directory_without_package_json(self, tmp_path): """Test getting info from a directory without package.json (falls back to directory mtime).""" package_dir = tmp_path / "empty-package" package_dir.mkdir() - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Should return directory mtime assert '_directory_mtime' in info assert info['_directory_mtime'] == package_dir.stat().st_mtime assert '_package_json' not in info - + def test_nonexistent_path(self, tmp_path): """Test getting info from a non-existent path.""" nonexistent_path = tmp_path / "does-not-exist" - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(nonexistent_path)) - + # Should return _not_found flag assert '_not_found' in info assert info['_not_found'] is True - + def test_invalid_json_in_package_json(self, tmp_path): """Test getting info when package.json contains invalid JSON.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + # Write invalid JSON package_json_path = package_dir / "package.json" package_json_path.write_text("{ invalid json content }") - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Should return error information assert '_error' in info assert 'JSONDecodeError' in info['_error'] or 'Expecting' in info['_error'] - + def test_package_info_detects_changes(self, tmp_path): """Test that package info changes when files are modified.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + # Create initial package.json package_json_path = package_dir / "package.json" package_json_v1 = {"name": "test", "version": "1.0.0"} package_json_path.write_text(json.dumps(package_json_v1)) - + # Get initial info info1 = install_dynamic_plugins.get_local_package_info(str(package_dir)) initial_mtime = info1['_package_json_mtime'] - + # Wait a bit and modify the file import time time.sleep(0.01) - + package_json_v2 = {"name": "test", "version": "2.0.0"} package_json_path.write_text(json.dumps(package_json_v2)) - + # Get updated info info2 = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify that content and mtime changed assert info2['_package_json'] != info1['_package_json'] assert info2['_package_json']['version'] == "2.0.0" assert info2['_package_json_mtime'] > initial_mtime - + def test_lock_file_mtime_detection(self, tmp_path): """Test that lock file changes are detected via mtime.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps({"name": "test", "version": "1.0.0"})) - + # Get info without lock file info1 = install_dynamic_plugins.get_local_package_info(str(package_dir)) assert '_package-lock.json_mtime' not in info1 - + # Add lock file import time time.sleep(0.01) - + package_lock_path = package_dir / "package-lock.json" package_lock_path.write_text(json.dumps({"lockfileVersion": 2})) - + # Get info with lock file info2 = install_dynamic_plugins.get_local_package_info(str(package_dir)) assert '_package-lock.json_mtime' in info2 - + # Hashes should be different due to lock file addition hash1 = hashlib.sha256(json.dumps(info1, sort_keys=True).encode('utf-8')).hexdigest() hash2 = hashlib.sha256(json.dumps(info2, sort_keys=True).encode('utf-8')).hexdigest() assert hash1 != hash2 + +class TestExtractCatalogIndex: + """Test cases for extract_catalog_index() function.""" + + @pytest.fixture + def mock_oci_image(self, tmp_path): + """Create a mock OCI image structure with manifest and layer.""" + import tarfile + + # Create a temporary directory for the OCI image + oci_dir = tmp_path / "oci-image" + oci_dir.mkdir() + + # Create manifest.json + manifest = { + "schemaVersion": 2, + "mediaType": "application/vnd.oci.image.manifest.v1+json", + "config": { + "mediaType": "application/vnd.oci.image.config.v1+json", + "digest": "sha256:test123", + "size": 100 + }, + "layers": [ + { + "mediaType": "application/vnd.oci.image.layer.v1.tar+gzip", + "digest": "sha256:abc123def456", + "size": 1000 + } + ] + } + manifest_path = oci_dir / "manifest.json" + manifest_path.write_text(json.dumps(manifest)) + + # Create a layer tarball with dynamic-plugins.default.yaml + layer_content_dir = tmp_path / "layer-content" + layer_content_dir.mkdir() + + yaml_file = layer_content_dir / "dynamic-plugins.default.yaml" + yaml_content = """plugins: + - package: '@backstage/plugin-catalog' + integrity: sha512-test +""" + yaml_file.write_text(yaml_content) + + # Create the layer tarball + layer_tarball = oci_dir / "abc123def456" + with tarfile.open(layer_tarball, 'w:gz') as tar: + tar.add(str(yaml_file), arcname="dynamic-plugins.default.yaml") + + return { + "oci_dir": str(oci_dir), + "manifest_path": str(manifest_path), + "layer_tarball": str(layer_tarball), + "yaml_content": yaml_content + } + + def test_extract_catalog_index_no_image_specified(self, tmp_path): + """Test that function returns None when no image is specified.""" + result = install_dynamic_plugins.extract_catalog_index("", str(tmp_path)) + assert result is None + + def test_extract_catalog_index_skopeo_not_found(self, tmp_path, mocker): + """Test that function returns None when skopeo is not available.""" + mocker.patch('shutil.which', return_value=None) + + result = install_dynamic_plugins.extract_catalog_index( + "quay.io/test/image:latest", + str(tmp_path) + ) + assert result is None + + def test_extract_catalog_index_skopeo_copy_fails(self, tmp_path, mocker): + """Test that function returns None when skopeo copy fails.""" + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') + + # Mock subprocess.run to simulate skopeo failure + mock_result = mocker.Mock() + mock_result.returncode = 1 + mock_result.stderr = "Error: image not found" + mocker.patch('subprocess.run', return_value=mock_result) + + result = install_dynamic_plugins.extract_catalog_index( + "quay.io/test/image:latest", + str(tmp_path) + ) + assert result is None + + def test_extract_catalog_index_no_manifest(self, tmp_path, mocker): + """Test that function returns None when manifest.json is not found.""" + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') + + # Mock subprocess.run to simulate successful skopeo copy + mock_result = mocker.Mock() + mock_result.returncode = 0 + mocker.patch('subprocess.run', return_value=mock_result) + + result = install_dynamic_plugins.extract_catalog_index( + "quay.io/test/image:latest", + str(tmp_path) + ) + assert result is None + + def test_extract_catalog_index_success(self, tmp_path, mocker, mock_oci_image): + """Test successful extraction of catalog index with dynamic-plugins.default.yaml.""" + catalog_mount = tmp_path / "catalog-mount" + catalog_mount.mkdir() + + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') + + # Mock subprocess.run to simulate successful skopeo copy + mock_result = mocker.Mock() + mock_result.returncode = 0 + + def mock_subprocess_run(cmd, **kwargs): + # When skopeo copy is called, set up the OCI directory structure + if 'copy' in cmd: + # Extract the destination directory from the command + dest_arg = [arg for arg in cmd if arg.startswith('dir:')] + if dest_arg: + dest_dir = dest_arg[0].replace('dir:', '') + os.makedirs(dest_dir, exist_ok=True) + + # Copy mock OCI image files to destination + import shutil as sh + sh.copy(mock_oci_image['manifest_path'], dest_dir) + sh.copy(mock_oci_image['layer_tarball'], dest_dir) + + return mock_result + + mocker.patch('subprocess.run', side_effect=mock_subprocess_run) + + result = install_dynamic_plugins.extract_catalog_index( + "quay.io/test/catalog-index:1.9", + str(catalog_mount) + ) + + # Verify the function returned a path + assert result is not None + assert result.endswith('dynamic-plugins.default.yaml') + + # Verify the file exists and contains expected content + assert os.path.isfile(result) + with open(result, 'r') as f: + content = f.read() + assert '@backstage/plugin-catalog' in content + + def test_extract_catalog_index_no_yaml_file(self, tmp_path, mocker): + """Test that function returns None when dynamic-plugins.default.yaml is not found in the image.""" + import tarfile + + catalog_mount = tmp_path / "catalog-mount" + catalog_mount.mkdir() + + # Create OCI structure without the YAML file + oci_dir = tmp_path / "oci-no-yaml" + oci_dir.mkdir() + + manifest = { + "schemaVersion": 2, + "layers": [ + { + "digest": "sha256:xyz789", + "size": 500 + } + ] + } + manifest_path = oci_dir / "manifest.json" + manifest_path.write_text(json.dumps(manifest)) + + # Create empty layer tarball + layer_tarball = oci_dir / "xyz789" + with tarfile.open(layer_tarball, 'w:gz') as tar: + # Add a different file + readme = tmp_path / "README.md" + readme.write_text("# Test") + tar.add(str(readme), arcname="README.md") + + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') + + mock_result = mocker.Mock() + mock_result.returncode = 0 + + def mock_subprocess_run(cmd, **kwargs): + if 'copy' in cmd: + dest_arg = [arg for arg in cmd if arg.startswith('dir:')] + if dest_arg: + dest_dir = dest_arg[0].replace('dir:', '') + os.makedirs(dest_dir, exist_ok=True) + import shutil as sh + sh.copy(str(manifest_path), dest_dir) + sh.copy(str(layer_tarball), dest_dir) + return mock_result + + mocker.patch('subprocess.run', side_effect=mock_subprocess_run) + + result = install_dynamic_plugins.extract_catalog_index( + "quay.io/test/empty-index:latest", + str(catalog_mount) + ) + + assert result is None + + def test_extract_catalog_index_large_file_skipped(self, tmp_path, mocker, monkeypatch): + """Test that files larger than MAX_ENTRY_SIZE are skipped during extraction.""" + import tarfile + + catalog_mount = tmp_path / "catalog-mount" + catalog_mount.mkdir() + + # Set a very small MAX_ENTRY_SIZE for testing + monkeypatch.setenv('MAX_ENTRY_SIZE', '1000') + + # Create OCI structure with a "large" file (larger than our test threshold) + oci_dir = tmp_path / "oci-large-file" + oci_dir.mkdir() + + manifest = { + "schemaVersion": 2, + "layers": [ + { + "digest": "sha256:large123", + "size": 10000 + } + ] + } + manifest_path = oci_dir / "manifest.json" + manifest_path.write_text(json.dumps(manifest)) + + # Create layer with files + layer_tarball = oci_dir / "large123" + layer_content_dir = tmp_path / "large-content" + layer_content_dir.mkdir() + + yaml_file = layer_content_dir / "dynamic-plugins.default.yaml" + yaml_file.write_text("plugins: []") + + # Create a "large" file that's bigger than our test threshold of 1000 bytes + large_file = layer_content_dir / "large-file.bin" + large_file.write_text("x" * 2000) # 2KB - larger than our 1000 byte test limit + + with tarfile.open(layer_tarball, 'w:gz') as tar: + # Add YAML with normal size (smaller than 1000 bytes) + tar.add(str(yaml_file), arcname="dynamic-plugins.default.yaml") + + # Add "large" file (2KB, which exceeds our 1000 byte test limit) + tar.add(str(large_file), arcname="large-file.bin") + + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') + + mock_result = mocker.Mock() + mock_result.returncode = 0 + + def mock_subprocess_run(cmd, **kwargs): + if 'copy' in cmd: + dest_arg = [arg for arg in cmd if arg.startswith('dir:')] + if dest_arg: + dest_dir = dest_arg[0].replace('dir:', '') + os.makedirs(dest_dir, exist_ok=True) + import shutil as sh + sh.copy(str(manifest_path), dest_dir) + sh.copy(str(layer_tarball), dest_dir) + return mock_result + + mocker.patch('subprocess.run', side_effect=mock_subprocess_run) + + result = install_dynamic_plugins.extract_catalog_index( + "quay.io/test/large-file-index:latest", + str(catalog_mount) + ) + + # Should still succeed and find the YAML file + assert result is not None + assert os.path.isfile(result) + + # Verify large file was not extracted + catalog_temp_dir = catalog_mount / ".catalog-index-temp" + large_file_path = catalog_temp_dir / "large-file.bin" + assert not large_file_path.exists() + + def test_extract_catalog_index_exception_handling(self, tmp_path, mocker): + """Test that exceptions during extraction are caught and return None.""" + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') + + # Mock subprocess.run to raise an exception + mocker.patch('subprocess.run', side_effect=Exception("Unexpected error")) + + result = install_dynamic_plugins.extract_catalog_index( + "quay.io/test/image:latest", + str(tmp_path) + ) + + assert result is None + if __name__ == '__main__': pytest.main([__file__, '-v']) diff --git a/docs/dynamic-plugins/installing-plugins.md b/docs/dynamic-plugins/installing-plugins.md index 43e9d814ba..54026c12d7 100644 --- a/docs/dynamic-plugins/installing-plugins.md +++ b/docs/dynamic-plugins/installing-plugins.md @@ -36,6 +36,52 @@ While the plugin's default configuration comes from the `dynamic-plugins.default Note: The plugin's default configuration typically references environment variables, and it is essential to ensure that these variables are set in the Helm chart values or the Operator configuration. +## Using a Catalog Index Image for Default Plugin Configurations + +RHDH supports loading default plugin configurations from an OCI container image, known as a "catalog index". This feature allows you to maintain centralized plugin configurations that can be updated independently of the RHDH container image. + +When the `CATALOG_INDEX_IMAGE` environment variable is set, the `install-dynamic-plugins` init container will: + +1. Download and extract the specified OCI image +2. Look for a `dynamic-plugins.default.yaml` file within the image +3. Use this file as the primary source for default plugin configurations +4. Automatically override the embedded `dynamic-plugins.default.yaml` file if present + +### Configuring the Catalog Index Image + +Set the `CATALOG_INDEX_IMAGE` environment variable to specify the OCI image containing your plugin catalog: + +```yaml +# Example using Kubernetes/OpenShift deployment +env: + - name: CATALOG_INDEX_IMAGE + value: "quay.io/rhdh/plugin-catalog-index:1.9" +``` + +```yaml +# Example using Helm chart values +upstream: + backstage: + extraEnvVars: + - name: CATALOG_INDEX_IMAGE + value: "quay.io/rhdh/plugin-catalog-index:1.9" +``` + +### Catalog Index Image Structure + +The catalog index OCI image should contain a `dynamic-plugins.default.yaml` file at the root level with the same structure as the embedded default configuration file: + +```yaml +# Contents of dynamic-plugins.default.yaml in the OCI image +plugins: + - package: '@backstage/plugin-catalog' + disabled: true + pluginConfig: + # ... plugin configuration + - package: oci://quay.io/example/plugin:v1.0!my-plugin + disabled: true +``` + ## 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. From c0801c619702fdffcf28fefb3475a54e188a3ab1 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Wed, 26 Nov 2025 11:51:28 +0000 Subject: [PATCH 03/22] Update documentation wording Signed-off-by: Fortune Ndlovu --- docs/dynamic-plugins/installing-plugins.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/dynamic-plugins/installing-plugins.md b/docs/dynamic-plugins/installing-plugins.md index 54026c12d7..57cd45ea08 100644 --- a/docs/dynamic-plugins/installing-plugins.md +++ b/docs/dynamic-plugins/installing-plugins.md @@ -38,7 +38,7 @@ Note: The plugin's default configuration typically references environment variab ## Using a Catalog Index Image for Default Plugin Configurations -RHDH supports loading default plugin configurations from an OCI container image, known as a "catalog index". This feature allows you to maintain centralized plugin configurations that can be updated independently of the RHDH container image. +RHDH supports loading default plugin configurations from an OCI container image. This feature allows you to maintain centralized plugin configurations that can be updated independently of the RHDH container image. When the `CATALOG_INDEX_IMAGE` environment variable is set, the `install-dynamic-plugins` init container will: From bddef47f23f722e703eb30fb64fd8ba082679586 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Wed, 26 Nov 2025 12:19:24 +0000 Subject: [PATCH 04/22] fix: remove trailing whitespace changes from catalog index feature Signed-off-by: Fortune Ndlovu --- docker/install-dynamic-plugins.py | 182 +++++++++++++++--------------- 1 file changed, 91 insertions(+), 91 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index 2a957889e4..dad597732a 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -140,11 +140,11 @@ def __init__(self, plugin: dict, dynamicPluginsFile: str, allPlugins: dict): self.plugin = plugin self.dynamicPluginsFile = dynamicPluginsFile self.allPlugins = allPlugins - + def parse_plugin_key(self, package: str) -> str: """Parses the package and returns the plugin key. Must be implemented by subclasses.""" return package - + def add_new_plugin(self, pluginKey: str): """Adds a new plugin to the allPlugins dict.""" self.allPlugins[pluginKey] = self.plugin @@ -157,7 +157,7 @@ def merge_plugin(self, level: int): if not isinstance(pluginKey, str): raise InstallException(f"content of the \'package\' field must be a string in {self.dynamicPluginsFile}") pluginKey = self.parse_plugin_key(pluginKey) - + if pluginKey not in self.allPlugins: print(f'\n======= Adding new dynamic plugin configuration for {pluginKey}', flush=True) # Keep track of the level of the plugin modification to know when dupe conflicts occur in `includes` and main config files @@ -166,11 +166,11 @@ def merge_plugin(self, level: int): else: # Override the included plugins with fields in the main plugins list print('\n======= Overriding dynamic plugin configuration', pluginKey, flush=True) - + # Check for duplicate plugin configurations defined at the same level (level = 0 for `includes` and 1 for the main config file) if self.allPlugins[pluginKey].get("last_modified_level") == level: raise InstallException(f"Duplicate plugin configuration for {self.plugin['package']} found in {self.dynamicPluginsFile}.") - + self.allPlugins[pluginKey]["last_modified_level"] = level self.override_plugin(pluginKey) @@ -240,10 +240,10 @@ class NPMPackageMerger(PackageMerger): r'$' ) ] - + def __init__(self, plugin: dict, dynamicPluginsFile: str, allPlugins: dict): super().__init__(plugin, dynamicPluginsFile, allPlugins) - + def parse_plugin_key(self, package: str) -> str: """ Parses NPM package specification and returns a version-stripped plugin key. @@ -256,15 +256,15 @@ def parse_plugin_key(self, package: str) -> str: - Local paths: ./path -> ./path (unchanged) - Tarballs: kept as-is since there is no standard format for them """ - + # Local packages don't need version stripping if package.startswith('./'): return package - + # Tarballs are kept as-is since there is no standard format for them if package.endswith('.tgz'): return package - + # remove @version from NPM aliases: alias@npm:package[@version] alias_match = re.match(self.NPM_ALIAS_PATTERN, package) if alias_match: @@ -275,12 +275,12 @@ def parse_plugin_key(self, package: str) -> str: # Recursively parse the npm package part to strip its version npm_key = self._strip_npm_package_version(package_scope + npm_package) return f"{alias_name}@npm:{npm_key}" - + # Check for git URLs for git_pattern in self.GIT_URL_PATTERNS: git_match = re.match(git_pattern, package) - + if git_match: # Remove the #ref part if present return package.split('#')[0] @@ -294,31 +294,31 @@ def _strip_npm_package_version(self, package: str) -> str: scope = npm_match.group(1) or '' pkg_name = npm_match.group(2) return f"{scope}{pkg_name}" - + # If no pattern matches, return as-is (could be tarball URL or other format) return package class PluginInstaller: """Base class for plugin installers with common functionality.""" - + def __init__(self, destination: str, skip_integrity_check: bool = False): self.destination = destination self.skip_integrity_check = skip_integrity_check - + def should_skip_installation(self, plugin: dict, plugin_path_by_hash: dict) -> tuple[bool, str]: """Check if plugin installation should be skipped based on pull policy and current state.""" plugin_hash = plugin['hash'] pull_policy = plugin.get('pullPolicy', PullPolicy.IF_NOT_PRESENT) force_download = plugin.get('forceDownload', False) - + if plugin_hash not in plugin_path_by_hash: return False, "not_installed" - + if pull_policy == PullPolicy.ALWAYS or force_download: return False, "force_download" - + return True, "already_installed" - + def install(self, plugin: dict, plugin_path_by_hash: dict) -> str: """Install a plugin and return the plugin path. Must be implemented by subclasses.""" raise NotImplementedError() @@ -347,11 +347,11 @@ def parse_plugin_key(self, package: str) -> tuple[str, str, bool]: pluginKey: plugin key generated from the OCI package name version: detected tag or digest of the plugin inheritVersion: boolean indicating if the `{{inherit}}` tag is used - """ + """ match = re.match(self.EXPECTED_OCI_PATTERN, package) if not match: raise InstallException(f"oci package \'{package}\' is not in the expected format \'oci://:!\' or \'oci://@sha:!\' in {self.dynamicPluginsFile} where is one of {RECOGNIZED_ALGORITHMS}") - + # Strip away the version (tag or digest) from the package string, resulting in oci://:! # This helps ensure keys used to identify OCI plugins are independent of the version of the plugin registry = match.group(1) @@ -359,13 +359,13 @@ def parse_plugin_key(self, package: str) -> tuple[str, str, bool]: digest_version = match.group(3) version = tag_version if tag_version else digest_version - - path = match.group(4) - + + path = match.group(4) + # {{inherit}} tag indicates that the version should be inherited from the included configuration. Must NOT have a SHA digest included. inheritVersion = (tag_version == "{{inherit}}" and digest_version == None) pluginKey = f"{registry}:!{path}" - + return pluginKey, version, inheritVersion def add_new_plugin(self, version: str, inheritVersion: bool, pluginKey: str): """ @@ -383,26 +383,26 @@ def override_plugin(self, version: str, inheritVersion: bool, pluginKey: str): If `inheritVersion` is True, the version of the existing plugin config will be ignored. """ if inheritVersion is not True: - self.allPlugins[pluginKey]['package'] = self.plugin['package'] # Override package since no version inheritance - + self.allPlugins[pluginKey]['package'] = self.plugin['package'] # Override package since no version inheritance + if self.allPlugins[pluginKey]['version'] != version: print(f"INFO: Overriding version for {pluginKey} from `{self.allPlugins[pluginKey]['version']}` to `{version}`") - + self.allPlugins[pluginKey]["version"] = version - + for key in self.plugin: if key == 'package': continue if key == "version": continue self.allPlugins[pluginKey][key] = self.plugin[key] - + def merge_plugin(self, level: int): package = self.plugin['package'] if not isinstance(package, str): raise InstallException(f"content of the \'package\' field must be a string in {self.dynamicPluginsFile}") pluginKey, version, inheritVersion = self.parse_plugin_key(package) - + # If package does not already exist, add it if pluginKey not in self.allPlugins: print(f'\n======= Adding new dynamic plugin configuration for version `{version}` of {pluginKey}', flush=True) @@ -412,16 +412,16 @@ def merge_plugin(self, level: int): else: # Override the included plugins with fields in the main plugins list print('\n======= Overriding dynamic plugin configuration', pluginKey, flush=True) - + # Check for duplicate plugin configurations defined at the same level (level = 0 for `includes` and 1 for the main config file) if self.allPlugins[pluginKey].get("last_modified_level") == level: raise InstallException(f"Duplicate plugin configuration for {self.plugin['package']} found in {self.dynamicPluginsFile}.") - + self.allPlugins[pluginKey]["last_modified_level"] = level self.override_plugin(version, inheritVersion, pluginKey) class OciDownloader: """Helper class for downloading and extracting plugins from OCI container images.""" - + def __init__(self, destination: str): self._skopeo = shutil.which('skopeo') if self._skopeo is None: @@ -488,7 +488,7 @@ def download(self, package: str) -> str: shutil.rmtree(plugin_directory, ignore_errors=True, onerror=None) self.extract_plugin(tar_file=tar_file, plugin_path=plugin_path) return plugin_path - + def digest(self, package: str) -> str: (image, _) = package.split('!') image_url = image.replace('oci://', 'docker://') @@ -500,39 +500,39 @@ def digest(self, package: str) -> str: class OciPluginInstaller(PluginInstaller): """Handles OCI container-based plugin installation using skopeo.""" - + def __init__(self, destination: str, skip_integrity_check: bool = False): super().__init__(destination, skip_integrity_check) self.downloader = OciDownloader(destination) - + def should_skip_installation(self, plugin: dict, plugin_path_by_hash: dict) -> tuple[bool, str]: """OCI packages have special digest-based checking for ALWAYS pull policy.""" package = plugin['package'] plugin_hash = plugin['hash'] pull_policy = plugin.get('pullPolicy', PullPolicy.ALWAYS if ':latest!' in package else PullPolicy.IF_NOT_PRESENT) - + if plugin_hash not in plugin_path_by_hash: return False, "not_installed" - + if pull_policy == PullPolicy.IF_NOT_PRESENT: return True, "already_installed" - + if pull_policy == PullPolicy.ALWAYS: # Check if digest has changed installed_path = plugin_path_by_hash[plugin_hash] digest_file_path = os.path.join(self.destination, installed_path, 'dynamic-plugin-image.hash') - + local_digest = None if os.path.isfile(digest_file_path): with open(digest_file_path, 'r') as f: local_digest = f.read().strip() - + remote_digest = self.downloader.digest(package) if remote_digest == local_digest: return True, "digest_unchanged" - + return False, "force_download" - + def install(self, plugin: dict, plugin_path_by_hash: dict) -> str: """Install an OCI plugin package.""" package = plugin['package'] @@ -541,112 +541,112 @@ def install(self, plugin: dict, plugin_path_by_hash: dict) -> str: try: plugin_path = self.downloader.download(package) - + # Save digest for future comparison plugin_directory = os.path.join(self.destination, plugin_path) os.makedirs(plugin_directory, exist_ok=True) # Ensure directory exists digest_file_path = os.path.join(plugin_directory, 'dynamic-plugin-image.hash') with open(digest_file_path, 'w') as f: f.write(self.downloader.digest(package)) - + # Clean up duplicate hashes for key in [k for k, v in plugin_path_by_hash.items() if v == plugin_path]: plugin_path_by_hash.pop(key) - + return plugin_path - + except Exception as e: raise InstallException(f"Error while installing OCI plugin {package}: {e}") class NpmPluginInstaller(PluginInstaller): """Handles NPM and local package installation using npm pack.""" - + def __init__(self, destination: str, skip_integrity_check: bool = False): super().__init__(destination, skip_integrity_check) self.max_entry_size = int(os.environ.get('MAX_ENTRY_SIZE', 20000000)) - + def install(self, plugin: dict, plugin_path_by_hash: dict) -> str: """Install an NPM or local plugin package.""" package = plugin['package'] package_is_local = package.startswith('./') - + if package_is_local: package = os.path.join(os.getcwd(), package[2:]) - + # Verify integrity requirements if not package_is_local and not self.skip_integrity_check and 'integrity' not in plugin: raise InstallException(f"No integrity hash provided for Package {package}") - + # Download package print('\t==> Grabbing package archive through `npm pack`', flush=True) result = subprocess.run(['npm', 'pack', package], capture_output=True, cwd=self.destination) if result.returncode != 0: raise InstallException(f'Error while installing plugin {package} with \'npm pack\' : {result.stderr.decode("utf-8")}') - + archive = os.path.join(self.destination, result.stdout.decode('utf-8').strip()) - + # Verify integrity for remote packages if not (package_is_local or self.skip_integrity_check): print('\t==> Verifying package integrity', flush=True) verify_package_integrity(plugin, archive, self.destination) - + # Extract package plugin_path = self._extract_npm_package(archive) - + return plugin_path - + def _extract_npm_package(self, archive: str) -> str: """Extract NPM package archive with security protections.""" directory = archive.replace('.tgz', '') directory_realpath = os.path.realpath(directory) plugin_path = os.path.basename(directory_realpath) - + if os.path.exists(directory): print('\t==> Removing previous plugin directory', directory, flush=True) shutil.rmtree(directory, ignore_errors=True) os.mkdir(directory) - + print('\t==> Extracting package archive', archive, flush=True) with tarfile.open(archive, 'r:*') as tar: for member in tar.getmembers(): if member.isreg(): if not member.name.startswith('package/'): raise InstallException(f"NPM package archive does not start with 'package/' as it should: {member.name}") - + if member.size > self.max_entry_size: raise InstallException(f'Zip bomb detected in {member.name}') - + member.name = member.name.removeprefix('package/') tar.extract(member, path=directory, filter='tar') - + elif member.isdir(): print('\t\tSkipping directory entry', member.name, flush=True) - + elif member.islnk() or member.issym(): if not member.linkpath.startswith('package/'): raise InstallException(f'NPM package archive contains a link outside of the archive: {member.name} -> {member.linkpath}') - + member.name = member.name.removeprefix('package/') member.linkpath = member.linkpath.removeprefix('package/') - + realpath = os.path.realpath(os.path.join(directory, *os.path.split(member.linkname))) if not realpath.startswith(directory_realpath): raise InstallException(f'NPM package archive contains a link outside of the archive: {member.name} -> {member.linkpath}') - + tar.extract(member, path=directory, filter='tar') - + else: type_mapping = { tarfile.CHRTYPE: "character device", - tarfile.BLKTYPE: "block device", + tarfile.BLKTYPE: "block device", tarfile.FIFOTYPE: "FIFO" } type_str = type_mapping.get(member.type, "unknown") raise InstallException(f'NPM package archive contains a non regular file: {member.name} - {type_str}') - + print('\t==> Removing package archive', archive, flush=True) os.remove(archive) - + return plugin_path def create_plugin_installer(package: str, destination: str, skip_integrity_check: bool = False) -> PluginInstaller: @@ -659,15 +659,15 @@ def create_plugin_installer(package: str, destination: str, skip_integrity_check def install_plugin(plugin: dict, plugin_path_by_hash: dict, destination: str, skip_integrity_check: bool = False) -> tuple[str, dict]: """Install a single plugin and handle configuration merging.""" package = plugin['package'] - + # Check if plugin is disabled if plugin.get('disabled', False): print(f'\n======= Skipping disabled dynamic plugin {package}', flush=True) return None, {} - + # Create appropriate installer installer = create_plugin_installer(package, destination, skip_integrity_check) - + # Check if installation should be skipped should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) if should_skip: @@ -676,18 +676,18 @@ def install_plugin(plugin: dict, plugin_path_by_hash: dict, destination: str, sk if plugin['hash'] in plugin_path_by_hash: plugin_path_by_hash.pop(plugin['hash']) return None, plugin.get('pluginConfig', {}) - + # Install the plugin print(f'\n======= Installing dynamic plugin {package}', flush=True) plugin_path = installer.install(plugin, plugin_path_by_hash) - + # Create hash file for tracking hash_file_path = os.path.join(destination, plugin_path, 'dynamic-plugin-config.hash') with open(hash_file_path, 'w') as f: f.write(plugin['hash']) - + print(f'\t==> Successfully installed dynamic plugin {package}', flush=True) - + return plugin_path, plugin.get('pluginConfig', {}) RECOGNIZED_ALGORITHMS = ( @@ -703,9 +703,9 @@ def get_local_package_info(package_path: str) -> dict: abs_package_path = os.path.join(os.getcwd(), package_path[2:]) else: abs_package_path = package_path - + package_json_path = os.path.join(abs_package_path, 'package.json') - + if not os.path.isfile(package_json_path): # If no package.json, fall back to directory modification time if os.path.isdir(abs_package_path): @@ -713,25 +713,25 @@ def get_local_package_info(package_path: str) -> dict: return {'_directory_mtime': mtime} else: return {'_not_found': True} - + with open(package_json_path, 'r') as f: package_json = json.load(f) - + # Extract relevant fields that indicate package changes info = {} info['_package_json'] = package_json - + # Also include package.json modification time as additional change detection info['_package_json_mtime'] = os.path.getmtime(package_json_path) - + # Include package-lock.json or yarn.lock modification time if present for lock_file in ['package-lock.json', 'yarn.lock']: lock_path = os.path.join(abs_package_path, lock_file) if os.path.isfile(lock_path): info[f'_{lock_file}_mtime'] = os.path.getmtime(lock_path) - + return info - + except (json.JSONDecodeError, OSError, IOError) as e: # If we can't read the package info, include the error in hash # This ensures we'll try to reinstall if there are permission issues, etc. @@ -977,7 +977,7 @@ def main(): for plugin in plugins: mergePlugin(plugin, allPlugins, dynamicPluginsFile, level=1) - + # add a hash for each plugin configuration to detect changes and check if version field is set for OCI packages for plugin in allPlugins.values(): hash_dict = copy.deepcopy(plugin) @@ -985,12 +985,12 @@ def main(): hash_dict.pop('pluginConfig', None) # Don't track the internal version field used to track version inheritance hash_dict.pop('version', None) - + package = plugin['package'] if package.startswith('./'): local_info = get_local_package_info(package) hash_dict['_local_package_info'] = local_info - + hash = hashlib.sha256(json.dumps(hash_dict, sort_keys=True).encode('utf-8')).hexdigest() plugin['hash'] = hash @@ -1004,11 +1004,11 @@ def main(): with open(hash_file_path, 'r') as hash_file: hash_value = hash_file.read().strip() plugin_path_by_hash[hash_value] = dir_name - + # iterate through the list of plugins for plugin in allPlugins.values(): _, plugin_config = install_plugin(plugin, plugin_path_by_hash, dynamicPluginsRoot, skipIntegrityCheck) - + # Merge plugin configuration if provided if plugin_config: globalConfig = maybeMergeConfig(plugin_config, globalConfig) From ff65e78cc8847a6828f66983e8636a5a31672b38 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Wed, 26 Nov 2025 12:40:03 +0000 Subject: [PATCH 05/22] Restore test file from upstream/main to preserve original trailing whitespace,while keeping only the functional additions for catalog index testing Signed-off-by: Fortune Ndlovu --- docker/test_install-dynamic-plugins.py | 720 ++++++++++++------------- 1 file changed, 359 insertions(+), 361 deletions(-) diff --git a/docker/test_install-dynamic-plugins.py b/docker/test_install-dynamic-plugins.py index fe2fd51d2a..3353c70760 100644 --- a/docker/test_install-dynamic-plugins.py +++ b/docker/test_install-dynamic-plugins.py @@ -20,7 +20,7 @@ - OciPackageMerger.parse_plugin_key() - Parsing OCI package formats - NPMPackageMerger.merge_plugin() - Plugin config merging and override logic - OciPackageMerger.merge_plugin() - OCI plugin merging with version inheritance -- extract_catalog_index() - Catalog index OCI image extraction and validation +- extract_catalog_index() - Extracting plugin catalog index from OCI images Installation: To install test dependencies: @@ -29,11 +29,10 @@ Running tests: Run all tests: $ pytest test_install-dynamic-plugins.py -v - + Run specific test class: $ pytest test_install-dynamic-plugins.py::TestNPMPackageMergerParsePluginKey -v - $ pytest test_install-dynamic-plugins.py::TestExtractCatalogIndex -v - + Run with coverage: $ pytest test_install-dynamic-plugins.py --cov -v """ @@ -62,13 +61,13 @@ class TestNPMPackageMergerParsePluginKey: """Test cases for NPMPackageMerger.parse_plugin_key() method.""" - + @pytest.fixture def npm_merger(self): """Create an NPMPackageMerger instance for testing.""" plugin = {'package': 'test-package'} return NPMPackageMerger(plugin, 'test-file.yaml', {}) - + @pytest.mark.parametrize("input_package,expected_output", [ # Standard NPM packages with version stripping ('@npmcli/arborist@latest', '@npmcli/arborist'), @@ -77,18 +76,18 @@ def npm_merger(self): ('package-name@^1.0.0', 'package-name'), ('package-name@~2.1.0', 'package-name'), ('package-name@1.x', 'package-name'), - + # Packages without version (unchanged) ('package-name', 'package-name'), ('@scope/package', '@scope/package'), - + # NPM aliases with version stripping ('semver:@npm:semver@7.2.2', 'semver:@npm:semver'), ('my-alias@npm:@npmcli/semver-with-patch', 'my-alias@npm:@npmcli/semver-with-patch'), ('semver:@npm:@npmcli/semver-with-patch@1.0.0', 'semver:@npm:@npmcli/semver-with-patch'), ('alias@npm:package@1.0.0', 'alias@npm:package'), ('alias@npm:@scope/package@2.0.0', 'alias@npm:@scope/package'), - + # Git URLs with ref stripping ('npm/cli#c12ea07', 'npm/cli'), ('user/repo#main', 'user/repo'), @@ -99,11 +98,11 @@ def npm_merger(self): ('git+ssh://git@github.com/user/repo.git#tag', 'git+ssh://git@github.com/user/repo.git'), ('git://github.com/user/repo#commit', 'git://github.com/user/repo'), ('https://github.com/user/repo.git#v1.0.0', 'https://github.com/user/repo.git'), - + # Local paths (unchanged) ('./my-local-plugin', './my-local-plugin'), ('./path/to/plugin', './path/to/plugin'), - + # Tarballs (unchanged) ('package.tgz', 'package.tgz'), ('my-package-1.0.0.tgz', 'my-package-1.0.0.tgz'), @@ -117,13 +116,13 @@ def test_parse_plugin_key_success_cases(self, npm_merger, input_package, expecte class TestOciPackageMergerParsePluginKey: """Test cases for OciPackageMerger.parse_plugin_key() method.""" - + @pytest.fixture def oci_merger(self): """Create an OciPackageMerger instance for testing.""" plugin = {'package': 'oci://example.com:v1.0!plugin'} return OciPackageMerger(plugin, 'test-file.yaml', {}) - + @pytest.mark.parametrize("input_package,expected_key,expected_version,expected_inherit", [ # Tag-based packages ( @@ -150,7 +149,7 @@ def oci_merger(self): 'v2.0.0', False ), - + # Digest-based packages with different algorithms ( 'oci://quay.io/user/plugin@sha256:abc123def456!plugin', @@ -170,7 +169,7 @@ def oci_merger(self): 'blake3:1234567890abcdef', False ), - + # Inherit version pattern ( 'oci://quay.io/user/plugin:{{inherit}}!plugin', @@ -190,36 +189,36 @@ def test_parse_plugin_key_success_cases( ): """Test that parse_plugin_key correctly parses valid OCI package formats.""" plugin_key, version, inherit_version = oci_merger.parse_plugin_key(input_package) - + assert plugin_key == expected_key, f"Expected key {expected_key}, got {plugin_key}" assert version == expected_version, f"Expected version {expected_version}, got {version}" assert inherit_version == expected_inherit, f"Expected inherit {expected_inherit}, got {inherit_version}" - + @pytest.mark.parametrize("invalid_package,error_substring", [ # Missing ! separator ('oci://registry.io/plugin:v1.0', 'not in the expected format'), - + # Missing tag/digest ('oci://registry.io/plugin!path', 'not in the expected format'), - + # Invalid format - no tag or digest before ! ('oci://registry.io!path', 'not in the expected format'), - + # Invalid digest algorithm (md5 not in RECOGNIZED_ALGORITHMS) ('oci://registry.io/plugin@md5:abc123!plugin', 'not in the expected format'), - + # Invalid format - multiple @ symbols ('oci://registry.io/plugin@@sha256:abc!plugin', 'not in the expected format'), - + # Invalid format - multiple : symbols in tag ('oci://registry.io/plugin:v1:v2!plugin', 'not in the expected format'), - + # Empty tag ('oci://registry.io/plugin:!plugin', 'not in the expected format'), - + # Empty path after ! ('oci://registry.io/plugin:v1.0!', 'not in the expected format'), - + # No oci:// prefix (but this should fail the regex) ('registry.io/plugin:v1.0!plugin', 'not in the expected format'), ]) @@ -227,37 +226,37 @@ def test_parse_plugin_key_error_cases(self, oci_merger, invalid_package, error_s """Test that parse_plugin_key raises InstallException for invalid OCI package formats.""" with pytest.raises(InstallException) as exc_info: oci_merger.parse_plugin_key(invalid_package) - + assert error_substring in str(exc_info.value), \ f"Expected error message to contain '{error_substring}', got: {str(exc_info.value)}" - + def test_parse_plugin_key_complex_digest(self, oci_merger): """Test parsing OCI package with complex digest value.""" # Note: The pattern allows any value after @ including special strings like {{inherit}} # though this would be semantically incorrect for digest format input_pkg = 'oci://registry.io/plugin@sha256:abc123def456789!plugin' plugin_key, version, inherit = oci_merger.parse_plugin_key(input_pkg) - + assert plugin_key == 'oci://registry.io/plugin:!plugin' assert version == 'sha256:abc123def456789' assert inherit is False - + def test_parse_plugin_key_strips_version_from_key(self, oci_merger): """Test that the plugin key does not contain version information.""" input_pkg = 'oci://quay.io/user/plugin:v1.0.0!my-plugin' plugin_key, version, _ = oci_merger.parse_plugin_key(input_pkg) - + # The key should not contain the version assert ':v1.0.0' not in plugin_key assert plugin_key == 'oci://quay.io/user/plugin:!my-plugin' # But the version should be returned separately assert version == 'v1.0.0' - + def test_parse_plugin_key_with_nested_path(self, oci_merger): """Test parsing OCI package with nested path after !.""" input_pkg = 'oci://registry.io/plugin:v1.0!path/to/nested/plugin' plugin_key, version, inherit = oci_merger.parse_plugin_key(input_pkg) - + assert plugin_key == 'oci://registry.io/plugin:!path/to/nested/plugin' assert version == 'v1.0' assert inherit is False @@ -265,32 +264,32 @@ def test_parse_plugin_key_with_nested_path(self, oci_merger): class TestEdgeCases: """Test edge cases and boundary conditions.""" - + def test_npm_merger_empty_string(self): """Test NPM merger with empty package string.""" plugin = {'package': ''} merger = NPMPackageMerger(plugin, 'test.yaml', {}) result = merger.parse_plugin_key('') assert result == '' - + def test_npm_merger_special_characters_in_package(self): """Test NPM packages with special characters.""" plugin = {'package': 'test'} merger = NPMPackageMerger(plugin, 'test.yaml', {}) - + # Package name with underscores and hyphens result = merger.parse_plugin_key('my_special-package@1.0.0') assert result == 'my_special-package' - + def test_oci_merger_long_digest(self): """Test OCI package with realistic long SHA256 digest.""" plugin = {'package': 'oci://example.com:v1!plugin'} merger = OciPackageMerger(plugin, 'test.yaml', {}) - + long_digest = 'sha256:' + 'a' * 64 input_pkg = f'oci://quay.io/user/plugin@{long_digest}!plugin' plugin_key, version, inherit = merger.parse_plugin_key(input_pkg) - + assert plugin_key == 'oci://quay.io/user/plugin:!plugin' assert version == long_digest assert inherit is False @@ -298,45 +297,45 @@ def test_oci_merger_long_digest(self): class TestNPMPackageMergerMergePlugin: """Test cases for NPMPackageMerger.merge_plugin() method.""" - + def test_add_new_plugin_level_0(self): """Test adding a new plugin at level 0.""" all_plugins = {} plugin = {'package': 'test-package@1.0.0', 'disabled': False} merger = NPMPackageMerger(plugin, 'test-file.yaml', all_plugins) - + merger.merge_plugin(level=0) - + # Check plugin was added assert 'test-package' in all_plugins assert all_plugins['test-package']['package'] == 'test-package@1.0.0' assert all_plugins['test-package']['disabled'] is False assert all_plugins['test-package']['last_modified_level'] == 0 - + def test_override_plugin_level_0_to_1(self): """Test overriding a plugin from level 0 to level 1.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'test-package@1.0.0', 'disabled': False} merger1 = NPMPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 plugin2 = {'package': 'test-package@2.0.0', 'disabled': True} merger2 = NPMPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check override succeeded assert all_plugins['test-package']['disabled'] is True assert all_plugins['test-package']['last_modified_level'] == 1 # Package field should be overridden assert all_plugins['test-package']['package'] == 'test-package@2.0.0' - + def test_override_multiple_config_fields(self): """Test overriding multiple plugin config fields.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = { 'package': '@scope/plugin@1.0.0', @@ -346,7 +345,7 @@ def test_override_multiple_config_fields(self): } merger1 = NPMPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 plugin2 = { 'package': '@scope/plugin@2.0.0', @@ -357,7 +356,7 @@ def test_override_multiple_config_fields(self): } merger2 = NPMPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check all fields were updated except package assert all_plugins['@scope/plugin']['disabled'] is True assert all_plugins['@scope/plugin']['pullPolicy'] == 'Always' @@ -365,74 +364,74 @@ def test_override_multiple_config_fields(self): assert all_plugins['@scope/plugin']['integrity'] == 'sha256-abc123' # Package field not overridden assert all_plugins['@scope/plugin']['package'] == '@scope/plugin@2.0.0' - + def test_duplicate_plugin_same_level_0_raises_error(self): """Test that duplicate plugin at same level 0 raises InstallException.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'duplicate-package@1.0.0'} merger1 = NPMPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Try to add same plugin again at level 0 plugin2 = {'package': 'duplicate-package@2.0.0'} merger2 = NPMPackageMerger(plugin2, 'included-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger2.merge_plugin(level=0) - + assert 'Duplicate plugin configuration' in str(exc_info.value) assert 'duplicate-package@2.0.0' in str(exc_info.value) - + def test_duplicate_plugin_same_level_1_raises_error(self): """Test that duplicate plugin at same level 1 raises InstallException.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'test-package@1.0.0'} merger1 = NPMPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 plugin2 = {'package': 'test-package@2.0.0'} merger2 = NPMPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Try to add same plugin again at level 1 plugin3 = {'package': 'test-package@3.0.0'} merger3 = NPMPackageMerger(plugin3, 'main-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger3.merge_plugin(level=1) - + assert 'Duplicate plugin configuration' in str(exc_info.value) - + def test_invalid_package_field_type_raises_error(self): """Test that non-string package field raises InstallException.""" all_plugins = {} plugin = {'package': 123} merger = NPMPackageMerger(plugin, 'test-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger.merge_plugin(level=0) - + assert 'must be a string' in str(exc_info.value) - + def test_version_stripping_in_plugin_key(self): """Test that version is stripped from plugin key.""" all_plugins = {} - + # Add plugin with version plugin1 = {'package': 'my-plugin@1.0.0'} merger1 = NPMPackageMerger(plugin1, 'test-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override with different version plugin2 = {'package': 'my-plugin@2.0.0', 'disabled': True} merger2 = NPMPackageMerger(plugin2, 'test-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Both should map to same key assert 'my-plugin' in all_plugins assert all_plugins['my-plugin']['disabled'] is True @@ -440,73 +439,73 @@ def test_version_stripping_in_plugin_key(self): class TestOciPackageMergerMergePlugin: """Test cases for OciPackageMerger.merge_plugin() method.""" - + def test_add_new_plugin_with_tag(self): """Test adding a new OCI plugin with tag.""" all_plugins = {} plugin = {'package': 'oci://registry.io/plugin:v1.0!path'} merger = OciPackageMerger(plugin, 'test-file.yaml', all_plugins) - + merger.merge_plugin(level=0) - + plugin_key = 'oci://registry.io/plugin:!path' assert plugin_key in all_plugins assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin:v1.0!path' assert all_plugins[plugin_key]['version'] == 'v1.0' assert all_plugins[plugin_key]['last_modified_level'] == 0 - + def test_add_new_plugin_with_digest(self): """Test adding a new OCI plugin with digest.""" all_plugins = {} plugin = {'package': 'oci://registry.io/plugin@sha256:abc123!path'} merger = OciPackageMerger(plugin, 'test-file.yaml', all_plugins) - + merger.merge_plugin(level=0) - + plugin_key = 'oci://registry.io/plugin:!path' assert plugin_key in all_plugins assert all_plugins[plugin_key]['version'] == 'sha256:abc123' - + def test_override_plugin_version(self, capsys): """Test overriding OCI plugin version from level 0 to 1.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 with new version plugin2 = {'package': 'oci://registry.io/plugin:v2.0!path'} merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check version was updated plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'v2.0' assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin:v2.0!path' assert all_plugins[plugin_key]['last_modified_level'] == 1 - + # Check that override message was printed captured = capsys.readouterr() assert 'Overriding version' in captured.out assert 'v1.0' in captured.out assert 'v2.0' in captured.out - + def test_use_inherit_to_preserve_version(self): """Test using {{inherit}} to preserve existing version.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 with {{inherit}} plugin2 = {'package': 'oci://registry.io/plugin:{{inherit}}!path', 'disabled': True} merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check version was preserved plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'v1.0' @@ -514,11 +513,11 @@ def test_use_inherit_to_preserve_version(self): assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin:v1.0!path' # But other config should be updated assert all_plugins[plugin_key]['disabled'] is True - + def test_override_config_with_version_inheritance(self): """Test overriding plugin config while preserving version with {{inherit}}.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = { 'package': 'oci://registry.io/plugin:v1.0!path', @@ -526,7 +525,7 @@ def test_override_config_with_version_inheritance(self): } merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override config at level 1 with {{inherit}} plugin2 = { 'package': 'oci://registry.io/plugin:{{inherit}}!path', @@ -534,16 +533,16 @@ def test_override_config_with_version_inheritance(self): } merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check version preserved and config updated plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'v1.0' assert all_plugins[plugin_key]['pluginConfig'] == {'key2': 'value2'} - + def test_override_config_without_version_inheritance(self): """Test overriding both version and config.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = { 'package': 'oci://registry.io/plugin:v1.0!path', @@ -551,7 +550,7 @@ def test_override_config_without_version_inheritance(self): } merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override both at level 1 plugin2 = { 'package': 'oci://registry.io/plugin:v2.0!path', @@ -559,111 +558,111 @@ def test_override_config_without_version_inheritance(self): } merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check both were updated plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'v2.0' assert all_plugins[plugin_key]['pluginConfig'] == {'key2': 'value2'} assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin:v2.0!path' - + def test_override_from_tag_to_digest(self): """Test overriding from tag to digest.""" all_plugins = {} - + # Add plugin with tag at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override with digest at level 1 plugin2 = {'package': 'oci://registry.io/plugin@sha256:abc123def456!path'} merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Check version updated to digest format plugin_key = 'oci://registry.io/plugin:!path' assert all_plugins[plugin_key]['version'] == 'sha256:abc123def456' assert all_plugins[plugin_key]['package'] == 'oci://registry.io/plugin@sha256:abc123def456!path' - + def test_new_plugin_with_inherit_raises_error(self): """Test that using {{inherit}} on a new plugin raises InstallException.""" all_plugins = {} plugin = {'package': 'oci://registry.io/plugin:{{inherit}}!path'} merger = OciPackageMerger(plugin, 'test-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger.merge_plugin(level=0) - + assert '{{inherit}}' in str(exc_info.value) assert 'no resolved tag or digest' in str(exc_info.value) - + def test_duplicate_oci_plugin_same_level_0_raises_error(self): """Test that duplicate OCI plugin at same level 0 raises InstallException.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Try to add same plugin again at level 0 plugin2 = {'package': 'oci://registry.io/plugin:v2.0!path'} merger2 = OciPackageMerger(plugin2, 'included-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger2.merge_plugin(level=0) - + assert 'Duplicate plugin configuration' in str(exc_info.value) - + def test_duplicate_oci_plugin_same_level_1_raises_error(self): """Test that duplicate OCI plugin at same level 1 raises InstallException.""" all_plugins = {} - + # Add plugin at level 0 plugin1 = {'package': 'oci://registry.io/plugin:v1.0!path'} merger1 = OciPackageMerger(plugin1, 'included-file.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Override at level 1 plugin2 = {'package': 'oci://registry.io/plugin:v2.0!path'} merger2 = OciPackageMerger(plugin2, 'main-file.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Try to add same plugin again at level 1 plugin3 = {'package': 'oci://registry.io/plugin:v3.0!path'} merger3 = OciPackageMerger(plugin3, 'main-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger3.merge_plugin(level=1) - + assert 'Duplicate plugin configuration' in str(exc_info.value) - + def test_invalid_package_field_type_raises_error(self): """Test that non-string package field raises InstallException.""" all_plugins = {} plugin = {'package': ['not', 'a', 'string']} merger = OciPackageMerger(plugin, 'test-file.yaml', all_plugins) - + with pytest.raises(InstallException) as exc_info: merger.merge_plugin(level=0) - + assert 'must be a string' in str(exc_info.value) class TestPluginInstallerShouldSkipInstallation: """Test cases for PluginInstaller.should_skip_installation() method.""" - + def test_plugin_not_installed_returns_false(self, tmp_path): """Test that plugin not in hash dict returns False.""" plugin = {'hash': 'abc123', 'package': 'test-pkg'} plugin_path_by_hash = {} # Empty - nothing installed installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "not_installed" - + def test_plugin_installed_if_not_present_skips(self, tmp_path): """Test that installed plugin with IF_NOT_PRESENT policy skips.""" plugin = { @@ -673,12 +672,12 @@ def test_plugin_installed_if_not_present_skips(self, tmp_path): } plugin_path_by_hash = {'abc123': 'test-pkg-1.0.0'} installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is True assert reason == "already_installed" - + def test_plugin_installed_always_policy_forces_download(self, tmp_path): """Test that ALWAYS policy forces download.""" plugin = { @@ -688,12 +687,12 @@ def test_plugin_installed_always_policy_forces_download(self, tmp_path): } plugin_path_by_hash = {'abc123': 'test-pkg-1.0.0'} installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "force_download" - + def test_plugin_installed_force_download_flag(self, tmp_path): """Test that forceDownload flag forces download.""" plugin = { @@ -703,27 +702,27 @@ def test_plugin_installed_force_download_flag(self, tmp_path): } plugin_path_by_hash = {'abc123': 'test-pkg-1.0.0'} installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "force_download" - + def test_default_pull_policy_if_not_present(self, tmp_path): """Test that default pull policy is IF_NOT_PRESENT.""" plugin = {'hash': 'abc123', 'package': 'test-pkg'} # No pullPolicy plugin_path_by_hash = {'abc123': 'test-pkg-1.0.0'} installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is True assert reason == "already_installed" class TestOciPluginInstallerShouldSkipInstallation: """Test cases for OciPluginInstaller.should_skip_installation() method.""" - + def test_plugin_not_installed_returns_false(self, tmp_path, mocker): """Test that plugin not in hash dict returns False.""" plugin = { @@ -731,17 +730,17 @@ def test_plugin_not_installed_returns_false(self, tmp_path, mocker): 'package': 'oci://registry.io/plugin:latest!path' } plugin_path_by_hash = {} - + # Mock OciDownloader mock_downloader = mocker.MagicMock() installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "not_installed" - + def test_always_policy_unchanged_digest_skips(self, tmp_path, mocker): """Test that ALWAYS policy with unchanged digest skips download.""" plugin_path = 'plugin-dir' @@ -751,24 +750,24 @@ def test_always_policy_unchanged_digest_skips(self, tmp_path, mocker): 'pullPolicy': 'Always' } plugin_path_by_hash = {'abc123': plugin_path} - + # Create digest file with matching digest digest_file = tmp_path / plugin_path / 'dynamic-plugin-image.hash' digest_file.parent.mkdir(parents=True) digest_file.write_text('matching_digest') - + # Mock downloader to return same digest mock_downloader = mocker.MagicMock() mock_downloader.digest.return_value = 'matching_digest' - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is True assert reason == "digest_unchanged" - + def test_always_policy_changed_digest_forces_download(self, tmp_path, mocker): """Test that ALWAYS policy with changed digest forces download.""" plugin_path = 'plugin-dir' @@ -778,21 +777,21 @@ def test_always_policy_changed_digest_forces_download(self, tmp_path, mocker): 'pullPolicy': 'Always' } plugin_path_by_hash = {'abc123': plugin_path} - + # Create digest file with old digest digest_file = tmp_path / plugin_path / 'dynamic-plugin-image.hash' digest_file.parent.mkdir(parents=True) digest_file.write_text('old_digest') - + # Mock downloader to return different digest mock_downloader = mocker.MagicMock() mock_downloader.digest.return_value = 'new_digest' - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is False assert reason == "force_download" def test_if_not_present_policy_skips(self, tmp_path, mocker): @@ -804,28 +803,28 @@ def test_if_not_present_policy_skips(self, tmp_path, mocker): 'pullPolicy': 'IfNotPresent' } plugin_path_by_hash = {'abc123': plugin_path} - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) should_skip, reason = installer.should_skip_installation(plugin, plugin_path_by_hash) - + assert should_skip is True assert reason == "already_installed" class TestNpmPluginInstallerInstall: """Test cases for NpmPluginInstaller.install() method and verify_package_integrity() (mocked).""" - + def test_missing_integrity_remote_package_raises_exception(self, tmp_path): """Test that missing integrity for remote package raises exception.""" plugin = {'package': 'test-package@1.0.0'} # No integrity plugin_path_by_hash = {} - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path), skip_integrity_check=False) - + with pytest.raises(InstallException) as exc_info: installer.install(plugin, plugin_path_by_hash) - + assert 'No integrity hash provided' in str(exc_info.value) - + def test_invalid_integrity_hash_type_raises_exception(self, tmp_path, mocker): """Test that invalid integrity hash type raises exception.""" plugin = {'package': 'test-package@1.0.0', 'integrity': 1234567890} @@ -845,7 +844,7 @@ def test_invalid_integrity_hash_format_raises_exception(self, tmp_path, mocker): def test_invalid_integrity_algorithm_raises_exception(self, tmp_path, mocker): """Test that unrecognized integrity algorithm raises exception.""" plugin = {'package': 'test-package@1.0.0', 'integrity': 'invalidalgo-1234567890abcdef'} - + with pytest.raises(InstallException) as exc_info: install_dynamic_plugins.verify_package_integrity(plugin, "dummy-archive.tgz", str(tmp_path)) assert 'is not supported' in str(exc_info.value) @@ -853,7 +852,7 @@ def test_invalid_integrity_algorithm_raises_exception(self, tmp_path, mocker): def test_invalid_integrity_hash_base64_encoding_raises_exception(self, tmp_path, mocker): """Test invalid base64 encoding in hash triggers exception.""" plugin = {'package': 'test-package@1.0.0', 'integrity': 'sha256-not@base64!'} - + with pytest.raises(InstallException) as exc_info: install_dynamic_plugins.verify_package_integrity(plugin, "dummy-archive.tgz", str(tmp_path)) assert 'is not a valid base64 encoding' in str(exc_info.value) @@ -871,53 +870,53 @@ def test_skip_integrity_check_flag_works(self, tmp_path, mocker): """Test that skip_integrity_check flag bypasses integrity check.""" plugin = {'package': 'test-package@1.0.0'} # No integrity plugin_path_by_hash = {} - + # Mock npm pack mock_result = mocker.MagicMock() mock_result.returncode = 0 mock_result.stdout = b'test-package-1.0.0.tgz' mocker.patch('subprocess.run', return_value=mock_result) - + # Mock tarball extraction mock_tarfile = mocker.patch('tarfile.open') mock_tar = mocker.MagicMock() mock_tar.getmembers.return_value = [] mock_tarfile.return_value.__enter__.return_value = mock_tar - + # Mock file operations mocker.patch('os.path.exists', return_value=False) mocker.patch('os.mkdir') mocker.patch('os.remove') - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path), skip_integrity_check=True) plugin_path = installer.install(plugin, plugin_path_by_hash) - + assert plugin_path == 'test-package-1.0.0' @pytest.mark.integration class TestNpmPluginInstallerIntegration: """Integration tests with real file operations.""" - + @pytest.mark.integration def test_verify_package_integrity_with_real_tarball(self, tmp_path): """Test integrity verification with actual openssl commands.""" import tarfile import subprocess import shutil - + # Skip if openssl not available if not shutil.which('openssl'): pytest.skip("openssl not available") - + # Create a real test tarball test_dir = tmp_path / "test-package" test_dir.mkdir() (test_dir / "index.js").write_text("console.log('test');") - + tarball_path = tmp_path / "test-package.tgz" with tarfile.open(tarball_path, "w:gz") as tar: tar.add(test_dir, arcname="package") - + # Calculate actual integrity hash using openssl cat_process = subprocess.Popen(["cat", str(tarball_path)], stdout=subprocess.PIPE) openssl_dgst = subprocess.Popen( @@ -932,32 +931,32 @@ def test_verify_package_integrity_with_real_tarball(self, tmp_path): ) integrity_hash, _ = openssl_b64.communicate() integrity_hash = integrity_hash.decode('utf-8').strip() - + # Create plugin with real integrity plugin = { 'package': 'test-package', 'integrity': f'sha256-{integrity_hash}' } - + # Test verification succeeds with correct hash install_dynamic_plugins.verify_package_integrity(plugin, str(tarball_path), str(tmp_path)) - + # Test verification fails with wrong hash (valid base64 but wrong hash) plugin_wrong = { 'package': 'test-package', 'integrity': 'sha256-YWJjZGVmZ2hpamtsbW5vcHFyc3R1dnd4eXowMTIzNDU2' } - + with pytest.raises(InstallException) as exc_info: install_dynamic_plugins.verify_package_integrity(plugin_wrong, str(tarball_path), str(tmp_path)) - + assert 'does not match' in str(exc_info.value) - + @pytest.mark.integration def test_extract_npm_package_with_real_tarball(self, tmp_path): """Test tarball extraction with real tar file.""" import tarfile - + # Create a realistic NPM package structure package_dir = tmp_path / "source" / "package" package_dir.mkdir(parents=True) @@ -965,55 +964,55 @@ def test_extract_npm_package_with_real_tarball(self, tmp_path): (package_dir / "index.js").write_text("module.exports = {};") (package_dir / "lib").mkdir() (package_dir / "lib" / "helper.js").write_text("exports.helper = () => {};") - + # Create tarball following NPM format (with 'package/' prefix) tarball_path = tmp_path / "test-package-1.0.0.tgz" with tarfile.open(tarball_path, "w:gz") as tar: tar.add(package_dir, arcname="package") - + # Test extraction installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) plugin_path = installer._extract_npm_package(str(tarball_path)) - + # Verify extracted files extracted_dir = tmp_path / "test-package-1.0.0" assert extracted_dir.exists() assert (extracted_dir / "package.json").exists() assert (extracted_dir / "index.js").exists() assert (extracted_dir / "lib" / "helper.js").exists() - + # Verify tarball was removed assert not tarball_path.exists() - + @pytest.mark.integration def test_zip_bomb_protection_real_tarball(self, tmp_path): """Test that extraction rejects tarballs with oversized files.""" import tarfile - + # Create a tarball with a file exceeding MAX_ENTRY_SIZE large_content = b"x" * 25_000_000 # 25MB (exceeds default 20MB) - + package_dir = tmp_path / "source" / "package" package_dir.mkdir(parents=True) (package_dir / "huge-file.bin").write_bytes(large_content) - + tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: tar.add(package_dir / "huge-file.bin", arcname="package/huge-file.bin") - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'Zip bomb' in str(exc_info.value) - + @pytest.mark.integration def test_path_traversal_protection_real_tarball(self, tmp_path): """Test that extraction rejects tarballs with without package/ prefix.""" import tarfile import io - + # Create tarball with path traversal attempt tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1021,20 +1020,20 @@ def test_path_traversal_protection_real_tarball(self, tmp_path): info = tarfile.TarInfo(name="test") info.size = 10 tar.addfile(info, io.BytesIO(b"malicious!")) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'does not start with' in str(exc_info.value) - + @pytest.mark.integration def test_symlink_with_invalid_linkpath_prefix(self, tmp_path): """Test that extraction rejects symlinks with linkpath not starting with 'package/'.""" import tarfile import io - + # Create tarball with a symlink that has invalid linkpath prefix tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1042,27 +1041,27 @@ def test_symlink_with_invalid_linkpath_prefix(self, tmp_path): info = tarfile.TarInfo(name="package/index.js") info.size = 10 tar.addfile(info, io.BytesIO(b"console.log")) - + # Add a symlink with linkpath not starting with 'package/' link_info = tarfile.TarInfo(name="package/malicious-link") link_info.type = tarfile.SYMTYPE link_info.linkname = "../../../etc/passwd" # Does not start with 'package/' tar.addfile(link_info) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'contains a link outside of the archive' in str(exc_info.value) assert 'malicious-link' in str(exc_info.value) - + @pytest.mark.integration def test_symlink_resolving_outside_directory(self, tmp_path): """Test that extraction rejects symlinks that resolve outside the target directory.""" import tarfile import io - + # Create tarball with a symlink that resolves outside the extraction directory tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1070,27 +1069,27 @@ def test_symlink_resolving_outside_directory(self, tmp_path): info = tarfile.TarInfo(name="package/index.js") info.size = 10 tar.addfile(info, io.BytesIO(b"console.log")) - + # Add a symlink with proper prefix but resolves outside # Using relative path traversal that starts with package/ but goes outside link_info = tarfile.TarInfo(name="package/subdir/malicious-link") link_info.type = tarfile.SYMTYPE link_info.linkname = "package/../../../etc/passwd" # Starts with 'package/' but resolves outside tar.addfile(link_info) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'contains a link outside of the archive' in str(exc_info.value) - + @pytest.mark.integration def test_hardlink_resolving_outside_directory(self, tmp_path): """Test that extraction rejects hardlinks that resolve outside the target directory.""" import tarfile import io - + # Create tarball with a hardlink that resolves outside the extraction directory tarball_path = tmp_path / "malicious.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1098,26 +1097,26 @@ def test_hardlink_resolving_outside_directory(self, tmp_path): info = tarfile.TarInfo(name="package/index.js") info.size = 10 tar.addfile(info, io.BytesIO(b"console.log")) - + # Add a hardlink with proper prefix but resolves outside link_info = tarfile.TarInfo(name="package/subdir/malicious-hardlink") link_info.type = tarfile.LNKTYPE link_info.linkname = "package/../../../etc/passwd" # Starts with 'package/' but resolves outside tar.addfile(link_info) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer._extract_npm_package(str(tarball_path)) - + assert 'contains a link outside of the archive' in str(exc_info.value) - + @pytest.mark.integration def test_valid_symlink_extraction(self, tmp_path): """Test that valid symlinks within the package are extracted correctly.""" import tarfile import io - + # Create tarball with valid internal symlinks tarball_path = tmp_path / "valid-package.tgz" with tarfile.open(tarball_path, "w:gz") as tar: @@ -1126,41 +1125,41 @@ def test_valid_symlink_extraction(self, tmp_path): content = b"module.exports = { helper: () => {} };" info.size = len(content) tar.addfile(info, io.BytesIO(content)) - + # Add a valid symlink pointing to the file within package/ link_info = tarfile.TarInfo(name="package/index.js") link_info.type = tarfile.SYMTYPE link_info.linkname = "package/lib/helper.js" tar.addfile(link_info) - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) plugin_path = installer._extract_npm_package(str(tarball_path)) - + # Verify extraction succeeded extracted_dir = tmp_path / plugin_path assert extracted_dir.exists() assert (extracted_dir / "lib" / "helper.js").exists() assert (extracted_dir / "index.js").exists() assert (extracted_dir / "index.js").is_symlink() - + @pytest.mark.integration def test_install_real_npm_package(self, tmp_path): """Integration test with actual npm pack on a real package.""" import shutil - + # Only run if npm is available if not shutil.which('npm'): pytest.skip("npm not available") - + plugin = { 'package': 'semver@7.0.0', # Small, stable package 'integrity': 'sha512-+GB6zVA9LWh6zovYQLALHwv5rb2PHGlJi3lfiqIHxR0uuwCgefcOJc59v9fv1w8GbStwxuuqqAjI9NMAOOgq1A==' } plugin_path_by_hash = {} - + installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path), skip_integrity_check=False) plugin_path = installer.install(plugin, plugin_path_by_hash) - + # Verify plugin was installed installed_dir = tmp_path / plugin_path assert installed_dir.exists() @@ -1168,80 +1167,80 @@ def test_install_real_npm_package(self, tmp_path): class TestOciDownloader: """Test cases for OciDownloader class.""" - + def test_skopeo_command_execution(self, tmp_path, mocker): """Test that skopeo commands are executed correctly.""" # Mock shutil.which to return a fake skopeo path mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Mock subprocess.run mock_run = mocker.patch('subprocess.run') mock_run.return_value.returncode = 0 mock_run.return_value.stdout = b'output' - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) result = downloader.skopeo(['inspect', 'docker://example.com/image:latest']) - + # Verify skopeo was called with correct arguments mock_run.assert_called_once() call_args = mock_run.call_args[0][0] assert call_args[0] == '/usr/bin/skopeo' assert call_args[1] == 'inspect' assert result == b'output' - + def test_skopeo_not_found_raises_exception(self, tmp_path, mocker): """Test that missing skopeo raises InstallException.""" mocker.patch('shutil.which', return_value=None) - + with pytest.raises(InstallException) as exc_info: install_dynamic_plugins.OciDownloader(str(tmp_path)) - + assert 'skopeo executable not found' in str(exc_info.value) - + def test_get_plugin_tar_caches_downloads(self, tmp_path, mocker): """Test that get_plugin_tar caches downloaded images.""" mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Mock skopeo copy mock_run = mocker.patch('subprocess.run') mock_run.return_value.returncode = 0 - + # Create fake manifest manifest_data = { 'layers': [{'digest': 'sha256:abc123'}] } - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) - + # Mock the manifest file read mocker.patch('builtins.open', mocker.mock_open(read_data=json.dumps(manifest_data))) mocker.patch('os.path.join', side_effect=lambda *args: '/'.join(args)) - + image = 'oci://registry.io/plugin:v1.0' - + # First call should execute skopeo tar_path1 = downloader.get_plugin_tar(image) - + # Second call should return cached result tar_path2 = downloader.get_plugin_tar(image) - + # Should return same path assert tar_path1 == tar_path2 - + # Verify image is cached assert image in downloader.image_to_tarball - + def test_extract_plugin_with_valid_path(self, tmp_path, mocker): """Test extracting a plugin from a tar file.""" import tarfile import io - + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Create a real test tarball with plugin files plugin_path = "internal-backstage-plugin-test" tarball_path = tmp_path / "test.tar.gz" - + with tarfile.open(tarball_path, "w:gz") as tar: # Add plugin files for filename in ["package.json", "index.js"]: @@ -1249,45 +1248,45 @@ def test_extract_plugin_with_valid_path(self, tmp_path, mocker): content = b'{"name": "test"}' if filename.endswith('.json') else b'console.log("test");' info.size = len(content) tar.addfile(info, io.BytesIO(content)) - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) downloader.extract_plugin(str(tarball_path), plugin_path) - + # Verify files were extracted extracted_dir = tmp_path / plugin_path assert extracted_dir.exists() assert (extracted_dir / "package.json").exists() assert (extracted_dir / "index.js").exists() - + def test_extract_plugin_rejects_oversized_files(self, tmp_path, mocker): """Test that extract_plugin rejects files larger than max_entry_size.""" import tarfile import io - + mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + plugin_path = "plugin" tarball_path = tmp_path / "malicious.tar.gz" - + # Create tarball with oversized file (needs actual content matching size) large_content = b"x" * 25_000_000 # 25MB, exceeds default 20MB - + with tarfile.open(tarball_path, "w:gz") as tar: info = tarfile.TarInfo(name=f"{plugin_path}/huge.bin") info.size = len(large_content) tar.addfile(info, io.BytesIO(large_content)) - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: downloader.extract_plugin(str(tarball_path), plugin_path) - + assert 'Zip bomb' in str(exc_info.value) - + def test_download_removes_previous_installation(self, tmp_path, mocker): """Test that download removes previous plugin directory.""" mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Create existing plugin directory with old content plugin_path = "internal-backstage-plugin-test" existing_dir = tmp_path / plugin_path @@ -1297,67 +1296,67 @@ def test_download_removes_previous_installation(self, tmp_path, mocker): old_subdir = existing_dir / "old-subdir" old_subdir.mkdir() (old_subdir / "old-nested.txt").write_text("old nested content") - + # Verify old content exists before assert existing_dir.exists() assert old_file.exists() assert old_subdir.exists() - + # Mock get_plugin_tar and extract_plugin to simulate extraction downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) mocker.patch.object(downloader, 'get_plugin_tar', return_value='/fake/tar/path') - + def mock_extract(tar_file, plugin_path): # Simulate extraction by creating new files plugin_dir = tmp_path / plugin_path plugin_dir.mkdir(parents=True, exist_ok=True) (plugin_dir / "package.json").write_text('{"name": "new-plugin"}') (plugin_dir / "index.js").write_text("console.log('new');") - + mocker.patch.object(downloader, 'extract_plugin', side_effect=mock_extract) - + package = f'oci://registry.io/plugin:v1.0!{plugin_path}' result = downloader.download(package) - + # Verify extraction was called downloader.extract_plugin.assert_called_once() assert result == plugin_path - + # Verify old content was removed assert not old_file.exists(), "Old file should have been removed" assert not old_subdir.exists(), "Old subdirectory should have been removed" - + # Verify new content exists new_dir = tmp_path / plugin_path assert new_dir.exists(), "New plugin directory should exist" assert (new_dir / "package.json").exists(), "New package.json should exist" assert (new_dir / "index.js").exists(), "New index.js should exist" - + # Verify old content is definitely gone assert not (new_dir / "old-file.txt").exists(), "Old file should not exist in new installation" assert not (new_dir / "old-subdir").exists(), "Old subdirectory should not exist in new installation" - + def test_digest_returns_image_digest(self, tmp_path, mocker): """Test that digest() returns the correct digest from remote image.""" mocker.patch('shutil.which', return_value='/usr/bin/skopeo') - + # Mock skopeo inspect output inspect_output = { 'Digest': 'sha256:abc123def456789' } - + mock_run = mocker.patch('subprocess.run') mock_run.return_value.returncode = 0 mock_run.return_value.stdout = json.dumps(inspect_output).encode('utf-8') - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) package = 'oci://registry.io/plugin:v1.0!path' - + digest = downloader.digest(package) - + # Should return just the hash part assert digest == 'abc123def456789' - + # Verify skopeo inspect was called mock_run.assert_called_once() call_args = mock_run.call_args[0][0] @@ -1367,7 +1366,7 @@ def test_digest_returns_image_digest(self, tmp_path, mocker): class TestOciPluginInstallerInstall: """Test cases for OciPluginInstaller.install() method.""" - + def test_install_creates_digest_file(self, tmp_path, mocker): """Test that install creates a digest file for tracking.""" plugin_path = "test-plugin" @@ -1375,41 +1374,41 @@ def test_install_creates_digest_file(self, tmp_path, mocker): 'package': f'oci://registry.io/plugin:v1.0!{plugin_path}', 'version': 'v1.0' } - + # Mock the downloader mock_downloader = mocker.MagicMock() mock_downloader.download.return_value = plugin_path mock_downloader.digest.return_value = 'abc123digest' - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + # Create the plugin directory that download would create plugin_dir = tmp_path / plugin_path plugin_dir.mkdir() - + result = installer.install(plugin, {}) - + # Verify digest file was created digest_file = plugin_dir / 'dynamic-plugin-image.hash' assert digest_file.exists() assert digest_file.read_text() == 'abc123digest' assert result == plugin_path - + def test_install_missing_version_raises_exception(self, tmp_path, mocker): """Test that install raises exception when version is not set.""" plugin = { 'package': 'oci://registry.io/plugin:v1.0!path', 'version': None } - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) - + with pytest.raises(InstallException) as exc_info: installer.install(plugin, {}) - + assert 'Tag or Digest is not set' in str(exc_info.value) - + def test_install_cleans_up_duplicate_hashes(self, tmp_path, mocker): """Test that install removes duplicate hash entries.""" plugin_path = "test-plugin" @@ -1418,48 +1417,48 @@ def test_install_cleans_up_duplicate_hashes(self, tmp_path, mocker): 'version': 'v1.0', 'hash': 'newhash' } - + plugin_path_by_hash = { 'oldhash': plugin_path, 'anotherhash': plugin_path } - + # Mock the downloader mock_downloader = mocker.MagicMock() mock_downloader.download.return_value = plugin_path mock_downloader.digest.return_value = 'digest123' - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + # Create plugin directory plugin_dir = tmp_path / plugin_path plugin_dir.mkdir() - + result = installer.install(plugin, plugin_path_by_hash) - + # Verify old hashes were removed assert 'oldhash' not in plugin_path_by_hash assert 'anotherhash' not in plugin_path_by_hash assert result == plugin_path - + def test_install_handles_download_errors(self, tmp_path, mocker): """Test that install properly handles download errors.""" plugin = { 'package': 'oci://registry.io/plugin:v1.0!path', 'version': 'v1.0' } - + # Mock downloader to raise an exception mock_downloader = mocker.MagicMock() mock_downloader.download.side_effect = Exception("Network error") - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) installer.downloader = mock_downloader - + with pytest.raises(InstallException) as exc_info: installer.install(plugin, {}) - + assert 'Error while installing OCI plugin' in str(exc_info.value) assert 'Network error' in str(exc_info.value) @@ -1467,118 +1466,118 @@ def test_install_handles_download_errors(self, tmp_path, mocker): @pytest.mark.integration class TestOciIntegration: """Integration tests with real OCI images.""" - + @pytest.mark.integration def test_download_real_oci_image(self, tmp_path): """Test downloading and extracting a real OCI image.""" import shutil - + # Skip if skopeo not available if not shutil.which('skopeo'): pytest.skip("skopeo not available") - + package = 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-simple-chat' - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) plugin_path = downloader.download(package) - + # Verify plugin was extracted plugin_dir = tmp_path / plugin_path assert plugin_dir.exists() assert (plugin_dir / "package.json").exists() - + # Verify we can read package.json package_json = json.loads((plugin_dir / "package.json").read_text()) assert 'name' in package_json - + @pytest.mark.integration def test_get_digest_from_real_image(self, tmp_path): """Test getting digest from a real OCI image.""" import shutil - + if not shutil.which('skopeo'): pytest.skip("skopeo not available") - + package = 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-simple-chat' - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) digest = downloader.digest(package) - + # Digest should be a hex string assert isinstance(digest, str) assert len(digest) > 0 - + @pytest.mark.integration def test_install_oci_plugin_creates_hash_file(self, tmp_path): """Test full installation of OCI plugin with hash file creation.""" import shutil - + if not shutil.which('skopeo'): pytest.skip("skopeo not available") - + plugin_path_name = 'internal-backstage-plugin-simple-chat' plugin = { 'package': f'oci://quay.io/gashcrumb/example-root-http-middleware:latest!{plugin_path_name}', 'version': 'latest' } - + installer = install_dynamic_plugins.OciPluginInstaller(str(tmp_path)) plugin_path = installer.install(plugin, {}) - + # Verify installation plugin_dir = tmp_path / plugin_path assert plugin_dir.exists() assert (plugin_dir / "package.json").exists() - + # Verify digest hash file was created hash_file = plugin_dir / 'dynamic-plugin-image.hash' assert hash_file.exists() digest = hash_file.read_text().strip() assert len(digest) > 0 - + @pytest.mark.integration def test_download_multiple_plugins_from_same_image(self, tmp_path): """Test downloading multiple plugins from the same OCI image.""" import shutil - + if not shutil.which('skopeo'): pytest.skip("skopeo not available") - + # Two plugins from the same image packages = [ 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-simple-chat', 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-middleware-header-example-dynamic' ] - + downloader = install_dynamic_plugins.OciDownloader(str(tmp_path)) - + plugin_paths = [] for package in packages: plugin_path = downloader.download(package) plugin_paths.append(plugin_path) - + # Verify plugin was extracted plugin_dir = tmp_path / plugin_path assert plugin_dir.exists() assert (plugin_dir / "package.json").exists() - + # Verify both plugins were extracted assert len(plugin_paths) == 2 assert plugin_paths[0] != plugin_paths[1] - + @pytest.mark.integration def test_oci_plugin_with_inherit_version(self, tmp_path): """Test that inherit version pattern works in plugin merge.""" # This tests the version inheritance at the merge level all_plugins = {} - + # First add a plugin with explicit version plugin1 = { 'package': 'oci://quay.io/gashcrumb/example-root-http-middleware:latest!internal-backstage-plugin-simple-chat-backend-dynamic' } merger1 = install_dynamic_plugins.OciPackageMerger(plugin1, 'test.yaml', all_plugins) merger1.merge_plugin(level=0) - + # Then override with {{inherit}} plugin2 = { 'package': 'oci://quay.io/gashcrumb/example-root-http-middleware:{{inherit}}!internal-backstage-plugin-simple-chat-backend-dynamic', @@ -1586,7 +1585,7 @@ def test_oci_plugin_with_inherit_version(self, tmp_path): } merger2 = install_dynamic_plugins.OciPackageMerger(plugin2, 'test.yaml', all_plugins) merger2.merge_plugin(level=1) - + # Version should be inherited from plugin1 plugin_key = 'oci://quay.io/gashcrumb/example-root-http-middleware:!internal-backstage-plugin-simple-chat-backend-dynamic' assert plugin_key in all_plugins @@ -1596,13 +1595,13 @@ def test_oci_plugin_with_inherit_version(self, tmp_path): class TestGetLocalPackageInfo: """Test cases for get_local_package_info() function.""" - + def test_package_with_valid_package_json(self, tmp_path): """Test getting info from a package with valid package.json.""" # Create a package directory with package.json package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json = { "name": "test-package", "version": "1.0.0", @@ -1610,10 +1609,10 @@ def test_package_with_valid_package_json(self, tmp_path): } package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps(package_json)) - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify the info contains expected fields assert '_package_json' in info assert info['_package_json'] == package_json @@ -1622,185 +1621,184 @@ def test_package_with_valid_package_json(self, tmp_path): # Should not have lock file mtimes assert '_package-lock.json_mtime' not in info assert '_yarn.lock_mtime' not in info - + def test_package_with_relative_path(self, tmp_path, monkeypatch): """Test getting info from a package using relative path (./).""" # Create a package directory package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json = {"name": "test-package", "version": "2.0.0"} (package_dir / "package.json").write_text(json.dumps(package_json)) - + # Change to tmp_path directory and use relative path monkeypatch.chdir(tmp_path) - + # Get package info with relative path info = install_dynamic_plugins.get_local_package_info('./test-package') - + # Verify the info is correct assert info['_package_json'] == package_json assert '_package_json_mtime' in info - + def test_package_with_package_lock_json(self, tmp_path): """Test getting info from a package with package-lock.json.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps({"name": "test", "version": "1.0.0"})) - + package_lock_path = package_dir / "package-lock.json" package_lock_path.write_text(json.dumps({"lockfileVersion": 2})) - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify lock file mtime is included assert '_package-lock.json_mtime' in info assert info['_package-lock.json_mtime'] == package_lock_path.stat().st_mtime assert '_yarn.lock_mtime' not in info - + def test_package_with_yarn_lock(self, tmp_path): """Test getting info from a package with yarn.lock.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps({"name": "test", "version": "1.0.0"})) - + yarn_lock_path = package_dir / "yarn.lock" yarn_lock_path.write_text("# yarn lockfile v1") - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify lock file mtime is included assert '_yarn.lock_mtime' in info assert info['_yarn.lock_mtime'] == yarn_lock_path.stat().st_mtime assert '_package-lock.json_mtime' not in info - + def test_package_with_both_lock_files(self, tmp_path): """Test getting info from a package with both package-lock.json and yarn.lock.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps({"name": "test", "version": "1.0.0"})) - + package_lock_path = package_dir / "package-lock.json" package_lock_path.write_text(json.dumps({"lockfileVersion": 2})) - + yarn_lock_path = package_dir / "yarn.lock" yarn_lock_path.write_text("# yarn lockfile v1") - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify both lock file mtimes are included assert '_package-lock.json_mtime' in info assert '_yarn.lock_mtime' in info assert info['_package-lock.json_mtime'] == package_lock_path.stat().st_mtime assert info['_yarn.lock_mtime'] == yarn_lock_path.stat().st_mtime - + def test_directory_without_package_json(self, tmp_path): """Test getting info from a directory without package.json (falls back to directory mtime).""" package_dir = tmp_path / "empty-package" package_dir.mkdir() - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Should return directory mtime assert '_directory_mtime' in info assert info['_directory_mtime'] == package_dir.stat().st_mtime assert '_package_json' not in info - + def test_nonexistent_path(self, tmp_path): """Test getting info from a non-existent path.""" nonexistent_path = tmp_path / "does-not-exist" - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(nonexistent_path)) - + # Should return _not_found flag assert '_not_found' in info assert info['_not_found'] is True - + def test_invalid_json_in_package_json(self, tmp_path): """Test getting info when package.json contains invalid JSON.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + # Write invalid JSON package_json_path = package_dir / "package.json" package_json_path.write_text("{ invalid json content }") - + # Get package info info = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Should return error information assert '_error' in info assert 'JSONDecodeError' in info['_error'] or 'Expecting' in info['_error'] - + def test_package_info_detects_changes(self, tmp_path): """Test that package info changes when files are modified.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + # Create initial package.json package_json_path = package_dir / "package.json" package_json_v1 = {"name": "test", "version": "1.0.0"} package_json_path.write_text(json.dumps(package_json_v1)) - + # Get initial info info1 = install_dynamic_plugins.get_local_package_info(str(package_dir)) initial_mtime = info1['_package_json_mtime'] - + # Wait a bit and modify the file import time time.sleep(0.01) - + package_json_v2 = {"name": "test", "version": "2.0.0"} package_json_path.write_text(json.dumps(package_json_v2)) - + # Get updated info info2 = install_dynamic_plugins.get_local_package_info(str(package_dir)) - + # Verify that content and mtime changed assert info2['_package_json'] != info1['_package_json'] assert info2['_package_json']['version'] == "2.0.0" assert info2['_package_json_mtime'] > initial_mtime - + def test_lock_file_mtime_detection(self, tmp_path): """Test that lock file changes are detected via mtime.""" package_dir = tmp_path / "test-package" package_dir.mkdir() - + package_json_path = package_dir / "package.json" package_json_path.write_text(json.dumps({"name": "test", "version": "1.0.0"})) - + # Get info without lock file info1 = install_dynamic_plugins.get_local_package_info(str(package_dir)) assert '_package-lock.json_mtime' not in info1 - + # Add lock file import time time.sleep(0.01) - + package_lock_path = package_dir / "package-lock.json" package_lock_path.write_text(json.dumps({"lockfileVersion": 2})) - + # Get info with lock file info2 = install_dynamic_plugins.get_local_package_info(str(package_dir)) assert '_package-lock.json_mtime' in info2 - + # Hashes should be different due to lock file addition hash1 = hashlib.sha256(json.dumps(info1, sort_keys=True).encode('utf-8')).hexdigest() hash2 = hashlib.sha256(json.dumps(info2, sort_keys=True).encode('utf-8')).hexdigest() assert hash1 != hash2 - class TestExtractCatalogIndex: """Test cases for extract_catalog_index() function.""" From 512608c5d8794cecfe7f4a2a82c8f3f8e31fdfff Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Thu, 27 Nov 2025 13:53:36 +0000 Subject: [PATCH 06/22] Create minimal documentation Signed-off-by: Fortune Ndlovu --- docs/dynamic-plugins/installing-plugins.md | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/docs/dynamic-plugins/installing-plugins.md b/docs/dynamic-plugins/installing-plugins.md index 57cd45ea08..c3c5b5d31a 100644 --- a/docs/dynamic-plugins/installing-plugins.md +++ b/docs/dynamic-plugins/installing-plugins.md @@ -60,13 +60,14 @@ env: ```yaml # Example using Helm chart values -upstream: - backstage: - extraEnvVars: - - name: CATALOG_INDEX_IMAGE - value: "quay.io/rhdh/plugin-catalog-index:1.9" +global: + dynamic: + catalogIndex: + image: "quay.io/rhdh/plugin-catalog-index:1.9" ``` +The Helm chart automatically templates this value into the `CATALOG_INDEX_IMAGE` environment variable. To update the catalog index, modify this value and run `helm upgrade`. + ### Catalog Index Image Structure The catalog index OCI image should contain a `dynamic-plugins.default.yaml` file at the root level with the same structure as the embedded default configuration file: From 8a7852e8044a12f88759fb0bb82c6c4006fa07dc Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Thu, 27 Nov 2025 14:07:33 +0000 Subject: [PATCH 07/22] Update minimal documenation to include operator CR context Signed-off-by: Fortune Ndlovu --- docs/dynamic-plugins/installing-plugins.md | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/docs/dynamic-plugins/installing-plugins.md b/docs/dynamic-plugins/installing-plugins.md index c3c5b5d31a..e79bc7b333 100644 --- a/docs/dynamic-plugins/installing-plugins.md +++ b/docs/dynamic-plugins/installing-plugins.md @@ -49,13 +49,21 @@ When the `CATALOG_INDEX_IMAGE` environment variable is set, the `install-dynamic ### Configuring the Catalog Index Image -Set the `CATALOG_INDEX_IMAGE` environment variable to specify the OCI image containing your plugin catalog: +Set the `CATALOG_INDEX_IMAGE` environment variable in the `install-dynamic-plugins` init container to specify the OCI image containing your plugin catalog: ```yaml -# Example using Kubernetes/OpenShift deployment -env: - - name: CATALOG_INDEX_IMAGE - value: "quay.io/rhdh/plugin-catalog-index:1.9" +# Example using RHDH Operator (Kubernetes/OpenShift) +apiVersion: rhdh.redhat.com/v1alpha4 +kind: Backstage +metadata: + name: my-backstage +spec: + application: + extraEnvs: + envs: + - name: CATALOG_INDEX_IMAGE + value: "quay.io/rhdh/plugin-catalog-index:1.9" + containers: ["install-dynamic-plugins"] ``` ```yaml From cd2ea19f6c298108c7666533a641f84358493c13 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sat, 29 Nov 2025 13:43:12 +0000 Subject: [PATCH 08/22] fix: improve catalog index extraction behavior and error handling. Only replace dynamic-plugins.default.yaml if present in includes list. Fail fast on catalog index extraction errors and Propagate exceptions instead of silent warnings. Update documentation to reflect current Helm chart behavior Resolves: RHIDP-9761 Signed-off-by: Fortune Ndlovu --- docker/install-dynamic-plugins.py | 138 ++++++++++----------- docs/dynamic-plugins/installing-plugins.md | 22 +++- 2 files changed, 80 insertions(+), 80 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index dad597732a..bc6ce22149 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -797,81 +797,73 @@ def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> if not catalog_index_image: print("======= No CATALOG_INDEX_IMAGE specified, skipping catalog index extraction", flush=True) return None - try: - print(f"\n======= Extracting catalog index from {catalog_index_image}", flush=True) - skopeo_path = shutil.which('skopeo') - if skopeo_path is None: - print("WARNING: skopeo executable not found in PATH, skipping catalog index extraction", flush=True) - return None - - catalog_index_temp_dir = os.path.join(catalog_index_mount, '.catalog-index-temp') - os.makedirs(catalog_index_temp_dir, exist_ok=True) - - with tempfile.TemporaryDirectory() as tmp_dir: - if not catalog_index_image.startswith('docker://'): - image_url = f'docker://{catalog_index_image}' - else: - image_url = catalog_index_image - print(f"\t==> Copying catalog index image to local filesystem", flush=True) - local_dir = os.path.join(tmp_dir, 'catalog-index-oci') - - # Download the OCI image using skopeo - result = subprocess.run( - [skopeo_path, 'copy', image_url, f'dir:{local_dir}'], - capture_output=True, - text=True - ) - if result.returncode != 0: - print(f"WARNING: Failed to download catalog index image: {result.stderr}", flush=True) - return None + + print(f"\n======= Extracting catalog index from {catalog_index_image}", flush=True) + skopeo_path = shutil.which('skopeo') + if skopeo_path is None: + raise InstallException("CATALOG_INDEX_IMAGE is set but skopeo executable not found in PATH. Cannot extract catalog index.") - manifest_path = os.path.join(local_dir, 'manifest.json') - if not os.path.isfile(manifest_path): - print("WARNING: manifest.json not found in catalog index image", flush=True) - return None + catalog_index_temp_dir = os.path.join(catalog_index_mount, '.catalog-index-temp') + os.makedirs(catalog_index_temp_dir, exist_ok=True) - with open(manifest_path, 'r') as f: - manifest = json.load(f) + with tempfile.TemporaryDirectory() as tmp_dir: + if not catalog_index_image.startswith('docker://'): + image_url = f'docker://{catalog_index_image}' + else: + image_url = catalog_index_image + print(f"\t==> Copying catalog index image to local filesystem", flush=True) + local_dir = os.path.join(tmp_dir, 'catalog-index-oci') + + # Download the OCI image using skopeo + result = subprocess.run( + [skopeo_path, 'copy', image_url, f'dir:{local_dir}'], + capture_output=True, + text=True + ) + if result.returncode != 0: + raise InstallException(f"Failed to download catalog index image {catalog_index_image}: {result.stderr}") - print(f"\t==> Extracting catalog index layers", flush=True) - max_entry_size = int(os.environ.get('MAX_ENTRY_SIZE', 20000000)) + manifest_path = os.path.join(local_dir, 'manifest.json') + if not os.path.isfile(manifest_path): + raise InstallException(f"manifest.json not found in catalog index image {catalog_index_image}") - for layer in manifest.get('layers', []): - layer_digest = layer.get('digest', '') - if not layer_digest: - continue + with open(manifest_path, 'r') as f: + manifest = json.load(f) - (_sha, filename) = layer_digest.split(':') - layer_file = os.path.join(local_dir, filename) - if not os.path.isfile(layer_file): - print(f"\t==> WARNING: Layer file {filename} not found", flush=True) - continue + print(f"\t==> Extracting catalog index layers", flush=True) + max_entry_size = int(os.environ.get('MAX_ENTRY_SIZE', 20000000)) - print(f"\t==> Extracting layer {filename}", flush=True) - with tarfile.open(layer_file, 'r:*') as tar: - for member in tar.getmembers(): - # Security checks - if member.size > max_entry_size: - print(f"\t==> WARNING: Skipping large file {member.name} in catalog index", flush=True) + for layer in manifest.get('layers', []): + layer_digest = layer.get('digest', '') + if not layer_digest: + continue + + (_sha, filename) = layer_digest.split(':') + layer_file = os.path.join(local_dir, filename) + if not os.path.isfile(layer_file): + print(f"\t==> WARNING: Layer file {filename} not found", flush=True) + continue + + print(f"\t==> Extracting layer {filename}", flush=True) + with tarfile.open(layer_file, 'r:*') as tar: + for member in tar.getmembers(): + # Security checks + if member.size > max_entry_size: + print(f"\t==> WARNING: Skipping large file {member.name} in catalog index", flush=True) + continue + if member.islnk() or member.issym(): + realpath = os.path.realpath(os.path.join(catalog_index_temp_dir, *os.path.split(member.linkname))) + if not realpath.startswith(catalog_index_temp_dir): + print(f"\t==> WARNING: Skipping link outside archive: {member.name}", flush=True) continue - if member.islnk() or member.issym(): - realpath = os.path.realpath(os.path.join(catalog_index_temp_dir, *os.path.split(member.linkname))) - if not realpath.startswith(catalog_index_temp_dir): - print(f"\t==> WARNING: Skipping link outside archive: {member.name}", flush=True) - continue - tar.extract(member, path=catalog_index_temp_dir, filter='tar') - - default_plugins_file = os.path.join(catalog_index_temp_dir, 'dynamic-plugins.default.yaml') - if os.path.isfile(default_plugins_file): - print(f"\t==> Successfully extracted catalog index with dynamic-plugins.default.yaml", flush=True) - return default_plugins_file - else: - print(f"\t==> Catalog index extracted but dynamic-plugins.default.yaml not found", flush=True) - return None + tar.extract(member, path=catalog_index_temp_dir, filter='tar') - except Exception as e: - print(f"WARNING: Error extracting catalog index: {e}", flush=True) - return None + default_plugins_file = os.path.join(catalog_index_temp_dir, 'dynamic-plugins.default.yaml') + if os.path.isfile(default_plugins_file): + print(f"\t==> Successfully extracted catalog index with dynamic-plugins.default.yaml", flush=True) + return default_plugins_file + else: + raise InstallException(f"Catalog index image {catalog_index_image} does not contain the expected dynamic-plugins.default.yaml file") def main(): @@ -933,16 +925,14 @@ def main(): if not isinstance(includes, list): raise InstallException(f"content of the \'includes\' field must be a list in {dynamicPluginsFile}") - # Prepend catalog index default file to includes if it was extracted + # Replace dynamic-plugins.default.yaml with catalog index if it was extracted if catalog_index_default_file and os.path.isfile(catalog_index_default_file): - print(f"\n======= Prepending catalog index default plugins file: {catalog_index_default_file}", flush=True) - includes.insert(0, catalog_index_default_file) - - # Remove the embedded default file from includes to avoid duplicates embedded_default = 'dynamic-plugins.default.yaml' if embedded_default in includes: - print(f"\t==> Removing embedded default file from includes (replaced by catalog index)", flush=True) - includes.remove(embedded_default) + print(f"\n======= Replacing {embedded_default} with catalog index: {catalog_index_default_file}", flush=True) + # Replace the embedded default file with the catalog index at the same position + index = includes.index(embedded_default) + includes[index] = catalog_index_default_file for include in includes: if not isinstance(include, str): diff --git a/docs/dynamic-plugins/installing-plugins.md b/docs/dynamic-plugins/installing-plugins.md index e79bc7b333..e3d654eea4 100644 --- a/docs/dynamic-plugins/installing-plugins.md +++ b/docs/dynamic-plugins/installing-plugins.md @@ -45,7 +45,7 @@ When the `CATALOG_INDEX_IMAGE` environment variable is set, the `install-dynamic 1. Download and extract the specified OCI image 2. Look for a `dynamic-plugins.default.yaml` file within the image 3. Use this file as the primary source for default plugin configurations -4. Automatically override the embedded `dynamic-plugins.default.yaml` file if present +4. Replace the embedded `dynamic-plugins.default.yaml` if it's present in the `includes` list ### Configuring the Catalog Index Image @@ -68,13 +68,23 @@ spec: ```yaml # Example using Helm chart values -global: - dynamic: - catalogIndex: - image: "quay.io/rhdh/plugin-catalog-index:1.9" +# Note: Until native support is added to the Helm chart, you need to customize the +# install-dynamic-plugins init container definition to add the CATALOG_INDEX_IMAGE env var. + +# In your custom values.yaml, add the CATALOG_INDEX_IMAGE environment variable: + +upstream: + backstage: + initContainers: + - name: install-dynamic-plugins + # ... other configuration from the chart ... + env: + - name: CATALOG_INDEX_IMAGE + value: "quay.io/rhdh/plugin-catalog-index:1.9" + # ... other environment variables ... ``` -The Helm chart automatically templates this value into the `CATALOG_INDEX_IMAGE` environment variable. To update the catalog index, modify this value and run `helm upgrade`. +To update the catalog index, modify the `CATALOG_INDEX_IMAGE` value in your custom values file and run `helm upgrade`. ### Catalog Index Image Structure From ad059f8b1c57e44a1434cf0902c86a01752ccfcd Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sat, 29 Nov 2025 13:55:51 +0000 Subject: [PATCH 09/22] update unit tests --- docker/test_install-dynamic-plugins.py | 60 +++++++++++++------------- 1 file changed, 29 insertions(+), 31 deletions(-) diff --git a/docker/test_install-dynamic-plugins.py b/docker/test_install-dynamic-plugins.py index 3353c70760..25e655f9dc 100644 --- a/docker/test_install-dynamic-plugins.py +++ b/docker/test_install-dynamic-plugins.py @@ -1860,17 +1860,17 @@ def test_extract_catalog_index_no_image_specified(self, tmp_path): assert result is None def test_extract_catalog_index_skopeo_not_found(self, tmp_path, mocker): - """Test that function returns None when skopeo is not available.""" + """Test that function raises InstallException when skopeo is not available.""" mocker.patch('shutil.which', return_value=None) - result = install_dynamic_plugins.extract_catalog_index( - "quay.io/test/image:latest", - str(tmp_path) - ) - assert result is None + with pytest.raises(install_dynamic_plugins.InstallException, match="skopeo executable not found in PATH"): + install_dynamic_plugins.extract_catalog_index( + "quay.io/test/image:latest", + str(tmp_path) + ) def test_extract_catalog_index_skopeo_copy_fails(self, tmp_path, mocker): - """Test that function returns None when skopeo copy fails.""" + """Test that function raises InstallException when skopeo copy fails.""" mocker.patch('shutil.which', return_value='/usr/bin/skopeo') # Mock subprocess.run to simulate skopeo failure @@ -1879,14 +1879,14 @@ def test_extract_catalog_index_skopeo_copy_fails(self, tmp_path, mocker): mock_result.stderr = "Error: image not found" mocker.patch('subprocess.run', return_value=mock_result) - result = install_dynamic_plugins.extract_catalog_index( - "quay.io/test/image:latest", - str(tmp_path) - ) - assert result is None + with pytest.raises(install_dynamic_plugins.InstallException, match="Failed to download catalog index image"): + install_dynamic_plugins.extract_catalog_index( + "quay.io/test/image:latest", + str(tmp_path) + ) def test_extract_catalog_index_no_manifest(self, tmp_path, mocker): - """Test that function returns None when manifest.json is not found.""" + """Test that function raises InstallException when manifest.json is not found.""" mocker.patch('shutil.which', return_value='/usr/bin/skopeo') # Mock subprocess.run to simulate successful skopeo copy @@ -1894,11 +1894,11 @@ def test_extract_catalog_index_no_manifest(self, tmp_path, mocker): mock_result.returncode = 0 mocker.patch('subprocess.run', return_value=mock_result) - result = install_dynamic_plugins.extract_catalog_index( - "quay.io/test/image:latest", - str(tmp_path) - ) - assert result is None + with pytest.raises(install_dynamic_plugins.InstallException, match="manifest.json not found in catalog index image"): + install_dynamic_plugins.extract_catalog_index( + "quay.io/test/image:latest", + str(tmp_path) + ) def test_extract_catalog_index_success(self, tmp_path, mocker, mock_oci_image): """Test successful extraction of catalog index with dynamic-plugins.default.yaml.""" @@ -1993,12 +1993,11 @@ def mock_subprocess_run(cmd, **kwargs): mocker.patch('subprocess.run', side_effect=mock_subprocess_run) - result = install_dynamic_plugins.extract_catalog_index( - "quay.io/test/empty-index:latest", - str(catalog_mount) - ) - - assert result is None + with pytest.raises(install_dynamic_plugins.InstallException, match="does not contain the expected dynamic-plugins.default.yaml file"): + install_dynamic_plugins.extract_catalog_index( + "quay.io/test/empty-index:latest", + str(catalog_mount) + ) def test_extract_catalog_index_large_file_skipped(self, tmp_path, mocker, monkeypatch): """Test that files larger than MAX_ENTRY_SIZE are skipped during extraction.""" @@ -2078,18 +2077,17 @@ def mock_subprocess_run(cmd, **kwargs): assert not large_file_path.exists() def test_extract_catalog_index_exception_handling(self, tmp_path, mocker): - """Test that exceptions during extraction are caught and return None.""" + """Test that unexpected exceptions during extraction propagate.""" mocker.patch('shutil.which', return_value='/usr/bin/skopeo') # Mock subprocess.run to raise an exception mocker.patch('subprocess.run', side_effect=Exception("Unexpected error")) - result = install_dynamic_plugins.extract_catalog_index( - "quay.io/test/image:latest", - str(tmp_path) - ) - - assert result is None + with pytest.raises(Exception, match="Unexpected error"): + install_dynamic_plugins.extract_catalog_index( + "quay.io/test/image:latest", + str(tmp_path) + ) if __name__ == '__main__': pytest.main([__file__, '-v']) From 18ecc2076c4ff2ffdc735d6a76f92c361f0bd59d Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sat, 29 Nov 2025 14:04:40 +0000 Subject: [PATCH 10/22] fix SonarCloud issues --- docker/install-dynamic-plugins.py | 78 +++++++++++++++++-------------- 1 file changed, 44 insertions(+), 34 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index bc6ce22149..01750711f7 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -419,6 +419,8 @@ def merge_plugin(self, level: int): self.allPlugins[pluginKey]["last_modified_level"] = level self.override_plugin(version, inheritVersion, pluginKey) +DOCKER_PROTOCOL_PREFIX = 'docker://' + class OciDownloader: """Helper class for downloading and extracting plugins from OCI container images.""" @@ -446,7 +448,7 @@ def get_plugin_tar(self, image: str) -> str: image_digest = hashlib.sha256(image.encode('utf-8'), usedforsecurity=False).hexdigest() local_dir = os.path.join(self.tmp_dir, image_digest) # replace oci:// prefix with docker:// - image_url = image.replace('oci://', 'docker://') + image_url = image.replace('oci://', DOCKER_PROTOCOL_PREFIX) self.skopeo(['copy', image_url, f'dir:{local_dir}']) manifest_path = os.path.join(local_dir, 'manifest.json') manifest = json.load(open(manifest_path)) @@ -491,7 +493,7 @@ def download(self, package: str) -> str: def digest(self, package: str) -> str: (image, _) = package.split('!') - image_url = image.replace('oci://', 'docker://') + image_url = image.replace('oci://', DOCKER_PROTOCOL_PREFIX) output = self.skopeo(['inspect', image_url]) data = json.loads(output) # OCI artifact digest field is defined as "hash method" ":" "hash" @@ -792,7 +794,40 @@ def wait_for_lock_release(lock_file_path): time.sleep(1) print("======= Lock released.") -def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> str: +def _extract_catalog_index_layers(manifest: dict, local_dir: str, catalog_index_temp_dir: str) -> None: + """Extract layers from the catalog index OCI image.""" + max_entry_size = int(os.environ.get('MAX_ENTRY_SIZE', 20000000)) + + for layer in manifest.get('layers', []): + layer_digest = layer.get('digest', '') + if not layer_digest: + continue + + (_sha, filename) = layer_digest.split(':') + layer_file = os.path.join(local_dir, filename) + if not os.path.isfile(layer_file): + print(f"\t==> WARNING: Layer file {filename} not found", flush=True) + continue + + print(f"\t==> Extracting layer {filename}", flush=True) + _extract_layer_tarball(layer_file, catalog_index_temp_dir, max_entry_size) + +def _extract_layer_tarball(layer_file: str, catalog_index_temp_dir: str, max_entry_size: int) -> None: + """Extract a single layer tarball with security checks.""" + with tarfile.open(layer_file, 'r:*') as tar: + for member in tar.getmembers(): + # Security checks + if member.size > max_entry_size: + print(f"\t==> WARNING: Skipping large file {member.name} in catalog index", flush=True) + continue + if member.islnk() or member.issym(): + realpath = os.path.realpath(os.path.join(catalog_index_temp_dir, *os.path.split(member.linkname))) + if not realpath.startswith(catalog_index_temp_dir): + print(f"\t==> WARNING: Skipping link outside archive: {member.name}", flush=True) + continue + tar.extract(member, path=catalog_index_temp_dir, filter='tar') + +def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> str | None: """Extract the catalog index OCI image and return the path to dynamic-plugins.default.yaml if found.""" if not catalog_index_image: print("======= No CATALOG_INDEX_IMAGE specified, skipping catalog index extraction", flush=True) @@ -807,11 +842,11 @@ def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> os.makedirs(catalog_index_temp_dir, exist_ok=True) with tempfile.TemporaryDirectory() as tmp_dir: - if not catalog_index_image.startswith('docker://'): - image_url = f'docker://{catalog_index_image}' + if not catalog_index_image.startswith(DOCKER_PROTOCOL_PREFIX): + image_url = f'{DOCKER_PROTOCOL_PREFIX}{catalog_index_image}' else: image_url = catalog_index_image - print(f"\t==> Copying catalog index image to local filesystem", flush=True) + print("\t==> Copying catalog index image to local filesystem", flush=True) local_dir = os.path.join(tmp_dir, 'catalog-index-oci') # Download the OCI image using skopeo @@ -830,37 +865,12 @@ def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> with open(manifest_path, 'r') as f: manifest = json.load(f) - print(f"\t==> Extracting catalog index layers", flush=True) - max_entry_size = int(os.environ.get('MAX_ENTRY_SIZE', 20000000)) - - for layer in manifest.get('layers', []): - layer_digest = layer.get('digest', '') - if not layer_digest: - continue - - (_sha, filename) = layer_digest.split(':') - layer_file = os.path.join(local_dir, filename) - if not os.path.isfile(layer_file): - print(f"\t==> WARNING: Layer file {filename} not found", flush=True) - continue - - print(f"\t==> Extracting layer {filename}", flush=True) - with tarfile.open(layer_file, 'r:*') as tar: - for member in tar.getmembers(): - # Security checks - if member.size > max_entry_size: - print(f"\t==> WARNING: Skipping large file {member.name} in catalog index", flush=True) - continue - if member.islnk() or member.issym(): - realpath = os.path.realpath(os.path.join(catalog_index_temp_dir, *os.path.split(member.linkname))) - if not realpath.startswith(catalog_index_temp_dir): - print(f"\t==> WARNING: Skipping link outside archive: {member.name}", flush=True) - continue - tar.extract(member, path=catalog_index_temp_dir, filter='tar') + print("\t==> Extracting catalog index layers", flush=True) + _extract_catalog_index_layers(manifest, local_dir, catalog_index_temp_dir) default_plugins_file = os.path.join(catalog_index_temp_dir, 'dynamic-plugins.default.yaml') if os.path.isfile(default_plugins_file): - print(f"\t==> Successfully extracted catalog index with dynamic-plugins.default.yaml", flush=True) + print("\t==> Successfully extracted catalog index with dynamic-plugins.default.yaml", flush=True) return default_plugins_file else: raise InstallException(f"Catalog index image {catalog_index_image} does not contain the expected dynamic-plugins.default.yaml file") From a89eac2848da3f22808543f655dd9380da34dcc3 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sat, 29 Nov 2025 14:11:34 +0000 Subject: [PATCH 11/22] Fix print statement formatting in cleanup section --- docker/install-dynamic-plugins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index 01750711f7..37786dc84d 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -1024,7 +1024,7 @@ def main(): # Clean up temporary catalog index directory if it exists catalog_index_temp_dir = os.path.join(dynamicPluginsRoot, '.catalog-index-temp') if os.path.exists(catalog_index_temp_dir): - print(f'\n======= Cleaning up temporary catalog index directory', flush=True) + print('\n======= Cleaning up temporary catalog index directory', flush=True) shutil.rmtree(catalog_index_temp_dir, ignore_errors=True, onerror=None) if __name__ == '__main__': From cc7e319a38a4b7148336e64fe8f4af5c1a2aba26 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sat, 29 Nov 2025 14:37:47 +0000 Subject: [PATCH 12/22] Update unit tests to create helper functions --- docker/test_install-dynamic-plugins.py | 39 +++++++++++++++++--------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/docker/test_install-dynamic-plugins.py b/docker/test_install-dynamic-plugins.py index 25e655f9dc..b7cbc3b675 100644 --- a/docker/test_install-dynamic-plugins.py +++ b/docker/test_install-dynamic-plugins.py @@ -58,6 +58,19 @@ OciPackageMerger = install_dynamic_plugins.OciPackageMerger InstallException = install_dynamic_plugins.InstallException +# Test helper functions +import tarfile # noqa: E402 + +def create_test_tarball(tarball_path, mode='w:gz'): # noqa: S202 + """ + Helper function to create test tarballs. + + Note: This is safe for test code as we're creating controlled test data, + not opening untrusted archives. The noqa: S202 suppresses security warnings + about tarfile usage which are not applicable to test fixtures. + """ + return tarfile.open(tarball_path, mode) # noqa: S202 + class TestNPMPackageMergerParsePluginKey: """Test cases for NPMPackageMerger.parse_plugin_key() method.""" @@ -914,7 +927,7 @@ def test_verify_package_integrity_with_real_tarball(self, tmp_path): (test_dir / "index.js").write_text("console.log('test');") tarball_path = tmp_path / "test-package.tgz" - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: tar.add(test_dir, arcname="package") # Calculate actual integrity hash using openssl @@ -967,7 +980,7 @@ def test_extract_npm_package_with_real_tarball(self, tmp_path): # Create tarball following NPM format (with 'package/' prefix) tarball_path = tmp_path / "test-package-1.0.0.tgz" - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: tar.add(package_dir, arcname="package") # Test extraction @@ -997,7 +1010,7 @@ def test_zip_bomb_protection_real_tarball(self, tmp_path): (package_dir / "huge-file.bin").write_bytes(large_content) tarball_path = tmp_path / "malicious.tgz" - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: tar.add(package_dir / "huge-file.bin", arcname="package/huge-file.bin") installer = install_dynamic_plugins.NpmPluginInstaller(str(tmp_path)) @@ -1015,7 +1028,7 @@ def test_path_traversal_protection_real_tarball(self, tmp_path): # Create tarball with path traversal attempt tarball_path = tmp_path / "malicious.tgz" - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: # Create a TarInfo with malicious path info = tarfile.TarInfo(name="test") info.size = 10 @@ -1036,7 +1049,7 @@ def test_symlink_with_invalid_linkpath_prefix(self, tmp_path): # Create tarball with a symlink that has invalid linkpath prefix tarball_path = tmp_path / "malicious.tgz" - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: # First add a regular file info = tarfile.TarInfo(name="package/index.js") info.size = 10 @@ -1064,7 +1077,7 @@ def test_symlink_resolving_outside_directory(self, tmp_path): # Create tarball with a symlink that resolves outside the extraction directory tarball_path = tmp_path / "malicious.tgz" - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: # Add a regular file info = tarfile.TarInfo(name="package/index.js") info.size = 10 @@ -1092,7 +1105,7 @@ def test_hardlink_resolving_outside_directory(self, tmp_path): # Create tarball with a hardlink that resolves outside the extraction directory tarball_path = tmp_path / "malicious.tgz" - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: # Add a regular file info = tarfile.TarInfo(name="package/index.js") info.size = 10 @@ -1119,7 +1132,7 @@ def test_valid_symlink_extraction(self, tmp_path): # Create tarball with valid internal symlinks tarball_path = tmp_path / "valid-package.tgz" - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: # Add a regular file info = tarfile.TarInfo(name="package/lib/helper.js") content = b"module.exports = { helper: () => {} };" @@ -1241,7 +1254,7 @@ def test_extract_plugin_with_valid_path(self, tmp_path, mocker): plugin_path = "internal-backstage-plugin-test" tarball_path = tmp_path / "test.tar.gz" - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: # Add plugin files for filename in ["package.json", "index.js"]: info = tarfile.TarInfo(name=f"{plugin_path}/{filename}") @@ -1271,7 +1284,7 @@ def test_extract_plugin_rejects_oversized_files(self, tmp_path, mocker): # Create tarball with oversized file (needs actual content matching size) large_content = b"x" * 25_000_000 # 25MB, exceeds default 20MB - with tarfile.open(tarball_path, "w:gz") as tar: + with create_test_tarball(tarball_path) as tar: info = tarfile.TarInfo(name=f"{plugin_path}/huge.bin") info.size = len(large_content) tar.addfile(info, io.BytesIO(large_content)) @@ -1844,7 +1857,7 @@ def mock_oci_image(self, tmp_path): # Create the layer tarball layer_tarball = oci_dir / "abc123def456" - with tarfile.open(layer_tarball, 'w:gz') as tar: + with create_test_tarball(layer_tarball) as tar: tar.add(str(yaml_file), arcname="dynamic-plugins.default.yaml") return { @@ -1969,7 +1982,7 @@ def test_extract_catalog_index_no_yaml_file(self, tmp_path, mocker): # Create empty layer tarball layer_tarball = oci_dir / "xyz789" - with tarfile.open(layer_tarball, 'w:gz') as tar: + with create_test_tarball(layer_tarball) as tar: # Add a different file readme = tmp_path / "README.md" readme.write_text("# Test") @@ -2037,7 +2050,7 @@ def test_extract_catalog_index_large_file_skipped(self, tmp_path, mocker, monkey large_file = layer_content_dir / "large-file.bin" large_file.write_text("x" * 2000) # 2KB - larger than our 1000 byte test limit - with tarfile.open(layer_tarball, 'w:gz') as tar: + with create_test_tarball(layer_tarball) as tar: # Add YAML with normal size (smaller than 1000 bytes) tar.add(str(yaml_file), arcname="dynamic-plugins.default.yaml") From 89f9273da7f803bf80a258bff1dcc9d5da1399e4 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sat, 29 Nov 2025 14:47:26 +0000 Subject: [PATCH 13/22] fix all 3 security hotspots and remove ALL duplication in test file --- docker/install-dynamic-plugins.py | 4 +- docker/test_install-dynamic-plugins.py | 73 +++++++++++--------------- 2 files changed, 34 insertions(+), 43 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index 37786dc84d..7995da1ae7 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -609,7 +609,7 @@ def _extract_npm_package(self, archive: str) -> str: os.mkdir(directory) print('\t==> Extracting package archive', archive, flush=True) - with tarfile.open(archive, 'r:*') as tar: + with tarfile.open(archive, 'r:*') as tar: # noqa: S202 - Safe: extracts with filter='tar', size checks, and path validation for member in tar.getmembers(): if member.isreg(): if not member.name.startswith('package/'): @@ -814,7 +814,7 @@ def _extract_catalog_index_layers(manifest: dict, local_dir: str, catalog_index_ def _extract_layer_tarball(layer_file: str, catalog_index_temp_dir: str, max_entry_size: int) -> None: """Extract a single layer tarball with security checks.""" - with tarfile.open(layer_file, 'r:*') as tar: + with tarfile.open(layer_file, 'r:*') as tar: # noqa: S202 - Safe: extracts with filter='tar', size checks, and symlink validation for member in tar.getmembers(): # Security checks if member.size > max_entry_size: diff --git a/docker/test_install-dynamic-plugins.py b/docker/test_install-dynamic-plugins.py index b7cbc3b675..99de553e1e 100644 --- a/docker/test_install-dynamic-plugins.py +++ b/docker/test_install-dynamic-plugins.py @@ -71,6 +71,31 @@ def create_test_tarball(tarball_path, mode='w:gz'): # noqa: S202 """ return tarfile.open(tarball_path, mode) # noqa: S202 +def create_mock_skopeo_copy(manifest_path, layer_tarball, mock_result): + """ + Helper function to create mock subprocess.run for skopeo copy operations. + + Args: + manifest_path: Path to manifest.json file to copy + layer_tarball: Path to layer tarball file to copy + mock_result: Mock result object to return + + Returns: + A function that can be used as side_effect for subprocess.run mock + """ + def mock_subprocess_run(cmd, **kwargs): + if 'copy' in cmd: + dest_arg = [arg for arg in cmd if arg.startswith('dir:')] + if dest_arg: + dest_dir = dest_arg[0].replace('dir:', '') + os.makedirs(dest_dir, exist_ok=True) + import shutil as sh + sh.copy(str(manifest_path), dest_dir) + sh.copy(str(layer_tarball), dest_dir) + return mock_result + + return mock_subprocess_run + class TestNPMPackageMergerParsePluginKey: """Test cases for NPMPackageMerger.parse_plugin_key() method.""" @@ -1923,23 +1948,11 @@ def test_extract_catalog_index_success(self, tmp_path, mocker, mock_oci_image): # Mock subprocess.run to simulate successful skopeo copy mock_result = mocker.Mock() mock_result.returncode = 0 - - def mock_subprocess_run(cmd, **kwargs): - # When skopeo copy is called, set up the OCI directory structure - if 'copy' in cmd: - # Extract the destination directory from the command - dest_arg = [arg for arg in cmd if arg.startswith('dir:')] - if dest_arg: - dest_dir = dest_arg[0].replace('dir:', '') - os.makedirs(dest_dir, exist_ok=True) - - # Copy mock OCI image files to destination - import shutil as sh - sh.copy(mock_oci_image['manifest_path'], dest_dir) - sh.copy(mock_oci_image['layer_tarball'], dest_dir) - - return mock_result - + mock_subprocess_run = create_mock_skopeo_copy( + mock_oci_image['manifest_path'], + mock_oci_image['layer_tarball'], + mock_result + ) mocker.patch('subprocess.run', side_effect=mock_subprocess_run) result = install_dynamic_plugins.extract_catalog_index( @@ -1992,18 +2005,7 @@ def test_extract_catalog_index_no_yaml_file(self, tmp_path, mocker): mock_result = mocker.Mock() mock_result.returncode = 0 - - def mock_subprocess_run(cmd, **kwargs): - if 'copy' in cmd: - dest_arg = [arg for arg in cmd if arg.startswith('dir:')] - if dest_arg: - dest_dir = dest_arg[0].replace('dir:', '') - os.makedirs(dest_dir, exist_ok=True) - import shutil as sh - sh.copy(str(manifest_path), dest_dir) - sh.copy(str(layer_tarball), dest_dir) - return mock_result - + mock_subprocess_run = create_mock_skopeo_copy(manifest_path, layer_tarball, mock_result) mocker.patch('subprocess.run', side_effect=mock_subprocess_run) with pytest.raises(install_dynamic_plugins.InstallException, match="does not contain the expected dynamic-plugins.default.yaml file"): @@ -2061,18 +2063,7 @@ def test_extract_catalog_index_large_file_skipped(self, tmp_path, mocker, monkey mock_result = mocker.Mock() mock_result.returncode = 0 - - def mock_subprocess_run(cmd, **kwargs): - if 'copy' in cmd: - dest_arg = [arg for arg in cmd if arg.startswith('dir:')] - if dest_arg: - dest_dir = dest_arg[0].replace('dir:', '') - os.makedirs(dest_dir, exist_ok=True) - import shutil as sh - sh.copy(str(manifest_path), dest_dir) - sh.copy(str(layer_tarball), dest_dir) - return mock_result - + mock_subprocess_run = create_mock_skopeo_copy(manifest_path, layer_tarball, mock_result) mocker.patch('subprocess.run', side_effect=mock_subprocess_run) result = install_dynamic_plugins.extract_catalog_index( From 20ae3d0d76cf0912dbc0a79e0ac0ee6d9c286fd7 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sat, 29 Nov 2025 14:55:43 +0000 Subject: [PATCH 14/22] make sonar cloud happy by using Python's safest tarfile extraction filters --- docker/install-dynamic-plugins.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index 7995da1ae7..21b1a7829c 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -609,7 +609,7 @@ def _extract_npm_package(self, archive: str) -> str: os.mkdir(directory) print('\t==> Extracting package archive', archive, flush=True) - with tarfile.open(archive, 'r:*') as tar: # noqa: S202 - Safe: extracts with filter='tar', size checks, and path validation + with tarfile.open(archive, 'r:*') as tar: for member in tar.getmembers(): if member.isreg(): if not member.name.startswith('package/'): @@ -619,7 +619,7 @@ def _extract_npm_package(self, archive: str) -> str: raise InstallException(f'Zip bomb detected in {member.name}') member.name = member.name.removeprefix('package/') - tar.extract(member, path=directory, filter='tar') + tar.extract(member, path=directory, filter='data') elif member.isdir(): print('\t\tSkipping directory entry', member.name, flush=True) @@ -635,7 +635,7 @@ def _extract_npm_package(self, archive: str) -> str: if not realpath.startswith(directory_realpath): raise InstallException(f'NPM package archive contains a link outside of the archive: {member.name} -> {member.linkpath}') - tar.extract(member, path=directory, filter='tar') + tar.extract(member, path=directory, filter='data') else: type_mapping = { @@ -814,7 +814,7 @@ def _extract_catalog_index_layers(manifest: dict, local_dir: str, catalog_index_ def _extract_layer_tarball(layer_file: str, catalog_index_temp_dir: str, max_entry_size: int) -> None: """Extract a single layer tarball with security checks.""" - with tarfile.open(layer_file, 'r:*') as tar: # noqa: S202 - Safe: extracts with filter='tar', size checks, and symlink validation + with tarfile.open(layer_file, 'r:*') as tar: for member in tar.getmembers(): # Security checks if member.size > max_entry_size: @@ -825,7 +825,7 @@ def _extract_layer_tarball(layer_file: str, catalog_index_temp_dir: str, max_ent if not realpath.startswith(catalog_index_temp_dir): print(f"\t==> WARNING: Skipping link outside archive: {member.name}", flush=True) continue - tar.extract(member, path=catalog_index_temp_dir, filter='tar') + tar.extract(member, path=catalog_index_temp_dir, filter='data') def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> str | None: """Extract the catalog index OCI image and return the path to dynamic-plugins.default.yaml if found.""" From 8ec3b9f8040d92d4e5d7d87b7dc02a6662f75a2a Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sat, 29 Nov 2025 15:04:55 +0000 Subject: [PATCH 15/22] fixed all 3 security hotspots, adding # NOSONAR comments to suppress the false positives --- docker/install-dynamic-plugins.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index 21b1a7829c..a0cb6be442 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -609,7 +609,7 @@ def _extract_npm_package(self, archive: str) -> str: os.mkdir(directory) print('\t==> Extracting package archive', archive, flush=True) - with tarfile.open(archive, 'r:*') as tar: + with tarfile.open(archive, 'r:*') as tar: # NOSONAR for member in tar.getmembers(): if member.isreg(): if not member.name.startswith('package/'): @@ -814,7 +814,7 @@ def _extract_catalog_index_layers(manifest: dict, local_dir: str, catalog_index_ def _extract_layer_tarball(layer_file: str, catalog_index_temp_dir: str, max_entry_size: int) -> None: """Extract a single layer tarball with security checks.""" - with tarfile.open(layer_file, 'r:*') as tar: + with tarfile.open(layer_file, 'r:*') as tar: # NOSONAR for member in tar.getmembers(): # Security checks if member.size > max_entry_size: From 35cf7f1707905957effdf4fc1b748d9f2348fa73 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Sat, 29 Nov 2025 15:10:02 +0000 Subject: [PATCH 16/22] fix last LOW Security Hotspot in test_install-dynamic-plugins --- docker/test_install-dynamic-plugins.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/test_install-dynamic-plugins.py b/docker/test_install-dynamic-plugins.py index 99de553e1e..78ea798167 100644 --- a/docker/test_install-dynamic-plugins.py +++ b/docker/test_install-dynamic-plugins.py @@ -69,7 +69,7 @@ def create_test_tarball(tarball_path, mode='w:gz'): # noqa: S202 not opening untrusted archives. The noqa: S202 suppresses security warnings about tarfile usage which are not applicable to test fixtures. """ - return tarfile.open(tarball_path, mode) # noqa: S202 + return tarfile.open(tarball_path, mode) # NOSONAR def create_mock_skopeo_copy(manifest_path, layer_tarball, mock_result): """ From c5620bea9d3f87b99dec95054f24ebf7f87d584e Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Mon, 1 Dec 2025 15:57:46 +0000 Subject: [PATCH 17/22] Add DOCKER_PROTOCOL_PREFIX = 'docker://' after RECOGNIZED_ALGORITHMS and Simplify the if/else logic --- docker/install-dynamic-plugins.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index a0cb6be442..4f536415e2 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -102,6 +102,8 @@ class InstallException(Exception): 'blake3', ) +DOCKER_PROTOCOL_PREFIX = 'docker://' + def merge(source, destination, prefix = ''): for key, value in source.items(): if isinstance(value, dict): @@ -419,7 +421,6 @@ def merge_plugin(self, level: int): self.allPlugins[pluginKey]["last_modified_level"] = level self.override_plugin(version, inheritVersion, pluginKey) -DOCKER_PROTOCOL_PREFIX = 'docker://' class OciDownloader: """Helper class for downloading and extracting plugins from OCI container images.""" @@ -842,10 +843,9 @@ def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> os.makedirs(catalog_index_temp_dir, exist_ok=True) with tempfile.TemporaryDirectory() as tmp_dir: - if not catalog_index_image.startswith(DOCKER_PROTOCOL_PREFIX): - image_url = f'{DOCKER_PROTOCOL_PREFIX}{catalog_index_image}' - else: - image_url = catalog_index_image + image_url = catalog_index_image + if not image_url.startswith(DOCKER_PROTOCOL_PREFIX): + image_url = f'{DOCKER_PROTOCOL_PREFIX}{image_url}' print("\t==> Copying catalog index image to local filesystem", flush=True) local_dir = os.path.join(tmp_dir, 'catalog-index-oci') From 0d1d05b6c4068087358781f6b11da638e9449fa3 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Mon, 1 Dec 2025 16:00:39 +0000 Subject: [PATCH 18/22] use the early return pattern --- docker/install-dynamic-plugins.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index 4f536415e2..255d60f77b 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -869,11 +869,10 @@ def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> _extract_catalog_index_layers(manifest, local_dir, catalog_index_temp_dir) default_plugins_file = os.path.join(catalog_index_temp_dir, 'dynamic-plugins.default.yaml') - if os.path.isfile(default_plugins_file): - print("\t==> Successfully extracted catalog index with dynamic-plugins.default.yaml", flush=True) - return default_plugins_file - else: + if not os.path.isfile(default_plugins_file): raise InstallException(f"Catalog index image {catalog_index_image} does not contain the expected dynamic-plugins.default.yaml file") + print("\t==> Successfully extracted catalog index with dynamic-plugins.default.yaml", flush=True) + return default_plugins_file def main(): From 0ae49ceeb06bf1d3078c80d1c167b71eb8b430dd Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Mon, 1 Dec 2025 16:03:23 +0000 Subject: [PATCH 19/22] remove only the redundant check and update the return type --- docker/install-dynamic-plugins.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index 255d60f77b..0e314bd5ed 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -828,12 +828,8 @@ def _extract_layer_tarball(layer_file: str, catalog_index_temp_dir: str, max_ent continue tar.extract(member, path=catalog_index_temp_dir, filter='data') -def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> str | None: +def extract_catalog_index(catalog_index_image: str, catalog_index_mount: str) -> str: """Extract the catalog index OCI image and return the path to dynamic-plugins.default.yaml if found.""" - if not catalog_index_image: - print("======= No CATALOG_INDEX_IMAGE specified, skipping catalog index extraction", flush=True) - return None - print(f"\n======= Extracting catalog index from {catalog_index_image}", flush=True) skopeo_path = shutil.which('skopeo') if skopeo_path is None: From 10dc8b262226d082991789a1e67e2297a31fdf6b Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Mon, 1 Dec 2025 16:13:37 +0000 Subject: [PATCH 20/22] Update clean up temp dir --- docker/install-dynamic-plugins.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index 0e314bd5ed..dfcacb4245 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -795,6 +795,14 @@ def wait_for_lock_release(lock_file_path): time.sleep(1) print("======= Lock released.") +# Clean up temporary catalog index directory +def cleanup_catalog_index_temp_dir(dynamicPluginsRoot): + """Clean up temporary catalog index directory.""" + catalog_index_temp_dir = os.path.join(dynamicPluginsRoot, '.catalog-index-temp') + if os.path.exists(catalog_index_temp_dir): + print('\n======= Cleaning up temporary catalog index directory', flush=True) + shutil.rmtree(catalog_index_temp_dir, ignore_errors=True, onerror=None) + def _extract_catalog_index_layers(manifest: dict, local_dir: str, catalog_index_temp_dir: str) -> None: """Extract layers from the catalog index OCI image.""" max_entry_size = int(os.environ.get('MAX_ENTRY_SIZE', 20000000)) @@ -876,6 +884,7 @@ def main(): lock_file_path = os.path.join(dynamicPluginsRoot, 'install-dynamic-plugins.lock') atexit.register(remove_lock, lock_file_path) + atexit.register(cleanup_catalog_index_temp_dir, dynamicPluginsRoot) signal.signal(signal.SIGTERM, lambda signum, frame: sys.exit(0)) create_lock(lock_file_path) @@ -931,7 +940,7 @@ def main(): raise InstallException(f"content of the \'includes\' field must be a list in {dynamicPluginsFile}") # Replace dynamic-plugins.default.yaml with catalog index if it was extracted - if catalog_index_default_file and os.path.isfile(catalog_index_default_file): + if catalog_index_image: embedded_default = 'dynamic-plugins.default.yaml' if embedded_default in includes: print(f"\n======= Replacing {embedded_default} with catalog index: {catalog_index_default_file}", flush=True) @@ -1016,11 +1025,5 @@ def main(): print('\n======= Removing previously installed dynamic plugin', plugin_path_by_hash[hash_value], flush=True) shutil.rmtree(plugin_directory, ignore_errors=True, onerror=None) - # Clean up temporary catalog index directory if it exists - catalog_index_temp_dir = os.path.join(dynamicPluginsRoot, '.catalog-index-temp') - if os.path.exists(catalog_index_temp_dir): - print('\n======= Cleaning up temporary catalog index directory', flush=True) - shutil.rmtree(catalog_index_temp_dir, ignore_errors=True, onerror=None) - if __name__ == '__main__': main() From 76e5923c1e6b288f076f250c1c0bec1d37853247 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Mon, 1 Dec 2025 16:16:38 +0000 Subject: [PATCH 21/22] fix invalid test due to the function no longer handling empty strings --- docker/test_install-dynamic-plugins.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/docker/test_install-dynamic-plugins.py b/docker/test_install-dynamic-plugins.py index 78ea798167..4d69a0ab0a 100644 --- a/docker/test_install-dynamic-plugins.py +++ b/docker/test_install-dynamic-plugins.py @@ -1892,11 +1892,6 @@ def mock_oci_image(self, tmp_path): "yaml_content": yaml_content } - def test_extract_catalog_index_no_image_specified(self, tmp_path): - """Test that function returns None when no image is specified.""" - result = install_dynamic_plugins.extract_catalog_index("", str(tmp_path)) - assert result is None - def test_extract_catalog_index_skopeo_not_found(self, tmp_path, mocker): """Test that function raises InstallException when skopeo is not available.""" mocker.patch('shutil.which', return_value=None) From 54801aa2a30c9ef276b451f47db439686de47858 Mon Sep 17 00:00:00 2001 From: Fortune Ndlovu Date: Mon, 1 Dec 2025 16:22:03 +0000 Subject: [PATCH 22/22] resolve SonarQube issue: Change the parameter name from dynamicPluginsRoot to dynamic_plugins_root in the cleanup function to follow Python's PEP 8 snake_case naming convention --- docker/install-dynamic-plugins.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/install-dynamic-plugins.py b/docker/install-dynamic-plugins.py index dfcacb4245..26523b501e 100755 --- a/docker/install-dynamic-plugins.py +++ b/docker/install-dynamic-plugins.py @@ -796,9 +796,9 @@ def wait_for_lock_release(lock_file_path): print("======= Lock released.") # Clean up temporary catalog index directory -def cleanup_catalog_index_temp_dir(dynamicPluginsRoot): +def cleanup_catalog_index_temp_dir(dynamic_plugins_root): """Clean up temporary catalog index directory.""" - catalog_index_temp_dir = os.path.join(dynamicPluginsRoot, '.catalog-index-temp') + catalog_index_temp_dir = os.path.join(dynamic_plugins_root, '.catalog-index-temp') if os.path.exists(catalog_index_temp_dir): print('\n======= Cleaning up temporary catalog index directory', flush=True) shutil.rmtree(catalog_index_temp_dir, ignore_errors=True, onerror=None)