From abf692ccd5072fe6765950cfd8f526004de97522 Mon Sep 17 00:00:00 2001 From: Kristin Laemmert Date: Mon, 30 Mar 2026 12:32:08 -0400 Subject: [PATCH] actions: move condition evaluation into configs package We have several requirements for action conditions which can all be checked while loading configuration. This PR moves action condition validation into the configs package (without removing any now-duplicated checks from the terraform package). The (now) duplicated checks will be removed as the actions work continues. I've also added tests to the config package, and removed tests that are now failing because we catch the error much earlier. --- internal/configs/action.go | 42 ++++ internal/configs/action_test.go | 108 ++++++++- .../terraform/context_plan_actions_test.go | 224 ------------------ 3 files changed, 138 insertions(+), 236 deletions(-) diff --git a/internal/configs/action.go b/internal/configs/action.go index 30c474dd8f..fe9e3dc0c8 100644 --- a/internal/configs/action.go +++ b/internal/configs/action.go @@ -83,8 +83,24 @@ func decodeActionTriggerBlock(block *hcl.Block) (*ActionTrigger, hcl.Diagnostics content, bodyDiags := block.Body.Content(actionTriggerSchema) diags = append(diags, bodyDiags...) + var refs []*addrs.Reference + var refDiags tfdiags.Diagnostics if attr, exists := content.Attributes["condition"]; exists { a.Condition = attr.Expr + + refs, refDiags = langrefs.ReferencesInExpr(addrs.ParseRef, attr.Expr) + diags = append(diags, refDiags.ToHCL()...) + + for _, ref := range refs { + if ref.Subject == addrs.Self { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Self reference not allowed", + Detail: `The condition expression cannot reference "self".`, + Subject: attr.Expr.Range().Ptr(), + }) + } + } } if attr, exists := content.Attributes["events"]; exists { @@ -92,16 +108,19 @@ func decodeActionTriggerBlock(block *hcl.Block) (*ActionTrigger, hcl.Diagnostics diags = append(diags, ediags...) events := []ActionTriggerEvent{} + containsBefore := false for _, expr := range exprs { var event ActionTriggerEvent switch hcl.ExprAsKeyword(expr) { case "before_create": event = BeforeCreate + containsBefore = true case "after_create": event = AfterCreate case "before_update": event = BeforeUpdate + containsBefore = true case "after_update": event = AfterUpdate default: @@ -125,6 +144,29 @@ func decodeActionTriggerBlock(block *hcl.Block) (*ActionTrigger, hcl.Diagnostics continue } + // Check that there aren't any before_ events using self, count or for_each in the condition + if containsBefore && refs != nil { // if refs isn't empty, there was a condition + for _, ref := range refs { + if _, ok := ref.Subject.(addrs.CountAttr); ok { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Count reference not allowed", + Detail: `The condition expression cannot reference "count" if the action is run before the resource is applied.`, + Subject: a.Condition.Range().Ptr(), + }) + } + + if _, ok := ref.Subject.(addrs.ForEachAttr); ok { + diags = diags.Append(&hcl.Diagnostic{ + Severity: hcl.DiagError, + Summary: "Each reference not allowed", + Detail: `The condition expression cannot reference "each" if the action is run before the resource is applied.`, + Subject: a.Condition.Range().Ptr(), + }) + } + } + } + events = append(events, event) } diff --git a/internal/configs/action_test.go b/internal/configs/action_test.go index f80b4cb0f8..26fe50a6fe 100644 --- a/internal/configs/action_test.go +++ b/internal/configs/action_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/hashicorp/hcl/v2" + "github.com/hashicorp/hcl/v2/hclsyntax" "github.com/hashicorp/hcl/v2/hcltest" "github.com/zclconf/go-cty/cty" ) @@ -66,7 +67,16 @@ func TestDecodeActionBlock(t *testing.T) { } func TestDecodeActionTriggerBlock(t *testing.T) { - conditionExpr := hcltest.MockExprLiteral(cty.True) + trueConditionExpr := hcltest.MockExprLiteral(cty.True) + countExpr, hclDiags := hclsyntax.ParseExpression([]byte("test_resource.a[count.index]"), "", hcl.InitialPos) + if hclDiags.HasErrors() { + t.Fatal(hclDiags) + } + eachExpr, hclDiags := hclsyntax.ParseExpression([]byte("test_resource.a[each.key]"), "", hcl.InitialPos) + if hclDiags.HasErrors() { + t.Fatal(hclDiags) + } + eventsListExpr := hcltest.MockExprList([]hcl.Expression{hcltest.MockExprTraversalSrc("after_create"), hcltest.MockExprTraversalSrc("after_update")}) fooActionExpr := hcltest.MockExprTraversalSrc("action.action_type.foo") @@ -87,14 +97,14 @@ func TestDecodeActionTriggerBlock(t *testing.T) { Type: "action_trigger", Body: hcltest.MockBody(&hcl.BodyContent{ Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ - "condition": conditionExpr, + "condition": trueConditionExpr, "events": eventsListExpr, "actions": fooAndBarExpr, }), }), }, &ActionTrigger{ - Condition: conditionExpr, + Condition: trueConditionExpr, Events: []ActionTriggerEvent{AfterCreate, AfterUpdate}, Actions: []ActionRef{ { @@ -114,14 +124,14 @@ func TestDecodeActionTriggerBlock(t *testing.T) { Type: "action_trigger", Body: hcltest.MockBody(&hcl.BodyContent{ Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ - "condition": conditionExpr, + "condition": trueConditionExpr, "events": eventsListExpr, "actions": hcltest.MockExprList([]hcl.Expression{moduleActionExpr}), }), }), }, &ActionTrigger{ - Condition: conditionExpr, + Condition: trueConditionExpr, Events: []ActionTriggerEvent{AfterCreate, AfterUpdate}, Actions: []ActionRef{ { @@ -140,14 +150,14 @@ func TestDecodeActionTriggerBlock(t *testing.T) { Type: "action_trigger", Body: hcltest.MockBody(&hcl.BodyContent{ Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ - "condition": conditionExpr, + "condition": trueConditionExpr, "events": eventsListExpr, "actions": hcltest.MockExprList([]hcl.Expression{fooDataSourceExpr}), }), }), }, &ActionTrigger{ - Condition: conditionExpr, + Condition: trueConditionExpr, Events: []ActionTriggerEvent{AfterCreate, AfterUpdate}, Actions: []ActionRef{ { @@ -166,14 +176,14 @@ func TestDecodeActionTriggerBlock(t *testing.T) { Type: "action_trigger", Body: hcltest.MockBody(&hcl.BodyContent{ Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ - "condition": conditionExpr, + "condition": trueConditionExpr, "events": hcltest.MockExprList([]hcl.Expression{hcltest.MockExprTraversalSrc("not_an_event")}), "actions": hcltest.MockExprList([]hcl.Expression{fooActionExpr}), }), }), }, &ActionTrigger{ - Condition: conditionExpr, + Condition: trueConditionExpr, Events: []ActionTriggerEvent{}, Actions: []ActionRef{ { @@ -187,20 +197,19 @@ func TestDecodeActionTriggerBlock(t *testing.T) { ":0,0-0: No events specified; At least one event must be specified for an action_trigger.", }, }, - "error - duplicate event": { &hcl.Block{ Type: "action_trigger", Body: hcltest.MockBody(&hcl.BodyContent{ Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ - "condition": conditionExpr, + "condition": trueConditionExpr, "events": hcltest.MockExprList([]hcl.Expression{hcltest.MockExprTraversalSrc("before_create"), hcltest.MockExprTraversalSrc("before_create")}), "actions": hcltest.MockExprList([]hcl.Expression{fooActionExpr}), }), }), }, &ActionTrigger{ - Condition: conditionExpr, + Condition: trueConditionExpr, Events: []ActionTriggerEvent{BeforeCreate}, Actions: []ActionRef{ { @@ -213,6 +222,81 @@ func TestDecodeActionTriggerBlock(t *testing.T) { `MockExprTraversal:0,0-13: Duplicate "before_create" event; The event is already defined in this action_trigger block.`, }, }, + "error - condition references self": { + &hcl.Block{ + Type: "action_trigger", + Body: hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ + "condition": hcltest.MockExprTraversalSrc("self.id"), + "events": hcltest.MockExprList([]hcl.Expression{hcltest.MockExprTraversalSrc("before_create"), hcltest.MockExprTraversalSrc("after_create")}), + "actions": hcltest.MockExprList([]hcl.Expression{fooActionExpr}), + }), + }), + }, + &ActionTrigger{ + Condition: hcltest.MockExprTraversalSrc("self.id"), + Events: []ActionTriggerEvent{BeforeCreate, AfterCreate}, + Actions: []ActionRef{ + { + fooActionExpr, + fooActionExpr.Range(), + }, + }, + }, + []string{ + `MockExprTraversal:0,0-7: Self reference not allowed; The condition expression cannot reference "self".`, + }, + }, + "error - condition uses count.index and includes before_event": { + &hcl.Block{ + Type: "action_trigger", + Body: hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ + "condition": countExpr, + "events": hcltest.MockExprList([]hcl.Expression{hcltest.MockExprTraversalSrc("before_create"), hcltest.MockExprTraversalSrc("after_create")}), + "actions": hcltest.MockExprList([]hcl.Expression{fooActionExpr}), + }), + }), + }, + &ActionTrigger{ + Condition: countExpr, + Events: []ActionTriggerEvent{BeforeCreate, AfterCreate}, + Actions: []ActionRef{ + { + fooActionExpr, + fooActionExpr.Range(), + }, + }, + }, + []string{ + `:1,1-29: Count reference not allowed; The condition expression cannot reference "count" if the action is run before the resource is applied.`, + }, + }, + "error - condition uses each.value and includes before_event": { + &hcl.Block{ + Type: "action_trigger", + Body: hcltest.MockBody(&hcl.BodyContent{ + Attributes: hcltest.MockAttrs(map[string]hcl.Expression{ + "condition": eachExpr, + "events": hcltest.MockExprList([]hcl.Expression{hcltest.MockExprTraversalSrc("before_create"), hcltest.MockExprTraversalSrc("after_create")}), + "actions": hcltest.MockExprList([]hcl.Expression{fooActionExpr}), + }), + }), + }, + &ActionTrigger{ + Condition: eachExpr, + Events: []ActionTriggerEvent{BeforeCreate, AfterCreate}, + Actions: []ActionRef{ + { + fooActionExpr, + fooActionExpr.Range(), + }, + }, + }, + []string{ + `:1,1-26: Each reference not allowed; The condition expression cannot reference "each" if the action is run before the resource is applied.`, + }, + }, } for name, test := range tests { diff --git a/internal/terraform/context_plan_actions_test.go b/internal/terraform/context_plan_actions_test.go index 18752837a4..c98ef4dd32 100644 --- a/internal/terraform/context_plan_actions_test.go +++ b/internal/terraform/context_plan_actions_test.go @@ -3215,83 +3215,6 @@ resource "test_object" "a" { }, }, - "using self in before_* condition": { - module: map[string]string{ - "main.tf": ` -action "test_action" "hello" {} -action "test_action" "world" {} -resource "test_object" "a" { - name = "foo" - lifecycle { - action_trigger { - events = [before_create] - condition = self.name == "foo" - actions = [action.test_action.hello] - } - action_trigger { - events = [after_update] - condition = self.name == "bar" - actions = [action.test_action.world] - } - } -} -`, - }, - expectPlanActionCalled: false, - - expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics { - return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Self reference not allowed", - Detail: `The condition expression cannot reference "self".`, - Subject: &hcl.Range{ - Filename: filepath.Join(m.Module.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 9, Column: 19, Byte: 193}, - End: hcl.Pos{Line: 9, Column: 37, Byte: 211}, - }, - }) - }, - }, - - "using self in after_* condition": { - module: map[string]string{ - "main.tf": ` -action "test_action" "hello" {} -action "test_action" "world" {} -resource "test_object" "a" { - name = "foo" - lifecycle { - action_trigger { - events = [after_create] - condition = self.name == "foo" - actions = [action.test_action.hello] - } - action_trigger { - events = [after_update] - condition = self.name == "bar" - actions = [action.test_action.world] - } - } -} -`, - }, - expectPlanActionCalled: false, - - expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics { - // We only expect one diagnostic, as the other condition is valid - return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Self reference not allowed", - Detail: `The condition expression cannot reference "self".`, - Subject: &hcl.Range{ - Filename: filepath.Join(m.Module.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 9, Column: 19, Byte: 192}, - End: hcl.Pos{Line: 9, Column: 37, Byte: 210}, - }, - }) - }, - }, - "referencing triggering resource in before_* condition": { module: map[string]string{ "main.tf": ` @@ -3359,49 +3282,6 @@ resource "test_object" "a" { }, }, - "using each in before_* condition": { - module: map[string]string{ - "main.tf": ` -action "test_action" "hello" {} -action "test_action" "world" {} -resource "test_object" "a" { - for_each = toset(["foo", "bar"]) - name = each.key - lifecycle { - action_trigger { - events = [before_create] - condition = each.key == "foo" - actions = [action.test_action.hello] - } - } -} -`, - }, - expectPlanActionCalled: false, - - expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics { - return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Each reference not allowed", - Detail: `The condition expression cannot reference "each" if the action is run before the resource is applied.`, - Subject: &hcl.Range{ - Filename: filepath.Join(m.Module.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 10, Column: 19, Byte: 231}, - End: hcl.Pos{Line: 10, Column: 36, Byte: 248}, - }, - }).Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Each reference not allowed", - Detail: `The condition expression cannot reference "each" if the action is run before the resource is applied.`, - Subject: &hcl.Range{ - Filename: filepath.Join(m.Module.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 10, Column: 19, Byte: 231}, - End: hcl.Pos{Line: 10, Column: 36, Byte: 248}, - }, - }) - }, - }, - "using each in after_* condition": { module: map[string]string{ "main.tf": ` @@ -3436,62 +3316,6 @@ resource "test_object" "a" { }, }, - "using count.index in before_* condition": { - module: map[string]string{ - "main.tf": ` -action "test_action" "hello" {} -action "test_action" "world" {} -resource "test_object" "a" { - count = 3 - name = "item-${count.index}" - lifecycle { - action_trigger { - events = [before_create] - condition = count.index == 1 - actions = [action.test_action.hello] - } - action_trigger { - events = [before_update] - condition = count.index == 2 - actions = [action.test_action.world] - } - } -}`, - }, - expectPlanActionCalled: false, - - expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics { - return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Count reference not allowed", - Detail: `The condition expression cannot reference "count" if the action is run before the resource is applied.`, - Subject: &hcl.Range{ - Filename: filepath.Join(m.Module.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 10, Column: 21, Byte: 210}, - End: hcl.Pos{Line: 10, Column: 37, Byte: 226}, - }, - }).Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Count reference not allowed", - Detail: `The condition expression cannot reference "count" if the action is run before the resource is applied.`, - Subject: &hcl.Range{ - Filename: filepath.Join(m.Module.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 10, Column: 21, Byte: 210}, - End: hcl.Pos{Line: 10, Column: 37, Byte: 226}, - }, - }).Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Count reference not allowed", - Detail: `The condition expression cannot reference "count" if the action is run before the resource is applied.`, - Subject: &hcl.Range{ - Filename: filepath.Join(m.Module.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 10, Column: 21, Byte: 210}, - End: hcl.Pos{Line: 10, Column: 37, Byte: 226}, - }, - }) - }, - }, - "using count.index in after_* condition": { module: map[string]string{ "main.tf": ` @@ -3527,54 +3351,6 @@ resource "test_object" "a" { }, }, - "using each.value in before_* condition": { - module: map[string]string{ - "main.tf": ` -action "test_action" "hello" {} -action "test_action" "world" {} -resource "test_object" "a" { - for_each = {"foo" = "value1", "bar" = "value2"} - name = each.value - lifecycle { - action_trigger { - events = [before_create] - condition = each.value == "value1" - actions = [action.test_action.hello] - } - action_trigger { - events = [before_update] - condition = each.value == "value2" - actions = [action.test_action.world] - } - } -} - `, - }, - expectPlanActionCalled: false, - - expectPlanDiagnostics: func(m *configs.Config) tfdiags.Diagnostics { - return tfdiags.Diagnostics{}.Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Each reference not allowed", - Detail: `The condition expression cannot reference "each" if the action is run before the resource is applied.`, - Subject: &hcl.Range{ - Filename: filepath.Join(m.Module.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 10, Column: 21, Byte: 260}, - End: hcl.Pos{Line: 10, Column: 43, Byte: 282}, - }, - }).Append(&hcl.Diagnostic{ - Severity: hcl.DiagError, - Summary: "Each reference not allowed", - Detail: `The condition expression cannot reference "each" if the action is run before the resource is applied.`, - Subject: &hcl.Range{ - Filename: filepath.Join(m.Module.SourceDir, "main.tf"), - Start: hcl.Pos{Line: 10, Column: 21, Byte: 260}, - End: hcl.Pos{Line: 10, Column: 43, Byte: 282}, - }, - }) - }, - }, - "using each.value in after_* condition": { module: map[string]string{ "main.tf": `