Refactor repository logic

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

controllers/helmchart_controller.go

@@ -518,7 +518,7 @@ func (r *HelmChartReconciler) buildFromHelmRepository(ctx context.Context, obj *
 	}
 
 	// Initialize the chart repository
-	var chartRepo chart.Remote
+	var chartRepo chart.Repository
 	switch repo.Spec.Type {
 	case sourcev1.HelmRepositoryTypeOCI:
 		if !helmreg.IsOCI(repo.Spec.URL) {

internal/helm/chart/builder_remote.go

@@ -25,7 +25,6 @@ import (
 	"path/filepath"
 
 	"github.com/Masterminds/semver/v3"
-	"github.com/fluxcd/source-controller/internal/helm/repository"
 	helmchart "helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/repo"
@@ -37,22 +36,22 @@ import (
 	"github.com/fluxcd/source-controller/internal/helm/chart/secureloader"
 )
 
-// Remote is a repository.ChartRepository or a repository.OCIChartRepository.
+// 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 Remote interface {
-	// GetChart returns a chart.Chart from the remote repository.
-	Get(name, version string) (*repo.ChartVersion, error)
+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 Remote
+	remote Repository
 }
 
 // NewRemoteBuilder returns a Builder capable of building a Helm
 // chart with a RemoteReference in the given repository.ChartRepository.
-func NewRemoteBuilder(repository Remote) Builder {
+func NewRemoteBuilder(repository Repository) Builder {
 	return &remoteChartBuilder{
 		remote: repository,
 	}
@@ -83,31 +82,12 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 		return nil, &BuildError{Reason: ErrChartReference, Err: err}
 	}
 
-	var (
-		res *bytes.Buffer
-		err error
-	)
-
-	result := &Build{}
-	switch b.remote.(type) {
-	case *repository.ChartRepository:
-		res, err = b.downloadFromRepository(b.remote.(*repository.ChartRepository), remoteRef, result, opts)
-		if err != nil {
-			return nil, &BuildError{Reason: ErrChartPull, Err: err}
-		}
-		if res == nil {
-			return result, nil
-		}
-	case *repository.OCIChartRepository:
-		res, err = b.downloadFromOCIRepository(b.remote.(*repository.OCIChartRepository), remoteRef, result, opts)
-		if err != nil {
-			return nil, &BuildError{Reason: ErrChartPull, Err: err}
-		}
-		if res == nil {
-			return result, nil
-		}
-	default:
-		return nil, &BuildError{Reason: ErrChartReference, Err: fmt.Errorf("unsupported remote type %T", b.remote)}
+	res, result, err := b.downloadFromRepository(b.remote, remoteRef, opts)
+	if err != nil {
+		return nil, &BuildError{Reason: ErrChartPull, Err: err}
+	}
+	if res == nil {
+		return result, nil
 	}
 
 	requiresPackaging := len(opts.GetValuesFiles()) != 0 || opts.VersionMetadata != ""
@@ -152,66 +132,31 @@ func (b *remoteChartBuilder) Build(_ context.Context, ref Reference, p string, o
 	return result, nil
 }
 
-func (b *remoteChartBuilder) downloadFromOCIRepository(remote *repository.OCIChartRepository, remoteRef RemoteReference, buildResult *Build, opts BuildOptions) (*bytes.Buffer, error) {
-	cv, err := remote.Get(remoteRef.Name, remoteRef.Version)
-	if err != nil {
-		err = fmt.Errorf("failed to get chart version for remote reference: %w", err)
-		return nil, &BuildError{Reason: ErrChartPull, Err: err}
-	}
-
-	result, shouldReturn, err := generateBuildResult(cv, opts)
-	if err != nil {
-		return nil, err
-	}
-
-	if shouldReturn {
-		*buildResult = *result
-		return nil, nil
-	}
-
-	// Download the package for the resolved version
-	res, err := remote.DownloadChart(cv)
-	if err != nil {
-		err = fmt.Errorf("failed to download chart for remote reference: %w", err)
-		return nil, &BuildError{Reason: ErrChartPull, Err: err}
-	}
-
-	*buildResult = *result
-
-	return res, nil
-}
-
-func (b *remoteChartBuilder) downloadFromRepository(remote *repository.ChartRepository, remoteRef RemoteReference, buildResult *Build, opts BuildOptions) (*bytes.Buffer, error) {
-	if err := remote.StrategicallyLoadIndex(); err != nil {
-		err = fmt.Errorf("could not load repository index for remote chart reference: %w", err)
-		return nil, &BuildError{Reason: ErrChartPull, Err: err}
-	}
-
+func (b *remoteChartBuilder) downloadFromRepository(remote Repository, remoteRef RemoteReference, opts BuildOptions) (*bytes.Buffer, *Build, error) {
 	// Get the current version for the RemoteReference
-	cv, err := remote.Get(remoteRef.Name, remoteRef.Version)
+	cv, err := remote.GetChartVersion(remoteRef.Name, remoteRef.Version)
 	if err != nil {
 		err = fmt.Errorf("failed to get chart version for remote reference: %w", err)
-		return nil, &BuildError{Reason: ErrChartReference, Err: err}
+		return nil, nil, &BuildError{Reason: ErrChartReference, Err: err}
 	}
 
 	result, shouldReturn, err := generateBuildResult(cv, opts)
 	if err != nil {
-		return nil, err
+		return nil, nil, err
 	}
-	*buildResult = *result
 
 	if shouldReturn {
-		return nil, nil
+		return nil, result, nil
 	}
 
 	// Download the package for the resolved version
 	res, err := remote.DownloadChart(cv)
 	if err != nil {
 		err = fmt.Errorf("failed to download chart for remote reference: %w", err)
-		return nil, &BuildError{Reason: ErrChartPull, Err: err}
+		return nil, nil, &BuildError{Reason: ErrChartPull, Err: err}
 	}
 
-	return res, nil
+	return res, result, nil
 }
 
 // generateBuildResult returns a Build object generated from the given chart version and build options. It also returns

internal/helm/chart/dependency_manager.go

@@ -240,7 +240,7 @@ func (dm *DependencyManager) addRemoteDependency(chart *chartWithLock, dep *helm
 		return fmt.Errorf("failed to load index for '%s': %w", dep.Name, err)
 	}
 
-	ver, err := repo.Get(dep.Name, dep.Version)
+	ver, err := repo.GetChartVersion(dep.Name, dep.Version)
 	if err != nil {
 		return err
 	}

internal/helm/repository/chart_repository.go

@@ -150,10 +150,15 @@ func newChartRepository() *ChartRepository {
 	}
 }
 
-// Get returns the repo.ChartVersion for the given name, the version is expected
+// GetChartVersion returns the repo.ChartVersion for the given name, the version is expected
 // to be a semver.Constraints compatible string. If version is empty, the latest
 // stable version will be returned and prerelease versions will be ignored.
-func (r *ChartRepository) Get(name, ver string) (*repo.ChartVersion, error) {
+func (r *ChartRepository) GetChartVersion(name, ver string) (*repo.ChartVersion, error) {
+	// See if we already have the index in cache or try to load it.
+	if err := r.StrategicallyLoadIndex(); err != nil {
+		return nil, err
+	}
+
 	r.RLock()
 	defer r.RUnlock()
 
@@ -471,6 +476,22 @@ func (r *ChartRepository) Unload() {
 	r.Index = nil
 }
 
+// Clear cache the index in memory before unloading it.
+// It cleans up temporary files and directories created by the repository.
+func (r *ChartRepository) Clear() (errs []error) {
+	if err := r.CacheIndexInMemory(); err != nil {
+		errs = append(errs, err)
+	}
+
+	r.Unload()
+
+	if err := r.RemoveCache(); err != nil {
+		errs = append(errs, err)
+	}
+
+	return
+}
+
 // SetMemCache sets the cache to use for this repository.
 func (r *ChartRepository) SetMemCache(key string, c *cache.Cache, ttl time.Duration, rec RecordMetricsFunc) {
 	r.IndexKey = key

internal/helm/repository/chart_repository_test.go

@@ -181,7 +181,7 @@ func TestChartRepository_Get(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			cv, err := r.Get(tt.chartName, tt.chartVersion)
+			cv, err := r.GetChartVersion(tt.chartName, tt.chartVersion)
 			if tt.wantErr != "" {
 				g.Expect(err).To(HaveOccurred())
 				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))

internal/helm/repository/oci_chart_repository.go

@@ -115,11 +115,11 @@ func NewOCIChartRepository(repositoryURL string, chartRepoOpts ...OCIChartReposi
 	return r, nil
 }
 
-// Get returns the repo.ChartVersion for the given name, the version is expected
+// GetChartVersion returns the repo.ChartVersion for the given name, the version is expected
 // to be a semver.Constraints compatible string. If version is empty, the latest
 // stable version will be returned and prerelease versions will be ignored.
 // adapted from https://github.com/helm/helm/blob/49819b4ef782e80b0c7f78c30bd76b51ebb56dc8/pkg/downloader/chart_downloader.go#L162
-func (r *OCIChartRepository) Get(name, ver string) (*repo.ChartVersion, error) {
+func (r *OCIChartRepository) GetChartVersion(name, ver string) (*repo.ChartVersion, error) {
 	// Find chart versions matching the given name.
 	// Either in an index file or from a registry.
 	cpURL := r.URL

internal/helm/repository/oci_chart_repository_test.go

@@ -183,7 +183,7 @@ func TestOCIChartRepository_Get(t *testing.T) {
 			g.Expect(r).ToNot(BeNil())
 
 			chart := "podinfo"
-			cv, err := r.Get(chart, tc.version)
+			cv, err := r.GetChartVersion(chart, tc.version)
 			if tc.expectedErr != "" {
 				g.Expect(err).To(HaveOccurred())
 				g.Expect(err.Error()).To(Equal(tc.expectedErr))