Enable multiple remote dependency

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

Author: souleb

controllers/helmchart_controller.go

@@ -510,7 +510,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 	}
 
 	// Initialize the chart repository
-	var chartRepo chart.Repository
+	var chartRepo chart.Downloader
 	switch repo.Spec.Type {
 	case sourcev1.HelmRepositoryTypeOCI:
 		if !helmreg.IsOCI(repo.Spec.URL) {
@@ -676,7 +676,7 @@ 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()
 
@@ -905,11 +905,11 @@ 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,
 // or a shim with defaults if no object could be found.
-func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartRepositoryCallback {
-	return func(url string) (*repository.ChartRepository, error) {
+func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
+	return func(url string) (chart.CleanDownloader, error) {
 		var tlsConfig *tls.Config
 		repo, err := r.resolveDependencyRepository(ctx, url, namespace)
 		if err != nil {

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]CleanDownloader
 		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]CleanDownloader{
 				"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]CleanDownloader{
 				"https://grafana.github.io/helm-charts/": mockRepo(),
 			},
 			dependentChartPaths: []string{"../testdata/charts/helmchart-v1"},

internal/helm/chart/builder_remote.go

@@ -36,22 +36,22 @@ import (
 	"github.com/fluxcd/source-controller/internal/helm/chart/secureloader"
 )
 
-// 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.
+// Downloader is used to download a chart from a remote Helm repository or OCI registry.
+type Downloader interface {
+	// GetChartVersion returns the repo.ChartVersion for the given name and version
+	// from the remote repository.ChartRepository.
 	GetChartVersion(name, version string) (*repo.ChartVersion, error)
-	// GetChartVersion returns a chart.ChartVersion from the remote repository.
+	// DownloadChart downloads a chart from the remote Helm repository or OCI registry.
 	DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer, error)
 }
 
 type remoteChartBuilder struct {
-	remote Repository
+	remote Downloader
 }
 
 // NewRemoteBuilder returns a Builder capable of building a Helm
-// chart with a RemoteReference in the given repository.ChartRepository.
-func NewRemoteBuilder(repository Repository) Builder {
+// chart with a RemoteReference in the given Downloader.
+func NewRemoteBuilder(repository Downloader) Builder {
 	return &remoteChartBuilder{
 		remote: repository,
 	}
@@ -132,7 +132,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 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

@@ -35,21 +35,29 @@ import (
 	"github.com/fluxcd/source-controller/internal/helm/repository"
 )
 
-// GetChartRepositoryCallback must return a repository.ChartRepository for the
+// CleanDownloader is a Downloader that cleans temporary files created by the downloader,
+// caching the files if the cache is configured,
+// and calling gc to remove unused files.
+type CleanDownloader interface {
+	Clean() []error
+	Downloader
+}
+
+// GetChartDownloaderCallback must return a Downloader for the
 // URL, or an error describing why it could not be returned.
-type GetChartRepositoryCallback func(url string) (*repository.ChartRepository, error)
+type GetChartDownloaderCallback func(url string) (CleanDownloader, 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]CleanDownloader
 
-	// getRepositoryCallback can be set to an on-demand GetChartRepositoryCallback
-	// whose returned result is cached to repositories.
-	getRepositoryCallback GetChartRepositoryCallback
+	// getChartDownloaderCallback can be set to an on-demand GetChartDownloaderCallback
+	// whose returned result is cached to downloaders.
+	getChartDownloaderCallback GetChartDownloaderCallback
 
 	// concurrent is the number of concurrent chart-add operations during
 	// Build. Defaults to 1 (non-concurrent).
@@ -64,16 +72,16 @@ type DependencyManagerOption interface {
 	applyToDependencyManager(dm *DependencyManager)
 }
 
-type WithRepositories map[string]*repository.ChartRepository
+type WithRepositories map[string]CleanDownloader
 
 func (o WithRepositories) applyToDependencyManager(dm *DependencyManager) {
-	dm.repositories = o
+	dm.downloaders = o
 }
 
-type WithRepositoryCallback GetChartRepositoryCallback
+type WithDownloaderCallback GetChartDownloaderCallback
 
-func (o WithRepositoryCallback) applyToDependencyManager(dm *DependencyManager) {
-	dm.getRepositoryCallback = GetChartRepositoryCallback(o)
+func (o WithDownloaderCallback) applyToDependencyManager(dm *DependencyManager) {
+	dm.getChartDownloaderCallback = GetChartDownloaderCallback(o)
 }
 
 type WithConcurrent int64
@@ -92,18 +100,12 @@ func NewDependencyManager(opts ...DependencyManagerOption) *DependencyManager {
 	return dm
 }
 
-// Clear iterates over the repositories, calling Unload and RemoveCache on all
-// items. It returns a collection of (cache removal) errors.
+// Clear iterates over the downloaders, calling Clean on all
+// items. It returns a collection of errors.
 func (dm *DependencyManager) Clear() []error {
 	var errs []error
-	for _, v := range dm.repositories {
-		if err := v.CacheIndexInMemory(); err != nil {
-			errs = append(errs, err)
-		}
-		v.Unload()
-		if err := v.RemoveCache(); err != nil {
-			errs = append(errs, err)
-		}
+	for _, v := range dm.downloaders {
+		errs = append(errs, v.Clean()...)
 	}
 	return errs
 }
@@ -236,10 +238,6 @@ 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
@@ -259,27 +257,27 @@ func (dm *DependencyManager) addRemoteDependency(chart *chartWithLock, dep *helm
 	return nil
 }
 
-// resolveRepository first attempts to resolve the url from the repositories, falling back
-// to getRepositoryCallback if set. It returns the resolved Index, or an error.
-func (dm *DependencyManager) resolveRepository(url string) (_ *repository.ChartRepository, err error) {
+// resolveRepository first attempts to resolve the url from the downloaders, falling back
+// to getDownloaderCallback if set. It returns the resolved Index, or an error.
+func (dm *DependencyManager) resolveRepository(url string) (_ Downloader, err error) {
 	dm.mu.Lock()
 	defer dm.mu.Unlock()
 
 	nUrl := repository.NormalizeURL(url)
-	if _, ok := dm.repositories[nUrl]; !ok {
-		if dm.getRepositoryCallback == nil {
+	if _, ok := dm.downloaders[nUrl]; !ok {
+		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]CleanDownloader{}
 		}
-		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.