From 51281c195a019d821082021acdc0fa4614dd74d5 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 16 Jan 2023 02:29:19 -0600 Subject: [PATCH] feat(helm): add ability for --dry-run to do lookup functions When a helm command is run with the --dry-run flag, it will try to connect to the cluster if the value is 'server' to be able to render lookup functions. Closes helm#8137 Signed-off-by: Tapas Kapadia --- cmd/helm/install.go | 24 +++++++++++++++++++++++- cmd/helm/template.go | 2 +- cmd/helm/upgrade.go | 8 +++++++- pkg/action/action.go | 6 +++--- pkg/action/install.go | 16 +++++++++------- pkg/action/install_test.go | 6 +++--- pkg/action/upgrade.go | 16 +++++++++------- 7 files changed, 55 insertions(+), 23 deletions(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 281679e5c..c3555fd05 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -154,7 +154,8 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Install, valueOpts *values.Options) { f.BoolVar(&client.CreateNamespace, "create-namespace", false, "create the release namespace if not present") - f.BoolVar(&client.DryRun, "dry-run", false, "simulate an install") + f.StringVar(&client.DryRun, "dry-run", "none", "simulate an install. If --dry-run is set with no option being specified or as 'client', it will not attempt cluster connections. Setting option as 'server' allows attempting cluster connections.") + f.Lookup("dry-run").NoOptDefVal = "client" f.BoolVar(&client.Force, "force", false, "force resource updates through a replacement strategy") f.BoolVar(&client.DisableHooks, "no-hooks", false, "prevent hooks from running during install") f.BoolVar(&client.Replace, "replace", false, "re-use the given name, only if that name is a deleted release which remains in the history. This is unsafe in production") @@ -261,6 +262,11 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options client.Namespace = settings.Namespace() + // validate dry-run flag value is one of the allowed values + if err := validateDryRunFlag(client); err != nil { + return nil, err + } + // Create context and prepare the handle of SIGTERM ctx := context.Background() ctx, cancel := context.WithCancel(ctx) @@ -301,3 +307,19 @@ func compInstall(args []string, toComplete string, client *action.Install) ([]st } return nil, cobra.ShellCompDirectiveNoFileComp } + +func validateDryRunFlag(client *action.Install) error { + // validate dry-run flag value with set of allowed value + allowedDryRunValues := []string{"false", "true", "none", "client", "server"} + isAllowed := false + for _, v := range allowedDryRunValues { + if client.DryRun == v { + isAllowed = true + break + } + } + if !isAllowed { + return errors.New("Invalid dry-run flag. Flag must one of the following: false, true, none, client, sever") + } + return nil +} diff --git a/cmd/helm/template.go b/cmd/helm/template.go index ce2be55bc..d341ddab9 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -73,7 +73,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.KubeVersion = parsedKubeVersion } - client.DryRun = true + client.DryRun = "client" client.ReleaseName = "release-name" client.Replace = true // Skip the name check client.ClientOnly = !validate diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 02f4cf2a9..3b2325a69 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -120,6 +120,11 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { instClient.Description = client.Description instClient.DependencyUpdate = client.DependencyUpdate + // validate dry-run flag value is one of the allowed values + if err := validateDryRunFlag(instClient); err != nil { + return err + } + rel, err := runInstall(args, instClient, valueOpts, out) if err != nil { return err @@ -214,7 +219,8 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { f.BoolVar(&createNamespace, "create-namespace", false, "if --install is set, create the release namespace if not present") f.BoolVarP(&client.Install, "install", "i", false, "if a release by this name doesn't already exist, run an install") f.BoolVar(&client.Devel, "devel", false, "use development versions, too. Equivalent to version '>0.0.0-0'. If --version is set, this is ignored") - f.BoolVar(&client.DryRun, "dry-run", false, "simulate an upgrade") + f.StringVar(&client.DryRun, "dry-run", "none", "simulate an install. If --dry-run is set with no option being specified or as 'client', it will not attempt cluster connections. Setting option as 'server' allows attempting cluster connections.") + f.Lookup("dry-run").NoOptDefVal = "client" f.BoolVar(&client.Recreate, "recreate-pods", false, "performs pods restart for the resource if applicable") f.MarkDeprecated("recreate-pods", "functionality will no longer be updated. Consult the documentation for other methods to recreate pods") f.BoolVar(&client.Force, "force", false, "force resource updates through a replacement strategy") diff --git a/pkg/action/action.go b/pkg/action/action.go index 16e8e010e..c8a1a9d03 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -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 (cfg *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) { +func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Values, releaseName, outputDir string, subNotes, useReleaseName, includeCrds bool, pr postrender.PostRenderer, dryRun string) ([]*release.Hook, *bytes.Buffer, string, error) { hs := []*release.Hook{} b := bytes.NewBuffer(nil) @@ -121,11 +121,11 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu var err2 error // 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. + // with `--dry-run` with the value of false, none, or sever should 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 && cfg.RESTClientGetter != nil { + if (dryRun == "server" || dryRun == "none" || dryRun == "false") && cfg.RESTClientGetter != nil { restConfig, err := cfg.RESTClientGetter.ToRESTConfig() if err != nil { return hs, b, "", err diff --git a/pkg/action/install.go b/pkg/action/install.go index e2dea1355..4eb802f1f 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -71,7 +71,7 @@ type Install struct { ClientOnly bool Force bool CreateNamespace bool - DryRun bool + DryRun string DisableHooks bool Replace bool Wait bool @@ -128,6 +128,8 @@ type ChartPathOptions struct { func NewInstall(cfg *Configuration) *Install { in := &Install{ cfg: cfg, + // Set default value of DryRun for before flags are binded (tests) + DryRun: "none", } in.ChartPathOptions.registryClient = cfg.RegistryClient @@ -207,7 +209,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma // contacts the upstream server and builds the capabilities object. if crds := chrt.CRDObjects(); !i.ClientOnly && !i.SkipCRDs && len(crds) > 0 { // On dry run, bail here - if i.DryRun { + if i.DryRun != "none" && i.DryRun != "false" { i.cfg.Log("WARNING: This chart or one of its subcharts contains CRDs. Rendering may fail or contain inaccuracies.") } else if err := i.installCRDs(crds); err != nil { return nil, err @@ -241,7 +243,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma } // special case for helm template --is-upgrade - isUpgrade := i.IsUpgrade && i.DryRun + isUpgrade := i.IsUpgrade && (i.DryRun != "none" && i.DryRun != "false") options := chartutil.ReleaseOptions{ Name: i.ReleaseName, Namespace: i.Namespace, @@ -261,8 +263,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma // 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) + 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) // Even for errors, attach this if available if manifestDoc != nil { rel.Manifest = manifestDoc.String() @@ -303,7 +304,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma } // Bail out here if it is a dry run - if i.DryRun { + if i.DryRun != "none" && i.DryRun != "false" { rel.Info.Description = "Dry run complete" return rel, nil } @@ -472,7 +473,8 @@ func (i *Install) availableName() error { if err := chartutil.ValidateReleaseName(start); err != nil { return errors.Wrapf(err, "release name %q", start) } - if i.DryRun { + // On dry run, bail here + if i.DryRun != "none" && i.DryRun != "false" { return nil } diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 3bf3380f9..c669619e3 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -234,7 +234,7 @@ func TestInstallRelease_WithChartAndDependencyAllNotes(t *testing.T) { func TestInstallRelease_DryRun(t *testing.T) { is := assert.New(t) instAction := installAction(t) - instAction.DryRun = true + instAction.DryRun = "true" vals := map[string]interface{}{} res, err := instAction.Run(buildChart(withSampleTemplates()), vals) if err != nil { @@ -258,7 +258,7 @@ func TestInstallRelease_DryRun(t *testing.T) { func TestInstallRelease_DryRun_Lookup(t *testing.T) { is := assert.New(t) instAction := installAction(t) - instAction.DryRun = true + instAction.DryRun = "true" vals := map[string]interface{}{} mockChart := buildChart(withSampleTemplates()) @@ -278,7 +278,7 @@ func TestInstallRelease_DryRun_Lookup(t *testing.T) { func TestInstallReleaseIncorrectTemplate_DryRun(t *testing.T) { is := assert.New(t) instAction := installAction(t) - instAction.DryRun = true + instAction.DryRun = "true" vals := map[string]interface{}{} _, err := instAction.Run(buildChart(withSampleIncludingIncorrectTemplates()), vals) expectedErr := "\"hello/templates/incorrect\" at <.Values.bad.doh>: nil pointer evaluating interface {}.doh" diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 7bdeaae5b..c82704d31 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -70,8 +70,7 @@ type Upgrade struct { // DisableHooks disables hook processing if set to true. DisableHooks bool // DryRun controls whether the operation is prepared, but not executed. - // If `true`, the upgrade is prepared but not performed. - DryRun bool + DryRun string // Force will, if set to `true`, ignore certain warnings and perform the upgrade anyway. // // This should be used with caution. @@ -114,6 +113,8 @@ type resultMessage struct { func NewUpgrade(cfg *Configuration) *Upgrade { up := &Upgrade{ cfg: cfg, + // Set default value of DryRun for before flags are binded (tests) + DryRun: "none", } up.ChartPathOptions.registryClient = cfg.RegistryClient @@ -152,8 +153,8 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, chart *chart. if err != nil { return res, err } - - if !u.DryRun { + // Do not update for dry runs + if u.DryRun == "none" || u.DryRun == "false" { u.cfg.Log("updating status for upgraded release for %s", name) if err := u.cfg.Releases.Update(upgradedRelease); err != nil { return res, err @@ -230,8 +231,8 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin if err != nil { return nil, nil, err } - // Interacts with cluster if possible - hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, true) + + hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, u.DryRun) if err != nil { return nil, nil, err } @@ -309,7 +310,8 @@ func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedR return nil }) - if u.DryRun { + // Run if it is a dry run + if u.DryRun != "none" && u.DryRun != "false" { u.cfg.Log("dry run for %s", upgradedRelease.Name) if len(u.Description) > 0 { upgradedRelease.Info.Description = u.Description