Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tfprotov5+tfprotov6: Replace usage of diagnostics in CallFunction RPC with function error #380

Merged
merged 7 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changes/unreleased/BREAKING CHANGES-20240221-103010.yaml
Original file line number Diff line number Diff line change
@@ -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"
7 changes: 7 additions & 0 deletions .changes/unreleased/NOTES-20240222-080818.yaml
Original file line number Diff line number Diff line change
@@ -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 terraform-plugin-framework@v1.6.0,
terraform-plugin-mux@v0.15.0, and terraform-plugin-sdk/v2@v2.33.0, or greater, respectively'
time: 2024-02-22T08:08:18.434191Z
custom:
Issue: "380"
12 changes: 9 additions & 3 deletions internal/logging/keys.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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"

Expand Down
4 changes: 0 additions & 4 deletions tfprotov5/diagnostic.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions tfprotov5/function.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions tfprotov5/function_error.go
Original file line number Diff line number Diff line change
@@ -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
}
4 changes: 0 additions & 4 deletions tfprotov5/internal/diag/diagnostics.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 6 additions & 0 deletions tfprotov5/internal/funcerr/doc.go
Original file line number Diff line number Diff line change
@@ -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
50 changes: 50 additions & 0 deletions tfprotov5/internal/funcerr/function_error.go
Original file line number Diff line number Diff line change
@@ -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,
})
}
}
152 changes: 152 additions & 0 deletions tfprotov5/internal/funcerr/function_error_test.go
Original file line number Diff line number Diff line change
@@ -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)
}
})
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand Down
22 changes: 22 additions & 0 deletions tfprotov5/internal/tf5serverlogging/downstream_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
Loading
Loading