Skip to content

Commit

Permalink
Merge pull request hashicorp#34603 from hashicorp/jbardin/remove-prov…
Browse files Browse the repository at this point in the history 
…ider-funtion-warnings

provider functions can only return an error
  • Loading branch information
@jbardin
jbardin authored Feb 27, 2024
2 parents 31a7fa8 + a8701f6 commit 300f66b
Show file tree
Hide file tree
Showing 12 changed files with 2,322 additions and 2,134 deletions.
11 changes: 7 additions & 4 deletions docs/plugin-protocol/tfplugin5.5.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@ message Diagnostic {
string summary = 2;
string detail = 3;
AttributePath attribute = 4;
}

// function_argument is the positional function argument for aligning
// configuration source.
optional int64 function_argument = 5;
message FunctionError {
string text = 1;
// The optional function_argument records the index position of the
// argument which caused the error.
optional int64 function_argument = 2;
}

message AttributePath {
Expand Down Expand Up @@ -569,6 +572,6 @@ message CallFunction {
}
message Response {
DynamicValue result = 1;
repeated Diagnostic diagnostics = 2;
FunctionError error = 2;
}
}
11 changes: 7 additions & 4 deletions docs/plugin-protocol/tfplugin6.5.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,13 @@ message Diagnostic {
string summary = 2;
string detail = 3;
AttributePath attribute = 4;
}

// function_argument is the positional function argument for aligning
// configuration source.
optional int64 function_argument = 5;
message FunctionError {
string text = 1;
// The optional function_argument records the index position of the
// argument which caused the error.
optional int64 function_argument = 2;
}

message AttributePath {
Expand Down Expand Up @@ -549,6 +552,6 @@ message CallFunction {
}
message Response {
DynamicValue result = 1;
repeated Diagnostic diagnostics = 2;
FunctionError error = 2;
}
}
35 changes: 27 additions & 8 deletions internal/grpcwrap/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ package grpcwrap

import (
"context"
"fmt"

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

var argTy cty.Type
if i < len(funcSchema.Parameters) {
argTy = funcSchema.Parameters[i].Type
} else {
if funcSchema.VariadicParameter == nil {
resp.Diagnostics = convert.AppendProtoDiag(
resp.Diagnostics, fmt.Errorf("too many arguments for non-variadic function"),
)

resp.Error = &tfplugin5.FunctionError{
Text: "too many arguments for non-variadic function",
FunctionArgument: &idx,
}
return resp, nil
}
argTy = funcSchema.VariadicParameter.Type
}

argVal, err := decodeDynamicValue(rawArg, argTy)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
resp.Error = &tfplugin5.FunctionError{
Text: err.Error(),
FunctionArgument: &idx,
}
return resp, nil
}

args[i] = argVal
}
}
Expand All @@ -471,14 +478,26 @@ func (p *provider) CallFunction(_ context.Context, req *tfplugin5.CallFunction_R
FunctionName: req.Name,
Arguments: args,
})
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, callResp.Diagnostics)
if callResp.Diagnostics.HasErrors() {

if callResp.Err != nil {
resp.Error = &tfplugin5.FunctionError{
Text: callResp.Err.Error(),
}

if argErr, ok := callResp.Err.(function.ArgError); ok {
idx := int64(argErr.Index)
resp.Error.FunctionArgument = &idx
}

return resp, nil
}

resp.Result, err = encodeDynamicValue(callResp.Result, funcSchema.ReturnType)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
resp.Error = &tfplugin5.FunctionError{
Text: err.Error(),
}

return resp, nil
}

Expand Down
33 changes: 25 additions & 8 deletions internal/grpcwrap/provider6.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,9 @@ package grpcwrap

import (
"context"
"fmt"

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

var argTy cty.Type
if i < len(funcSchema.Parameters) {
argTy = funcSchema.Parameters[i].Type
} else {
if funcSchema.VariadicParameter == nil {
resp.Diagnostics = convert.AppendProtoDiag(
resp.Diagnostics, fmt.Errorf("too many arguments for non-variadic function"),
)
resp.Error = &tfplugin6.FunctionError{
Text: "too many arguments for non-variadic function",
FunctionArgument: &idx,
}
return resp, nil
}
argTy = funcSchema.VariadicParameter.Type
}

argVal, err := decodeDynamicValue6(rawArg, argTy)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
resp.Error = &tfplugin6.FunctionError{
Text: err.Error(),
FunctionArgument: &idx,
}
return resp, nil
}

args[i] = argVal
}
}
Expand All @@ -472,14 +478,25 @@ func (p *provider6) CallFunction(_ context.Context, req *tfplugin6.CallFunction_
FunctionName: req.Name,
Arguments: args,
})
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, callResp.Diagnostics)
if callResp.Diagnostics.HasErrors() {
if callResp.Err != nil {
resp.Error = &tfplugin6.FunctionError{
Text: callResp.Err.Error(),
}

if argErr, ok := callResp.Err.(function.ArgError); ok {
idx := int64(argErr.Index)
resp.Error.FunctionArgument = &idx
}

return resp, nil
}

resp.Result, err = encodeDynamicValue6(callResp.Result, funcSchema.ReturnType)
if err != nil {
resp.Diagnostics = convert.AppendProtoDiag(resp.Diagnostics, err)
resp.Error = &tfplugin6.FunctionError{
Text: err.Error(),
}

return resp, nil
}

Expand Down
30 changes: 21 additions & 9 deletions internal/plugin/grpc_provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/zclconf/go-cty/cty"

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

schema := p.GetProviderSchema()
if schema.Diagnostics.HasErrors() {
resp.Diagnostics = schema.Diagnostics
resp.Err = schema.Diagnostics.Err()
return resp
}

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

argValRaw, err := msgpack.Marshal(argVal, paramDecl.Type)
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(err)
resp.Err = err
return resp
}
args[i] = &proto.DynamicValue{
Expand All @@ -800,17 +801,28 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
Arguments: args,
})
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err))
// functions can only support simple errors, but use our grpcError
// diagnostic function to format common problems is a more
// user-friendly manner.
resp.Err = grpcErr(err).Err()
return resp
}
resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
if resp.Diagnostics.HasErrors() {

if protoResp.Error != nil {
resp.Err = errors.New(protoResp.Error.Text)

// If this is a problem with a specific argument, we can wrap the error
// in a function.ArgError
if protoResp.Error.FunctionArgument != nil {
resp.Err = function.NewArgError(int(*protoResp.Error.FunctionArgument), resp.Err)
}

return resp
}

resultVal, err := decodeDynamicValue(protoResp.Result, funcDecl.ReturnType)
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(err)
resp.Err = err
return resp
}

Expand Down
30 changes: 21 additions & 9 deletions internal/plugin6/grpc_provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/zclconf/go-cty/cty"

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

schema := p.GetProviderSchema()
if schema.Diagnostics.HasErrors() {
resp.Diagnostics = schema.Diagnostics
resp.Err = schema.Diagnostics.Err()
return resp
}

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

argValRaw, err := msgpack.Marshal(argVal, paramDecl.Type)
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(err)
resp.Err = err
return resp
}
args[i] = &proto6.DynamicValue{
Expand All @@ -789,17 +790,28 @@ func (p *GRPCProvider) CallFunction(r providers.CallFunctionRequest) (resp provi
Arguments: args,
})
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(grpcErr(err))
// functions can only support simple errors, but use our grpcError
// diagnostic function to format common problems is a more
// user-friendly manner.
resp.Err = grpcErr(err).Err()
return resp
}
resp.Diagnostics = resp.Diagnostics.Append(convert.ProtoToDiagnostics(protoResp.Diagnostics))
if resp.Diagnostics.HasErrors() {

if protoResp.Error != nil {
resp.Err = errors.New(protoResp.Error.Text)

// If this is a problem with a specific argument, we can wrap the error
// in a function.ArgError
if protoResp.Error.FunctionArgument != nil {
resp.Err = function.NewArgError(int(*protoResp.Error.FunctionArgument), resp.Err)
}

return resp
}

resultVal, err := decodeDynamicValue(protoResp.Result, funcDecl.ReturnType)
if err != nil {
resp.Diagnostics = resp.Diagnostics.Append(err)
resp.Err = err
return resp
}

Expand Down
2 changes: 1 addition & 1 deletion internal/provider-simple-v6/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ func (s simple) ReadDataSource(req providers.ReadDataSourceRequest) (resp provid

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

Expand Down
7 changes: 2 additions & 5 deletions internal/providers/functions.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,11 +114,8 @@ func (d FunctionDecl) BuildFunction(providerAddr addrs.Provider, name string, re
FunctionName: name,
Arguments: args,
})
// NOTE: We don't actually have any way to surface warnings
// from the function here, because functions just return normal
// Go errors rather than diagnostics.
if resp.Diagnostics.HasErrors() {
return cty.UnknownVal(retType), resp.Diagnostics.Err()
if resp.Err != nil {
return cty.UnknownVal(retType), resp.Err
}

if resp.Result == cty.NilVal {
Expand Down
6 changes: 4 additions & 2 deletions internal/providers/provider.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,8 @@ type CallFunctionResponse struct {
// so can be left as cty.NilVal to represent the absense of a value.
Result cty.Value

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

0 comments on commit 300f66b

Please sign in to comment.