chartsync: use target helm version for download

This commit achieves three things:

1. The logic for resolving the full chart URL has been moved to the
   client version implementations, adding two new methods `Pull` and
   `PullWithRepoURL`. The latter implements the logic for retrieving
   the full chart URL that was earlier present in the `chartsync`
   package itself. This ensures the accurate repositories index is
   used when we look for chart repository credentials.
2. The newly introduced `Pull` method makes use of the
   `downloader.ChartDownloader` for that Helm version. This makes it
   easier to integrate e.g. the pull of OCI charts once this feature
   becomes available in Helm v3. It also provides some ground work for
   working with named repositories which we may want to use later on
   when we have introduced the Custom Resource for manging Helm
   repositories, as it now understands the `repo/name` format.
3. The chartsync now maintains a chart cache per Helm version by
   appending the `helm.Version()` to the `base`.

pkg/chartsync/download.go

@@ -4,32 +4,27 @@ import (
 	"encoding/base64"
 	"errors"
 	"fmt"
-	"io/ioutil"
-	"net/url"
 	"os"
 	"path/filepath"
-	"strings"
-
-	"github.com/spf13/pflag"
-	"k8s.io/helm/pkg/getter"
-	helmenv "k8s.io/helm/pkg/helm/environment"
-	"k8s.io/helm/pkg/repo"
 
 	helmfluxv1 "github.com/fluxcd/helm-operator/pkg/apis/helm.fluxcd.io/v1"
+	"github.com/fluxcd/helm-operator/pkg/helm"
 )
 
 // EnsureChartFetched returns the path to a downloaded chart, fetching
 // it first if necessary. It always returns the expected path to the
 // chart, and either an error or nil.
-func EnsureChartFetched(base string, source *helmfluxv1.RepoChartSource) (string, error) {
-	chartPath, err := makeChartPath(base, source)
+func EnsureChartFetched(client helm.Client, base string, source *helmfluxv1.RepoChartSource) (string, error) {
+	repoPath, filename, err := makeChartPath(base, client.Version(), source)
 	if err != nil {
-		return chartPath, ChartUnavailableError{err}
+		return "", ChartUnavailableError{err}
 	}
+	chartPath := filepath.Join(repoPath, filename)
 	stat, err := os.Stat(chartPath)
 	switch {
 	case os.IsNotExist(err):
-		if err := downloadChart(chartPath, source); err != nil {
+		chartPath, err = downloadChart(client, repoPath, source)
+		if err != nil {
 			return chartPath, ChartUnavailableError{err}
 		}
 		return chartPath, nil
@@ -43,97 +38,21 @@ func EnsureChartFetched(base string, source *helmfluxv1.RepoChartSource) (string
 
 // makeChartPath gives the expected filesystem location for a chart,
 // without testing whether the file exists or not.
-func makeChartPath(base string, source *helmfluxv1.RepoChartSource) (string, error) {
+func makeChartPath(base string, clientVersion string, source *helmfluxv1.RepoChartSource) (string, string, error) {
 	// We don't need to obscure the location of the charts in the
 	// filesystem; but we do need a stable, filesystem-friendly path
-	// to them that is based on the URL.
-	repoPath := filepath.Join(base, base64.URLEncoding.EncodeToString([]byte(source.CleanRepoURL())))
+	// to them that is based on the URL and the client version.
+	repoPath := filepath.Join(base, clientVersion, base64.URLEncoding.EncodeToString([]byte(source.CleanRepoURL())))
 	if err := os.MkdirAll(repoPath, 00750); err != nil {
-		return "", err
+		return "", "", err
 	}
 	filename := fmt.Sprintf("%s-%s.tgz", source.Name, source.Version)
-	return filepath.Join(repoPath, filename), nil
+	return repoPath, filename, nil
 }
 
-// downloadChart attempts to fetch a chart tarball, given the name,
+// downloadChart attempts to pull a chart tarball, given the name,
 // version and repo URL in `source`, and the path to write the file
-// to in `destFile`.
-func downloadChart(destFile string, source *helmfluxv1.RepoChartSource) error {
-	// Helm's support libs are designed to be driven by the
-	// command-line client, so there are some inevitable CLI-isms,
-	// like getting values from flags and the environment. None of
-	// these things are directly relevant here, _except_ the HELM_HOME
-	// environment entry. Since there's that exception, we must go
-	// through the ff (following faff).
-	var settings helmenv.EnvSettings
-	// Add the flag definitions ..
-	flags := pflag.NewFlagSet("helm-env", pflag.ContinueOnError)
-	settings.AddFlags(flags)
-	// .. but we're not expecting any _actual_ flags, so there's no
-	// Parse. This next bit will use any settings from the
-	// environment.
-	settings.Init(flags)
-	getters := getter.All(settings) // <-- aaaand this is the payoff
-
-	// This resolves the repo URL, chart name and chart version to a
-	// URL for the chart. To be able to resolve the chart name and
-	// version to a URL, we have to have the index file; and to have
-	// that, we may need to authenticate. The credentials will be in
-	// repositories.yaml.
-	repoFile, err := repo.LoadRepositoriesFile(settings.Home.RepositoryFile())
-	if err != nil {
-		return err
-	}
-
-	// Now find the entry for the repository, if there is one. If not,
-	// we'll assume there's no auth needed.
-	repoEntry := &repo.Entry{}
-	for _, entry := range repoFile.Repositories {
-		if urlsMatch(entry.URL, source.CleanRepoURL()) {
-			repoEntry = entry
-			break
-		}
-	}
-
-	// TODO(michael): could look for an existing index file here,
-	// and/or update it. Then we're _pretty_ close to just using
-	// `repo.DownloadTo(...)`.
-	chartURL, err := repo.FindChartInAuthRepoURL(source.CleanRepoURL(), repoEntry.Username, repoEntry.Password, source.Name, source.Version, repoEntry.CertFile, repoEntry.KeyFile, repoEntry.CAFile, getters)
-	if err != nil {
-		return err
-	}
-
-	// Here I'm reproducing the useful part (for us) of
-	// `k8s.io/helm/pkg/downloader.Downloader.ResolveChartVersion(...)`,
-	// stepping around `DownloadTo(...)` as it's too general. The
-	// former interacts with Helm's local caching, which would mean
-	// having to maintain the local cache. Since we already have the
-	// information we need, we can just go ahead and get the file.
-	u, err := url.Parse(chartURL)
-	if err != nil {
-		return err
-	}
-	getterConstructor, err := getters.ByScheme(u.Scheme)
-	if err != nil {
-		return err
-	}
-
-	g, err := getterConstructor(chartURL, repoEntry.CertFile, repoEntry.KeyFile, repoEntry.CAFile)
-	if t, ok := g.(*getter.HttpGetter); ok {
-		t.SetCredentials(repoEntry.Username, repoEntry.Password)
-	}
-
-	chartBytes, err := g.Get(u.String())
-	if err != nil {
-		return err
-	}
-	if err := ioutil.WriteFile(destFile, chartBytes.Bytes(), 0644); err != nil {
-		return err
-	}
-
-	return nil
-}
-
-func urlsMatch(entryURL, sourceURL string) bool {
-	return strings.TrimRight(entryURL, "/") == strings.TrimRight(sourceURL, "/")
+// to in `destFolder`.
+func downloadChart(helm helm.Client, destFolder string, source *helmfluxv1.RepoChartSource) (string, error) {
+	return helm.PullWithRepoURL(source.RepoURL, source.Name, source.Version, destFolder)
 }

pkg/helm/helm.go

@@ -34,6 +34,8 @@ type Client interface {
 	RepositoryAdd(name, url, username, password, certFile, keyFile, caFile string) error
 	RepositoryRemove(name string) error
 	RepositoryImport(path string) error
+	Pull(ref, version, dest string) (string, error)
+	PullWithRepoURL(repoURL, name, version, dest string) (string, error)
 	Uninstall(releaseName string, opts UninstallOptions) error
 	Version() string
 }

pkg/helm/logwriter.go

@@ -0,0 +1,26 @@
+package helm
+
+import (
+	"fmt"
+	"io"
+
+	"github.com/go-kit/kit/log"
+)
+
+// logWriter wraps a `log.Logger` so it can be used as an `io.Writer`
+type logWriter struct {
+	log.Logger
+}
+
+func NewLogWriter(logger log.Logger) io.Writer {
+	return &logWriter{logger}
+}
+
+func (l *logWriter) Write(p []byte) (n int, err error) {
+	origLen := len(p)
+	if len(p) > 0 && p[len(p)-1] == '\n' {
+		p = p[:len(p)-1] // Cut terminating newline
+	}
+	l.Log("info", fmt.Sprintf("%s", p))
+	return origLen, nil
+}

pkg/helm/v2/dependency.go

@@ -1,18 +1,16 @@
 package v2
 
 import (
-	"fmt"
-
-	"github.com/go-kit/kit/log"
-
 	"k8s.io/helm/pkg/downloader"
+
+	"github.com/fluxcd/helm-operator/pkg/helm"
 )
 
 func (h *HelmV2) DependencyUpdate(chartPath string) error {
 	repositoryConfigLock.RLock()
 	defer repositoryConfigLock.RUnlock()
 
-	out := &logWriter{h.logger}
+	out := helm.NewLogWriter(h.logger)
 	man := downloader.Manager{
 		Out: out,
 		ChartPath: chartPath,
@@ -21,17 +19,3 @@ func (h *HelmV2) DependencyUpdate(chartPath string) error {
 	}
 	return man.Update()
 }
-
-// logWriter wraps a `log.Logger` so it can be used as an `io.Writer`
-type logWriter struct {
-	log.Logger
-}
-
-func (l *logWriter) Write(p []byte) (n int, err error) {
-	origLen := len(p)
-	if len(p) > 0 && p[len(p)-1] == '\n' {
-		p = p[:len(p)-1] // Cut terminating newline
-	}
-	l.Log("info", fmt.Sprintf("%s", p))
-	return origLen, nil
-}

pkg/helm/v2/helm.go

@@ -50,28 +50,18 @@ func (h *HelmV2) Version() string {
 	return VERSION
 }
 
-// getVersion retrieves the Tiller version. This is a _V2 only_  method
-// and used internally during the setup of the client.
-func (h *HelmV2) getVersion() (string, error) {
-	v, err := h.client.GetVersion()
-	if err != nil {
-		return "", fmt.Errorf("error getting tiller version: %v", err)
-	}
-	return v.GetVersion().String(), nil
-}
-
-// New creates a new HelmV2 client
+// New attempts to setup a Helm client
 func New(logger log.Logger, kubeClient *kubernetes.Clientset, opts TillerOptions) helm.Client {
-	var helm *HelmV2
+	var h *HelmV2
 	for {
 		client, host, err := newHelmClient(kubeClient, opts)
 		if err != nil {
 			logger.Log("error", fmt.Sprintf("error creating Client (v2) client: %s", err.Error()))
 			time.Sleep(20 * time.Second)
 			continue
 		}
-		helm = &HelmV2{client: client, logger: logger}
-		version, err := helm.getVersion()
+		h = &HelmV2{client: client, logger: logger}
+		version, err := h.getVersion()
 		if err != nil {
 			logger.Log("warning", "unable to connect to Tiller", "err", err, "host", host, "options", fmt.Sprintf("%+v", opts))
 			time.Sleep(20 * time.Second)
@@ -80,10 +70,20 @@ func New(logger log.Logger, kubeClient *kubernetes.Clientset, opts TillerOptions
 		logger.Log("info", "connected to Tiller", "version", version, "host", host, "options", fmt.Sprintf("%+v", opts))
 		break
 	}
-	return helm
+	return h
+}
+
+// getVersion retrieves the Tiller version. This is a _V2 only_  method
+// and used internally during the setup of the client.
+func (h *HelmV2) getVersion() (string, error) {
+	v, err := h.client.GetVersion()
+	if err != nil {
+		return "", fmt.Errorf("error getting tiller version: %v", err)
+	}
+	return v.GetVersion().String(), nil
 }
 
-// newHelmClient creates a new Client v2 client
+// newHelmClient creates a new Helm v2 client
 func newHelmClient(kubeClient *kubernetes.Clientset, opts TillerOptions) (*helmv2.Client, string, error) {
 	host, err := tillerHost(kubeClient, opts)
 	if err != nil {

pkg/helm/v2/pull.go

@@ -0,0 +1,64 @@
+package v2
+
+import (
+	"k8s.io/helm/pkg/downloader"
+	"k8s.io/helm/pkg/repo"
+	"k8s.io/helm/pkg/urlutil"
+
+	"github.com/fluxcd/helm-operator/pkg/helm"
+)
+
+func (h *HelmV2) Pull(ref, version, dest string) (string, error) {
+	repositoryConfigLock.RLock()
+	defer repositoryConfigLock.RUnlock()
+
+	out := helm.NewLogWriter(h.logger)
+	c := downloader.ChartDownloader{
+		Out:      out,
+		HelmHome: helmHome(),
+		Verify:   downloader.VerifyNever,
+		Getters:  getters,
+	}
+	d, _, err := c.DownloadTo(ref, version, dest)
+	return d, err
+}
+
+func (h *HelmV2) PullWithRepoURL(repoURL, name, version, dest string) (string, error) {
+	// This resolves the repo URL, chart name and chart version to a
+	// URL for the chart. To be able to resolve the chart name and
+	// version to a URL, we have to have the index file; and to have
+	// that, we may need to authenticate. The credentials will be in
+	// the repository config.
+	repositoryConfigLock.RLock()
+	repoFile, err := loadRepositoryConfig()
+	repositoryConfigLock.RUnlock()
+	if err != nil {
+		return "", err
+	}
+
+	// Now find the entry for the repository, if there is one. If not,
+	// we'll assume there's no auth needed.
+	repoEntry := &repo.Entry{}
+	repoEntry.URL = repoURL
+	for _, entry := range repoFile.Repositories {
+		if urlutil.Equal(repoEntry.URL, entry.URL) {
+			repoEntry = entry
+			// Ensure we have the repository index as this is
+			// later used by Helm.
+			if r, err := repo.NewChartRepository(repoEntry, getters); err == nil {
+				r.DownloadIndexFile(repositoryCache)
+			}
+			break
+		}
+	}
+
+	// Look up the full URL of the chart with the collected credentials
+	// and given chart name and version.
+	chartURL, err := repo.FindChartInAuthRepoURL(repoEntry.URL, repoEntry.Username, repoEntry.Password, name, version,
+		repoEntry.CertFile, repoEntry.KeyFile, repoEntry.CAFile, getters)
+	if err != nil {
+		return "", err
+	}
+
+	return h.Pull(chartURL, version, dest)
+}

pkg/helm/v3/dependency.go

@@ -1,18 +1,16 @@
 package v3
 
 import (
-	"fmt"
-
-	"github.com/go-kit/kit/log"
-
 	"helm.sh/helm/v3/pkg/downloader"
+
+	"github.com/fluxcd/helm-operator/pkg/helm"
 )
 
 func (h *HelmV3) DependencyUpdate(chartPath string) error {
 	repositoryConfigLock.RLock()
 	defer repositoryConfigLock.RUnlock()
 
-	out := &logWriter{h.logger}
+	out := helm.NewLogWriter(h.logger)
 	man := &downloader.Manager{
 		Out:              out,
 		ChartPath:        chartPath,
@@ -22,17 +20,3 @@ func (h *HelmV3) DependencyUpdate(chartPath string) error {
 	}
 	return man.Update()
 }
-
-// logWriter wraps a `log.Logger` so it can be used as an `io.Writer`
-type logWriter struct {
-	log.Logger
-}
-
-func (l *logWriter) Write(p []byte) (n int, err error) {
-	origLen := len(p)
-	if len(p) > 0 && p[len(p)-1] == '\n' {
-		p = p[:len(p)-1] // Cut terminating newline
-	}
-	l.Log("info", fmt.Sprintf("%s", p))
-	return origLen, nil
-}