diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index f5b28da2b4..071c80db9e 100644 --- a/terraform/context_apply_test.go +++ b/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") } diff --git a/terraform/context_refresh_test.go b/terraform/context_refresh_test.go index 77535d7355..b8efe480c0 100644 --- a/terraform/context_refresh_test.go +++ b/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") + } +} diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index 7e19e13434..354ee53880 100644 --- a/terraform/graph_builder_plan.go +++ b/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, } } } diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 3ffe994600..3810484d05 100644 --- a/terraform/node_resource_plan_instance.go +++ b/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, diff --git a/terraform/node_resource_plan_orphan.go b/terraform/node_resource_plan_orphan.go index ca02000545..515caec58d 100644 --- a/terraform/node_resource_plan_orphan.go +++ b/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, diff --git a/terraform/node_resource_plan_orphan_test.go b/terraform/node_resource_plan_orphan_test.go index 45078cd436..f4396448ed 100644 --- a/terraform/node_resource_plan_orphan_test.go +++ b/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{ diff --git a/terraform/provider_mock.go b/terraform/provider_mock.go index 45397fe22a..bf3aca3274 100644 --- a/terraform/provider_mock.go +++ b/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 }