diff --git a/internal/plans/action_invocation.go b/internal/plans/action_invocation.go index 0546846378..cacf6b68f7 100644 --- a/internal/plans/action_invocation.go +++ b/internal/plans/action_invocation.go @@ -40,6 +40,10 @@ type ActionTrigger interface { Less(other ActionTrigger) bool } +var ( + _ ActionTrigger = (*LifecycleActionTrigger)(nil) +) + type LifecycleActionTrigger struct { TriggeringResourceAddr addrs.AbsResourceInstance // Information about the trigger diff --git a/internal/plans/deferring/deferred.go b/internal/plans/deferring/deferred.go index 02bc0a150c..90d73a8547 100644 --- a/internal/plans/deferring/deferred.go +++ b/internal/plans/deferring/deferred.go @@ -658,6 +658,45 @@ func (d *Deferred) ReportActionInvocationDeferred(ai plans.ActionInvocationInsta }) } +// ShouldDeferActionInvocation returns true if there is a reason to defer the action invocation instance +// We want to defer an action invocation if +// a) the resource was deferred +// or +// b) a previously run action was deferred +func (d *Deferred) ShouldDeferActionInvocation(ai plans.ActionInvocationInstance) bool { + d.mu.Lock() + defer d.mu.Unlock() + + // We only want to defer actions that are lifecycle triggered + at, ok := ai.ActionTrigger.(plans.LifecycleActionTrigger) + if !ok { + return false + } + + // If the resource was deferred, we also need to defer any action potentially triggering from this + if configResourceMap, ok := d.resourceInstancesDeferred.GetOk(at.TriggeringResourceAddr.ConfigResource()); ok { + if configResourceMap.Has(at.TriggeringResourceAddr) { + return true + } + } + + // Since all actions plan in order we can just check if an action for this resource instance + // has been deferred already + for _, deferred := range d.actionInvocationDeferred { + deferredAt, deferredOk := deferred.ActionInvocationInstance.ActionTrigger.(plans.LifecycleActionTrigger) + if !deferredOk { + continue // We only care about lifecycle triggered actions here + } + + if deferredAt.TriggeringResourceAddr.Equal(at.TriggeringResourceAddr) { + return true + } + } + + // We found no reason, so we return false + return false +} + // UnexpectedProviderDeferralDiagnostic is a diagnostic that indicates that a // provider was deferred although deferrals were not allowed. func UnexpectedProviderDeferralDiagnostic(addrs fmt.Stringer) tfdiags.Diagnostic { diff --git a/internal/terraform/context_plan_actions_test.go b/internal/terraform/context_plan_actions_test.go index fe8e7c1374..788e768e57 100644 --- a/internal/terraform/context_plan_actions_test.go +++ b/internal/terraform/context_plan_actions_test.go @@ -26,7 +26,8 @@ func TestContextPlan_actions(t *testing.T) { toBeImplemented bool module map[string]string buildState func(*states.SyncState) - planActionFn func(providers.PlanActionRequest) providers.PlanActionResponse + planActionFn func(*testing.T, providers.PlanActionRequest) providers.PlanActionResponse + planResourceFn func(*testing.T, providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse planOpts *PlanOpts expectPlanActionCalled bool @@ -682,7 +683,8 @@ resource "test_object" "a" { `, }, - planActionFn: func(_ providers.PlanActionRequest) providers.PlanActionResponse { + planActionFn: func(_ *testing.T, _ providers.PlanActionRequest) providers.PlanActionResponse { + t.Helper() return providers.PlanActionResponse{ Diagnostics: tfdiags.Diagnostics{ tfdiags.Sourceless(tfdiags.Error, "Planning failed", "Test case simulates an error while planning"), @@ -1311,6 +1313,336 @@ resource "test_object" "a" { } }, }, + "provider deferring action while not allowed": { + module: map[string]string{ + "main.tf": ` +action "test_unlinked" "hello" {} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_unlinked.hello] + } + } +} +`, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + DeferralAllowed: false, + }, + planActionFn: func(*testing.T, providers.PlanActionRequest) providers.PlanActionResponse { + return providers.PlanActionResponse{ + Deferred: &providers.Deferred{ + Reason: providers.DeferredReasonAbsentPrereq, + }, + } + }, + expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics { + return tfdiags.Diagnostics{ + tfdiags.Sourceless( + tfdiags.Error, + "Provider deferred changes when Terraform did not allow deferrals", + `The provider signaled a deferred action for "action.test_unlinked.hello", but in this context deferrals are disabled. This is a bug in the provider, please file an issue with the provider developers.`, + ), + } + }, + }, + + "provider deferring action": { + module: map[string]string{ + "main.tf": ` +action "test_unlinked" "hello" {} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_unlinked.hello] + } + } +} +`, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + DeferralAllowed: true, + }, + planActionFn: func(*testing.T, providers.PlanActionRequest) providers.PlanActionResponse { + return providers.PlanActionResponse{ + Deferred: &providers.Deferred{ + Reason: providers.DeferredReasonAbsentPrereq, + }, + } + }, + + assertPlan: func(t *testing.T, p *plans.Plan) { + if len(p.Changes.ActionInvocations) != 0 { + t.Fatalf("expected 0 actions in plan, got %d", len(p.Changes.ActionInvocations)) + } + + if len(p.DeferredActionInvocations) != 1 { + t.Fatalf("expected 1 deferred action in plan, got %d", len(p.DeferredActionInvocations)) + } + deferredActionInvocation := p.DeferredActionInvocations[0] + if deferredActionInvocation.DeferredReason != providers.DeferredReasonAbsentPrereq { + t.Fatalf("expected deferred action to be deferred due to absent prereq, but got %s", deferredActionInvocation.DeferredReason) + } + if deferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { + t.Fatalf("expected deferred action to be triggered by test_object.a, but got %s", deferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) + } + + if deferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.test_unlinked.hello" { + t.Fatalf("expected deferred action to be triggered by action.test_unlinked.hello, but got %s", deferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) + } + }, + }, + + "deferred after actions defer following actions": { + module: map[string]string{ + "main.tf": ` +// Using this provider to have another provider type for an easier assertion +terraform { + required_providers { + ecosystem = { + source = "danielmschmidt/ecosystem" + } + } +} +action "test_unlinked" "hello" {} +action "ecosystem_unlinked" "world" {} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [after_create] + actions = [action.test_unlinked.hello, action.ecosystem_unlinked.world] + } + } +} +`, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + DeferralAllowed: true, + }, + planActionFn: func(t *testing.T, r providers.PlanActionRequest) providers.PlanActionResponse { + if r.ActionType == "ecosystem_unlinked" { + t.Fatalf("expected second action to not be planned, but it was planned") + } + return providers.PlanActionResponse{ + Deferred: &providers.Deferred{ + Reason: providers.DeferredReasonAbsentPrereq, + }, + } + }, + + assertPlan: func(t *testing.T, p *plans.Plan) { + if len(p.Changes.ActionInvocations) != 0 { + t.Fatalf("expected 0 actions in plan, got %d", len(p.Changes.ActionInvocations)) + } + + if len(p.DeferredActionInvocations) != 2 { + t.Fatalf("expected 2 deferred actions in plan, got %d", len(p.DeferredActionInvocations)) + } + firstDeferredActionInvocation := p.DeferredActionInvocations[0] + if firstDeferredActionInvocation.DeferredReason != providers.DeferredReasonAbsentPrereq { + t.Fatalf("expected deferred action to be deferred due to absent prereq, but got %s", firstDeferredActionInvocation.DeferredReason) + } + if firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { + t.Fatalf("expected deferred action to be triggered by test_object.a, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) + } + + if firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.test_unlinked.hello" { + t.Fatalf("expected deferred action to be triggered by action.test_unlinked.hello, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) + } + + secondDeferredActionInvocation := p.DeferredActionInvocations[1] + if secondDeferredActionInvocation.DeferredReason != providers.DeferredReasonDeferredPrereq { + t.Fatalf("expected second deferred action to be deferred due to deferred prereq, but got %s", secondDeferredActionInvocation.DeferredReason) + } + if secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { + t.Fatalf("expected second deferred action to be triggered by test_object.a, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) + } + + if secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.ecosystem_unlinked.world" { + t.Fatalf("expected second deferred action to be triggered by action.ecosystem_unlinked.world, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) + } + }, + }, + + "deferred before actions defer following actions and resource": { + module: map[string]string{ + "main.tf": ` +// Using this provider to have another provider type for an easier assertion +terraform { + required_providers { + ecosystem = { + source = "danielmschmidt/ecosystem" + } + } +} +action "test_unlinked" "hello" {} +action "ecosystem_unlinked" "world" {} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_unlinked.hello] + } + action_trigger { + events = [after_create] + actions = [action.ecosystem_unlinked.world] + } + } +} +`, + }, + expectPlanActionCalled: true, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + DeferralAllowed: true, + }, + planActionFn: func(t *testing.T, r providers.PlanActionRequest) providers.PlanActionResponse { + if r.ActionType == "ecosystem_unlinked" { + t.Fatalf("expected second action to not be planned, but it was planned") + } + return providers.PlanActionResponse{ + Deferred: &providers.Deferred{ + Reason: providers.DeferredReasonAbsentPrereq, + }, + } + }, + + assertPlan: func(t *testing.T, p *plans.Plan) { + if len(p.Changes.ActionInvocations) != 0 { + t.Fatalf("expected 0 actions in plan, got %d", len(p.Changes.ActionInvocations)) + } + + if len(p.DeferredActionInvocations) != 2 { + t.Fatalf("expected 2 deferred actions in plan, got %d", len(p.DeferredActionInvocations)) + } + firstDeferredActionInvocation := p.DeferredActionInvocations[0] + if firstDeferredActionInvocation.DeferredReason != providers.DeferredReasonAbsentPrereq { + t.Fatalf("expected deferred action to be deferred due to absent prereq, but got %s", firstDeferredActionInvocation.DeferredReason) + } + if firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { + t.Fatalf("expected deferred action to be triggered by test_object.a, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) + } + + if firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.test_unlinked.hello" { + t.Fatalf("expected deferred action to be triggered by action.test_unlinked.hello, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) + } + + secondDeferredActionInvocation := p.DeferredActionInvocations[1] + if secondDeferredActionInvocation.DeferredReason != providers.DeferredReasonDeferredPrereq { + t.Fatalf("expected second deferred action to be deferred due to deferred prereq, but got %s", secondDeferredActionInvocation.DeferredReason) + } + if secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { + t.Fatalf("expected second deferred action to be triggered by test_object.a, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) + } + + if secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.ecosystem_unlinked.world" { + t.Fatalf("expected second deferred action to be triggered by action.ecosystem_unlinked.world, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) + } + + if len(p.DeferredResources) != 1 { + t.Fatalf("expected 1 resource to be deferred, got %d", len(p.DeferredResources)) + } + deferredResource := p.DeferredResources[0] + + if deferredResource.ChangeSrc.Addr.String() != "test_object.a" { + t.Fatalf("Expected resource %s to be deferred, but it was not", deferredResource.ChangeSrc.Addr) + } + + if deferredResource.DeferredReason != providers.DeferredReasonDeferredPrereq { + t.Fatalf("Expected deferred reason to be deferred prereq, got %s", deferredResource.DeferredReason) + } + }, + }, + + "deferred resources also defer the actions they trigger": { + module: map[string]string{ + "main.tf": ` +action "test_unlinked" "hello" {} +resource "test_object" "a" { + lifecycle { + action_trigger { + events = [before_create] + actions = [action.test_unlinked.hello] + } + action_trigger { + events = [after_create] + actions = [action.test_unlinked.hello] + } + } +} +`, + }, + expectPlanActionCalled: false, + planOpts: &PlanOpts{ + Mode: plans.NormalMode, + DeferralAllowed: true, + }, + + planResourceFn: func(_ *testing.T, req providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + return providers.PlanResourceChangeResponse{ + PlannedState: req.ProposedNewState, + PlannedPrivate: req.PriorPrivate, + Diagnostics: nil, + Deferred: &providers.Deferred{ + Reason: providers.DeferredReasonAbsentPrereq, + }, + } + }, + + assertPlan: func(t *testing.T, p *plans.Plan) { + if len(p.Changes.ActionInvocations) != 0 { + t.Fatalf("expected 0 actions in plan, got %d", len(p.Changes.ActionInvocations)) + } + + if len(p.DeferredActionInvocations) != 2 { + t.Fatalf("expected 2 deferred actions in plan, got %d", len(p.DeferredActionInvocations)) + } + firstDeferredActionInvocation := p.DeferredActionInvocations[0] + if firstDeferredActionInvocation.DeferredReason != providers.DeferredReasonDeferredPrereq { + t.Fatalf("expected deferred action to be deferred due to deferred prereq, but got %s", firstDeferredActionInvocation.DeferredReason) + } + if firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { + t.Fatalf("expected deferred action to be triggered by test_object.a, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) + } + + if firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.test_unlinked.hello" { + t.Fatalf("expected deferred action to be triggered by action.test_unlinked.hello, but got %s", firstDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) + } + + secondDeferredActionInvocation := p.DeferredActionInvocations[1] + if secondDeferredActionInvocation.DeferredReason != providers.DeferredReasonDeferredPrereq { + t.Fatalf("expected second deferred action to be deferred due to deferred prereq, but got %s", secondDeferredActionInvocation.DeferredReason) + } + if secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String() != "test_object.a" { + t.Fatalf("expected second deferred action to be triggered by test_object.a, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.ActionTrigger.(plans.LifecycleActionTrigger).TriggeringResourceAddr.String()) + } + + if secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String() != "action.test_unlinked.hello" { + t.Fatalf("expected second deferred action to be triggered by action.test_unlinked.hello, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String()) + } + + if len(p.DeferredResources) != 1 { + t.Fatalf("expected 1 resource to be deferred, got %d", len(p.DeferredResources)) + } + deferredResource := p.DeferredResources[0] + + if deferredResource.ChangeSrc.Addr.String() != "test_object.a" { + t.Fatalf("Expected resource %s to be deferred, but it was not", deferredResource.ChangeSrc.Addr) + } + + if deferredResource.DeferredReason != providers.DeferredReasonAbsentPrereq { + t.Fatalf("Expected deferred reason to be absent prereq, got %s", deferredResource.DeferredReason) + } + }, + }, } { t.Run(name, func(t *testing.T) { if tc.toBeImplemented { @@ -1423,7 +1755,15 @@ resource "test_object" "a" { } if tc.planActionFn != nil { - p.PlanActionFn = tc.planActionFn + p.PlanActionFn = func(r providers.PlanActionRequest) providers.PlanActionResponse { + return tc.planActionFn(t, r) + } + } + + if tc.planResourceFn != nil { + p.PlanResourceChangeFn = func(r providers.PlanResourceChangeRequest) providers.PlanResourceChangeResponse { + return tc.planResourceFn(t, r) + } } ctx := testContext2(t, &ContextOpts{ diff --git a/internal/terraform/node_action_trigger_instance_plan.go b/internal/terraform/node_action_trigger_instance_plan.go index a2e3f4c108..818054948d 100644 --- a/internal/terraform/node_action_trigger_instance_plan.go +++ b/internal/terraform/node_action_trigger_instance_plan.go @@ -10,6 +10,7 @@ import ( "github.com/hashicorp/terraform/internal/addrs" "github.com/hashicorp/terraform/internal/configs" "github.com/hashicorp/terraform/internal/plans" + "github.com/hashicorp/terraform/internal/plans/deferring" "github.com/hashicorp/terraform/internal/providers" "github.com/hashicorp/terraform/internal/tfdiags" ) @@ -35,6 +36,15 @@ func (at *lifecycleActionTriggerInstance) Name() string { return fmt.Sprintf("%s.lifecycle.action_trigger[%d].actions[%d]", at.resourceAddress.String(), at.actionTriggerBlockIndex, at.actionListIndex) } +func (at *lifecycleActionTriggerInstance) ActionTrigger(triggeringEvent configs.ActionTriggerEvent) plans.LifecycleActionTrigger { + return plans.LifecycleActionTrigger{ + TriggeringResourceAddr: at.resourceAddress, + ActionTriggerBlockIndex: at.actionTriggerBlockIndex, + ActionsListIndex: at.actionListIndex, + ActionTriggerEvent: triggeringEvent, + } +} + var ( _ GraphNodeModuleInstance = (*nodeActionTriggerPlanInstance)(nil) _ GraphNodeExecutable = (*nodeActionTriggerPlanInstance)(nil) @@ -53,11 +63,37 @@ func (n *nodeActionTriggerPlanInstance) Name() string { func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkOperation) tfdiags.Diagnostics { var diags tfdiags.Diagnostics + deferrals := ctx.Deferrals() if n.lifecycleActionTrigger == nil { panic("Only actions triggered by plan and apply are supported") } + actionInstance, ok := ctx.Actions().GetActionInstance(n.actionAddress) + if !ok { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Reference to non-existant action instance", + Detail: "Action instance was not found in the current context.", + Subject: n.lifecycleActionTrigger.invokingSubject, + }) + return diags + } + + // We need the action invocation early to check if we need to + ai := plans.ActionInvocationInstance{ + Addr: n.actionAddress, + ProviderAddr: actionInstance.ProviderAddr, + ActionTrigger: n.lifecycleActionTrigger.ActionTrigger(configs.Unknown), + ConfigValue: actionInstance.ConfigValue, + } + + // If we already deferred an action invocation on the same resource with an earlier trigger we can defer this one as well + if deferrals.DeferralAllowed() && deferrals.ShouldDeferActionInvocation(ai) { + deferrals.ReportActionInvocationDeferred(ai, providers.DeferredReasonDeferredPrereq) + return diags + } + change := ctx.Changes().GetResourceInstanceChange(n.lifecycleActionTrigger.resourceAddress, n.lifecycleActionTrigger.resourceAddress.CurrentObject().DeposedKey) if change == nil { panic("change cannot be nil") @@ -69,18 +105,8 @@ func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkO if triggeringEvent == nil { panic("triggeringEvent cannot be nil") } - - actionInstance, ok := ctx.Actions().GetActionInstance(n.actionAddress) - - if !ok { - diags = diags.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Reference to non-existant action instance", - Detail: "Action instance was not found in the current context.", - Subject: n.lifecycleActionTrigger.invokingSubject, - }) - return diags - } + // We need to set the triggering event on the action invocation + ai.ActionTrigger = n.lifecycleActionTrigger.ActionTrigger(*triggeringEvent) provider, _, err := getProvider(ctx, actionInstance.ProviderAddr) if err != nil { @@ -117,23 +143,24 @@ func (n *nodeActionTriggerPlanInstance) Execute(ctx EvalContext, operation walkO Subject: n.lifecycleActionTrigger.invokingSubject, }) } + if resp.Deferred != nil && !deferrals.DeferralAllowed() { + diags = diags.Append(deferring.UnexpectedProviderDeferralDiagnostic(n.actionAddress)) + } if resp.Diagnostics.HasErrors() { return diags } - // TODO: Deal with deferred responses - ctx.Changes().AppendActionInvocation(&plans.ActionInvocationInstance{ - Addr: n.actionAddress, - ProviderAddr: actionInstance.ProviderAddr, - ActionTrigger: plans.LifecycleActionTrigger{ - TriggeringResourceAddr: n.lifecycleActionTrigger.resourceAddress, - ActionTriggerEvent: *triggeringEvent, - ActionTriggerBlockIndex: n.lifecycleActionTrigger.actionTriggerBlockIndex, - ActionsListIndex: n.lifecycleActionTrigger.actionListIndex, - }, - ConfigValue: actionInstance.ConfigValue, - }) + if resp.Deferred != nil { + deferrals.ReportActionInvocationDeferred(ai, resp.Deferred.Reason) + + // If we run as part of a before action we need to retrospectively defer the triggering resource + // For this we remove the change and report the deferral + ctx.Changes().RemoveResourceInstanceChange(change.Addr, change.Addr.CurrentObject().DeposedKey) + deferrals.ReportResourceInstanceDeferred(change.Addr, providers.DeferredReasonDeferredPrereq, change) + return diags + } + ctx.Changes().AppendActionInvocation(&ai) return diags }