Skip to content

Commit 333de76

Browse files
mildwonkeymildwonkey
authored
feat(actions): extend action validation inside configs package (#37355)
* Add extended action validation during config parsing - Verify that actions and linked_resources are in the current module - Verify that given traversals reference the right types (action, resource) - Add some validation tests
1 parent 0374f04 commit 333de76

File tree

2 files changed

+255
-35
lines changed

2 files changed

+255
-35
lines changed

‎internal/configs/action.go

Lines changed: 82 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,24 @@ import (
1212
"github.com/hashicorp/terraform/internal/addrs"
1313
)
1414

15+
func invalidLinkedResourceDiag(subj *hcl.Range) *hcl.Diagnostic {
16+
return &hcl.Diagnostic{
17+
Severity: hcl.DiagError,
18+
Summary: `Invalid "linked_resource"`,
19+
Detail: `"linked_resource" must only refer to a managed resource in the current module.`,
20+
Subject: subj,
21+
}
22+
}
23+
24+
func invalidLinkedResourcesDiag(subj *hcl.Range) *hcl.Diagnostic {
25+
return &hcl.Diagnostic{
26+
Severity: hcl.DiagError,
27+
Summary: `Invalid "linked_resources"`,
28+
Detail: `"linked_resources" must only refer to managed resources in the current module.`,
29+
Subject: subj,
30+
}
31+
}
32+
1533
// Action represents an "action" block inside a configuration
1634
type Action struct {
1735
Name string
@@ -105,6 +123,7 @@ func decodeActionTriggerBlock(block *hcl.Block) (*ActionTrigger, hcl.Diagnostics
105123
Detail: "The \"event\" argument supports the following values: before_create, after_create, before_update, after_update, before_destroy, after_destroy.",
106124
Subject: expr.Range().Ptr(),
107125
})
126+
continue
108127
}
109128

110129
if event == BeforeDestroy || event == AfterDestroy {
@@ -114,8 +133,8 @@ func decodeActionTriggerBlock(block *hcl.Block) (*ActionTrigger, hcl.Diagnostics
114133
Detail: "The destroy events (before_destroy, after_destroy) are not supported as of right now. They will be supported in a future release.",
115134
Subject: expr.Range().Ptr(),
116135
})
136+
continue
117137
}
118-
119138
events = append(events, event)
120139
}
121140
a.Events = events
@@ -128,25 +147,29 @@ func decodeActionTriggerBlock(block *hcl.Block) (*ActionTrigger, hcl.Diagnostics
128147
for _, expr := range exprs {
129148
traversal, travDiags := hcl.AbsTraversalForExpr(expr)
130149
diags = append(diags, travDiags...)
131-
// verify that the traversal is an action
132-
if traversal.RootName() != "action" {
133-
diags = append(diags, &hcl.Diagnostic{
134-
Severity: hcl.DiagError,
135-
Summary: "Invalid actions argument inside action_triggers",
136-
Detail: "action_triggers.actions accepts a list of one or more actions",
137-
Subject: block.DefRange.Ptr(),
138-
})
139-
continue
140-
}
141150

142-
if len(traversal) != 0 {
143-
actionRef := ActionRef{
144-
Traversal: traversal,
145-
Range: expr.Range(),
151+
if len(traversal) > 0 {
152+
// verify that the traversal is an action
153+
ref, refDiags := addrs.ParseRef(traversal)
154+
diags = append(diags, refDiags.ToHCL()...)
155+
156+
switch ref.Subject.(type) {
157+
case addrs.ActionInstance, addrs.Action:
158+
actionRef := ActionRef{
159+
Traversal: traversal,
160+
Range: expr.Range(),
161+
}
162+
actions = append(actions, actionRef)
163+
default:
164+
diags = append(diags, &hcl.Diagnostic{
165+
Severity: hcl.DiagError,
166+
Summary: "Invalid actions argument inside action_triggers",
167+
Detail: "action_triggers.actions accepts a list of one or more actions, which must be in the current module.",
168+
Subject: expr.Range().Ptr(),
169+
})
170+
continue
146171
}
147-
actions = append(actions, actionRef)
148172
}
149-
150173
}
151174
a.Actions = actions
152175
}
@@ -222,15 +245,33 @@ func decodeActionBlock(block *hcl.Block) (*Action, hcl.Diagnostics) {
222245
diags = append(diags, &hcl.Diagnostic{
223246
Severity: hcl.DiagError,
224247
Summary: `Invalid use of "linked_resource"`,
225-
Detail: `The "linked_resource" and "linked_resources" are mutually exclusive, only one should be used.`,
248+
Detail: `"linked_resource" and "linked_resources" are mutually exclusive, only one should be used.`,
226249
Subject: &attr.NameRange,
227250
})
228251
}
229252

230253
traversal, travDiags := hcl.AbsTraversalForExpr(attr.Expr)
231254
diags = append(diags, travDiags...)
232255
if len(traversal) != 0 {
233-
a.LinkedResources = []hcl.Traversal{traversal}
256+
ref, refDiags := addrs.ParseRef(traversal)
257+
diags = append(diags, refDiags.ToHCL()...)
258+
259+
switch res := ref.Subject.(type) {
260+
case addrs.Resource:
261+
if res.Mode != addrs.ManagedResourceMode {
262+
diags = append(diags, invalidLinkedResourceDiag(&attr.NameRange))
263+
} else {
264+
a.LinkedResources = []hcl.Traversal{traversal}
265+
}
266+
case addrs.ResourceInstance:
267+
if res.Resource.Mode != addrs.ManagedResourceMode {
268+
diags = append(diags, invalidLinkedResourceDiag(&attr.NameRange))
269+
} else {
270+
a.LinkedResources = []hcl.Traversal{traversal}
271+
}
272+
default:
273+
diags = append(diags, invalidLinkedResourceDiag(&attr.NameRange))
274+
}
234275
}
235276
}
236277

@@ -239,22 +280,34 @@ func decodeActionBlock(block *hcl.Block) (*Action, hcl.Diagnostics) {
239280
diags = append(diags, &hcl.Diagnostic{
240281
Severity: hcl.DiagError,
241282
Summary: `Invalid use of "linked_resources"`,
242-
Detail: `The "linked_resource" and "linked_resources" are mutually exclusive, only one should be used.`,
283+
Detail: `"linked_resource" and "linked_resources" are mutually exclusive, only one should be used.`,
243284
Subject: &attr.NameRange,
244285
})
245286
}
246287

247-
exprs, diags := hcl.ExprList(attr.Expr)
248-
lrs := make([]hcl.Traversal, len(exprs))
249-
250-
for i, expr := range exprs {
251-
traversal, travDiags := hcl.AbsTraversalForExpr(expr)
252-
diags = append(diags, travDiags...)
253-
if len(traversal) != 0 {
254-
lrs[i] = traversal
288+
exprs, exprDiags := hcl.ExprList(attr.Expr)
289+
diags = append(diags, exprDiags...)
290+
291+
if len(exprs) > 0 {
292+
lrs := make([]hcl.Traversal, 0, len(exprs))
293+
for _, expr := range exprs {
294+
traversal, travDiags := hcl.AbsTraversalForExpr(expr)
295+
diags = append(diags, travDiags...)
296+
297+
if len(traversal) != 0 {
298+
ref, refDiags := addrs.ParseRef(traversal)
299+
diags = append(diags, refDiags.ToHCL()...)
300+
301+
switch ref.Subject.(type) {
302+
case addrs.Resource, addrs.ResourceInstance:
303+
lrs = append(lrs, traversal)
304+
default:
305+
diags = append(diags, invalidLinkedResourcesDiag(&attr.NameRange))
306+
}
307+
}
255308
}
309+
a.LinkedResources = lrs
256310
}
257-
a.LinkedResources = lrs
258311
}
259312

260313
for _, block := range content.Blocks {

‎internal/configs/action_test.go

Lines changed: 173 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,20 @@ import (
88

99
"github.com/hashicorp/hcl/v2"
1010
"github.com/hashicorp/hcl/v2/hcltest"
11+
"github.com/zclconf/go-cty/cty"
1112
)
1213

1314
func TestDecodeActionBlock(t *testing.T) {
1415
fooResourceExpr := hcltest.MockExprTraversalSrc("resource_type.foo")
1516
barResourceExpr := hcltest.MockExprTraversalSrc("resource_type.bar")
16-
1717
fooAndBarExpr := hcltest.MockExprList([]hcl.Expression{fooResourceExpr, barResourceExpr})
18+
moduleResourceExpr := hcltest.MockExprTraversalSrc("module.foo.resource_type.bar")
19+
fooDataSourceExpr := hcltest.MockExprTraversalSrc("data.example.foo")
1820

1921
tests := map[string]struct {
2022
input *hcl.Block
2123
want *Action
22-
expectDiags []hcl.Diagnostic
24+
expectDiags []string
2325
}{
2426
"one linked resource": {
2527
&hcl.Block{
@@ -67,16 +69,181 @@ func TestDecodeActionBlock(t *testing.T) {
6769
},
6870
nil,
6971
},
72+
"invalid linked resources (module ref)": { // for now! This test will change when we support cross-module actions
73+
&hcl.Block{
74+
Type: "action",
75+
Labels: []string{"an_action", "foo"},
76+
Body: hcltest.MockBody(&hcl.BodyContent{
77+
Attributes: hcl.Attributes{
78+
"linked_resources": {
79+
Name: "linked_resources",
80+
Expr: hcltest.MockExprList([]hcl.Expression{moduleResourceExpr}),
81+
},
82+
},
83+
}),
84+
DefRange: blockRange,
85+
LabelRanges: []hcl.Range{{}},
86+
},
87+
&Action{
88+
Type: "an_action",
89+
Name: "foo",
90+
LinkedResources: []hcl.Traversal{},
91+
DeclRange: blockRange,
92+
},
93+
[]string{`:0,0-0: Invalid "linked_resources"; "linked_resources" must only refer to managed resources in the current module.`},
94+
},
95+
"invalid linked resource (datasource ref)": {
96+
&hcl.Block{
97+
Type: "action",
98+
Labels: []string{"an_action", "foo"},
99+
Body: hcltest.MockBody(&hcl.BodyContent{
100+
Attributes: hcl.Attributes{
101+
"linked_resource": {
102+
Name: "linked_resource",
103+
Expr: fooDataSourceExpr,
104+
},
105+
},
106+
}),
107+
DefRange: blockRange,
108+
LabelRanges: []hcl.Range{{}},
109+
},
110+
&Action{
111+
Type: "an_action",
112+
Name: "foo",
113+
LinkedResources: nil,
114+
DeclRange: blockRange,
115+
},
116+
[]string{`:0,0-0: Invalid "linked_resource"; "linked_resource" must only refer to a managed resource in the current module.`},
117+
},
70118
}
71119

72120
for name, test := range tests {
73121
t.Run(name, func(t *testing.T) {
74122
got, diags := decodeActionBlock(test.input)
75-
if len(diags) != len(test.expectDiags) {
76-
t.Error(diags.Error())
77-
t.Fatalf("Wrong result! Expected %d diagnostics, got %d", len(test.expectDiags), len(diags))
78-
}
123+
assertExactDiagnostics(t, diags, test.expectDiags)
124+
assertResultDeepEqual(t, got, test.want)
125+
})
126+
}
127+
}
128+
129+
func TestDecodeActionTriggerBlock(t *testing.T) {
130+
conditionExpr := hcltest.MockExprLiteral(cty.True)
131+
eventsListExpr := hcltest.MockExprList([]hcl.Expression{hcltest.MockExprTraversalSrc("after_create"), hcltest.MockExprTraversalSrc("after_update")})
132+
133+
fooActionExpr := hcltest.MockExprTraversalSrc("action.action_type.foo")
134+
barActionExpr := hcltest.MockExprTraversalSrc("action.action_type.bar")
135+
fooAndBarExpr := hcltest.MockExprList([]hcl.Expression{fooActionExpr, barActionExpr})
136+
137+
// bad inputs!
138+
moduleActionExpr := hcltest.MockExprTraversalSrc("module.foo.action.action_type.bar")
139+
fooDataSourceExpr := hcltest.MockExprTraversalSrc("data.example.foo")
140+
141+
tests := map[string]struct {
142+
input *hcl.Block
143+
want *ActionTrigger
144+
expectDiags []string
145+
}{
146+
"simple example": {
147+
&hcl.Block{
148+
Type: "action_trigger",
149+
Body: hcltest.MockBody(&hcl.BodyContent{
150+
Attributes: hcltest.MockAttrs(map[string]hcl.Expression{
151+
"condition": conditionExpr,
152+
"events": eventsListExpr,
153+
"actions": fooAndBarExpr,
154+
}),
155+
}),
156+
},
157+
&ActionTrigger{
158+
Condition: conditionExpr,
159+
Events: []ActionTriggerEvent{AfterCreate, AfterUpdate},
160+
Actions: []ActionRef{
161+
{
162+
mustAbsTraversalForExpr(fooActionExpr),
163+
fooActionExpr.Range(),
164+
},
165+
{
166+
mustAbsTraversalForExpr(barActionExpr),
167+
barActionExpr.Range(),
168+
},
169+
},
170+
},
171+
nil,
172+
},
173+
"error - referencing actions in other modules": {
174+
&hcl.Block{
175+
Type: "action_trigger",
176+
Body: hcltest.MockBody(&hcl.BodyContent{
177+
Attributes: hcltest.MockAttrs(map[string]hcl.Expression{
178+
"condition": conditionExpr,
179+
"events": eventsListExpr,
180+
"actions": hcltest.MockExprList([]hcl.Expression{moduleActionExpr}),
181+
}),
182+
}),
183+
},
184+
&ActionTrigger{
185+
Condition: conditionExpr,
186+
Events: []ActionTriggerEvent{AfterCreate, AfterUpdate},
187+
Actions: []ActionRef{},
188+
},
189+
[]string{
190+
"MockExprTraversal:0,0-33: Invalid actions argument inside action_triggers; action_triggers.actions accepts a list of one or more actions, which must be in the current module.",
191+
":0,0-0: No actions specified; At least one action must be specified for an action_trigger.",
192+
},
193+
},
194+
"error - action is not an action": {
195+
&hcl.Block{
196+
Type: "action_trigger",
197+
Body: hcltest.MockBody(&hcl.BodyContent{
198+
Attributes: hcltest.MockAttrs(map[string]hcl.Expression{
199+
"condition": conditionExpr,
200+
"events": eventsListExpr,
201+
"actions": hcltest.MockExprList([]hcl.Expression{fooDataSourceExpr}),
202+
}),
203+
}),
204+
},
205+
&ActionTrigger{
206+
Condition: conditionExpr,
207+
Events: []ActionTriggerEvent{AfterCreate, AfterUpdate},
208+
Actions: []ActionRef{},
209+
},
210+
[]string{
211+
"MockExprTraversal:0,0-16: Invalid actions argument inside action_triggers; action_triggers.actions accepts a list of one or more actions, which must be in the current module.",
212+
":0,0-0: No actions specified; At least one action must be specified for an action_trigger.",
213+
},
214+
},
215+
"error - invalid event": {
216+
&hcl.Block{
217+
Type: "action_trigger",
218+
Body: hcltest.MockBody(&hcl.BodyContent{
219+
Attributes: hcltest.MockAttrs(map[string]hcl.Expression{
220+
"condition": conditionExpr,
221+
"events": hcltest.MockExprList([]hcl.Expression{hcltest.MockExprTraversalSrc("not_an_event")}),
222+
"actions": hcltest.MockExprList([]hcl.Expression{fooActionExpr}),
223+
}),
224+
}),
225+
},
226+
&ActionTrigger{
227+
Condition: conditionExpr,
228+
Events: []ActionTriggerEvent{},
229+
Actions: []ActionRef{
230+
{
231+
mustAbsTraversalForExpr(fooActionExpr),
232+
fooActionExpr.Range(),
233+
},
234+
},
235+
},
236+
[]string{
237+
"MockExprTraversal:0,0-12: Invalid \"event\" value not_an_event; The \"event\" argument supports the following values: before_create, after_create, before_update, after_update, before_destroy, after_destroy.",
238+
":0,0-0: No events specified; At least one event must be specified for an action_trigger.",
239+
},
240+
},
241+
}
79242

243+
for name, test := range tests {
244+
t.Run(name, func(t *testing.T) {
245+
got, diags := decodeActionTriggerBlock(test.input)
246+
assertExactDiagnostics(t, diags, test.expectDiags)
80247
assertResultDeepEqual(t, got, test.want)
81248
})
82249
}

0 commit comments

Comments
 (0)