From 92a6640f8a56507f27c2f2732115a787d8d1ed6d Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Sun, 28 Feb 2021 02:02:55 -0600 Subject: [PATCH 01/12] 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 to be able to render lookup functions. Closes #8137 Signed-off-by: Tapas Kapadia --- pkg/action/action.go | 14 +++++++------- pkg/action/install.go | 7 ++++++- pkg/action/install_test.go | 2 +- pkg/action/upgrade.go | 4 ++-- 4 files changed, 16 insertions(+), 11 deletions(-) diff --git a/pkg/action/action.go b/pkg/action/action.go index 82760250f..16e8e010e 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, dryRun 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, interactWithRemote bool) ([]*release.Hook, *bytes.Buffer, string, error) { hs := []*release.Hook{} b := bytes.NewBuffer(nil) @@ -120,12 +120,12 @@ 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 { + // 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 && 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 425b66f69..e2dea1355 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -257,7 +257,12 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma 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() diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index 45e5a2670..3bf3380f9 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -254,7 +254,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) diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 690397d4a..7bdeaae5b 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -230,8 +230,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 } From 51281c195a019d821082021acdc0fa4614dd74d5 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 16 Jan 2023 02:29:19 -0600 Subject: [PATCH 02/12] 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 From 4d67dfabaa916183297200ed8e6b05f4de261fb1 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 16 Jan 2023 12:25:43 -0600 Subject: [PATCH 03/12] 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 #8137 Signed-off-by: Tapas Kapadia --- cmd/helm/install.go | 6 +++--- cmd/helm/upgrade.go | 9 ++++----- pkg/action/action.go | 2 +- pkg/action/install.go | 2 +- pkg/action/upgrade.go | 2 +- 5 files changed, 10 insertions(+), 11 deletions(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index c3555fd05..57d70cabf 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -263,7 +263,7 @@ 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 { + if err := validateDryRunFlag(client.DryRun); err != nil { return nil, err } @@ -308,12 +308,12 @@ func compInstall(args []string, toComplete string, client *action.Install) ([]st return nil, cobra.ShellCompDirectiveNoFileComp } -func validateDryRunFlag(client *action.Install) error { +func validateDryRunFlag(dryRunFlagValue string) 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 { + if dryRunFlagValue == v { isAllowed = true break } diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 3b2325a69..adbacd1fd 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -120,11 +120,6 @@ 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 @@ -144,6 +139,10 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if err != nil { return err } + // validate dry-run flag value is one of the allowed values + if err := validateDryRunFlag(client.DryRun); err != nil { + return err + } p := getter.All(settings) vals, err := valueOpts.MergeValues(p) diff --git a/pkg/action/action.go b/pkg/action/action.go index c8a1a9d03..f59a31853 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -125,7 +125,7 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu // 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 (dryRun == "server" || dryRun == "none" || dryRun == "false") && 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 4eb802f1f..f1c9176fc 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -243,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 != "none" && i.DryRun != "false") + isUpgrade := i.IsUpgrade && (i.DryRun != "none" && i.DryRun != "false") options := chartutil.ReleaseOptions{ Name: i.ReleaseName, Namespace: i.Namespace, diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index c82704d31..f0e246156 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -154,7 +154,7 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, chart *chart. return res, err } // Do not update for dry runs - if u.DryRun == "none" || u.DryRun == "false" { + 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 From fc16ea7d44e353cc365c6d59031ef8157d2d9865 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 16 Jan 2023 12:34:01 -0600 Subject: [PATCH 04/12] 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 #8137 Signed-off-by: Tapas Kapadia --- cmd/helm/install.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 57d70cabf..a5589918d 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -320,6 +320,6 @@ func validateDryRunFlag(dryRunFlagValue string) error { } if !isAllowed { return errors.New("Invalid dry-run flag. Flag must one of the following: false, true, none, client, sever") - } + } return nil } From be99ebe8af768b5e83892771c0eccdaa329771d9 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 16 Jan 2023 16:10:25 -0600 Subject: [PATCH 05/12] 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 #8137 Signed-off-by: Tapas Kapadia --- pkg/action/install.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index f1c9176fc..4d3e6ce6d 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -259,10 +259,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma rel := i.createRelease(chrt, vals) var manifestDoc *bytes.Buffer - // 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. + 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 { From ddb33580dbcfd8443208c814cd4df4220b57e8dd Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 23 Jan 2023 13:18:59 -0600 Subject: [PATCH 06/12] feat(helm): add ability for a dry-run to evaluate lookup functions When a helm command is run with the --dry-run-option=server flag, it will try to connect to the cluster to be able to render lookup functions. Closes #8137 Signed-off-by: Tapas Kapadia --- cmd/helm/install.go | 13 ++++++++----- cmd/helm/template.go | 1 - cmd/helm/upgrade.go | 11 +++++++---- pkg/action/action.go | 8 ++++---- pkg/action/install.go | 26 +++++++++++++++++--------- pkg/action/install_test.go | 6 +++--- pkg/action/upgrade.go | 25 +++++++++++++++++++------ 7 files changed, 58 insertions(+), 32 deletions(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index a5589918d..580a194dd 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -154,8 +154,9 @@ 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.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.DryRun, "dry-run", false, "simulate an install") + f.StringVar(&client.DryRunOption, "dry-run-option", "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-option").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") @@ -174,6 +175,8 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal addValueOptionsFlags(f, valueOpts) addChartPathOptionsFlags(f, &client.ChartPathOptions) + cmd.MarkFlagsMutuallyExclusive("dry-run", "dry-run-option") + err := cmd.RegisterFlagCompletionFunc("version", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { requiredArgs := 2 if client.GenerateName { @@ -263,7 +266,7 @@ 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.DryRun); err != nil { + if err := validateDryRunOptionFlag(client.DryRunOption); err != nil { return nil, err } @@ -308,12 +311,12 @@ func compInstall(args []string, toComplete string, client *action.Install) ([]st return nil, cobra.ShellCompDirectiveNoFileComp } -func validateDryRunFlag(dryRunFlagValue string) error { +func validateDryRunOptionFlag(dryRunOptionFlagValue string) 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 dryRunFlagValue == v { + if dryRunOptionFlagValue == v { isAllowed = true break } diff --git a/cmd/helm/template.go b/cmd/helm/template.go index d341ddab9..93f454fe5 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -73,7 +73,6 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { client.KubeVersion = parsedKubeVersion } - 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 adbacd1fd..34d8ab24b 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -106,6 +106,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { instClient.ChartPathOptions = client.ChartPathOptions instClient.Force = client.Force instClient.DryRun = client.DryRun + instClient.DryRunOption = client.DryRunOption instClient.DisableHooks = client.DisableHooks instClient.SkipCRDs = client.SkipCRDs instClient.Timeout = client.Timeout @@ -119,7 +120,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { instClient.SubNotes = client.SubNotes instClient.Description = client.Description instClient.DependencyUpdate = client.DependencyUpdate - rel, err := runInstall(args, instClient, valueOpts, out) if err != nil { return err @@ -140,7 +140,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { return err } // validate dry-run flag value is one of the allowed values - if err := validateDryRunFlag(client.DryRun); err != nil { + if err := validateDryRunOptionFlag(client.DryRunOption); err != nil { return err } @@ -218,8 +218,9 @@ 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.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.DryRun, "dry-run", false, "simulate an upgrade") + f.StringVar(&client.DryRunOption, "dry-run-option", "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-option").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") @@ -242,6 +243,8 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { bindOutputFlag(cmd, &outfmt) bindPostRenderFlag(cmd, &client.PostRenderer) + cmd.MarkFlagsMutuallyExclusive("dry-run", "dry-run-option") + err := cmd.RegisterFlagCompletionFunc("version", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if len(args) != 2 { return nil, cobra.ShellCompDirectiveNoFileComp diff --git a/pkg/action/action.go b/pkg/action/action.go index f59a31853..01a49e477 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, dryRun string) ([]*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, interactWithRemote bool) ([]*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` with the value of false, none, or sever should try to connect to the cluster. - // This enables the ability to render 'lookup' functions. + // with the flag `--dry-run-option` with the value of false, none, or sever + // or with the flag `--dry-run` with the value of false should try to interact with the cluster. // It may break in interesting and exotic ways because other data (e.g. discovery) // is mocked. - if (dryRun == "server" || dryRun == "none" || dryRun == "false") && cfg.RESTClientGetter != nil { + if interactWithRemote && 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 4d3e6ce6d..3abe102a4 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -71,7 +71,8 @@ type Install struct { ClientOnly bool Force bool CreateNamespace bool - DryRun string + DryRun bool + DryRunOption string DisableHooks bool Replace bool Wait bool @@ -128,8 +129,6 @@ 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 @@ -205,11 +204,21 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma return nil, err } + // determine dry run behavior + if i.DryRun || i.DryRunOption == "client" || i.DryRunOption == "server" || i.DryRunOption == "true" { + i.DryRun = true + } + + var interactWithRemote bool + if !i.DryRun || i.DryRunOption == "server" { + interactWithRemote = true + } + // Pre-install anything in the crd/ directory. We do this before Helm // 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 != "none" && i.DryRun != "false" { + if i.DryRun { 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 @@ -243,7 +252,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 != "none" && i.DryRun != "false") + isUpgrade := i.IsUpgrade && i.DryRun options := chartutil.ReleaseOptions{ Name: i.ReleaseName, Namespace: i.Namespace, @@ -259,8 +268,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma 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) + 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() @@ -301,7 +309,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 != "none" && i.DryRun != "false" { + if i.DryRun { rel.Info.Description = "Dry run complete" return rel, nil } @@ -471,7 +479,7 @@ func (i *Install) availableName() error { return errors.Wrapf(err, "release name %q", start) } // On dry run, bail here - if i.DryRun != "none" && i.DryRun != "false" { + if i.DryRun { return nil } diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index c669619e3..3bf3380f9 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 f0e246156..817486465 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -70,7 +70,9 @@ type Upgrade struct { // DisableHooks disables hook processing if set to true. DisableHooks bool // DryRun controls whether the operation is prepared, but not executed. - DryRun string + DryRun bool + // DryRunOption controls whether the operation is prepared, but not executed with options on whether or not to interact with the remote cluster. + DryRunOption string // Force will, if set to `true`, ignore certain warnings and perform the upgrade anyway. // // This should be used with caution. @@ -113,8 +115,6 @@ 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 @@ -140,6 +140,12 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, chart *chart. if err := chartutil.ValidateReleaseName(name); err != nil { return nil, errors.Errorf("release name is invalid: %s", name) } + + // determine dry run behavior + if u.DryRun || u.DryRunOption == "client" || u.DryRunOption == "server" || u.DryRunOption == "true" { + u.DryRun = true + } + u.cfg.Log("preparing upgrade for %s", name) currentRelease, upgradedRelease, err := u.prepareUpgrade(name, chart, vals) if err != nil { @@ -153,8 +159,9 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, chart *chart. if err != nil { return res, err } + // Do not update for dry runs - if u.DryRun == "none" || u.DryRun == "false" { + if !u.DryRun { u.cfg.Log("updating status for upgraded release for %s", name) if err := u.cfg.Releases.Update(upgradedRelease); err != nil { return res, err @@ -232,7 +239,13 @@ 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) + // determine whether or not to interact with remote + var interactWithRemote bool + if !u.DryRun || u.DryRunOption == "server" { + interactWithRemote = true + } + + hooks, manifestDoc, notesTxt, err := u.cfg.renderResources(chart, valuesToRender, "", "", u.SubNotes, false, false, u.PostRenderer, interactWithRemote) if err != nil { return nil, nil, err } @@ -311,7 +324,7 @@ func (u *Upgrade) performUpgrade(ctx context.Context, originalRelease, upgradedR }) // Run if it is a dry run - if u.DryRun != "none" && u.DryRun != "false" { + if u.DryRun { u.cfg.Log("dry run for %s", upgradedRelease.Name) if len(u.Description) > 0 { upgradedRelease.Info.Description = u.Description From d66c7db55a56a88f312ca579f16feaf2b060d7be Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 23 Jan 2023 13:25:32 -0600 Subject: [PATCH 07/12] feat(helm): add ability for a dry-run to evaluate lookup functions When a helm command is run with the --dry-run-option=server flag, it will try to connect to the cluster to be able to render lookup functions. Closes #8137 Signed-off-by: Tapas Kapadia --- pkg/action/action.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/action.go b/pkg/action/action.go index 01a49e477..c19a65842 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -121,7 +121,7 @@ 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 the flag `--dry-run-option` with the value of false, none, or sever + // with the flag `--dry-run-option` with the value of false, none, or sever // or with the flag `--dry-run` with the value of false should try to interact with the cluster. // It may break in interesting and exotic ways because other data (e.g. discovery) // is mocked. From 5ec7913fd41920e117e20b5229676972ebea06a1 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 23 Jan 2023 13:38:41 -0600 Subject: [PATCH 08/12] feat(helm): add ability for a dry-run to evaluate lookup functions When a helm command is run with the --dry-run-option=server flag, it will try to connect to the cluster to be able to render lookup functions. Closes #8137 Signed-off-by: Tapas Kapadia --- cmd/helm/install.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 580a194dd..25e52810c 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -265,7 +265,7 @@ 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 + // validate dry-run-option flag value is one of the allowed values if err := validateDryRunOptionFlag(client.DryRunOption); err != nil { return nil, err } @@ -312,7 +312,7 @@ func compInstall(args []string, toComplete string, client *action.Install) ([]st } func validateDryRunOptionFlag(dryRunOptionFlagValue string) error { - // validate dry-run flag value with set of allowed value + // validate dry-run-option flag value with set of allowed value allowedDryRunValues := []string{"false", "true", "none", "client", "server"} isAllowed := false for _, v := range allowedDryRunValues { From 9a0025f96388bcb45f91b2206e0e9d24b498d339 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 23 Jan 2023 14:28:29 -0600 Subject: [PATCH 09/12] 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 #8137 Signed-off-by: Tapas Kapadia --- cmd/helm/install.go | 7 ++----- cmd/helm/upgrade.go | 7 ++----- pkg/action/install.go | 3 ++- 3 files changed, 6 insertions(+), 11 deletions(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 25e52810c..410505ef1 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -154,9 +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.DryRunOption, "dry-run-option", "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-option").NoOptDefVal = "client" + f.StringVar(&client.DryRunOption, "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") @@ -175,8 +174,6 @@ func addInstallFlags(cmd *cobra.Command, f *pflag.FlagSet, client *action.Instal addValueOptionsFlags(f, valueOpts) addChartPathOptionsFlags(f, &client.ChartPathOptions) - cmd.MarkFlagsMutuallyExclusive("dry-run", "dry-run-option") - err := cmd.RegisterFlagCompletionFunc("version", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { requiredArgs := 2 if client.GenerateName { diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 34d8ab24b..355b181e3 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -218,9 +218,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.DryRunOption, "dry-run-option", "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-option").NoOptDefVal = "client" + f.StringVar(&client.DryRunOption, "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") @@ -243,8 +242,6 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { bindOutputFlag(cmd, &outfmt) bindPostRenderFlag(cmd, &client.PostRenderer) - cmd.MarkFlagsMutuallyExclusive("dry-run", "dry-run-option") - err := cmd.RegisterFlagCompletionFunc("version", func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { if len(args) != 2 { return nil, cobra.ShellCompDirectiveNoFileComp diff --git a/pkg/action/install.go b/pkg/action/install.go index 3abe102a4..013cf7e94 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -210,7 +210,8 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma } var interactWithRemote bool - if !i.DryRun || i.DryRunOption == "server" { + // `helm template` is the only command that Install.APIVersions field will not be nil. + if (!i.DryRun || i.DryRunOption == "server") && i.APIVersions == nil { interactWithRemote = true } From 25ac62e153d14b025c1b0460e82e48bf28ac07a6 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Fri, 27 Jan 2023 20:08:44 -0600 Subject: [PATCH 10/12] 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 #8137 Signed-off-by: Tapas Kapadia --- pkg/action/install.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/action/install.go b/pkg/action/install.go index 013cf7e94..f4c0c6695 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -211,7 +211,7 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma var interactWithRemote bool // `helm template` is the only command that Install.APIVersions field will not be nil. - if (!i.DryRun || i.DryRunOption == "server") && i.APIVersions == nil { + if !i.DryRun || i.DryRunOption == "server" { interactWithRemote = true } From f9e54b6079100510d2956df2cbb70aa4b34ef969 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 30 Jan 2023 17:04:10 -0600 Subject: [PATCH 11/12] 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 #8137 Signed-off-by: Tapas Kapadia --- cmd/helm/install.go | 6 +++--- cmd/helm/upgrade.go | 2 +- pkg/action/action.go | 8 +++----- pkg/action/install.go | 3 +-- pkg/action/upgrade.go | 4 ++-- 5 files changed, 10 insertions(+), 13 deletions(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 410505ef1..134bc6d6d 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -262,7 +262,7 @@ func runInstall(args []string, client *action.Install, valueOpts *values.Options client.Namespace = settings.Namespace() - // validate dry-run-option flag value is one of the allowed values + // Validate DryRunOption member is one of the allowed values if err := validateDryRunOptionFlag(client.DryRunOption); err != nil { return nil, err } @@ -309,7 +309,7 @@ func compInstall(args []string, toComplete string, client *action.Install) ([]st } func validateDryRunOptionFlag(dryRunOptionFlagValue string) error { - // validate dry-run-option flag value with set of allowed value + // Validate dry-run flag value with a set of allowed value allowedDryRunValues := []string{"false", "true", "none", "client", "server"} isAllowed := false for _, v := range allowedDryRunValues { @@ -319,7 +319,7 @@ func validateDryRunOptionFlag(dryRunOptionFlagValue string) error { } } if !isAllowed { - return errors.New("Invalid dry-run flag. Flag must one of the following: false, true, none, client, sever") + return errors.New("Invalid dry-run flag. Flag must one of the following: false, true, none, client, server") } return nil } diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 355b181e3..7c4ff473e 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -139,7 +139,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { if err != nil { return err } - // validate dry-run flag value is one of the allowed values + // Validate dry-run flag value is one of the allowed values if err := validateDryRunOptionFlag(client.DryRunOption); err != nil { return err } diff --git a/pkg/action/action.go b/pkg/action/action.go index c19a65842..44c85ecd7 100644 --- a/pkg/action/action.go +++ b/pkg/action/action.go @@ -120,11 +120,9 @@ func (cfg *Configuration) renderResources(ch *chart.Chart, values chartutil.Valu var files map[string]string var err2 error - // A `helm template` should not talk to the remote cluster. However, commands - // with the flag `--dry-run-option` with the value of false, none, or sever - // or with the flag `--dry-run` with the value of false should try to interact with the cluster. - // It may break in interesting and exotic ways because other data (e.g. discovery) - // is mocked. + // A `helm template` should not talk to the remote cluster. However, commands with the flag + //`--dry-run` with the value of `false`, `none`, or `server` should try to interact with the cluster. + // It may break in interesting and exotic ways because other data (e.g. discovery) is mocked. if interactWithRemote && cfg.RESTClientGetter != nil { restConfig, err := cfg.RESTClientGetter.ToRESTConfig() if err != nil { diff --git a/pkg/action/install.go b/pkg/action/install.go index f4c0c6695..c09ae5420 100644 --- a/pkg/action/install.go +++ b/pkg/action/install.go @@ -204,13 +204,12 @@ func (i *Install) RunWithContext(ctx context.Context, chrt *chart.Chart, vals ma return nil, err } - // determine dry run behavior + // Determine dry run behavior if i.DryRun || i.DryRunOption == "client" || i.DryRunOption == "server" || i.DryRunOption == "true" { i.DryRun = true } var interactWithRemote bool - // `helm template` is the only command that Install.APIVersions field will not be nil. if !i.DryRun || i.DryRunOption == "server" { interactWithRemote = true } diff --git a/pkg/action/upgrade.go b/pkg/action/upgrade.go index 817486465..2d372f017 100644 --- a/pkg/action/upgrade.go +++ b/pkg/action/upgrade.go @@ -141,7 +141,7 @@ func (u *Upgrade) RunWithContext(ctx context.Context, name string, chart *chart. return nil, errors.Errorf("release name is invalid: %s", name) } - // determine dry run behavior + // Determine dry run behavior if u.DryRun || u.DryRunOption == "client" || u.DryRunOption == "server" || u.DryRunOption == "true" { u.DryRun = true } @@ -239,7 +239,7 @@ func (u *Upgrade) prepareUpgrade(name string, chart *chart.Chart, vals map[strin return nil, nil, err } - // determine whether or not to interact with remote + // Determine whether or not to interact with remote var interactWithRemote bool if !u.DryRun || u.DryRunOption == "server" { interactWithRemote = true From b7a2d47eca70e18b821b415efa5f47971c8a5302 Mon Sep 17 00:00:00 2001 From: Tapas Kapadia Date: Mon, 1 May 2023 00:04:04 -0500 Subject: [PATCH 12/12] 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 to be able to render lookup functions. Closes #8137 Signed-off-by: Tapas Kapadia --- cmd/helm/install.go | 4 ++-- cmd/helm/template.go | 2 +- cmd/helm/upgrade.go | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 1ab659449..c34f82206 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -142,7 +142,7 @@ func newInstallCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } client.SetRegistryClient(registryClient) - if client.DryRunOption == "unchanged" { + if client.DryRunOption == "" { client.DryRunOption = "none" } rel, err := runInstall(args, client, valueOpts, out) @@ -163,7 +163,7 @@ 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.StringVar(&client.DryRunOption, "dry-run", "unchanged", "simulate an install. If --dry-run is set with no option being specified or as '--dry-run=client', it will not attempt cluster connections. Setting '--dry-run=server' allows attempting cluster connections.") + f.StringVar(&client.DryRunOption, "dry-run", "", "simulate an install. If --dry-run is set with no option being specified or as '--dry-run=client', it will not attempt cluster connections. Setting '--dry-run=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") diff --git a/cmd/helm/template.go b/cmd/helm/template.go index 0aa7e138d..2cf89131b 100644 --- a/cmd/helm/template.go +++ b/cmd/helm/template.go @@ -79,7 +79,7 @@ func newTemplateCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } client.SetRegistryClient(registryClient) - if client.DryRunOption == "unchanged" { + if client.DryRunOption == "" { client.DryRunOption = "true" } client.DryRun = true diff --git a/cmd/helm/upgrade.go b/cmd/helm/upgrade.go index 658b26b11..5e23f8347 100644 --- a/cmd/helm/upgrade.go +++ b/cmd/helm/upgrade.go @@ -96,7 +96,7 @@ func newUpgradeCmd(cfg *action.Configuration, out io.Writer) *cobra.Command { } client.SetRegistryClient(registryClient) - if client.DryRunOption == "unchanged" { + if client.DryRunOption == "" { client.DryRunOption = "none" } // Fixes #7002 - Support reading values from STDIN for `upgrade` command @@ -229,7 +229,7 @@ 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.StringVar(&client.DryRunOption, "dry-run", "unchanged", "simulate an install. If --dry-run is set with no option being specified or as '--dry-run=client', it will not attempt cluster connections. Setting '--dry-run=server' allows attempting cluster connections.") + f.StringVar(&client.DryRunOption, "dry-run", "", "simulate an install. If --dry-run is set with no option being specified or as '--dry-run=client', it will not attempt cluster connections. Setting '--dry-run=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")