From e2f53f3b080fe159f0f9b14cbcbcfbe0f1e81008 Mon Sep 17 00:00:00 2001
From: Erin Call <erin@liffft.com>
Date: Mon, 23 Dec 2019 15:31:40 -0800
Subject: [PATCH] Process non-prefixed forms of all config settings [#9]

Trying to guess in advance which part of the config a user will put in
the `settings` section and which they'll put in `environment` is a
fool's errand. Just let everything go in either place.

The ServiceAccount field only had an `envconfig` tag (as opposed to
`split_words`) because that triggered envconfig to look for the non-
prefixed form. Now that we're finding non-prefixed forms of everything,
we can use the clearer/more concise tag.

Note that TestPopulateWithConflictingVariables isn't meant to say
"here's what behavior we *want*" so much as "here's what the behavior
*is*." I don't think one thing is any better than the other, but we
should know which one we're getting.
---
 internal/helm/config.go      | 14 +++++++++++---
 internal/helm/config_test.go | 31 +++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 3 deletions(-)

diff --git a/internal/helm/config.go b/internal/helm/config.go
index afdc978..bb0f06e 100644
--- a/internal/helm/config.go
+++ b/internal/helm/config.go
@@ -25,7 +25,7 @@ type Config struct {
 	SkipTLSVerify      bool     `envconfig:"SKIP_TLS_VERIFY"`        // Put insecure-skip-tls-verify in .kube/config
 	Certificate        string   `envconfig:"KUBERNETES_CERTIFICATE"` // The Kubernetes cluster CA's self-signed certificate (must be base64-encoded)
 	APIServer          string   `envconfig:"API_SERVER"`             // The Kubernetes cluster's API endpoint
-	ServiceAccount     string   `envconfig:"SERVICE_ACCOUNT"`        // Account to use for connecting to the Kubernetes cluster
+	ServiceAccount     string   `split_words:"true"`                 // Account to use for connecting to the Kubernetes cluster
 	ChartVersion       string   `split_words:"true"`                 // Specific chart version to use in `helm upgrade`
 	DryRun             bool     `split_words:"true"`                 // Pass --dry-run to applicable helm commands
 	Wait               bool     ``                                   // Pass --wait to applicable helm commands
@@ -36,7 +36,15 @@ type Config struct {
 	Force              bool     ``                                   // Pass --force to applicable helm commands
 }
 
-// Populate reads environment variables into the Config.
+// Populate reads environment variables into the Config, accounting for several possible formats.
 func (cfg *Config) Populate() error {
-	return envconfig.Process("plugin", cfg)
+	if err := envconfig.Process("plugin", cfg); err != nil {
+		return err
+	}
+
+	if err := envconfig.Process("", cfg); err != nil {
+		return err
+	}
+
+	return nil
 }
diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go
index a594c09..7427e1a 100644
--- a/internal/helm/config_test.go
+++ b/internal/helm/config_test.go
@@ -18,6 +18,10 @@ func TestConfigTestSuite(t *testing.T) {
 }
 
 func (suite *ConfigTestSuite) TestPopulateWithPluginPrefix() {
+	suite.unsetenv("HELM_COMMAND")
+	suite.unsetenv("UPDATE_DEPENDENCIES")
+	suite.unsetenv("DEBUG")
+
 	suite.setenv("PLUGIN_HELM_COMMAND", "execute order 66")
 	suite.setenv("PLUGIN_UPDATE_DEPENDENCIES", "true")
 	suite.setenv("PLUGIN_DEBUG", "true")
@@ -30,6 +34,33 @@ func (suite *ConfigTestSuite) TestPopulateWithPluginPrefix() {
 	suite.True(cfg.Debug)
 }
 
+func (suite *ConfigTestSuite) TestPopulateWithNoPrefix() {
+	suite.unsetenv("PLUGIN_HELM_COMMAND")
+	suite.unsetenv("PLUGIN_UPDATE_DEPENDENCIES")
+	suite.unsetenv("PLUGIN_DEBUG")
+
+	suite.setenv("HELM_COMMAND", "execute order 66")
+	suite.setenv("UPDATE_DEPENDENCIES", "true")
+	suite.setenv("DEBUG", "true")
+
+	cfg := Config{}
+	cfg.Populate()
+
+	suite.Equal("execute order 66", cfg.Command)
+	suite.True(cfg.UpdateDependencies)
+	suite.True(cfg.Debug)
+}
+
+func (suite *ConfigTestSuite) TestPopulateWithConflictingVariables() {
+	suite.setenv("PLUGIN_HELM_COMMAND", "execute order 66")
+	suite.setenv("HELM_COMMAND", "defend the jedi")
+
+	cfg := Config{}
+	cfg.Populate()
+
+	suite.Equal("defend the jedi", cfg.Command)
+}
+
 func (suite *ConfigTestSuite) setenv(key, val string) {
 	orig, ok := os.LookupEnv(key)
 	if ok {