diff --git a/pkg/action/action.go b/pkg/action/action.go index f093ed7f8..d485cf901 100644 --- a/pkg/action/action.go +++ b/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. - // 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) - } + files, err = render(ch, values) - if err2 != nil { + if err != nil { return hs, b, "", err2 } diff --git a/pkg/action/action_test.go b/pkg/action/action_test.go index fedf260fb..96b3b1653 100644 --- a/pkg/action/action_test.go +++ b/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") +} diff --git a/pkg/action/install.go b/pkg/action/install.go index c33d6bf3c..d75f86764 100644 --- a/pkg/action/install.go +++ b/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() diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 07d9cb40e..b1133904d 100644 --- a/pkg/action/upgrade.go +++ b/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 }