Skip to content

Commit c9a4f80

Browse files
bfladbflad
authored
tftypes: Guarantee Type interface MarshalJSON error safety (#371)
Reference: #238 Reference: #365 This change removes the necessity for protocol type conversion to handle JSON encoding errors, therefore greatly simplifying those packages. Type system conversions into the protocol should always be possible, albeit potentially not valid for usage in Terraform. The protocol conversion code is not the place to handle that consideration, rather if developers want to validate their schema/function/type definitions before its sent across the protocol with respect to the validation rules imposed by Terraform, separate validation functionality should be introduced. This is not conceptually new, does not introduce behavior changes, and uses fuzz testing to verify that a panic is not possible today in the one way that developers could potentially introduce a new panic. The fuzz testing has been run for 60 seconds on a currently powerful workstation without generating any failure cases: ```console $ go test -fuzz=Fuzz -fuzztime=60s ./tftypes fuzz: elapsed: 0s, gathering baseline coverage: 0/138 completed fuzz: elapsed: 0s, gathering baseline coverage: 138/138 completed, now fuzzing with 10 workers fuzz: elapsed: 3s, execs: 406242 (135409/sec), new interesting: 6 (total: 144) fuzz: elapsed: 6s, execs: 833268 (142312/sec), new interesting: 7 (total: 145) fuzz: elapsed: 9s, execs: 1269960 (145562/sec), new interesting: 7 (total: 145) fuzz: elapsed: 12s, execs: 1684371 (138143/sec), new interesting: 8 (total: 146) fuzz: elapsed: 15s, execs: 2137768 (151160/sec), new interesting: 9 (total: 147) fuzz: elapsed: 18s, execs: 2574729 (145641/sec), new interesting: 11 (total: 149) fuzz: elapsed: 21s, execs: 3011973 (145722/sec), new interesting: 12 (total: 150) fuzz: elapsed: 24s, execs: 3442147 (143418/sec), new interesting: 12 (total: 150) fuzz: elapsed: 27s, execs: 3868833 (142210/sec), new interesting: 12 (total: 150) fuzz: elapsed: 30s, execs: 4313780 (148320/sec), new interesting: 14 (total: 152) fuzz: elapsed: 33s, execs: 4763813 (150034/sec), new interesting: 14 (total: 152) fuzz: elapsed: 36s, execs: 5201686 (145936/sec), new interesting: 15 (total: 153) fuzz: elapsed: 39s, execs: 5613562 (137285/sec), new interesting: 15 (total: 153) fuzz: elapsed: 42s, execs: 6051164 (145875/sec), new interesting: 15 (total: 153) fuzz: elapsed: 45s, execs: 6485230 (144677/sec), new interesting: 15 (total: 153) fuzz: elapsed: 48s, execs: 6912045 (142294/sec), new interesting: 15 (total: 153) fuzz: elapsed: 51s, execs: 7349416 (145756/sec), new interesting: 16 (total: 154) fuzz: elapsed: 54s, execs: 7792201 (147626/sec), new interesting: 16 (total: 154) fuzz: elapsed: 57s, execs: 8225683 (144471/sec), new interesting: 18 (total: 156) fuzz: elapsed: 1m0s, execs: 8637257 (137227/sec), new interesting: 18 (total: 156) fuzz: elapsed: 1m0s, execs: 8637257 (0/sec), new interesting: 18 (total: 156) PASS ok github.com/hashicorp/terraform-plugin-go/tftypes 60.883s ```
1 parent 73243d5 commit c9a4f80

25 files changed

+386
-729
lines changed

‎tfprotov5/internal/toproto/dynamic_value.go‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
package toproto
55

66
import (
7-
"fmt"
8-
97
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
108
"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5"
119
"github.com/hashicorp/terraform-plugin-go/tftypes"
@@ -24,17 +22,14 @@ func DynamicValue(in *tfprotov5.DynamicValue) *tfplugin5.DynamicValue {
2422
return resp
2523
}
2624

27-
func CtyType(in tftypes.Type) ([]byte, error) {
25+
func CtyType(in tftypes.Type) []byte {
2826
if in == nil {
29-
return nil, nil
27+
return nil
3028
}
3129

32-
switch {
33-
case in.Is(tftypes.String), in.Is(tftypes.Bool), in.Is(tftypes.Number),
34-
in.Is(tftypes.List{}), in.Is(tftypes.Map{}),
35-
in.Is(tftypes.Set{}), in.Is(tftypes.Object{}),
36-
in.Is(tftypes.Tuple{}), in.Is(tftypes.DynamicPseudoType):
37-
return in.MarshalJSON() //nolint:staticcheck
38-
}
39-
return nil, fmt.Errorf("unknown type %s", in)
30+
// MarshalJSON is always error safe.
31+
// nolint:staticcheck // Intended first-party usage
32+
resp, _ := in.MarshalJSON()
33+
34+
return resp
4035
}

‎tfprotov5/internal/toproto/function.go‎

Lines changed: 21 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
package toproto
55

66
import (
7-
"fmt"
8-
97
"github.com/hashicorp/terraform-plugin-go/tfprotov5"
108
"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5"
119
)
@@ -23,61 +21,31 @@ func CallFunction_Response(in *tfprotov5.CallFunctionResponse) *tfplugin5.CallFu
2321
return resp
2422
}
2523

26-
func Function(in *tfprotov5.Function) (*tfplugin5.Function, error) {
24+
func Function(in *tfprotov5.Function) *tfplugin5.Function {
2725
if in == nil {
28-
return nil, nil
26+
return nil
2927
}
3028

3129
resp := &tfplugin5.Function{
3230
Description: in.Description,
3331
DescriptionKind: StringKind(in.DescriptionKind),
3432
DeprecationMessage: in.DeprecationMessage,
3533
Parameters: make([]*tfplugin5.Function_Parameter, 0, len(in.Parameters)),
34+
Return: Function_Return(in.Return),
3635
Summary: in.Summary,
36+
VariadicParameter: Function_Parameter(in.VariadicParameter),
3737
}
3838

39-
for position, parameter := range in.Parameters {
40-
if parameter == nil {
41-
return nil, fmt.Errorf("missing function parameter definition at position: %d", position)
42-
}
43-
44-
functionParameter, err := Function_Parameter(parameter)
45-
46-
if err != nil {
47-
return nil, fmt.Errorf("unable to marshal function parameter at position %d: %w", position, err)
48-
}
49-
50-
resp.Parameters = append(resp.Parameters, functionParameter)
51-
}
52-
53-
if in.Return == nil {
54-
return nil, fmt.Errorf("missing function return definition")
55-
}
56-
57-
functionReturn, err := Function_Return(in.Return)
58-
59-
if err != nil {
60-
return nil, fmt.Errorf("unable to marshal function return: %w", err)
39+
for _, parameter := range in.Parameters {
40+
resp.Parameters = append(resp.Parameters, Function_Parameter(parameter))
6141
}
6242

63-
resp.Return = functionReturn
64-
65-
if in.VariadicParameter != nil {
66-
variadicParameter, err := Function_Parameter(in.VariadicParameter)
67-
68-
if err != nil {
69-
return nil, fmt.Errorf("unable to marshal variadic function parameter: %w", err)
70-
}
71-
72-
resp.VariadicParameter = variadicParameter
73-
}
74-
75-
return resp, nil
43+
return resp
7644
}
7745

78-
func Function_Parameter(in *tfprotov5.FunctionParameter) (*tfplugin5.Function_Parameter, error) {
46+
func Function_Parameter(in *tfprotov5.FunctionParameter) *tfplugin5.Function_Parameter {
7947
if in == nil {
80-
return nil, nil
48+
return nil
8149
}
8250

8351
resp := &tfplugin5.Function_Parameter{
@@ -86,66 +54,39 @@ func Function_Parameter(in *tfprotov5.FunctionParameter) (*tfplugin5.Function_Pa
8654
Description: in.Description,
8755
DescriptionKind: StringKind(in.DescriptionKind),
8856
Name: in.Name,
57+
Type: CtyType(in.Type),
8958
}
9059

91-
if in.Type == nil {
92-
return nil, fmt.Errorf("missing function parameter type definition")
93-
}
94-
95-
ctyType, err := CtyType(in.Type)
96-
97-
if err != nil {
98-
return resp, fmt.Errorf("error marshaling function parameter type: %w", err)
99-
}
100-
101-
resp.Type = ctyType
102-
103-
return resp, nil
60+
return resp
10461
}
10562

106-
func Function_Return(in *tfprotov5.FunctionReturn) (*tfplugin5.Function_Return, error) {
63+
func Function_Return(in *tfprotov5.FunctionReturn) *tfplugin5.Function_Return {
10764
if in == nil {
108-
return nil, nil
109-
}
110-
111-
resp := &tfplugin5.Function_Return{}
112-
113-
if in.Type == nil {
114-
return nil, fmt.Errorf("missing function return type definition")
65+
return nil
11566
}
11667

117-
ctyType, err := CtyType(in.Type)
118-
119-
if err != nil {
120-
return resp, fmt.Errorf("error marshaling function return type: %w", err)
68+
resp := &tfplugin5.Function_Return{
69+
Type: CtyType(in.Type),
12170
}
12271

123-
resp.Type = ctyType
124-
125-
return resp, nil
72+
return resp
12673
}
12774

128-
func GetFunctions_Response(in *tfprotov5.GetFunctionsResponse) (*tfplugin5.GetFunctions_Response, error) {
75+
func GetFunctions_Response(in *tfprotov5.GetFunctionsResponse) *tfplugin5.GetFunctions_Response {
12976
if in == nil {
130-
return nil, nil
77+
return nil
13178
}
13279

13380
resp := &tfplugin5.GetFunctions_Response{
13481
Diagnostics: Diagnostics(in.Diagnostics),
13582
Functions: make(map[string]*tfplugin5.Function, len(in.Functions)),
13683
}
13784

138-
for name, functionPtr := range in.Functions {
139-
function, err := Function(functionPtr)
140-
141-
if err != nil {
142-
return nil, fmt.Errorf("error marshaling function definition for %q: %w", name, err)
143-
}
144-
145-
resp.Functions[name] = function
85+
for name, function := range in.Functions {
86+
resp.Functions[name] = Function(function)
14687
}
14788

148-
return resp, nil
89+
return resp
14990
}
15091

15192
func GetMetadata_FunctionMetadata(in *tfprotov5.FunctionMetadata) *tfplugin5.GetMetadata_FunctionMetadata {

0 commit comments

Comments
 (0)