fix: error on missing secret key when using vals by yxxhero · Pull Request #2496 · helmfile/helmfile

yxxhero · 2026-03-22T01:15:49Z

Summary

Add HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP environment variable to control whether vals should fail when a referenced key does not exist in the secret map.

Previously, when a secret reference like ref+vault://path#/nonexistent-key pointed to a non-existent key, vals would silently return an empty string without error. This could lead to deployments with missing configuration.

Changes

Add HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP environment variable in pkg/envvar/const.go
Update vals initialization to read this config and set FailOnMissingKeyInMap option
Add test cases for the new configuration

Usage

Default behavior (backward compatible):

helmfile apply  # Missing key returns empty string

Strict mode:

export HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP=true
helmfile apply  # Missing key causes error

Fixes #1563

Copilot

Pull request overview

Adds an opt-in strict mode for vals secret-map lookups so Helmfile can fail fast when a ref+...#/missing-key reference points to a non-existent key, addressing silent empty-string rendering reported in #1563.

Changes:

Introduces HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP env var to control strict missing-key behavior.
Wires the env var into vals.Options.FailOnMissingKeyInMap during ValsInstance() initialization.
Adds unit tests covering env var parsing expectations.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
pkg/envvar/const.go	Adds env var constant (and doc) for controlling strict missing-key handling in vals.
pkg/plugins/vals.go	Reads the env var and passes `FailOnMissingKeyInMap` into vals runtime options.
pkg/plugins/vals_test.go	Adds a new table-driven test for parsing the strict-mode env var.

Copilot · 2026-03-22T01:18:57Z

+func TestFailOnMissingKeyInMap(t *testing.T) {
+	tests := []struct {
+		name     string
+		envValue string
+		expected bool
+	}{


This test only re-implements the env var parsing (TrimSpace(... ) == "true") and doesn’t verify that ValsInstance() actually wires the value into vals.Options.FailOnMissingKeyInMap or changes missing-key behavior. Consider resetting the package-level once/instance in the test (same package) and asserting the resulting runtime behavior, or factoring the env parsing into a helper that ValsInstance() calls and testing that helper to avoid drift.

- Extract buildValsOptions() to make vals configuration testable - Use t.Setenv instead of manual env save/restore in tests - Test actual vals.Options output including FailOnMissingKeyInMap Addresses PR review comments on #2496 Signed-off-by: yxxhero <[email protected]>

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

Copilot · 2026-03-22T02:43:46Z

 			if tt.envValue != "" {
-				logLevel = strings.TrimSpace(tt.envValue)
+				t.Setenv(envvar.AWSSDKLogLevel, tt.envValue)
 			}



TestAWSSDKLogLevelConfiguration doesn't clear HELMFILE_AWS_SDK_LOG_LEVEL for the empty envValue case (it just doesn't call t.Setenv). This makes the test depend on the caller's environment and can fail when that env var is set. Consider always setting it (including to "").

Copilot · 2026-03-22T02:43:47Z

+	// 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
+	// See issue #1563
+	failOnMissingKey := strings.TrimSpace(os.Getenv(envvar.ValsFailOnMissingKeyInMap)) == "true"
+
+	// Default to "off" for security if not specified
+	if logLevel == "" {
+		logLevel = "off"
+	}


FailOnMissingKeyInMap is enabled only when the env var equals the exact string "true" after trimming. Unless this strict/case-sensitive behavior is intentional, consider using strconv.ParseBool (or normalizing to lower-case) so common values like "TRUE"/"1" work and so the docs don't need to specify casing quirks.

Copilot · 2026-03-22T02:43:47Z

+			if tt.awsLogLevel != "" {
+				t.Setenv(envvar.AWSSDKLogLevel, tt.awsLogLevel)
+			}
+			if tt.failOnMissingKey != "" {
+				t.Setenv(envvar.ValsFailOnMissingKeyInMap, tt.failOnMissingKey)
+			}


TestBuildValsOptions only calls t.Setenv when the test value is non-empty, so if HELMFILE_AWS_SDK_LOG_LEVEL or HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP is already set in the external environment, the "defaults"/empty cases can become flaky. Set these env vars for every test case (including empty-string) so the test is hermetic.

Suggested change

if tt.awsLogLevel != "" {

t.Setenv(envvar.AWSSDKLogLevel, tt.awsLogLevel)

}

if tt.failOnMissingKey != "" {

t.Setenv(envvar.ValsFailOnMissingKeyInMap, tt.failOnMissingKey)

}

// Always set env vars explicitly, even to empty string, so tests are hermetic.

t.Setenv(envvar.AWSSDKLogLevel, tt.awsLogLevel)

t.Setenv(envvar.ValsFailOnMissingKeyInMap, tt.failOnMissingKey)

Add HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP environment variable to control whether vals should fail when a referenced key does not exist in the secret map. Previously, when a secret reference like ref+vault://path#/nonexistent-key pointed to a non-existent key, vals would silently return an empty string without error. This could lead to deployments with missing configuration. Default behavior remains backward compatible (returns empty string). Set HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP=true to enable strict mode. Fixes #1563 Signed-off-by: yxxhero <[email protected]>

- Extract buildValsOptions() to make vals configuration testable - Use t.Setenv instead of manual env save/restore in tests - Test actual vals.Options output including FailOnMissingKeyInMap Addresses PR review comments on #2496 Signed-off-by: yxxhero <[email protected]>

- Use strconv.ParseBool for FailOnMissingKeyInMap parsing to support common boolean values like 'TRUE', '1', '0', etc. - Always set env vars explicitly in tests (even to empty string) to prevent flaky tests when env vars are set externally - Add test cases for various boolean formats Signed-off-by: yxxhero <[email protected]>

Add documentation for: - HELMFILE_AWS_SDK_LOG_LEVEL: configure AWS SDK logging for vals - HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP: enable strict mode for secret refs Signed-off-by: yxxhero <[email protected]>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

Copilot · 2026-03-23T05:25:31Z

+	// 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
+	failOnMissingKey, _ := strconv.ParseBool(strings.TrimSpace(os.Getenv(envvar.ValsFailOnMissingKeyInMap)))


strconv.ParseBool errors are currently ignored, which means invalid values (e.g., "ture") silently disable strict mode by defaulting to false. This is risky because users may believe strict mode is enabled when it isn't. Consider capturing the error and returning it from ValsInstance() (e.g., change buildValsOptions() to return (vals.Options, error)), or explicitly rejecting invalid non-empty values with a clear error message.

Copilot · 2026-03-23T05:25:31Z

+	// - 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))


The off check is case-sensitive. If a user sets HELMFILE_AWS_SDK_LOG_LEVEL=OFF (or mixed case), opts.LogOutput will not be discarded, potentially re-enabling vals internal logging unexpectedly. Normalize known levels to a canonical form (e.g., map EqualFold(..., "off") to "off" before both setting AWSLogLevel and deciding on LogOutput).

Copilot · 2026-03-23T05:25:32Z

+	if logLevel == "off" {
+		opts.LogOutput = io.Discard
+	}


The off check is case-sensitive. If a user sets HELMFILE_AWS_SDK_LOG_LEVEL=OFF (or mixed case), opts.LogOutput will not be discarded, potentially re-enabling vals internal logging unexpectedly. Normalize known levels to a canonical form (e.g., map EqualFold(..., "off") to "off" before both setting AWSLogLevel and deciding on LogOutput).

Copilot · 2026-03-23T05:25:32Z

+func TestBuildValsOptions(t *testing.T) {
+	tests := []struct {
+		name                       string
+		awsLogLevel                string
+		failOnMissingKey           string
+		expectedLogLevel           string
+		expectedFailOnMissingKey   bool
+		expectedLogOutputDiscarded bool
+	}{


There is no test case covering invalid values for HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP (e.g., "notabool"). Given the current implementation silently treats invalid inputs as false, add a test that asserts the chosen behavior. If you adopt error-on-invalid (recommended), add a test that expects ValsInstance() (or buildValsOptions()) to return an error for invalid non-empty values.

- buildValsOptions now returns error for invalid boolean values instead of silently defaulting to false - Use strings.EqualFold for case-insensitive 'off' comparison to handle OFF, Off, etc. - Add test cases for invalid boolean and uppercase OFF - Update docs to mention case-insensitive and error behavior Signed-off-by: yxxhero <[email protected]>

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

Copilot · 2026-03-23T05:55:32Z

+	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
+	// Use case-insensitive comparison for "off" to handle OFF, Off, etc.
+	if strings.EqualFold(logLevel, "off") {
+		opts.LogOutput = io.Discard
+	}


The code treats "off" as case-insensitive only for opts.LogOutput, but still passes the original-cased logLevel through to opts.AWSLogLevel. If the vals/AWS SDK log-level parsing is case-sensitive, values like OFF/Off may not actually disable AWS SDK logging even though helmfile suppresses vals’ internal logging—this is inconsistent with the documented “case-insensitive” behavior. Consider normalizing logLevel to the canonical "off" when EqualFold(logLevel, "off") (or normalizing all known values) before assigning opts.AWSLogLevel, and update tests accordingly.

Copilot · 2026-03-23T05:55:33Z

+func ValsInstance() (*vals.Runtime, error) {
+	var err error
+	once.Do(func() {
+		var opts vals.Options
+		opts, err = buildValsOptions()
+		if err != nil {
+			return
+		}
 		instance, err = vals.New(opts)
 	})


once.Do executes even when buildValsOptions() returns an error, which permanently prevents re-initialization for the lifetime of the process. This makes a one-time configuration mistake (e.g., invalid boolean env var) unrecoverable within the same process, and can also make tests brittle if env vars change between subtests and ValsInstance() is used elsewhere. A more robust pattern is to only “lock in” the singleton after successful initialization (e.g., guard with a mutex + if instance == nil and allow retry when the previous attempt errored, or store initErr and decide whether retries are allowed).

Copilot · 2026-03-23T05:55:33Z

 * `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


The docs claim HELMFILE_AWS_SDK_LOG_LEVEL is “case-insensitive”, but the implementation currently only treats "off" as case-insensitive for suppressing opts.LogOutput; it still forwards the original value to opts.AWSLogLevel. Either normalize/canonicalize the value in code (recommended, to match docs and user expectations) or adjust the docs to describe the exact behavior.

Suggested change

* `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_AWS_SDK_LOG_LEVEL` - configure AWS SDK logging level for vals library. Valid values: `off` (default, secure; matching is case-insensitive only for disabling logging output), `minimal`, `standard`, `verbose`, or custom comma-separated values like `request,response`. The raw value is forwarded to the AWS SDK as-is, so non-`off` values must use the exact form expected by the SDK. See issue #2270 for details

Copilot

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

Copilot · 2026-03-23T06:11:37Z

 * `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 at startup. Default is `false` (when unset or empty) for backward compatibility. See issue #1563 for details


Docs say invalid values for HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP "will cause an error at startup", but the error is raised when ValsInstance() initializes vals (i.e., when vals is first used), which may not be at process startup in all command paths. Consider rewording to reflect when initialization happens (e.g., "when vals is initialized" / "when secret refs are evaluated").

Suggested change

* `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 at startup. Default is `false` (when unset or empty) for backward compatibility. See issue #1563 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 (for example, when secret references are first evaluated). Default is `false` (when unset or empty) for backward compatibility. See issue #1563 for details

- Normalize AWS log level 'off' to lowercase for true case-insensitivity - Replace sync.Once with mutex to allow recovery from config errors - Update tests to expect normalized 'off' value - Update docs to clarify when error is raised Signed-off-by: yxxhero <[email protected]>

yxxhero requested review from aditmeno, itscaro, mumoshu, xiaomudk and zhaque44 as code owners March 22, 2026 01:15

yxxhero requested a review from Copilot March 22, 2026 01:16

Copilot started reviewing on behalf of yxxhero March 22, 2026 01:17 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

yxxhero requested a review from Copilot March 22, 2026 02:41

Copilot started reviewing on behalf of yxxhero March 22, 2026 02:41 View session

Copilot AI reviewed Mar 22, 2026

View reviewed changes

xiaomudk previously approved these changes Mar 23, 2026

View reviewed changes

yxxhero dismissed xiaomudk’s stale review via 17af6ac March 23, 2026 05:16

yxxhero added 4 commits March 23, 2026 13:19

docs: add documentation for vals-related environment variables

ea7808f

Add documentation for: - HELMFILE_AWS_SDK_LOG_LEVEL: configure AWS SDK logging for vals - HELMFILE_VALS_FAIL_ON_MISSING_KEY_IN_MAP: enable strict mode for secret refs Signed-off-by: yxxhero <[email protected]>

yxxhero force-pushed the fix-vals-missing-key-error-1563 branch from 9904af0 to ea7808f Compare March 23, 2026 05:19

yxxhero requested a review from Copilot March 23, 2026 05:19

Copilot AI reviewed Mar 23, 2026

View reviewed changes

Copilot started reviewing on behalf of yxxhero March 23, 2026 05:31 View session

yxxhero requested a review from Copilot March 23, 2026 05:48

Copilot started reviewing on behalf of yxxhero March 23, 2026 05:55 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

yxxhero requested a review from Copilot March 23, 2026 06:08

Copilot started reviewing on behalf of yxxhero March 23, 2026 06:08 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

xiaomudk approved these changes Mar 24, 2026

View reviewed changes

yxxhero merged commit 472e8c7 into main Mar 24, 2026
16 checks passed

yxxhero deleted the fix-vals-missing-key-error-1563 branch March 24, 2026 01:42

Uh oh!

Uh oh!

Conversation

yxxhero commented Mar 22, 2026

Summary

Changes

Usage

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants