From 521bdcc241a6cfa81577263ad2d1cffe734a514f Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 4 Mar 2020 21:00:16 -0500 Subject: [PATCH] implement GraphNodeModulePath GraphNodeModulePath is similar to GraphNodeSubPath, except that it returns an addrs.Module rather than an addrs.ModuleInstance. This is used by the ReferenceTransformer to connect references, when modules may not yet be expanded. Because references only exist within the scope of a module, we can connect everything knowing only the module path. If the reference is to an expanded module instance output, we can still properly order the reference because we'll wait for the entire module to complete evaluation. --- terraform/graph_interface_subgraph.go | 6 ++++++ terraform/node_local.go | 5 +++++ terraform/node_module_expand.go | 8 ++++++++ terraform/node_module_removed.go | 8 ++++++++ terraform/node_module_variable.go | 10 ++++++++++ terraform/node_output.go | 15 +++++++++++++++ terraform/node_output_orphan.go | 5 +++++ terraform/node_provider_abstract.go | 5 +++++ terraform/node_resource_abstract.go | 5 +++++ terraform/node_root_variable.go | 4 ++++ terraform/transform_reference.go | 27 ++++++++++++--------------- terraform/transform_reference_test.go | 8 ++++++++ 12 files changed, 91 insertions(+), 15 deletions(-) diff --git a/terraform/graph_interface_subgraph.go b/terraform/graph_interface_subgraph.go index 768590fb0..b61bac5a1 100644 --- a/terraform/graph_interface_subgraph.go +++ b/terraform/graph_interface_subgraph.go @@ -9,3 +9,9 @@ import ( type GraphNodeSubPath interface { Path() addrs.ModuleInstance } + +// GraphNodeModulePath is implemented by all referenceable nodes, to indicate +// their configuration path in unexpanded modules. +type GraphNodeModulePath interface { + ModulePath() addrs.Module +} diff --git a/terraform/node_local.go b/terraform/node_local.go index 591eb305a..06da3381f 100644 --- a/terraform/node_local.go +++ b/terraform/node_local.go @@ -34,6 +34,11 @@ func (n *NodeLocal) Path() addrs.ModuleInstance { return n.Addr.Module } +// GraphNodeModulePath +func (n *NodeLocal) ModulePath() addrs.Module { + return n.Addr.Module.Module() +} + // RemovableIfNotTargeted func (n *NodeLocal) RemoveIfNotTargeted() bool { return true diff --git a/terraform/node_module_expand.go b/terraform/node_module_expand.go index 71d8a177f..a519bd543 100644 --- a/terraform/node_module_expand.go +++ b/terraform/node_module_expand.go @@ -39,6 +39,14 @@ func (n *nodeExpandModule) Path() addrs.ModuleInstance { return n.CallerAddr } +// GraphNodeModulePath implementation +func (n *nodeExpandModule) ModulePath() addrs.Module { + // This node represents the module call within a module, + // so return the CallerAddr as the path as the module + // call may expand into multiple child instances + return n.Addr +} + // GraphNodeReferencer implementation func (n *nodeExpandModule) References() []*addrs.Reference { var refs []*addrs.Reference diff --git a/terraform/node_module_removed.go b/terraform/node_module_removed.go index 43d9bcd4b..d281a2c61 100644 --- a/terraform/node_module_removed.go +++ b/terraform/node_module_removed.go @@ -29,6 +29,14 @@ func (n *NodeModuleRemoved) Path() addrs.ModuleInstance { return n.Addr } +// GraphNodeModulePath implementation +func (n *NodeModuleRemoved) ModulePath() addrs.Module { + // This node represents the module call within a module, + // so return the CallerAddr as the path as the module + // call may expand into multiple child instances + return n.Addr.Module() +} + // GraphNodeEvalable func (n *NodeModuleRemoved) EvalTree() EvalNode { return &EvalOpFilter{ diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index 25954edbc..87074e913 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -54,6 +54,11 @@ func (n *NodePlannableModuleVariable) Path() addrs.ModuleInstance { return n.Module.UnkeyedInstanceShim() } +// GraphNodeModulePath +func (n *NodePlannableModuleVariable) ModulePath() addrs.Module { + return n.Module +} + // GraphNodeReferencer func (n *NodePlannableModuleVariable) References() []*addrs.Reference { @@ -137,6 +142,11 @@ func (n *NodeApplyableModuleVariable) Path() addrs.ModuleInstance { return n.Addr.Module.Parent() } +// GraphNodeModulePath +func (n *NodeApplyableModuleVariable) ModulePath() addrs.Module { + return n.Addr.Module.Parent().Module() +} + // RemovableIfNotTargeted func (n *NodeApplyableModuleVariable) RemoveIfNotTargeted() bool { // We need to add this so that this node will be removed if diff --git a/terraform/node_output.go b/terraform/node_output.go index 063611916..88e62a984 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -51,6 +51,11 @@ func (n *NodePlannableOutput) Path() addrs.ModuleInstance { return n.Module.UnkeyedInstanceShim() } +// GraphNodeModulePath +func (n *NodePlannableOutput) ModulePath() addrs.Module { + return n.Module +} + // GraphNodeReferenceable func (n *NodePlannableOutput) ReferenceableAddrs() []addrs.Referenceable { // An output in the root module can't be referenced at all. @@ -128,6 +133,11 @@ func (n *NodeApplyableOutput) Path() addrs.ModuleInstance { return n.Addr.Module } +// GraphNodeModulePath +func (n *NodeApplyableOutput) ModulePath() addrs.Module { + return n.Addr.Module.Module() +} + // RemovableIfNotTargeted func (n *NodeApplyableOutput) RemoveIfNotTargeted() bool { // We need to add this so that this node will be removed if @@ -254,6 +264,11 @@ func (n *NodeDestroyableOutput) Path() addrs.ModuleInstance { return n.Module.UnkeyedInstanceShim() } +// GraphNodeModulePath +func (n *NodeDestroyableOutput) ModulePath() addrs.Module { + return n.Module +} + // RemovableIfNotTargeted func (n *NodeDestroyableOutput) RemoveIfNotTargeted() bool { // We need to add this so that this node will be removed if diff --git a/terraform/node_output_orphan.go b/terraform/node_output_orphan.go index f8f7124c6..d52dd8fa2 100644 --- a/terraform/node_output_orphan.go +++ b/terraform/node_output_orphan.go @@ -37,6 +37,11 @@ func (n *NodeOutputOrphan) Path() addrs.ModuleInstance { return n.Addr.Module } +// GraphNodeModulePath +func (n *NodeOutputOrphan) ModulePath() addrs.Module { + return n.Addr.Module.Module() +} + // GraphNodeEvalable func (n *NodeOutputOrphan) EvalTree() EvalNode { return &EvalOpFilter{ diff --git a/terraform/node_provider_abstract.go b/terraform/node_provider_abstract.go index a0cdcfe01..c05751bc3 100644 --- a/terraform/node_provider_abstract.go +++ b/terraform/node_provider_abstract.go @@ -44,6 +44,11 @@ func (n *NodeAbstractProvider) Path() addrs.ModuleInstance { return n.Addr.Module } +// GraphNodeModulePath +func (n *NodeAbstractProvider) ModulePath() addrs.Module { + return n.Addr.Module.Module() +} + // RemovableIfNotTargeted func (n *NodeAbstractProvider) RemoveIfNotTargeted() bool { // We need to add this so that this node will be removed if diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index e14c02180..8ad80f7fb 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -153,6 +153,11 @@ func (n *NodeAbstractResource) Path() addrs.ModuleInstance { return n.Addr.Module } +// GraphNodeModulePath +func (n *NodeAbstractResource) ModulePath() addrs.Module { + return n.Addr.Module.Module() +} + // GraphNodeReferenceable func (n *NodeAbstractResource) ReferenceableAddrs() []addrs.Referenceable { return []addrs.Referenceable{n.Addr.Resource} diff --git a/terraform/node_root_variable.go b/terraform/node_root_variable.go index e3aee6fc8..5c17619b7 100644 --- a/terraform/node_root_variable.go +++ b/terraform/node_root_variable.go @@ -27,6 +27,10 @@ func (n *NodeRootVariable) Path() addrs.ModuleInstance { return addrs.RootModuleInstance } +func (n *NodeRootVariable) ModulePath() addrs.Module { + return addrs.RootModule +} + // GraphNodeReferenceable func (n *NodeRootVariable) ReferenceableAddrs() []addrs.Referenceable { return []addrs.Referenceable{n.Addr} diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index 9b397ccd4..fe44c53eb 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -22,7 +22,7 @@ import ( // be referenced and other methods of referencing may still be possible (such // as by path!) type GraphNodeReferenceable interface { - GraphNodeSubPath + GraphNodeModulePath // ReferenceableAddrs returns a list of addresses through which this can be // referenced. @@ -32,7 +32,7 @@ type GraphNodeReferenceable interface { // GraphNodeReferencer must be implemented by nodes that reference other // Terraform items and therefore depend on them. type GraphNodeReferencer interface { - GraphNodeSubPath + GraphNodeModulePath // References returns a list of references made by this node, which // include both a referenced address and source location information for @@ -252,9 +252,6 @@ func (m *ReferenceMap) References(v dag.Vertex) []dag.Vertex { if !ok { return nil } - if _, ok := v.(GraphNodeSubPath); !ok { - return nil - } var matches []dag.Vertex @@ -298,11 +295,11 @@ func (m *ReferenceMap) mapKey(path addrs.ModuleInstance, addr addrs.Referenceabl // // Only GraphNodeSubPath implementations can be referenced, so this method will // panic if the given vertex does not implement that interface. -func (m *ReferenceMap) vertexReferenceablePath(v dag.Vertex) addrs.ModuleInstance { - sp, ok := v.(GraphNodeSubPath) +func vertexReferenceablePath(v dag.Vertex) addrs.ModuleInstance { + sp, ok := v.(GraphNodeModulePath) if !ok { // Only nodes with paths can participate in a reference map. - panic(fmt.Errorf("vertexMapKey on vertex type %T which doesn't implement GraphNodeSubPath", sp)) + panic(fmt.Errorf("vertexMapKey on vertex type %T which doesn't implement GraphNodeModulePath", sp)) } if outside, ok := v.(GraphNodeReferenceOutside); ok { @@ -313,7 +310,7 @@ func (m *ReferenceMap) vertexReferenceablePath(v dag.Vertex) addrs.ModuleInstanc } // Vertex is referenced from the same module as where it was declared. - return sp.Path() + return sp.ModulePath().UnkeyedInstanceShim() } // vertexReferencePath returns the path in which references _from_ the given @@ -321,14 +318,14 @@ func (m *ReferenceMap) vertexReferenceablePath(v dag.Vertex) addrs.ModuleInstanc // // Only GraphNodeSubPath implementations can have references, so this method // will panic if the given vertex does not implement that interface. -func vertexReferencePath(referrer dag.Vertex) addrs.ModuleInstance { - sp, ok := referrer.(GraphNodeSubPath) +func vertexReferencePath(v dag.Vertex) addrs.ModuleInstance { + sp, ok := v.(GraphNodeModulePath) if !ok { // Only nodes with paths can participate in a reference map. - panic(fmt.Errorf("vertexReferencePath on vertex type %T which doesn't implement GraphNodeSubPath", sp)) + panic(fmt.Errorf("vertexReferencePath on vertex type %T which doesn't implement GraphNodeModulePath", v)) } - if outside, ok := referrer.(GraphNodeReferenceOutside); ok { + if outside, ok := v.(GraphNodeReferenceOutside); ok { // Vertex makes references to objects in a different module than where // it was declared. _, path := outside.ReferenceOutside() @@ -337,7 +334,7 @@ func vertexReferencePath(referrer dag.Vertex) addrs.ModuleInstance { // Vertex makes references to objects in the same module as where it // was declared. - return sp.Path() + return sp.ModulePath().UnkeyedInstanceShim() } // referenceMapKey produces keys for the "edges" map. "referrer" is the vertex @@ -374,7 +371,7 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { continue } - path := m.vertexReferenceablePath(v) + path := vertexReferenceablePath(v) // Go through and cache them for _, addr := range rn.ReferenceableAddrs() { diff --git a/terraform/transform_reference_test.go b/terraform/transform_reference_test.go index 004dbde48..073d02d34 100644 --- a/terraform/transform_reference_test.go +++ b/terraform/transform_reference_test.go @@ -153,6 +153,10 @@ func (n *graphNodeRefParentTest) Path() addrs.ModuleInstance { return normalizeModulePath(n.PathValue) } +func (n *graphNodeRefParentTest) ModulePath() addrs.Module { + return normalizeModulePath(n.PathValue).Module() +} + type graphNodeRefChildTest struct { NameValue string PathValue []string @@ -179,6 +183,10 @@ func (n *graphNodeRefChildTest) Path() addrs.ModuleInstance { return normalizeModulePath(n.PathValue) } +func (n *graphNodeRefChildTest) ModulePath() addrs.Module { + return normalizeModulePath(n.PathValue).Module() +} + const testTransformRefBasicStr = ` A B