From 40f09027f02462f5e2fd4be9fe5f7246c9530e3f Mon Sep 17 00:00:00 2001 From: James Bardin Date: Fri, 20 Mar 2020 15:19:01 -0400 Subject: [PATCH] expand planned resources While the Expander itself now handles the recursive expansion of modules, Resources themselves still need to be expanded twice, because the evaluation of the Resource, which entails evaluating the for_each or count expressions, is separate from the ResourceInstance expansion. Add a nodeExpandPlannableResource to do handle this expansion to allow all NodePlannableResources to call EvalWriteResourceState with an absolute address. --- terraform/eval_context_builtin_test.go | 5 +- terraform/eval_output.go | 4 +- terraform/eval_state.go | 36 ++++++------- terraform/graph_builder_plan.go | 2 +- terraform/node_output.go | 5 +- terraform/node_output_orphan.go | 2 +- terraform/node_resource_apply.go | 2 +- terraform/node_resource_plan.go | 75 +++++++++++++++++++++++++- terraform/transform_output.go | 3 +- 9 files changed, 101 insertions(+), 33 deletions(-) diff --git a/terraform/eval_context_builtin_test.go b/terraform/eval_context_builtin_test.go index bdcd09484..c6d27d498 100644 --- a/terraform/eval_context_builtin_test.go +++ b/terraform/eval_context_builtin_test.go @@ -15,12 +15,12 @@ func TestBuiltinEvalContextProviderInput(t *testing.T) { cache := make(map[string]map[string]cty.Value) ctx1 := testBuiltinEvalContext(t) - ctx1.PathValue = addrs.RootModuleInstance + ctx1 = ctx1.WithPath(addrs.RootModuleInstance).(*BuiltinEvalContext) ctx1.ProviderInputConfig = cache ctx1.ProviderLock = &lock ctx2 := testBuiltinEvalContext(t) - ctx2.PathValue = addrs.RootModuleInstance.Child("child", addrs.NoKey) + ctx2 = ctx2.WithPath(addrs.RootModuleInstance.Child("child", addrs.NoKey)).(*BuiltinEvalContext) ctx2.ProviderInputConfig = cache ctx2.ProviderLock = &lock @@ -56,6 +56,7 @@ func TestBuildingEvalContextInitProvider(t *testing.T) { testP := &MockProvider{} ctx := testBuiltinEvalContext(t) + ctx = ctx.WithPath(addrs.RootModuleInstance).(*BuiltinEvalContext) ctx.ProviderLock = &lock ctx.ProviderCache = make(map[string]providers.Interface) ctx.Components = &basicComponentFactory{ diff --git a/terraform/eval_output.go b/terraform/eval_output.go index 181e55635..f1a195f6d 100644 --- a/terraform/eval_output.go +++ b/terraform/eval_output.go @@ -15,7 +15,7 @@ import ( // EvalDeleteOutput is an EvalNode implementation that deletes an output // from the state. type EvalDeleteOutput struct { - Addr addrs.OutputValue + Addr addrs.AbsOutputValue } // TODO: test @@ -25,7 +25,7 @@ func (n *EvalDeleteOutput) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } - state.RemoveOutputValue(n.Addr.Absolute(ctx.Path())) + state.RemoveOutputValue(n.Addr) return nil, nil } diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 8f134d465..16655080e 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -452,16 +452,20 @@ func (n *EvalMaybeRestoreDeposedObject) Eval(ctx EvalContext) (interface{}, erro // in that case, allowing expression evaluation to see it as a zero-element // list rather than as not set at all. type EvalWriteResourceState struct { - Addr addrs.ConfigResource + Addr addrs.AbsResource Config *configs.Resource ProviderAddr addrs.AbsProviderConfig } -// TODO: test func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) { var diags tfdiags.Diagnostics state := ctx.State() + // We'll record our expansion decision in the shared "expander" object + // so that later operations (i.e. DynamicExpand and expression evaluation) + // can refer to it. Since this node represents the abstract module, we need + // to expand the module here to create all resources. + expander := ctx.InstanceExpander() count, countDiags := evaluateResourceCountExpression(n.Config.Count, ctx) diags = diags.Append(countDiags) if countDiags.HasErrors() { @@ -482,25 +486,17 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) { if forEach != nil { eachMode = states.EachMap } + // This method takes care of all of the business logic of updating this + // while ensuring that any existing instances are preserved, etc. + state.SetResourceMeta(n.Addr, eachMode, n.ProviderAddr) - // We'll record our expansion decision in the shared "expander" object - // so that later operations (i.e. DynamicExpand and expression evaluation) - // can refer to it. Since this node represents the abstract module, we need - // to expand the module here to create all resources. - expander := ctx.InstanceExpander() - for _, module := range expander.ExpandModule(n.Addr.Module) { - // This method takes care of all of the business logic of updating this - // while ensuring that any existing instances are preserved, etc. - state.SetResourceMeta(n.Addr.Absolute(module), eachMode, n.ProviderAddr) - - switch eachMode { - case states.EachList: - expander.SetResourceCount(module, n.Addr.Resource, count) - case states.EachMap: - expander.SetResourceForEach(module, n.Addr.Resource, forEach) - default: - expander.SetResourceSingle(module, n.Addr.Resource) - } + switch eachMode { + case states.EachList: + expander.SetResourceCount(n.Addr.Module, n.Addr.Resource, count) + case states.EachMap: + expander.SetResourceForEach(n.Addr.Module, n.Addr.Resource, forEach) + default: + expander.SetResourceSingle(n.Addr.Module, n.Addr.Resource) } return nil, nil diff --git a/terraform/graph_builder_plan.go b/terraform/graph_builder_plan.go index cbf3d2670..baeda03f5 100644 --- a/terraform/graph_builder_plan.go +++ b/terraform/graph_builder_plan.go @@ -196,7 +196,7 @@ func (b *PlanGraphBuilder) init() { } b.ConcreteResource = func(a *NodeAbstractResource) dag.Vertex { - return &NodePlannableResource{ + return &nodeExpandPlannableResource{ NodeAbstractResource: a, } } diff --git a/terraform/node_output.go b/terraform/node_output.go index 6aa91da87..ce9fc0d22 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -235,8 +235,7 @@ func (n *NodeApplyableOutput) DotNode(name string, opts *dag.DotOpts) *dag.DotNo // NodeDestroyableOutput represents an output that is "destroybale": // its application will remove the output from the state. type NodeDestroyableOutput struct { - Addr addrs.OutputValue - Module addrs.Module + Addr addrs.AbsOutputValue Config *configs.Output // Config is the output in the config } @@ -254,7 +253,7 @@ func (n *NodeDestroyableOutput) Name() string { // GraphNodeModulePath func (n *NodeDestroyableOutput) ModulePath() addrs.Module { - return n.Module + return n.Addr.Module.Module() } // RemovableIfNotTargeted diff --git a/terraform/node_output_orphan.go b/terraform/node_output_orphan.go index 060f27ea7..bb13234b6 100644 --- a/terraform/node_output_orphan.go +++ b/terraform/node_output_orphan.go @@ -47,7 +47,7 @@ func (n *NodeOutputOrphan) EvalTree() EvalNode { return &EvalOpFilter{ Ops: []walkOperation{walkRefresh, walkApply, walkDestroy}, Node: &EvalDeleteOutput{ - Addr: n.Addr.OutputValue, + Addr: n.Addr, }, } } diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index 2423ad04b..b323db732 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -60,7 +60,7 @@ func (n *NodeApplyableResource) EvalTree() EvalNode { } return &EvalWriteResourceState{ - Addr: n.Addr, + Addr: n.Addr.Resource.Absolute(n.Addr.Module.UnkeyedInstanceShim()), Config: n.Config, ProviderAddr: n.ResolvedProvider, } diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 73e059a64..df057b468 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -3,15 +3,82 @@ package terraform import ( "log" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/tfdiags" ) +// nodeExpandPlannableResource handles the first layer of resource +// expansion. We need this extra layer so DynamicExpand is called twice for +// the resource, the first to expand the Resource for each module instance, and +// the second to expand each ResourceInstnace for the expanded Resources. +// +// Even though the Expander can handle this recursive expansion, the +// EvalWriteState nodes need to be expanded and Evaluated first, and our +// current graph doesn't allow evaluation within DynamicExpand, and doesn't +// call it recursively. +type nodeExpandPlannableResource struct { + *NodeAbstractResource + + // TODO: can we eliminate the need for this flag and combine this with the + // apply expander? + // ForceCreateBeforeDestroy might be set via our GraphNodeDestroyerCBD + // during graph construction, if dependencies require us to force this + // on regardless of what the configuration says. + ForceCreateBeforeDestroy *bool +} + +var ( + _ GraphNodeDestroyerCBD = (*nodeExpandPlannableResource)(nil) + _ GraphNodeDynamicExpandable = (*nodeExpandPlannableResource)(nil) + _ GraphNodeReferenceable = (*nodeExpandPlannableResource)(nil) + _ GraphNodeReferencer = (*nodeExpandPlannableResource)(nil) + _ GraphNodeConfigResource = (*nodeExpandPlannableResource)(nil) + _ GraphNodeAttachResourceConfig = (*nodeExpandPlannableResource)(nil) +) + +// GraphNodeDestroyerCBD +func (n *nodeExpandPlannableResource) CreateBeforeDestroy() bool { + if n.ForceCreateBeforeDestroy != nil { + return *n.ForceCreateBeforeDestroy + } + + // If we have no config, we just assume no + if n.Config == nil || n.Config.Managed == nil { + return false + } + + return n.Config.Managed.CreateBeforeDestroy +} + +// GraphNodeDestroyerCBD +func (n *nodeExpandPlannableResource) ModifyCreateBeforeDestroy(v bool) error { + n.ForceCreateBeforeDestroy = &v + return nil +} + +func (n *nodeExpandPlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { + var g Graph + + expander := ctx.InstanceExpander() + for _, module := range expander.ExpandModule(n.Addr.Module) { + g.Add(&NodePlannableResource{ + NodeAbstractResource: n.NodeAbstractResource, + Addr: n.Addr.Resource.Absolute(module), + ForceCreateBeforeDestroy: n.ForceCreateBeforeDestroy, + }) + } + + return &g, nil +} + // NodePlannableResource represents a resource that is "plannable": // it is ready to be planned in order to create a diff. type NodePlannableResource struct { *NodeAbstractResource + Addr addrs.AbsResource + // ForceCreateBeforeDestroy might be set via our GraphNodeDestroyerCBD // during graph construction, if dependencies require us to force this // on regardless of what the configuration says. @@ -19,6 +86,7 @@ type NodePlannableResource struct { } var ( + _ GraphNodeModuleInstance = (*NodePlannableResource)(nil) _ GraphNodeDestroyerCBD = (*NodePlannableResource)(nil) _ GraphNodeDynamicExpandable = (*NodePlannableResource)(nil) _ GraphNodeReferenceable = (*NodePlannableResource)(nil) @@ -27,6 +95,11 @@ var ( _ GraphNodeAttachResourceConfig = (*NodePlannableResource)(nil) ) +// GraphNodeModuleInstance +func (n *NodePlannableResource) ModuleInstance() addrs.ModuleInstance { + return n.Addr.Module +} + // GraphNodeEvalable func (n *NodePlannableResource) EvalTree() EvalNode { if n.Config == nil { @@ -85,7 +158,7 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { if countDiags.HasErrors() { return nil, diags.Err() } - fixResourceCountSetTransition(ctx, n.ResourceAddr(), count != -1) + fixResourceCountSetTransition(ctx, n.Addr.Config(), count != -1) // Our graph transformers require access to the full state, so we'll // temporarily lock it while we work on this. diff --git a/terraform/transform_output.go b/terraform/transform_output.go index 7eb8e4884..d594b9f1d 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -79,8 +79,7 @@ func (t *DestroyOutputTransformer) Transform(g *Graph) error { // create the destroy node for this output node := &NodeDestroyableOutput{ - Addr: output.Addr, - Module: output.Module, + Addr: output.Addr.Absolute(addrs.RootModuleInstance), Config: output.Config, }