Merge pull request #26495 from hashicorp/jbardin/evaluate-walks

Audit some graph building and GraphNodeExecutable

terraform/context_eval_test.go

@@ -38,7 +38,7 @@ func TestContextEval(t *testing.T) {
 		},
 		{
 			`module.child.result`,
-			cty.UnknownVal(cty.Number),
+			cty.NumberIntVal(6),
 			false,
 		},
 	}

terraform/graph_builder_apply.go

@@ -87,6 +87,12 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 			Config:   b.Config,
 		},
 
+		// Add dynamic values
+		&RootVariableTransformer{Config: b.Config},
+		&ModuleVariableTransformer{Config: b.Config},
+		&LocalTransformer{Config: b.Config},
+		&OutputTransformer{Config: b.Config},
+
 		// Creates all the resource instances represented in the diff, along
 		// with dependency edges against the whole-resource nodes added by
 		// ConfigTransformer above.
@@ -96,31 +102,19 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 			Changes:  b.Changes,
 		},
 
+		// Attach the state
+		&AttachStateTransformer{State: b.State},
+
 		// Create orphan output nodes
 		&OrphanOutputTransformer{Config: b.Config, State: b.State},
 
 		// Attach the configuration to any resources
 		&AttachResourceConfigTransformer{Config: b.Config},
 
-		// Attach the state
-		&AttachStateTransformer{State: b.State},
-
 		// Provisioner-related transformations
 		&MissingProvisionerTransformer{Provisioners: b.Components.ResourceProvisioners()},
 		&ProvisionerTransformer{},
 
-		// Add root variables
-		&RootVariableTransformer{Config: b.Config},
-
-		// Add the local values
-		&LocalTransformer{Config: b.Config},
-
-		// Add the outputs
-		&OutputTransformer{Config: b.Config},
-
-		// Add module variables
-		&ModuleVariableTransformer{Config: b.Config},
-
 		// add providers
 		TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config),
 
@@ -150,7 +144,6 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer {
 			State:   b.State,
 			Schemas: b.Schemas,
 		},
-
 		&CBDEdgeTransformer{
 			Config:  b.Config,
 			State:   b.State,

terraform/graph_builder_destroy_plan.go

@@ -92,8 +92,13 @@ func (b *DestroyPlanGraphBuilder) Steps() []GraphTransformer {
 		// created proper destroy ordering.
 		&TargetsTransformer{Targets: b.Targets},
 
+		// Close opened plugin connections
+		&CloseProviderTransformer{},
+
 		// Close the root module
 		&CloseRootModuleTransformer{},
+
+		&TransitiveReductionTransformer{},
 	}
 
 	return steps

terraform/graph_builder_eval.go

@@ -60,28 +60,20 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer {
 		// Creates all the data resources that aren't in the state. This will also
 		// add any orphans from scaling in as destroy nodes.
 		&ConfigTransformer{
-			Concrete: nil, // just use the abstract type
 			Config: b.Config,
-			Unique:   true,
 		},
 
-		// Attach the state
-		&AttachStateTransformer{State: b.State},
+		// Add dynamic values
+		&RootVariableTransformer{Config: b.Config},
+		&ModuleVariableTransformer{Config: b.Config},
+		&LocalTransformer{Config: b.Config},
+		&OutputTransformer{Config: b.Config},
 
 		// Attach the configuration to any resources
 		&AttachResourceConfigTransformer{Config: b.Config},
 
-		// Add root variables
-		&RootVariableTransformer{Config: b.Config},
-
-		// Add the local values
-		&LocalTransformer{Config: b.Config},
-
-		// Add the outputs
-		&OutputTransformer{Config: b.Config},
-
-		// Add module variables
-		&ModuleVariableTransformer{Config: b.Config},
+		// Attach the state
+		&AttachStateTransformer{State: b.State},
 
 		TransformProviders(b.Components.ResourceProviders(), concreteProvider, b.Config),
 
@@ -92,9 +84,7 @@ func (b *EvalGraphBuilder) Steps() []GraphTransformer {
 		// Create expansion nodes for all of the module calls. This must
 		// come after all other transformers that create nodes representing
 		// objects that can belong to modules.
-		&ModuleExpansionTransformer{
-			Config: b.Config,
-		},
+		&ModuleExpansionTransformer{Config: b.Config},
 
 		// Connect so that the references are ready for targeting. We'll
 		// have to connect again later for providers and so on.

terraform/graph_builder_import.go

@@ -55,26 +55,20 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer {
 		// Create all our resources from the configuration and state
 		&ConfigTransformer{Config: config},
 
+		// Add dynamic values
+		&RootVariableTransformer{Config: b.Config},
+		&ModuleVariableTransformer{Config: b.Config},
+		&LocalTransformer{Config: b.Config},
+		&OutputTransformer{Config: b.Config},
+
 		// Attach the configuration to any resources
 		&AttachResourceConfigTransformer{Config: b.Config},
 
 		// Add the import steps
 		&ImportStateTransformer{Targets: b.ImportTargets, Config: b.Config},
 
-		// Add root variables
-		&RootVariableTransformer{Config: b.Config},
-
 		TransformProviders(b.Components.ResourceProviders(), concreteProvider, config),
 
-		// Add the local values
-		&LocalTransformer{Config: b.Config},
-
-		// Add the outputs
-		&OutputTransformer{Config: b.Config},
-
-		// Add module variables
-		&ModuleVariableTransformer{Config: b.Config},
-
 		// Must attach schemas before ReferenceTransformer so that we can
 		// analyze the configuration to find references.
 		&AttachSchemaTransformer{Schemas: b.Schemas, Config: b.Config},
@@ -82,9 +76,7 @@ func (b *ImportGraphBuilder) Steps() []GraphTransformer {
 		// Create expansion nodes for all of the module calls. This must
 		// come after all other transformers that create nodes representing
 		// objects that can belong to modules.
-		&ModuleExpansionTransformer{
-			Config: b.Config,
-		},
+		&ModuleExpansionTransformer{Config: b.Config},
 
 		// Connect so that the references are ready for targeting. We'll
 		// have to connect again later for providers and so on.

terraform/graph_builder_plan.go

@@ -84,10 +84,10 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 			Config:   b.Config,
 		},
 
-		// Add the local values
+		// Add dynamic values
+		&RootVariableTransformer{Config: b.Config},
+		&ModuleVariableTransformer{Config: b.Config},
 		&LocalTransformer{Config: b.Config},
-
-		// Add the outputs
 		&OutputTransformer{Config: b.Config},
 
 		// Add orphan resources
@@ -106,29 +106,20 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 			State:           b.State,
 		},
 
+		// Attach the state
+		&AttachStateTransformer{State: b.State},
+
 		// Create orphan output nodes
-		&OrphanOutputTransformer{
-			Config: b.Config,
-			State:  b.State,
-		},
+		&OrphanOutputTransformer{Config: b.Config, State: b.State},
 
 		// Attach the configuration to any resources
 		&AttachResourceConfigTransformer{Config: b.Config},
 
-		// Attach the state
-		&AttachStateTransformer{State: b.State},
-
-		// Add root variables
-		&RootVariableTransformer{Config: b.Config},
-
+		// Provisioner-related transformations
 		&MissingProvisionerTransformer{Provisioners: b.Components.ResourceProvisioners()},
 		&ProvisionerTransformer{},
 
-		// Add module variables
-		&ModuleVariableTransformer{
-			Config: b.Config,
-		},
-
+		// add providers
 		TransformProviders(b.Components.ResourceProviders(), b.ConcreteProvider, b.Config),
 
 		// Remove modules no longer present in the config
@@ -141,10 +132,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 		// Create expansion nodes for all of the module calls. This must
 		// come after all other transformers that create nodes representing
 		// objects that can belong to modules.
-		&ModuleExpansionTransformer{
-			Concrete: b.ConcreteModule,
-			Config:   b.Config,
-		},
+		&ModuleExpansionTransformer{Concrete: b.ConcreteModule, Config: b.Config},
 
 		// Connect so that the references are ready for targeting. We'll
 		// have to connect again later for providers and so on.
@@ -156,9 +144,7 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 		&attachDataResourceDependenciesTransformer{},
 
 		// Target
-		&TargetsTransformer{
-			Targets: b.Targets,
-		},
+		&TargetsTransformer{Targets: b.Targets},
 
 		// Detect when create_before_destroy must be forced on for a particular
 		// node due to dependency edges, to avoid graph cycles during apply.
@@ -171,7 +157,6 @@ func (b *PlanGraphBuilder) Steps() []GraphTransformer {
 
 		// Close opened plugin connections
 		&CloseProviderTransformer{},
-		&CloseProvisionerTransformer{},
 
 		// Close the root module
 		&CloseRootModuleTransformer{},

terraform/node_module_variable.go

@@ -153,13 +153,13 @@ func (n *nodeModuleVariable) Execute(ctx EvalContext, op walkOperation) error {
 	var err error
 
 	switch op {
-	case walkPlan, walkApply, walkDestroy:
-		vals, err = n.EvalModuleCallArgument(ctx, false)
+	case walkValidate:
+		vals, err = n.EvalModuleCallArgument(ctx, true)
 		if err != nil {
 			return err
 		}
-	case walkValidate:
-		vals, err = n.EvalModuleCallArgument(ctx, true)
+	default:
+		vals, err = n.EvalModuleCallArgument(ctx, false)
 		if err != nil {
 			return err
 		}

terraform/node_output.go

@@ -199,9 +199,6 @@ func (n *NodeApplyableOutput) References() []*addrs.Reference {
 
 // GraphNodeExecutable
 func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) error {
-	switch op {
-	// Everything except walkImport
-	case walkEval, walkPlan, walkApply, walkValidate, walkDestroy, walkPlanDestroy:
 	// This has to run before we have a state lock, since evaluation also
 	// reads the state
 	val, diags := ctx.EvaluateExpr(n.Config.Expr, cty.DynamicPseudoType, nil)
@@ -251,9 +248,6 @@ func (n *NodeApplyableOutput) Execute(ctx EvalContext, op walkOperation) error {
 	}
 
 	return nil
-	default:
-		return nil
-	}
 }
 
 // dag.GraphNodeDotter impl.

terraform/node_resource_destroy.go

@@ -136,8 +136,6 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 	var state *states.ResourceInstanceObject
 	var provisionerErr error
 
-	switch op {
-	case walkApply, walkDestroy:
 	provider, providerSchema, err := GetProvider(ctx, n.ResolvedProvider)
 	if err != nil {
 		return err
@@ -263,7 +261,6 @@ func (n *NodeDestroyResourceInstance) Execute(ctx EvalContext, op walkOperation)
 	if err != nil {
 		return err
 	}
-	}
 
 	return nil
 }

terraform/node_resource_destroy_deposed.go

@@ -76,9 +76,6 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk
 	var change *plans.ResourceInstanceChange
 	var state *states.ResourceInstanceObject
 
-	switch op {
-	case walkPlan, walkPlanDestroy:
-
 	readStateDeposed := &EvalReadStateDeposed{
 		Addr:           addr.Resource,
 		Output:         &state,
@@ -113,7 +110,7 @@ func (n *NodePlanDeposedResourceInstanceObject) Execute(ctx EvalContext, op walk
 	if err != nil {
 		return err
 	}
-	}
+
 	return nil
 }
 

terraform/transform_config.go

@@ -25,9 +25,6 @@ type ConfigTransformer struct {
 	// Module is the module to add resources from.
 	Config *configs.Config
 
-	// Unique will only add resources that aren't already present in the graph.
-	Unique bool
-
 	// Mode will only add resources that match the given mode
 	ModeFilter bool
 	Mode       addrs.ResourceMode

terraform/transform_reference.go

@@ -301,7 +301,7 @@ func (m ReferenceMap) References(v dag.Vertex) []dag.Vertex {
 			case addrs.ModuleCallInstance:
 				subject = ri.Call
 			default:
-				log.Printf("[WARN] ReferenceTransformer: reference not found: %q", subject)
+				log.Printf("[INFO] ReferenceTransformer: reference not found: %q", subject)
 				continue
 			}
 			key = m.referenceMapKey(v, subject)