Skip to content

Commit 300f66b

Browse files
jbardinjbardin
authored
Merge pull request #34603 from hashicorp/jbardin/remove-provider-funtion-warnings
provider functions can only return an error
2 parents 31a7fa8 + a8701f6 commit 300f66b

File tree

12 files changed

+2322
-2134
lines changed

12 files changed

+2322
-2134
lines changed

‎docs/plugin-protocol/tfplugin5.5.proto‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,13 @@ message Diagnostic {
4141
string summary = 2;
4242
string detail = 3;
4343
AttributePath attribute = 4;
44+
}
4445

45-
// function_argument is the positional function argument for aligning
46-
// configuration source.
47-
optional int64 function_argument = 5;
46+
message FunctionError {
47+
string text = 1;
48+
// The optional function_argument records the index position of the
49+
// argument which caused the error.
50+
optional int64 function_argument = 2;
4851
}
4952

5053
message AttributePath {
@@ -569,6 +572,6 @@ message CallFunction {
569572
}
570573
message Response {
571574
DynamicValue result = 1;
572-
repeated Diagnostic diagnostics = 2;
575+
FunctionError error = 2;
573576
}
574577
}

‎docs/plugin-protocol/tfplugin6.5.proto‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,10 +41,13 @@ message Diagnostic {
4141
string summary = 2;
4242
string detail = 3;
4343
AttributePath attribute = 4;
44+
}
4445

45-
// function_argument is the positional function argument for aligning
46-
// configuration source.
47-
optional int64 function_argument = 5;
46+
message FunctionError {
47+
string text = 1;
48+
// The optional function_argument records the index position of the
49+
// argument which caused the error.
50+
optional int64 function_argument = 2;
4851
}
4952

5053
message AttributePath {
@@ -549,6 +552,6 @@ message CallFunction {
549552
}
550553
message Response {
551554
DynamicValue result = 1;
552-
repeated Diagnostic diagnostics = 2;
555+
FunctionError error = 2;
553556
}
554557
}

‎internal/grpcwrap/provider.go‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ package grpcwrap
55

66
import (
77
"context"
8-
"fmt"
98

109
"github.com/zclconf/go-cty/cty"
10+
"github.com/zclconf/go-cty/cty/function"
1111
ctyjson "github.com/zclconf/go-cty/cty/json"
1212
"github.com/zclconf/go-cty/cty/msgpack"
1313
"google.golang.org/grpc/codes"
@@ -444,25 +444,32 @@ func (p *provider) CallFunction(_ context.Context, req *tfplugin5.CallFunction_R
444444
if len(req.Arguments) != 0 {
445445
args = make([]cty.Value, len(req.Arguments))
446446
for i, rawArg := range req.Arguments {
447+
idx := int64(i)
447448

448449
var argTy cty.Type
449450
if i < len(funcSchema.Parameters) {
450451
argTy = funcSchema.Parameters[i].Type
451452
} else {
452453
if funcSchema.VariadicParameter == nil {
453-
resp.Diagnostics = convert.AppendProtoDiag(
454-
resp.Diagnostics, fmt.Errorf("too many arguments for non-variadic function"),
455-
)
454+
455+
resp.Error = &tfplugin5.FunctionError{
456+
Text: "too many arguments for non-variadic function",
457+
FunctionArgument: &idx,
458+
}
456459
return resp, nil
457460
}
458461
argTy = funcSchema.VariadicParameter.Type
459462
}
460463

461464
argVal, err := decodeDynamicValue(rawArg, argTy)
462465
if err != nil {
463-
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
466+
resp.Error = &tfplugin5.FunctionError{
467+
Text: err.Error(),
468+
FunctionArgument: &idx,
469+
}
464470
return resp, nil
465471
}
472+
466473
args[i] = argVal
467474
}
468475
}
@@ -471,14 +478,26 @@ func (p *provider) CallFunction(_ context.Context, req *tfplugin5.CallFunction_R
471478
FunctionName: req.Name,
472479
Arguments: args,
473480
})
474-
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, callResp.Diagnostics)
475-
if callResp.Diagnostics.HasErrors() {
481+
482+
if callResp.Err != nil {
483+
resp.Error = &tfplugin5.FunctionError{
484+
Text: callResp.Err.Error(),
485+
}
486+
487+
if argErr, ok := callResp.Err.(function.ArgError); ok {
488+
idx := int64(argErr.Index)
489+
resp.Error.FunctionArgument = &idx
490+
}
491+
476492
return resp, nil
477493
}
478494

479495
resp.Result, err = encodeDynamicValue(callResp.Result, funcSchema.ReturnType)
480496
if err != nil {
481-
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
497+
resp.Error = &tfplugin5.FunctionError{
498+
Text: err.Error(),
499+
}
500+
482501
return resp, nil
483502
}
484503

‎internal/grpcwrap/provider6.go‎

Lines changed: 25 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@ package grpcwrap
55

66
import (
77
"context"
8-
"fmt"
98

109
"github.com/zclconf/go-cty/cty"
10+
"github.com/zclconf/go-cty/cty/function"
1111
ctyjson "github.com/zclconf/go-cty/cty/json"
1212
"github.com/zclconf/go-cty/cty/msgpack"
1313
"google.golang.org/grpc/codes"
@@ -445,25 +445,31 @@ func (p *provider6) CallFunction(_ context.Context, req *tfplugin6.CallFunction_
445445
if len(req.Arguments) != 0 {
446446
args = make([]cty.Value, len(req.Arguments))
447447
for i, rawArg := range req.Arguments {
448+
idx := int64(i)
448449

449450
var argTy cty.Type
450451
if i < len(funcSchema.Parameters) {
451452
argTy = funcSchema.Parameters[i].Type
452453
} else {
453454
if funcSchema.VariadicParameter == nil {
454-
resp.Diagnostics = convert.AppendProtoDiag(
455-
resp.Diagnostics, fmt.Errorf("too many arguments for non-variadic function"),
456-
)
455+
resp.Error = &tfplugin6.FunctionError{
456+
Text: "too many arguments for non-variadic function",
457+
FunctionArgument: &idx,
458+
}
457459
return resp, nil
458460
}
459461
argTy = funcSchema.VariadicParameter.Type
460462
}
461463

462464
argVal, err := decodeDynamicValue6(rawArg, argTy)
463465
if err != nil {
464-
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
466+
resp.Error = &tfplugin6.FunctionError{
467+
Text: err.Error(),
468+
FunctionArgument: &idx,
469+
}
465470
return resp, nil
466471
}
472+
467473
args[i] = argVal
468474
}
469475
}
@@ -472,14 +478,25 @@ func (p *provider6) CallFunction(_ context.Context, req *tfplugin6.CallFunction_
472478
FunctionName: req.Name,
473479
Arguments: args,
474480
})
475-
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, callResp.Diagnostics)
476-
if callResp.Diagnostics.HasErrors() {
481+
if callResp.Err != nil {
482+
resp.Error = &tfplugin6.FunctionError{
483+
Text: callResp.Err.Error(),
484+
}
485+
486+
if argErr, ok := callResp.Err.(function.ArgError); ok {
487+
idx := int64(argErr.Index)
488+
resp.Error.FunctionArgument = &idx
489+
}
490+
477491
return resp, nil
478492
}
479493

480494
resp.Result, err = encodeDynamicValue6(callResp.Result, funcSchema.ReturnType)
481495
if err != nil {
482-
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
496+
resp.Error = &tfplugin6.FunctionError{
497+
Text: err.Error(),
498+
}
499+
483500
return resp, nil
484501
}
485502

‎internal/plugin/grpc_provider.go‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/zclconf/go-cty/cty"
1313

1414
plugin "github.com/hashicorp/go-plugin"
15+
"github.com/zclconf/go-cty/cty/function"
1516
ctyjson "github.com/zclconf/go-cty/cty/json"
1617
"github.com/zclconf/go-cty/cty/msgpack"
1718
"google.golang.org/grpc"
@@ -751,7 +752,7 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
751752

752753
schema := p.GetProviderSchema()
753754
if schema.Diagnostics.HasErrors() {
754-
resp.Diagnostics = schema.Diagnostics
755+
resp.Err = schema.Diagnostics.Err()
755756
return resp
756757
}
757758

@@ -765,15 +766,15 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
765766
// Should only get here if the caller has a bug, because we should
766767
// have detected earlier any attempt to call a function that the
767768
// provider didn't declare.
768-
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("provider has no function named %q", r.FunctionName))
769+
resp.Err = fmt.Errorf("provider has no function named %q", r.FunctionName)
769770
return resp
770771
}
771772
if len(r.Arguments) < len(funcDecl.Parameters) {
772-
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("not enough arguments for function %q", r.FunctionName))
773+
resp.Err = fmt.Errorf("not enough arguments for function %q", r.FunctionName)
773774
return resp
774775
}
775776
if funcDecl.VariadicParameter == nil && len(r.Arguments) > len(funcDecl.Parameters) {
776-
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("too many arguments for function %q", r.FunctionName))
777+
resp.Err = fmt.Errorf("too many arguments for function %q", r.FunctionName)
777778
return resp
778779
}
779780
args := make([]*proto.DynamicValue, len(r.Arguments))
@@ -787,7 +788,7 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
787788

788789
argValRaw, err := msgpack.Marshal(argVal, paramDecl.Type)
789790
if err != nil {
790-
resp.Diagnostics = resp.Diagnostics.Append(err)
791+
resp.Err = err
791792
return resp
792793
}
793794
args[i] = &proto.DynamicValue{
@@ -800,17 +801,28 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
800801
Arguments: args,
801802
})
802803
if err != nil {
803-
resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err))
804+
// functions can only support simple errors, but use our grpcError
805+
// diagnostic function to format common problems is a more
806+
// user-friendly manner.
807+
resp.Err = grpcErr(err).Err()
804808
return resp
805809
}
806-
resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
807-
if resp.Diagnostics.HasErrors() {
810+
811+
if protoResp.Error != nil {
812+
resp.Err = errors.New(protoResp.Error.Text)
813+
814+
// If this is a problem with a specific argument, we can wrap the error
815+
// in a function.ArgError
816+
if protoResp.Error.FunctionArgument != nil {
817+
resp.Err = function.NewArgError(int(*protoResp.Error.FunctionArgument), resp.Err)
818+
}
819+
808820
return resp
809821
}
810822

811823
resultVal, err := decodeDynamicValue(protoResp.Result, funcDecl.ReturnType)
812824
if err != nil {
813-
resp.Diagnostics = resp.Diagnostics.Append(err)
825+
resp.Err = err
814826
return resp
815827
}
816828

‎internal/plugin6/grpc_provider.go‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"github.com/zclconf/go-cty/cty"
1313

1414
plugin "github.com/hashicorp/go-plugin"
15+
"github.com/zclconf/go-cty/cty/function"
1516
ctyjson "github.com/zclconf/go-cty/cty/json"
1617
"github.com/zclconf/go-cty/cty/msgpack"
1718
"google.golang.org/grpc"
@@ -740,7 +741,7 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
740741

741742
schema := p.GetProviderSchema()
742743
if schema.Diagnostics.HasErrors() {
743-
resp.Diagnostics = schema.Diagnostics
744+
resp.Err = schema.Diagnostics.Err()
744745
return resp
745746
}
746747

@@ -754,15 +755,15 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
754755
// Should only get here if the caller has a bug, because we should
755756
// have detected earlier any attempt to call a function that the
756757
// provider didn't declare.
757-
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("provider has no function named %q", r.FunctionName))
758+
resp.Err = fmt.Errorf("provider has no function named %q", r.FunctionName)
758759
return resp
759760
}
760761
if len(r.Arguments) < len(funcDecl.Parameters) {
761-
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("not enough arguments for function %q", r.FunctionName))
762+
resp.Err = fmt.Errorf("not enough arguments for function %q", r.FunctionName)
762763
return resp
763764
}
764765
if funcDecl.VariadicParameter == nil && len(r.Arguments) > len(funcDecl.Parameters) {
765-
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("too many arguments for function %q", r.FunctionName))
766+
resp.Err = fmt.Errorf("too many arguments for function %q", r.FunctionName)
766767
return resp
767768
}
768769
args := make([]*proto6.DynamicValue, len(r.Arguments))
@@ -776,7 +777,7 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
776777

777778
argValRaw, err := msgpack.Marshal(argVal, paramDecl.Type)
778779
if err != nil {
779-
resp.Diagnostics = resp.Diagnostics.Append(err)
780+
resp.Err = err
780781
return resp
781782
}
782783
args[i] = &proto6.DynamicValue{
@@ -789,17 +790,28 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
789790
Arguments: args,
790791
})
791792
if err != nil {
792-
resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err))
793+
// functions can only support simple errors, but use our grpcError
794+
// diagnostic function to format common problems is a more
795+
// user-friendly manner.
796+
resp.Err = grpcErr(err).Err()
793797
return resp
794798
}
795-
resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
796-
if resp.Diagnostics.HasErrors() {
799+
800+
if protoResp.Error != nil {
801+
resp.Err = errors.New(protoResp.Error.Text)
802+
803+
// If this is a problem with a specific argument, we can wrap the error
804+
// in a function.ArgError
805+
if protoResp.Error.FunctionArgument != nil {
806+
resp.Err = function.NewArgError(int(*protoResp.Error.FunctionArgument), resp.Err)
807+
}
808+
797809
return resp
798810
}
799811

800812
resultVal, err := decodeDynamicValue(protoResp.Result, funcDecl.ReturnType)
801813
if err != nil {
802-
resp.Diagnostics = resp.Diagnostics.Append(err)
814+
resp.Err = err
803815
return resp
804816
}
805817

‎internal/provider-simple-v6/provider.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ func (s simple) ReadDataSource(req providers.ReadDataSourceRequest) (resp provid
173173

174174
func (s simple) CallFunction(req providers.CallFunctionRequest) (resp providers.CallFunctionResponse) {
175175
if req.FunctionName != "noop" {
176-
resp.Diagnostics = resp.Diagnostics.Append(fmt.Errorf("CallFunction for undefined function %q", req.FunctionName))
176+
resp.Err = fmt.Errorf("CallFunction for undefined function %q", req.FunctionName)
177177
return resp
178178
}
179179

‎internal/providers/functions.go‎

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -114,11 +114,8 @@ func (d FunctionDecl) BuildFunction(providerAddr addrs.Provider, name string, re
114114
FunctionName: name,
115115
Arguments: args,
116116
})
117-
// NOTE: We don't actually have any way to surface warnings
118-
// from the function here, because functions just return normal
119-
// Go errors rather than diagnostics.
120-
if resp.Diagnostics.HasErrors() {
121-
return cty.UnknownVal(retType), resp.Diagnostics.Err()
117+
if resp.Err != nil {
118+
return cty.UnknownVal(retType), resp.Err
122119
}
123120

124121
if resp.Result == cty.NilVal {

‎internal/providers/provider.go‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,8 @@ type CallFunctionResponse struct {
501501
// so can be left as cty.NilVal to represent the absense of a value.
502502
Result cty.Value
503503

504-
// Diagnostics contains any warnings or errors from the function call.
505-
Diagnostics tfdiags.Diagnostics
504+
// Err is the error value from the function call. This may be an instance
505+
// of function.ArgError from the go-cty package to specify a problem with a
506+
// specific argument.
507+
Err error
506508
}

0 commit comments

Comments
 (0)