internal/helm: add repository cache helpers

This commits adds simple caching capabilities to the
`ChartRepository`, which makes it possible to load the `Index` from a
defined `CachePath` using `LoadFromCache()`, and to download the index
to a new `CachePath` using `CacheIndex()`.

In addition, the repository tests have been updated to make use of
Gomega, and some missing ones have been added.

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

internal/helm/repository.go

@@ -18,12 +18,17 @@ package helm
 
 import (
 	"bytes"
+	"crypto/sha256"
+	"encoding/hex"
+	"errors"
 	"fmt"
 	"io"
 	"net/url"
+	"os"
 	"path"
 	"sort"
 	"strings"
+	"sync"
 
 	"github.com/Masterminds/semver/v3"
 	"helm.sh/helm/v3/pkg/getter"
@@ -33,20 +38,37 @@ import (
 	"github.com/fluxcd/pkg/version"
 )
 
+var ErrNoChartIndex = errors.New("no chart index")
+
 // ChartRepository represents a Helm chart repository, and the configuration
-// required to download the chart index, and charts from the repository.
+// required to download the chart index and charts from the repository.
+// All methods are thread safe unless defined otherwise.
 type ChartRepository struct {
-	URL     string
+	// URL the ChartRepository's index.yaml can be found at,
-	Index   *repo.IndexFile
+	// without the index.yaml suffix.
-	Client  getter.Getter
+	URL string
+	// Client to use while downloading the Index or a chart from the URL.
+	Client getter.Getter
+	// Options to configure the Client with while downloading the Index
+	// or a chart from the URL.
 	Options []getter.Option
+	// CachePath is the path of a cached index.yaml for read-only operations.
+	CachePath string
+	// Index contains a loaded chart repository index if not nil.
+	Index *repo.IndexFile
+	// Checksum contains the SHA256 checksum of the loaded chart repository
+	// index bytes.
+	Checksum string
+
+	*sync.RWMutex
 }
 
 // NewChartRepository constructs and returns a new ChartRepository with
 // the ChartRepository.Client configured to the getter.Getter for the
 // repository URL scheme. It returns an error on URL parsing failures,
 // or if there is no getter available for the scheme.
-func NewChartRepository(repositoryURL string, providers getter.Providers, opts []getter.Option) (*ChartRepository, error) {
+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
@@ -55,17 +77,29 @@ func NewChartRepository(repositoryURL string, providers getter.Providers, opts [
 	if err != nil {
 		return nil, err
 	}
+	r.URL = repositoryURL
+	r.CachePath = cachePath
+	r.Client = c
+	r.Options = opts
+	return r, nil
+}
+
+func newChartRepository() *ChartRepository {
 	return &ChartRepository{
-		URL:     repositoryURL,
+		RWMutex: &sync.RWMutex{},
-		Client:  c,
+	}
-		Options: opts,
-	}, nil
 }
 
 // Get returns the repo.ChartVersion for the given name, the version is expected
 // to be a semver.Constraints compatible string. If version is empty, the latest
 // stable version will be returned and prerelease versions will be ignored.
 func (r *ChartRepository) Get(name, ver string) (*repo.ChartVersion, error) {
+	r.RLock()
+	defer r.RUnlock()
+
+	if r.Index == nil {
+		return nil, ErrNoChartIndex
+	}
 	cvs, ok := r.Index.Entries[name]
 	if !ok {
 		return nil, repo.ErrNoChartName
@@ -114,7 +148,7 @@ func (r *ChartRepository) Get(name, ver string) (*repo.ChartVersion, error) {
 		lookup[v] = cv
 	}
 	if len(matchedVersions) == 0 {
-		return nil, fmt.Errorf("no chart version found for %s-%s", name, ver)
+		return nil, fmt.Errorf("no '%s' chart with version matching '%s' found", name, ver)
 	}
 
 	// Sort versions
@@ -145,7 +179,7 @@ func (r *ChartRepository) Get(name, ver string) (*repo.ChartVersion, error) {
 // ChartRepository. It returns a bytes.Buffer containing the chart data.
 func (r *ChartRepository) DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer, error) {
 	if len(chart.URLs) == 0 {
-		return nil, fmt.Errorf("chart %q has no downloadable URLs", chart.Name)
+		return nil, fmt.Errorf("chart '%s' has no downloadable URLs", chart.Name)
 	}
 
 	// TODO(hidde): according to the Helm source the first item is not
@@ -175,13 +209,9 @@ func (r *ChartRepository) DownloadChart(chart *repo.ChartVersion) (*bytes.Buffer
 	return r.Client.Get(u.String(), r.Options...)
 }
 
-// LoadIndex loads the given bytes into the Index while performing
+// LoadIndexFromBytes loads Index from the given bytes.
-// minimal validity checks. It fails if the API version is not set
+// It returns a repo.ErrNoAPIVersion error if the API version is not set
-// (repo.ErrNoAPIVersion), or if the unmarshal fails.
+func (r *ChartRepository) LoadIndexFromBytes(b []byte) error {
-//
-// The logic is derived from and on par with:
-// https://github.com/helm/helm/blob/v3.3.4/pkg/repo/index.go#L301
-func (r *ChartRepository) LoadIndex(b []byte) error {
 	i := &repo.IndexFile{}
 	if err := yaml.UnmarshalStrict(b, i); err != nil {
 		return err
@@ -190,14 +220,68 @@ func (r *ChartRepository) LoadIndex(b []byte) error {
 		return repo.ErrNoAPIVersion
 	}
 	i.SortEntries()
+
+	r.Lock()
 	r.Index = i
+	r.Checksum = fmt.Sprintf("%x", sha256.Sum256(b))
+	r.Unlock()
 	return nil
 }
 
+// LoadFromFile reads the file at the given path and loads it into Index.
+func (r *ChartRepository) LoadFromFile(path string) error {
+	b, err := os.ReadFile(path)
+	if err != nil {
+		return err
+	}
+	return r.LoadIndexFromBytes(b)
+}
+
+// CacheIndex attempts to write the index from the remote into a new temporary file
+// using DownloadIndex, and sets CachePath.
+// 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.
+func (r *ChartRepository) CacheIndex() (string, error) {
+	f, err := os.CreateTemp("", "chart-index-*.yaml")
+	if err != nil {
+		return "", fmt.Errorf("failed to create temp file to cache index to: %w", err)
+	}
+
+	h := sha256.New()
+	mw := io.MultiWriter(f, h)
+	if err = r.DownloadIndex(mw); err != nil {
+		f.Close()
+		os.RemoveAll(f.Name())
+		return "", fmt.Errorf("failed to cache index to '%s': %w", f.Name(), err)
+	}
+	if err = f.Close(); err != nil {
+		os.RemoveAll(f.Name())
+		return "", fmt.Errorf("failed to close cached index file '%s': %w", f.Name(), err)
+	}
+
+	r.Lock()
+	r.CachePath = f.Name()
+	r.Unlock()
+	return hex.EncodeToString(h.Sum(nil)), nil
+}
+
+// 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")
+}
+
 // DownloadIndex attempts to download the chart repository index using
-// the Client and set Options, and loads the index file into the Index.
+// the Client and set Options, and writes the index to the given io.Writer.
-// It returns an error on URL parsing and Client failures.
+// It returns an url.Error if the URL failed to parse.
-func (r *ChartRepository) DownloadIndex() error {
+func (r *ChartRepository) DownloadIndex(w io.Writer) (err error) {
 	u, err := url.Parse(r.URL)
 	if err != nil {
 		return err
@@ -205,14 +289,36 @@ func (r *ChartRepository) DownloadIndex() error {
 	u.RawPath = path.Join(u.RawPath, "index.yaml")
 	u.Path = path.Join(u.Path, "index.yaml")
 
-	res, err := r.Client.Get(u.String(), r.Options...)
+	var res *bytes.Buffer
+	res, err = r.Client.Get(u.String(), r.Options...)
 	if err != nil {
 		return err
 	}
-	b, err := io.ReadAll(res)
+	if _, err = io.Copy(w, res); err != nil {
-	if err != nil {
 		return err
 	}
-
+	return nil
-	return r.LoadIndex(b)
+}
+
+// HasIndex returns true if the Index is not nil.
+func (r *ChartRepository) HasIndex() bool {
+	r.RLock()
+	defer r.RUnlock()
+	return r.Index != nil
+}
+
+// HasCacheFile returns true if CachePath is not empty.
+func (r *ChartRepository) HasCacheFile() bool {
+	r.RLock()
+	defer r.RUnlock()
+	return r.CachePath != ""
+}
+
+// UnloadIndex sets the Index to nil.
+func (r *ChartRepository) UnloadIndex() {
+	if r != nil {
+		r.Lock()
+		r.Index = nil
+		r.Unlock()
+	}
 }

internal/helm/repository_test.go

@@ -18,45 +18,38 @@ package helm
 
 import (
 	"bytes"
+	"crypto/sha256"
+	"fmt"
 	"net/url"
 	"os"
-	"reflect"
-	"strings"
 	"testing"
 	"time"
 
+	. "github.com/onsi/gomega"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/getter"
 	"helm.sh/helm/v3/pkg/repo"
 )
 
+var now = time.Now()
+
 const (
-	testfile            = "testdata/local-index.yaml"
+	testFile            = "testdata/local-index.yaml"
-	chartmuseumtestfile = "testdata/chartmuseum-index.yaml"
+	chartmuseumTestFile = "testdata/chartmuseum-index.yaml"
-	unorderedtestfile   = "testdata/local-index-unordered.yaml"
+	unorderedTestFile   = "testdata/local-index-unordered.yaml"
-	indexWithDuplicates = `
-apiVersion: v1
-entries:
-  nginx:
-    - urls:
-        - https://kubernetes-charts.storage.googleapis.com/nginx-0.2.0.tgz
-      name: nginx
-      description: string
-      version: 0.2.0
-      home: https://github.com/something/else
-      digest: "sha256:1234567890abcdef"
-  nginx:
-    - urls:
-        - https://kubernetes-charts.storage.googleapis.com/alpine-1.0.0.tgz
-        - http://storage2.googleapis.com/kubernetes-charts/alpine-1.0.0.tgz
-      name: alpine
-      description: string
-      version: 1.0.0
-      home: https://github.com/something
-      digest: "sha256:1234567890abcdef"
-`
 )
 
+// mockGetter can be used as a simple mocking getter.Getter implementation.
+type mockGetter struct {
+	requestedURL string
+	response     []byte
+}
+
+func (g *mockGetter) Get(url string, _ ...getter.Option) (*bytes.Buffer, error) {
+	g.requestedURL = url
+	return bytes.NewBuffer(g.response), nil
+}
+
 func TestNewChartRepository(t *testing.T) {
 	repositoryURL := "https://example.com"
 	providers := getter.Providers{
@@ -68,60 +61,74 @@ func TestNewChartRepository(t *testing.T) {
 	options := []getter.Option{getter.WithBasicAuth("username", "password")}
 
 	t.Run("should construct chart repository", func(t *testing.T) {
-		r, err := NewChartRepository(repositoryURL, providers, options)
+		g := NewWithT(t)
-		if err != nil {
+
-			t.Error(err)
+		r, err := NewChartRepository(repositoryURL, "", providers, options)
-		}
+		g.Expect(err).ToNot(HaveOccurred())
-		if got := r.URL; got != repositoryURL {
+		g.Expect(r).ToNot(BeNil())
-			t.Fatalf("Expecting %q repository URL, got: %q", repositoryURL, got)
+		g.Expect(r.URL).To(Equal(repositoryURL))
-		}
+		g.Expect(r.Client).ToNot(BeNil())
-		if r.Client == nil {
+		g.Expect(r.Options).To(Equal(options))
-			t.Fatalf("Expecting client, got nil")
-		}
-		if !reflect.DeepEqual(r.Options, options) {
-			t.Fatalf("Client options mismatth")
-		}
 	})
 
 	t.Run("should error on URL parsing failure", func(t *testing.T) {
-		_, err := NewChartRepository("https://ex ample.com", nil, nil)
+		g := NewWithT(t)
-		switch err.(type) {
+		r, err := NewChartRepository("https://ex ample.com", "", nil, nil)
-		case *url.Error:
+		g.Expect(err).To(HaveOccurred())
-		default:
+		g.Expect(err).To(BeAssignableToTypeOf(&url.Error{}))
-			t.Fatalf("Expecting URL error, got: %v", err)
+		g.Expect(r).To(BeNil())
-		}
+
 	})
 
 	t.Run("should error on unsupported scheme", func(t *testing.T) {
-		_, err := NewChartRepository("http://example.com", providers, nil)
+		g := NewWithT(t)
-		if err == nil {
+
-			t.Fatalf("Expecting unsupported scheme error")
+		r, err := NewChartRepository("http://example.com", "", providers, nil)
-		}
+		g.Expect(err).To(HaveOccurred())
+		g.Expect(err.Error()).To(Equal("scheme \"http\" not supported"))
+		g.Expect(r).To(BeNil())
 	})
 }
 
 func TestChartRepository_Get(t *testing.T) {
-	i := repo.NewIndexFile()
+	g := NewWithT(t)
-	i.Add(&chart.Metadata{Name: "chart", Version: "0.0.1"}, "chart-0.0.1.tgz", "http://example.com/charts", "sha256:1234567890")
+
-	i.Add(&chart.Metadata{Name: "chart", Version: "0.1.0"}, "chart-0.1.0.tgz", "http://example.com/charts", "sha256:1234567890abc")
+	r := newChartRepository()
-	i.Add(&chart.Metadata{Name: "chart", Version: "0.1.1"}, "chart-0.1.1.tgz", "http://example.com/charts", "sha256:1234567890abc")
+	r.Index = repo.NewIndexFile()
-	i.Add(&chart.Metadata{Name: "chart", Version: "0.1.5+b.min.minute"}, "chart-0.1.5+b.min.minute.tgz", "http://example.com/charts", "sha256:1234567890abc")
+	charts := []struct {
-	i.Entries["chart"][len(i.Entries["chart"])-1].Created = time.Now().Add(-time.Minute)
+		name    string
-	i.Add(&chart.Metadata{Name: "chart", Version: "0.1.5+a.min.hour"}, "chart-0.1.5+a.min.hour.tgz", "http://example.com/charts", "sha256:1234567890abc")
+		version string
-	i.Entries["chart"][len(i.Entries["chart"])-1].Created = time.Now().Add(-time.Hour)
+		url     string
-	i.Add(&chart.Metadata{Name: "chart", Version: "0.1.5+c.now"}, "chart-0.1.5+c.now.tgz", "http://example.com/charts", "sha256:1234567890abc")
+		digest  string
-	i.Add(&chart.Metadata{Name: "chart", Version: "0.2.0"}, "chart-0.2.0.tgz", "http://example.com/charts", "sha256:1234567890abc")
+		created time.Time
-	i.Add(&chart.Metadata{Name: "chart", Version: "1.0.0"}, "chart-1.0.0.tgz", "http://example.com/charts", "sha256:1234567890abc")
+	}{
-	i.Add(&chart.Metadata{Name: "chart", Version: "1.1.0-rc.1"}, "chart-1.1.0-rc.1.tgz", "http://example.com/charts", "sha256:1234567890abc")
+		{name: "chart", version: "0.0.1", url: "http://example.com/charts", digest: "sha256:1234567890"},
-	i.SortEntries()
+		{name: "chart", version: "0.1.0", url: "http://example.com/charts", digest: "sha256:1234567890abc"},
-	r := &ChartRepository{Index: i}
+		{name: "chart", version: "0.1.1", url: "http://example.com/charts", digest: "sha256:1234567890abc"},
+		{name: "chart", version: "0.1.5+b.min.minute", url: "http://example.com/charts", digest: "sha256:1234567890abc", created: now.Add(-time.Minute)},
+		{name: "chart", version: "0.1.5+a.min.hour", url: "http://example.com/charts", digest: "sha256:1234567890abc", created: now.Add(-time.Hour)},
+		{name: "chart", version: "0.1.5+c.now", url: "http://example.com/charts", digest: "sha256:1234567890abc", created: now},
+		{name: "chart", version: "0.2.0", url: "http://example.com/charts", digest: "sha256:1234567890abc"},
+		{name: "chart", version: "1.0.0", url: "http://example.com/charts", digest: "sha256:1234567890abc"},
+		{name: "chart", version: "1.1.0-rc.1", url: "http://example.com/charts", digest: "sha256:1234567890abc"},
+	}
+	for _, c := range charts {
+		g.Expect(r.Index.MustAdd(
+			&chart.Metadata{Name: c.name, Version: c.version},
+			fmt.Sprintf("%s-%s.tgz", c.name, c.version), c.url, c.digest),
+		).To(Succeed())
+		if !c.created.IsZero() {
+			r.Index.Entries["chart"][len(r.Index.Entries["chart"])-1].Created = c.created
+		}
+	}
+	r.Index.SortEntries()
 
 	tests := []struct {
 		name         string
 		chartName    string
 		chartVersion string
 		wantVersion  string
-		wantErr      bool
+		wantErr      string
 	}{
 		{
 			name:         "exact match",
@@ -151,12 +158,12 @@ func TestChartRepository_Get(t *testing.T) {
 			name:         "unfulfilled range",
 			chartName:    "chart",
 			chartVersion: ">2.0.0",
-			wantErr:      true,
+			wantErr:      "no 'chart' chart with version matching '>2.0.0' found",
 		},
 		{
 			name:      "invalid chart",
 			chartName: "non-existing",
-			wantErr:   true,
+			wantErr:   repo.ErrNoChartName.Error(),
 		},
 		{
 			name:         "match newest if ambiguous",
@@ -168,14 +175,19 @@ func TestChartRepository_Get(t *testing.T) {
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
 			cv, err := r.Get(tt.chartName, tt.chartVersion)
-			if (err != nil) != tt.wantErr {
+			if tt.wantErr != "" {
-				t.Errorf("Get() error = %v, wantErr %v", err, tt.wantErr)
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				g.Expect(cv).To(BeNil())
 				return
 			}
-			if err == nil && !strings.Contains(cv.Metadata.Version, tt.wantVersion) {
+			g.Expect(cv).ToNot(BeNil())
-				t.Errorf("Get() unexpected version = %s, want = %s", cv.Metadata.Version, tt.wantVersion)
+			g.Expect(cv.Metadata.Name).To(Equal(tt.chartName))
-			}
+			g.Expect(cv.Metadata.Version).To(Equal(tt.wantVersion))
+			g.Expect(err).ToNot(HaveOccurred())
 		})
 	}
 }
@@ -212,117 +224,257 @@ func TestChartRepository_DownloadChart(t *testing.T) {
 		},
 	}
 	for _, tt := range tests {
+		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			t.Parallel()
+
 			mg := mockGetter{}
 			r := &ChartRepository{
 				URL:    tt.url,
 				Client: &mg,
 			}
-			_, err := r.DownloadChart(tt.chartVersion)
+			res, err := r.DownloadChart(tt.chartVersion)
-			if (err != nil) != tt.wantErr {
+			if tt.wantErr {
-				t.Errorf("DownloadChart() error = %v, wantErr %v", err, tt.wantErr)
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(res).To(BeNil())
 				return
 			}
-			if err == nil && mg.requestedURL != tt.wantURL {
+			g.Expect(mg.requestedURL).To(Equal(tt.wantURL))
-				t.Errorf("DownloadChart() requested URL = %s, wantURL %s", mg.requestedURL, tt.wantURL)
+			g.Expect(res).ToNot(BeNil())
-			}
+			g.Expect(err).ToNot(HaveOccurred())
 		})
 	}
 }
 
 func TestChartRepository_DownloadIndex(t *testing.T) {
-	b, err := os.ReadFile(chartmuseumtestfile)
+	g := NewWithT(t)
-	if err != nil {
+
-		t.Fatal(err)
+	b, err := os.ReadFile(chartmuseumTestFile)
-	}
+	g.Expect(err).ToNot(HaveOccurred())
+
 	mg := mockGetter{response: b}
 	r := &ChartRepository{
 		URL:    "https://example.com",
 		Client: &mg,
 	}
-	if err := r.DownloadIndex(); err != nil {
+
+	buf := bytes.NewBuffer([]byte{})
+	g.Expect(r.DownloadIndex(buf)).To(Succeed())
+	g.Expect(buf.Bytes()).To(Equal(b))
+	g.Expect(mg.requestedURL).To(Equal(r.URL + "/index.yaml"))
+	g.Expect(err).To(BeNil())
+}
+
+func TestChartRepository_LoadIndexFromBytes(t *testing.T) {
+	tests := []struct {
+		name        string
+		b           []byte
+		wantName    string
+		wantVersion string
+		wantDigest  string
+		wantErr     string
+	}{
+		{
+			name: "index",
+			b: []byte(`
+apiVersion: v1
+entries:
+  nginx:
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/nginx-0.2.0.tgz
+      name: nginx
+      description: string
+      version: 0.2.0
+      home: https://github.com/something/else
+      digest: "sha256:1234567890abcdef"
+`),
+			wantName:    "nginx",
+			wantVersion: "0.2.0",
+			wantDigest:  "sha256:1234567890abcdef",
+		},
+		{
+			name: "index without API version",
+			b: []byte(`entries:
+  nginx:
+    - name: nginx`),
+			wantErr: "no API version specified",
+		},
+		{
+			name: "index with duplicate entry",
+			b: []byte(`apiVersion: v1
+entries:
+  nginx:
+    - name: nginx"
+  nginx:
+    - name: nginx`),
+			wantErr: "key \"nginx\" already set in map",
+		},
+	}
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			t.Parallel()
+
+			r := newChartRepository()
+			err := r.LoadIndexFromBytes(tt.b)
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				g.Expect(r.Index).To(BeNil())
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(r.Index).ToNot(BeNil())
+			got, err := r.Index.Get(tt.wantName, tt.wantVersion)
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(got.Digest).To(Equal(tt.wantDigest))
+		})
+	}
+}
+
+func TestChartRepository_LoadIndexFromBytes_Unordered(t *testing.T) {
+	b, err := os.ReadFile(unorderedTestFile)
+	if err != nil {
 		t.Fatal(err)
 	}
-	if expected := r.URL + "/index.yaml"; mg.requestedURL != expected {
+	r := newChartRepository()
-		t.Errorf("DownloadIndex() requested URL = %s, wantURL %s", mg.requestedURL, expected)
+	err = r.LoadIndexFromBytes(b)
+	if err != nil {
+		t.Fatal(err)
 	}
 	verifyLocalIndex(t, r.Index)
 }
 
 // Index load tests are derived from https://github.com/helm/helm/blob/v3.3.4/pkg/repo/index_test.go#L108
 // to ensure parity with Helm behaviour.
-func TestChartRepository_LoadIndex(t *testing.T) {
+func TestChartRepository_LoadIndexFromFile(t *testing.T) {
 	tests := []struct {
 		name     string
 		filename string
 	}{
 		{
 			name:     "regular index file",
-			filename: testfile,
+			filename: testFile,
 		},
 		{
 			name:     "chartmuseum index file",
-			filename: chartmuseumtestfile,
+			filename: chartmuseumTestFile,
 		},
 	}
 
 	for _, tt := range tests {
 		tt := tt
 		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
 			t.Parallel()
-			b, err := os.ReadFile(tt.filename)
+
-			if err != nil {
+			r := newChartRepository()
-				t.Fatal(err)
+			err := r.LoadFromFile(testFile)
-			}
+			g.Expect(err).ToNot(HaveOccurred())
-			r := &ChartRepository{}
+
-			err = r.LoadIndex(b)
-			if err != nil {
-				t.Fatal(err)
-			}
 			verifyLocalIndex(t, r.Index)
 		})
 	}
 }
 
-func TestChartRepository_LoadIndex_Duplicates(t *testing.T) {
+func TestChartRepository_CacheIndex(t *testing.T) {
-	r := &ChartRepository{}
+	g := NewWithT(t)
-	if err := r.LoadIndex([]byte(indexWithDuplicates)); err == nil {
+
-		t.Errorf("Expected an error when duplicate entries are present")
+	mg := mockGetter{response: []byte("foo")}
+	expectSum := fmt.Sprintf("%x", sha256.Sum256(mg.response))
+
+	r := newChartRepository()
+	r.URL = "https://example.com"
+	r.Client = &mg
+
+	sum, err := r.CacheIndex()
+	g.Expect(err).To(Not(HaveOccurred()))
+
+	g.Expect(r.CachePath).ToNot(BeEmpty())
+	defer os.RemoveAll(r.CachePath)
+	g.Expect(r.CachePath).To(BeARegularFile())
+	b, _ := os.ReadFile(r.CachePath)
+
+	g.Expect(b).To(Equal(mg.response))
+	g.Expect(sum).To(BeEquivalentTo(expectSum))
+}
+
+func TestChartRepository_LoadIndexFromCache(t *testing.T) {
+	tests := []struct {
+		name      string
+		cachePath string
+		wantErr   string
+	}{
+		{
+			name:      "cache path",
+			cachePath: chartmuseumTestFile,
+		},
+		{
+			name:      "invalid cache path",
+			cachePath: "invalid",
+			wantErr:   "open invalid: no such file",
+		},
+		{
+			name:      "no cache path",
+			cachePath: "",
+			wantErr:   "no cache path set",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			r := newChartRepository()
+			r.CachePath = tt.cachePath
+			err := r.LoadFromCache()
+			if tt.wantErr != "" {
+				g.Expect(err).To(HaveOccurred())
+				g.Expect(err.Error()).To(ContainSubstring(tt.wantErr))
+				g.Expect(r.Index).To(BeNil())
+				return
+			}
+
+			g.Expect(err).ToNot(HaveOccurred())
+			verifyLocalIndex(t, r.Index)
+		})
 	}
 }
 
-func TestChartRepository_LoadIndex_Unordered(t *testing.T) {
+func TestChartRepository_HasIndex(t *testing.T) {
-	b, err := os.ReadFile(unorderedtestfile)
+	g := NewWithT(t)
-	if err != nil {
+
-		t.Fatal(err)
+	r := newChartRepository()
-	}
+	g.Expect(r.HasIndex()).To(BeFalse())
-	r := &ChartRepository{}
+	r.Index = repo.NewIndexFile()
-	err = r.LoadIndex(b)
+	g.Expect(r.HasIndex()).To(BeTrue())
-	if err != nil {
+}
-		t.Fatal(err)
+
-	}
+func TestChartRepository_UnloadIndex(t *testing.T) {
-	verifyLocalIndex(t, r.Index)
+	g := NewWithT(t)
+
+	r := newChartRepository()
+	g.Expect(r.HasIndex()).To(BeFalse())
+	r.Index = repo.NewIndexFile()
+	r.UnloadIndex()
+	g.Expect(r.Index).To(BeNil())
 }
 
 func verifyLocalIndex(t *testing.T, i *repo.IndexFile) {
-	numEntries := len(i.Entries)
+	g := NewWithT(t)
-	if numEntries != 3 {
+
-		t.Errorf("Expected 3 entries in index file but got %d", numEntries)
+	g.Expect(i.Entries).ToNot(BeNil())
-	}
+	g.Expect(i.Entries).To(HaveLen(3), "expected 3 entries in index file")
 
 	alpine, ok := i.Entries["alpine"]
-	if !ok {
+	g.Expect(ok).To(BeTrue(), "expected 'alpine' entry to exist")
-		t.Fatalf("'alpine' section not found.")
+	g.Expect(alpine).To(HaveLen(1), "'alpine' should have 1 entry")
-	}
-
-	if l := len(alpine); l != 1 {
-		t.Fatalf("'alpine' should have 1 chart, got %d", l)
-	}
 
 	nginx, ok := i.Entries["nginx"]
-	if !ok || len(nginx) != 2 {
+	g.Expect(ok).To(BeTrue(), "expected 'nginx' entry to exist")
-		t.Fatalf("Expected 2 nginx entries")
+	g.Expect(nginx).To(HaveLen(2), "'nginx' should have 2 entries")
-	}
 
 	expects := []*repo.ChartVersion{
 		{
@@ -370,41 +522,12 @@ func verifyLocalIndex(t *testing.T, i *repo.IndexFile) {
 
 	for i, tt := range tests {
 		expect := expects[i]
-		if tt.Name != expect.Name {
+		g.Expect(tt.Name).To(Equal(expect.Name))
-			t.Errorf("Expected name %q, got %q", expect.Name, tt.Name)
+		g.Expect(tt.Description).To(Equal(expect.Description))
-		}
+		g.Expect(tt.Version).To(Equal(expect.Version))
-		if tt.Description != expect.Description {
+		g.Expect(tt.Digest).To(Equal(expect.Digest))
-			t.Errorf("Expected description %q, got %q", expect.Description, tt.Description)
+		g.Expect(tt.Home).To(Equal(expect.Home))
-		}
+		g.Expect(tt.URLs).To(ContainElements(expect.URLs))
-		if tt.Version != expect.Version {
+		g.Expect(tt.Keywords).To(ContainElements(expect.Keywords))
-			t.Errorf("Expected version %q, got %q", expect.Version, tt.Version)
-		}
-		if tt.Digest != expect.Digest {
-			t.Errorf("Expected digest %q, got %q", expect.Digest, tt.Digest)
-		}
-		if tt.Home != expect.Home {
-			t.Errorf("Expected home %q, got %q", expect.Home, tt.Home)
-		}
-
-		for i, url := range tt.URLs {
-			if url != expect.URLs[i] {
-				t.Errorf("Expected URL %q, got %q", expect.URLs[i], url)
-			}
-		}
-		for i, kw := range tt.Keywords {
-			if kw != expect.Keywords[i] {
-				t.Errorf("Expected keywords %q, got %q", expect.Keywords[i], kw)
-			}
-		}
 	}
 }
-
-type mockGetter struct {
-	requestedURL string
-	response     []byte
-}
-
-func (g *mockGetter) Get(url string, options ...getter.Option) (*bytes.Buffer, error) {
-	g.requestedURL = url
-	return bytes.NewBuffer(g.response), nil
-}

internal/helm/utils_test.go

@@ -0,0 +1,60 @@
+/*
+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 (
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+func TestNormalizeChartRepositoryURL(t *testing.T) {
+	tests := []struct {
+		name string
+		url  string
+		want string
+	}{
+		{
+			name: "with slash",
+			url:  "http://example.com/",
+			want: "http://example.com/",
+		},
+		{
+			name: "without slash",
+			url:  "http://example.com",
+			want: "http://example.com/",
+		},
+		{
+			name: "double slash",
+			url:  "http://example.com//",
+			want: "http://example.com/",
+		},
+		{
+			name: "empty",
+			url:  "",
+			want: "",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+
+			got := NormalizeChartRepositoryURL(tt.url)
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}