destroy time eval of resources pending deletion

Allow the evaluation of resource pending deleting only during a full
destroy. With this change we can ensure deposed instances are not
evaluated under normal circumstances, but can be references when needed.
This also allows us to remove the fixup transformer that added
connections so temporary values would evaluate in the correct order when
referencing destroy nodes.

In the majority of cases, we do not want to evaluate resources that are
pending deletion since configuration references only can refer to
resources that is intended to be managed by the configuration. An
exception to that rule is when Terraform is performing a full `destroy`
operation, and providers need to evaluate existing resources for their
configuration.

terraform/evaluate.go

@@ -621,6 +621,11 @@ func (d *evaluationStateData) GetPathAttr(addr addrs.PathAttr, rng tfdiags.Sourc
 	}
 }
 
+// Try to guess if we have a pending destroy for all resources.
+func (s *evaluationStateData) pendingDestroy() bool {
+	return true
+}
+
 func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 	// First we'll consult the configuration to see if an resource of this
@@ -695,6 +700,7 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc
 
 	// Decode all instances in the current state
 	instances := map[addrs.InstanceKey]cty.Value{}
+	pendingDestroy := d.Evaluator.Changes.FullDestroy()
 	for key, is := range rs.Instances {
 		if is == nil || is.Current == nil {
 			// Assume we're dealing with an instance that hasn't been created yet.
@@ -711,18 +717,9 @@ func (d *evaluationStateData) GetResource(addr addrs.Resource, rng tfdiags.Sourc
 			// instances will be in the state, as they are not destroyed until
 			// after their dependants are updated.
 			if change.Action == plans.Delete {
-				// FIXME: we should not be evaluating resources that are going
+				if !pendingDestroy {
-				// to be destroyed, but this needs to happen always since
+					continue
-				// destroy-time provisioners need to reference their self
+				}
-				// value, and providers need to evaluate their configuration
-				// during a full destroy, even of they depend on resources
-				// being destroyed.
-				//
-				// Since this requires a special transformer to try and fixup
-				// the order of evaluation when possible, reference it here to
-				// ensure that we remove the transformer when this is fixed.
-				_ = GraphTransformer((*applyDestroyNodeReferenceFixupTransformer)(nil))
-				// continue
 			}
 		}
 

terraform/graph_builder_apply.go

@@ -185,10 +185,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 		&CloseProviderTransformer{},
 		&CloseProvisionerTransformer{},
 
-		// Add destroy node reference edges where needed, until we can fix
-		// full-destroy evaluation.
-		&applyDestroyNodeReferenceFixupTransformer{},
-
 		// close the root module
 		&CloseRootModuleTransformer{},
 

terraform/transform_reference.go

@@ -540,123 +540,3 @@ func modulePrefixList(result []string, prefix string) []string {
 
 	return result
 }
-
-// destroyNodeReferenceFixupTransformer is a GraphTransformer that connects all
-// temporary values to any destroy instances of their references. This ensures
-// that they are evaluated after the destroy operations of all instances, since
-// the evaluator will currently return data from instances that are scheduled
-// for deletion.
-//
-// This breaks the rules that destroy nodes are not referencable, and can cause
-// cycles in the current graph structure. The cycles however are usually caused
-// by passing through a provider node, and that is the specific case we do not
-// want to wait for destroy evaluation since the evaluation result may need to
-// be used in the provider for a full destroy operation.
-//
-// Once the evaluator can again ignore any instances scheduled for deletion,
-// this transformer should be removed.
-type applyDestroyNodeReferenceFixupTransformer struct{}
-
-func (t *applyDestroyNodeReferenceFixupTransformer) Transform(g *Graph) error {
-	// Create mapping of destroy nodes by address.
-	// Because the values which are providing the references won't yet be
-	// expanded, we need to index these by configuration address, rather than
-	// absolute.
-	destroyers := map[string][]dag.Vertex{}
-	for _, v := range g.Vertices() {
-		if v, ok := v.(GraphNodeDestroyer); ok {
-			addr := v.DestroyAddr().ContainingResource().Config().String()
-			destroyers[addr] = append(destroyers[addr], v)
-		}
-	}
-	_ = destroyers
-
-	// nothing being destroyed
-	if len(destroyers) == 0 {
-		return nil
-	}
-
-	// Now find any temporary values (variables, locals, outputs) that might
-	// reference the resources with instances being destroyed.
-	for _, v := range g.Vertices() {
-		rn, ok := v.(GraphNodeReferencer)
-		if !ok {
-			continue
-		}
-
-		// we only want temporary value referencers
-		if _, ok := v.(graphNodeTemporaryValue); !ok {
-			continue
-		}
-
-		modulePath := rn.ModulePath()
-
-		// If this value is possibly consumed by a provider configuration, we
-		// must attempt to evaluate early during a full destroy, and cannot
-		// wait on the resource destruction. This would also likely cause a
-		// cycle in most configurations.
-		des, _ := g.Descendents(rn)
-		providerDescendant := false
-		for _, v := range des {
-			if _, ok := v.(GraphNodeProvider); ok {
-				providerDescendant = true
-				break
-			}
-		}
-
-		if providerDescendant {
-			log.Printf("[WARN] Value %q has provider descendant, not waiting on referenced destroy instance", dag.VertexName(rn))
-			continue
-		}
-
-		refs := rn.References()
-		for _, ref := range refs {
-
-			var addr addrs.ConfigResource
-			// get the configuration level address for this reference, since
-			// that is how we indexed the destroyers
-			switch tr := ref.Subject.(type) {
-			case addrs.Resource:
-				addr = addrs.ConfigResource{
-					Module:   modulePath,
-					Resource: tr,
-				}
-			case addrs.ResourceInstance:
-				addr = addrs.ConfigResource{
-					Module:   modulePath,
-					Resource: tr.ContainingResource(),
-				}
-			default:
-				// this is not a resource reference
-				continue
-			}
-
-			// see if there are any destroyers registered for this address
-			for _, dest := range destroyers[addr.String()] {
-				// check that we are not introducing a cycle, by looking for
-				// our own node in the ancestors of the destroy node.
-				// This should theoretically only happen if we had a provider
-				// descendant which was checked already, but since this edge is
-				// being added outside the normal rules of the graph, check
-				// again to be certain.
-				anc, _ := g.Ancestors(dest)
-				cycle := false
-				for _, a := range anc {
-					if a == rn {
-						log.Printf("[WARN] Not adding fixup edge %q->%q which introduces a cycle", dag.VertexName(rn), dag.VertexName(dest))
-						cycle = true
-						break
-					}
-				}
-				if cycle {
-					continue
-				}
-
-				log.Printf("[DEBUG] adding fixup edge %q->%q to prevent destroy node evaluation", dag.VertexName(rn), dag.VertexName(dest))
-				g.Connect(dag.BasicEdge(rn, dest))
-			}
-		}
-	}
-
-	return nil
-}