From 337f52c566f10d8b68591bb917c8be38b6a3658b Mon Sep 17 00:00:00 2001
From: Karuppiah Natarajan <karuppiah7890@gmail.com>
Date: Mon, 30 Sep 2019 18:20:19 +0530
Subject: [PATCH] fix(pkg/action): fix conditional dependencies not working
 with reuse values

Closes #6530

Signed-off-by: Karuppiah Natarajan <karuppiah7890@gmail.com>
---
 pkg/action/action_test.go  | 18 ++++++++++
 pkg/action/upgrade.go      |  8 ++---
 pkg/action/upgrade_test.go | 73 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 95 insertions(+), 4 deletions(-)

diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go
index 716132853..37a321898 100644
--- a/pkg/action/action_test.go
+++ b/pkg/action/action_test.go
@@ -166,6 +166,12 @@ func buildChart(opts ...chartOption) *chart.Chart {
 	return c.Chart
 }
 
+func withName(name string) chartOption {
+	return func(opts *chartOptions) {
+		opts.Metadata.Name = name
+	}
+}
+
 func withSampleValues() chartOption {
 	values := map[string]interface{}{"someKey": "someValue"}
 	return func(opts *chartOptions) {
@@ -173,6 +179,12 @@ func withSampleValues() chartOption {
 	}
 }
 
+func withValues(values map[string]interface{}) chartOption {
+	return func(opts *chartOptions) {
+		opts.Values = values
+	}
+}
+
 func withNotes(notes string) chartOption {
 	return func(opts *chartOptions) {
 		opts.Templates = append(opts.Templates, &chart.File{
@@ -188,6 +200,12 @@ func withDependency(dependencyOpts ...chartOption) chartOption {
 	}
 }
 
+func withMetadataDependency(dependency chart.Dependency) chartOption {
+	return func(opts *chartOptions) {
+		opts.Metadata.Dependencies = append(opts.Metadata.Dependencies, &dependency)
+	}
+}
+
 func withSampleTemplates() chartOption {
 	return func(opts *chartOptions) {
 		sampleTemplates := []*chart.File{
diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go
index 3d53ff99e..ef91c7ca2 100644
--- a/pkg/action/upgrade.go
+++ b/pkg/action/upgrade.go
@@ -70,10 +70,6 @@ func NewUpgrade(cfg *Configuration) *Upgrade {
 
 // Run executes the upgrade on the given release.
 func (u *Upgrade) Run(name string, chart *chart.Chart, vals map[string]interface{}) (*release.Release, error) {
-	if err := chartutil.ProcessDependencies(chart, vals); err != nil {
-		return nil, err
-	}
-
 	// Make sure if Atomic is set, that wait is set as well. This makes it so
 	// the user doesn't have to specify both
 	u.Wait = u.Wait || u.Atomic
@@ -135,6 +131,10 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
 		return nil, nil, err
 	}
 
+	if err := chartutil.ProcessDependencies(chart, vals); err != nil {
+		return nil, nil, err
+	}
+
 	// finds the non-deleted release with the given name
 	lastRelease, err := u.cfg.Releases.Last(name)
 	if err != nil {
diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go
index 81004e907..13636eede 100644
--- a/pkg/action/upgrade_test.go
+++ b/pkg/action/upgrade_test.go
@@ -19,6 +19,9 @@ package action
 import (
 	"fmt"
 	"testing"
+	"time"
+
+	"helm.sh/helm/pkg/chart"
 
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -155,4 +158,74 @@ func TestUpgradeRelease_ReuseValues(t *testing.T) {
 		is.Equal(release.StatusDeployed, updatedRes.Info.Status)
 		is.Equal(expectedValues, updatedRes.Config)
 	})
+
+	t.Run("reuse values should not install disabled charts", func(t *testing.T) {
+		upAction := upgradeAction(t)
+		chartDefaultValues := map[string]interface{}{
+			"subchart": map[string]interface{}{
+				"enabled": true,
+			},
+		}
+		dependency := chart.Dependency{
+			Name:       "subchart",
+			Version:    "0.1.0",
+			Repository: "http://some-repo.com",
+			Condition:  "subchart.enabled",
+		}
+		sampleChart := buildChart(
+			withName("sample"),
+			withValues(chartDefaultValues),
+			withMetadataDependency(dependency),
+		)
+		now := time.Now()
+		existingValues := map[string]interface{}{
+			"subchart": map[string]interface{}{
+				"enabled": false,
+			},
+		}
+		rel := &release.Release{
+			Name: "nuketown",
+			Info: &release.Info{
+				FirstDeployed: now,
+				LastDeployed:  now,
+				Status:        release.StatusDeployed,
+				Description:   "Named Release Stub",
+			},
+			Chart:   sampleChart,
+			Config:  existingValues,
+			Version: 1,
+		}
+		err := upAction.cfg.Releases.Create(rel)
+		is.NoError(err)
+
+		upAction.ReuseValues = true
+		sampleChartWithSubChart := buildChart(
+			withName(sampleChart.Name()),
+			withValues(sampleChart.Values),
+			withDependency(withName("subchart")),
+			withMetadataDependency(dependency),
+		)
+		// reusing values and upgrading
+		res, err := upAction.Run(rel.Name, sampleChartWithSubChart, map[string]interface{}{})
+		is.NoError(err)
+
+		// Now get the upgraded release
+		updatedRes, err := upAction.cfg.Releases.Get(res.Name, 2)
+		is.NoError(err)
+
+		if updatedRes == nil {
+			is.Fail("Updated Release is nil")
+			return
+		}
+		is.Equal(release.StatusDeployed, updatedRes.Info.Status)
+		is.Equal(0, len(updatedRes.Chart.Dependencies()), "expected 0 dependencies")
+
+		expectedValues := map[string]interface{}{
+			"subchart": map[string]interface{}{
+				"enabled": false,
+				"global":  map[string]interface{}{},
+			},
+		}
+		is.Equal(expectedValues, updatedRes.Config)
+	})
 }