Merge pull request #28267 from hashicorp/jbardin/data-depends_on-refs

Don't force data reads from dependency changes in sibling modules

terraform/context_apply2_test.go

@@ -356,8 +356,6 @@ resource "aws_instance" "bin" {
 		t.Fatal(diags.Err())
 	}
 
-	fmt.Println(state)
-
 	bar = state.ResourceInstance(barAddr)
 	if len(bar.Current.Dependencies) == 0 || !bar.Current.Dependencies[0].Equal(fooAddr.ContainingResource().Config()) {
 		t.Fatalf("bar should still depend on foo after apply, but got %s", bar.Current.Dependencies)

terraform/context_plan2_test.go

@@ -249,3 +249,87 @@ resource "test_object" "a" {
 		t.Fatal(diags.Err())
 	}
 }
+
+func TestContext2Plan_dataReferencesResourceInModules(t *testing.T) {
+	p := testProvider("test")
+	p.ReadDataSourceFn = func(req providers.ReadDataSourceRequest) (resp providers.ReadDataSourceResponse) {
+		cfg := req.Config.AsValueMap()
+		cfg["id"] = cty.StringVal("d")
+		resp.State = cty.ObjectVal(cfg)
+		return resp
+	}
+
+	m := testModuleInline(t, map[string]string{
+		"main.tf": `
+locals {
+  things = {
+    old = "first"
+    new = "second"
+  }
+}
+
+module "mod" {
+  source = "./mod"
+  for_each = local.things
+}
+`,
+
+		"./mod/main.tf": `
+resource "test_resource" "a" {
+}
+
+data "test_data_source" "d" {
+  depends_on = [test_resource.a]
+}
+
+resource "test_resource" "b" {
+  value = data.test_data_source.d.id
+}
+`})
+
+	oldDataAddr := mustResourceInstanceAddr(`module.mod["old"].data.test_data_source.d`)
+
+	state := states.BuildState(func(s *states.SyncState) {
+		s.SetResourceInstanceCurrent(
+			mustResourceInstanceAddr(`module.mod["old"].test_resource.a`),
+			&states.ResourceInstanceObjectSrc{
+				AttrsJSON: []byte(`{"id":"a"}`),
+				Status:    states.ObjectReady,
+			}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
+		)
+		s.SetResourceInstanceCurrent(
+			mustResourceInstanceAddr(`module.mod["old"].test_resource.b`),
+			&states.ResourceInstanceObjectSrc{
+				AttrsJSON: []byte(`{"id":"b","value":"d"}`),
+				Status:    states.ObjectReady,
+			}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
+		)
+		s.SetResourceInstanceCurrent(
+			oldDataAddr,
+			&states.ResourceInstanceObjectSrc{
+				AttrsJSON: []byte(`{"id":"d"}`),
+				Status:    states.ObjectReady,
+			}, mustProviderConfig(`provider["registry.terraform.io/hashicorp/test"]`),
+		)
+	})
+
+	ctx := testContext2(t, &ContextOpts{
+		Config: m,
+		Providers: map[addrs.Provider]providers.Factory{
+			addrs.NewDefaultProvider("test"): testProviderFuncFixed(p),
+		},
+		State: state,
+	})
+
+	plan, diags := ctx.Plan()
+	assertNoErrors(t, diags)
+
+	oldMod := oldDataAddr.Module
+
+	for _, c := range plan.Changes.Resources {
+		// there should be no changes from the old module instance
+		if c.Addr.Module.Equal(oldMod) && c.Action != plans.NoOp {
+			t.Errorf("unexpected change %s for %s\n", c.Action, c.Addr)
+		}
+	}
+}

terraform/node_resource_abstract_instance.go

@@ -1447,6 +1447,9 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, currentSt
 // immediately reading from the data source where possible, instead forcing us
 // to generate a plan.
 func (n *NodeAbstractResourceInstance) forcePlanReadData(ctx EvalContext) bool {
+	nModInst := n.Addr.Module
+	nMod := nModInst.Module()
+
 	// Check and see if any depends_on dependencies have
 	// changes, since they won't show up as changes in the
 	// configuration.
@@ -1461,6 +1464,18 @@ func (n *NodeAbstractResourceInstance) forcePlanReadData(ctx EvalContext) bool {
 		}
 
 		for _, change := range changes.GetChangesForConfigResource(d) {
+			changeModInst := change.Addr.Module
+			changeMod := changeModInst.Module()
+
+			if changeMod.Equal(nMod) && !changeModInst.Equal(nModInst) {
+				// Dependencies are tracked by configuration address, which
+				// means we may have changes from other instances of parent
+				// modules. The actual reference can only take effect within
+				// the same module instance, so skip any that aren't an exact
+				// match
+				continue
+			}
+
 			if change != nil && change.Action != plans.NoOp {
 				return true
 			}