feat: 911 helm resource repo#2130
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (10)
📝 WalkthroughWalkthroughThis PR updates Helm package import paths across the codebase from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
9cfa492 to
31b0179
Compare
#!/bin/zsh
set -euo pipefail
# Test: Download a helm chart resource using the Helm ResourceRepository.
# This creates a CTF with a podinfo helm access resource, then downloads
# the chart from the remote helm repository via the ResourceRepository plugin.
alias OCM='go run ../../main.go'
SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)"
WORK_DIR="$SCRIPT_DIR/test-download-helm-resource-output"
rm -rf "$WORK_DIR"
mkdir -p "$WORK_DIR"
CTF_DIR="$WORK_DIR/ctf"
DOWNLOAD_DIR="$WORK_DIR/downloaded"
CONSTRUCTOR_PATH="$WORK_DIR/constructor.yaml"
echo "=== Test: Download Helm Resource (podinfo) ==="
echo "Working directory: $WORK_DIR"
# 1. Create constructor with helm access pointing to the real podinfo chart repo
cat <<EOF > "$CONSTRUCTOR_PATH"
components:
- name: ocm.software/podinfo
version: 6.9.1
provider:
name: ocm.software
resources:
- name: podinfo
version: 6.9.1
type: helmChart
access:
type: helm/v1
helmRepository: https://stefanprodan.github.io/podinfo
helmChart: podinfo-6.9.1.tgz
EOF
# 2. Add component version to CTF (skip digest processing since we have remote access)
echo "--- Adding component version to CTF ---"
OCM add cv --repository "ctf::$CTF_DIR" --constructor "$CONSTRUCTOR_PATH" --skip-reference-digest-processing
# 3. Download the resource - this triggers the Helm ResourceRepository to fetch from remote
echo "--- Downloading helm resource ---"
OCM download resource "ctf::$CTF_DIR//ocm.software/podinfo:6.9.1" \
--identity name=podinfo,version=6.9.1 \
--output "$DOWNLOAD_DIR" \
--extraction-policy disable
# 4. Verify the download produced a non-empty file
if [ ! -f "$DOWNLOAD_DIR" ]; then
echo "FAIL: Downloaded file does not exist at $DOWNLOAD_DIR"
exit 1
fi
FILE_SIZE=$(stat -f%z "$DOWNLOAD_DIR" 2>/dev/null || stat -c%s "$DOWNLOAD_DIR" 2>/dev/null)
if [ "$FILE_SIZE" -eq 0 ]; then
echo "FAIL: Downloaded file is empty"
exit 1
fi
echo "--- Downloaded file size: $FILE_SIZE bytes ---"
# 5. Verify the tar contains the expected chart file
TAR_CONTENTS=$(tar tf "$DOWNLOAD_DIR")
echo "--- Tar contents ---"
echo "$TAR_CONTENTS"
if ! echo "$TAR_CONTENTS" | grep -q "podinfo-6.9.1.tgz"; then
echo "FAIL: Tar does not contain podinfo-6.9.1.tgz"
exit 1
fi
# 6. Extract the tar and verify SHA256 of the chart file
EXTRACT_DIR="$WORK_DIR/extracted"
mkdir -p "$EXTRACT_DIR"
tar xf "$DOWNLOAD_DIR" -C "$EXTRACT_DIR"
CHART_FILE=$(find "$EXTRACT_DIR" -name "podinfo-6.9.1.tgz" -type f)
if [ -z "$CHART_FILE" ]; then
echo "FAIL: Could not find extracted chart file"
exit 1
fi
EXPECTED_SHA256="6d082dc0d4e90fbb525c0c1fc8a52d5279581750b8888688b07ce00f96d947e8"
ACTUAL_SHA256=$(shasum -a 256 "$CHART_FILE" | awk '{print $1}')
echo "--- Chart SHA256: $ACTUAL_SHA256 ---"
if [ "$ACTUAL_SHA256" != "$EXPECTED_SHA256" ]; then
echo "FAIL: SHA256 mismatch"
echo " expected: $EXPECTED_SHA256"
echo " actual: $ACTUAL_SHA256"
exit 1
fi
echo ""
echo "=== PASS: Helm resource download succeeded ==="This should download podinfo helm chart, umpack it and verify. You can check the contents in Expected output If you run this on e.g. main without the resource-repo, you will see something like this: |
31b0179 to
ce37ee4
Compare
#### 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
✅ Deploy Preview for ocm-website canceled.
|
89af7d9 to
fb6f41e
Compare
<!-- markdownlint-disable MD041 --> #### What this PR does / why we need it Follow up of #2128 `transfer` are refactored to use the new Helm ResourceRepository. #### Which issue(s) this PR fixes Contributes open-component-model/ocm-project#911 #### Testing ##### How to test the changes Check comment: #2130 (comment) ##### Verification - [x] I have tested the changes locally by running `ocm` --------- Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
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 Signed-off-by: Matthias Bruns <git@matthiasbruns.com> # Conflicts: # bindings/go/helm/access/access.go # bindings/go/helm/access/access_test.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
On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
34d196e to
af3258d
Compare
On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
af3258d to
a2f6a2d
Compare
On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
On-behalf-of: SAP <matthias.bruns@sap.com> Signed-off-by: Matthias Bruns <git@matthiasbruns.com>
Update Scheme reference in tests to use helminputspec.Scheme after helm restructuring in open-component-model#2130. Add legacy type resolution test. Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
Update Scheme reference in tests to use helminputspec.Scheme after helm restructuring in open-component-model#2130. Add legacy type resolution test. Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
Update Scheme reference in tests to use helminputspec.Scheme after helm restructuring in open-component-model#2130. Add legacy type resolution test. Signed-off-by: Gerald Morrison (SAP) <gerald.morrison@sap.com>
What this PR does / why we need it
This PR tied the helm ResourceRepository changes together and updates the spec location changes.
Follow up of #2128
cliandtransferare refactored to use the new Helm ResourceRepository.Also removes the deprecated
AccessandResourceConsumerIdentityProviderWhich issue(s) this PR fixes
Contributes open-component-model/ocm-project#911
Testing
How to test the changes
Check comment: #2130 (comment)
Verification
ocm