From a29e610938b1a4f59ea2c038d0d23193d54bb2f9 Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Thu, 13 Jul 2017 16:36:39 -0700 Subject: [PATCH 1/2] fix(helm): fix flag parsing once and for all --- cmd/helm/create_test.go | 4 +- cmd/helm/fetch_test.go | 4 +- cmd/helm/helm.go | 61 +++-------- cmd/helm/helm_test.go | 12 +- cmd/helm/init.go | 2 +- cmd/helm/install.go | 2 +- cmd/helm/load_plugins.go | 16 +-- cmd/helm/plugin_test.go | 5 +- cmd/helm/reset.go | 2 +- pkg/helm/environment/environment.go | 61 ++++++++--- pkg/helm/environment/environment_test.go | 134 +++++++++++++++++++++++ pkg/plugin/plugin.go | 9 +- pkg/tiller/environment/environment.go | 4 - 13 files changed, 219 insertions(+), 97 deletions(-) create mode 100644 pkg/helm/environment/environment_test.go diff --git a/cmd/helm/create_test.go b/cmd/helm/create_test.go index d45843eda..34a3b25e5 100644 --- a/cmd/helm/create_test.go +++ b/cmd/helm/create_test.go @@ -23,8 +23,6 @@ import ( "testing" "k8s.io/helm/pkg/chartutil" - "k8s.io/helm/pkg/helm/environment" - "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/proto/hapi/chart" ) @@ -87,7 +85,7 @@ func TestCreateStarterCmd(t *testing.T) { if err != nil { t.Fatal(err) } - old := helmpath.Home(environment.DefaultHelmHome) + old := settings.Home settings.Home = thome defer func() { settings.Home = old diff --git a/cmd/helm/fetch_test.go b/cmd/helm/fetch_test.go index 4d81ed38e..b039c9ff4 100644 --- a/cmd/helm/fetch_test.go +++ b/cmd/helm/fetch_test.go @@ -24,8 +24,6 @@ import ( "regexp" "testing" - "k8s.io/helm/pkg/helm/environment" - "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/repo/repotest" ) @@ -34,7 +32,7 @@ func TestFetchCmd(t *testing.T) { if err != nil { t.Fatal(err) } - old := helmpath.Home(environment.DefaultHelmHome) + old := settings.Home settings.Home = hh defer func() { settings.Home = old diff --git a/cmd/helm/helm.go b/cmd/helm/helm.go index 3e244e0cd..bc8885b4b 100644 --- a/cmd/helm/helm.go +++ b/cmd/helm/helm.go @@ -35,7 +35,6 @@ import ( helm_env "k8s.io/helm/pkg/helm/environment" "k8s.io/helm/pkg/helm/portforwarder" "k8s.io/helm/pkg/kube" - tiller_env "k8s.io/helm/pkg/tiller/environment" "k8s.io/helm/pkg/tlsutil" ) @@ -46,7 +45,6 @@ var ( tlsVerify bool // enable TLS and verify remote certificates tlsEnable bool // enable TLS - kubeContext string tillerTunnel *kube.Tunnel settings helm_env.EnvSettings ) @@ -75,57 +73,25 @@ Environment: $KUBECONFIG set an alternative Kubernetes configuration file (default "~/.kube/config") ` -func setFlagFromEnv(name, envar string, cmd *cobra.Command) { - if cmd.Flags().Changed(name) { - return - } - if v, ok := os.LookupEnv(envar); ok { - cmd.Flags().Set(name, v) - } -} - -func setFlagsFromEnv(flags map[string]string, cmd *cobra.Command) { - for name, envar := range flags { - setFlagFromEnv(name, envar, cmd) - } -} - -func addRootFlags(cmd *cobra.Command) { - pf := cmd.PersistentFlags() - pf.StringVar((*string)(&settings.Home), "home", helm_env.DefaultHelmHome, "location of your Helm config. Overrides $HELM_HOME") - pf.StringVar(&settings.TillerHost, "host", "", "address of Tiller. Overrides $HELM_HOST") - pf.StringVar(&kubeContext, "kube-context", "", "name of the kubeconfig context to use") - pf.BoolVar(&settings.Debug, "debug", false, "enable verbose output") - pf.StringVar(&settings.TillerNamespace, "tiller-namespace", tiller_env.DefaultTillerNamespace, "namespace of Tiller") -} - -func initRootFlags(cmd *cobra.Command) { - setFlagsFromEnv(map[string]string{ - "debug": helm_env.DebugEnvVar, - "home": helm_env.HomeEnvVar, - "host": helm_env.HostEnvVar, - "tiller-namespace": tiller_env.TillerNamespaceEnvVar, - }, cmd.Root()) - - tlsCaCertFile = os.ExpandEnv(tlsCaCertFile) - tlsCertFile = os.ExpandEnv(tlsCertFile) - tlsKeyFile = os.ExpandEnv(tlsKeyFile) -} - -func newRootCmd() *cobra.Command { +func newRootCmd(args []string) *cobra.Command { cmd := &cobra.Command{ Use: "helm", Short: "The Helm package manager for Kubernetes.", Long: globalUsage, SilenceUsage: true, - PersistentPreRun: func(cmd *cobra.Command, _ []string) { - initRootFlags(cmd) + PersistentPreRun: func(*cobra.Command, []string) { + tlsCaCertFile = os.ExpandEnv(tlsCaCertFile) + tlsCertFile = os.ExpandEnv(tlsCertFile) + tlsKeyFile = os.ExpandEnv(tlsKeyFile) }, PersistentPostRun: func(*cobra.Command, []string) { teardown() }, } - addRootFlags(cmd) + flags := cmd.PersistentFlags() + + settings.AddFlags(flags) + out := cmd.OutOrStdout() cmd.AddCommand( @@ -167,6 +133,11 @@ func newRootCmd() *cobra.Command { markDeprecated(newRepoUpdateCmd(out), "use 'helm repo update'\n"), ) + flags.Parse(args) + + // set defaults from environment + settings.Init(flags) + // Find and add plugins loadPlugins(cmd, out) @@ -179,7 +150,7 @@ func init() { } func main() { - cmd := newRootCmd() + cmd := newRootCmd(os.Args[1:]) if err := cmd.Execute(); err != nil { os.Exit(1) } @@ -192,7 +163,7 @@ func markDeprecated(cmd *cobra.Command, notice string) *cobra.Command { func setupConnection(c *cobra.Command, args []string) error { if settings.TillerHost == "" { - config, client, err := getKubeClient(kubeContext) + config, client, err := getKubeClient(settings.KubeContext) if err != nil { return err } diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index 8e25d3089..b5808229d 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -25,6 +25,7 @@ import ( "os" "path/filepath" "regexp" + "strings" "testing" "github.com/golang/protobuf/ptypes/timestamp" @@ -232,8 +233,13 @@ func ensureTestHome(home helmpath.Home, t *testing.T) error { } func TestRootCmd(t *testing.T) { - oldhome := os.Getenv("HELM_HOME") - defer os.Setenv("HELM_HOME", oldhome) + // reset env + defer func(origEnv []string) { + for _, pair := range origEnv { + kv := strings.SplitN(pair, "=", 2) + os.Setenv(kv[0], kv[1]) + } + }(os.Environ()) tests := []struct { name string @@ -287,7 +293,7 @@ func TestRootCmd(t *testing.T) { os.Setenv(k, v) } - cmd := newRootCmd() + cmd := newRootCmd(tt.args) cmd.SetOutput(ioutil.Discard) cmd.SetArgs(tt.args) cmd.Run = func(*cobra.Command, []string) {} diff --git a/cmd/helm/init.go b/cmd/helm/init.go index 868a5bf0d..b67dc3a23 100644 --- a/cmd/helm/init.go +++ b/cmd/helm/init.go @@ -230,7 +230,7 @@ func (i *initCmd) run() error { if !i.clientOnly { if i.kubeClient == nil { - _, c, err := getKubeClient(kubeContext) + _, c, err := getKubeClient(settings.KubeContext) if err != nil { return fmt.Errorf("could not get kubernetes client: %s", err) } diff --git a/cmd/helm/install.go b/cmd/helm/install.go index 988e7f0a1..b480b0b93 100644 --- a/cmd/helm/install.go +++ b/cmd/helm/install.go @@ -434,7 +434,7 @@ func generateName(nameTemplate string) (string, error) { } func defaultNamespace() string { - if ns, _, err := kube.GetConfig(kubeContext).Namespace(); err == nil { + if ns, _, err := kube.GetConfig(settings.KubeContext).Namespace(); err == nil { return ns } return "default" diff --git a/cmd/helm/load_plugins.go b/cmd/helm/load_plugins.go index ba056bd8c..2a02e8196 100644 --- a/cmd/helm/load_plugins.go +++ b/cmd/helm/load_plugins.go @@ -24,9 +24,7 @@ import ( "strings" "github.com/spf13/cobra" - "github.com/spf13/pflag" - helm_env "k8s.io/helm/pkg/helm/environment" "k8s.io/helm/pkg/plugin" ) @@ -38,20 +36,11 @@ import ( func loadPlugins(baseCmd *cobra.Command, out io.Writer) { // If HELM_NO_PLUGINS is set to 1, do not load plugins. - if os.Getenv(helm_env.PluginDisableEnvVar) == "1" { + if os.Getenv("HELM_NO_PLUGINS") == "1" { return } - // manually handel processing of HELM_HOME and --home - helmHome := "$HOME/.helm" - if h, ok := os.LookupEnv("HELM_HOME"); ok { - helmHome = h - } - - fs := pflag.NewFlagSet("homer", pflag.ContinueOnError) - fs.StringVar((*string)(&settings.Home), "home", helmHome, "location of your Helm config. Overrides $HELM_HOME") - fs.Parse(os.Args) - + // debug("HELM_PLUGIN_DIRS=%s", settings.PluginDirs()) found, err := findPlugins(settings.PluginDirs()) if err != nil { fmt.Fprintf(os.Stderr, "failed to load plugins: %s", err) @@ -63,7 +52,6 @@ func loadPlugins(baseCmd *cobra.Command, out io.Writer) { if err := cmd.Parent().ParseFlags(k); err != nil { return nil, err } - initRootFlags(cmd) return u, nil } diff --git a/cmd/helm/plugin_test.go b/cmd/helm/plugin_test.go index 335cd281e..5c4d6be61 100644 --- a/cmd/helm/plugin_test.go +++ b/cmd/helm/plugin_test.go @@ -23,7 +23,6 @@ import ( "strings" "testing" - helm_env "k8s.io/helm/pkg/helm/environment" "k8s.io/helm/pkg/helm/helmpath" "k8s.io/helm/pkg/plugin" @@ -152,10 +151,10 @@ func TestLoadPlugins_HelmNoPlugins(t *testing.T) { // Set helm home to point to testdata old := settings.Home settings.Home = "testdata/helmhome" - os.Setenv(helm_env.PluginDisableEnvVar, "1") + cleanup := resetEnv("HELM_NO_PLUGINS", "1") defer func() { settings.Home = old - os.Unsetenv(helm_env.PluginDisableEnvVar) + cleanup() }() out := bytes.NewBuffer(nil) diff --git a/cmd/helm/reset.go b/cmd/helm/reset.go index f73632ebf..cc0415061 100644 --- a/cmd/helm/reset.go +++ b/cmd/helm/reset.go @@ -86,7 +86,7 @@ func newResetCmd(client helm.Interface, out io.Writer) *cobra.Command { // runReset uninstalls tiller from Kubernetes Cluster and deletes local config func (d *resetCmd) run() error { if d.kubeClient == nil { - _, c, err := getInternalKubeClient(kubeContext) + _, c, err := getInternalKubeClient(settings.KubeContext) if err != nil { return fmt.Errorf("could not get kubernetes client: %s", err) } diff --git a/pkg/helm/environment/environment.go b/pkg/helm/environment/environment.go index 2092450e1..3ddd90449 100644 --- a/pkg/helm/environment/environment.go +++ b/pkg/helm/environment/environment.go @@ -26,20 +26,9 @@ import ( "os" "path/filepath" - "k8s.io/helm/pkg/helm/helmpath" -) + "github.com/spf13/pflag" -const ( - // HomeEnvVar is the HELM_HOME environment variable key. - HomeEnvVar = "HELM_HOME" - // PluginEnvVar is the HELM_PLUGIN environment variable key. - PluginEnvVar = "HELM_PLUGIN" - // PluginDisableEnvVar is the HELM_NO_PLUGINS environment variable key. - PluginDisableEnvVar = "HELM_NO_PLUGINS" - // HostEnvVar is the HELM_HOST environment variable key. - HostEnvVar = "HELM_HOST" - // DebugEnvVar is the HELM_DEBUG environment variable key. - DebugEnvVar = "HELM_DEBUG" + "k8s.io/helm/pkg/helm/helmpath" ) // DefaultHelmHome is the default HELM_HOME. @@ -55,12 +44,56 @@ type EnvSettings struct { Home helmpath.Home // Debug indicates whether or not Helm is running in Debug mode. Debug bool + // KubeContext is the name of the kubeconfig context. + KubeContext string +} + +// AddFlags binds flags to the given flagset. +func (s *EnvSettings) AddFlags(fs *pflag.FlagSet) { + fs.StringVar((*string)(&s.Home), "home", DefaultHelmHome, "location of your Helm config. Overrides $HELM_HOME") + fs.StringVar(&s.TillerHost, "host", "", "address of Tiller. Overrides $HELM_HOST") + fs.StringVar(&s.KubeContext, "kube-context", "", "name of the kubeconfig context to use") + fs.BoolVar(&s.Debug, "debug", false, "enable verbose output") + fs.StringVar(&s.TillerNamespace, "tiller-namespace", "kube-system", "namespace of Tiller") +} + +// Init sets values from the environment. +func (s *EnvSettings) Init(fs *pflag.FlagSet) { + for name, envar := range envMap { + setFlagFromEnv(name, envar, fs) + } } // PluginDirs is the path to the plugin directories. func (s EnvSettings) PluginDirs() string { - if d := os.Getenv(PluginEnvVar); d != "" { + if d, ok := os.LookupEnv("HELM_PLUGIN"); ok { return d } return s.Home.Plugins() } + +// envMap maps flag names to envvars +var envMap = map[string]string{ + "debug": "HELM_DEBUG", + "home": "HELM_HOME", + "host": "HELM_HOST", + "tiller-namespace": "TILLER_NAMESPACE", +} + +func setFlagFromEnv(name, envar string, fs *pflag.FlagSet) { + if fs.Changed(name) { + return + } + if v, ok := os.LookupEnv(envar); ok { + fs.Set(name, v) + } +} + +// Deprecated +const ( + HomeEnvVar = "HELM_HOME" + PluginEnvVar = "HELM_PLUGIN" + PluginDisableEnvVar = "HELM_NO_PLUGINS" + HostEnvVar = "HELM_HOST" + DebugEnvVar = "HELM_DEBUG" +) diff --git a/pkg/helm/environment/environment_test.go b/pkg/helm/environment/environment_test.go new file mode 100644 index 000000000..8f0caa388 --- /dev/null +++ b/pkg/helm/environment/environment_test.go @@ -0,0 +1,134 @@ +/* +Copyright 2016 The Kubernetes Authors All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + +http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package environment + +import ( + "os" + "strings" + "testing" + + "k8s.io/helm/pkg/helm/helmpath" + + "github.com/spf13/pflag" +) + +func TestEnvSettings(t *testing.T) { + tests := []struct { + name string + + // input + args []string + envars map[string]string + + // expected values + home, host, ns, kcontext, plugins string + debug bool + }{ + { + name: "defaults", + args: []string{}, + home: DefaultHelmHome, + plugins: helmpath.Home(DefaultHelmHome).Plugins(), + ns: "kube-system", + }, + { + name: "with flags set", + args: []string{"--home", "/foo", "--host=here", "--debug", "--tiller-namespace=myns"}, + home: "/foo", + plugins: helmpath.Home("/foo").Plugins(), + host: "here", + ns: "myns", + debug: true, + }, + { + name: "with envvars set", + args: []string{}, + envars: map[string]string{"HELM_HOME": "/bar", "HELM_HOST": "there", "HELM_DEBUG": "1", "TILLER_NAMESPACE": "yourns"}, + home: "/bar", + plugins: helmpath.Home("/bar").Plugins(), + host: "there", + ns: "yourns", + debug: true, + }, + { + name: "with flags and envvars set", + args: []string{"--home", "/foo", "--host=here", "--debug", "--tiller-namespace=myns"}, + envars: map[string]string{"HELM_HOME": "/bar", "HELM_HOST": "there", "HELM_DEBUG": "1", "TILLER_NAMESPACE": "yourns", "HELM_PLUGIN": "glade"}, + home: "/foo", + plugins: "glade", + host: "here", + ns: "myns", + debug: true, + }, + } + + cleanup := resetEnv() + defer cleanup() + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + for k, v := range tt.envars { + os.Setenv(k, v) + } + + flags := pflag.NewFlagSet("testing", pflag.ContinueOnError) + + settings := &EnvSettings{} + settings.AddFlags(flags) + flags.Parse(tt.args) + + settings.Init(flags) + + if settings.Home != helmpath.Home(tt.home) { + t.Errorf("expected home %q, got %q", tt.home, settings.Home) + } + if settings.PluginDirs() != tt.plugins { + t.Errorf("expected plugins %q, got %q", tt.plugins, settings.PluginDirs()) + } + if settings.TillerHost != tt.host { + t.Errorf("expected host %q, got %q", tt.host, settings.TillerHost) + } + if settings.Debug != tt.debug { + t.Errorf("expected debug %t, got %t", tt.debug, settings.Debug) + } + if settings.TillerNamespace != tt.ns { + t.Errorf("expected tiller-namespace %q, got %q", tt.ns, settings.TillerNamespace) + } + if settings.KubeContext != tt.kcontext { + t.Errorf("expected kube-context %q, got %q", tt.kcontext, settings.KubeContext) + } + + cleanup() + }) + } +} + +func resetEnv() func() { + origEnv := os.Environ() + + // ensure any local envvars do not hose us + for _, e := range envMap { + os.Unsetenv(e) + } + + return func() { + for _, pair := range origEnv { + kv := strings.SplitN(pair, "=", 2) + os.Setenv(kv[0], kv[1]) + } + } +} diff --git a/pkg/plugin/plugin.go b/pkg/plugin/plugin.go index 75d808701..b3458c2d8 100644 --- a/pkg/plugin/plugin.go +++ b/pkg/plugin/plugin.go @@ -22,7 +22,6 @@ import ( "strings" helm_env "k8s.io/helm/pkg/helm/environment" - tiller_env "k8s.io/helm/pkg/tiller/environment" "github.com/ghodss/yaml" ) @@ -179,8 +178,8 @@ func SetupPluginEnv(settings helm_env.EnvSettings, // Set vars that may not have been set, and save client the // trouble of re-parsing. - helm_env.PluginEnvVar: settings.PluginDirs(), - helm_env.HomeEnvVar: settings.Home.String(), + "HELM_PLUGIN": settings.PluginDirs(), + "HELM_HOME": settings.Home.String(), // Set vars that convey common information. "HELM_PATH_REPOSITORY": settings.Home.Repository(), @@ -189,8 +188,8 @@ func SetupPluginEnv(settings helm_env.EnvSettings, "HELM_PATH_LOCAL_REPOSITORY": settings.Home.LocalRepository(), "HELM_PATH_STARTER": settings.Home.Starters(), - "TILLER_HOST": settings.TillerHost, - tiller_env.TillerNamespaceEnvVar: settings.TillerNamespace, + "TILLER_HOST": settings.TillerHost, + "TILLER_NAMESPACE": settings.TillerNamespace, } { os.Setenv(key, val) } diff --git a/pkg/tiller/environment/environment.go b/pkg/tiller/environment/environment.go index 64bbcffc8..b4c6b2b6f 100644 --- a/pkg/tiller/environment/environment.go +++ b/pkg/tiller/environment/environment.go @@ -37,10 +37,6 @@ import ( "k8s.io/helm/pkg/storage/driver" ) -// TillerNamespaceEnvVar is the environment variable name for the Tiller -// namespace in the kubernetes cluster. -const TillerNamespaceEnvVar = "TILLER_NAMESPACE" - // DefaultTillerNamespace is the default namespace for Tiller. const DefaultTillerNamespace = "kube-system" From 7112a48af635ea8085a86857075b876c6aa2655c Mon Sep 17 00:00:00 2001 From: Adam Reese Date: Fri, 14 Jul 2017 00:01:37 -0700 Subject: [PATCH 2/2] ref(helm): refactor cleanup of environment after tests run --- cmd/helm/create_test.go | 7 +++--- cmd/helm/dependency_build_test.go | 7 +++--- cmd/helm/dependency_update_test.go | 16 +++++++------- cmd/helm/fetch_test.go | 7 +++--- cmd/helm/helm_test.go | 21 ++++++++++++------ cmd/helm/init_test.go | 7 +++--- cmd/helm/package_test.go | 7 +++--- cmd/helm/plugin_test.go | 34 +++++++++--------------------- cmd/helm/repo_add_test.go | 14 ++++++------ cmd/helm/repo_remove_test.go | 7 +++--- cmd/helm/repo_update_test.go | 15 +++++++------ cmd/helm/search_test.go | 5 +++-- 12 files changed, 76 insertions(+), 71 deletions(-) diff --git a/cmd/helm/create_test.go b/cmd/helm/create_test.go index 34a3b25e5..afaed6483 100644 --- a/cmd/helm/create_test.go +++ b/cmd/helm/create_test.go @@ -85,13 +85,14 @@ func TestCreateStarterCmd(t *testing.T) { if err != nil { t.Fatal(err) } - old := settings.Home - settings.Home = thome + cleanup := resetEnv() defer func() { - settings.Home = old os.RemoveAll(thome.String()) + cleanup() }() + settings.Home = thome + // Create a starter. starterchart := filepath.Join(thome.String(), "starters") os.Mkdir(starterchart, 0755) diff --git a/cmd/helm/dependency_build_test.go b/cmd/helm/dependency_build_test.go index 0ea073fd0..2d7c88654 100644 --- a/cmd/helm/dependency_build_test.go +++ b/cmd/helm/dependency_build_test.go @@ -29,17 +29,18 @@ import ( ) func TestDependencyBuildCmd(t *testing.T) { - oldhome := settings.Home hh, err := tempHelmHome(t) if err != nil { t.Fatal(err) } - settings.Home = hh + cleanup := resetEnv() defer func() { os.RemoveAll(hh.String()) - settings.Home = oldhome + cleanup() }() + settings.Home = hh + srv := repotest.NewServer(hh.String()) defer srv.Stop() _, err = srv.CopyCharts("testdata/testcharts/*.tgz") diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index 6e7f2fd3c..227959c53 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -34,18 +34,18 @@ import ( ) func TestDependencyUpdateCmd(t *testing.T) { - // Set up a testing helm home - oldhome := settings.Home hh, err := tempHelmHome(t) if err != nil { t.Fatal(err) } - settings.Home = hh + cleanup := resetEnv() defer func() { os.RemoveAll(hh.String()) - settings.Home = oldhome + cleanup() }() + settings.Home = hh + srv := repotest.NewServer(hh.String()) defer srv.Stop() copied, err := srv.CopyCharts("testdata/testcharts/*.tgz") @@ -129,18 +129,18 @@ func TestDependencyUpdateCmd(t *testing.T) { } func TestDependencyUpdateCmd_SkipRefresh(t *testing.T) { - // Set up a testing helm home - oldhome := settings.Home hh, err := tempHelmHome(t) if err != nil { t.Fatal(err) } - settings.Home = hh + cleanup := resetEnv() defer func() { os.RemoveAll(hh.String()) - settings.Home = oldhome + cleanup() }() + settings.Home = hh + srv := repotest.NewServer(hh.String()) defer srv.Stop() copied, err := srv.CopyCharts("testdata/testcharts/*.tgz") diff --git a/cmd/helm/fetch_test.go b/cmd/helm/fetch_test.go index b039c9ff4..13247ee99 100644 --- a/cmd/helm/fetch_test.go +++ b/cmd/helm/fetch_test.go @@ -32,15 +32,16 @@ func TestFetchCmd(t *testing.T) { if err != nil { t.Fatal(err) } - old := settings.Home - settings.Home = hh + cleanup := resetEnv() defer func() { - settings.Home = old os.RemoveAll(hh.String()) + cleanup() }() srv := repotest.NewServer(hh.String()) defer srv.Stop() + settings.Home = hh + // all flags will get "--home=TMDIR -d outdir" appended. tests := []struct { name string diff --git a/cmd/helm/helm_test.go b/cmd/helm/helm_test.go index b5808229d..2a2af1359 100644 --- a/cmd/helm/helm_test.go +++ b/cmd/helm/helm_test.go @@ -233,13 +233,8 @@ func ensureTestHome(home helmpath.Home, t *testing.T) error { } func TestRootCmd(t *testing.T) { - // reset env - defer func(origEnv []string) { - for _, pair := range origEnv { - kv := strings.SplitN(pair, "=", 2) - os.Setenv(kv[0], kv[1]) - } - }(os.Environ()) + cleanup := resetEnv() + defer cleanup() tests := []struct { name string @@ -312,3 +307,15 @@ func TestRootCmd(t *testing.T) { }) } } + +func resetEnv() func() { + origSettings := settings + origEnv := os.Environ() + return func() { + settings = origSettings + for _, pair := range origEnv { + kv := strings.SplitN(pair, "=", 2) + os.Setenv(kv[0], kv[1]) + } + } +} diff --git a/cmd/helm/init_test.go b/cmd/helm/init_test.go index 55b62d284..6547e2342 100644 --- a/cmd/helm/init_test.go +++ b/cmd/helm/init_test.go @@ -140,13 +140,14 @@ func TestInitCmd_dryRun(t *testing.T) { if err != nil { t.Fatal(err) } - dbg := settings.Debug - settings.Debug = true + cleanup := resetEnv() defer func() { os.Remove(home) - settings.Debug = dbg + cleanup() }() + settings.Debug = true + var buf bytes.Buffer fc := fake.NewSimpleClientset() cmd := &initCmd{ diff --git a/cmd/helm/package_test.go b/cmd/helm/package_test.go index 2a0f6d9f5..eb6e02830 100644 --- a/cmd/helm/package_test.go +++ b/cmd/helm/package_test.go @@ -143,14 +143,15 @@ func TestPackage(t *testing.T) { } ensureTestHome(helmpath.Home(tmp), t) - oldhome := settings.Home - settings.Home = helmpath.Home(tmp) + cleanup := resetEnv() defer func() { - settings.Home = oldhome os.Chdir(origDir) os.RemoveAll(tmp) + cleanup() }() + settings.Home = helmpath.Home(tmp) + for _, tt := range tests { buf := bytes.NewBuffer(nil) c := newPackageCmd(buf) diff --git a/cmd/helm/plugin_test.go b/cmd/helm/plugin_test.go index 5c4d6be61..2a4a0e9aa 100644 --- a/cmd/helm/plugin_test.go +++ b/cmd/helm/plugin_test.go @@ -63,25 +63,13 @@ func TestManuallyProcessArgs(t *testing.T) { } -// resetEnv sets an env var, and returns a defer function to reset the env -func resetEnv(name, val string) func() { - original, ok := os.LookupEnv(name) - os.Setenv(name, val) - if ok { - return func() { os.Setenv(name, original) } - } - return func() { os.Unsetenv(name) } -} - func TestLoadPlugins(t *testing.T) { - // Set helm home to point to testdata - old := settings.Home + cleanup := resetEnv() + defer cleanup() + settings.Home = "testdata/helmhome" - cleanup := resetEnv("HELM_HOME", settings.Home.String()) - defer func() { - settings.Home = old - cleanup() - }() + + os.Setenv("HELM_HOME", settings.Home.String()) hh := settings.Home out := bytes.NewBuffer(nil) @@ -148,14 +136,12 @@ func TestLoadPlugins(t *testing.T) { } func TestLoadPlugins_HelmNoPlugins(t *testing.T) { - // Set helm home to point to testdata - old := settings.Home + cleanup := resetEnv() + defer cleanup() + settings.Home = "testdata/helmhome" - cleanup := resetEnv("HELM_NO_PLUGINS", "1") - defer func() { - settings.Home = old - cleanup() - }() + + os.Setenv("HELM_NO_PLUGINS", "1") out := bytes.NewBuffer(nil) cmd := &cobra.Command{} diff --git a/cmd/helm/repo_add_test.go b/cmd/helm/repo_add_test.go index 005826dea..2e36decbf 100644 --- a/cmd/helm/repo_add_test.go +++ b/cmd/helm/repo_add_test.go @@ -33,17 +33,18 @@ func TestRepoAddCmd(t *testing.T) { t.Fatal(err) } - oldhome := settings.Home - settings.Home = thome + cleanup := resetEnv() defer func() { srv.Stop() - settings.Home = oldhome os.Remove(thome.String()) + cleanup() }() if err := ensureTestHome(thome, t); err != nil { t.Fatal(err) } + settings.Home = thome + tests := []releaseCase{ { name: "add a repository", @@ -67,18 +68,19 @@ func TestRepoAdd(t *testing.T) { t.Fatal(err) } - oldhome := settings.Home - settings.Home = thome + cleanup := resetEnv() hh := thome defer func() { ts.Stop() - settings.Home = oldhome os.Remove(thome.String()) + cleanup() }() if err := ensureTestHome(hh, t); err != nil { t.Fatal(err) } + settings.Home = thome + if err := addRepository(testName, ts.URL(), hh, "", "", "", true); err != nil { t.Error(err) } diff --git a/cmd/helm/repo_remove_test.go b/cmd/helm/repo_remove_test.go index a0c5a1f5a..84244ae98 100644 --- a/cmd/helm/repo_remove_test.go +++ b/cmd/helm/repo_remove_test.go @@ -33,18 +33,19 @@ func TestRepoRemove(t *testing.T) { t.Fatal(err) } - oldhome := settings.Home - settings.Home = thome hh := helmpath.Home(thome) + cleanup := resetEnv() defer func() { ts.Stop() - settings.Home = oldhome os.Remove(thome.String()) + cleanup() }() if err := ensureTestHome(hh, t); err != nil { t.Fatal(err) } + settings.Home = thome + b := bytes.NewBuffer(nil) if err := removeRepoLine(b, testName, hh); err == nil { diff --git a/cmd/helm/repo_update_test.go b/cmd/helm/repo_update_test.go index 17eaed60b..0d0a31350 100644 --- a/cmd/helm/repo_update_test.go +++ b/cmd/helm/repo_update_test.go @@ -34,13 +34,15 @@ func TestUpdateCmd(t *testing.T) { if err != nil { t.Fatal(err) } - oldhome := settings.Home - settings.Home = thome + + cleanup := resetEnv() defer func() { - settings.Home = oldhome os.Remove(thome.String()) + cleanup() }() + settings.Home = thome + out := bytes.NewBuffer(nil) // Instead of using the HTTP updater, we provide our own for this test. // The TestUpdateCharts test verifies the HTTP behavior independently. @@ -69,18 +71,19 @@ func TestUpdateCharts(t *testing.T) { t.Fatal(err) } - oldhome := settings.Home - settings.Home = thome hh := helmpath.Home(thome) + cleanup := resetEnv() defer func() { ts.Stop() - settings.Home = oldhome os.Remove(thome.String()) + cleanup() }() if err := ensureTestHome(hh, t); err != nil { t.Fatal(err) } + settings.Home = thome + r, err := repo.NewChartRepository(&repo.Entry{ Name: "charts", URL: ts.URL(), diff --git a/cmd/helm/search_test.go b/cmd/helm/search_test.go index b878ef4c4..26878fd0f 100644 --- a/cmd/helm/search_test.go +++ b/cmd/helm/search_test.go @@ -68,9 +68,10 @@ func TestSearchCmd(t *testing.T) { }, } - oldhome := settings.Home + cleanup := resetEnv() + defer cleanup() + settings.Home = "testdata/helmhome" - defer func() { settings.Home = oldhome }() for _, tt := range tests { buf := bytes.NewBuffer(nil)