Skip to content

Go based plugin installer - #3466

Open
gazarenkov wants to merge 16 commits into
redhat-developer:mainfrom
gazarenkov:go-based-plugin-installer
Open

Go based plugin installer#3466
gazarenkov wants to merge 16 commits into
redhat-developer:mainfrom
gazarenkov:go-based-plugin-installer

Conversation

@gazarenkov

Copy link
Copy Markdown
Member

Description

Replace external tools-based plugin installer (skopeo, curl, jq) with a Go implementation maintained, tested, and delivered as part of the operator codebase.

  • cmd/plugin-fetch/ - Main binary entry point
  • pkg/fetcher/ - Fetcher packages (OCI, NPM, HTTP, local file)
  • plugin-installer/Dockerfile - Go-based container image
  • .github/workflows/plugin-installer.yaml - CI for tests and image builds

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

https://redhat.atlassian.net/browse/RHIDP-16681

PR acceptance criteria

  • Tests
  • Documentation

How to test changes / Special notes to the reviewer

make dp-installer-test # Run unit + integration tests
make dp-installer-buildx # Build multiplatform image

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.

@gazarenkov
gazarenkov requested a review from a team as a code owner September 2, 2026 10:41
Comment thread pkg/fetcher/npm_extract.go Fixed
@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Sep 2, 2026

Copy link
Copy Markdown

PR Summary by Qodo

Replace external-tool plugin installer with Go implementation

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Replaces shell and external-tool plugin installation with a maintained Go binary.
• Supports OCI, NPM, HTTP, and local sources with secure, concurrent extraction.
• Adds Go tests, minimal image builds, CI coverage, and registry-authentication documentation.
Diagram

graph TD
  A["Package List"] --> B["plugin-fetch"] --> C{"Source Type"}
  C --> D["OCI Fetcher"] --> H["Plugin Output"]
  C --> E["NPM Fetcher"] --> H
  C --> F["HTTP Fetcher"] --> H
  C --> G["Local Fetcher"] --> H
Loading
High-Level Assessment

The in-process Go implementation is the preferred approach because it removes runtime dependencies on shell utilities, skopeo, oras, curl, and text-based JSON parsing while reusing the repository's Go toolchain and OCI libraries. Retaining the shell implementation or invoking external tools would preserve larger images, platform variability, and weaker testability without a compensating architectural benefit.

Files changed (24) +2591 / -160

Enhancement (8) +1177 / -12
main.goImplement the Go plugin installer command +402/-0

Implement the Go plugin installer command

• Adds environment-driven installer orchestration with file locking, signal cancellation, bounded parallelism, temporary extraction, and atomic destination renames. It also configures OCI and NPM authentication and optionally extracts catalog entities.

cmd/plugin-fetch/main.go

fetcher.goAdd protocol-aware artifact routing +66/-0

Add protocol-aware artifact routing

• Introduces the top-level Fetcher abstraction and options for OCI and NPM behavior. Routes OCI, HTTP, NPM, and local references to dedicated implementations with optional integrity metadata.

pkg/fetcher/fetcher.go

http.goAdd verified HTTP tarball fetching +103/-0

Add verified HTTP tarball fetching

• Downloads HTTP and HTTPS archives with context cancellation and status validation. Detects supported tar formats, verifies optional SRI hashes, and extracts compressed or uncompressed archives.

pkg/fetcher/http.go

local.goAdd local file and directory fetching +70/-0

Add local file and directory fetching

• Implements recursive directory copying and single-file copying for file-based plugin references.

pkg/fetcher/local.go

lock.goAdd cross-process installer locking +66/-0

Add cross-process installer locking

• Implements timeout-bound exclusive filesystem locking with flock, allowing installer containers sharing a volume to serialize writes safely.

pkg/fetcher/lock.go

npm.goImplement native NPM package fetching +307/-0

Implement native NPM package fetching

• Adds package and version parsing, latest-version resolution, registry metadata requests, bearer authentication, and basic npmrc configuration. Downloads tarballs and verifies SHA-256, SHA-384, or SHA-512 SRI hashes without Node.js.

pkg/fetcher/npm.go

npm_extract.goAdd secure NPM tarball extraction +93/-0

Add secure NPM tarball extraction

• Extracts NPM archives while stripping the package prefix, limiting entry size, blocking traversal, and ignoring symlinks.

pkg/fetcher/npm_extract.go

oci.goExtend OCI fetching with plugin mode +70/-12

Extend OCI fetching with plugin mode

• Expands plugin validation into a mode that also locates and moves plugin content from extracted OCI layers. Non-plugin OCI artifacts such as catalog indexes continue to support direct extraction.

pkg/fetcher/oci.go

Bug fix (1) +5 / -0
extract.goStrengthen archive path containment checks +5/-0

Strengthen archive path containment checks

• Adds a defense-in-depth check ensuring securely joined archive paths remain inside the extraction destination.

pkg/fetcher/extract.go

Tests (6) +1137 / -8
integration_test.goAdd end-to-end fetcher integration coverage +198/-0

Add end-to-end fetcher integration coverage

• Tests real NPM and OCI downloads, integrity failures, fetcher routing, atomic extraction, and plugin-name derivation across supported reference formats.

cmd/plugin-fetch/integration_test.go

http_test.goTest HTTP download and archive handling +276/-0

Test HTTP download and archive handling

• Covers compressed and uncompressed tar extraction, content detection, integrity verification, HTTP errors, and context cancellation using local test servers.

pkg/fetcher/http_test.go

local_test.goTest local artifact copying and routing +241/-0

Test local artifact copying and routing

• Covers files, nested directories, large content, missing sources, destination creation, and multiple file URL forms through the unified fetcher.

pkg/fetcher/local_test.go

npm_test.goTest NPM parsing, authentication, and integrity +340/-0

Test NPM parsing, authentication, and integrity

• Covers package syntax, npmrc parsing, SRI algorithms, fetcher options, latest-version resolution, metadata retrieval, authentication headers, and registry errors.

pkg/fetcher/npm_test.go

oci_test.goTest OCI plugin-mode extraction +77/-8

Test OCI plugin-mode extraction

• Updates option tests for plugin mode and verifies plugin content discovery at archive roots or nested directories, including invalid layouts.

pkg/fetcher/oci_test.go

dynamic-plugins-reference_test.goCover OCI references containing tags and digests +5/-0

Cover OCI references containing tags and digests

• Adds a model test ensuring plugin names are derived correctly when an OCI reference includes both a tag and digest.

pkg/model/dynamic-plugins-reference_test.go

Documentation (2) +199 / -111
configuration.mdDocument private OCI registry configuration +89/-0

Document private OCI registry configuration

• Explains how to mount Docker credentials and custom CA certificates into the installer init container. Documents the DOCKER_CONFIG, CA_FILE, and INSECURE settings with a Backstage CR example.

docs/configuration.md

README.mdRewrite installer documentation for plugin-fetch +110/-111

Rewrite installer documentation for plugin-fetch

• Replaces shell and external-tool instructions with Go binary configuration, supported sources, authentication, testing, image builds, signal behavior, and troubleshooting guidance.

plugin-installer/README.md

Other (7) +73 / -29
plugin-installer.yamlRun Go installer tests and builds in CI +10/-10

Run Go installer tests and builds in CI

• Extends workflow path filters to the new command and fetcher packages. Replaces shell and skopeo test setup with Go tests and uses the renamed multiplatform image target.

.github/workflows/plugin-installer.yaml

MakefileAdd Go installer test and image targets +8/-11

Add Go installer test and image targets

• Replaces the skopeo-specific build and push targets with a unified multiplatform Dockerfile build. Adds a target for running fetcher unit tests and command integration tests.

Makefile

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

Refresh Backstage bundle creation timestamp

• Updates the generated ClusterServiceVersion creation timestamp.

bundle/backstage.io/manifests/backstage-operator.clusterserviceversion.yaml

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

Refresh RHDH bundle creation timestamp

• Updates the generated ClusterServiceVersion creation timestamp.

bundle/rhdh/manifests/backstage-operator.clusterserviceversion.yaml

go.modRefresh transitive container dependencies +2/-2

Refresh transitive container dependencies

• Updates the indirect Docker CLI and compression module versions used by the Go dependency graph.

go.mod

go.sumUpdate checksums for refreshed dependencies +4/-4

Update checksums for refreshed dependencies

• Records checksums for the updated Docker CLI and compression module versions.

go.sum

DockerfileBuild a minimal Go installer image +47/-0

Build a minimal Go installer image

• Adds a multiplatform build stage for a static plugin-fetch binary and packages it with CA certificates in a non-root UBI Micro runtime image.

plugin-installer/Dockerfile

@rhdh-qodo-merge

rhdh-qodo-merge Bot commented Sep 2, 2026

Copy link
Copy Markdown

Code Review by Qodo

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

Grey Divider


Action required

1. Unscoped NPM packages rejected ✗ Dismissed 🐞 Bug ≡ Correctness
Description
Fetcher.FetchWithIntegrity routes only strings beginning with @ or containing @npm: to the NPM
fetcher. Documented unscoped packages such as lodash and is-odd@3.0.1 instead return
“unsupported URL scheme.”
Code

pkg/fetcher/fetcher.go[R59-64]

+	case strings.HasPrefix(url, "@") || strings.Contains(url, "@npm:"):
+		return f.npm.FetchWithIntegrity(ctx, url, destDir, integrity)
+	case strings.HasPrefix(url, "file:"):
+		return copyLocal(strings.TrimPrefix(url, "file:"), destDir)
+	default:
+		return fmt.Errorf("unsupported URL scheme: %s", url)
Relevance

●●● Strong

Router conditions exclude documented unscoped NPM forms despite parser support, causing
deterministic unsupported-scheme errors.

PR-#3289

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The router's NPM condition excludes both documented unscoped forms, while parseNPMPackage supports
them and the README explicitly lists them as supported.

pkg/fetcher/fetcher.go[51-65]
pkg/fetcher/npm.go[296-307]
plugin-installer/README.md[110-128]

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

## Issue description
Documented unscoped NPM package forms are rejected by the top-level fetcher router.

## Issue Context
The NPM fetcher itself can parse both versioned and unversioned unscoped package names, but the router never invokes it for those inputs.

## Fix Focus Areas
- pkg/fetcher/fetcher.go[51-65]
- pkg/fetcher/npm.go[296-307]
- plugin-installer/README.md[110-128]

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


2. NPM symlinks escape destination ✓ Resolved 🐞 Bug ⛨ Security
Description
The npm extractor accepts a symlink target such as ../dest-evil because it tests only whether the
cleaned path starts with the destination string. A later archive entry beneath that symlink is
opened through the filesystem and can write outside the plugin directory.
Code

pkg/fetcher/npm_extract.go[R80-84]

+			linkTarget := filepath.Join(filepath.Dir(target), header.Linkname)
+			if !strings.HasPrefix(filepath.Clean(linkTarget), filepath.Clean(destDir)) {
+				return fmt.Errorf("symlink escapes destination: %s -> %s", header.Name, header.Linkname)
+			}
+			if err := os.Symlink(header.Linkname, target); err != nil {
Relevance

●●● Strong

String-prefix validation permits destination-prefix collisions and symlink-based writes outside the
extraction directory.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
For destination /tmp/dest, cleaned /tmp/dest-evil passes the current prefix test. Subsequent
regular entries use os.OpenFile on paths beneath the created symlink, which follows the symlink
outside the destination.

pkg/fetcher/npm_extract.go[45-59]
pkg/fetcher/npm_extract.go[71-85]
pkg/fetcher/extract.go[107-120]

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

## Issue description
An npm archive can create a symlink to a sibling path sharing the destination prefix and then write files outside the destination through it.

## Issue Context
A string prefix is not a path-boundary check, and regular-file extraction follows existing parent symlinks. Use secure path resolution for every entry or skip symlinks as the general extractor does.

## Fix Focus Areas
- pkg/fetcher/npm_extract.go[45-59]
- pkg/fetcher/npm_extract.go[71-85]
- pkg/fetcher/extract.go[107-120]

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


3. Legacy installer still published ✓ Resolved 🐞 Bug ≡ Correctness
Description
dp-installer-buildx still builds plugin-installer/Dockerfile.skopeo, so the main-branch workflow
publishes the old shell/skopeo installer rather than the new Go binary. The principal feature of
this PR is therefore absent from the released image.
Code

Makefile[R280-281]

+dp-installer-buildx: ## Build and push multiplatform plugin installer image (skopeo variant)
	$(CONTAINER_TOOL) buildx build --push --platform=$(MIN_PLATFORMS) -t $(INSTALL_DP_IMAGE) --label $(LABEL) -f plugin-installer/Dockerfile.skopeo .
Relevance

●●● Strong

Publishing explicitly selects the legacy Dockerfile, contradicting the PR’s stated Go-installer
delivery objective.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The publishing workflow calls this target, while the target explicitly selects the retained legacy
Dockerfile. That Dockerfile installs skopeo and executes install_plugins.sh, whereas the new
Dockerfile builds and runs plugin-fetch.

Makefile[279-281]
.github/workflows/plugin-installer.yaml[72-74]
plugin-installer/Dockerfile.skopeo[4-38]
plugin-installer/Dockerfile[14-18]

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 release target still builds the legacy skopeo-based Dockerfile, so the new Go installer is not delivered.

## Issue Context
The plugin-installer workflow invokes `dp-installer-buildx` on main pushes, and that target selects `Dockerfile.skopeo` instead of the newly added `plugin-installer/Dockerfile`.

## Fix Focus Areas
- Makefile[279-281]
- .github/workflows/plugin-installer.yaml[72-74]
- plugin-installer/Dockerfile[1-47]

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


View high (7)
4. Gzip archives never decompressed ✗ Dismissed 🐞 Bug ≡ Correctness
Description
extractTarGzBytes passes compressed bytes directly to the tar reader instead of creating a gzip
reader. Consequently HTTP .tgz and .tar.gz plugin downloads fail with an invalid tar archive.
Code

pkg/fetcher/extract.go[R40-43]

+// extractTarGzBytes extracts a gzipped tarball from bytes to destDir
+func extractTarGzBytes(data []byte, destDir string) error {
+	return extractTarGz(bytes.NewReader(data), destDir)
+}
Relevance

●●● Strong

Compressed tarballs are passed to an uncompressed tar reader, an obvious deterministic download
failure.

PR-#3289

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The byte helper directly calls extractTar, whose first operation is tar.NewReader; unlike
extractTarGz, no gzip.NewReader is used. HTTP routes every detected compressed tarball through
this helper.

pkg/fetcher/extract.go[29-47]
pkg/fetcher/extract.go[50-52]
pkg/fetcher/http.go[50-66]

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

## Issue description
`extractTarGzBytes` does not decompress its input, causing gzip-compressed HTTP plugins to fail extraction.

## Issue Context
The streaming equivalent already creates a gzip reader correctly. HTTP tarball handling calls the broken byte-slice function.

## Fix Focus Areas
- pkg/fetcher/extract.go[29-47]
- pkg/fetcher/http.go[50-66]
- pkg/fetcher/extract_test.go[68-96]

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


5. OCI plugin paths unsupported ✗ Dismissed 🔗 Cross-repo conflict ≡ Correctness
Description
rhdh-plugins defines OCI packages as <image>!<plugin-path> and installs only the selected subtree,
but the Go installer passes the complete string to the OCI reference parser and extracts the entire
artifact layer directly into the final plugin directory. Valid references may fail parsing at !,
and artifacts with a plugin-name wrapper may be installed as OUTPUT_DIR/plugin/plugin/... instead
of OUTPUT_DIR/plugin/....
Code

pkg/fetcher/fetcher.go[R54-56]

+	case strings.HasPrefix(url, "oci://"):
+		// OCI uses digest in URL for verification, integrity param ignored
+		return f.oci.Fetch(ctx, strings.TrimPrefix(url, "oci://"), destDir)
Relevance

●●● Strong

The repository accepts fixes involving OCI references with exclamation-delimited plugin paths and
subtree selection.

PR-#2231

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR forwards everything after oci://, including the !plugin-path suffix, as an OCI image
reference and then extracts the first layer wholesale into the destination. In contrast, the
rhdh-plugins contract requires splitting at !, using the suffix to filter archive entries, and
installing only that path; the previous installer likewise located the plugin-named child within the
extracted layer and moved that child into the final destination, confirming that writing the layer
root directly introduces an extra nesting level.

pkg/fetcher/fetcher.go[51-60]
pkg/fetcher/oci.go[93-146]
cmd/plugin-fetch/main.go[296-310]
pkg/fetcher/oci.go[139-146]
plugin-installer/install_plugins.sh[263-282]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts [32-46]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/installer-oci.ts [93-108]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/tar-extract.ts [27-45]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/tar-extract.ts [61-69]

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

## Issue description
Support the rhdh-plugins OCI package contract, where a package reference consists of an image reference followed by `!plugin-path`. The current implementation passes the full value, including the suffix, to the registry parser and extracts the complete artifact layer into the final plugin directory, which can reject valid references or preserve the plugin-name wrapper and create an extra nesting level.

## Issue Context
Remove the plugin-path suffix before parsing or pulling the image, then use it to select and install only the intended archive subtree while preserving the expected relative destination layout. The replaced installer explicitly located `${EXTRACT_DIR}/${plugin_name}` and moved that directory into the final destination; preserve this established artifact contract, preferably by extracting into a temporary directory and atomically moving the expected child.

## Fix Focus Areas
- pkg/fetcher/fetcher.go[51-60]
- pkg/fetcher/oci.go[93-146]
- cmd/plugin-fetch/main.go[296-310]
- plugin-installer/install_plugins.sh[263-282]

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


6. Single-file integrity silently ignored ✓ Resolved 🐞 Bug ⛨ Security
Description
HTTP integrity verification is performed only inside the tarball branch. A non-tar HTTP response
with an incorrect integrity value is saved successfully, violating the documented integrity
guarantee.
Code

pkg/fetcher/http.go[R59-66]

+		// Verify integrity if provided
+		if integrity != "" {
+			if err := verifyIntegrity(data, integrity); err != nil {
+				return fmt.Errorf("integrity verification failed: %w", err)
+			}
+		}
+
+		return extractTarGzBytes(data, destDir)
Relevance

●●● Strong

The documented integrity guarantee clearly applies to HTTP files, but single-file responses bypass
verification.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The only call to verifyIntegrity is inside isTarball; the single-file branch directly copies
resp.Body to disk. The README states that HTTP integrity is verified when provided.

pkg/fetcher/http.go[50-71]
plugin-installer/README.md[99-108]
plugin-installer/README.md[175-184]

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

## Issue description
Integrity values are ignored for HTTP responses not classified as tarballs.

## Issue Context
Verification must cover the downloaded bytes before either extraction or direct-file persistence. Stream to a bounded temporary file or buffer, verify it, and only then publish the result.

## Fix Focus Areas
- pkg/fetcher/http.go[50-71]
- pkg/fetcher/npm.go[264-294]
- plugin-installer/README.md[99-108]

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


7. Invalid parallelism hangs installer ✓ Resolved 🐞 Bug ☼ Reliability
Description
PARALLEL=0 creates an unbuffered semaphore and blocks forever before starting the first download,
while a negative value panics during channel creation. IntEnvVar accepts both values without
validating that parallelism is positive.
Code

cmd/plugin-fetch/main.go[R278-280]

+func processParallel(ctx context.Context, f *fetcher.Fetcher, packages []Package, outputDir string, parallel int) error {
+	sem := make(chan struct{}, parallel)
+	var wg sync.WaitGroup
Relevance

●●● Strong

Zero or negative externally configured parallelism causes a hang or panic; validation is an
established accepted requirement.

PR-#2870

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The parsed integer is passed directly to make(chan struct{}, parallel). A negative channel
capacity panics, and with zero capacity the producer send cannot complete because workers are
started only after the send.

cmd/plugin-fetch/main.go[51-60]
cmd/plugin-fetch/main.go[278-289]
pkg/utils/utils.go[260-267]
PR-#2870

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

## Issue description
Zero or negative `PARALLEL` values hang or panic the installer.

## Issue Context
Validate the environment value before constructing the semaphore and terminate with a clear configuration error or normalize it to the documented default.

## Fix Focus Areas
- cmd/plugin-fetch/main.go[51-60]
- cmd/plugin-fetch/main.go[278-289]
- pkg/utils/utils.go[260-267]

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


8. Registry configuration ignored 🔗 Cross-repo conflict ≡ Correctness
Description
RHDH disconnected automation supplies OCI authentication, mirrors, custom CAs, and policy through
containers/image configuration mounts, while the Go installer only loads authentication or a CA when
new explicit environment variables are set. RHDH disconnected deployments will consequently bypass
configured mirrors and attempt anonymous or untrusted direct registry access.
Code

cmd/plugin-fetch/main.go[R110-115]

+	if dockerConfig != "" {
+		secret, err := os.ReadFile(dockerConfig)
+		if err != nil {
+			fatal(termLog, startTime, "Error reading docker config: %v", err)
+		}
+		ociOpts = append(ociOpts, fetcher.WithDockerConfig(secret))
Relevance

●●● Strong

Disconnected deployments depend on registry mirrors and policy; historical precedents accept fixes
for missing mirror configuration handling.

PR-#1646

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR retains the default keychain unless DOCKER_CONFIG names a file and only trusts a custom CA
supplied via CA_FILE. RHDH's disconnected workflow instead creates an auth.json secret and
mounts containers/image mirror, policy, CA, and authentication configuration for the installer.

cmd/plugin-fetch/main.go[54-56]
cmd/plugin-fetch/main.go[103-115]
pkg/fetcher/oci.go[29-34]
pkg/fetcher/oci.go[70-77]
External repo: redhat-developer/rhdh, .ci/pipelines/lib/disconnected/namespace.sh [27-41]
External repo: redhat-developer/rhdh, .ci/pipelines/resources/rhdh-operator/rhdh-start-disconnected-smoke.yaml [46-65]
External repo: redhat-developer/rhdh, .ci/pipelines/jobs/ocp-disconnected-helm.sh [152-157]

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

## Issue description
Preserve compatibility with the containers/image configuration used by RHDH disconnected deployments. The Go OCI client currently ignores mounted `auth.json`, `registries.conf`, registry-specific CA directories, and policy configuration unless separate new environment variables are supplied.

## Issue Context
RHDH CI and deployments mount these files for the installer without setting `DOCKER_CONFIG` or `CA_FILE`. The replacement must either consume the existing configuration directly or translate it before creating the OCI client.

## Fix Focus Areas
- cmd/plugin-fetch/main.go[98-119]
- pkg/fetcher/oci.go[29-78]
- pkg/fetcher/oci.go[93-115]

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


9. Registry ports corrupt plugin names ✓ Resolved 🐞 Bug ≡ Correctness
Description
After stripping an OCI digest, pluginName removes everything after the last colon anywhere in the
reference, including a registry port. For oci://registry:5000/org/plugin@sha256:..., the
destination name becomes registry, causing wrong paths and collisions.
Code

cmd/plugin-fetch/main.go[R347-350]

+	if idx := strings.LastIndex(url, ":"); idx > 0 {
+		// Remove tag like :latest or :v1.0.0
+		url = url[:idx]
+	}
Relevance

●●● Strong

This exactly matches the repository’s recently accepted host-port-as-tag bug fix.

PR-#3215

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The last-colon operation runs after digest removal and without comparing the colon position to the
final slash. The resulting name is used directly as the installation destination.

cmd/plugin-fetch/main.go[335-353]
cmd/plugin-fetch/main.go[296-298]
PR-#3215

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 registry ports are mistaken for image tags when deriving destination directory names.

## Issue Context
Only treat a colon after the final slash as a tag separator. Reject registry-only references that do not contain an image path.

## Fix Focus Areas
- cmd/plugin-fetch/main.go[325-360]
- cmd/plugin-fetch/main.go[296-298]

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


10. Partial installs become permanent ✓ Resolved 🐞 Bug ☼ Reliability
Description
Fetchers write directly into the final destination, but failed downloads do not remove that
destination and subsequent runs skip any non-empty directory. An extraction failure after one file
therefore permanently marks an incomplete plugin as installed.
Code

cmd/plugin-fetch/main.go[R299-305]

+			// Skip if already exists
+			if info, err := os.Stat(destDir); err == nil && info.IsDir() {
+				entries, _ := os.ReadDir(destDir)
+				if len(entries) > 0 {
+					fmt.Printf("[SKIP] %s (exists)\n", name)
+					return
+				}
Relevance

●●● Strong

Failed downloads can leave misleading installation state; historical reliability reviews accept
cleanup and retry protections.

PR-#3289

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The skip test accepts any non-empty destination, while the failure path records the error but
performs no cleanup. OCI and archive fetchers write files directly into that final directory.

cmd/plugin-fetch/main.go[296-317]
pkg/fetcher/oci.go[139-146]
pkg/fetcher/http.go[45-71]
pkg/fetcher/npm.go[165-183]

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

## Issue description
Failed extractions leave partial directories that all later installer runs treat as completed plugins.

## Issue Context
Fetch into a temporary sibling directory, clean it on every failure, and atomically rename it to the final destination only after successful verification and extraction.

## Fix Focus Areas
- cmd/plugin-fetch/main.go[296-317]
- pkg/fetcher/oci.go[139-146]
- pkg/fetcher/http.go[45-71]
- pkg/fetcher/npm.go[165-183]

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



Remediation recommended

11. Duplicate plugins make installs fail 🐞 Bug ☼ Reliability ⭐ New
Description
processParallel derives both destDir and the shared <name>.tmp directory solely from
pluginName(pkg.URL). When duplicate entries or distinct URLs have the same basename, concurrent
workers remove, populate, and rename the same paths, so at least one worker can fail and abort the
installation.
Code

cmd/plugin-fetch/main.go[301]

+			tempDir := destDir + ".tmp"
Relevance

●●● Strong

Clear concurrency bug; non-unique temporary paths can corrupt or abort parallel installs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Each worker derives its temporary path from the non-unique plugin name, unconditionally removes that
path, fetches into it, and then renames it. Since pluginName reduces URLs to their final path
component, duplicate entries and different sources with the same basename collide.

cmd/plugin-fetch/main.go[294-320]
cmd/plugin-fetch/main.go[328-336]
cmd/plugin-fetch/main.go[345-386]

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

## Issue description
Concurrent packages resolving to the same plugin name share one temporary directory and race during cleanup, extraction, and rename.

## Issue Context
Duplicate package entries and URLs with the same basename must not corrupt each other's extraction or fail unpredictably.

## Fix Focus Areas
- cmd/plugin-fetch/main.go[299-336]

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


12. Signed plugin URLs are rejected 🐞 Bug ≡ Correctness ⭐ New
Description
isGzipped and isUncompressedTar apply filename-suffix checks to the complete URL instead of its
parsed path, so query parameters hide valid .tar, .tar.gz, and .tgz extensions. Signed or
presigned HTTP plugin URLs trigger this when the server omits one of the narrowly recognized tar
content types, causing the response to fail isTarball before it is read or extracted.
Code

pkg/fetcher/http.go[R81-83]

+func isGzipped(url, contentType string) bool {
+	lowerURL := strings.ToLower(url)
+	if strings.HasSuffix(lowerURL, ".tar.gz") || strings.HasSuffix(lowerURL, ".tgz") {
Relevance

●●● Strong

Recent fetcher reviews accepted hardening changes preventing silent HTTP and authentication
failures.

PR-#3422

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The fetcher rejects a response before reading it whenever isTarball returns false. Both gzip and
uncompressed-tar detection use strings.HasSuffix against the raw complete URL rather than the
parsed path, while the existing tests cover only suffix-only URLs without query strings, leaving
signed URLs such as .tar.gz?token=... unrecognized.

pkg/fetcher/http.go[49-52]
pkg/fetcher/http.go[81-100]
pkg/fetcher/http_test.go[170-194]
pkg/fetcher/http.go[49-59]
pkg/fetcher/http.go[75-102]
pkg/fetcher/http_test.go[136-195]

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

## Issue description
HTTP tarball detection checks archive suffixes against the complete URL, so query strings hide valid `.tar`, `.tar.gz`, and `.tgz` path extensions. Signed or presigned download URLs may therefore be rejected when the response has a generic, absent, differently cased, or otherwise unrecognized content type.

## Issue Context
Parse the URL and apply extension checks to its path component rather than the raw URL. Normalize media types before matching so MIME casing does not alter archive detection, and add coverage for archive URLs containing query parameters.

## Fix Focus Areas
- pkg/fetcher/http.go[75-102]
- pkg/fetcher/http_test.go[136-220]

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


13. Installer never receives credentials 🔗 Cross-repo conflict ≡ Correctness ⭐ New
Description
The new private-registry example declares rhdh.redhat.com/v1alpha3 while using containers
selectors that only exist in the newer Backstage custom-resource contract. After schema pruning,
RHDH defaults the files and variables to the Backstage container rather than
install-dynamic-plugins, so private plugin pulls receive neither credential nor custom
certificate.
Code

docs/configuration.md[813]

+apiVersion: rhdh.redhat.com/v1alpha3
Relevance

●●● Strong

Repository history accepts correcting invalid API versions and documentation examples.

PR-#1670

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The added example selects the installer through containers but declares the older API version. The
v1alpha3 file and environment types have no container selector, while RHDH pins v1alpha5 and uses
its selector specifically for installer variables.

docs/configuration.md[813-843]
api/v1alpha3/backstage_types.go[187-220]
api/v1alpha5/backstage_types.go[214-241]
External repo: redhat-developer/rhdh, e2e-tests/playwright/utils/runtime-config.ts [13-15]
External repo: redhat-developer/rhdh, e2e-tests/playwright/utils/runtime-config.ts [28-29]

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 private-registry example uses `v1alpha3`, whose file and environment references do not support per-container selectors. Consequently, the documented credentials are not delivered to the plugin installer.

## Issue Context
RHDH consumers use `rhdh.redhat.com/v1alpha5`, where `containers` is part of the custom-resource schema.

## Fix Focus Areas
- docs/configuration.md[813-843]

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


View medium (7)
14. Registry path aborts installer 🔗 Cross-repo conflict ≡ Correctness ⭐ New
Description
DOCKER_CONFIG in the Backstage example and configuration table points to
/run/secrets/plugin-registry and is described as a directory, but plugin-fetch passes the value
directly to os.ReadFile even though the selected .dockerconfigjson secret key is mounted beneath
that path. Users following the example therefore provide a directory instead of
/run/secrets/plugin-registry/.dockerconfigjson, causing startup to terminate before registry
authentication or any private OCI download occurs.
Code

docs/configuration.md[R836-837]

+        - name: DOCKER_CONFIG
+          value: /run/secrets/plugin-registry
Relevance

●●● Strong

Documentation gives a directory while implementation reads a file, causing deterministic installer
startup failure.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The documentation exports the keyed Secret's mount directory as DOCKER_CONFIG, while main reads
that exact environment value as a file and terminates if the read fails. The operator's keyed
extraFiles.secrets handling places the selected .dockerconfigjson key beneath the configured
directory, making the actual credential path /run/secrets/plugin-registry/.dockerconfigjson;
RHDH's existing disconnected configuration confirms this file-path contract by mounting auth.json
under /tmp and setting its registry variable to /tmp/auth.json.

docs/configuration.md[820-837]
cmd/plugin-fetch/main.go[113-118]
docs/configuration.md[820-850]
pkg/model/secretfiles.go[84-98]
pkg/model/deployment.go[417-428]
pkg/model/deployment.go[417-429]
External repo: redhat-developer/rhdh, .ci/pipelines/resources/rhdh-operator/rhdh-start-disconnected-smoke.yaml [40-43]
External repo: redhat-developer/rhdh, .ci/pipelines/resources/rhdh-operator/rhdh-start-disconnected-smoke.yaml [61-65]

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 documented private OCI registry configuration supplies a directory as `DOCKER_CONFIG`, while `plugin-fetch` currently requires the environment variable to name a readable credential JSON file. Align the documentation and executable contract so users can authenticate before downloading plugins.

## Issue Context
A keyed `extraFiles.secrets` item places the selected key beneath its configured mount directory. For the documented Secret and mount path, the credential file is `/run/secrets/plugin-registry/.dockerconfigjson`, not `/run/secrets/plugin-registry`. RHDH's existing disconnected configuration follows the same pattern by appending `auth.json` to its mount directory.

Either update every example and description to pass the mounted credential file path, or change the installer to recognize Docker-config directory semantics and load the supported credential filename or filenames consistently.

## Fix Focus Areas
- docs/configuration.md[808-852]
- cmd/plugin-fetch/main.go[113-118]
- pkg/model/secretfiles.go[84-98]
- pkg/model/deployment.go[417-428]
- plugin-installer/README.md[242-270]

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


15. Readers cannot apply the registry example 📘 Rule violation ✧ Quality ⭐ New
Description
The Kubernetes deployment example adds volumeMounts named registry-auth and registry-ca but
provides no matching volumes entries or Secret/ConfigMap manifests in that example block. When a
reader incorporates the fragment as shown, Kubernetes rejects the workload because both mount names
are unresolved, and the linked operator example does not supply those Pod volume definitions.
Code

plugin-installer/README.md[R265-268]

+  - name: registry-auth
+    mountPath: /run/secrets/registry
+    readOnly: true
+  - name: registry-ca
Relevance

●● Moderate

The snippet appears unusable, but historical reviewers rejected some broader README
example-completeness requests.

PR-#3289

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 1 requires README YAML examples to define every referenced volume, Secret, and
ConfigMap in the same example set. The new snippet references two volume names but contains neither
matching Pod volume definitions nor manifests for their sources.

Rule 1: Include all dependent Kubernetes resources in example manifests
plugin-installer/README.md[255-270]

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 Kubernetes example mounts `registry-auth` and `registry-ca` without defining the corresponding Pod volumes or their source resources.

## Issue Context
The example must include all resources and volume definitions needed to use the shown mounts, with clearly fake placeholder data where applicable.

## Fix Focus Areas
- plugin-installer/README.md[255-270]

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


16. Operators can mistake sample credentials 📘 Rule violation § Compliance ⭐ New
Description
The registry JSON sets auth to dXNlcm5hbWU6cGFzc3dvcmQ=, a valid base64 encoding of
username:password, without marking it as dummy or requiring replacement. The same credential is
repeated in both authentication guides, so copying either example carries a plausible reusable
username/password pair into user configuration.
Code

docs/configuration.md[786]

+      "auth": "dXNl******************Q="
Relevance

●● Moderate

Recent precedent rejected a README credential-placeholder concern, but this compliance rule
specifically targets realistic encoded credentials.

PR-#3289

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Compliance rule 18 prohibits realistic base64 credential values in examples unless they are clearly
identified as dummy values that must be replaced. Both newly added registry examples use the same
valid encoded credential without that warning.

Rule 18: Use only clearly non-sensitive dummy values in example secrets and dependent resources
docs/configuration.md[781-789]
plugin-installer/README.md[242-253]

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 authentication examples contain a valid base64-encoded username and password without clearly identifying the value as non-sensitive dummy data.

## Issue Context
Replace the value with an unmistakable placeholder and explicitly instruct users to substitute their own encoded credentials before deployment.

## Fix Focus Areas
- docs/configuration.md[781-789]
- plugin-installer/README.md[242-253]

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


17. PR image validation removed 🐞 Bug ☼ Reliability
Description
The workflow removed its pull-request image build without adding a replacement, while every
remaining build step is restricted to main-branch pushes. Broken plugin-installer Dockerfiles can
therefore merge after only Go tests pass.
Code

.github/workflows/plugin-installer.yaml[L52-54]

-      - name: Build image (PR validation)
-        if: github.event_name == 'pull_request'
-        run: make install-dp-build
Relevance

●●● Strong

PR image validation was removed without replacement; workflow precedents accept restoring missing
build or validation coverage.

PR-#3055
PR-#1882

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The deleted step was the only PR-specific image build. QEMU, registry login, and buildx publishing
are all guarded by the main-push condition, leaving PR build jobs with checkout only.

.github/workflows/plugin-installer.yaml[45-74]
Makefile[279-286]

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

## Issue description
Pull requests no longer build the plugin-installer container image.

## Issue Context
Add a non-pushing PR build using the new Go Dockerfile. Keep publishing and multiplatform setup restricted to main pushes as appropriate.

## Fix Focus Areas
- .github/workflows/plugin-installer.yaml[45-74]
- Makefile[279-286]
- plugin-installer/Dockerfile[1-47]

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


18. Catalog layers are omitted 🔗 Cross-repo conflict ≡ Correctness
Description
The Go OCI fetcher extracts only layers[0], whereas the rhdh-plugins catalog-index contract
applies every manifest layer in order. Catalog entities or dynamic-plugins.default.yaml contained
in later layers will be absent, causing incomplete Extension catalogs or extraction failure.
Code

pkg/fetcher/oci.go[R139-142]

+	// 7. Stream layer directly to extraction (no buffering)
+	reader, err := layers[0].Uncompressed()
+	if err != nil {
+		return fmt.Errorf("failed to uncompress layer: %w", err)
Relevance

●●● Strong

Catalog extraction must process all manifest layers; limiting extraction to layers[0] risks
incomplete catalogs.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
The PR explicitly opens only the first element returned by img.Layers(). The rhdh-plugins
implementation reads the full manifest layer list and extracts each layer serially because
catalog-index contents may span them.

pkg/fetcher/oci.go[130-146]
cmd/plugin-fetch/main.go[186-200]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/catalog-index.ts [80-85]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/catalog-index.ts [111-126]
External repo: redhat-developer/rhdh-plugins, workspaces/install-dynamic-plugins/packages/install-dynamic-plugins/src/catalog-index.ts [126-156]

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

## Issue description
Process all OCI layers for catalog-index images rather than extracting only the first layer. OCI filesystem contents can be spread across multiple ordered layers, and the established installer applies every layer.

## Issue Context
Layer application should preserve OCI order and safely handle files overwritten or removed by later layers. Plugin artifact extraction may continue using its separately defined package-layer contract.

## Fix Focus Areas
- pkg/fetcher/oci.go[130-146]
- cmd/plugin-fetch/main.go[175-224]

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


19. Downloads buffer without limits 🐞 Bug ☼ Reliability
Description
HTTP and NPM tarballs are read entirely into memory before integrity verification, so
MAX_ENTRY_SIZE does not constrain response memory use. A large or malicious response can exhaust
the init container's memory before archive entry checks run.
Code

pkg/fetcher/http.go[R53-56]

+		// Read body into memory for integrity check
+		data, err := io.ReadAll(resp.Body)
+		if err != nil {
+			return fmt.Errorf("failed to read response: %w", err)
Relevance

●●● Strong

Unbounded io.ReadAll defeats configured extraction limits and creates an obvious memory-exhaustion
risk.

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
Both remote fetchers call io.ReadAll before extraction. The only configured limit is checked
against individual tar headers later, after the complete response has already occupied memory.

pkg/fetcher/http.go[50-66]
pkg/fetcher/npm.go[165-183]
pkg/fetcher/extract.go[19-26]
pkg/fetcher/extract.go[73-75]

Agent prompt

[Comment truncated to fit github's 65,536-char limit.]

Comment thread Makefile Outdated
Comment thread pkg/fetcher/extract.go
Comment thread pkg/fetcher/fetcher.go
Comment thread pkg/fetcher/npm_extract.go Outdated
Comment thread pkg/fetcher/http.go Outdated
Comment thread cmd/plugin-fetch/main.go
Comment thread cmd/plugin-fetch/main.go Outdated
Comment thread cmd/plugin-fetch/main.go Outdated
Comment thread pkg/fetcher/fetcher.go
Comment thread cmd/plugin-fetch/main.go Outdated
@rhdh-qodo-merge rhdh-qodo-merge Bot added documentation Improvements or additions to documentation enhancement New feature or request Tests labels Sep 2, 2026
@gazarenkov
gazarenkov marked this pull request as draft September 2, 2026 12:46
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 36.15917% with 369 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.09%. Comparing base (fe0ada5) to head (30a8189).

Files with missing lines Patch % Lines
cmd/plugin-fetch/main.go 10.84% 189 Missing ⚠️
pkg/fetcher/npm.go 62.74% 46 Missing and 11 partials ⚠️
pkg/fetcher/npm_extract.go 0.00% 44 Missing ⚠️
pkg/fetcher/lock.go 0.00% 27 Missing ⚠️
pkg/fetcher/fetcher.go 42.85% 15 Missing and 1 partial ⚠️
pkg/fetcher/local.go 61.11% 7 Missing and 7 partials ⚠️
pkg/fetcher/oci.go 54.83% 13 Missing and 1 partial ⚠️
pkg/fetcher/http.go 86.66% 3 Missing and 3 partials ⚠️
pkg/fetcher/extract.go 0.00% 1 Missing and 1 partial ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3466      +/-   ##
==========================================
- Coverage   63.60%   59.09%   -4.51%     
==========================================
  Files          42       49       +7     
  Lines        2907     3479     +572     
==========================================
+ Hits         1849     2056     +207     
- Misses        893     1234     +341     
- Partials      165      189      +24     
Flag Coverage Δ
nightly ?
unittests 59.09% <36.15%> (-4.51%) ⬇️

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

Files with missing lines Coverage Δ
pkg/fetcher/extract.go 54.68% <0.00%> (-1.77%) ⬇️
pkg/fetcher/http.go 86.66% <86.66%> (ø)
pkg/fetcher/local.go 61.11% <61.11%> (ø)
pkg/fetcher/oci.go 60.00% <54.83%> (ø)
pkg/fetcher/fetcher.go 42.85% <42.85%> (ø)
pkg/fetcher/lock.go 0.00% <0.00%> (ø)
pkg/fetcher/npm_extract.go 0.00% <0.00%> (ø)
pkg/fetcher/npm.go 62.74% <62.74%> (ø)
cmd/plugin-fetch/main.go 10.84% <10.84%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…taller

# Conflicts:
#	bundle/backstage.io/manifests/backstage-operator.clusterserviceversion.yaml
#	bundle/rhdh/manifests/backstage-operator.clusterserviceversion.yaml
…taller

# Conflicts:
#	Makefile
#	bundle/backstage.io/manifests/backstage-operator.clusterserviceversion.yaml
#	bundle/rhdh/manifests/backstage-operator.clusterserviceversion.yaml
#	go.mod
#	go.sum
#	pkg/fetcher/extract.go
#	pkg/fetcher/oci.go
#	pkg/fetcher/oci_test.go
#	plugin-installer/README.md
@sonarqubecloud

sonarqubecloud Bot commented Sep 9, 2026

Copy link
Copy Markdown

@gazarenkov
gazarenkov marked this pull request as ready for review September 9, 2026 07:10
@rhdh-qodo-merge

Copy link
Copy Markdown

Code review by qodo was updated up to the latest commit 30a8189

@rhdh-qodo-merge

Copy link
Copy Markdown

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request Tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants