Warnings indicating ignored and replaced parameters (#14962)

* Warnings indicating ignored and replaced parameters

* Avoid additional var creation

* Add warnings only if the response is non-nil

* Return the response even when error is non-nil

* Fix tests

* Rearrange comments

* Print warning in the log

* Fix another test

* Add CL

builtin/logical/database/backend_test.go

@@ -689,7 +689,7 @@ func TestBackend_connectionCrud(t *testing.T) {
 	if err != nil || (resp != nil && resp.IsError()) {
 		t.Fatalf("err:%s resp:%#v\n", err, resp)
 	}
-	if len(resp.Warnings) != 1 {
+	if len(resp.Warnings) == 0 {
 		t.Fatalf("expected warning about password in url %s, resp:%#v\n", connURL, resp)
 	}
 

changelog/14962.txt

@@ -0,0 +1,6 @@
+```release-note:improvement
+api: If the parameters supplied over the API payload are ignored due to not
+being what the endpoints were expecting, or if the parameters supplied get
+replaced by the values in the endpoint's path itself, warnings will be added to
+the non-empty responses listing all the ignored and replaced parameters.
+```

http/logical_test.go

@@ -212,7 +212,6 @@ func TestLogical_CreateToken(t *testing.T) {
 	})
 
 	var actual map[string]interface{}
-	var nilWarnings interface{}
 	expected := map[string]interface{}{
 		"lease_id":       "",
 		"renewable":      false,
@@ -231,12 +230,12 @@ func TestLogical_CreateToken(t *testing.T) {
 			"mfa_requirement": nil,
 			"num_uses":        json.Number("0"),
 		},
-		"warnings": nilWarnings,
 	}
 	testResponseStatus(t, resp, 200)
 	testResponseBody(t, resp, &actual)
 	delete(actual["auth"].(map[string]interface{}), "client_token")
 	delete(actual["auth"].(map[string]interface{}), "accessor")
+	delete(actual, "warnings")
 	expected["request_id"] = actual["request_id"]
 	if !reflect.DeepEqual(actual, expected) {
 		t.Fatalf("bad:\nexpected:\n%#v\nactual:\n%#v", expected, actual)

sdk/framework/backend.go

@@ -218,10 +218,19 @@ func (b *Backend) HandleRequest(ctx context.Context, req *logical.Request) (*log
 	// Build up the data for the route, with the URL taking priority
 	// for the fields over the PUT data.
 	raw := make(map[string]interface{}, len(path.Fields))
+	var ignored []string
 	for k, v := range req.Data {
 		raw[k] = v
+		if path.Fields[k] == nil {
+			ignored = append(ignored, k)
 		}
+	}
+
+	var replaced []string
 	for k, v := range captures {
+		if raw[k] != nil {
+			replaced = append(replaced, k)
+		}
 		raw[k] = v
 	}
 
@@ -275,7 +284,29 @@ func (b *Backend) HandleRequest(ctx context.Context, req *logical.Request) (*log
 		}
 	}
 
-	return callback(ctx, req, &fd)
+	resp, err := callback(ctx, req, &fd)
+	if err != nil {
+		return resp, err
+	}
+
+	switch resp {
+	case nil:
+	default:
+		// If fields supplied in the request are not present in the field schema
+		// of the path, add a warning to the response indicating that those
+		// parameters will be ignored.
+		if len(ignored) != 0 {
+			resp.AddWarning(fmt.Sprintf("Endpoint ignored these unrecognized parameters: %v", ignored))
+		}
+		// If fields supplied in the request is being overwritten by the values
+		// supplied in the API request path, add a warning to the response
+		// indicating that those parameters will be replaced.
+		if len(replaced) != 0 {
+			resp.AddWarning(fmt.Sprintf("Endpoint replaced the value of these parameters with the values captured from the endpoint's path: %v", replaced))
+		}
+	}
+
+	return resp, nil
 }
 
 // HandlePatchOperation acts as an abstraction for performing JSON merge patch

sdk/framework/backend_test.go

@@ -2,6 +2,8 @@ package framework
 
 import (
 	"context"
+	"github.com/hashicorp/go-secure-stdlib/strutil"
+	"github.com/stretchr/testify/require"
 	"net/http"
 	"reflect"
 	"strings"
@@ -45,6 +47,52 @@ func TestBackend_impl(t *testing.T) {
 	var _ logical.Backend = new(Backend)
 }
 
+func TestBackendHandleRequestFieldWarnings(t *testing.T) {
+	handler := func(ctx context.Context, req *logical.Request, data *FieldData) (*logical.Response, error) {
+		return &logical.Response{
+			Data: map[string]interface{}{
+				"an_int":   data.Get("an_int"),
+				"a_string": data.Get("a_string"),
+				"name":     data.Get("name"),
+			},
+		}, nil
+	}
+
+	backend := &Backend{
+		Paths: []*Path{
+			{
+				Pattern: "foo/bar/(?P<name>.+)",
+				Fields: map[string]*FieldSchema{
+					"an_int":   {Type: TypeInt},
+					"a_string": {Type: TypeString},
+					"name":     {Type: TypeString},
+				},
+				Operations: map[logical.Operation]OperationHandler{
+					logical.UpdateOperation: &PathOperation{Callback: handler},
+				},
+			},
+		},
+	}
+	ctx := context.Background()
+	resp, err := backend.HandleRequest(ctx, &logical.Request{
+		Operation: logical.UpdateOperation,
+		Path:      "foo/bar/baz",
+		Data: map[string]interface{}{
+			"an_int":        10,
+			"a_string":      "accepted",
+			"unrecognized1": "unrecognized",
+			"unrecognized2": 20.2,
+			"name":          "noop",
+		},
+	})
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+	t.Log(resp.Warnings)
+	require.Len(t, resp.Warnings, 2)
+	require.True(t, strutil.StrListContains(resp.Warnings, "Endpoint ignored these unrecognized parameters: [unrecognized1 unrecognized2]"))
+	require.True(t, strutil.StrListContains(resp.Warnings, "Endpoint replaced the value of these parameters with the values captured from the endpoint's path: [name]"))
+}
+
 func TestBackendHandleRequest(t *testing.T) {
 	callback := func(ctx context.Context, req *logical.Request, data *FieldData) (*logical.Response, error) {
 		return &logical.Response{