Allow accessing Warnings directly in Response. (#2806)

A change in copystructure has caused some panics due to the custom copy
function. I'm more nervous about production panics than I am about
keeping some bad code wiping out some existing warnings, so remove the
custom copy function and just allow direct setting of Warnings.

builtin/credential/aws/path_role.go

@@ -669,7 +669,7 @@ func (b *backend) pathRoleCreateUpdate(
 		return nil, err
 	}
 
-	if len(resp.Warnings()) == 0 {
+	if len(resp.Warnings) == 0 {
 		return nil, nil
 	}
 

builtin/credential/ldap/backend.go

@@ -184,8 +184,8 @@ func (b *backend) Login(req *logical.Request, username string, password string)
 
 	if len(policies) == 0 {
 		errStr := "user is not a member of any authorized group"
-		if len(ldapResponse.Warnings()) > 0 {
-			errStr = fmt.Sprintf("%s; additionally, %s", errStr, ldapResponse.Warnings()[0])
+		if len(ldapResponse.Warnings) > 0 {
+			errStr = fmt.Sprintf("%s; additionally, %s", errStr, ldapResponse.Warnings[0])
 		}
 
 		ldapResponse.Data["error"] = errStr

builtin/credential/ldap/backend_test.go

@@ -463,8 +463,8 @@ func testAccStepLoginNoGroupDN(t *testing.T, user string, pass string) logicalte
 
 		// Verifies a search without defined GroupDN returns a warnting rather than failing
 		Check: func(resp *logical.Response) error {
-			if len(resp.Warnings()) != 1 {
-				return fmt.Errorf("expected a warning due to no group dn, got: %#v", resp.Warnings())
+			if len(resp.Warnings) != 1 {
+				return fmt.Errorf("expected a warning due to no group dn, got: %#v", resp.Warnings)
 			}
 
 			return logicaltest.TestCheckAuth([]string{"bar", "default"})(resp)

builtin/credential/okta/backend.go

@@ -104,8 +104,8 @@ func (b *backend) Login(req *logical.Request, username string, password string)
 
 	if len(policies) == 0 {
 		errStr := "user is not a member of any authorized policy"
-		if len(oktaResponse.Warnings()) > 0 {
-			errStr = fmt.Sprintf("%s; additionally, %s", errStr, oktaResponse.Warnings()[0])
+		if len(oktaResponse.Warnings) > 0 {
+			errStr = fmt.Sprintf("%s; additionally, %s", errStr, oktaResponse.Warnings[0])
 		}
 
 		oktaResponse.Data["error"] = errStr

builtin/logical/transit/path_config.go

@@ -104,7 +104,7 @@ func (b *backend) pathConfigWrite(
 		return nil, nil
 	}
 
-	if len(resp.Warnings()) == 0 {
+	if len(resp.Warnings) == 0 {
 		return nil, p.Persist(req.Storage)
 	}
 

logical/response.go

@@ -2,11 +2,8 @@ package logical
 
 import (
 	"errors"
-	"fmt"
-	"reflect"
 
 	"github.com/hashicorp/vault/helper/wrapping"
-	"github.com/mitchellh/copystructure"
 )
 
 const (
@@ -52,85 +49,18 @@ type Response struct {
 
 	// Warnings allow operations or backends to return warnings in response
 	// to user actions without failing the action outright.
-	// Making it private helps ensure that it is easy for various parts of
-	// Vault (backend, core, etc.) to add warnings without accidentally
-	// replacing what exists.
-	warnings []string `json:"warnings" structs:"warnings" mapstructure:"warnings"`
+	Warnings []string `json:"warnings" structs:"warnings" mapstructure:"warnings"`
 
 	// Information for wrapping the response in a cubbyhole
 	WrapInfo *wrapping.ResponseWrapInfo `json:"wrap_info" structs:"wrap_info" mapstructure:"wrap_info"`
 }
 
-func init() {
-	copystructure.Copiers[reflect.TypeOf(Response{})] = func(v interface{}) (interface{}, error) {
-		input := v.(Response)
-		ret := Response{
-			Redirect: input.Redirect,
-		}
-
-		if input.Secret != nil {
-			retSec, err := copystructure.Copy(input.Secret)
-			if err != nil {
-				return nil, fmt.Errorf("error copying Secret: %v", err)
-			}
-			ret.Secret = retSec.(*Secret)
-		}
-
-		if input.Auth != nil {
-			retAuth, err := copystructure.Copy(input.Auth)
-			if err != nil {
-				return nil, fmt.Errorf("error copying Auth: %v", err)
-			}
-			ret.Auth = retAuth.(*Auth)
-		}
-
-		if input.Data != nil {
-			retData, err := copystructure.Copy(&input.Data)
-			if err != nil {
-				return nil, fmt.Errorf("error copying Data: %v", err)
-			}
-			ret.Data = *(retData.(*map[string]interface{}))
-		}
-
-		if input.Warnings() != nil {
-			for _, warning := range input.Warnings() {
-				ret.AddWarning(warning)
-			}
-		}
-
-		if input.WrapInfo != nil {
-			retWrapInfo, err := copystructure.Copy(input.WrapInfo)
-			if err != nil {
-				return nil, fmt.Errorf("error copying WrapInfo: %v", err)
-			}
-			ret.WrapInfo = retWrapInfo.(*wrapping.ResponseWrapInfo)
-		}
-
-		return &ret, nil
-	}
-}
-
 // AddWarning adds a warning into the response's warning list
 func (r *Response) AddWarning(warning string) {
-	if r.warnings == nil {
-		r.warnings = make([]string, 0, 1)
+	if r.Warnings == nil {
+		r.Warnings = make([]string, 0, 1)
 	}
-	r.warnings = append(r.warnings, warning)
-}
-
-// Warnings returns the list of warnings set on the response
-func (r *Response) Warnings() []string {
-	return r.warnings
-}
-
-// ClearWarnings clears the response's warning list
-func (r *Response) ClearWarnings() {
-	r.warnings = make([]string, 0, 1)
-}
-
-// Copies the warnings from the other response to this one
-func (r *Response) CloneWarnings(other *Response) {
-	r.warnings = other.warnings
+	r.Warnings = append(r.Warnings, warning)
 }
 
 // IsError returns true if this response seems to indicate an error.

logical/translate_response.go

@@ -14,7 +14,7 @@ import (
 func LogicalResponseToHTTPResponse(input *Response) *HTTPResponse {
 	httpResp := &HTTPResponse{
 		Data:     input.Data,
-		Warnings: input.Warnings(),
+		Warnings: input.Warnings,
 	}
 
 	if input.Secret != nil {
@@ -42,7 +42,7 @@ func LogicalResponseToHTTPResponse(input *Response) *HTTPResponse {
 func HTTPResponseToLogicalResponse(input *HTTPResponse) *Response {
 	logicalResp := &Response{
 		Data:     input.Data,
-		warnings: input.Warnings,
+		Warnings: input.Warnings,
 	}
 
 	if input.LeaseID != "" {

vault/request_handling.go

@@ -77,8 +77,8 @@ func (c *Core) HandleRequest(req *logical.Request) (resp *logical.Response, err
 		} else {
 			wrappingResp := &logical.Response{
 				WrapInfo: resp.WrapInfo,
+				Warnings: resp.Warnings,
 			}
-			wrappingResp.CloneWarnings(resp)
 			resp = wrappingResp
 		}
 	}

vault/token_store_test.go

@@ -312,8 +312,8 @@ func TestTokenStore_HandleRequest_ListAccessors(t *testing.T) {
 	if len(keys) != len(testKeys) {
 		t.Fatalf("wrong number of accessors found")
 	}
-	if len(resp.Warnings()) != 0 {
-		t.Fatalf("got warnings:\n%#v", resp.Warnings())
+	if len(resp.Warnings) != 0 {
+		t.Fatalf("got warnings:\n%#v", resp.Warnings)
 	}
 
 	// Test upgrade from old struct method of accessor storage (of token id)
@@ -347,8 +347,8 @@ func TestTokenStore_HandleRequest_ListAccessors(t *testing.T) {
 	if len(keys) != len(testKeys) {
 		t.Fatalf("wrong number of accessors found")
 	}
-	if len(resp.Warnings()) != 0 {
-		t.Fatalf("got warnings:\n%#v", resp.Warnings())
+	if len(resp.Warnings) != 0 {
+		t.Fatalf("got warnings:\n%#v", resp.Warnings)
 	}
 
 	for _, accessor := range keys2 {
@@ -2347,7 +2347,7 @@ func TestTokenStore_RoleExplicitMaxTTL(t *testing.T) {
 	if err != nil {
 		t.Fatalf("expected an error")
 	}
-	if len(resp.Warnings()) == 0 {
+	if len(resp.Warnings) == 0 {
 		t.Fatalf("expected a warning")
 	}