Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 28 additions & 40 deletions internal/plans/deferring/deferred.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"fmt"
"sync"

"github.com/hashicorp/hcl/v2"
"github.com/zclconf/go-cty/cty"

"github.com/hashicorp/terraform/internal/addrs"
Expand Down Expand Up @@ -729,64 +730,51 @@ func (d *Deferred) ReportActionDeferred(addr addrs.AbsActionInstance, reason pro
configMap.Put(addr, reason)
}

// 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 {
// ShouldDeferActionInvocation returns true if there is a reason to defer the
// action invocation instance. We want to defer an action invocation only if
// the triggering resource was deferred. In addition, we will check if the
// underlying action was deferred via a reference, and consider it an error if
// the triggering resource wasn't also deferred.
//
// The reason behind the slightly different behaviour here, is that if an
// action invocation is deferred, then that implies the triggering action
// should also be deferred.
//
// We don't yet have the capability to retroactively defer a resource, so for
// now actions initiating deferrals themselves is considered an error.
func (d *Deferred) ShouldDeferActionInvocation(ai plans.ActionInvocationInstance, triggerRange *hcl.Range) (bool, tfdiags.Diagnostics) {
d.mu.Lock()
defer d.mu.Unlock()

// The expansion of the action itself is deferred
if ai.Addr.Action.Key == addrs.WildcardKey {
return true
}

if c, ok := d.actionExpansionDeferred.GetOk(ai.Addr.ConfigAction()); ok {
if c.Has(ai.Addr) {
return true
}

for _, k := range c.Keys() {
if k.Action.Key == addrs.WildcardKey {
return true
}
}
}

if d.partialExpandedActionsDeferred.Has(ai.Addr.ConfigAction()) {
return true
}
var diags tfdiags.Diagnostics

// We only want to defer actions that are lifecycle triggered
at, ok := ai.ActionTrigger.(*plans.LifecycleActionTrigger)
if !ok {
return false
return false, diags
}

// 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
return true, diags
}
}

// 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
if c, ok := d.actionExpansionDeferred.GetOk(ai.Addr.ConfigAction()); ok {
if c.Has(ai.Addr) {
// Then in this case, the resource wasn't deferred but the action
// was and so we will consider this to be an error.
diags = diags.Append(&hcl.Diagnostic{
Severity: hcl.DiagError,
Summary: "Invalid action deferral",
Detail: fmt.Sprintf("The action %s was marked as deferred, but was triggered by a non-deferred resource %s. To work around this, use the -target argument to first apply only the resources that the action block depends on.", ai.Addr, at.TriggeringResourceAddr),
Subject: triggerRange,
})
}
}

// We found no reason, so we return false
return false
return false, diags
}

// ShouldDeferAction returns true if the action should be deferred. This is the case if a
Expand Down
187 changes: 44 additions & 143 deletions internal/terraform/context_plan_actions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1818,7 +1818,7 @@ resource "test_object" "a" {
expectPlanActionCalled: true,
planOpts: &PlanOpts{
Mode: plans.NormalMode,
DeferralAllowed: true,
DeferralAllowed: true, // actions should ignore this setting
},
planActionFn: func(*testing.T, providers.PlanActionRequest) providers.PlanActionResponse {
return providers.PlanActionResponse{
Expand All @@ -1827,25 +1827,13 @@ resource "test_object" "a" {
},
}
},

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_action.hello" {
t.Fatalf("expected deferred action to be triggered by action.test_action.hello, but got %s", deferredActionInvocation.ActionInvocationInstanceSrc.Addr.String())
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_action.hello", but in this context deferrals are disabled. This is a bug in the provider, please file an issue with the provider developers.`,
),
}
},
},
Expand Down Expand Up @@ -1888,37 +1876,17 @@ resource "test_object" "a" {
},
}
},

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_action.hello" {
t.Fatalf("expected deferred action to be triggered by action.test_action.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.world" {
t.Fatalf("expected second deferred action to be triggered by action.ecosystem.world, but got %s", secondDeferredActionInvocation.ActionInvocationInstanceSrc.Addr.String())
expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics {
// for now, it's just an error for any deferrals but when
// this gets implemented we should check that all the
// actions are deferred even though only one of them
// was actually marked as deferred.
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_action.hello", but in this context deferrals are disabled. This is a bug in the provider, please file an issue with the provider developers.`,
),
}
},
},
Expand Down Expand Up @@ -1965,50 +1933,17 @@ resource "test_object" "a" {
},
}
},

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_action.hello" {
t.Fatalf("expected deferred action to be triggered by action.test_action.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.world" {
t.Fatalf("expected second deferred action to be triggered by action.ecosystem.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)
expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics {
// for now, it's just an error for any deferrals but when
// this gets implemented we should check that all the
// actions are deferred even though only one of them
// was actually marked as deferred.
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_action.hello", but in this context deferrals are disabled. This is a bug in the provider, please file an issue with the provider developers.`,
),
}
},
},
Expand Down Expand Up @@ -2130,26 +2065,17 @@ resource "test_object" "a" {
},
},
},
assertPlan: func(t *testing.T, p *plans.Plan) {
if len(p.DeferredActionInvocations) != 1 {
t.Fatalf("expected exactly one invocation, and found %d", len(p.DeferredActionInvocations))
assertPlanDiagnostics: func(t *testing.T, diagnostics tfdiags.Diagnostics) {
if len(diagnostics) != 1 {
t.Fatal("wrong number of diagnostics")
}

if p.DeferredActionInvocations[0].DeferredReason != providers.DeferredReasonDeferredPrereq {
t.Fatalf("expected.DeferredReasonDeferredPrereq, got %s", p.DeferredActionInvocations[0].DeferredReason)
if diagnostics[0].Severity() != tfdiags.Error {
t.Error("expected error severity")
}

ai := p.DeferredActionInvocations[0].ActionInvocationInstanceSrc
if ai.Addr.String() != `action.test_action.hello["a"]` {
t.Fatalf(`expected action invocation for action.test_action.hello["a"], got %s`, ai.Addr.String())
}

if len(p.DeferredResources) != 1 {
t.Fatalf("expected 1 deferred resource, got %d", len(p.DeferredResources))
}

if p.DeferredResources[0].ChangeSrc.Addr.String() != "test_object.a" {
t.Fatalf("expected test_object.a, got %s", p.DeferredResources[0].ChangeSrc.Addr.String())
if diagnostics[0].Description().Summary != "Invalid for_each argument" {
t.Errorf("expected for_each argument to be source of error but was %s", diagnostics[0].Description().Summary)
}
},
},
Expand Down Expand Up @@ -2335,42 +2261,17 @@ resource "test_object" "a" {
}
},

assertPlan: func(t *testing.T, p *plans.Plan) {
if len(p.DeferredActionInvocations) != 1 {
t.Errorf("Expected 1 deferred action invocation, got %d", len(p.DeferredActionInvocations))
}
if p.DeferredActionInvocations[0].ActionInvocationInstanceSrc.Addr.String() != "action.test_action.hello" {
t.Errorf("Expected action. test_action.hello, got %s", p.DeferredActionInvocations[0].ActionInvocationInstanceSrc.Addr.String())
}
if p.DeferredActionInvocations[0].DeferredReason != providers.DeferredReasonDeferredPrereq {
t.Errorf("Expected DeferredReasonDeferredPrereq, got %s", p.DeferredActionInvocations[0].DeferredReason)
assertPlanDiagnostics: func(t *testing.T, diagnostics tfdiags.Diagnostics) {
if len(diagnostics) != 1 {
t.Fatal("wrong number of diagnostics")
}

if len(p.DeferredResources) != 2 {
t.Fatalf("Expected 2 deferred resources, got %d", len(p.DeferredResources))
if diagnostics[0].Severity() != tfdiags.Error {
t.Error("expected error diagnostics")
}

slices.SortFunc(p.DeferredResources, func(a, b *plans.DeferredResourceInstanceChangeSrc) int {
if a.ChangeSrc.Addr.Less(b.ChangeSrc.Addr) {
return -1
}
if b.ChangeSrc.Addr.Less(a.ChangeSrc.Addr) {
return 1
}
return 0
})

if p.DeferredResources[0].ChangeSrc.Addr.String() != "test_object.a" {
t.Errorf("Expected test_object.a to be first, got %s", p.DeferredResources[0].ChangeSrc.Addr.String())
}
if p.DeferredResources[0].DeferredReason != providers.DeferredReasonDeferredPrereq {
t.Errorf("Expected DeferredReasonDeferredPrereq, got %s", p.DeferredResources[0].DeferredReason)
}
if p.DeferredResources[1].ChangeSrc.Addr.String() != "test_object.origin" {
t.Errorf("Expected test_object.origin to be second, got %s", p.DeferredResources[1].ChangeSrc.Addr.String())
}
if p.DeferredResources[1].DeferredReason != providers.DeferredReasonAbsentPrereq {
t.Errorf("Expected DeferredReasonAbsentPrereq, got %s", p.DeferredResources[1].DeferredReason)
if diagnostics[0].Description().Summary != "Invalid action deferral" {
t.Errorf("expected deferral to be source of error was %s", diagnostics[0].Description().Summary)
}
},
},
Expand Down
Loading
Loading