You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
❌ Patch coverage is 82.66667% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.49%. Comparing base (3713974) to head (e24758f). ⚠️ Report is 8 commits behind head on main.
• Add ref:// resolution and name-based {{inherit}} matching for dynamic plugins.
• Validate package URL prefixes; reject unsupported formats during operator-side reference
resolution.
• Update docs and tests for new URL formats and local-path defaults.
Diagram
graph TD
U["User plugins YAML"] --> M["MergePluginsData"] --> R["resolveReferences"] --> D{"Package type?"}
D --> REF["resolveRefReference"] --> N["Name() extraction"]
D --> INH["resolveInheritReference"] --> N
D --> DIR["Direct link"] --> OUT["Merged plugins YAML"]
N --> OUT
Loading
High-Level Assessment
The following are alternative approaches to this PR:
➕ Potentially aligns with Backstage ecosystem packaging expectations
➖ Not aligned with current operator install flow (OCI/archive/path oriented)
➖ Increases validation surface and resolution ambiguity
Recommendation: The PR’s approach (explicit allowed-prefix validation + name-based lookup for both ref:// and {{inherit}}) is coherent and simplifies user overrides across registries/paths. If this logic becomes more complex over time, consider moving from string parsing toward URL/OCI parsers; for now the current implementation is a reasonable trade-off, especially with the added tests and explicit errors for unsupported formats.
Files changed (12) +368 / -190
Enhancement (1) +167 / -67
dynamic-plugins-reference.goImplement ref:// resolution, name-based inherit matching, and prefix validation+167/-67
Implement ref:// resolution, name-based inherit matching, and prefix validation
• Implements ref://plugin-name lookup against base plugins and changes {{inherit}} resolution to match by extracted plugin name rather than registry/path. Adds direct-link detection for supported prefixes (oci/http(s)/./), rejects unsupported formats, and expands Name() extraction to support OCI, HTTP(S) archives, and local paths.
dynamic-plugins-reference_test.goAdd coverage for ref://, name extraction, and unsupported prefix errors+119/-67
Add coverage for ref://, name extraction, and unsupported prefix errors
• Reworks inherit tests to use basePlugins directly (name-based matching) and adds cases for cross-registry name matches. Adds new tests for ref:// resolution, Name() extraction from OCI/HTTP(S)/local formats, and validates errors for unsupported protocols and npm-style packages.
dynamic-plugins_test.goUpdate dynamic plugin merge tests to use './' package prefixes+29/-35
Update dynamic plugin merge tests to use './' package prefixes
• Rewrites multiple YAML fixtures embedded in tests to use local-path style packages (e.g., './plugin-a') so they are treated as valid direct links under the new validation behavior.
dynamic-plugins-base.yamlUse './' package prefixes in base dynamic-plugins testdata+2/-2
Use './' package prefixes in base dynamic-plugins testdata
• Updates test ConfigMap data to represent plugin packages as local paths, ensuring they pass package URL prefix validation during reference resolution.
dynamic-plugins.mdDocument supported package URL formats and name-based reference resolution+35/-3
Document supported package URL formats and name-based reference resolution
• Adds a table of supported package URL formats and clarifies that both ref:// and :{{inherit}} resolve by plugin name only. Includes examples showing registry/path are ignored for inherit matching and adds a TODO note for documenting processing mode.
⚠️ Tickets: not configured — ticket URL found in PR but could not be fetched — check ticket provider credentials
1. ref:// leaks to installer✗ Dismissed🔗 Cross-repo conflict☼ Reliability
Description
The PR documents ref://plugin-name as a supported package format, but the operator only resolves
references when OPERATOR_DP_PROCESSING is enabled; otherwise ref://... reaches
install-dynamic-plugins, which treats non-OCI/non-./ packages as NPM specs and has no ref://
handling. This can cause runtime install failures across the operator → installer boundary when
users follow the new docs.
+The operator optionally supports special URL reference syntax in plugin package URLs, allowing users to reference plugins from the default configuration by name.++TODO: document Operator Dynamic Plugins processing mode
**Operator behavior:**
- The operator resolves all references during ConfigMap merge (before passing to the init container)- If a reference cannot be resolved, the operator returns an error and the Backstage CR will not reconcile+- Both reference types use **name-based matching** - only the plugin name matters for lookup++### Ref Reference (`ref://`)++Look up a plugin by name and use its full package URL from the default configuration.++```yaml+plugins:+ - package: "ref://backstage-plugin-catalog"+ pluginConfig:+ # your config overrides+```
Relevance
⭐⭐⭐ High
Team often fixes docs to match operator behavior; OPERATOR_DP_PROCESSING mode recently formalized in
PR #3190.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The operator docs add ref:// usage and claim operator-side resolution, but operator code gates
reference resolution behind OPERATOR_DP_PROCESSING. The downstream installer categorizes any
non-OCI/non-./ package as NPM, so ref://... will be misrouted/unsupported if it reaches the
installer.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`ref://` is newly documented as a supported package format, but it is only resolved in operator code paths when `OPERATOR_DP_PROCESSING` is enabled. In the normal (default) flow, the init container runs the `rhdh-plugins` installer, which does not recognize `ref://` and will treat it as an NPM spec.
### Issue Context
This is a cross-repo contract issue: the operator advertises a syntax that the downstream installer does not implement, and the operator does not always resolve it before handing off.
### Fix Focus Areas
- docs/dynamic-plugins.md[98-129]
- pkg/model/dynamic-plugins.go[74-76]
- pkg/model/dynamic-plugins.go[313-320]
### What to change
Choose one of these coordinated fixes:
1) **Always resolve `ref://` (and `{{inherit}}`) during merge** regardless of `OPERATOR_DP_PROCESSING`, so the installer never sees `ref://`.
2) **Hard-reject `ref://` when processing mode is disabled**, with a clear error message telling users how to enable the required mode.
3) **Add `ref://` support to the installer** (rhdh-plugins) and coordinate releases + docs.
Also update the docs to explicitly state the prerequisite (if any) and the exact execution mode in which `ref://` works.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
DynaPlugin.Name() strips everything after the last : in an OCI reference, which misinterprets
registry host:port as an image tag when no tag is present (e.g.,
oci://localhost:5000/path/plugin). This breaks name-based matching for ref:// and
:{{inherit}}, causing failed resolution or wrong base-plugin selection.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The code strips the OCI tag by truncating at the last :, with no check that the colon is after the
last /; this will truncate registry:5000/... and return the wrong name, which is then used for
name-based lookups in resolveRefReference/resolveInheritReference.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`DynaPlugin.Name()` currently removes an OCI tag by doing `LastIndex(":")` on the post-`oci://` string. This incorrectly treats `host:port` as a tag delimiter when the reference has no tag, producing an incorrect plugin name and breaking `ref://` / `:{{inherit}}` resolution.
### Issue Context
OCI references commonly include a registry port (e.g., `localhost:5000`). Tag separators should only be treated as a tag if they occur in the last path segment (i.e., after the last `/`), not in the registry authority.
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[163-183]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
3. Malformed OCI name accepted✓ Resolved🐞 Bug≡ Correctness⭐ New
Description
DynaPlugin.Name() will return a non-empty name for malformed OCI references that contain only a
registry host:port and no repository/image (e.g., oci://localhost:5000 becomes localhost). When
such a value is used with :{{inherit}}, resolveInheritReference uses that derived name for
matching and can select an unintended base plugin or emit misleading “no plugin named …” errors.
+ // Extract the last path component (the image name, possibly with tag)+ imageName := url+ if idx := strings.LastIndex(url, "/"); idx != -1 {+ imageName = url[idx+1:]+ }++ // Remove tag (:tag) from image name only (not port from registry)+ if idx := strings.LastIndex(imageName, ":"); idx != -1 {+ imageName = imageName[:idx]+ }++ return imageName
Relevance
⭐⭐⭐ High
Repo accepts strict validation to avoid misleading behavior (PR #3021). {{inherit}} resolution error
semantics discussed in PR #3144.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
Name() initializes imageName to the entire trimmed OCI string and only switches to the last path
component if a / exists; it then strips the last : segment from imageName, which turns a
registry-only host:port into host. resolveInheritReference() depends on Name() and only
errors when the extracted name is empty, so a non-empty (but incorrect) name can be used for
matching.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`DynaPlugin.Name()` derives the plugin name from the OCI URL by taking the last path component and stripping a tag. If the OCI reference has **no `/` path component** (registry-only, e.g. `oci://localhost:5000`), the function currently treats the entire host:port as the “image name” and then strips the port, returning `localhost`.
This is problematic because `resolveInheritReference()` uses `Name()` output for name-based matching; malformed inputs can therefore match the wrong base plugin name or produce confusing lookup errors.
## Issue Context
- `Name()` is used for both `ref://` and `:{{inherit}}` name-based lookups.
- `resolveInheritReference()` only errors when `Name()` returns an empty string; returning `localhost` for a registry-only reference bypasses that guard.
## Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[173-184]
- pkg/model/dynamic-plugins-reference_test.go[257-266]
## Suggested fix
- In the OCI parsing branch of `Name()`, require that the trimmed OCI string contains a `/` and that the extracted last path component is non-empty.
- If there is no `/` (or the last component is empty), return `""` so callers fail cleanly with the existing `pluginName == ""` guard.
- Add a regression test case (near the existing host:port tests) to assert `Name()` returns `""` (or otherwise indicate invalid) for `oci://localhost:5000` and/or that `oci://localhost:5000:{{inherit}}` fails resolution as “unable to extract plugin name”.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
When OPERATOR_DP_PROCESSING is enabled, the operator now errors on any plugin package that isn’t
oci/http(s)/./ or ref://, which rejects NPM package specs that the rhdh-plugins installer and RHDH
docs explicitly support. This creates a cross-repo contract break where valid dynamic-plugins.yaml
inputs for install-dynamic-plugins will fail reconciliation in the operator.
+ case plugin.IsDirectLink():+ // Direct link - no resolution needed
continue
+ default:+ return nil, fmt.Errorf("unsupported package URL format %q: must start with oci://, https://, http://, ./ or use ref:// for catalog lookup", plugin.Package)
}
Relevance
⭐⭐⭐ High
Likely treated as contract-breaking regression; operator/dynamic-plugins behavior changes are
actively refined (e.g., #3144, #3190).
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The PR adds an operator-side allowlist that errors on NPM package specs, while the downstream
rhdh-plugins installer and RHDH docs explicitly support NPM packages in dynamic-plugins.yaml
(and the installer categorizes non-OCI/non-./ as NPM).
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
When `OPERATOR_DP_PROCESSING` is enabled, `resolveReferences()` enforces a strict allowlist of package URL prefixes and returns an error for anything else. This rejects NPM package references (and other npm-supported specs) that are valid inputs for `rhdh-plugins`' `install-dynamic-plugins` and documented in `rhdh`.
### Issue Context
`rhdh-plugins` installer supports NPM-sourced plugins and treats any non-OCI and non-local (`./`) package string as an NPM spec to install. RHDH documentation also describes using `@scope/name@version` in `dynamic-plugins.yaml`.
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[46-58]
- pkg/model/dynamic-plugins-reference_test.go[128-140]
### What to change
- Make `resolveReferences()` only resolve *reference syntaxes* (`ref://...` and `...:{{inherit}}...`) and **not** hard-fail other package formats.
- Concretely: after handling `ref://` and `{{inherit}}`, `default` should `continue` (leave package unchanged) rather than returning an error.
- Optionally add a narrower validation mode that can be enabled explicitly, but keep default behavior compatible with installer-supported formats.
- Update/adjust tests accordingly (remove the expectation that `@backstage/plugin-catalog` errors) or gate that test behind a dedicated strict-validation flag.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The documentation states the :{{inherit}} behavior is "slightly different" but does not specify
the version boundary where the behavior changed. This violates the requirement to document behavior
changes with an explicit version, which can confuse users on older/newer releases.
+Note: this behavior is similar to `ref://` and slightly different from what is described in [OCI Package Version Inheritance](https://github.com/redhat-developer/rhdh/blob/main/docs/dynamic-plugins/installing-plugins.md#oci-package-version-inheritance).
Relevance
⭐⭐ Medium
Mixed history on adding explicit version boundaries in docs (accepted in #1406, rejected in #1484).
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
PR Compliance ID 13 requires explicit version boundaries when docs describe behavior differences
across versions. The added note says behavior is "slightly different" from the linked documentation,
but provides no "since/before vX.Y.Z" boundary.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`docs/dynamic-plugins.md` describes a behavior difference for `:{{inherit}}` (and its similarity to `ref://`) without stating which versions the new vs old behavior applies to.
## Issue Context
Compliance requires that any documentation describing behavior differences across versions includes an explicit version boundary (e.g., "Before vX.Y.Z" / "Since vX.Y.Z").
## Fix Focus Areas
- docs/dynamic-plugins.md[131-131]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
View more (2) 6. Overbroad version stripping 🐞 Bug≡ Correctness
Description
stripVersionSuffix treats any trailing -<digit> as a version start, so valid plugin names like
plugin-2fa or date-suffixed names can be truncated during HTTP(S) Name() extraction. This can
prevent ref:// / :{{inherit}} name matching for HTTP(S)-based plugin packages.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
Name() applies stripVersionSuffix() for HTTP(S) URLs, and stripVersionSuffix() strips at any
- followed by a digit, which can remove non-version parts of a plugin name and change the lookup
key used by reference resolution.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`stripVersionSuffix()` removes everything after the last `-` that precedes *any* digit. This is too broad and can truncate legitimate plugin names that happen to contain `-<digit>` but are not versioned.
### Issue Context
`Name()` applies `stripVersionSuffix()` to HTTP(S) archive names, so incorrect stripping changes the key used for name-based resolution.
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[185-209]
- pkg/model/dynamic-plugins-reference.go[223-233]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The operator now resolves :{{inherit}} by **plugin name only** (ignoring registry/path), but the
rhdh-plugins installer (and RHDH docs) define inheritance matching using an OCI image key (image
name + plugin path) from included configs. This cross-repo semantic drift can make operator-side
resolution behave differently than installer-side expectations and documentation.
+ // Extract plugin name from the package URL (strip :{{inherit}} first)+ tempPackage := strings.Replace(packageURL, inheritSuffix, "", 1)+ tempPlugin := DynaPlugin{Package: tempPackage}+ pluginName := tempPlugin.Name()- // Look up the full URL in basePlugins- fullURL, found := baseURLMap[baseURL]- if !found {- return "", fmt.Errorf("cannot resolve {{inherit}} reference: no matching plugin found for base URL %q in default plugins", baseURL)+ if pluginName == "" {+ return "", fmt.Errorf("cannot resolve {{inherit}} reference: unable to extract plugin name from %q", packageURL)
}
- // If user specified !plugin-path, use it; otherwise use full default URL- if pluginPath != "" {- // Extract image part from default (without !plugin-path)- if idx := strings.LastIndex(fullURL, "!"); idx != -1 {- fullURL = fullURL[:idx]+ // Look up the plugin by name in basePlugins+ for i := range basePlugins {+ plugin := &basePlugins[i]+ if plugin.Package == "" {+ continue+ }+ if plugin.Name() == pluginName {+ fullURL := plugin.Package++ // If user specified !plugin-path, use it; otherwise use full default URL+ if pluginPath != "" {+ // Extract image part from default (without !plugin-path)+ if idx := strings.LastIndex(fullURL, "!"); idx != -1 {+ fullURL = fullURL[:idx]+ }+ return fullURL + pluginPath, nil+ }+
Relevance
⭐⭐ Medium
{{inherit}} semantics were recently changed/iterated; could be intentional divergence or require
alignment—unclear from history.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The PR explicitly documents and implements name-only inheritance, while the rhdh-plugins installer
resolves {{inherit}} by matching keys derived from the OCI image reference (and RHDH docs define
the matching key as OCI image name + plugin path).
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`resolveInheritReference()` now matches base plugins by `DynaPlugin.Name()` (last path component), explicitly ignoring registry/path. However, `rhdh-plugins`' installer resolves `{{inherit}}` by matching plugins from the *same OCI image key* (registry/path + plugin path), and RHDH docs describe the matching key similarly.
### Issue Context
If operator-side reference resolution is enabled/used, it should align with the installer’s documented contract to avoid surprising differences between environments where the operator resolves vs where the installer resolves.
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[76-121]
- docs/dynamic-plugins.md[120-131]
### What to change
- Change operator-side `{{inherit}}` resolution to match using the same key semantics as the installer/docs (OCI image name + plugin path), rather than name-only.
- At minimum: require the same registry/path image reference when inheriting (and optionally also include the `!plugin-path` when present).
- Consider parsing OCI packages similarly to the installer’s `ociPluginKey` behavior (image key + optional path).
- Update operator docs to match the chosen canonical semantics and remove/clarify statements that “registry/path is ignored” if that’s no longer true.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
8. Inefficient base plugin scans 🐞 Bug➹ Performance
Description
Reference resolution repeatedly rescans basePlugins and recomputes plugin.Name() for each
reference, making the overall resolution cost O(#references × #basePlugins). This can slow
reconciliation when the default plugin catalog grows.
+ // Look up the plugin by name in basePlugins+ for i := range basePlugins {+ plugin := &basePlugins[i]+ if plugin.Package == "" {+ continue+ }+ if plugin.Name() == pluginName {+ fullURL := plugin.Package++ // If user specified !plugin-path, use it; otherwise use full default URL+ if pluginPath != "" {+ // Extract image part from default (without !plugin-path)+ if idx := strings.LastIndex(fullURL, "!"); idx != -1 {+ fullURL = fullURL[:idx]+ }+ return fullURL + pluginPath, nil+ }++ return fullURL, nil
}
- return fullURL + pluginPath, nil
Relevance
⭐ Low
No prior evidence team optimizes small O(n×m) scans in model code without measured impact.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
Both ref and inherit resolvers iterate over all base plugins and call plugin.Name() inside the
loop, so resolving many references causes repeated full scans and repeated name parsing.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`resolveInheritReference()` and `resolveRefReference()` linearly scan `basePlugins` on every call and call `Name()` for each candidate. Across multiple references this becomes O(N×M) work and repeats parsing.
### Issue Context
The previous implementation used a precomputed lookup map; the new name-based matching can still use a precomputed `map[name]packageURL` (and ideally detect duplicate names).
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[34-66]
- pkg/model/dynamic-plugins-reference.go[99-121]
- pkg/model/dynamic-plugins-reference.go[124-147]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
1. ref:// leaks to installer✗ Dismissed🔗 Cross-repo conflict☼ Reliability
Description
The PR documents ref://plugin-name as a supported package format, but the operator only resolves
references when OPERATOR_DP_PROCESSING is enabled; otherwise ref://... reaches
install-dynamic-plugins, which treats non-OCI/non-./ packages as NPM specs and has no ref://
handling. This can cause runtime install failures across the operator → installer boundary when
users follow the new docs.
+The operator optionally supports special URL reference syntax in plugin package URLs, allowing users to reference plugins from the default configuration by name.++TODO: document Operator Dynamic Plugins processing mode
**Operator behavior:**
- The operator resolves all references during ConfigMap merge (before passing to the init container)- If a reference cannot be resolved, the operator returns an error and the Backstage CR will not reconcile+- Both reference types use **name-based matching** - only the plugin name matters for lookup++### Ref Reference (`ref://`)++Look up a plugin by name and use its full package URL from the default configuration.++```yaml+plugins:+ - package: "ref://backstage-plugin-catalog"+ pluginConfig:+ # your config overrides+```
Relevance
⭐⭐⭐ High
Team often fixes docs to match operator behavior; OPERATOR_DP_PROCESSING mode recently formalized in
PR #3190.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The operator docs add ref:// usage and claim operator-side resolution, but operator code gates
reference resolution behind OPERATOR_DP_PROCESSING. The downstream installer categorizes any
non-OCI/non-./ package as NPM, so ref://... will be misrouted/unsupported if it reaches the
installer.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`ref://` is newly documented as a supported package format, but it is only resolved in operator code paths when `OPERATOR_DP_PROCESSING` is enabled. In the normal (default) flow, the init container runs the `rhdh-plugins` installer, which does not recognize `ref://` and will treat it as an NPM spec.
### Issue Context
This is a cross-repo contract issue: the operator advertises a syntax that the downstream installer does not implement, and the operator does not always resolve it before handing off.
### Fix Focus Areas
- docs/dynamic-plugins.md[98-129]
- pkg/model/dynamic-plugins.go[74-76]
- pkg/model/dynamic-plugins.go[313-320]
### What to change
Choose one of these coordinated fixes:
1) **Always resolve `ref://` (and `{{inherit}}`) during merge** regardless of `OPERATOR_DP_PROCESSING`, so the installer never sees `ref://`.
2) **Hard-reject `ref://` when processing mode is disabled**, with a clear error message telling users how to enable the required mode.
3) **Add `ref://` support to the installer** (rhdh-plugins) and coordinate releases + docs.
Also update the docs to explicitly state the prerequisite (if any) and the exact execution mode in which `ref://` works.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
DynaPlugin.Name() strips everything after the last : in an OCI reference, which misinterprets
registry host:port as an image tag when no tag is present (e.g.,
oci://localhost:5000/path/plugin). This breaks name-based matching for ref:// and
:{{inherit}}, causing failed resolution or wrong base-plugin selection.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The code strips the OCI tag by truncating at the last :, with no check that the colon is after the
last /; this will truncate registry:5000/... and return the wrong name, which is then used for
name-based lookups in resolveRefReference/resolveInheritReference.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`DynaPlugin.Name()` currently removes an OCI tag by doing `LastIndex(":")` on the post-`oci://` string. This incorrectly treats `host:port` as a tag delimiter when the reference has no tag, producing an incorrect plugin name and breaking `ref://` / `:{{inherit}}` resolution.
### Issue Context
OCI references commonly include a registry port (e.g., `localhost:5000`). Tag separators should only be treated as a tag if they occur in the last path segment (i.e., after the last `/`), not in the registry authority.
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[163-183]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
When OPERATOR_DP_PROCESSING is enabled, the operator now errors on any plugin package that isn’t
oci/http(s)/./ or ref://, which rejects NPM package specs that the rhdh-plugins installer and RHDH
docs explicitly support. This creates a cross-repo contract break where valid dynamic-plugins.yaml
inputs for install-dynamic-plugins will fail reconciliation in the operator.
+ case plugin.IsDirectLink():+ // Direct link - no resolution needed
continue
+ default:+ return nil, fmt.Errorf("unsupported package URL format %q: must start with oci://, https://, http://, ./ or use ref:// for catalog lookup", plugin.Package)
}
Relevance
⭐⭐⭐ High
Likely treated as contract-breaking regression; operator/dynamic-plugins behavior changes are
actively refined (e.g., #3144, #3190).
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The PR adds an operator-side allowlist that errors on NPM package specs, while the downstream
rhdh-plugins installer and RHDH docs explicitly support NPM packages in dynamic-plugins.yaml
(and the installer categorizes non-OCI/non-./ as NPM).
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
When `OPERATOR_DP_PROCESSING` is enabled, `resolveReferences()` enforces a strict allowlist of package URL prefixes and returns an error for anything else. This rejects NPM package references (and other npm-supported specs) that are valid inputs for `rhdh-plugins`' `install-dynamic-plugins` and documented in `rhdh`.
### Issue Context
`rhdh-plugins` installer supports NPM-sourced plugins and treats any non-OCI and non-local (`./`) package string as an NPM spec to install. RHDH documentation also describes using `@scope/name@version` in `dynamic-plugins.yaml`.
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[46-58]
- pkg/model/dynamic-plugins-reference_test.go[128-140]
### What to change
- Make `resolveReferences()` only resolve *reference syntaxes* (`ref://...` and `...:{{inherit}}...`) and **not** hard-fail other package formats.
- Concretely: after handling `ref://` and `{{inherit}}`, `default` should `continue` (leave package unchanged) rather than returning an error.
- Optionally add a narrower validation mode that can be enabled explicitly, but keep default behavior compatible with installer-supported formats.
- Update/adjust tests accordingly (remove the expectation that `@backstage/plugin-catalog` errors) or gate that test behind a dedicated strict-validation flag.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
4. {{inherit}} note lacks version 📘 Rule violation§ Compliance
Description
The documentation states the :{{inherit}} behavior is "slightly different" but does not specify
the version boundary where the behavior changed. This violates the requirement to document behavior
changes with an explicit version, which can confuse users on older/newer releases.
+Note: this behavior is similar to `ref://` and slightly different from what is described in [OCI Package Version Inheritance](https://github.com/redhat-developer/rhdh/blob/main/docs/dynamic-plugins/installing-plugins.md#oci-package-version-inheritance).
Relevance
⭐⭐ Medium
Mixed history on adding explicit version boundaries in docs (accepted in #1406, rejected in #1484).
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
PR Compliance ID 13 requires explicit version boundaries when docs describe behavior differences
across versions. The added note says behavior is "slightly different" from the linked documentation,
but provides no "since/before vX.Y.Z" boundary.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
## Issue description
`docs/dynamic-plugins.md` describes a behavior difference for `:{{inherit}}` (and its similarity to `ref://`) without stating which versions the new vs old behavior applies to.
## Issue Context
Compliance requires that any documentation describing behavior differences across versions includes an explicit version boundary (e.g., "Before vX.Y.Z" / "Since vX.Y.Z").
## Fix Focus Areas
- docs/dynamic-plugins.md[131-131]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
5. Overbroad version stripping 🐞 Bug≡ Correctness
Description
stripVersionSuffix treats any trailing -<digit> as a version start, so valid plugin names like
plugin-2fa or date-suffixed names can be truncated during HTTP(S) Name() extraction. This can
prevent ref:// / :{{inherit}} name matching for HTTP(S)-based plugin packages.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
Name() applies stripVersionSuffix() for HTTP(S) URLs, and stripVersionSuffix() strips at any
- followed by a digit, which can remove non-version parts of a plugin name and change the lookup
key used by reference resolution.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`stripVersionSuffix()` removes everything after the last `-` that precedes *any* digit. This is too broad and can truncate legitimate plugin names that happen to contain `-<digit>` but are not versioned.
### Issue Context
`Name()` applies `stripVersionSuffix()` to HTTP(S) archive names, so incorrect stripping changes the key used for name-based resolution.
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[185-209]
- pkg/model/dynamic-plugins-reference.go[223-233]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
The operator now resolves :{{inherit}} by **plugin name only** (ignoring registry/path), but the
rhdh-plugins installer (and RHDH docs) define inheritance matching using an OCI image key (image
name + plugin path) from included configs. This cross-repo semantic drift can make operator-side
resolution behave differently than installer-side expectations and documentation.
+ // Extract plugin name from the package URL (strip :{{inherit}} first)+ tempPackage := strings.Replace(packageURL, inheritSuffix, "", 1)+ tempPlugin := DynaPlugin{Package: tempPackage}+ pluginName := tempPlugin.Name()- // Look up the full URL in basePlugins- fullURL, found := baseURLMap[baseURL]- if !found {- return "", fmt.Errorf("cannot resolve {{inherit}} reference: no matching plugin found for base URL %q in default plugins", baseURL)+ if pluginName == "" {+ return "", fmt.Errorf("cannot resolve {{inherit}} reference: unable to extract plugin name from %q", packageURL)
}
- // If user specified !plugin-path, use it; otherwise use full default URL- if pluginPath != "" {- // Extract image part from default (without !plugin-path)- if idx := strings.LastIndex(fullURL, "!"); idx != -1 {- fullURL = fullURL[:idx]+ // Look up the plugin by name in basePlugins+ for i := range basePlugins {+ plugin := &basePlugins[i]+ if plugin.Package == "" {+ continue+ }+ if plugin.Name() == pluginName {+ fullURL := plugin.Package++ // If user specified !plugin-path, use it; otherwise use full default URL+ if pluginPath != "" {+ // Extract image part from default (without !plugin-path)+ if idx := strings.LastIndex(fullURL, "!"); idx != -1 {+ fullURL = fullURL[:idx]+ }+ return fullURL + pluginPath, nil+ }+
Relevance
⭐⭐ Medium
{{inherit}} semantics were recently changed/iterated; could be intentional divergence or require
alignment—unclear from history.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
The PR explicitly documents and implements name-only inheritance, while the rhdh-plugins installer
resolves {{inherit}} by matching keys derived from the OCI image reference (and RHDH docs define
the matching key as OCI image name + plugin path).
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`resolveInheritReference()` now matches base plugins by `DynaPlugin.Name()` (last path component), explicitly ignoring registry/path. However, `rhdh-plugins`' installer resolves `{{inherit}}` by matching plugins from the *same OCI image key* (registry/path + plugin path), and RHDH docs describe the matching key similarly.
### Issue Context
If operator-side reference resolution is enabled/used, it should align with the installer’s documented contract to avoid surprising differences between environments where the operator resolves vs where the installer resolves.
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[76-121]
- docs/dynamic-plugins.md[120-131]
### What to change
- Change operator-side `{{inherit}}` resolution to match using the same key semantics as the installer/docs (OCI image name + plugin path), rather than name-only.
- At minimum: require the same registry/path image reference when inheriting (and optionally also include the `!plugin-path` when present).
- Consider parsing OCI packages similarly to the installer’s `ociPluginKey` behavior (image key + optional path).
- Update operator docs to match the chosen canonical semantics and remove/clarify statements that “registry/path is ignored” if that’s no longer true.
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
7. Inefficient base plugin scans 🐞 Bug➹ Performance
Description
Reference resolution repeatedly rescans basePlugins and recomputes plugin.Name() for each
reference, making the overall resolution cost O(#references × #basePlugins). This can slow
reconciliation when the default plugin catalog grows.
+ // Look up the plugin by name in basePlugins+ for i := range basePlugins {+ plugin := &basePlugins[i]+ if plugin.Package == "" {+ continue+ }+ if plugin.Name() == pluginName {+ fullURL := plugin.Package++ // If user specified !plugin-path, use it; otherwise use full default URL+ if pluginPath != "" {+ // Extract image part from default (without !plugin-path)+ if idx := strings.LastIndex(fullURL, "!"); idx != -1 {+ fullURL = fullURL[:idx]+ }+ return fullURL + pluginPath, nil+ }++ return fullURL, nil
}
- return fullURL + pluginPath, nil
Relevance
⭐ Low
No prior evidence team optimizes small O(n×m) scans in model code without measured impact.
ⓘ Recommendations generated based on similar findings in past PRs
Evidence
Both ref and inherit resolvers iterate over all base plugins and call plugin.Name() inside the
loop, so resolving many references causes repeated full scans and repeated name parsing.
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution
### Issue description
`resolveInheritReference()` and `resolveRefReference()` linearly scan `basePlugins` on every call and call `Name()` for each candidate. Across multiple references this becomes O(N×M) work and repeats parsing.
### Issue Context
The previous implementation used a precomputed lookup map; the new name-based matching can still use a precomputed `map[name]packageURL` (and ideally detect duplicate names).
### Fix Focus Areas
- pkg/model/dynamic-plugins-reference.go[34-66]
- pkg/model/dynamic-plugins-reference.go[99-121]
- pkg/model/dynamic-plugins-reference.go[124-147]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
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
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
Which issue(s) does this PR fix or relate to
https://redhat.atlassian.net/browse/RHIDP-15014
PR acceptance criteria
How to test changes / Special notes to the reviewer
Run the controller locally with
make install runkubectl apply -f examples/dyna-plugins.yaml -n
Look for
./dynamic-plugins/dist/backstage-community-plugin-catalog-backend-module-keycloak-dynamicin
backstage-dynamic-plugins-bs1ConfigMapLook for:
in
backstage-appconfig-bs1-plugins-appconfigConfigMapNOTE: the CR will be deployed but not functional, as dynamic-pkugins list is empty.
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.