diff --git a/terraform/context_apply2_test.go b/terraform/context_apply2_test.go index a9e816fc4b..a739db2ad4 100644 --- a/terraform/context_apply2_test.go +++ b/terraform/context_apply2_test.go @@ -1,11 +1,14 @@ package terraform import ( + "errors" + "fmt" "testing" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/providers" "github.com/hashicorp/terraform/states" + "github.com/zclconf/go-cty/cty" ) // Test that the PreApply hook is called with the correct deposed key @@ -69,3 +72,108 @@ func TestContext2Apply_createBeforeDestroy_deposedKeyPreApply(t *testing.T) { t.Errorf("expected gen to be %q, but was %q", deposedKey, gen) } } + +func TestContext2Apply_destroyWithDataSourceExpansion(t *testing.T) { + // While managed resources store their destroy-time dependencies, data + // sources do not. This means that if a provider were only included in a + // destroy graph because of data sources, it could have dependencies which + // are not correctly ordered. Here we verify that the provider is not + // included in the destroy operation, and all dependency evaluations + // succeed. + + m := testModuleInline(t, map[string]string{ + "main.tf": ` +module "mod" { + source = "./mod" +} + +provider "other" { + foo = module.mod.data +} + +# this should not require the provider be present during destroy +data "other_data_source" "a" { +} +`, + + "mod/main.tf": ` +data "test_data_source" "a" { + count = 1 +} + +data "test_data_source" "b" { + count = data.test_data_source.a[0].foo == "ok" ? 1 : 0 +} + +output "data" { + value = data.test_data_source.a[0].foo == "ok" ? data.test_data_source.b[0].foo : "nope" +} +`, + }) + + testP := testProvider("test") + otherP := testProvider("other") + + readData := func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + return providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("data_source"), + "foo": cty.StringVal("ok"), + }), + } + } + + testP.ReadDataSourceFn = readData + otherP.ReadDataSourceFn = readData + + ps := map[addrs.Provider]providers.Factory{ + addrs.NewDefaultProvider("test"): testProviderFuncFixed(testP), + addrs.NewDefaultProvider("other"): testProviderFuncFixed(otherP), + } + + otherP.ConfigureProviderFn = func(req providers.ConfigureProviderRequest) (resp providers.ConfigureProviderResponse) { + foo := req.Config.GetAttr("foo") + if foo.IsNull() || foo.AsString() != "ok" { + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("incorrect config val: %#v\n", foo)) + } + return resp + } + + ctx := testContext2(t, &ContextOpts{ + Config: m, + Providers: ps, + }) + + _, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + _, diags = ctx.Apply() + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + // now destroy the whole thing + ctx = testContext2(t, &ContextOpts{ + Config: m, + Providers: ps, + Destroy: true, + }) + + _, diags = ctx.Plan() + if diags.HasErrors() { + t.Fatal(diags.Err()) + } + + otherP.ConfigureProviderFn = func(req providers.ConfigureProviderRequest) (resp providers.ConfigureProviderResponse) { + // should not be used to destroy data sources + resp.Diagnostics = resp.Diagnostics.Append(errors.New("provider should not be used")) + return resp + } + + _, diags = ctx.Apply() + if diags.HasErrors() { + t.Fatal(diags.Err()) + } +} diff --git a/terraform/graph_builder_import.go b/terraform/graph_builder_import.go index 8754f3ebc7..e2bd1788f9 100644 --- a/terraform/graph_builder_import.go +++ b/terraform/graph_builder_import.go @@ -84,7 +84,7 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer { // Make sure data sources are aware of any depends_on from the // configuration - &attachDataResourceDependenciesTransformer{}, + &attachDataResourceDependsOnTransformer{}, // Close opened plugin connections &CloseProviderTransformer{}, diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index 2e3a35ccbd..d9e92606b0 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -137,7 +137,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer { // Make sure data sources are aware of any depends_on from the // configuration - &attachDataResourceDependenciesTransformer{}, + &attachDataResourceDependsOnTransformer{}, // Target &TargetsTransformer{Targets: b.Targets}, diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index e152a6cb61..85e9dfeb7f 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -62,7 +62,7 @@ type NodeAbstractResource struct { // Set from GraphNodeTargetable Targets []addrs.Targetable - // Set from AttachResourceDependencies + // Set from AttachDataResourceDependsOn dependsOn []addrs.ConfigResource forceDependsOn bool @@ -71,18 +71,18 @@ type NodeAbstractResource struct { } var ( - _ GraphNodeReferenceable = (*NodeAbstractResource)(nil) - _ GraphNodeReferencer = (*NodeAbstractResource)(nil) - _ GraphNodeProviderConsumer = (*NodeAbstractResource)(nil) - _ GraphNodeProvisionerConsumer = (*NodeAbstractResource)(nil) - _ GraphNodeConfigResource = (*NodeAbstractResource)(nil) - _ GraphNodeAttachResourceConfig = (*NodeAbstractResource)(nil) - _ GraphNodeAttachResourceSchema = (*NodeAbstractResource)(nil) - _ GraphNodeAttachProvisionerSchema = (*NodeAbstractResource)(nil) - _ GraphNodeAttachProviderMetaConfigs = (*NodeAbstractResource)(nil) - _ GraphNodeTargetable = (*NodeAbstractResource)(nil) - _ graphNodeAttachResourceDependencies = (*NodeAbstractResource)(nil) - _ dag.GraphNodeDotter = (*NodeAbstractResource)(nil) + _ GraphNodeReferenceable = (*NodeAbstractResource)(nil) + _ GraphNodeReferencer = (*NodeAbstractResource)(nil) + _ GraphNodeProviderConsumer = (*NodeAbstractResource)(nil) + _ GraphNodeProvisionerConsumer = (*NodeAbstractResource)(nil) + _ GraphNodeConfigResource = (*NodeAbstractResource)(nil) + _ GraphNodeAttachResourceConfig = (*NodeAbstractResource)(nil) + _ GraphNodeAttachResourceSchema = (*NodeAbstractResource)(nil) + _ GraphNodeAttachProvisionerSchema = (*NodeAbstractResource)(nil) + _ GraphNodeAttachProviderMetaConfigs = (*NodeAbstractResource)(nil) + _ GraphNodeTargetable = (*NodeAbstractResource)(nil) + _ graphNodeAttachDataResourceDependsOn = (*NodeAbstractResource)(nil) + _ dag.GraphNodeDotter = (*NodeAbstractResource)(nil) ) // NewNodeAbstractResource creates an abstract resource graph node for @@ -264,8 +264,8 @@ func (n *NodeAbstractResource) SetTargets(targets []addrs.Targetable) { n.Targets = targets } -// graphNodeAttachResourceDependencies -func (n *NodeAbstractResource) AttachResourceDependencies(deps []addrs.ConfigResource, force bool) { +// graphNodeAttachDataResourceDependsOn +func (n *NodeAbstractResource) AttachDataResourceDependsOn(deps []addrs.ConfigResource, force bool) { n.dependsOn = deps n.forceDependsOn = force } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index fa5ba3dc82..09d2dc1d63 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -43,6 +43,14 @@ func (n *NodeDestroyResourceInstance) Name() string { return n.ResourceInstanceAddr().String() + " (destroy)" } +func (n *NodeDestroyResourceInstance) ProvidedBy() (addr addrs.ProviderConfig, exact bool) { + if n.Addr.Resource.Resource.Mode == addrs.DataResourceMode { + // indicate that this node does not require a configured provider + return nil, true + } + return n.NodeAbstractResourceInstance.ProvidedBy() +} + // GraphNodeDestroyer func (n *NodeDestroyResourceInstance) DestroyAddr() *addrs.AbsResourceInstance { addr := n.ResourceInstanceAddr() @@ -126,6 +134,20 @@ func (n *NodeDestroyResourceInstance) References() []*addrs.Reference { func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) (diags tfdiags.Diagnostics) { 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 *NodeDestroyResourceInstance) managedResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { + addr := n.ResourceInstanceAddr() + // Get our state is := n.instanceState if is == nil { @@ -172,7 +194,7 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) } // Run destroy provisioners if not tainted - if state != nil && state.Status != states.ObjectTainted { + if state.Status != states.ObjectTainted { applyProvisionersDiags := n.evalApplyProvisioners(ctx, state, false, configs.ProvisionerWhenDestroy) diags = diags.Append(applyProvisionersDiags) // keep the diags separate from the main set until we handle the cleanup @@ -187,21 +209,15 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) // Managed resources need to be destroyed, while data sources // are only removed from state. - if addr.Resource.Resource.Mode == addrs.ManagedResourceMode { - // we pass a nil configuration to apply because we are destroying - s, d := n.apply(ctx, state, changeApply, nil, false) - state, diags = s, diags.Append(d) - // we don't return immediately here on error, so that the state can be - // finalized - - err := n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState) - if err != nil { - return diags.Append(err) - } - } else { - log.Printf("[TRACE] NodeDestroyResourceInstance: removing state object for %s", n.Addr) - state := ctx.State() - state.SetResourceInstanceCurrent(n.Addr, nil, n.ResolvedProvider) + // we pass a nil configuration to apply because we are destroying + s, d := n.apply(ctx, state, changeApply, nil, false) + state, diags = s, diags.Append(d) + // we don't return immediately here on error, so that the state can be + // finalized + + err = n.writeResourceInstanceState(ctx, state, n.Dependencies, workingState) + if err != nil { + return diags.Append(err) } // create the err value for postApplyHook @@ -209,3 +225,9 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation) diags = diags.Append(updateStateHook(ctx)) return diags } + +func (n *NodeDestroyResourceInstance) dataResourceExecute(ctx EvalContext) (diags tfdiags.Diagnostics) { + log.Printf("[TRACE] NodeDestroyResourceInstance: removing state object for %s", n.Addr) + ctx.State().SetResourceInstanceCurrent(n.Addr, nil, n.ResolvedProvider) + return diags.Append(updateStateHook(ctx)) +} diff --git a/terraform/transform_destroy_edge.go b/terraform/transform_destroy_edge.go index c6ec449fcd..c3f15871b6 100644 --- a/terraform/transform_destroy_edge.go +++ b/terraform/transform_destroy_edge.go @@ -221,6 +221,13 @@ func (t *pruneUnusedNodesTransformer) Transform(g *Graph) error { } } + case GraphNodeProvider: + // Providers that may have been required by expansion nodes + // that we no longer need can also be removed. + if g.UpEdges(n).Len() > 0 { + return + } + default: return } diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index 3760b6c3ad..37937d05e3 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -50,7 +50,7 @@ type graphNodeDependsOn interface { DependsOn() []*addrs.Reference } -// graphNodeAttachResourceDependencies records all resources that are transitively +// graphNodeAttachDataResourceDependsOn records all resources that are transitively // referenced through depends_on in the configuration. This is used by data // resources to determine if they can be read during the plan, or if they need // to be further delayed until apply. @@ -60,18 +60,18 @@ type graphNodeDependsOn interface { // unrelated module instances, the fact that it only happens when there are any // resource updates pending means we can still avoid the problem of the // "perpetual diff" -type graphNodeAttachResourceDependencies interface { +type graphNodeAttachDataResourceDependsOn interface { GraphNodeConfigResource graphNodeDependsOn - // AttachResourceDependencies stored the discovered dependencies in the + // AttachDataResourceDependsOn stores the discovered dependencies in the // resource node for evaluation later. // // The force parameter indicates that even if there are no dependencies, // force the data source to act as though there are for refresh purposes. // This is needed because yet-to-be-created resources won't be in the // initial refresh graph, but may still be referenced through depends_on. - AttachResourceDependencies(deps []addrs.ConfigResource, force bool) + AttachDataResourceDependsOn(deps []addrs.ConfigResource, force bool) } // GraphNodeReferenceOutside is an interface that can optionally be implemented. @@ -157,12 +157,12 @@ func (m depMap) add(v dag.Vertex) { } } -// attachDataResourceDependenciesTransformer records all resources transitively referenced -// through a configuration depends_on. -type attachDataResourceDependenciesTransformer struct { +// attachDataResourceDependsOnTransformer records all resources transitively +// referenced through a configuration depends_on. +type attachDataResourceDependsOnTransformer struct { } -func (t attachDataResourceDependenciesTransformer) Transform(g *Graph) error { +func (t attachDataResourceDependsOnTransformer) Transform(g *Graph) error { // First we need to make a map of referenceable addresses to their vertices. // This is very similar to what's done in ReferenceTransformer, but we keep // implementation separate as they may need to change independently. @@ -170,7 +170,7 @@ func (t attachDataResourceDependenciesTransformer) Transform(g *Graph) error { refMap := NewReferenceMap(vertices) for _, v := range vertices { - depender, ok := v.(graphNodeAttachResourceDependencies) + depender, ok := v.(graphNodeAttachDataResourceDependsOn) if !ok { continue } @@ -195,7 +195,7 @@ func (t attachDataResourceDependenciesTransformer) Transform(g *Graph) error { } log.Printf("[TRACE] attachDataDependenciesTransformer: %s depends on %s", depender.ResourceAddr(), res) - depender.AttachResourceDependencies(res, fromModule) + depender.AttachDataResourceDependsOn(res, fromModule) } return nil