diff --git a/tfprotov5/internal/toproto/dynamic_value.go b/tfprotov5/internal/toproto/dynamic_value.go index 11b3cbd4..7f86517a 100644 --- a/tfprotov5/internal/toproto/dynamic_value.go +++ b/tfprotov5/internal/toproto/dynamic_value.go @@ -4,8 +4,6 @@ package toproto import ( - "fmt" - "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5" "github.com/hashicorp/terraform-plugin-go/tftypes" @@ -24,17 +22,14 @@ func DynamicValue(in *tfprotov5.DynamicValue) *tfplugin5.DynamicValue { return resp } -func CtyType(in tftypes.Type) ([]byte, error) { +func CtyType(in tftypes.Type) []byte { if in == nil { - return nil, nil + return nil } - switch { - case in.Is(tftypes.String), in.Is(tftypes.Bool), in.Is(tftypes.Number), - in.Is(tftypes.List{}), in.Is(tftypes.Map{}), - in.Is(tftypes.Set{}), in.Is(tftypes.Object{}), - in.Is(tftypes.Tuple{}), in.Is(tftypes.DynamicPseudoType): - return in.MarshalJSON() //nolint:staticcheck - } - return nil, fmt.Errorf("unknown type %s", in) + // MarshalJSON is always error safe. + // nolint:staticcheck // Intended first-party usage + resp, _ := in.MarshalJSON() + + return resp } diff --git a/tfprotov5/internal/toproto/function.go b/tfprotov5/internal/toproto/function.go index 20c825c3..bb66552f 100644 --- a/tfprotov5/internal/toproto/function.go +++ b/tfprotov5/internal/toproto/function.go @@ -4,8 +4,6 @@ package toproto import ( - "fmt" - "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5" ) @@ -23,9 +21,9 @@ func CallFunction_Response(in *tfprotov5.CallFunctionResponse) *tfplugin5.CallFu return resp } -func Function(in *tfprotov5.Function) (*tfplugin5.Function, error) { +func Function(in *tfprotov5.Function) *tfplugin5.Function { if in == nil { - return nil, nil + return nil } resp := &tfplugin5.Function{ @@ -33,51 +31,21 @@ func Function(in *tfprotov5.Function) (*tfplugin5.Function, error) { DescriptionKind: StringKind(in.DescriptionKind), DeprecationMessage: in.DeprecationMessage, Parameters: make([]*tfplugin5.Function_Parameter, 0, len(in.Parameters)), + Return: Function_Return(in.Return), Summary: in.Summary, + VariadicParameter: Function_Parameter(in.VariadicParameter), } - for position, parameter := range in.Parameters { - if parameter == nil { - return nil, fmt.Errorf("missing function parameter definition at position: %d", position) - } - - functionParameter, err := Function_Parameter(parameter) - - if err != nil { - return nil, fmt.Errorf("unable to marshal function parameter at position %d: %w", position, err) - } - - resp.Parameters = append(resp.Parameters, functionParameter) - } - - if in.Return == nil { - return nil, fmt.Errorf("missing function return definition") - } - - functionReturn, err := Function_Return(in.Return) - - if err != nil { - return nil, fmt.Errorf("unable to marshal function return: %w", err) + for _, parameter := range in.Parameters { + resp.Parameters = append(resp.Parameters, Function_Parameter(parameter)) } - resp.Return = functionReturn - - if in.VariadicParameter != nil { - variadicParameter, err := Function_Parameter(in.VariadicParameter) - - if err != nil { - return nil, fmt.Errorf("unable to marshal variadic function parameter: %w", err) - } - - resp.VariadicParameter = variadicParameter - } - - return resp, nil + return resp } -func Function_Parameter(in *tfprotov5.FunctionParameter) (*tfplugin5.Function_Parameter, error) { +func Function_Parameter(in *tfprotov5.FunctionParameter) *tfplugin5.Function_Parameter { if in == nil { - return nil, nil + return nil } resp := &tfplugin5.Function_Parameter{ @@ -86,48 +54,27 @@ func Function_Parameter(in *tfprotov5.FunctionParameter) (*tfplugin5.Function_Pa Description: in.Description, DescriptionKind: StringKind(in.DescriptionKind), Name: in.Name, + Type: CtyType(in.Type), } - if in.Type == nil { - return nil, fmt.Errorf("missing function parameter type definition") - } - - ctyType, err := CtyType(in.Type) - - if err != nil { - return resp, fmt.Errorf("error marshaling function parameter type: %w", err) - } - - resp.Type = ctyType - - return resp, nil + return resp } -func Function_Return(in *tfprotov5.FunctionReturn) (*tfplugin5.Function_Return, error) { +func Function_Return(in *tfprotov5.FunctionReturn) *tfplugin5.Function_Return { if in == nil { - return nil, nil - } - - resp := &tfplugin5.Function_Return{} - - if in.Type == nil { - return nil, fmt.Errorf("missing function return type definition") + return nil } - ctyType, err := CtyType(in.Type) - - if err != nil { - return resp, fmt.Errorf("error marshaling function return type: %w", err) + resp := &tfplugin5.Function_Return{ + Type: CtyType(in.Type), } - resp.Type = ctyType - - return resp, nil + return resp } -func GetFunctions_Response(in *tfprotov5.GetFunctionsResponse) (*tfplugin5.GetFunctions_Response, error) { +func GetFunctions_Response(in *tfprotov5.GetFunctionsResponse) *tfplugin5.GetFunctions_Response { if in == nil { - return nil, nil + return nil } resp := &tfplugin5.GetFunctions_Response{ @@ -135,17 +82,11 @@ func GetFunctions_Response(in *tfprotov5.GetFunctionsResponse) (*tfplugin5.GetFu Functions: make(map[string]*tfplugin5.Function, len(in.Functions)), } - for name, functionPtr := range in.Functions { - function, err := Function(functionPtr) - - if err != nil { - return nil, fmt.Errorf("error marshaling function definition for %q: %w", name, err) - } - - resp.Functions[name] = function + for name, function := range in.Functions { + resp.Functions[name] = Function(function) } - return resp, nil + return resp } func GetMetadata_FunctionMetadata(in *tfprotov5.FunctionMetadata) *tfplugin5.GetMetadata_FunctionMetadata { diff --git a/tfprotov5/internal/toproto/function_test.go b/tfprotov5/internal/toproto/function_test.go index dc4d470d..604bca58 100644 --- a/tfprotov5/internal/toproto/function_test.go +++ b/tfprotov5/internal/toproto/function_test.go @@ -91,58 +91,36 @@ func TestFunction(t *testing.T) { expected: nil, }, "zero": { - in: &tfprotov5.Function{}, - expected: nil, + in: &tfprotov5.Function{}, + expected: &tfplugin5.Function{ + Parameters: []*tfplugin5.Function_Parameter{}, + }, }, "Description": { in: &tfprotov5.Function{ Description: "test", - // Return will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Return: &tfprotov5.FunctionReturn{ - Type: tftypes.Bool, - }, }, expected: &tfplugin5.Function{ Description: "test", Parameters: []*tfplugin5.Function_Parameter{}, - Return: &tfplugin5.Function_Return{ - Type: []byte(`"bool"`), - }, }, }, "DescriptionKind": { in: &tfprotov5.Function{ DescriptionKind: tfprotov5.StringKindMarkdown, - // Return will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Return: &tfprotov5.FunctionReturn{ - Type: tftypes.Bool, - }, }, expected: &tfplugin5.Function{ DescriptionKind: tfplugin5.StringKind_MARKDOWN, Parameters: []*tfplugin5.Function_Parameter{}, - Return: &tfplugin5.Function_Return{ - Type: []byte(`"bool"`), - }, }, }, "DeprecationMessage": { in: &tfprotov5.Function{ DeprecationMessage: "test", - // Return will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Return: &tfprotov5.FunctionReturn{ - Type: tftypes.Bool, - }, }, expected: &tfplugin5.Function{ DeprecationMessage: "test", Parameters: []*tfplugin5.Function_Parameter{}, - Return: &tfplugin5.Function_Return{ - Type: []byte(`"bool"`), - }, }, }, "Parameters": { @@ -152,9 +130,6 @@ func TestFunction(t *testing.T) { Type: tftypes.Bool, }, }, - Return: &tfprotov5.FunctionReturn{ - Type: tftypes.Bool, - }, }, expected: &tfplugin5.Function{ Parameters: []*tfplugin5.Function_Parameter{ @@ -162,9 +137,6 @@ func TestFunction(t *testing.T) { Type: []byte(`"bool"`), }, }, - Return: &tfplugin5.Function_Return{ - Type: []byte(`"bool"`), - }, }, }, "Return": { @@ -182,35 +154,21 @@ func TestFunction(t *testing.T) { }, "Summary": { in: &tfprotov5.Function{ - // Return will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Return: &tfprotov5.FunctionReturn{ - Type: tftypes.Bool, - }, Summary: "test", }, expected: &tfplugin5.Function{ Parameters: []*tfplugin5.Function_Parameter{}, - Return: &tfplugin5.Function_Return{ - Type: []byte(`"bool"`), - }, - Summary: "test", + Summary: "test", }, }, "VariadicParameter": { in: &tfprotov5.Function{ - Return: &tfprotov5.FunctionReturn{ - Type: tftypes.Bool, - }, VariadicParameter: &tfprotov5.FunctionParameter{ Type: tftypes.Bool, }, }, expected: &tfplugin5.Function{ Parameters: []*tfplugin5.Function_Parameter{}, - Return: &tfplugin5.Function_Return{ - Type: []byte(`"bool"`), - }, VariadicParameter: &tfplugin5.Function_Parameter{ Type: []byte(`"bool"`), }, @@ -224,10 +182,7 @@ func TestFunction(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it will be removed - // in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Function(testCase.in) + got := toproto.Function(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -259,66 +214,46 @@ func TestFunction_Parameter(t *testing.T) { }, "zero": { in: &tfprotov5.FunctionParameter{}, - expected: nil, + expected: &tfplugin5.Function_Parameter{}, }, "AllowNullValue": { in: &tfprotov5.FunctionParameter{ AllowNullValue: true, - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin5.Function_Parameter{ AllowNullValue: true, - Type: []byte(`"bool"`), }, }, "AllowUnknownValues": { in: &tfprotov5.FunctionParameter{ AllowUnknownValues: true, - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin5.Function_Parameter{ AllowUnknownValues: true, - Type: []byte(`"bool"`), }, }, "Description": { in: &tfprotov5.FunctionParameter{ Description: "test", - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin5.Function_Parameter{ Description: "test", - Type: []byte(`"bool"`), }, }, "DescriptionKind": { in: &tfprotov5.FunctionParameter{ DescriptionKind: tfprotov5.StringKindMarkdown, - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin5.Function_Parameter{ DescriptionKind: tfplugin5.StringKind_MARKDOWN, - Type: []byte(`"bool"`), }, }, "Name": { in: &tfprotov5.FunctionParameter{ Name: "test", - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin5.Function_Parameter{ Name: "test", - Type: []byte(`"bool"`), }, }, "Type": { @@ -337,10 +272,7 @@ func TestFunction_Parameter(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it will be removed - // in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Function_Parameter(testCase.in) + got := toproto.Function_Parameter(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -370,7 +302,7 @@ func TestFunction_Return(t *testing.T) { }, "zero": { in: &tfprotov5.FunctionReturn{}, - expected: nil, + expected: &tfplugin5.Function_Return{}, }, "Type": { in: &tfprotov5.FunctionReturn{ @@ -388,10 +320,7 @@ func TestFunction_Return(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it will be removed - // in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Function_Return(testCase.in) + got := toproto.Function_Return(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -469,11 +398,7 @@ func TestGetFunctions_Response(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.GetFunctions_Response(testCase.in) + got := toproto.GetFunctions_Response(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than diff --git a/tfprotov5/internal/toproto/provider.go b/tfprotov5/internal/toproto/provider.go index aca29a5c..4891c538 100644 --- a/tfprotov5/internal/toproto/provider.go +++ b/tfprotov5/internal/toproto/provider.go @@ -4,8 +4,6 @@ package toproto import ( - "fmt" - "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5" ) @@ -38,64 +36,34 @@ func GetMetadata_Response(in *tfprotov5.GetMetadataResponse) *tfplugin5.GetMetad return resp } -func GetProviderSchema_Response(in *tfprotov5.GetProviderSchemaResponse) (*tfplugin5.GetProviderSchema_Response, error) { +func GetProviderSchema_Response(in *tfprotov5.GetProviderSchemaResponse) *tfplugin5.GetProviderSchema_Response { if in == nil { - return nil, nil - } - - provider, err := Schema(in.Provider) - - if err != nil { - return nil, fmt.Errorf("error marshaling provider schema: %w", err) - } - - providerMeta, err := Schema(in.ProviderMeta) - - if err != nil { - return nil, fmt.Errorf("error marshaling provider_meta schema: %w", err) + return nil } resp := &tfplugin5.GetProviderSchema_Response{ DataSourceSchemas: make(map[string]*tfplugin5.Schema, len(in.DataSourceSchemas)), Diagnostics: Diagnostics(in.Diagnostics), Functions: make(map[string]*tfplugin5.Function, len(in.Functions)), - Provider: provider, - ProviderMeta: providerMeta, + Provider: Schema(in.Provider), + ProviderMeta: Schema(in.ProviderMeta), ResourceSchemas: make(map[string]*tfplugin5.Schema, len(in.ResourceSchemas)), ServerCapabilities: ServerCapabilities(in.ServerCapabilities), } - for k, v := range in.ResourceSchemas { - schema, err := Schema(v) - - if err != nil { - return nil, fmt.Errorf("error marshaling resource schema for %q: %w", k, err) - } - - resp.ResourceSchemas[k] = schema + for name, schema := range in.ResourceSchemas { + resp.ResourceSchemas[name] = Schema(schema) } - for k, v := range in.DataSourceSchemas { - schema, err := Schema(v) - - if err != nil { - return nil, fmt.Errorf("error marshaling data source schema for %q: %w", k, err) - } - - resp.DataSourceSchemas[k] = schema + for name, schema := range in.DataSourceSchemas { + resp.DataSourceSchemas[name] = Schema(schema) } - for name, functionPtr := range in.Functions { - function, err := Function(functionPtr) - - if err != nil { - return nil, fmt.Errorf("error marshaling function definition for %q: %w", name, err) - } - - resp.Functions[name] = function + for name, function := range in.Functions { + resp.Functions[name] = Function(function) } - return resp, nil + return resp } func PrepareProviderConfig_Response(in *tfprotov5.PrepareProviderConfigResponse) *tfplugin5.PrepareProviderConfig_Response { diff --git a/tfprotov5/internal/toproto/provider_test.go b/tfprotov5/internal/toproto/provider_test.go index d02b3778..fd544441 100644 --- a/tfprotov5/internal/toproto/provider_test.go +++ b/tfprotov5/internal/toproto/provider_test.go @@ -412,11 +412,7 @@ func TestGetProviderSchema_Response(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.GetProviderSchema_Response(testCase.in) + got := toproto.GetProviderSchema_Response(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than diff --git a/tfprotov5/internal/toproto/schema.go b/tfprotov5/internal/toproto/schema.go index a975e355..69d47af1 100644 --- a/tfprotov5/internal/toproto/schema.go +++ b/tfprotov5/internal/toproto/schema.go @@ -4,69 +4,43 @@ package toproto import ( - "fmt" - "github.com/hashicorp/terraform-plugin-go/tfprotov5" "github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/tfplugin5" ) -func Schema(in *tfprotov5.Schema) (*tfplugin5.Schema, error) { +func Schema(in *tfprotov5.Schema) *tfplugin5.Schema { if in == nil { - return nil, nil - } - - block, err := Schema_Block(in.Block) - - if err != nil { - return nil, fmt.Errorf("error marshalling block: %w", err) + return nil } resp := &tfplugin5.Schema{ - Block: block, + Block: Schema_Block(in.Block), Version: in.Version, } - return resp, nil + return resp } -func Schema_Block(in *tfprotov5.SchemaBlock) (*tfplugin5.Schema_Block, error) { +func Schema_Block(in *tfprotov5.SchemaBlock) *tfplugin5.Schema_Block { if in == nil { - return nil, nil - } - - attributes, err := Schema_Attributes(in.Attributes) - - if err != nil { - return nil, err - } - - blockTypes, err := Schema_NestedBlocks(in.BlockTypes) - - if err != nil { - return nil, err + return nil } resp := &tfplugin5.Schema_Block{ - Attributes: attributes, - BlockTypes: blockTypes, + Attributes: Schema_Attributes(in.Attributes), + BlockTypes: Schema_NestedBlocks(in.BlockTypes), Deprecated: in.Deprecated, Description: in.Description, DescriptionKind: StringKind(in.DescriptionKind), Version: in.Version, } - return resp, nil + return resp } -func Schema_Attribute(in *tfprotov5.SchemaAttribute) (*tfplugin5.Schema_Attribute, error) { +func Schema_Attribute(in *tfprotov5.SchemaAttribute) *tfplugin5.Schema_Attribute { if in == nil { - return nil, nil - } - - typ, err := CtyType(in.Type) - - if err != nil { - return nil, fmt.Errorf("error marshaling %q type to JSON: %w", in.Name, err) + return nil } resp := &tfplugin5.Schema_Attribute{ @@ -78,64 +52,46 @@ func Schema_Attribute(in *tfprotov5.SchemaAttribute) (*tfplugin5.Schema_Attribut Optional: in.Optional, Required: in.Required, Sensitive: in.Sensitive, - Type: typ, + Type: CtyType(in.Type), } - return resp, nil + return resp } -func Schema_Attributes(in []*tfprotov5.SchemaAttribute) ([]*tfplugin5.Schema_Attribute, error) { +func Schema_Attributes(in []*tfprotov5.SchemaAttribute) []*tfplugin5.Schema_Attribute { resp := make([]*tfplugin5.Schema_Attribute, 0, len(in)) for _, a := range in { - attr, err := Schema_Attribute(a) - - if err != nil { - return nil, err - } - - resp = append(resp, attr) + resp = append(resp, Schema_Attribute(a)) } - return resp, nil + return resp } -func Schema_NestedBlock(in *tfprotov5.SchemaNestedBlock) (*tfplugin5.Schema_NestedBlock, error) { +func Schema_NestedBlock(in *tfprotov5.SchemaNestedBlock) *tfplugin5.Schema_NestedBlock { if in == nil { - return nil, nil - } - - block, err := Schema_Block(in.Block) - - if err != nil { - return nil, fmt.Errorf("error marshaling %q nested block: %w", in.TypeName, err) + return nil } resp := &tfplugin5.Schema_NestedBlock{ - Block: block, + Block: Schema_Block(in.Block), MaxItems: in.MaxItems, MinItems: in.MinItems, Nesting: Schema_NestedBlock_NestingMode(in.Nesting), TypeName: in.TypeName, } - return resp, nil + return resp } -func Schema_NestedBlocks(in []*tfprotov5.SchemaNestedBlock) ([]*tfplugin5.Schema_NestedBlock, error) { +func Schema_NestedBlocks(in []*tfprotov5.SchemaNestedBlock) []*tfplugin5.Schema_NestedBlock { resp := make([]*tfplugin5.Schema_NestedBlock, 0, len(in)) for _, b := range in { - block, err := Schema_NestedBlock(b) - - if err != nil { - return nil, err - } - - resp = append(resp, block) + resp = append(resp, Schema_NestedBlock(b)) } - return resp, nil + return resp } func Schema_NestedBlock_NestingMode(in tfprotov5.SchemaNestedBlockNestingMode) tfplugin5.Schema_NestedBlock_NestingMode { diff --git a/tfprotov5/internal/toproto/schema_test.go b/tfprotov5/internal/toproto/schema_test.go index 6329899d..dd044107 100644 --- a/tfprotov5/internal/toproto/schema_test.go +++ b/tfprotov5/internal/toproto/schema_test.go @@ -66,11 +66,7 @@ func TestSchema(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema(testCase.in) + got := toproto.Schema(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -185,11 +181,7 @@ func TestSchema_Attribute(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_Attribute(testCase.in) + got := toproto.Schema_Attribute(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -259,11 +251,7 @@ func TestSchema_Attributes(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_Attributes(testCase.in) + got := toproto.Schema_Attributes(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -380,11 +368,7 @@ func TestSchema_Block(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_Block(testCase.in) + got := toproto.Schema_Block(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -479,11 +463,7 @@ func TestSchema_NestedBlock(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_NestedBlock(testCase.in) + got := toproto.Schema_NestedBlock(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -555,11 +535,7 @@ func TestSchema_NestedBlocks(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_NestedBlocks(testCase.in) + got := toproto.Schema_NestedBlocks(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than diff --git a/tfprotov5/tf5server/server.go b/tfprotov5/tf5server/server.go index c57cbafe..2c26c4c0 100644 --- a/tfprotov5/tf5server/server.go +++ b/tfprotov5/tf5server/server.go @@ -535,12 +535,7 @@ func (s *server) GetSchema(ctx context.Context, protoReq *tfplugin5.GetProviderS tf5serverlogging.DownstreamResponse(ctx, resp.Diagnostics) tf5serverlogging.ServerCapabilities(ctx, resp.ServerCapabilities) - protoResp, err := toproto.GetProviderSchema_Response(resp) - - if err != nil { - logging.ProtocolError(ctx, "Error converting response to protobuf", map[string]interface{}{logging.KeyError: err}) - return nil, err - } + protoResp := toproto.GetProviderSchema_Response(resp) return protoResp, nil } @@ -1043,12 +1038,7 @@ func (s *server) GetFunctions(ctx context.Context, protoReq *tfplugin5.GetFuncti tf5serverlogging.DownstreamResponse(ctx, resp.Diagnostics) - protoResp, err := toproto.GetFunctions_Response(resp) - - if err != nil { - logging.ProtocolError(ctx, "Error converting response to protobuf", map[string]any{logging.KeyError: err}) - return nil, err - } + protoResp := toproto.GetFunctions_Response(resp) return protoResp, nil } diff --git a/tfprotov6/internal/toproto/dynamic_value.go b/tfprotov6/internal/toproto/dynamic_value.go index 918f0652..881be181 100644 --- a/tfprotov6/internal/toproto/dynamic_value.go +++ b/tfprotov6/internal/toproto/dynamic_value.go @@ -4,8 +4,6 @@ package toproto import ( - "fmt" - "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tfprotov6/internal/tfplugin6" "github.com/hashicorp/terraform-plugin-go/tftypes" @@ -24,17 +22,14 @@ func DynamicValue(in *tfprotov6.DynamicValue) *tfplugin6.DynamicValue { return resp } -func CtyType(in tftypes.Type) ([]byte, error) { +func CtyType(in tftypes.Type) []byte { if in == nil { - return nil, nil + return nil } - switch { - case in.Is(tftypes.String), in.Is(tftypes.Bool), in.Is(tftypes.Number), - in.Is(tftypes.List{}), in.Is(tftypes.Map{}), - in.Is(tftypes.Set{}), in.Is(tftypes.Object{}), - in.Is(tftypes.Tuple{}), in.Is(tftypes.DynamicPseudoType): - return in.MarshalJSON() //nolint:staticcheck - } - return nil, fmt.Errorf("unknown type %s", in) + // MarshalJSON is always error safe. + // nolint:staticcheck // Intended first-party usage + resp, _ := in.MarshalJSON() + + return resp } diff --git a/tfprotov6/internal/toproto/function.go b/tfprotov6/internal/toproto/function.go index 3a0c6b1b..050d27e4 100644 --- a/tfprotov6/internal/toproto/function.go +++ b/tfprotov6/internal/toproto/function.go @@ -4,8 +4,6 @@ package toproto import ( - "fmt" - "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tfprotov6/internal/tfplugin6" ) @@ -23,9 +21,9 @@ func CallFunction_Response(in *tfprotov6.CallFunctionResponse) *tfplugin6.CallFu return resp } -func Function(in *tfprotov6.Function) (*tfplugin6.Function, error) { +func Function(in *tfprotov6.Function) *tfplugin6.Function { if in == nil { - return nil, nil + return nil } resp := &tfplugin6.Function{ @@ -33,51 +31,21 @@ func Function(in *tfprotov6.Function) (*tfplugin6.Function, error) { DescriptionKind: StringKind(in.DescriptionKind), DeprecationMessage: in.DeprecationMessage, Parameters: make([]*tfplugin6.Function_Parameter, 0, len(in.Parameters)), + Return: Function_Return(in.Return), Summary: in.Summary, + VariadicParameter: Function_Parameter(in.VariadicParameter), } - for position, parameter := range in.Parameters { - if parameter == nil { - return nil, fmt.Errorf("missing function parameter definition at position: %d", position) - } - - functionParameter, err := Function_Parameter(parameter) - - if err != nil { - return nil, fmt.Errorf("unable to marshal function parameter at position %d: %w", position, err) - } - - resp.Parameters = append(resp.Parameters, functionParameter) - } - - if in.Return == nil { - return nil, fmt.Errorf("missing function return definition") - } - - functionReturn, err := Function_Return(in.Return) - - if err != nil { - return nil, fmt.Errorf("unable to marshal function return: %w", err) + for _, parameter := range in.Parameters { + resp.Parameters = append(resp.Parameters, Function_Parameter(parameter)) } - resp.Return = functionReturn - - if in.VariadicParameter != nil { - variadicParameter, err := Function_Parameter(in.VariadicParameter) - - if err != nil { - return nil, fmt.Errorf("unable to marshal variadic function parameter: %w", err) - } - - resp.VariadicParameter = variadicParameter - } - - return resp, nil + return resp } -func Function_Parameter(in *tfprotov6.FunctionParameter) (*tfplugin6.Function_Parameter, error) { +func Function_Parameter(in *tfprotov6.FunctionParameter) *tfplugin6.Function_Parameter { if in == nil { - return nil, nil + return nil } resp := &tfplugin6.Function_Parameter{ @@ -86,48 +54,27 @@ func Function_Parameter(in *tfprotov6.FunctionParameter) (*tfplugin6.Function_Pa Description: in.Description, DescriptionKind: StringKind(in.DescriptionKind), Name: in.Name, + Type: CtyType(in.Type), } - if in.Type == nil { - return nil, fmt.Errorf("missing function parameter type definition") - } - - ctyType, err := CtyType(in.Type) - - if err != nil { - return resp, fmt.Errorf("error marshaling function parameter type: %w", err) - } - - resp.Type = ctyType - - return resp, nil + return resp } -func Function_Return(in *tfprotov6.FunctionReturn) (*tfplugin6.Function_Return, error) { +func Function_Return(in *tfprotov6.FunctionReturn) *tfplugin6.Function_Return { if in == nil { - return nil, nil - } - - resp := &tfplugin6.Function_Return{} - - if in.Type == nil { - return nil, fmt.Errorf("missing function return type definition") + return nil } - ctyType, err := CtyType(in.Type) - - if err != nil { - return resp, fmt.Errorf("error marshaling function return type: %w", err) + resp := &tfplugin6.Function_Return{ + Type: CtyType(in.Type), } - resp.Type = ctyType - - return resp, nil + return resp } -func GetFunctions_Response(in *tfprotov6.GetFunctionsResponse) (*tfplugin6.GetFunctions_Response, error) { +func GetFunctions_Response(in *tfprotov6.GetFunctionsResponse) *tfplugin6.GetFunctions_Response { if in == nil { - return nil, nil + return nil } resp := &tfplugin6.GetFunctions_Response{ @@ -135,17 +82,11 @@ func GetFunctions_Response(in *tfprotov6.GetFunctionsResponse) (*tfplugin6.GetFu Functions: make(map[string]*tfplugin6.Function, len(in.Functions)), } - for name, functionPtr := range in.Functions { - function, err := Function(functionPtr) - - if err != nil { - return nil, fmt.Errorf("error marshaling function definition for %q: %w", name, err) - } - - resp.Functions[name] = function + for name, function := range in.Functions { + resp.Functions[name] = Function(function) } - return resp, nil + return resp } func GetMetadata_FunctionMetadata(in *tfprotov6.FunctionMetadata) *tfplugin6.GetMetadata_FunctionMetadata { diff --git a/tfprotov6/internal/toproto/function_test.go b/tfprotov6/internal/toproto/function_test.go index 1eeda6ef..9ee506a2 100644 --- a/tfprotov6/internal/toproto/function_test.go +++ b/tfprotov6/internal/toproto/function_test.go @@ -91,58 +91,36 @@ func TestFunction(t *testing.T) { expected: nil, }, "zero": { - in: &tfprotov6.Function{}, - expected: nil, + in: &tfprotov6.Function{}, + expected: &tfplugin6.Function{ + Parameters: []*tfplugin6.Function_Parameter{}, + }, }, "Description": { in: &tfprotov6.Function{ Description: "test", - // Return will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Return: &tfprotov6.FunctionReturn{ - Type: tftypes.Bool, - }, }, expected: &tfplugin6.Function{ Description: "test", Parameters: []*tfplugin6.Function_Parameter{}, - Return: &tfplugin6.Function_Return{ - Type: []byte(`"bool"`), - }, }, }, "DescriptionKind": { in: &tfprotov6.Function{ DescriptionKind: tfprotov6.StringKindMarkdown, - // Return will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Return: &tfprotov6.FunctionReturn{ - Type: tftypes.Bool, - }, }, expected: &tfplugin6.Function{ DescriptionKind: tfplugin6.StringKind_MARKDOWN, Parameters: []*tfplugin6.Function_Parameter{}, - Return: &tfplugin6.Function_Return{ - Type: []byte(`"bool"`), - }, }, }, "DeprecationMessage": { in: &tfprotov6.Function{ DeprecationMessage: "test", - // Return will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Return: &tfprotov6.FunctionReturn{ - Type: tftypes.Bool, - }, }, expected: &tfplugin6.Function{ DeprecationMessage: "test", Parameters: []*tfplugin6.Function_Parameter{}, - Return: &tfplugin6.Function_Return{ - Type: []byte(`"bool"`), - }, }, }, "Parameters": { @@ -152,9 +130,6 @@ func TestFunction(t *testing.T) { Type: tftypes.Bool, }, }, - Return: &tfprotov6.FunctionReturn{ - Type: tftypes.Bool, - }, }, expected: &tfplugin6.Function{ Parameters: []*tfplugin6.Function_Parameter{ @@ -162,9 +137,6 @@ func TestFunction(t *testing.T) { Type: []byte(`"bool"`), }, }, - Return: &tfplugin6.Function_Return{ - Type: []byte(`"bool"`), - }, }, }, "Return": { @@ -182,35 +154,21 @@ func TestFunction(t *testing.T) { }, "Summary": { in: &tfprotov6.Function{ - // Return will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Return: &tfprotov6.FunctionReturn{ - Type: tftypes.Bool, - }, Summary: "test", }, expected: &tfplugin6.Function{ Parameters: []*tfplugin6.Function_Parameter{}, - Return: &tfplugin6.Function_Return{ - Type: []byte(`"bool"`), - }, - Summary: "test", + Summary: "test", }, }, "VariadicParameter": { in: &tfprotov6.Function{ - Return: &tfprotov6.FunctionReturn{ - Type: tftypes.Bool, - }, VariadicParameter: &tfprotov6.FunctionParameter{ Type: tftypes.Bool, }, }, expected: &tfplugin6.Function{ Parameters: []*tfplugin6.Function_Parameter{}, - Return: &tfplugin6.Function_Return{ - Type: []byte(`"bool"`), - }, VariadicParameter: &tfplugin6.Function_Parameter{ Type: []byte(`"bool"`), }, @@ -224,10 +182,7 @@ func TestFunction(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it will be removed - // in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Function(testCase.in) + got := toproto.Function(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -259,66 +214,46 @@ func TestFunction_Parameter(t *testing.T) { }, "zero": { in: &tfprotov6.FunctionParameter{}, - expected: nil, + expected: &tfplugin6.Function_Parameter{}, }, "AllowNullValue": { in: &tfprotov6.FunctionParameter{ AllowNullValue: true, - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin6.Function_Parameter{ AllowNullValue: true, - Type: []byte(`"bool"`), }, }, "AllowUnknownValues": { in: &tfprotov6.FunctionParameter{ AllowUnknownValues: true, - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin6.Function_Parameter{ AllowUnknownValues: true, - Type: []byte(`"bool"`), }, }, "Description": { in: &tfprotov6.FunctionParameter{ Description: "test", - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin6.Function_Parameter{ Description: "test", - Type: []byte(`"bool"`), }, }, "DescriptionKind": { in: &tfprotov6.FunctionParameter{ DescriptionKind: tfprotov6.StringKindMarkdown, - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin6.Function_Parameter{ DescriptionKind: tfplugin6.StringKind_MARKDOWN, - Type: []byte(`"bool"`), }, }, "Name": { in: &tfprotov6.FunctionParameter{ Name: "test", - // Type will no longer be required in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - Type: tftypes.Bool, }, expected: &tfplugin6.Function_Parameter{ Name: "test", - Type: []byte(`"bool"`), }, }, "Type": { @@ -337,10 +272,7 @@ func TestFunction_Parameter(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it will be removed - // in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Function_Parameter(testCase.in) + got := toproto.Function_Parameter(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -370,7 +302,7 @@ func TestFunction_Return(t *testing.T) { }, "zero": { in: &tfprotov6.FunctionReturn{}, - expected: nil, + expected: &tfplugin6.Function_Return{}, }, "Type": { in: &tfprotov6.FunctionReturn{ @@ -388,10 +320,7 @@ func TestFunction_Return(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it will be removed - // in a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Function_Return(testCase.in) + got := toproto.Function_Return(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -469,11 +398,7 @@ func TestGetFunctions_Response(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.GetFunctions_Response(testCase.in) + got := toproto.GetFunctions_Response(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than diff --git a/tfprotov6/internal/toproto/provider.go b/tfprotov6/internal/toproto/provider.go index cb89d6a1..7b283c9d 100644 --- a/tfprotov6/internal/toproto/provider.go +++ b/tfprotov6/internal/toproto/provider.go @@ -4,8 +4,6 @@ package toproto import ( - "fmt" - "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tfprotov6/internal/tfplugin6" ) @@ -38,64 +36,34 @@ func GetMetadata_Response(in *tfprotov6.GetMetadataResponse) *tfplugin6.GetMetad return resp } -func GetProviderSchema_Response(in *tfprotov6.GetProviderSchemaResponse) (*tfplugin6.GetProviderSchema_Response, error) { +func GetProviderSchema_Response(in *tfprotov6.GetProviderSchemaResponse) *tfplugin6.GetProviderSchema_Response { if in == nil { - return nil, nil - } - - provider, err := Schema(in.Provider) - - if err != nil { - return nil, fmt.Errorf("error marshaling provider schema: %w", err) - } - - providerMeta, err := Schema(in.ProviderMeta) - - if err != nil { - return nil, fmt.Errorf("error marshaling provider_meta schema: %w", err) + return nil } resp := &tfplugin6.GetProviderSchema_Response{ DataSourceSchemas: make(map[string]*tfplugin6.Schema, len(in.DataSourceSchemas)), Diagnostics: Diagnostics(in.Diagnostics), Functions: make(map[string]*tfplugin6.Function, len(in.Functions)), - Provider: provider, - ProviderMeta: providerMeta, + Provider: Schema(in.Provider), + ProviderMeta: Schema(in.ProviderMeta), ResourceSchemas: make(map[string]*tfplugin6.Schema, len(in.ResourceSchemas)), ServerCapabilities: ServerCapabilities(in.ServerCapabilities), } - for k, v := range in.ResourceSchemas { - schema, err := Schema(v) - - if err != nil { - return nil, fmt.Errorf("error marshaling resource schema for %q: %w", k, err) - } - - resp.ResourceSchemas[k] = schema + for name, schema := range in.ResourceSchemas { + resp.ResourceSchemas[name] = Schema(schema) } - for k, v := range in.DataSourceSchemas { - schema, err := Schema(v) - - if err != nil { - return nil, fmt.Errorf("error marshaling data source schema for %q: %w", k, err) - } - - resp.DataSourceSchemas[k] = schema + for name, schema := range in.DataSourceSchemas { + resp.DataSourceSchemas[name] = Schema(schema) } - for name, functionPtr := range in.Functions { - function, err := Function(functionPtr) - - if err != nil { - return nil, fmt.Errorf("error marshaling function definition for %q: %w", name, err) - } - - resp.Functions[name] = function + for name, function := range in.Functions { + resp.Functions[name] = Function(function) } - return resp, nil + return resp } func ValidateProviderConfig_Response(in *tfprotov6.ValidateProviderConfigResponse) *tfplugin6.ValidateProviderConfig_Response { diff --git a/tfprotov6/internal/toproto/provider_test.go b/tfprotov6/internal/toproto/provider_test.go index 2cf63a2b..728a7945 100644 --- a/tfprotov6/internal/toproto/provider_test.go +++ b/tfprotov6/internal/toproto/provider_test.go @@ -412,11 +412,7 @@ func TestGetProviderSchema_Response(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.GetProviderSchema_Response(testCase.in) + got := toproto.GetProviderSchema_Response(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than diff --git a/tfprotov6/internal/toproto/schema.go b/tfprotov6/internal/toproto/schema.go index bb13db64..fb46bd67 100644 --- a/tfprotov6/internal/toproto/schema.go +++ b/tfprotov6/internal/toproto/schema.go @@ -4,75 +4,43 @@ package toproto import ( - "fmt" - "github.com/hashicorp/terraform-plugin-go/tfprotov6" "github.com/hashicorp/terraform-plugin-go/tfprotov6/internal/tfplugin6" ) -func Schema(in *tfprotov6.Schema) (*tfplugin6.Schema, error) { +func Schema(in *tfprotov6.Schema) *tfplugin6.Schema { if in == nil { - return nil, nil - } - - block, err := Schema_Block(in.Block) - - if err != nil { - return nil, fmt.Errorf("error marshalling block: %w", err) + return nil } resp := &tfplugin6.Schema{ - Block: block, + Block: Schema_Block(in.Block), Version: in.Version, } - return resp, nil + return resp } -func Schema_Block(in *tfprotov6.SchemaBlock) (*tfplugin6.Schema_Block, error) { +func Schema_Block(in *tfprotov6.SchemaBlock) *tfplugin6.Schema_Block { if in == nil { - return nil, nil - } - - attributes, err := Schema_Attributes(in.Attributes) - - if err != nil { - return nil, err - } - - blockTypes, err := Schema_NestedBlocks(in.BlockTypes) - - if err != nil { - return nil, err + return nil } resp := &tfplugin6.Schema_Block{ - Attributes: attributes, - BlockTypes: blockTypes, + Attributes: Schema_Attributes(in.Attributes), + BlockTypes: Schema_NestedBlocks(in.BlockTypes), Deprecated: in.Deprecated, Description: in.Description, DescriptionKind: StringKind(in.DescriptionKind), Version: in.Version, } - return resp, nil + return resp } -func Schema_Attribute(in *tfprotov6.SchemaAttribute) (*tfplugin6.Schema_Attribute, error) { +func Schema_Attribute(in *tfprotov6.SchemaAttribute) *tfplugin6.Schema_Attribute { if in == nil { - return nil, nil - } - - typ, err := CtyType(in.Type) - - if err != nil { - return nil, fmt.Errorf("error marshaling %q type to JSON: %w", in.Name, err) - } - - nestedType, err := Schema_Object(in.NestedType) - - if err != nil { - return nil, fmt.Errorf("error marshaling %q nested type to JSON: %w", in.Name, err) + return nil } resp := &tfplugin6.Schema_Attribute{ @@ -81,68 +49,50 @@ func Schema_Attribute(in *tfprotov6.SchemaAttribute) (*tfplugin6.Schema_Attribut Description: in.Description, DescriptionKind: StringKind(in.DescriptionKind), Name: in.Name, - NestedType: nestedType, + NestedType: Schema_Object(in.NestedType), Optional: in.Optional, Required: in.Required, Sensitive: in.Sensitive, - Type: typ, + Type: CtyType(in.Type), } - return resp, nil + return resp } -func Schema_Attributes(in []*tfprotov6.SchemaAttribute) ([]*tfplugin6.Schema_Attribute, error) { +func Schema_Attributes(in []*tfprotov6.SchemaAttribute) []*tfplugin6.Schema_Attribute { resp := make([]*tfplugin6.Schema_Attribute, 0, len(in)) for _, a := range in { - attr, err := Schema_Attribute(a) - - if err != nil { - return nil, err - } - - resp = append(resp, attr) + resp = append(resp, Schema_Attribute(a)) } - return resp, nil + return resp } -func Schema_NestedBlock(in *tfprotov6.SchemaNestedBlock) (*tfplugin6.Schema_NestedBlock, error) { +func Schema_NestedBlock(in *tfprotov6.SchemaNestedBlock) *tfplugin6.Schema_NestedBlock { if in == nil { - return nil, nil - } - - block, err := Schema_Block(in.Block) - - if err != nil { - return nil, fmt.Errorf("error marshaling %q nested block: %w", in.TypeName, err) + return nil } resp := &tfplugin6.Schema_NestedBlock{ - Block: block, + Block: Schema_Block(in.Block), MaxItems: in.MaxItems, MinItems: in.MinItems, Nesting: Schema_NestedBlock_NestingMode(in.Nesting), TypeName: in.TypeName, } - return resp, nil + return resp } -func Schema_NestedBlocks(in []*tfprotov6.SchemaNestedBlock) ([]*tfplugin6.Schema_NestedBlock, error) { +func Schema_NestedBlocks(in []*tfprotov6.SchemaNestedBlock) []*tfplugin6.Schema_NestedBlock { resp := make([]*tfplugin6.Schema_NestedBlock, 0, len(in)) for _, b := range in { - block, err := Schema_NestedBlock(b) - - if err != nil { - return nil, err - } - - resp = append(resp, block) + resp = append(resp, Schema_NestedBlock(b)) } - return resp, nil + return resp } func Schema_NestedBlock_NestingMode(in tfprotov6.SchemaNestedBlockNestingMode) tfplugin6.Schema_NestedBlock_NestingMode { @@ -153,21 +103,15 @@ func Schema_Object_NestingMode(in tfprotov6.SchemaObjectNestingMode) tfplugin6.S return tfplugin6.Schema_Object_NestingMode(in) } -func Schema_Object(in *tfprotov6.SchemaObject) (*tfplugin6.Schema_Object, error) { +func Schema_Object(in *tfprotov6.SchemaObject) *tfplugin6.Schema_Object { if in == nil { - return nil, nil - } - - attributes, err := Schema_Attributes(in.Attributes) - - if err != nil { - return nil, err + return nil } resp := &tfplugin6.Schema_Object{ - Attributes: attributes, + Attributes: Schema_Attributes(in.Attributes), Nesting: Schema_Object_NestingMode(in.Nesting), } - return resp, nil + return resp } diff --git a/tfprotov6/internal/toproto/schema_test.go b/tfprotov6/internal/toproto/schema_test.go index 7a68286c..aa8acb3d 100644 --- a/tfprotov6/internal/toproto/schema_test.go +++ b/tfprotov6/internal/toproto/schema_test.go @@ -66,11 +66,7 @@ func TestSchema(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema(testCase.in) + got := toproto.Schema(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -207,11 +203,7 @@ func TestSchema_Attribute(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_Attribute(testCase.in) + got := toproto.Schema_Attribute(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -282,11 +274,7 @@ func TestSchema_Attributes(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_Attributes(testCase.in) + got := toproto.Schema_Attributes(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -403,11 +391,7 @@ func TestSchema_Block(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_Block(testCase.in) + got := toproto.Schema_Block(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -502,11 +486,7 @@ func TestSchema_NestedBlock(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_NestedBlock(testCase.in) + got := toproto.Schema_NestedBlock(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -578,11 +558,7 @@ func TestSchema_NestedBlocks(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_NestedBlocks(testCase.in) + got := toproto.Schema_NestedBlocks(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than @@ -649,11 +625,7 @@ func TestSchema_Object(t *testing.T) { t.Run(name, func(t *testing.T) { t.Parallel() - // Intentionally not checking the error return as it is impossible - // to implement a test case which would raise an error. This return - // will be removed in preference of a panic a future change. - // Reference: https://github.com/hashicorp/terraform-plugin-go/issues/365 - got, _ := toproto.Schema_Object(testCase.in) + got := toproto.Schema_Object(testCase.in) // Protocol Buffers generated types must have unexported fields // ignored or cmp.Diff() will raise an error. This is easier than diff --git a/tfprotov6/tf6server/server.go b/tfprotov6/tf6server/server.go index cc96518b..2a59db5e 100644 --- a/tfprotov6/tf6server/server.go +++ b/tfprotov6/tf6server/server.go @@ -535,12 +535,7 @@ func (s *server) GetProviderSchema(ctx context.Context, protoReq *tfplugin6.GetP tf6serverlogging.DownstreamResponse(ctx, resp.Diagnostics) tf6serverlogging.ServerCapabilities(ctx, resp.ServerCapabilities) - protoResp, err := toproto.GetProviderSchema_Response(resp) - - if err != nil { - logging.ProtocolError(ctx, "Error converting response to protobuf", map[string]interface{}{logging.KeyError: err}) - return nil, err - } + protoResp := toproto.GetProviderSchema_Response(resp) return protoResp, nil } @@ -1043,12 +1038,7 @@ func (s *server) GetFunctions(ctx context.Context, protoReq *tfplugin6.GetFuncti tf6serverlogging.DownstreamResponse(ctx, resp.Diagnostics) - protoResp, err := toproto.GetFunctions_Response(resp) - - if err != nil { - logging.ProtocolError(ctx, "Error converting response to protobuf", map[string]any{logging.KeyError: err}) - return nil, err - } + protoResp := toproto.GetFunctions_Response(resp) return protoResp, nil } diff --git a/tftypes/list.go b/tftypes/list.go index 512c7c90..fdf9b1db 100644 --- a/tftypes/list.go +++ b/tftypes/list.go @@ -4,6 +4,7 @@ package tftypes import ( + "bytes" "fmt" ) @@ -114,9 +115,15 @@ func valueFromList(typ Type, in interface{}) (Value, error) { // // Deprecated: this is not meant to be called by third-party code. func (l List) MarshalJSON() ([]byte, error) { - elementType, err := l.ElementType.MarshalJSON() - if err != nil { - return nil, fmt.Errorf("error marshaling tftypes.List's element type %T to JSON: %w", l.ElementType, err) - } - return []byte(`["list",` + string(elementType) + `]`), nil + var buf bytes.Buffer + + buf.WriteString(`["list",`) + + // MarshalJSON is always error safe + elementTypeBytes, _ := l.ElementType.MarshalJSON() + + buf.Write(elementTypeBytes) + buf.WriteString(`]`) + + return buf.Bytes(), nil } diff --git a/tftypes/map.go b/tftypes/map.go index 0d299dcb..cfcab04f 100644 --- a/tftypes/map.go +++ b/tftypes/map.go @@ -4,6 +4,7 @@ package tftypes import ( + "bytes" "fmt" "sort" ) @@ -87,11 +88,17 @@ func (m Map) supportedGoTypes() []string { // // Deprecated: this is not meant to be called by third-party code. func (m Map) MarshalJSON() ([]byte, error) { - attributeType, err := m.ElementType.MarshalJSON() - if err != nil { - return nil, fmt.Errorf("error marshaling tftypes.Map's attribute type %T to JSON: %w", m.ElementType, err) - } - return []byte(`["map",` + string(attributeType) + `]`), nil + var buf bytes.Buffer + + buf.WriteString(`["map",`) + + // MarshalJSON is always error safe + elementTypeBytes, _ := m.ElementType.MarshalJSON() + + buf.Write(elementTypeBytes) + buf.WriteString(`]`) + + return buf.Bytes(), nil } func valueFromMap(typ Type, in interface{}) (Value, error) { diff --git a/tftypes/object.go b/tftypes/object.go index b901c99e..0e340bee 100644 --- a/tftypes/object.go +++ b/tftypes/object.go @@ -4,6 +4,7 @@ package tftypes import ( + "bytes" "encoding/json" "fmt" "sort" @@ -230,27 +231,89 @@ func valueFromObject(types map[string]Type, optionalAttrs map[string]struct{}, i } // MarshalJSON returns a JSON representation of the full type signature of `o`, -// including the AttributeTypes. +// including the AttributeTypes and, if present, OptionalAttributes. // // Deprecated: this is not meant to be called by third-party code. func (o Object) MarshalJSON() ([]byte, error) { - attrs, err := json.Marshal(o.AttributeTypes) - if err != nil { - return nil, err + var buf bytes.Buffer + + buf.WriteString(`["object",{`) + + attributeTypeNames := make([]string, 0, len(o.AttributeTypes)) + + for attributeTypeName := range o.AttributeTypes { + attributeTypeNames = append(attributeTypeNames, attributeTypeName) } - var optionalAttrs []byte + + // Ensure consistent ordering for human readability and unit testing. + // The slices package was introduced in Go 1.21, so it is not usable until + // this Go module is updated to Go 1.21 minimum. + sort.Strings(attributeTypeNames) + + for index, attributeTypeName := range attributeTypeNames { + if index > 0 { + buf.WriteString(`,`) + } + + buf.Write(marshalJSONObjectAttributeName(attributeTypeName)) + buf.WriteString(`:`) + + // MarshalJSON is always error safe + attributeTypeBytes, _ := o.AttributeTypes[attributeTypeName].MarshalJSON() + + buf.Write(attributeTypeBytes) + } + + buf.WriteString(`}`) + if len(o.OptionalAttributes) > 0 { - optionalAttrs = append(optionalAttrs, []byte(",")...) - names := make([]string, 0, len(o.OptionalAttributes)) - for k := range o.OptionalAttributes { - names = append(names, k) + buf.WriteString(`,[`) + + optionalAttributeNames := make([]string, 0, len(o.OptionalAttributes)) + + for optionalAttributeName := range o.OptionalAttributes { + optionalAttributeNames = append(optionalAttributeNames, optionalAttributeName) } - sort.Strings(names) - optionalsJSON, err := json.Marshal(names) - if err != nil { - return nil, err + + // Ensure consistent ordering for human readability and unit testing. + // The slices package was introduced in Go 1.21, so it is not usable + // until this Go module is updated to Go 1.21 minimum. + sort.Strings(optionalAttributeNames) + + for index, optionalAttributeName := range optionalAttributeNames { + if index > 0 { + buf.WriteString(`,`) + } + + buf.Write(marshalJSONObjectAttributeName(optionalAttributeName)) } - optionalAttrs = append(optionalAttrs, optionalsJSON...) + + buf.WriteString(`]`) } - return []byte(`["object",` + string(attrs) + string(optionalAttrs) + `]`), nil + + buf.WriteString(`]`) + + return buf.Bytes(), nil +} + +// marshalJSONObjectAttributeName an object attribute name string into JSON or +// panics. +// +// JSON encoding a string has some non-trivial rules and go-cty already depends +// on the Go standard library for this, so for now this logic also offloads this +// effort the same way to handle user input. As of Go 1.21, it is not possible +// for a caller to input something that would trigger an encoding error. There +// is FuzzMarshalJSONObjectAttributeName to verify this assertion. +// +// If a panic can be induced, a Type Validate() method or requiring the use of +// Type construction functions that require validation are better solutions than +// handling validation errors at this point. +func marshalJSONObjectAttributeName(name string) []byte { + result, err := json.Marshal(name) + + if err != nil { + panic(fmt.Sprintf("unable to JSON encode object attribute name: %s", name)) + } + + return result } diff --git a/tftypes/object_test.go b/tftypes/object_test.go index 28337b4c..09ea71cd 100644 --- a/tftypes/object_test.go +++ b/tftypes/object_test.go @@ -4,8 +4,10 @@ package tftypes import ( + "encoding/json" "errors" "testing" + "unicode/utf8" "github.com/google/go-cmp/cmp" ) @@ -652,3 +654,49 @@ func TestObjectUsableAs(t *testing.T) { }) } } + +func FuzzMarshalJSONObjectAttributeName(f *testing.F) { + // NOTE: Seed comments are placed above due to incorrect go fmt alignment + // behavior when mixing single and multiple byte characters. + seeds := []string{ + "ASCII", + "こんにちは", + // "ffl" is a single-character ligature + "baffle", + // These "e" have multiple combining diacritics + "wé́́é́́é́́!", + // Astral-plane characters + "😸😾", + // neither byte is valid UTF-8 + "\xff\xff", + // invalid bytes interleaved with single byte characters + "t\xffe\xffst", + // invalid bytes interleaved with multiple byte sequences + "\xffこんにちは\xffこんにちは", + } + + for _, seed := range seeds { + f.Add(seed) + } + + f.Fuzz(func(t *testing.T, name string) { + got := marshalJSONObjectAttributeName(name) + + // The result should always unmarshal to the same string, when given + // valid UTF-8. This function is not and should not be concerned with + // validating the given input. + if !utf8.ValidString(name) { + return + } + + var unmarshaled string + + if err := json.Unmarshal(got, &unmarshaled); err != nil { + t.Fatalf("failed to unmarshal: %s", err) + } + + if unmarshaled != name { + t.Fatalf("expected %q, got %q", name, unmarshaled) + } + }) +} diff --git a/tftypes/primitive.go b/tftypes/primitive.go index 5a631bae..1a604281 100644 --- a/tftypes/primitive.go +++ b/tftypes/primitive.go @@ -86,7 +86,10 @@ func (p primitive) MarshalJSON() ([]byte, error) { case DynamicPseudoType.name: return []byte(`"dynamic"`), nil } - return nil, fmt.Errorf("unknown primitive type %q", p) + + // MarshalJSON should always be error safe and reaching this panic implies + // a new primitive type was added that needs to be handled above. + panic(fmt.Sprintf("unimplemented tftypes.primitive type: %+v", p)) } func (p primitive) supportedGoTypes() []string { diff --git a/tftypes/set.go b/tftypes/set.go index e4549b59..1865c1fa 100644 --- a/tftypes/set.go +++ b/tftypes/set.go @@ -4,6 +4,7 @@ package tftypes import ( + "bytes" "fmt" ) @@ -110,9 +111,15 @@ func valueFromSet(typ Type, in interface{}) (Value, error) { // // Deprecated: this is not meant to be called by third-party code. func (s Set) MarshalJSON() ([]byte, error) { - elementType, err := s.ElementType.MarshalJSON() - if err != nil { - return nil, fmt.Errorf("error marshaling tftypes.Set's element type %T to JSON: %w", s.ElementType, err) - } - return []byte(`["set",` + string(elementType) + `]`), nil + var buf bytes.Buffer + + buf.WriteString(`["set",`) + + // MarshalJSON is always error safe + elementTypeBytes, _ := s.ElementType.MarshalJSON() + + buf.Write(elementTypeBytes) + buf.WriteString(`]`) + + return buf.Bytes(), nil } diff --git a/tftypes/tuple.go b/tftypes/tuple.go index a5c5e523..9e735dce 100644 --- a/tftypes/tuple.go +++ b/tftypes/tuple.go @@ -4,7 +4,7 @@ package tftypes import ( - "encoding/json" + "bytes" "fmt" "strings" ) @@ -148,9 +148,22 @@ func valueFromTuple(types []Type, in interface{}) (Value, error) { // // Deprecated: this is not meant to be called by third-party code. func (tu Tuple) MarshalJSON() ([]byte, error) { - elements, err := json.Marshal(tu.ElementTypes) - if err != nil { - return nil, err + var buf bytes.Buffer + + buf.WriteString(`["tuple",[`) + + for index, elementType := range tu.ElementTypes { + if index > 0 { + buf.WriteString(",") + } + + // MarshalJSON is always error safe + elementTypeBytes, _ := elementType.MarshalJSON() + + buf.Write(elementTypeBytes) } - return []byte(`["tuple",` + string(elements) + `]`), nil + + buf.WriteString(`]]`) + + return buf.Bytes(), nil } diff --git a/tftypes/type.go b/tftypes/type.go index c5965992..be0749dc 100644 --- a/tftypes/type.go +++ b/tftypes/type.go @@ -43,7 +43,7 @@ type Type interface { // MarshalJSON returns a JSON representation of the Type's signature. // It is modeled based on Terraform's requirements for type signature // JSON representations, and may change over time to match Terraform's - // formatting. + // formatting. The error return should always be nil. // // Deprecated: this is not meant to be called by third-party code. MarshalJSON() ([]byte, error) diff --git a/tftypes/type_json_test.go b/tftypes/type_json_test.go index cc90c9d3..52d69126 100644 --- a/tftypes/type_json_test.go +++ b/tftypes/type_json_test.go @@ -60,6 +60,27 @@ func TestTypeJSON(t *testing.T) { json: `["map","string"]`, typ: Map{ElementType: String}, }, + "object-empty": { + json: `["object",{}]`, + typ: Object{AttributeTypes: map[string]Type{}}, + }, + "object-string": { + json: `["object",{"test":"string"}]`, + typ: Object{AttributeTypes: map[string]Type{ + "test": String, + }}, + }, + "object-string-optional": { + json: `["object",{"test":"string"},["test"]]`, + typ: Object{ + AttributeTypes: map[string]Type{ + "test": String, + }, + OptionalAttributes: map[string]struct{}{ + "test": {}, + }, + }, + }, "object-string_number_bool": { json: `["object",{"foo":"string","bar":"number","baz":"bool"}]`, typ: Object{AttributeTypes: map[string]Type{ @@ -110,6 +131,16 @@ func TestTypeJSON(t *testing.T) { }, }, }, + "tuple-empty": { + json: `["tuple",[]]`, + typ: Tuple{ElementTypes: []Type{}}, + }, + "tuple-string": { + json: `["tuple",["string"]]`, + typ: Tuple{ElementTypes: []Type{ + String, + }}, + }, "tuple-string_number_bool": { json: `["tuple",["string","number","bool"]]`, typ: Tuple{ElementTypes: []Type{