tfprotov5+tfprotov6: Replace usage of diagnostics in CallFunction RPC…

… with function error (#380)

* tfprotov5+tfprotov6: Replace usage of diagnostics in CallFunction RPC with function error
Reference: hashicorp/terraform#34603

The next version of the plugin protocol (5.5/6.5) includes support for provider defined functions. This change modifies the response returned from the CallFunction RPC, replacing diagnostics with function error.

* Fix protoc version

* Add changelog

* Removing function argument from diagnostics

* Renaming param from msg to text for function errorr

* Adding change log

.changes/unreleased/BREAKING CHANGES-20240221-103010.yaml

@@ -0,0 +1,6 @@
+kind: BREAKING CHANGES
+body: 'tfprotov5+tfprotov6: Modified the response returned from the CallFunction RPC,
+  replacing diagnostics with function error'
+time: 2024-02-21T10:30:10.223878Z
+custom:
+  Issue: "380"

.changes/unreleased/NOTES-20240222-080818.yaml

@@ -0,0 +1,7 @@
+kind: NOTES
+body: 'all: If using terraform-plugin-framework, terraform-plugin-mux, or terraform-plugin-sdk,
+  only upgrade this Go module when upgrading those Go modules to [email protected], 
+  [email protected], and terraform-plugin-sdk/[email protected], or greater, respectively'
+time: 2024-02-22T08:08:18.434191Z
+custom:
+  Issue: "380"

internal/logging/keys.go

@@ -11,9 +11,6 @@ const (
 	// Attribute of the diagnostic being logged.
 	KeyDiagnosticAttribute = "diagnostic_attribute"
 
-	// Function Argument of the diagnostic being logged.
-	KeyDiagnosticFunctionArgument = "diagnostic_function_argument"
-
 	// Number of the error diagnostics.
 	KeyDiagnosticErrorCount = "diagnostic_error_count"
 
@@ -32,6 +29,15 @@ const (
 	// Underlying error string
 	KeyError = "error"
 
+	// Argument position of the function error.
+	KeyFunctionErrorArgument = "function_error_argument"
+
+	// Boolean indicating presence of function error
+	KeyFunctionErrorExists = "function_error_exists"
+
+	// Message of the function error.
+	KeyFunctionErrorText = "function_error_text"
+
 	// Duration in milliseconds for the RPC request
 	KeyRequestDurationMs = "tf_req_duration_ms"
 

tfprotov5/diagnostic.go

@@ -42,10 +42,6 @@ type Diagnostic struct {
 	// indicate that the problem is with a certain field in the resource,
 	// which helps users find the source of the problem.
 	Attribute *tftypes.AttributePath
-
-	// FunctionArgument is the positional function argument for aligning
-	// configuration source.
-	FunctionArgument *int64
 }
 
 // DiagnosticSeverity represents different classes of Diagnostic which affect

tfprotov5/function.go

@@ -109,10 +109,10 @@ type CallFunctionRequest struct {
 // CallFunctionResponse is the response from the provider with the result of
 // executing the logic of the function.
 type CallFunctionResponse struct {
-	// Diagnostics report errors or warnings related to the execution of the
-	// function logic. Returning an empty slice indicates a successful response
-	// with no warnings or errors presented to practitioners.
-	Diagnostics []*Diagnostic
+	// Error reports errors related to the execution of the
+	// function logic. Returning a nil error indicates a successful response
+	// with no errors presented to practitioners.
+	Error *FunctionError
 
 	// Result is the return value from the called function, matching the result
 	// type in the function definition.

tfprotov5/function_error.go

@@ -0,0 +1,14 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package tfprotov5
+
+// FunctionError is used to convey information back to the user running Terraform.
+type FunctionError struct {
+	// Text is the description of the error.
+	Text string
+
+	// FunctionArgument is the positional function argument for aligning
+	// configuration source.
+	FunctionArgument *int64
+}

tfprotov5/internal/diag/diagnostics.go

@@ -53,10 +53,6 @@ func (d Diagnostics) Log(ctx context.Context) {
 			diagnosticFields[logging.KeyDiagnosticAttribute] = diagnostic.Attribute.String()
 		}
 
-		if diagnostic.FunctionArgument != nil {
-			diagnosticFields[logging.KeyDiagnosticFunctionArgument] = *diagnostic.FunctionArgument
-		}
-
 		switch diagnostic.Severity {
 		case tfprotov5.DiagnosticSeverityError:
 			logging.ProtocolError(ctx, "Response contains error diagnostic", diagnosticFields)

tfprotov5/internal/funcerr/doc.go

@@ -0,0 +1,6 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+// Package funcerr contains function error helpers. These implementations are
+// intentionally outside the public API.
+package funcerr

tfprotov5/internal/funcerr/function_error.go

@@ -0,0 +1,50 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package funcerr
+
+import (
+	"context"
+
+	"github.com/hashicorp/terraform-plugin-go/internal/logging"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
+)
+
+// FunctionError is a single FunctionError.
+type FunctionError tfprotov5.FunctionError
+
+// HasError returns true if the FunctionError is not empty.
+func (e *FunctionError) HasError() bool {
+	if e == nil {
+		return false
+	}
+
+	return e.Text != "" || e.FunctionArgument != nil
+}
+
+// Log will log the function error:
+func (e *FunctionError) Log(ctx context.Context) {
+	if e == nil {
+		return
+	}
+
+	if !e.HasError() {
+		return
+	}
+
+	switch {
+	case e.FunctionArgument != nil && e.Text != "":
+		logging.ProtocolError(ctx, "Response contains function error", map[string]interface{}{
+			logging.KeyFunctionErrorText:     e.Text,
+			logging.KeyFunctionErrorArgument: *e.FunctionArgument,
+		})
+	case e.FunctionArgument != nil:
+		logging.ProtocolError(ctx, "Response contains function error", map[string]interface{}{
+			logging.KeyFunctionErrorArgument: *e.FunctionArgument,
+		})
+	case e.Text != "":
+		logging.ProtocolError(ctx, "Response contains function error", map[string]interface{}{
+			logging.KeyFunctionErrorText: e.Text,
+		})
+	}
+}

tfprotov5/internal/funcerr/function_error_test.go

@@ -0,0 +1,152 @@
+// Copyright (c) HashiCorp, Inc.
+// SPDX-License-Identifier: MPL-2.0
+
+package funcerr_test
+
+import (
+	"bytes"
+	"context"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+	"github.com/hashicorp/terraform-plugin-log/tfsdklog"
+	"github.com/hashicorp/terraform-plugin-log/tfsdklogtest"
+
+	"github.com/hashicorp/terraform-plugin-go/internal/logging"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/funcerr"
+)
+
+func TestFunctionErrorHasError(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		functionError *funcerr.FunctionError
+		expected      bool
+	}{
+		"nil": {
+			functionError: nil,
+			expected:      false,
+		},
+		"empty": {
+			functionError: &funcerr.FunctionError{},
+			expected:      false,
+		},
+		"argument": {
+			functionError: &funcerr.FunctionError{
+				FunctionArgument: pointer(int64(0)),
+			},
+			expected: true,
+		},
+		"message": {
+			functionError: &funcerr.FunctionError{
+				Text: "test function error",
+			},
+			expected: true,
+		},
+		"argument-and-message": {
+			functionError: &funcerr.FunctionError{
+				FunctionArgument: pointer(int64(0)),
+				Text:             "test function error",
+			},
+			expected: true,
+		},
+	}
+
+	for name, testCase := range testCases {
+		name, testCase := name, testCase
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			got := testCase.functionError.HasError()
+
+			if diff := cmp.Diff(got, testCase.expected); diff != "" {
+				t.Errorf("unexpected difference: %s", diff)
+			}
+		})
+	}
+}
+
+func TestFunctionErrorLog(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		functionError *funcerr.FunctionError
+		expected      []map[string]interface{}
+	}{
+		"nil": {
+			functionError: nil,
+			expected:      nil,
+		},
+		"empty": {
+			functionError: &funcerr.FunctionError{},
+			expected:      nil,
+		},
+		"argument": {
+			functionError: &funcerr.FunctionError{
+				FunctionArgument: pointer(int64(0)),
+			},
+			expected: []map[string]interface{}{
+				{
+					"@level":                  "error",
+					"@message":                "Response contains function error",
+					"@module":                 "sdk.proto",
+					"function_error_argument": float64(0),
+				},
+			},
+		},
+		"message": {
+			functionError: &funcerr.FunctionError{
+				Text: "test function error",
+			},
+			expected: []map[string]interface{}{
+				{
+					"@level":              "error",
+					"@message":            "Response contains function error",
+					"@module":             "sdk.proto",
+					"function_error_text": "test function error",
+				},
+			},
+		},
+		"argument-and-message": {
+			functionError: &funcerr.FunctionError{
+				FunctionArgument: pointer(int64(0)),
+				Text:             "test function error",
+			},
+			expected: []map[string]interface{}{
+				{
+					"@level":                  "error",
+					"@message":                "Response contains function error",
+					"@module":                 "sdk.proto",
+					"function_error_text":     "test function error",
+					"function_error_argument": float64(0),
+				},
+			},
+		},
+	}
+
+	for name, testCase := range testCases {
+		name, testCase := name, testCase
+
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			var output bytes.Buffer
+
+			ctx := tfsdklogtest.RootLogger(context.Background(), &output)
+			ctx = logging.ProtoSubsystemContext(ctx, tfsdklog.Options{})
+
+			testCase.functionError.Log(ctx)
+
+			entries, err := tfsdklogtest.MultilineJSONDecode(&output)
+
+			if err != nil {
+				t.Fatalf("unable to read multiple line JSON: %s", err)
+			}
+
+			if diff := cmp.Diff(entries, testCase.expected); diff != "" {
+				t.Errorf("unexpected difference: %s", diff)
+			}
+		})
+	}
+}

tfprotov5/internal/toproto/pointer_test.go → tfprotov5/internal/funcerr/pointer_test.go

@@ -1,7 +1,7 @@
 // Copyright (c) HashiCorp, Inc.
 // SPDX-License-Identifier: MPL-2.0
 
-package toproto_test
+package funcerr_test
 
 func pointer[T any](value T) *T {
 	return &value

tfprotov5/internal/tf5serverlogging/downstream_request.go

@@ -8,7 +8,9 @@ import (
 	"time"
 
 	"github.com/hashicorp/terraform-plugin-go/internal/logging"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5"
 	"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/diag"
+	"github.com/hashicorp/terraform-plugin-go/tfprotov5/internal/funcerr"
 )
 
 // DownstreamRequest sets a request duration start time context key and
@@ -40,3 +42,23 @@ func DownstreamResponse(ctx context.Context, diagnostics diag.Diagnostics) {
 	logging.ProtocolTrace(ctx, "Received downstream response", responseFields)
 	diagnostics.Log(ctx)
 }
+
+// DownstreamResponseWithError generates the following logging:
+//
+//   - TRACE "Received downstream response" log with request duration and
+//     whether a function error is present
+//   - Log with function error details
+func DownstreamResponseWithError(ctx context.Context, funcErr *tfprotov5.FunctionError) {
+	fe := (*funcerr.FunctionError)(funcErr)
+
+	responseFields := map[string]interface{}{
+		logging.KeyFunctionErrorExists: fe.HasError(),
+	}
+
+	if requestStart, ok := ctx.Value(ContextKeyDownstreamRequestStartTime{}).(time.Time); ok {
+		responseFields[logging.KeyRequestDurationMs] = time.Since(requestStart).Milliseconds()
+	}
+
+	logging.ProtocolTrace(ctx, "Received downstream response", responseFields)
+	fe.Log(ctx)
+}