From 4755f502b5cde4cd2b6775f2343b0e712767d76b Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 12:43:17 -0800 Subject: [PATCH 01/11] Always use the default kubeconfig file path [#20] --- internal/helm/config.go | 3 +-- internal/helm/plan.go | 7 +++++-- internal/helm/plan_test.go | 4 ++-- internal/run/config.go | 1 - internal/run/initkube.go | 9 +++++---- internal/run/initkube_test.go | 19 ++++++++----------- internal/run/uninstall.go | 2 +- internal/run/uninstall_test.go | 27 ++++++++++----------------- internal/run/upgrade.go | 2 +- internal/run/upgrade_test.go | 25 +++++++++---------------- 10 files changed, 42 insertions(+), 57 deletions(-) diff --git a/internal/helm/config.go b/internal/helm/config.go index 9795cd2..a57c578 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -19,8 +19,7 @@ type Config struct { Prefix string `` // Prefix to use when looking up secret env vars // Global helm config - Debug bool `` // global helm flag (also applies to drone-helm itself) - KubeConfig string `split_words:"true" default:"/root/.kube/config"` // path to the kube config file + Debug bool `` // global helm flag (also applies to drone-helm itself) Values string `` StringValues string `split_words:"true"` ValuesFiles []string `split_words:"true"` diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 1d4ced9..62083c9 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -6,7 +6,10 @@ import ( "os" ) -const kubeConfigTemplate = "/root/.kube/config.tpl" +const ( + kubeConfigTemplate = "/root/.kube/config.tpl" + kubeConfigFile = "/root/.kube/config" +) // A Step is one step in the plan. type Step interface { @@ -27,7 +30,6 @@ func NewPlan(cfg Config) (*Plan, error) { cfg: cfg, runCfg: run.Config{ Debug: cfg.Debug, - KubeConfig: cfg.KubeConfig, Values: cfg.Values, StringValues: cfg.StringValues, ValuesFiles: cfg.ValuesFiles, @@ -141,6 +143,7 @@ func initKube(cfg Config) []Step { ServiceAccount: cfg.ServiceAccount, Token: cfg.KubeToken, TemplateFile: kubeConfigTemplate, + ConfigFile: kubeConfigFile, }, } } diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index e5a1a96..48ee4cc 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -32,7 +32,6 @@ func (suite *PlanTestSuite) TestNewPlan() { cfg := Config{ Command: "help", Debug: false, - KubeConfig: "/branch/.sfere/profig", Values: "steadfastness,forthrightness", StringValues: "tensile_strength,flexibility", ValuesFiles: []string{"/root/price_inventory.yml"}, @@ -41,7 +40,6 @@ func (suite *PlanTestSuite) TestNewPlan() { runCfg := run.Config{ Debug: false, - KubeConfig: "/branch/.sfere/profig", Values: "steadfastness,forthrightness", StringValues: "tensile_strength,flexibility", ValuesFiles: []string{"/root/price_inventory.yml"}, @@ -142,6 +140,7 @@ func (suite *PlanTestSuite) TestDel() { ServiceAccount: "greathelm", Token: "b2YgbXkgYWZmZWN0aW9u", TemplateFile: kubeConfigTemplate, + ConfigFile: kubeConfigFile, } suite.Equal(expected, init) @@ -176,6 +175,7 @@ func (suite *PlanTestSuite) TestInitKube() { ServiceAccount: "helmet", Token: "cXVlZXIgY2hhcmFjdGVyCg==", TemplateFile: kubeConfigTemplate, + ConfigFile: kubeConfigFile, } suite.Equal(expected, init) } diff --git a/internal/run/config.go b/internal/run/config.go index 09b9642..4f9b99a 100644 --- a/internal/run/config.go +++ b/internal/run/config.go @@ -7,7 +7,6 @@ import ( // Config contains configuration applicable to all helm commands type Config struct { Debug bool - KubeConfig string Values string StringValues string ValuesFiles []string diff --git a/internal/run/initkube.go b/internal/run/initkube.go index 4af29af..c868890 100644 --- a/internal/run/initkube.go +++ b/internal/run/initkube.go @@ -16,6 +16,7 @@ type InitKube struct { ServiceAccount string Token string TemplateFile string + ConfigFile string template *template.Template configFile io.WriteCloser @@ -34,7 +35,7 @@ type kubeValues struct { // Execute generates a kubernetes config file from drone-helm3's template. func (i *InitKube) Execute(cfg Config) error { if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "writing kubeconfig file to %s\n", cfg.KubeConfig) + fmt.Fprintf(cfg.Stderr, "writing kubeconfig file to %s\n", i.ConfigFile) } defer i.configFile.Close() return i.template.Execute(i.configFile, i.values) @@ -76,16 +77,16 @@ func (i *InitKube) Prepare(cfg Config) error { } if cfg.Debug { - if _, err := os.Stat(cfg.KubeConfig); err != nil { + if _, err := os.Stat(i.ConfigFile); err != nil { // non-nil err here isn't an actual error state; the kubeconfig just doesn't exist fmt.Fprint(cfg.Stderr, "creating ") } else { fmt.Fprint(cfg.Stderr, "truncating ") } - fmt.Fprintf(cfg.Stderr, "kubeconfig file at %s\n", cfg.KubeConfig) + fmt.Fprintf(cfg.Stderr, "kubeconfig file at %s\n", i.ConfigFile) } - i.configFile, err = os.Create(cfg.KubeConfig) + i.configFile, err = os.Create(i.ConfigFile) if err != nil { return fmt.Errorf("could not open kubeconfig file for writing: %w", err) } diff --git a/internal/run/initkube_test.go b/internal/run/initkube_test.go index fb32b15..74f6225 100644 --- a/internal/run/initkube_test.go +++ b/internal/run/initkube_test.go @@ -34,10 +34,10 @@ namespace: {{ .Namespace }} Certificate: "CCNA", Token: "Aspire virtual currency", TemplateFile: templateFile.Name(), + ConfigFile: configFile.Name(), } cfg := Config{ - Namespace: "Cisco", - KubeConfig: configFile.Name(), + Namespace: "Cisco", } err = init.Prepare(cfg) suite.Require().Nil(err) @@ -95,11 +95,10 @@ func (suite *InitKubeTestSuite) TestPrepareCannotOpenDestinationFile() { Certificate: "CCNA", Token: "Aspire virtual currency", TemplateFile: templateFile.Name(), + ConfigFile: "/usr/foreign/exclude/kubeprofig", } - cfg := Config{ - KubeConfig: "/usr/foreign/exclude/kubeprofig", - } + cfg := Config{} err = init.Prepare(cfg) suite.Error(err) suite.Regexp("could not open .* for writing: .* no such file or directory", err) @@ -120,11 +119,10 @@ func (suite *InitKubeTestSuite) TestPrepareRequiredConfig() { Certificate: "CCNA", Token: "Aspire virtual currency", TemplateFile: templateFile.Name(), + ConfigFile: configFile.Name(), } - cfg := Config{ - KubeConfig: configFile.Name(), - } + cfg := Config{} suite.NoError(init.Prepare(cfg)) // consistency check; we should be starting in a happy state @@ -157,11 +155,10 @@ func (suite *InitKubeTestSuite) TestPrepareDefaultsServiceAccount() { Certificate: "CCNA", Token: "Aspire virtual currency", TemplateFile: templateFile.Name(), + ConfigFile: configFile.Name(), } - cfg := Config{ - KubeConfig: configFile.Name(), - } + cfg := Config{} init.Prepare(cfg) suite.Equal("helm", init.ServiceAccount) diff --git a/internal/run/uninstall.go b/internal/run/uninstall.go index 1b9d0b2..e24daca 100644 --- a/internal/run/uninstall.go +++ b/internal/run/uninstall.go @@ -22,7 +22,7 @@ func (u *Uninstall) Prepare(cfg Config) error { return fmt.Errorf("release is required") } - args := []string{"--kubeconfig", cfg.KubeConfig} + args := make([]string, 0) if cfg.Namespace != "" { args = append(args, "--namespace", cfg.Namespace) diff --git a/internal/run/uninstall_test.go b/internal/run/uninstall_test.go index 9e826c9..fce7fee 100644 --- a/internal/run/uninstall_test.go +++ b/internal/run/uninstall_test.go @@ -58,11 +58,9 @@ func (suite *UninstallTestSuite) TestPrepareAndExecute() { Run(). Times(1) - cfg := Config{ - KubeConfig: "/root/.kube/config", - } + cfg := Config{} suite.NoError(u.Prepare(cfg)) - expected := []string{"--kubeconfig", "/root/.kube/config", "uninstall", "zayde_wølf_king"} + expected := []string{"uninstall", "zayde_wølf_king"} suite.Equal(expected, actual) u.Execute(cfg) @@ -73,15 +71,13 @@ func (suite *UninstallTestSuite) TestPrepareDryRunFlag() { Release: "firefox_ak_wildfire", DryRun: true, } - cfg := Config{ - KubeConfig: "/root/.kube/config", - } + cfg := Config{} suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() suite.NoError(u.Prepare(cfg)) - expected := []string{"--kubeconfig", "/root/.kube/config", "uninstall", "--dry-run", "firefox_ak_wildfire"} + expected := []string{"uninstall", "--dry-run", "firefox_ak_wildfire"} suite.Equal(expected, suite.actualArgs) } @@ -90,16 +86,14 @@ func (suite *UninstallTestSuite) TestPrepareNamespaceFlag() { Release: "carly_simon_run_away_with_me", } cfg := Config{ - KubeConfig: "/root/.kube/config", - Namespace: "emotion", + Namespace: "emotion", } suite.mockCmd.EXPECT().Stdout(gomock.Any()).AnyTimes() suite.mockCmd.EXPECT().Stderr(gomock.Any()).AnyTimes() suite.NoError(u.Prepare(cfg)) - expected := []string{"--kubeconfig", "/root/.kube/config", - "--namespace", "emotion", "uninstall", "carly_simon_run_away_with_me"} + expected := []string{"--namespace", "emotion", "uninstall", "carly_simon_run_away_with_me"} suite.Equal(expected, suite.actualArgs) } @@ -109,9 +103,8 @@ func (suite *UninstallTestSuite) TestPrepareDebugFlag() { } stderr := strings.Builder{} cfg := Config{ - KubeConfig: "/root/.kube/config", - Debug: true, - Stderr: &stderr, + Debug: true, + Stderr: &stderr, } command = func(path string, args ...string) cmd { @@ -126,8 +119,8 @@ func (suite *UninstallTestSuite) TestPrepareDebugFlag() { suite.mockCmd.EXPECT().Stderr(&stderr).AnyTimes() suite.NoError(u.Prepare(cfg)) - suite.Equal(fmt.Sprintf("Generated command: '%s --kubeconfig /root/.kube/config "+ - "--debug uninstall just_a_band_huff_and_puff'\n", helmBin), stderr.String()) + suite.Equal(fmt.Sprintf("Generated command: '%s --debug "+ + "uninstall just_a_band_huff_and_puff'\n", helmBin), stderr.String()) } func (suite *UninstallTestSuite) TestPrepareRequiresRelease() { diff --git a/internal/run/upgrade.go b/internal/run/upgrade.go index cff2c70..dd50527 100644 --- a/internal/run/upgrade.go +++ b/internal/run/upgrade.go @@ -33,7 +33,7 @@ func (u *Upgrade) Prepare(cfg Config) error { return fmt.Errorf("release is required") } - args := []string{"--kubeconfig", cfg.KubeConfig} + args := make([]string, 0) if cfg.Namespace != "" { args = append(args, "--namespace", cfg.Namespace) diff --git a/internal/run/upgrade_test.go b/internal/run/upgrade_test.go index 13b2080..aae1af4 100644 --- a/internal/run/upgrade_test.go +++ b/internal/run/upgrade_test.go @@ -41,8 +41,7 @@ func (suite *UpgradeTestSuite) TestPrepareAndExecute() { command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) - suite.Equal([]string{"--kubeconfig", "/root/.kube/config", "upgrade", "--install", - "jonas_brothers_only_human", "at40"}, args) + suite.Equal([]string{"upgrade", "--install", "jonas_brothers_only_human", "at40"}, args) return suite.mockCmd } @@ -55,9 +54,7 @@ func (suite *UpgradeTestSuite) TestPrepareAndExecute() { Run(). Times(1) - cfg := Config{ - KubeConfig: "/root/.kube/config", - } + cfg := Config{} err := u.Prepare(cfg) suite.Require().Nil(err) u.Execute(cfg) @@ -73,8 +70,7 @@ func (suite *UpgradeTestSuite) TestPrepareNamespaceFlag() { command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) - suite.Equal([]string{"--kubeconfig", "/root/.kube/config", "--namespace", "melt", "upgrade", - "--install", "shaed_trampoline", "at40"}, args) + suite.Equal([]string{"--namespace", "melt", "upgrade", "--install", "shaed_trampoline", "at40"}, args) return suite.mockCmd } @@ -83,8 +79,7 @@ func (suite *UpgradeTestSuite) TestPrepareNamespaceFlag() { suite.mockCmd.EXPECT().Stderr(gomock.Any()) cfg := Config{ - Namespace: "melt", - KubeConfig: "/root/.kube/config", + Namespace: "melt", } err := u.Prepare(cfg) suite.Require().Nil(err) @@ -105,7 +100,6 @@ func (suite *UpgradeTestSuite) TestPrepareWithUpgradeFlags() { } cfg := Config{ - KubeConfig: "/root/.kube/config", Values: "age=35", StringValues: "height=5ft10in", ValuesFiles: []string{"/usr/local/stats", "/usr/local/grades"}, @@ -113,7 +107,7 @@ func (suite *UpgradeTestSuite) TestPrepareWithUpgradeFlags() { command = func(path string, args ...string) cmd { suite.Equal(helmBin, path) - suite.Equal([]string{"--kubeconfig", "/root/.kube/config", "upgrade", "--install", + suite.Equal([]string{"upgrade", "--install", "--version", "radio_edit", "--dry-run", "--wait", @@ -165,10 +159,9 @@ func (suite *UpgradeTestSuite) TestPrepareDebugFlag() { stdout := strings.Builder{} stderr := strings.Builder{} cfg := Config{ - Debug: true, - KubeConfig: "/root/.kube/config", - Stdout: &stdout, - Stderr: &stderr, + Debug: true, + Stdout: &stdout, + Stderr: &stderr, } command = func(path string, args ...string) cmd { @@ -186,7 +179,7 @@ func (suite *UpgradeTestSuite) TestPrepareDebugFlag() { u.Prepare(cfg) - want := fmt.Sprintf("Generated command: '%s --kubeconfig /root/.kube/config --debug upgrade "+ + want := fmt.Sprintf("Generated command: '%s --debug upgrade "+ "--install lewis_capaldi_someone_you_loved at40'\n", helmBin) suite.Equal(want, stderr.String()) suite.Equal("", stdout.String()) From ad5baea3e6cf2c5193932ba2e1596ed921141497 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 14:02:46 -0800 Subject: [PATCH 02/11] Document helm.Config's struct fields more clearly [#9] --- internal/helm/config.go | 50 +++++++++++++++++++---------------------- 1 file changed, 23 insertions(+), 27 deletions(-) diff --git a/internal/helm/config.go b/internal/helm/config.go index a57c578..f2041e0 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -12,33 +12,29 @@ import ( // `envconfig:` tag so that envconfig will look for a non-prefixed env var. type Config struct { // Configuration for drone-helm itself - Command helmCommand `envconfig:"HELM_COMMAND"` // Helm command to run - DroneEvent string `envconfig:"DRONE_BUILD_EVENT"` // Drone event that invoked this plugin. - UpdateDependencies bool `split_words:"true"` // call `helm dependency update` before the main command - Repos []string `envconfig:"HELM_REPOS"` // call `helm repo add` before the main command - Prefix string `` // Prefix to use when looking up secret env vars - - // Global helm config - Debug bool `` // global helm flag (also applies to drone-helm itself) - Values string `` - StringValues string `split_words:"true"` - ValuesFiles []string `split_words:"true"` - Namespace string `` - KubeToken string `envconfig:"KUBERNETES_TOKEN"` - SkipTLSVerify bool `envconfig:"SKIP_TLS_VERIFY"` - Certificate string `envconfig:"KUBERNETES_CERTIFICATE"` - APIServer string `envconfig:"API_SERVER"` - ServiceAccount string `envconfig:"SERVICE_ACCOUNT"` // Can't just use split_words; need envconfig to find the non-prefixed form - - // Config specifically for `helm upgrade` - ChartVersion string `split_words:"true"` // - DryRun bool `split_words:"true"` // also available for `delete` - Wait bool `` // - ReuseValues bool `split_words:"true"` // - Timeout string `` // - Chart string `` // Also available for `lint`, in which case it must be a path to a chart directory - Release string `` - Force bool `` // + Command helmCommand `envconfig:"HELM_COMMAND"` // Helm command to run + DroneEvent string `envconfig:"DRONE_BUILD_EVENT"` // Drone event that invoked this plugin. + UpdateDependencies bool `split_words:"true"` // Call `helm dependency update` before the main command + Repos []string `envconfig:"HELM_REPOS"` // Call `helm repo add` before the main command + Prefix string `` // Prefix to use when looking up secret env vars + Debug bool `` // Generate debug output and pass --debug to all helm commands + Values string `` // Argument to pass to --set in applicable helm commands + StringValues string `split_words:"true"` // Argument to pass to --set-string in applicable helm commands + ValuesFiles []string `split_words:"true"` // Arguments to pass to --values in applicable helm commands + Namespace string `` // Kubernetes namespace for all helm commands + KubeToken string `envconfig:"KUBERNETES_TOKEN"` // Kubernetes authentication token to put in .kube/config + 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 + 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 + ReuseValues bool `split_words:"true"` // Pass --reuse-values to `helm upgrade` + Timeout string `` // Argument to pass to --timeout in applicable helm commands + Chart string `` // Chart argument to use in applicable helm commands + Release string `` // Release argument to use in applicable helm commands + Force bool `` // Pass --force to applicable helm commands } type helmCommand string From ae9cb59a1f21803d250a0fa099faa80a0ff71464 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 14:03:51 -0800 Subject: [PATCH 03/11] No typo inthe helm.Config docs [#9] --- internal/helm/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/helm/config.go b/internal/helm/config.go index f2041e0..19bcec4 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -5,7 +5,7 @@ import ( "strings" ) -// The Config struct captures the `settings` and `environment` blocks inthe application's drone +// The Config struct captures the `settings` and `environment` blocks in the application's drone // config. Configuration in drone's `settings` block arrives as uppercase env vars matching the // config key, prefixed with `PLUGIN_`. Config from the `environment` block is *not* prefixed; any // keys that are likely to be in that block (i.e. things that use `from_secret` need an explicit From ef4db923cd3c832e45d56d552216812dc6c1abef Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 14:06:05 -0800 Subject: [PATCH 04/11] Use a plain string for helm.Config.Command [#9] I'm leaving the no-op test file in place because my next step is to add new behavior that will require testing. --- internal/helm/config.go | 71 ++++++++++++------------------------ internal/helm/config_test.go | 14 ------- 2 files changed, 24 insertions(+), 61 deletions(-) diff --git a/internal/helm/config.go b/internal/helm/config.go index 19bcec4..c193f94 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -1,9 +1,6 @@ package helm -import ( - "fmt" - "strings" -) +import () // The Config struct captures the `settings` and `environment` blocks in the application's drone // config. Configuration in drone's `settings` block arrives as uppercase env vars matching the @@ -12,47 +9,27 @@ import ( // `envconfig:` tag so that envconfig will look for a non-prefixed env var. type Config struct { // Configuration for drone-helm itself - Command helmCommand `envconfig:"HELM_COMMAND"` // Helm command to run - DroneEvent string `envconfig:"DRONE_BUILD_EVENT"` // Drone event that invoked this plugin. - UpdateDependencies bool `split_words:"true"` // Call `helm dependency update` before the main command - Repos []string `envconfig:"HELM_REPOS"` // Call `helm repo add` before the main command - Prefix string `` // Prefix to use when looking up secret env vars - Debug bool `` // Generate debug output and pass --debug to all helm commands - Values string `` // Argument to pass to --set in applicable helm commands - StringValues string `split_words:"true"` // Argument to pass to --set-string in applicable helm commands - ValuesFiles []string `split_words:"true"` // Arguments to pass to --values in applicable helm commands - Namespace string `` // Kubernetes namespace for all helm commands - KubeToken string `envconfig:"KUBERNETES_TOKEN"` // Kubernetes authentication token to put in .kube/config - 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 - 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 - ReuseValues bool `split_words:"true"` // Pass --reuse-values to `helm upgrade` - Timeout string `` // Argument to pass to --timeout in applicable helm commands - Chart string `` // Chart argument to use in applicable helm commands - Release string `` // Release argument to use in applicable helm commands - Force bool `` // Pass --force to applicable helm commands -} - -type helmCommand string - -// helmCommand.Decode checks the given value against the list of known commands and generates a helpful error if the command is unknown. -func (cmd *helmCommand) Decode(value string) error { - known := []string{"upgrade", "delete", "lint", "help"} - for _, c := range known { - if value == c { - *cmd = helmCommand(value) - return nil - } - } - - if value == "" { - return nil - } - known[len(known)-1] = fmt.Sprintf("or %s", known[len(known)-1]) - return fmt.Errorf("unknown command '%s'. If specified, command must be %s", - value, strings.Join(known, ", ")) + Command string `envconfig:"HELM_COMMAND"` // Helm command to run + DroneEvent string `envconfig:"DRONE_BUILD_EVENT"` // Drone event that invoked this plugin. + UpdateDependencies bool `split_words:"true"` // Call `helm dependency update` before the main command + Repos []string `envconfig:"HELM_REPOS"` // Call `helm repo add` before the main command + Prefix string `` // Prefix to use when looking up secret env vars + Debug bool `` // Generate debug output and pass --debug to all helm commands + Values string `` // Argument to pass to --set in applicable helm commands + StringValues string `split_words:"true"` // Argument to pass to --set-string in applicable helm commands + ValuesFiles []string `split_words:"true"` // Arguments to pass to --values in applicable helm commands + Namespace string `` // Kubernetes namespace for all helm commands + KubeToken string `envconfig:"KUBERNETES_TOKEN"` // Kubernetes authentication token to put in .kube/config + 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 + 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 + ReuseValues bool `split_words:"true"` // Pass --reuse-values to `helm upgrade` + Timeout string `` // Argument to pass to --timeout in applicable helm commands + Chart string `` // Chart argument to use in applicable helm commands + Release string `` // Release argument to use in applicable helm commands + Force bool `` // Pass --force to applicable helm commands } diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index 1df2d4f..de7538f 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -12,17 +12,3 @@ type ConfigTestSuite struct { func TestConfigTestSuite(t *testing.T) { suite.Run(t, new(ConfigTestSuite)) } - -func (suite *ConfigTestSuite) TestHelmCommandDecodeSuccess() { - cmd := helmCommand("") - err := cmd.Decode("upgrade") - suite.Require().Nil(err) - - suite.EqualValues(cmd, "upgrade") -} - -func (suite *ConfigTestSuite) TestHelmCommandDecodeFailure() { - cmd := helmCommand("") - err := cmd.Decode("execute order 66") - suite.EqualError(err, "unknown command 'execute order 66'. If specified, command must be upgrade, delete, lint, or help") -} From c4c136b021b53822b7ddf5c08b153f21ba0415c0 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 14:44:59 -0800 Subject: [PATCH 05/11] Do envconfig-loading in config.go (and test it!) [#9] --- cmd/drone-helm/main.go | 3 +-- internal/helm/config.go | 9 ++++++- internal/helm/config_test.go | 51 ++++++++++++++++++++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/cmd/drone-helm/main.go b/cmd/drone-helm/main.go index 61673b2..e42cb1e 100644 --- a/cmd/drone-helm/main.go +++ b/cmd/drone-helm/main.go @@ -2,7 +2,6 @@ package main import ( "fmt" - "github.com/kelseyhightower/envconfig" "os" "github.com/pelotech/drone-helm3/internal/helm" @@ -11,7 +10,7 @@ import ( func main() { var c helm.Config - if err := envconfig.Process("plugin", &c); err != nil { + if err := c.Populate(); err != nil { fmt.Fprintf(os.Stderr, "%s\n", err.Error()) return } diff --git a/internal/helm/config.go b/internal/helm/config.go index c193f94..afdc978 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -1,6 +1,8 @@ package helm -import () +import ( + "github.com/kelseyhightower/envconfig" +) // The Config struct captures the `settings` and `environment` blocks in the application's drone // config. Configuration in drone's `settings` block arrives as uppercase env vars matching the @@ -33,3 +35,8 @@ type Config struct { Release string `` // Release argument to use in applicable helm commands Force bool `` // Pass --force to applicable helm commands } + +// Populate reads environment variables into the Config. +func (cfg *Config) Populate() error { + return envconfig.Process("plugin", cfg) +} diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index de7538f..a594c09 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -2,13 +2,64 @@ package helm import ( "github.com/stretchr/testify/suite" + "os" "testing" ) type ConfigTestSuite struct { suite.Suite + // These tests need to mutate the environment, so the suite.setenv and .unsetenv functions store the original contents of the + // relevant variable in this map. Its use of *string is so they can distinguish between "not set" and "set to empty string" + envBackup map[string]*string } func TestConfigTestSuite(t *testing.T) { suite.Run(t, new(ConfigTestSuite)) } + +func (suite *ConfigTestSuite) TestPopulateWithPluginPrefix() { + suite.setenv("PLUGIN_HELM_COMMAND", "execute order 66") + suite.setenv("PLUGIN_UPDATE_DEPENDENCIES", "true") + suite.setenv("PLUGIN_DEBUG", "true") + + cfg := Config{} + cfg.Populate() + + suite.Equal("execute order 66", cfg.Command) + suite.True(cfg.UpdateDependencies) + suite.True(cfg.Debug) +} + +func (suite *ConfigTestSuite) setenv(key, val string) { + orig, ok := os.LookupEnv(key) + if ok { + suite.envBackup[key] = &orig + } else { + suite.envBackup[key] = nil + } + os.Setenv(key, val) +} + +func (suite *ConfigTestSuite) unsetenv(key string) { + orig, ok := os.LookupEnv(key) + if ok { + suite.envBackup[key] = &orig + } else { + suite.envBackup[key] = nil + } + os.Unsetenv(key) +} + +func (suite *ConfigTestSuite) BeforeTest(_, _ string) { + suite.envBackup = make(map[string]*string) +} + +func (suite *ConfigTestSuite) AfterTest(_, _ string) { + for key, val := range suite.envBackup { + if val == nil { + os.Unsetenv(key) + } else { + os.Setenv(key, *val) + } + } +} From e2f53f3b080fe159f0f9b14cbcbcfbe0f1e81008 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 15:31:40 -0800 Subject: [PATCH 06/11] 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 { From db87bd05070aacd65556f72d0a33d460d6182854 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 15:52:01 -0800 Subject: [PATCH 07/11] Require no-error in config tests [#9] --- internal/helm/config_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index 7427e1a..ceff436 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -27,7 +27,7 @@ func (suite *ConfigTestSuite) TestPopulateWithPluginPrefix() { suite.setenv("PLUGIN_DEBUG", "true") cfg := Config{} - cfg.Populate() + suite.Require().NoError(cfg.Populate()) suite.Equal("execute order 66", cfg.Command) suite.True(cfg.UpdateDependencies) @@ -44,7 +44,7 @@ func (suite *ConfigTestSuite) TestPopulateWithNoPrefix() { suite.setenv("DEBUG", "true") cfg := Config{} - cfg.Populate() + suite.Require().NoError(cfg.Populate()) suite.Equal("execute order 66", cfg.Command) suite.True(cfg.UpdateDependencies) @@ -56,7 +56,7 @@ func (suite *ConfigTestSuite) TestPopulateWithConflictingVariables() { suite.setenv("HELM_COMMAND", "defend the jedi") cfg := Config{} - cfg.Populate() + suite.Require().NoError(cfg.Populate()) suite.Equal("defend the jedi", cfg.Command) } From 285e9d98a492f2d4730a91b688bb4336c63aecd4 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 16:07:49 -0800 Subject: [PATCH 08/11] Allow a configurable env var prefix [#19] I'd like to keep Prefix's scope fairly limited, because it has potential to spiral into something magnificently complex. You get one prefix setting, it goes in `settings` not `environment`, end of feature. --- internal/helm/config.go | 8 +++++++ internal/helm/config_test.go | 46 +++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/internal/helm/config.go b/internal/helm/config.go index bb0f06e..04c70c6 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -42,9 +42,17 @@ func (cfg *Config) Populate() error { return err } + prefix := cfg.Prefix + if err := envconfig.Process("", cfg); err != nil { return err } + if prefix != "" { + if err := envconfig.Process(cfg.Prefix, cfg); err != nil { + return err + } + } + return nil } diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index ceff436..db556e8 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -18,6 +18,7 @@ func TestConfigTestSuite(t *testing.T) { } func (suite *ConfigTestSuite) TestPopulateWithPluginPrefix() { + suite.unsetenv("PLUGIN_PREFIX") suite.unsetenv("HELM_COMMAND") suite.unsetenv("UPDATE_DEPENDENCIES") suite.unsetenv("DEBUG") @@ -35,6 +36,7 @@ func (suite *ConfigTestSuite) TestPopulateWithPluginPrefix() { } func (suite *ConfigTestSuite) TestPopulateWithNoPrefix() { + suite.unsetenv("PLUGIN_PREFIX") suite.unsetenv("PLUGIN_HELM_COMMAND") suite.unsetenv("PLUGIN_UPDATE_DEPENDENCIES") suite.unsetenv("PLUGIN_DEBUG") @@ -51,14 +53,56 @@ func (suite *ConfigTestSuite) TestPopulateWithNoPrefix() { suite.True(cfg.Debug) } +func (suite *ConfigTestSuite) TestPopulateWithConfigurablePrefix() { + suite.unsetenv("API_SERVER") + suite.unsetenv("PLUGIN_API_SERVER") + + suite.setenv("PLUGIN_PREFIX", "prix_fixe") + suite.setenv("PRIX_FIXE_API_SERVER", "your waiter this evening") + + cfg := Config{} + suite.Require().NoError(cfg.Populate()) + + suite.Equal("prix_fixe", cfg.Prefix) + suite.Equal("your waiter this evening", cfg.APIServer) +} + +func (suite *ConfigTestSuite) TestPrefixSettingDoesNotAffectPluginPrefix() { + suite.setenv("PLUGIN_PREFIX", "IXFREP") + suite.setenv("PLUGIN_HELM_COMMAND", "wake me up") + suite.setenv("IXFREP_PLUGIN_HELM_COMMAND", "send me to sleep inside") + + cfg := Config{} + suite.Require().NoError(cfg.Populate()) + + suite.Equal("wake me up", cfg.Command) +} + +func (suite *ConfigTestSuite) TestPrefixSettingMustHavePluginPrefix() { + suite.unsetenv("PLUGIN_PREFIX") + suite.setenv("PREFIX", "refpix") + suite.setenv("HELM_COMMAND", "gimme more") + suite.setenv("REFPIX_HELM_COMMAND", "gimme less") + + cfg := Config{} + suite.Require().NoError(cfg.Populate()) + + suite.Equal("gimme more", cfg.Command) +} + func (suite *ConfigTestSuite) TestPopulateWithConflictingVariables() { suite.setenv("PLUGIN_HELM_COMMAND", "execute order 66") - suite.setenv("HELM_COMMAND", "defend the jedi") + suite.setenv("HELM_COMMAND", "defend the jedi") // values from the `environment` block override those from `settings` + + suite.setenv("PLUGIN_PREFIX", "prod") + suite.setenv("TIMEOUT", "5m0s") + suite.setenv("PROD_TIMEOUT", "2m30s") // values from prefixed env vars override those from non-prefixed ones cfg := Config{} suite.Require().NoError(cfg.Populate()) suite.Equal("defend the jedi", cfg.Command) + suite.Equal("2m30s", cfg.Timeout) } func (suite *ConfigTestSuite) setenv(key, val string) { From 10e7e7fee5716a8421d2207b98c56281a1e82cda Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 16:45:09 -0800 Subject: [PATCH 09/11] Document the Config struct's behavior correctly [#19] [#9] --- internal/helm/config.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/internal/helm/config.go b/internal/helm/config.go index 04c70c6..9e88dab 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -6,9 +6,8 @@ import ( // The Config struct captures the `settings` and `environment` blocks in the application's drone // config. Configuration in drone's `settings` block arrives as uppercase env vars matching the -// config key, prefixed with `PLUGIN_`. Config from the `environment` block is *not* prefixed; any -// keys that are likely to be in that block (i.e. things that use `from_secret` need an explicit -// `envconfig:` tag so that envconfig will look for a non-prefixed env var. +// config key, prefixed with `PLUGIN_`. Config from the `environment` block is uppercased, but does +// not have the `PLUGIN_` prefix. It may, however, be prefixed with the value in `$PLUGIN_PREFIX`. type Config struct { // Configuration for drone-helm itself Command string `envconfig:"HELM_COMMAND"` // Helm command to run From 4ba1e694d922f929aac700e3846277d19b8ced64 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 24 Dec 2019 09:34:38 -0800 Subject: [PATCH 10/11] Use a go-idiomatic constructor for helm.Config [#9] --- cmd/drone-helm/main.go | 6 +++--- internal/helm/config.go | 19 ++++++++++--------- internal/helm/config_test.go | 32 ++++++++++++++++---------------- 3 files changed, 29 insertions(+), 28 deletions(-) diff --git a/cmd/drone-helm/main.go b/cmd/drone-helm/main.go index e42cb1e..a6a4d09 100644 --- a/cmd/drone-helm/main.go +++ b/cmd/drone-helm/main.go @@ -8,15 +8,15 @@ import ( ) func main() { - var c helm.Config + cfg, err := helm.NewConfig() - if err := c.Populate(); err != nil { + if err != nil { fmt.Fprintf(os.Stderr, "%s\n", err.Error()) return } // Make the plan - plan, err := helm.NewPlan(c) + plan, err := helm.NewPlan(*cfg) if err != nil { fmt.Fprintf(os.Stderr, "%w\n", err) os.Exit(1) diff --git a/internal/helm/config.go b/internal/helm/config.go index 9e88dab..54d0d42 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -35,23 +35,24 @@ type Config struct { Force bool `` // Pass --force to applicable helm commands } -// Populate reads environment variables into the Config, accounting for several possible formats. -func (cfg *Config) Populate() error { - if err := envconfig.Process("plugin", cfg); err != nil { - return err +// NewConfig creates a Config and reads environment variables into it, accounting for several possible formats. +func NewConfig() (*Config, error) { + cfg := Config{} + if err := envconfig.Process("plugin", &cfg); err != nil { + return nil, err } prefix := cfg.Prefix - if err := envconfig.Process("", cfg); err != nil { - return err + if err := envconfig.Process("", &cfg); err != nil { + return nil, err } if prefix != "" { - if err := envconfig.Process(cfg.Prefix, cfg); err != nil { - return err + if err := envconfig.Process(cfg.Prefix, &cfg); err != nil { + return nil, err } } - return nil + return &cfg, nil } diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index db556e8..88d1933 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -17,7 +17,7 @@ func TestConfigTestSuite(t *testing.T) { suite.Run(t, new(ConfigTestSuite)) } -func (suite *ConfigTestSuite) TestPopulateWithPluginPrefix() { +func (suite *ConfigTestSuite) TestNewConfigWithPluginPrefix() { suite.unsetenv("PLUGIN_PREFIX") suite.unsetenv("HELM_COMMAND") suite.unsetenv("UPDATE_DEPENDENCIES") @@ -27,15 +27,15 @@ func (suite *ConfigTestSuite) TestPopulateWithPluginPrefix() { suite.setenv("PLUGIN_UPDATE_DEPENDENCIES", "true") suite.setenv("PLUGIN_DEBUG", "true") - cfg := Config{} - suite.Require().NoError(cfg.Populate()) + cfg, err := NewConfig() + suite.Require().NoError(err) suite.Equal("execute order 66", cfg.Command) suite.True(cfg.UpdateDependencies) suite.True(cfg.Debug) } -func (suite *ConfigTestSuite) TestPopulateWithNoPrefix() { +func (suite *ConfigTestSuite) TestNewConfigWithNoPrefix() { suite.unsetenv("PLUGIN_PREFIX") suite.unsetenv("PLUGIN_HELM_COMMAND") suite.unsetenv("PLUGIN_UPDATE_DEPENDENCIES") @@ -45,23 +45,23 @@ func (suite *ConfigTestSuite) TestPopulateWithNoPrefix() { suite.setenv("UPDATE_DEPENDENCIES", "true") suite.setenv("DEBUG", "true") - cfg := Config{} - suite.Require().NoError(cfg.Populate()) + cfg, err := NewConfig() + suite.Require().NoError(err) suite.Equal("execute order 66", cfg.Command) suite.True(cfg.UpdateDependencies) suite.True(cfg.Debug) } -func (suite *ConfigTestSuite) TestPopulateWithConfigurablePrefix() { +func (suite *ConfigTestSuite) TestNewConfigWithConfigurablePrefix() { suite.unsetenv("API_SERVER") suite.unsetenv("PLUGIN_API_SERVER") suite.setenv("PLUGIN_PREFIX", "prix_fixe") suite.setenv("PRIX_FIXE_API_SERVER", "your waiter this evening") - cfg := Config{} - suite.Require().NoError(cfg.Populate()) + cfg, err := NewConfig() + suite.Require().NoError(err) suite.Equal("prix_fixe", cfg.Prefix) suite.Equal("your waiter this evening", cfg.APIServer) @@ -72,8 +72,8 @@ func (suite *ConfigTestSuite) TestPrefixSettingDoesNotAffectPluginPrefix() { suite.setenv("PLUGIN_HELM_COMMAND", "wake me up") suite.setenv("IXFREP_PLUGIN_HELM_COMMAND", "send me to sleep inside") - cfg := Config{} - suite.Require().NoError(cfg.Populate()) + cfg, err := NewConfig() + suite.Require().NoError(err) suite.Equal("wake me up", cfg.Command) } @@ -84,13 +84,13 @@ func (suite *ConfigTestSuite) TestPrefixSettingMustHavePluginPrefix() { suite.setenv("HELM_COMMAND", "gimme more") suite.setenv("REFPIX_HELM_COMMAND", "gimme less") - cfg := Config{} - suite.Require().NoError(cfg.Populate()) + cfg, err := NewConfig() + suite.Require().NoError(err) suite.Equal("gimme more", cfg.Command) } -func (suite *ConfigTestSuite) TestPopulateWithConflictingVariables() { +func (suite *ConfigTestSuite) TestNewConfigWithConflictingVariables() { suite.setenv("PLUGIN_HELM_COMMAND", "execute order 66") suite.setenv("HELM_COMMAND", "defend the jedi") // values from the `environment` block override those from `settings` @@ -98,8 +98,8 @@ func (suite *ConfigTestSuite) TestPopulateWithConflictingVariables() { suite.setenv("TIMEOUT", "5m0s") suite.setenv("PROD_TIMEOUT", "2m30s") // values from prefixed env vars override those from non-prefixed ones - cfg := Config{} - suite.Require().NoError(cfg.Populate()) + cfg, err := NewConfig() + suite.Require().NoError(err) suite.Equal("defend the jedi", cfg.Command) suite.Equal("2m30s", cfg.Timeout) From 08ddf5e27a792f39141e99693e10a7a03002a25e Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 24 Dec 2019 11:08:09 -0800 Subject: [PATCH 11/11] Log debug output in helm.Config [#9] Redacting KubeToken may not be sufficient, since it's possible that someone would put secrets in Values or StringValues. Unilaterally redacting those seems unhelpful, though, since they may be the very thing the user is trying to debug. I've settled on redacting the obvious field without trying to promise that all sensitive data will be hidden. --- cmd/drone-helm/main.go | 2 +- internal/helm/config.go | 23 ++++++++++++++-- internal/helm/config_test.go | 52 +++++++++++++++++++++++++++++++----- internal/helm/plan.go | 6 ++--- internal/helm/plan_test.go | 10 ++++--- 5 files changed, 78 insertions(+), 15 deletions(-) diff --git a/cmd/drone-helm/main.go b/cmd/drone-helm/main.go index a6a4d09..43d2e30 100644 --- a/cmd/drone-helm/main.go +++ b/cmd/drone-helm/main.go @@ -8,7 +8,7 @@ import ( ) func main() { - cfg, err := helm.NewConfig() + cfg, err := helm.NewConfig(os.Stdout, os.Stderr) if err != nil { fmt.Fprintf(os.Stderr, "%s\n", err.Error()) diff --git a/internal/helm/config.go b/internal/helm/config.go index 54d0d42..987b6de 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -1,7 +1,9 @@ package helm import ( + "fmt" "github.com/kelseyhightower/envconfig" + "io" ) // The Config struct captures the `settings` and `environment` blocks in the application's drone @@ -33,11 +35,17 @@ type Config struct { Chart string `` // Chart argument to use in applicable helm commands Release string `` // Release argument to use in applicable helm commands Force bool `` // Pass --force to applicable helm commands + + Stdout io.Writer `ignored:"true"` + Stderr io.Writer `ignored:"true"` } // NewConfig creates a Config and reads environment variables into it, accounting for several possible formats. -func NewConfig() (*Config, error) { - cfg := Config{} +func NewConfig(stdout, stderr io.Writer) (*Config, error) { + cfg := Config{ + Stdout: stdout, + Stderr: stderr, + } if err := envconfig.Process("plugin", &cfg); err != nil { return nil, err } @@ -54,5 +62,16 @@ func NewConfig() (*Config, error) { } } + if cfg.Debug && cfg.Stderr != nil { + cfg.logDebug() + } + return &cfg, nil } + +func (cfg Config) logDebug() { + if cfg.KubeToken != "" { + cfg.KubeToken = "(redacted)" + } + fmt.Fprintf(cfg.Stderr, "Generated config: %+v\n", cfg) +} diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index 88d1933..3ca2f91 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -3,6 +3,7 @@ package helm import ( "github.com/stretchr/testify/suite" "os" + "strings" "testing" ) @@ -27,7 +28,7 @@ func (suite *ConfigTestSuite) TestNewConfigWithPluginPrefix() { suite.setenv("PLUGIN_UPDATE_DEPENDENCIES", "true") suite.setenv("PLUGIN_DEBUG", "true") - cfg, err := NewConfig() + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) suite.Equal("execute order 66", cfg.Command) @@ -45,7 +46,7 @@ func (suite *ConfigTestSuite) TestNewConfigWithNoPrefix() { suite.setenv("UPDATE_DEPENDENCIES", "true") suite.setenv("DEBUG", "true") - cfg, err := NewConfig() + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) suite.Equal("execute order 66", cfg.Command) @@ -60,7 +61,7 @@ func (suite *ConfigTestSuite) TestNewConfigWithConfigurablePrefix() { suite.setenv("PLUGIN_PREFIX", "prix_fixe") suite.setenv("PRIX_FIXE_API_SERVER", "your waiter this evening") - cfg, err := NewConfig() + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) suite.Equal("prix_fixe", cfg.Prefix) @@ -72,7 +73,7 @@ func (suite *ConfigTestSuite) TestPrefixSettingDoesNotAffectPluginPrefix() { suite.setenv("PLUGIN_HELM_COMMAND", "wake me up") suite.setenv("IXFREP_PLUGIN_HELM_COMMAND", "send me to sleep inside") - cfg, err := NewConfig() + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) suite.Equal("wake me up", cfg.Command) @@ -84,7 +85,7 @@ func (suite *ConfigTestSuite) TestPrefixSettingMustHavePluginPrefix() { suite.setenv("HELM_COMMAND", "gimme more") suite.setenv("REFPIX_HELM_COMMAND", "gimme less") - cfg, err := NewConfig() + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) suite.Equal("gimme more", cfg.Command) @@ -98,13 +99,52 @@ func (suite *ConfigTestSuite) TestNewConfigWithConflictingVariables() { suite.setenv("TIMEOUT", "5m0s") suite.setenv("PROD_TIMEOUT", "2m30s") // values from prefixed env vars override those from non-prefixed ones - cfg, err := NewConfig() + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) suite.Equal("defend the jedi", cfg.Command) suite.Equal("2m30s", cfg.Timeout) } +func (suite *ConfigTestSuite) TestNewConfigSetsWriters() { + stdout := &strings.Builder{} + stderr := &strings.Builder{} + cfg, err := NewConfig(stdout, stderr) + suite.Require().NoError(err) + + suite.Equal(stdout, cfg.Stdout) + suite.Equal(stderr, cfg.Stderr) +} + +func (suite *ConfigTestSuite) TestLogDebug() { + suite.setenv("DEBUG", "true") + suite.setenv("HELM_COMMAND", "upgrade") + + stderr := strings.Builder{} + stdout := strings.Builder{} + _, err := NewConfig(&stdout, &stderr) + suite.Require().NoError(err) + + suite.Equal("", stdout.String()) + + suite.Regexp(`^Generated config: \{Command:upgrade.*\}`, stderr.String()) +} + +func (suite *ConfigTestSuite) TestLogDebugCensorsKubeToken() { + stderr := &strings.Builder{} + kubeToken := "I'm shy! Don't put me in your build logs!" + cfg := Config{ + Debug: true, + KubeToken: kubeToken, + Stderr: stderr, + } + + cfg.logDebug() + + suite.Contains(stderr.String(), "KubeToken:(redacted)") + suite.Equal(kubeToken, cfg.KubeToken) // The actual config value should be left unchanged +} + func (suite *ConfigTestSuite) setenv(key, val string) { orig, ok := os.LookupEnv(key) if ok { diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 62083c9..599fb55 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -34,8 +34,8 @@ func NewPlan(cfg Config) (*Plan, error) { StringValues: cfg.StringValues, ValuesFiles: cfg.ValuesFiles, Namespace: cfg.Namespace, - Stdout: os.Stdout, - Stderr: os.Stderr, + Stdout: cfg.Stdout, + Stderr: cfg.Stderr, }, } @@ -83,7 +83,7 @@ func determineSteps(cfg Config) *func(Config) []Step { func (p *Plan) Execute() error { for i, step := range p.steps { if p.cfg.Debug { - fmt.Fprintf(os.Stderr, "calling %T.Execute (step %d)\n", step, i) + fmt.Fprintf(p.cfg.Stderr, "calling %T.Execute (step %d)\n", step, i) } if err := step.Execute(p.runCfg); err != nil { diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index 48ee4cc..ce5311a 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -4,7 +4,7 @@ import ( "fmt" "github.com/golang/mock/gomock" "github.com/stretchr/testify/suite" - "os" + "strings" "testing" "github.com/pelotech/drone-helm3/internal/run" @@ -29,6 +29,8 @@ func (suite *PlanTestSuite) TestNewPlan() { } defer func() { help = origHelp }() + stdout := strings.Builder{} + stderr := strings.Builder{} cfg := Config{ Command: "help", Debug: false, @@ -36,6 +38,8 @@ func (suite *PlanTestSuite) TestNewPlan() { StringValues: "tensile_strength,flexibility", ValuesFiles: []string{"/root/price_inventory.yml"}, Namespace: "outer", + Stdout: &stdout, + Stderr: &stderr, } runCfg := run.Config{ @@ -44,8 +48,8 @@ func (suite *PlanTestSuite) TestNewPlan() { StringValues: "tensile_strength,flexibility", ValuesFiles: []string{"/root/price_inventory.yml"}, Namespace: "outer", - Stdout: os.Stdout, - Stderr: os.Stderr, + Stdout: &stdout, + Stderr: &stderr, } stepOne.EXPECT().