Merge pull request #24697 from hashicorp/jbardin/get-module-data

Always return all module instances during evaluation

addrs/parse_ref.go

@@ -120,10 +120,9 @@ func parseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
 			return nil, diags
 		}
 
-		// A traversal starting with "module" can either be a reference to
+		// A traversal starting with "module" can either be a reference to an
-		// an entire module instance or to a single output from a module
+		// entire module, or to a single output from a module instance,
-		// instance, depending on what we find after this introducer.
+		// depending on what we find after this introducer.
-
 		callInstance := ModuleCallInstance{
 			Call: ModuleCall{
 				Name: callName,
@@ -132,12 +131,12 @@ func parseRef(traversal hcl.Traversal) (*Reference, tfdiags.Diagnostics) {
 		}
 
 		if len(remain) == 0 {
-			// Reference to an entire module instance. Might alternatively
+			// Reference to an entire module. Might alternatively be a
-			// be a reference to a collection of instances of a particular
+			// reference to a single instance of a particular module, but the
-			// module, but the caller will need to deal with that ambiguity
+			// caller will need to deal with that ambiguity since we don't have
-			// since we don't have enough context here.
+			// enough context here.
 			return &Reference{
-				Subject:     callInstance,
+				Subject:     callInstance.Call,
 				SourceRange: tfdiags.SourceRangeFromHCL(callRange),
 				Remaining:   remain,
 			}, diags

addrs/parse_ref_test.go

@@ -281,11 +281,9 @@ func TestParseRef(t *testing.T) {
 		{
 			`module.foo`,
 			&Reference{
-				Subject: ModuleCallInstance{
+				Subject: ModuleCall{
-					Call: ModuleCall{
 					Name: "foo",
 				},
-				},
 				SourceRange: tfdiags.SourceRange{
 					Start: tfdiags.SourcePos{Line: 1, Column: 1, Byte: 0},
 					End:   tfdiags.SourcePos{Line: 1, Column: 11, Byte: 10},

lang/data.go

@@ -26,8 +26,7 @@ type Data interface {
 	GetForEachAttr(addrs.ForEachAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetResource(addrs.Resource, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetLocalValue(addrs.LocalValue, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
-	GetModuleInstance(addrs.ModuleCallInstance, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
+	GetModule(addrs.ModuleCall, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
-	GetModuleInstanceOutput(addrs.AbsModuleCallOutput, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetPathAttr(addrs.PathAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetTerraformAttr(addrs.TerraformAttr, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)
 	GetInputVariable(addrs.InputVariable, tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics)

lang/data_test.go

@@ -43,7 +43,7 @@ func (d *dataForTests) GetLocalValue(addr addrs.LocalValue, rng tfdiags.SourceRa
 	return d.LocalValues[addr.Name], nil
 }
 
-func (d *dataForTests) GetModuleInstance(addr addrs.ModuleCallInstance, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
+func (d *dataForTests) GetModule(addr addrs.ModuleCall, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
 	return d.Modules[addr.String()], nil
 }
 

lang/eval.go

@@ -2,8 +2,6 @@ package lang
 
 import (
 	"fmt"
-	"log"
-	"strconv"
 
 	"github.com/hashicorp/hcl/v2"
 	"github.com/hashicorp/hcl/v2/ext/dynblock"
@@ -196,8 +194,7 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 	// that's redundant in the process of populating our values map.
 	dataResources := map[string]map[string]cty.Value{}
 	managedResources := map[string]map[string]cty.Value{}
-	wholeModules := map[string]map[addrs.InstanceKey]cty.Value{}
+	wholeModules := map[string]cty.Value{}
-	moduleOutputs := map[string]map[addrs.InstanceKey]map[string]cty.Value{}
 	inputVariables := map[string]cty.Value{}
 	localValues := map[string]cty.Value{}
 	pathAttrs := map[string]cty.Value{}
@@ -258,9 +255,14 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 
 		// This type switch must cover all of the "Referenceable" implementations
 		// in package addrs, however we are removing the possibility of
-		// ResourceInstance beforehand.
+		// Instances beforehand.
-		if addr, ok := rawSubj.(addrs.ResourceInstance); ok {
+		switch addr := rawSubj.(type) {
+		case addrs.ResourceInstance:
 			rawSubj = addr.ContainingResource()
+		case addrs.ModuleCallInstance:
+			rawSubj = addr.Call
+		case addrs.AbsModuleCallOutput:
+			rawSubj = addr.Call.Call
 		}
 
 		switch subj := rawSubj.(type) {
@@ -284,28 +286,10 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 			}
 			into[r.Type][r.Name] = val
 
-		case addrs.ModuleCallInstance:
+		case addrs.ModuleCall:
-			val, valDiags := normalizeRefValue(s.Data.GetModuleInstance(subj, rng))
+			val, valDiags := normalizeRefValue(s.Data.GetModule(subj, rng))
 			diags = diags.Append(valDiags)
-
+			wholeModules[subj.Name] = val
-			if wholeModules[subj.Call.Name] == nil {
-				wholeModules[subj.Call.Name] = make(map[addrs.InstanceKey]cty.Value)
-			}
-			wholeModules[subj.Call.Name][subj.Key] = val
-
-		case addrs.AbsModuleCallOutput:
-			val, valDiags := normalizeRefValue(s.Data.GetModuleInstanceOutput(subj, rng))
-			diags = diags.Append(valDiags)
-
-			callName := subj.Call.Call.Name
-			callKey := subj.Call.Key
-			if moduleOutputs[callName] == nil {
-				moduleOutputs[callName] = make(map[addrs.InstanceKey]map[string]cty.Value)
-			}
-			if moduleOutputs[callName][callKey] == nil {
-				moduleOutputs[callName][callKey] = make(map[string]cty.Value)
-			}
-			moduleOutputs[callName][callKey][subj.Name] = val
 
 		case addrs.InputVariable:
 			val, valDiags := normalizeRefValue(s.Data.GetInputVariable(subj, rng))
@@ -347,7 +331,7 @@ func (s *Scope) evalContext(refs []*addrs.Reference, selfAddr addrs.Referenceabl
 		vals[k] = v
 	}
 	vals["data"] = cty.ObjectVal(buildResourceObjects(dataResources))
-	vals["module"] = cty.ObjectVal(buildModuleObjects(wholeModules, moduleOutputs))
+	vals["module"] = cty.ObjectVal(wholeModules)
 	vals["var"] = cty.ObjectVal(inputVariables)
 	vals["local"] = cty.ObjectVal(localValues)
 	vals["path"] = cty.ObjectVal(pathAttrs)
@@ -369,102 +353,6 @@ func buildResourceObjects(resources map[string]map[string]cty.Value) map[string]
 	return vals
 }
 
-func buildModuleObjects(wholeModules map[string]map[addrs.InstanceKey]cty.Value, moduleOutputs map[string]map[addrs.InstanceKey]map[string]cty.Value) map[string]cty.Value {
-	vals := make(map[string]cty.Value)
-
-	for name, keys := range wholeModules {
-		vals[name] = buildInstanceObjects(keys)
-	}
-
-	for name, keys := range moduleOutputs {
-		if _, exists := wholeModules[name]; exists {
-			// If we also have a whole module value for this name then we'll
-			// skip this since the individual outputs are embedded in that result.
-			continue
-		}
-
-		// The shape of this collection isn't compatible with buildInstanceObjects,
-		// but rather than replicating most of the buildInstanceObjects logic
-		// here we'll instead first transform the structure to be what that
-		// function expects and then use it. This is a little wasteful, but
-		// we do not expect this these maps to be large and so the extra work
-		// here should not hurt too much.
-		flattened := make(map[addrs.InstanceKey]cty.Value, len(keys))
-		for k, vals := range keys {
-			flattened[k] = cty.ObjectVal(vals)
-		}
-		vals[name] = buildInstanceObjects(flattened)
-	}
-
-	return vals
-}
-
-func buildInstanceObjects(keys map[addrs.InstanceKey]cty.Value) cty.Value {
-	if val, exists := keys[addrs.NoKey]; exists {
-		// If present, a "no key" value supersedes all other values,
-		// since they should be embedded inside it.
-		return val
-	}
-
-	// If we only have individual values then we need to construct
-	// either a list or a map, depending on what sort of keys we
-	// have.
-	haveInt := false
-	haveString := false
-	maxInt := 0
-
-	for k := range keys {
-		switch tk := k.(type) {
-		case addrs.IntKey:
-			haveInt = true
-			if int(tk) > maxInt {
-				maxInt = int(tk)
-			}
-		case addrs.StringKey:
-			haveString = true
-		}
-	}
-
-	// We should either have ints or strings and not both, but
-	// if we have both then we'll prefer strings and let the
-	// language interpreter try to convert the int keys into
-	// strings in a map.
-	switch {
-	case haveString:
-		vals := make(map[string]cty.Value)
-		for k, v := range keys {
-			switch tk := k.(type) {
-			case addrs.StringKey:
-				vals[string(tk)] = v
-			case addrs.IntKey:
-				sk := strconv.Itoa(int(tk))
-				vals[sk] = v
-			}
-		}
-		return cty.ObjectVal(vals)
-	case haveInt:
-		// We'll make a tuple that is long enough for our maximum
-		// index value. It doesn't matter if we end up shorter than
-		// the number of instances because if length(...) were
-		// being evaluated we would've got a NoKey reference and
-		// thus not ended up in this codepath at all.
-		vals := make([]cty.Value, maxInt+1)
-		for i := range vals {
-			if v, exists := keys[addrs.IntKey(i)]; exists {
-				vals[i] = v
-			} else {
-				// Just a placeholder, since nothing will access this anyway
-				vals[i] = cty.DynamicVal
-			}
-		}
-		return cty.TupleVal(vals)
-	default:
-		// Should never happen because there are no other key types.
-		log.Printf("[ERROR] strange makeInstanceObjects call with no supported key types")
-		return cty.EmptyObjectVal
-	}
-}
-
 func normalizeRefValue(val cty.Value, diags tfdiags.Diagnostics) (cty.Value, tfdiags.Diagnostics) {
 	if diags.HasErrors() {
 		// If there are errors then we will force an unknown result so that

lang/eval_test.go

@@ -178,6 +178,22 @@ func TestScopeEvalContext(t *testing.T) {
 				}),
 			},
 		},
+		{
+			// at this level, all instance references return the entire resource
+			`null_resource.each["each1"].attr`,
+			map[string]cty.Value{
+				"null_resource": cty.ObjectVal(map[string]cty.Value{
+					"each": cty.ObjectVal(map[string]cty.Value{
+						"each0": cty.ObjectVal(map[string]cty.Value{
+							"attr": cty.StringVal("each0"),
+						}),
+						"each1": cty.ObjectVal(map[string]cty.Value{
+							"attr": cty.StringVal("each1"),
+						}),
+					}),
+				}),
+			},
+		},
 		{
 			`foo(null_resource.multi, null_resource.multi[1])`,
 			map[string]cty.Value{
@@ -216,11 +232,13 @@ func TestScopeEvalContext(t *testing.T) {
 				}),
 			},
 		},
+		// any module reference returns the entire module
 		{
 			`module.foo.output1`,
 			map[string]cty.Value{
 				"module": cty.ObjectVal(map[string]cty.Value{
 					"foo": cty.ObjectVal(map[string]cty.Value{
+						"output0": cty.StringVal("bar0"),
 						"output1": cty.StringVal("bar1"),
 					}),
 				}),

plans/changes.go

@@ -81,6 +81,36 @@ func (c *Changes) OutputValue(addr addrs.AbsOutputValue) *OutputChangeSrc {
 	return nil
 }
 
+// OutputValues returns planned changes for all outputs for all module
+// instances that reside in the parent path.  Returns nil if no changes are
+// planned.
+func (c *Changes) OutputValues(parent addrs.ModuleInstance, module addrs.ModuleCall) []*OutputChangeSrc {
+	var res []*OutputChangeSrc
+
+	for _, oc := range c.Outputs {
+		// we can't evaluate root module outputs
+		if oc.Addr.Module.Equal(addrs.RootModuleInstance) {
+			continue
+		}
+
+		changeMod, changeCall := oc.Addr.Module.Call()
+		// this does not reside on our parent instance path
+		if !changeMod.Equal(parent) {
+			continue
+		}
+
+		// this is not the module you're looking for
+		if changeCall.Name != module.Name {
+			continue
+		}
+
+		res = append(res, oc)
+
+	}
+
+	return res
+}
+
 // SyncWrapper returns a wrapper object around the receiver that can be used
 // to make certain changes to the receiver in a concurrency-safe way, as long
 // as all callers share the same wrapper object.

plans/changes_sync.go

@@ -123,6 +123,23 @@ func (cs *ChangesSync) GetOutputChange(addr addrs.AbsOutputValue) *OutputChangeS
 	return cs.changes.OutputValue(addr)
 }
 
+// GetOutputChanges searches the set of output changes for any that reside in
+// module instances beneath the given module. If no changes exist, nil
+// is returned.
+//
+// The returned objects are a deep copy of the change recorded in the plan, so
+// callers may mutate them although it's generally better (less confusing) to
+// treat planned changes as immutable after they've been initially constructed.
+func (cs *ChangesSync) GetOutputChanges(parent addrs.ModuleInstance, module addrs.ModuleCall) []*OutputChangeSrc {
+	if cs == nil {
+		panic("GetOutputChange on nil ChangesSync")
+	}
+	cs.lock.Lock()
+	defer cs.lock.Unlock()
+
+	return cs.changes.OutputValues(parent, module)
+}
+
 // RemoveOutputChange searches the set of output value changes for one matching
 // the given address, and removes it from the set if it exists.
 func (cs *ChangesSync) RemoveOutputChange(addr addrs.AbsOutputValue) {

repl/session_test.go

@@ -125,7 +125,7 @@ func TestSession_basicState(t *testing.T) {
 				{
 					Input:         "module.module.foo",
 					Error:         true,
-					ErrorContains: `An output value with the name "foo" has not been declared in module.module`,
+					ErrorContains: `Unsupported attribute: This object does not have an attribute named "foo"`,
 				},
 			},
 		})

states/module.go

@@ -259,6 +259,12 @@ func (ms *Module) maybeRestoreResourceInstanceDeposed(addr addrs.ResourceInstanc
 // existing value of the same name.
 func (ms *Module) SetOutputValue(name string, value cty.Value, sensitive bool) *OutputValue {
 	os := &OutputValue{
+		Addr: addrs.AbsOutputValue{
+			Module: ms.Addr,
+			OutputValue: addrs.OutputValue{
+				Name: name,
+			},
+		},
 		Value:     value,
 		Sensitive: sensitive,
 	}

states/output_value.go

@@ -1,6 +1,7 @@
 package states
 
 import (
+	"github.com/hashicorp/terraform/addrs"
 	"github.com/zclconf/go-cty/cty"
 )
 
@@ -9,6 +10,7 @@ import (
 // It is not valid to mutate an OutputValue object once it has been created.
 // Instead, create an entirely new OutputValue to replace the previous one.
 type OutputValue struct {
+	Addr      addrs.AbsOutputValue
 	Value     cty.Value
 	Sensitive bool
 }

states/state.go

@@ -82,6 +82,35 @@ func (s *State) ModuleInstances(addr addrs.Module) []*Module {
 	return ms
 }
 
+// ModuleOutputs returns all outputs for the given module call under the
+// parentAddr instance.
+func (s *State) ModuleOutputs(parentAddr addrs.ModuleInstance, module addrs.ModuleCall) []*OutputValue {
+	var os []*OutputValue
+	for _, m := range s.Modules {
+		// can't get outputs from the root module
+		if m.Addr.IsRoot() {
+			continue
+		}
+
+		parent, call := m.Addr.Call()
+		// make sure this is a descendent in the correct path
+		if !parentAddr.Equal(parent) {
+			continue
+		}
+
+		// and check if this is the correct child
+		if call.Name != module.Name {
+			continue
+		}
+
+		for _, o := range m.OutputValues {
+			os = append(os, o)
+		}
+	}
+
+	return os
+}
+
 // RemoveModule removes the module with the given address from the state,
 // unless it is the root module. The root module cannot be deleted, and so
 // this method will panic if that is attempted.

states/state_deepcopy.go

@@ -226,6 +226,7 @@ func (os *OutputValue) DeepCopy() *OutputValue {
 	}
 
 	return &OutputValue{
+		Addr:      os.Addr,
 		Value:     os.Value,
 		Sensitive: os.Sensitive,
 	}

states/state_test.go

@@ -43,6 +43,10 @@ func TestState(t *testing.T) {
 
 	childModule := state.EnsureModule(addrs.RootModuleInstance.Child("child", addrs.NoKey))
 	childModule.SetOutputValue("pizza", cty.StringVal("hawaiian"), false)
+	multiModA := state.EnsureModule(addrs.RootModuleInstance.Child("multi", addrs.StringKey("a")))
+	multiModA.SetOutputValue("pizza", cty.StringVal("cheese"), false)
+	multiModB := state.EnsureModule(addrs.RootModuleInstance.Child("multi", addrs.StringKey("b")))
+	multiModB.SetOutputValue("pizza", cty.StringVal("sausage"), false)
 
 	want := &State{
 		Modules: map[string]*Module{
@@ -53,10 +57,20 @@ func TestState(t *testing.T) {
 				},
 				OutputValues: map[string]*OutputValue{
 					"bar": {
+						Addr: addrs.AbsOutputValue{
+							OutputValue: addrs.OutputValue{
+								Name: "bar",
+							},
+						},
 						Value:     cty.StringVal("bar value"),
 						Sensitive: false,
 					},
 					"secret": {
+						Addr: addrs.AbsOutputValue{
+							OutputValue: addrs.OutputValue{
+								Name: "secret",
+							},
+						},
 						Value:     cty.StringVal("secret value"),
 						Sensitive: true,
 					},
@@ -92,12 +106,52 @@ func TestState(t *testing.T) {
 				LocalValues: map[string]cty.Value{},
 				OutputValues: map[string]*OutputValue{
 					"pizza": {
+						Addr: addrs.AbsOutputValue{
+							Module: addrs.RootModuleInstance.Child("child", addrs.NoKey),
+							OutputValue: addrs.OutputValue{
+								Name: "pizza",
+							},
+						},
 						Value:     cty.StringVal("hawaiian"),
 						Sensitive: false,
 					},
 				},
 				Resources: map[string]*Resource{},
 			},
+			`module.multi["a"]`: {
+				Addr:        addrs.RootModuleInstance.Child("multi", addrs.StringKey("a")),
+				LocalValues: map[string]cty.Value{},
+				OutputValues: map[string]*OutputValue{
+					"pizza": {
+						Addr: addrs.AbsOutputValue{
+							Module: addrs.RootModuleInstance.Child("multi", addrs.StringKey("a")),
+							OutputValue: addrs.OutputValue{
+								Name: "pizza",
+							},
+						},
+						Value:     cty.StringVal("cheese"),
+						Sensitive: false,
+					},
+				},
+				Resources: map[string]*Resource{},
+			},
+			`module.multi["b"]`: {
+				Addr:        addrs.RootModuleInstance.Child("multi", addrs.StringKey("b")),
+				LocalValues: map[string]cty.Value{},
+				OutputValues: map[string]*OutputValue{
+					"pizza": {
+						Addr: addrs.AbsOutputValue{
+							Module: addrs.RootModuleInstance.Child("multi", addrs.StringKey("b")),
+							OutputValue: addrs.OutputValue{
+								Name: "pizza",
+							},
+						},
+						Value:     cty.StringVal("sausage"),
+						Sensitive: false,
+					},
+				},
+				Resources: map[string]*Resource{},
+			},
 		},
 	}
 
@@ -117,6 +171,25 @@ func TestState(t *testing.T) {
 	for _, problem := range deep.Equal(state, want) {
 		t.Error(problem)
 	}
+
+	expectedOutputs := map[string]string{
+		`module.multi["a"].output.pizza`: "cheese",
+		`module.multi["b"].output.pizza`: "sausage",
+	}
+
+	for _, o := range state.ModuleOutputs(addrs.RootModuleInstance, addrs.ModuleCall{Name: "multi"}) {
+		addr := o.Addr.String()
+		expected := expectedOutputs[addr]
+		delete(expectedOutputs, addr)
+
+		if expected != o.Value.AsString() {
+			t.Fatalf("expected %q:%q, got %q", addr, expected, o.Value.AsString())
+		}
+	}
+
+	for addr, o := range expectedOutputs {
+		t.Fatalf("missing output %q:%q", addr, o)
+	}
 }
 
 func TestStateDeepCopy(t *testing.T) {

states/statefile/version4.go

@@ -281,7 +281,13 @@ func prepareStateV4(sV4 *stateV4) (*File, tfdiags.Diagnostics) {
 	{
 		rootModule := state.RootModule()
 		for name, fos := range sV4.RootOutputs {
-			os := &states.OutputValue{}
+			os := &states.OutputValue{
+				Addr: addrs.AbsOutputValue{
+					OutputValue: addrs.OutputValue{
+						Name: name,
+					},
+				},
+			}
 			os.Sensitive = fos.Sensitive
 
 			ty, err := ctyjson.UnmarshalType([]byte(fos.ValueTypeRaw))

states/sync.go

@@ -48,6 +48,18 @@ func (s *SyncState) Module(addr addrs.ModuleInstance) *Module {
 	return ret
 }
 
+// ModuleOutputs returns the set of OutputValues that matches the given path.
+func (s *SyncState) ModuleOutputs(parentAddr addrs.ModuleInstance, module addrs.ModuleCall) []*OutputValue {
+	s.lock.RLock()
+	defer s.lock.RUnlock()
+	var os []*OutputValue
+
+	for _, o := range s.state.ModuleOutputs(parentAddr, module) {
+		os = append(os, o.DeepCopy())
+	}
+	return os
+}
+
 // RemoveModule removes the entire state for the given module, taking with
 // it any resources associated with the module. This should generally be
 // called only for modules whose resources have all been destroyed, but

terraform/context_plan_test.go

@@ -5832,3 +5832,47 @@ resource "aws_instance" "foo" {
 		t.Errorf("missing %s change for %s", action, res)
 	}
 }
+
+func TestContext2Plan_indexInVar(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+module "a" {
+  count = 1
+  source = "./mod"
+  in = "test"
+}
+
+module "b" {
+  count = 1
+  source = "./mod"
+  in = length(module.a)
+}
+`,
+		"mod/main.tf": `
+resource "aws_instance" "foo" {
+  foo = var.in
+}
+
+variable "in" {
+}
+
+output"out" {
+  value = aws_instance.foo.id
+}
+`,
+	})
+
+	p := testProvider("aws")
+	p.DiffFn = testDiffFn
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("aws"): testProviderFuncFixed(p),
+		},
+	})
+
+	_, diags := ctx.Plan()
+	if diags.HasErrors() {
+		t.Fatal(diags.ErrWithWarnings())
+	}
+}

terraform/context_validate_test.go

@@ -1421,6 +1421,7 @@ module "mod1" {
 module "mod2" {
   for_each = module.mod1
   source = "./mod"
+  input = module.mod1["a"].out
 }
 
 module "mod3" {
@@ -1432,6 +1433,15 @@ module "mod3" {
 resource "aws_instance" "foo" {
 }
 
+output "out" {
+  value = 1
+}
+
+variable "input" {
+  type = number
+  default = 0
+}
+
 module "nested" {
   count = 2
   source = "./nested"

terraform/evaluate.go

@@ -336,72 +336,16 @@ func (d *evaluationStateData) GetLocalValue(addr addrs.LocalValue, rng tfdiags.S
 	return val, diags
 }
 
-func (d *evaluationStateData) GetModuleInstance(addr addrs.ModuleCallInstance, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
+func (d *evaluationStateData) GetModule(addr addrs.ModuleCall, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
 	var diags tfdiags.Diagnostics
 
 	// Output results live in the module that declares them, which is one of
 	// the child module instances of our current module path.
-	moduleAddr := addr.ModuleInstance(d.ModulePath)
+	moduleAddr := d.ModulePath.Module().Child(addr.Name)
 
-	// We'll consult the configuration to see what output names we are
+	parentCfg := d.Evaluator.Config.DescendentForInstance(d.ModulePath)
-	// expecting, so we can ensure the resulting object is of the expected
+	callConfig, ok := parentCfg.Module.ModuleCalls[addr.Name]
-	// type even if our data is incomplete for some reason.
+	if !ok {
-	moduleConfig := d.Evaluator.Config.DescendentForInstance(moduleAddr)
-	if moduleConfig == nil {
-		// should never happen, since this should've been caught during
-		// static validation.
-		panic(fmt.Sprintf("output value read from %s, which has no configuration", moduleAddr))
-	}
-	outputConfigs := moduleConfig.Module.Outputs
-
-	vals := map[string]cty.Value{}
-	for n := range outputConfigs {
-		addr := addrs.OutputValue{Name: n}.Absolute(moduleAddr)
-
-		// If a pending change is present in our current changeset then its value
-		// takes priority over what's in state. (It will usually be the same but
-		// will differ if the new value is unknown during planning.)
-		if changeSrc := d.Evaluator.Changes.GetOutputChange(addr); changeSrc != nil {
-			change, err := changeSrc.Decode()
-			if err != nil {
-				// This should happen only if someone has tampered with a plan
-				// file, so we won't bother with a pretty error for it.
-				diags = diags.Append(fmt.Errorf("planned change for %s could not be decoded: %s", addr, err))
-				vals[n] = cty.DynamicVal
-				continue
-			}
-			// We care only about the "after" value, which is the value this output
-			// will take on after the plan is applied.
-			vals[n] = change.After
-		} else {
-			os := d.Evaluator.State.OutputValue(addr)
-			if os == nil {
-				// Not evaluated yet?
-				vals[n] = cty.DynamicVal
-				continue
-			}
-			vals[n] = os.Value
-		}
-	}
-	return cty.ObjectVal(vals), diags
-}
-
-func (d *evaluationStateData) GetModuleInstanceOutput(addr addrs.AbsModuleCallOutput, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {
-	var diags tfdiags.Diagnostics
-
-	// Output results live in the module that declares them, which is one of
-	// the child module instances of our current module path.
-	absAddr := addr.AbsOutputValue(d.ModulePath)
-	moduleAddr := absAddr.Module
-
-	// First we'll consult the configuration to see if an output of this
-	// name is declared at all.
-	moduleConfig := d.Evaluator.Config.DescendentForInstance(moduleAddr)
-	if moduleConfig == nil {
-		// this doesn't happen in normal circumstances due to our validation
-		// pass, but it can turn up in some unusual situations, like in the
-		// "terraform console" repl where arbitrary expressions can be
-		// evaluated.
 		diags = diags.Append(&hcl.Diagnostic{
 			Severity: hcl.DiagError,
 			Summary:  `Reference to undeclared module`,
@@ -411,49 +355,167 @@ func (d *evaluationStateData) GetModuleInstanceOutput(addr addrs.AbsModuleCallOu
 		return cty.DynamicVal, diags
 	}
 
-	config := moduleConfig.Module.Outputs[addr.Name]
+	// We'll consult the configuration to see what output names we are
-	if config == nil {
+	// expecting, so we can ensure the resulting object is of the expected
-		var suggestions []string
+	// type even if our data is incomplete for some reason.
-		for k := range moduleConfig.Module.Outputs {
+	moduleConfig := d.Evaluator.Config.Descendent(moduleAddr)
-			suggestions = append(suggestions, k)
+	if moduleConfig == nil {
+		// should never happen, since we have a valid module call above, this
+		// should be caught during static validation.
+		panic(fmt.Sprintf("output value read from %s, which has no configuration", moduleAddr))
 	}
-		suggestion := nameSuggestion(addr.Name, suggestions)
+	outputConfigs := moduleConfig.Module.Outputs
-		if suggestion != "" {
+
-			suggestion = fmt.Sprintf(" Did you mean %q?", suggestion)
+	// Collect all the relevant outputs that current exist in the state.
+	// We know the instance path up to this point, and the child module name,
+	// so we only need to store these by instance key.
+	stateMap := map[addrs.InstanceKey]map[string]cty.Value{}
+	for _, output := range d.Evaluator.State.ModuleOutputs(d.ModulePath, addr) {
+		_, callInstance := output.Addr.Module.CallInstance()
+		instance, ok := stateMap[callInstance.Key]
+		if !ok {
+			instance = map[string]cty.Value{}
+			stateMap[callInstance.Key] = instance
 		}
 
-		diags = diags.Append(&hcl.Diagnostic{
+		instance[output.Addr.OutputValue.Name] = output.Value
-			Severity: hcl.DiagError,
+	}
-			Summary:  `Reference to undeclared output value`,
+
-			Detail:   fmt.Sprintf(`An output value with the name %q has not been declared in %s.%s`, addr.Name, moduleDisplayAddr(moduleAddr), suggestion),
+	// Get all changes that reside for this module call within our path.
-			Subject:  rng.ToHCL().Ptr(),
+	// The change contains the full addr, so we can key these with strings.
-		})
+	changesMap := map[addrs.InstanceKey]map[string]*plans.OutputChangeSrc{}
-		return cty.DynamicVal, diags
+	for _, change := range d.Evaluator.Changes.GetOutputChanges(d.ModulePath, addr) {
+		_, callInstance := change.Addr.Module.CallInstance()
+		instance, ok := changesMap[callInstance.Key]
+		if !ok {
+			instance = map[string]*plans.OutputChangeSrc{}
+			changesMap[callInstance.Key] = instance
+		}
+
+		instance[change.Addr.OutputValue.Name] = change
+	}
+
+	// Build up all the module objects, creating a map of values for each
+	// module instance.
+	moduleInstances := map[addrs.InstanceKey]map[string]cty.Value{}
+
+	// the structure is based on the configuration, so iterate through all the
+	// defined outputs, and add any instance state or changes we find.
+	for _, cfg := range outputConfigs {
+		// get all instance output for this path from the state
+		for key, states := range stateMap {
+			outputState, ok := states[cfg.Name]
+			if !ok {
+				continue
+			}
+
+			instance, ok := moduleInstances[key]
+			if !ok {
+				instance = map[string]cty.Value{}
+				moduleInstances[key] = instance
+			}
+
+			instance[cfg.Name] = outputState
+		}
+
+		// any pending changes override the state state values
+		for key, changes := range changesMap {
+			changeSrc, ok := changes[cfg.Name]
+			if !ok {
+				continue
+			}
+
+			instance, ok := moduleInstances[key]
+			if !ok {
+				instance = map[string]cty.Value{}
+				moduleInstances[key] = instance
 			}
 
-	// If a pending change is present in our current changeset then its value
-	// takes priority over what's in state. (It will usually be the same but
-	// will differ if the new value is unknown during planning.)
-	if changeSrc := d.Evaluator.Changes.GetOutputChange(absAddr); changeSrc != nil {
 			change, err := changeSrc.Decode()
 			if err != nil {
 				// This should happen only if someone has tampered with a plan
 				// file, so we won't bother with a pretty error for it.
-			diags = diags.Append(fmt.Errorf("planned change for %s could not be decoded: %s", absAddr, err))
+				diags = diags.Append(fmt.Errorf("planned change for %s could not be decoded: %s", addr, err))
-			return cty.DynamicVal, diags
+				instance[cfg.Name] = cty.DynamicVal
-		}
+				continue
-		// We care only about the "after" value, which is the value this output
-		// will take on after the plan is applied.
-		return change.After, diags
 			}
 
-	os := d.Evaluator.State.OutputValue(absAddr)
+			instance[cfg.Name] = change.After
-	if os == nil {
+		}
-		// Not evaluated yet?
-		return cty.DynamicVal, diags
 	}
 
-	return os.Value, diags
+	var ret cty.Value
+
+	// compile the outputs into the correct value type for the each mode
+	switch {
+	case callConfig.Count != nil:
+		vals := make([]cty.Value, len(moduleInstances))
+		for key, instance := range moduleInstances {
+			intKey, ok := key.(addrs.IntKey)
+			if !ok {
+				// old key from state which is being dropped
+				continue
+			}
+
+			vals[int(intKey)] = cty.ObjectVal(instance)
+		}
+
+		if len(vals) > 0 {
+			// we shouldn't have any holes, but insert real values just in case,
+			// while trimming off any extra values that we may have from guessing
+			// the length via the state instances.
+			last := 0
+			for i, v := range vals {
+				if v.IsNull() {
+					vals[i] = cty.DynamicVal
+					continue
+				}
+				last = i
+			}
+			vals = vals[:last+1]
+			ret = cty.ListVal(vals)
+		} else {
+			ret = cty.ListValEmpty(cty.DynamicPseudoType)
+		}
+
+	case callConfig.ForEach != nil:
+		vals := make(map[string]cty.Value)
+		for key, instance := range moduleInstances {
+			strKey, ok := key.(addrs.StringKey)
+			if !ok {
+				continue
+			}
+
+			vals[string(strKey)] = cty.ObjectVal(instance)
+		}
+
+		if len(vals) > 0 {
+			ret = cty.MapVal(vals)
+		} else {
+			ret = cty.MapValEmpty(cty.DynamicPseudoType)
+		}
+
+	default:
+		val, ok := moduleInstances[addrs.NoKey]
+		if !ok {
+			// create the object if there wasn't one known
+			val = map[string]cty.Value{}
+			for k := range outputConfigs {
+				val[k] = cty.DynamicVal
+			}
+		}
+
+		ret = cty.ObjectVal(val)
+	}
+
+	// The module won't be expanded during validation, so we need to return an
+	// unknown value. This will ensure the types looks correct, since we built
+	// the objects based on the configuration.
+	if d.Operation == walkValidate {
+		return cty.UnknownVal(ret.Type()), diags
+	}
+
+	return ret, diags
 }
 
 func (d *evaluationStateData) GetPathAttr(addr addrs.PathAttr, rng tfdiags.SourceRange) (cty.Value, tfdiags.Diagnostics) {