Merge 5f18c08711 into eba25e2fed

internal/backend/backend_test.go

@@ -8,7 +8,9 @@ package backend
 import (
 	"io"
 	"os"
+	"os/exec"
 	"os/user"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -76,12 +78,19 @@ func TestRead_PathNoPermission(t *testing.T) {
 	}
 	f.Close()
 
-	if err := os.Chmod(f.Name(), 0); err != nil {
-		t.Fatalf("err: %s", err)
+	if runtime.GOOS == "windows" {
+		// Use cacls to remove all permissions for this file on Windows
+		cmd := exec.Command("cmd", "/c", "cacls", f.Name(), "/E", "/R", os.Getenv("USERNAME")) // #nosec G204
+		if err := cmd.Run(); err != nil {
+			t.Fatalf("Failed to set file permissions with cacls: %s", err)
+		}
+	} else {
+		if err := os.Chmod(f.Name(), 0); err != nil {
+			t.Fatalf("Failed to chmod file: %s", err)
+		}
 	}
 
 	contents, err := ReadPathOrContents(f.Name())
-
 	if err == nil {
 		t.Fatal("Expected error, got none!")
 	}

internal/backend/local/testing.go

@@ -7,6 +7,7 @@ package local
 
 import (
 	"path/filepath"
+	"runtime"
 	"testing"
 
 	"github.com/zclconf/go-cty/cty"
@@ -40,6 +41,13 @@ func TestLocal(t *testing.T) *Local {
 	local.StateWorkspaceDir = filepath.Join(tempDir, "state.tfstate.d")
 	local.ContextOpts = &tofu.ContextOpts{}
 
+	// Trigger garbage collection to ensure that all open file handles are closed.
+	// This prevents TempDir cleanup errors on Windows.
+	t.Cleanup(func() {
+		if runtime.GOOS == "windows" {
+			runtime.GC()
+		}
+	})
 	return local
 }
 

internal/backend/remote-state/s3/backend_complete_test.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"path/filepath"
 	"regexp"
+	"runtime"
 	"testing"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
@@ -147,7 +148,12 @@ func (m noopMatcher) String() string {
 	return ""
 }
 
+const osWindows = "windows"
+
 func TestBackendConfig_Authentication(t *testing.T) {
+	testDirectory := t.TempDir()
+	sysRoot := os.Getenv("SYSTEMROOT")
+
 	testCases := map[string]struct {
 		config                     map[string]any
 		EnableEc2MetadataServer    bool
@@ -634,6 +640,19 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey
 		t.Run(name, func(t *testing.T) {
 			servicemocks.InitSessionTestEnv(t)
 
+			// Set Windows-specific environment variables
+			if runtime.GOOS == osWindows {
+				t.Setenv("TEMP", testDirectory)
+				t.Setenv("TMP", testDirectory)
+				t.Setenv("SYSTEMROOT", sysRoot)
+
+				// Trigger garbage collection to ensure that all open file handles are closed.
+				// This prevents TempDir RemoveAll cleanup errors on Windows.
+				t.Cleanup(func() {
+					runtime.GC()
+				})
+			}
+
 			// Populate required fields
 			tc.config["region"] = "us-east-1"
 			tc.config["bucket"] = "bucket"
@@ -754,6 +773,9 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey
 	}
 }
 func TestBackendConfig_Authentication_AssumeRoleInline(t *testing.T) {
+	testDirectory := t.TempDir()
+	sysRoot := os.Getenv("SYSTEMROOT")
+
 	testCases := map[string]struct {
 		config                     map[string]any
 		EnableEc2MetadataServer    bool
@@ -1100,6 +1122,19 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey
 		t.Run(name, func(t *testing.T) {
 			servicemocks.InitSessionTestEnv(t)
 
+			// Set Windows-specific environment variables
+			if runtime.GOOS == osWindows {
+				t.Setenv("TEMP", testDirectory)
+				t.Setenv("TMP", testDirectory)
+				t.Setenv("SYSTEMROOT", sysRoot)
+
+				// Trigger garbage collection to ensure that all open file handles are closed.
+				// This prevents TempDir RemoveAll cleanup errors on Windows.
+				t.Cleanup(func() {
+					runtime.GC()
+				})
+			}
+
 			ctx := context.TODO()
 
 			// Populate required fields
@@ -1194,6 +1229,9 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey
 }
 
 func TestBackendConfig_Authentication_AssumeRoleNested(t *testing.T) {
+	testDirectory := t.TempDir()
+	sysRoot := os.Getenv("SYSTEMROOT")
+
 	testCases := map[string]struct {
 		config                     map[string]any
 		EnableEc2MetadataServer    bool
@@ -1502,6 +1540,19 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey
 		t.Run(name, func(t *testing.T) {
 			servicemocks.InitSessionTestEnv(t)
 
+			// Set Windows-specific environment variables
+			if runtime.GOOS == osWindows {
+				t.Setenv("TEMP", testDirectory)
+				t.Setenv("TMP", testDirectory)
+				t.Setenv("SYSTEMROOT", sysRoot)
+
+				// Trigger garbage collection to ensure that all open file handles are closed.
+				// This prevents TempDir RemoveAll cleanup errors on Windows.
+				t.Cleanup(func() {
+					runtime.GC()
+				})
+			}
+
 			ctx := context.TODO()
 
 			// Populate required fields
@@ -1598,6 +1649,9 @@ aws_secret_access_key = DefaultSharedCredentialsSecretKey
 }
 
 func TestBackendConfig_Authentication_AssumeRoleWithWebIdentity(t *testing.T) {
+	testDirectory := t.TempDir()
+	sysRoot := os.Getenv("SYSTEMROOT")
+
 	testCases := map[string]struct {
 		config                          map[string]any
 		SetConfig                       bool
@@ -1775,6 +1829,19 @@ web_identity_token_file = no-such-file
 		t.Run(name, func(t *testing.T) {
 			servicemocks.InitSessionTestEnv(t)
 
+			// Set Windows-specific environment variables
+			if runtime.GOOS == osWindows {
+				t.Setenv("TEMP", testDirectory)
+				t.Setenv("TMP", testDirectory)
+				t.Setenv("SYSTEMROOT", sysRoot)
+
+				// Trigger garbage collection to ensure that all open file handles are closed.
+				// This prevents TempDir RemoveAll cleanup errors on Windows.
+				t.Cleanup(func() {
+					runtime.GC()
+				})
+			}
+
 			ctx := context.TODO()
 
 			// Populate required fields
@@ -1812,13 +1879,20 @@ web_identity_token_file = no-such-file
 			}
 
 			if tc.ExpandEnvVars {
+				var prefix string
 				tmpdir := os.Getenv("TMPDIR")
+				if runtime.GOOS == osWindows {
+					tmpdir = os.Getenv("TEMP")
+					prefix = tmpdir
+				} else {
+					prefix = "$TMPDIR"
+				}
 				rel, err := filepath.Rel(tmpdir, tokenFileName)
 				if err != nil {
 					t.Fatalf("error making path relative: %s", err)
 				}
 				t.Logf("relative: %s", rel)
-				tokenFileName = filepath.Join("$TMPDIR", rel)
+				tokenFileName = filepath.Join(prefix, rel)
 				t.Logf("env tempfile: %s", tokenFileName)
 			}
 
@@ -1876,6 +1950,9 @@ web_identity_token_file = no-such-file
 }
 
 func TestBackendConfig_Region(t *testing.T) {
+	testDirectory := t.TempDir()
+	sysRoot := os.Getenv("SYSTEMROOT")
+
 	testCases := map[string]struct {
 		config                  map[string]any
 		EnvironmentVariables    map[string]string
@@ -2034,6 +2111,19 @@ region = us-west-2
 		t.Run(name, func(t *testing.T) {
 			servicemocks.InitSessionTestEnv(t)
 
+			// Set Windows-specific environment variables
+			if runtime.GOOS == osWindows {
+				t.Setenv("TEMP", testDirectory)
+				t.Setenv("TMP", testDirectory)
+				t.Setenv("SYSTEMROOT", sysRoot)
+
+				// Trigger garbage collection to ensure that all open file handles are closed.
+				// This prevents TempDir RemoveAll cleanup errors on Windows.
+				t.Cleanup(func() {
+					runtime.GC()
+				})
+			}
+
 			// Populate required fields
 			tc.config["bucket"] = "bucket"
 			tc.config["key"] = "key"
@@ -2092,6 +2182,9 @@ region = us-west-2
 }
 
 func TestBackendConfig_RetryMode(t *testing.T) {
+	testDirectory := t.TempDir()
+	sysRoot := os.Getenv("SYSTEMROOT")
+
 	testCases := map[string]struct {
 		config               map[string]any
 		EnvironmentVariables map[string]string
@@ -2143,6 +2236,19 @@ func TestBackendConfig_RetryMode(t *testing.T) {
 		t.Run(name, func(t *testing.T) {
 			servicemocks.InitSessionTestEnv(t)
 
+			// Set Windows-specific environment variables
+			if runtime.GOOS == osWindows {
+				t.Setenv("TEMP", testDirectory)
+				t.Setenv("TMP", testDirectory)
+				t.Setenv("SYSTEMROOT", sysRoot)
+
+				// Trigger garbage collection to ensure that all open file handles are closed.
+				// This prevents TempDir RemoveAll cleanup errors on Windows.
+				t.Cleanup(func() {
+					runtime.GC()
+				})
+			}
+
 			// Populate required fields
 			tc.config["bucket"] = "bucket"
 			tc.config["key"] = "key"

internal/backend/testing.go

@@ -8,6 +8,7 @@ package backend
 import (
 	"fmt"
 	"reflect"
+	"runtime"
 	"sort"
 	"testing"
 
@@ -304,6 +305,7 @@ func TestBackendStates(t *testing.T, b Backend) {
 	if v := foo.State(); v.HasManagedResourceInstanceObjects() {
 		t.Fatalf("should be empty: %s", v)
 	}
+
 	// and delete it again
 	if err := b.DeleteWorkspace("foo", true); err != nil {
 		t.Fatalf("err: %s", err)
@@ -325,6 +327,14 @@ func TestBackendStates(t *testing.T, b Backend) {
 			t.Fatalf("wrong workspaces list\ngot:  %#v\nwant: %#v", workspaces, expected)
 		}
 	}
+
+	// Trigger garbage collection to ensure that all open file handles are closed.
+	// This prevents TempDir cleanup errors on Windows.
+	t.Cleanup(func() {
+		if runtime.GOOS == "windows" {
+			runtime.GC()
+		}
+	})
 }
 
 // TestBackendStateLocks will test the locking functionality of the remote

internal/states/statemgr/filesystem.go

@@ -172,7 +172,16 @@ func (s *Filesystem) persistState(schemas *tofu.Schemas) error {
 			return nil
 		}
 	}
-	defer s.stateFileOut.Sync()
+
+	// Sync and close the file handle after all operations are complete
+	defer func() {
+		if err := s.stateFileOut.Sync(); err != nil {
+			log.Printf("Error syncing statefile: %s", err)
+		}
+
+		s.stateFileOut.Close()
+		s.stateFileOut = nil
+	}()
 
 	if s.file == nil {
 		s.file = NewStateFile()
@@ -389,23 +398,25 @@ func (s *Filesystem) Unlock(id string) error {
 	} else {
 		log.Printf("[TRACE] statemgr.Filesystem: removed lock metadata file %s", lockInfoPath)
 	}
-	fileName := s.stateFileOut.Name()
-
 	unlockErr := s.unlock()
+	// Perform cleanup if stateFileOut is not nil.
+	if s.stateFileOut != nil {
+		fileName := s.stateFileOut.Name()
 
-	s.stateFileOut.Close()
-	s.stateFileOut = nil
-	s.lockID = ""
+		s.stateFileOut.Close()
+		s.stateFileOut = nil
+		s.lockID = ""
 
-	// clean up the state file if we created it an never wrote to it
-	stat, err := os.Stat(fileName)
-	if err == nil && stat.Size() == 0 && s.created {
-		err = os.Remove(fileName)
-		if err != nil {
-			log.Printf("[ERROR] stagemgr.Filesystem: error removing empty state file %q: %s", fileName, err)
+		// Clean up the state file if we created it and never wrote to it
+		stat, err := os.Stat(fileName)
+		if err == nil && stat.Size() == 0 && s.created {
+			err = os.Remove(fileName)
+			if err != nil {
+				log.Printf("[ERROR] stagemgr.Filesystem: error removing empty state file %q: %s", fileName, err)
+			}
 		}
 	}
-
+	s.lockID = ""
 	return unlockErr
 }
 

internal/states/statemgr/filesystem_lock_unix.go

@@ -30,6 +30,19 @@ func (s *Filesystem) lock() error {
 }
 
 func (s *Filesystem) unlock() error {
+	// Handle case where s.stateFileOut is nil, indicating no lock to release.
+	if s.stateFileOut == nil {
+		log.Print("[TRACE] statemgr.Filesystem: statefileout is nil, cannot unlock")
+		return nil
+	}
+
+	// Check if file descriptor is invalid
+	fd := s.stateFileOut.Fd()
+	if fd == ^uintptr(0) {
+		log.Print("[TRACE] statemgr.Filesystem: fd is invalid, cannot unlock")
+		return nil
+	}
+
 	log.Printf("[TRACE] statemgr.Filesystem: unlocking %s using fcntl flock", s.path)
 	flock := &syscall.Flock_t{
 		Type:   syscall.F_UNLCK,
@@ -38,6 +51,5 @@ func (s *Filesystem) unlock() error {
 		Len:    0,
 	}
 
-	fd := s.stateFileOut.Fd()
 	return syscall.FcntlFlock(fd, syscall.F_SETLK, flock)
 }

internal/states/statemgr/filesystem_test.go

@@ -189,8 +189,6 @@ func TestFilesystem_backup(t *testing.T) {
 // not the contents of the input file (which is left unchanged).
 func TestFilesystem_backupAndReadPath(t *testing.T) {
 	defer testOverrideVersion(t, "1.2.3")()
-	info := NewLockInfo()
-	info.Operation = "test"
 
 	workDir := t.TempDir()
 	markerOutput := addrs.OutputValue{Name: "foo"}.Absolute(addrs.RootModuleInstance)
@@ -256,15 +254,6 @@ func TestFilesystem_backupAndReadPath(t *testing.T) {
 		t.Fatalf("failed to write new state: %s", err)
 	}
 
-	lockID, err := ls.Lock(info)
-	if err != nil {
-		t.Fatal(err)
-	}
-
-	if err := ls.Unlock(lockID); err != nil {
-		t.Fatal(err)
-	}
-
 	// The backup functionality should've saved a copy of the original contents
 	// of the _output_ file, even though the first snapshot was read from
 	// the _input_ file.