From c2499433600577934c70a7b758cdb13c420242e7 Mon Sep 17 00:00:00 2001 From: Pam Selle Date: Mon, 24 Feb 2020 17:42:32 -0500 Subject: [PATCH] Module Expansion: Part 2 (#24154) * WIP: dynamic expand * WIP: add variable and local support * WIP: outputs * WIP: Add referencer * String representation, fixing tests it impacts * Fixes TestContext2Apply_outputOrphanModule * Fix TestContext2Apply_plannedDestroyInterpolatedCount * Update DestroyOutputTransformer and associated types to reflect PlannableOutputs * Remove comment about locals * Remove module count enablement * Removes allowing count for modules, and reverts the test, while adding a Skip()'d test that works when you re-enable the config * update TargetDownstream signature to match master * remove unnecessary method Co-authored-by: James Bardin --- addrs/input_variable.go | 9 ++ addrs/module.go | 6 +- .../testdata/show-json/modules/output.json | 4 +- internal/modsdir/manifest.go | 7 +- terraform/context_apply_test.go | 1 + terraform/context_plan_test.go | 75 ++++++++++++ terraform/eval_state.go | 19 +-- terraform/node_module_expand.go | 85 ++++++++++---- terraform/node_module_removed.go | 4 +- terraform/node_module_variable.go | 103 +++++++++++++++- terraform/node_output.go | 111 ++++++++++++++++-- terraform/node_output_orphan.go | 2 +- terraform/node_resource_abstract.go | 4 + terraform/node_resource_refresh.go | 19 +-- .../testdata/plan-modules-count/child/main.tf | 12 ++ terraform/testdata/plan-modules-count/main.tf | 28 +++++ terraform/transform_module_expansion.go | 2 + terraform/transform_module_variable.go | 15 ++- terraform/transform_output.go | 22 ++-- terraform/transform_reference.go | 103 +++------------- terraform/transform_reference_test.go | 50 +------- 21 files changed, 468 insertions(+), 213 deletions(-) create mode 100644 terraform/testdata/plan-modules-count/child/main.tf create mode 100644 terraform/testdata/plan-modules-count/main.tf diff --git a/addrs/input_variable.go b/addrs/input_variable.go index 79829f969..975c72f1e 100644 --- a/addrs/input_variable.go +++ b/addrs/input_variable.go @@ -14,6 +14,15 @@ func (v InputVariable) String() string { return "var." + v.Name } +// Absolute converts the receiver into an absolute address within the given +// module instance. +func (v InputVariable) Absolute(m ModuleInstance) AbsInputVariableInstance { + return AbsInputVariableInstance{ + Module: m, + Variable: v, + } +} + // AbsInputVariableInstance is the address of an input variable within a // particular module instance. type AbsInputVariableInstance struct { diff --git a/addrs/module.go b/addrs/module.go index 6420c6301..e2984662c 100644 --- a/addrs/module.go +++ b/addrs/module.go @@ -33,7 +33,11 @@ func (m Module) String() string { if len(m) == 0 { return "" } - return strings.Join([]string(m), ".") + var steps []string + for _, s := range m { + steps = append(steps, "module", s) + } + return strings.Join(steps, ".") } // Child returns the address of a child call in the receiver, identified by the diff --git a/command/testdata/show-json/modules/output.json b/command/testdata/show-json/modules/output.json index be1b6fdf9..13a8702ab 100644 --- a/command/testdata/show-json/modules/output.json +++ b/command/testdata/show-json/modules/output.json @@ -274,8 +274,8 @@ } }, "provider_config": { - "module_test_foo:test": { - "module_address": "module_test_foo", + "module.module_test_foo:test": { + "module_address": "module.module_test_foo", "name": "test" } } diff --git a/internal/modsdir/manifest.go b/internal/modsdir/manifest.go index 92b40899a..56332523d 100644 --- a/internal/modsdir/manifest.go +++ b/internal/modsdir/manifest.go @@ -8,6 +8,7 @@ import ( "log" "os" "path/filepath" + "strings" version "github.com/hashicorp/go-version" @@ -48,7 +49,11 @@ type Record struct { type Manifest map[string]Record func (m Manifest) ModuleKey(path addrs.Module) string { - return path.String() + if len(path) == 0 { + return "" + } + return strings.Join([]string(path), ".") + } // manifestSnapshotFile is an internal struct used only to assist in our JSON diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 2bf81d050..fc26f446b 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -767,6 +767,7 @@ aws_instance.foo: } func TestContext2Apply_emptyModule(t *testing.T) { + // A module with only outputs (no resources) m := testModule(t, "apply-empty-module") p := testProvider("aws") p.ApplyFn = testApplyFn diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index 34e71dd0e..8288d5121 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -429,6 +429,81 @@ func TestContext2Plan_modules(t *testing.T) { checkVals(t, expected, ric.After) } } +func TestContext2Plan_moduleCount(t *testing.T) { + // This test is skipped with count disabled. + t.Skip() + //FIXME: add for_each and single modules to this test + + m := testModule(t, "plan-modules-count") + p := testProvider("aws") + p.DiffFn = testDiffFn + ctx := testContext2(t, &ContextOpts{ + Config: m, + ProviderResolver: providers.ResolverFixed( + map[addrs.Provider]providers.Factory{ + addrs.NewLegacyProvider("aws"): testProviderFuncFixed(p), + }, + ), + }) + + plan, diags := ctx.Plan() + if diags.HasErrors() { + t.Fatalf("unexpected errors: %s", diags.Err()) + } + + if len(plan.Changes.Resources) != 6 { + t.Error("expected 6 resource in plan, got", len(plan.Changes.Resources)) + } + + schema := p.GetSchemaReturn.ResourceTypes["aws_instance"] + ty := schema.ImpliedType() + + expectFoo := objectVal(t, schema, map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "foo": cty.StringVal("2"), + "type": cty.StringVal("aws_instance")}, + ) + + expectNum := objectVal(t, schema, map[string]cty.Value{ + "id": cty.UnknownVal(cty.String), + "num": cty.NumberIntVal(2), + "type": cty.StringVal("aws_instance"), + }) + + expectExpansion := objectVal(t, schema, map[string]cty.Value{ + "bar": cty.StringVal("baz"), + "id": cty.UnknownVal(cty.String), + "num": cty.NumberIntVal(2), + "type": cty.StringVal("aws_instance"), + }) + + for _, res := range plan.Changes.Resources { + if res.Action != plans.Create { + t.Fatalf("expected resource creation, got %s", res.Action) + } + ric, err := res.Decode(ty) + if err != nil { + t.Fatal(err) + } + + var expected cty.Value + switch i := ric.Addr.String(); i { + case "aws_instance.bar": + expected = expectFoo + case "aws_instance.foo": + expected = expectNum + case "module.child[0].aws_instance.foo[0]", + "module.child[0].aws_instance.foo[1]", + "module.child[1].aws_instance.foo[0]", + "module.child[1].aws_instance.foo[1]": + expected = expectExpansion + default: + t.Fatal("unknown instance:", i) + } + + checkVals(t, expected, ric.After) + } +} // GH-1475 func TestContext2Plan_moduleCycle(t *testing.T) { diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 6f8d8ae25..ab55835e2 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -490,15 +490,18 @@ func (n *EvalWriteResourceState) Eval(ctx EvalContext) (interface{}, error) { // 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. + // 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() - switch eachMode { - case states.EachList: - expander.SetResourceCount(ctx.Path(), n.Addr, count) - case states.EachMap: - expander.SetResourceForEach(ctx.Path(), n.Addr, forEach) - default: - expander.SetResourceSingle(ctx.Path(), n.Addr) + for _, module := range expander.ExpandModule(ctx.Path().Module()) { + switch eachMode { + case states.EachList: + expander.SetResourceCount(module, n.Addr, count) + case states.EachMap: + expander.SetResourceForEach(module, n.Addr, forEach) + default: + expander.SetResourceSingle(module, n.Addr) + } } return nil, nil diff --git a/terraform/node_module_expand.go b/terraform/node_module_expand.go index 26501decc..71d8a177f 100644 --- a/terraform/node_module_expand.go +++ b/terraform/node_module_expand.go @@ -5,6 +5,8 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" + "github.com/hashicorp/terraform/lang" + "github.com/hashicorp/terraform/states" ) // nodeExpandModule represents a module call in the configuration that @@ -12,8 +14,10 @@ import ( // configured. type nodeExpandModule struct { CallerAddr addrs.ModuleInstance + Addr addrs.Module Call addrs.ModuleCall Config *configs.Module + ModuleCall *configs.ModuleCall } var ( @@ -29,14 +33,20 @@ func (n *nodeExpandModule) Name() string { // GraphNodeSubPath implementation func (n *nodeExpandModule) Path() addrs.ModuleInstance { - // Notice that the node represents the module call and so we report - // the parent module as the path. The module call we're representing - // might expand into multiple child module instances during our work here. + // 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.CallerAddr } // GraphNodeReferencer implementation func (n *nodeExpandModule) References() []*addrs.Reference { + var refs []*addrs.Reference + + if n.ModuleCall == nil { + return nil + } + // Expansion only uses the count and for_each expressions, so this // particular graph node only refers to those. // Individual variable values in the module call definition might also @@ -47,18 +57,14 @@ func (n *nodeExpandModule) References() []*addrs.Reference { // our call, these references will be correctly interpreted as being // in the calling module's namespace, not the namespaces of any of the // child module instances we might expand to during our evaluation. - var ret []*addrs.Reference - // TODO: Once count and for_each are actually supported, analyze their - // expressions for references here. - /* - if n.Config.Count != nil { - ret = append(ret, n.Config.Count.References()...) - } - if n.Config.ForEach != nil { - ret = append(ret, n.Config.ForEach.References()...) - } - */ - return ret + + if n.ModuleCall.Count != nil { + refs, _ = lang.ReferencesInExpr(n.ModuleCall.Count) + } + if n.ModuleCall.ForEach != nil { + refs, _ = lang.ReferencesInExpr(n.ModuleCall.ForEach) + } + return appendResourceDestroyReferences(refs) } // RemovableIfNotTargeted implementation @@ -74,23 +80,56 @@ func (n *nodeExpandModule) EvalTree() EvalNode { CallerAddr: n.CallerAddr, Call: n.Call, Config: n.Config, + ModuleCall: n.ModuleCall, } } +// evalPrepareModuleExpansion is an EvalNode implementation +// that sets the count or for_each on the instance expander type evalPrepareModuleExpansion struct { CallerAddr addrs.ModuleInstance Call addrs.ModuleCall Config *configs.Module + ModuleCall *configs.ModuleCall } func (n *evalPrepareModuleExpansion) Eval(ctx EvalContext) (interface{}, error) { - // Modules don't support any of the repetition arguments yet, so their - // expansion type is always "single". We just record this here to make - // the expander data structure consistent for now. - // FIXME: Once the rest of Terraform Core is ready to support expanding - // modules, evaluate the "count" and "for_each" arguments here in a - // similar way as in EvalWriteResourceState. - log.Printf("[TRACE] evalPrepareModuleExpansion: %s is a singleton", n.CallerAddr.Child(n.Call.Name, addrs.NoKey)) - ctx.InstanceExpander().SetModuleSingle(n.CallerAddr, n.Call) + eachMode := states.NoEach + expander := ctx.InstanceExpander() + + if n.ModuleCall == nil { + // FIXME: should we have gotten here with no module call? + log.Printf("[TRACE] evalPrepareModuleExpansion: %s is a singleton", n.CallerAddr.Child(n.Call.Name, addrs.NoKey)) + expander.SetModuleSingle(n.CallerAddr, n.Call) + return nil, nil + } + + count, countDiags := evaluateResourceCountExpression(n.ModuleCall.Count, ctx) + if countDiags.HasErrors() { + return nil, countDiags.Err() + } + + if count >= 0 { // -1 signals "count not set" + eachMode = states.EachList + } + + forEach, forEachDiags := evaluateResourceForEachExpression(n.ModuleCall.ForEach, ctx) + if forEachDiags.HasErrors() { + return nil, forEachDiags.Err() + } + + if forEach != nil { + eachMode = states.EachMap + } + + switch eachMode { + case states.EachList: + expander.SetModuleCount(ctx.Path(), n.Call, count) + case states.EachMap: + expander.SetModuleForEach(ctx.Path(), n.Call, forEach) + default: + expander.SetModuleSingle(n.CallerAddr, n.Call) + } + return nil, nil } diff --git a/terraform/node_module_removed.go b/terraform/node_module_removed.go index 99e440903..43d9bcd4b 100644 --- a/terraform/node_module_removed.go +++ b/terraform/node_module_removed.go @@ -39,12 +39,12 @@ func (n *NodeModuleRemoved) EvalTree() EvalNode { } } -func (n *NodeModuleRemoved) ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) { +func (n *NodeModuleRemoved) ReferenceOutside() (selfPath, referencePath addrs.Module) { // Our "References" implementation indicates that this node depends on // the call to the module it represents, which implicitly depends on // everything inside the module. That reference must therefore be // interpreted in terms of our parent module. - return n.Addr, n.Addr.Parent() + return n.Addr.Module(), n.Addr.Parent().Module() } func (n *NodeModuleRemoved) References() []*addrs.Reference { diff --git a/terraform/node_module_variable.go b/terraform/node_module_variable.go index 7494e0045..25954edbc 100644 --- a/terraform/node_module_variable.go +++ b/terraform/node_module_variable.go @@ -1,6 +1,8 @@ package terraform import ( + "fmt" + "github.com/hashicorp/hcl/v2" "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" @@ -9,6 +11,101 @@ import ( "github.com/zclconf/go-cty/cty" ) +// NodePlannableModuleVariable is the placeholder for an variable that has not yet had +// its module path expanded. +type NodePlannableModuleVariable struct { + Addr addrs.InputVariable + Module addrs.Module + Config *configs.Variable + Expr hcl.Expression +} + +var ( + _ GraphNodeDynamicExpandable = (*NodePlannableModuleVariable)(nil) + _ GraphNodeReferenceOutside = (*NodePlannableModuleVariable)(nil) + _ GraphNodeReferenceable = (*NodePlannableModuleVariable)(nil) + _ GraphNodeReferencer = (*NodePlannableModuleVariable)(nil) + _ GraphNodeSubPath = (*NodePlannableModuleVariable)(nil) + _ RemovableIfNotTargeted = (*NodePlannableModuleVariable)(nil) +) + +func (n *NodePlannableModuleVariable) DynamicExpand(ctx EvalContext) (*Graph, error) { + var g Graph + expander := ctx.InstanceExpander() + for _, module := range expander.ExpandModule(ctx.Path().Module()) { + o := &NodeApplyableModuleVariable{ + Addr: n.Addr.Absolute(module), + Config: n.Config, + Expr: n.Expr, + } + g.Add(o) + } + return &g, nil +} + +func (n *NodePlannableModuleVariable) Name() string { + return fmt.Sprintf("%s.%s", n.Module, n.Addr.String()) +} + +// GraphNodeSubPath +func (n *NodePlannableModuleVariable) Path() addrs.ModuleInstance { + // Return an UnkeyedInstanceShim as our placeholder, + // given that modules will be unexpanded at this point in the walk + return n.Module.UnkeyedInstanceShim() +} + +// GraphNodeReferencer +func (n *NodePlannableModuleVariable) References() []*addrs.Reference { + + // If we have no value expression, we cannot depend on anything. + if n.Expr == nil { + return nil + } + + // Variables in the root don't depend on anything, because their values + // are gathered prior to the graph walk and recorded in the context. + if len(n.Module) == 0 { + return nil + } + + // Otherwise, we depend on anything referenced by our value expression. + // We ignore diagnostics here under the assumption that we'll re-eval + // all these things later and catch them then; for our purposes here, + // we only care about valid references. + // + // Due to our GraphNodeReferenceOutside implementation, the addresses + // returned by this function are interpreted in the _parent_ module from + // where our associated variable was declared, which is correct because + // our value expression is assigned within a "module" block in the parent + // module. + refs, _ := lang.ReferencesInExpr(n.Expr) + return refs +} + +// GraphNodeReferenceOutside implementation +func (n *NodePlannableModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.Module) { + return n.Module, n.Module.Parent() +} + +// GraphNodeReferenceable +func (n *NodePlannableModuleVariable) ReferenceableAddrs() []addrs.Referenceable { + // FIXME: References for module variables probably need to be thought out a bit more + // Otherwise, we can reference the output via the address itself, or the + // module call + _, call := n.Module.Call() + return []addrs.Referenceable{n.Addr, call} +} + +// RemovableIfNotTargeted +func (n *NodePlannableModuleVariable) RemoveIfNotTargeted() bool { + return true +} + +// GraphNodeTargetDownstream +func (n *NodePlannableModuleVariable) TargetDownstream(targetedDeps, untargetedDeps dag.Set) bool { + return true +} + // NodeApplyableModuleVariable represents a module variable input during // the apply step. type NodeApplyableModuleVariable struct { @@ -48,15 +145,15 @@ func (n *NodeApplyableModuleVariable) RemoveIfNotTargeted() bool { } // GraphNodeReferenceOutside implementation -func (n *NodeApplyableModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) { +func (n *NodeApplyableModuleVariable) ReferenceOutside() (selfPath, referencePath addrs.Module) { // Module input variables have their value expressions defined in the // context of their calling (parent) module, and so references from // a node of this type should be resolved in the parent module instance. - referencePath = n.Addr.Module.Parent() + referencePath = n.Addr.Module.Parent().Module() // Input variables are _referenced_ from their own module, though. - selfPath = n.Addr.Module + selfPath = n.Addr.Module.Module() return // uses named return values } diff --git a/terraform/node_output.go b/terraform/node_output.go index 94300f27b..063611916 100644 --- a/terraform/node_output.go +++ b/terraform/node_output.go @@ -9,6 +9,98 @@ import ( "github.com/hashicorp/terraform/lang" ) +// NodePlannableOutput is the placeholder for an output that has not yet had +// its module path expanded. +type NodePlannableOutput struct { + Addr addrs.OutputValue + Module addrs.Module + Config *configs.Output +} + +var ( + _ GraphNodeSubPath = (*NodePlannableOutput)(nil) + _ RemovableIfNotTargeted = (*NodePlannableOutput)(nil) + _ GraphNodeReferenceable = (*NodePlannableOutput)(nil) + //_ GraphNodeEvalable = (*NodePlannableOutput)(nil) + _ GraphNodeReferencer = (*NodePlannableOutput)(nil) + _ GraphNodeDynamicExpandable = (*NodePlannableOutput)(nil) +) + +func (n *NodePlannableOutput) DynamicExpand(ctx EvalContext) (*Graph, error) { + var g Graph + expander := ctx.InstanceExpander() + for _, module := range expander.ExpandModule(ctx.Path().Module()) { + o := &NodeApplyableOutput{ + Addr: n.Addr.Absolute(module), + Config: n.Config, + } + // log.Printf("[TRACE] Expanding output: adding %s as %T", o.Addr.String(), o) + g.Add(o) + } + return &g, nil +} + +func (n *NodePlannableOutput) Name() string { + return n.Addr.Absolute(n.Module.UnkeyedInstanceShim()).String() +} + +// GraphNodeSubPath +func (n *NodePlannableOutput) Path() addrs.ModuleInstance { + // Return an UnkeyedInstanceShim as our placeholder, + // given that modules will be unexpanded at this point in the walk + return n.Module.UnkeyedInstanceShim() +} + +// GraphNodeReferenceable +func (n *NodePlannableOutput) ReferenceableAddrs() []addrs.Referenceable { + // An output in the root module can't be referenced at all. + if n.Module.IsRoot() { + return nil + } + + // the output is referenced through the module call, and via the + // module itself. + _, call := n.Module.Call() + + // FIXME: make something like ModuleCallOutput for this type of reference + // that doesn't need an instance shim + callOutput := addrs.ModuleCallOutput{ + Call: call.Instance(addrs.NoKey), + Name: n.Addr.Name, + } + + // Otherwise, we can reference the output via the + // module call itself + return []addrs.Referenceable{call, callOutput} +} + +// GraphNodeReferenceOutside implementation +func (n *NodePlannableOutput) ReferenceOutside() (selfPath, referencePath addrs.Module) { + // Output values have their expressions resolved in the context of the + // module where they are defined. + referencePath = n.Module + + // ...but they are referenced in the context of their calling module. + selfPath = referencePath.Parent() + + return // uses named return values +} + +// GraphNodeReferencer +func (n *NodePlannableOutput) References() []*addrs.Reference { + return appendResourceDestroyReferences(referencesForOutput(n.Config)) +} + +// RemovableIfNotTargeted +func (n *NodePlannableOutput) RemoveIfNotTargeted() bool { + return true +} + +// GraphNodeTargetDownstream +func (n *NodePlannableOutput) TargetDownstream(targetedDeps, untargetedDeps dag.Set) bool { + return true +} + // NodeApplyableOutput represents an output that is "applyable": // it is ready to be applied. type NodeApplyableOutput struct { @@ -51,21 +143,19 @@ func (n *NodeApplyableOutput) TargetDownstream(targetedDeps, untargetedDeps dag. return true } -func referenceOutsideForOutput(addr addrs.AbsOutputValue) (selfPath, referencePath addrs.ModuleInstance) { - +func referenceOutsideForOutput(addr addrs.AbsOutputValue) (selfPath, referencePath addrs.Module) { // Output values have their expressions resolved in the context of the // module where they are defined. - referencePath = addr.Module + referencePath = addr.Module.Module() // ...but they are referenced in the context of their calling module. - selfPath = addr.Module.Parent() + selfPath = addr.Module.Parent().Module() return // uses named return values - } // GraphNodeReferenceOutside implementation -func (n *NodeApplyableOutput) ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) { +func (n *NodeApplyableOutput) ReferenceOutside() (selfPath, referencePath addrs.Module) { return referenceOutsideForOutput(n.Addr) } @@ -83,8 +173,8 @@ func referenceableAddrsForOutput(addr addrs.AbsOutputValue) []addrs.Referenceabl // was declared. _, outp := addr.ModuleCallOutput() _, call := addr.Module.CallInstance() - return []addrs.Referenceable{outp, call} + return []addrs.Referenceable{outp, call} } // GraphNodeReferenceable @@ -141,7 +231,8 @@ 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.AbsOutputValue + Addr addrs.OutputValue + Module addrs.Module Config *configs.Output // Config is the output in the config } @@ -160,7 +251,7 @@ func (n *NodeDestroyableOutput) Name() string { // GraphNodeSubPath func (n *NodeDestroyableOutput) Path() addrs.ModuleInstance { - return n.Addr.Module + return n.Module.UnkeyedInstanceShim() } // RemovableIfNotTargeted @@ -184,7 +275,7 @@ func (n *NodeDestroyableOutput) References() []*addrs.Reference { // GraphNodeEvalable func (n *NodeDestroyableOutput) EvalTree() EvalNode { return &EvalDeleteOutput{ - Addr: n.Addr.OutputValue, + Addr: n.Addr, } } diff --git a/terraform/node_output_orphan.go b/terraform/node_output_orphan.go index 518b8aa09..f8f7124c6 100644 --- a/terraform/node_output_orphan.go +++ b/terraform/node_output_orphan.go @@ -23,7 +23,7 @@ func (n *NodeOutputOrphan) Name() string { } // GraphNodeReferenceOutside implementation -func (n *NodeOutputOrphan) ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) { +func (n *NodeOutputOrphan) ReferenceOutside() (selfPath, referencePath addrs.Module) { return referenceOutsideForOutput(n.Addr) } diff --git a/terraform/node_resource_abstract.go b/terraform/node_resource_abstract.go index bab23fd5e..09ca14abd 100644 --- a/terraform/node_resource_abstract.go +++ b/terraform/node_resource_abstract.go @@ -43,6 +43,10 @@ type GraphNodeResourceInstance interface { // operations. It registers all the interfaces for a resource that common // across multiple operation types. type NodeAbstractResource struct { + //FIXME: AbstractResources are no longer absolute, because modules are not expanded. + // Addr addrs.Resource + // Module addrs.Module + Addr addrs.AbsResource // Addr is the address for this resource // The fields below will be automatically set using the Attach diff --git a/terraform/node_resource_refresh.go b/terraform/node_resource_refresh.go index 945516747..fa1590093 100644 --- a/terraform/node_resource_refresh.go +++ b/terraform/node_resource_refresh.go @@ -14,7 +14,7 @@ import ( "github.com/hashicorp/terraform/tfdiags" ) -// NodeRefreshableManagedResource represents a resource that is expanabled into +// NodeRefreshableManagedResource represents a resource that is expandable into // NodeRefreshableManagedResourceInstance. Resource count orphans are also added. type NodeRefreshableManagedResource struct { *NodeAbstractResource @@ -61,13 +61,16 @@ 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() - 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 _, module := range expander.ExpandModule(ctx.Path().Module()) { + switch { + case count >= 0: + expander.SetResourceCount(module, n.ResourceAddr().Resource, count) + case forEachMap != nil: + expander.SetResourceForEach(module, n.ResourceAddr().Resource, forEachMap) + default: + expander.SetResourceSingle(module, n.ResourceAddr().Resource) + } } instanceAddrs := expander.ExpandResource(ctx.Path().Module(), n.ResourceAddr().Resource) diff --git a/terraform/testdata/plan-modules-count/child/main.tf b/terraform/testdata/plan-modules-count/child/main.tf new file mode 100644 index 000000000..612478f79 --- /dev/null +++ b/terraform/testdata/plan-modules-count/child/main.tf @@ -0,0 +1,12 @@ +variable "foo" {} +variable "bar" {} + +resource "aws_instance" "foo" { + count = 2 + num = var.foo + bar = "baz" #var.bar +} + +output "out" { + value = aws_instance.foo[0].id +} diff --git a/terraform/testdata/plan-modules-count/main.tf b/terraform/testdata/plan-modules-count/main.tf new file mode 100644 index 000000000..eeb5fa001 --- /dev/null +++ b/terraform/testdata/plan-modules-count/main.tf @@ -0,0 +1,28 @@ +locals { + val = 2 + bar = "baz" +} + +variable "myvar" { + default = "baz" +} + + +module "child" { + count = local.val + foo = 2 + bar = var.myvar + source = "./child" +} + +output "out" { + value = module.child[*].out +} + +resource "aws_instance" "foo" { + num = 2 +} + +resource "aws_instance" "bar" { + foo = "${aws_instance.foo.num}" +} diff --git a/terraform/transform_module_expansion.go b/terraform/transform_module_expansion.go index 9af58861e..e8d2c4a32 100644 --- a/terraform/transform_module_expansion.go +++ b/terraform/transform_module_expansion.go @@ -43,10 +43,12 @@ func (t *ModuleExpansionTransformer) transform(g *Graph, c *configs.Config, pare fullAddr := c.Path.UnkeyedInstanceShim() callerAddr, callAddr := fullAddr.Call() + modulecall := c.Parent.Module.ModuleCalls["child"] v := &nodeExpandModule{ CallerAddr: callerAddr, Call: callAddr, Config: c.Module, + ModuleCall: modulecall, } g.Add(v) log.Printf("[TRACE] ModuleExpansionTransformer: Added %s as %T", fullAddr, v) diff --git a/terraform/transform_module_variable.go b/terraform/transform_module_variable.go index 18e0b2d1f..2cbb5cd11 100644 --- a/terraform/transform_module_variable.go +++ b/terraform/transform_module_variable.go @@ -4,6 +4,7 @@ import ( "fmt" "github.com/hashicorp/hcl/v2/hclsyntax" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/tfdiags" "github.com/zclconf/go-cty/cty" @@ -110,15 +111,17 @@ func (t *ModuleVariableTransformer) transformSingle(g *Graph, parent, c *configs } } - // For now we treat all module variables as "applyable", even though - // such nodes are valid to use on other walks too. We may specialize - // this in future if we find reasons to employ different behaviors - // in different scenarios. - node := &NodeApplyableModuleVariable{ - Addr: path.InputVariable(v.Name), + // Add a plannable node, as the variable may expand + // during module expansion + node := &NodePlannableModuleVariable{ + Addr: addrs.InputVariable{ + Name: v.Name, + }, + Module: c.Path, Config: v, Expr: expr, } + g.Add(node) } diff --git a/terraform/transform_output.go b/terraform/transform_output.go index 7f80e3ec3..8e19000af 100644 --- a/terraform/transform_output.go +++ b/terraform/transform_output.go @@ -3,6 +3,7 @@ package terraform import ( "log" + "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/configs" "github.com/hashicorp/terraform/dag" ) @@ -36,20 +37,16 @@ func (t *OutputTransformer) transform(g *Graph, c *configs.Config) error { } } - // Our addressing system distinguishes between modules and module instances, - // but we're not yet ready to make that distinction here (since we don't - // support "count"/"for_each" on modules) and so we just do a naive - // transform of the module path into a module instance path, assuming that - // no keys are in use. This should be removed when "count" and "for_each" - // are implemented for modules. - path := c.Path.UnkeyedInstanceShim() - + // Add plannable outputs to the graph, which will be dynamically expanded + // into NodeApplyableOutputs to reflect possible expansion + // through the presence of "count" or "for_each" on the modules. for _, o := range c.Module.Outputs { - addr := path.OutputValue(o.Name) - node := &NodeApplyableOutput{ - Addr: addr, + node := &NodePlannableOutput{ + Addr: addrs.OutputValue{Name: o.Name}, + Module: c.Path, Config: o, } + log.Printf("[TRACE] OutputTransformer: adding %s as %T", o.Name, node) g.Add(node) } @@ -70,7 +67,7 @@ func (t *DestroyOutputTransformer) Transform(g *Graph) error { } for _, v := range g.Vertices() { - output, ok := v.(*NodeApplyableOutput) + output, ok := v.(*NodePlannableOutput) if !ok { continue } @@ -78,6 +75,7 @@ func (t *DestroyOutputTransformer) Transform(g *Graph) error { // create the destroy node for this output node := &NodeDestroyableOutput{ Addr: output.Addr, + Module: output.Module, Config: output.Config, } diff --git a/terraform/transform_reference.go b/terraform/transform_reference.go index 48d38db4f..9b397ccd4 100644 --- a/terraform/transform_reference.go +++ b/terraform/transform_reference.go @@ -66,7 +66,7 @@ type GraphNodeAttachDependencies interface { type GraphNodeReferenceOutside interface { // ReferenceOutside returns a path in which any references from this node // are resolved. - ReferenceOutside() (selfPath, referencePath addrs.ModuleInstance) + ReferenceOutside() (selfPath, referencePath addrs.Module) } // ReferenceTransformer is a GraphTransformer that connects all the @@ -85,8 +85,7 @@ func (t *ReferenceTransformer) Transform(g *Graph) error { // use their own state. continue } - - parents, _ := m.References(v) + parents := m.References(v) parentsDbg := make([]string, len(parents)) for i, v := range parents { parentsDbg[i] = dag.VertexName(v) @@ -196,7 +195,12 @@ func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error { if v.Addr.Module.IsRoot() && !t.Destroy { continue } - case *NodeLocal, *NodeApplyableModuleVariable: + case *NodePlannableOutput: + // Have similar guardrails for plannable outputs as applyable above + if v.Module.IsRoot() && !t.Destroy { + continue + } + case *NodeLocal, *NodeApplyableModuleVariable, *NodePlannableModuleVariable: // OK default: // We're only concerned with variables, locals and outputs @@ -239,27 +243,20 @@ type ReferenceMap struct { // A particular reference key might actually identify multiple vertices, // e.g. in situations where one object is contained inside another. vertices map[string][]dag.Vertex - - // edges is a map whose keys are a subset of the internal reference keys - // from "vertices", and whose values are the nodes that refer to each - // key. The values in this map are the referrers, while values in - // "verticies" are the referents. The keys in both cases are referents. - edges map[string][]dag.Vertex } // References returns the set of vertices that the given vertex refers to, // and any referenced addresses that do not have corresponding vertices. -func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []addrs.Referenceable) { +func (m *ReferenceMap) References(v dag.Vertex) []dag.Vertex { rn, ok := v.(GraphNodeReferencer) if !ok { - return nil, nil + return nil } if _, ok := v.(GraphNodeSubPath); !ok { - return nil, nil + return nil } var matches []dag.Vertex - var missing []addrs.Referenceable for _, ref := range rn.References() { subject := ref.Subject @@ -278,7 +275,6 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []addrs.Reference } key = m.referenceMapKey(v, subject) } - vertices := m.vertices[key] for _, rv := range vertices { // don't include self-references @@ -287,47 +283,6 @@ func (m *ReferenceMap) References(v dag.Vertex) ([]dag.Vertex, []addrs.Reference } matches = append(matches, rv) } - if len(vertices) == 0 { - missing = append(missing, ref.Subject) - } - } - - return matches, missing -} - -// Referrers returns the set of vertices that refer to the given vertex. -func (m *ReferenceMap) Referrers(v dag.Vertex) []dag.Vertex { - rn, ok := v.(GraphNodeReferenceable) - if !ok { - return nil - } - sp, ok := v.(GraphNodeSubPath) - if !ok { - return nil - } - - var matches []dag.Vertex - for _, addr := range rn.ReferenceableAddrs() { - key := m.mapKey(sp.Path(), addr) - referrers, ok := m.edges[key] - if !ok { - continue - } - - // If the referrer set includes our own given vertex then we skip, - // since we don't want to return self-references. - selfRef := false - for _, p := range referrers { - if p == v { - selfRef = true - break - } - } - if selfRef { - continue - } - - matches = append(matches, referrers...) } return matches @@ -354,7 +309,7 @@ func (m *ReferenceMap) vertexReferenceablePath(v dag.Vertex) addrs.ModuleInstanc // Vertex is referenced from a different module than where it was // declared. path, _ := outside.ReferenceOutside() - return path + return path.UnkeyedInstanceShim() } // Vertex is referenced from the same module as where it was declared. @@ -373,12 +328,11 @@ func vertexReferencePath(referrer dag.Vertex) addrs.ModuleInstance { panic(fmt.Errorf("vertexReferencePath on vertex type %T which doesn't implement GraphNodeSubPath", sp)) } - var path addrs.ModuleInstance if outside, ok := referrer.(GraphNodeReferenceOutside); ok { // Vertex makes references to objects in a different module than where // it was declared. - _, path = outside.ReferenceOutside() - return path + _, path := outside.ReferenceOutside() + return path.UnkeyedInstanceShim() } // Vertex makes references to objects in the same module as where it @@ -443,34 +397,7 @@ func NewReferenceMap(vs []dag.Vertex) *ReferenceMap { } } - // Build the lookup table for referenced by - edges := make(map[string][]dag.Vertex) - for _, v := range vs { - _, ok := v.(GraphNodeSubPath) - if !ok { - // Only nodes with paths can participate in a reference map. - continue - } - - rn, ok := v.(GraphNodeReferencer) - if !ok { - // We're only looking for referenceable nodes - continue - } - - // Go through and cache them - for _, ref := range rn.References() { - if ref.Subject == nil { - // Should never happen - panic(fmt.Sprintf("%T.References returned reference with nil subject", rn)) - } - key := m.referenceMapKey(v, ref.Subject) - edges[key] = append(edges[key], v) - } - } - m.vertices = vertices - m.edges = edges return &m } @@ -503,6 +430,8 @@ func appendResourceDestroyReferences(refs []*addrs.Reference) []*addrs.Reference newRef.Subject = tr.Phase(addrs.ResourceInstancePhaseDestroy) refs = append(refs, &newRef) } + // FIXME: Using this method in module expansion references, + // May want to refactor this method beyond resources } return refs } diff --git a/terraform/transform_reference_test.go b/terraform/transform_reference_test.go index ad32b1376..004dbde48 100644 --- a/terraform/transform_reference_test.go +++ b/terraform/transform_reference_test.go @@ -113,55 +113,7 @@ func TestReferenceMapReferences(t *testing.T) { for tn, tc := range cases { t.Run(tn, func(t *testing.T) { rm := NewReferenceMap(tc.Nodes) - result, _ := rm.References(tc.Check) - - var resultStr []string - for _, v := range result { - resultStr = append(resultStr, dag.VertexName(v)) - } - - sort.Strings(resultStr) - sort.Strings(tc.Result) - if !reflect.DeepEqual(resultStr, tc.Result) { - t.Fatalf("bad: %#v", resultStr) - } - }) - } -} - -func TestReferenceMapReferencedBy(t *testing.T) { - cases := map[string]struct { - Nodes []dag.Vertex - Check dag.Vertex - Result []string - }{ - "simple": { - Nodes: []dag.Vertex{ - &graphNodeRefChildTest{ - NameValue: "A", - Refs: []string{"A"}, - }, - &graphNodeRefChildTest{ - NameValue: "B", - Refs: []string{"A"}, - }, - &graphNodeRefChildTest{ - NameValue: "C", - Refs: []string{"B"}, - }, - }, - Check: &graphNodeRefParentTest{ - NameValue: "foo", - Names: []string{"A"}, - }, - Result: []string{"A", "B"}, - }, - } - - for tn, tc := range cases { - t.Run(tn, func(t *testing.T) { - rm := NewReferenceMap(tc.Nodes) - result := rm.Referrers(tc.Check) + result := rm.References(tc.Check) var resultStr []string for _, v := range result {