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"
@@ -460,9 +461,10 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
  loginOpts []helmreg.LoginOption
  )
 
+ normalizedURL := strings.TrimSuffix(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),
  }
@@ -490,7 +492,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),
@@ -502,7 +504,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),
@@ -517,11 +519,11 @@ 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) {
- err := fmt.Errorf("invalid OCI registry URL: %s", repo.Spec.URL)
+ if !helmreg.IsOCI(normalizedURL) {
+ err := fmt.Errorf("invalid OCI registry URL: %s", normalizedURL)
  return chartRepoConfigErrorReturn(err, obj)
  }
 
@@ -550,7 +552,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)
  }
@@ -570,7 +572,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)
  }))
@@ -683,7 +685,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()
 
@@ -912,12 +914,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,
 // 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
+// The callback returns an object with a state, so the caller has to do the necessary cleanup.
+func (r *HelmChartReconciler) namespacedChartRepositoryCallback(ctx context.Context, name, namespace string) chart.GetChartDownloaderCallback {
+ return func(url string) (chart.CleanDownloader, error) {
+ var (
+ tlsConfig *tls.Config
+ loginOpts []helmreg.LoginOption
+ )
+ normalizedURL := strings.TrimSuffix(url, "/")
  repo, err := r.resolveDependencyRepository(ctx, url, namespace)
  if err != nil {
  // Return Kubernetes client errors, but ignore others
@@ -932,7 +939,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),
  }
@@ -946,26 +953,72 @@ 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)
  }
- }
 
- 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(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)
  }
 
- // 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(normalizedURL) {
+ 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(normalizedURL, repository.WithOCIGetter(r.Getters),
+ repository.WithOCIGetterOptions(clientOpts),
+ repository.WithOCIRegistryClient(registryClient),
+ repository.WithCredentialFile(file))
+ if err != nil {
+ // clean up the file
+ if err := os.Remove(file); err != nil {
+ errs = append(errs, err)
+ }
+ errs = append(errs, fmt.Errorf("failed to create OCI chart repository for HelmRepository '%s': %w", repo.Name, 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 {
+ return nil, fmt.Errorf("failed to login to OCI chart repository for HelmRepository '%s': %w", repo.Name, err)
+ }
+ }
+
+ 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
  }
+
  return chartRepo, nil
  }
 }

controllers/helmchart_controller_test.go

@@ -1070,7 +1070,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 +1178,11 @@ func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
  g := NewWithT(t)
 
  r := &HelmChartReconciler{
- Client: fake.NewClientBuilder().Build(),
- EventRecorder: record.NewFakeRecorder(32),
- Storage: storage,
- Getters: testGetters,
+ Client: fake.NewClientBuilder().Build(),
+ EventRecorder: record.NewFakeRecorder(32),
+ Storage: storage,
+ Getters: testGetters,
+ RegistryClientGenerator: registry.ClientGenerator,
  }
 
  obj := &sourcev1.HelmChart{

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]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 {