Process "!plugin-path" in dynamic plugins by gazarenkov · Pull Request #3556 · redhat-developer/rhdh-operator

gazarenkov · 2026-09-17T11:58:07Z

Description

Show the deprecation warning and strip deprecated !plugin-path syntax from plugin URLs

Which issue(s) does this PR fix or relate to

PR acceptance criteria

Tests
Documentation

How to test changes / Special notes to the reviewer

Building Container Images for Testing

Need to test container images from this PR?

For Maintainers: To trigger a test image build, review the code and comment /build-images.
This always builds the HEAD of the PR branch.

For Contributors: Ask a maintainer to run /build-images.

Images will be built and pushed to Quay with links posted in comments.

rhdh-qodo-merge · 2026-09-17T11:58:34Z

PR Summary by Qodo

Support !plugin-path selection for dynamic plugin packages

🐞 Bug fix ✨ Enhancement 🧪 Tests 🕐 20-40 Minutes

AI Description

• Warns when dynamic plugin packages use deprecated !plugin-path suffixes.
• Preserves selectors through configuration and strips them before OCI image retrieval.
• Extracts requested plugin directories from multi-plugin artifacts with validation.

Diagram

graph TD
  A["Plugin config"] --> B{"Suffix present?"}
  B -- "Yes" --> C["Log warning"] --> D["packages.txt"] --> E["Plugin fetch"]
  B -- "No" --> D
  E --> F["Parse selector"] --> G["Fetch OCI image"] --> H["Select plugin dir"]

High-Level Assessment

The following are alternative approaches to this PR:

1. Structured plugin selector field

➕ Avoids overloading OCI URLs with nonstandard syntax
➕ Provides a clearer migration target for multi-plugin packages
➕ Allows selector validation before invoking the installer
➖ Requires API, configuration, and installer contract changes
➖ Would be less compatible with existing dynamic plugin configurations
➖ Expands the scope beyond deprecated syntax support

Recommendation: Keep the current compatibility-focused approach for this PR because it preserves existing package declarations while ensuring only valid OCI references reach the registry client. A structured selector field is a cleaner long-term replacement and could be introduced separately as the migration target for the deprecated syntax.

Files changed (10) +170 / -54

Enhancement (2) +11 / -0

main.goDerive installation names from plugin selectors +5/-0

Derive installation names from plugin selectors

• Updates plugin name extraction to use the text following the final '!' when present. This gives each plugin in a shared artifact a distinct destination directory.

cmd/plugin-fetch/main.go

dynamic-plugins.goWarn on deprecated plugin selector syntax +6/-0

Warn on deprecated plugin selector syntax

• Emits an operator warning when a remotely fetched dynamic plugin package contains '!plugin-path' syntax. The full package value remains in 'packages.txt' so the installer can select the requested plugin.

pkg/model/dynamic-plugins.go

Bug fix (2) +53 / -31

oci.goSelect explicit directories from multi-plugin OCI artifacts +46/-28

Select explicit directories from multi-plugin OCI artifacts

• Separates the plugin selector before parsing and downloading an OCI reference, then uses it to select the expected artifact directory. Extraction now validates the requested directory and its 'package.json' instead of choosing the first plugin-like directory.

pkg/fetcher/oci.go

dynamic-plugins-reference.goUse plugin selectors as dynamic plugin names +7/-3

Use plugin selectors as dynamic plugin names
• Changes dynamic plugin name resolution to return the '!plugin-path' selector when present. This allows references to distinguish plugins sharing one package artifact.
pkg/model/dynamic-plugins-reference.go

Tests (4) +104 / -21

integration_test.goTest selector-based plugin destination names +5/-0

Test selector-based plugin destination names
• Adds plugin-name cases for tagged, digest-based, and HTTPS multi-plugin package URLs. Verifies the suffix becomes the installation directory name.
cmd/plugin-fetch/integration_test.go

oci_test.goExpand plugin directory extraction coverage +51/-15

Expand plugin directory extraction coverage

• Updates extraction tests to supply expected directory names and covers multi-plugin selection, root-level packages, missing directories, missing manifests, and file-versus-directory errors.

pkg/fetcher/oci_test.go

dynamic-plugins-reference_test.goTest selector-aware plugin references and names +26/-6

Test selector-aware plugin references and names
• Aligns reference fixtures with selector-derived names and adds naming coverage for OCI, HTTPS, and local multi-plugin package URLs.
pkg/model/dynamic-plugins-reference_test.go

dynamic-plugins_test.goVerify selectors reach the plugin installer +22/-0

Verify selectors reach the plugin installer
• Adds a model test confirming that multi-plugin package selectors are preserved in the generated 'packages.txt' ConfigMap data.
pkg/model/dynamic-plugins_test.go

Other (2) +2 / -2

backstage-operator.clusterserviceversion.yamlRefresh Backstage operator bundle timestamp +1/-1

Refresh Backstage operator bundle timestamp
• Updates the generated ClusterServiceVersion creation timestamp for the Backstage operator bundle.
bundle/backstage.io/manifests/backstage-operator.clusterserviceversion.yaml

backstage-operator.clusterserviceversion.yamlRefresh RHDH operator bundle timestamp +1/-1

Refresh RHDH operator bundle timestamp
• Updates the generated ClusterServiceVersion creation timestamp for the RHDH operator bundle.
bundle/rhdh/manifests/backstage-operator.clusterserviceversion.yaml

rhdh-qodo-merge · 2026-09-17T12:01:44Z

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 🔗 Cross-repo conflicts (3) 📜 Skill insights (0)

1. ~~Valid selectors fetch wrong images~~ ✓ Resolved 🔗 Cross-repo conflict ≡ Correctness ⭐ New

Description

OCIFetcher.Fetch splits the package at the last !, although the RHDH installer defines the first
! as the image-selector separator and permits ! within the selector itself. A valid package such
as oci://registry/plugin:latest!my!plugin is consequently parsed as image plugin:latest!my and
cannot be fetched.

Code

pkg/fetcher/oci.go[R112-115]

+	if c.pluginMode {
+		if idx := strings.LastIndex(ref, "!"); idx != -1 {
+			pluginPath = ref[idx+1:]
+			ref = ref[:idx] // Strip !pluginPath for download

Relevance

●●● Strong
Using the wrong selector delimiter deterministically breaks valid OCI references.
PR-#3466

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR uses LastIndex, but the cross-repository installer documents, implements, and tests
first-separator parsing because selectors containing additional exclamation marks are valid.
pkg/fetcher/oci.go[110-116]
cmd/plugin-fetch/main.go[336-339]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts [32-51]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.test.ts [280-302]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
OCI plugin selectors may contain `!`, so splitting on the last occurrence corrupts the registry image reference.

## Fix Focus Areas
- pkg/fetcher/oci.go[110-116]
- cmd/plugin-fetch/main.go[336-340]

## Recommended Fix
Split OCI packages at the first `!` and preserve the entire remaining string as the plugin selector. Apply the same parsing rule wherever the selector or destination name is derived.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

2. Nested plugin paths fail installation 🔗 Cross-repo conflict ≡ Correctness ⭐ New

Description

pluginName uses the complete selector as the destination name, causing a selector such as
!path/to/plugin to place the temporary destination under parent directories that were never
created. Nested selectors supported by the RHDH installer therefore fail before OCI extraction when
MkdirTemp receives the missing nested parent.

Code

cmd/plugin-fetch/main.go[R336-339]

+	// If !pluginPath present, use it as the directory name
+	if idx := strings.LastIndex(url, "!"); idx != -1 {
+		return url[idx+1:]
+	}

Relevance

●●● Strong
Nested selector handling is a deterministic installation failure requiring parent-path support.
PR-#3466

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR turns the full nested selector into the output path, while its OCI fetcher creates a
temporary directory beneath that path's parent; the shared installer explicitly supports and safely
creates nested selector directories.
cmd/plugin-fetch/main.go[282-298]
cmd/plugin-fetch/main.go[334-340]
pkg/fetcher/oci.go[163-169]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/plugin-name.test.ts [42-47]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/tar-extract.ts [27-45]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Valid nested plugin selectors currently produce temporary destinations whose parent directories do not exist.

## Fix Focus Areas
- cmd/plugin-fetch/main.go[336-340]
- pkg/fetcher/oci.go[302-329]

## Recommended Fix
Preserve the complete selector for extraction while creating every required destination parent before temporary extraction. Ensure the resulting installed layout remains compatible with nested plugin paths.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

3. Plugin references stop resolving 🔗 Cross-repo conflict ≡ Correctness ⭐ New

Description

DynaPlugin.Name now returns the text after !, while resolveInheritReference and
resolveRefReference derive lookup keys from the selector-stripped artifact URL or reference text
and compare them with Name(). When an included package such as multi-plugin!plugin-A has a
selector that differs from its OCI image basename, inheritance by image URL and existing
ref://multi-plugin or {{inherit}} entries fails before model creation.

Code

pkg/model/dynamic-plugins-reference.go[R168-170]

+	// If !plugin-path present, return it as the plugin name
	if idx := strings.LastIndex(packageURL, "!"); idx != -1 {
-		packageURL = packageURL[:idx]
+		return packageURL[idx+1:]

Relevance

●●● Strong
Selector-based identity must remain consistent with inheritance and reference resolution.
PR-#3466

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR changes package identity to the selector, but inheritance strips the caller's selector before
deriving its lookup name, and both resolvers compare their artifact-derived keys with Name(); the
shared RHDH installer and its documentation likewise define reference identity by stripping
selectors and using the OCI image basename. The updated tests make every selector identical to the
artifact basename, so they do not exercise the demonstrated multi-plugin!plugin-A mismatch.
pkg/model/dynamic-plugins-reference.go[165-170]
pkg/model/dynamic-plugins-reference.go[83-105]
pkg/model/dynamic-plugins-reference.go[136-150]
pkg/model/dynamic-plugins-reference_test.go[10-46]
pkg/model/dynamic-plugins-reference_test.go[207-228]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/plugin-name.ts [28-45]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer.ts [240-285]
External repo: redhat-developer/rhdh, docs/dynamic-plugins/installing-plugins.md [182-218]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Reference resolution currently compares artifact-derived names with selected subdirectory names, preventing real multi-plugin entries from resolving through existing image URL, `ref://<image-name>`, and `{{inherit}}` forms. Artifact basename and selected plugin identity must remain distinct.

## Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[83-105]
- pkg/model/dynamic-plugins-reference.go[136-149]
- pkg/model/dynamic-plugins-reference.go[168-170]
- pkg/model/dynamic-plugins-reference_test.go[11-46]

## Recommended Fix
Represent the OCI artifact basename and selected plugin name separately. Strip the `!plugin-path` suffix before deriving the OCI image name in `DynaPlugin.Name`, keep selector extraction separate for installation and destination naming, and update inheritance and reference resolution to use the appropriate identity while preserving unambiguous legacy artifact-name lookup and supporting selector-name lookup. Add coverage for a base artifact whose basename differs from its selector, such as `multi-plugin!plugin-A`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

View high (3)

4. ~~Plugin packages can escape staging~~ ✓ Resolved 🐞 Bug ⛨ Security ⭐ New

Description

pluginName returns the raw suffix after !, and processParallel and OCI handling incorporate
that unchecked value directly into destination, cleanup, extraction, and rename paths beneath
outputDir. A configured suffix such as !../../target escapes the plugin staging directory,
including through OCI's root-package fallback, so operations can reach paths outside the configured
plugin directory.

Code

cmd/plugin-fetch/main.go[R336-340]

+	// If !pluginPath present, use it as the directory name
+	if idx := strings.LastIndex(url, "!"); idx != -1 {
+		return url[idx+1:]
+	}
+

Relevance

●●● Strong
Path traversal is a concrete security defect; recent fetcher security fixes were accepted.
PR-#3466

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The changed helper returns the user-configurable suffix verbatim, and callers join it directly with
the output directory for temporary, cleanup, extraction, and final rename paths without validating
containment. OCI independently repeats this selector-to-path conversion and can rename root-level
extracted content to the escaped destination; tar-path protection does not cover these later joins.
cmd/plugin-fetch/main.go[281-306]
cmd/plugin-fetch/main.go[336-340]
pkg/fetcher/oci.go[110-119]
pkg/fetcher/oci.go[186-200]
pkg/fetcher/oci.go[302-329]
cmd/plugin-fetch/main.go[286-305]
pkg/fetcher/oci.go[302-315]
pkg/fetcher/extract.go[58-90]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The selector text after `!` is used verbatim as a filesystem directory name. Empty, absolute, dot, traversal, or separator-containing selectors can escape the plugin output directory before cleanup, extraction, and rename operations, including through OCI's root-package fallback.

## Fix Focus Areas
- cmd/plugin-fetch/main.go[281-306]
- cmd/plugin-fetch/main.go[336-340]
- pkg/fetcher/oci.go[110-116]
- pkg/fetcher/oci.go[186-200]
- pkg/fetcher/oci.go[302-329]

## Recommended Fix
Parse selector-derived directory names through one validation helper that requires a non-empty single path component and rejects absolute paths, `.` and `..`, traversal paths, and names containing platform path separators. Apply the same validation to `expectedDir` before constructing the OCI extraction target, then resolve and verify before filesystem access that extraction and destination paths remain beneath their intended roots; add traversal-focused tests.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

5. HTTPS multi-plugin downloads fail 🐞 Bug ≡ Correctness ⭐ New

Description

pluginName and DynaPlugin.Name parse the text after an HTTPS !pluginPath only for the
destination name, while processParallel passes the unchanged pkg.URL through Fetcher to
HTTPFetcher, whose extractor expands the entire archive without applying the selector. For a URL
such as https://host/multi.tar.gz!plugin-A, the HTTP client requests the suffixed URL—typically
reaching another endpoint or a 404—and, even if an archive is returned, installs all of it rather
than only the requested plugin directory.

Code

cmd/plugin-fetch/main.go[R336-338]

+	// If !pluginPath present, use it as the directory name
+	if idx := strings.LastIndex(url, "!"); idx != -1 {
+		return url[idx+1:]

Relevance

●●● Strong
Recent plugin-fetch history accepts correctness fixes for URL parsing and download-path handling.
PR-#3466

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The new integration case establishes that HTTPS URLs with selectors are recognized, but it verifies
only destination-name extraction. The production path still forwards the original HTTPS URL through
fetcher dispatch, the HTTP client uses that URL directly, and HTTP extraction has no selector-aware
directory-selection or moving step comparable to OCI, proving that the suffix is neither removed
before the request nor applied to the extracted content.
cmd/plugin-fetch/integration_test.go[187-194]
cmd/plugin-fetch/main.go[336-340]
pkg/fetcher/fetcher.go[49-51]
pkg/fetcher/http.go[28-50]
pkg/fetcher/http.go[53-69]
pkg/fetcher/oci.go[110-116]
cmd/plugin-fetch/integration_test.go[190-194]
cmd/plugin-fetch/main.go[281-303]
pkg/fetcher/fetcher.go[45-63]
pkg/fetcher/http.go[27-47]
pkg/model/dynamic-plugins_test.go[1013-1017]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
HTTPS `!pluginPath` selectors are recognized for naming, but the fetch pipeline passes the unsanitized package URL to source fetchers and does not apply the selector during HTTP archive extraction. As a result, HTTP requests include the selector suffix, and a returned multi-plugin archive is expanded in full rather than installing only the selected plugin directory.

## Fix Focus Areas
- cmd/plugin-fetch/main.go[336-340]
- cmd/plugin-fetch/integration_test.go[190-194]
- pkg/model/dynamic-plugins-reference.go[168-170]
- pkg/fetcher/fetcher.go[45-63]
- pkg/fetcher/http.go[27-47]

## Recommended Fix
Parse and validate the package selector once before fetcher dispatch, pass the base URL without `!pluginPath` to the source fetcher, and retain the selector for destination and content selection. For HTTP archives, select the validated plugin directory after extraction using the same semantics as OCI; for any non-OCI source that cannot support equivalent subdirectory selection, reject selectors explicitly. Add an integration test that downloads a multi-plugin HTTP archive and verifies that only the selected directory is installed.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

6. Pathless plugins fail installation 🔗 Cross-repo conflict ≡ Correctness ⭐ New

Description

OCIFetcher.Fetch substitutes the OCI repository basename when no selector is supplied instead of
using the artifact’s declared plugin path, while movePluginContent checks only that derived
directory or the archive root. Normal single-plugin images whose immediate package directory differs
from the image name—including supported directories ending in -dynamic—therefore fail when the
expected directory and root package are absent, despite previously supported package discovery and
the RHDH installer contract.

Code

pkg/fetcher/oci.go[R188-191]

+	expectedDir := pluginPath
+	if expectedDir == "" {
+		// Extract image name from OCI reference (last path component)
+		repo := imgRef.Context().RepositoryStr()

Relevance

●● Moderate
Likely compatibility regression, but automatic OCI package discovery is a more semantic behavior
change.
PR-#3466

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR hard-codes the repository basename for normal plugin fetching and then fails when neither
that immediate directory nor a package at the archive root exists. The cross-repository installer
contract instead auto-detects the sole path from OCI dynamic-packages metadata and tests a path
different from the image name; repository examples likewise use package directories ending in
-dynamic while their OCI repository names omit that suffix.
pkg/fetcher/oci.go[186-200]
cmd/plugin-fetch/main.go[122-124]
pkg/fetcher/fetcher.go[49-55]
pkg/fetcher/oci.go[302-329]
docs/configuration.md[810-810]
examples/rhdh-cr-with-app-configs.yaml[99-101]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/oci-key.ts [47-59]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/oci-key.ts [91-134]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/oci-key.test.ts [195-207]
External repo: redhat-developer/rhdh, docs/dynamic-plugins/installing-plugins.md [218-229]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Pathless single-plugin OCI artifacts must not assume that the extracted directory equals the repository basename. They should use the sole plugin path declared by the dynamic-packages annotation, preserving support for artifact layouts where the package directory differs from the image name.

## Fix Focus Areas
- pkg/fetcher/oci.go[186-200]
- pkg/fetcher/oci.go[302-329]
- pkg/fetcher/oci_test.go[339-367]

## Recommended Fix
Use exact directory selection only when an explicit `!pluginPath` selector is supplied. When no selector is present, read the OCI dynamic-packages annotation and accept its sole declared path rather than deriving the repository basename; preserve the existing error for zero plugins, require an explicit selector when multiple paths are declared, and add a regression test where the package directory differs from the repository basename.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

7. ~~One cost plugin is omitted~~ ✓ Resolved 🔗 Cross-repo conflict ≡ Correctness

Description

DynamicPlugins.addToModel truncates every non-local package at the last !, discarding both
legacy OCI plugin-directory selectors and valid ! characters in supported HTTP(S) paths or query
strings. This collapses the Cost Management frontend and backend references that share an image,
while URLs such as https://example.com/plugin.tgz?token=a!b reach packages.txt as a different
fetch URL and OCI installation derives one destination and moves only the first plugin directory
found.

Code

pkg/model/dynamic-plugins.go[R173-175]

+				if idx := strings.LastIndex(plugin.Package, "!"); idx != -1 {
+					klog.Warningf("package %q contains deprecated '!plugin-path' syntax", plugin.Package)
+					plugin.Package = plugin.Package[:idx]

Relevance

● Weak
Recent precedent rejected preserving multi-plugin OCI identity with !plugin-path suffix; this PR
explicitly deprecates and strips that syntax.
PR-#3422

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The operator change removes text after ! before adding package references to enabledPlugins and
writing packages.txt, even though HTTP and HTTPS direct links are supported and their complete
URLs are otherwise preserved for fetching. The rhdh-plugins documentation also defines the Cost
Management frontend and backend with the same OCI image but different suffixes; once those selectors
are removed, destination naming receives identical URLs and OCI extraction selects only the first
plugin directory, demonstrating both the direct-link corruption and the loss of independent
bundled-plugin selection.
pkg/model/dynamic-plugins.go[172-181]
cmd/plugin-fetch/main.go[286-305]
pkg/fetcher/oci.go[280-311]
pkg/model/dynamic-plugins-reference.go[27-30]
pkg/model/dynamic-plugins-reference.go[68-73]
pkg/model/dynamic-plugins.go[173-181]
cmd/plugin-fetch/main.go[239-251]
External repo: redhat-developer/rhdh-plugins, workspaces/cost-management/docs/dynamic-plugin.md [34-37]
External repo: redhat-developer/rhdh-plugins, workspaces/cost-management/docs/dynamic-plugin.md [67-74]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The normalization in `DynamicPlugins.addToModel` treats every final `!` as deprecated plugin-path syntax. This both truncates valid HTTP(S) URLs whose paths or query strings contain `!` and strips the selectors that distinguish multiple plugins bundled in one OCI image, including the Cost Management frontend and backend.

## Fix Focus Areas
- pkg/model/dynamic-plugins.go[172-181]
- pkg/model/dynamic-plugins_test.go[998-1019]
- cmd/plugin-fetch/main.go[286-305]
- pkg/fetcher/oci.go[280-311]
- /cross_repos/rhdh-plugins/workspaces/cost-management/docs/dynamic-plugin.md[34-74]

## Recommended Fix
Only parse deprecated `!plugin-path` suffixes for package formats where that legacy syntax is defined, such as OCI references, and preserve `!` characters in HTTP(S) paths and query strings. Retain a parsed OCI suffix as the requested plugin-directory selector and update extraction to move that selected directory instead of the first directory found; alternatively, coordinate republishing and documenting each Cost Management plugin as a separate image before removing bundled-image support. Add regression tests covering HTTP(S) URLs containing `!`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

Context sources

✅ Compliance rules (platform): 14 rules

✅ Cross-repo context — repo relationships

Explored: repo: redhat-appstudio/backstage-community-plugins (sha: 7eb22666)

Explored: repo: redhat-developer/rhdh (sha: 585b986c)

Explored: repo: redhat-developer/rhdh-plugins (sha: 58cfc520)

Review mode: 🧠 Deep: This push contains substantial, independent logic across CI workflows, shell tooling, Kubernetes configuration, Go fetch/model code, and end-to-end tests, creating a high density of easy-to-miss defects.

Tip of the day

💡 Did you know, you can reply 'qodo' on any finding to push back, ask questions, or dig deeper

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Previous reviews

Review updated until commit 8bc99b8

Results up to commit 1cfda03 ⚖️ Balanced

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. ~~One cost plugin is omitted~~ ✓ Resolved 🔗 Cross-repo conflict ≡ Correctness

Description

DynamicPlugins.addToModel truncates every non-local package at the last !, discarding both
legacy OCI plugin-directory selectors and valid ! characters in supported HTTP(S) paths or query
strings. This collapses the Cost Management frontend and backend references that share an image,
while URLs such as https://example.com/plugin.tgz?token=a!b reach packages.txt as a different
fetch URL and OCI installation derives one destination and moves only the first plugin directory
found.

Code

pkg/model/dynamic-plugins.go[R173-175]

+				if idx := strings.LastIndex(plugin.Package, "!"); idx != -1 {
+					klog.Warningf("package %q contains deprecated '!plugin-path' syntax", plugin.Package)
+					plugin.Package = plugin.Package[:idx]

Relevance

● Weak
Recent precedent rejected preserving multi-plugin OCI identity with !plugin-path suffix; this PR
explicitly deprecates and strips that syntax.
PR-#3422

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The operator change removes text after ! before adding package references to enabledPlugins and
writing packages.txt, even though HTTP and HTTPS direct links are supported and their complete
URLs are otherwise preserved for fetching. The rhdh-plugins documentation also defines the Cost
Management frontend and backend with the same OCI image but different suffixes; once those selectors
are removed, destination naming receives identical URLs and OCI extraction selects only the first
plugin directory, demonstrating both the direct-link corruption and the loss of independent
bundled-plugin selection.
pkg/model/dynamic-plugins.go[172-181]
cmd/plugin-fetch/main.go[286-305]
pkg/fetcher/oci.go[280-311]
pkg/model/dynamic-plugins-reference.go[27-30]
pkg/model/dynamic-plugins-reference.go[68-73]
pkg/model/dynamic-plugins.go[173-181]
cmd/plugin-fetch/main.go[239-251]
External repo: redhat-developer/rhdh-plugins, workspaces/cost-management/docs/dynamic-plugin.md [34-37]
External repo: redhat-developer/rhdh-plugins, workspaces/cost-management/docs/dynamic-plugin.md [67-74]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The normalization in `DynamicPlugins.addToModel` treats every final `!` as deprecated plugin-path syntax. This both truncates valid HTTP(S) URLs whose paths or query strings contain `!` and strips the selectors that distinguish multiple plugins bundled in one OCI image, including the Cost Management frontend and backend.

## Fix Focus Areas
- pkg/model/dynamic-plugins.go[172-181]
- pkg/model/dynamic-plugins_test.go[998-1019]
- cmd/plugin-fetch/main.go[286-305]
- pkg/fetcher/oci.go[280-311]
- /cross_repos/rhdh-plugins/workspaces/cost-management/docs/dynamic-plugin.md[34-74]

## Recommended Fix
Only parse deprecated `!plugin-path` suffixes for package formats where that legacy syntax is defined, such as OCI references, and preserve `!` characters in HTTP(S) paths and query strings. Retain a parsed OCI suffix as the requested plugin-directory selector and update extraction to move that selected directory instead of the first directory found; alternatively, coordinate republishing and documenting each Cost Management plugin as a separate image before removing bundled-image support. Add regression tests covering HTTP(S) URLs containing `!`.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

rhdh-qodo-merge · 2026-09-17T12:01:52Z

Important

The /generate_labels command by Qodo is sunsetting on the 1st of October 2026 and will no longer be available. We recommend switching to the latest Qodo review capabilities. Learn more

codecov · 2026-09-17T12:03:15Z

Codecov Report

❌ Patch coverage is 59.57447% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.84%. Comparing base (6f75d0a) to head (8bc99b8).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
pkg/fetcher/oci.go	53.65%	18 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3556      +/-   ##
==========================================
- Coverage   60.00%   59.84%   -0.16%     
==========================================
  Files          51       51              
  Lines        3560     3586      +26     
==========================================
+ Hits         2136     2146      +10     
- Misses       1230     1246      +16     
  Partials      194      194

Flag	Coverage Δ
nightly	`?`
unittests	`59.84% <59.57%> (-0.16%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
cmd/plugin-fetch/main.go	`11.11% <ø> (ø)`
pkg/model/dynamic-plugins-reference.go	`87.00% <100.00%> (ø)`
pkg/model/dynamic-plugins.go	`80.18% <100.00%> (+0.18%)`	⬆️
pkg/fetcher/oci.go	`55.70% <53.65%> (-4.30%)`	⬇️

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-09-23T15:23:29Z

✅ PR images built and pushed successfully!

Images are available for testing (expires in 7 days):

Image	Full tag	PR tag
Operator	`quay.io/rhdh-community/operator:2.1.0-pr-3556-2b0bf1c`	`quay.io/rhdh-community/operator:2.1.0-pr-3556`
Bundle	`quay.io/rhdh-community/operator-bundle:2.1.0-pr-3556-2b0bf1c`	`quay.io/rhdh-community/operator-bundle:2.1.0-pr-3556`
Catalog	`quay.io/rhdh-community/operator-catalog:2.1.0-pr-3556-2b0bf1c`	`quay.io/rhdh-community/operator-catalog:2.1.0-pr-3556`

github-actions · 2026-09-23T15:27:43Z

✅ PR images built and pushed successfully!

Images are available for testing (expires in 7 days):

Image	Full tag	PR tag
Operator	`quay.io/rhdh-community/operator:2.1.0-pr-3556-2b0bf1c`	`quay.io/rhdh-community/operator:2.1.0-pr-3556`
Bundle	`quay.io/rhdh-community/operator-bundle:2.1.0-pr-3556-2b0bf1c`	`quay.io/rhdh-community/operator-bundle:2.1.0-pr-3556`
Catalog	`quay.io/rhdh-community/operator-catalog:2.1.0-pr-3556-2b0bf1c`	`quay.io/rhdh-community/operator-catalog:2.1.0-pr-3556`

rhdh-qodo-merge · 2026-09-23T15:29:06Z

Code review by qodo was updated up to the latest commit 2b0bf1c

…into warn-on-!plugin-path-occurence

sonarqubecloud · 2026-09-23T19:09:52Z

Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

github-actions · 2026-09-23T19:13:56Z

✅ PR images built and pushed successfully!

Images are available for testing (expires in 7 days):

Image	Full tag	PR tag
Operator	`quay.io/rhdh-community/operator:2.1.0-pr-3556-8bc99b8`	`quay.io/rhdh-community/operator:2.1.0-pr-3556`
Bundle	`quay.io/rhdh-community/operator-bundle:2.1.0-pr-3556-8bc99b8`	`quay.io/rhdh-community/operator-bundle:2.1.0-pr-3556`
Catalog	`quay.io/rhdh-community/operator-catalog:2.1.0-pr-3556-8bc99b8`	`quay.io/rhdh-community/operator-catalog:2.1.0-pr-3556`

warn and strip "!plugin"

1cfda03

gazarenkov requested a review from a team as a code owner September 17, 2026 11:58

openshift-ci Bot requested review from Fortune-Ndlovu and OpinionatedHeron September 17, 2026 11:58

rhdh-qodo-merge Bot added Tests Bug fix labels Sep 17, 2026

gazarenkov requested a review from jonkoops September 17, 2026 12:29

jonkoops approved these changes Sep 17, 2026

View reviewed changes

openshift-ci Bot assigned jonkoops Sep 17, 2026

openshift-ci Bot added the lgtm label Sep 17, 2026

gazarenkov marked this pull request as draft September 22, 2026 14:16

openshift-ci Bot added the do-not-merge/work-in-progress label Sep 22, 2026

gazarenkov changed the title ~~Warn on and strip "!plugin-path" in dynamic plugins package~~ Process "!plugin-path" in dynamic plugins Sep 23, 2026

support multiplugins

8c88bbb

openshift-ci Bot added needs-rebase and removed lgtm labels Sep 23, 2026

Merge branch 'main' into warn-on-!plugin-path-occurence

2b0bf1c

openshift-ci Bot removed the needs-rebase label Sep 23, 2026

gazarenkov requested a review from jonkoops September 23, 2026 15:19

gazarenkov marked this pull request as ready for review September 23, 2026 15:23

openshift-ci Bot removed the do-not-merge/work-in-progress label Sep 23, 2026

openshift-ci Bot requested review from rm3l and subhashkhileri September 23, 2026 15:23

rhdh-qodo-merge Bot reviewed Sep 23, 2026

View reviewed changes

Comment thread cmd/plugin-fetch/main.go Resolved

Comment thread cmd/plugin-fetch/main.go Resolved

Comment thread pkg/model/dynamic-plugins-reference.go Resolved

Comment thread pkg/fetcher/oci.go Resolved

Comment thread cmd/plugin-fetch/main.go Resolved

Comment thread pkg/fetcher/oci.go Resolved

rhdh-qodo-merge Bot added enhancement New feature or request and removed Bug fix labels Sep 23, 2026

gazarenkov added 2 commits September 23, 2026 22:09

fix

21358d0

Merge remote-tracking branch 'origin/warn-on-!plugin-path-occurence' …

8bc99b8

…into warn-on-!plugin-path-occurence

jonkoops approved these changes Sep 24, 2026

View reviewed changes

openshift-ci Bot added the lgtm label Sep 24, 2026

rm3l approved these changes Sep 24, 2026

View reviewed changes

openshift-ci Bot assigned rm3l Sep 24, 2026

openshift-merge-bot Bot merged commit b0d6a0d into redhat-developer:main Sep 24, 2026
11 checks passed

gazarenkov deleted the warn-on-!plugin-path-occurence branch October 1, 2026 08:59

Navigation Menu

Repository navigation

Process "!plugin-path" in dynamic plugins - #3556

Process "!plugin-path" in dynamic plugins #3556
openshift-merge-bot[bot] merged 5 commits into
redhat-developer:mainfrom
gazarenkov:warn-on-!plugin-path-occurence

gazarenkov commented Sep 17, 2026

Uh oh!

rhdh-qodo-merge Bot commented Sep 17, 2026 •

edited

Loading

Uh oh!

rhdh-qodo-merge Bot commented Sep 17, 2026 •

edited

Loading

Uh oh!

rhdh-qodo-merge Bot commented Sep 17, 2026

Uh oh!

codecov Bot commented Sep 17, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhdh-qodo-merge Bot commented Sep 23, 2026

Uh oh!

sonarqubecloud Bot commented Sep 23, 2026

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

gazarenkov commented Sep 17, 2026

Description

Which issue(s) does this PR fix or relate to

PR acceptance criteria

How to test changes / Special notes to the reviewer

Building Container Images for Testing

Uh oh!

rhdh-qodo-merge Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Summary by Qodo

Uh oh!

rhdh-qodo-merge Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Review by Qodo

Uh oh!

rhdh-qodo-merge Bot commented Sep 17, 2026

Uh oh!

codecov Bot commented Sep 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rhdh-qodo-merge Bot commented Sep 23, 2026

Uh oh!

sonarqubecloud Bot commented Sep 23, 2026

Quality Gate passed

Uh oh!

github-actions Bot commented Sep 23, 2026

Uh oh!

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

rhdh-qodo-merge Bot commented Sep 17, 2026 •

edited

Loading

rhdh-qodo-merge Bot commented Sep 17, 2026 •

edited

Loading

codecov Bot commented Sep 17, 2026 •

edited

Loading