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

Backport of report intermediate error messages during request forwarding into release/1.13.x #20649

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
2 changes: 1 addition & 1 deletion builtin/logical/pki/crl_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -751,7 +751,7 @@ func (cb *crlBuilder) processRevocationQueue(sc *storageContext) error {
}

if err := sc.Storage.Put(sc.Context, confirmedEntry); err != nil {
return fmt.Errorf("error persisting cross-cluster revocation confirmation: %w\nThis may occur when the active node of the primary performance replication cluster is unavailable.", err)
return fmt.Errorf("error persisting cross-cluster revocation confirmation: %w", err)
}
} else {
// Since we're the active node of the primary cluster, go ahead
Expand Down
2 changes: 1 addition & 1 deletion builtin/logical/pki/path_revoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -374,7 +374,7 @@ func (b *backend) maybeRevokeCrossCluster(sc *storageContext, config *crlConfig,
}

if err := sc.Storage.Put(sc.Context, reqEntry); err != nil {
return nil, fmt.Errorf("error persisting cross-cluster revocation request: %w\nThis may occur when the active node of the primary performance replication cluster is unavailable.", err)
return nil, fmt.Errorf("error persisting cross-cluster revocation request: %w", err)
}

resp := &logical.Response{
Expand Down
3 changes: 3 additions & 0 deletions changelog/20643.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
core: report intermediate error messages during request forwarding
```
11 changes: 11 additions & 0 deletions sdk/logical/response_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,10 +73,21 @@ func RespondErrorCommon(req *Request, resp *Response, err error) (int, error) {
var allErrors error
var codedErr *ReplicationCodedError
errwrap.Walk(err, func(inErr error) {
// The Walk function does not just traverse leaves, and execute the
// callback function on the entire error first. So, if the error is
// of type multierror.Error, we may want to skip storing the entire
// error first to avoid adding duplicate errors when walking down
// the leaf errors
if _, ok := inErr.(*multierror.Error); ok {
return
}
newErr, ok := inErr.(*ReplicationCodedError)
if ok {
codedErr = newErr
} else {
// if the error is of type fmt.wrapError which is typically
// made by calling fmt.Errorf("... %w", err), allErrors will
// contain duplicated error messages
allErrors = multierror.Append(allErrors, inErr)
}
})
Expand Down
25 changes: 24 additions & 1 deletion vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -826,7 +826,30 @@ func (c *Core) doRouting(ctx context.Context, req *logical.Request) (*logical.Re
// If we're replicating and we get a read-only error from a backend, need to forward to primary
resp, err := c.router.Route(ctx, req)
if shouldForward(c, resp, err) {
return forward(ctx, c, req)
fwdResp, fwdErr := forward(ctx, c, req)
if fwdErr != nil && err != logical.ErrReadOnly {
// When handling the request locally, we got an error that
// contained ErrReadOnly, but had additional information.
// Since we've now forwarded this request and got _another_
// error, we should tell the user about both errors, so
// they know about both.
//
// When there is no error from forwarding, the request
// succeeded and so no additional context is necessary. When
// the initial error here was only ErrReadOnly, it's likely
// the plugin authors intended to forward this request
// remotely anyway.
repErr, ok := fwdErr.(*logical.ReplicationCodedError)
if ok {
fwdErr = &logical.ReplicationCodedError{
Msg: fmt.Sprintf("errors from both primary and secondary; primary error was %s; secondary errors follow: %s", repErr.Error(), err.Error()),
Code: repErr.Code,
}
} else {
fwdErr = multierror.Append(fwdErr, err)
}
}
return fwdResp, fwdErr
}
return resp, err
}
Expand Down