From f2eb84a4941968b5936f62bed4586aa66a2bca24 Mon Sep 17 00:00:00 2001
From: Travis Thompson <trthomps@confluent.io>
Date: Tue, 13 Nov 2018 18:08:18 -0800
Subject: [PATCH] fix(helm): fix 'helm dep up' to check lock digest

When `helm dependency update` is run helm will now correctly check the
digest of the old and new lock file dependencies instead of the digest
of `requirements.yaml` when choosing whether or not to write
`requirements.lock` to disk.

Closes #4011

Signed-off-by: Travis Thompson <trthomps@confluent.io>
---
 pkg/downloader/manager.go      | 29 +++++++++++++----
 pkg/downloader/manager_test.go | 58 ++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 7 deletions(-)

diff --git a/pkg/downloader/manager.go b/pkg/downloader/manager.go
index 67f9dc7bf..f0cf17411 100644
--- a/pkg/downloader/manager.go
+++ b/pkg/downloader/manager.go
@@ -156,14 +156,13 @@ func (m *Manager) Update() error {
 		return err
 	}
 
-	// If the lock file hasn't changed, don't write a new one.
-	oldLock, err := chartutil.LoadRequirementsLock(c)
-	if err == nil && oldLock.Digest == lock.Digest {
-		return nil
+	// Finally, we need to write the lockfile if the dependencies changed.
+	if oldLock, err := chartutil.LoadRequirementsLock(c); err == nil {
+		if changed, err := dependenciesChanged(oldLock.Dependencies, lock.Dependencies); err == nil && changed {
+			return writeLock(m.ChartPath, lock)
+		}
 	}
-
-	// Finally, we need to write the lockfile.
-	return writeLock(m.ChartPath, lock)
+	return nil
 }
 
 func (m *Manager) loadChartDir() (*chart.Chart, error) {
@@ -591,6 +590,22 @@ func (m *Manager) loadChartRepositories() (map[string]*repo.ChartRepository, err
 	return indices, nil
 }
 
+// dependenciesChanged compares the chksum of old and new dependencies and returns
+// true or false if there's a difference
+func dependenciesChanged(oldDep, newDep []*chartutil.Dependency) (bool, error) {
+	newDigest, err := resolver.HashReq(&chartutil.Requirements{Dependencies: newDep})
+	if err != nil {
+		return false, err
+	}
+
+	oldDigest, err := resolver.HashReq(&chartutil.Requirements{Dependencies: oldDep})
+	if err != nil {
+		return false, err
+	}
+
+	return newDigest != oldDigest, nil
+}
+
 // writeLock writes a lockfile to disk
 func writeLock(chartpath string, lock *chartutil.RequirementsLock) error {
 	data, err := yaml.Marshal(lock)
diff --git a/pkg/downloader/manager_test.go b/pkg/downloader/manager_test.go
index 8c2377e47..c9d8c3029 100644
--- a/pkg/downloader/manager_test.go
+++ b/pkg/downloader/manager_test.go
@@ -18,6 +18,7 @@ package downloader
 import (
 	"bytes"
 	"reflect"
+	"strconv"
 	"testing"
 
 	"k8s.io/helm/pkg/chartutil"
@@ -168,3 +169,60 @@ func TestGetRepoNames(t *testing.T) {
 		}
 	}
 }
+
+func TestDependenciesChanged(t *testing.T) {
+	tests := []struct {
+		name   string
+		expect bool
+		oldDep []*chartutil.Dependency
+		newDep []*chartutil.Dependency
+	}{
+		{
+			name:   "did not change",
+			expect: false,
+			oldDep: []*chartutil.Dependency{
+				{
+					Name:       "test",
+					Version:    "1.0.0",
+					Repository: "http://example.com",
+				},
+			},
+			newDep: []*chartutil.Dependency{
+				{
+					Name:       "test",
+					Version:    "1.0.0",
+					Repository: "http://example.com",
+				},
+			},
+		},
+		{
+			name:   "did change",
+			expect: true,
+			oldDep: []*chartutil.Dependency{
+				{
+					Name:       "test",
+					Version:    "1.0.0",
+					Repository: "http://example.com",
+				},
+			},
+			newDep: []*chartutil.Dependency{
+				{
+					Name:       "test",
+					Version:    "1.1.0",
+					Repository: "http://example.com",
+				},
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		changed, err := dependenciesChanged(tt.oldDep, tt.newDep)
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		if changed != tt.expect {
+			t.Errorf("%s: Expected %s, got %s", tt.name, strconv.FormatBool(tt.expect), strconv.FormatBool(changed))
+		}
+	}
+}