Merge pull request #30629 from hashicorp/jbardin/data-read-hook

ensure UI hooks are called for data sources

Author: jbardin

internal/terraform/context_apply_test.go

@@ -1428,7 +1428,9 @@ func TestContext2Apply_dataBasic(t *testing.T) {
 		}),
 	}
 
+	hook := new(MockHook)
 	ctx := testContext2(t, &ContextOpts{
+		Hooks: []Hook{hook},
 		Providers: map[addrs.Provider]providers.Factory{
 			addrs.NewDefaultProvider("null"): testProviderFuncFixed(p),
 		},
@@ -1449,6 +1451,13 @@ func TestContext2Apply_dataBasic(t *testing.T) {
 	if actual != expected {
 		t.Fatalf("wrong result\n\ngot:\n%s\n\nwant:\n%s", actual, expected)
 	}
+
+	if !hook.PreApplyCalled {
+		t.Fatal("PreApply not called for data source read")
+	}
+	if !hook.PostApplyCalled {
+		t.Fatal("PostApply not called for data source read")
+	}
 }
 
 func TestContext2Apply_destroyData(t *testing.T) {
@@ -1503,10 +1512,8 @@ func TestContext2Apply_destroyData(t *testing.T) {
 	}
 
 	wantHookCalls := []*testHookCall{
-		{"PreDiff", "data.null_data_source.testing"},
-		{"PostDiff", "data.null_data_source.testing"},
-		{"PreDiff", "data.null_data_source.testing"},
-		{"PostDiff", "data.null_data_source.testing"},
+		{"PreApply", "data.null_data_source.testing"},
+		{"PostApply", "data.null_data_source.testing"},
 		{"PostStateUpdate", ""},
 	}
 	if !reflect.DeepEqual(hook.Calls, wantHookCalls) {

internal/terraform/node_resource_abstract_instance.go

@@ -410,18 +410,6 @@ func (n *NodeAbstractResourceInstance) planDestroy(ctx EvalContext, currentState
 		return noop, nil
 	}
 
-	// Call pre-diff hook
-	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
-		return h.PreDiff(
-			absAddr, deposedKey.Generation(),
-			currentState.Value,
-			cty.NullVal(cty.DynamicPseudoType),
-		)
-	}))
-	if diags.HasErrors() {
-		return nil, diags
-	}
-
 	// Plan is always the same for a destroy. We don't need the provider's
 	// help for this one.
 	plan := &plans.ResourceInstanceChange{
@@ -437,17 +425,6 @@ func (n *NodeAbstractResourceInstance) planDestroy(ctx EvalContext, currentState
 		ProviderAddr: n.ResolvedProvider,
 	}
 
-	// Call post-diff hook
-	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
-		return h.PostDiff(
-			absAddr,
-			deposedKey.Generation(),
-			plan.Action,
-			plan.Before,
-			plan.After,
-		)
-	}))
-
 	return plan, diags
 }
 
@@ -1381,6 +1358,13 @@ func (n *NodeAbstractResourceInstance) readDataSource(ctx EvalContext, configVal
 	// to actually call the provider to read the data.
 	log.Printf("[TRACE] readDataSource: %s configuration is complete, so reading from provider", n.Addr)
 
+	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
+		return h.PreApply(n.Addr, states.CurrentGen, plans.Read, cty.NullVal(configVal.Type()), configVal)
+	}))
+	if diags.HasErrors() {
+		return newVal, diags
+	}
+
 	resp := provider.ReadDataSource(providers.ReadDataSourceRequest{
 		TypeName:     n.Addr.ContainingResource().Resource.Type,
 		Config:       configVal,
@@ -1445,6 +1429,10 @@ func (n *NodeAbstractResourceInstance) readDataSource(ctx EvalContext, configVal
 		newVal = newVal.MarkWithPaths(pvm)
 	}
 
+	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
+		return h.PostApply(n.Addr, states.CurrentGen, newVal, diags.Err())
+	}))
+
 	return newVal, diags
 }
 
@@ -1556,13 +1544,6 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, currentSt
 		}
 
 		proposedNewVal := objchange.PlannedDataResourceObject(schema, unmarkedConfigVal)
-
-		diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
-			return h.PreDiff(n.Addr, states.CurrentGen, priorVal, proposedNewVal)
-		}))
-		if diags.HasErrors() {
-			return nil, nil, keyData, diags
-		}
 		proposedNewVal = proposedNewVal.MarkWithPaths(configMarkPaths)
 
 		// Apply detects that the data source will need to be read by the After
@@ -1590,13 +1571,6 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, currentSt
 		return plannedChange, plannedNewState, keyData, diags
 	}
 
-	// While this isn't a "diff", continue to call this for data sources.
-	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
-		return h.PreDiff(n.Addr, states.CurrentGen, priorVal, configVal)
-	}))
-	if diags.HasErrors() {
-		return nil, nil, keyData, diags
-	}
 	// We have a complete configuration with no dependencies to wait on, so we
 	// can read the data source into the state.
 	newVal, readDiags := n.readDataSource(ctx, configVal)
@@ -1632,9 +1606,6 @@ func (n *NodeAbstractResourceInstance) planDataSource(ctx EvalContext, currentSt
 		Status: states.ObjectReady,
 	}
 
-	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
-		return h.PostDiff(n.Addr, states.CurrentGen, plans.Update, priorVal, newVal)
-	}))
 	return nil, plannedNewState, keyData, diags
 }
 
@@ -1703,13 +1674,6 @@ func (n *NodeAbstractResourceInstance) applyDataSource(ctx EvalContext, planned
 		return nil, keyData, diags
 	}
 
-	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
-		return h.PreApply(n.Addr, states.CurrentGen, planned.Action, planned.Before, planned.After)
-	}))
-	if diags.HasErrors() {
-		return nil, keyData, diags
-	}
-
 	config := *n.Config
 	schema, _ := providerSchema.SchemaForResourceAddr(n.Addr.ContainingResource().Resource)
 	if schema == nil {
@@ -1751,10 +1715,6 @@ func (n *NodeAbstractResourceInstance) applyDataSource(ctx EvalContext, planned
 		Status: states.ObjectReady,
 	}
 
-	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
-		return h.PostApply(n.Addr, states.CurrentGen, newVal, diags.Err())
-	}))
-
 	return state, keyData, diags
 }