all: Modify CallFunction implementations (#226) (#226)

Reference: hashicorp/terraform-plugin-go#380

The next versions of the plugin protocol (5.5/6.5) include support for the CallFunction RPC and server capability. This change includes a modified implementation of the new RPC in all server implementations which accounts for returning a FunctionError rather than Diagnostics.

.changes/unreleased/ENHANCEMENTS-20240223-094729.yaml

@@ -0,0 +1,6 @@
+kind: ENHANCEMENTS
+body: 'all: Upgrade protocol versions to support modified `CallFunction` RPC which
+  returns a FunctionError rather than Diagnostics'
+time: 2024-02-23T09:47:29.580412Z
+custom:
+  Issue: "226"

go.mod

@@ -1,12 +1,12 @@
 module github.com/hashicorp/terraform-plugin-mux
 
-go 1.20
+go 1.21
 
 require (
 	github.com/google/go-cmp v0.6.0
-	github.com/hashicorp/terraform-plugin-go v0.21.0
+	github.com/hashicorp/terraform-plugin-go v0.22.0
 	github.com/hashicorp/terraform-plugin-log v0.9.0
-	google.golang.org/grpc v1.61.0
+	google.golang.org/grpc v1.62.0
 )
 
 require (
@@ -24,9 +24,9 @@ require (
 	github.com/oklog/run v1.0.0 // indirect
 	github.com/vmihailenco/msgpack/v5 v5.4.1 // indirect
 	github.com/vmihailenco/tagparser/v2 v2.0.0 // indirect
-	golang.org/x/net v0.18.0 // indirect
-	golang.org/x/sys v0.14.0 // indirect
+	golang.org/x/net v0.20.0 // indirect
+	golang.org/x/sys v0.16.0 // indirect
 	golang.org/x/text v0.14.0 // indirect
-	google.golang.org/genproto/googleapis/rpc v0.0.0-20231106174013-bbf56f31fb17 // indirect
+	google.golang.org/genproto/googleapis/rpc v0.0.0-20240123012728-ef4313101c80 // indirect
 	google.golang.org/protobuf v1.32.0 // indirect
 )

go.sum

@@ -1,4 +1,5 @@
 github.com/bufbuild/protocompile v0.4.0 h1:LbFKd2XowZvQ/kajzguUp2DC9UEIQhIq77fZZlaQsNA=
+github.com/bufbuild/protocompile v0.4.0/go.mod h1:3v93+mbWn/v3xzN+31nwkJfrEpAUwp+BagBSZWx+TP8=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
@@ -16,8 +17,8 @@ github.com/hashicorp/go-plugin v1.6.0 h1:wgd4KxHJTVGGqWBq4QPB1i5BZNEx9BR8+OFmHDm
 github.com/hashicorp/go-plugin v1.6.0/go.mod h1:lBS5MtSSBZk0SHc66KACcjjlU6WzEVP/8pwz68aMkCI=
 github.com/hashicorp/go-uuid v1.0.3 h1:2gKiV6YVmrJ1i2CKKa9obLvRieoRGviZFL26PcT/Co8=
 github.com/hashicorp/go-uuid v1.0.3/go.mod h1:6SBZvOh/SIDV7/2o3Jml5SYk/TvGqwFJ/bN7x4byOro=
-github.com/hashicorp/terraform-plugin-go v0.21.0 h1:VSjdVQYNDKR0l2pi3vsFK1PdMQrw6vGOshJXMNFeVc0=
-github.com/hashicorp/terraform-plugin-go v0.21.0/go.mod h1:piJp8UmO1uupCvC9/H74l2C6IyKG0rW4FDedIpwW5RQ=
+github.com/hashicorp/terraform-plugin-go v0.22.0 h1:1OS1Jk5mO0f5hrziWJGXXIxBrMe2j/B8E+DVGw43Xmc=
+github.com/hashicorp/terraform-plugin-go v0.22.0/go.mod h1:mPULV91VKss7sik6KFEcEu7HuTogMLLO/EvWCuFkRVE=
 github.com/hashicorp/terraform-plugin-log v0.9.0 h1:i7hOA+vdAItN1/7UrfBqBwvYPQ9TFvymaRGZED3FCV0=
 github.com/hashicorp/terraform-plugin-log v0.9.0/go.mod h1:rKL8egZQ/eXSyDqzLUuwUYLVdlYeamldAHSxjUFADow=
 github.com/hashicorp/terraform-registry-address v0.2.3 h1:2TAiKJ1A3MAkZlH1YI/aTVcLZRu7JseiXNRHbOAyoTI=
@@ -27,6 +28,7 @@ github.com/hashicorp/terraform-svchost v0.1.1/go.mod h1:mNsjQfZyf/Jhz35v6/0LWcv2
 github.com/hashicorp/yamux v0.1.1 h1:yrQxtgseBDrq9Y652vSRDvsKCJKOUD+GzTS4Y0Y8pvE=
 github.com/hashicorp/yamux v0.1.1/go.mod h1:CtWFDAQgb7dxtzFs4tWbplKIe2jSi3+5vKbgIO0SLnQ=
 github.com/jhump/protoreflect v1.15.1 h1:HUMERORf3I3ZdX05WaQ6MIpd/NJ434hTp5YiKgfCL6c=
+github.com/jhump/protoreflect v1.15.1/go.mod h1:jD/2GMKKE6OqX8qTjhADU1e6DShO+gavG9e0Q693nKo=
 github.com/mattn/go-colorable v0.1.9/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc=
 github.com/mattn/go-colorable v0.1.12 h1:jF+Du6AlPIjs2BiUiQlKOX0rt3SujHxPnksPKZbaA40=
 github.com/mattn/go-colorable v0.1.12/go.mod h1:u5H1YNBxpqRaxsYJYSkiCWKzEfiAb1Gb520KVy5xxl4=
@@ -46,22 +48,22 @@ github.com/vmihailenco/msgpack/v5 v5.4.1 h1:cQriyiUvjTwOHg8QZaPihLWeRAAVoCpE00IU
 github.com/vmihailenco/msgpack/v5 v5.4.1/go.mod h1:GaZTsDaehaPpQVyxrf5mtQlH+pc21PIudVV/E3rRQok=
 github.com/vmihailenco/tagparser/v2 v2.0.0 h1:y09buUbR+b5aycVFQs/g70pqKVZNBmxwAhO7/IwNM9g=
 github.com/vmihailenco/tagparser/v2 v2.0.0/go.mod h1:Wri+At7QHww0WTrCBeu4J6bNtoV6mEfg5OIWRZA9qds=
-golang.org/x/net v0.18.0 h1:mIYleuAkSbHh0tCv7RvjL3F6ZVbLjq4+R7zbOn3Kokg=
-golang.org/x/net v0.18.0/go.mod h1:/czyP5RqHAH4odGYxBJ1qz0+CE5WZ+2j1YgoEo8F2jQ=
+golang.org/x/net v0.20.0 h1:aCL9BSgETF1k+blQaYUBx9hJ9LOGP3gAVemcZlf1Kpo=
+golang.org/x/net v0.20.0/go.mod h1:z8BVo6PvndSri0LbOE3hAn0apkU+1YvI6E70E9jsnvY=
 golang.org/x/sys v0.0.0-20200116001909-b77594299b42/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20200223170610-d5e6a3e2c0ae/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
 golang.org/x/sys v0.0.0-20210630005230-0f9fa26af87c/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20210927094055-39ccf1dd6fa6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.0.0-20220503163025-988cb79eb6c6/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
-golang.org/x/sys v0.14.0 h1:Vz7Qs629MkJkGyHxUlRHizWJRG2j8fbQKjELVSNhy7Q=
-golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.16.0 h1:xWw16ngr6ZMtmxDyKyIgsE93KNKz5HKmMa3b8ALHidU=
+golang.org/x/sys v0.16.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ=
 golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20231106174013-bbf56f31fb17 h1:Jyp0Hsi0bmHXG6k9eATXoYtjd6e2UzZ1SCn/wIupY14=
-google.golang.org/genproto/googleapis/rpc v0.0.0-20231106174013-bbf56f31fb17/go.mod h1:oQ5rr10WTTMvP4A36n8JpR1OrO1BEiV4f78CneXZxkA=
-google.golang.org/grpc v1.61.0 h1:TOvOcuXn30kRao+gfcvsebNEa5iZIiLkisYEkf7R7o0=
-google.golang.org/grpc v1.61.0/go.mod h1:VUbo7IFqmF1QtCAstipjG0GIoq49KvMe9+h1jFLBNJs=
+google.golang.org/genproto/googleapis/rpc v0.0.0-20240123012728-ef4313101c80 h1:AjyfHzEPEFp/NpvfN5g+KDla3EMojjhRVZc1i7cj+oM=
+google.golang.org/genproto/googleapis/rpc v0.0.0-20240123012728-ef4313101c80/go.mod h1:PAREbraiVEVGVdTZsVWjSbbTtSyGbAgIIvni8a8CD5s=
+google.golang.org/grpc v1.62.0 h1:HQKZ/fa1bXkX1oFOvSjmZEUL8wLSaZTjCcLAlmZRtdk=
+google.golang.org/grpc v1.62.0/go.mod h1:IWTG0VlJLCh1SkC58F7np9ka9mx/WNkjl4PGJaiq+QE=
 google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp09yW+WbY/TyQbw=
 google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
 google.golang.org/protobuf v1.32.0 h1:pPC6BG5ex8PDFnkbrGU3EixyhKcQ2aDuBS36lqK/C7I=

internal/tfprotov5tov6/tfprotov5tov6.go

@@ -59,8 +59,8 @@ func CallFunctionResponse(in *tfprotov5.CallFunctionResponse) *tfprotov6.CallFun
 	}
 
 	return &tfprotov6.CallFunctionResponse{
-		Diagnostics: Diagnostics(in.Diagnostics),
-		Result:      DynamicValue(in.Result),
+		Error:  FunctionError(in.Error),
+		Result: DynamicValue(in.Result),
 	}
 }
 
@@ -148,6 +148,19 @@ func Function(in *tfprotov5.Function) *tfprotov6.Function {
 	return out
 }
 
+func FunctionError(in *tfprotov5.FunctionError) *tfprotov6.FunctionError {
+	if in == nil {
+		return nil
+	}
+
+	out := &tfprotov6.FunctionError{
+		Text:             in.Text,
+		FunctionArgument: in.FunctionArgument,
+	}
+
+	return out
+}
+
 func FunctionMetadata(in tfprotov5.FunctionMetadata) tfprotov6.FunctionMetadata {
 	return tfprotov6.FunctionMetadata{
 		Name: in.Name,

internal/tfprotov5tov6/tfprotov5tov6_test.go

@@ -10,6 +10,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
+
 	"github.com/hashicorp/terraform-plugin-mux/internal/tfprotov5tov6"
 )
 
@@ -49,6 +50,16 @@ var (
 		},
 	}
 
+	testTfprotov5FunctionError *tfprotov5.FunctionError = &tfprotov5.FunctionError{
+		Text:             "test error",
+		FunctionArgument: pointer(int64(0)),
+	}
+
+	testTfprotov6FunctionError *tfprotov6.FunctionError = &tfprotov6.FunctionError{
+		Text:             "test error",
+		FunctionArgument: pointer(int64(0)),
+	}
+
 	testTfprotov5FunctionMetadata tfprotov5.FunctionMetadata = tfprotov5.FunctionMetadata{
 		Name: "test_function",
 	}
@@ -246,12 +257,12 @@ func TestCallFunctionResponse(t *testing.T) {
 		},
 		"all-valid-fields": {
 			in: &tfprotov5.CallFunctionResponse{
-				Diagnostics: testTfprotov5Diagnostics,
-				Result:      &testTfprotov5DynamicValue,
+				Error:  testTfprotov5FunctionError,
+				Result: &testTfprotov5DynamicValue,
 			},
 			expected: &tfprotov6.CallFunctionResponse{
-				Diagnostics: testTfprotov6Diagnostics,
-				Result:      &testTfprotov6DynamicValue,
+				Error:  testTfprotov6FunctionError,
+				Result: &testTfprotov6DynamicValue,
 			},
 		},
 	}
@@ -2037,3 +2048,7 @@ func TestValidateResourceConfigResponse(t *testing.T) {
 		})
 	}
 }
+
+func pointer[T any](value T) *T {
+	return &value
+}

internal/tfprotov6tov5/tfprotov6tov5.go

@@ -64,8 +64,8 @@ func CallFunctionResponse(in *tfprotov6.CallFunctionResponse) *tfprotov5.CallFun
 	}
 
 	return &tfprotov5.CallFunctionResponse{
-		Diagnostics: Diagnostics(in.Diagnostics),
-		Result:      DynamicValue(in.Result),
+		Error:  FunctionError(in.Error),
+		Result: DynamicValue(in.Result),
 	}
 }
 
@@ -153,6 +153,19 @@ func Function(in *tfprotov6.Function) *tfprotov5.Function {
 	return out
 }
 
+func FunctionError(in *tfprotov6.FunctionError) *tfprotov5.FunctionError {
+	if in == nil {
+		return nil
+	}
+
+	out := &tfprotov5.FunctionError{
+		Text:             in.Text,
+		FunctionArgument: in.FunctionArgument,
+	}
+
+	return out
+}
+
 func FunctionMetadata(in tfprotov6.FunctionMetadata) tfprotov5.FunctionMetadata {
 	return tfprotov5.FunctionMetadata{
 		Name: in.Name,

internal/tfprotov6tov5/tfprotov6tov5_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov6"
 	"github.com/hashicorp/terraform-plugin-go/tftypes"
+
 	"github.com/hashicorp/terraform-plugin-mux/internal/tfprotov6tov5"
 )
 
@@ -62,6 +63,16 @@ var (
 		},
 	}
 
+	testTfprotov5FunctionError *tfprotov5.FunctionError = &tfprotov5.FunctionError{
+		Text:             "test error",
+		FunctionArgument: pointer(int64(0)),
+	}
+
+	testTfprotov6FunctionError *tfprotov6.FunctionError = &tfprotov6.FunctionError{
+		Text:             "test error",
+		FunctionArgument: pointer(int64(0)),
+	}
+
 	testTfprotov6FunctionMetadata tfprotov6.FunctionMetadata = tfprotov6.FunctionMetadata{
 		Name: "test_function",
 	}
@@ -248,12 +259,12 @@ func TestCallFunctionResponse(t *testing.T) {
 		},
 		"all-valid-fields": {
 			in: &tfprotov6.CallFunctionResponse{
-				Diagnostics: testTfprotov6Diagnostics,
-				Result:      &testTfprotov6DynamicValue,
+				Error:  testTfprotov6FunctionError,
+				Result: &testTfprotov6DynamicValue,
 			},
 			expected: &tfprotov5.CallFunctionResponse{
-				Diagnostics: testTfprotov5Diagnostics,
-				Result:      &testTfprotov5DynamicValue,
+				Error:  testTfprotov5FunctionError,
+				Result: &testTfprotov5DynamicValue,
 			},
 		},
 	}
@@ -2286,3 +2297,7 @@ func TestValidateResourceTypeConfigResponse(t *testing.T) {
 		})
 	}
 }
+
+func pointer[T any](value T) *T {
+	return &value
+}

tf5muxserver/mux_server_CallFunction.go

@@ -5,8 +5,10 @@ package tf5muxserver
 
 import (
 	"context"
+	"fmt"
 
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
+
 	"github.com/hashicorp/terraform-plugin-mux/internal/logging"
 )
 
@@ -24,8 +26,22 @@ func (s *muxServer) CallFunction(ctx context.Context, req *tfprotov5.CallFunctio
 	}
 
 	if diagnosticsHasError(diags) {
+		var text string
+
+		for _, d := range diags {
+			if d.Severity == tfprotov5.DiagnosticSeverityError {
+				if text != "" {
+					text += "\n"
+				}
+
+				text += fmt.Sprintf("%s: %s", d.Summary, d.Detail)
+			}
+		}
+
 		return &tfprotov5.CallFunctionResponse{
-			Diagnostics: diags,
+			Error: &tfprotov5.FunctionError{
+				Text: text,
+			},
 		}, nil
 	}
 
@@ -37,13 +53,9 @@ func (s *muxServer) CallFunction(ctx context.Context, req *tfprotov5.CallFunctio
 
 	if !ok {
 		resp := &tfprotov5.CallFunctionResponse{
-			Diagnostics: []*tfprotov5.Diagnostic{
-				{
-					Severity: tfprotov5.DiagnosticSeverityError,
-					Summary:  "Provider Functions Not Implemented",
-					Detail: "A provider-defined function call was received by the provider, however the provider does not implement functions. " +
-						"Either upgrade the provider to a version that implements provider-defined functions or this is a bug in Terraform that should be reported to the Terraform maintainers.",
-				},
+			Error: &tfprotov5.FunctionError{
+				Text: "Provider Functions Not Implemented: A provider-defined function call was received by the provider, however the provider does not implement functions. " +
+					"Either upgrade the provider to a version that implements provider-defined functions or this is a bug in Terraform that should be reported to the Terraform maintainers.",
 			},
 		}
 

tf5muxserver/mux_server_test.go

@@ -492,15 +492,11 @@ func TestMuxServerGetFunctionServer_GetProviderSchema_Duplicate(t *testing.T) {
 	// Terraform to verify the mutex does not deadlock.
 	var wg sync.WaitGroup
 
-	expectedDiags := []*tfprotov5.Diagnostic{
-		{
-			Severity: tfprotov5.DiagnosticSeverityError,
-			Summary:  "Invalid Provider Server Combination",
-			Detail: "The combined provider has multiple implementations of the same function name across underlying providers. " +
-				"Functions must be implemented by only one underlying provider. " +
-				"This is always an issue in the provider implementation and should be reported to the provider developers.\n\n" +
-				"Duplicate function: test_function",
-		},
+	expectedError := &tfprotov5.FunctionError{
+		Text: "Invalid Provider Server Combination: The combined provider has multiple implementations of the same function name across underlying providers. " +
+			"Functions must be implemented by only one underlying provider. " +
+			"This is always an issue in the provider implementation and should be reported to the provider developers.\n\n" +
+			"Duplicate function: test_function",
 	}
 
 	terraformOp := func() {
@@ -518,8 +514,8 @@ func TestMuxServerGetFunctionServer_GetProviderSchema_Duplicate(t *testing.T) {
 			Name: "test_function",
 		})
 
-		if diff := cmp.Diff(resp.Diagnostics, expectedDiags); diff != "" {
-			t.Errorf("unexpected diagnostics difference: %s", diff)
+		if diff := cmp.Diff(resp.Error, expectedError); diff != "" {
+			t.Errorf("unexpected error difference: %s", diff)
 		}
 	}
 
@@ -639,15 +635,11 @@ func TestMuxServerGetFunctionServer_GetMetadata_Duplicate(t *testing.T) {
 	// Terraform to verify the mutex does not deadlock.
 	var wg sync.WaitGroup
 
-	expectedDiags := []*tfprotov5.Diagnostic{
-		{
-			Severity: tfprotov5.DiagnosticSeverityError,
-			Summary:  "Invalid Provider Server Combination",
-			Detail: "The combined provider has multiple implementations of the same function name across underlying providers. " +
-				"Functions must be implemented by only one underlying provider. " +
-				"This is always an issue in the provider implementation and should be reported to the provider developers.\n\n" +
-				"Duplicate function: test_function",
-		},
+	expectedError := &tfprotov5.FunctionError{
+		Text: "Invalid Provider Server Combination: The combined provider has multiple implementations of the same function name across underlying providers. " +
+			"Functions must be implemented by only one underlying provider. " +
+			"This is always an issue in the provider implementation and should be reported to the provider developers.\n\n" +
+			"Duplicate function: test_function",
 	}
 
 	terraformOp := func() {
@@ -665,7 +657,7 @@ func TestMuxServerGetFunctionServer_GetMetadata_Duplicate(t *testing.T) {
 			Name: "test_function",
 		})
 
-		if diff := cmp.Diff(resp.Diagnostics, expectedDiags); diff != "" {
+		if diff := cmp.Diff(resp.Error, expectedError); diff != "" {
 			t.Errorf("unexpected diagnostics difference: %s", diff)
 		}
 	}
@@ -784,14 +776,10 @@ func TestMuxServerGetFunctionServer_Missing(t *testing.T) {
 	// Terraform to verify the mutex does not deadlock.
 	var wg sync.WaitGroup
 
-	expectedDiags := []*tfprotov5.Diagnostic{
-		{
-			Severity: tfprotov5.DiagnosticSeverityError,
-			Summary:  "Function Not Implemented",
-			Detail: "The combined provider does not implement the requested function. " +
-				"This is always an issue in the provider implementation and should be reported to the provider developers.\n\n" +
-				"Missing function: test_function_nonexistent",
-		},
+	expectedError := &tfprotov5.FunctionError{
+		Text: "Function Not Implemented: The combined provider does not implement the requested function. " +
+			"This is always an issue in the provider implementation and should be reported to the provider developers.\n\n" +
+			"Missing function: test_function_nonexistent",
 	}
 
 	terraformOp := func() {
@@ -809,7 +797,7 @@ func TestMuxServerGetFunctionServer_Missing(t *testing.T) {
 			Name: "test_function_nonexistent",
 		})
 
-		if diff := cmp.Diff(resp.Diagnostics, expectedDiags); diff != "" {
+		if diff := cmp.Diff(resp.Error, expectedError); diff != "" {
 			t.Errorf("unexpected diagnostics difference: %s", diff)
 		}
 	}