From ab7abb699a88439d85771f7c5a8944fd1b389df3 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 31 Dec 2019 09:28:42 -0800 Subject: [PATCH 1/8] Remove support for the prefix setting [#48] The setting isn't necessary with modern versions of Drone, and it creates a lot of edge-cases. The use-case doesn't justify the added complexity. --- README.md | 2 +- docs/parameter_reference.md | 45 +----------------------------------- internal/helm/config.go | 11 +-------- internal/helm/config_test.go | 44 ----------------------------------- 4 files changed, 3 insertions(+), 99 deletions(-) diff --git a/README.md b/README.md index 592baee..ffb565b 100644 --- a/README.md +++ b/README.md @@ -58,7 +58,7 @@ steps: drone-helm3 is largely backwards-compatible with drone-helm. There are some known differences: -* `prefix` must be supplied via the `settings` block, not `environment`. +* The `prefix` setting is no longer supported. If you were relying on the `prefix` setting with `secrets: [...]`, you'll need to switch to the `from_secret` syntax. * Several settings no longer have any effect: * `purge` -- this is the default behavior in Helm 3 * `recreate_pods` diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 52f3dd7..9575d41 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -7,7 +7,6 @@ | 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/`. | | 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. | | 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 @@ -62,7 +61,7 @@ Uninstallations are triggered when the `helm_command` setting is "uninstall" or ### 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. +Any setting can go in either the `settings` or `environment` section. ### Formatting non-string values @@ -87,45 +86,3 @@ 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" ] ``` - -### 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/internal/helm/config.go b/internal/helm/config.go index a4d1914..3406c1f 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -12,14 +12,13 @@ var justNumbers = regexp.MustCompile(`^\d+$`) // 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 uppercased, but does -// not have the `PLUGIN_` prefix. It may, however, be prefixed with the value in `$PLUGIN_PREFIX`. +// not have the `PLUGIN_` prefix. type Config struct { // Configuration for drone-helm itself 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 AddRepos []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 @@ -53,18 +52,10 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { return nil, err } - prefix := cfg.Prefix - if err := envconfig.Process("", &cfg); err != nil { return nil, err } - if prefix != "" { - if err := envconfig.Process(cfg.Prefix, &cfg); err != nil { - return nil, err - } - } - if justNumbers.MatchString(cfg.Timeout) { cfg.Timeout = fmt.Sprintf("%ss", cfg.Timeout) } diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index f39dd0c..40ddfd6 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -19,7 +19,6 @@ func TestConfigTestSuite(t *testing.T) { } func (suite *ConfigTestSuite) TestNewConfigWithPluginPrefix() { - suite.unsetenv("PLUGIN_PREFIX") suite.unsetenv("HELM_COMMAND") suite.unsetenv("UPDATE_DEPENDENCIES") suite.unsetenv("DEBUG") @@ -37,7 +36,6 @@ func (suite *ConfigTestSuite) TestNewConfigWithPluginPrefix() { } func (suite *ConfigTestSuite) TestNewConfigWithNoPrefix() { - suite.unsetenv("PLUGIN_PREFIX") suite.unsetenv("PLUGIN_HELM_COMMAND") suite.unsetenv("PLUGIN_UPDATE_DEPENDENCIES") suite.unsetenv("PLUGIN_DEBUG") @@ -54,56 +52,14 @@ func (suite *ConfigTestSuite) TestNewConfigWithNoPrefix() { suite.True(cfg.Debug) } -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, err := NewConfig(&strings.Builder{}, &strings.Builder{}) - suite.Require().NoError(err) - - 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, err := NewConfig(&strings.Builder{}, &strings.Builder{}) - suite.Require().NoError(err) - - 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, err := NewConfig(&strings.Builder{}, &strings.Builder{}) - suite.Require().NoError(err) - - suite.Equal("gimme more", cfg.Command) -} - 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_PREFIX", "prod") - suite.setenv("TIMEOUT", "5m0s") - suite.setenv("PROD_TIMEOUT", "2m30s") // values from prefixed env vars override those from non-prefixed ones - 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) TestNewConfigInfersNumbersAreSeconds() { From 7cd46bb8b19e15971abd2202de3a3ec8ef584462 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 31 Dec 2019 10:03:53 -0800 Subject: [PATCH 2/8] Emit warnings about deprecated settings [#10] These aren't an error case--the plugin will work just fine--but users should be aware they (the settings) aren't being respected. --- README.md | 2 +- internal/helm/config.go | 19 ++++++++++++++++++- internal/helm/config_test.go | 19 +++++++++++++++++++ 3 files changed, 38 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 592baee..1ad4c9e 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ steps: drone-helm3 is largely backwards-compatible with drone-helm. There are some known differences: * `prefix` must be supplied via the `settings` block, not `environment`. -* Several settings no longer have any effect: +* Several settings no longer have any effect. The plugin will produce warnings if any of these are present: * `purge` -- this is the default behavior in Helm 3 * `recreate_pods` * `tiller_ns` diff --git a/internal/helm/config.go b/internal/helm/config.go index a4d1914..f2086d8 100644 --- a/internal/helm/config.go +++ b/internal/helm/config.go @@ -4,10 +4,15 @@ import ( "fmt" "github.com/kelseyhightower/envconfig" "io" + "os" "regexp" + "strings" ) -var justNumbers = regexp.MustCompile(`^\d+$`) +var ( + justNumbers = regexp.MustCompile(`^\d+$`) + deprecatedVars = []string{"PURGE", "RECREATE_PODS", "TILLER_NS", "UPGRADE", "CANARY_IMAGE", "CLIENT_ONLY", "STABLE_REPO_URL"} +) // 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 @@ -73,6 +78,8 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { cfg.logDebug() } + cfg.deprecationWarn() + return &cfg, nil } @@ -82,3 +89,13 @@ func (cfg Config) logDebug() { } fmt.Fprintf(cfg.Stderr, "Generated config: %+v\n", cfg) } + +func (cfg *Config) deprecationWarn() { + for _, varname := range deprecatedVars { + _, barePresent := os.LookupEnv(varname) + _, prefixedPresent := os.LookupEnv("PLUGIN_" + varname) + if barePresent || prefixedPresent { + fmt.Fprintf(cfg.Stderr, "Warning: ignoring deprecated '%s' setting\n", strings.ToLower(varname)) + } + } +} diff --git a/internal/helm/config_test.go b/internal/helm/config_test.go index f39dd0c..849fbd5 100644 --- a/internal/helm/config_test.go +++ b/internal/helm/config_test.go @@ -1,6 +1,7 @@ package helm import ( + "fmt" "github.com/stretchr/testify/suite" "os" "strings" @@ -123,6 +124,24 @@ func (suite *ConfigTestSuite) TestNewConfigSetsWriters() { suite.Equal(stderr, cfg.Stderr) } +func (suite *ConfigTestSuite) TestDeprecatedSettingWarnings() { + for _, varname := range deprecatedVars { + suite.setenv(varname, "deprecoat") // environment-block entries should cause warnings + } + + suite.unsetenv("PURGE") + suite.setenv("PLUGIN_PURGE", "true") // settings-block entries should cause warnings + suite.setenv("UPGRADE", "") // entries should cause warnings even when set to empty string + + stderr := &strings.Builder{} + _, err := NewConfig(&strings.Builder{}, stderr) + suite.NoError(err) + + for _, varname := range deprecatedVars { + suite.Contains(stderr.String(), fmt.Sprintf("Warning: ignoring deprecated '%s' setting\n", strings.ToLower(varname))) + } +} + func (suite *ConfigTestSuite) TestLogDebug() { suite.setenv("DEBUG", "true") suite.setenv("HELM_COMMAND", "upgrade") From 680989754a9b42b20e0c2d63f3b3ccc66bf64999 Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Tue, 31 Dec 2019 14:19:56 -0500 Subject: [PATCH 3/8] Directions on how to setup custom build of images till PR gets merged --- .gitignore | 2 ++ README.md | 8 ++++++++ 2 files changed, 10 insertions(+) diff --git a/.gitignore b/.gitignore index feb6e6e..7205b04 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,8 @@ *.so *.dylib +.idea + # Test binary, built with `go test -c` *.test diff --git a/README.md b/README.md index 592baee..85a2d79 100644 --- a/README.md +++ b/README.md @@ -69,3 +69,11 @@ drone-helm3 is largely backwards-compatible with drone-helm. There are some know * `stable_repo_url` Since helm 3 does not require Tiller, we also recommend switching to a service account with less-expansive permissions. + +### Contribution + +This repo is setup in a way that if you enable a personal drone server to build your fork it will + build and publish your image (makes it easier to test PRs and use the image till the contributions get merged) + +* Build local ```DRONE_REPO_OWNER=josmo DRONE_REPO_NAME=drone-ecs drone exec``` +* on your server (or cloud.drone.io) just make sure you have DOCKER_USERNAME, DOCKER_PASSWORD, and PLUGIN_REPO set as secrets \ No newline at end of file From fe7ee09350efa59ed04ca710961e92c10ec07295 Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Tue, 31 Dec 2019 14:48:21 -0500 Subject: [PATCH 4/8] add link to contributing --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 85a2d79..cf21bc2 100644 --- a/README.md +++ b/README.md @@ -70,7 +70,7 @@ drone-helm3 is largely backwards-compatible with drone-helm. There are some know Since helm 3 does not require Tiller, we also recommend switching to a service account with less-expansive permissions. -### Contribution +### [Contributing](docs/contributing.md) This repo is setup in a way that if you enable a personal drone server to build your fork it will build and publish your image (makes it easier to test PRs and use the image till the contributions get merged) From 2389268fa2347b34cf31616efac580b74f6bd4bd Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Tue, 31 Dec 2019 17:24:12 -0500 Subject: [PATCH 5/8] Adding basic badges --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 592baee..bf9e260 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,9 @@ # Drone plugin for Helm 3 +[![Build Status](https://cloud.drone.io/api/badges/pelotech/drone-helm3/status.svg)](https://cloud.drone.io/pelotech/drone-helm3) +[![Go Report](https://goreportcard.com/badge/github.com/pelotech/drone-helm3)](https://goreportcard.com/report/github.com/pelotech/drone-helm3) +[![](https://images.microbadger.com/badges/image/pelotech/drone-helm3.svg)](https://microbadger.com/images/pelotech/drone-helm3 "Get your own image badge on microbadger.com") + This plugin provides an interface between [Drone](https://drone.io/) and [Helm 3](https://github.com/kubernetes/helm): * Lint your charts From e6411027701481a9bad716b7cd9b6d80abefdfb9 Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Tue, 31 Dec 2019 17:34:53 -0500 Subject: [PATCH 6/8] link to migrate the deployments in the cluster --- README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/README.md b/README.md index bf9e260..c987b8e 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,8 @@ steps: drone-helm3 is largely backwards-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/) + * `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 From fed4de2ed9f764b459b84e662960a05a95ff9f9b Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Tue, 31 Dec 2019 17:54:46 -0500 Subject: [PATCH 7/8] Update README.md Co-Authored-By: Erin Call --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index c987b8e..6981db0 100644 --- a/README.md +++ b/README.md @@ -62,7 +62,7 @@ steps: drone-helm3 is largely backwards-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/) +* 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/). * `prefix` must be supplied via the `settings` block, not `environment`. * Several settings no longer have any effect: From 3fa2d71559acfdde880be779d42bb8f89590b723 Mon Sep 17 00:00:00 2001 From: Joachim Hill-Grannec Date: Tue, 31 Dec 2019 17:57:32 -0500 Subject: [PATCH 8/8] pin version of docker image to alpine/helm:3.0.2 --- Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dockerfile b/Dockerfile index c8fa2ae..18f29a8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,4 +1,4 @@ -FROM alpine/helm +FROM alpine/helm:3.0.2 MAINTAINER Erin Call COPY build/drone-helm /bin/drone-helm