Enable multiple remote dependency

Signed-off-by: Soule BA <[email protected]>

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.