actions: fix race condition in tests and capture errors from hooks (#37759)

Author: mildwonkey

internal/terraform/context_apply_action_test.go

@@ -5,6 +5,7 @@ package terraform
 
 import (
 	"path/filepath"
+	"sync"
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
@@ -2603,7 +2604,7 @@ lifecycle {
 				InvokeActionFn: invokeActionFn,
 			}
 
-			hookCapture := actionHookCapture{}
+			hookCapture := newActionHookCapture()
 			ctx := testContext2(t, &ContextOpts{
 				Providers: map[addrs.Provider]providers.Factory{
 					addrs.NewDefaultProvider("test"):   testProviderFuncFixed(testProvider),
@@ -2694,10 +2695,17 @@ lifecycle {
 var _ Hook = (*actionHookCapture)(nil)
 
 type actionHookCapture struct {
+	mu                  *sync.Mutex
 	startActionHooks    []HookActionIdentity
 	completeActionHooks []HookActionIdentity
 }
 
+func newActionHookCapture() actionHookCapture {
+	return actionHookCapture{
+		mu: &sync.Mutex{},
+	}
+}
+
 func (a *actionHookCapture) PreApply(HookResourceIdentity, addrs.DeposedKey, plans.Action, cty.Value, cty.Value) (HookAction, error) {
 	return HookActionContinue, nil
 }
@@ -2781,6 +2789,8 @@ func (a *actionHookCapture) PostListQuery(HookResourceIdentity, plans.QueryResul
 }
 
 func (a *actionHookCapture) StartAction(identity HookActionIdentity) (HookAction, error) {
+	a.mu.Lock()
+	defer a.mu.Unlock()
 	a.startActionHooks = append(a.startActionHooks, identity)
 	return HookActionContinue, nil
 }
@@ -2790,6 +2800,8 @@ func (a *actionHookCapture) ProgressAction(HookActionIdentity, string) (HookActi
 }
 
 func (a *actionHookCapture) CompleteAction(identity HookActionIdentity, _ error) (HookAction, error) {
+	a.mu.Lock()
+	defer a.mu.Unlock()
 	a.completeActionHooks = append(a.completeActionHooks, identity)
 	return HookActionContinue, nil
 }

internal/terraform/node_action_trigger_instance_apply.go

@@ -136,9 +136,12 @@ func (n *nodeActionTriggerApplyInstance) Execute(ctx EvalContext, wo walkOperati
 		ActionTrigger: ai.ActionTrigger,
 	}
 
-	ctx.Hook(func(h Hook) (HookAction, error) {
+	diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
 		return h.StartAction(hookIdentity)
-	})
+	}))
+	if diags.HasErrors() {
+		return diags
+	}
 
 	// We don't want to send the marks, but all marks are okay in the context
 	// of an action invocation. We can't reuse our ephemeral free value from
@@ -153,28 +156,34 @@ func (n *nodeActionTriggerApplyInstance) Execute(ctx EvalContext, wo walkOperati
 	respDiags := n.AddSubjectToDiagnostics(resp.Diagnostics)
 	diags = diags.Append(respDiags)
 	if respDiags.HasErrors() {
-		ctx.Hook(func(h Hook) (HookAction, error) {
+		diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
 			return h.CompleteAction(hookIdentity, respDiags.Err())
-		})
+		}))
 		return diags
 	}
 
 	if resp.Events != nil { // should only occur in misconfigured tests
 		for event := range resp.Events {
 			switch ev := event.(type) {
 			case providers.InvokeActionEvent_Progress:
-				ctx.Hook(func(h Hook) (HookAction, error) {
+				diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
 					return h.ProgressAction(hookIdentity, ev.Message)
-				})
+				}))
+				if diags.HasErrors() {
+					return diags
+				}
 			case providers.InvokeActionEvent_Completed:
 				// Enhance the diagnostics
 				diags = diags.Append(n.AddSubjectToDiagnostics(ev.Diagnostics))
-				ctx.Hook(func(h Hook) (HookAction, error) {
+				diags = diags.Append(ctx.Hook(func(h Hook) (HookAction, error) {
 					return h.CompleteAction(hookIdentity, ev.Diagnostics.Err())
-				})
+				}))
 				if ev.Diagnostics.HasErrors() {
 					return diags
 				}
+				if diags.HasErrors() {
+					return diags
+				}
 			default:
 				panic(fmt.Sprintf("unexpected action event type %T", ev))
 			}