feat: introduce Helm ResourceRepository as builtin plugin by matthiasbruns · Pull Request #2094 · open-component-model/open-component-model

matthiasbruns · 2026-03-27T19:04:24Z

What this PR does / why we need it

Introduces a Helm-based ResourceRepository implementing repository.ResourceRepository and bindings/go/plugin/manager/registries/resource/contract.go, registered as a builtin plugin. This enables download resource to work with helm/v1 access types and replaces the temporary ResourceConsumerIdentityProvider interface with a proper ResourceRepository following the OCI pattern.

Which issue(s) this PR fixes

Fixes open-component-model/ocm-project#911

Sub PRs

bindings feat!: helm resource repo bindings #2128

coderabbitai · 2026-03-27T19:04:45Z

📝 Walkthrough

Walkthrough

Removed the temporary HelmAccess type and ResourceConsumerIdentityProvider interface, replacing them with a proper ResourceRepository implementation that handles credential resolution and chart downloads. Updated all consumers, tests, and plugin registration accordingly.

Changes

Cohort / File(s)	Summary
Helm Access & Interface Removal `bindings/go/helm/access/access.go`, `bindings/go/helm/interface.go`	Deleted `HelmAccess` type and its `GetResourceCredentialConsumerIdentity` method, along with the public `ResourceConsumerIdentityProvider` interface that was previously exposed in the helm package.
ResourceRepository Implementation `bindings/go/helm/repository/resource/resource_repository.go`	Added new `ResourceRepository` type implementing credential identity resolution (converts Helm access specs, parses repository URLs, assigns OCI or legacy identity types), resource download (via `helmdownload.NewReadOnlyChartFromRemote`), and upload rejection. Includes internal `convertAccess` helper for access conversion.
ResourceRepository Tests `bindings/go/helm/repository/resource/resource_repository_test.go`	Added comprehensive test coverage for `ResourceRepository` including scheme retrieval, credential identity resolution for HTTP/OCI URLs, empty repository handling, upload rejection, and chart download with error cases.
Test Updates `bindings/go/helm/access/access_test.go`, `bindings/go/helm/transformation/get_helm_chart_test.go`	Updated test stubs and test execution: access test now uses `helmresource.NewResourceRepository` instead of `HelmAccess`; transformation test replaces `stubIdentityProvider` with `stubResourceRepository` and updates field wiring.
Consumer Updates `bindings/go/helm/transformation/get_helm_chart.go`, `bindings/go/transfer/internal/builder.go`, `cli/internal/plugin/builtin/builtin.go`	Updated `GetHelmChart` struct to replace `ResourceConsumerIdentityProvider` field with `ResourceRepository`; updated builder to wire new repository; added registration of Helm resource repository plugin in CLI builtin registration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

feat(883): helm transfer #1832: Initial Helm access and transformer implementation that is now being refactored to use ResourceRepository instead of HelmAccess.
feat: explicit transfer module #2047: Related transfer module builder changes to wire ResourceRepository for Helm handling.
feat(helm): add digest processor for Helm access type #2064: Related Helm credential resolution logic that also parses repository URLs and handles identity conversion.

Suggested labels

kind/feature, size/l

Suggested reviewers

jakobmoellerdev
fabianburth

Poem

🐰 A ResourceRepository hops into place,
Ditching old interfaces with grace,
Credentials resolved, charts downloaded with care,
The Helm abstraction is now truly fair!
✨ nose twitch ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.38% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately summarizes the main change: introducing a Helm ResourceRepository as a builtin plugin. It is concise, clear, and directly reflects the primary objective of the changeset.
Linked Issues check	✅ Passed	The PR successfully addresses the requirements from issue `#911`: implements Helm ResourceRepository, registers it as a builtin plugin, replaces the temporary interface, includes unit tests, updates the transformer, and integrates with the plugin system.
Out of Scope Changes check	✅ Passed	All changes are directly related to implementing the Helm ResourceRepository and integrating it into the system as specified in the linked issue. No out-of-scope modifications were introduced.
Description check	✅ Passed	The PR description clearly articulates the purpose: introducing a Helm-based ResourceRepository as a builtin plugin to enable download resource functionality for helm/v1 access types and replacing a temporary interface.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

coderabbitai

Actionable comments posted: 2

🧹 Nitpick comments (1)

bindings/go/helm/transformation/get_helm_chart.go (1)
57-58: Keep Helm downloads behind one implementation.

Credential lookup now goes through ResourceRepository, but the actual fetch still rebuilds the URL and calls helmdownload.NewReadOnlyChartFromRemote directly. That leaves this transformer and bindings/go/helm/repository/resource/resource_repository.go with separate temp-dir, auth, and future verification behavior to keep in sync. Extract the shared download logic or extend the repository API so both flows stay aligned.

Also applies to: 74-101
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@bindings/go/helm/transformation/get_helm_chart.go` around lines 57 - 58, This
transformer is duplicating download behavior: it gets credentials via
ResourceRepository.GetResourceCredentialConsumerIdentity but then rebuilds the
URL and calls helmdownload.NewReadOnlyChartFromRemote directly, causing
divergent temp-dir/auth/verification logic; fix by moving the remote-chart fetch
into a single shared implementation on ResourceRepository (e.g., add a method
like FetchChartFromRemote or DownloadChartWithCredentials that accepts ctx,
targetResource, consumerId and returns a ready-to-use chart path/reader) and
update this file to call that new repository method (replace direct
helmdownload.NewReadOnlyChartFromRemote usage and any local temp-dir/auth
handling) so both this transformer and
bindings/go/helm/repository/resource/resource_repository.go use the exact same
download/auth/verify flow.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@bindings/go/helm/repository/resource/resource_repository.go`:
- Around line 99-106: In convertAccess, guard against nil inputs before calling
resource.Access.GetType(): check that resource != nil and resource.Access != nil
and return a validation-style error (e.g., fmt.Errorf or a typed validation
error) instead of letting GetType panic; update the function (convertAccess) to
perform these nil checks at the top and return a clear error message like
"invalid resource: missing access" so callers receive an error for malformed
descriptor input rather than crashing.
- Line 24: The DownloadResource implementation in ResourceRepository currently
returns the fetched chart without verifying the downloaded blob against the
resource descriptor’s integrity metadata; update the DownloadResource method
(and the similar code path around lines 61-92) to, after downloading the blob,
compute the blob’s digest(s)/checksum(s) using the same algorithm(s) expected by
the descriptor and compare them against the descriptor’s integrity fields (e.g.,
the Resource/descriptor integrity/checksum values exposed by the descriptor
API); if the verification fails, return an error and do not return the blob—only
return the blob when the digest comparison matches the descriptor’s integrity
metadata.

---

Nitpick comments:
In `@bindings/go/helm/transformation/get_helm_chart.go`:
- Around line 57-58: This transformer is duplicating download behavior: it gets
credentials via ResourceRepository.GetResourceCredentialConsumerIdentity but
then rebuilds the URL and calls helmdownload.NewReadOnlyChartFromRemote
directly, causing divergent temp-dir/auth/verification logic; fix by moving the
remote-chart fetch into a single shared implementation on ResourceRepository
(e.g., add a method like FetchChartFromRemote or DownloadChartWithCredentials
that accepts ctx, targetResource, consumerId and returns a ready-to-use chart
path/reader) and update this file to call that new repository method (replace
direct helmdownload.NewReadOnlyChartFromRemote usage and any local temp-dir/auth
handling) so both this transformer and
bindings/go/helm/repository/resource/resource_repository.go use the exact same
download/auth/verify flow.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

Push a commit to this branch (recommended)
Create a new PR with the fixes

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8dd302d7-716e-4939-9294-1f7c40530595

📥 Commits

Reviewing files that changed from the base of the PR and between 047711f and 64cc329.

📒 Files selected for processing (9)

bindings/go/helm/access/access.go
bindings/go/helm/access/access_test.go
bindings/go/helm/interface.go
bindings/go/helm/repository/resource/resource_repository.go
bindings/go/helm/repository/resource/resource_repository_test.go
bindings/go/helm/transformation/get_helm_chart.go
bindings/go/helm/transformation/get_helm_chart_test.go
bindings/go/transfer/internal/builder.go
cli/internal/plugin/builtin/builtin.go

💤 Files with no reviewable changes (2)

bindings/go/helm/access/access.go
bindings/go/helm/interface.go

Implement a Helm-based ResourceRepository that allows `download resource` to work with helm/v1 access types. This replaces the temporary ResourceConsumerIdentityProvider interface with a proper ResourceRepository implementation following the OCI pattern. Closes open-component-model/ocm-project#911

On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>

#### What this PR does / why we need it Introduces a Helm-based `ResourceRepository` implementing `repository.ResourceRepository` and `bindings/go/plugin/manager/registries/resource/contract.go`. Full usage can be seen here: #2130 #### Which issue(s) this PR fixes Contributes open-component-model/ocm-project#911 #### CLI PR #2094 #### Testing Fully tested in #2130 #### Helm ResourceRepository — Changes compared to legacy OCM - **CLI & transfer** will use the `ResourceRepository` interface instead of the deprecated `HelmAccess` struct for credential identity resolution and chart downloads - **Deprecated types removed**: `helm.ResourceConsumerIdentityProvider` interface, `helmaccess.HelmAccess` struct, and their tests — functionality is covered by `ResourceRepository` - **Blob format**: Downloads return a `ChartBlob` (tar archive wrapping `.tgz` + optional `.prov`), replacing the legacy pattern of separate blob accessors for chart and provenance - **Plugin registration**: The helm `ResourceRepository` is registered as a builtin plugin alongside the existing OCI and digest processor plugins in #2130 ##### Intentional differences from legacy | | Legacy OCM | New | |--------------------------------------|-------------------------------------------------------------------|--------------------------------------------------------------| | `CACert` / `Keyring` on access spec | Supported as inline fields | Omitted — use credential resolver instead | | OCI-hosted helm charts (`oci://`) | Handled by helm access method with separate OCI download path | Should use the OCI ResourceRepository (separate access type) | | Blob return format (only internally) | Raw `.tgz` blob (`ChartLayerMediaType`), prov accessed separately | `ChartBlob` tar wrapping both `.tgz` and `.prov` | | Local charts | Not in access method (`IsLocal() = false`) | Not in ResourceRepository — handled by helm input plugin | ##### Test coverage - Unit tests for `ResourceRepository` (credential identity, download, temp folder usage, nil guards) - Unit tests for `ChartBlob` extraction (using real testdata charts) - Integration test: `add cv` with helm access to OCI registry - Integration test: `download resource` with helm access from CTF (byte-level verification) - Shell test: end-to-end download of podinfo chart with SHA256 verification --------- Signed-off-by: Matthias Bruns <git@matthiasbruns.com> Signed-off-by: Jakob Möller <contact@jakob-moeller.com> Co-authored-by: Jakob Möller <contact@jakob-moeller.com>

#### What this PR does / why we need it Introduces a Helm-based `ResourceRepository` implementing `repository.ResourceRepository` and `bindings/go/plugin/manager/registries/resource/contract.go`. Full usage can be seen here: #2130 #### Which issue(s) this PR fixes Contributes open-component-model/ocm-project#911 #### CLI PR #2094 #### Testing Fully tested in #2130 #### Helm ResourceRepository — Changes compared to legacy OCM - **CLI & transfer** will use the `ResourceRepository` interface instead of the deprecated `HelmAccess` struct for credential identity resolution and chart downloads - **Deprecated types removed**: `helm.ResourceConsumerIdentityProvider` interface, `helmaccess.HelmAccess` struct, and their tests — functionality is covered by `ResourceRepository` - **Blob format**: Downloads return a `ChartBlob` (tar archive wrapping `.tgz` + optional `.prov`), replacing the legacy pattern of separate blob accessors for chart and provenance - **Plugin registration**: The helm `ResourceRepository` is registered as a builtin plugin alongside the existing OCI and digest processor plugins in #2130 ##### Intentional differences from legacy | | Legacy OCM | New | |--------------------------------------|-------------------------------------------------------------------|--------------------------------------------------------------| | `CACert` / `Keyring` on access spec | Supported as inline fields | Omitted — use credential resolver instead | | OCI-hosted helm charts (`oci://`) | Handled by helm access method with separate OCI download path | Should use the OCI ResourceRepository (separate access type) | | Blob return format (only internally) | Raw `.tgz` blob (`ChartLayerMediaType`), prov accessed separately | `ChartBlob` tar wrapping both `.tgz` and `.prov` | | Local charts | Not in access method (`IsLocal() = false`) | Not in ResourceRepository — handled by helm input plugin | ##### Test coverage - Unit tests for `ResourceRepository` (credential identity, download, temp folder usage, nil guards) - Unit tests for `ChartBlob` extraction (using real testdata charts) - Integration test: `add cv` with helm access to OCI registry - Integration test: `download resource` with helm access from CTF (byte-level verification) - Shell test: end-to-end download of podinfo chart with SHA256 verification --------- Signed-off-by: Matthias Bruns <git@matthiasbruns.com> Signed-off-by: Jakob Möller <contact@jakob-moeller.com> Co-authored-by: Jakob Möller <contact@jakob-moeller.com> 2207446

matthiasbruns requested a review from a team as a code owner March 27, 2026 19:04

github-actions Bot added kind/feature new feature, enhancement, improvement, extension size/m Medium labels Mar 27, 2026

matthiasbruns marked this pull request as draft March 27, 2026 19:09

coderabbitai Bot reviewed Mar 27, 2026

View reviewed changes

Comment thread bindings/go/helm/repository/resource/resource_repository.go

Comment thread bindings/go/helm/repository/resource/resource_repository.go

matthiasbruns added 3 commits March 31, 2026 07:55

fix: add nil guards in convertAccess to prevent panic on malformed input

517cabc

feat. indroduce helmchartblob

656e441

On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>

matthiasbruns force-pushed the feat/911_helm_resource_repo branch from cde2c95 to 656e441 Compare March 31, 2026 05:58

github-actions Bot added the size/l Large label Mar 31, 2026

matthiasbruns mentioned this pull request Mar 31, 2026

feat!: helm resource repo bindings #2128

Merged

matthiasbruns closed this Mar 31, 2026

matthiasbruns deleted the feat/911_helm_resource_repo branch March 31, 2026 06:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: introduce Helm ResourceRepository as builtin plugin#2094

feat: introduce Helm ResourceRepository as builtin plugin#2094
matthiasbruns wants to merge 3 commits into
open-component-model:mainfrom
matthiasbruns:feat/911_helm_resource_repo

matthiasbruns commented Mar 27, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Mar 27, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

matthiasbruns commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What this PR does / why we need it

Which issue(s) this PR fixes

Sub PRs

Uh oh!

coderabbitai Bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested labels

Suggested reviewers

Poem

❌ Failed checks (1 warning)

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

matthiasbruns commented Mar 27, 2026 •

edited

Loading

coderabbitai Bot commented Mar 27, 2026 •

edited

Loading