From c72caf6a11c986e6607e583337e4b77e58a45bdb Mon Sep 17 00:00:00 2001
From: Hang Park <hangpark@kaist.ac.kr>
Date: Tue, 22 Oct 2019 12:38:49 +0900
Subject: [PATCH] fix(pkg/downloader): Calculate digest from both Chart.lock &
 Chart.yaml deps

To make digests include information about Chart.yaml dependencies, not
only the lock file, digest calculation is changed to accept both
contents. This terminates the `dep build` command if Chart.yaml
dependencies have been updated so that `dep up` should be executed
properly, to prevent downloading wrong versions or mismatched subcharts.

Note that previous Helm cannot know whether Chart.yaml dependencies were changed
or not since the Chart.lock's digest is calculated by only Chart.lock
contents, which don't include information about SemVer ranges and extra
dependency fields such as aliases, conditions, and tags. Specially,
SemVer can be written as a version range in Chart.yaml, but Chart.lock
has the specific, resolved version of that range.

Signed-off-by: Hang Park <hangpark@kaist.ac.kr>
---
 internal/resolver/resolver.go      |  6 +--
 internal/resolver/resolver_test.go | 75 +++++++++++++++++++++++-------
 pkg/downloader/manager.go          |  2 +-
 3 files changed, 61 insertions(+), 22 deletions(-)

diff --git a/internal/resolver/resolver.go b/internal/resolver/resolver.go
index 20b62dc66..e4dbab227 100644
--- a/internal/resolver/resolver.go
+++ b/internal/resolver/resolver.go
@@ -132,7 +132,7 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 		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)
+	digest, err := HashReq(reqs, locked)
 	if err != nil {
 		return nil, err
 	}
@@ -148,8 +148,8 @@ func (r *Resolver) Resolve(reqs []*chart.Dependency, repoNames map[string]string
 //
 // This should be used only to compare against another hash generated by this
 // function.
-func HashReq(req []*chart.Dependency) (string, error) {
-	data, err := json.Marshal(req)
+func HashReq(req, lock []*chart.Dependency) (string, error) {
+	data, err := json.Marshal([2][]*chart.Dependency{req, lock})
 	if err != nil {
 		return "", err
 	}
diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go
index 3af62b811..3828771cc 100644
--- a/internal/resolver/resolver_test.go
+++ b/internal/resolver/resolver_test.go
@@ -130,7 +130,7 @@ func TestResolve(t *testing.T) {
 				t.Fatalf("Expected error in test %q", tt.name)
 			}
 
-			if h, err := HashReq(tt.expect.Dependencies); err != nil {
+			if h, err := HashReq(tt.req, tt.expect.Dependencies); err != nil {
 				t.Fatal(err)
 			} else if h != l.Digest {
 				t.Errorf("%q: hashes don't match.", tt.name)
@@ -156,24 +156,63 @@ func TestResolve(t *testing.T) {
 }
 
 func TestHashReq(t *testing.T) {
-	expect := "sha256:d661820b01ed7bcf26eed8f01cf16380e0a76326ba33058d3150f919d9b15bc0"
-	req := []*chart.Dependency{
-		{Name: "alpine", Version: "0.1.0", Repository: "http://localhost:8879/charts"},
-	}
-	h, err := HashReq(req)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if expect != h {
-		t.Errorf("Expected %q, got %q", expect, h)
-	}
+	expect := "sha256:fb239e836325c5fa14b29d1540a13b7d3ba13151b67fe719f820e0ef6d66aaaf"
 
-	req = []*chart.Dependency{}
-	h, err = HashReq(req)
-	if err != nil {
-		t.Fatal(err)
+	tests := []struct {
+		name         string
+		chartVersion string
+		lockVersion  string
+		wantError    bool
+	}{
+		{
+			name:         "chart with the expected digest",
+			chartVersion: "0.1.0",
+			lockVersion:  "0.1.0",
+			wantError:    false,
+		},
+		{
+			name:         "ranged version but same resolved lock version",
+			chartVersion: "^0.1.0",
+			lockVersion:  "0.1.0",
+			wantError:    true,
+		},
+		{
+			name:         "ranged version resolved as higher version",
+			chartVersion: "^0.1.0",
+			lockVersion:  "0.1.2",
+			wantError:    true,
+		},
+		{
+			name:         "different version",
+			chartVersion: "0.1.2",
+			lockVersion:  "0.1.2",
+			wantError:    true,
+		},
+		{
+			name:         "different version with a range",
+			chartVersion: "^0.1.2",
+			lockVersion:  "0.1.2",
+			wantError:    true,
+		},
 	}
-	if expect == h {
-		t.Errorf("Expected %q !=  %q", expect, h)
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			req := []*chart.Dependency{
+				{Name: "alpine", Version: tt.chartVersion, Repository: "http://localhost:8879/charts"},
+			}
+			lock := []*chart.Dependency{
+				{Name: "alpine", Version: tt.lockVersion, Repository: "http://localhost:8879/charts"},
+			}
+			h, err := HashReq(req, lock)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if !tt.wantError && expect != h {
+				t.Errorf("Expected %q, got %q", expect, h)
+			} else if tt.wantError && expect == h {
+				t.Errorf("Expected not %q, but same", expect)
+			}
+		})
 	}
 }
diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go
index cc3af74db..ac61ac5ac 100644
--- a/pkg/downloader/manager.go
+++ b/pkg/downloader/manager.go
@@ -79,7 +79,7 @@ func (m *Manager) Build() error {
 	}
 
 	req := c.Metadata.Dependencies
-	if sum, err := resolver.HashReq(req); err != nil || sum != lock.Digest {
+	if sum, err := resolver.HashReq(req, lock.Dependencies); err != nil || sum != lock.Digest {
 		return errors.New("Chart.lock is out of sync with Chart.yaml")
 	}