diff --git a/.changes/unreleased/BREAKING CHANGES-20240216-142501.yaml b/.changes/unreleased/BREAKING CHANGES-20240216-142501.yaml new file mode 100644 index 000000000..4fa7c3043 --- /dev/null +++ b/.changes/unreleased/BREAKING CHANGES-20240216-142501.yaml @@ -0,0 +1,8 @@ +kind: BREAKING CHANGES +body: 'function: Changed the framework type for variadic parameters to `types.TupleType`, + where each element is the same element type. Provider-defined functions using a + `types.List` for retrieving variadic argument data will need to update their code + to use `types.Tuple`.' +time: 2024-02-16T14:25:01.729428-05:00 +custom: + Issue: "923" diff --git a/function/arguments_data.go b/function/arguments_data.go index 9b75eaf05..7c6e36321 100644 --- a/function/arguments_data.go +++ b/function/arguments_data.go @@ -45,9 +45,9 @@ func (d ArgumentsData) Equal(o ArgumentsData) bool { // Each target type must be acceptable for the data type in the parameter // definition. // -// Variadic parameter argument data must be consumed by a types.List or Go slice +// Variadic parameter argument data must be consumed by a types.Tuple or Go slice // type with an element type appropriate for the parameter definition ([]T). The -// framework automatically populates this list with elements matching the zero, +// framework automatically populates this tuple with elements matching the zero, // one, or more arguments passed. func (d ArgumentsData) Get(ctx context.Context, targets ...any) diag.Diagnostics { var diags diag.Diagnostics @@ -112,10 +112,10 @@ func (d ArgumentsData) Get(ctx context.Context, targets ...any) diag.Diagnostics // position and populates the target with the value. The target type must be // acceptable for the data type in the parameter definition. // -// Variadic parameter argument data must be consumed by a types.List or Go slice +// Variadic parameter argument data must be consumed by a types.Tuple or Go slice // type with an element type appropriate for the parameter definition ([]T) at // the position after all parameters. The framework automatically populates this -// list with elements matching the zero, one, or more arguments passed. +// tuple with elements matching the zero, one, or more arguments passed. func (d ArgumentsData) GetArgument(ctx context.Context, position int, target any) diag.Diagnostics { var diags diag.Diagnostics @@ -134,8 +134,8 @@ func (d ArgumentsData) GetArgument(ctx context.Context, position int, target any diags.AddError( "Invalid Argument Data Position", "When attempting to fetch argument data during the function call, the provider code attempted to read a non-existent argument position. "+ - "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. "+ - "This is always an error in the provider code and should be reported to the provider developers.\n\n"+ + "Function argument positions are 0-based and any final variadic parameter is represented as one argument position with a tuple where each element "+ + "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"+ fmt.Sprintf("Given argument position: %d, last argument position: %d", position, len(d.values)-1), ) diff --git a/function/arguments_data_test.go b/function/arguments_data_test.go index bb9718118..01ab7b29b 100644 --- a/function/arguments_data_test.go +++ b/function/arguments_data_test.go @@ -173,6 +173,46 @@ func TestArgumentsDataGet(t *testing.T) { pointer(attr.Value(basetypes.NewStringValue("test"))), }, }, + "attr-value-variadic": { + argumentsData: function.NewArgumentsData([]attr.Value{ + basetypes.NewBoolNull(), + basetypes.NewInt64Unknown(), + basetypes.NewStringValue("test"), + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + }), + targets: []any{ + new(attr.Value), + new(attr.Value), + new(attr.Value), + new(attr.Value), + }, + expected: []any{ + pointer(attr.Value(basetypes.NewBoolNull())), + pointer(attr.Value(basetypes.NewInt64Unknown())), + pointer(attr.Value(basetypes.NewStringValue("test"))), + pointer(attr.Value( + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + )), + }, + }, "framework-type": { argumentsData: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolNull(), @@ -190,6 +230,46 @@ func TestArgumentsDataGet(t *testing.T) { pointer(basetypes.NewStringValue("test")), }, }, + "framework-type-variadic": { + argumentsData: function.NewArgumentsData([]attr.Value{ + basetypes.NewBoolNull(), + basetypes.NewInt64Unknown(), + basetypes.NewStringValue("test"), + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + }), + targets: []any{ + new(basetypes.BoolValue), + new(basetypes.Int64Value), + new(basetypes.StringValue), + new(basetypes.TupleValue), + }, + expected: []any{ + pointer(basetypes.NewBoolNull()), + pointer(basetypes.NewInt64Unknown()), + pointer(basetypes.NewStringValue("test")), + pointer( + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + ), + }, + }, "reflection": { argumentsData: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolNull(), @@ -204,6 +284,46 @@ func TestArgumentsDataGet(t *testing.T) { pointer("test"), }, }, + "reflection-variadic": { + argumentsData: function.NewArgumentsData([]attr.Value{ + basetypes.NewBoolNull(), + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, + []attr.Value{ + basetypes.NewStringValue("test1"), + basetypes.NewStringValue("test2"), + }, + ), + }), + targets: []any{ + new(*bool), + new([]string), + }, + expected: []any{ + pointer((*bool)(nil)), + pointer([]string{ + "test1", + "test2", + }), + }, + }, + "reflection-variadic-empty": { + argumentsData: function.NewArgumentsData([]attr.Value{ + basetypes.NewBoolNull(), + basetypes.NewTupleValueMust([]attr.Type{}, []attr.Value{}), + }), + targets: []any{ + new(*bool), + new([]string), + }, + expected: []any{ + pointer((*bool)(nil)), + pointer([]string{}), + }, + }, } for name, testCase := range testCases { @@ -225,6 +345,9 @@ func TestArgumentsDataGet(t *testing.T) { cmp.Transformer("StringValue", func(v *basetypes.StringValue) basetypes.StringValue { return *v }), + cmp.Transformer("TupleValue", func(v *basetypes.TupleValue) basetypes.TupleValue { + return *v + }), } if diff := cmp.Diff(testCase.targets, testCase.expected, options...); diff != "" { @@ -273,8 +396,8 @@ func TestArgumentsDataGetArgument(t *testing.T) { diag.NewErrorDiagnostic( "Invalid Argument Data Position", "When attempting to fetch argument data during the function call, the provider code attempted to read a non-existent argument position. "+ - "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. "+ - "This is always an error in the provider code and should be reported to the provider developers.\n\n"+ + "Function argument positions are 0-based and any final variadic parameter is represented as one argument position with a tuple where each element "+ + "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"+ "Given argument position: 1, last argument position: 0", ), }, diff --git a/function/definition.go b/function/definition.go index af14b2bfa..b95ee29bc 100644 --- a/function/definition.go +++ b/function/definition.go @@ -21,7 +21,7 @@ type Definition struct { // VariadicParameter is an optional final parameter which can accept zero or // more arguments when the function is called. The argument data is sent as - // an ordered list of the associated data type. + // a tuple, where all elements are of the same associated data type. VariadicParameter Parameter // Return is the function call response data type. diff --git a/internal/fromproto5/arguments_data.go b/internal/fromproto5/arguments_data.go index 57c43c6b0..3ce46cffd 100644 --- a/internal/fromproto5/arguments_data.go +++ b/internal/fromproto5/arguments_data.go @@ -51,7 +51,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def return function.NewArgumentsData(nil), nil } - // Variadic values are collected as a separate list to ease developer usage. + // Variadic values are collected as a separate tuple to ease developer usage. argumentValues := make([]attr.Value, 0, len(definition.Parameters)) variadicValues := make([]attr.Value, 0, len(arguments)-len(definition.Parameters)) var diags diag.Diagnostics @@ -133,7 +133,27 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov5.DynamicValue, def } if definition.VariadicParameter != nil { - variadicValue, variadicValueDiags := basetypes.NewListValue(definition.VariadicParameter.GetType(), variadicValues) + // MAINTAINER NOTE: Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter + // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a + // framework Tuple where each element type of the tuple matches the variadic parameter type. + // + // 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 + // variadic parameters, as each argument was allowed to be any type "individually", rather than having a single type constraint applied to all dynamic elements, + // 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 + // dynamic to a tftypes concrete value). + // + // 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. + // 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 + // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. + variadicType := definition.VariadicParameter.GetType() + tupleTypes := make([]attr.Type, len(variadicValues)) + tupleValues := make([]attr.Value, len(variadicValues)) + for i, val := range variadicValues { + tupleTypes[i] = variadicType + tupleValues[i] = val + } + + variadicValue, variadicValueDiags := basetypes.NewTupleValue(tupleTypes, tupleValues) diags.Append(variadicValueDiags...) diff --git a/internal/fromproto5/arguments_data_test.go b/internal/fromproto5/arguments_data_test.go index 73b45b7db..5dba9b16f 100644 --- a/internal/fromproto5/arguments_data_test.go +++ b/internal/fromproto5/arguments_data_test.go @@ -183,8 +183,8 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -202,8 +202,10 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), }, @@ -224,8 +226,11 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), basetypes.NewStringValue("varg-arg2"), @@ -264,8 +269,8 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -286,8 +291,10 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg2"), }, @@ -311,8 +318,11 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg2"), basetypes.NewStringValue("varg-arg3"), @@ -326,8 +336,8 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -340,8 +350,10 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg0"), }, @@ -358,8 +370,10 @@ func TestArgumentsData(t *testing.T) { }, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - testtypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + testtypes.StringType{}, + }, []attr.Value{ testtypes.String{ CreatedBy: testtypes.StringType{}, @@ -378,8 +392,11 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg0"), basetypes.NewStringValue("varg-arg1"), diff --git a/internal/fromproto6/arguments_data.go b/internal/fromproto6/arguments_data.go index a41b1ce7d..4dd6db7bc 100644 --- a/internal/fromproto6/arguments_data.go +++ b/internal/fromproto6/arguments_data.go @@ -51,7 +51,7 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def return function.NewArgumentsData(nil), nil } - // Variadic values are collected as a separate list to ease developer usage. + // Variadic values are collected as a separate tuple to ease developer usage. argumentValues := make([]attr.Value, 0, len(definition.Parameters)) variadicValues := make([]attr.Value, 0, len(arguments)-len(definition.Parameters)) var diags diag.Diagnostics @@ -133,7 +133,26 @@ func ArgumentsData(ctx context.Context, arguments []*tfprotov6.DynamicValue, def } if definition.VariadicParameter != nil { - variadicValue, variadicValueDiags := basetypes.NewListValue(definition.VariadicParameter.GetType(), variadicValues) + // MAINTAINER NOTE: Variadic parameters are represented as individual arguments in the CallFunction RPC and Terraform core applies the variadic parameter + // type constraint to each argument individually. For developer convenience, the framework logic below, groups the variadic arguments into a + // framework Tuple where each element type of the tuple matches the variadic parameter type. + // + // 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 + // variadic parameters, as each argument was allowed to be any type "individually", rather than having a single type constraint applied to all dynamic elements, + // 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 + // dynamic to a tftypes concrete value). + // + // 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. + // 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 + // that is valid in Terraform core and `terraform-plugin-go`, which we are doing here with a Tuple. + variadicType := definition.VariadicParameter.GetType() + tupleTypes := make([]attr.Type, len(variadicValues)) + tupleValues := make([]attr.Value, len(variadicValues)) + for i, val := range variadicValues { + tupleTypes[i] = variadicType + tupleValues[i] = val + } + variadicValue, variadicValueDiags := basetypes.NewTupleValue(tupleTypes, tupleValues) diags.Append(variadicValueDiags...) diff --git a/internal/fromproto6/arguments_data_test.go b/internal/fromproto6/arguments_data_test.go index c88f97b49..a6ff6ed04 100644 --- a/internal/fromproto6/arguments_data_test.go +++ b/internal/fromproto6/arguments_data_test.go @@ -183,8 +183,8 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -202,8 +202,10 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), }, @@ -224,8 +226,11 @@ func TestArgumentsData(t *testing.T) { }, expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), basetypes.NewStringValue("varg-arg2"), @@ -264,8 +269,8 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -286,8 +291,10 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg2"), }, @@ -311,8 +318,11 @@ func TestArgumentsData(t *testing.T) { expected: function.NewArgumentsData([]attr.Value{ basetypes.NewBoolValue(true), basetypes.NewBoolValue(false), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg2"), basetypes.NewStringValue("varg-arg3"), @@ -326,8 +336,8 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{}, []attr.Value{}, ), }), @@ -340,8 +350,10 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg0"), }, @@ -358,8 +370,10 @@ func TestArgumentsData(t *testing.T) { }, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - testtypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + testtypes.StringType{}, + }, []attr.Value{ testtypes.String{ CreatedBy: testtypes.StringType{}, @@ -378,8 +392,11 @@ func TestArgumentsData(t *testing.T) { VariadicParameter: function.StringParameter{}, }, expected: function.NewArgumentsData([]attr.Value{ - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg0"), basetypes.NewStringValue("varg-arg1"), diff --git a/internal/fwserver/server_callfunction_test.go b/internal/fwserver/server_callfunction_test.go index c56d371f1..5564f74cb 100644 --- a/internal/fwserver/server_callfunction_test.go +++ b/internal/fwserver/server_callfunction_test.go @@ -138,8 +138,11 @@ func TestServerCallFunction(t *testing.T) { request: &fwserver.CallFunctionRequest{ Arguments: function.NewArgumentsData([]attr.Value{ basetypes.NewStringValue("arg0"), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("arg1-element0"), basetypes.NewStringValue("arg1-element1"), @@ -149,13 +152,16 @@ func TestServerCallFunction(t *testing.T) { Function: &testprovider.Function{ RunMethod: func(ctx context.Context, req function.RunRequest, resp *function.RunResponse) { var arg0 basetypes.StringValue - var arg1 basetypes.ListValue + var arg1 basetypes.TupleValue resp.Diagnostics.Append(req.Arguments.Get(ctx, &arg0, &arg1)...) expectedArg0 := basetypes.NewStringValue("arg0") - expectedArg1 := basetypes.NewListValueMust( - basetypes.StringType{}, + expectedArg1 := basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("arg1-element0"), basetypes.NewStringValue("arg1-element1"), @@ -297,8 +303,11 @@ func TestServerCallFunction(t *testing.T) { request: &fwserver.CallFunctionRequest{ Arguments: function.NewArgumentsData([]attr.Value{ basetypes.NewStringValue("arg0"), - basetypes.NewListValueMust( - basetypes.StringType{}, + basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("arg1-element0"), basetypes.NewStringValue("arg1-element1"), @@ -308,14 +317,17 @@ func TestServerCallFunction(t *testing.T) { Function: &testprovider.Function{ RunMethod: func(ctx context.Context, req function.RunRequest, resp *function.RunResponse) { var arg0 basetypes.StringValue - var arg1 basetypes.ListValue + var arg1 basetypes.TupleValue resp.Diagnostics.Append(req.Arguments.GetArgument(ctx, 0, &arg0)...) resp.Diagnostics.Append(req.Arguments.GetArgument(ctx, 1, &arg1)...) expectedArg0 := basetypes.NewStringValue("arg0") - expectedArg1 := basetypes.NewListValueMust( - basetypes.StringType{}, + expectedArg1 := basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("arg1-element0"), basetypes.NewStringValue("arg1-element1"), diff --git a/internal/proto5server/server_callfunction_test.go b/internal/proto5server/server_callfunction_test.go index 273008754..394dc4ea9 100644 --- a/internal/proto5server/server_callfunction_test.go +++ b/internal/proto5server/server_callfunction_test.go @@ -123,13 +123,16 @@ func TestServerCallFunction(t *testing.T) { }, RunMethod: func(ctx context.Context, req function.RunRequest, resp *function.RunResponse) { var arg0 basetypes.StringValue - var arg1 basetypes.ListValue + var arg1 basetypes.TupleValue resp.Diagnostics.Append(req.Arguments.Get(ctx, &arg0, &arg1)...) expectedArg0 := basetypes.NewStringValue("arg0") - expectedArg1 := basetypes.NewListValueMust( - basetypes.StringType{}, + expectedArg1 := basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), basetypes.NewStringValue("varg-arg2"), diff --git a/internal/proto6server/server_callfunction_test.go b/internal/proto6server/server_callfunction_test.go index aae148006..8d21e1888 100644 --- a/internal/proto6server/server_callfunction_test.go +++ b/internal/proto6server/server_callfunction_test.go @@ -123,13 +123,16 @@ func TestServerCallFunction(t *testing.T) { }, RunMethod: func(ctx context.Context, req function.RunRequest, resp *function.RunResponse) { var arg0 basetypes.StringValue - var arg1 basetypes.ListValue + var arg1 basetypes.TupleValue resp.Diagnostics.Append(req.Arguments.Get(ctx, &arg0, &arg1)...) expectedArg0 := basetypes.NewStringValue("arg0") - expectedArg1 := basetypes.NewListValueMust( - basetypes.StringType{}, + expectedArg1 := basetypes.NewTupleValueMust( + []attr.Type{ + basetypes.StringType{}, + basetypes.StringType{}, + }, []attr.Value{ basetypes.NewStringValue("varg-arg1"), basetypes.NewStringValue("varg-arg2"), diff --git a/internal/reflect/into_test.go b/internal/reflect/into_test.go index 023a28705..a0d776347 100644 --- a/internal/reflect/into_test.go +++ b/internal/reflect/into_test.go @@ -50,7 +50,7 @@ func TestInto_Slices(t *testing.T) { target: make([]string, 0), expected: []string{"hello", "world"}, }, - "tuple-to-go-slice-unsupported": { + "tuple-to-go-slice": { typ: types.TupleType{ElemTypes: []attr.Type{types.StringType, types.StringType}}, value: tftypes.NewValue(tftypes.Tuple{ ElementTypes: []tftypes.Type{tftypes.String, tftypes.String}, @@ -59,6 +59,35 @@ func TestInto_Slices(t *testing.T) { tftypes.NewValue(tftypes.String, "world"), }), target: make([]string, 0), + expected: []string{"hello", "world"}, + }, + "tuple-to-go-slice-no-element-types-no-values": { + typ: types.TupleType{ElemTypes: []attr.Type{}}, + value: tftypes.NewValue(tftypes.Tuple{ElementTypes: []tftypes.Type{}}, []tftypes.Value{}), + target: make([]string, 0), + expected: []string{}, + }, + "tuple-to-go-slice-one-element": { + typ: types.TupleType{ElemTypes: []attr.Type{types.StringType}}, + value: tftypes.NewValue(tftypes.Tuple{ + ElementTypes: []tftypes.Type{tftypes.String}, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "hello"), + }), + target: make([]string, 0), + expected: []string{"hello"}, + }, + "tuple-to-go-slice-unsupported-no-element-types-with-values": { + typ: types.TupleType{ElemTypes: []attr.Type{}}, + value: tftypes.NewValue(tftypes.Tuple{ + ElementTypes: []tftypes.Type{tftypes.String, tftypes.String}, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "hello"), + tftypes.NewValue(tftypes.String, "world"), + }), + // Target isn't relevant for this test, as the wrapping reflection logic doesn't attempt to determine the underlying type of an `any` + // The test will successfully reject the reflection attempt based on the element types of the TupleType. + target: make([]string, 0), expected: make([]string, 0), expectedDiags: diag.Diagnostics{ diag.WithPath( @@ -71,7 +100,35 @@ func TestInto_Slices(t *testing.T) { tftypes.NewValue(tftypes.String, "world"), }), TargetType: reflect.TypeOf([]string{}), - Err: errors.New("cannot reflect tftypes.Tuple[tftypes.String, tftypes.String] using type information provided by basetypes.TupleType, reflection support is currently not implemented for tuples"), + Err: errors.New("cannot reflect tftypes.Tuple[tftypes.String, tftypes.String] using type information provided by basetypes.TupleType, tuple type contained no element types but received values"), + }, + ), + }, + }, + "tuple-to-go-slice-unsupported-multiple-element-types": { + typ: types.TupleType{ElemTypes: []attr.Type{types.StringType, types.BoolType}}, + value: tftypes.NewValue(tftypes.Tuple{ + ElementTypes: []tftypes.Type{tftypes.String, tftypes.Bool}, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "hello"), + tftypes.NewValue(tftypes.Bool, true), + }), + // Target isn't relevant for this test, as the wrapping reflection logic doesn't attempt to determine the underlying type of an `any` + // The test will successfully reject the reflection attempt based on the element types of the TupleType. + target: make([]string, 0), + expected: make([]string, 0), + expectedDiags: diag.Diagnostics{ + diag.WithPath( + path.Empty(), + refl.DiagIntoIncompatibleType{ + Val: tftypes.NewValue(tftypes.Tuple{ + ElementTypes: []tftypes.Type{tftypes.String, tftypes.Bool}, + }, []tftypes.Value{ + tftypes.NewValue(tftypes.String, "hello"), + tftypes.NewValue(tftypes.Bool, true), + }), + TargetType: reflect.TypeOf([]string{}), + Err: errors.New("cannot reflect tftypes.Tuple[tftypes.String, tftypes.Bool] using type information provided by basetypes.TupleType, reflection support for tuples is limited to multiple elements of the same element type. Expected all element types to be basetypes.StringType"), }, ), }, diff --git a/internal/reflect/outof_test.go b/internal/reflect/outof_test.go index 33f451218..6661c24c7 100644 --- a/internal/reflect/outof_test.go +++ b/internal/reflect/outof_test.go @@ -61,15 +61,58 @@ func TestFromValue_go_types(t *testing.T) { }, ), }, - "go-slice-to-tuple-value-unsupported": { + "go-slice-to-tuple-value": { typ: types.TupleType{ElemTypes: []attr.Type{types.StringType, types.StringType}}, - value: []any{"hello", "world"}, + value: []string{"hello", "world"}, + expected: types.TupleValueMust( + []attr.Type{ + types.StringType, + types.StringType, + }, + []attr.Value{ + types.StringValue("hello"), + types.StringValue("world"), + }, + ), + }, + "go-slice-to-tuple-value-empty": { + typ: types.TupleType{ElemTypes: []attr.Type{}}, + value: []any{}, + expected: types.TupleValueMust([]attr.Type{}, []attr.Value{}), + }, + "go-slice-to-tuple-value-one-element": { + typ: types.TupleType{ElemTypes: []attr.Type{types.BoolType}}, + value: []bool{true}, + expected: types.TupleValueMust( + []attr.Type{ + types.BoolType, + }, + []attr.Value{ + types.BoolValue(true), + }, + ), + }, + "go-slice-to-tuple-value-unsupported-no-element-types-with-values": { + typ: types.TupleType{ElemTypes: []attr.Type{}}, + value: []string{"hello", "world"}, + expectedDiags: diag.Diagnostics{ + diag.NewAttributeErrorDiagnostic( + path.Empty(), + "Value Conversion Error", + "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ + "cannot use type []string as schema type basetypes.TupleType; tuple type contained no element types but received values", + ), + }, + }, + "go-slice-to-tuple-value-unsupported-multiple-element-types": { + typ: types.TupleType{ElemTypes: []attr.Type{types.StringType, types.BoolType}}, + value: []any{"hello", true}, expectedDiags: diag.Diagnostics{ diag.NewAttributeErrorDiagnostic( path.Empty(), "Value Conversion Error", "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+ - "cannot use type []interface {} as schema type basetypes.TupleType; reflection support is currently not implemented for tuples", + "cannot use type []interface {} as schema type basetypes.TupleType; reflection support for tuples is limited to multiple elements of the same element type. Expected all element types to be basetypes.StringType", ), }, }, diff --git a/internal/reflect/slice.go b/internal/reflect/slice.go index a160a4a6f..5ffaddd9c 100644 --- a/internal/reflect/slice.go +++ b/internal/reflect/slice.go @@ -90,14 +90,75 @@ func reflectSlice(ctx context.Context, typ attr.Type, val tftypes.Value, target } return slice, diags - // Tuple reflection from slices is currently not supported as usage is limited + + // Tuple reflection into slices is currently limited to use-cases where all tuple element types are the same. + // + // Overall, Tuple support is limited in the framework, but the main path that executes tuple reflection is the provider-defined function variadic + // parameter. All tuple elements in this variadic parameter will have the same element type. For use-cases where the variadic parameter is a dynamic type, + // all elements will have the same type of `DynamicType` and value of `DynamicValue`, with an underlying value that may be different. case attr.TypeWithElementTypes: - diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ - Val: val, - TargetType: target.Type(), - Err: fmt.Errorf("cannot reflect %s using type information provided by %T, reflection support is currently not implemented for tuples", val.Type(), t), - })) - return target, diags + // we need to know the type the slice is wrapping + elemType := target.Type().Elem() + + // we want an empty version of the slice + slice := reflect.MakeSlice(target.Type(), 0, len(values)) + + if len(t.ElementTypes()) <= 0 { + // If the tuple values are empty as well, we can just pass back an empty slice of the type we received. + if len(values) == 0 { + return slice, diags + } + + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ + Val: val, + TargetType: target.Type(), + Err: fmt.Errorf("cannot reflect %s using type information provided by %T, tuple type contained no element types but received values", val.Type(), t), + })) + return target, diags + } + + // Ensure that all tuple element types are the same by comparing each element type to the first + multipleTypes := false + allElemTypes := t.ElementTypes() + elemAttrType := allElemTypes[0] + for _, elemType := range allElemTypes[1:] { + if !elemAttrType.Equal(elemType) { + multipleTypes = true + break + } + } + + if multipleTypes { + diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ + Val: val, + TargetType: target.Type(), + Err: fmt.Errorf("cannot reflect %s using type information provided by %T, reflection support for tuples is limited to multiple elements of the same element type. Expected all element types to be %T", val.Type(), t, elemAttrType), + })) + return target, diags + } + + // go over each of the values passed in, create a Go value of the right + // type for them, and add it to our new slice + for pos, value := range values { + // create a new Go value of the type that can go in the slice + targetValue := reflect.Zero(elemType) + + // update our path so we can have nice errors + valPath := path.AtTupleIndex(pos) + + // reflect the value into our new target + val, valDiags := BuildValue(ctx, elemAttrType, value, targetValue, opts, valPath) + diags.Append(valDiags...) + + if diags.HasError() { + return target, diags + } + + // add the new target to our slice + slice = reflect.Append(slice, val) + } + + return slice, diags default: diags.Append(diag.WithPath(path, DiagIntoIncompatibleType{ Val: val, @@ -184,15 +245,73 @@ func FromSlice(ctx context.Context, typ attr.Type, val reflect.Value, path path. tfElems = append(tfElems, tfVal) } - // Tuple reflection from slices is currently not supported as usage is limited + + // Tuple reflection from slices is currently limited to use-cases where all tuple element types are the same. + // + // Overall, Tuple support is limited in the framework, but the main path that executes tuple reflection is the provider-defined function variadic + // parameter. All tuple elements in this variadic parameter will have the same element type. For use-cases where the variadic parameter is a dynamic type, + // all elements will have the same type of `DynamicType` and value of `DynamicValue`, with an underlying value that may be different. case attr.TypeWithElementTypes: - err := fmt.Errorf("cannot use type %s as schema type %T; reflection support is currently not implemented for tuples", val.Type(), t) - diags.AddAttributeError( - path, - "Value Conversion Error", - "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), - ) - return nil, diags + if len(t.ElementTypes()) <= 0 { + // If the tuple values are empty as well, we can just pass back an empty slice of the type we received. + if val.Len() == 0 { + break + } + + err := fmt.Errorf("cannot use type %s as schema type %T; tuple type contained no element types but received values", val.Type(), t) + diags.AddAttributeError( + path, + "Value Conversion Error", + "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return nil, diags + } + + // Ensure that all tuple element types are the same by comparing each element type to the first + multipleTypes := false + allElemTypes := t.ElementTypes() + elemAttrType := allElemTypes[0] + for _, elemType := range allElemTypes[1:] { + if !elemAttrType.Equal(elemType) { + multipleTypes = true + break + } + } + + if multipleTypes { + err := fmt.Errorf("cannot use type %s as schema type %T; reflection support for tuples is limited to multiple elements of the same element type. Expected all element types to be %T", val.Type(), t, elemAttrType) + diags.AddAttributeError( + path, + "Value Conversion Error", + "An unexpected error was encountered trying to convert from slice value. This is always an error in the provider. Please report the following to the provider developer:\n\n"+err.Error(), + ) + return nil, diags + } + + for i := 0; i < val.Len(); i++ { + valPath := path.AtTupleIndex(i) + + val, valDiags := FromValue(ctx, elemAttrType, val.Index(i).Interface(), valPath) + diags.Append(valDiags...) + + if diags.HasError() { + return nil, diags + } + + tfVal, err := val.ToTerraformValue(ctx) + if err != nil { + return nil, append(diags, toTerraformValueErrorDiag(err, path)) + } + + if typeWithValidate, ok := elemAttrType.(xattr.TypeWithValidate); ok { + diags.Append(typeWithValidate.Validate(ctx, tfVal, valPath)...) + if diags.HasError() { + return nil, diags + } + } + + tfElems = append(tfElems, tfVal) + } default: err := fmt.Errorf("cannot use type %s as schema type %T; %T must be an attr.TypeWithElementType or attr.TypeWithElementTypes", val.Type(), t, t) diags.AddAttributeError( diff --git a/path/path.go b/path/path.go index e29a46fe0..4e060bd9a 100644 --- a/path/path.go +++ b/path/path.go @@ -52,6 +52,18 @@ func (p Path) AtListIndex(index int) Path { return copiedPath } +// AtTupleIndex returns a copied path with a new tuple index step at the end. +// The returned path is safe to modify without affecting the original. +// +// Tuple indices are 0-based. The first element of a tuple is 0. +func (p Path) AtTupleIndex(index int) Path { + copiedPath := p.Copy() + + copiedPath.steps.Append(PathStepElementKeyInt(index)) + + return copiedPath +} + // AtMapKey returns a copied path with a new map key step at the end. // The returned path is safe to modify without affecting the original. func (p Path) AtMapKey(key string) Path { diff --git a/path/path_test.go b/path/path_test.go index 7e7d78c63..7742e4335 100644 --- a/path/path_test.go +++ b/path/path_test.go @@ -52,6 +52,46 @@ func TestPathAtListIndex(t *testing.T) { } } +func TestPathAtTupleIndex(t *testing.T) { + t.Parallel() + + testCases := map[string]struct { + path path.Path + index int + expected path.Path + }{ + "empty": { + path: path.Empty(), + index: 1, + expected: path.Empty().AtTupleIndex(1), + }, + "shallow": { + path: path.Root("test"), + index: 1, + expected: path.Root("test").AtTupleIndex(1), + }, + "deep": { + path: path.Root("test1").AtTupleIndex(0).AtName("test2"), + index: 1, + expected: path.Root("test1").AtTupleIndex(0).AtName("test2").AtTupleIndex(1), + }, + } + + for name, testCase := range testCases { + name, testCase := name, testCase + + t.Run(name, func(t *testing.T) { + t.Parallel() + + got := testCase.path.AtTupleIndex(testCase.index) + + if diff := cmp.Diff(got, testCase.expected); diff != "" { + t.Errorf("unexpected difference: %s", diff) + } + }) + } +} + func TestPathAtMapKey(t *testing.T) { t.Parallel() diff --git a/website/docs/plugin/framework/functions/implementation.mdx b/website/docs/plugin/framework/functions/implementation.mdx index 9fb430a75..9da2c893f 100644 --- a/website/docs/plugin/framework/functions/implementation.mdx +++ b/website/docs/plugin/framework/functions/implementation.mdx @@ -114,7 +114,7 @@ The `Return` field must be defined as all functions must return a result. This i There may be zero or more parameters, which are defined with the `Parameters` field. They are ordered, which influences how practitioners call the function in their configurations and how the [Run method](#run-method) must read the argument data. Refer to the [parameters](/terraform/plugin/framework/functions/parameters) documentation for details about all available types and how to handle data with each type. -An optional `VariadicParameter` field enables a final variadic parameter which accepts zero, one, or more values of the same type. It may be optionally combined with `Parameters`, meaning it represents the any argument data after the final parameter. When reading argument data, a `VariadicParameter` is represented as an ordered list of the parameter type, where the list has zero or more elements to match the given arguments. +An optional `VariadicParameter` field enables a final variadic parameter which accepts zero, one, or more values of the same type. It may be optionally combined with `Parameters`, meaning it represents the any argument data after the final parameter. When reading argument data, a `VariadicParameter` is represented as a tuple, with each element matching the parameter type; the tuple has zero or more elements to match the given arguments. By default, Terraform will not pass null or unknown values to the provider logic when a function is called. Within each parameter, use the `AllowNullValue` and/or `AllowUnknownValues` fields to explicitly allow those kinds of values. Enabling `AllowNullValue` requires using a pointer type or [framework type](/terraform/plugin/framework/handling-data/types) when reading argument data. Enabling `AllowUnknownValues` requires using a [framework type](/terraform/plugin/framework/handling-data/types) when reading argument data. @@ -194,11 +194,11 @@ func (f *ExampleFunction) Run(ctx context.Context, req function.RunRequest, resp #### Reading Variadic Parameter Argument Data -The optional `VariadicParameter` field in a function definition enables a final variadic parameter which accepts zero, one, or more values of the same type. It may be optionally combined with `Parameters`, meaning it represents the argument data after the final parameter. When reading argument data, a `VariadicParameter` is represented as an ordered list of the parameter type, where the list has zero or more elements to match the given arguments. +The optional `VariadicParameter` field in a function definition enables a final variadic parameter which accepts zero, one, or more values of the same type. It may be optionally combined with `Parameters`, meaning it represents the argument data after the final parameter. When reading argument data, a `VariadicParameter` is represented as a tuple, with each element matching the parameter type; the tuple has zero or more elements to match the given arguments. -Use either the [framework list type](/terraform/plugin/framework/handling-data/types/list) or a Go slice of an appropriate type to match the variadic parameter `[]T`. +Use either the [framework tuple type](/terraform/plugin/framework/handling-data/types/tuple) or a Go slice of an appropriate type to match the variadic parameter `[]T`. -In this example, there is a boolean parameter and string variadic parameter, where the variadic parameter argument data is always fetched as a list: +In this example, there is a boolean parameter and string variadic parameter, where the variadic parameter argument data is always fetched as a slice of `string`: ```go func (f *ExampleFunction) Definition(ctx context.Context, req function.DefinitionRequest, resp *function.DefinitionResponse) { @@ -207,7 +207,9 @@ func (f *ExampleFunction) Definition(ctx context.Context, req function.Definitio Parameters: []function.Parameter{ function.BoolParameter{}, }, - VariadicParameter: function.StringParameter{}, + VariadicParameter: function.StringParameter{ + Name: "variadic_param", + }, } } @@ -221,7 +223,7 @@ func (f *ExampleFunction) Run(ctx context.Context, req function.RunRequest, resp } ``` -If necessary to return diagnostics for a specific variadic argument, note that Terraform treats each zero-based argument position individually unlike how the framework exposes the argument data. Add the number of non-variadic parameters (if any) to the variadic argument list element index to ensure the diagnostic is aligned to the correct argument in the configuration. +If necessary to return diagnostics for a specific variadic argument, note that Terraform treats each zero-based argument position individually unlike how the framework exposes the argument data. Add the number of non-variadic parameters (if any) to the variadic argument tuple element index to ensure the diagnostic is aligned to the correct argument in the configuration. In this example with two parameters and one variadic parameter, warning diagnostics are returned for all variadic arguments: @@ -233,7 +235,9 @@ func (f *ExampleFunction) Definition(ctx context.Context, req function.Definitio function.BoolParameter{}, function.Int64Parameter{}, }, - VariadicParameter: function.StringParameter{}, + VariadicParameter: function.StringParameter{ + Name: "variadic_param", + }, } } diff --git a/website/docs/plugin/framework/handling-data/types/tuple.mdx b/website/docs/plugin/framework/handling-data/types/tuple.mdx index 88571fd63..85a59dfcf 100644 --- a/website/docs/plugin/framework/handling-data/types/tuple.mdx +++ b/website/docs/plugin/framework/handling-data/types/tuple.mdx @@ -5,7 +5,7 @@ description: >- --- -The tuple type intentionally includes less functionality than other types in the type system as it has limited real world application and therefore is not exposed to provider developers except when working with dynamic values. +The tuple type doesn't have associated schema attributes as it has limited real world application. Provider developers will only encounter tuples when handling provider-defined function variadic parameters or dynamic values. # Tuple Type