Merge pull request #24120 from hashicorp/alisdair/fix-stale-lock-when-exiting-early

command: Fix stale lock when exiting early

command/console.go

@@ -96,11 +96,6 @@ func (c *ConsoleCommand) Run(args []string) int {
 
 	// Get the context
 	ctx, _, ctxDiags := local.Context(opReq)
-	diags = diags.Append(ctxDiags)
-	if ctxDiags.HasErrors() {
-		c.showDiagnostics(diags)
-		return 1
-	}
 
 	defer func() {
 		err := opReq.StateLocker.Unlock(nil)
@@ -109,6 +104,12 @@ func (c *ConsoleCommand) Run(args []string) int {
 		}
 	}()
 
+	diags = diags.Append(ctxDiags)
+	if ctxDiags.HasErrors() {
+		c.showDiagnostics(diags)
+		return 1
+	}
+
 	// Setup the UI so we can output directly to stdout
 	ui := &cli.BasicUi{
 		Writer:      wrappedstreams.Stdout(),

command/graph.go

@@ -111,13 +111,6 @@ func (c *GraphCommand) Run(args []string) int {
 		return 1
 	}
 
-	defer func() {
-		err := opReq.StateLocker.Unlock(nil)
-		if err != nil {
-			c.Ui.Error(err.Error())
-		}
-	}()
-
 	// Determine the graph type
 	graphType := terraform.GraphTypePlan
 	if plan != nil {

command/import.go

@@ -203,13 +203,7 @@ func (c *ImportCommand) Run(args []string) int {
 
 	// Get the context
 	ctx, state, ctxDiags := local.Context(opReq)
-	diags = diags.Append(ctxDiags)
-	if ctxDiags.HasErrors() {
-		c.showDiagnostics(diags)
-		return 1
-	}
 
-	// Make sure to unlock the state
 	defer func() {
 		err := opReq.StateLocker.Unlock(nil)
 		if err != nil {
@@ -217,6 +211,12 @@ func (c *ImportCommand) Run(args []string) int {
 		}
 	}()
 
+	diags = diags.Append(ctxDiags)
+	if ctxDiags.HasErrors() {
+		c.showDiagnostics(diags)
+		return 1
+	}
+
 	// Perform the import. Note that as you can see it is possible for this
 	// API to import more than one resource at once. For now, we only allow
 	// one while we stabilize this feature.

command/import_test.go

@@ -258,6 +258,75 @@ func TestImport_remoteState(t *testing.T) {
 	testStateOutput(t, statePath, testImportStr)
 }
 
+// early failure on import should not leave stale lock
+func TestImport_initializationErrorShouldUnlock(t *testing.T) {
+	td := tempDir(t)
+	copy.CopyDir(testFixturePath("import-provider-remote-state"), td)
+	defer os.RemoveAll(td)
+	defer testChdir(t, td)()
+
+	statePath := "imported.tfstate"
+
+	// init our backend
+	ui := cli.NewMockUi()
+	m := Meta{
+		testingOverrides: metaOverridesForProvider(testProvider()),
+		Ui:               ui,
+	}
+
+	ic := &InitCommand{
+		Meta: m,
+		providerInstaller: &mockProviderInstaller{
+			Providers: map[string][]string{
+				"test": []string{"1.2.3"},
+			},
+
+			Dir: m.pluginDir(),
+		},
+	}
+
+	// (Using log here rather than t.Log so that these messages interleave with other trace logs)
+	log.Print("[TRACE] TestImport_initializationErrorShouldUnlock running: terraform init")
+	if code := ic.Run([]string{}); code != 0 {
+		t.Fatalf("init failed\n%s", ui.ErrorWriter)
+	}
+
+	// overwrite the config with one including a resource from an invalid provider
+	copy.CopyFile(filepath.Join(testFixturePath("import-provider-invalid"), "main.tf"), filepath.Join(td, "main.tf"))
+
+	p := testProvider()
+	ui = new(cli.MockUi)
+	c := &ImportCommand{
+		Meta: Meta{
+			testingOverrides: metaOverridesForProvider(p),
+			Ui:               ui,
+		},
+	}
+
+	args := []string{
+		"unknown_instance.baz",
+		"bar",
+	}
+	log.Printf("[TRACE] TestImport_initializationErrorShouldUnlock running: terraform import %s %s", args[0], args[1])
+
+	// this should fail
+	if code := c.Run(args); code != 1 {
+		fmt.Println(ui.OutputWriter)
+		t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String())
+	}
+
+	// specifically, it should fail due to a missing provider
+	msg := ui.ErrorWriter.String()
+	if want := "Could not satisfy plugin requirements"; !strings.Contains(msg, want) {
+		t.Errorf("incorrect message\nwant substring: %s\ngot:\n%s", want, msg)
+	}
+
+	// verify that the local state was unlocked after initialization error
+	if _, err := os.Stat(filepath.Join(td, fmt.Sprintf(".%s.lock.info", statePath))); !os.IsNotExist(err) {
+		t.Fatal("state left locked after import")
+	}
+}
+
 func TestImport_providerConfigWithVar(t *testing.T) {
 	defer testChdir(t, testFixturePath("import-provider-var"))()
 

command/testdata/import-provider-invalid/main.tf

@@ -0,0 +1,15 @@
+terraform {
+	backend "local" {
+		path = "imported.tfstate"
+	}
+}
+
+provider "test" {
+    foo = "bar"
+}
+
+resource "test_instance" "foo" {
+}
+
+resource "unknown_instance" "baz" {
+}

command/testdata/import-provider-mismatch/main.tf

@@ -1,7 +0,0 @@
-provider "test-beta" {
-  foo = "baz"
-}
-
-resource "test_instance" "foo" {
-  provider = "test-beta"
-}