From 9cb90a8b234f98c16aef0918cba92d83061d7c97 Mon Sep 17 00:00:00 2001 From: Matt Farina Date: Fri, 17 Jul 2020 11:54:44 -0400 Subject: [PATCH] Make unmanaged repositories version resolvable If a repository was not know to helm (e.g. added using helm repo add) then Helm would use the range set in the depenencies as the version in the lock file. Lock files should not have ranges since they are locked to versions. Helm did this because the version information for repositories was not know to Helm. This change fixes that by making the repository and chart information known to Helm so it can resolve the versions. Closes #8449 Signed-off-by: Matt Farina --- pkg/downloader/manager.go | 132 ++++++++++++++++++++++++--------- pkg/downloader/manager_test.go | 30 ++++++++ 2 files changed, 126 insertions(+), 36 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 2d6918739..9db5ed928 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -16,6 +16,8 @@ limitations under the License. package downloader import ( + "crypto" + "encoding/hex" "fmt" "io" "io/ioutil" @@ -106,7 +108,13 @@ func (m *Manager) Build() error { } } - if _, err := m.resolveRepoNames(req); err != nil { + repoNames, err := m.resolveRepoNames(req) + if err != nil { + return err + } + + _, err = m.ensureMissingRepos(repoNames, req) + if err != nil { return err } @@ -124,11 +132,6 @@ func (m *Manager) Build() error { } } - // Check that all of the repos we're dependent on actually exist. - if err := m.hasAllRepos(lock.Dependencies); err != nil { - return err - } - if !m.SkipUpdate { // For each repo in the file, update the cached copy of that repo if err := m.UpdateRepositories(); err != nil { @@ -158,14 +161,23 @@ func (m *Manager) Update() error { return nil } - // Check that all of the repos we're dependent on actually exist and - // the repo index names. + // Get the names of the repositories the dependencies need that Helm is + // configured to know about. repoNames, err := m.resolveRepoNames(req) if err != nil { return err } - // For each repo in the file, update the cached copy of that repo + // For the repositories Helm is not configured to know about, ensure Helm + // has some information about them and, when possible, the index files + // locally. + repoNames, err = m.ensureMissingRepos(repoNames, req) + if err != nil { + return err + } + + // For each of the repositories Helm is configured to know about, update + // the index information locally. if !m.SkipUpdate { if err := m.UpdateRepositories(); err != nil { return err @@ -393,38 +405,60 @@ func (m *Manager) safeDeleteDep(name, dir string) error { return nil } -// hasAllRepos ensures that all of the referenced deps are in the local repo cache. -func (m *Manager) hasAllRepos(deps []*chart.Dependency) error { - rf, err := loadRepoConfig(m.RepositoryConfig) - if err != nil { - return err - } - repos := rf.Repositories +// ensureMissingRepos attempts to ensure the repository information for repos +// not managed by Helm is present. This takes in the repoNames Helm is configured +// to work with along with the chart dependencies. It will find the deps not +// in a known repo and attempt to ensure the data is present for steps like +// version resolution. +func (m *Manager) ensureMissingRepos(repoNames map[string]string, deps []*chart.Dependency) (map[string]string, error) { + + var ru []*repo.Entry - // Verify that all repositories referenced in the deps are actually known - // by Helm. - missing := []string{} -Loop: for _, dd := range deps { - // If repo is from local path, continue - if strings.HasPrefix(dd.Repository, "file://") { + + // When the repoName for a dependency is known we can skip ensuring + if _, ok := repoNames[dd.Name]; ok { continue } - if dd.Repository == "" { - continue + // The generated repository name, which will result in an index being + // locally cached, has a name pattern of "helm-manager-" followed by a + // sha256 of the repo name. This assumes end users will never create + // repositories with these names pointing to other repositories. Using + // this method of naming allows the existing repository pulling and + // resolution code to do most of the work. + rn, err := key(dd.Repository) + if err != nil { + return repoNames, err } - for _, repo := range repos { - if urlutil.Equal(repo.URL, strings.TrimSuffix(dd.Repository, "/")) { - continue Loop - } + rn = "helm-manager-" + rn + + repoNames[dd.Name] = rn + + // Assuming the repository is generally available. For Helm managed + // access controls the repository needs to be added through the user + // managed system. This path will work for public charts, like those + // supplied by Bitnami, but not for protected charts, like corp ones + // behind a username and pass. + ri := &repo.Entry{ + Name: rn, + URL: dd.Repository, } - missing = append(missing, dd.Repository) + ru = append(ru, ri) } - if len(missing) > 0 { - return ErrRepoNotFound{missing} + + // Calls to UpdateRepositories (a public function) will only update + // repositories configured by the user. Here we update repos found in + // the dependencies that are not known to the user if update skipping + // is not configured. + if !m.SkipUpdate && len(ru) > 0 { + fmt.Fprintln(m.Out, "Getting updates for unmanaged Helm repositories...") + if err := m.parallelRepoUpdate(ru); err != nil { + return repoNames, err + } } - return nil + + return repoNames, nil } // resolveRepoNames returns the repo names of the referenced deps which can be used to fetch the cached index file @@ -517,16 +551,18 @@ func (m *Manager) UpdateRepositories() error { } repos := rf.Repositories if len(repos) > 0 { + fmt.Fprintln(m.Out, "Hang tight while we grab the latest from your chart repositories...") // This prints warnings straight to out. if err := m.parallelRepoUpdate(repos); err != nil { return err } + fmt.Fprintln(m.Out, "Update Complete. ⎈Happy Helming!⎈") } return nil } func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error { - fmt.Fprintln(m.Out, "Hang tight while we grab the latest from your chart repositories...") + var wg sync.WaitGroup for _, c := range repos { r, err := repo.NewChartRepository(c, m.Getters) @@ -536,15 +572,27 @@ func (m *Manager) parallelRepoUpdate(repos []*repo.Entry) error { wg.Add(1) go func(r *repo.ChartRepository) { if _, err := r.DownloadIndexFile(); err != nil { - fmt.Fprintf(m.Out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", r.Config.Name, r.Config.URL, err) + // For those dependencies that are not known to helm and using a + // generated key name we display the repo url. + if strings.HasPrefix(r.Config.Name, "helm-manager-") { + fmt.Fprintf(m.Out, "...Unable to get an update from the %q chart repository:\n\t%s\n", r.Config.URL, err) + } else { + fmt.Fprintf(m.Out, "...Unable to get an update from the %q chart repository (%s):\n\t%s\n", r.Config.Name, r.Config.URL, err) + } } else { - fmt.Fprintf(m.Out, "...Successfully got an update from the %q chart repository\n", r.Config.Name) + // For those dependencies that are not known to helm and using a + // generated key name we display the repo url. + if strings.HasPrefix(r.Config.Name, "helm-manager-") { + fmt.Fprintf(m.Out, "...Successfully got an update from the %q chart repository\n", r.Config.URL) + } else { + fmt.Fprintf(m.Out, "...Successfully got an update from the %q chart repository\n", r.Config.Name) + } } wg.Done() }(r) } wg.Wait() - fmt.Fprintln(m.Out, "Update Complete. ⎈Happy Helming!⎈") + return nil } @@ -739,3 +787,15 @@ func move(tmpPath, destPath string) error { } return nil } + +// key is used to turn a name, such as a repository url, into a filesystem +// safe name that is unique for querying. To accomplish this a unique hash of +// the string is used. +func key(name string) (string, error) { + in := strings.NewReader(name) + hash := crypto.SHA256.New() + if _, err := io.Copy(hash, in); err != nil { + return "", nil + } + return hex.EncodeToString(hash.Sum(nil)), nil +} diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go index dd3cbbd7b..e60cf7624 100644 --- a/pkg/downloader/manager_test.go +++ b/pkg/downloader/manager_test.go @@ -389,3 +389,33 @@ func TestErrRepoNotFound_Error(t *testing.T) { }) } } + +func TestKey(t *testing.T) { + tests := []struct { + name string + expect string + }{ + { + name: "file:////tmp", + expect: "afeed3459e92a874f6373aca264ce1459bfa91f9c1d6612f10ae3dc2ee955df3", + }, + { + name: "https://example.com/charts", + expect: "7065c57c94b2411ad774638d76823c7ccb56415441f5ab2f5ece2f3845728e5d", + }, + { + name: "foo/bar/baz", + expect: "15c46a4f8a189ae22f36f201048881d6c090c93583bedcf71f5443fdef224c82", + }, + } + + for _, tt := range tests { + o, err := key(tt.name) + if err != nil { + t.Fatalf("unable to generate key for %q with error: %s", tt.name, err) + } + if o != tt.expect { + t.Errorf("wrong key name generated for %q, expected %q but got %q", tt.name, tt.expect, o) + } + } +}