From 78330ba3d960802d106a2f79e8595ba56e4babe2 Mon Sep 17 00:00:00 2001 From: Matthew Fisher Date: Tue, 11 Jun 2019 16:09:21 -0700 Subject: [PATCH] fix(resolver): compare hash of lockfile against resolved dependencies Signed-off-by: Matthew Fisher --- pkg/downloader/manager.go | 13 +++---------- pkg/resolver/resolver.go | 10 ++++++++-- pkg/resolver/resolver_test.go | 9 ++------- 3 files changed, 13 insertions(+), 19 deletions(-) diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go index 779857405..422c5b438 100644 --- a/pkg/downloader/manager.go +++ b/pkg/downloader/manager.go @@ -121,13 +121,6 @@ func (m *Manager) Update() error { return nil } - // Hash dependencies - // FIXME should this hash all of Chart.yaml - hash, err := resolver.HashReq(req) - if err != nil { - return err - } - // Check that all of the repos we're dependent on actually exist and // the repo index names. repoNames, err := m.getRepoNames(req) @@ -144,7 +137,7 @@ func (m *Manager) Update() error { // Now we need to find out which version of a chart best satisfies the // dependencies in the Chart.yaml - lock, err := m.resolve(req, repoNames, hash) + lock, err := m.resolve(req, repoNames) if err != nil { return err } @@ -176,9 +169,9 @@ func (m *Manager) loadChartDir() (*chart.Chart, error) { // resolve takes a list of dependencies and translates them into an exact version to download. // // This returns a lock file, which has all of the dependencies normalized to a specific version. -func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string, hash string) (*chart.Lock, error) { +func (m *Manager) resolve(req []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) { res := resolver.New(m.ChartPath, m.HelmHome) - return res.Resolve(req, repoNames, hash) + return res.Resolve(req, repoNames) } // downloadAll takes a list of dependencies and downloads them into charts/ diff --git a/pkg/resolver/resolver.go b/pkg/resolver/resolver.go index 1242be8b8..866574f81 100644 --- a/pkg/resolver/resolver.go +++ b/pkg/resolver/resolver.go @@ -47,7 +47,7 @@ func New(chartpath string, helmhome helmpath.Home) *Resolver { } // Resolve resolves dependencies and returns a lock file with the resolution. -func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string, d string) (*chart.Lock, error) { +func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string) (*chart.Lock, error) { // Now we clone the dependencies, locking as we go. locked := make([]*chart.Dependency, len(reqs)) @@ -107,9 +107,15 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string if len(missing) > 0 { return nil, errors.Errorf("can't get a valid version for repositories %s. Try changing the version constraint in Chart.yaml", strings.Join(missing, ", ")) } + + digest, err := HashReq(locked) + if err != nil { + return nil, err + } + return &chart.Lock{ Generated: time.Now(), - Digest: d, + Digest: digest, Dependencies: locked, }, nil } diff --git a/pkg/resolver/resolver_test.go b/pkg/resolver/resolver_test.go index c52088fc8..3e1b55b12 100644 --- a/pkg/resolver/resolver_test.go +++ b/pkg/resolver/resolver_test.go @@ -90,12 +90,7 @@ func TestResolve(t *testing.T) { repoNames := map[string]string{"alpine": "kubernetes-charts", "redis": "kubernetes-charts"} r := New("testdata/chartpath", "testdata/helmhome") for _, tt := range tests { - hash, err := HashReq(tt.req) - if err != nil { - t.Fatal(err) - } - - l, err := r.Resolve(tt.req, repoNames, hash) + l, err := r.Resolve(tt.req, repoNames) if err != nil { if tt.err { continue @@ -107,7 +102,7 @@ func TestResolve(t *testing.T) { t.Fatalf("Expected error in test %q", tt.name) } - if h, err := HashReq(tt.req); err != nil { + if h, err := HashReq(tt.expect.Dependencies); err != nil { t.Fatal(err) } else if h != l.Digest { t.Errorf("%q: hashes don't match.", tt.name)