From 691aeccbaa374c81e1975717c84f08ddf8b65d14 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Wed, 11 Dec 2019 17:17:55 +0100 Subject: [PATCH 1/2] helm/v2: use download manager for dep updates This avoids having to shell out to the `helm2` binary while still maintaining the same functionality. It aligns with the approach used in `helm/v3`. --- pkg/helm/v2/dependency.go | 81 ++++++++++++--------------------------- 1 file changed, 25 insertions(+), 56 deletions(-) diff --git a/pkg/helm/v2/dependency.go b/pkg/helm/v2/dependency.go index 6725c53b1..d9c618916 100644 --- a/pkg/helm/v2/dependency.go +++ b/pkg/helm/v2/dependency.go @@ -1,68 +1,37 @@ package v2 import ( - "errors" "fmt" - "os" - "os/exec" - "path/filepath" -) - -func (h *HelmV2) DependencyUpdate(chartPath string) error { - var hasLockFile bool - // sanity check: does the chart directory exist - if chartPath == "" { - return errors.New("empty path to chart supplied") - } - chartInfo, err := os.Stat(chartPath) - switch { - case os.IsNotExist(err): - return fmt.Errorf("chart path %s does not exist", chartPath) - case err != nil: - return err - case !chartInfo.IsDir(): - return fmt.Errorf("chart path %s is not a directory", chartPath) - } - - // check if the requirements file exists - reqFilePath := filepath.Join(chartPath, "requirements.yaml") - reqInfo, err := os.Stat(reqFilePath) - if err != nil || reqInfo.IsDir() { - return nil - } + "github.com/go-kit/kit/log" - // We are going to use `helm dep build`, which tries to update the - // dependencies in charts/ by looking at the file - // `requirements.lock` in the chart directory. If the lockfile - // does not match what is specified in requirements.yaml, it will - // error out. - // - // If that file doesn't exist, `helm dep build` will fall back on - // `helm dep update`, which populates the charts/ directory _and_ - // creates the lockfile. So that it will have the same behaviour - // the next time it attempts a release, remove the lockfile if it - // was created by helm. - lockfilePath := filepath.Join(chartPath, "requirements.lock") - info, err := os.Stat(lockfilePath) - hasLockFile = (err == nil && !info.IsDir()) - if !hasLockFile { - defer os.Remove(lockfilePath) - } + "k8s.io/helm/pkg/downloader" +) - cmd := exec.Command("helm2", "repo", "update") - out, err := cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("could not update repo: %s", string(out)) +func (h *HelmV2) DependencyUpdate(chartPath string) error { + repositoryConfigLock.RLock() + defer repositoryConfigLock.RUnlock() + + out := &logWriter{h.logger} + man := downloader.Manager{ + Out: out, + ChartPath: chartPath, + HelmHome: helmHome(), + Getters: getters, } + return man.Update() +} - cmd = exec.Command("helm2", "dep", "build", ".") - cmd.Dir = chartPath +// logWriter wraps a `log.Logger` so it can be used as an `io.Writer` +type logWriter struct { + log.Logger +} - out, err = cmd.CombinedOutput() - if err != nil { - return fmt.Errorf("could not update dependencies in %s: %s", chartPath, string(out)) +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 } - - return nil + l.Log("info", fmt.Sprintf("%s", p)) + return origLen, nil } From 9bacf8768b9968bef8a4d3466fed6b1b45d5b473 Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Thu, 12 Dec 2019 23:06:26 +0100 Subject: [PATCH 2/2] 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 groundwork 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 | 113 ++++++-------------------------------- pkg/helm/helm.go | 2 + pkg/helm/logwriter.go | 26 +++++++++ pkg/helm/v2/dependency.go | 22 +------- pkg/helm/v2/helm.go | 32 +++++------ pkg/helm/v2/pull.go | 64 +++++++++++++++++++++ pkg/helm/v3/dependency.go | 22 +------- pkg/helm/v3/pull.go | 65 ++++++++++++++++++++++ pkg/release/release.go | 2 +- 9 files changed, 196 insertions(+), 152 deletions(-) create mode 100644 pkg/helm/logwriter.go create mode 100644 pkg/helm/v2/pull.go create mode 100644 pkg/helm/v3/pull.go diff --git a/pkg/chartsync/download.go b/pkg/chartsync/download.go index bbe2bfc3c..7e93c894c 100644 --- a/pkg/chartsync/download.go +++ b/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) } diff --git a/pkg/helm/helm.go b/pkg/helm/helm.go index 30aa19d54..b342d1f6f 100644 --- a/pkg/helm/helm.go +++ b/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 } diff --git a/pkg/helm/logwriter.go b/pkg/helm/logwriter.go new file mode 100644 index 000000000..007391d63 --- /dev/null +++ b/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 +} diff --git a/pkg/helm/v2/dependency.go b/pkg/helm/v2/dependency.go index d9c618916..2ad1cb71a 100644 --- a/pkg/helm/v2/dependency.go +++ b/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 -} diff --git a/pkg/helm/v2/helm.go b/pkg/helm/v2/helm.go index 86820e6a5..f3e577b6f 100644 --- a/pkg/helm/v2/helm.go +++ b/pkg/helm/v2/helm.go @@ -50,19 +50,9 @@ 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 { @@ -70,8 +60,8 @@ func New(logger log.Logger, kubeClient *kubernetes.Clientset, opts TillerOptions 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 { diff --git a/pkg/helm/v2/pull.go b/pkg/helm/v2/pull.go new file mode 100644 index 000000000..1553fbe20 --- /dev/null +++ b/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) +} diff --git a/pkg/helm/v3/dependency.go b/pkg/helm/v3/dependency.go index 1aaf37cc1..06a1cff9d 100644 --- a/pkg/helm/v3/dependency.go +++ b/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 -} diff --git a/pkg/helm/v3/pull.go b/pkg/helm/v3/pull.go new file mode 100644 index 000000000..45e72693a --- /dev/null +++ b/pkg/helm/v3/pull.go @@ -0,0 +1,65 @@ +package v3 + +import ( + "helm.sh/helm/v3/pkg/downloader" + "helm.sh/helm/v3/pkg/repo" + "k8s.io/helm/pkg/urlutil" + + "github.com/fluxcd/helm-operator/pkg/helm" +) + +func (h *HelmV3) Pull(ref, version, dest string) (string, error) { + repositoryConfigLock.RLock() + defer repositoryConfigLock.RUnlock() + + out := helm.NewLogWriter(h.logger) + c := downloader.ChartDownloader{ + Out: out, + Verify: downloader.VerifyNever, + RepositoryConfig: repositoryConfig, + RepositoryCache: repositoryCache, + Getters: getters, + } + d, _, err := c.DownloadTo(ref, version, dest) + return d, err +} + +func (h *HelmV3) 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() + } + 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) +} diff --git a/pkg/release/release.go b/pkg/release/release.go index 8fda6f60f..d34d2a9c5 100644 --- a/pkg/release/release.go +++ b/pkg/release/release.go @@ -128,7 +128,7 @@ func (r *Release) Sync(client helm.Client, hr *v1.HelmRelease) (rHr *v1.HelmRele case hr.Spec.RepoChartSource != nil: var err error - chartPath, err = chartsync.EnsureChartFetched(r.config.ChartCache, hr.Spec.RepoChartSource) + chartPath, err = chartsync.EnsureChartFetched(client, r.config.ChartCache, hr.Spec.RepoChartSource) revision = hr.Spec.RepoChartSource.Version if err != nil {