upgrade [--dry-run] is allowed to contact cluster

Signed-off-by: christopher dykstra <chrdyks@gmail.com>

pkg/action/action.go

@@ -97,12 +97,44 @@ type Configuration struct {
 	Log func(string, ...interface{})
 }
 
-// renderResources renders the templates in a chart
+type renderStrategyFunction = func(ch *chart.Chart, values chartutil.Values) (map[string]string, error)
+
+var renderStrategy = func(cfg *Configuration) renderStrategyFunction {
+	if cfg.RESTClientGetter != nil {
+		return func(ch *chart.Chart, values chartutil.Values) (map[string]string, error) {
+			restConfig, err := cfg.RESTClientGetter.ToRESTConfig()
+			if err != nil {
+				return nil, err
+			}
+
+			return engine.RenderWithClient(ch, values, restConfig)
+		}
+	}
+
+	return localRenderStrategy
+}
+
+var localRenderStrategy renderStrategyFunction = engine.Render
+
+func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer) ([]*release.Hook, *bytes.Buffer, string, error) {
+	return cfg.render(ch, values, releaseName, outputDir, subNotes, useReleaseName, includeCrds, pr, renderStrategy(cfg))
+}
+
+// A `helm template` or `helm install --dry-run` should not talk to the remote cluster.
+// It will break in interesting and exotic ways because other data (e.g. discovery)
+// is mocked. It is not up to the template author to decide when the user wants to
+// connect to the cluster. So when the user says to dry run, respect the user's
+// wishes and do not connect to the cluster.
+func (cfg *Configuration) renderResourcesLocally(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer) ([]*release.Hook, *bytes.Buffer, string, error) {
+	return cfg.render(ch, values, releaseName, outputDir, subNotes, useReleaseName, includeCrds, pr, localRenderStrategy)
+}
+
+// render renders the templates in a chart
 //
 // TODO: This function is badly in need of a refactor.
 // TODO: As part of the refactor the duplicate code in cmd/helm/template.go should be removed
 //       This code has to do with writing files to disk.
-func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, dryRun bool) ([]*release.Hook, *bytes.Buffer, string, error) {
+func (cfg *Configuration) render(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, render renderStrategyFunction) ([]*release.Hook, *bytes.Buffer, string, error) {
 	hs := []*release.Hook{}
 	b := bytes.NewBuffer(nil)
 
@@ -120,22 +152,9 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu
 	var files map[string]string
 	var err2 error
 
-	// A `helm template` or `helm install --dry-run` should not talk to the remote cluster.
+	files, err = render(ch, values)
-	// It will break in interesting and exotic ways because other data (e.g. discovery)
-	// is mocked. It is not up to the template author to decide when the user wants to
-	// connect to the cluster. So when the user says to dry run, respect the user's
-	// wishes and do not connect to the cluster.
-	if !dryRun && cfg.RESTClientGetter != nil {
-		restConfig, err := cfg.RESTClientGetter.ToRESTConfig()
-		if err != nil {
-			return hs, b, "", err
-		}
-		files, err2 = engine.RenderWithClient(ch, values, restConfig)
-	} else {
-		files, err2 = engine.Render(ch, values)
-	}
 
-	if err2 != nil {
+	if err != nil {
 		return hs, b, "", err2
 	}
 

pkg/action/action_test.go

@@ -25,12 +25,14 @@ import (
 	"testing"
 
 	dockerauth "github.com/deislabs/oras/pkg/auth/docker"
+	"github.com/stretchr/testify/assert"
 	fakeclientset "k8s.io/client-go/kubernetes/fake"
 
 	"helm.sh/helm/v3/internal/experimental/registry"
 	"helm.sh/helm/v3/pkg/chart"
 	"helm.sh/helm/v3/pkg/chartutil"
 	kubefake "helm.sh/helm/v3/pkg/kube/fake"
+	"helm.sh/helm/v3/pkg/postrender"
 	"helm.sh/helm/v3/pkg/release"
 	"helm.sh/helm/v3/pkg/storage"
 	"helm.sh/helm/v3/pkg/storage/driver"
@@ -319,3 +321,73 @@ func TestGetVersionSet(t *testing.T) {
 		t.Error("Non-existent version is reported found.")
 	}
 }
+
+func TestRenderResources_RendersWithStrategy(t *testing.T) {
+	var renderedLocally = false
+	var rendered = false
+	var renderedChart *chart.Chart
+	var renderedValues *chartutil.Values
+	var nullRenderStrategy = func(ch *chart.Chart, values chartutil.Values) (map[string]string, error) {
+		renderedChart = ch
+		renderedValues = &values
+
+		return make(map[string]string), nil
+	}
+	renderStrategy = func(cfg *Configuration) func(ch *chart.Chart, values chartutil.Values) (map[string]string, error) {
+		rendered = true
+
+		return nullRenderStrategy
+	}
+	localRenderStrategy = func(ch *chart.Chart, values chartutil.Values) (map[string]string, error) {
+		renderedLocally = true
+
+		return nullRenderStrategy(ch, values)
+	}
+
+	var chart = buildChart()
+	values := chartutil.Values{
+		"is": "me",
+	}
+	var postRender postrender.PostRenderer
+	actionConfigFixture(t).renderResources(chart, values, "", "", false, false, false, postRender)
+
+	assert.True(t, rendered, "the chart should be rendered with cluster contact")
+	assert.False(t, renderedLocally, "the chart should not be rendered without cluster contact")
+	assert.Same(t, chart, renderedChart, "the rendered chart and the chart resource should be the same")
+	assert.Equal(t, values, *renderedValues, "the rendered values and the values resource should be the same")
+}
+
+func TestRenderResourcesLocally_RendersWithLocalStrategy(t *testing.T) {
+	var renderedLocally = false
+	var rendered = false
+	var renderedChart *chart.Chart
+	var renderedValues *chartutil.Values
+	var nullRenderStrategy = func(ch *chart.Chart, values chartutil.Values) (map[string]string, error) {
+		renderedChart = ch
+		renderedValues = &values
+
+		return make(map[string]string), nil
+	}
+	renderStrategy = func(cfg *Configuration) func(ch *chart.Chart, values chartutil.Values) (map[string]string, error) {
+		rendered = true
+
+		return nullRenderStrategy
+	}
+	localRenderStrategy = func(ch *chart.Chart, values chartutil.Values) (map[string]string, error) {
+		renderedLocally = true
+
+		return nullRenderStrategy(ch, values)
+	}
+
+	var chart = buildChart()
+	values := chartutil.Values{
+		"is": "me",
+	}
+	var postRender postrender.PostRenderer
+	actionConfigFixture(t).renderResourcesLocally(chart, values, "", "", false, false, false, postRender)
+
+	assert.False(t, rendered, "the chart should note be rendered with cluster contact")
+	assert.True(t, renderedLocally, "the chart should be rendered without cluster contact")
+	assert.Same(t, chart, renderedChart, "the rendered chart and the chart resource should be the same")
+	assert.Equal(t, values, *renderedValues, "the rendered values and the values resource should be the same")
+}

pkg/action/install.go

@@ -242,7 +242,11 @@ func (i *Install) Run(chrt *chart.Chart, vals map[string]interface{}) (*release.
 	rel := i.createRelease(chrt, vals)
 
 	var manifestDoc *bytes.Buffer
-	rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, i.DryRun)
+	if i.DryRun {
+		rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResourcesLocally(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer)
+	} else {
+		rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer)
+	}
 	// Even for errors, attach this if available
 	if manifestDoc != nil {
 		rel.Manifest = manifestDoc.String()

pkg/action/upgrade.go

@@ -214,7 +214,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
 		return nil, nil, err
 	}
 
-	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun)
+	hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer)
 	if err != nil {
 		return nil, nil, err
 	}