controllers: rough wiring of Helm chart builder

This commit starts wiring the factored out Helm chart build logic into
the reconciler to ensure, validating the API capabilities.

Signed-off-by: Hidde Beydals <hello@hidde.co>

controllers/helmchart_controller.go

@@ -19,7 +19,6 @@ package controllers
 import (
 	"context"
 	"fmt"
-	"io"
 	"net/url"
 	"os"
 	"path/filepath"
@@ -27,14 +26,11 @@ import (
 	"strings"
 	"time"
 
-	"github.com/Masterminds/semver/v3"
 	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/go-logr/logr"
-	helmchart "helm.sh/helm/v3/pkg/chart"
-	"helm.sh/helm/v3/pkg/chart/loader"
-	"helm.sh/helm/v3/pkg/chartutil"
 	"helm.sh/helm/v3/pkg/getter"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/errors"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -50,13 +46,11 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 	"sigs.k8s.io/controller-runtime/pkg/source"
-	"sigs.k8s.io/yaml"
 
 	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/fluxcd/pkg/runtime/events"
 	"github.com/fluxcd/pkg/runtime/metrics"
 	"github.com/fluxcd/pkg/runtime/predicates"
-	"github.com/fluxcd/pkg/runtime/transform"
 	"github.com/fluxcd/pkg/untar"
 
 	sourcev1 "github.com/fluxcd/source-controller/api/v1beta1"
@@ -202,6 +196,19 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		return ctrl.Result{Requeue: true}, err
 	}
 
+	// Create working directory
+	workDir, err := os.MkdirTemp("", chart.Kind + "-" + chart.Namespace + "-" + chart.Name + "-")
+	if err != nil {
+		err = fmt.Errorf("failed to create temporary working directory: %w", err)
+		chart = sourcev1.HelmChartNotReady(*chart.DeepCopy(), sourcev1.ChartPullFailedReason, err.Error())
+		if err := r.updateStatus(ctx, req, chart.Status); err != nil {
+			log.Error(err, "unable to update status")
+		}
+		r.recordReadiness(ctx, chart)
+		return ctrl.Result{Requeue: true}, err
+	}
+	defer os.RemoveAll(workDir)
+
 	// Perform the reconciliation for the chart source type
 	var reconciledChart sourcev1.HelmChart
 	var reconcileErr error
@@ -222,10 +229,10 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 			// Do not requeue as there is no chance on recovery.
 			return ctrl.Result{Requeue: false}, nil
 		}
-		reconciledChart, reconcileErr = r.reconcileFromHelmRepository(ctx, *typedSource, *chart.DeepCopy(), changed)
+		reconciledChart, reconcileErr = r.fromHelmRepository(ctx, *typedSource, *chart.DeepCopy(), workDir, changed)
 	case *sourcev1.GitRepository, *sourcev1.Bucket:
-		reconciledChart, reconcileErr = r.reconcileFromTarballArtifact(ctx, *typedSource.GetArtifact(),
-			*chart.DeepCopy(), changed)
+		reconciledChart, reconcileErr = r.fromTarballArtifact(ctx, *typedSource.GetArtifact(), *chart.DeepCopy(),
+			workDir, changed)
 	default:
 		err := fmt.Errorf("unable to reconcile unsupported source reference kind '%s'", chart.Spec.SourceRef.Kind)
 		return ctrl.Result{Requeue: false}, err
@@ -297,8 +304,8 @@ func (r *HelmChartReconciler) getSource(ctx context.Context, chart sourcev1.Helm
 	return source, nil
 }
 
-func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
-	repository sourcev1.HelmRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
+func (r *HelmChartReconciler) fromHelmRepository(ctx context.Context, repository sourcev1.HelmRepository,
+	chart sourcev1.HelmChart, workDir string, force bool) (sourcev1.HelmChart, error) {
 	// Configure ChartRepository getter options
 	clientOpts := []getter.Option{
 		getter.WithURL(repository.Spec.URL),
@@ -308,17 +315,21 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	if secret, err := r.getHelmRepositorySecret(ctx, &repository); err != nil {
 		return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
 	} else if secret != nil {
-		opts, cleanup, err := helm.ClientOptionsFromSecret(*secret)
+		// Create temporary working directory for credentials
+		authDir := filepath.Join(workDir, "creds")
+		if err := os.Mkdir(authDir, 0700); err != nil {
+			err = fmt.Errorf("failed to create temporary directory for repository credentials: %w", err)
+		}
+		opts, err := helm.ClientOptionsFromSecret(authDir, *secret)
 		if err != nil {
-			err = fmt.Errorf("auth options error: %w", err)
+			err = fmt.Errorf("failed to create client options for HelmRepository '%s': %w", repository.Name, err)
 			return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
 		}
-		defer cleanup()
 		clientOpts = append(clientOpts, opts...)
 	}
 
-	// Initialize the chart repository and load the index file
-	chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Getters, clientOpts)
+	// Initialize the chart repository
+	chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Storage.LocalPath(*repository.GetArtifact()), r.Getters, clientOpts)
 	if err != nil {
 		switch err.(type) {
 		case *url.Error:
@@ -327,29 +338,33 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
 		}
 	}
-	indexFile, err := os.Open(r.Storage.LocalPath(*repository.GetArtifact()))
-	if err != nil {
-		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-	}
-	b, err := io.ReadAll(indexFile)
-	if err != nil {
-		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-	}
-	if err = chartRepo.LoadIndex(b); err != nil {
-		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
+
+	var cachedChart string
+	if artifact := chart.GetArtifact(); artifact != nil {
+		cachedChart = artifact.Path
 	}
 
-	// Lookup the chart version in the chart repository index
-	chartVer, err := chartRepo.Get(chart.Spec.Chart, chart.Spec.Version)
+	// Build the chart
+	cBuilder := helm.NewRemoteChartBuilder(chartRepo)
+	ref := helm.RemoteChartReference{Name: chart.Spec.Chart, Version: chart.Spec.Version}
+	opts := helm.BuildOptions{
+		ValueFiles:  chart.GetValuesFiles(),
+		CachedChart: cachedChart,
+		Force:       force,
+	}
+	build, err := cBuilder.Build(ctx, ref, filepath.Join(workDir, "chart.tgz"), opts)
 	if err != nil {
 		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
 	}
 
-	// Return early if the revision is still the same as the current artifact
-	newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), chartVer.Version,
-		fmt.Sprintf("%s-%s.tgz", chartVer.Name, chartVer.Version))
-	if !force && repository.GetArtifact().HasRevision(newArtifact.Revision) {
-		if newArtifact.URL != chart.GetArtifact().URL {
+	newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), build.Version,
+		fmt.Sprintf("%s-%s.tgz", build.Name, build.Version))
+
+	// If the path of the returned build equals the cache path,
+	// there are no changes to the chart
+	if build.Path == cachedChart {
+		// Ensure hostname is updated
+		if chart.GetArtifact().URL != newArtifact.URL {
 			r.Storage.SetArtifactURL(chart.GetArtifact())
 			chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
 		}
@@ -371,362 +386,106 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	}
 	defer unlock()
 
-	// Attempt to download the chart
-	res, err := chartRepo.DownloadChart(chartVer)
-	if err != nil {
-		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-	}
-	tmpFile, err := os.CreateTemp("", fmt.Sprintf("%s-%s-", chart.Namespace, chart.Name))
-	if err != nil {
-		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-	}
-	defer os.RemoveAll(tmpFile.Name())
-	if _, err = io.Copy(tmpFile, res); err != nil {
-		tmpFile.Close()
-		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-	}
-	tmpFile.Close()
-
-	// Check if we need to repackage the chart with the declared defaults files.
-	var (
-		pkgPath      = tmpFile.Name()
-		readyReason  = sourcev1.ChartPullSucceededReason
-		readyMessage = fmt.Sprintf("Fetched revision: %s", newArtifact.Revision)
-	)
-
-	switch {
-	case len(chart.GetValuesFiles()) > 0:
-		valuesMap := make(map[string]interface{})
-
-		// Load the chart
-		helmChart, err := loader.LoadFile(pkgPath)
-		if err != nil {
-			err = fmt.Errorf("load chart error: %w", err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-		}
-
-		for _, v := range chart.GetValuesFiles() {
-			if v == "values.yaml" {
-				valuesMap = transform.MergeMaps(valuesMap, helmChart.Values)
-				continue
-			}
-
-			var valuesData []byte
-			cfn := filepath.Clean(v)
-			for _, f := range helmChart.Files {
-				if f.Name == cfn {
-					valuesData = f.Data
-					break
-				}
-			}
-			if valuesData == nil {
-				err = fmt.Errorf("invalid values file path: %s", v)
-				return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-			}
-
-			yamlMap := make(map[string]interface{})
-			err = yaml.Unmarshal(valuesData, &yamlMap)
-			if err != nil {
-				err = fmt.Errorf("unmarshaling values from %s failed: %w", v, err)
-				return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-			}
-
-			valuesMap = transform.MergeMaps(valuesMap, yamlMap)
-		}
-
-		yamlBytes, err := yaml.Marshal(valuesMap)
-		if err != nil {
-			err = fmt.Errorf("marshaling values failed: %w", err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
-		}
-
-		// Overwrite values file
-		if changed, err := helm.OverwriteChartDefaultValues(helmChart, yamlBytes); err != nil {
-			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
-		} else if !changed {
-			break
-		}
-
-		// Create temporary working directory
-		tmpDir, err := os.MkdirTemp("", fmt.Sprintf("%s-%s-", chart.Namespace, chart.Name))
-		if err != nil {
-			err = fmt.Errorf("tmp dir error: %w", err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-		}
-		defer os.RemoveAll(tmpDir)
-
-		// Package the chart with the new default values
-		pkgPath, err = chartutil.Save(helmChart, tmpDir)
-		if err != nil {
-			err = fmt.Errorf("chart package error: %w", err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
-		}
-
-		// Copy the packaged chart to the artifact path
-		if err := r.Storage.CopyFromPath(&newArtifact, pkgPath); err != nil {
-			err = fmt.Errorf("failed to write chart package to storage: %w", err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-		}
-
-		readyMessage = fmt.Sprintf("Fetched and packaged revision: %s", newArtifact.Revision)
-		readyReason = sourcev1.ChartPackageSucceededReason
-	}
-
-	// Write artifact to storage
-	if err := r.Storage.CopyFromPath(&newArtifact, pkgPath); err != nil {
-		err = fmt.Errorf("unable to write chart file: %w", err)
+	// Copy the packaged chart to the artifact path
+	if err = r.Storage.CopyFromPath(&newArtifact, build.Path); err != nil {
+		err = fmt.Errorf("failed to write chart package to storage: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 
 	// Update symlink
-	chartUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", chartVer.Name))
+	cUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", build.Name))
 	if err != nil {
 		err = fmt.Errorf("storage error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
-
-	return sourcev1.HelmChartReady(chart, newArtifact, chartUrl, readyReason, readyMessage), nil
+	return sourcev1.HelmChartReady(chart, newArtifact, cUrl, sourcev1.ChartPullSucceededReason, build.Summary()), nil
 }
 
-func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
-	artifact sourcev1.Artifact, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
-	// Create temporary working directory
-	tmpDir, err := os.MkdirTemp("", fmt.Sprintf("%s-%s-", chart.Namespace, chart.Name))
-	if err != nil {
-		err = fmt.Errorf("tmp dir error: %w", err)
+func (r *HelmChartReconciler) fromTarballArtifact(ctx context.Context, source sourcev1.Artifact,
+	chart sourcev1.HelmChart, workDir string, force bool) (sourcev1.HelmChart, error) {
+	// Create temporary working directory to untar into
+	sourceDir := filepath.Join(workDir, "source")
+	if err := os.Mkdir(sourceDir, 0700); err != nil {
+		err = fmt.Errorf("failed to create temporary directory to untar source into: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
-	defer os.RemoveAll(tmpDir)
 
 	// Open the tarball artifact file and untar files into working directory
-	f, err := os.Open(r.Storage.LocalPath(artifact))
+	f, err := os.Open(r.Storage.LocalPath(source))
 	if err != nil {
 		err = fmt.Errorf("artifact open error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
-	if _, err = untar.Untar(f, tmpDir); err != nil {
-		f.Close()
+	if _, err = untar.Untar(f, sourceDir); err != nil {
+		_ = f.Close()
 		err = fmt.Errorf("artifact untar error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
-	f.Close()
-
-	// Load the chart
-	chartPath, err := securejoin.SecureJoin(tmpDir, chart.Spec.Chart)
-	if err != nil {
-		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-	}
-	chartFileInfo, err := os.Stat(chartPath)
-	if err != nil {
-		err = fmt.Errorf("chart location read error: %w", err)
-		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-	}
-	helmChart, err := loader.Load(chartPath)
-	if err != nil {
-		err = fmt.Errorf("load chart error: %w", err)
+	if err =f.Close(); err != nil {
+		err = fmt.Errorf("artifact close error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 
-	v, err := semver.NewVersion(helmChart.Metadata.Version)
+	chartPath, err := securejoin.SecureJoin(sourceDir, chart.Spec.Chart)
 	if err != nil {
-		err = fmt.Errorf("semver parse error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 
-	version := v.String()
+	// Setup dependency manager
+	authDir := filepath.Join(workDir, "creds")
+	if err = os.Mkdir(authDir, 0700); err != nil {
+		err = fmt.Errorf("failed to create temporaRy directory for dependency credentials: %w", err)
+		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
+	}
+	dm := helm.NewDependencyManager(
+		helm.WithRepositoryCallback(r.getNamespacedChartRepositoryCallback(ctx, authDir, chart.GetNamespace())),
+	)
+	defer dm.Clear()
+
+	// Get any cached chart
+	var cachedChart string
+	if artifact := chart.Status.Artifact; artifact != nil {
+		cachedChart = artifact.Path
+	}
+
+	buildsOpts := helm.BuildOptions{
+		ValueFiles:  chart.GetValuesFiles(),
+		CachedChart: cachedChart,
+		Force:       force,
+	}
+
+	// Add revision metadata to chart build
 	if chart.Spec.ReconcileStrategy == sourcev1.ReconcileStrategyRevision {
 		// Isolate the commit SHA from GitRepository type artifacts by removing the branch/ prefix.
-		splitRev := strings.Split(artifact.Revision, "/")
-		v, err := v.SetMetadata(splitRev[len(splitRev)-1])
-		if err != nil {
-			err = fmt.Errorf("semver parse error: %w", err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-		}
-
-		version = v.String()
-		helmChart.Metadata.Version = v.String()
+		splitRev := strings.Split(source.Revision, "/")
+		buildsOpts.VersionMetadata = splitRev[len(splitRev)-1]
 	}
 
-	// Return early if the revision is still the same as the current chart artifact
-	newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.ObjectMeta.GetObjectMeta(), version,
-		fmt.Sprintf("%s-%s.tgz", helmChart.Metadata.Name, version))
-	if !force && apimeta.IsStatusConditionTrue(chart.Status.Conditions, meta.ReadyCondition) && chart.GetArtifact().HasRevision(newArtifact.Revision) {
-		if newArtifact.URL != artifact.URL {
+	// Build chart
+	chartB := helm.NewLocalChartBuilder(dm)
+	build, err := chartB.Build(ctx, helm.LocalChartReference{BaseDir: sourceDir, Path: chartPath}, filepath.Join(workDir, "chart.tgz"), buildsOpts)
+	if err != nil {
+		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
+	}
+
+	newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), build.Version,
+		fmt.Sprintf("%s-%s.tgz", build.Name, build.Version))
+
+	// If the path of the returned build equals the cache path,
+	// there are no changes to the chart
+	if build.Path == cachedChart {
+		// Ensure hostname is updated
+		if chart.GetArtifact().URL != newArtifact.URL {
 			r.Storage.SetArtifactURL(chart.GetArtifact())
 			chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
 		}
 		return chart, nil
 	}
 
-	// Either (re)package the chart with the declared default values file,
-	// or write the chart directly to storage.
-	pkgPath := chartPath
-	isValuesFileOverriden := false
-	if len(chart.GetValuesFiles()) > 0 {
-		valuesMap := make(map[string]interface{})
-		for _, v := range chart.GetValuesFiles() {
-			srcPath, err := securejoin.SecureJoin(tmpDir, v)
-			if err != nil {
-				return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-			}
-			if f, err := os.Stat(srcPath); os.IsNotExist(err) || !f.Mode().IsRegular() {
-				err = fmt.Errorf("invalid values file path: %s", v)
-				return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-			}
-
-			valuesData, err := os.ReadFile(srcPath)
-			if err != nil {
-				err = fmt.Errorf("failed to read from values file '%s': %w", v, err)
-				return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-			}
-
-			yamlMap := make(map[string]interface{})
-			err = yaml.Unmarshal(valuesData, &yamlMap)
-			if err != nil {
-				err = fmt.Errorf("unmarshaling values from %s failed: %w", v, err)
-				return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-			}
-
-			valuesMap = transform.MergeMaps(valuesMap, yamlMap)
-		}
-
-		yamlBytes, err := yaml.Marshal(valuesMap)
-		if err != nil {
-			err = fmt.Errorf("marshaling values failed: %w", err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
-		}
-
-		isValuesFileOverriden, err = helm.OverwriteChartDefaultValues(helmChart, yamlBytes)
-		if err != nil {
-			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
-		}
-	}
-
-	isDir := chartFileInfo.IsDir()
-	switch {
-	case isDir:
-		// Determine chart dependencies
-		deps := helmChart.Dependencies()
-		reqs := helmChart.Metadata.Dependencies
-		lock := helmChart.Lock
-		if lock != nil {
-			// Load from lockfile if exists
-			reqs = lock.Dependencies
-		}
-		var dwr []*helm.DependencyWithRepository
-		for _, dep := range reqs {
-			// Exclude existing dependencies
-			found := false
-			for _, existing := range deps {
-				if existing.Name() == dep.Name {
-					found = true
-				}
-			}
-			if found {
-				continue
-			}
-
-			// Continue loop if file scheme detected
-			if dep.Repository == "" || strings.HasPrefix(dep.Repository, "file://") {
-				dwr = append(dwr, &helm.DependencyWithRepository{
-					Dependency: dep,
-					Repository: nil,
-				})
-				continue
-			}
-
-			// Discover existing HelmRepository by URL
-			repository, err := r.resolveDependencyRepository(ctx, dep, chart.Namespace)
-			if err != nil {
-				repository = &sourcev1.HelmRepository{
-					Spec: sourcev1.HelmRepositorySpec{
-						URL:     dep.Repository,
-						Timeout: &metav1.Duration{Duration: 60 * time.Second},
-					},
-				}
-			}
-
-			// Configure ChartRepository getter options
-			clientOpts := []getter.Option{
-				getter.WithURL(repository.Spec.URL),
-				getter.WithTimeout(repository.Spec.Timeout.Duration),
-				getter.WithPassCredentialsAll(repository.Spec.PassCredentials),
-			}
-			if secret, err := r.getHelmRepositorySecret(ctx, repository); err != nil {
-				return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
-			} else if secret != nil {
-				opts, cleanup, err := helm.ClientOptionsFromSecret(*secret)
-				if err != nil {
-					err = fmt.Errorf("auth options error: %w", err)
-					return sourcev1.HelmChartNotReady(chart, sourcev1.AuthenticationFailedReason, err.Error()), err
-				}
-				defer cleanup()
-				clientOpts = append(clientOpts, opts...)
-			}
-
-			// Initialize the chart repository and load the index file
-			chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Getters, clientOpts)
-			if err != nil {
-				switch err.(type) {
-				case *url.Error:
-					return sourcev1.HelmChartNotReady(chart, sourcev1.URLInvalidReason, err.Error()), err
-				default:
-					return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-				}
-			}
-			if repository.Status.Artifact != nil {
-				indexFile, err := os.Open(r.Storage.LocalPath(*repository.GetArtifact()))
-				if err != nil {
-					return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-				}
-				b, err := io.ReadAll(indexFile)
-				if err != nil {
-					return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-				}
-				if err = chartRepo.LoadIndex(b); err != nil {
-					return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-				}
-			} else {
-				// Download index
-				err = chartRepo.DownloadIndex()
-				if err != nil {
-					return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-				}
-			}
-
-			dwr = append(dwr, &helm.DependencyWithRepository{
-				Dependency: dep,
-				Repository: chartRepo,
-			})
-		}
-
-		// Construct dependencies for chart if any
-		if len(dwr) > 0 {
-			dm := &helm.DependencyManager{
-				WorkingDir:   tmpDir,
-				ChartPath:    chart.Spec.Chart,
-				Chart:        helmChart,
-				Dependencies: dwr,
-			}
-			err = dm.Build(ctx)
-			if err != nil {
-				return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
-			}
-		}
-
-		fallthrough
-	case isValuesFileOverriden:
-		pkgPath, err = chartutil.Save(helmChart, tmpDir)
-		if err != nil {
-			err = fmt.Errorf("chart package error: %w", err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
-		}
-	}
-
 	// Ensure artifact directory exists
 	err = r.Storage.MkdirAll(newArtifact)
 	if err != nil {
-		err = fmt.Errorf("unable to create artifact directory: %w", err)
+		err = fmt.Errorf("unable to create chart directory: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 
@@ -739,20 +498,59 @@ func (r *HelmChartReconciler) reconcileFromTarballArtifact(ctx context.Context,
 	defer unlock()
 
 	// Copy the packaged chart to the artifact path
-	if err := r.Storage.CopyFromPath(&newArtifact, pkgPath); err != nil {
+	if err = r.Storage.CopyFromPath(&newArtifact, build.Path); err != nil {
 		err = fmt.Errorf("failed to write chart package to storage: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 
 	// Update symlink
-	cUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", helmChart.Metadata.Name))
+	cUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", chart.Name))
 	if err != nil {
 		err = fmt.Errorf("storage error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
 	}
 
-	message := fmt.Sprintf("Fetched and packaged revision: %s", newArtifact.Revision)
-	return sourcev1.HelmChartReady(chart, newArtifact, cUrl, sourcev1.ChartPackageSucceededReason, message), nil
+	return sourcev1.HelmChartReady(chart, newArtifact, cUrl, sourcev1.ChartPackageSucceededReason, build.Summary()), nil
+}
+
+// TODO(hidde): factor out to helper?
+func (r *HelmChartReconciler) getNamespacedChartRepositoryCallback(ctx context.Context, dir, namespace string) helm.GetChartRepositoryCallback {
+	return func(url string) (*helm.ChartRepository, error) {
+		repo, err := r.resolveDependencyRepository(ctx, url, namespace)
+		if err != nil {
+			if errors.ReasonForError(err) != metav1.StatusReasonUnknown {
+				return nil, err
+			}
+			repo = &sourcev1.HelmRepository{
+				Spec: sourcev1.HelmRepositorySpec{
+					URL:     url,
+					Timeout: &metav1.Duration{Duration: 60 * time.Second},
+				},
+			}
+		}
+		clientOpts := []getter.Option{
+			getter.WithURL(repo.Spec.URL),
+			getter.WithTimeout(repo.Spec.Timeout.Duration),
+			getter.WithPassCredentialsAll(repo.Spec.PassCredentials),
+		}
+		if secret, err := r.getHelmRepositorySecret(ctx, repo); err != nil {
+			return nil, err
+		} else if secret != nil {
+			opts, err := helm.ClientOptionsFromSecret(dir, *secret)
+			if err != nil {
+				return nil, err
+			}
+			clientOpts = append(clientOpts, opts...)
+		}
+		chartRepo, err := helm.NewChartRepository(repo.Spec.URL, "", r.Getters, clientOpts)
+		if err != nil {
+			return nil, err
+		}
+		if repo.Status.Artifact != nil {
+			chartRepo.CachePath = r.Storage.LocalPath(*repo.GetArtifact())
+		}
+		return chartRepo, nil
+	}
 }
 
 func (r *HelmChartReconciler) reconcileDelete(ctx context.Context, chart sourcev1.HelmChart) (ctrl.Result, error) {
@@ -880,15 +678,10 @@ func (r *HelmChartReconciler) indexHelmChartBySource(o client.Object) []string {
 	return []string{fmt.Sprintf("%s/%s", hc.Spec.SourceRef.Kind, hc.Spec.SourceRef.Name)}
 }
 
-func (r *HelmChartReconciler) resolveDependencyRepository(ctx context.Context, dep *helmchart.Dependency, namespace string) (*sourcev1.HelmRepository, error) {
-	u := helm.NormalizeChartRepositoryURL(dep.Repository)
-	if u == "" {
-		return nil, fmt.Errorf("invalid repository URL")
-	}
-
+func (r *HelmChartReconciler) resolveDependencyRepository(ctx context.Context, url string, namespace string) (*sourcev1.HelmRepository, error) {
 	listOpts := []client.ListOption{
 		client.InNamespace(namespace),
-		client.MatchingFields{sourcev1.HelmRepositoryURLIndexKey: u},
+		client.MatchingFields{sourcev1.HelmRepositoryURLIndexKey: url},
 	}
 	var list sourcev1.HelmRepositoryList
 	err := r.Client.List(ctx, &list, listOpts...)
@@ -898,8 +691,7 @@ func (r *HelmChartReconciler) resolveDependencyRepository(ctx context.Context, d
 	if len(list.Items) > 0 {
 		return &list.Items[0], nil
 	}
-
-	return nil, fmt.Errorf("no HelmRepository found")
+	return nil, fmt.Errorf("no HelmRepository found for '%s' in '%s' namespace", url, namespace)
 }
 
 func (r *HelmChartReconciler) getHelmRepositorySecret(ctx context.Context, repository *sourcev1.HelmRepository) (*corev1.Secret, error) {
@@ -917,7 +709,6 @@ func (r *HelmChartReconciler) getHelmRepositorySecret(ctx context.Context, repos
 		}
 		return &secret, nil
 	}
-
 	return nil, nil
 }
 

controllers/helmchart_controller_test.go

@@ -732,6 +732,7 @@ var _ = Describe("HelmChartReconciler", func() {
 			}, timeout, interval).Should(BeTrue())
 			helmChart, err := loader.Load(storage.LocalPath(*now.Status.Artifact))
 			Expect(err).NotTo(HaveOccurred())
+			Expect(helmChart.Values).ToNot(BeNil())
 			Expect(helmChart.Values["testDefault"]).To(BeTrue())
 			Expect(helmChart.Values["testOverride"]).To(BeFalse())
 

controllers/helmrepository_controller.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"fmt"
 	"net/url"
+	"os"
 	"time"
 
 	"github.com/fluxcd/pkg/apis/meta"
@@ -186,12 +187,18 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
 			return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
 		}
 
-		opts, cleanup, err := helm.ClientOptionsFromSecret(secret)
+		authDir, err := os.MkdirTemp("", "helm-repository-")
+		if err != nil {
+			err = fmt.Errorf("failed to create temporary working directory for credentials: %w", err)
+			return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
+		}
+		defer os.RemoveAll(authDir)
+
+		opts, err := helm.ClientOptionsFromSecret(authDir, secret)
 		if err != nil {
 			err = fmt.Errorf("auth options error: %w", err)
 			return sourcev1.HelmRepositoryNotReady(repository, sourcev1.AuthenticationFailedReason, err.Error()), err
 		}
-		defer cleanup()
 		clientOpts = append(clientOpts, opts...)
 	}