From 04de280821da2b1cabc7bb12212b434e9468d02a Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 7 Jan 2020 15:25:54 -0800 Subject: [PATCH 1/8] Rough draft of aliased settings [#66] --- internal/helm/config.go | 32 ++++++++++++++++++++++++++++++++ internal/helm/config_test.go | 29 +++++++++++++++++++++++++++++ 2 files changed, 61 insertions(+) diff --git a/internal/helm/config.go b/internal/helm/config.go index 1c8a393..2eda281 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -65,6 +65,29 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { return nil, err } + var aliases settingAliases + if err := envconfig.Process("plugin", &aliases); err != nil { + return nil, err + } + if aliases.Command != nil { + cfg.Command = *aliases.Command + } + if aliases.AddRepos != nil { + cfg.AddRepos = *aliases.AddRepos + } + if aliases.APIServer != nil { + cfg.APIServer = *aliases.APIServer + } + if aliases.ServiceAccount != nil { + cfg.ServiceAccount = *aliases.ServiceAccount + } + if aliases.Wait != nil { + cfg.Wait = *aliases.Wait + } + if aliases.Force != nil { + cfg.Force = *aliases.Force + } + if justNumbers.MatchString(cfg.Timeout) { cfg.Timeout = fmt.Sprintf("%ss", cfg.Timeout) } @@ -94,3 +117,12 @@ func (cfg *Config) deprecationWarn() { } } } + +type settingAliases struct { + Command *string `envconfig:"mode"` + AddRepos *[]string `envconfig:"add_repos"` + APIServer *string `envconfig:"kubernetes_api_server"` + ServiceAccount *string `envconfig:"kubernetes_service_account"` + Wait *bool `envconfig:"wait_for_upgrade"` + Force *bool `envconfig:"force_upgrade"` +} diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index 6cad789..cb4c3ee 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -70,6 +70,35 @@ func (suite *ConfigTestSuite) TestNewConfigInfersNumbersAreSeconds() { suite.Equal("42s", cfg.Timeout) } +func (suite *ConfigTestSuite) TestNewConfigWithAliases() { + for _, varname := range []string{ + "HELM_COMMAND", + "HELM_REPOS", + "API_SERVER", + "SERVICE_ACCOUNT", + "WAIT", + "FORCE", + } { + suite.unsetenv(varname) + suite.unsetenv("PLUGIN_" + varname) + } + suite.setenv("PLUGIN_MODE", "iambic") + suite.setenv("PLUGIN_ADD_REPOS", "chortle=http://calloo.callay/frabjous/day") + suite.setenv("PLUGIN_KUBERNETES_API_SERVER", "http://tumtum.tree") + suite.setenv("PLUGIN_KUBERNETES_SERVICE_ACCOUNT", "tulgey") + suite.setenv("PLUGIN_WAIT_FOR_UPGRADE", "true") + suite.setenv("PLUGIN_FORCE_UPGRADE", "true") + + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) + suite.Require().NoError(err) + suite.Equal("iambic", cfg.Command) + suite.Equal([]string{"chortle=http://calloo.callay/frabjous/day"}, cfg.AddRepos) + suite.Equal("http://tumtum.tree", cfg.APIServer) + suite.Equal("tulgey", cfg.ServiceAccount) + suite.True(cfg.Wait, "Wait should be aliased") + suite.True(cfg.Force, "Force should be aliased") +} + func (suite *ConfigTestSuite) TestNewConfigSetsWriters() { stdout := &strings.Builder{} stderr := &strings.Builder{} From 1d1117ba4981cace9fd76a6d9d6df83560e9d962 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 7 Jan 2020 15:39:05 -0800 Subject: [PATCH 2/8] Use "kube" in setting aliases [#66] Nobody likes typing "kubernetes"! Writing out that whole word without typos is the third hard problem in computer science. --- internal/helm/config.go | 12 ++++++++++-- internal/helm/config_test.go | 10 ++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/internal/helm/config.go b/internal/helm/config.go index 2eda281..3f5991c 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -87,6 +87,12 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { if aliases.Force != nil { cfg.Force = *aliases.Force } + if aliases.KubeToken != nil { + cfg.KubeToken = *aliases.KubeToken + } + if aliases.Certificate != nil { + cfg.Certificate = *aliases.Certificate + } if justNumbers.MatchString(cfg.Timeout) { cfg.Timeout = fmt.Sprintf("%ss", cfg.Timeout) @@ -121,8 +127,10 @@ func (cfg *Config) deprecationWarn() { type settingAliases struct { Command *string `envconfig:"mode"` AddRepos *[]string `envconfig:"add_repos"` - APIServer *string `envconfig:"kubernetes_api_server"` - ServiceAccount *string `envconfig:"kubernetes_service_account"` + APIServer *string `envconfig:"kube_api_server"` + ServiceAccount *string `envconfig:"kube_service_account"` Wait *bool `envconfig:"wait_for_upgrade"` Force *bool `envconfig:"force_upgrade"` + KubeToken *string `envconfig:"kube_token"` + Certificate *string `envconfig:"kube_certificate"` } diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index cb4c3ee..70ce91a 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -78,16 +78,20 @@ func (suite *ConfigTestSuite) TestNewConfigWithAliases() { "SERVICE_ACCOUNT", "WAIT", "FORCE", + "KUBERNETES_TOKEN", + "KUBERNETES_CERTIFICATE", } { suite.unsetenv(varname) suite.unsetenv("PLUGIN_" + varname) } suite.setenv("PLUGIN_MODE", "iambic") suite.setenv("PLUGIN_ADD_REPOS", "chortle=http://calloo.callay/frabjous/day") - suite.setenv("PLUGIN_KUBERNETES_API_SERVER", "http://tumtum.tree") - suite.setenv("PLUGIN_KUBERNETES_SERVICE_ACCOUNT", "tulgey") + suite.setenv("PLUGIN_KUBE_API_SERVER", "http://tumtum.tree") + suite.setenv("PLUGIN_KUBE_SERVICE_ACCOUNT", "tulgey") suite.setenv("PLUGIN_WAIT_FOR_UPGRADE", "true") suite.setenv("PLUGIN_FORCE_UPGRADE", "true") + suite.setenv("PLUGIN_KUBE_TOKEN", "Y29tZSB0byBteSBhcm1z") + suite.setenv("PLUGIN_KUBE_CERTIFICATE", "d2l0aCBpdHMgaGVhZA==") cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) @@ -97,6 +101,8 @@ func (suite *ConfigTestSuite) TestNewConfigWithAliases() { suite.Equal("tulgey", cfg.ServiceAccount) suite.True(cfg.Wait, "Wait should be aliased") suite.True(cfg.Force, "Force should be aliased") + suite.Equal("Y29tZSB0byBteSBhcm1z", cfg.KubeToken, "KubeToken should be aliased") + suite.Equal("d2l0aCBpdHMgaGVhZA==", cfg.Certificate, "Certificate should be aliased") } func (suite *ConfigTestSuite) TestNewConfigSetsWriters() { From 6aa1d79d56013c27e8a09fe4027cd762735b7bab Mon Sep 17 00:00:00 2001 From: Erin Call Date: Wed, 8 Jan 2020 09:31:22 -0800 Subject: [PATCH 3/8] Stabilize the logic for setting-alias conflicts [#66] This includes a refactor to the way aliases are processed. I had been thinking in terms of locking down the aliases names pretty tightly, in order to provide an error if there are conflicts. After discussion with @josmo, though, it seems like we can do it the same way we do for "PLUGIN_"/non-prefixed variables, i.e. quietly override them. --- internal/helm/config.go | 63 +++++++++++++++--------------------- internal/helm/config_test.go | 10 ++++++ 2 files changed, 36 insertions(+), 37 deletions(-) diff --git a/internal/helm/config.go b/internal/helm/config.go index 3f5991c..7c9a78f 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -53,7 +53,25 @@ type Config struct { // NewConfig creates a Config and reads environment variables into it, accounting for several possible formats. func NewConfig(stdout, stderr io.Writer) (*Config, error) { + var aliases settingAliases + if err := envconfig.Process("plugin", &aliases); err != nil { + return nil, err + } + + if err := envconfig.Process("", &aliases); err != nil { + return nil, err + } + cfg := Config{ + Command: aliases.Command, + AddRepos: aliases.AddRepos, + APIServer: aliases.APIServer, + ServiceAccount: aliases.ServiceAccount, + Wait: aliases.Wait, + Force: aliases.Force, + KubeToken: aliases.KubeToken, + Certificate: aliases.Certificate, + Stdout: stdout, Stderr: stderr, } @@ -65,35 +83,6 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { return nil, err } - var aliases settingAliases - if err := envconfig.Process("plugin", &aliases); err != nil { - return nil, err - } - if aliases.Command != nil { - cfg.Command = *aliases.Command - } - if aliases.AddRepos != nil { - cfg.AddRepos = *aliases.AddRepos - } - if aliases.APIServer != nil { - cfg.APIServer = *aliases.APIServer - } - if aliases.ServiceAccount != nil { - cfg.ServiceAccount = *aliases.ServiceAccount - } - if aliases.Wait != nil { - cfg.Wait = *aliases.Wait - } - if aliases.Force != nil { - cfg.Force = *aliases.Force - } - if aliases.KubeToken != nil { - cfg.KubeToken = *aliases.KubeToken - } - if aliases.Certificate != nil { - cfg.Certificate = *aliases.Certificate - } - if justNumbers.MatchString(cfg.Timeout) { cfg.Timeout = fmt.Sprintf("%ss", cfg.Timeout) } @@ -125,12 +114,12 @@ func (cfg *Config) deprecationWarn() { } type settingAliases struct { - Command *string `envconfig:"mode"` - AddRepos *[]string `envconfig:"add_repos"` - APIServer *string `envconfig:"kube_api_server"` - ServiceAccount *string `envconfig:"kube_service_account"` - Wait *bool `envconfig:"wait_for_upgrade"` - Force *bool `envconfig:"force_upgrade"` - KubeToken *string `envconfig:"kube_token"` - Certificate *string `envconfig:"kube_certificate"` + Command string `envconfig:"mode"` + AddRepos []string `envconfig:"add_repos"` + APIServer string `envconfig:"kube_api_server"` + ServiceAccount string `envconfig:"kube_service_account"` + Wait bool `envconfig:"wait_for_upgrade"` + Force bool `envconfig:"force_upgrade"` + KubeToken string `envconfig:"kube_token"` + Certificate string `envconfig:"kube_certificate"` } diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index 70ce91a..a59e3dd 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -105,6 +105,16 @@ func (suite *ConfigTestSuite) TestNewConfigWithAliases() { suite.Equal("d2l0aCBpdHMgaGVhZA==", cfg.Certificate, "Certificate should be aliased") } +func (suite *ConfigTestSuite) TestNewConfigWithAliasConflicts() { + suite.unsetenv("FORCE") + suite.setenv("PLUGIN_FORCE_UPGRADE", "true") + suite.setenv("PLUGIN_FORCE", "false") // should override even when set to the zero value + + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) + suite.NoError(err) + suite.False(cfg.Force, "official names should override alias names") +} + func (suite *ConfigTestSuite) TestNewConfigSetsWriters() { stdout := &strings.Builder{} stderr := &strings.Builder{} From a5342d7bace5c98d119851f8b845dbc2ba85d271 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Wed, 8 Jan 2020 10:45:53 -0800 Subject: [PATCH 4/8] Default to the canonical setting names [#66] The goal with these changes was to give users a clearer, more readable interface, so we should present that interface up front and only document the aliases as a backward-compatibility option. I've renamed the envconfig tags to reflect the switch, but I left the actual field names the same. I think they're sufficiently meaningful inside the code, and leaving them unchanged avoids making a bunch of churn in the rest of the code. --- README.md | 23 +++++++++----- docs/parameter_reference.md | 30 +++++++++--------- internal/helm/config.go | 32 +++++++++---------- internal/helm/config_test.go | 60 ++++++++++++++++++------------------ 4 files changed, 77 insertions(+), 68 deletions(-) diff --git a/README.md b/README.md index 6900962..13fe41e 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ steps: - name: lint image: pelotech/drone-helm3 settings: - helm_command: lint + mode: lint chart: ./ ``` @@ -34,12 +34,12 @@ steps: - name: deploy image: pelotech/drone-helm3 settings: - helm_command: upgrade + mode: upgrade chart: ./ release: my-project environment: - API_SERVER: https://my.kubernetes.installation/clusters/a-1234 - KUBERNETES_TOKEN: + KUBE_API_SERVER: https://my.kubernetes.installation/clusters/a-1234 + KUBE_TOKEN: from_secret: kubernetes_token ``` @@ -50,11 +50,11 @@ steps: - name: uninstall image: pelotech/drone-helm3 settings: - helm_command: uninstall + mode: uninstall release: my-project environment: - API_SERVER: https://my.kubernetes.installation/clusters/a-1234 - KUBERNETES_TOKEN: + KUBE_API_SERVER: https://my.kubernetes.installation/clusters/a-1234 + KUBE_TOKEN: from_secret: kubernetes_token ``` @@ -74,6 +74,15 @@ drone-helm3 is largely backwards-compatible with drone-helm. There are some know * `canary_image` * `client_only` * `stable_repo_url` +* Several settings have been renamed, to clarify their purpose and provide a more consistent naming scheme. For backward-compatibility, the old names are still available as aliases. If the old and new names are both present, the updated form takes priority. Conflicting settings will make your `.drone.yml` harder to understand, so we recommend updating to the new names: + * `helm_command` is now `mode` + ° `helm_repos` is now `add_repos` + * `api_server` is now `kube_api_server` + * `service_account` is now `kube_service_account` + * `kubernetes_token` is now `kube_token` + * `kubernetes_certificate` is now `kube_certificate` + * `wait` is now `wait_for_upgrade` + * `force` is now `force_upgrade` Since helm 3 does not require Tiller, we also recommend switching to a service account with less-expansive permissions. diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 6ef32a5..7b14db5 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -3,15 +3,15 @@ ## 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`. | +| mode | 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.| -| helm_repos | list\ | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. | +| add_repos | list\ | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. | | namespace | string | Kubernetes namespace to use for this operation. | | 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". +Linting is only triggered when the `mode` setting is "lint". | Param name | Type | Required | Purpose | |---------------|----------------|----------|---------| @@ -23,21 +23,21 @@ Linting is only triggered when the `helm_command` setting is "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. +Installations are triggered when the `mode` 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. | +| kube_api_server | string | yes | API endpoint for the Kubernetes cluster. | +| kube_token | string | yes | Token for authenticating to Kubernetes. | +| kube_service_account | string | | Service account for authenticating to Kubernetes. Default is `helm`. | +| kube_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. | +| wait_for_upgrade | 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`. | +| force_upgrade | boolean | | Pass `--force` to `helm upgrade`. | | atomic_upgrade | boolean | | Pass `--atomic` to `helm upgrade`. | | cleanup_failed_upgrade | boolean | | Pass `--cleanup-on-fail` to `helm upgrade`. | | values | list\ | | Chart values to use as the `--set` argument to `helm upgrade`. | @@ -48,15 +48,15 @@ Installations are triggered when the `helm_command` setting is "upgrade." They c ## 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. +Uninstallations are triggered when the `mode` 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. | +| kube_api_server | string | yes | API endpoint for the Kubernetes cluster. | +| kube_token | string | yes | Token for authenticating to Kubernetes. | +| kube_service_account | string | | Service account for authenticating to Kubernetes. Default is `helm`. | +| kube_certificate | string | | Base64 encoded TLS certificate used by the Kubernetes cluster's certificate authority. | | keep_history | boolean | | Pass `--keep-history` to `helm uninstall`, to retain the release history. | | 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. | diff --git a/internal/helm/config.go b/internal/helm/config.go index 7c9a78f..b633439 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -20,29 +20,29 @@ var ( // not have the `PLUGIN_` prefix. type Config struct { // Configuration for drone-helm itself - Command string `envconfig:"HELM_COMMAND"` // Helm command to run + Command string `envconfig:"mode"` // 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 - AddRepos []string `envconfig:"HELM_REPOS"` // Call `helm repo add` before the main command + AddRepos []string `split_words:"true"` // Call `helm repo add` before the main command 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 + KubeToken string `split_words:"true"` // 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 `split_words:"true"` // Account to use for connecting to the Kubernetes cluster + Certificate string `envconfig:"kube_certificate"` // The Kubernetes cluster CA's self-signed certificate (must be base64-encoded) + APIServer string `envconfig:"kube_api_server"` // The Kubernetes cluster's API endpoint + ServiceAccount string `envconfig:"kube_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 + Wait bool `envconfig:"wait_for_upgrade"` // Pass --wait to applicable helm commands ReuseValues bool `split_words:"true"` // Pass --reuse-values to `helm upgrade` KeepHistory bool `split_words:"true"` // Pass --keep-history to `helm uninstall` 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 + Force bool `envconfig:"force_upgrade"` // Pass --force to applicable helm commands AtomicUpgrade bool `split_words:"true"` // Pass --atomic to `helm upgrade` CleanupOnFail bool `envconfig:"CLEANUP_FAILED_UPGRADE"` // Pass --cleanup-on-fail to `helm upgrade` LintStrictly bool `split_words:"true"` // Pass --strict to `helm lint` @@ -114,12 +114,12 @@ func (cfg *Config) deprecationWarn() { } type settingAliases struct { - Command string `envconfig:"mode"` - AddRepos []string `envconfig:"add_repos"` - APIServer string `envconfig:"kube_api_server"` - ServiceAccount string `envconfig:"kube_service_account"` - Wait bool `envconfig:"wait_for_upgrade"` - Force bool `envconfig:"force_upgrade"` - KubeToken string `envconfig:"kube_token"` - Certificate string `envconfig:"kube_certificate"` + Command string `envconfig:"helm_command"` + AddRepos []string `envconfig:"helm_repos"` + APIServer string `envconfig:"api_server"` + ServiceAccount string `split_words:"true"` + Wait bool `` + Force bool `` + KubeToken string `envconfig:"kubernetes_token"` + Certificate string `envconfig:"kubernetes_certificate"` } diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index a59e3dd..4f4916b 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -20,47 +20,47 @@ func TestConfigTestSuite(t *testing.T) { } func (suite *ConfigTestSuite) TestNewConfigWithPluginPrefix() { - suite.unsetenv("HELM_COMMAND") + suite.unsetenv("MODE") suite.unsetenv("UPDATE_DEPENDENCIES") suite.unsetenv("DEBUG") - suite.setenv("PLUGIN_HELM_COMMAND", "execute order 66") + suite.setenv("PLUGIN_MODE", "iambic") suite.setenv("PLUGIN_UPDATE_DEPENDENCIES", "true") suite.setenv("PLUGIN_DEBUG", "true") cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) - suite.Equal("execute order 66", cfg.Command) + suite.Equal("iambic", cfg.Command) suite.True(cfg.UpdateDependencies) suite.True(cfg.Debug) } func (suite *ConfigTestSuite) TestNewConfigWithNoPrefix() { - suite.unsetenv("PLUGIN_HELM_COMMAND") + suite.unsetenv("PLUGIN_MODE") suite.unsetenv("PLUGIN_UPDATE_DEPENDENCIES") suite.unsetenv("PLUGIN_DEBUG") - suite.setenv("HELM_COMMAND", "execute order 66") + suite.setenv("MODE", "iambic") suite.setenv("UPDATE_DEPENDENCIES", "true") suite.setenv("DEBUG", "true") cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) - suite.Equal("execute order 66", cfg.Command) + suite.Equal("iambic", cfg.Command) suite.True(cfg.UpdateDependencies) suite.True(cfg.Debug) } 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` + suite.setenv("PLUGIN_MODE", "iambic") + suite.setenv("MODE", "haiku") // values from the `environment` block override those from `settings` cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) - suite.Equal("defend the jedi", cfg.Command) + suite.Equal("haiku", cfg.Command) } func (suite *ConfigTestSuite) TestNewConfigInfersNumbersAreSeconds() { @@ -72,30 +72,30 @@ func (suite *ConfigTestSuite) TestNewConfigInfersNumbersAreSeconds() { func (suite *ConfigTestSuite) TestNewConfigWithAliases() { for _, varname := range []string{ - "HELM_COMMAND", - "HELM_REPOS", - "API_SERVER", - "SERVICE_ACCOUNT", - "WAIT", - "FORCE", - "KUBERNETES_TOKEN", - "KUBERNETES_CERTIFICATE", + "MODE", + "ADD_REPOS", + "KUBE_API_SERVER", + "KUBE_SERVICE_ACCOUNT", + "WAIT_FOR_UPGRADE", + "FORCE_UPGRADE", + "KUBE_TOKEN", + "KUBE_CERTIFICATE", } { suite.unsetenv(varname) suite.unsetenv("PLUGIN_" + varname) } - suite.setenv("PLUGIN_MODE", "iambic") - suite.setenv("PLUGIN_ADD_REPOS", "chortle=http://calloo.callay/frabjous/day") - suite.setenv("PLUGIN_KUBE_API_SERVER", "http://tumtum.tree") - suite.setenv("PLUGIN_KUBE_SERVICE_ACCOUNT", "tulgey") - suite.setenv("PLUGIN_WAIT_FOR_UPGRADE", "true") - suite.setenv("PLUGIN_FORCE_UPGRADE", "true") - suite.setenv("PLUGIN_KUBE_TOKEN", "Y29tZSB0byBteSBhcm1z") - suite.setenv("PLUGIN_KUBE_CERTIFICATE", "d2l0aCBpdHMgaGVhZA==") + suite.setenv("PLUGIN_HELM_COMMAND", "beware the jabberwock") + suite.setenv("PLUGIN_HELM_REPOS", "chortle=http://calloo.callay/frabjous/day") + suite.setenv("PLUGIN_API_SERVER", "http://tumtum.tree") + suite.setenv("PLUGIN_SERVICE_ACCOUNT", "tulgey") + suite.setenv("PLUGIN_WAIT", "true") + suite.setenv("PLUGIN_FORCE", "true") + suite.setenv("PLUGIN_KUBERNETES_TOKEN", "Y29tZSB0byBteSBhcm1z") + suite.setenv("PLUGIN_KUBERNETES_CERTIFICATE", "d2l0aCBpdHMgaGVhZA==") cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.Require().NoError(err) - suite.Equal("iambic", cfg.Command) + suite.Equal("beware the jabberwock", cfg.Command) suite.Equal([]string{"chortle=http://calloo.callay/frabjous/day"}, cfg.AddRepos) suite.Equal("http://tumtum.tree", cfg.APIServer) suite.Equal("tulgey", cfg.ServiceAccount) @@ -106,9 +106,9 @@ func (suite *ConfigTestSuite) TestNewConfigWithAliases() { } func (suite *ConfigTestSuite) TestNewConfigWithAliasConflicts() { - suite.unsetenv("FORCE") - suite.setenv("PLUGIN_FORCE_UPGRADE", "true") - suite.setenv("PLUGIN_FORCE", "false") // should override even when set to the zero value + suite.unsetenv("FORCE_UPGRADE") + suite.setenv("PLUGIN_FORCE", "true") + suite.setenv("PLUGIN_FORCE_UPGRADE", "false") // should override even when set to the zero value cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) suite.NoError(err) @@ -145,7 +145,7 @@ func (suite *ConfigTestSuite) TestDeprecatedSettingWarnings() { func (suite *ConfigTestSuite) TestLogDebug() { suite.setenv("DEBUG", "true") - suite.setenv("HELM_COMMAND", "upgrade") + suite.setenv("MODE", "upgrade") stderr := strings.Builder{} stdout := strings.Builder{} From cfd8e3399560c53dff03797a2b431d0b152fd0d4 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Wed, 8 Jan 2020 11:04:30 -0800 Subject: [PATCH 5/8] Use "backward-compatible", not "backwardS" While writing docs in the previous commit, I noticed that we'd been inconsistent in the naming scheme. Wikipedia's back-compat article redirects from "backwards" to "backward", so I figure that's a reasonable source of authority for which form to use. --- README.md | 4 ++-- internal/helm/plan_test.go | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 13fe41e..27e63e4 100644 --- a/README.md +++ b/README.md @@ -10,7 +10,7 @@ This plugin provides an interface between [Drone](https://drone.io/) and [Helm 3 * 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. +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 backward-compatible. ## Example configuration @@ -60,7 +60,7 @@ steps: ## Upgrading from drone-helm -drone-helm3 is largely backwards-compatible with drone-helm. There are some known differences: +drone-helm3 is largely backward-compatible with drone-helm. There are some known differences: * You'll need to migrate the deployments in the cluster [helm-v2-to-helm-v3](https://helm.sh/blog/migrate-from-helm-v2-to-helm-v3/). * EKS is not supported. See [#5](https://github.com/pelotech/drone-helm3/issues/5) for more information. diff --git a/internal/helm/plan_test.go b/internal/helm/plan_test.go index f6cf9bb..1bc3e11 100644 --- a/internal/helm/plan_test.go +++ b/internal/helm/plan_test.go @@ -370,7 +370,7 @@ func (suite *PlanTestSuite) TestDeterminePlanUninstallCommand() { suite.Same(&uninstall, stepsMaker) } -// helm_command = delete is provided as an alias for backwards-compatibility with drone-helm +// helm_command = delete is provided as an alias for backward-compatibility with drone-helm func (suite *PlanTestSuite) TestDeterminePlanDeleteCommand() { cfg := Config{ Command: "delete", From 71421fbaa5965ebecaf30d44c59d42535fca6757 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Wed, 8 Jan 2020 11:08:39 -0800 Subject: [PATCH 6/8] Fix godotenv dependency in go.mod go.mod got an update when I ran the tests. It should've happened in 51058470e; I'm not sure why it was never updated. --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index e3fc56f..3d45e7f 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.13 require ( github.com/golang/mock v1.3.1 - github.com/joho/godotenv v1.3.0 // indirect + github.com/joho/godotenv v1.3.0 github.com/kelseyhightower/envconfig v1.4.0 github.com/stretchr/testify v1.4.0 golang.org/x/lint v0.0.0-20191125180803-fdd1cda4f05f // indirect From a826f66425fb996a4fac47a9266de9a72d55c5b3 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Wed, 8 Jan 2020 12:37:34 -0800 Subject: [PATCH 7/8] Test settings aliases without the plugin_ prefix [#66] --- internal/helm/config_test.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index 4f4916b..13bf22a 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -105,6 +105,17 @@ func (suite *ConfigTestSuite) TestNewConfigWithAliases() { suite.Equal("d2l0aCBpdHMgaGVhZA==", cfg.Certificate, "Certificate should be aliased") } +func (suite *ConfigTestSuite) TestAliasedSettingWithoutPluginPrefix() { + suite.unsetenv("FORCE_UPGRADE") + suite.unsetenv("PLUGIN_FORCE_UPGRADE") + suite.unsetenv("PLUGIN_FORCE") + suite.setenv("FORCE", "true") + + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) + suite.Require().NoError(err) + suite.True(cfg.Force) +} + func (suite *ConfigTestSuite) TestNewConfigWithAliasConflicts() { suite.unsetenv("FORCE_UPGRADE") suite.setenv("PLUGIN_FORCE", "true") From 3d1a2227dabdd11dc2e70018847612bc034aedd4 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Thu, 9 Jan 2020 10:20:20 -0800 Subject: [PATCH 8/8] Mention aliased settings in parameter_reference [#66] --- docs/parameter_reference.md | 93 +++++++++++++++++++++---------------- 1 file changed, 54 insertions(+), 39 deletions(-) diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 7b14db5..1379ba5 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -1,13 +1,13 @@ # Parameter reference ## Global -| Param name | Type | Purpose | -|---------------------|-----------------|---------| -| mode | 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.| -| add_repos | list\ | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. | -| namespace | string | Kubernetes namespace to use for this operation. | -| 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. | +| Param name | Type | Alias | Purpose | +|---------------------|-----------------|--------------|---------| +| mode | string | helm_command | 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.| +| add_repos | list\ | helm_repos | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. | +| namespace | string | | Kubernetes namespace to use for this operation. | +| 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 @@ -25,43 +25,43 @@ Linting is only triggered when the `mode` setting is "lint". Installations are triggered when the `mode` 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. | -| kube_api_server | string | yes | API endpoint for the Kubernetes cluster. | -| kube_token | string | yes | Token for authenticating to Kubernetes. | -| kube_service_account | string | | Service account for authenticating to Kubernetes. Default is `helm`. | -| kube_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_for_upgrade | 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_upgrade | boolean | | Pass `--force` to `helm upgrade`. | -| atomic_upgrade | boolean | | Pass `--atomic` to `helm upgrade`. | -| cleanup_failed_upgrade | boolean | | Pass `--cleanup-on-fail` 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. | +| Param name | Type | Required | Alias | Purpose | +|------------------------|----------------|----------|------------------------|---------| +| chart | string | yes | | The chart to use for this installation. | +| release | string | yes | | The release name for helm to use. | +| kube_api_server | string | yes | api_server | API endpoint for the Kubernetes cluster. | +| kube_token | string | yes | kubernetes_token | Token for authenticating to Kubernetes. | +| kube_service_account | string | | service_account | Service account for authenticating to Kubernetes. Default is `helm`. | +| kube_certificate | string | | kubernetes_certificate | 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_for_upgrade | boolean | | wait | 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_upgrade | boolean | | force | Pass `--force` to `helm upgrade`. | +| atomic_upgrade | boolean | | | Pass `--atomic` to `helm upgrade`. | +| cleanup_failed_upgrade | boolean | | | Pass `--cleanup-on-fail` 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 `mode` 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. | -| kube_api_server | string | yes | API endpoint for the Kubernetes cluster. | -| kube_token | string | yes | Token for authenticating to Kubernetes. | -| kube_service_account | string | | Service account for authenticating to Kubernetes. Default is `helm`. | -| kube_certificate | string | | Base64 encoded TLS certificate used by the Kubernetes cluster's certificate authority. | -| keep_history | boolean | | Pass `--keep-history` to `helm uninstall`, to retain the release history. | -| 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. | -| chart | string | | Required when the global `update_dependencies` parameter is true. No effect otherwise. | +| Param name | Type | Required | Alias | Purpose | +|------------------------|----------|----------|------------------------|---------| +| release | string | yes | | The release name for helm to use. | +| kube_api_server | string | yes | api_server | API endpoint for the Kubernetes cluster. | +| kube_token | string | yes | kubernetes_token | Token for authenticating to Kubernetes. | +| kube_service_account | string | | service_account | Service account for authenticating to Kubernetes. Default is `helm`. | +| kube_certificate | string | | kubernetes_certificate | Base64 encoded TLS certificate used by the Kubernetes cluster's certificate authority. | +| keep_history | boolean | | | Pass `--keep-history` to `helm uninstall`, to retain the release history. | +| 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. | +| chart | string | | | Required when the global `update_dependencies` parameter is true. No effect otherwise. | ### Where to put settings @@ -92,3 +92,18 @@ Note that **list members must not contain commas**. Both of the following are eq values_files: [ "./over_9,000.yml" ] values_files: [ "./over_9", "000.yml" ] ``` + +### Backward-compatibility aliases + +Some settings have alternate names, for backward-compatibility with drone-helm. We recommend using the canonical name unless you require the backward-compatible form. + +| Canonical name | Alias | +|----------------------|-------| +| mode | helm_command | +| add_repos | helm_repos | +| kube_api_server | api_server | +| kube_service_account | service_account | +| kube_token | kubernetes_token | +| kube_certificate | kubernetes_certificate | +| wait_for_upgrade | wait | +| force_upgrade | force |