From 58babccc7a44dcfc5859063c4c0363c34d3447eb Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 3 Jun 2020 16:22:09 -0400 Subject: [PATCH 1/6] improve depends_on test to check ordering --- terraform/context_apply_test.go | 44 +++++++++++++++---- .../apply-module-depends-on/moda/main.tf | 1 + .../apply-module-depends-on/modb/main.tf | 1 + 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index f7b1130a1..a99aae428 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -11121,17 +11121,36 @@ func TestContext2Apply_moduleDependsOn(t *testing.T) { m := testModule(t, "apply-module-depends-on") p := testProvider("test") - p.ReadDataSourceResponse = providers.ReadDataSourceResponse{ - State: cty.ObjectVal(map[string]cty.Value{ - "id": cty.StringVal("data"), - "foo": cty.NullVal(cty.String), - }), - } - p.DiffFn = testDiffFn // each instance being applied should happen in sequential order applied := int64(0) + p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse { + cfg := req.Config.AsValueMap() + foo := cfg["foo"].AsString() + ord := atomic.LoadInt64(&applied) + + resp := providers.ReadDataSourceResponse{ + State: cty.ObjectVal(map[string]cty.Value{ + "id": cty.StringVal("data"), + "foo": cfg["foo"], + }), + } + + if foo == "a" && ord < 4 { + // due to data source "a"'s module depending on instance 4, this + // should not be less than 4 + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("data source a read too early")) + } + if foo == "b" && ord < 1 { + // due to data source "b"'s module depending on instance 1, this + // should not be less than 1 + resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("data source b read too early")) + } + return resp + } + p.DiffFn = testDiffFn + p.ApplyResourceChangeFn = func(req providers.ApplyResourceChangeRequest) (resp providers.ApplyResourceChangeResponse) { state := req.PlannedState.AsValueMap() num, _ := state["num"].AsBigFloat().Float64() @@ -11154,7 +11173,12 @@ func TestContext2Apply_moduleDependsOn(t *testing.T) { }, }) - _, diags := ctx.Plan() + _, diags := ctx.Refresh() + if diags.HasErrors() { + t.Fatal(diags.ErrWithWarnings()) + } + + _, diags = ctx.Plan() if diags.HasErrors() { t.Fatal(diags.ErrWithWarnings()) } @@ -11165,6 +11189,10 @@ func TestContext2Apply_moduleDependsOn(t *testing.T) { } // run the plan again to ensure that data sources are not going to be re-read + _, diags = ctx.Refresh() + if diags.HasErrors() { + t.Fatal(diags.ErrWithWarnings()) + } plan, diags := ctx.Plan() if diags.HasErrors() { t.Fatal(diags.ErrWithWarnings()) diff --git a/terraform/testdata/apply-module-depends-on/moda/main.tf b/terraform/testdata/apply-module-depends-on/moda/main.tf index 914e545c8..e60d300ba 100644 --- a/terraform/testdata/apply-module-depends-on/moda/main.tf +++ b/terraform/testdata/apply-module-depends-on/moda/main.tf @@ -3,6 +3,7 @@ resource "test_instance" "a" { } data "test_data_source" "a" { + foo = "a" } output "out" { diff --git a/terraform/testdata/apply-module-depends-on/modb/main.tf b/terraform/testdata/apply-module-depends-on/modb/main.tf index 17192b039..961c5d560 100644 --- a/terraform/testdata/apply-module-depends-on/modb/main.tf +++ b/terraform/testdata/apply-module-depends-on/modb/main.tf @@ -3,6 +3,7 @@ resource "test_instance" "b" { } data "test_data_source" "b" { + foo = "b" } output "out" { From 535267e986746b358c8467d701c6ad835d59c832 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 3 Jun 2020 19:44:43 -0400 Subject: [PATCH 2/6] add dependsOn to evalDataRead this is also needed during refresh, so move it into the base struct type --- terraform/eval_read_data.go | 15 ++++++++++++++- terraform/eval_read_data_plan.go | 7 ------- terraform/graph_builder_refresh.go | 1 + terraform/node_data_refresh.go | 2 ++ terraform/node_resource_plan_instance.go | 2 +- 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index b00f031a3..4630841b7 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -52,6 +52,12 @@ type evalReadData struct { // - If planned action is plans.Update, it indicates that the data source // was read, and the result needs to be stored in state during apply. OutputChange **plans.ResourceInstanceChange + + // dependsOn stores the list of transitive resource addresses that any + // configuration depends_on references may resolve to. This is used to + // determine if there are any changes that will force this data sources to + // be deferred to apply. + dependsOn []addrs.ConfigResource } // readDataSource handles everything needed to call ReadDataSource on the provider. @@ -235,7 +241,7 @@ func (n *evalReadDataRefresh) Eval(ctx EvalContext) (interface{}, error) { // refresh, that we can read this resource. If there are dependency updates // in the config, they we be discovered in plan and the data source will be // read again. - if !configKnown || (priorVal.IsNull() && len(n.Config.DependsOn) > 0) { + if !configKnown || (priorVal.IsNull() && n.forcePlanRead()) { if configKnown { log.Printf("[TRACE] evalReadDataRefresh: %s configuration is fully known, but we're forcing a read plan to be created", absAddr) } else { @@ -291,3 +297,10 @@ func (n *evalReadDataRefresh) Eval(ctx EvalContext) (interface{}, error) { return nil, diags.ErrWithWarnings() } + +// forcePlanRead determines if we need to override the usual behavior of +// immediately reading from the data source where possible, instead forcing us +// to generate a plan. +func (n *evalReadDataRefresh) forcePlanRead() bool { + return len(n.Config.DependsOn) > 0 || len(n.dependsOn) > 0 +} diff --git a/terraform/eval_read_data_plan.go b/terraform/eval_read_data_plan.go index 0a6d4c431..fd7fd276f 100644 --- a/terraform/eval_read_data_plan.go +++ b/terraform/eval_read_data_plan.go @@ -6,7 +6,6 @@ import ( "github.com/zclconf/go-cty/cty" - "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" "github.com/hashicorp/terraform/plans/objchange" "github.com/hashicorp/terraform/states" @@ -18,12 +17,6 @@ import ( // or generating a plan to do so. type evalReadDataPlan struct { evalReadData - - // dependsOn stores the list of transitive resource addresses that any - // configuration depends_on references may resolve to. This is used to - // determine if there are any changes that will force this data sources to - // be deferred to apply. - dependsOn []addrs.ConfigResource } func (n *evalReadDataPlan) Eval(ctx EvalContext) (interface{}, error) { diff --git a/terraform/graph_builder_refresh.go b/terraform/graph_builder_refresh.go index e34d05647..546fdff63 100644 --- a/terraform/graph_builder_refresh.go +++ b/terraform/graph_builder_refresh.go @@ -171,6 +171,7 @@ func (b *RefreshGraphBuilder) Steps() []GraphTransformer { // have to connect again later for providers and so on. &ReferenceTransformer{}, &AttachDependenciesTransformer{}, + &attachDataResourceDependenciesTransformer{}, // Target &TargetsTransformer{ diff --git a/terraform/node_data_refresh.go b/terraform/node_data_refresh.go index 98eda631c..0c191d272 100644 --- a/terraform/node_data_refresh.go +++ b/terraform/node_data_refresh.go @@ -121,6 +121,7 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er a.Config = n.Config a.ResolvedProvider = n.ResolvedProvider a.ProviderMetas = n.ProviderMetas + a.dependsOn = n.dependsOn return &NodeRefreshableDataResourceInstance{ NodeAbstractResourceInstance: a, @@ -227,6 +228,7 @@ func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode { ProviderSchema: &providerSchema, OutputChange: &change, State: &state, + dependsOn: n.dependsOn, }, }, diff --git a/terraform/node_resource_plan_instance.go b/terraform/node_resource_plan_instance.go index 19f2b633f..4b9c2bde4 100644 --- a/terraform/node_resource_plan_instance.go +++ b/terraform/node_resource_plan_instance.go @@ -83,8 +83,8 @@ func (n *NodePlannableResourceInstance) evalTreeDataResource(addr addrs.AbsResou ProviderSchema: &providerSchema, OutputChange: &change, State: &state, + dependsOn: n.dependsOn, }, - dependsOn: n.dependsOn, }, &EvalWriteState{ From a03c86f612a869d8ee9704535eff3e196dddf656 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 3 Jun 2020 19:45:59 -0400 Subject: [PATCH 3/6] add DependsOn method to moduleExpandModule We'll need this again for getting the transitive depends_on references from parent module calls. This is needed to inform us how to handle data sources during refresh and plan. --- terraform/node_module_expand.go | 35 ++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/terraform/node_module_expand.go b/terraform/node_module_expand.go index cfc519a33..892b6f4bd 100644 --- a/terraform/node_module_expand.go +++ b/terraform/node_module_expand.go @@ -47,18 +47,7 @@ func (n *nodeExpandModule) References() []*addrs.Reference { return nil } - for _, traversal := range n.ModuleCall.DependsOn { - ref, diags := addrs.ParseRef(traversal) - if diags.HasErrors() { - // We ignore this here, because this isn't a suitable place to return - // errors. This situation should be caught and rejected during - // validation. - log.Printf("[ERROR] Can't parse %#v from depends_on as reference: %s", traversal, diags.Err()) - continue - } - - refs = append(refs, ref) - } + refs = append(refs, n.DependsOn()...) // Expansion only uses the count and for_each expressions, so this // particular graph node only refers to those. @@ -82,6 +71,28 @@ func (n *nodeExpandModule) References() []*addrs.Reference { return appendResourceDestroyReferences(refs) } +func (n *nodeExpandModule) DependsOn() []*addrs.Reference { + if n.ModuleCall == nil { + return nil + } + + var refs []*addrs.Reference + for _, traversal := range n.ModuleCall.DependsOn { + ref, diags := addrs.ParseRef(traversal) + if diags.HasErrors() { + // We ignore this here, because this isn't a suitable place to return + // errors. This situation should be caught and rejected during + // validation. + log.Printf("[ERROR] Can't parse %#v from depends_on as reference: %s", traversal, diags.Err()) + continue + } + + refs = append(refs, ref) + } + + return refs +} + // GraphNodeReferenceOutside func (n *nodeExpandModule) ReferenceOutside() (selfPath, referencePath addrs.Module) { return n.Addr, n.Addr.Parent() From e74ebe178757b156bc0ca6eebb1a8209309be817 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 3 Jun 2020 21:36:42 -0400 Subject: [PATCH 4/6] get more depends_on info for data source Since data sources may be read during refresh or early in plan, they need to know if there are any inherited dependencies from containing modules too. --- terraform/transform_reference.go | 85 ++++++++++++++++++++++---------- 1 file changed, 58 insertions(+), 27 deletions(-) diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index 5ea5bd8e0..ba5fc2b51 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -43,6 +43,12 @@ type GraphNodeAttachDependencies interface { AttachDependencies([]addrs.ConfigResource) } +// graphNodeDependsOn is implemented by resources that need to expose any +// references set via DependsOn in their configuration. +type graphNodeDependsOn interface { + DependsOn() []*addrs.Reference +} + // graphNodeAttachResourceDependencies records all resources that are transitively // referenced through depends_on in the configuration. This is used by data // resources to determine if they can be read during the plan, or if they need @@ -55,8 +61,8 @@ type GraphNodeAttachDependencies interface { // "perpetual diff" type graphNodeAttachResourceDependencies interface { GraphNodeConfigResource + graphNodeDependsOn AttachResourceDependencies([]addrs.ConfigResource) - DependsOn() []*addrs.Reference } // GraphNodeReferenceOutside is an interface that can optionally be implemented. @@ -122,7 +128,7 @@ func (t *ReferenceTransformer) Transform(g *Graph) error { type depMap map[string]addrs.ConfigResource -// addDep adds the vertex if it represents a resource in the +// add stores the vertex if it represents a resource in the // graph. func (m depMap) add(v dag.Vertex) { // we're only concerned with resources which may have changes that @@ -155,34 +161,32 @@ func (t attachDataResourceDependenciesTransformer) Transform(g *Graph) error { if !ok { continue } - selfAddr := depender.ResourceAddr() // Only data need to attach depends_on, so they can determine if they // are eligible to be read during plan. - if selfAddr.Resource.Mode != addrs.DataResourceMode { + if depender.ResourceAddr().Resource.Mode != addrs.DataResourceMode { continue } - // depMap will only add resource references and dedupe - m := make(depMap) - - for _, dep := range refMap.DependsOn(v) { + // depMap will only add resource references then dedupe + deps := make(depMap) + for _, dep := range refMap.dependsOn(g, depender) { // any the dependency - m.add(dep) - // and check any ancestors + deps.add(dep) + // and check any ancestors for transitive dependencies ans, _ := g.Ancestors(dep) for _, v := range ans { - m.add(v) + deps.add(v) } } - deps := make([]addrs.ConfigResource, 0, len(m)) - for _, d := range m { - deps = append(deps, d) + res := make([]addrs.ConfigResource, 0, len(deps)) + for _, d := range deps { + res = append(res, d) } - log.Printf("[TRACE] AttachDependsOnTransformer: %s depends on %s", depender.ResourceAddr(), deps) - depender.AttachResourceDependencies(deps) + log.Printf("[TRACE] attachDataDependenciesTransformer: %s depends on %s", depender.ResourceAddr(), res) + depender.AttachResourceDependencies(res) } return nil @@ -304,37 +308,64 @@ func (m ReferenceMap) References(v dag.Vertex) []dag.Vertex { return matches } -// DependsOn returns the set of vertices that the given vertex refers to from +// dependsOn returns the set of vertices that the given vertex refers to from // the configured depends_on. -func (m ReferenceMap) DependsOn(v dag.Vertex) []dag.Vertex { - depender, ok := v.(graphNodeAttachResourceDependencies) - if !ok { - return nil - } - +func (m ReferenceMap) dependsOn(g *Graph, depender graphNodeDependsOn) []dag.Vertex { var matches []dag.Vertex + refs := depender.DependsOn() - for _, ref := range depender.DependsOn() { + for _, ref := range refs { subject := ref.Subject - key := m.referenceMapKey(v, subject) + key := m.referenceMapKey(depender, subject) vertices, ok := m[key] if !ok { - log.Printf("[WARN] DependOn: reference not found: %q", subject) + log.Printf("[WARN] DependsOn: reference not found: %q", subject) continue } for _, rv := range vertices { // don't include self-references - if rv == v { + if rv == depender { continue } matches = append(matches, rv) } } + matches = append(matches, m.parentModuleDependsOn(g, depender)...) + return matches } +// parentModuleDependsOn returns the set of vertices that a data sources parent +// module references through the module call's depends_on. +func (n ReferenceMap) parentModuleDependsOn(g *Graph, depender graphNodeDependsOn) []dag.Vertex { + var res []dag.Vertex + + // look for containing modules with DependsOn + for _, v := range g.DownEdges(depender) { + mod, ok := v.(*nodeExpandModule) + if !ok { + continue + } + + for _, dep := range n.dependsOn(g, mod) { + // add the dependency + res = append(res, dep) + // and check any ancestors + ans, _ := g.Ancestors(dep) + for _, v := range ans { + res = append(res, v) + } + } + + // recurse up through parent modules + res = append(res, n.parentModuleDependsOn(g, mod)...) + } + + return res +} + func (m *ReferenceMap) mapKey(path addrs.Module, addr addrs.Referenceable) string { return fmt.Sprintf("%s|%s", path.String(), addr.String()) } From 4637be437786670dc5cd2a7649043530c0c27857 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 4 Jun 2020 12:39:36 -0400 Subject: [PATCH 5/6] add a way to force depends_on behavior of data Resources that are not yet created will not be in the graph during refresh, and therefore cannot be attached to the data source nodes. In this case we still need to indicate if there are depends_on entries inherited from the module call, which we can do with the forceDependsOn field. --- terraform/eval_read_data.go | 5 ++++- terraform/node_data_refresh.go | 2 ++ terraform/node_resource_abstract.go | 8 +++++--- terraform/transform_reference.go | 12 ++++++++++-- 4 files changed, 21 insertions(+), 6 deletions(-) diff --git a/terraform/eval_read_data.go b/terraform/eval_read_data.go index 4630841b7..9235130bd 100644 --- a/terraform/eval_read_data.go +++ b/terraform/eval_read_data.go @@ -58,6 +58,9 @@ type evalReadData struct { // determine if there are any changes that will force this data sources to // be deferred to apply. dependsOn []addrs.ConfigResource + // forceDependsOn indicates that resources may be missing from dependsOn, + // but the parent module may have depends_on configured. + forceDependsOn bool } // readDataSource handles everything needed to call ReadDataSource on the provider. @@ -302,5 +305,5 @@ func (n *evalReadDataRefresh) Eval(ctx EvalContext) (interface{}, error) { // immediately reading from the data source where possible, instead forcing us // to generate a plan. func (n *evalReadDataRefresh) forcePlanRead() bool { - return len(n.Config.DependsOn) > 0 || len(n.dependsOn) > 0 + return len(n.Config.DependsOn) > 0 || len(n.dependsOn) > 0 || n.forceDependsOn } diff --git a/terraform/node_data_refresh.go b/terraform/node_data_refresh.go index 0c191d272..135bcb2a2 100644 --- a/terraform/node_data_refresh.go +++ b/terraform/node_data_refresh.go @@ -122,6 +122,7 @@ func (n *NodeRefreshableDataResource) DynamicExpand(ctx EvalContext) (*Graph, er a.ResolvedProvider = n.ResolvedProvider a.ProviderMetas = n.ProviderMetas a.dependsOn = n.dependsOn + a.forceDependsOn = n.forceDependsOn return &NodeRefreshableDataResourceInstance{ NodeAbstractResourceInstance: a, @@ -229,6 +230,7 @@ func (n *NodeRefreshableDataResourceInstance) EvalTree() EvalNode { OutputChange: &change, State: &state, dependsOn: n.dependsOn, + forceDependsOn: n.forceDependsOn, }, }, diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index 11eca0d96..aada8d09c 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -61,8 +61,9 @@ type NodeAbstractResource struct { // Set from GraphNodeTargetable Targets []addrs.Targetable - // Set from GraphNodeDependsOn - dependsOn []addrs.ConfigResource + // Set from AttachResourceDependencies + dependsOn []addrs.ConfigResource + forceDependsOn bool // The address of the provider this resource will use ResolvedProvider addrs.AbsProviderConfig @@ -402,8 +403,9 @@ func (n *NodeAbstractResource) SetTargets(targets []addrs.Targetable) { } // graphNodeAttachResourceDependencies -func (n *NodeAbstractResource) AttachResourceDependencies(deps []addrs.ConfigResource) { +func (n *NodeAbstractResource) AttachResourceDependencies(deps []addrs.ConfigResource, force bool) { n.dependsOn = deps + n.forceDependsOn = force } // GraphNodeAttachResourceState diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index ba5fc2b51..34f214319 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -62,7 +62,15 @@ type graphNodeDependsOn interface { type graphNodeAttachResourceDependencies interface { GraphNodeConfigResource graphNodeDependsOn - AttachResourceDependencies([]addrs.ConfigResource) + + // AttachResourceDependencies stored the discovered dependencies in the + // resource node for evaluation later. + // + // The force parameter indicates that even if there are no dependencies, + // force the data source to act as though there are for refresh purposes. + // This is needed because yet-to-be-created resources won't be in the + // initial refresh graph, but may still be referenced through depends_on. + AttachResourceDependencies(deps []addrs.ConfigResource, force bool) } // GraphNodeReferenceOutside is an interface that can optionally be implemented. @@ -186,7 +194,7 @@ func (t attachDataResourceDependenciesTransformer) Transform(g *Graph) error { } log.Printf("[TRACE] attachDataDependenciesTransformer: %s depends on %s", depender.ResourceAddr(), res) - depender.AttachResourceDependencies(res) + depender.AttachResourceDependencies(res, false) } return nil From c3ec4d8e260d4fd59a9ca2e7522834e47f8a47cb Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 4 Jun 2020 14:21:50 -0400 Subject: [PATCH 6/6] get whether parent modules have depends_on set During refresh, data sources need to know if their parent modules have depends_on configured at all. Pass this info back through the search for depends_on resources, and delay refresh when it's set. --- terraform/transform_reference.go | 79 ++++++++++++++++++++++---------- 1 file changed, 55 insertions(+), 24 deletions(-) diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index 34f214319..94659e380 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -178,14 +178,10 @@ func (t attachDataResourceDependenciesTransformer) Transform(g *Graph) error { // depMap will only add resource references then dedupe deps := make(depMap) - for _, dep := range refMap.dependsOn(g, depender) { + dependsOnDeps, fromModule := refMap.dependsOn(g, depender) + for _, dep := range dependsOnDeps { // any the dependency deps.add(dep) - // and check any ancestors for transitive dependencies - ans, _ := g.Ancestors(dep) - for _, v := range ans { - deps.add(v) - } } res := make([]addrs.ConfigResource, 0, len(deps)) @@ -194,7 +190,7 @@ func (t attachDataResourceDependenciesTransformer) Transform(g *Graph) error { } log.Printf("[TRACE] attachDataDependenciesTransformer: %s depends on %s", depender.ResourceAddr(), res) - depender.AttachResourceDependencies(res, false) + depender.AttachResourceDependencies(res, fromModule) } return nil @@ -266,6 +262,16 @@ func (t AttachDependenciesTransformer) Transform(g *Graph) error { return nil } +func isDependableResource(v dag.Vertex) bool { + switch v.(type) { + case GraphNodeResourceInstance: + return true + case GraphNodeConfigResource: + return true + } + return false +} + // ReferenceMap is a structure that can be used to efficiently check // for references on a graph, mapping internal reference keys (as produced by // the mapKey method) to one or more vertices that are identified by each key. @@ -317,18 +323,27 @@ func (m ReferenceMap) References(v dag.Vertex) []dag.Vertex { } // dependsOn returns the set of vertices that the given vertex refers to from -// the configured depends_on. -func (m ReferenceMap) dependsOn(g *Graph, depender graphNodeDependsOn) []dag.Vertex { - var matches []dag.Vertex +// the configured depends_on. The bool return value indicates if depends_on was +// found in a parent module configuration. +func (m ReferenceMap) dependsOn(g *Graph, depender graphNodeDependsOn) ([]dag.Vertex, bool) { + var res []dag.Vertex + fromModule := false + refs := depender.DependsOn() + // This is where we record that a module has depends_on configured. + if _, ok := depender.(*nodeExpandModule); ok && len(refs) > 0 { + fromModule = true + } + for _, ref := range refs { subject := ref.Subject key := m.referenceMapKey(depender, subject) vertices, ok := m[key] if !ok { - log.Printf("[WARN] DependsOn: reference not found: %q", subject) + // the ReferenceMap generates all possible keys, so any warning + // here is probably not useful for this implementation. continue } for _, rv := range vertices { @@ -336,42 +351,58 @@ func (m ReferenceMap) dependsOn(g *Graph, depender graphNodeDependsOn) []dag.Ver if rv == depender { continue } - matches = append(matches, rv) + res = append(res, rv) + + // and check any ancestors for transitive dependencies + ans, _ := g.Ancestors(rv) + for _, v := range ans { + if isDependableResource(v) { + res = append(res, v) + } + } } } - matches = append(matches, m.parentModuleDependsOn(g, depender)...) + parentDeps, fromParentModule := m.parentModuleDependsOn(g, depender) + res = append(res, parentDeps...) - return matches + return res, fromModule || fromParentModule } // parentModuleDependsOn returns the set of vertices that a data sources parent -// module references through the module call's depends_on. -func (n ReferenceMap) parentModuleDependsOn(g *Graph, depender graphNodeDependsOn) []dag.Vertex { +// module references through the module call's depends_on. The bool return +// value indicates if depends_on was found in a parent module configuration. +func (n ReferenceMap) parentModuleDependsOn(g *Graph, depender graphNodeDependsOn) ([]dag.Vertex, bool) { var res []dag.Vertex + fromModule := false - // look for containing modules with DependsOn + // Look for containing modules with DependsOn. + // This should be connected directly to the module node, so we only need to + // look one step away. for _, v := range g.DownEdges(depender) { + // we're only concerned with module expansion nodes here. mod, ok := v.(*nodeExpandModule) if !ok { continue } - for _, dep := range n.dependsOn(g, mod) { + deps, fromParentModule := n.dependsOn(g, mod) + for _, dep := range deps { // add the dependency res = append(res, dep) - // and check any ancestors + + // and check any transitive resource dependencies for more resources ans, _ := g.Ancestors(dep) for _, v := range ans { - res = append(res, v) + if isDependableResource(v) { + res = append(res, v) + } } } - - // recurse up through parent modules - res = append(res, n.parentModuleDependsOn(g, mod)...) + fromModule = fromModule || fromParentModule } - return res + return res, fromModule } func (m *ReferenceMap) mapKey(path addrs.Module, addr addrs.Referenceable) string {