feat: add HELMFILE_DISABLE_VALS env var to skip vals processing (#2471)

Signed-off-by: Jim Robinson <1643772+jimmyR@users.noreply.github.com> Signed-off-by: yxxhero <aiopsclub@163.com> Co-authored-by: yxxhero <aiopsclub@163.com>
2026-09-30 06:37:27 +02:00 · 2026-09-13 15:40:12 +08:00
parent 0d6f3ab160
commit bc0c103d79
6 changed files with 516 additions and 22 deletions
@@ -1,9 +1,11 @@
 package plugins

 import (
+	"errors"
 	"fmt"
 	"io"
 	"os"
+	"regexp"
 	"strconv"
 	"strings"
 	"sync"
@@ -18,9 +20,105 @@ const (
 	valsCacheSize = 512
 )

-var instance *vals.Runtime
+var instance vals.Evaluator
 var mu sync.Mutex

+// ErrValsDisabled is returned by the evaluator when HELMFILE_DISABLE_VALS_STRICT is set
+// and a `ref+` expression is encountered.
+var ErrValsDisabled = errors.New("vals is disabled via HELMFILE_DISABLE_VALS_STRICT environment variable")
+
+// refPlusRegexp mirrors the reference syntax understood by the vals library
+// (`ref+<provider>://...` and `secretref+<provider>://...`), so that disabled-vals
+// modes only report values that vals itself would have tried to resolve.
+var refPlusRegexp = regexp.MustCompile(`(secret)?ref\+[^\s+:]*://`)
+
+// passthroughEvaluator passes values through unchanged (for external vals)
+type passthroughEvaluator struct{}
+
+func (p *passthroughEvaluator) Eval(m map[string]any) (map[string]any, error) {
+	return normalizeMap(m), nil
+}
+
+// strictEvaluator passes through values but errors if ref+ is detected
+type strictEvaluator struct{}
+
+func (s *strictEvaluator) Eval(m map[string]any) (map[string]any, error) {
+	if containsRefPlus(m) {
+		return nil, ErrValsDisabled
+	}
+	return normalizeMap(m), nil
+}
+
+// normalizeMap converts []string values to []any to match vals.Eval behavior.
+func normalizeMap(m map[string]any) map[string]any {
+	out := make(map[string]any, len(m))
+	for k, v := range m {
+		out[k] = normalizeValue(v)
+	}
+	return out
+}
+
+// normalizeValue recursively converts []string to []any and map[any]any to
+// map[string]any, matching the type normalization performed by vals.Eval.
+func normalizeValue(v any) any {
+	switch typed := v.(type) {
+	case map[string]any:
+		return normalizeMap(typed)
+	case map[any]any:
+		strmap := make(map[string]any, len(typed))
+		for k, v := range typed {
+			strmap[fmt.Sprintf("%v", k)] = normalizeValue(v)
+		}
+		return strmap
+	case []any:
+		a := make([]any, len(typed))
+		for i, e := range typed {
+			a[i] = normalizeValue(e)
+		}
+		return a
+	case []string:
+		a := make([]any, len(typed))
+		for i, s := range typed {
+			a[i] = s
+		}
+		return a
+	default:
+		return v
+	}
+}
+
+func containsRefPlus(v any) bool {
+	switch val := v.(type) {
+	case string:
+		return refPlusRegexp.MatchString(val)
+	case map[string]any:
+		for _, v := range val {
+			if containsRefPlus(v) {
+				return true
+			}
+		}
+	case map[any]any:
+		for _, v := range val {
+			if containsRefPlus(v) {
+				return true
+			}
+		}
+	case []any:
+		for _, v := range val {
+			if containsRefPlus(v) {
+				return true
+			}
+		}
+	case []string:
+		for _, s := range val {
+			if refPlusRegexp.MatchString(s) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 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
@@ -78,7 +176,7 @@ func buildValsOptions() (vals.Options, error) {
 	return opts, nil
 }

-func ValsInstance() (*vals.Runtime, error) {
+func ValsInstance() (vals.Evaluator, error) {
 	mu.Lock()
 	defer mu.Unlock()

@@ -86,6 +184,20 @@ func ValsInstance() (*vals.Runtime, error) {
 		return instance, nil
 	}

+	// HELMFILE_DISABLE_VALS_STRICT: error on ref+ usage
+	strict, _ := strconv.ParseBool(os.Getenv(envvar.DisableValsStrict))
+	if strict {
+		instance = &strictEvaluator{}
+		return instance, nil
+	}
+
+	// HELMFILE_DISABLE_VALS: pass-through for external vals
+	disabled, _ := strconv.ParseBool(os.Getenv(envvar.DisableVals))
+	if disabled {
+		instance = &passthroughEvaluator{}
+		return instance, nil
+	}
+
 	opts, err := buildValsOptions()
 	if err != nil {
 		return nil, err
@@ -2,6 +2,7 @@ package plugins

 import (
 	"io"
+	"os"
 	"testing"

 	"github.com/helmfile/vals"
@@ -11,20 +12,347 @@ import (
 	"github.com/helmfile/helmfile/pkg/envvar"
 )

-func TestValsInstance(t *testing.T) {
-	i, err := ValsInstance()
+// resetInstance resets the singleton for testing
+func resetInstance() {
+	mu.Lock()
+	defer mu.Unlock()
+	instance = nil
+}

+// setenvForTest sets the environment variable to value, or unsets it when value is empty,
+// restoring the original value when the test finishes.
+func setenvForTest(t *testing.T, key, value string) {
+	t.Helper()
+	if value == "" {
+		orig, had := os.LookupEnv(key)
+		os.Unsetenv(key)
+		t.Cleanup(func() {
+			if had {
+				os.Setenv(key, orig)
+			} else {
+				os.Unsetenv(key)
+			}
+		})
+		return
+	}
+	t.Setenv(key, value)
+}
+
+func TestValsInstance(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "")
+	setenvForTest(t, envvar.DisableValsStrict, "")
+
+	i, err := ValsInstance()
 	if err != nil {
 		t.Errorf("unexpected error: %v", err)
 	}

 	i2, _ := ValsInstance()
-
 	if i != i2 {
 		t.Error("Instances should be equal")
 	}
 }

+func TestDisableVals(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "true")
+	setenvForTest(t, envvar.DisableValsStrict, "")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Should pass through values unchanged
+	input := map[string]any{"key": "ref+echo://secret"}
+	output, err := evaluator.Eval(input)
+	if err != nil {
+		t.Fatalf("passthrough should not error: %v", err)
+	}
+
+	if output["key"] != "ref+echo://secret" {
+		t.Errorf("expected ref+ to pass through unchanged, got %v", output["key"])
+	}
+}
+
+func TestDisableValsStrict(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "")
+	setenvForTest(t, envvar.DisableValsStrict, "true")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Should error on ref+
+	input := map[string]any{"key": "ref+echo://secret"}
+	_, err = evaluator.Eval(input)
+	if err == nil {
+		t.Fatal("strict mode should error on ref+")
+	}
+	if err != ErrValsDisabled {
+		t.Errorf("expected ErrValsDisabled, got %v", err)
+	}
+}
+
+func TestDisableValsStrictTakesPrecedence(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "true")
+	setenvForTest(t, envvar.DisableValsStrict, "true")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Strict mode should win over pass-through mode
+	input := map[string]any{"key": "ref+echo://secret"}
+	_, err = evaluator.Eval(input)
+	if err != ErrValsDisabled {
+		t.Errorf("expected ErrValsDisabled when both env vars are set, got %v", err)
+	}
+}
+
+func TestDisableValsStrictAllowsNonRef(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "")
+	setenvForTest(t, envvar.DisableValsStrict, "true")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Should pass through non-ref+ values, including strings that merely
+	// contain "ref+" without a provider scheme (not a vals reference)
+	input := map[string]any{
+		"key":        "normal-value",
+		"literal":    "some ref+ text without scheme",
+		"secretRef2": "ref+2",
+	}
+	output, err := evaluator.Eval(input)
+	if err != nil {
+		t.Fatalf("strict mode should allow non-ref+ values: %v", err)
+	}
+	if output["key"] != "normal-value" {
+		t.Errorf("expected value to pass through, got %v", output["key"])
+	}
+}
+
+func TestDisableValsStrictDetectsSecretRef(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "")
+	setenvForTest(t, envvar.DisableValsStrict, "true")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// secretref+ is also a vals reference and must be rejected
+	input := map[string]any{"key": "secretref+vault://secret/data/x#y"}
+	if _, err := evaluator.Eval(input); err != ErrValsDisabled {
+		t.Errorf("expected ErrValsDisabled for secretref+ expression, got %v", err)
+	}
+}
+
+func TestDisableValsStrictNestedRef(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "")
+	setenvForTest(t, envvar.DisableValsStrict, "true")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Should detect nested ref+ in map[string]any
+	input := map[string]any{
+		"outer": map[string]any{
+			"inner": "ref+vault://secret",
+		},
+	}
+	_, err = evaluator.Eval(input)
+	if err == nil {
+		t.Fatal("strict mode should detect nested ref+")
+	}
+}
+
+func TestDisableValsStrictMapAnyAny(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "")
+	setenvForTest(t, envvar.DisableValsStrict, "true")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Should detect ref+ inside map[any]any (yaml.v2 nested maps)
+	input := map[string]any{
+		"outer": map[any]any{
+			"inner": "ref+vault://secret",
+		},
+	}
+	_, err = evaluator.Eval(input)
+	if err == nil {
+		t.Fatal("strict mode should detect ref+ in map[any]any")
+	}
+}
+
+func TestDisableValsStrictArrayRef(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "")
+	setenvForTest(t, envvar.DisableValsStrict, "true")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Should detect ref+ in []any arrays
+	input := map[string]any{
+		"values": []any{"normal", "ref+awssecrets://db/password"},
+	}
+	_, err = evaluator.Eval(input)
+	if err == nil {
+		t.Fatal("strict mode should detect ref+ in arrays")
+	}
+}
+
+func TestDisableValsStrictStringSlice(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "")
+	setenvForTest(t, envvar.DisableValsStrict, "true")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Should detect ref+ in []string arrays (matches renderValsSecrets usage)
+	input := map[string]any{
+		"values": []string{"normal", "ref+awssecrets://db/password"},
+	}
+	_, err = evaluator.Eval(input)
+	if err == nil {
+		t.Fatal("strict mode should detect ref+ in []string arrays")
+	}
+}
+
+func TestDisableValsPassThroughStringSlice(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "true")
+	setenvForTest(t, envvar.DisableValsStrict, "")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	input := map[string]any{
+		"values": []string{"normal", "ref+awssecrets://db/password"},
+	}
+	out, err := evaluator.Eval(input)
+	if err != nil {
+		t.Fatalf("pass-through should not error: %v", err)
+	}
+
+	values, ok := out["values"].([]any)
+	if !ok {
+		t.Fatalf("expected out[\"values\"] to be []any, got %T", out["values"])
+	}
+	if values[0] != "normal" || values[1] != "ref+awssecrets://db/password" {
+		t.Errorf("unexpected values: %v", values)
+	}
+}
+
+func TestDisableValsPassThroughNestedTypes(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	setenvForTest(t, envvar.DisableVals, "true")
+	setenvForTest(t, envvar.DisableValsStrict, "")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// Nested map[any]any and []string should be normalized to map[string]any
+	// and []any respectively, matching vals.Eval behavior
+	input := map[string]any{
+		"outer": map[any]any{
+			"list": []string{"normal", "ref+echo://secret"},
+		},
+	}
+	out, err := evaluator.Eval(input)
+	if err != nil {
+		t.Fatalf("pass-through should not error: %v", err)
+	}
+
+	outer, ok := out["outer"].(map[string]any)
+	if !ok {
+		t.Fatalf("expected out[\"outer\"] to be map[string]any, got %T", out["outer"])
+	}
+	list, ok := outer["list"].([]any)
+	if !ok {
+		t.Fatalf("expected outer[\"list\"] to be []any, got %T", outer["list"])
+	}
+	if list[0] != "normal" || list[1] != "ref+echo://secret" {
+		t.Errorf("unexpected values: %v", list)
+	}
+}
+
+func TestNormalValsProcessing(t *testing.T) {
+	resetInstance()
+	defer resetInstance()
+
+	// Ensure both are unset
+	setenvForTest(t, envvar.DisableVals, "")
+	setenvForTest(t, envvar.DisableValsStrict, "")
+
+	evaluator, err := ValsInstance()
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+
+	// ref+echo should expand to the value after ://
+	input := map[string]any{"key": "ref+echo://myvalue"}
+	output, err := evaluator.Eval(input)
+	if err != nil {
+		t.Fatalf("normal vals should process ref+echo: %v", err)
+	}
+
+	if output["key"] != "myvalue" {
+		t.Errorf("expected 'myvalue', got %v", output["key"])
+	}
+}
+
 func TestBuildValsOptions(t *testing.T) {
 	tests := []struct {
 		name                       string
@@ -129,14 +457,6 @@ func TestBuildValsOptions(t *testing.T) {
 			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",
@@ -172,6 +492,7 @@ func TestBuildValsOptions(t *testing.T) {
 	}
 }

+// TestAWSSDKLogLevelConfiguration tests the AWS SDK log level configuration logic
 func TestAWSSDKLogLevelConfiguration(t *testing.T) {
 	tests := []struct {
 		name              string