Factor out Helm repo index and chart download

controllers/helmchart_controller.go

@@ -231,52 +231,6 @@ func (r *HelmChartReconciler) getSource(ctx context.Context, chart sourcev1.Helm
 
 func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	repository sourcev1.HelmRepository, chart sourcev1.HelmChart, force bool) (sourcev1.HelmChart, error) {
-	cv, err := helm.GetDownloadableChartVersionFromIndex(r.Storage.LocalPath(*repository.GetArtifact()),
-		chart.Spec.Chart, chart.Spec.Version)
-	if err != nil {
-		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-	}
-
-	// Return early if the revision is still the same as the current artifact
-	newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), cv.Version,
-		fmt.Sprintf("%s-%s.tgz", cv.Name, cv.Version))
-	if !force && sourcev1.InReadyCondition(chart.Status.Conditions) && chart.GetArtifact().HasRevision(newArtifact.Revision) {
-		if newArtifact.URL != repository.GetArtifact().URL {
-			r.Storage.SetArtifactURL(chart.GetArtifact())
-			chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
-		}
-		return chart, nil
-	}
-
-	// TODO(hidde): according to the Helm source the first item is not
-	//  always the correct one to pick, check for updates once in awhile.
-	//  Ref: https://github.com/helm/helm/blob/v3.3.0/pkg/downloader/chart_downloader.go#L241
-	ref := cv.URLs[0]
-	u, err := url.Parse(ref)
-	if err != nil {
-		err = fmt.Errorf("invalid chart URL format '%s': %w", ref, err)
-	}
-
-	// Prepend the chart repository base URL if the URL is relative
-	if !u.IsAbs() {
-		repoURL, err := url.Parse(repository.Spec.URL)
-		if err != nil {
-			err = fmt.Errorf("invalid repository URL format '%s': %w", repository.Spec.URL, err)
-			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-		}
-		q := repoURL.Query()
-		// Trailing slash is required for ResolveReference to work
-		repoURL.Path = strings.TrimSuffix(repoURL.Path, "/") + "/"
-		u = repoURL.ResolveReference(u)
-		u.RawQuery = q.Encode()
-	}
-
-	// Get the getter for the protocol
-	c, err := r.Getters.ByScheme(u.Scheme)
-	if err != nil {
-		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
-	}
-
 	var clientOpts []getter.Option
 	if repository.Spec.SecretRef != nil {
 		name := types.NamespacedName{
@@ -299,6 +253,46 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 		defer cleanup()
 		clientOpts = opts
 	}
+	clientOpts = append(clientOpts, getter.WithTimeout(repository.GetTimeout()))
+
+	// Initialize the chart repository and load the index file
+	chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Getters, clientOpts)
+	if err != nil {
+		switch err.(type) {
+		case *url.Error:
+			return sourcev1.HelmChartNotReady(chart, sourcev1.URLInvalidReason, err.Error()), err
+		default:
+			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
+		}
+	}
+	indexFile, err := os.Open(r.Storage.LocalPath(*repository.GetArtifact()))
+	if err != nil {
+		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err
+	}
+	b, err := ioutil.ReadAll(indexFile)
+	if err != nil {
+		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
+	}
+	if err = chartRepo.LoadIndex(b); err != nil {
+		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
+	}
+
+	// Lookup the chart version in the chart repository index
+	chartVer, err := chartRepo.Get(chart.Spec.Chart, chart.Spec.Version)
+	if err != nil {
+		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
+	}
+
+	// Return early if the revision is still the same as the current artifact
+	newArtifact := r.Storage.NewArtifactFor(chart.Kind, chart.GetObjectMeta(), chartVer.Version,
+		fmt.Sprintf("%s-%s.tgz", chartVer.Name, chartVer.Version))
+	if !force && repository.GetArtifact().HasRevision(newArtifact.Revision) {
+		if newArtifact.URL != chart.GetArtifact().URL {
+			r.Storage.SetArtifactURL(chart.GetArtifact())
+			chart.Status.URL = r.Storage.SetHostname(chart.Status.URL)
+		}
+		return chart, nil
+	}
 
 	// Ensure artifact directory exists
 	err = r.Storage.MkdirAll(newArtifact)
@@ -315,9 +309,8 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	}
 	defer unlock()
 
-	// TODO(hidde): implement timeout from the HelmRepository
+	// Attempt to download the chart
-	//  https://github.com/helm/helm/pull/7950
+	res, err := chartRepo.DownloadChart(chartVer)
-	res, err := c.Get(u.String(), clientOpts...)
 	if err != nil {
 		return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPullFailedReason, err.Error()), err
 	}
@@ -345,7 +338,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 		}
 
 		// Overwrite values file
-		chartPath := path.Join(tmpDir, cv.Name)
+		chartPath := path.Join(tmpDir, chartVer.Name)
 		if err := helm.OverwriteChartDefaultValues(chartPath, chart.Spec.ValuesFile); err != nil {
 			return sourcev1.HelmChartNotReady(chart, sourcev1.ChartPackageFailedReason, err.Error()), err
 		}
@@ -376,7 +369,7 @@ func (r *HelmChartReconciler) reconcileFromHelmRepository(ctx context.Context,
 	}
 
 	// Update symlink
-	chartUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", cv.Name))
+	chartUrl, err := r.Storage.Symlink(newArtifact, fmt.Sprintf("%s-latest.tgz", chartVer.Name))
 	if err != nil {
 		err = fmt.Errorf("storage error: %w", err)
 		return sourcev1.HelmChartNotReady(chart, sourcev1.StorageOperationFailedReason, err.Error()), err

controllers/helmrepository_controller.go

@@ -20,15 +20,12 @@ import (
 	"bytes"
 	"context"
 	"fmt"
-	"io/ioutil"
 	"net/url"
-	"path"
 	"strings"
 	"time"
 
 	"github.com/go-logr/logr"
 	"helm.sh/helm/v3/pkg/getter"
-	"helm.sh/helm/v3/pkg/repo"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/types"
@@ -163,19 +160,6 @@ func (r *HelmRepositoryReconciler) SetupWithManagerAndOptions(mgr ctrl.Manager,
 }
 
 func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sourcev1.HelmRepository) (sourcev1.HelmRepository, error) {
-	u, err := url.Parse(repository.Spec.URL)
-	if err != nil {
-		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.URLInvalidReason, err.Error()), err
-	}
-
-	c, err := r.Getters.ByScheme(u.Scheme)
-	if err != nil {
-		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.URLInvalidReason, err.Error()), err
-	}
-
-	u.RawPath = path.Join(u.RawPath, "index.yaml")
-	u.Path = path.Join(u.Path, "index.yaml")
-
 	var clientOpts []getter.Option
 	if repository.Spec.SecretRef != nil {
 		name := types.NamespacedName{
@@ -198,25 +182,27 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
 		defer cleanup()
 		clientOpts = opts
 	}
-
 	clientOpts = append(clientOpts, getter.WithTimeout(repository.GetTimeout()))
-	res, err := c.Get(u.String(), clientOpts...)
-	if err != nil {
-		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
-	}
 
-	b, err := ioutil.ReadAll(res)
+	chartRepo, err := helm.NewChartRepository(repository.Spec.URL, r.Getters, clientOpts)
 	if err != nil {
+		switch err.(type) {
+		case *url.Error:
+			return sourcev1.HelmRepositoryNotReady(repository, sourcev1.URLInvalidReason, err.Error()), err
+		default:
 			return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
 		}
-	i := repo.IndexFile{}
+	}
-	if err := yaml.Unmarshal(b, &i); err != nil {
+	if err := chartRepo.DownloadIndex(); err != nil {
+		err = fmt.Errorf("failed to download repository index: %w", err)
 		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
 	}
 
 	// return early on unchanged generation
-	artifact := r.Storage.NewArtifactFor(repository.Kind, repository.ObjectMeta.GetObjectMeta(), i.Generated.Format(time.RFC3339Nano),
+	artifact := r.Storage.NewArtifactFor(repository.Kind,
-		fmt.Sprintf("index-%s.yaml", url.PathEscape(i.Generated.Format(time.RFC3339Nano))))
+		repository.ObjectMeta.GetObjectMeta(),
+		chartRepo.Index.Generated.Format(time.RFC3339Nano),
+		fmt.Sprintf("index-%s.yaml", url.PathEscape(chartRepo.Index.Generated.Format(time.RFC3339Nano))))
 	if sourcev1.InReadyCondition(repository.Status.Conditions) && repository.GetArtifact().HasRevision(artifact.Revision) {
 		if artifact.URL != repository.GetArtifact().URL {
 			r.Storage.SetArtifactURL(repository.GetArtifact())
@@ -225,12 +211,6 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
 		return repository, nil
 	}
 
-	i.SortEntries()
-	b, err = yaml.Marshal(&i)
-	if err != nil {
-		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
-	}
-
 	// create artifact dir
 	err = r.Storage.MkdirAll(artifact)
 	if err != nil {
@@ -247,6 +227,10 @@ func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sou
 	defer unlock()
 
 	// save artifact to storage
+	b, err := yaml.Marshal(&chartRepo.Index)
+	if err != nil {
+		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.IndexationFailedReason, err.Error()), err
+	}
 	if err := r.Storage.AtomicWriteFile(&artifact, bytes.NewReader(b), 0644); err != nil {
 		err = fmt.Errorf("unable to write repository index file: %w", err)
 		return sourcev1.HelmRepositoryNotReady(repository, sourcev1.StorageOperationFailedReason, err.Error()), err

controllers/helmrepository_controller_test.go

@@ -121,7 +121,7 @@ var _ = Describe("HelmRepositoryReconciler", func() {
 			Eventually(func() bool {
 				_ = k8sClient.Get(context.Background(), key, updated)
 				for _, c := range updated.Status.Conditions {
-					if c.Reason == sourcev1.URLInvalidReason {
+					if c.Reason == sourcev1.IndexationFailedReason {
 						return true
 					}
 				}

internal/helm/getter.go

@@ -26,6 +26,8 @@ import (
 	corev1 "k8s.io/api/core/v1"
 )
 
+// ClientOptionsFromSecret constructs a getter.Option slice for the given secret.
+// It returns the slice, and a callback to remove temporary files.
 func ClientOptionsFromSecret(secret corev1.Secret) ([]getter.Option, func(), error) {
 	var opts []getter.Option
 	basicAuth, err := BasicAuthFromSecret(secret)
@@ -45,6 +47,11 @@ func ClientOptionsFromSecret(secret corev1.Secret) ([]getter.Option, func(), err
 	return opts, cleanup, nil
 }
 
+// BasicAuthFromSecret attempts to construct a basic auth getter.Option for the
+// given v1.Secret and returns the result.
+//
+// Secrets with no username AND password are ignored, if only one is defined it
+// returns an error.
 func BasicAuthFromSecret(secret corev1.Secret) (getter.Option, error) {
 	username, password := string(secret.Data["username"]), string(secret.Data["password"])
 	switch {
@@ -56,6 +63,12 @@ func BasicAuthFromSecret(secret corev1.Secret) (getter.Option, error) {
 	return getter.WithBasicAuth(username, password), nil
 }
 
+// TLSClientConfigFromSecret attempts to construct a TLS client config
+// getter.Option for the given v1.Secret. It returns the getter.Option and a
+// callback to remove the temporary TLS files.
+//
+// Secrets with no certFile, keyFile, AND caFile are ignored, if only a
+// certBytes OR keyBytes is defined it returns an error.
 func TLSClientConfigFromSecret(secret corev1.Secret) (getter.Option, func(), error) {
 	certBytes, keyBytes, caBytes := secret.Data["certFile"], secret.Data["keyFile"], secret.Data["caFile"]
 	switch {

internal/helm/repository.go

@@ -17,64 +17,188 @@ limitations under the License.
 package helm
 
 import (
+	"bytes"
 	"fmt"
 	"io/ioutil"
+	"net/url"
+	"path"
+	"sort"
+	"strings"
 
 	"github.com/blang/semver/v4"
+	"helm.sh/helm/v3/pkg/getter"
 	"helm.sh/helm/v3/pkg/repo"
 	"sigs.k8s.io/yaml"
 )
 
-func GetDownloadableChartVersionFromIndex(path, chart, version string) (*repo.ChartVersion, error) {
+// ChartRepository represents a Helm chart repository, and the configuration
-	b, err := ioutil.ReadFile(path)
+// required to download the chart index, and charts from the repository.
-	if err != nil {
+type ChartRepository struct {
-		return nil, fmt.Errorf("failed to read Helm repository index file: %w", err)
+	URL     string
-	}
+	Index   *repo.IndexFile
-	index := &repo.IndexFile{}
+	Client  getter.Getter
-	if err := yaml.Unmarshal(b, index); err != nil {
+	Options []getter.Option
-		return nil, fmt.Errorf("failed to unmarshal Helm repository index file: %w", err)
+}
-	}
 
-	var cv *repo.ChartVersion
+// NewChartRepository constructs and returns a new ChartRepository with
-	if version == "" || version == "*" {
+// the ChartRepository.Client configured to the getter.Getter for the
-		cv, err = index.Get(chart, version)
+// 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) {
+	u, err := url.Parse(repositoryURL)
 	if err != nil {
-			if err == repo.ErrNoChartName {
-				err = fmt.Errorf("chart '%s' could not be found in Helm repository index", chart)
-			}
 		return nil, err
 	}
-	} else {
+	c, err := providers.ByScheme(u.Scheme)
-		entries, ok := index.Entries[chart]
+	if err != nil {
+		return nil, err
+	}
+	return &ChartRepository{
+		URL:     repositoryURL,
+		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, version string) (*repo.ChartVersion, error) {
+	cvs, ok := r.Index.Entries[name]
 	if !ok {
-			return nil, fmt.Errorf("chart '%s' could not be found in Helm repository index", chart)
+		return nil, repo.ErrNoChartName
+	}
+	if len(cvs) == 0 {
+		return nil, repo.ErrNoChartVersion
 	}
 
+	// Check for exact matches first
+	if len(version) != 0 {
+		for _, cv := range cvs {
+			if version == cv.Version {
+				return cv, nil
+			}
+		}
+	}
+
+	// Continue to look for a (semantic) version match
+	latestStable := len(version) == 0 || version == "*"
+	var match semver.Range
+	if !latestStable {
 		rng, err := semver.ParseRange(version)
 		if err != nil {
-			return nil, fmt.Errorf("semver range parse error: %w", err)
+			return nil, err
 		}
-		versionEntryLookup := make(map[string]*repo.ChartVersion)
+		match = rng
-		var versionsInRange []semver.Version
-		for _, e := range entries {
-			v, _ := semver.ParseTolerant(e.Version)
-			if rng(v) {
-				versionsInRange = append(versionsInRange, v)
-				versionEntryLookup[v.String()] = e
 	}
+	var filteredVersions semver.Versions
+	lookup := make(map[string]*repo.ChartVersion)
+	for _, cv := range cvs {
+		v, err := semver.ParseTolerant(cv.Version)
+		if err != nil {
+			continue
 		}
-		if len(versionsInRange) == 0 {
+		// NB: given the entries are already sorted in LoadIndex,
-			return nil, fmt.Errorf("no match found for semver: %s", version)
+		// there is a high probability the first match would be
+		// the right match to return. However, due to the fact that
+		// we use a different semver package than Helm does, we still
+		// need to sort it by our own rules.
+		if match != nil && !match(v) {
+			continue
 		}
-		semver.Sort(versionsInRange)
+		filteredVersions = append(filteredVersions, v)
+		lookup[v.String()] = cv
+	}
+	if len(filteredVersions) == 0 {
+		return nil, fmt.Errorf("no chart version found for %s-%s", name, version)
+	}
+	sort.Sort(sort.Reverse(filteredVersions))
 
-		latest := versionsInRange[len(versionsInRange)-1]
+	latest := filteredVersions[0]
-		cv = versionEntryLookup[latest.String()]
+	if latestStable {
+		for _, v := range filteredVersions {
+			if len(v.Pre) == 0 {
+				latest = v
+				break
 			}
-
-	if len(cv.URLs) == 0 {
-		return nil, fmt.Errorf("no downloadable URLs for chart '%s' with version '%s'", cv.Name, cv.Version)
 		}
-
+	}
-	return cv, nil
+	return lookup[latest.String()], nil
+}
+
+// DownloadChart confirms the given repo.ChartVersion has a downloadable URL,
+// and then attempts to download the chart using the Client and Options of the
+// 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)
+	}
+
+	// TODO(hidde): according to the Helm source the first item is not
+	//  always the correct one to pick, check for updates once in awhile.
+	//  Ref: https://github.com/helm/helm/blob/v3.3.0/pkg/downloader/chart_downloader.go#L241
+	ref := chart.URLs[0]
+	u, err := url.Parse(ref)
+	if err != nil {
+		err = fmt.Errorf("invalid chart URL format '%s': %w", ref, err)
+		return nil, err
+	}
+
+	// Prepend the chart repository base URL if the URL is relative
+	if !u.IsAbs() {
+		repoURL, err := url.Parse(r.URL)
+		if err != nil {
+			err = fmt.Errorf("invalid chart repository URL format '%s': %w", r.URL, err)
+			return nil, err
+		}
+		q := repoURL.Query()
+		// Trailing slash is required for ResolveReference to work
+		repoURL.Path = strings.TrimSuffix(repoURL.Path, "/") + "/"
+		u = repoURL.ResolveReference(u)
+		u.RawQuery = q.Encode()
+	}
+
+	return r.Client.Get(u.String(), r.Options...)
+}
+
+// LoadIndex loads the given bytes into the Index while performing
+// minimal validity checks. It fails if the API version is not set
+// (repo.ErrNoAPIVersion), or if the unmarshal fails.
+//
+// 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
+	}
+	if i.APIVersion == "" {
+		return repo.ErrNoAPIVersion
+	}
+	i.SortEntries()
+	r.Index = i
+	return nil
+}
+
+// DownloadIndex attempts to download the chart repository index using
+// the Client and set Options, and loads the index file into the Index.
+// It returns an error on URL parsing and Client failures.
+func (r *ChartRepository) DownloadIndex() error {
+	u, err := url.Parse(r.URL)
+	if err != nil {
+		return err
+	}
+	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...)
+	if err != nil {
+		return err
+	}
+	b, err := ioutil.ReadAll(res)
+	if err != nil {
+		return err
+	}
+
+	return r.LoadIndex(b)
 }

internal/helm/repository_test.go

@@ -0,0 +1,398 @@
+/*
+Copyright 2020 The Flux CD contributors.
+
+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 (
+	"bytes"
+	"io/ioutil"
+	"net/url"
+	"reflect"
+	"strings"
+	"testing"
+
+	"helm.sh/helm/v3/pkg/chart"
+	"helm.sh/helm/v3/pkg/getter"
+	"helm.sh/helm/v3/pkg/repo"
+)
+
+const (
+	testfile            = "testdata/local-index.yaml"
+	chartmuseumtestfile = "testdata/chartmuseum-index.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"
+`
+)
+
+func TestNewChartRepository(t *testing.T) {
+	repositoryURL := "https://example.com"
+	providers := getter.Providers{
+		getter.Provider{
+			Schemes: []string{"https"},
+			New:     getter.NewHTTPGetter,
+		},
+	}
+	options := []getter.Option{getter.WithBasicAuth("username", "password")}
+
+	t.Run("should construct chart repository", func(t *testing.T) {
+		r, err := NewChartRepository(repositoryURL, providers, options)
+		if err != nil {
+			t.Error(err)
+		}
+		if got := r.URL; got != repositoryURL {
+			t.Fatalf("Expecting %q repository URL, got: %q", repositoryURL, got)
+		}
+		if r.Client == nil {
+			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)
+		switch err.(type) {
+		case *url.Error:
+		default:
+			t.Fatalf("Expecting URL error, got: %v", err)
+		}
+	})
+
+	t.Run("should error on unsupported scheme", func(t *testing.T) {
+		_, err := NewChartRepository("http://example.com", providers, nil)
+		if err == nil {
+			t.Fatalf("Expecting unsupported scheme error")
+		}
+	})
+}
+
+func TestChartRepository_Get(t *testing.T) {
+	i := repo.NewIndexFile()
+	i.Add(&chart.Metadata{Name: "chart", Version: "exact"}, "chart-exact.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")
+	i.Add(&chart.Metadata{Name: "chart", Version: "0.1.1"}, "chart-0.1.1.tgz", "http://example.com/charts", "sha256:1234567890abc")
+	i.Add(&chart.Metadata{Name: "chart", Version: "0.2.0"}, "chart-0.2.0.tgz", "http://example.com/charts", "sha256:1234567890abc")
+	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")
+	i.SortEntries()
+	r := &ChartRepository{Index: i}
+
+	tests := []struct {
+		name         string
+		chartName    string
+		chartVersion string
+		wantVersion  string
+		wantErr      bool
+	}{
+		{
+			name:         "exact matth",
+			chartName:    "chart",
+			chartVersion: "exact",
+			wantVersion:  "exact",
+		},
+		{
+			name:         "stable version",
+			chartName:    "chart",
+			chartVersion: "",
+			wantVersion:  "1.0.0",
+		},
+		{
+			name:         "stable version (asterisk)",
+			chartName:    "chart",
+			chartVersion: "*",
+			wantVersion:  "1.0.0",
+		},
+		{
+			name:         "semver range",
+			chartName:    "chart",
+			chartVersion: "<1.0.0",
+			wantVersion:  "0.2.0",
+		},
+		{
+			name:         "unfulfilled range",
+			chartName:    "chart",
+			chartVersion: ">2.0.0",
+			wantErr:      true,
+		},
+		{
+			name:      "invalid chart",
+			chartName: "non-existing",
+			wantErr:   true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cv, err := r.Get(tt.chartName, tt.chartVersion)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("Get() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if err == nil && !strings.Contains(cv.Metadata.Version, tt.wantVersion) {
+				t.Errorf("Get() unexpected version = %s, want = %s", cv.Metadata.Version, tt.wantVersion)
+			}
+		})
+	}
+}
+
+func TestChartRepository_DownloadChart(t *testing.T) {
+	tests := []struct {
+		name         string
+		url          string
+		chartVersion *repo.ChartVersion
+		wantURL      string
+		wantErr      bool
+	}{
+		{
+			name: "relative URL",
+			url:  "https://example.com",
+			chartVersion: &repo.ChartVersion{
+				Metadata: &chart.Metadata{Name: "chart"},
+				URLs:     []string{"charts/foo-1.0.0.tgz"},
+			},
+			wantURL: "https://example.com/charts/foo-1.0.0.tgz",
+		},
+		{
+			name:         "no chart URL",
+			chartVersion: &repo.ChartVersion{Metadata: &chart.Metadata{Name: "chart"}},
+			wantErr:      true,
+		},
+		{
+			name: "invalid chart URL",
+			chartVersion: &repo.ChartVersion{
+				Metadata: &chart.Metadata{Name: "chart"},
+				URLs:     []string{"https://ex ample.com/charts/foo-1.0.0.tgz"},
+			},
+			wantErr: true,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			mg := mockGetter{}
+			r := &ChartRepository{
+				URL:    tt.url,
+				Client: &mg,
+			}
+			_, err := r.DownloadChart(tt.chartVersion)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("DownloadChart() error = %v, wantErr %v", err, tt.wantErr)
+				return
+			}
+			if err == nil && mg.requestedURL != tt.wantURL {
+				t.Errorf("DownloadChart() requested URL = %s, wantURL %s", mg.requestedURL, tt.wantURL)
+			}
+		})
+	}
+}
+
+func TestChartRepository_DownloadIndex(t *testing.T) {
+	b, err := ioutil.ReadFile(chartmuseumtestfile)
+	if err != nil {
+		t.Fatal(err)
+	}
+	mg := mockGetter{response: b}
+	r := &ChartRepository{
+		URL:    "https://example.com",
+		Client: &mg,
+	}
+	if err := r.DownloadIndex(); err != nil {
+		t.Fatal(err)
+	}
+	if expected := r.URL + "/index.yaml"; mg.requestedURL != expected {
+		t.Errorf("DownloadIndex() requested URL = %s, wantURL %s", mg.requestedURL, expected)
+	}
+	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) {
+	tests := []struct {
+		name     string
+		filename string
+	}{
+		{
+			name:     "regular index file",
+			filename: testfile,
+		},
+		{
+			name:     "chartmuseum index file",
+			filename: chartmuseumtestfile,
+		},
+	}
+
+	for _, tt := range tests {
+		tt := tt
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			b, err := ioutil.ReadFile(tt.filename)
+			if err != nil {
+				t.Fatal(err)
+			}
+			r := &ChartRepository{}
+			err = r.LoadIndex(b)
+			if err != nil {
+				t.Fatal(err)
+			}
+			verifyLocalIndex(t, r.Index)
+		})
+	}
+}
+
+func TestChartRepository_LoadIndex_Duplicates(t *testing.T) {
+	r := &ChartRepository{}
+	if err := r.LoadIndex([]byte(indexWithDuplicates)); err == nil {
+		t.Errorf("Expected an error when duplicate entries are present")
+	}
+}
+
+func TestChartRepository_LoadIndex_Unordered(t *testing.T) {
+	b, err := ioutil.ReadFile(unorderedtestfile)
+	if err != nil {
+		t.Fatal(err)
+	}
+	r := &ChartRepository{}
+	err = r.LoadIndex(b)
+	if err != nil {
+		t.Fatal(err)
+	}
+	verifyLocalIndex(t, r.Index)
+}
+
+func verifyLocalIndex(t *testing.T, i *repo.IndexFile) {
+	numEntries := len(i.Entries)
+	if numEntries != 3 {
+		t.Errorf("Expected 3 entries in index file but got %d", numEntries)
+	}
+
+	alpine, ok := i.Entries["alpine"]
+	if !ok {
+		t.Fatalf("'alpine' section not found.")
+	}
+
+	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 {
+		t.Fatalf("Expected 2 nginx entries")
+	}
+
+	expects := []*repo.ChartVersion{
+		{
+			Metadata: &chart.Metadata{
+				Name:        "alpine",
+				Description: "string",
+				Version:     "1.0.0",
+				Keywords:    []string{"linux", "alpine", "small", "sumtin"},
+				Home:        "https://github.com/something",
+			},
+			URLs: []string{
+				"https://kubernetes-charts.storage.googleapis.com/alpine-1.0.0.tgz",
+				"http://storage2.googleapis.com/kubernetes-charts/alpine-1.0.0.tgz",
+			},
+			Digest: "sha256:1234567890abcdef",
+		},
+		{
+			Metadata: &chart.Metadata{
+				Name:        "nginx",
+				Description: "string",
+				Version:     "0.2.0",
+				Keywords:    []string{"popular", "web server", "proxy"},
+				Home:        "https://github.com/something/else",
+			},
+			URLs: []string{
+				"https://kubernetes-charts.storage.googleapis.com/nginx-0.2.0.tgz",
+			},
+			Digest: "sha256:1234567890abcdef",
+		},
+		{
+			Metadata: &chart.Metadata{
+				Name:        "nginx",
+				Description: "string",
+				Version:     "0.1.0",
+				Keywords:    []string{"popular", "web server", "proxy"},
+				Home:        "https://github.com/something",
+			},
+			URLs: []string{
+				"https://kubernetes-charts.storage.googleapis.com/nginx-0.1.0.tgz",
+			},
+			Digest: "sha256:1234567890abcdef",
+		},
+	}
+	tests := []*repo.ChartVersion{alpine[0], nginx[0], nginx[1]}
+
+	for i, tt := range tests {
+		expect := expects[i]
+		if tt.Name != expect.Name {
+			t.Errorf("Expected name %q, got %q", expect.Name, tt.Name)
+		}
+		if tt.Description != expect.Description {
+			t.Errorf("Expected description %q, got %q", expect.Description, tt.Description)
+		}
+		if tt.Version != expect.Version {
+			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/testdata/chartmuseum-index.yaml

@@ -0,0 +1,50 @@
+serverInfo:
+  contextPath: /v1/helm
+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"
+      keywords:
+        - popular
+        - web server
+        - proxy
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/nginx-0.1.0.tgz
+      name: nginx
+      description: string
+      version: 0.1.0
+      home: https://github.com/something
+      digest: "sha256:1234567890abcdef"
+      keywords:
+        - popular
+        - web server
+        - proxy
+  alpine:
+    - 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
+      keywords:
+        - linux
+        - alpine
+        - small
+        - sumtin
+      digest: "sha256:1234567890abcdef"
+  chartWithNoURL:
+    - name: chartWithNoURL
+      description: string
+      version: 1.0.0
+      home: https://github.com/something
+      keywords:
+        - small
+        - sumtin
+      digest: "sha256:1234567890abcdef"

internal/helm/testdata/local-index-unordered.yaml

@@ -0,0 +1,48 @@
+apiVersion: v1
+entries:
+  nginx:
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/nginx-0.1.0.tgz
+      name: nginx
+      description: string
+      version: 0.1.0
+      home: https://github.com/something
+      digest: "sha256:1234567890abcdef"
+      keywords:
+        - popular
+        - web server
+        - proxy
+    - 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"
+      keywords:
+        - popular
+        - web server
+        - proxy
+  alpine:
+    - 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
+      keywords:
+        - linux
+        - alpine
+        - small
+        - sumtin
+      digest: "sha256:1234567890abcdef"
+  chartWithNoURL:
+    - name: chartWithNoURL
+      description: string
+      version: 1.0.0
+      home: https://github.com/something
+      keywords:
+        - small
+        - sumtin
+      digest: "sha256:1234567890abcdef"

internal/helm/testdata/local-index.yaml

@@ -0,0 +1,48 @@
+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"
+      keywords:
+        - popular
+        - web server
+        - proxy
+    - urls:
+        - https://kubernetes-charts.storage.googleapis.com/nginx-0.1.0.tgz
+      name: nginx
+      description: string
+      version: 0.1.0
+      home: https://github.com/something
+      digest: "sha256:1234567890abcdef"
+      keywords:
+        - popular
+        - web server
+        - proxy
+  alpine:
+    - 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
+      keywords:
+        - linux
+        - alpine
+        - small
+        - sumtin
+      digest: "sha256:1234567890abcdef"
+  chartWithNoURL:
+    - name: chartWithNoURL
+      description: string
+      version: 1.0.0
+      home: https://github.com/something
+      keywords:
+        - small
+        - sumtin
+      digest: "sha256:1234567890abcdef"