Skip to content

Commit 192e7cf

Browse files
kklimonda-clkklimonda-cl
committed
fix(terraform): skip validation of position attributes when not known at validation
Fixes PaloAltoNetworks/terraform-provider-panos#470
1 parent 961c089 commit 192e7cf

File tree

2 files changed

+317
-4
lines changed

2 files changed

+317
-4
lines changed

‎assets/terraform/internal/provider/position.go

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,47 @@ func (o *TerraformPositionObject) CopyToPango() movement.Position {
6969
func (o *TerraformPositionObject) ValidateConfig(resp *resource.ValidateConfigResponse) {
7070
allowedPositions := []string{"first", "last", "before", "after"}
7171

72-
if !slices.Contains(allowedPositions, o.Where.ValueString()) {
73-
resp.Diagnostics.AddAttributeWarning(
74-
path.Root("position").AtName("where"),
72+
var where string
73+
if !o.Where.IsUnknown() {
74+
where = o.Where.ValueString()
75+
if !slices.Contains(allowedPositions, o.Where.ValueString()) {
76+
resp.Diagnostics.AddAttributeError(
77+
path.Root("position").AtName("where"),
78+
"Missing attribute configuration",
79+
fmt.Sprintf("where attribute must be one of the valid values: first, last, before, after, found: '%s'", o.Where.ValueString()))
80+
return
81+
}
82+
}
83+
84+
// where is either a valid position, or an empty string at this point. If where position requires a valid pivot point, and
85+
// o.Pivot is known at this time, validate that o.Pivot is neither null nor an empty string.
86+
if (where == "after" || where == "before") && !o.Pivot.IsUnknown() && (o.Pivot.IsNull() || o.Pivot.ValueString() == "") {
87+
resp.Diagnostics.AddAttributeError(
88+
path.Root("position").AtName("pivot"),
7589
"Missing attribute configuration",
76-
fmt.Sprintf("where attribute must be one of the valid values: first, last, before, after, found: '%s'", o.Where.ValueString()))
90+
"position pivot attribute must be set to a valid object when where attribute is set to either 'after' or 'before'")
91+
return
92+
}
93+
94+
if where == "first" || where == "last" {
95+
if !o.Pivot.IsUnknown() && !o.Pivot.IsNull() {
96+
resp.Diagnostics.AddAttributeWarning(
97+
path.Root("position").AtName("pivot"),
98+
"Unexpected attribute configuration",
99+
"pivot attribute is ignored when where is set to 'first' or 'last'")
100+
}
101+
102+
if !o.Directly.IsUnknown() && !o.Directly.IsNull() {
103+
resp.Diagnostics.AddAttributeWarning(
104+
path.Root("position").AtName("directly"),
105+
"Unexpected attribute configuration",
106+
"directly attribute is ignored when where is set to 'first' or 'last'")
107+
}
108+
}
109+
110+
// If either pivot or direclty are unknown we can't validate that they are both set correcly.
111+
if o.Pivot.IsUnknown() || o.Directly.IsUnknown() {
112+
return
77113
}
78114

79115
if !o.Pivot.IsNull() && o.Directly.IsNull() {

‎assets/terraform/internal/provider/position_test.go

Lines changed: 277 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,277 @@
1+
package provider
2+
3+
import (
4+
"fmt"
5+
6+
. "github.com/onsi/ginkgo/v2"
7+
. "github.com/onsi/gomega"
8+
9+
"github.com/hashicorp/terraform-plugin-framework/diag"
10+
"github.com/hashicorp/terraform-plugin-framework/path"
11+
"github.com/hashicorp/terraform-plugin-framework/resource"
12+
"github.com/hashicorp/terraform-plugin-framework/types"
13+
)
14+
15+
var _ = Describe("Position", func() {
16+
var resp resource.ValidateConfigResponse
17+
JustBeforeEach(func() {
18+
resp.Diagnostics = diag.Diagnostics{}
19+
})
20+
21+
Context("when validating the 'where' attribute", func() {
22+
DescribeTable("allowed values",
23+
func(whereValue string, withPivot bool, expectError bool) {
24+
pivot := types.StringNull()
25+
directly := types.BoolNull()
26+
if withPivot {
27+
pivot = types.StringValue("mocked")
28+
directly = types.BoolValue(false)
29+
}
30+
31+
position := TerraformPositionObject{
32+
Where: types.StringValue(whereValue),
33+
Pivot: pivot,
34+
Directly: directly,
35+
}
36+
position.ValidateConfig(&resp)
37+
38+
if expectError {
39+
Expect(resp.Diagnostics.Errors()).To(HaveExactElements([]diag.Diagnostic{
40+
diag.NewAttributeErrorDiagnostic(
41+
path.Root("position").AtName("where"),
42+
"Missing attribute configuration",
43+
fmt.Sprintf("where attribute must be one of the valid values: first, last, before, after, found: '%s'", whereValue)),
44+
}))
45+
Expect(resp.Diagnostics.Warnings()).To(BeEmpty())
46+
} else {
47+
Expect(resp.Diagnostics.Errors()).To(BeEmpty())
48+
Expect(resp.Diagnostics.Warnings()).To(BeEmpty())
49+
}
50+
},
51+
Entry("should accept 'first'", "first", false, false),
52+
Entry("should accept 'last'", "last", false, false),
53+
Entry("should accept 'before'", "before", true, false),
54+
Entry("should accept 'after'", "after", true, false),
55+
Entry("should reject 'invalid'", "invalid", false, true),
56+
Entry("should reject empty string", "", false, true),
57+
)
58+
})
59+
Context("and where attribute is 'before' or 'after'", func() {
60+
Context("and pivot is null", func() {
61+
It("should return a diagnostic error about pivot not being set", func() {
62+
position := TerraformPositionObject{
63+
Where: types.StringValue("before"),
64+
Pivot: types.StringNull(),
65+
}
66+
position.ValidateConfig(&resp)
67+
Expect(resp.Diagnostics.Errors()).To(HaveExactElements([]diag.Diagnostic{
68+
diag.NewAttributeErrorDiagnostic(
69+
path.Root("position").AtName("pivot"),
70+
"Missing attribute configuration",
71+
"position pivot attribute must be set to a valid object when where attribute is set to either 'after' or 'before'"),
72+
}))
73+
Expect(resp.Diagnostics.Warnings()).To(HaveLen(0))
74+
75+
})
76+
})
77+
Context("and pivot is an empty string", func() {
78+
It("should return a diagnostic error about pivot not being set", func() {
79+
position := TerraformPositionObject{
80+
Where: types.StringValue("after"),
81+
Pivot: types.StringValue(""),
82+
}
83+
position.ValidateConfig(&resp)
84+
Expect(resp.Diagnostics.Errors()).To(HaveExactElements([]diag.Diagnostic{
85+
diag.NewAttributeErrorDiagnostic(
86+
path.Root("position").AtName("pivot"),
87+
"Missing attribute configuration",
88+
"position pivot attribute must be set to a valid object when where attribute is set to either 'after' or 'before'"),
89+
}))
90+
Expect(resp.Diagnostics.Warnings()).To(HaveLen(0))
91+
})
92+
})
93+
Context("and pivot is a valid string", func() {
94+
Context("and directly is null", func() {
95+
It("should return an error about directly not being configured properly", func() {
96+
position := TerraformPositionObject{
97+
Where: types.StringValue("after"),
98+
Pivot: types.StringValue("mocked"),
99+
Directly: types.BoolNull(),
100+
}
101+
position.ValidateConfig(&resp)
102+
Expect(resp.Diagnostics.Contains(diag.NewAttributeErrorDiagnostic(
103+
path.Root("position").AtName("directly"),
104+
"Missing attribute configuration",
105+
"Expected directly to be configured with pivot"),
106+
))
107+
Expect(resp.Diagnostics.Warnings()).To(HaveLen(0))
108+
})
109+
})
110+
})
111+
Context("and directly is set", func() {
112+
Context("and pivot is null", func() {
113+
It("should return an error about directly not being configured properly", func() {
114+
position := TerraformPositionObject{
115+
Where: types.StringValue("after"),
116+
Pivot: types.StringNull(),
117+
Directly: types.BoolValue(true),
118+
}
119+
position.ValidateConfig(&resp)
120+
Expect(resp.Diagnostics.Contains(diag.NewAttributeErrorDiagnostic(
121+
path.Root("position").AtName("pivot"),
122+
"Missing attribute configuration",
123+
"Expected pivot to be configured with directly"),
124+
))
125+
Expect(resp.Diagnostics.Warnings()).To(HaveLen(0))
126+
})
127+
})
128+
})
129+
})
130+
Context("when 'where' is 'first' or 'last' and 'pivot' is set", func() {
131+
DescribeTable("should return a warning about pivot being ignored",
132+
func(whereValue string) {
133+
position := TerraformPositionObject{
134+
Where: types.StringValue(whereValue),
135+
Pivot: types.StringValue("some-pivot"),
136+
Directly: types.BoolValue(false),
137+
}
138+
position.ValidateConfig(&resp)
139+
140+
Expect(resp.Diagnostics.Errors()).To(BeEmpty())
141+
Expect(resp.Diagnostics.Warnings()).To(HaveExactElements([]diag.Diagnostic{
142+
diag.NewAttributeWarningDiagnostic(
143+
path.Root("position").AtName("pivot"),
144+
"Unexpected attribute configuration",
145+
"pivot attribute is ignored when where is set to 'first' or 'last'"),
146+
diag.NewAttributeWarningDiagnostic(
147+
path.Root("position").AtName("directly"),
148+
"Unexpected attribute configuration",
149+
"directly attribute is ignored when where is set to 'first' or 'last'"),
150+
}))
151+
},
152+
Entry("when 'where' is 'first'", "first"),
153+
Entry("when 'where' is 'last'", "last"),
154+
)
155+
})
156+
Context("when 'where' is 'before' or 'after' and both 'pivot' and 'directly' are set", func() {
157+
DescribeTable("should not return any errors or warnings",
158+
func(whereValue string) {
159+
position := TerraformPositionObject{
160+
Where: types.StringValue(whereValue),
161+
Pivot: types.StringValue("some-pivot"),
162+
Directly: types.BoolValue(true),
163+
}
164+
position.ValidateConfig(&resp)
165+
166+
Expect(resp.Diagnostics.Errors()).To(BeEmpty())
167+
Expect(resp.Diagnostics.Warnings()).To(BeEmpty())
168+
},
169+
Entry("when 'where' is 'before'", "before"),
170+
Entry("when 'where' is 'after'", "after"),
171+
)
172+
})
173+
174+
Context("when 'where' is 'before' or 'after' and 'pivot' is set but 'directly' is not", func() {
175+
DescribeTable("should return an error about directly not being configured",
176+
func(whereValue string) {
177+
position := TerraformPositionObject{
178+
Where: types.StringValue(whereValue),
179+
Pivot: types.StringValue("some-pivot"),
180+
Directly: types.BoolNull(),
181+
}
182+
position.ValidateConfig(&resp)
183+
184+
Expect(resp.Diagnostics.Errors()).To(HaveExactElements([]diag.Diagnostic{
185+
diag.NewAttributeErrorDiagnostic(
186+
path.Root("position").AtName("directly"),
187+
"Missing attribute configuration",
188+
"Expected directly to be configured with pivot"),
189+
}))
190+
Expect(resp.Diagnostics.Warnings()).To(BeEmpty())
191+
},
192+
Entry("when 'where' is 'before'", "before"),
193+
Entry("when 'where' is 'after'", "after"),
194+
)
195+
})
196+
197+
Context("when 'where' is 'before' or 'after' and 'directly' is set but 'pivot' is not", func() {
198+
DescribeTable("should return an error about pivot not being configured",
199+
func(whereValue string) {
200+
position := TerraformPositionObject{
201+
Where: types.StringValue(whereValue),
202+
Pivot: types.StringNull(),
203+
Directly: types.BoolValue(true),
204+
}
205+
position.ValidateConfig(&resp)
206+
207+
Expect(resp.Diagnostics.Errors()).To(HaveExactElements([]diag.Diagnostic{
208+
diag.NewAttributeErrorDiagnostic(
209+
path.Root("position").AtName("pivot"),
210+
"Missing attribute configuration",
211+
"position pivot attribute must be set to a valid object when where attribute is set to either 'after' or 'before'"),
212+
}))
213+
Expect(resp.Diagnostics.Warnings()).To(BeEmpty())
214+
},
215+
Entry("when 'where' is 'before'", "before"),
216+
Entry("when 'where' is 'after'", "after"),
217+
)
218+
})
219+
220+
Context("when attributes are set to unknown", func() {
221+
Context("when 'where' is unknown", func() {
222+
It("should not return any errors or warnings", func() {
223+
position := TerraformPositionObject{
224+
Where: types.StringUnknown(),
225+
Pivot: types.StringNull(),
226+
Directly: types.BoolNull(),
227+
}
228+
position.ValidateConfig(&resp)
229+
230+
Expect(resp.Diagnostics.Errors()).To(BeEmpty())
231+
Expect(resp.Diagnostics.Warnings()).To(BeEmpty())
232+
})
233+
})
234+
235+
Context("when 'pivot' is unknown", func() {
236+
It("should not validate pivot-related conditions", func() {
237+
position := TerraformPositionObject{
238+
Where: types.StringValue("before"),
239+
Pivot: types.StringUnknown(),
240+
Directly: types.BoolValue(true),
241+
}
242+
position.ValidateConfig(&resp)
243+
244+
Expect(resp.Diagnostics.Errors()).To(BeEmpty())
245+
Expect(resp.Diagnostics.Warnings()).To(BeEmpty())
246+
})
247+
})
248+
249+
Context("when 'directly' is unknown", func() {
250+
It("should not validate directly-related conditions", func() {
251+
position := TerraformPositionObject{
252+
Where: types.StringValue("after"),
253+
Pivot: types.StringValue("some-pivot"),
254+
Directly: types.BoolUnknown(),
255+
}
256+
position.ValidateConfig(&resp)
257+
258+
Expect(resp.Diagnostics.Errors()).To(BeEmpty())
259+
Expect(resp.Diagnostics.Warnings()).To(BeEmpty())
260+
})
261+
})
262+
263+
Context("when both 'pivot' and 'directly' are unknown", func() {
264+
It("should not validate pivot and directly related conditions", func() {
265+
position := TerraformPositionObject{
266+
Where: types.StringValue("before"),
267+
Pivot: types.StringUnknown(),
268+
Directly: types.BoolUnknown(),
269+
}
270+
position.ValidateConfig(&resp)
271+
272+
Expect(resp.Diagnostics.Errors()).To(BeEmpty())
273+
Expect(resp.Diagnostics.Warnings()).To(BeEmpty())
274+
})
275+
})
276+
})
277+
})

0 commit comments

Comments
 (0)