diff --git a/terraform/evaluate.go b/terraform/evaluate.go index 5f5c944e6..2b9211288 100644 --- a/terraform/evaluate.go +++ b/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 - // to be destroyed, but this needs to happen always since - // 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 + if !pendingDestroy { + continue + } } } diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 2221c526c..b4e573142 100644 --- a/terraform/graph_builder_apply.go +++ b/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{}, diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index 44aae0fc8..110612e14 100644 --- a/terraform/transform_reference.go +++ b/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 -}