feat: support extra catalog index images for Extensions UI [RHIDP-12933] - #4655
Merged
Conversation
|
Skipping CI for Draft Pull Request. |
rm3l
commented
Apr 21, 2026
rm3l
changed the title
install-dynamic-plugins to parse and extract multiple catalog index images [RHIDP-12933]install-dynamic-plugins to support extra catalog index images [RHIDP-12933]
…12933] The Extensions UI currently only displays plugins from the primary CATALOG_INDEX_IMAGE. Organizations that maintain plugins across separate registries (e.g., community vs partner catalogs) have no way to surface all of them without rebuilding a single monolithic index. Introduce EXTRA_CATALOG_INDEX_IMAGES so that additional catalog index images can be listed (comma-separated). Each image's catalog entities are extracted to an isolated subdirectory under the extensions volume, making those plugins visible in the Extensions UI without affecting the primary index's dynamic-plugins.default.yaml. Ref: https://issues.redhat.com/browse/RHIDP-12933 Assisted-by: Claude
rm3l
force-pushed
the
RHIDP-12933--update-install-dynamic-plugins-to-parse-and-extract-multiple-catalog-index-images
branch
from
April 21, 2026 09:39
76e5126 to
2ea5ad5
Compare
rm3l
changed the title
install-dynamic-plugins to support extra catalog index images [RHIDP-12933]Prevent an extra catalog index entry like 'catalog-entities=<image>' from overwriting the primary index's catalog entities by extracting all extra images under a dedicated extra/ parent directory. Ref: https://issues.redhat.com/browse/RHIDP-12933 Assisted-by: Claude
The overwrite warning was printed before the extraction header line, making it look like it related to the previous entry. Move it into extract_extra_catalog_index() so it appears right after the header, clearly associating it with the entry being processed. Ref: https://issues.redhat.com/browse/RHIDP-12933 Assisted-by: Claude
…to-parse-and-extract-multiple-catalog-index-images
Member
Author
|
/test |
Member
Author
|
/agentic_review |
Code Review by Qodo
1. Unsafe extra dir name
|
Member
Author
|
/test e2e-ocp-helm |
gazarenkov
approved these changes
Apr 21, 2026
…to-parse-and-extract-multiple-catalog-index-images
|
New changes are detected. LGTM label has been removed. |
|
Member
Author
|
Re-applying lgtm label as it was previously approved. I've only merged main to retrigger the E2E tests. |
openshift-merge-bot
Bot
merged commit Apr 22, 2026
10eb2c3
into
redhat-developer:main
15 checks passed
rm3l
deleted the
RHIDP-12933--update-install-dynamic-plugins-to-parse-and-extract-multiple-catalog-index-images
branch
April 22, 2026 13:00
gustavolira
added a commit
to gustavolira/rhdh
that referenced
this pull request
May 26, 2026
… the TS implementation Brings the TS port back in sync with three commits that landed on main while this PR was open: - redhat-developer#4576 (OCI disable pre-merge): pre-compute the set of OCI registries that will be effectively disabled after merging, then filter them out of every plugin list before the merge calls skopeo. Avoids wasted remote fetches for plugins the operator already disabled at a higher level. New helpers in src/merger.ts (preMergeOciDisabledState, filterDisabledOciPlugins) and src/oci-key.ts (tryParseOciRegistryAndPath). loadAllPlugins() in src/index.ts now reads every include file up front so the pre-merge pass can run before mergePlugin() touches the OCI cache. - redhat-developer#4666 (MAX_ENTRY_SIZE bump): default from 20MB to 40MB. - redhat-developer#4655 (EXTRA_CATALOG_INDEX_IMAGES): comma-separated catalog index images, each extracted into <CATALOG_ENTITIES_EXTRACT_DIR>/extra/<name>/ catalog-entities. New helpers in src/catalog-index.ts (extractCatalogIndexLayers refactored out of extractCatalogIndex, extractExtraCatalogIndex, parseExtraCatalogIndexImages, imageRefToSubdirectory). New maybeExtractExtraCatalogIndexes() in src/index.ts wires it into runInstaller after the primary catalog index. Tests: 27 new cases under __tests__/merger-pre-merge.test.ts (15) and __tests__/extra-catalog-index.test.ts (12) covering the level-override matrix, ambiguous-pathless detection, same-level duplicates, invalid OCI strings, extensions/marketplace fallback, missing-entities warning, and the duplicate-subdir overwrite warning ordering. Existing tar-extract.test.ts + types.test.ts updated for the new 40MB default. All 155 vitest cases pass. Bundle rebuilt.
openshift-merge-bot Bot
pushed a commit
that referenced
this pull request
May 27, 2026
…#4574) * feat(install-dynamic-plugins): port from Python to TypeScript/Node.js * feat(install-dynamic-plugins): port main's Python script changes into the TS implementation Brings the TS port back in sync with three commits that landed on main while this PR was open: - #4576 (OCI disable pre-merge): pre-compute the set of OCI registries that will be effectively disabled after merging, then filter them out of every plugin list before the merge calls skopeo. Avoids wasted remote fetches for plugins the operator already disabled at a higher level. New helpers in src/merger.ts (preMergeOciDisabledState, filterDisabledOciPlugins) and src/oci-key.ts (tryParseOciRegistryAndPath). loadAllPlugins() in src/index.ts now reads every include file up front so the pre-merge pass can run before mergePlugin() touches the OCI cache. - #4666 (MAX_ENTRY_SIZE bump): default from 20MB to 40MB. - #4655 (EXTRA_CATALOG_INDEX_IMAGES): comma-separated catalog index images, each extracted into <CATALOG_ENTITIES_EXTRACT_DIR>/extra/<name>/ catalog-entities. New helpers in src/catalog-index.ts (extractCatalogIndexLayers refactored out of extractCatalogIndex, extractExtraCatalogIndex, parseExtraCatalogIndexImages, imageRefToSubdirectory). New maybeExtractExtraCatalogIndexes() in src/index.ts wires it into runInstaller after the primary catalog index. Tests: 27 new cases under __tests__/merger-pre-merge.test.ts (15) and __tests__/extra-catalog-index.test.ts (12) covering the level-override matrix, ambiguous-pathless detection, same-level duplicates, invalid OCI strings, extensions/marketplace fallback, missing-entities warning, and the duplicate-subdir overwrite warning ordering. Existing tar-extract.test.ts + types.test.ts updated for the new 40MB default. All 155 vitest cases pass. Bundle rebuilt. * fix(install-dynamic-plugins): inline prototype-pollution guard for CodeQL The shared Set.has() lookup wasn't recognised by CodeQL's js/prototype-polluting-function rule as an exhaustive sanitizer, even though Object.defineProperty already bypasses the __proto__ setter. Replace it with the inlined string-literal pattern CodeQL accepts. Closes CodeQL alert #116 on PR #4574 (safeSet at merger.ts:36). * fix(install-dynamic-plugins): address SonarQube findings on PR #4574 - catalog-index.ts: invert the eq===-1 branch so the positive case comes first (Sonar S3923 'Unexpected negated condition'). - index.ts: drop five redundant type assertions on PluginSpec / Plugin / IncludePluginList — TS already narrows correctly thanks to PluginSpec being a structural subset of Plugin and Array of being assignable to ReadonlyArray. - merger.ts: * Split preMergeOciDisabledState into top-level helpers (processOciEntry, recordEntryState, recordRegistryPath, validateAmbiguousPathless, effectiveRegistryDisabled, computeDisabledRegistries) so the orchestrator stays well under the cognitive-complexity ceiling. * Replace the nested ternary in the explicit-paths sort with String#localeCompare. * Replace 'bucket && bucket.size === 1' with optional-chain 'bucket?.size === 1'. * Drop a stray NUL byte in the entry-key template that crept in via an early Edit and was making the file impossible to diff in a few tools. 155 vitest cases still pass; bundle rebuilt. * fix(install-dynamic-plugins): validate EXTRA_CATALOG_INDEX_IMAGES subdir names Qodo's persistent review flagged that the explicit 'name=<ref>' form of EXTRA_CATALOG_INDEX_IMAGES does not validate 'name', so an operator could accidentally (or maliciously) extract an extra catalog index outside of '<entitiesDir>/extra/' by passing names containing '..' or path separators. The auto-derived form is already safe because imageRefToSubdirectory strips '/', ':', and '@'. Add isSafeSubdirectoryName() that rejects empty names, '.', '..', and any name containing '/' or '\\'. Parsing logs a warning and skips the entry instead of crashing the installer, matching the existing 'empty image reference' handling. 6 new vitest cases cover the rejected forms and document that the check is character-based (so URL-encoded separators like '%2F' are accepted verbatim rather than decoded). Bundle rebuilt (225.6 kB). * refactor(install-dynamic-plugins): apply self-review improvements Targeted cleanups identified in the in-conversation review. No behaviour changes outside of the two defense-in-depth guards listed below. Correctness/security: - installer-npm.ts: validate that the filename emitted by 'npm pack --json' is flat (no '/', no '\\', no leading '..') before joining it to 'destination'. A registry returning '../evil.tgz' would otherwise let extraction escape. - installer-oci.ts: replace 'pkg.split("!")' with an indexOf-based splitOciPackage helper so plugin paths containing a literal '!' are no longer silently truncated. Applied to both the install path and the Always-policy digest comparison. - catalog-index.ts: re-validate 'subdirectory' inside extractExtraCatalogIndex itself instead of trusting all callers to go through parseExtraCatalogIndexImages. Five new vitest cases cover the rejected forms. - index.ts: drop the existsSyncSafe wrapper and use Node's existsSync directly (the wrapper was a literal reimplementation that also conflated 'doesn't exist' with 'no read permission'). - image-cache.ts: surface a typed InstallException with the image ref when 'io.backstage.dynamic-packages' is unparseable, instead of letting JSON.parse crash the install. Maintainability: - types.ts: new effectivePullPolicy() helper centralises the ':latest!' fallback that was duplicated between installer-oci.ts and definitelyNoOp in index.ts. - merger.ts: copyPluginFields now uses a for-of loop with safeSet (same prototype-pollution guard as deepMerge) instead of building an intermediate object via Object.fromEntries. effectiveRegistryDisabled drops the 'as string' cast via destructuring + undefined check. IncludePluginList downgraded from exported to internal. - tar-extract.ts: comment the POSIX assumption around the pluginPathBoundary check; the trailing slash is unambiguous now. - index.ts: destructure process.argv to remove the 'as string' cast. - installer-npm.ts: extract isNpmPackJsonEntry() type guard so the install path stays linear. - concurrency.ts: replace the magic-number worker caps (6/3) with MAX_OCI_WORKERS / MAX_NPM_WORKERS named constants. - catalog-index.ts: simplify isSafeSubdirectoryName (the second clause was unreachable once '/' and '\\' were rejected upfront). 166 vitest cases (was 161; +5 new defense-in-depth tests). Bundle rebuilt (226.0 kB). * refactor(install-dynamic-plugins): polish merger.ts type assertions - Generic 'safeSet<T extends object>' removes the awkward 'dst as unknown as Record<string, unknown>' double-cast in copyPluginFields. The Object.defineProperty signature already accepts any object, so the constraint widening is enough. - Drop the redundant 'as unknown' on parseYaml's return in mergePluginsFromFile — yaml v2's parse() is already typed unknown. No behaviour changes. 166 vitest cases still pass. * ci(install-dynamic-plugins): emit JUnit XML for Codecov test analytics The 'Upload test results to Codecov' step at the monorepo level looks at $RUNNER_TEMP/test-results/ for JUnit reports. Because this PR doesn't touch any yarn workspace, 'yarn run test --affected' skips all workspace tests and the directory ends up empty — codecov[bot] then posts a 'JUnit XML file not found' warning on the PR. Run vitest with the junit reporter so it writes its own report into $RUNNER_TEMP/test-results/install-dynamic-plugins.junit.xml; the existing test-results-action then finds it (alongside any workspace reports) and the warning stops firing. No new upload step needed — test-results-action already searches that directory. Verified locally: 'JUNIT report written to /tmp/.../junit.xml' (166 testcases serialized). * ci(install-dynamic-plugins): run vitest before Codecov test-results upload The previous attempt at producing a JUnit report didn't take effect because the step ordering put it AFTER the 'Upload test results to Codecov' step. The Codecov log confirmed this: 14:33:08 error - No JUnit XML reports found. 14:33:16 JUNIT report written to .../install-dynamic-plugins.junit.xml Move the install-dynamic-plugins test step before the monorepo 'Run tests' so its JUnit report lands in $RUNNER_TEMP/test-results/ in time for the existing upload step to pick it up. The bundle freshness check stays where it was (after the uploads) so it doesn't delay the upload-bearing path further.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR introduces a new
EXTRA_CATALOG_INDEX_IMAGESenv var in theinstall-dynamic-plugins.pyscript, so that additional catalog index images can be listed alongside the primary CATALOG_INDEX_IMAGE.This is a comma-separated list of images.
Each extra image's catalog entities are extracted to an isolated sub-directory in the
/extensionsextraction dir, making those plugins automatically discoverable in the Extensions UI.Notes:
CATALOG_INDEX_IMAGEis considered primary source for extracting and handling thedynamic-plugins.default.yaml(DPDY). Extra catalog images contribute catalog entities only, since there is no use case yet for supporting multiple DPDY files<extraction_dir>/extra/, which should prevent entries from accidentally overwriting the primary index's catalog entitiesname=<image_ref>and plainimage_refformats: explicit names produce cleaner directory names (e.g.,/extra/community/), while plain refs auto-derive namesAssisted-by: Claude
Which issue(s) does this PR fix
PR acceptance criteria
Please make sure that the following steps are complete:
How to test changes / Special notes to the reviewer
Try running the install-dynamic-plugins script locally after setting the
EXTRA_CATALOG_INDEX_IMAGESenv var.You can also deploy the image from this PR with the
EXTRA_CATALOG_INDEX_IMAGESenv var in the install-dynamic-plugins container:Logs
And in the
/extensionsdir: