From fae5f9958d2e18877d61c207cea9f2875ebbebef Mon Sep 17 00:00:00 2001 From: James Bardin Date: Mon, 9 Mar 2020 15:43:56 -0400 Subject: [PATCH] remove GraphNodeModuleInstance from Resource types Remove the requirement for most *Resource types to be a GraphNodeModuleInstance, ensuring that they never call ctx.Path while being evaluated. This gets rid of most of the direct need for the Path method currently implemented by NodeResourceAbstract, leaving the provider and schema related calls for a subsequent PR. --- terraform/eval_state.go | 12 ++++++------ terraform/node_data_refresh.go | 24 ++++++++++++++---------- terraform/node_resource_abstract.go | 10 ++++++++-- terraform/node_resource_apply.go | 15 ++++++--------- terraform/node_resource_destroy.go | 6 +----- terraform/node_resource_plan.go | 13 +++++-------- terraform/node_resource_refresh.go | 8 +++----- terraform/node_resource_validate.go | 1 - 8 files changed, 43 insertions(+), 46 deletions(-) diff --git a/terraform/eval_state.go b/terraform/eval_state.go index ab55835e2..ccf63d1e4 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -453,6 +453,7 @@ func (n *EvalMaybeRestoreDeposedObject) Eval(ctx EvalContext) (interface{}, erro // list rather than as not set at all. type EvalWriteResourceState struct { Addr addrs.Resource + Module addrs.Module Config *configs.Resource ProviderAddr addrs.AbsProviderConfig } @@ -460,7 +461,6 @@ type EvalWriteResourceState struct { // TODO: test func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) { var diags tfdiags.Diagnostics - absAddr := n.Addr.Absolute(ctx.Path()) state := ctx.State() count, countDiags := evaluateResourceCountExpression(n.Config.Count, ctx) @@ -484,16 +484,16 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) { 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(absAddr, 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(ctx.Path().Module()) { + for _, module := range expander.ExpandModule(n.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, count) diff --git a/terraform/node_data_refresh.go b/terraform/node_data_refresh.go index dd0b43352..4e2460745 100644 --- a/terraform/node_data_refresh.go +++ b/terraform/node_data_refresh.go @@ -1,6 +1,7 @@ package terraform import ( + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/dag" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/providers" @@ -15,7 +16,6 @@ type NodeRefreshableDataResource struct { } var ( - _ GraphNodeModuleInstance = (*NodeRefreshableDataResource)(nil) _ GraphNodeDynamicExpandable = (*NodeRefreshableDataResource)(nil) _ GraphNodeReferenceable = (*NodeRefreshableDataResource)(nil) _ GraphNodeReferencer = (*NodeRefreshableDataResource)(nil) @@ -54,18 +54,22 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er // if we're transitioning whether "count" is set at all. fixResourceCountSetTransition(ctx, n.ResourceAddr(), count != -1) + var instanceAddrs []addrs.AbsResourceInstance + // Inform our instance expander about our expansion results above, // and then use it to calculate the instance addresses we'll expand for. expander := ctx.InstanceExpander() - switch { - case count >= 0: - expander.SetResourceCount(ctx.Path(), n.ResourceAddr().Resource, count) - case forEachMap != nil: - expander.SetResourceForEach(ctx.Path(), n.ResourceAddr().Resource, forEachMap) - default: - expander.SetResourceSingle(ctx.Path(), n.ResourceAddr().Resource) + for _, path := range expander.ExpandModule(n.Module) { + switch { + case count >= 0: + expander.SetResourceCount(path, n.ResourceAddr().Resource, count) + case forEachMap != nil: + expander.SetResourceForEach(path, n.ResourceAddr().Resource, forEachMap) + default: + expander.SetResourceSingle(path, n.ResourceAddr().Resource) + } + instanceAddrs = append(instanceAddrs, expander.ExpandResource(path.Module(), n.ResourceAddr().Resource)...) } - instanceAddrs := expander.ExpandResource(ctx.Path().Module(), n.ResourceAddr().Resource) // Our graph transformers require access to the full state, so we'll // temporarily lock it while we work on this. @@ -135,7 +139,7 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er Name: "NodeRefreshableDataResource", } - graph, diags := b.Build(ctx.Path()) + graph, diags := b.Build(nil) return graph, diags.ErrWithWarnings() } diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index d67bd5953..687fa0f1a 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -99,7 +99,8 @@ func NewNodeAbstractResource(addr addrs.AbsResource) *NodeAbstractResource { // the "count" or "for_each" arguments. type NodeAbstractResourceInstance struct { NodeAbstractResource - InstanceKey addrs.InstanceKey + ModuleInstance addrs.ModuleInstance + InstanceKey addrs.InstanceKey // The fields below will be automatically set using the Attach // interfaces if you're running those transforms, but also be explicitly @@ -138,7 +139,8 @@ func NewNodeAbstractResourceInstance(addr addrs.AbsResourceInstance) *NodeAbstra Addr: addr.Resource.Resource, Module: addr.Module.Module(), }, - InstanceKey: addr.Resource.Key, + ModuleInstance: addr.Module, + InstanceKey: addr.Resource.Key, } } @@ -155,6 +157,10 @@ func (n *NodeAbstractResource) Path() addrs.ModuleInstance { return n.Module.UnkeyedInstanceShim() } +func (n *NodeAbstractResourceInstance) Path() addrs.ModuleInstance { + return n.ModuleInstance +} + // GraphNodeModulePath func (n *NodeAbstractResource) ModulePath() addrs.Module { return n.Module diff --git a/terraform/node_resource_apply.go b/terraform/node_resource_apply.go index 3e2fff3a0..11aeb338a 100644 --- a/terraform/node_resource_apply.go +++ b/terraform/node_resource_apply.go @@ -53,19 +53,16 @@ func (n *NodeApplyableResource) References() []*addrs.Reference { // GraphNodeEvalable func (n *NodeApplyableResource) EvalTree() EvalNode { - addr := n.ResourceAddr() - config := n.Config - providerAddr := n.ResolvedProvider - - if config == nil { + if n.Config == nil { // Nothing to do, then. - log.Printf("[TRACE] NodeApplyableResource: no configuration present for %s", addr) + log.Printf("[TRACE] NodeApplyableResource: no configuration present for %s", n.Name()) return &EvalNoop{} } return &EvalWriteResourceState{ - Addr: addr.Resource, - Config: config, - ProviderAddr: providerAddr, + Addr: n.Addr, + Module: n.Module, + Config: n.Config, + ProviderAddr: n.ResolvedProvider, } } diff --git a/terraform/node_resource_destroy.go b/terraform/node_resource_destroy.go index ed2bafa69..54fba1dec 100644 --- a/terraform/node_resource_destroy.go +++ b/terraform/node_resource_destroy.go @@ -288,6 +288,7 @@ func (n *NodeDestroyResourceInstance) EvalTree() EvalNode { // all been destroyed. type NodeDestroyResource struct { *NodeAbstractResource + Addr addrs.AbsResource } var ( @@ -342,11 +343,6 @@ func (n *NodeDestroyResource) ResourceAddr() addrs.AbsResource { return n.NodeAbstractResource.ResourceAddr() } -// GraphNodeSubpath -func (n *NodeDestroyResource) Path() addrs.ModuleInstance { - return n.NodeAbstractResource.Path() -} - // GraphNodeNoProvider // FIXME: this should be removed once the node can be separated from the // Internal NodeAbstractResource behavior. diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index bd567e052..0455296c8 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -19,7 +19,6 @@ type NodePlannableResource struct { } var ( - _ GraphNodeModuleInstance = (*NodePlannableResource)(nil) _ GraphNodeDestroyerCBD = (*NodePlannableResource)(nil) _ GraphNodeDynamicExpandable = (*NodePlannableResource)(nil) _ GraphNodeReferenceable = (*NodePlannableResource)(nil) @@ -30,19 +29,17 @@ var ( // GraphNodeEvalable func (n *NodePlannableResource) EvalTree() EvalNode { - addr := n.ResourceAddr() - config := n.Config - - if config == nil { + if n.Config == nil { // Nothing to do, then. - log.Printf("[TRACE] NodeApplyableResource: no configuration present for %s", addr) + log.Printf("[TRACE] NodeApplyableResource: no configuration present for %s", n.Name()) return &EvalNoop{} } // this ensures we can reference the resource even if the count is 0 return &EvalWriteResourceState{ - Addr: addr.Resource, - Config: config, + Addr: n.Addr, + Module: n.Module, + Config: n.Config, ProviderAddr: n.ResolvedProvider, } } diff --git a/terraform/node_resource_refresh.go b/terraform/node_resource_refresh.go index 53fdf2cba..7871918e5 100644 --- a/terraform/node_resource_refresh.go +++ b/terraform/node_resource_refresh.go @@ -25,7 +25,6 @@ type NodeRefreshableManagedResource struct { } var ( - _ GraphNodeModuleInstance = (*NodeRefreshableManagedResource)(nil) _ GraphNodeDynamicExpandable = (*NodeRefreshableManagedResource)(nil) _ GraphNodeReferenceable = (*NodeRefreshableManagedResource)(nil) _ GraphNodeReferencer = (*NodeRefreshableManagedResource)(nil) @@ -61,8 +60,7 @@ func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, // Inform our instance expander about our expansion results above, // and then use it to calculate the instance addresses we'll expand for. expander := ctx.InstanceExpander() - - for _, module := range expander.ExpandModule(ctx.Path().Module()) { + for _, module := range expander.ExpandModule(n.Module) { switch { case count >= 0: expander.SetResourceCount(module, n.ResourceAddr().Resource, count) @@ -72,7 +70,7 @@ func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, expander.SetResourceSingle(module, n.ResourceAddr().Resource) } } - instanceAddrs := expander.ExpandResource(ctx.Path().Module(), n.ResourceAddr().Resource) + instanceAddrs := expander.ExpandResource(n.Module, n.ResourceAddr().Resource) // Our graph transformers require access to the full state, so we'll // temporarily lock it while we work on this. @@ -131,7 +129,7 @@ func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, Name: "NodeRefreshableManagedResource", } - graph, diags := b.Build(ctx.Path()) + graph, diags := b.Build(nil) return graph, diags.ErrWithWarnings() } diff --git a/terraform/node_resource_validate.go b/terraform/node_resource_validate.go index 3703d81ba..f8374f3fb 100644 --- a/terraform/node_resource_validate.go +++ b/terraform/node_resource_validate.go @@ -15,7 +15,6 @@ type NodeValidatableResource struct { } var ( - _ GraphNodeModuleInstance = (*NodeValidatableResource)(nil) _ GraphNodeEvalable = (*NodeValidatableResource)(nil) _ GraphNodeReferenceable = (*NodeValidatableResource)(nil) _ GraphNodeReferencer = (*NodeValidatableResource)(nil)