diff --git a/docs/index.md b/docs/index.md index 87efad75..218d9d10 100644 --- a/docs/index.md +++ b/docs/index.md @@ -593,6 +593,8 @@ Helmfile uses some OS environment variables to override default behaviour: * `HELMFILE_FILE_PATH` - specify the path to the helmfile.yaml file * `HELMFILE_INTERACTIVE` - enable interactive mode, expecting `true` lower case. The same as `--interactive` CLI flag * `HELMFILE_RENDER_YAML` - force helmfile.yaml to be rendered as a Go template regardless of file extension, expecting `true` lower case. Useful for migrating from v0 to v1 without renaming files to `.gotmpl` +* `HELMFILE_AWS_SDK_LOG_LEVEL` - configure AWS SDK logging level for vals library. Valid values: `off` (default, secure, case-insensitive), `minimal`, `standard`, `verbose`, or custom comma-separated values like `request,response`. See issue #2270 for details +* `HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP` - enable strict mode for vals secret references. When set to `true` (or any value accepted by Go's `strconv.ParseBool` like `TRUE`, `1`), vals will fail when a referenced key does not exist in the secret map. Invalid values will cause an error when vals is initialized (when secret refs are first evaluated). Default is `false` (when unset or empty) for backward compatibility. See issue #1563 for details ## CLI Reference diff --git a/pkg/envvar/const.go b/pkg/envvar/const.go index b5df7100..cd4d52bc 100644 --- a/pkg/envvar/const.go +++ b/pkg/envvar/const.go @@ -28,4 +28,10 @@ const ( // Can be overridden by AWS_SDK_GO_LOG_LEVEL environment variable // See issue #2270 and vals PR #893 AWSSDKLogLevel = "HELMFILE_AWS_SDK_LOG_LEVEL" + + // ValsFailOnMissingKeyInMap controls whether vals should fail when a key is missing in a map. + // When set to "true", vals returns an error if a referenced key does not exist in the secret map. + // Default is false for backward compatibility (returns empty string for missing keys). + // See issue #1563 + ValsFailOnMissingKeyInMap = "HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP" ) diff --git a/pkg/plugins/vals.go b/pkg/plugins/vals.go index 8c65ceaf..f0c2767f 100644 --- a/pkg/plugins/vals.go +++ b/pkg/plugins/vals.go @@ -1,8 +1,10 @@ package plugins import ( + "fmt" "io" "os" + "strconv" "strings" "sync" @@ -17,46 +19,82 @@ const ( ) var instance *vals.Runtime -var once sync.Once +var mu sync.Mutex + +func buildValsOptions() (vals.Options, error) { + // Configure AWS SDK logging via HELMFILE_AWS_SDK_LOG_LEVEL environment variable + // Default: "off" to prevent sensitive information (tokens, auth headers) from being exposed + // See issue #2270 and vals PR helmfile/vals#893 + // + // Valid values: + // - "off" (default): No AWS SDK logging - secure, prevents credential leakage + // - "minimal": Log retries only - minimal debugging info + // - "standard": Log retries + requests - moderate debugging (previous default) + // - "verbose": Log everything - full debugging (requests, responses, bodies, signing) + // - Custom: Comma-separated values like "request,response" + // + // Note: AWS_SDK_GO_LOG_LEVEL environment variable always takes precedence over this setting + // Note: Case-insensitive for known values like "off", "OFF", "Off" + logLevel := strings.TrimSpace(os.Getenv(envvar.AWSSDKLogLevel)) + + // Configure fail on missing key behavior + // Default to false for backward compatibility + // Set HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP=true to enable strict mode + // Supports common boolean values: "true", "TRUE", "1", etc. + // See issue #1563 + envVal := strings.TrimSpace(os.Getenv(envvar.ValsFailOnMissingKeyInMap)) + var failOnMissingKey bool + if envVal != "" { + var err error + failOnMissingKey, err = strconv.ParseBool(envVal) + if err != nil { + return vals.Options{}, fmt.Errorf("invalid value for %s: %q (must be a valid boolean)", envvar.ValsFailOnMissingKeyInMap, envVal) + } + } + + // Default to "off" for security if not specified + if logLevel == "" { + logLevel = "off" + } + + // Normalize known values to lowercase for case-insensitive handling + if strings.EqualFold(logLevel, "off") { + logLevel = "off" + } + + opts := vals.Options{ + CacheSize: valsCacheSize, + FailOnMissingKeyInMap: failOnMissingKey, + AWSLogLevel: logLevel, + } + + // Also suppress vals' own internal logging unless user wants verbose output + // This prevents vals' log messages (separate from AWS SDK logs) from exposing credentials + if logLevel == "off" { + opts.LogOutput = io.Discard + } + // For other levels, allow vals to log to default output for debugging + + return opts, nil +} func ValsInstance() (*vals.Runtime, error) { - var err error - once.Do(func() { - // Configure AWS SDK logging via HELMFILE_AWS_SDK_LOG_LEVEL environment variable - // Default: "off" to prevent sensitive information (tokens, auth headers) from being exposed - // See issue #2270 and vals PR helmfile/vals#893 - // - // Valid values: - // - "off" (default): No AWS SDK logging - secure, prevents credential leakage - // - "minimal": Log retries only - minimal debugging info - // - "standard": Log retries + requests - moderate debugging (previous default) - // - "verbose": Log everything - full debugging (requests, responses, bodies, signing) - // - Custom: Comma-separated values like "request,response" - // - // Note: AWS_SDK_GO_LOG_LEVEL environment variable always takes precedence over this setting - logLevel := strings.TrimSpace(os.Getenv(envvar.AWSSDKLogLevel)) + mu.Lock() + defer mu.Unlock() - opts := vals.Options{ - CacheSize: valsCacheSize, - } + if instance != nil { + return instance, nil + } - // Default to "off" for security if not specified - if logLevel == "" { - logLevel = "off" - } + opts, err := buildValsOptions() + if err != nil { + return nil, err + } - // Set AWS SDK log level for vals library - opts.AWSLogLevel = logLevel + instance, err = vals.New(opts) + if err != nil { + return nil, err + } - // Also suppress vals' own internal logging unless user wants verbose output - // This prevents vals' log messages (separate from AWS SDK logs) from exposing credentials - if logLevel == "off" { - opts.LogOutput = io.Discard - } - // For other levels, allow vals to log to default output for debugging - - instance, err = vals.New(opts) - }) - - return instance, err + return instance, nil } diff --git a/pkg/plugins/vals_test.go b/pkg/plugins/vals_test.go index 31d71d40..2b98489e 100644 --- a/pkg/plugins/vals_test.go +++ b/pkg/plugins/vals_test.go @@ -2,11 +2,11 @@ package plugins import ( "io" - "os" - "strings" "testing" "github.com/helmfile/vals" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/helmfile/helmfile/pkg/envvar" ) @@ -25,19 +25,165 @@ func TestValsInstance(t *testing.T) { } } -// TestAWSSDKLogLevelConfiguration tests the AWS SDK log level configuration logic +func TestBuildValsOptions(t *testing.T) { + tests := []struct { + name string + awsLogLevel string + failOnMissingKey string + expectedLogLevel string + expectedFailOnMissingKey bool + expectedLogOutputDiscarded bool + expectError bool + }{ + { + name: "defaults", + awsLogLevel: "", + failOnMissingKey: "", + expectedLogLevel: "off", + expectedFailOnMissingKey: false, + expectedLogOutputDiscarded: true, + }, + { + name: "explicit failOnMissingKey true", + awsLogLevel: "", + failOnMissingKey: "true", + expectedLogLevel: "off", + expectedFailOnMissingKey: true, + expectedLogOutputDiscarded: true, + }, + { + name: "failOnMissingKey false", + awsLogLevel: "", + failOnMissingKey: "false", + expectedLogLevel: "off", + expectedFailOnMissingKey: false, + expectedLogOutputDiscarded: true, + }, + { + name: "failOnMissingKey with whitespace", + awsLogLevel: "", + failOnMissingKey: " true ", + expectedLogLevel: "off", + expectedFailOnMissingKey: true, + expectedLogOutputDiscarded: true, + }, + { + name: "failOnMissingKey uppercase TRUE", + awsLogLevel: "", + failOnMissingKey: "TRUE", + expectedLogLevel: "off", + expectedFailOnMissingKey: true, + expectedLogOutputDiscarded: true, + }, + { + name: "failOnMissingKey numeric 1", + awsLogLevel: "", + failOnMissingKey: "1", + expectedLogLevel: "off", + expectedFailOnMissingKey: true, + expectedLogOutputDiscarded: true, + }, + { + name: "failOnMissingKey numeric 0", + awsLogLevel: "", + failOnMissingKey: "0", + expectedLogLevel: "off", + expectedFailOnMissingKey: false, + expectedLogOutputDiscarded: true, + }, + { + name: "failOnMissingKey invalid value", + awsLogLevel: "", + failOnMissingKey: "invalid", + expectError: true, + }, + { + name: "aws log level verbose", + awsLogLevel: "verbose", + failOnMissingKey: "", + expectedLogLevel: "verbose", + expectedFailOnMissingKey: false, + expectedLogOutputDiscarded: false, + }, + { + name: "aws log level with whitespace", + awsLogLevel: " minimal ", + failOnMissingKey: "", + expectedLogLevel: "minimal", + expectedFailOnMissingKey: false, + expectedLogOutputDiscarded: false, + }, + { + name: "aws log level OFF uppercase", + awsLogLevel: "OFF", + failOnMissingKey: "", + expectedLogLevel: "off", + expectedFailOnMissingKey: false, + expectedLogOutputDiscarded: true, + }, + { + name: "aws log level Off mixed case", + awsLogLevel: "Off", + failOnMissingKey: "", + expectedLogLevel: "off", + expectedFailOnMissingKey: false, + expectedLogOutputDiscarded: true, + }, + { + name: "aws log level Off mixed case", + awsLogLevel: "Off", + failOnMissingKey: "", + expectedLogLevel: "off", + expectedFailOnMissingKey: false, + expectedLogOutputDiscarded: true, + }, + { + name: "both options set", + awsLogLevel: "standard", + failOnMissingKey: "true", + expectedLogLevel: "standard", + expectedFailOnMissingKey: true, + expectedLogOutputDiscarded: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Setenv(envvar.AWSSDKLogLevel, tt.awsLogLevel) + t.Setenv(envvar.ValsFailOnMissingKeyInMap, tt.failOnMissingKey) + + opts, err := buildValsOptions() + + if tt.expectError { + require.Error(t, err) + assert.Contains(t, err.Error(), envvar.ValsFailOnMissingKeyInMap) + return + } + + require.NoError(t, err) + + assert.Equal(t, tt.expectedLogLevel, opts.AWSLogLevel) + assert.Equal(t, tt.expectedFailOnMissingKey, opts.FailOnMissingKeyInMap) + assert.Equal(t, valsCacheSize, opts.CacheSize) + + isDiscarded := opts.LogOutput == io.Discard + assert.Equal(t, tt.expectedLogOutputDiscarded, isDiscarded) + }) + } +} + func TestAWSSDKLogLevelConfiguration(t *testing.T) { tests := []struct { name string envValue string expectedLogLevel string - expectedLogOutput bool // true if LogOutput should be io.Discard + expectedLogOutput bool }{ { name: "no env var defaults to off", envValue: "", expectedLogLevel: "off", - expectedLogOutput: true, // LogOutput should be io.Discard + expectedLogOutput: true, }, { name: "explicit off", @@ -45,11 +191,17 @@ func TestAWSSDKLogLevelConfiguration(t *testing.T) { expectedLogLevel: "off", expectedLogOutput: true, }, + { + name: "OFF uppercase", + envValue: "OFF", + expectedLogLevel: "off", + expectedLogOutput: true, + }, { name: "minimal logging", envValue: "minimal", expectedLogLevel: "minimal", - expectedLogOutput: false, // LogOutput should NOT be io.Discard + expectedLogOutput: false, }, { name: "standard logging", @@ -73,94 +225,34 @@ func TestAWSSDKLogLevelConfiguration(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Note: This test verifies the configuration logic, not the actual vals.New() call - // since ValsInstance() uses sync.Once and can only be initialized once per test run. + t.Setenv(envvar.AWSSDKLogLevel, tt.envValue) - // Simulate the logic from ValsInstance() - var logLevel string - if tt.envValue != "" { - logLevel = strings.TrimSpace(tt.envValue) - } + opts, err := buildValsOptions() + require.NoError(t, err) - // Default to "off" for security if not specified - if logLevel == "" { - logLevel = "off" - } + assert.Equal(t, tt.expectedLogLevel, opts.AWSLogLevel) - // Verify expected log level - if logLevel != tt.expectedLogLevel { - t.Errorf("Expected log level %q, got %q", tt.expectedLogLevel, logLevel) - } - - // Verify LogOutput configuration logic - opts := vals.Options{ - CacheSize: valsCacheSize, - } - opts.AWSLogLevel = logLevel - - // Verify LogOutput is set to io.Discard only when level is "off" - if tt.expectedLogOutput { - opts.LogOutput = io.Discard - if opts.LogOutput != io.Discard { - t.Error("Expected LogOutput to be io.Discard for 'off' level") - } - } + isDiscarded := opts.LogOutput == io.Discard + assert.Equal(t, tt.expectedLogOutput, isDiscarded) }) } } -// TestEnvironmentVariableReading verifies that the HELMFILE_AWS_SDK_LOG_LEVEL env var is read correctly -func TestEnvironmentVariableReading(t *testing.T) { - tests := []struct { - name string - envValue string - expectedValue string - }{ - { - name: "empty defaults to off", - envValue: "", - expectedValue: "off", - }, - { - name: "whitespace trimmed", - envValue: " minimal ", - expectedValue: "minimal", - }, - { - name: "standard value preserved", - envValue: "standard", - expectedValue: "standard", - }, - } +func TestBuildValsOptionsIntegration(t *testing.T) { + t.Run("valid configuration produces working vals options", func(t *testing.T) { + t.Setenv(envvar.AWSSDKLogLevel, "off") + t.Setenv(envvar.ValsFailOnMissingKeyInMap, "true") - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Save and restore env var - original := os.Getenv(envvar.AWSSDKLogLevel) - defer func() { - if original == "" { - os.Unsetenv(envvar.AWSSDKLogLevel) - } else { - os.Setenv(envvar.AWSSDKLogLevel, original) - } - }() + opts, err := buildValsOptions() + require.NoError(t, err) - // Set test env var - if tt.envValue == "" { - os.Unsetenv(envvar.AWSSDKLogLevel) - } else { - os.Setenv(envvar.AWSSDKLogLevel, tt.envValue) - } + assert.Equal(t, valsCacheSize, opts.CacheSize) + assert.Equal(t, "off", opts.AWSLogLevel) + assert.True(t, opts.FailOnMissingKeyInMap) + assert.Equal(t, io.Discard, opts.LogOutput) - // Read and process like ValsInstance() does - logLevel := strings.TrimSpace(os.Getenv(envvar.AWSSDKLogLevel)) - if logLevel == "" { - logLevel = "off" - } - - if logLevel != tt.expectedValue { - t.Errorf("Expected %q, got %q", tt.expectedValue, logLevel) - } - }) - } + rt, err := vals.New(opts) + require.NoError(t, err) + assert.NotNil(t, rt) + }) }