only refresh an import state once

The imported resource was being stored in the wrong state, and only
ended up in the refresh state because ReadResource was being called a
second time in the normal refresh path.

Make sure to only refresh the imported resource once. This is still done
separately within importState so that we can handle the error slightly
differently to let the user know if an imported instance does not exist.

internal/terraform/context_plan2_test.go

@@ -4301,3 +4301,63 @@ import {
 		}
 	})
 }
+
+func TestContext2Plan_importRefreshOnce(t *testing.T) {
+	addr := mustResourceInstanceAddr("test_object.a")
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+resource "test_object" "a" {
+  test_string = "bar"
+}
+
+import {
+  to   = test_object.a
+  id   = "123"
+}
+`,
+	})
+
+	p := simpleMockProvider()
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	readCalled := 0
+	p.ReadResourceFn = func(req providers.ReadResourceRequest) providers.ReadResourceResponse {
+		readCalled++
+		state, _ := simpleTestSchema().CoerceValue(cty.ObjectVal(map[string]cty.Value{
+			"test_string": cty.StringVal("foo"),
+		}))
+
+		return providers.ReadResourceResponse{
+			NewState: state,
+		}
+	}
+
+	p.ImportResourceStateResponse = &providers.ImportResourceStateResponse{
+		ImportedResources: []providers.ImportedResource{
+			{
+				TypeName: "test_object",
+				State: cty.ObjectVal(map[string]cty.Value{
+					"test_string": cty.StringVal("foo"),
+				}),
+			},
+		},
+	}
+
+	_, diags := ctx.Plan(m, states.NewState(), &PlanOpts{
+		Mode: plans.NormalMode,
+		ForceReplace: []addrs.AbsResourceInstance{
+			addr,
+		},
+	})
+	if diags.HasErrors() {
+		t.Fatalf("unexpected errors\n%s", diags.Err().Error())
+	}
+
+	if readCalled > 1 {
+		t.Error("ReadResource called multiple times for import")
+	}
+}

internal/terraform/node_resource_plan_instance.go

@@ -152,9 +152,11 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
 		return diags
 	}
 
+	importing := n.importTarget.ID != ""
+
 	// If the resource is to be imported, we now ask the provider for an Import
 	// and a Refresh, and save the resulting state to instanceRefreshState.
-	if n.importTarget.ID != "" {
+	if importing {
 		instanceRefreshState, diags = n.importState(ctx, addr, provider)
 	} else {
 		var readDiags tfdiags.Diagnostics
@@ -192,7 +194,8 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext)
 	}
 
 	// Refresh, maybe
-	if !n.skipRefresh {
+	// The import process handles its own refresh
+	if !n.skipRefresh && !importing {
 		s, refreshDiags := n.refresh(ctx, states.NotDeposed, instanceRefreshState)
 		diags = diags.Append(refreshDiags)
 		if diags.HasErrors() {
@@ -383,14 +386,15 @@ func (n *NodePlannableResourceInstance) replaceTriggered(ctx EvalContext, repDat
 	return diags
 }
 
-func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.AbsResourceInstance, provider providers.Interface) (instanceRefreshState *states.ResourceInstanceObject, diags tfdiags.Diagnostics) {
+func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.AbsResourceInstance, provider providers.Interface) (*states.ResourceInstanceObject, tfdiags.Diagnostics) {
+	var diags tfdiags.Diagnostics
 	absAddr := addr.Resource.Absolute(ctx.Path())
 
 	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
 		return h.PreImportState(absAddr, n.importTarget.ID)
 	}))
 	if diags.HasErrors() {
-		return instanceRefreshState, diags
+		return nil, diags
 	}
 
 	resp := provider.ImportResourceState(providers.ImportResourceStateRequest{
@@ -399,7 +403,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
 	})
 	diags = diags.Append(resp.Diagnostics)
 	if diags.HasErrors() {
-		return instanceRefreshState, diags
+		return nil, diags
 	}
 
 	imported := resp.ImportedResources
@@ -413,7 +417,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
 				n.importTarget.ID,
 			),
 		))
-		return instanceRefreshState, diags
+		return nil, diags
 	}
 	for _, obj := range imported {
 		log.Printf("[TRACE] graphNodeImportState: import %s %q produced instance object of type %s", absAddr.String(), n.importTarget.ID, obj.TypeName)
@@ -428,7 +432,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
 				n.importTarget.ID,
 			),
 		))
-		return instanceRefreshState, diags
+		return nil, diags
 	}
 
 	// call post-import hook
@@ -438,11 +442,22 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
 
 	if imported[0].TypeName == "" {
 		diags = diags.Append(fmt.Errorf("import of %s didn't set type", n.importTarget.Addr.String()))
-		return instanceRefreshState, diags
+		return nil, diags
 	}
 
 	importedState := imported[0].AsInstanceObject()
 
+	if importedState.Value.IsNull() {
+		diags = diags.Append(tfdiags.Sourceless(
+			tfdiags.Error,
+			"Import returned null resource",
+			fmt.Sprintf("While attempting to import with ID %s, the provider"+
+				"returned an instance with no state.",
+				n.importTarget.ID,
+			),
+		))
+	}
+
 	// refresh
 	riNode := &NodeAbstractResourceInstance{
 		Addr: n.importTarget.Addr,
@@ -450,14 +465,14 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
 			ResolvedProvider: n.ResolvedProvider,
 		},
 	}
-	importedState, refreshDiags := riNode.refresh(ctx, states.NotDeposed, importedState)
+	instanceRefreshState, refreshDiags := riNode.refresh(ctx, states.NotDeposed, importedState)
 	diags = diags.Append(refreshDiags)
 	if diags.HasErrors() {
 		return instanceRefreshState, diags
 	}
 
 	// verify the existence of the imported resource
-	if importedState.Value.IsNull() {
+	if instanceRefreshState.Value.IsNull() {
 		var diags tfdiags.Diagnostics
 		diags = diags.Append(tfdiags.Sourceless(
 			tfdiags.Error,
@@ -475,8 +490,7 @@ func (n *NodePlannableResourceInstance) importState(ctx EvalContext, addr addrs.
 		return instanceRefreshState, diags
 	}
 
-	diags = diags.Append(riNode.writeResourceInstanceState(ctx, importedState, workingState))
-	instanceRefreshState = importedState
+	diags = diags.Append(riNode.writeResourceInstanceState(ctx, instanceRefreshState, refreshState))
 	return instanceRefreshState, diags
 }