Skip to content

Commit a8189f1

Browse files
austinvalleaustinvalle
authored
function: Switch the representation of variadic arguments to types.Tuple (#923)
* switch to tuples and add reflection support * update docs * add names to variadic param names * update doc note * update docs * update docs * add doc note * add changelog * switch changelog to breaking change * update to tuple index * update comment wording * update doc * add new maintainer note * remove top part of comment * fixed the odd spacing in the comment :)
1 parent bcbb70b commit a8189f1

18 files changed

+594
-97
lines changed

Diff for: ‎.changes/unreleased/BREAKING CHANGES-20240216-142501.yaml

+8
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
kind: BREAKING CHANGES
2+
body: 'function: Changed the framework type for variadic parameters to `types.TupleType`,
3+
where each element is the same element type. Provider-defined functions using a
4+
`types.List` for retrieving variadic argument data will need to update their code
5+
to use `types.Tuple`.'
6+
time: 2024-02-16T14:25:01.729428-05:00
7+
custom:
8+
Issue: "923"

Diff for: ‎function/arguments_data.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,9 @@ func (d ArgumentsData) Equal(o ArgumentsData) bool {
4545
// Each target type must be acceptable for the data type in the parameter
4646
// definition.
4747
//
48-
// Variadic parameter argument data must be consumed by a types.List or Go slice
48+
// Variadic parameter argument data must be consumed by a types.Tuple or Go slice
4949
// type with an element type appropriate for the parameter definition ([]T). The
50-
// framework automatically populates this list with elements matching the zero,
50+
// framework automatically populates this tuple with elements matching the zero,
5151
// one, or more arguments passed.
5252
func (d ArgumentsData) Get(ctx context.Context, targets ...any) diag.Diagnostics {
5353
var diags diag.Diagnostics
@@ -112,10 +112,10 @@ func (d ArgumentsData) Get(ctx context.Context, targets ...any) diag.Diagnostics
112112
// position and populates the target with the value. The target type must be
113113
// acceptable for the data type in the parameter definition.
114114
//
115-
// Variadic parameter argument data must be consumed by a types.List or Go slice
115+
// Variadic parameter argument data must be consumed by a types.Tuple or Go slice
116116
// type with an element type appropriate for the parameter definition ([]T) at
117117
// the position after all parameters. The framework automatically populates this
118-
// list with elements matching the zero, one, or more arguments passed.
118+
// tuple with elements matching the zero, one, or more arguments passed.
119119
func (d ArgumentsData) GetArgument(ctx context.Context, position int, target any) diag.Diagnostics {
120120
var diags diag.Diagnostics
121121

@@ -134,8 +134,8 @@ func (d ArgumentsData) GetArgument(ctx context.Context, position int, target any
134134
diags.AddError(
135135
"Invalid Argument Data Position",
136136
"When attempting to fetch argument data during the function call, the provider code attempted to read a non-existent argument position. "+
137-
"Function argument positions are 0-based and any final variadic parameter is represented as one argument position with an ordered list of the parameter data type. "+
138-
"This is always an error in the provider code and should be reported to the provider developers.\n\n"+
137+
"Function argument positions are 0-based and any final variadic parameter is represented as one argument position with a tuple where each element "+
138+
"type matches the parameter data type. This is always an error in the provider code and should be reported to the provider developers.\n\n"+
139139
fmt.Sprintf("Given argument position: %d, last argument position: %d", position, len(d.values)-1),
140140
)
141141

Diff for: ‎function/arguments_data_test.go

+125-2
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,46 @@ func TestArgumentsDataGet(t *testing.T) {
173173
pointer(attr.Value(basetypes.NewStringValue("test"))),
174174
},
175175
},
176+
"attr-value-variadic": {
177+
argumentsData: function.NewArgumentsData([]attr.Value{
178+
basetypes.NewBoolNull(),
179+
basetypes.NewInt64Unknown(),
180+
basetypes.NewStringValue("test"),
181+
basetypes.NewTupleValueMust(
182+
[]attr.Type{
183+
basetypes.StringType{},
184+
basetypes.StringType{},
185+
},
186+
[]attr.Value{
187+
basetypes.NewStringValue("test1"),
188+
basetypes.NewStringValue("test2"),
189+
},
190+
),
191+
}),
192+
targets: []any{
193+
new(attr.Value),
194+
new(attr.Value),
195+
new(attr.Value),
196+
new(attr.Value),
197+
},
198+
expected: []any{
199+
pointer(attr.Value(basetypes.NewBoolNull())),
200+
pointer(attr.Value(basetypes.NewInt64Unknown())),
201+
pointer(attr.Value(basetypes.NewStringValue("test"))),
202+
pointer(attr.Value(
203+
basetypes.NewTupleValueMust(
204+
[]attr.Type{
205+
basetypes.StringType{},
206+
basetypes.StringType{},
207+
},
208+
[]attr.Value{
209+
basetypes.NewStringValue("test1"),
210+
basetypes.NewStringValue("test2"),
211+
},
212+
),
213+
)),
214+
},
215+
},
176216
"framework-type": {
177217
argumentsData: function.NewArgumentsData([]attr.Value{
178218
basetypes.NewBoolNull(),
@@ -190,6 +230,46 @@ func TestArgumentsDataGet(t *testing.T) {
190230
pointer(basetypes.NewStringValue("test")),
191231
},
192232
},
233+
"framework-type-variadic": {
234+
argumentsData: function.NewArgumentsData([]attr.Value{
235+
basetypes.NewBoolNull(),
236+
basetypes.NewInt64Unknown(),
237+
basetypes.NewStringValue("test"),
238+
basetypes.NewTupleValueMust(
239+
[]attr.Type{
240+
basetypes.StringType{},
241+
basetypes.StringType{},
242+
},
243+
[]attr.Value{
244+
basetypes.NewStringValue("test1"),
245+
basetypes.NewStringValue("test2"),
246+
},
247+
),
248+
}),
249+
targets: []any{
250+
new(basetypes.BoolValue),
251+
new(basetypes.Int64Value),
252+
new(basetypes.StringValue),
253+
new(basetypes.TupleValue),
254+
},
255+
expected: []any{
256+
pointer(basetypes.NewBoolNull()),
257+
pointer(basetypes.NewInt64Unknown()),
258+
pointer(basetypes.NewStringValue("test")),
259+
pointer(
260+
basetypes.NewTupleValueMust(
261+
[]attr.Type{
262+
basetypes.StringType{},
263+
basetypes.StringType{},
264+
},
265+
[]attr.Value{
266+
basetypes.NewStringValue("test1"),
267+
basetypes.NewStringValue("test2"),
268+
},
269+
),
270+
),
271+
},
272+
},
193273
"reflection": {
194274
argumentsData: function.NewArgumentsData([]attr.Value{
195275
basetypes.NewBoolNull(),
@@ -204,6 +284,46 @@ func TestArgumentsDataGet(t *testing.T) {
204284
pointer("test"),
205285
},
206286
},
287+
"reflection-variadic": {
288+
argumentsData: function.NewArgumentsData([]attr.Value{
289+
basetypes.NewBoolNull(),
290+
basetypes.NewTupleValueMust(
291+
[]attr.Type{
292+
basetypes.StringType{},
293+
basetypes.StringType{},
294+
},
295+
[]attr.Value{
296+
basetypes.NewStringValue("test1"),
297+
basetypes.NewStringValue("test2"),
298+
},
299+
),
300+
}),
301+
targets: []any{
302+
new(*bool),
303+
new([]string),
304+
},
305+
expected: []any{
306+
pointer((*bool)(nil)),
307+
pointer([]string{
308+
"test1",
309+
"test2",
310+
}),
311+
},
312+
},
313+
"reflection-variadic-empty": {
314+
argumentsData: function.NewArgumentsData([]attr.Value{
315+
basetypes.NewBoolNull(),
316+
basetypes.NewTupleValueMust([]attr.Type{}, []attr.Value{}),
317+
}),
318+
targets: []any{
319+
new(*bool),
320+
new([]string),
321+
},
322+
expected: []any{
323+
pointer((*bool)(nil)),
324+
pointer([]string{}),
325+
},
326+
},
207327
}
208328

209329
for name, testCase := range testCases {
@@ -225,6 +345,9 @@ func TestArgumentsDataGet(t *testing.T) {
225345
cmp.Transformer("StringValue", func(v *basetypes.StringValue) basetypes.StringValue {
226346
return *v
227347
}),
348+
cmp.Transformer("TupleValue", func(v *basetypes.TupleValue) basetypes.TupleValue {
349+
return *v
350+
}),
228351
}
229352

230353
if diff := cmp.Diff(testCase.targets, testCase.expected, options...); diff != "" {
@@ -273,8 +396,8 @@ func TestArgumentsDataGetArgument(t *testing.T) {
273396
diag.NewErrorDiagnostic(
274397
"Invalid Argument Data Position",
275398
"When attempting to fetch argument data during the function call, the provider code attempted to read a non-existent argument position. "+
276-
"Function argument positions are 0-based and any final variadic parameter is represented as one argument position with an ordered list of the parameter data type. "+
277-
"This is always an error in the provider code and should be reported to the provider developers.\n\n"+
399+
"Function argument positions are 0-based and any final variadic parameter is represented as one argument position with a tuple where each element "+
400+
"type matches the parameter data type. This is always an error in the provider code and should be reported to the provider developers.\n\n"+
278401
"Given argument position: 1, last argument position: 0",
279402
),
280403
},

Diff for: ‎function/definition.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ type Definition struct {
2121

2222
// VariadicParameter is an optional final parameter which can accept zero or
2323
// more arguments when the function is called. The argument data is sent as
24-
// an ordered list of the associated data type.
24+
// a tuple, where all elements are of the same associated data type.
2525
VariadicParameter Parameter
2626

2727
// Return is the function call response data type.

Diff for: ‎internal/fromproto5/arguments_data.go

+22-2
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def
5151
return function.NewArgumentsData(nil), nil
5252
}
5353

54-
// Variadic values are collected as a separate list to ease developer usage.
54+
// Variadic values are collected as a separate tuple to ease developer usage.
5555
argumentValues := make([]attr.Value, 0, len(definition.Parameters))
5656
variadicValues := make([]attr.Value, 0, len(arguments)-len(definition.Parameters))
5757
var diags diag.Diagnostics
@@ -133,7 +133,27 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def
133133
}
134134

135135
if definition.VariadicParameter != nil {
136-
variadicValue, variadicValueDiags := basetypes.NewListValue(definition.VariadicParameter.GetType(), variadicValues)
136+
// MAINTAINER NOTE: Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter
137+
// type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a
138+
// framework Tuple where each element type of the tuple matches the variadic parameter type.
139+
//
140+
// Previously, this logic utilized a framework List with an element type that matched the variadic parameter type. Using a List presented an issue with dynamic
141+
// variadic parameters, as each argument was allowed to be any type "individually", rather than having a single type constraint applied to all dynamic elements,
142+
// like a cty.List in Terraform. This eventually results in an error attempting to create a tftypes.List with multiple element types (when unwrapping from a framework
143+
// dynamic to a tftypes concrete value).
144+
//
145+
// While a framework List type can handle multiple dynamic values of different types (due to it's wrapping of dynamic values), `terraform-plugin-go` and `tftypes.List` cannot.
146+
// Currently, the logic for retrieving argument data is dependent on the tftypes package to utilize the framework reflection logic, requiring us to apply a type constraint
147+
// that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple.
148+
variadicType := definition.VariadicParameter.GetType()
149+
tupleTypes := make([]attr.Type, len(variadicValues))
150+
tupleValues := make([]attr.Value, len(variadicValues))
151+
for i, val := range variadicValues {
152+
tupleTypes[i] = variadicType
153+
tupleValues[i] = val
154+
}
155+
156+
variadicValue, variadicValueDiags := basetypes.NewTupleValue(tupleTypes, tupleValues)
137157

138158
diags.Append(variadicValueDiags...)
139159

Diff for: ‎internal/fromproto5/arguments_data_test.go

+37-20
Original file line numberDiff line numberDiff line change
@@ -183,8 +183,8 @@ func TestArgumentsData(t *testing.T) {
183183
},
184184
expected: function.NewArgumentsData([]attr.Value{
185185
basetypes.NewBoolValue(true),
186-
basetypes.NewListValueMust(
187-
basetypes.StringType{},
186+
basetypes.NewTupleValueMust(
187+
[]attr.Type{},
188188
[]attr.Value{},
189189
),
190190
}),
@@ -202,8 +202,10 @@ func TestArgumentsData(t *testing.T) {
202202
},
203203
expected: function.NewArgumentsData([]attr.Value{
204204
basetypes.NewBoolValue(true),
205-
basetypes.NewListValueMust(
206-
basetypes.StringType{},
205+
basetypes.NewTupleValueMust(
206+
[]attr.Type{
207+
basetypes.StringType{},
208+
},
207209
[]attr.Value{
208210
basetypes.NewStringValue("varg-arg1"),
209211
},
@@ -224,8 +226,11 @@ func TestArgumentsData(t *testing.T) {
224226
},
225227
expected: function.NewArgumentsData([]attr.Value{
226228
basetypes.NewBoolValue(true),
227-
basetypes.NewListValueMust(
228-
basetypes.StringType{},
229+
basetypes.NewTupleValueMust(
230+
[]attr.Type{
231+
basetypes.StringType{},
232+
basetypes.StringType{},
233+
},
229234
[]attr.Value{
230235
basetypes.NewStringValue("varg-arg1"),
231236
basetypes.NewStringValue("varg-arg2"),
@@ -264,8 +269,8 @@ func TestArgumentsData(t *testing.T) {
264269
expected: function.NewArgumentsData([]attr.Value{
265270
basetypes.NewBoolValue(true),
266271
basetypes.NewBoolValue(false),
267-
basetypes.NewListValueMust(
268-
basetypes.StringType{},
272+
basetypes.NewTupleValueMust(
273+
[]attr.Type{},
269274
[]attr.Value{},
270275
),
271276
}),
@@ -286,8 +291,10 @@ func TestArgumentsData(t *testing.T) {
286291
expected: function.NewArgumentsData([]attr.Value{
287292
basetypes.NewBoolValue(true),
288293
basetypes.NewBoolValue(false),
289-
basetypes.NewListValueMust(
290-
basetypes.StringType{},
294+
basetypes.NewTupleValueMust(
295+
[]attr.Type{
296+
basetypes.StringType{},
297+
},
291298
[]attr.Value{
292299
basetypes.NewStringValue("varg-arg2"),
293300
},
@@ -311,8 +318,11 @@ func TestArgumentsData(t *testing.T) {
311318
expected: function.NewArgumentsData([]attr.Value{
312319
basetypes.NewBoolValue(true),
313320
basetypes.NewBoolValue(false),
314-
basetypes.NewListValueMust(
315-
basetypes.StringType{},
321+
basetypes.NewTupleValueMust(
322+
[]attr.Type{
323+
basetypes.StringType{},
324+
basetypes.StringType{},
325+
},
316326
[]attr.Value{
317327
basetypes.NewStringValue("varg-arg2"),
318328
basetypes.NewStringValue("varg-arg3"),
@@ -326,8 +336,8 @@ func TestArgumentsData(t *testing.T) {
326336
VariadicParameter: function.StringParameter{},
327337
},
328338
expected: function.NewArgumentsData([]attr.Value{
329-
basetypes.NewListValueMust(
330-
basetypes.StringType{},
339+
basetypes.NewTupleValueMust(
340+
[]attr.Type{},
331341
[]attr.Value{},
332342
),
333343
}),
@@ -340,8 +350,10 @@ func TestArgumentsData(t *testing.T) {
340350
VariadicParameter: function.StringParameter{},
341351
},
342352
expected: function.NewArgumentsData([]attr.Value{
343-
basetypes.NewListValueMust(
344-
basetypes.StringType{},
353+
basetypes.NewTupleValueMust(
354+
[]attr.Type{
355+
basetypes.StringType{},
356+
},
345357
[]attr.Value{
346358
basetypes.NewStringValue("varg-arg0"),
347359
},
@@ -358,8 +370,10 @@ func TestArgumentsData(t *testing.T) {
358370
},
359371
},
360372
expected: function.NewArgumentsData([]attr.Value{
361-
basetypes.NewListValueMust(
362-
testtypes.StringType{},
373+
basetypes.NewTupleValueMust(
374+
[]attr.Type{
375+
testtypes.StringType{},
376+
},
363377
[]attr.Value{
364378
testtypes.String{
365379
CreatedBy: testtypes.StringType{},
@@ -378,8 +392,11 @@ func TestArgumentsData(t *testing.T) {
378392
VariadicParameter: function.StringParameter{},
379393
},
380394
expected: function.NewArgumentsData([]attr.Value{
381-
basetypes.NewListValueMust(
382-
basetypes.StringType{},
395+
basetypes.NewTupleValueMust(
396+
[]attr.Type{
397+
basetypes.StringType{},
398+
basetypes.StringType{},
399+
},
383400
[]attr.Value{
384401
basetypes.NewStringValue("varg-arg0"),
385402
basetypes.NewStringValue("varg-arg1"),

0 commit comments

Comments
 (0)