From 5e2f2f3dc60a1788387815f0825fba9af55ee944 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 19 Dec 2019 14:53:53 -0800 Subject: [PATCH 01/28] First draft of a useful README [#8] --- README.md | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 69 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 04f55a3..aba89c8 100644 --- a/README.md +++ b/README.md @@ -1,3 +1,71 @@ # Drone plugin for Helm 3 -Dissatisfied with this empty README? Consider grabbing [the "put stuff in the README" issue](https://github.com/pelotech/drone-helm3/issues/8)! +This plugin provides an interface between [Drone](https://drone.io/) and [Helm 3](https://github.com/kubernetes/helm): + +* Lint your charts +* Deploy your service +* Delete your service + +The plugin is inpsired by [drone-helm](https://github.com/ipedrazas/drone-helm), which fills the same role for Helm 2. It provides a comparable feature-set and the configuration settings are backwards-compatible. + +## Example configuration + +These examples give a minimal and sufficient configuration for each use-case. For a full description of each case, see [docs/lint_example.yml](docs/lint_example.yml), [docs/upgrade_example.yml](docs/upgrade_example.yml), and [docs/delete_example.yml](docs/delete_example.yml). + +### Linting + +```yaml +steps: + - name: lint + image: pelotech/drone-helm3 + settings: + helm_command: lint + chart: ./ +``` + +### Deployment + +```yaml +steps: + - name: deploy + image: pelotech/drone-helm3 + settings: + helm_command: upgrade + chart: ./ + release: my-project + environment: + API_SERVER: https://my.kubernetes.installation/clusters/a-1234 + KUBERNETES_TOKEN: + from_secret: kubernetes_token +``` + +### Deletion + +```yaml +steps: + - name: delete + image: pelotech/drone-helm3 + settings: + helm_command: delete + release: my-project + environment: + API_SERVER: https://my.kubernetes.installation/clusters/a-1234 + KUBERNETES_TOKEN: + from_secret: kubernetes_token +``` + +## Upgrading from drone-helm + +The setting names for drone-helm3 are backwards-compatible with those for drone-helm, so the only mandatory step is to update the `image` clause so that drone uses the new plugin. + +There are several settings that no longer have any effect: + +* `purge` -- this is the default behavior in Helm 3 +* `recreate_pods` +* `tiller_ns` +* `upgrade` +* `canary_image` +* `client_only` +* `stable_repo_url` + +If your `.drone.yml` contains those settings, we recommend removing them. From 485eb4375c0b200af62ab0a7709134191f4f976e Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 19 Dec 2019 15:23:13 -0800 Subject: [PATCH 02/28] Rename "delete" to "uninstall" [#8] Helm 3 renamed the command, and I didn't realize it until just now. See also 161960e, where it was renamed in the code. --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index aba89c8..0ff3b73 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ The plugin is inpsired by [drone-helm](https://github.com/ipedrazas/drone-helm), ## Example configuration -These examples give a minimal and sufficient configuration for each use-case. For a full description of each case, see [docs/lint_example.yml](docs/lint_example.yml), [docs/upgrade_example.yml](docs/upgrade_example.yml), and [docs/delete_example.yml](docs/delete_example.yml). +These examples give a minimal and sufficient configuration for each use-case. For a full description of each case, see [docs/lint_example.yml](docs/lint_example.yml), [docs/upgrade_example.yml](docs/upgrade_example.yml), and [docs/uninstall_example.yml](docs/uninstall_example.yml). ### Linting @@ -43,10 +43,10 @@ steps: ```yaml steps: - - name: delete + - name: uninstall image: pelotech/drone-helm3 settings: - helm_command: delete + helm_command: uninstall release: my-project environment: API_SERVER: https://my.kubernetes.installation/clusters/a-1234 From 285af8a317e984320f24a6e88ebb5aac00e525a0 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 19 Dec 2019 16:37:04 -0800 Subject: [PATCH 03/28] Rough draft of an example lint stanza [#8] --- docs/lint_example.yml | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 docs/lint_example.yml diff --git a/docs/lint_example.yml b/docs/lint_example.yml new file mode 100644 index 0000000..677726b --- /dev/null +++ b/docs/lint_example.yml @@ -0,0 +1,36 @@ +--- +kind: pipeline +type: docker +name: default + +steps: + - name: lint + image: pelotech/drone-helm3 + settings: + # Helm_command must be set to "lint" in order to lint :) + helm_command: lint + + # Mandatory; must be a path to a chart directory. + chart: ./charts/bloge + + # Produce debug output from drone-helm itself and send --debug to all helm commands. + debug: true + + # The string given here will be passed verbatim to --set. + values: some=local,helm=values + + # The string given here will be passed verbatim to --set-string. + string_values: "notAnInt=5" + + # Each file will be passed to --values. Filenames must not contain commas. + values_files: "values/underrides.yml,values/overrides.yml" + + # Call `helm dependency update` before linting. + update_dependencies: true + + # Call `helm repo add` for each repo before linting. Repo names and urls must not contain commas. + repos: "eDeath=https://github.com/bug/e-death,idMaker=https://github.com/nmarks/id-maker" + + # Give the --namespace flag to all helm commands. + # TODO: I don't think this setting has any effect on linting; should it be documented? + namespace: my_kube_subset From 420014f9e5a89653eb69200dfe96e8fa935b5fb0 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 20 Dec 2019 09:41:36 -0800 Subject: [PATCH 04/28] Rename the setting description files to _settings [#8] --- README.md | 2 +- docs/{lint_example.yml => lint_settings.yml} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename docs/{lint_example.yml => lint_settings.yml} (100%) diff --git a/README.md b/README.md index 0ff3b73..c0505fb 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ The plugin is inpsired by [drone-helm](https://github.com/ipedrazas/drone-helm), ## Example configuration -These examples give a minimal and sufficient configuration for each use-case. For a full description of each case, see [docs/lint_example.yml](docs/lint_example.yml), [docs/upgrade_example.yml](docs/upgrade_example.yml), and [docs/uninstall_example.yml](docs/uninstall_example.yml). +The examples below give a minimal and sufficient configuration for each use-case. For a full description of each command's settings, see [docs/lint_settings.yml](docs/lint_settings.yml), [docs/upgrade_settings.yml](docs/upgrade_settings.yml), and [docs/uninstall_settings.yml](docs/uninstall_settings.yml). ### Linting diff --git a/docs/lint_example.yml b/docs/lint_settings.yml similarity index 100% rename from docs/lint_example.yml rename to docs/lint_settings.yml From aed59c251e708665dfcabd884897cd1774be0bb6 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 20 Dec 2019 09:56:51 -0800 Subject: [PATCH 05/28] Namespace is relevant in helm lint [#8] ...Or at least, the namespace is passed around in helm's linting code. I haven't proven that there's a case where omitting the namespace can cause a linting problem, but I've seen enough to go ahead and document the setting. --- docs/lint_settings.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/docs/lint_settings.yml b/docs/lint_settings.yml index 677726b..c47b30a 100644 --- a/docs/lint_settings.yml +++ b/docs/lint_settings.yml @@ -32,5 +32,4 @@ steps: repos: "eDeath=https://github.com/bug/e-death,idMaker=https://github.com/nmarks/id-maker" # Give the --namespace flag to all helm commands. - # TODO: I don't think this setting has any effect on linting; should it be documented? namespace: my_kube_subset From 197a377a8215c1ccd2c94d3e4c47c83feb7587af Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 20 Dec 2019 10:05:50 -0800 Subject: [PATCH 06/28] Prod maintainers to keep the docs and code in sync [#8] Offhand I don't see a way to ensure it programmatically, but I feel like I should at least make an attempt. --- internal/run/lint.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/run/lint.go b/internal/run/lint.go index e2843ca..1993b49 100644 --- a/internal/run/lint.go +++ b/internal/run/lint.go @@ -16,6 +16,8 @@ func (l *Lint) Execute(_ Config) error { } // Prepare gets the Lint ready to execute. +// Note: mandatory settings are documented in README.md, and the full list of settings is in docs/lint_settings.yml. +// Any additions or deletions here should be reflected there. func (l *Lint) Prepare(cfg Config) error { if l.Chart == "" { return fmt.Errorf("chart is required") From cab3a8ae95d7e9752d9daa870d4df99e96b85862 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 20 Dec 2019 10:37:32 -0800 Subject: [PATCH 07/28] Advise that some settings aren't yet functional [#8] --- docs/lint_settings.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/lint_settings.yml b/docs/lint_settings.yml index c47b30a..b309b6e 100644 --- a/docs/lint_settings.yml +++ b/docs/lint_settings.yml @@ -26,9 +26,11 @@ steps: values_files: "values/underrides.yml,values/overrides.yml" # Call `helm dependency update` before linting. + # Note: this setting is on the v1.0 roadmap, but not yet implemented. update_dependencies: true # Call `helm repo add` for each repo before linting. Repo names and urls must not contain commas. + # Note: this setting is on the v1.0 roadmap, but not yet implemented. repos: "eDeath=https://github.com/bug/e-death,idMaker=https://github.com/nmarks/id-maker" # Give the --namespace flag to all helm commands. From dc4ecb6b91a5322003b1ff554919362793c9c4db Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 20 Dec 2019 16:09:57 -0800 Subject: [PATCH 08/28] Allow an empty Certificate setting [#29] I just plain misunderstood how kubernetes CAs worked! --- internal/run/initkube.go | 3 --- internal/run/initkube_test.go | 7 ------- 2 files changed, 10 deletions(-) diff --git a/internal/run/initkube.go b/internal/run/initkube.go index 4af29af..a341dca 100644 --- a/internal/run/initkube.go +++ b/internal/run/initkube.go @@ -50,9 +50,6 @@ func (i *InitKube) Prepare(cfg Config) error { if i.Token == "" { return errors.New("token is needed to deploy") } - if i.Certificate == "" && !i.SkipTLSVerify { - return errors.New("certificate is needed to deploy") - } if i.ServiceAccount == "" { i.ServiceAccount = "helm" diff --git a/internal/run/initkube_test.go b/internal/run/initkube_test.go index fb32b15..09dc4df 100644 --- a/internal/run/initkube_test.go +++ b/internal/run/initkube_test.go @@ -134,13 +134,6 @@ func (suite *InitKubeTestSuite) TestPrepareRequiredConfig() { init.APIServer = "Sysadmin" init.Token = "" suite.Error(init.Prepare(cfg), "Token should be required.") - - init.Token = "Aspire virtual currency" - init.Certificate = "" - suite.Error(init.Prepare(cfg), "Certificate should be required.") - - init.SkipTLSVerify = true - suite.NoError(init.Prepare(cfg), "Certificate should not be required if SkipTLSVerify is true") } func (suite *InitKubeTestSuite) TestPrepareDefaultsServiceAccount() { From 044caebafd48ea6c8ad44c232f69e7d7fa0ecf17 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 20 Dec 2019 16:14:17 -0800 Subject: [PATCH 09/28] Omit empty CA data from the kubeconfig [#29] --- kubeconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kubeconfig b/kubeconfig index c7b2025..92ec2c7 100644 --- a/kubeconfig +++ b/kubeconfig @@ -3,7 +3,7 @@ clusters: - cluster: {{- if eq .SkipTLSVerify true }} insecure-skip-tls-verify: true -{{- else }} +{{- else if .Certificate }} certificate-authority-data: {{ .Certificate }} {{- end}} server: {{ .APIServer }} From 3eb90651d1f3f6b3b31f0b95fc343239cdaf16a0 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Fri, 20 Dec 2019 16:07:05 -0800 Subject: [PATCH 10/28] Rough-draft upgrade settings documentation [#8] --- docs/upgrade_settings.yml | 87 +++++++++++++++++++++++++++++++++++++++ internal/helm/plan.go | 1 + internal/run/upgrade.go | 2 + 3 files changed, 90 insertions(+) create mode 100644 docs/upgrade_settings.yml diff --git a/docs/upgrade_settings.yml b/docs/upgrade_settings.yml new file mode 100644 index 0000000..5531509 --- /dev/null +++ b/docs/upgrade_settings.yml @@ -0,0 +1,87 @@ +--- +kind: pipeline +type: docker +name: default + +steps: + - name: deploy + image: pelotech/drone-helm3 + settings: + # Setting helm_command to "upgrade" is recommended, but not mandatory. If no command is given, the plugin + # infers "upgrade" when triggered by a push, tag, deployment, pull_request, promote, or rollback event. + helm_command: upgrade + + # Mandatory. + # The chart to use for this release. + chart: ./charts/bloge + + # Mandatory. + # Release name for Helm to use. + release: bloge + + # Mandatory. + # API endpoint for the Kubernetes cluster. + api_server: https://k8s.mycompany.example.com/clusters/c-tr1sb + + # Mandatory. + # Token to use when connecting to kubernetes. + kubernetes_token: + from_secret: deploybot_token + + # Base-64 encoded TLS certificate used by the Kubernetes cluster's certificate authority. + kubernetes_certificate: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0t.... + + # Specific chart version to deploy. + chart_version: 1.2.3 + + # Simulate an upgrade without deploying it. + dry_run: true + + # Wait until kubernetes resources are in a ready state before marking the release successful. + wait: true + + # Timeout for any *individual* Kubernetes operation. The plugin's full runtime may exceed this duration. + timeout: 1m23s + + # Force resource updates (helm upgrade --force). + force: true + + # Values to set for this helm release. Keys and values must not contain commas. + values: + - image.tag=latest + - image.annotations.deployedDate="${DRONE_BUILD_CREATED}" + + # String values to set for this helm release. Keys and values must not contain commas. + string_values: "notAnInt=5" + + # Values files to use in this helm release. Filenames must not contain commas. + values_files: + - ./values/underrides.yml + - ./values/overrides.yml + + # Reuse the values from a previous release. + reuse_values: true + + # Put the kubernetes config file used for the deploy in an alternate location. + kube_config: /root/.spherernetes/config + + # Produce debug output from drone-helm itself and send --debug to all helm commands. + debug: true + + # Call `helm dependency update` before upgrading. + # Note: this setting is on the v1.0 roadmap, but not yet implemented. + update_dependencies: true + + # Call `helm repo add` for each repo before upgrading. Repo names and urls must not contain commas. + # Note: this setting is on the v1.0 roadmap, but not yet implemented. + repos: "eDeath=https://github.com/bug/e-death,idMaker=https://github.com/nmarks/id-maker" + + # Give the --namespace flag to all helm commands. + namespace: my_kube_subset + + # Connect insecurely to the kubernetes server. Using this setting in production is inadvisable. + skip_tls_verify: true + + # Service account to use when connecting to kubernetes. Defaults to "helm." + service_account: deploybot + from_secret: kubernetes_service_account diff --git a/internal/helm/plan.go b/internal/helm/plan.go index 1d4ced9..ffa3e52 100644 --- a/internal/helm/plan.go +++ b/internal/helm/plan.go @@ -67,6 +67,7 @@ func determineSteps(cfg Config) *func(Config) []Step { return &help default: switch cfg.DroneEvent { + // Note: These events are documented in docs/upgrade_settings.yml. Any changes here should be reflected there. case "push", "tag", "deployment", "pull_request", "promote", "rollback": return &upgrade case "delete": diff --git a/internal/run/upgrade.go b/internal/run/upgrade.go index cff2c70..9f57e05 100644 --- a/internal/run/upgrade.go +++ b/internal/run/upgrade.go @@ -25,6 +25,8 @@ func (u *Upgrade) Execute(_ Config) error { } // Prepare gets the Upgrade ready to execute. +// Note: mandatory settings are documented in README.md, and the full list of settings is in docs/upgrade_settings.yml. +// Any additions or deletions here should be reflected there. func (u *Upgrade) Prepare(cfg Config) error { if u.Chart == "" { return fmt.Errorf("chart is required") From 3d1c849e75fbcb738c085203b4423bb554809bd6 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 09:49:29 -0800 Subject: [PATCH 11/28] Don't document the kube_config setting [#8] See #30--there's no known use-case and no drone-helm users are using the setting, so it's on the chopping block. --- docs/upgrade_settings.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/docs/upgrade_settings.yml b/docs/upgrade_settings.yml index 5531509..90cd49c 100644 --- a/docs/upgrade_settings.yml +++ b/docs/upgrade_settings.yml @@ -62,9 +62,6 @@ steps: # Reuse the values from a previous release. reuse_values: true - # Put the kubernetes config file used for the deploy in an alternate location. - kube_config: /root/.spherernetes/config - # Produce debug output from drone-helm itself and send --debug to all helm commands. debug: true From 59a591eda512deedf1165ffc863a81d76f8af28a Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 09:57:05 -0800 Subject: [PATCH 12/28] Recommend removing tiller when upgrading [#8] --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c0505fb..3456f7d 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,11 @@ steps: The setting names for drone-helm3 are backwards-compatible with those for drone-helm, so the only mandatory step is to update the `image` clause so that drone uses the new plugin. -There are several settings that no longer have any effect: +There are some recommended changes, though: + +* If your `service_account` is currently `tiller`, change it to a service account with more restricted permissions. + * If possible, remove the tiller account entirely, since its superuser status presents a security risk. +* Remove outdated settings that have no effect in drone-helm3: * `purge` -- this is the default behavior in Helm 3 * `recreate_pods` @@ -67,5 +71,3 @@ There are several settings that no longer have any effect: * `canary_image` * `client_only` * `stable_repo_url` - -If your `.drone.yml` contains those settings, we recommend removing them. From 4755f502b5cde4cd2b6775f2343b0e712767d76b Mon Sep 17 00:00:00 2001 From: Erin Call Date: Mon, 23 Dec 2019 12:43:17 -0800 Subject: [PATCH 13/28] 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 14/28] 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 15/28] 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 16/28] 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 17/28] 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 18/28] 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 19/28] 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 20/28] 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 21/28] 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 22/28] 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 23/28] 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(). From ef66bc0f9210f458d3757d4fb22db0e7e5364b82 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 24 Dec 2019 14:36:39 -0800 Subject: [PATCH 24/28] Document parameters in a single markdown file [#8] I was unhappy with the comments-in-yaml approach; it required duplicating a lot of information and it was hard to find a balance between "usefully thorough" and "readably concise."" --- README.md | 2 +- docs/lint_settings.yml | 37 ---------- docs/parameter_reference.md | 133 ++++++++++++++++++++++++++++++++++++ docs/upgrade_settings.yml | 84 ----------------------- 4 files changed, 134 insertions(+), 122 deletions(-) delete mode 100644 docs/lint_settings.yml create mode 100644 docs/parameter_reference.md delete mode 100644 docs/upgrade_settings.yml diff --git a/README.md b/README.md index 3456f7d..b21ac90 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ The plugin is inpsired by [drone-helm](https://github.com/ipedrazas/drone-helm), ## Example configuration -The examples below give a minimal and sufficient configuration for each use-case. For a full description of each command's settings, see [docs/lint_settings.yml](docs/lint_settings.yml), [docs/upgrade_settings.yml](docs/upgrade_settings.yml), and [docs/uninstall_settings.yml](docs/uninstall_settings.yml). +The examples below give a minimal and sufficient configuration for each use-case. For a full description of each command's settings, see [docs/parameter_reference.md](docs/parameter_reference.md). ### Linting diff --git a/docs/lint_settings.yml b/docs/lint_settings.yml deleted file mode 100644 index b309b6e..0000000 --- a/docs/lint_settings.yml +++ /dev/null @@ -1,37 +0,0 @@ ---- -kind: pipeline -type: docker -name: default - -steps: - - name: lint - image: pelotech/drone-helm3 - settings: - # Helm_command must be set to "lint" in order to lint :) - helm_command: lint - - # Mandatory; must be a path to a chart directory. - chart: ./charts/bloge - - # Produce debug output from drone-helm itself and send --debug to all helm commands. - debug: true - - # The string given here will be passed verbatim to --set. - values: some=local,helm=values - - # The string given here will be passed verbatim to --set-string. - string_values: "notAnInt=5" - - # Each file will be passed to --values. Filenames must not contain commas. - values_files: "values/underrides.yml,values/overrides.yml" - - # Call `helm dependency update` before linting. - # Note: this setting is on the v1.0 roadmap, but not yet implemented. - update_dependencies: true - - # Call `helm repo add` for each repo before linting. Repo names and urls must not contain commas. - # Note: this setting is on the v1.0 roadmap, but not yet implemented. - repos: "eDeath=https://github.com/bug/e-death,idMaker=https://github.com/nmarks/id-maker" - - # Give the --namespace flag to all helm commands. - namespace: my_kube_subset diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md new file mode 100644 index 0000000..288bd53 --- /dev/null +++ b/docs/parameter_reference.md @@ -0,0 +1,133 @@ +# Parameter reference + +## Something here about how to specify non-string types + +* boolean +* list\ +* duration + +## Mention that everything can be settings or environment + +## Global +| Param name | Type | Purpose | +|---------------------|-----------------|---------| +| helm_command | string | Indicates the operation to perform. Recommended, but not required. Valid options are `upgrade`, `uninstall`, `lint`, and `help`. | +| update_dependencies | boolean | Calls `helm dependency update` before running the main command. **Not currently implemented**; see [#25](https://github.com/pelotech/drone-helm3/issues/25).| +| helm_repos | list\ | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. **Not currently implemented**; see [#26](https://github.com/pelotech/drone-helm3/issues/26). | +| namespace | string | Kubernetes namespace to use for this operation. | +| prefix | string | Expect environment variables to be prefixed with the given string. For more details, see "Using the prefix setting" below. **Not currently implemented**; see [#19](https://github.com/pelotech/drone-helm3/issues/19). | +| debug | boolean | Generate debug output within drone-helm3 and pass `--debug` to all helm commands. Use with care, since the debug output may include secrets. | + +## Linting + +Linting is only triggered when the `helm_command` setting is "lint". + +| Param name | Type | Required | Purpose | +|---------------|----------------|----------|---------| +| chart | string | yes | The chart to be linted. Must be a local path. | +| values | list\ | | Chart values to use as the `--set` argument to `helm lint`. | +| string_values | list\ | | Chart values to use as the `--set-string` argument to `helm lint`. | +| values_files | list\ | | Values to use as `--values` arguments to `helm lint`. | + +## Installation + +Installations are triggered when the `helm_command` setting is "upgrade." They can also be triggered when the build was triggered by a `push`, `tag`, `deployment`, `pull_request`, `promote`, or `rollback` Drone event. + +| Param name | Type | Required | Purpose | +|------------------------|----------------|----------|---------| +| chart | string | yes | The chart to use for this installation. | +| release | string | yes | The release name for helm to use. | +| api_server | string | yes | API endpoint for the Kubernetes cluster. | +| kubernetes_token | string | yes | Token for authenticating to Kubernetes. | +| service_account | string | | Service account for authenticating to Kubernetes. Default is `helm`. | +| kubernetes_certificate | string | | Base64 encoded TLS certificate used by the Kubernetes cluster's certificate authority. | +| chart_version | string | | Specific chart version to install. | +| dry_run | boolean | | Pass `--dry-run` to `helm upgrade`. | +| wait | boolean | | Wait until kubernetes resources are in a ready state before marking the installation successful. | +| timeout | duration | | Timeout for any *individual* Kubernetes operation. The installation's full runtime may exceed this duration. | +| force | boolean | | Pass `--force` to `helm upgrade`. | +| values | list\ | | Chart values to use as the `--set` argument to `helm upgrade`. | +| string_values | list\ | | Chart values to use as the `--set-string` argument to `helm upgrade`. | +| values_files | list\ | | Values to use as `--values` arguments to `helm upgrade`. | +| reuse_values | boolean | | Reuse the values from a previous release. | +| skip_tls_verify | boolean | | Connect to the Kubernetes cluster without checking for a valid TLS certificate. Not recommended in production. | + +## Uninstallation + +Uninstallations are triggered when the `helm_command` setting is "uninstall" or "delete." They can also be triggered when the build was triggered by a `delete` Drone event. + +| Param name | Type | Required | Purpose | +|------------------------|----------|----------|---------| +| release | string | yes | The release name for helm to use. | +| api_server | string | yes | API endpoint for the Kubernetes cluster. | +| kubernetes_token | string | yes | Token for authenticating to Kubernetes. | +| service_account | string | | Service account for authenticating to Kubernetes. Default is `helm`. | +| kubernetes_certificate | string | | Base64 encoded TLS certificate used by the Kubernetes cluster's certificate authority. | +| dry_run | boolean | | Pass `--dry-run` to `helm uninstall`. | +| timeout | duration | | Timeout for any *individual* Kubernetes operation. The uninstallation's full runtime may exceed this duration. | +| skip_tls_verify | boolean | | Connect to the Kubernetes cluster without checking for a valid TLS certificate. Not recommended in production. | + +### Formatting non-string values + +* Booleans can be yaml's `true` and `false` literals or the strings `"true"` and `"false"`. +* Durations are strings formatted with the syntax accepted by [golang's ParseDuration function](https://golang.org/pkg/time/#ParseDuration) (e.g. 5m30s) +* List\s can be a yaml sequence or a comma-separated string. + +All of the following are equivalent: + +```yaml +values: "foo=1,bar=2" +values: ["foo=1", "bar=2"] +values: + - foo=1 + - bar=2 +``` + +Note that **list members must not contain commas**. Both of the following are equivalent: + +```yaml +values_files: [ "./over_9,000.yml" ] +values_files: [ "./over_9", "000.yml" ] +``` + +### Using the `prefix` setting + +Because the prefix setting is meta-configuration, it has some inherent edge-cases. Here is what it does in the cases we've thought of: + +Unlike the other settings, it must be declared in the `settings` block, not `environment`. + +```yaml +settings: + prefix: helm # drone-helm3 will look for environment variables called HELM_VARNAME +environment: + prefix: armet # no effect +``` + +It does not apply to configuration in the `settings` block, only in `environment`. + +```yaml +settings: + prefix: helm + helm_timeout: 5m # no effect +environment: + helm_timeout: 2m # timeout will be 2 minutes +``` + +If the environment contains a variable in non-prefixed form, it will still be applied. + +```yaml +settings: + prefix: helm +environment: + timeout: 2m # timeout will be 2 minutes +``` + +If the environment contains both the prefixed and non-prefixed forms, drone-helm3 will use the prefixed form. + +```yaml +settings: + prefix: helm +environment: + timeout: 5m # overridden + helm_timeout: 2m # timeout will be 2 minutes +``` diff --git a/docs/upgrade_settings.yml b/docs/upgrade_settings.yml deleted file mode 100644 index 90cd49c..0000000 --- a/docs/upgrade_settings.yml +++ /dev/null @@ -1,84 +0,0 @@ ---- -kind: pipeline -type: docker -name: default - -steps: - - name: deploy - image: pelotech/drone-helm3 - settings: - # Setting helm_command to "upgrade" is recommended, but not mandatory. If no command is given, the plugin - # infers "upgrade" when triggered by a push, tag, deployment, pull_request, promote, or rollback event. - helm_command: upgrade - - # Mandatory. - # The chart to use for this release. - chart: ./charts/bloge - - # Mandatory. - # Release name for Helm to use. - release: bloge - - # Mandatory. - # API endpoint for the Kubernetes cluster. - api_server: https://k8s.mycompany.example.com/clusters/c-tr1sb - - # Mandatory. - # Token to use when connecting to kubernetes. - kubernetes_token: - from_secret: deploybot_token - - # Base-64 encoded TLS certificate used by the Kubernetes cluster's certificate authority. - kubernetes_certificate: LS0tLS1CRUdJTiBDRVJUSUZJQ0FURS0tLS0t.... - - # Specific chart version to deploy. - chart_version: 1.2.3 - - # Simulate an upgrade without deploying it. - dry_run: true - - # Wait until kubernetes resources are in a ready state before marking the release successful. - wait: true - - # Timeout for any *individual* Kubernetes operation. The plugin's full runtime may exceed this duration. - timeout: 1m23s - - # Force resource updates (helm upgrade --force). - force: true - - # Values to set for this helm release. Keys and values must not contain commas. - values: - - image.tag=latest - - image.annotations.deployedDate="${DRONE_BUILD_CREATED}" - - # String values to set for this helm release. Keys and values must not contain commas. - string_values: "notAnInt=5" - - # Values files to use in this helm release. Filenames must not contain commas. - values_files: - - ./values/underrides.yml - - ./values/overrides.yml - - # Reuse the values from a previous release. - reuse_values: true - - # Produce debug output from drone-helm itself and send --debug to all helm commands. - debug: true - - # Call `helm dependency update` before upgrading. - # Note: this setting is on the v1.0 roadmap, but not yet implemented. - update_dependencies: true - - # Call `helm repo add` for each repo before upgrading. Repo names and urls must not contain commas. - # Note: this setting is on the v1.0 roadmap, but not yet implemented. - repos: "eDeath=https://github.com/bug/e-death,idMaker=https://github.com/nmarks/id-maker" - - # Give the --namespace flag to all helm commands. - namespace: my_kube_subset - - # Connect insecurely to the kubernetes server. Using this setting in production is inadvisable. - skip_tls_verify: true - - # Service account to use when connecting to kubernetes. Defaults to "helm." - service_account: deploybot - from_secret: kubernetes_service_account From ff8e988122beb8b5c66aca163d2ad806307ce951 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 24 Dec 2019 15:22:25 -0800 Subject: [PATCH 25/28] Use "installation" rather than "deployment" [#8] "deploy" matches my mental model of what helm does, but "install" matches helm's own terminology more closely. --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b21ac90..023232a 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ steps: chart: ./ ``` -### Deployment +### Installation ```yaml steps: @@ -39,7 +39,7 @@ steps: from_secret: kubernetes_token ``` -### Deletion +### Uninstallation ```yaml steps: From d4506608d79d78ed4164c6953e167df23ca8395c Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 24 Dec 2019 15:25:44 -0800 Subject: [PATCH 26/28] Note a backwards-incompatibility in the README [#8] This probably isn't going to bite anyone, but it's technically possible, and it doesn't hurt to mention it. --- README.md | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 023232a..592baee 100644 --- a/README.md +++ b/README.md @@ -56,18 +56,16 @@ steps: ## Upgrading from drone-helm -The setting names for drone-helm3 are backwards-compatible with those for drone-helm, so the only mandatory step is to update the `image` clause so that drone uses the new plugin. +drone-helm3 is largely backwards-compatible with drone-helm. There are some known differences: -There are some recommended changes, though: +* `prefix` must be supplied via the `settings` block, not `environment`. +* Several settings no longer have any effect: + * `purge` -- this is the default behavior in Helm 3 + * `recreate_pods` + * `tiller_ns` + * `upgrade` + * `canary_image` + * `client_only` + * `stable_repo_url` -* If your `service_account` is currently `tiller`, change it to a service account with more restricted permissions. - * If possible, remove the tiller account entirely, since its superuser status presents a security risk. -* Remove outdated settings that have no effect in drone-helm3: - -* `purge` -- this is the default behavior in Helm 3 -* `recreate_pods` -* `tiller_ns` -* `upgrade` -* `canary_image` -* `client_only` -* `stable_repo_url` +Since helm 3 does not require Tiller, we also recommend switching to a service account with less-expansive permissions. From dc05855aa5c79fd530b1c064210ee9fc6b14fcb0 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 26 Dec 2019 09:41:10 -0800 Subject: [PATCH 27/28] Mention the `settings`/`environment` equivalency [#8] It seems like this needs more information, like why you'd want to put something in one stanza or the other, but I don't really know enough about drone to give useful advice. --- docs/parameter_reference.md | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 288bd53..ba492ed 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -1,13 +1,5 @@ # Parameter reference -## Something here about how to specify non-string types - -* boolean -* list\ -* duration - -## Mention that everything can be settings or environment - ## Global | Param name | Type | Purpose | |---------------------|-----------------|---------| @@ -67,6 +59,10 @@ Uninstallations are triggered when the `helm_command` setting is "uninstall" or | timeout | duration | | Timeout for any *individual* Kubernetes operation. The uninstallation's full runtime may exceed this duration. | | skip_tls_verify | boolean | | Connect to the Kubernetes cluster without checking for a valid TLS certificate. Not recommended in production. | +### Where to put settings + +Any setting (with the exception of `prefix`; [see below](#user-content-using-the-prefix-setting)), can go in either the `settings` or `environment` section. + ### Formatting non-string values * Booleans can be yaml's `true` and `false` literals or the strings `"true"` and `"false"`. From 568f613401cc67c91b62679b843eaa2fb74184a0 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 26 Dec 2019 09:44:46 -0800 Subject: [PATCH 28/28] Associate lines of text with their yaml blocks [#8] As I skimmed through that section I noticed it wasn't immediately clear whether a line of text was referring to the example above it or the one below it. --- docs/parameter_reference.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index ba492ed..07d8107 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -90,7 +90,7 @@ values_files: [ "./over_9", "000.yml" ] Because the prefix setting is meta-configuration, it has some inherent edge-cases. Here is what it does in the cases we've thought of: -Unlike the other settings, it must be declared in the `settings` block, not `environment`. +Unlike the other settings, it must be declared in the `settings` block, not `environment`: ```yaml settings: @@ -99,7 +99,7 @@ environment: prefix: armet # no effect ``` -It does not apply to configuration in the `settings` block, only in `environment`. +It does not apply to configuration in the `settings` block, only in `environment`: ```yaml settings: @@ -109,7 +109,7 @@ environment: helm_timeout: 2m # timeout will be 2 minutes ``` -If the environment contains a variable in non-prefixed form, it will still be applied. +If the environment contains a variable in non-prefixed form, it will still be applied: ```yaml settings: @@ -118,7 +118,7 @@ environment: timeout: 2m # timeout will be 2 minutes ``` -If the environment contains both the prefixed and non-prefixed forms, drone-helm3 will use the prefixed form. +If the environment contains both the prefixed and non-prefixed forms, drone-helm3 will use the prefixed form: ```yaml settings: