From 0d58537bf2911c2f2901c4d770d35f1f5abe977a Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 24 Mar 2020 12:00:07 -0400 Subject: [PATCH 1/2] Expander.ExpandResource It turns out that within terraform resource expansion will normally happen with an absolute address. This is because in order to evaluate the expansion expression, we need to already have the context within a module instance. This leaves the existing ExpandResource logic in place as ExpandModuleResource since it's working, and in case we do find a location where it's useful to get a full expansion (which may be needed for instance dependency tracking) Reword some of the resource related arguments and comments, as they were copied from the module methods and not entirely accurate. --- instances/expander.go | 50 ++++++++++++++++++++++++++------------ instances/expander_test.go | 48 +++++++++++++++++++++++++++--------- 2 files changed, 71 insertions(+), 27 deletions(-) diff --git a/instances/expander.go b/instances/expander.go index 357d0f9fd..e7fdceb5b 100644 --- a/instances/expander.go +++ b/instances/expander.go @@ -70,26 +70,26 @@ func (e *Expander) SetModuleForEach(parentAddr addrs.ModuleInstance, callAddr ad e.setModuleExpansion(parentAddr, callAddr, expansionForEach(mapping)) } -// SetResourceSingle records that the given module inside the given parent -// module does not use any repetition arguments and is therefore a singleton. -func (e *Expander) SetResourceSingle(parentAddr addrs.ModuleInstance, resourceAddr addrs.Resource) { - e.setResourceExpansion(parentAddr, resourceAddr, expansionSingleVal) +// SetResourceSingle records that the given resource inside the given module +// does not use any repetition arguments and is therefore a singleton. +func (e *Expander) SetResourceSingle(moduleAddr addrs.ModuleInstance, resourceAddr addrs.Resource) { + e.setResourceExpansion(moduleAddr, resourceAddr, expansionSingleVal) } -// SetResourceCount records that the given module inside the given parent -// module uses the "count" repetition argument, with the given value. -func (e *Expander) SetResourceCount(parentAddr addrs.ModuleInstance, resourceAddr addrs.Resource, count int) { - e.setResourceExpansion(parentAddr, resourceAddr, expansionCount(count)) +// SetResourceCount records that the given resource inside the given module +// uses the "count" repetition argument, with the given value. +func (e *Expander) SetResourceCount(moduleAddr addrs.ModuleInstance, resourceAddr addrs.Resource, count int) { + e.setResourceExpansion(moduleAddr, resourceAddr, expansionCount(count)) } -// SetResourceForEach records that the given module inside the given parent -// module uses the "for_each" repetition argument, with the given map value. +// SetResourceForEach records that the given resource inside the given module +// uses the "for_each" repetition argument, with the given map value. // // In the configuration language the for_each argument can also accept a set. // It's the caller's responsibility to convert that into an identity map before // calling this method. -func (e *Expander) SetResourceForEach(parentAddr addrs.ModuleInstance, resourceAddr addrs.Resource, mapping map[string]cty.Value) { - e.setResourceExpansion(parentAddr, resourceAddr, expansionForEach(mapping)) +func (e *Expander) SetResourceForEach(moduleAddr addrs.ModuleInstance, resourceAddr addrs.Resource, mapping map[string]cty.Value) { + e.setResourceExpansion(moduleAddr, resourceAddr, expansionForEach(mapping)) } // ExpandModule finds the exhaustive set of module instances resulting from @@ -120,13 +120,13 @@ func (e *Expander) ExpandModule(addr addrs.Module) []addrs.ModuleInstance { return ret } -// ExpandResource finds the exhaustive set of resource instances resulting from +// ExpandModuleResource finds the exhaustive set of resource instances resulting from // the expansion of the given resource and all of its containing modules. // // All of the modules on the path to the identified resource and the resource // itself must already have had their expansion registered using one of the // SetModule*/SetResource* methods before calling, or this method will panic. -func (e *Expander) ExpandResource(parentAddr addrs.Module, resourceAddr addrs.Resource) []addrs.AbsResourceInstance { +func (e *Expander) ExpandModuleResource(moduleAddr addrs.Module, resourceAddr addrs.Resource) []addrs.AbsResourceInstance { e.mu.RLock() defer e.mu.RUnlock() @@ -136,13 +136,33 @@ func (e *Expander) ExpandResource(parentAddr addrs.Module, resourceAddr addrs.Re // (moduleInstances does plenty of allocations itself, so the benefit of // pre-allocating this is marginal but it's not hard to do.) moduleInstanceAddr := make(addrs.ModuleInstance, 0, 4) - ret := e.exps.resourceInstances(parentAddr, resourceAddr, moduleInstanceAddr) + ret := e.exps.resourceInstances(moduleAddr, resourceAddr, moduleInstanceAddr) sort.SliceStable(ret, func(i, j int) bool { return ret[i].Less(ret[j]) }) return ret } +// ExpandResource finds the set of resource instances resulting from +// the expansion of the given resource within its module instance. +// +// All of the modules on the path to the identified resource and the resource +// itself must already have had their expansion registered using one of the +// SetModule*/SetResource* methods before calling, or this method will panic. +func (e *Expander) ExpandResource(resourceAddr addrs.AbsResource) []addrs.AbsResourceInstance { + var ret []addrs.AbsResourceInstance + + // Take the full set of expanded resource instances and filter for only + // those within our module instance. + for _, r := range e.ExpandModuleResource(resourceAddr.Module.Module(), resourceAddr.Resource) { + if !r.Module.Equal(resourceAddr.Module) { + continue + } + ret = append(ret, r) + } + return ret +} + // GetModuleInstanceRepetitionData returns an object describing the values // that should be available for each.key, each.value, and count.index within // the call block for the given module instance. diff --git a/instances/expander_test.go b/instances/expander_test.go index 63dd35113..e143d1b98 100644 --- a/instances/expander_test.go +++ b/instances/expander_test.go @@ -130,7 +130,7 @@ func TestExpander(t *testing.T) { } }) t.Run("resource single", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( addrs.RootModule, singleResourceAddr, ) @@ -142,7 +142,7 @@ func TestExpander(t *testing.T) { } }) t.Run("resource count2", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( addrs.RootModule, count2ResourceAddr, ) @@ -155,7 +155,7 @@ func TestExpander(t *testing.T) { } }) t.Run("resource count0", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( addrs.RootModule, count0ResourceAddr, ) @@ -165,7 +165,7 @@ func TestExpander(t *testing.T) { } }) t.Run("resource for_each", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( addrs.RootModule, forEachResourceAddr, ) @@ -187,7 +187,7 @@ func TestExpander(t *testing.T) { } }) t.Run("module single resource single", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( mustModuleAddr("single"), singleResourceAddr, ) @@ -199,7 +199,7 @@ func TestExpander(t *testing.T) { } }) t.Run("module single resource count2", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( mustModuleAddr(`single`), count2ResourceAddr, ) @@ -222,7 +222,7 @@ func TestExpander(t *testing.T) { } }) t.Run("module count2 resource single", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( mustModuleAddr(`count2`), singleResourceAddr, ) @@ -235,7 +235,7 @@ func TestExpander(t *testing.T) { } }) t.Run("module count2 resource count2", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( mustModuleAddr(`count2`), count2ResourceAddr, ) @@ -262,7 +262,7 @@ func TestExpander(t *testing.T) { } }) t.Run("module count2 resource count2 resource count2", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( mustModuleAddr(`count2.count2`), count2ResourceAddr, ) @@ -280,6 +280,18 @@ func TestExpander(t *testing.T) { t.Errorf("wrong result\n%s", diff) } }) + t.Run("module count2 resource count2 resource count2", func(t *testing.T) { + got := ex.ExpandResource( + count2ResourceAddr.Absolute(mustModuleInstanceAddr(`module.count2[0].module.count2[1]`)), + ) + want := []addrs.AbsResourceInstance{ + mustAbsResourceInstanceAddr(`module.count2[0].module.count2[1].test.count2[0]`), + mustAbsResourceInstanceAddr(`module.count2[0].module.count2[1].test.count2[1]`), + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) t.Run("module count0", func(t *testing.T) { got := ex.ExpandModule(mustModuleAddr(`count0`)) want := []addrs.ModuleInstance(nil) @@ -288,7 +300,7 @@ func TestExpander(t *testing.T) { } }) t.Run("module count0 resource single", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( mustModuleAddr(`count0`), singleResourceAddr, ) @@ -311,7 +323,7 @@ func TestExpander(t *testing.T) { } }) t.Run("module for_each resource single", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( mustModuleAddr(`for_each`), singleResourceAddr, ) @@ -324,7 +336,7 @@ func TestExpander(t *testing.T) { } }) t.Run("module for_each resource count2", func(t *testing.T) { - got := ex.ExpandResource( + got := ex.ExpandModuleResource( mustModuleAddr(`for_each`), count2ResourceAddr, ) @@ -338,6 +350,18 @@ func TestExpander(t *testing.T) { t.Errorf("wrong result\n%s", diff) } }) + t.Run("module for_each resource count2", func(t *testing.T) { + got := ex.ExpandResource( + count2ResourceAddr.Absolute(mustModuleInstanceAddr(`module.for_each["a"]`)), + ) + want := []addrs.AbsResourceInstance{ + mustAbsResourceInstanceAddr(`module.for_each["a"].test.count2[0]`), + mustAbsResourceInstanceAddr(`module.for_each["a"].test.count2[1]`), + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("wrong result\n%s", diff) + } + }) t.Run(`module.for_each["b"] repetitiondata`, func(t *testing.T) { got := ex.GetModuleInstanceRepetitionData( From 1b3f5beeec57162cfdeef4f38c670651d89a0b56 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 24 Mar 2020 12:11:27 -0400 Subject: [PATCH 2/2] udpate core to work with new ExpandResource This also calls ExpandModuleResource in one location, because the logic is not yet updated to handle actual module expansion, but that will be fixed in a forthcoming PR. --- terraform/node_data_refresh.go | 2 +- terraform/node_resource_plan.go | 3 +-- terraform/node_resource_refresh.go | 2 +- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/terraform/node_data_refresh.go b/terraform/node_data_refresh.go index 02d230155..f6c3f6d92 100644 --- a/terraform/node_data_refresh.go +++ b/terraform/node_data_refresh.go @@ -68,7 +68,7 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er default: expander.SetResourceSingle(path, n.ResourceAddr().Resource) } - instanceAddrs = append(instanceAddrs, expander.ExpandResource(path.Module(), n.ResourceAddr().Resource)...) + instanceAddrs = append(instanceAddrs, expander.ExpandResource(n.ResourceAddr().Absolute(path))...) } // Our graph transformers require access to the full state, so we'll diff --git a/terraform/node_resource_plan.go b/terraform/node_resource_plan.go index 0a339e6bb..73e059a64 100644 --- a/terraform/node_resource_plan.go +++ b/terraform/node_resource_plan.go @@ -70,9 +70,8 @@ func (n *NodePlannableResource) DynamicExpand(ctx EvalContext) (*Graph, error) { // Our instance expander should already have been informed about the // expansion of this resource and of all of its containing modules, so // it can tell us which instance addresses we need to process. - module := ctx.Path().Module() expander := ctx.InstanceExpander() - instanceAddrs := expander.ExpandResource(module, n.ResourceAddr().Resource) + instanceAddrs := expander.ExpandResource(n.ResourceAddr().Absolute(ctx.Path())) // We need to potentially rename an instance address in the state // if we're transitioning whether "count" is set at all. diff --git a/terraform/node_resource_refresh.go b/terraform/node_resource_refresh.go index 2284d1e43..68a087708 100644 --- a/terraform/node_resource_refresh.go +++ b/terraform/node_resource_refresh.go @@ -75,7 +75,7 @@ func (n *NodeRefreshableManagedResource) DynamicExpand(ctx EvalContext) (*Graph, expander.SetResourceSingle(module, n.ResourceAddr().Resource) } } - instanceAddrs := expander.ExpandResource(n.Addr.Module, n.ResourceAddr().Resource) + instanceAddrs := expander.ExpandModuleResource(n.Addr.Module, n.ResourceAddr().Resource) // Our graph transformers require access to the full state, so we'll // temporarily lock it while we work on this.