Enable remote dependencies from OCI repositories

If implemented, the source controller will be able to resolve charts
dependencies from OCI repositories.

The remote builder has been refactored as part of this work.

Signed-off-by: Soule BA <soule@weave.works>

controllers/helmchart_controller.go

@@ -35,6 +35,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"k8s.io/apimachinery/pkg/util/uuid"
 	kuberecorder "k8s.io/client-go/tools/record"
 	ctrl "sigs.k8s.io/controller-runtime"
@@ -461,9 +462,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		loginOpts []helmreg.LoginOption
 	)
 
+	normalizedURL := repository.NormalizeURL(repo.Spec.URL)
 	// Construct the Getter options from the HelmRepository data
 	clientOpts := []helmgetter.Option{
-		helmgetter.WithURL(repo.Spec.URL),
+		helmgetter.WithURL(normalizedURL),
 		helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
 		helmgetter.WithPassCredentialsAll(repo.Spec.PassCredentials),
 	}
@@ -491,7 +493,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		}
 		clientOpts = append(clientOpts, opts...)
 
-		tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL)
+		tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, normalizedURL)
 		if err != nil {
 			e := &serror.Event{
 				Err:    fmt.Errorf("failed to create TLS client config with secret data: %w", err),
@@ -503,7 +505,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		}
 
 		// Build registryClient options from secret
-		loginOpt, err := registry.LoginOptionFromSecret(repo.Spec.URL, *secret)
+		loginOpt, err := registry.LoginOptionFromSecret(normalizedURL, *secret)
 		if err != nil {
 			e := &serror.Event{
 				Err:    fmt.Errorf("failed to configure Helm client with secret data: %w", err),
@@ -518,11 +520,11 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 	}
 
 	// Initialize the chart repository
-	var chartRepo chart.Repository
+	var chartRepo repository.Downloader
 	switch repo.Spec.Type {
 	case sourcev1.HelmRepositoryTypeOCI:
-		if !helmreg.IsOCI(repo.Spec.URL) {
+		if !helmreg.IsOCI(normalizedURL) {
-			err := fmt.Errorf("invalid OCI registry URL: %s", repo.Spec.URL)
+			err := fmt.Errorf("invalid OCI registry URL: %s", normalizedURL)
 			return chartRepoConfigErrorReturn(err, obj)
 		}
 
@@ -530,7 +532,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 		// this is needed because otherwise the credentials are stored in ~/.docker/config.json.
 		// TODO@souleb: remove this once the registry move to Oras v2
 		// or rework to enable reusing credentials to avoid the unneccessary handshake operations
-		registryClient, file, err := r.RegistryClientGenerator(loginOpts != nil)
+		registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpts != nil)
 		if err != nil {
 			e := &serror.Event{
 				Err:    fmt.Errorf("failed to construct Helm client: %w", err),
@@ -540,9 +542,9 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 			return sreconcile.ResultEmpty, e
 		}
 
-		if file != "" {
+		if credentialsFile != "" {
 			defer func() {
-				if err := os.Remove(file); err != nil {
+				if err := os.Remove(credentialsFile); err != nil {
 					r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
 						"failed to delete temporary credentials file: %s", err)
 				}
@@ -551,7 +553,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 
 		// Tell the chart repository to use the OCI client with the configured getter
 		clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient))
-		ociChartRepo, err := repository.NewOCIChartRepository(repo.Spec.URL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient))
+		ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient))
 		if err != nil {
 			return chartRepoConfigErrorReturn(err, obj)
 		}
@@ -571,7 +573,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 			}
 		}
 	default:
-		httpChartRepo, err := repository.NewChartRepository(repo.Spec.URL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts,
+		httpChartRepo, err := repository.NewChartRepository(normalizedURL, r.Storage.LocalPath(*repo.GetArtifact()), r.Getters, tlsConfig, clientOpts,
 			repository.WithMemoryCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
 				r.IncCacheEvents(event, obj.Name, obj.Namespace)
 			}))
@@ -684,9 +686,15 @@ func (r *HelmChartReconciler) buildFromTarballArtifact(ctx context.Context, obj
 
 	// Setup dependency manager
 	dm := chart.NewDependencyManager(
-		chart.WithRepositoryCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetName(), obj.GetNamespace())),
+		chart.WithDownloaderCallback(r.namespacedChartRepositoryCallback(ctx, obj.GetName(), obj.GetNamespace())),
 	)
-	defer dm.Clear()
+	defer func() {
+		err := dm.Clear()
+		if err != nil {
+			r.eventLogf(ctx, obj, corev1.EventTypeWarning, meta.FailedReason,
+				"dependency manager cleanup error: %s", err)
+		}
+	}()
 
 	// Configure builder options, including any previously cached chart
 	opts := chart.BuildOptions{
@@ -913,12 +921,17 @@ func (r *HelmChartReconciler) garbageCollect(ctx context.Context, obj *sourcev1.
 	return nil
 }
 
-// namespacedChartRepositoryCallback returns a chart.GetChartRepositoryCallback scoped to the given namespace.
+// namespacedChartRepositoryCallback returns a chart.GetChartDownloaderCallback scoped to the given namespace.
-// The returned callback returns a repository.ChartRepository configured with the retrieved v1beta1.HelmRepository,
+// The returned callback returns a repository.Downloader configured with the retrieved v1beta1.HelmRepository,
 // or a shim with defaults if no object could be found.
-func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartRepositoryCallback {
+// The callback returns an object with a state, so the caller has to do the necessary cleanup.
-	return func(url string) (*repository.ChartRepository, error) {
+func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
-		var tlsConfig *tls.Config
+	return func(url string) (repository.Downloader, error) {
+		var (
+			tlsConfig *tls.Config
+			loginOpts []helmreg.LoginOption
+		)
+		normalizedURL := repository.NormalizeURL(url)
 		repo, err := r.resolveDependencyRepository(ctx, url, namespace)
 		if err != nil {
 			// Return Kubernetes client errors, but ignore others
@@ -933,7 +946,7 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
 			}
 		}
 		clientOpts := []helmgetter.Option{
-			helmgetter.WithURL(repo.Spec.URL),
+			helmgetter.WithURL(normalizedURL),
 			helmgetter.WithTimeout(repo.Spec.Timeout.Duration),
 			helmgetter.WithPassCredentialsAll(repo.Spec.PassCredentials),
 		}
@@ -947,26 +960,77 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
 			}
 			clientOpts = append(clientOpts, opts...)
 
-			tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL)
+			tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, normalizedURL)
 			if err != nil {
 				return nil, fmt.Errorf("failed to create TLS client config for HelmRepository '%s': %w", repo.Name, err)
 			}
+
+			// Build registryClient options from secret
+			loginOpt, err := registry.LoginOptionFromSecret(normalizedURL, *secret)
+			if err != nil {
+				return nil, fmt.Errorf("failed to create login options for HelmRepository '%s': %w", repo.Name, err)
+			}
+
+			loginOpts = append([]helmreg.LoginOption{}, loginOpt)
 		}
 
-		chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, tlsConfig, clientOpts)
+		var chartRepo repository.Downloader
-		if err != nil {
+		if helmreg.IsOCI(normalizedURL) {
-			return nil, err
+			registryClient, credentialsFile, err := r.RegistryClientGenerator(loginOpts != nil)
+			if err != nil {
+				return nil, fmt.Errorf("failed to create registry client for HelmRepository '%s': %w", repo.Name, err)
+			}
+
+			var errs []error
+			// Tell the chart repository to use the OCI client with the configured getter
+			clientOpts = append(clientOpts, helmgetter.WithRegistryClient(registryClient))
+			ociChartRepo, err := repository.NewOCIChartRepository(normalizedURL, repository.WithOCIGetter(r.Getters),
+				repository.WithOCIGetterOptions(clientOpts),
+				repository.WithOCIRegistryClient(registryClient),
+				repository.WithCredentialsFile(credentialsFile))
+			if err != nil {
+				errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", repo.Name, err))
+				// clean up the credentialsFile
+				if credentialsFile != "" {
+					if err := os.Remove(credentialsFile); err != nil {
+						errs = append(errs, err)
+					}
+				}
+				return nil, kerrors.NewAggregate(errs)
+			}
+
+			// If login options are configured, use them to login to the registry
+			// The OCIGetter will later retrieve the stored credentials to pull the chart
+			if loginOpts != nil {
+				err = ociChartRepo.Login(loginOpts...)
+				if err != nil {
+					errs = append(errs, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", repo.Name, err))
+					// clean up the credentialsFile
+					errs = append(errs, ociChartRepo.Clear())
+					return nil, kerrors.NewAggregate(errs)
+				}
+			}
+
+			chartRepo = ociChartRepo
+		} else {
+			httpChartRepo, err := repository.NewChartRepository(normalizedURL, "", r.Getters, tlsConfig, clientOpts)
+			if err != nil {
+				return nil, err
+			}
+
+			// Ensure that the cache key is the same as the artifact path
+			// otherwise don't enable caching. We don't want to cache indexes
+			// for repositories that are not reconciled by the source controller.
+			if repo.Status.Artifact != nil {
+				httpChartRepo.CachePath = r.Storage.LocalPath(*repo.GetArtifact())
+				httpChartRepo.SetMemCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
+					r.IncCacheEvents(event, name, namespace)
+				})
+			}
+
+			chartRepo = httpChartRepo
 		}
 
-		// Ensure that the cache key is the same as the artifact path
-		// otherwise don't enable caching. We don't want to cache indexes
-		// for repositories that are not reconciled by the source controller.
-		if repo.Status.Artifact != nil {
-			chartRepo.CachePath = r.Storage.LocalPath(*repo.GetArtifact())
-			chartRepo.SetMemCache(r.Storage.LocalPath(*repo.GetArtifact()), r.Cache, r.TTL, func(event string) {
-				r.IncCacheEvents(event, name, namespace)
-			})
-		}
 		return chartRepo, nil
 	}
 }

controllers/helmchart_controller_test.go

@@ -411,9 +411,6 @@ func TestHelmChartReconciler_reconcileSource(t *testing.T) {
 				}))
 			},
 		},
-		//{
-		//	name: "Error on transient build error",
-		//},
 		{
 			name: "Stalling on persistent build error",
 			source: &sourcev1.GitRepository{
@@ -1070,7 +1067,7 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
 			assertFunc: func(g *WithT, build chart.Build) {
 				g.Expect(build.Name).To(Equal("helmchartwithdeps"))
 				g.Expect(build.Version).To(Equal("0.1.0"))
-				g.Expect(build.ResolvedDependencies).To(Equal(3))
+				g.Expect(build.ResolvedDependencies).To(Equal(4))
 				g.Expect(build.Path).To(BeARegularFile())
 			},
 			cleanFunc: func(g *WithT, build *chart.Build) {
@@ -1178,10 +1175,11 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
 			g := NewWithT(t)
 
 			r := &HelmChartReconciler{
-				Client:        fake.NewClientBuilder().Build(),
+				Client:                  fake.NewClientBuilder().Build(),
-				EventRecorder: record.NewFakeRecorder(32),
+				EventRecorder:           record.NewFakeRecorder(32),
-				Storage:       storage,
+				Storage:                 storage,
-				Getters:       testGetters,
+				Getters:                 testGetters,
+				RegistryClientGenerator: registry.ClientGenerator,
 			}
 
 			obj := &sourcev1.HelmChart{

controllers/helmrepository_controller_oci.go

@@ -326,7 +326,7 @@ func (r *HelmRepositoryOCIReconciler) reconcile(ctx context.Context, obj *v1beta
 	if loginOpts != nil {
 		err = chartRepo.Login(loginOpts...)
 		if err != nil {
-			e := fmt.Errorf("failed to log into registry '%s': %w", obj.Spec.URL, err)
+			e := fmt.Errorf("failed to login to registry '%s': %w", obj.Spec.URL, err)
 			conditions.MarkFalse(obj, meta.ReadyCondition, sourcev1.AuthenticationFailedReason, e.Error())
 			result, retErr = ctrl.Result{}, e
 			return

controllers/testdata/charts/helmchartwithdeps/Chart.yaml

@@ -31,3 +31,6 @@ dependencies:
   - name: grafana
     version: ">=5.7.0"
     repository: "https://grafana.github.io/helm-charts"
+  - name: podinfo
+    version: ">=6.1.*"
+    repository: "oci://ghcr.io/stefanprodan/charts"

internal/helm/chart/builder_local_test.go

@@ -67,7 +67,7 @@ func TestLocalBuilder_Build(t *testing.T) {
 		reference           Reference
 		buildOpts           BuildOptions
 		valuesFiles         []helmchart.File
-		repositories        map[string]*repository.ChartRepository
+		repositories        map[string]repository.Downloader
 		dependentChartPaths []string
 		wantValues          chartutil.Values
 		wantVersion         string
@@ -146,7 +146,7 @@ fullnameOverride: "full-foo-name-override"`),
 		{
 			name:      "chart with dependencies",
 			reference: LocalReference{Path: "../testdata/charts/helmchartwithdeps"},
-			repositories: map[string]*repository.ChartRepository{
+			repositories: map[string]repository.Downloader{
 				"https://grafana.github.io/helm-charts/": mockRepo(),
 			},
 			dependentChartPaths: []string{"./../testdata/charts/helmchart"},
@@ -165,7 +165,7 @@ fullnameOverride: "full-foo-name-override"`),
 		{
 			name:      "v1 chart with dependencies",
 			reference: LocalReference{Path: "../testdata/charts/helmchartwithdeps-v1"},
-			repositories: map[string]*repository.ChartRepository{
+			repositories: map[string]repository.Downloader{
 				"https://grafana.github.io/helm-charts/": mockRepo(),
 			},
 			dependentChartPaths: []string{"../testdata/charts/helmchart-v1"},

internal/helm/chart/builder_remote.go

@@ -34,24 +34,16 @@ import (
 
 	"github.com/fluxcd/source-controller/internal/fs"
 	"github.com/fluxcd/source-controller/internal/helm/chart/secureloader"
+	"github.com/fluxcd/source-controller/internal/helm/repository"
 )
 
-// Repository is a repository.ChartRepository or a repository.OCIChartRepository.
-// It is used to download a chart from a remote Helm repository or OCI registry.
-type Repository interface {
-	// GetChartVersion returns the repo.ChartVersion for the given name and version.
-	GetChartVersion(name, version string) (*repo.ChartVersion, error)
-	// GetChartVersion returns a chart.ChartVersion from the remote repository.
-	DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer, error)
-}
-
 type remoteChartBuilder struct {
-	remote Repository
+	remote repository.Downloader
 }
 
 // NewRemoteBuilder returns a Builder capable of building a Helm
-// chart with a RemoteReference in the given repository.ChartRepository.
+// chart with a RemoteReference in the given repository.Downloader.
-func NewRemoteBuilder(repository Repository) Builder {
+func NewRemoteBuilder(repository repository.Downloader) Builder {
 	return &remoteChartBuilder{
 		remote: repository,
 	}
@@ -132,7 +124,7 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 	return result, nil
 }
 
-func (b *remoteChartBuilder) downloadFromRepository(remote Repository, remoteRef RemoteReference, opts BuildOptions) (*bytes.Buffer, *Build, error) {
+func (b *remoteChartBuilder) downloadFromRepository(remote repository.Downloader, remoteRef RemoteReference, opts BuildOptions) (*bytes.Buffer, *Build, error) {
 	// Get the current version for the RemoteReference
 	cv, err := remote.GetChartVersion(remoteRef.Name, remoteRef.Version)
 	if err != nil {

internal/helm/chart/dependency_manager.go

@@ -30,26 +30,27 @@ import (
 	"golang.org/x/sync/errgroup"
 	"golang.org/x/sync/semaphore"
 	helmchart "helm.sh/helm/v3/pkg/chart"
+	"k8s.io/apimachinery/pkg/util/errors"
 
 	"github.com/fluxcd/source-controller/internal/helm/chart/secureloader"
 	"github.com/fluxcd/source-controller/internal/helm/repository"
 )
 
-// GetChartRepositoryCallback must return a repository.ChartRepository for the
+// GetChartDownloaderCallback must return a Downloader for the
-// URL, or an error describing why it could not be returned.
+// URL or an error describing why it could not be returned.
-type GetChartRepositoryCallback func(url string) (*repository.ChartRepository, error)
+type GetChartDownloaderCallback func(url string) (repository.Downloader, error)
 
 // DependencyManager manages dependencies for a Helm chart.
 type DependencyManager struct {
-	// repositories contains a map of repository.ChartRepository objects
+	// downloaders contains a map of Downloader objects
 	// indexed by their repository.NormalizeURL.
 	// It is consulted as a lookup table for missing dependencies, based on
 	// the (repository) URL the dependency refers to.
-	repositories map[string]*repository.ChartRepository
+	downloaders map[string]repository.Downloader
 
-	// getRepositoryCallback can be set to an on-demand GetChartRepositoryCallback
+	// getChartDownloaderCallback can be set to an on-demand GetChartDownloaderCallback
-	// whose returned result is cached to repositories.
+	// whose returned result is cached to downloaders.
-	getRepositoryCallback GetChartRepositoryCallback
+	getChartDownloaderCallback GetChartDownloaderCallback
 
 	// concurrent is the number of concurrent chart-add operations during
 	// Build. Defaults to 1 (non-concurrent).
@@ -64,16 +65,16 @@ type DependencyManagerOption interface {
 	applyToDependencyManager(dm *DependencyManager)
 }
 
-type WithRepositories map[string]*repository.ChartRepository
+type WithRepositories map[string]repository.Downloader
 
 func (o WithRepositories) applyToDependencyManager(dm *DependencyManager) {
-	dm.repositories = o
+	dm.downloaders = o
 }
 
-type WithRepositoryCallback GetChartRepositoryCallback
+type WithDownloaderCallback GetChartDownloaderCallback
 
-func (o WithRepositoryCallback) applyToDependencyManager(dm *DependencyManager) {
+func (o WithDownloaderCallback) applyToDependencyManager(dm *DependencyManager) {
-	dm.getRepositoryCallback = GetChartRepositoryCallback(o)
+	dm.getChartDownloaderCallback = GetChartDownloaderCallback(o)
 }
 
 type WithConcurrent int64
@@ -92,20 +93,14 @@ func NewDependencyManager(opts ...DependencyManagerOption) *DependencyManager {
 	return dm
 }
 
-// Clear iterates over the repositories, calling Unload and RemoveCache on all
+// Clear iterates over the downloaders, calling Clear on all
-// items. It returns a collection of (cache removal) errors.
+// items. It returns an aggregate error of all Clear errors.
-func (dm *DependencyManager) Clear() []error {
+func (dm *DependencyManager) Clear() error {
 	var errs []error
-	for _, v := range dm.repositories {
+	for _, v := range dm.downloaders {
-		if err := v.CacheIndexInMemory(); err != nil {
+		errs = append(errs, v.Clear())
-			errs = append(errs, err)
-		}
-		v.Unload()
-		if err := v.RemoveCache(); err != nil {
-			errs = append(errs, err)
-		}
 	}
-	return errs
+	return errors.NewAggregate(errs)
 }
 
 // Build compiles a set of missing dependencies from chart.Chart, and attempts to
@@ -236,13 +231,9 @@ func (dm *DependencyManager) addRemoteDependency(chart *chartWithLock, dep *helm
 		return err
 	}
 
-	if err = repo.StrategicallyLoadIndex(); err != nil {
-		return fmt.Errorf("failed to load index for '%s': %w", dep.Name, err)
-	}
-
 	ver, err := repo.GetChartVersion(dep.Name, dep.Version)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to get chart '%s' version '%s' from '%s': %w", dep.Name, dep.Version, dep.Repository, err)
 	}
 	res, err := repo.DownloadChart(ver)
 	if err != nil {
@@ -259,27 +250,29 @@ func (dm *DependencyManager) addRemoteDependency(chart *chartWithLock, dep *helm
 	return nil
 }
 
-// resolveRepository first attempts to resolve the url from the repositories, falling back
+// resolveRepository first attempts to resolve the url from the downloaders, falling back
-// to getRepositoryCallback if set. It returns the resolved Index, or an error.
+// to getDownloaderCallback if set. It returns the resolved Index, or an error.
-func (dm *DependencyManager) resolveRepository(url string) (_ *repository.ChartRepository, err error) {
+func (dm *DependencyManager) resolveRepository(url string) (repo repository.Downloader, err error) {
 	dm.mu.Lock()
 	defer dm.mu.Unlock()
 
 	nUrl := repository.NormalizeURL(url)
-	if _, ok := dm.repositories[nUrl]; !ok {
+	if _, ok := dm.downloaders[nUrl]; !ok {
-		if dm.getRepositoryCallback == nil {
+		if dm.getChartDownloaderCallback == nil {
 			err = fmt.Errorf("no chart repository for URL '%s'", nUrl)
 			return
 		}
-		if dm.repositories == nil {
+
-			dm.repositories = map[string]*repository.ChartRepository{}
+		if dm.downloaders == nil {
+			dm.downloaders = map[string]repository.Downloader{}
 		}
-		if dm.repositories[nUrl], err = dm.getRepositoryCallback(nUrl); err != nil {
+
+		if dm.downloaders[nUrl], err = dm.getChartDownloaderCallback(nUrl); err != nil {
 			err = fmt.Errorf("failed to get chart repository for URL '%s': %w", nUrl, err)
 			return
 		}
 	}
-	return dm.repositories[nUrl], nil
+	return dm.downloaders[nUrl], nil
 }
 
 // secureLocalChartPath returns the secure absolute path of a local dependency.

internal/helm/chart/dependency_manager_test.go

@@ -21,6 +21,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"net/url"
 	"os"
 	"path/filepath"
 	"sync"
@@ -29,12 +30,38 @@ import (
 	. "github.com/onsi/gomega"
 	helmchart "helm.sh/helm/v3/pkg/chart"
 	helmgetter "helm.sh/helm/v3/pkg/getter"
+	"helm.sh/helm/v3/pkg/registry"
 	"helm.sh/helm/v3/pkg/repo"
 
 	"github.com/fluxcd/source-controller/internal/helm/chart/secureloader"
 	"github.com/fluxcd/source-controller/internal/helm/repository"
 )
 
+type mockTagsGetter struct {
+	tags map[string][]string
+}
+
+func (m *mockTagsGetter) Tags(requestURL string) ([]string, error) {
+	u, err := url.Parse(requestURL)
+	if err != nil {
+		return nil, err
+	}
+
+	name := filepath.Base(u.Path)
+	if tags, ok := m.tags[name]; ok {
+		return tags, nil
+	}
+	return nil, fmt.Errorf("no tags found for %s with requestURL %s", name, requestURL)
+}
+
+func (m *mockTagsGetter) Login(_ string, _ ...registry.LoginOption) error {
+	return nil
+}
+
+func (m *mockTagsGetter) Logout(_ string, _ ...registry.LogoutOption) error {
+	return nil
+}
+
 // mockGetter is a simple mocking getter.Getter implementation, returning
 // a byte response to any provided URL.
 type mockGetter struct {
@@ -49,25 +76,42 @@ func (g *mockGetter) Get(_ string, _ ...helmgetter.Option) (*bytes.Buffer, error
 func TestDependencyManager_Clear(t *testing.T) {
 	g := NewWithT(t)
 
-	repos := map[string]*repository.ChartRepository{
+	file, err := os.CreateTemp("", "")
-		"with index": {
+	g.Expect(err).ToNot(HaveOccurred())
+	ociRepoWithCreds, err := repository.NewOCIChartRepository("oci://example.com", repository.WithCredentialsFile(file.Name()))
+	g.Expect(err).ToNot(HaveOccurred())
+
+	downloaders := map[string]repository.Downloader{
+		"with index": &repository.ChartRepository{
 			Index:   repo.NewIndexFile(),
 			RWMutex: &sync.RWMutex{},
 		},
-		"cached cache path": {
+		"cached cache path": &repository.ChartRepository{
 			CachePath: "/invalid/path/resets",
 			Cached:    true,
 			RWMutex:   &sync.RWMutex{},
 		},
+		"with credentials":    ociRepoWithCreds,
+		"without credentials": &repository.OCIChartRepository{},
 	}
 
-	dm := NewDependencyManager(WithRepositories(repos))
+	dm := NewDependencyManager(WithRepositories(downloaders))
 	g.Expect(dm.Clear()).To(BeNil())
-	g.Expect(dm.repositories).To(HaveLen(len(repos)))
+	g.Expect(dm.downloaders).To(HaveLen(len(downloaders)))
-	for _, v := range repos {
+	for _, v := range downloaders {
-		g.Expect(v.Index).To(BeNil())
+		switch v := v.(type) {
-		g.Expect(v.CachePath).To(BeEmpty())
+		case *repository.ChartRepository:
-		g.Expect(v.Cached).To(BeFalse())
+			g.Expect(v.Index).To(BeNil())
+			g.Expect(v.CachePath).To(BeEmpty())
+			g.Expect(v.Cached).To(BeFalse())
+		case *repository.OCIChartRepository:
+			g.Expect(v.HasCredentials()).To(BeFalse())
+		}
+	}
+
+	if _, err := os.Stat(file.Name()); !errors.Is(err, os.ErrNotExist) {
+		err = os.Remove(file.Name())
+		g.Expect(err).ToNot(HaveOccurred())
 	}
 }
 
@@ -80,8 +124,22 @@ func TestDependencyManager_Build(t *testing.T) {
 	g.Expect(err).ToNot(HaveOccurred())
 	g.Expect(chartGrafana).ToNot(BeEmpty())
 
-	mockRepo := func() *repository.ChartRepository {
+	mockrepos := []repository.Downloader{
-		return &repository.ChartRepository{
+		&repository.OCIChartRepository{
+			URL: url.URL{
+				Scheme: "oci",
+				Host:   "example.com",
+			},
+			Client: &mockGetter{
+				Response: chartGrafana,
+			},
+			RegistryClient: &mockTagsGetter{
+				tags: map[string][]string{
+					"grafana": {"6.17.4"},
+				},
+			},
+		},
+		&repository.ChartRepository{
 			Client: &mockGetter{
 				Response: chartGrafana,
 			},
@@ -99,15 +157,21 @@ func TestDependencyManager_Build(t *testing.T) {
 				},
 			},
 			RWMutex: &sync.RWMutex{},
-		}
+		},
 	}
 
+	for _, repo := range mockrepos {
+		build(t, repo)
+	}
+}
+
+func build(t *testing.T, mockRepo repository.Downloader) {
 	tests := []struct {
 		name                       string
 		baseDir                    string
 		path                       string
-		repositories               map[string]*repository.ChartRepository
+		downloaders                map[string]repository.Downloader
-		getChartRepositoryCallback GetChartRepositoryCallback
+		getChartDownloaderCallback GetChartDownloaderCallback
 		want                       int
 		wantChartFunc              func(g *WithT, c *helmchart.Chart)
 		wantErr                    string
@@ -140,10 +204,10 @@ func TestDependencyManager_Build(t *testing.T) {
 			name:    "build with dependencies using lock file",
 			baseDir: "./../testdata/charts",
 			path:    "helmchartwithdeps",
-			repositories: map[string]*repository.ChartRepository{
+			downloaders: map[string]repository.Downloader{
-				"https://grafana.github.io/helm-charts/": mockRepo(),
+				"https://grafana.github.io/helm-charts/": mockRepo,
 			},
-			getChartRepositoryCallback: func(url string) (*repository.ChartRepository, error) {
+			getChartDownloaderCallback: func(url string) (repository.Downloader, error) {
 				return &repository.ChartRepository{URL: "https://grafana.github.io/helm-charts/"}, nil
 			},
 			wantChartFunc: func(g *WithT, c *helmchart.Chart) {
@@ -170,8 +234,8 @@ func TestDependencyManager_Build(t *testing.T) {
 			g.Expect(err).ToNot(HaveOccurred())
 
 			dm := NewDependencyManager(
-				WithRepositories(tt.repositories),
+				WithRepositories(tt.downloaders),
-				WithRepositoryCallback(tt.getChartRepositoryCallback),
+				WithDownloaderCallback(tt.getChartDownloaderCallback),
 			)
 			absBaseDir, err := filepath.Abs(tt.baseDir)
 			g.Expect(err).ToNot(HaveOccurred())
@@ -319,16 +383,16 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
 	g.Expect(chartB).ToNot(BeEmpty())
 
 	tests := []struct {
-		name         string
+		name        string
-		repositories map[string]*repository.ChartRepository
+		downloaders map[string]repository.Downloader
-		dep          *helmchart.Dependency
+		dep         *helmchart.Dependency
-		wantFunc     func(g *WithT, c *helmchart.Chart)
+		wantFunc    func(g *WithT, c *helmchart.Chart)
-		wantErr      string
+		wantErr     string
 	}{
 		{
 			name: "adds remote dependency",
-			repositories: map[string]*repository.ChartRepository{
+			downloaders: map[string]repository.Downloader{
-				"https://example.com/": {
+				"https://example.com/": &repository.ChartRepository{
 					Client: &mockGetter{
 						Response: chartB,
 					},
@@ -357,8 +421,8 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
 			},
 		},
 		{
-			name:         "resolve repository error",
+			name:        "resolve repository error",
-			repositories: map[string]*repository.ChartRepository{},
+			downloaders: map[string]repository.Downloader{},
 			dep: &helmchart.Dependency{
 				Repository: "https://example.com",
 			},
@@ -366,8 +430,8 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
 		},
 		{
 			name: "strategic load error",
-			repositories: map[string]*repository.ChartRepository{
+			downloaders: map[string]repository.Downloader{
-				"https://example.com/": {
+				"https://example.com/": &repository.ChartRepository{
 					CachePath: "/invalid/cache/path/foo",
 					RWMutex:   &sync.RWMutex{},
 				},
@@ -379,8 +443,8 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
 		},
 		{
 			name: "repository get error",
-			repositories: map[string]*repository.ChartRepository{
+			downloaders: map[string]repository.Downloader{
-				"https://example.com/": {
+				"https://example.com/": &repository.ChartRepository{
 					Index:   &repo.IndexFile{},
 					RWMutex: &sync.RWMutex{},
 				},
@@ -392,8 +456,8 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
 		},
 		{
 			name: "repository version constraint error",
-			repositories: map[string]*repository.ChartRepository{
+			downloaders: map[string]repository.Downloader{
-				"https://example.com/": {
+				"https://example.com/": &repository.ChartRepository{
 					Index: &repo.IndexFile{
 						Entries: map[string]repo.ChartVersions{
 							chartName: {
@@ -418,8 +482,8 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
 		},
 		{
 			name: "repository chart download error",
-			repositories: map[string]*repository.ChartRepository{
+			downloaders: map[string]repository.Downloader{
-				"https://example.com/": {
+				"https://example.com/": &repository.ChartRepository{
 					Index: &repo.IndexFile{
 						Entries: map[string]repo.ChartVersions{
 							chartName: {
@@ -444,8 +508,8 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
 		},
 		{
 			name: "chart load error",
-			repositories: map[string]*repository.ChartRepository{
+			downloaders: map[string]repository.Downloader{
-				"https://example.com/": {
+				"https://example.com/": &repository.ChartRepository{
 					Client: &mockGetter{},
 					Index: &repo.IndexFile{
 						Entries: map[string]repo.ChartVersions{
@@ -476,7 +540,137 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
 			g := NewWithT(t)
 
 			dm := &DependencyManager{
-				repositories: tt.repositories,
+				downloaders: tt.downloaders,
+			}
+			chart := &helmchart.Chart{}
+			err := dm.addRemoteDependency(&chartWithLock{Chart: chart}, tt.dep)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				return
+			}
+			g.Expect(err).ToNot(HaveOccurred())
+			if tt.wantFunc != nil {
+				tt.wantFunc(g, chart)
+			}
+		})
+	}
+}
+
+func TestDependencyManager_addRemoteOCIDependency(t *testing.T) {
+	g := NewWithT(t)
+
+	chartB, err := os.ReadFile("../testdata/charts/helmchart-0.1.0.tgz")
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(chartB).ToNot(BeEmpty())
+
+	tests := []struct {
+		name        string
+		downloaders map[string]repository.Downloader
+		dep         *helmchart.Dependency
+		wantFunc    func(g *WithT, c *helmchart.Chart)
+		wantErr     string
+	}{
+		{
+			name: "adds remote oci dependency",
+			downloaders: map[string]repository.Downloader{
+				"oci://example.com": &repository.OCIChartRepository{
+					URL: url.URL{
+						Scheme: "oci",
+						Host:   "example.com",
+					},
+					Client: &mockGetter{
+						Response: chartB,
+					},
+					RegistryClient: &mockTagsGetter{
+						tags: map[string][]string{
+							"helmchart": {"0.1.0"},
+						},
+					},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Repository: "oci://example.com",
+			},
+			wantFunc: func(g *WithT, c *helmchart.Chart) {
+				g.Expect(c.Dependencies()).To(HaveLen(1))
+			},
+		},
+		{
+			name: "remote oci repository fetch tags error",
+			downloaders: map[string]repository.Downloader{
+				"oci://example.com": &repository.OCIChartRepository{
+					URL: url.URL{
+						Scheme: "oci",
+						Host:   "example.com",
+					},
+					RegistryClient: &mockTagsGetter{
+						tags: map[string][]string{},
+					},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Repository: "oci://example.com",
+			},
+			wantErr: fmt.Sprintf("no tags found for %s", chartName),
+		},
+		{
+			name: "remote oci repository version constraint error",
+			downloaders: map[string]repository.Downloader{
+				"oci://example.com": &repository.OCIChartRepository{
+					URL: url.URL{
+						Scheme: "oci",
+						Host:   "example.com",
+					},
+					Client: &mockGetter{
+						Response: chartB,
+					},
+					RegistryClient: &mockTagsGetter{
+						tags: map[string][]string{
+							"helmchart": {"0.1.0"},
+						},
+					},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    "0.2.0",
+				Repository: "oci://example.com",
+			},
+			wantErr: "could not locate a version matching provided version string 0.2.0",
+		},
+		{
+			name: "chart load error",
+			downloaders: map[string]repository.Downloader{
+				"oci://example.com": &repository.OCIChartRepository{
+					URL: url.URL{
+						Scheme: "oci",
+						Host:   "example.com",
+					},
+					Client: &mockGetter{},
+					RegistryClient: &mockTagsGetter{
+						tags: map[string][]string{
+							"helmchart": {"0.1.0"},
+						},
+					},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    chartVersion,
+				Repository: "oci://example.com",
+			},
+			wantErr: "failed to load downloaded archive of version '0.1.0'",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			dm := &DependencyManager{
+				downloaders: tt.downloaders,
 			}
 			chart := &helmchart.Chart{}
 			err := dm.addRemoteDependency(&chartWithLock{Chart: chart}, tt.dep)
@@ -496,54 +690,98 @@ func TestDependencyManager_addRemoteDependency(t *testing.T) {
 func TestDependencyManager_resolveRepository(t *testing.T) {
 	tests := []struct {
 		name                       string
-		repositories               map[string]*repository.ChartRepository
+		downloaders                map[string]repository.Downloader
-		getChartRepositoryCallback GetChartRepositoryCallback
+		getChartDownloaderCallback GetChartDownloaderCallback
 		url                        string
-		want                       *repository.ChartRepository
+		want                       repository.Downloader
-		wantRepositories           map[string]*repository.ChartRepository
+		wantDownloaders            map[string]repository.Downloader
 		wantErr                    string
 	}{
 		{
-			name: "resolves from repositories index",
+			name: "resolves from downloaders index",
 			url:  "https://example.com",
-			repositories: map[string]*repository.ChartRepository{
+			downloaders: map[string]repository.Downloader{
-				"https://example.com/": {URL: "https://example.com"},
+				"https://example.com/": &repository.ChartRepository{URL: "https://example.com"},
 			},
 			want: &repository.ChartRepository{URL: "https://example.com"},
 		},
 		{
 			name: "resolves from callback",
 			url:  "https://example.com",
-			getChartRepositoryCallback: func(url string) (*repository.ChartRepository, error) {
+			getChartDownloaderCallback: func(_ string) (repository.Downloader, error) {
 				return &repository.ChartRepository{URL: "https://example.com"}, nil
 			},
 			want: &repository.ChartRepository{URL: "https://example.com"},
-			wantRepositories: map[string]*repository.ChartRepository{
+			wantDownloaders: map[string]repository.Downloader{
-				"https://example.com/": {URL: "https://example.com"},
+				"https://example.com/": &repository.ChartRepository{URL: "https://example.com"},
 			},
 		},
 		{
 			name: "error from callback",
 			url:  "https://example.com",
-			getChartRepositoryCallback: func(url string) (*repository.ChartRepository, error) {
+			getChartDownloaderCallback: func(_ string) (repository.Downloader, error) {
 				return nil, errors.New("a very unique error")
 			},
-			wantErr:          "a very unique error",
+			wantErr:         "a very unique error",
-			wantRepositories: map[string]*repository.ChartRepository{},
+			wantDownloaders: map[string]repository.Downloader{},
 		},
 		{
 			name:    "error on not found",
 			url:     "https://example.com",
 			wantErr: "no chart repository for URL",
 		},
+		{
+			name: "resolves from oci repository",
+			url:  "oci://example.com",
+			downloaders: map[string]repository.Downloader{
+				"oci://example.com": &repository.OCIChartRepository{
+					URL: url.URL{
+						Scheme: "oci",
+						Host:   "example.com",
+					},
+				},
+			},
+			want: &repository.OCIChartRepository{
+				URL: url.URL{
+					Scheme: "oci",
+					Host:   "example.com",
+				},
+			},
+		},
+		{
+			name: "resolves oci repository from callback",
+			url:  "oci://example.com",
+			getChartDownloaderCallback: func(_ string) (repository.Downloader, error) {
+				return &repository.OCIChartRepository{
+					URL: url.URL{
+						Scheme: "oci",
+						Host:   "example.com"},
+				}, nil
+			},
+			want: &repository.OCIChartRepository{
+				URL: url.URL{
+					Scheme: "oci",
+					Host:   "example.com",
+				},
+			},
+
+			wantDownloaders: map[string]repository.Downloader{
+				"oci://example.com": &repository.OCIChartRepository{
+					URL: url.URL{
+						Scheme: "oci",
+						Host:   "example.com",
+					},
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
 			dm := &DependencyManager{
-				repositories:          tt.repositories,
+				downloaders:                tt.downloaders,
-				getRepositoryCallback: tt.getChartRepositoryCallback,
+				getChartDownloaderCallback: tt.getChartDownloaderCallback,
 			}
 
 			got, err := dm.resolveRepository(tt.url)
@@ -556,8 +794,8 @@ func TestDependencyManager_resolveRepository(t *testing.T) {
 
 			g.Expect(err).ToNot(HaveOccurred())
 			g.Expect(got).To(Equal(tt.want))
-			if tt.wantRepositories != nil {
+			if tt.wantDownloaders != nil {
-				g.Expect(dm.repositories).To(Equal(tt.wantRepositories))
+				g.Expect(dm.downloaders).To(Equal(tt.wantDownloaders))
 			}
 		})
 	}

internal/helm/registry/client.go

@@ -21,6 +21,7 @@ import (
 	"os"
 
 	"helm.sh/helm/v3/pkg/registry"
+	"k8s.io/apimachinery/pkg/util/errors"
 )
 
 // ClientGenerator generates a registry client and a temporary credential file.
@@ -30,16 +31,25 @@ func ClientGenerator(isLogin bool) (*registry.Client, string, error) {
 	if isLogin {
 		// create a temporary file to store the credentials
 		// this is needed because otherwise the credentials are stored in ~/.docker/config.json.
-		credentialFile, err := os.CreateTemp("", "credentials")
+		credentialsFile, err := os.CreateTemp("", "credentials")
 		if err != nil {
 			return nil, "", err
 		}
 
-		rClient, err := registry.NewClient(registry.ClientOptWriter(io.Discard), registry.ClientOptCredentialsFile(credentialFile.Name()))
+		var errs []error
+		rClient, err := registry.NewClient(registry.ClientOptWriter(io.Discard), registry.ClientOptCredentialsFile(credentialsFile.Name()))
 		if err != nil {
-			return nil, "", err
+			errs = append(errs, err)
+			// attempt to delete the temporary file
+			if credentialsFile != nil {
+				err := os.Remove(credentialsFile.Name())
+				if err != nil {
+					errs = append(errs, err)
+				}
+			}
+			return nil, "", errors.NewAggregate(errs)
 		}
-		return rClient, credentialFile.Name(), nil
+		return rClient, credentialsFile.Name(), nil
 	}
 
 	rClient, err := registry.NewClient(registry.ClientOptWriter(io.Discard))

internal/helm/repository/chart_repository.go

@@ -35,6 +35,7 @@ import (
 	"github.com/Masterminds/semver/v3"
 	"helm.sh/helm/v3/pkg/getter"
 	"helm.sh/helm/v3/pkg/repo"
+	kerrors "k8s.io/apimachinery/pkg/util/errors"
 	"sigs.k8s.io/yaml"
 
 	"github.com/fluxcd/pkg/version"
@@ -476,9 +477,10 @@ func (r *ChartRepository) Unload() {
 	r.Index = nil
 }
 
-// Clear cache the index in memory before unloading it.
+// Clear caches the index in memory before unloading it.
 // It cleans up temporary files and directories created by the repository.
-func (r *ChartRepository) Clear() (errs []error) {
+func (r *ChartRepository) Clear() error {
+	var errs []error
 	if err := r.CacheIndexInMemory(); err != nil {
 		errs = append(errs, err)
 	}
@@ -489,7 +491,7 @@ func (r *ChartRepository) Clear() (errs []error) {
 		errs = append(errs, err)
 	}
 
-	return
+	return kerrors.NewAggregate(errs)
 }
 
 // SetMemCache sets the cache to use for this repository.

internal/helm/repository/oci_chart_repository.go

@@ -21,6 +21,7 @@ import (
 	"crypto/tls"
 	"fmt"
 	"net/url"
+	"os"
 	"path"
 	"sort"
 	"strings"
@@ -60,6 +61,8 @@ type OCIChartRepository struct {
 
 	// RegistryClient is a client to use while downloading tags or charts from a registry.
 	RegistryClient RegistryClient
+	// credentialsFile is a temporary credentials file to use while downloading tags or charts from a registry.
+	credentialsFile string
 }
 
 // OCIChartRepositoryOption is a function that can be passed to NewOCIChartRepository
@@ -94,6 +97,14 @@ func WithOCIGetterOptions(getterOpts []getter.Option) OCIChartRepositoryOption {
 	}
 }
 
+// WithCredentialsFile returns a ChartRepositoryOption that will set the credentials file
+func WithCredentialsFile(credentialsFile string) OCIChartRepositoryOption {
+	return func(r *OCIChartRepository) error {
+		r.credentialsFile = credentialsFile
+		return nil
+	}
+}
+
 // NewOCIChartRepository constructs and returns a new ChartRepository with
 // the ChartRepository.Client configured to the getter.Getter for the
 // repository URL scheme. It returns an error on URL parsing failures.
@@ -126,7 +137,7 @@ func (r *OCIChartRepository) GetChartVersion(name, ver string) (*repo.ChartVersi
 	cpURL.Path = path.Join(cpURL.Path, name)
 	cvs, err := r.getTags(cpURL.String())
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("could not get tags for %q: %s", name, err)
 	}
 
 	if len(cvs) == 0 {
@@ -153,7 +164,7 @@ func (r *OCIChartRepository) getTags(ref string) ([]string, error) {
 	// Retrieve list of repository tags
 	tags, err := r.RegistryClient.Tags(strings.TrimPrefix(ref, fmt.Sprintf("%s://", registry.OCIScheme)))
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("could not fetch tags for %q: %s", ref, err)
 	}
 	if len(tags) == 0 {
 		return nil, fmt.Errorf("unable to locate any tags in provided repository: %s", ref)
@@ -206,6 +217,23 @@ func (r *OCIChartRepository) Logout() error {
 	return nil
 }
 
+// HasCredentials returns true if the OCIChartRepository has credentials.
+func (r *OCIChartRepository) HasCredentials() bool {
+	return r.credentialsFile != ""
+}
+
+// Clear deletes the OCI registry credentials file.
+func (r *OCIChartRepository) Clear() error {
+	// clean the credentials file if it exists
+	if r.credentialsFile != "" {
+		if err := os.Remove(r.credentialsFile); err != nil {
+			return err
+		}
+	}
+	r.credentialsFile = ""
+	return nil
+}
+
 // getLastMatchingVersionOrConstraint returns the last version that matches the given version string.
 // If the version string is empty, the highest available version is returned.
 func getLastMatchingVersionOrConstraint(cvs []string, ver string) (string, error) {

internal/helm/repository/repository.go

@@ -0,0 +1,35 @@
+/*
+Copyright 2022 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package repository
+
+import (
+	"bytes"
+
+	"helm.sh/helm/v3/pkg/repo"
+)
+
+// Downloader is used to download a chart from a remote Helm repository or OCI Helm repository.
+type Downloader interface {
+	// GetChartVersion returns the repo.ChartVersion for the given name and version
+	// from the remote Helm repository or OCI Helm repository.
+	GetChartVersion(name, version string) (*repo.ChartVersion, error)
+	// DownloadChart downloads a chart from the remote Helm repository or OCI Helm repository.
+	DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer, error)
+	// Clear removes all temporary files created by the downloader, caching the files if the cache is configured,
+	// and calling garbage collector to remove unused files.
+	Clear() error
+}

internal/helm/repository/utils.go

@@ -18,13 +18,20 @@ package repository
 
 import (
 	"strings"
+
+	helmreg "helm.sh/helm/v3/pkg/registry"
 )
 
-// NormalizeURL normalizes a ChartRepository URL by ensuring it ends with a
+// NormalizeURL normalizes a ChartRepository URL by its scheme.
-// single "/".
+func NormalizeURL(repositoryURL string) string {
-func NormalizeURL(url string) string {
+	if repositoryURL == "" {
-	if url != "" {
+		return ""
-		return strings.TrimRight(url, "/") + "/"
 	}
-	return url
+
+	if strings.Contains(repositoryURL, helmreg.OCIScheme) {
+		return strings.TrimRight(repositoryURL, "/")
+	}
+
+	return strings.TrimRight(repositoryURL, "/") + "/"
+
 }

internal/helm/repository/utils_test.go

@@ -48,6 +48,16 @@ func TestNormalizeURL(t *testing.T) {
 			url:  "",
 			want: "",
 		},
+		{
+			name: "oci with slash",
+			url:  "oci://example.com/",
+			want: "oci://example.com",
+		},
+		{
+			name: "oci double slash",
+			url:  "oci://example.com//",
+			want: "oci://example.com",
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {