Skip to content

Commit

Permalink
feat(helm): add ability for --dry-run to do lookup functions
Browse files Browse the repository at this point in the history
When a helm command is run with the --dry-run flag, it will try to connect to the cluster
to be able to render lookup functions.
Closes #8137

Signed-off-by: Tapas Kapadia <tapaskapadia10@gmail.com>
  • Loading branch information
tapaskapadia committed Feb 28, 2021
1 parent b30f61d commit eb60bb6
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 11 deletions.
14 changes: 7 additions & 7 deletions pkg/action/action.go
Expand Up @@ -102,7 +102,7 @@ type Configuration struct {
// 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 (c *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 (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, interactWithRemote bool) ([]*release.Hook, *bytes.Buffer, string, error) {
hs := []*release.Hook{}
b := bytes.NewBuffer(nil)

Expand All @@ -120,12 +120,12 @@ func (c *Configuration) renderResources(ch *chart.Chart, values chartutil.Values
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 && c.RESTClientGetter != nil {
// A `helm template` should not talk to the remote cluster. However, commands
// with `--dry-run` should be able to try to connect to the cluster.
// This enables the ability to render 'lookup' functions.
// It may break in interesting and exotic ways because other data (e.g. discovery)
// is mocked.
if interactWithRemote && c.RESTClientGetter != nil {
rest, err := c.RESTClientGetter.ToRESTConfig()
if err != nil {
return hs, b, "", err
Expand Down
7 changes: 6 additions & 1 deletion pkg/action/install.go
Expand Up @@ -237,7 +237,12 @@ 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)
// Determines whether `helm template` was used or another command with the --dry-run flag
// as they both set the Install.DryRun field to `true`. The `--dry-run` flag should be able
// to connect to remote for the lookup function. `helm template` is the only command that
// Install.APIVersions field will not be nil.
interactWithRemote := !i.DryRun || i.APIVersions == nil
rel.Hooks, manifestDoc, rel.Info.Notes, err = i.cfg.renderResources(chrt, valuesToRender, i.ReleaseName, i.OutputDir, i.SubNotes, i.UseReleaseName, i.IncludeCRDs, i.PostRenderer, interactWithRemote)
// Even for errors, attach this if available
if manifestDoc != nil {
rel.Manifest = manifestDoc.String()
Expand Down
2 changes: 1 addition & 1 deletion pkg/action/install_test.go
Expand Up @@ -241,7 +241,7 @@ func TestInstallRelease_DryRun(t *testing.T) {
is.Equal(res.Info.Description, "Dry run complete")
}

// Regression test for #7955: Lookup must not connect to Kubernetes on a dry-run.
// Regression test for #7955
func TestInstallRelease_DryRun_Lookup(t *testing.T) {
is := assert.New(t)
instAction := installAction(t)
Expand Down
4 changes: 2 additions & 2 deletions pkg/action/upgrade.go
Expand Up @@ -211,8 +211,8 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin
if err != nil {
return nil, nil, err
}

hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun)
// Interacts with cluster if possible
hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, true)
if err != nil {
return nil, nil, err
}
Expand Down

0 comments on commit eb60bb6

Please sign in to comment.