Merge pull request #26674 from hashicorp/jbardin/refresh-orphan

Read orphaned instances during plan

terraform/context_apply_test.go

@@ -267,7 +267,6 @@ func TestContext2Apply_resourceDependsOnModule(t *testing.T) {
 	}
 
 	if !reflect.DeepEqual(order, []string{"child", "parent"}) {
-		fmt.Println("ORDER:", order)
 		t.Fatal("resources applied out of order")
 	}
 

terraform/context_refresh_test.go

@@ -1743,3 +1743,54 @@ resource "aws_instance" "bar" {
 		t.Fatal("create_before_destroy not updated in instance state")
 	}
 }
+
+func TestContext2Refresh_dataSourceOrphan(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": ``,
+	})
+
+	state := states.NewState()
+	root := state.EnsureModule(addrs.RootModuleInstance)
+	root.SetResourceInstanceCurrent(
+		addrs.Resource{
+			Mode: addrs.DataResourceMode,
+			Type: "test_data_source",
+			Name: "foo",
+		}.Instance(addrs.NoKey),
+		&states.ResourceInstanceObjectSrc{
+			Status:       states.ObjectReady,
+			AttrsJSON:    []byte(`{"id":"foo"}`),
+			Dependencies: []addrs.ConfigResource{},
+		},
+		addrs.AbsProviderConfig{
+			Provider: addrs.NewDefaultProvider("test"),
+			Module:   addrs.RootModule,
+		},
+	)
+	p := testProvider("test")
+	p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) (resp providers.ReadDataSourceResponse) {
+		resp.State = cty.NullVal(req.Config.Type())
+		return
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		State: state,
+	})
+
+	_, diags := ctx.Refresh()
+	if diags.HasErrors() {
+		t.Fatal(diags.Err())
+	}
+
+	if p.ReadResourceCalled {
+		t.Fatal("there are no managed resources to read")
+	}
+
+	if p.ReadDataSourceCalled {
+		t.Fatal("orphaned data source instance should not be read")
+	}
+}

terraform/graph_builder_plan.go

@@ -191,6 +191,7 @@ func (b *PlanGraphBuilder) init() {
 	b.ConcreteResourceOrphan = func(a *NodeAbstractResourceInstance) dag.Vertex {
 		return &NodePlannableResourceInstanceOrphan{
 			NodeAbstractResourceInstance: a,
+			skipRefresh:                  b.skipRefresh,
 		}
 	}
 }

terraform/node_resource_plan_instance.go

@@ -36,7 +36,7 @@ func (n *NodePlannableResourceInstance) Execute(ctx EvalContext, op walkOperatio
 	// Eval info is different depending on what kind of resource this is
 	switch addr.Resource.Resource.Mode {
 	case addrs.ManagedResourceMode:
-		return n.managedResourceExecute(ctx, n.skipRefresh)
+		return n.managedResourceExecute(ctx)
 	case addrs.DataResourceMode:
 		return n.dataResourceExecute(ctx)
 	default:
@@ -123,7 +123,7 @@ func (n *NodePlannableResourceInstance) dataResourceExecute(ctx EvalContext) err
 	return err
 }
 
-func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext, skipRefresh bool) error {
+func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext) error {
 	config := n.Config
 	addr := n.ResourceInstanceAddr()
 
@@ -162,7 +162,7 @@ func (n *NodePlannableResourceInstance) managedResourceExecute(ctx EvalContext,
 	}
 
 	// Refresh, maybe
-	if !skipRefresh {
+	if !n.skipRefresh {
 		refresh := &EvalRefresh{
 			Addr:           addr.Resource,
 			ProviderAddr:   n.ResolvedProvider,

terraform/node_resource_plan_orphan.go

@@ -1,6 +1,10 @@
 package terraform
 
 import (
+	"fmt"
+	"log"
+
+	"github.com/hashicorp/terraform/addrs"
 	"github.com/hashicorp/terraform/plans"
 	"github.com/hashicorp/terraform/states"
 )
@@ -9,6 +13,8 @@ import (
 // it is ready to be applied and is represented by a diff.
 type NodePlannableResourceInstanceOrphan struct {
 	*NodeAbstractResourceInstance
+
+	skipRefresh bool
 }
 
 var (
@@ -30,12 +36,34 @@ func (n *NodePlannableResourceInstanceOrphan) Name() string {
 func (n *NodePlannableResourceInstanceOrphan) Execute(ctx EvalContext, op walkOperation) error {
 	addr := n.ResourceInstanceAddr()
 
+	// Eval info is different depending on what kind of resource this is
+	switch addr.Resource.Resource.Mode {
+	case addrs.ManagedResourceMode:
+		return n.managedResourceExecute(ctx)
+	case addrs.DataResourceMode:
+		return n.dataResourceExecute(ctx)
+	default:
+		panic(fmt.Errorf("unsupported resource mode %s", n.Config.Mode))
+	}
+}
+
+func (n *NodePlannableResourceInstanceOrphan) dataResourceExecute(ctx EvalContext) error {
+	// A data source that is no longer in the config is removed from the state
+	log.Printf("[TRACE] NodePlannableResourceInstanceOrphan: removing state object for %s", n.Addr)
+	state := ctx.RefreshState()
+	state.SetResourceInstanceCurrent(n.Addr, nil, n.ResolvedProvider)
+	return nil
+}
+
+func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalContext) error {
+	addr := n.ResourceInstanceAddr()
+
 	// Declare a bunch of variables that are used for state during
 	// evaluation. These are written to by-address below.
 	var change *plans.ResourceInstanceChange
 	var state *states.ResourceInstanceObject
 
-	_, providerSchema, err := GetProvider(ctx, n.ResolvedProvider)
+	provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider)
 	if err != nil {
 		return err
 	}
@@ -45,6 +73,40 @@ func (n *NodePlannableResourceInstanceOrphan) Execute(ctx EvalContext, op walkOp
 		return err
 	}
 
+	if !n.skipRefresh {
+		// Refresh this instance even though it is going to be destroyed, in
+		// order to catch missing resources. If this is a normal plan,
+		// providers expect a Read request to remove missing resources from the
+		// plan before apply, and may not handle a missing resource during
+		// Delete correctly.  If this is a simple refresh, Terraform is
+		// expected to remove the missing resource from the state entirely
+		refresh := &EvalRefresh{
+			Addr:           addr.Resource,
+			ProviderAddr:   n.ResolvedProvider,
+			Provider:       &provider,
+			ProviderMetas:  n.ProviderMetas,
+			ProviderSchema: &providerSchema,
+			State:          &state,
+			Output:         &state,
+		}
+		_, err = refresh.Eval(ctx)
+		if err != nil {
+			return err
+		}
+
+		writeRefreshState := &EvalWriteState{
+			Addr:           addr.Resource,
+			ProviderAddr:   n.ResolvedProvider,
+			ProviderSchema: &providerSchema,
+			State:          &state,
+			targetState:    refreshState,
+		}
+		_, err = writeRefreshState.Eval(ctx)
+		if err != nil {
+			return err
+		}
+	}
+
 	diffDestroy := &EvalDiffDestroy{
 		Addr:         addr.Resource,
 		State:        &state,

terraform/node_resource_plan_orphan_test.go

@@ -33,6 +33,7 @@ func TestNodeResourcePlanOrphanExecute(t *testing.T) {
 	p := simpleMockProvider()
 	ctx := &MockEvalContext{
 		StateState:               state.SyncWrapper(),
+		RefreshStateState:        state.SyncWrapper(),
 		InstanceExpanderExpander: instances.NewExpander(),
 		ProviderProvider:         p,
 		ProviderSchemaSchema: &ProviderSchema{

terraform/provider_mock.go

@@ -242,14 +242,20 @@ func (p *MockProvider) ReadResource(r providers.ReadResourceRequest) providers.R
 		return p.ReadResourceFn(r)
 	}
 
-	// make sure the NewState fits the schema
-	newState, err := p.GetSchemaReturn.ResourceTypes[r.TypeName].CoerceValue(p.ReadResourceResponse.NewState)
-	if err != nil {
-		panic(err)
-	}
 	resp := p.ReadResourceResponse
-	resp.NewState = newState
+	if resp.NewState != cty.NilVal {
+		// make sure the NewState fits the schema
+		// This isn't always the case for the existing tests
+		newState, err := p.GetSchemaReturn.ResourceTypes[r.TypeName].CoerceValue(resp.NewState)
+		if err != nil {
+			panic(err)
+		}
+		resp.NewState = newState
+		return resp
+	}
 
+	// just return the same state we received
+	resp.NewState = r.PriorState
 	return resp
 }