aherman
/
terraform
mirror of https://github.com/hashicorp/terraform
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

actions: move condition evaluation into configs package

Browse Source 
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.
pull/38327/head
Kristin Laemmert 2 months ago
parent 259c468fb1
commit abf692ccd5
3 changed files with 138 additions and 236 deletions
Show Stats Download Patch File Download Diff File

42
internal/configs/action.go
Unescape View File

@ -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)
}

108
internal/configs/action_test.go
Unescape View File

@ -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 {

224
internal/terraform/context_plan_actions_test.go
Unescape View File

@ -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": `

Write Preview
Loading…
Cancel
Save