Merge pull request #33151 from hashicorp/jbardin/import-refresh

Import: only refresh an imported state once

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
 }