Enable multiple remote dependency

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

controllers/helmchart_controller.go

@@ -54,6 +54,7 @@ import (
  "github.com/fluxcd/pkg/runtime/patch"
  "github.com/fluxcd/pkg/runtime/predicates"
  "github.com/fluxcd/pkg/untar"
+ "github.com/hashicorp/go-multierror"
 
  sourcev1 "github.com/fluxcd/source-controller/api/v1beta2"
  "github.com/fluxcd/source-controller/internal/cache"
@@ -510,7 +511,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 +677,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,17 +906,21 @@ 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) {
- var tlsConfig *tls.Config
+func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
+ return func(url string) (chart.CleanDownloader, string, error) {
+ var (
+ tlsConfig *tls.Config
+ loginOpts []helmreg.LoginOption
+ credentialFile string
+ )
  repo, err := r.resolveDependencyRepository(ctx, url, namespace)
  if err != nil {
  // Return Kubernetes client errors, but ignore others
  if apierrs.ReasonForError(err) != metav1.StatusReasonUnknown {
- return nil, err
+ return nil, "", err
  }
  repo = &sourcev1.HelmRepository{
  Spec: sourcev1.HelmRepositorySpec{
@@ -931,35 +936,79 @@ func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Cont
  }
  if secret, err := r.getHelmRepositorySecret(ctx, repo); secret != nil || err != nil {
  if err != nil {
- return nil, err
+ return nil, "", err
  }
  opts, err := getter.ClientOptionsFromSecret(*secret)
  if err != nil {
- return nil, err
+ return nil, "", err
  }
  clientOpts = append(clientOpts, opts...)
 
  tlsConfig, err = getter.TLSClientConfigFromSecret(*secret, repo.Spec.URL)
  if err != nil {
- return nil, fmt.Errorf("failed to create TLS client config for HelmRepository '%s': %w", repo.Name, err)
+ return nil, "", fmt.Errorf("failed to create TLS client config for HelmRepository '%s': %w", repo.Name, err)
  }
- }
 
- chartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", r.Getters, tlsConfig, clientOpts)
- if err != nil {
- return nil, err
+ // Build registryClient options from secret
+ loginOpt, err := registry.LoginOptionFromSecret(repo.Spec.URL, *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)
  }
 
- // 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)
- })
+ var chartRepo chart.CleanDownloader
+ if helmreg.IsOCI(repo.Spec.URL) {
+ registryClient, file, 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(repo.Spec.URL, repository.WithOCIGetter(r.Getters), repository.WithOCIGetterOptions(clientOpts), repository.WithOCIRegistryClient(registryClient))
+ if err != nil {
+ // clean up the file
+ if err := os.Remove(file); err != nil {
+ errs = multierror.Append(errs, err)
+ }
+ errs = multierror.Append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", repo.Name, err))
+ return nil, "", 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 {
+ return nil, "", fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", repo.Name, err)
+ }
+ }
+
+ credentialFile = file
+ chartRepo = ociChartRepo
+ } else {
+ httpChartRepo, err := repository.NewChartRepository(repo.Spec.URL, "", 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
  }
- return chartRepo, nil
+
+ return chartRepo, credentialFile, nil
  }
 }
 

go.mod

@@ -132,7 +132,9 @@ require (
  github.com/gorilla/mux v1.8.0 // indirect
  github.com/gosuri/uitable v0.0.4 // indirect
  github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 // indirect
+ github.com/hashicorp/errwrap v1.1.0 // indirect
  github.com/hashicorp/go-cleanhttp v0.5.2 // indirect
+ github.com/hashicorp/go-multierror v1.1.1 // indirect
  github.com/hashicorp/go-retryablehttp v0.7.1 // indirect
  github.com/huandu/xstrings v1.3.2 // indirect
  github.com/imdario/mergo v0.3.12 // indirect

go.sum

@@ -489,6 +489,8 @@ github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFb
 github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q=
 github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8=
 github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
+github.com/hashicorp/errwrap v1.1.0 h1:OxrOeh75EUXMY8TBjag2fzXGZ40LB6IKw45YeGUDY2I=
+github.com/hashicorp/errwrap v1.1.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4=
 github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80=
 github.com/hashicorp/go-cleanhttp v0.5.2 h1:035FKYIWjmULyFRBKPs8TBQoi0x6d9G4xc9neXJWAZQ=
 github.com/hashicorp/go-cleanhttp v0.5.2/go.mod h1:kO/YDlP8L1346E6Sodw+PrpBSV4/SoxCXGY6BqNFT48=
@@ -497,6 +499,8 @@ github.com/hashicorp/go-hclog v0.9.2/go.mod h1:5CU+agLiy3J7N7QjHK5d05KxGsuXiQLrj
 github.com/hashicorp/go-immutable-radix v1.0.0/go.mod h1:0y9vanUI8NX6FsYoO3zeMjhV/C5i9g4Q3DwcSNZ4P60=
 github.com/hashicorp/go-msgpack v0.5.3/go.mod h1:ahLV/dePpqEmjfWmKiqvPkv/twdG7iPBM1vqhUKIvfM=
 github.com/hashicorp/go-multierror v1.0.0/go.mod h1:dHtQlpGsu+cZNNAkkCN/P3hoUDHhCYQXV3UM06sGGrk=
+github.com/hashicorp/go-multierror v1.1.1 h1:H5DkEtf6CXdFp0N0Em5UCwQpXMWke8IA0+lD48awMYo=
+github.com/hashicorp/go-multierror v1.1.1/go.mod h1:iw975J/qwKPdAO1clOe2L8331t/9/fmwbPZ6JB6eMoM=
 github.com/hashicorp/go-retryablehttp v0.7.1 h1:sUiuQAnLlbvmExtFQs72iFW/HXeUn8Z1aJLQ4LJJbTQ=
 github.com/hashicorp/go-retryablehttp v0.7.1/go.mod h1:vAew36LZh98gCBJNLH42IQ1ER/9wtLZZ8meHqQvEYWY=
 github.com/hashicorp/go-rootcerts v1.0.0/go.mod h1:K6zTfqpRlCUIjkwsN4Z+hiSfzSTQa6eBIzfwKfwNnHU=

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,45 +35,57 @@ import (
  "github.com/fluxcd/source-controller/internal/helm/repository"
 )
 
-// GetChartRepositoryCallback must return a repository.ChartRepository for the
-// URL, or an error describing why it could not be returned.
-type GetChartRepositoryCallback func(url string) (*repository.ChartRepository, error)
+// 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 and an optional credential file name,
+// or an error describing why it could not be returned.
+type GetChartDownloaderCallback func(url string) (CleanDownloader, string, 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).
  concurrent int64
 
  // mu contains the lock for chart writes.
  mu sync.Mutex
+
+ // credentialFiles is a list of temporary credential files created by the downloaders.
+ credentialFiles map[string]string
 }
 
 // DependencyManagerOption configures an option on a DependencyManager.
 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 +104,21 @@ 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 removes all temporary files created by the downloaders.
+// 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 k, v := range dm.downloaders {
+ // clean the downloader temp credential file
+ if file, ok := dm.credentialFiles[k]; ok && file != "" {
+ if err := os.Remove(file); err != nil {
+ errs = append(errs, err)
+ }
  }
+
+ errs = append(errs, v.Clean()...)
  }
  return errs
 }
@@ -236,10 +251,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 +270,33 @@ 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.credentialFiles == nil {
+ dm.credentialFiles = map[string]string{}
  }
- if dm.repositories[nUrl], err = dm.getRepositoryCallback(nUrl); err != nil {
+
+ if dm.downloaders[nUrl], dm.credentialFiles[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.