From e24861959c09141ab621eeaf5d8c65a13e6987ab Mon Sep 17 00:00:00 2001 From: dongjune8931 Date: Tue, 14 Apr 2026 00:43:13 +0900 Subject: [PATCH 1/4] fix: replace panic with error diagnostics for invalid TF_STATE_PERSIST_INTERVAL When TF_STATE_PERSIST_INTERVAL is set to a non-integer value or an integer below the minimum of 20, the previous implementation called panic(), which triggered the panicHandler and displayed an alarming 'OPENTOFU CRASH' message for what is simply a user configuration error. Replace both panic() calls with tfdiags.Sourceless error diagnostics so that invalid values produce a clear, actionable error message and exit cleanly instead of crashing. Fixes #4024 Signed-off-by: dongjune8931 --- internal/backend/local/backend_apply.go | 32 ++++++-- internal/backend/local/backend_apply_test.go | 82 ++++++++++++++++++++ 2 files changed, 106 insertions(+), 8 deletions(-) diff --git a/internal/backend/local/backend_apply.go b/internal/backend/local/backend_apply.go index cde8fc93f8..8da9b5d993 100644 --- a/internal/backend/local/backend_apply.go +++ b/internal/backend/local/backend_apply.go @@ -34,15 +34,21 @@ const ( persistIntervalEnvironmentVariableName = "TF_STATE_PERSIST_INTERVAL" ) -func getEnvAsInt(envName string, defaultValue int) int { +func getEnvAsInt(envName string, defaultValue int) (int, tfdiags.Diagnostics) { + var diags tfdiags.Diagnostics if val, exists := os.LookupEnv(envName); exists { parsedVal, err := strconv.Atoi(val) - if err == nil { - return parsedVal + if err != nil { + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid environment variable value", + fmt.Sprintf("The environment variable %s must be a valid integer, got %q.", envName, val), + )) + return 0, diags } - panic(fmt.Sprintf("Can't parse value '%s' of environment variable '%s'", val, envName)) + return parsedVal, diags } - return defaultValue + return defaultValue, diags } func (b *Local) opApply( @@ -112,10 +118,20 @@ func (b *Local) opApply( // stateHook uses schemas for when it periodically persists state to the // persistent storage backend. stateHook.Schemas = schemas - persistInterval := getEnvAsInt(persistIntervalEnvironmentVariableName, defaultPersistInterval) + persistInterval, intervalDiags := getEnvAsInt(persistIntervalEnvironmentVariableName, defaultPersistInterval) + diags = diags.Append(intervalDiags) + if intervalDiags.HasErrors() { + op.ReportResult(runningOp, diags) + return + } if persistInterval < defaultPersistInterval { - panic(fmt.Sprintf("Can't use value lower than %d for env variable %s, got %d", - defaultPersistInterval, persistIntervalEnvironmentVariableName, persistInterval)) + diags = diags.Append(tfdiags.Sourceless( + tfdiags.Error, + "Invalid environment variable value", + fmt.Sprintf("The environment variable %s must be at least %d, got %d.", persistIntervalEnvironmentVariableName, defaultPersistInterval, persistInterval), + )) + op.ReportResult(runningOp, diags) + return } stateHook.PersistInterval = time.Duration(persistInterval) * time.Second diff --git a/internal/backend/local/backend_apply_test.go b/internal/backend/local/backend_apply_test.go index ae973058e0..87d98424f8 100644 --- a/internal/backend/local/backend_apply_test.go +++ b/internal/backend/local/backend_apply_test.go @@ -428,3 +428,85 @@ func TestApply_applyCanceledAutoApprove(t *testing.T) { } } + +func TestGetEnvAsInt(t *testing.T) { + const testEnv = "TEST_GET_ENV_AS_INT" + + t.Run("env not set returns default", func(t *testing.T) { + os.Unsetenv(testEnv) + got, diags := getEnvAsInt(testEnv, 20) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) + } + if got != 20 { + t.Errorf("got %d, want 20", got) + } + }) + + t.Run("valid integer is parsed", func(t *testing.T) { + t.Setenv(testEnv, "30") + got, diags := getEnvAsInt(testEnv, 20) + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) + } + if got != 30 { + t.Errorf("got %d, want 30", got) + } + }) + + t.Run("non-integer value returns error", func(t *testing.T) { + t.Setenv(testEnv, "abc") + _, diags := getEnvAsInt(testEnv, 20) + if !diags.HasErrors() { + t.Error("expected error but got none") + } + }) + + t.Run("float value returns error", func(t *testing.T) { + t.Setenv(testEnv, "1.5") + _, diags := getEnvAsInt(testEnv, 20) + if !diags.HasErrors() { + t.Error("expected error but got none") + } + }) +} + +func TestLocal_applyInvalidPersistInterval(t *testing.T) { + t.Run("non-integer value causes error diagnostic", func(t *testing.T) { + t.Setenv(persistIntervalEnvironmentVariableName, "abc") + + b := TestLocal(t) + TestLocalProvider(t, b, "test", applyFixtureSchema()) + + op, done := testOperationApply(t, "./testdata/apply") + defer done(t) + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("unexpected error starting operation: %v", err) + } + <-run.Done() + if run.Result == backend.OperationSuccess { + t.Fatalf("expected operation to fail with invalid %s=abc", persistIntervalEnvironmentVariableName) + } + }) + + t.Run("below minimum value causes error diagnostic", func(t *testing.T) { + t.Setenv(persistIntervalEnvironmentVariableName, "5") + + b := TestLocal(t) + TestLocalProvider(t, b, "test", applyFixtureSchema()) + + op, done := testOperationApply(t, "./testdata/apply") + defer done(t) + + run, err := b.Operation(context.Background(), op) + if err != nil { + t.Fatalf("unexpected error starting operation: %v", err) + } + <-run.Done() + if run.Result == backend.OperationSuccess { + t.Fatalf("expected operation to fail with invalid %s=5", persistIntervalEnvironmentVariableName) + } + }) +} From 21ac1b53071416805644ead5267c755984680b3d Mon Sep 17 00:00:00 2001 From: dongjune8931 Date: Tue, 14 Apr 2026 18:10:23 +0900 Subject: [PATCH 2/4] test: remove unnecessary os.Unsetenv in TestGetEnvAsInt Signed-off-by: dongjune8931 --- internal/backend/local/backend_apply_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/internal/backend/local/backend_apply_test.go b/internal/backend/local/backend_apply_test.go index 87d98424f8..29e73910dc 100644 --- a/internal/backend/local/backend_apply_test.go +++ b/internal/backend/local/backend_apply_test.go @@ -433,7 +433,6 @@ func TestGetEnvAsInt(t *testing.T) { const testEnv = "TEST_GET_ENV_AS_INT" t.Run("env not set returns default", func(t *testing.T) { - os.Unsetenv(testEnv) got, diags := getEnvAsInt(testEnv, 20) if diags.HasErrors() { t.Fatalf("unexpected error: %s", diags.Err()) From 28d2ae801beb23fae6950f7b3069bcab7fe855ff Mon Sep 17 00:00:00 2001 From: dongjune8931 Date: Tue, 14 Apr 2026 18:11:55 +0900 Subject: [PATCH 3/4] test: assert error message in stderr for invalid TF_STATE_PERSIST_INTERVAL Signed-off-by: dongjune8931 --- internal/backend/local/backend_apply_test.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/internal/backend/local/backend_apply_test.go b/internal/backend/local/backend_apply_test.go index 29e73910dc..09b6238d74 100644 --- a/internal/backend/local/backend_apply_test.go +++ b/internal/backend/local/backend_apply_test.go @@ -478,7 +478,6 @@ func TestLocal_applyInvalidPersistInterval(t *testing.T) { TestLocalProvider(t, b, "test", applyFixtureSchema()) op, done := testOperationApply(t, "./testdata/apply") - defer done(t) run, err := b.Operation(context.Background(), op) if err != nil { @@ -488,6 +487,9 @@ func TestLocal_applyInvalidPersistInterval(t *testing.T) { if run.Result == backend.OperationSuccess { t.Fatalf("expected operation to fail with invalid %s=abc", persistIntervalEnvironmentVariableName) } + if got, want := done(t).Stderr(), "Invalid environment variable value"; !strings.Contains(got, want) { + t.Errorf("expected stderr to contain %q, got:\n%s", want, got) + } }) t.Run("below minimum value causes error diagnostic", func(t *testing.T) { @@ -497,7 +499,6 @@ func TestLocal_applyInvalidPersistInterval(t *testing.T) { TestLocalProvider(t, b, "test", applyFixtureSchema()) op, done := testOperationApply(t, "./testdata/apply") - defer done(t) run, err := b.Operation(context.Background(), op) if err != nil { @@ -507,5 +508,8 @@ func TestLocal_applyInvalidPersistInterval(t *testing.T) { if run.Result == backend.OperationSuccess { t.Fatalf("expected operation to fail with invalid %s=5", persistIntervalEnvironmentVariableName) } + if got, want := done(t).Stderr(), "Invalid environment variable value"; !strings.Contains(got, want) { + t.Errorf("expected stderr to contain %q, got:\n%s", want, got) + } }) } From 1df56deed956201487241ca7a31fad1c5947cc42 Mon Sep 17 00:00:00 2001 From: dongjune8931 Date: Tue, 14 Apr 2026 18:24:43 +0900 Subject: [PATCH 4/4] test: convert TestGetEnvAsInt to table-driven tests Signed-off-by: dongjune8931 --- internal/backend/local/backend_apply_test.go | 88 ++++++++++++-------- 1 file changed, 54 insertions(+), 34 deletions(-) diff --git a/internal/backend/local/backend_apply_test.go b/internal/backend/local/backend_apply_test.go index 09b6238d74..57df94e6e9 100644 --- a/internal/backend/local/backend_apply_test.go +++ b/internal/backend/local/backend_apply_test.go @@ -432,42 +432,62 @@ func TestApply_applyCanceledAutoApprove(t *testing.T) { func TestGetEnvAsInt(t *testing.T) { const testEnv = "TEST_GET_ENV_AS_INT" - t.Run("env not set returns default", func(t *testing.T) { - got, diags := getEnvAsInt(testEnv, 20) - if diags.HasErrors() { - t.Fatalf("unexpected error: %s", diags.Err()) - } - if got != 20 { - t.Errorf("got %d, want 20", got) - } - }) + tests := []struct { + name string + envValue string + defaultValue int + wantValue int + wantError bool + }{ + { + name: "env not set returns default", + envValue: "", + defaultValue: 20, + wantValue: 20, + wantError: false, + }, + { + name: "valid integer is parsed", + envValue: "30", + defaultValue: 20, + wantValue: 30, + wantError: false, + }, + { + name: "non-integer value returns error", + envValue: "abc", + defaultValue: 20, + wantError: true, + }, + { + name: "float value returns error", + envValue: "1.5", + defaultValue: 20, + wantError: true, + }, + } - t.Run("valid integer is parsed", func(t *testing.T) { - t.Setenv(testEnv, "30") - got, diags := getEnvAsInt(testEnv, 20) - if diags.HasErrors() { - t.Fatalf("unexpected error: %s", diags.Err()) - } - if got != 30 { - t.Errorf("got %d, want 30", got) - } - }) + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.envValue != "" { + t.Setenv(testEnv, tt.envValue) + } - t.Run("non-integer value returns error", func(t *testing.T) { - t.Setenv(testEnv, "abc") - _, diags := getEnvAsInt(testEnv, 20) - if !diags.HasErrors() { - t.Error("expected error but got none") - } - }) - - t.Run("float value returns error", func(t *testing.T) { - t.Setenv(testEnv, "1.5") - _, diags := getEnvAsInt(testEnv, 20) - if !diags.HasErrors() { - t.Error("expected error but got none") - } - }) + got, diags := getEnvAsInt(testEnv, tt.defaultValue) + if tt.wantError { + if !diags.HasErrors() { + t.Errorf("expected error but got none, value=%d", got) + } + return + } + if diags.HasErrors() { + t.Fatalf("unexpected error: %s", diags.Err()) + } + if got != tt.wantValue { + t.Errorf("got %d, want %d", got, tt.wantValue) + } + }) + } } func TestLocal_applyInvalidPersistInterval(t *testing.T) {