Merge pull request #17711 from hashicorp/jbardin/cleanup-temps

cleanup temp files

backend/local/backend_apply_test.go

@@ -18,7 +18,8 @@ import (
 )
 
 func TestLocal_applyBasic(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 
 	p.ApplyReturn = &terraform.InstanceState{ID: "yes"}
@@ -58,7 +59,9 @@ test_instance.foo:
 }
 
 func TestLocal_applyEmptyDir(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
+
 	p := TestLocalProvider(t, b, "test")
 
 	p.ApplyReturn = &terraform.InstanceState{ID: "yes"}
@@ -85,7 +88,8 @@ func TestLocal_applyEmptyDir(t *testing.T) {
 }
 
 func TestLocal_applyEmptyDirDestroy(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 
 	p.ApplyReturn = nil
@@ -111,7 +115,8 @@ func TestLocal_applyEmptyDirDestroy(t *testing.T) {
 }
 
 func TestLocal_applyError(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 
 	var lock sync.Mutex
@@ -169,7 +174,9 @@ func TestLocal_applyBackendFail(t *testing.T) {
 	mod, modCleanup := module.TestTree(t, "./test-fixtures/apply")
 	defer modCleanup()
 
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
+
 	wd, err := os.Getwd()
 	if err != nil {
 		t.Fatalf("failed to get current working directory")

backend/local/backend_plan_test.go

@@ -15,7 +15,8 @@ import (
 )
 
 func TestLocal_planBasic(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 
 	mod, modCleanup := module.TestTree(t, "./test-fixtures/plan")
@@ -40,7 +41,8 @@ func TestLocal_planBasic(t *testing.T) {
 }
 
 func TestLocal_planInAutomation(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	TestLocalProvider(t, b, "test")
 
 	mod, modCleanup := module.TestTree(t, "./test-fixtures/plan")
@@ -103,7 +105,8 @@ func TestLocal_planInAutomation(t *testing.T) {
 }
 
 func TestLocal_planNoConfig(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	TestLocalProvider(t, b, "test")
 
 	op := testOperationPlan()
@@ -126,7 +129,8 @@ func TestLocal_planNoConfig(t *testing.T) {
 }
 
 func TestLocal_planRefreshFalse(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 	terraform.TestStateFile(t, b.StatePath, testPlanState())
 
@@ -155,7 +159,8 @@ func TestLocal_planRefreshFalse(t *testing.T) {
 }
 
 func TestLocal_planDestroy(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 	terraform.TestStateFile(t, b.StatePath, testPlanState())
 
@@ -200,7 +205,8 @@ func TestLocal_planDestroy(t *testing.T) {
 }
 
 func TestLocal_planOutPathNoChange(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	TestLocalProvider(t, b, "test")
 	terraform.TestStateFile(t, b.StatePath, testPlanState())
 
@@ -237,7 +243,8 @@ func TestLocal_planOutPathNoChange(t *testing.T) {
 // checks to make sure the correct resource count is ultimately given to the
 // UI.
 func TestLocal_planScaleOutNoDupeCount(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	TestLocalProvider(t, b, "test")
 	state := &terraform.State{
 		Version: 2,

backend/local/backend_refresh_test.go

@@ -11,7 +11,9 @@ import (
 )
 
 func TestLocal_refresh(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
+
 	p := TestLocalProvider(t, b, "test")
 	terraform.TestStateFile(t, b.StatePath, testRefreshState())
 
@@ -42,7 +44,8 @@ test_instance.foo:
 }
 
 func TestLocal_refreshNilModule(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 	terraform.TestStateFile(t, b.StatePath, testRefreshState())
 
@@ -71,7 +74,8 @@ test_instance.foo:
 
 // GH-12174
 func TestLocal_refreshNilModuleWithInput(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 	terraform.TestStateFile(t, b.StatePath, testRefreshState())
 
@@ -101,7 +105,8 @@ test_instance.foo:
 }
 
 func TestLocal_refreshInput(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 	terraform.TestStateFile(t, b.StatePath, testRefreshState())
 
@@ -145,7 +150,8 @@ test_instance.foo:
 }
 
 func TestLocal_refreshValidate(t *testing.T) {
-	b := TestLocal(t)
+	b, cleanup := TestLocal(t)
+	defer cleanup()
 	p := TestLocalProvider(t, b, "test")
 	terraform.TestStateFile(t, b.StatePath, testRefreshState())
 

backend/local/testing.go

@@ -2,6 +2,7 @@ package local
 
 import (
 	"io/ioutil"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -15,15 +16,22 @@ import (
 //
 // No operations will be called on the returned value, so you can still set
 // public fields without any locks.
-func TestLocal(t *testing.T) *Local {
+func TestLocal(t *testing.T) (*Local, func()) {
 	tempDir := testTempDir(t)
-	return &Local{
+	local := &Local{
 		StatePath:         filepath.Join(tempDir, "state.tfstate"),
 		StateOutPath:      filepath.Join(tempDir, "state.tfstate"),
 		StateBackupPath:   filepath.Join(tempDir, "state.tfstate.bak"),
 		StateWorkspaceDir: filepath.Join(tempDir, "state.tfstate.d"),
 		ContextOpts:       &terraform.ContextOpts{},
 	}
+	cleanup := func() {
+		if err := os.RemoveAll(tempDir); err != nil {
+			t.Fatal("error clecanup up test:", err)
+		}
+	}
+
+	return local, cleanup
 }
 
 // TestLocalProvider modifies the ContextOpts of the *Local parameter to

command/apply_test.go

@@ -252,10 +252,7 @@ func TestApply_configInvalid(t *testing.T) {
 }
 
 func TestApply_defaultState(t *testing.T) {
-	td, err := ioutil.TempDir("", "tf")
+	td := testTempDir(t)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
 	statePath := filepath.Join(td, DefaultStateFilename)
 
 	// Change to the temporary directory
@@ -735,10 +732,7 @@ func TestApply_planVars(t *testing.T) {
 // we should be able to apply a plan file with no other file dependencies
 func TestApply_planNoModuleFiles(t *testing.T) {
 	// temporary data directory which we can remove between commands
-	td, err := ioutil.TempDir("", "tf")
+	td := testTempDir(t)
-	if err != nil {
-		t.Fatal(err)
-	}
 	defer os.RemoveAll(td)
 
 	defer testChdir(t, td)()

command/command_test.go

@@ -27,6 +27,9 @@ import (
 // This is the directory where our test fixtures are.
 var fixtureDir = "./test-fixtures"
 
+// a top level temp directory which will be cleaned after all tests
+var testingDir string
+
 func init() {
 	test = true
 
@@ -37,9 +40,16 @@ func init() {
 	if err != nil {
 		panic(err)
 	}
+
+	testingDir, err = ioutil.TempDir(testingDir, "tf")
+	if err != nil {
+		panic(err)
+	}
 }
 
 func TestMain(m *testing.M) {
+	defer os.RemoveAll(testingDir)
+
 	flag.Parse()
 	if testing.Verbose() {
 		// if we're verbose, use the logging requested by TF_LOG
@@ -55,7 +65,7 @@ func TestMain(m *testing.M) {
 func tempDir(t *testing.T) string {
 	t.Helper()
 
-	dir, err := ioutil.TempDir("", "tf")
+	dir, err := ioutil.TempDir(testingDir, "tf")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -305,7 +315,7 @@ func testTempFile(t *testing.T) string {
 func testTempDir(t *testing.T) string {
 	t.Helper()
 
-	d, err := ioutil.TempDir("", "tf")
+	d, err := ioutil.TempDir(testingDir, "tf")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -358,7 +368,7 @@ func testChdir(t *testing.T, new string) func() {
 func testCwd(t *testing.T) (string, string) {
 	t.Helper()
 
-	tmp, err := ioutil.TempDir("", "tf")
+	tmp, err := ioutil.TempDir(testingDir, "tf")
 	if err != nil {
 		t.Fatalf("err: %v", err)
 	}
@@ -595,7 +605,7 @@ func testRemoteState(t *testing.T, s *terraform.State, c int) (*terraform.Remote
 // supplied to locate the statelocker.go source.
 func testLockState(sourceDir, path string) (func(), error) {
 	// build and run the binary ourselves so we can quickly terminate it for cleanup
-	buildDir, err := ioutil.TempDir("", "locker")
+	buildDir, err := ioutil.TempDir(testingDir, "locker")
 	if err != nil {
 		return nil, err
 	}

command/debug_json2dot_test.go

@@ -12,7 +12,7 @@ import (
 
 func TestDebugJSON2Dot(t *testing.T) {
 	// create the graph JSON output
-	logFile, err := ioutil.TempFile("", "tf")
+	logFile, err := ioutil.TempFile(testingDir, "tf")
 	if err != nil {
 		t.Fatal(err)
 	}

command/fmt_test.go

@@ -13,11 +13,7 @@ import (
 )
 
 func TestFmt_errorReporting(t *testing.T) {
-	tempDir, err := fmtFixtureWriteDir()
+	tempDir := fmtFixtureWriteDir(t)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.RemoveAll(tempDir)
 
 	ui := new(cli.MockUi)
 	c := &FmtCommand{
@@ -63,11 +59,7 @@ func TestFmt_tooManyArgs(t *testing.T) {
 }
 
 func TestFmt_workingDirectory(t *testing.T) {
-	tempDir, err := fmtFixtureWriteDir()
+	tempDir := fmtFixtureWriteDir(t)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.RemoveAll(tempDir)
 
 	cwd, err := os.Getwd()
 	if err != nil {
@@ -99,11 +91,7 @@ func TestFmt_workingDirectory(t *testing.T) {
 }
 
 func TestFmt_directoryArg(t *testing.T) {
-	tempDir, err := fmtFixtureWriteDir()
+	tempDir := fmtFixtureWriteDir(t)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.RemoveAll(tempDir)
 
 	ui := new(cli.MockUi)
 	c := &FmtCommand{
@@ -149,11 +137,7 @@ func TestFmt_stdinArg(t *testing.T) {
 }
 
 func TestFmt_nonDefaultOptions(t *testing.T) {
-	tempDir, err := fmtFixtureWriteDir()
+	tempDir := fmtFixtureWriteDir(t)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.RemoveAll(tempDir)
 
 	ui := new(cli.MockUi)
 	c := &FmtCommand{
@@ -180,11 +164,7 @@ func TestFmt_nonDefaultOptions(t *testing.T) {
 }
 
 func TestFmt_check(t *testing.T) {
-	tempDir, err := fmtFixtureWriteDir()
+	tempDir := fmtFixtureWriteDir(t)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.RemoveAll(tempDir)
 
 	ui := new(cli.MockUi)
 	c := &FmtCommand{
@@ -244,17 +224,13 @@ var fmtFixture = struct {
 `),
 }
 
-func fmtFixtureWriteDir() (string, error) {
+func fmtFixtureWriteDir(t *testing.T) string {
-	dir, err := ioutil.TempDir("", "tf")
+	dir := testTempDir(t)
+
+	err := ioutil.WriteFile(filepath.Join(dir, fmtFixture.filename), fmtFixture.input, 0644)
 	if err != nil {
-		return "", err
+		t.Fatal(err)
 	}
 
-	err = ioutil.WriteFile(filepath.Join(dir, fmtFixture.filename), fmtFixture.input, 0644)
+	return dir
-	if err != nil {
-		os.RemoveAll(dir)
-		return "", err
-	}
-
-	return dir, nil
 }

command/init_test.go

@@ -998,11 +998,7 @@ func TestInit_providerLockFile(t *testing.T) {
 }
 
 func TestInit_pluginDirReset(t *testing.T) {
-	td, err := ioutil.TempDir("", "tf")
+	td := testTempDir(t)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(td)
 	defer testChdir(t, td)()
 
 	ui := new(cli.MockUi)

command/output_test.go

@@ -1,7 +1,6 @@
 package command
 
 import (
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strings"
@@ -451,10 +450,7 @@ func TestOutput_stateDefault(t *testing.T) {
 
 	// Write the state file in a temporary directory with the
 	// default filename.
-	td, err := ioutil.TempDir("", "tf")
+	td := testTempDir(t)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
 	statePath := filepath.Join(td, DefaultStateFilename)
 
 	f, err := os.Create(statePath)

command/plan_test.go

@@ -196,12 +196,8 @@ func TestPlan_outPath(t *testing.T) {
 	tmp, cwd := testCwd(t)
 	defer testFixCwd(t, tmp, cwd)
 
-	tf, err := ioutil.TempFile("", "tf")
+	td := testTempDir(t)
-	if err != nil {
+	outPath := filepath.Join(td, "test.plan")
-		t.Fatalf("err: %s", err)
-	}
-	outPath := tf.Name()
-	os.Remove(tf.Name())
 
 	p := testProvider()
 	ui := new(cli.MockUi)
@@ -253,12 +249,8 @@ func TestPlan_outPathNoChange(t *testing.T) {
 	}
 	statePath := testStateFile(t, originalState)
 
-	tf, err := ioutil.TempFile("", "tf")
+	td := testTempDir(t)
-	if err != nil {
+	outPath := filepath.Join(td, "test.plan")
-		t.Fatalf("err: %s", err)
-	}
-	outPath := tf.Name()
-	os.Remove(tf.Name())
 
 	p := testProvider()
 	ui := new(cli.MockUi)
@@ -433,20 +425,8 @@ func TestPlan_refresh(t *testing.T) {
 }
 
 func TestPlan_state(t *testing.T) {
-	// Write out some prior state
-	tf, err := ioutil.TempFile("", "tf")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	statePath := tf.Name()
-	defer os.Remove(tf.Name())
-
 	originalState := testState()
-	err = terraform.WriteState(originalState, tf)
+	statePath := testStateFile(t, originalState)
-	tf.Close()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
 
 	p := testProvider()
 	ui := new(cli.MockUi)
@@ -475,24 +455,7 @@ func TestPlan_state(t *testing.T) {
 
 func TestPlan_stateDefault(t *testing.T) {
 	originalState := testState()
-
+	statePath := testStateFile(t, originalState)
-	// Write the state file in a temporary directory with the
-	// default filename.
-	td, err := ioutil.TempDir("", "tf")
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	statePath := filepath.Join(td, DefaultStateFilename)
-
-	f, err := os.Create(statePath)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	err = terraform.WriteState(originalState, f)
-	f.Close()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
 
 	// Change to that directory
 	cwd, err := os.Getwd()
@@ -514,6 +477,7 @@ func TestPlan_stateDefault(t *testing.T) {
 	}
 
 	args := []string{
+		"-state", statePath,
 		testFixturePath("plan"),
 	}
 	if code := c.Run(args); code != 0 {

command/plugins_lock_test.go

@@ -7,7 +7,7 @@ import (
 )
 
 func TestPluginSHA256LockFile(t *testing.T) {
-	f, err := ioutil.TempFile("", "tf-pluginsha1lockfile-test-")
+	f, err := ioutil.TempFile(testingDir, "tf-pluginsha1lockfile-test-")
 	if err != nil {
 		t.Fatalf("failed to create temporary file: %s", err)
 	}

command/plugins_test.go

@@ -2,7 +2,6 @@ package command
 
 import (
 	"fmt"
-	"io/ioutil"
 	"os"
 	"path/filepath"
 	"reflect"
@@ -95,11 +94,7 @@ func TestMultiVersionProviderResolver(t *testing.T) {
 }
 
 func TestPluginPath(t *testing.T) {
-	td, err := ioutil.TempDir("", "tf")
+	td := testTempDir(t)
-	if err != nil {
-		t.Fatal(err)
-	}
-	defer os.RemoveAll(td)
 	defer testChdir(t, td)()
 
 	pluginPath := []string{"a", "b", "c"}

command/refresh_test.go

@@ -189,17 +189,17 @@ func TestRefresh_defaultState(t *testing.T) {
 
 	// Write the state file in a temporary directory with the
 	// default filename.
-	td, err := ioutil.TempDir("", "tf")
+	statePath := testStateFile(t, originalState)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	statePath := filepath.Join(td, DefaultStateFilename)
 
 	localState := &state.LocalState{Path: statePath}
-	if err := localState.WriteState(originalState); err != nil {
+	if err := localState.RefreshState(); err != nil {
 		t.Fatal(err)
 	}
-	serial := localState.State().Serial
+	s := localState.State()
+	if s == nil {
+		t.Fatal("empty test state")
+	}
+	serial := s.Serial
 
 	// Change to that directory
 	cwd, err := os.Getwd()
@@ -224,6 +224,7 @@ func TestRefresh_defaultState(t *testing.T) {
 	p.RefreshReturn = newInstanceState("yes")
 
 	args := []string{
+		"-state", statePath,
 		testFixturePath("refresh"),
 	}
 	if code := c.Run(args); code != 0 {
@@ -364,7 +365,7 @@ func TestRefresh_outPath(t *testing.T) {
 	statePath := testStateFile(t, state)
 
 	// Output path
-	outf, err := ioutil.TempFile("", "tf")
+	outf, err := ioutil.TempFile(testingDir, "tf")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -585,7 +586,7 @@ func TestRefresh_backup(t *testing.T) {
 	statePath := testStateFile(t, state)
 
 	// Output path
-	outf, err := ioutil.TempFile("", "tf")
+	outf, err := ioutil.TempFile(testingDir, "tf")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -594,7 +595,7 @@ func TestRefresh_backup(t *testing.T) {
 	os.Remove(outPath)
 
 	// Backup path
-	backupf, err := ioutil.TempFile("", "tf")
+	backupf, err := ioutil.TempFile(testingDir, "tf")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}
@@ -679,7 +680,7 @@ func TestRefresh_disableBackup(t *testing.T) {
 	statePath := testStateFile(t, state)
 
 	// Output path
-	outf, err := ioutil.TempFile("", "tf")
+	outf, err := ioutil.TempFile(testingDir, "tf")
 	if err != nil {
 		t.Fatalf("err: %s", err)
 	}

command/show_test.go

@@ -1,8 +1,6 @@
 package command
 
 import (
-	"io/ioutil"
-	"os"
 	"path/filepath"
 	"strings"
 	"testing"
@@ -32,31 +30,8 @@ func TestShow(t *testing.T) {
 
 func TestShow_noArgs(t *testing.T) {
 	// Create the default state
-	td, err := ioutil.TempDir("", "tf")
+	statePath := testStateFile(t, testState())
-	if err != nil {
+	defer testChdir(t, filepath.Dir(statePath))()
-		t.Fatalf("err: %s", err)
-	}
-	statePath := filepath.Join(td, DefaultStateFilename)
-
-	f, err := os.Create(statePath)
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	err = terraform.WriteState(testState(), f)
-	f.Close()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-
-	// Change to the temporary directory
-	cwd, err := os.Getwd()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if err := os.Chdir(filepath.Dir(statePath)); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.Chdir(cwd)
 
 	ui := new(cli.MockUi)
 	c := &ShowCommand{
@@ -74,21 +49,8 @@ func TestShow_noArgs(t *testing.T) {
 
 func TestShow_noArgsNoState(t *testing.T) {
 	// Create the default state
-	td, err := ioutil.TempDir("", "tf")
+	statePath := testStateFile(t, testState())
-	if err != nil {
+	defer testChdir(t, filepath.Dir(statePath))()
-		t.Fatalf("err: %s", err)
-	}
-	statePath := filepath.Join(td, DefaultStateFilename)
-
-	// Change to the temporary directory
-	cwd, err := os.Getwd()
-	if err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	if err := os.Chdir(filepath.Dir(statePath)); err != nil {
-		t.Fatalf("err: %s", err)
-	}
-	defer os.Chdir(cwd)
 
 	ui := new(cli.MockUi)
 	c := &ShowCommand{

config/module/get.go

@@ -3,6 +3,7 @@ package module
 import (
 	"io/ioutil"
 	"os"
+	"path/filepath"
 
 	"github.com/hashicorp/go-getter"
 )
@@ -37,13 +38,10 @@ func GetCopy(dst, src string) error {
 	if err != nil {
 		return err
 	}
-	// FIXME: This isn't completely safe. Creating and removing our temp path
-	//        exposes where to race to inject files.
-	if err := os.RemoveAll(tmpDir); err != nil {
-		return err
-	}
 	defer os.RemoveAll(tmpDir)
 
+	tmpDir = filepath.Join(tmpDir, "module")
+
 	// Get to that temporary dir
 	if err := getter.Get(tmpDir, src); err != nil {
 		return err