From e843b267590b7959cbf88134594279fcfaf3c407 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 21 Jan 2020 15:37:59 -0800 Subject: [PATCH 1/4] Expand env vars in Values/StringValues [#34] --- docs/parameter_reference.md | 20 ++++++++++++++++++++ internal/env/config.go | 20 ++++++++++++++++++++ internal/env/config_test.go | 15 +++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/docs/parameter_reference.md b/docs/parameter_reference.md index 244460f..3073f06 100644 --- a/docs/parameter_reference.md +++ b/docs/parameter_reference.md @@ -95,6 +95,26 @@ values_files: [ "./over_9,000.yml" ] values_files: [ "./over_9", "000.yml" ] ``` +### Interpolating secrets into the `values` and `string_values` settings + +If you want to send secrets to your charts, you can use syntax similar to shell variable interpolation--either `$VARNAME` or `$${VARNAME}`. The double dollar-sign is necessary when using curly brackets; using curly brackets with a single dollar-sign will trigger Drone's string substitution (which can't use arbitrary environment variables). If an environment variable is not set, it will be treated as if it were set to the empty string. + +```yaml +environment: + DB_PASSWORD: + from_secret: db_password + SESSION_KEY: + from_secret: session_key +settings: + values: + - db_password=$DB_PASSWORD # db_password will be set to the contents of the db_password secret + - db_pass=$DB_PASS # db_pass will be set to "" since $DB_PASS is not set + - session_key=$${SESSION_KEY} # session_key will be set to the contents of the session_key secret + - sess_key=${SESSION_KEY} # sess_key will be set to "" by Drone's variable substitution +``` + +Variables intended for interpolation must be set in the `environment` section, not `settings`. + ### 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. diff --git a/internal/env/config.go b/internal/env/config.go index 6c4f43c..cae2a7e 100644 --- a/internal/env/config.go +++ b/internal/env/config.go @@ -93,11 +93,31 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { cfg.logDebug() } + cfg.loadValuesSecrets() + cfg.deprecationWarn() return &cfg, nil } +func (cfg *Config) loadValuesSecrets() { + findVar := regexp.MustCompile(`\$\{?(\w+)\}?`) + + replacer := func(varName string) string { + sigils := regexp.MustCompile(`[${}]`) + varName = sigils.ReplaceAllString(varName, "") + + if value, ok := os.LookupEnv(varName); ok { + return value + } + + return "" + } + + cfg.Values = findVar.ReplaceAllStringFunc(cfg.Values, replacer) + cfg.StringValues = findVar.ReplaceAllStringFunc(cfg.StringValues, replacer) +} + func (cfg Config) logDebug() { if cfg.KubeToken != "" { cfg.KubeToken = "(redacted)" diff --git a/internal/env/config_test.go b/internal/env/config_test.go index 40123dd..cf1d7c6 100644 --- a/internal/env/config_test.go +++ b/internal/env/config_test.go @@ -183,6 +183,21 @@ func (suite *ConfigTestSuite) TestLogDebugCensorsKubeToken() { suite.Equal(kubeToken, cfg.KubeToken) // The actual config value should be left unchanged } +func (suite *ConfigTestSuite) TestNewConfigWithValuesSecrets() { + suite.unsetenv("VALUES") + suite.unsetenv("STRING_VALUES") + suite.setenv("SECRET_FIRE", "Eru_Ilúvatar") + suite.setenv("SECRET_RINGS", "1") + suite.setenv("PLUGIN_VALUES", "fire=$SECRET_FIRE,water=${SECRET_WATER}") + suite.setenv("PLUGIN_STRING_VALUES", "rings=${SECRET_RINGS}") + + cfg, err := NewConfig(&strings.Builder{}, &strings.Builder{}) + suite.Require().NoError(err) + + suite.Equal("fire=Eru_Ilúvatar,water=", cfg.Values) + suite.Equal("rings=1", cfg.StringValues) +} + func (suite *ConfigTestSuite) setenv(key, val string) { orig, ok := os.LookupEnv(key) if ok { From 8f7b481934a5675f66e77456460dffcfda6844ea Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 21 Jan 2020 16:04:05 -0800 Subject: [PATCH 2/4] Log debug information in loadValuesSecrets [#34] --- internal/env/config.go | 12 ++++++++++++ internal/env/config_test.go | 21 +++++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/internal/env/config.go b/internal/env/config.go index cae2a7e..71f6392 100644 --- a/internal/env/config.go +++ b/internal/env/config.go @@ -108,13 +108,25 @@ func (cfg *Config) loadValuesSecrets() { varName = sigils.ReplaceAllString(varName, "") if value, ok := os.LookupEnv(varName); ok { + if cfg.Debug { + fmt.Fprintf(cfg.Stderr, "Replaced $%s with value in environment\n", varName) + } return value } + if cfg.Debug { + fmt.Fprintf(cfg.Stderr, "$%s not present in environment, replaced with \"\"\n", varName) + } return "" } + if cfg.Debug { + fmt.Fprintf(cfg.Stderr, "Replacing environment variable references in Values\n") + } cfg.Values = findVar.ReplaceAllStringFunc(cfg.Values, replacer) + if cfg.Debug { + fmt.Fprintf(cfg.Stderr, "Replacing environment variable references in StringValues\n") + } cfg.StringValues = findVar.ReplaceAllStringFunc(cfg.StringValues, replacer) } diff --git a/internal/env/config_test.go b/internal/env/config_test.go index cf1d7c6..a17fffa 100644 --- a/internal/env/config_test.go +++ b/internal/env/config_test.go @@ -198,6 +198,27 @@ func (suite *ConfigTestSuite) TestNewConfigWithValuesSecrets() { suite.Equal("rings=1", cfg.StringValues) } +func (suite *ConfigTestSuite) TestValuesSecretsWithDebugLogging() { + suite.unsetenv("VALUES") + suite.setenv("SECRET_FIRE", "Eru_Ilúvatar") + suite.setenv("PLUGIN_DEBUG", "true") + suite.setenv("PLUGIN_STRING_VALUES", "fire=$SECRET_FIRE") + suite.setenv("PLUGIN_VALUES", "fire=$SECRET_FIRE,water=$SECRET_WATER") + stderr := strings.Builder{} + _, err := NewConfig(&strings.Builder{}, &stderr) + suite.Require().NoError(err) + + // Make a good-faith effort to avoid putting secrets in the log output, but still mention they were found + suite.Contains(stderr.String(), "Values:fire=$SECRET_FIRE,water=$SECRET_WATER") + suite.Contains(stderr.String(), ` +Replacing environment variable references in Values +Replaced $SECRET_FIRE with value in environment +$SECRET_WATER not present in environment, replaced with "" +Replacing environment variable references in StringValues +Replaced $SECRET_FIRE with value in environment +`) +} + func (suite *ConfigTestSuite) setenv(key, val string) { orig, ok := os.LookupEnv(key) if ok { From 22aa1df894c1f6a3476d42c13b78755b1fc592ec Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 21 Jan 2020 16:23:55 -0800 Subject: [PATCH 3/4] Don't bother trying to hide secrets in values [#34] While testing this I discovered the secrets are revealed anyway, since the lint/upgrade jobs' debug output includes the command they generated. Might as well make the code a little simpler. --- internal/env/config.go | 13 ++----------- internal/env/config_test.go | 11 ++--------- 2 files changed, 4 insertions(+), 20 deletions(-) diff --git a/internal/env/config.go b/internal/env/config.go index 71f6392..68e49f3 100644 --- a/internal/env/config.go +++ b/internal/env/config.go @@ -89,12 +89,12 @@ func NewConfig(stdout, stderr io.Writer) (*Config, error) { cfg.Timeout = fmt.Sprintf("%ss", cfg.Timeout) } + cfg.loadValuesSecrets() + if cfg.Debug && cfg.Stderr != nil { cfg.logDebug() } - cfg.loadValuesSecrets() - cfg.deprecationWarn() return &cfg, nil @@ -108,9 +108,6 @@ func (cfg *Config) loadValuesSecrets() { varName = sigils.ReplaceAllString(varName, "") if value, ok := os.LookupEnv(varName); ok { - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "Replaced $%s with value in environment\n", varName) - } return value } @@ -120,13 +117,7 @@ func (cfg *Config) loadValuesSecrets() { return "" } - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "Replacing environment variable references in Values\n") - } cfg.Values = findVar.ReplaceAllStringFunc(cfg.Values, replacer) - if cfg.Debug { - fmt.Fprintf(cfg.Stderr, "Replacing environment variable references in StringValues\n") - } cfg.StringValues = findVar.ReplaceAllStringFunc(cfg.StringValues, replacer) } diff --git a/internal/env/config_test.go b/internal/env/config_test.go index a17fffa..4288342 100644 --- a/internal/env/config_test.go +++ b/internal/env/config_test.go @@ -208,15 +208,8 @@ func (suite *ConfigTestSuite) TestValuesSecretsWithDebugLogging() { _, err := NewConfig(&strings.Builder{}, &stderr) suite.Require().NoError(err) - // Make a good-faith effort to avoid putting secrets in the log output, but still mention they were found - suite.Contains(stderr.String(), "Values:fire=$SECRET_FIRE,water=$SECRET_WATER") - suite.Contains(stderr.String(), ` -Replacing environment variable references in Values -Replaced $SECRET_FIRE with value in environment -$SECRET_WATER not present in environment, replaced with "" -Replacing environment variable references in StringValues -Replaced $SECRET_FIRE with value in environment -`) + suite.Contains(stderr.String(), "Values:fire=Eru_Ilúvatar,water=") + suite.Contains(stderr.String(), `$SECRET_WATER not present in environment, replaced with ""`) } func (suite *ConfigTestSuite) setenv(key, val string) { From dbcef2699e026b3aabc97b418234d83fd0425985 Mon Sep 17 00:00:00 2001 From: Erin Call Date: Tue, 21 Jan 2020 16:25:58 -0800 Subject: [PATCH 4/4] Avoid polluted-env problems in config tests [#34] I mean...it's *possible* someone will have SECRET_WATER set in their env, right? Might as well be paranoid; it doesn't cost much. --- internal/env/config_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/env/config_test.go b/internal/env/config_test.go index 4288342..15510e0 100644 --- a/internal/env/config_test.go +++ b/internal/env/config_test.go @@ -186,6 +186,7 @@ func (suite *ConfigTestSuite) TestLogDebugCensorsKubeToken() { func (suite *ConfigTestSuite) TestNewConfigWithValuesSecrets() { suite.unsetenv("VALUES") suite.unsetenv("STRING_VALUES") + suite.unsetenv("SECRET_WATER") suite.setenv("SECRET_FIRE", "Eru_Ilúvatar") suite.setenv("SECRET_RINGS", "1") suite.setenv("PLUGIN_VALUES", "fire=$SECRET_FIRE,water=${SECRET_WATER}") @@ -200,6 +201,7 @@ func (suite *ConfigTestSuite) TestNewConfigWithValuesSecrets() { func (suite *ConfigTestSuite) TestValuesSecretsWithDebugLogging() { suite.unsetenv("VALUES") + suite.unsetenv("SECRET_WATER") suite.setenv("SECRET_FIRE", "Eru_Ilúvatar") suite.setenv("PLUGIN_DEBUG", "true") suite.setenv("PLUGIN_STRING_VALUES", "fire=$SECRET_FIRE")