Merge 4e9ae3e into backport/liamcervante/34976/frankly-assuring-boxer

Author: The Terraform Team

internal/terraform/context_apply2_test.go

@@ -3377,3 +3377,35 @@ resource "test_object" "a" {
 		t.Errorf("Unexpected %s change for %s", c.Action, c.Addr)
 	}
 }
+
+// This test explicitly reproduces the issue described in #34976.
+func TestContext2Apply_34976(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+module "a" {
+  source = "./mod"
+  count = 1
+}
+
+resource "test_object" "obj" {
+  test_number = length(module.a)
+}
+`,
+		"mod/main.tf": ``, // just an empty module
+	})
+
+	p := simpleMockProvider()
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	plan, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)))
+	assertNoErrors(t, diags)
+
+	// Just don't crash.
+	_, diags = ctx.Apply(plan, m, nil)
+	assertNoErrors(t, diags)
+}

internal/terraform/context_plan2_test.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 	"sync"
 	"testing"
+	"time"
 
 	"github.com/davecgh/go-spew/spew"
 	"github.com/google/go-cmp/cmp"
@@ -5493,3 +5494,49 @@ resource "test_object" "obj" {
 		t.Fatal("expected no change in plan")
 	}
 }
+
+// This test explicitly reproduces the issue described in #34976.
+func TestContext2Plan_34976(t *testing.T) {
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+data "test_object" "obj" {}
+
+module "a" {
+  depends_on = [data.test_object.obj]
+  source = "./mod"
+}
+
+output "value" {
+  value = try(module.a.notreal, null)
+}
+`,
+		"mod/main.tf": ``,
+	})
+
+	p := new(testing_provider.MockProvider)
+	p.GetProviderSchemaResponse = getProviderSchemaResponseFromProviderSchema(&providerSchema{
+		DataSources: map[string]*configschema.Block{
+			"test_object": {},
+		},
+	})
+	p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) providers.ReadDataSourceResponse {
+
+		// Just very small delay that means the output will be processed before
+		// the module.
+		time.Sleep(50 * time.Millisecond)
+
+		return providers.ReadDataSourceResponse{
+			State: req.Config,
+		}
+	}
+
+	ctx := testContext2(t, &ContextOpts{
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+	})
+
+	// Just shouldn't crash.
+	_, diags := ctx.Plan(m, states.NewState(), SimplePlanOpts(plans.NormalMode, testInputValuesUnset(m.Module.Variables)))
+	assertNoErrors(t, diags)
+}

internal/terraform/node_module_expand.go

@@ -27,6 +27,7 @@ type nodeExpandModule struct {
 
 var (
 	_ GraphNodeExecutable       = (*nodeExpandModule)(nil)
+	_ GraphNodeReferenceable    = (*nodeExpandModule)(nil)
 	_ GraphNodeReferencer       = (*nodeExpandModule)(nil)
 	_ GraphNodeReferenceOutside = (*nodeExpandModule)(nil)
 	_ graphNodeExpandsInstances = (*nodeExpandModule)(nil)
@@ -75,6 +76,14 @@ func (n *nodeExpandModule) References() []*addrs.Reference {
 	return refs
 }
 
+func (n *nodeExpandModule) ReferenceableAddrs() []addrs.Referenceable {
+	// Anything referencing this module must do so after the ExpandModule call
+	// has been made to the expander, so we return the module call address as
+	// the only referenceable address.
+	_, call := n.Addr.Call()
+	return []addrs.Referenceable{call}
+}
+
 func (n *nodeExpandModule) DependsOn() []*addrs.Reference {
 	if n.ModuleCall == nil {
 		return nil
@@ -99,7 +108,7 @@ func (n *nodeExpandModule) DependsOn() []*addrs.Reference {
 
 // GraphNodeReferenceOutside
 func (n *nodeExpandModule) ReferenceOutside() (selfPath, referencePath addrs.Module) {
-	return n.Addr, n.Addr.Parent()
+	return n.Addr.Parent(), n.Addr.Parent()
 }
 
 // GraphNodeExecutable

internal/terraform/transform_reference.go

@@ -522,21 +522,50 @@ func (m ReferenceMap) referenceMapKey(path addrs.Module, addr addrs.Referenceabl
 		// might be in a resource-oriented graph rather than an
 		// instance-oriented graph, and so we'll see if we have the
 		// resource itself instead.
-		switch ri := addr.(type) {
-		case addrs.ResourceInstance:
-			addr = ri.ContainingResource()
-		case addrs.ResourceInstancePhase:
-			addr = ri.ContainingResource()
-		case addrs.ModuleCallInstanceOutput:
-			addr = ri.ModuleCallOutput()
-		case addrs.ModuleCallInstance:
-			addr = ri.Call
-		default:
-			return key
+
+		if ri, ok := addr.(addrs.ResourceInstance); ok {
+			return m.mapKey(path, ri.ContainingResource())
+		}
+
+		if rip, ok := addr.(addrs.ResourceInstancePhase); ok {
+			return m.mapKey(path, rip.ContainingResource())
 		}
-		// if we matched any of the resource node types above, generate a new
-		// key
-		key = m.mapKey(path, addr)
+
+		if mcio, ok := addr.(addrs.ModuleCallInstanceOutput); ok {
+
+			// A module call instance output is a reference to an output of a
+			// specific module call. If we can't find that, we'll look first
+			// for the general non-instanced output.
+
+			key = m.mapKey(path, mcio.ModuleCallOutput())
+			if _, exists := m[key]; exists {
+				// We found it, so we can just use that.
+				return key
+			}
+
+			// Otherwise we'll look just for the instanced module call itself.
+
+			key = m.mapKey(path, mcio.Call)
+			if _, exists := m[key]; exists {
+				// We found it, so we can just use that.
+				return key
+			}
+
+			// If we still can't find it, then we'll look for the non-instanced
+			// module call. This is the same as we'd do if the original call had
+			// just been for a ModuleCallInstance, so we'll let that fall
+			// through.
+
+			addr = mcio.Call
+
+		}
+
+		if mci, ok := addr.(addrs.ModuleCallInstance); ok {
+			return m.mapKey(path, mci.Call)
+		}
+
+		// If nothing matched, then we'll just return the original key
+		// unchanged.
 	}
 	return key
 }