internal/helm: introduce ChartBuilder

This commit starts with the creation of a `ChartBuilder` to facilitate
the (conditional) build of a chart outside of the reconciler logic.

The builder can be configured with a set of (modifying) options, which
define together with the type of chart source what steps are taken
during the build.

To better facilitate the builder's needs and attempt to be more
efficient, changes have been made to the `DependencyBuilder` and
`ChartRepository` around (order of) operations and/or lazy-load
capabilities.

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

internal/helm/chart.go

@@ -70,17 +70,17 @@ func OverwriteChartDefaultValues(chart *helmchart.Chart, data []byte) (bool, err
 // LoadChartMetadata attempts to load the chart.Metadata from the "Chart.yaml" file in the directory or archive at the
 // given chartPath. It takes "requirements.yaml" files into account, and is therefore compatible with the
 // chart.APIVersionV1 format.
-func LoadChartMetadata(chartPath string) (*helmchart.Metadata, error) {
+func LoadChartMetadata(chartPath string) (meta *helmchart.Metadata, err error) {
 	i, err := os.Stat(chartPath)
 	if err != nil {
 		return nil, err
 	}
-	switch {
+	if i.IsDir() {
-	case i.IsDir():
+		meta, err = LoadChartMetadataFromDir(chartPath)
-		return LoadChartMetadataFromDir(chartPath)
+		return
-	default:
-		return LoadChartMetadataFromArchive(chartPath)
 	}
+	meta, err = LoadChartMetadataFromArchive(chartPath)
+	return
 }
 
 // LoadChartMetadataFromDir loads the chart.Metadata from the "Chart.yaml" file in the directory at the given path.

internal/helm/chart_builder.go

@@ -0,0 +1,384 @@
+/*
+Copyright 2021 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package helm
+
+import (
+	"context"
+	"fmt"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+
+	securejoin "github.com/cyphar/filepath-securejoin"
+	"github.com/fluxcd/source-controller/internal/fs"
+	helmchart "helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/chart/loader"
+	"helm.sh/helm/v3/pkg/chartutil"
+	"sigs.k8s.io/yaml"
+
+	"github.com/fluxcd/pkg/runtime/transform"
+)
+
+// ChartBuilder aims to efficiently build a Helm chart from a directory or packaged chart.
+// It avoids or delays loading the chart into memory in full, working with chart.Metadata
+// as much as it can, and returns early (by copying over the already packaged source chart)
+// if no modifications were made during the build process.
+type ChartBuilder struct {
+	// baseDir is the chroot for the chart builder when path isDir.
+	// It must be (a higher) relative to path. File references (during e.g.
+	// value file merge operations) are not allowed to traverse out of it.
+	baseDir string
+
+	// path is the file or directory path to a chart source.
+	path string
+
+	// chart holds a (partly) loaded chart.Chart, it contains at least the
+	// chart.Metadata, which may expand to the full chart.Chart if required
+	// for Build operations.
+	chart *helmchart.Chart
+
+	// valueFiles holds a list of path references of valueFiles that should be
+	// merged and packaged as a single "values.yaml" during Build.
+	valueFiles []string
+
+	// repositories holds an index of repository URLs and their ChartRepository.
+	// They are used to configure a DependencyManager for missing chart dependencies
+	// if isDir is true.
+	repositories map[string]*ChartRepository
+
+	// getChartRepositoryCallback is used to configure a DependencyManager for
+	// missing chart dependencies if isDir is true.
+	getChartRepositoryCallback GetChartRepositoryCallback
+
+	mu sync.Mutex
+}
+
+// NewChartBuilder constructs a new ChartBuilder for the given chart path.
+// It returns an error if no chart.Metadata can be loaded from the path.
+func NewChartBuilder(path string) (*ChartBuilder, error) {
+	metadata, err := LoadChartMetadata(path)
+	if err != nil {
+		return nil, fmt.Errorf("could not create new chart builder: %w", err)
+	}
+	return &ChartBuilder{
+		path:  path,
+		chart: &helmchart.Chart{
+			Metadata: metadata,
+		},
+	}, nil
+}
+
+// WithBaseDir configures the base dir on the ChartBuilder.
+func (b *ChartBuilder) WithBaseDir(p string) *ChartBuilder {
+	b.mu.Lock()
+	b.baseDir = p
+	b.mu.Unlock()
+	return b
+}
+
+// WithValueFiles appends the given paths to the ChartBuilder's valueFiles.
+func (b *ChartBuilder) WithValueFiles(path ...string) *ChartBuilder {
+	b.mu.Lock()
+	b.valueFiles = append(b.valueFiles, path...)
+	b.mu.Unlock()
+	return b
+}
+
+// WithChartRepository indexes the given ChartRepository by the NormalizeChartRepositoryURL,
+// used to configure the DependencyManager if the chart is not packaged.
+func (b *ChartBuilder) WithChartRepository(url string, index *ChartRepository) *ChartBuilder {
+	b.mu.Lock()
+	b.repositories[NormalizeChartRepositoryURL(url)] = index
+	b.mu.Unlock()
+	return b
+}
+
+// WithChartRepositoryCallback configures the GetChartRepositoryCallback used by the
+// DependencyManager if the chart is not packaged.
+func (b *ChartBuilder) WithChartRepositoryCallback(c GetChartRepositoryCallback) *ChartBuilder {
+	b.mu.Lock()
+	b.getChartRepositoryCallback = c
+	b.mu.Unlock()
+	return b
+}
+
+// ChartBuildResult contains the ChartBuilder result, including build specific
+// information about the chart.
+type ChartBuildResult struct {
+	// SourceIsDir indicates if the chart was build from a directory.
+	SourceIsDir bool
+	// Path contains the absolute path to the packaged chart.
+	Path string
+	// ValuesOverwrite holds a structured map with the merged values used
+	// to overwrite chart default "values.yaml".
+	ValuesOverwrite map[string]interface{}
+	// CollectedDependencies contains the number of missing local and remote
+	// dependencies that were collected by the DependencyManager before building
+	// the chart.
+	CollectedDependencies int
+	// Packaged indicates if the ChartBuilder has packaged the chart.
+	// This can for example be false if SourceIsDir is false and ValuesOverwrite
+	// is nil, which makes the ChartBuilder copy the chart source to Path without
+	// making any modifications.
+	Packaged bool
+}
+
+// String returns the Path of the ChartBuildResult.
+func (b *ChartBuildResult) String() string {
+	if b != nil {
+		return b.Path
+	}
+	return ""
+}
+
+// Build attempts to build a new chart using ChartBuilder configuration,
+// writing it to the provided path.
+// It returns a ChartBuildResult containing all information about the resulting chart,
+// or an error.
+func (b *ChartBuilder) Build(ctx context.Context, p string) (_ *ChartBuildResult, err error) {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+
+	if b.chart == nil {
+		err = fmt.Errorf("chart build failed: no initial chart (metadata) loaded")
+		return
+	}
+	if b.path == "" {
+		err = fmt.Errorf("chart build failed: no path set")
+		return
+	}
+
+	result := &ChartBuildResult{}
+	result.SourceIsDir = pathIsDir(b.path)
+	result.Path = p
+
+	// Merge chart values
+	if err = b.mergeValues(result); err != nil {
+		err = fmt.Errorf("chart build failed: %w", err)
+		return
+	}
+
+	// Ensure chart has all dependencies
+	if err = b.buildDependencies(ctx, result); err != nil {
+		err = fmt.Errorf("chart build failed: %w", err)
+		return
+	}
+
+	// Package (or copy) chart
+	if err = b.packageChart(result); err != nil {
+		err = fmt.Errorf("chart package failed: %w", err)
+		return
+	}
+	return result, nil
+}
+
+// load lazy-loads chart.Chart into chart from the set path, it replaces any previously set
+// chart.Metadata shim.
+func (b *ChartBuilder) load() (err error) {
+	if b.chart == nil || len(b.chart.Files) <= 0 {
+		if b.path == "" {
+			return fmt.Errorf("failed to load chart: path not set")
+		}
+		chart, err := loader.Load(b.path)
+		if err != nil {
+			return fmt.Errorf("failed to load chart: %w", err)
+		}
+		b.chart = chart
+	}
+	return
+}
+
+// buildDependencies builds the missing dependencies for a chart from a directory.
+// Using the chart using a NewDependencyManager and the configured repositories
+// and getChartRepositoryCallback
+// It returns the number of dependencies it collected, or an error.
+func (b *ChartBuilder) buildDependencies(ctx context.Context, result *ChartBuildResult) (err error) {
+	if !result.SourceIsDir {
+		return
+	}
+
+	if err = b.load(); err != nil {
+		err = fmt.Errorf("failed to ensure chart has no missing dependencies: %w", err)
+		return
+	}
+
+	 dm := NewDependencyManager(b.chart, b.baseDir, strings.TrimLeft(b.path, b.baseDir)).
+		WithRepositories(b.repositories).
+		WithChartRepositoryCallback(b.getChartRepositoryCallback)
+
+	result.CollectedDependencies, err = dm.Build(ctx)
+	return
+}
+
+// mergeValues strategically merges the valueFiles, it merges using mergeFileValues
+// or mergeChartValues depending on if the chart is sourced from a package or directory.
+// Ir only calls load to propagate the chart if required by the strategy.
+// It returns the merged values, or an error.
+func (b *ChartBuilder) mergeValues(result *ChartBuildResult) (err error) {
+	if len(b.valueFiles) == 0 {
+		return
+	}
+
+	if result.SourceIsDir {
+		result.ValuesOverwrite, err = mergeFileValues(b.baseDir, b.valueFiles)
+		if err != nil {
+			err = fmt.Errorf("failed to merge value files: %w", err)
+		}
+		return
+	}
+
+	// Values equal to default
+	if len(b.valueFiles) == 1 && b.valueFiles[0] == chartutil.ValuesfileName {
+		return
+	}
+
+	if err = b.load(); err != nil {
+		err = fmt.Errorf("failed to merge chart values: %w", err)
+		return
+	}
+
+	if result.ValuesOverwrite, err = mergeChartValues(b.chart, b.valueFiles); err != nil {
+		err = fmt.Errorf("failed to merge chart values: %w", err)
+		return
+	}
+	return nil
+}
+
+// packageChart determines if it should copyFileToPath or packageToPath
+// based on the provided result. It sets Packaged on ChartBuildResult to
+// true if packageToPath is successful.
+func (b *ChartBuilder) packageChart(result *ChartBuildResult) error {
+	// If we are not building from a directory, and we do not have any
+	// replacement values, we can copy over the already packaged source
+	// chart without making any modifications
+	if !result.SourceIsDir && len(result.ValuesOverwrite) == 0 {
+		if err := copyFileToPath(b.path, result.Path); err != nil {
+			return fmt.Errorf("chart build failed: %w", err)
+		}
+		return nil
+	}
+
+	// Package chart to a new temporary directory
+	if err := packageToPath(b.chart, result.Path); err != nil {
+		return fmt.Errorf("chart build failed: %w", err)
+	}
+	result.Packaged = true
+	return nil
+}
+
+// mergeChartValues merges the given chart.Chart Files paths into a single "values.yaml" map.
+// It returns the merge result, or an error.
+func mergeChartValues(chart *helmchart.Chart, paths []string) (map[string]interface{}, error) {
+	mergedValues := make(map[string]interface{})
+	for _, p := range paths {
+		cfn := filepath.Clean(p)
+		if cfn == chartutil.ValuesfileName {
+			mergedValues = transform.MergeMaps(mergedValues, chart.Values)
+			continue
+		}
+		var b []byte
+		for _, f := range chart.Files {
+			if f.Name == cfn {
+				b = f.Data
+				break
+			}
+		}
+		if b == nil {
+			return nil, fmt.Errorf("no values file found at path '%s'", p)
+		}
+		values := make(map[string]interface{})
+		if err := yaml.Unmarshal(b, &values); err != nil {
+			return nil, fmt.Errorf("unmarshaling values from '%s' failed: %w", p, err)
+		}
+		mergedValues = transform.MergeMaps(mergedValues, values)
+	}
+	return mergedValues, nil
+}
+
+// mergeFileValues merges the given value file paths into a single "values.yaml" map.
+// The provided (relative) paths may not traverse outside baseDir. It returns the merge
+// result, or an error.
+func mergeFileValues(baseDir string, paths []string) (map[string]interface{}, error) {
+	mergedValues := make(map[string]interface{})
+	for _, p := range paths {
+		secureP, err := securejoin.SecureJoin(baseDir, p)
+		if err != nil {
+			return nil, err
+		}
+		if f, err := os.Stat(secureP); os.IsNotExist(err) || !f.Mode().IsRegular() {
+			return nil, fmt.Errorf("no values file found at path '%s' (reference '%s')",
+				strings.TrimPrefix(secureP, baseDir), p)
+		}
+		b, err := os.ReadFile(secureP)
+		if err != nil {
+			return nil, fmt.Errorf("could not read values from file '%s': %w", p, err)
+		}
+		values := make(map[string]interface{})
+		err = yaml.Unmarshal(b, &values)
+		if err != nil {
+			return nil, fmt.Errorf("unmarshaling values from '%s' failed: %w", p, err)
+		}
+		mergedValues = transform.MergeMaps(mergedValues, values)
+	}
+	return mergedValues, nil
+}
+
+// copyFileToPath attempts to copy in to out. It returns an error if out already exists.
+func copyFileToPath(in, out string) error {
+	o, err := os.Create(out)
+	if err != nil {
+		return fmt.Errorf("failed to create copy target: %w", err)
+	}
+	defer o.Close()
+	i, err := os.Open(in)
+	if err != nil {
+		return fmt.Errorf("failed to open file to copy from: %w", err)
+	}
+	defer i.Close()
+	if _, err := o.ReadFrom(i); err != nil {
+		return fmt.Errorf("failed to read from source during copy: %w", err)
+	}
+	return nil
+}
+
+// packageToPath attempts to package the given chart.Chart to the out filepath.
+func packageToPath(chart *helmchart.Chart, out string) error {
+	o, err := os.MkdirTemp("", "chart-build-*")
+	if err != nil {
+		return fmt.Errorf("failed to create temporary directory for chart: %w", err)
+	}
+	defer os.RemoveAll(o)
+
+	p, err := chartutil.Save(chart, o)
+	if err != nil {
+		return fmt.Errorf("failed to package chart: %w", err)
+	}
+	return fs.RenameWithFallback(p, out)
+}
+
+// pathIsDir returns a boolean indicating if the given path points to a directory.
+// In case os.Stat on the given path returns an error it returns false as well.
+func pathIsDir(p string) bool {
+	if p == "" {
+		return false
+	}
+	if i, err := os.Stat(p); err != nil || !i.IsDir() {
+		return false
+	}
+	return true
+}

internal/helm/chart_builder_test.go

@@ -0,0 +1,598 @@
+/*
+Copyright 2021 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package helm
+
+import (
+	"context"
+	"encoding/hex"
+	"fmt"
+	"math/rand"
+	"os"
+	"path/filepath"
+	"sync"
+	"testing"
+
+	. "github.com/onsi/gomega"
+	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/repo"
+)
+
+func TestChartBuildResult_String(t *testing.T) {
+	g := NewWithT(t)
+
+	var result *ChartBuildResult
+	g.Expect(result.String()).To(Equal(""))
+	result = &ChartBuildResult{}
+	g.Expect(result.String()).To(Equal(""))
+	result = &ChartBuildResult{Path: "/foo/"}
+	g.Expect(result.String()).To(Equal("/foo/"))
+}
+
+func TestChartBuilder_Build(t *testing.T) {
+	tests := []struct {
+		name                       string
+		baseDir                    string
+		path                       string
+		valueFiles                 []string
+		repositories               map[string]*ChartRepository
+		getChartRepositoryCallback GetChartRepositoryCallback
+		wantErr                    string
+	}{
+		{
+			name: "builds chart from directory",
+			path: "testdata/charts/helmchart",
+		},
+		{
+			name: "builds chart from package",
+			path: "testdata/charts/helmchart-0.1.0.tgz",
+		},
+		{
+			// TODO(hidde): add more diverse tests
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			b, err := NewChartBuilder(tt.path)
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(b).ToNot(BeNil())
+
+			b.WithBaseDir(tt.baseDir)
+			b.WithValueFiles(tt.valueFiles...)
+			b.WithChartRepositoryCallback(b.getChartRepositoryCallback)
+			for k, v := range tt.repositories {
+				b.WithChartRepository(k, v)
+			}
+
+			out := tmpFile("build-0.1.0", ".tgz")
+			defer os.RemoveAll(out)
+			got, err := b.Build(context.TODO(), out)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				g.Expect(got).To(BeNil())
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(got).ToNot(BeNil())
+
+			g.Expect(got.Path).ToNot(BeEmpty())
+			g.Expect(got.Path).To(Equal(out))
+			g.Expect(got.Path).To(BeARegularFile())
+			_, err = loader.Load(got.Path)
+			g.Expect(err).ToNot(HaveOccurred())
+		})
+	}
+}
+
+func TestChartBuilder_load(t *testing.T) {
+	tests := []struct {
+		name     string
+		path     string
+		chart    *helmchart.Chart
+		wantFunc func(g *WithT, c *helmchart.Chart)
+		wantErr  string
+	}{
+		{
+			name:  "loads chart",
+			chart: nil,
+			path:  "testdata/charts/helmchart-0.1.0.tgz",
+			wantFunc: func(g *WithT, c *helmchart.Chart) {
+				g.Expect(c.Metadata.Name).To(Equal("helmchart"))
+				g.Expect(c.Files).ToNot(BeZero())
+			},
+		},
+		{
+			name: "overwrites chart without any files (metadata shim)",
+			chart: &helmchart.Chart{
+				Metadata: &helmchart.Metadata{Name: "dummy"},
+			},
+			path: "testdata/charts/helmchart-0.1.0.tgz",
+			wantFunc: func(g *WithT, c *helmchart.Chart) {
+				g.Expect(c.Metadata.Name).To(Equal("helmchart"))
+				g.Expect(c.Files).ToNot(BeZero())
+			},
+		},
+		{
+			name: "does not overwrite loaded chart",
+			chart: &helmchart.Chart{
+				Metadata: &helmchart.Metadata{Name: "dummy"},
+				Files: []*helmchart.File{
+					{Name: "mock.yaml", Data: []byte("loaded chart")},
+				},
+			},
+			path: "testdata/charts/helmchart-0.1.0.tgz",
+			wantFunc: func(g *WithT, c *helmchart.Chart) {
+				g.Expect(c.Metadata.Name).To(Equal("dummy"))
+				g.Expect(c.Files).To(HaveLen(1))
+			},
+		},
+		{
+			name:    "no path",
+			wantErr: "failed to load chart: path not set",
+		},
+		{
+			name:    "invalid chart",
+			path:    "testdata/charts/empty.tgz",
+			wantErr: "failed to load chart: no files in chart archive",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			b := &ChartBuilder{
+				path:  tt.path,
+				chart: tt.chart,
+			}
+			err := b.load()
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			if tt.wantFunc != nil {
+				tt.wantFunc(g, b.chart)
+			}
+		})
+	}
+}
+
+func TestChartBuilder_buildDependencies(t *testing.T) {
+	g := NewWithT(t)
+
+	chartB, err := os.ReadFile("testdata/charts/helmchart-0.1.0.tgz")
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(chartB).ToNot(BeEmpty())
+
+	mockRepo := func() *ChartRepository {
+		return &ChartRepository{
+			Client: &mockGetter{
+				response: chartB,
+			},
+			Index: &repo.IndexFile{
+				Entries: map[string]repo.ChartVersions{
+					"grafana": {
+						&repo.ChartVersion{
+							Metadata: &helmchart.Metadata{
+								Name:    "grafana",
+								Version: "6.17.4",
+							},
+							URLs: []string{"https://example.com/chart.tgz"},
+						},
+					},
+				},
+			},
+			RWMutex: &sync.RWMutex{},
+		}
+	}
+
+	var mockCallback GetChartRepositoryCallback = func(url string) (*ChartRepository, error) {
+		if url == "https://grafana.github.io/helm-charts/" {
+			return mockRepo(), nil
+		}
+		return nil, fmt.Errorf("no repository for URL")
+	}
+
+	tests := []struct {
+		name                       string
+		baseDir                    string
+		path                       string
+		chart                      *helmchart.Chart
+		fromDir                    bool
+		repositories               map[string]*ChartRepository
+		getChartRepositoryCallback GetChartRepositoryCallback
+		wantCollectedDependencies  int
+		wantErr                    string
+	}{
+		{
+			name:                       "builds dependencies using callback",
+			fromDir:                    true,
+			baseDir:                    "testdata/charts",
+			path:                       "testdata/charts/helmchartwithdeps",
+			getChartRepositoryCallback: mockCallback,
+			wantCollectedDependencies:  2,
+		},
+		{
+			name:    "builds dependencies using repositories",
+			fromDir: true,
+			baseDir: "testdata/charts",
+			path:    "testdata/charts/helmchartwithdeps",
+			repositories: map[string]*ChartRepository{
+				"https://grafana.github.io/helm-charts/": mockRepo(),
+			},
+			wantCollectedDependencies: 2,
+		},
+		{
+			name: "skips dependency build for packaged chart",
+			path: "testdata/charts/helmchart-0.1.0.tgz",
+		},
+		{
+			name:    "attempts to load chart",
+			fromDir: true,
+			path:    "testdata",
+			wantErr: "failed to ensure chart has no missing dependencies",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			b := &ChartBuilder{
+				baseDir:                    tt.baseDir,
+				path:                       tt.path,
+				chart:                      tt.chart,
+				repositories:               tt.repositories,
+				getChartRepositoryCallback: tt.getChartRepositoryCallback,
+			}
+
+			result := &ChartBuildResult{SourceIsDir: tt.fromDir}
+			err := b.buildDependencies(context.TODO(), result)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				g.Expect(result.CollectedDependencies).To(BeZero())
+				g.Expect(b.chart).To(Equal(tt.chart))
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(result).ToNot(BeNil())
+			g.Expect(result.CollectedDependencies).To(Equal(tt.wantCollectedDependencies))
+			if tt.wantCollectedDependencies > 0 {
+				g.Expect(b.chart).ToNot(Equal(tt.chart))
+			}
+		})
+	}
+}
+
+func TestChartBuilder_mergeValues(t *testing.T) {
+	tests := []struct {
+		name       string
+		baseDir    string
+		path       string
+		isDir      bool
+		chart      *helmchart.Chart
+		valueFiles []string
+		want       map[string]interface{}
+		wantErr    string
+	}{
+		{
+			name: "merges chart values",
+			chart: &helmchart.Chart{
+				Files: []*helmchart.File{
+					{Name: "a.yaml", Data: []byte("a: b")},
+					{Name: "b.yaml", Data: []byte("a: c")},
+				},
+			},
+			valueFiles: []string{"a.yaml", "b.yaml"},
+			want: map[string]interface{}{
+				"a": "c",
+			},
+		},
+		{
+			name: "chart values merge error",
+			chart: &helmchart.Chart{
+				Files: []*helmchart.File{
+					{Name: "b.yaml", Data: []byte("a: c")},
+				},
+			},
+			valueFiles: []string{"a.yaml"},
+			wantErr:    "failed to merge chart values",
+		},
+		{
+			name:       "merges file values",
+			isDir:      true,
+			baseDir:    "testdata/charts",
+			path:       "helmchart",
+			valueFiles: []string{"helmchart/values-prod.yaml"},
+			want: map[string]interface{}{
+				"replicaCount": float64(2),
+			},
+		},
+		{
+			name:       "file values merge error",
+			isDir:      true,
+			baseDir:    "testdata/charts",
+			path:       "helmchart",
+			valueFiles: []string{"invalid.yaml"},
+			wantErr:    "failed to merge value files",
+		},
+		{
+			name:    "error on chart load failure",
+			baseDir: "testdata/charts",
+			path:    "invalid",
+			wantErr: "failed to load chart",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			b := &ChartBuilder{
+				baseDir:    tt.baseDir,
+				path:       tt.path,
+				chart:      tt.chart,
+				valueFiles: tt.valueFiles,
+			}
+
+			result := &ChartBuildResult{SourceIsDir: tt.isDir}
+			err := b.mergeValues(result)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				g.Expect(result.ValuesOverwrite).To(BeNil())
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(result.ValuesOverwrite).To(Equal(tt.want))
+		})
+	}
+}
+
+func Test_mergeChartValues(t *testing.T) {
+	tests := []struct {
+		name    string
+		chart   *helmchart.Chart
+		paths   []string
+		want    map[string]interface{}
+		wantErr string
+	}{
+		{
+			name: "merges values",
+			chart: &helmchart.Chart{
+				Files: []*helmchart.File{
+					{Name: "a.yaml", Data: []byte("a: b")},
+					{Name: "b.yaml", Data: []byte("b: c")},
+					{Name: "c.yaml", Data: []byte("b: d")},
+				},
+			},
+			paths: []string{"a.yaml", "b.yaml", "c.yaml"},
+			want: map[string]interface{}{
+				"a": "b",
+				"b": "d",
+			},
+		},
+		{
+			name: "uses chart values",
+			chart: &helmchart.Chart{
+				Files: []*helmchart.File{
+					{Name: "c.yaml", Data: []byte("b: d")},
+				},
+				Values: map[string]interface{}{
+					"a": "b",
+				},
+			},
+			paths: []string{chartutil.ValuesfileName, "c.yaml"},
+			want: map[string]interface{}{
+				"a": "b",
+				"b": "d",
+			},
+		},
+		{
+			name: "unmarshal error",
+			chart: &helmchart.Chart{
+				Files: []*helmchart.File{
+					{Name: "invalid", Data: []byte("abcd")},
+				},
+			},
+			paths:   []string{"invalid"},
+			wantErr: "unmarshaling values from 'invalid' failed",
+		},
+		{
+			name:    "error on invalid path",
+			chart:   &helmchart.Chart{},
+			paths:   []string{"a.yaml"},
+			wantErr: "no values file found at path 'a.yaml'",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			got, err := mergeChartValues(tt.chart, tt.paths)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				g.Expect(got).To(BeNil())
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}
+
+func Test_mergeFileValues(t *testing.T) {
+	tests := []struct {
+		name    string
+		files   []*helmchart.File
+		paths   []string
+		want    map[string]interface{}
+		wantErr string
+	}{
+		{
+			name: "merges values from files",
+			files: []*helmchart.File{
+				{Name: "a.yaml", Data: []byte("a: b")},
+				{Name: "b.yaml", Data: []byte("b: c")},
+				{Name: "c.yaml", Data: []byte("b: d")},
+			},
+			paths: []string{"a.yaml", "b.yaml", "c.yaml"},
+			want: map[string]interface{}{
+				"a": "b",
+				"b": "d",
+			},
+		},
+		{
+			name:    "illegal traverse",
+			paths:   []string{"../../../traversing/illegally/a/p/a/b"},
+			wantErr: "no values file found at path '/traversing/illegally/a/p/a/b'",
+		},
+		{
+			name: "unmarshal error",
+			files: []*helmchart.File{
+				{Name: "invalid", Data: []byte("abcd")},
+			},
+			paths:   []string{"invalid"},
+			wantErr: "unmarshaling values from 'invalid' failed",
+		},
+		{
+			name:    "error on invalid path",
+			paths:   []string{"a.yaml"},
+			wantErr: "no values file found at path '/a.yaml'",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			baseDir, err := os.MkdirTemp("", "merge-file-values-*")
+			g.Expect(err).ToNot(HaveOccurred())
+			defer os.RemoveAll(baseDir)
+
+			for _, f := range tt.files {
+				g.Expect(os.WriteFile(filepath.Join(baseDir, f.Name), f.Data, 0644)).To(Succeed())
+			}
+
+			got, err := mergeFileValues(baseDir, tt.paths)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				g.Expect(got).To(BeNil())
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}
+
+func Test_copyFileToPath(t *testing.T) {
+	tests := []struct {
+		name    string
+		in      string
+		wantErr string
+	}{
+		{
+			name: "copies input file",
+			in:   "testdata/local-index.yaml",
+		},
+		{
+			name:    "invalid input file",
+			in:      "testdata/invalid.tgz",
+			wantErr: "failed to open file to copy from",
+		},
+		{
+			name:    "invalid input directory",
+			in:      "testdata/charts",
+			wantErr: "failed to read from source during copy",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			out := tmpFile("copy-0.1.0", ".tgz")
+			defer os.RemoveAll(out)
+			err := copyFileToPath(tt.in, out)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(out).To(BeARegularFile())
+			f1, err := os.ReadFile(tt.in)
+			g.Expect(err).ToNot(HaveOccurred())
+			f2, err := os.ReadFile(out)
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(f2).To(Equal(f1))
+		})
+	}
+}
+
+func Test_packageToPath(t *testing.T) {
+	g := NewWithT(t)
+
+	chart, err := loader.Load("testdata/charts/helmchart-0.1.0.tgz")
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(chart).ToNot(BeNil())
+
+	out := tmpFile("chart-0.1.0", ".tgz")
+	defer os.RemoveAll(out)
+	err = packageToPath(chart, out)
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(out).To(BeARegularFile())
+	_, err = loader.Load(out)
+	g.Expect(err).ToNot(HaveOccurred())
+}
+
+func Test_pathIsDir(t *testing.T) {
+	tests := []struct {
+		name string
+		p    string
+		want bool
+	}{
+		{name: "directory", p: "testdata/", want: true},
+		{name: "file", p: "testdata/local-index.yaml", want: false},
+		{name: "not found error", p: "testdata/does-not-exist.yaml", want: false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			g.Expect(pathIsDir(tt.p)).To(Equal(tt.want))
+		})
+	}
+}
+
+func tmpFile(prefix, suffix string) string {
+	randBytes := make([]byte, 16)
+	rand.Read(randBytes)
+	return filepath.Join(os.TempDir(), prefix+hex.EncodeToString(randBytes)+suffix)
+}

internal/helm/chart_test.go

@@ -17,7 +17,6 @@ limitations under the License.
 package helm
 
 import (
-	"reflect"
 	"testing"
 
 	. "github.com/onsi/gomega"
@@ -87,33 +86,35 @@ func TestOverwriteChartDefaultValues(t *testing.T) {
 	}
 	for _, tt := range testCases {
 		t.Run(tt.desc, func(t *testing.T) {
+			g := NewWithT(t)
+
 			fixture := tt.chart
 			ok, err := OverwriteChartDefaultValues(&fixture, tt.data)
-			if ok != tt.ok {
+			g.Expect(ok).To(Equal(tt.ok))
-				t.Fatalf("should return %v, returned %v", tt.ok, ok)
+
-			}
+			if tt.expectErr {
-			if err != nil && !tt.expectErr {
+				g.Expect(err).To(HaveOccurred())
-				t.Fatalf("returned unexpected error: %v", err)
+				g.Expect(ok).To(Equal(tt.ok))
-			}
+				return
-			if err == nil && tt.expectErr {
-				t.Fatal("expected error")
 			}
 
+			if tt.ok {
 				for _, f := range fixture.Raw {
-				if f.Name == chartutil.ValuesfileName && reflect.DeepEqual(f.Data, originalValuesFixture) && tt.ok {
+					if f.Name == chartutil.ValuesfileName {
-					t.Error("should override values.yaml in Raw field")
+						g.Expect(f.Data).To(Equal(tt.data))
 					}
 				}
 				for _, f := range fixture.Files {
-				if f.Name == chartutil.ValuesfileName && reflect.DeepEqual(f.Data, originalValuesFixture) && tt.ok {
+					if f.Name == chartutil.ValuesfileName {
-					t.Error("should override values.yaml in Files field")
+						g.Expect(f.Data).To(Equal(tt.data))
+					}
 				}
 			}
 		})
 	}
 }
 
-func Test_LoadChartMetadataFromDir(t *testing.T) {
+func TestLoadChartMetadataFromDir(t *testing.T) {
 	tests := []struct {
 		name                string
 		dir                 string

internal/helm/dependency_manager.go

@@ -33,165 +33,282 @@ import (
 	"helm.sh/helm/v3/pkg/chart/loader"
 )
 
-// DependencyWithRepository is a container for a Helm chart dependency
+// GetChartRepositoryCallback must return a ChartRepository for the URL,
-// and its respective repository.
+// or an error describing why it could not be returned.
-type DependencyWithRepository struct {
+type GetChartRepositoryCallback func(url string) (*ChartRepository, error)
-	// Dependency holds the reference to a chart.Chart dependency.
-	Dependency *helmchart.Dependency
-	// Repository is the ChartRepository the dependency should be
-	// available at and can be downloaded from. If there is none,
-	// a local ('file://') dependency is assumed.
-	Repository *ChartRepository
-}
 
-// DependencyManager manages dependencies for a Helm chart.
+// DependencyManager manages dependencies for a Helm chart, downloading
+// only those that are missing from the chart it holds.
 type DependencyManager struct {
-	// WorkingDir is the chroot path for dependency manager operations,
+	// chart contains the chart.Chart from the path.
+	chart *helmchart.Chart
+
+	// baseDir is the chroot path for dependency manager operations,
 	// Dependencies that hold a local (relative) path reference are not
 	// allowed to traverse outside this directory.
-	WorkingDir string
+	baseDir string
-	// ChartPath is the path of the Chart relative to the WorkingDir,
-	// the combination of the WorkingDir and ChartPath is used to
-	// determine the absolute path of a local dependency.
-	ChartPath string
-	// Chart holds the loaded chart.Chart from the ChartPath.
-	Chart *helmchart.Chart
-	// Dependencies contains a list of dependencies, and the respective
-	// repository the dependency can be found at.
-	Dependencies []*DependencyWithRepository
-	// Workers is the number of concurrent chart-add operations during
-	// Build. Defaults to 1 (non-concurrent).
-	Workers int64
 
+	// path is the path of the chart relative to the baseDir,
+	// the combination of the baseDir and path is used to
+	// determine the absolute path of a local dependency.
+	path string
+
+	// repositories contains a map of ChartRepository indexed by their
+	// normalized URL. It is used as a lookup table for missing
+	// dependencies.
+	repositories map[string]*ChartRepository
+
+	// getChartRepositoryCallback can be set to an on-demand get
+	// callback which returned result is cached to repositories.
+	getChartRepositoryCallback GetChartRepositoryCallback
+
+	// workers is the number of concurrent chart-add operations during
+	// Build. Defaults to 1 (non-concurrent).
+	workers int64
+
+	// mu contains the lock for chart writes.
 	mu sync.Mutex
 }
 
-// Build compiles and builds the dependencies of the Chart with the
+func NewDependencyManager(chart *helmchart.Chart, baseDir, path string) *DependencyManager {
-// configured number of Workers.
+	return &DependencyManager{
-func (dm *DependencyManager) Build(ctx context.Context) error {
+		chart:   chart,
-	if len(dm.Dependencies) == 0 {
+		baseDir: baseDir,
-		return nil
+		path:    path,
+	}
 }
 
-	workers := dm.Workers
+func (dm *DependencyManager) WithRepositories(r map[string]*ChartRepository) *DependencyManager {
+	dm.repositories = r
+	return dm
+}
+
+func (dm *DependencyManager) WithChartRepositoryCallback(c GetChartRepositoryCallback) *DependencyManager {
+	dm.getChartRepositoryCallback = c
+	return dm
+}
+
+func (dm *DependencyManager) WithWorkers(w int64) *DependencyManager {
+	dm.workers = w
+	return dm
+}
+
+// Build compiles and builds the dependencies of the chart with the
+// configured number of workers.
+func (dm *DependencyManager) Build(ctx context.Context) (int, error) {
+	// Collect dependency metadata
+	var (
+		deps = dm.chart.Dependencies()
+		reqs = dm.chart.Metadata.Dependencies
+	)
+	// Lock file takes precedence
+	if lock := dm.chart.Lock; lock != nil {
+		reqs = lock.Dependencies
+	}
+
+	// Collect missing dependencies
+	missing := collectMissing(deps, reqs)
+	if len(missing) == 0 {
+		return 0, nil
+	}
+
+	// Run the build for the missing dependencies
+	if err := dm.build(ctx, missing); err != nil {
+		return 0, err
+	}
+	return len(missing), nil
+}
+
+// build (concurrently) adds the given list of deps to the chart with the configured
+// number of workers. It returns the first error, cancelling all other workers.
+func (dm *DependencyManager) build(ctx context.Context, deps map[string]*helmchart.Dependency) error {
+	workers := dm.workers
 	if workers <= 0 {
 		workers = 1
 	}
 
+	// Garbage collect temporary cached ChartRepository indexes
 	defer func() {
-		for _, dep := range dm.Dependencies {
+		for _, v := range dm.repositories {
-			dep.Repository.UnloadIndex()
+			v.Unload()
+			_ = v.RemoveCache()
 		}
 	}()
 
 	group, groupCtx := errgroup.WithContext(ctx)
 	group.Go(func() error {
 		sem := semaphore.NewWeighted(workers)
-		for _, dep := range dm.Dependencies {
+		for name, dep := range deps {
-			dep := dep
+			name, dep := name, dep
 			if err := sem.Acquire(groupCtx, 1); err != nil {
 				return err
 			}
-			group.Go(func() error {
+			group.Go(func() (err error) {
 				defer sem.Release(1)
-				if dep.Repository == nil {
+				if isLocalDep(dep) {
-					return dm.addLocalDependency(dep)
+					if err = dm.addLocalDependency(dep); err != nil {
+						err = fmt.Errorf("failed to add local dependency '%s': %w", name, err)
 					}
-				return dm.addRemoteDependency(dep)
+					return
+				}
+				if err = dm.addRemoteDependency(dep); err != nil {
+					err = fmt.Errorf("failed to add remote dependency '%s': %w", name, err)
+				}
+				return
 			})
 		}
 		return nil
 	})
-
 	return group.Wait()
 }
 
-func (dm *DependencyManager) addLocalDependency(dpr *DependencyWithRepository) error {
+// addLocalDependency attempts to resolve and add the given local chart.Dependency to the chart.
-	sLocalChartPath, err := dm.secureLocalChartPath(dpr)
+func (dm *DependencyManager) addLocalDependency(dep *helmchart.Dependency) error {
+	sLocalChartPath, err := dm.secureLocalChartPath(dep)
 	if err != nil {
 		return err
 	}
 
 	if _, err := os.Stat(sLocalChartPath); err != nil {
 		if os.IsNotExist(err) {
-			return fmt.Errorf("no chart found at '%s' (reference '%s') for dependency '%s'",
+			return fmt.Errorf("no chart found at '%s' (reference '%s')",
-				strings.TrimPrefix(sLocalChartPath, dm.WorkingDir), dpr.Dependency.Repository, dpr.Dependency.Name)
+				strings.TrimPrefix(sLocalChartPath, dm.baseDir), dep.Repository)
 		}
 		return err
 	}
 
+	constraint, err := semver.NewConstraint(dep.Version)
+	if err != nil {
+		err = fmt.Errorf("invalid version/constraint format '%s': %w", dep.Version, err)
+		return err
+	}
+
 	ch, err := loader.Load(sLocalChartPath)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to load chart from '%s' (reference '%s'): %w",
+			strings.TrimPrefix(sLocalChartPath, dm.baseDir), dep.Repository, err)
 	}
 
-	constraint, err := semver.NewConstraint(dpr.Dependency.Version)
+	ver, err := semver.NewVersion(ch.Metadata.Version)
-	if err != nil {
-		err := fmt.Errorf("dependency '%s' has an invalid version/constraint format: %w", dpr.Dependency.Name, err)
-		return err
-	}
-
-	v, err := semver.NewVersion(ch.Metadata.Version)
 	if err != nil {
 		return err
 	}
 
-	if !constraint.Check(v) {
+	if !constraint.Check(ver) {
-		err = fmt.Errorf("can't get a valid version for dependency '%s'", dpr.Dependency.Name)
+		err = fmt.Errorf("can't get a valid version for constraint '%s'", dep.Version)
 		return err
 	}
 
 	dm.mu.Lock()
-	dm.Chart.AddDependency(ch)
+	dm.chart.AddDependency(ch)
 	dm.mu.Unlock()
-
 	return nil
 }
 
-func (dm *DependencyManager) addRemoteDependency(dpr *DependencyWithRepository) error {
+// addRemoteDependency attempts to resolve and add the given remote chart.Dependency to the chart.
-	if dpr.Repository == nil {
+func (dm *DependencyManager) addRemoteDependency(dep *helmchart.Dependency) error {
-		return fmt.Errorf("no HelmRepository for '%s' dependency", dpr.Dependency.Name)
+	repo, err := dm.resolveRepository(dep.Repository)
-	}
-
-	if !dpr.Repository.HasIndex() {
-		if !dpr.Repository.HasCacheFile() {
-			if _, err := dpr.Repository.CacheIndex(); err != nil {
-				return err
-			}
-		}
-		if err := dpr.Repository.LoadFromCache(); err != nil {
-			return err
-		}
-	}
-
-	chartVer, err := dpr.Repository.Get(dpr.Dependency.Name, dpr.Dependency.Version)
 	if err != nil {
 		return err
 	}
 
-	res, err := dpr.Repository.DownloadChart(chartVer)
+	if err = repo.StrategicallyLoadIndex(); err != nil {
+		return fmt.Errorf("failed to load index for '%s': %w", dep.Name, err)
+	}
+
+
+	ver, err := repo.Get(dep.Name, dep.Version)
 	if err != nil {
 		return err
 	}
-
+	res, err := repo.DownloadChart(ver)
+	if err != nil {
+		return fmt.Errorf("chart download of version '%s' failed: %w", ver.Version, err)
+	}
 	ch, err := loader.LoadArchive(res)
 	if err != nil {
-		return err
+		return fmt.Errorf("failed to load downloaded archive of version '%s': %w", ver.Version, err)
 	}
 
 	dm.mu.Lock()
-	dm.Chart.AddDependency(ch)
+	dm.chart.AddDependency(ch)
 	dm.mu.Unlock()
 	return nil
 }
 
-func (dm *DependencyManager) secureLocalChartPath(dep *DependencyWithRepository) (string, error) {
+// resolveRepository first attempts to resolve the url from the repositories, falling back
-	localUrl, err := url.Parse(dep.Dependency.Repository)
+// to getChartRepositoryCallback if set. It returns the resolved ChartRepository, or an error.
+func (dm *DependencyManager) resolveRepository(url string) (_ *ChartRepository, err error) {
+	dm.mu.Lock()
+	defer dm.mu.Unlock()
+
+	nUrl := NormalizeChartRepositoryURL(url)
+	if _, ok := dm.repositories[nUrl]; !ok {
+		if dm.getChartRepositoryCallback == nil {
+			err = fmt.Errorf("no chart repository for URL '%s'", nUrl)
+			return
+		}
+		if dm.repositories == nil {
+			dm.repositories = map[string]*ChartRepository{}
+		}
+		if dm.repositories[nUrl], err = dm.getChartRepositoryCallback(nUrl); err != nil {
+			err = fmt.Errorf("failed to get chart repository for URL '%s': %w", nUrl, err)
+			return
+		}
+	}
+	return dm.repositories[nUrl], nil
+}
+
+// secureLocalChartPath returns the secure absolute path of a local dependency.
+// It does not allow the dependency's path to be outside the scope of baseDir.
+func (dm *DependencyManager) secureLocalChartPath(dep *helmchart.Dependency) (string, error) {
+	localUrl, err := url.Parse(dep.Repository)
 	if err != nil {
 		return "", fmt.Errorf("failed to parse alleged local chart reference: %w", err)
 	}
 	if localUrl.Scheme != "" && localUrl.Scheme != "file" {
-		return "", fmt.Errorf("'%s' is not a local chart reference", dep.Dependency.Repository)
+		return "", fmt.Errorf("'%s' is not a local chart reference", dep.Repository)
 	}
-	return securejoin.SecureJoin(dm.WorkingDir, filepath.Join(dm.ChartPath, localUrl.Host, localUrl.Path))
+	return securejoin.SecureJoin(dm.baseDir, filepath.Join(dm.path, localUrl.Host, localUrl.Path))
+}
+
+// collectMissing returns a map with reqs that are missing from current,
+// indexed by their alias or name. All dependencies of a chart are present
+// if len of returned value == 0.
+func collectMissing(current []*helmchart.Chart, reqs []*helmchart.Dependency) map[string]*helmchart.Dependency {
+	// If the number of dependencies equals the number of requested
+	// dependencies, there are no missing dependencies
+	if len(current) == len(reqs) {
+		return nil
+	}
+
+	// Build up a map of reqs that are not in current, indexed by their
+	// alias or name
+	var missing map[string]*helmchart.Dependency
+	for _, dep := range reqs {
+		name := dep.Name
+		if dep.Alias != "" {
+			name = dep.Alias
+		}
+		// Exclude existing dependencies
+		found := false
+		for _, existing := range current {
+			if existing.Name() == name {
+				found = true
+			}
+		}
+		if found {
+			continue
+		}
+		if missing == nil {
+			missing = map[string]*helmchart.Dependency{}
+		}
+		missing[name] = dep
+	}
+	return missing
+}
+
+// isLocalDep returns true if the given chart.Dependency contains a local (file) path reference.
+func isLocalDep(dep *helmchart.Dependency) bool {
+	return dep.Repository == "" || strings.HasPrefix(dep.Repository, "file://")
 }

internal/helm/dependency_manager_test.go

@@ -18,12 +18,16 @@ package helm
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
-	"strings"
+	"path/filepath"
+	"sync"
 	"testing"
 
+	. "github.com/onsi/gomega"
 	helmchart "helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/chart/loader"
 	"helm.sh/helm/v3/pkg/repo"
 )
 
@@ -47,177 +51,585 @@ var (
 	chartVersionV1    = "0.3.0"
 )
 
-func TestBuild_WithEmptyDependencies(t *testing.T) {
+func TestDependencyManager_Build(t *testing.T) {
-	dm := DependencyManager{
-		Dependencies: nil,
-	}
-	if err := dm.Build(context.TODO()); err != nil {
-		t.Errorf("Build() should return nil")
-	}
-}
-
-func TestBuild_WithLocalChart(t *testing.T) {
 	tests := []struct {
 		name                       string
-		dep     helmchart.Dependency
+		baseDir                    string
-		wantErr bool
+		path                       string
-		errMsg  string
+		repositories               map[string]*ChartRepository
+		getChartRepositoryCallback GetChartRepositoryCallback
+		want                       int
+		wantChartFunc              func(g *WithT, c *helmchart.Chart)
+		wantErr                    string
 	}{
+		//{
+		//	// TODO(hidde): add various happy paths
+		//},
+		//{
+		//	// TODO(hidde): test Chart.lock
+		//},
 		{
-			name: "valid path",
+			name:    "build failure returns error",
-			dep: helmchart.Dependency{
+			baseDir: "testdata/charts",
-				Name:       chartName,
+			path:    "helmchartwithdeps",
-				Version:    chartVersion,
+			wantErr: "failed to add remote dependency 'grafana': no chart repository for URL",
-				Repository: chartLocalRepository,
-			},
 		},
 		{
-			name: "valid path",
+			name:    "no dependencies returns zero",
-			dep: helmchart.Dependency{
+			baseDir: "testdata/charts",
-				Name:       chartName,
+			path:    "helmchart",
-				Alias:      "aliased",
+			want:    0,
-				Version:    chartVersion,
-				Repository: chartLocalRepository,
-			},
-		},
-		{
-			name: "allowed traversing path",
-			dep: helmchart.Dependency{
-				Name:       chartName,
-				Alias:      "aliased",
-				Version:    chartVersion,
-				Repository: "file://../../../testdata/charts/helmchartwithdeps/../helmchart",
-			},
-		},
-		{
-			name: "invalid path",
-			dep: helmchart.Dependency{
-				Name:       chartName,
-				Version:    chartVersion,
-				Repository: "file://../invalid",
-			},
-			wantErr: true,
-			errMsg:  "no chart found at",
-		},
-		{
-			name: "illegal traversing path",
-			dep: helmchart.Dependency{
-				Name:       chartName,
-				Version:    chartVersion,
-				Repository: "file://../../../../../controllers/testdata/charts/helmchart",
-			},
-			wantErr: true,
-			errMsg:  "no chart found at",
-		},
-		{
-			name: "invalid version constraint format",
-			dep: helmchart.Dependency{
-				Name:       chartName,
-				Version:    "!2.0",
-				Repository: chartLocalRepository,
-			},
-			wantErr: true,
-			errMsg:  "has an invalid version/constraint format",
-		},
-		{
-			name: "invalid version",
-			dep: helmchart.Dependency{
-				Name:       chartName,
-				Version:    chartVersion,
-				Repository: chartLocalRepository,
-			},
-			wantErr: true,
-			errMsg:  "can't get a valid version for dependency",
 		},
 	}
-
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			c := chartFixture
+			g := NewWithT(t)
-			dm := DependencyManager{
-				WorkingDir: "./",
-				ChartPath:  "testdata/charts/helmchart",
-				Chart:      &c,
-				Dependencies: []*DependencyWithRepository{
-					{
-						Dependency: &tt.dep,
-						Repository: nil,
-					},
-				},
-			}
 
-			err := dm.Build(context.TODO())
+			chart, err := loader.Load(filepath.Join(tt.baseDir, tt.path))
-			deps := dm.Chart.Dependencies()
+			g.Expect(err).ToNot(HaveOccurred())
 
-			if (err != nil) && tt.wantErr {
+			got, err := NewDependencyManager(chart, tt.baseDir, tt.path).
-				if !strings.Contains(err.Error(), tt.errMsg) {
+				WithRepositories(tt.repositories).
-					t.Errorf("Build() expected to return error: %s, got: %s", tt.errMsg, err)
+				WithChartRepositoryCallback(tt.getChartRepositoryCallback).
-				}
+				Build(context.TODO())
-				if len(deps) > 0 {
+
-					t.Fatalf("chart expected to have no dependencies registered")
+			if tt.wantErr != "" {
-				}
+				g.Expect(err).To(HaveOccurred())
-				return
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
-			} else if err != nil {
+				g.Expect(got).To(BeZero())
-				t.Errorf("Build() not expected to return an error: %s", err)
 				return
 			}
 
-			if len(deps) == 0 {
+			g.Expect(err).ToNot(HaveOccurred())
-				t.Fatalf("chart expected to have at least one dependency registered")
+			g.Expect(got).To(Equal(tt.want))
-			}
+			if tt.wantChartFunc != nil {
-			if deps[0].Metadata.Name != chartName {
+				tt.wantChartFunc(g, chart)
-				t.Errorf("chart dependency has incorrect name, expected: %s, got: %s", chartName, deps[0].Metadata.Name)
-			}
-			if deps[0].Metadata.Version != chartVersion {
-				t.Errorf("chart dependency has incorrect version, expected: %s, got: %s", chartVersion, deps[0].Metadata.Version)
 			}
 		})
 	}
 }
 
-func TestBuild_WithRemoteChart(t *testing.T) {
+func TestDependencyManager_build(t *testing.T) {
-	chart := chartFixture
+	tests := []struct {
-	b, err := os.ReadFile(helmPackageFile)
+		name    string
-	if err != nil {
+		deps    map[string]*helmchart.Dependency
-		t.Fatal(err)
+		wantErr string
-	}
+	}{
-	i := repo.NewIndexFile()
-	i.MustAdd(&helmchart.Metadata{Name: chartName, Version: chartVersion}, fmt.Sprintf("%s-%s.tgz", chartName, chartVersion), "http://example.com/charts", "sha256:1234567890")
-	mg := mockGetter{response: b}
-	cr := newChartRepository()
-	cr.URL = remoteDepFixture.Repository
-	cr.Index = i
-	cr.Client = &mg
-	dm := DependencyManager{
-		Chart: &chart,
-		Dependencies: []*DependencyWithRepository{
 		{
-				Dependency: &remoteDepFixture,
+			name: "error remote dependency",
-				Repository: cr,
+			deps: map[string]*helmchart.Dependency{
+				"example": {Repository: "https://example.com"},
 			},
+			wantErr: "failed to add remote dependency",
+		},
+		{
+			name: "error local dependency",
+			deps: map[string]*helmchart.Dependency{
+				"example": {Repository: "file:///invalid"},
+			},
+			wantErr: "failed to add remote dependency",
 		},
 	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
 
-	if err := dm.Build(context.TODO()); err != nil {
+			dm := &DependencyManager{
-		t.Errorf("Build() expected to not return error: %s", err)
+				baseDir: "testdata/charts",
+			}
+			err := dm.build(context.TODO(), tt.deps)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				return
 			}
 
-	deps := dm.Chart.Dependencies()
+			g.Expect(err).ToNot(HaveOccurred())
-	if len(deps) != 1 {
+		})
-		t.Fatalf("chart expected to have one dependency registered")
 	}
-	if deps[0].Metadata.Name != chartName {
-		t.Errorf("chart dependency has incorrect name, expected: %s, got: %s", chartName, deps[0].Metadata.Name)
-	}
-	if deps[0].Metadata.Version != chartVersion {
-		t.Errorf("chart dependency has incorrect version, expected: %s, got: %s", chartVersion, deps[0].Metadata.Version)
 }
 
-	// When repo is not set
+func TestDependencyManager_addLocalDependency(t *testing.T) {
-	dm.Dependencies[0].Repository = nil
+	tests := []struct {
-	if err := dm.Build(context.TODO()); err == nil {
+		name     string
-		t.Errorf("Build() expected to return error")
+		dep      *helmchart.Dependency
-	} else if !strings.Contains(err.Error(), "is not a local chart reference") {
+		wantErr  string
-		t.Errorf("Build() expected to return different error, got: %s", err)
+		wantFunc func(g *WithT, c *helmchart.Chart)
+	}{
+		{
+			name: "local dependency",
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    chartVersion,
+				Repository: "file://../helmchart",
+			},
+			wantFunc: func(g *WithT, c *helmchart.Chart) {
+				g.Expect(c.Dependencies()).To(HaveLen(1))
+			},
+		},
+		{
+			name: "version not matching constraint",
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    "0.2.0",
+				Repository: "file://../helmchart",
+			},
+			wantErr: "can't get a valid version for constraint '0.2.0'",
+		},
+		{
+			name: "invalid local reference",
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    chartVersion,
+				Repository: "file://../../../absolutely/invalid",
+			},
+			wantErr: "no chart found at 'absolutely/invalid'",
+		},
+		{
+			name: "invalid chart archive",
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    chartVersion,
+				Repository: "file://../empty.tgz",
+			},
+			wantErr: "failed to load chart from 'empty.tgz'",
+		},
+		{
+			name: "invalid constraint",
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    "invalid",
+				Repository: "file://../helmchart",
+			},
+			wantErr: "invalid version/constraint format 'invalid'",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			dm := &DependencyManager{
+				chart:   &helmchart.Chart{},
+				baseDir: "testdata/charts/",
+				path:    "helmchartwithdeps",
+			}
+
+			err := dm.addLocalDependency(tt.dep)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				return
+			}
+			g.Expect(err).ToNot(HaveOccurred())
+		})
+	}
+}
+
+func TestDependencyManager_addRemoteDependency(t *testing.T) {
+	g := NewWithT(t)
+
+	chartB, err := os.ReadFile("testdata/charts/helmchart-0.1.0.tgz")
+	g.Expect(err).ToNot(HaveOccurred())
+	g.Expect(chartB).ToNot(BeEmpty())
+
+	tests := []struct {
+		name         string
+		repositories map[string]*ChartRepository
+		dep          *helmchart.Dependency
+		wantFunc     func(g *WithT, c *helmchart.Chart)
+		wantErr      string
+	}{
+		{
+			name: "adds remote dependency",
+			repositories: map[string]*ChartRepository{
+				"https://example.com/": {
+					Client: &mockGetter{
+						response: chartB,
+					},
+					Index: &repo.IndexFile{
+						Entries: map[string]repo.ChartVersions{
+							chartName: {
+								&repo.ChartVersion{
+									Metadata: &helmchart.Metadata{
+										Name:    chartName,
+										Version: chartVersion,
+									},
+									URLs: []string{"https://example.com/foo.tgz"},
+								},
+							},
+						},
+					},
+					RWMutex: &sync.RWMutex{},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Repository: "https://example.com",
+			},
+			wantFunc: func(g *WithT, c *helmchart.Chart) {
+				g.Expect(c.Dependencies()).To(HaveLen(1))
+			},
+		},
+		{
+			name:         "resolve repository error",
+			repositories: map[string]*ChartRepository{},
+			dep: &helmchart.Dependency{
+				Repository: "https://example.com",
+			},
+			wantErr: "no chart repository for URL",
+		},
+		{
+			name: "strategic load error",
+			repositories: map[string]*ChartRepository{
+				"https://example.com/": {
+					CachePath: "/invalid/cache/path/foo",
+					RWMutex:   &sync.RWMutex{},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Repository: "https://example.com",
+			},
+			wantErr: "failed to strategically load index",
+		},
+		{
+			name: "repository get error",
+			repositories: map[string]*ChartRepository{
+				"https://example.com/": {
+					Index:   &repo.IndexFile{},
+					RWMutex: &sync.RWMutex{},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Repository: "https://example.com",
+			},
+			wantErr: "no chart name found",
+		},
+		{
+			name: "repository version constraint error",
+			repositories: map[string]*ChartRepository{
+				"https://example.com/": {
+					Index: &repo.IndexFile{
+						Entries: map[string]repo.ChartVersions{
+							chartName: {
+								&repo.ChartVersion{
+									Metadata: &helmchart.Metadata{
+										Name:    chartName,
+										Version: "0.1.0",
+									},
+								},
+							},
+						},
+					},
+					RWMutex: &sync.RWMutex{},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    "0.2.0",
+				Repository: "https://example.com",
+			},
+			wantErr: fmt.Sprintf("no '%s' chart with version matching '0.2.0' found", chartName),
+		},
+		{
+			name: "repository chart download error",
+			repositories: map[string]*ChartRepository{
+				"https://example.com/": {
+					Index: &repo.IndexFile{
+						Entries: map[string]repo.ChartVersions{
+							chartName: {
+								&repo.ChartVersion{
+									Metadata: &helmchart.Metadata{
+										Name:    chartName,
+										Version: chartVersion,
+									},
+								},
+							},
+						},
+					},
+					RWMutex: &sync.RWMutex{},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    chartVersion,
+				Repository: "https://example.com",
+			},
+			wantErr: "chart download of version '0.1.0' failed",
+		},
+		{
+			name: "chart load error",
+			repositories: map[string]*ChartRepository{
+				"https://example.com/": {
+					Client: &mockGetter{},
+					Index: &repo.IndexFile{
+						Entries: map[string]repo.ChartVersions{
+							chartName: {
+								&repo.ChartVersion{
+									Metadata: &helmchart.Metadata{
+										Name:    chartName,
+										Version: chartVersion,
+									},
+									URLs: []string{"https://example.com/foo.tgz"},
+								},
+							},
+						},
+					},
+					RWMutex: &sync.RWMutex{},
+				},
+			},
+			dep: &helmchart.Dependency{
+				Name:       chartName,
+				Version:    chartVersion,
+				Repository: "https://example.com",
+			},
+			wantErr: "failed to load downloaded archive of version '0.1.0'",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			dm := &DependencyManager{
+				chart:        &helmchart.Chart{},
+				repositories: tt.repositories,
+			}
+			err := dm.addRemoteDependency(tt.dep)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				return
+			}
+			g.Expect(err).ToNot(HaveOccurred())
+			if tt.wantFunc != nil {
+				tt.wantFunc(g, dm.chart)
+			}
+		})
+	}
+}
+
+func TestDependencyManager_resolveRepository(t *testing.T) {
+	tests := []struct {
+		name                       string
+		repositories               map[string]*ChartRepository
+		getChartRepositoryCallback GetChartRepositoryCallback
+		url                        string
+		want                       *ChartRepository
+		wantRepositories           map[string]*ChartRepository
+		wantErr                    string
+	}{
+		{
+			name: "resolves from repositories index",
+			url:  "https://example.com",
+			repositories: map[string]*ChartRepository{
+				"https://example.com/": {URL: "https://example.com"},
+			},
+			want: &ChartRepository{URL: "https://example.com"},
+		},
+		{
+			name: "resolves from callback",
+			url:  "https://example.com",
+			getChartRepositoryCallback: func(url string) (*ChartRepository, error) {
+				return &ChartRepository{URL: "https://example.com"}, nil
+			},
+			want: &ChartRepository{URL: "https://example.com"},
+			wantRepositories: map[string]*ChartRepository{
+				"https://example.com/": {URL: "https://example.com"},
+			},
+		},
+		{
+			name: "error from callback",
+			url:  "https://example.com",
+			getChartRepositoryCallback: func(url string) (*ChartRepository, error) {
+				return nil, errors.New("a very unique error")
+			},
+			wantErr:          "a very unique error",
+			wantRepositories: map[string]*ChartRepository{},
+		},
+		{
+			name:    "error on not found",
+			url:     "https://example.com",
+			wantErr: "no chart repository for URL",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			dm := &DependencyManager{
+				repositories:               tt.repositories,
+				getChartRepositoryCallback: tt.getChartRepositoryCallback,
+			}
+
+			got, err := dm.resolveRepository(tt.url)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				g.Expect(got).To(BeNil())
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(got).To(Equal(tt.want))
+			if tt.wantRepositories != nil {
+				g.Expect(dm.repositories).To(Equal(tt.wantRepositories))
+			}
+		})
+	}
+}
+
+func TestDependencyManager_secureLocalChartPath(t *testing.T) {
+	tests := []struct {
+		name    string
+		baseDir string
+		path    string
+		dep     *helmchart.Dependency
+		want    string
+		wantErr string
+	}{
+		{
+			name:    "secure local file path",
+			baseDir: "/tmp/workdir",
+			path:    "/chart",
+			dep: &helmchart.Dependency{
+				Repository: "../dep",
+			},
+			want: "/tmp/workdir/dep",
+		},
+		{
+			name:    "insecure local file path",
+			baseDir: "/tmp/workdir",
+			path:    "/",
+			dep: &helmchart.Dependency{
+				Repository: "/../../dep",
+			},
+			want: "/tmp/workdir/dep",
+		},
+		{
+			name: "URL parse error",
+			dep: &helmchart.Dependency{
+				Repository: ": //example.com",
+			},
+			wantErr: "missing protocol scheme",
+		},
+		{
+			name: "error on URL scheme other than file",
+			dep: &helmchart.Dependency{
+				Repository: "https://example.com",
+			},
+			wantErr: "not a local chart reference",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			dm := &DependencyManager{
+				baseDir: tt.baseDir,
+				path:    tt.path,
+			}
+			got, err := dm.secureLocalChartPath(tt.dep)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				return
+			}
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(got).ToNot(BeEmpty())
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}
+
+func Test_collectMissing(t *testing.T) {
+	tests := []struct {
+		name    string
+		current []*helmchart.Chart
+		reqs    []*helmchart.Dependency
+		want    map[string]*helmchart.Dependency
+	}{
+		{
+			name:    "one missing",
+			current: []*helmchart.Chart{},
+			reqs: []*helmchart.Dependency{
+				{Name: chartName},
+			},
+			want: map[string]*helmchart.Dependency{
+				chartName: {Name: chartName},
+			},
+		},
+		{
+			name: "alias missing",
+			current: []*helmchart.Chart{
+				{
+					Metadata: &helmchart.Metadata{
+						Name: chartName,
+					},
+				},
+			},
+			reqs: []*helmchart.Dependency{
+				{Name: chartName},
+				{Name: chartName, Alias: chartName + "-alias"},
+			},
+			want: map[string]*helmchart.Dependency{
+				chartName + "-alias": {Name: chartName, Alias: chartName + "-alias"},
+			},
+		},
+		{
+			name: "all current",
+			current: []*helmchart.Chart{
+				{
+					Metadata: &helmchart.Metadata{
+						Name: chartName,
+					},
+				},
+			},
+			reqs: []*helmchart.Dependency{
+				{Name: chartName},
+			},
+			want: nil,
+		},
+		{
+			name:    "nil",
+			current: nil,
+			reqs:    nil,
+			want:    nil,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Run(tt.name, func(t *testing.T) {
+				g := NewWithT(t)
+				g.Expect(collectMissing(tt.current, tt.reqs)).To(Equal(tt.want))
+			})
+		})
+	}
+}
+
+func Test_isLocalDep(t *testing.T) {
+	tests := []struct {
+		name string
+		dep  *helmchart.Dependency
+		want bool
+	}{
+		{
+			name: "file protocol",
+			dep:  &helmchart.Dependency{Repository: "file:///some/path"},
+			want: true,
+		},
+		{
+			name: "empty",
+			dep:  &helmchart.Dependency{Repository: ""},
+			want: true,
+		},
+		{
+			name: "https url",
+			dep:  &helmchart.Dependency{Repository: "https://example.com"},
+			want: false,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			g.Expect(isLocalDep(tt.dep)).To(Equal(tt.want))
+		})
 	}
 }

internal/helm/repository.go

@@ -54,6 +54,9 @@ type ChartRepository struct {
 	Options []getter.Option
 	// CachePath is the path of a cached index.yaml for read-only operations.
 	CachePath string
+	// Cached indicates if the ChartRepository index.yaml has been cached
+	// to CachePath.
+	Cached bool
 	// Index contains a loaded chart repository index if not nil.
 	Index *repo.IndexFile
 	// Checksum contains the SHA256 checksum of the loaded chart repository
@@ -68,7 +71,6 @@ type ChartRepository struct {
 // repository URL scheme. It returns an error on URL parsing failures,
 // or if there is no getter available for the scheme.
 func NewChartRepository(repositoryURL, cachePath string, providers getter.Providers, opts []getter.Option) (*ChartRepository, error) {
-	r := newChartRepository()
 	u, err := url.Parse(repositoryURL)
 	if err != nil {
 		return nil, err
@@ -77,6 +79,8 @@ func NewChartRepository(repositoryURL, cachePath string, providers getter.Provid
 	if err != nil {
 		return nil, err
 	}
+
+	r := newChartRepository()
 	r.URL = repositoryURL
 	r.CachePath = cachePath
 	r.Client = c
@@ -238,7 +242,7 @@ func (r *ChartRepository) LoadFromFile(path string) error {
 }
 
 // CacheIndex attempts to write the index from the remote into a new temporary file
-// using DownloadIndex, and sets CachePath.
+// using DownloadIndex, and sets CachePath and Cached.
 // It returns the SHA256 checksum of the downloaded index bytes, or an error.
 // The caller is expected to handle the garbage collection of CachePath, and to
 // load the Index separately using LoadFromCache if required.
@@ -262,19 +266,40 @@ func (r *ChartRepository) CacheIndex() (string, error) {
 
 	r.Lock()
 	r.CachePath = f.Name()
+	r.Cached = true
 	r.Unlock()
 	return hex.EncodeToString(h.Sum(nil)), nil
 }
 
+// StrategicallyLoadIndex lazy-loads the Index from CachePath using
+// LoadFromCache if it does not HasIndex.
+// If it not HasCacheFile, a cache attempt is made using CacheIndex
+// before continuing to load.
+// It returns a boolean indicating if it cached the index before
+// loading, or an error.
+func (r *ChartRepository) StrategicallyLoadIndex() (err error) {
+	if r.HasIndex() {
+		return
+	}
+	if !r.HasCacheFile() {
+		if _, err = r.CacheIndex(); err != nil {
+			err = fmt.Errorf("failed to strategically load index: %w", err)
+			return
+		}
+	}
+	if err = r.LoadFromCache(); err != nil {
+		err = fmt.Errorf("failed to strategically load index: %w", err)
+		return
+	}
+	return
+}
+
 // LoadFromCache attempts to load the Index from the configured CachePath.
 // It returns an error if no CachePath is set, or if the load failed.
 func (r *ChartRepository) LoadFromCache() error {
-	r.RLock()
 	if cachePath := r.CachePath; cachePath != "" {
-		r.RUnlock()
 		return r.LoadFromFile(cachePath)
 	}
-	r.RUnlock()
 	return fmt.Errorf("no cache path set")
 }
 
@@ -314,11 +339,34 @@ func (r *ChartRepository) HasCacheFile() bool {
 	return r.CachePath != ""
 }
 
-// UnloadIndex sets the Index to nil.
+// Unload can be used to signal the Go garbage collector the Index can
-func (r *ChartRepository) UnloadIndex() {
+// be freed from memory if the ChartRepository object is expected to
-	if r != nil {
+// continue to exist in the stack for some time.
+func (r *ChartRepository) Unload()  {
+	if r == nil {
+		return
+	}
+
 	r.Lock()
+	defer r.Unlock()
 	r.Index = nil
-		r.Unlock()
 }
+
+// RemoveCache removes the CachePath if Cached.
+func (r *ChartRepository) RemoveCache() error {
+	if r == nil {
+		return nil
+	}
+
+	r.Lock()
+	defer r.Unlock()
+
+	if r.Cached {
+		if err := os.Remove(r.CachePath); err != nil && !os.IsNotExist(err) {
+			return err
+		}
+		r.CachePath = ""
+		r.Cached = false
+	}
+	return nil
 }

internal/helm/repository_test.go

@@ -47,7 +47,8 @@ type mockGetter struct {
 
 func (g *mockGetter) Get(url string, _ ...getter.Option) (*bytes.Buffer, error) {
 	g.requestedURL = url
-	return bytes.NewBuffer(g.response), nil
+	r := g.response
+	return bytes.NewBuffer(r), nil
 }
 
 func TestNewChartRepository(t *testing.T) {
@@ -402,7 +403,7 @@ func TestChartRepository_CacheIndex(t *testing.T) {
 	g.Expect(sum).To(BeEquivalentTo(expectSum))
 }
 
-func TestChartRepository_LoadIndexFromCache(t *testing.T) {
+func TestChartRepository_LoadFromCache(t *testing.T) {
 	tests := []struct {
 		name      string
 		cachePath string
@@ -458,7 +459,7 @@ func TestChartRepository_UnloadIndex(t *testing.T) {
 	r := newChartRepository()
 	g.Expect(r.HasIndex()).To(BeFalse())
 	r.Index = repo.NewIndexFile()
-	r.UnloadIndex()
+	r.Unload()
 	g.Expect(r.Index).To(BeNil())
 }
 

internal/helm/testdata/charts/helmchart/values-prod.yaml

@@ -0,0 +1 @@
+replicaCount: 2

internal/helm/testdata/charts/helmchartwithdeps/Chart.lock

@@ -0,0 +1,12 @@
+dependencies:
+- name: helmchart
+  repository: file://../helmchart
+  version: 0.1.0
+- name: helmchart
+  repository: file://../helmchart
+  version: 0.1.0
+- name: grafana
+  repository: https://grafana.github.io/helm-charts
+  version: 6.17.4
+digest: sha256:1e41c97e27347f433ff0212bf52c344bc82dd435f70129d15e96cd2c8fcc32bb
+generated: "2021-11-02T01:25:59.624290788+01:00"