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.
2020-01-08 10:45:53 -08:00 · 2020-01-08 10:45:53 -08:00 · a5342d7bac
parent 6aa1d79d56
commit a5342d7bac
4 changed files with 77 additions and 68 deletions
--- 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.

--- 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\<string\>  | Calls `helm repo add $repo` before running the main command. Each string should be formatted as `repo_name=https://repo.url/`. |
+| add_repos           | list\<string\>  | 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\<string\> |          | 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. |
--- 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"`
 }
--- 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{}