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 VAULT-31393: Return 500 for some gRPC errors during login on perf standbys into release/1.18.x #28854

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
3 changes: 3 additions & 0 deletions changelog/28807.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:change
login (enterprise): Return a 500 error during logins when performance standby nodes make failed gRPC requests to the active node.
```
29 changes: 28 additions & 1 deletion vault/request_handling.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ import (
"github.com/hashicorp/vault/vault/quotas"
"github.com/hashicorp/vault/vault/tokens"
uberAtomic "go.uber.org/atomic"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

const (
Expand Down Expand Up @@ -2079,7 +2081,7 @@ func (c *Core) LoginCreateToken(ctx context.Context, ns *namespace.Namespace, re
if auth.TokenType != logical.TokenTypeBatch {
leaseGenerated = true
}
case err == ErrInternalError:
case errors.Is(err, ErrInternalError), isRetryableRPCError(ctx, err):
return false, nil, err
default:
return false, logical.ErrorResponse(err.Error()), logical.ErrInvalidRequest
Expand Down Expand Up @@ -2109,6 +2111,31 @@ func (c *Core) LoginCreateToken(ctx context.Context, ns *namespace.Namespace, re
return leaseGenerated, resp, nil
}

func isRetryableRPCError(ctx context.Context, err error) bool {
stat, ok := status.FromError(err)
if !ok {
return false
}

switch stat.Code() {
case codes.Unavailable:
return true
case codes.Canceled:
// if the request context is canceled, we want to return false
// but if the request context hasn't been canceled, then there was
// either an EOF or the RPC client context has been canceled which
// should be retried
return ctx.Err() == nil
case codes.Unknown:
// sometimes a missing HTTP content-type error can happen when multiple
// HTTP statuses have been written. This can happen when the error
// occurs in the middle of a response. This should be retried.
return strings.Contains(err.Error(), "malformed header: missing HTTP content-type")
default:
return false
}
}

// failedUserLoginProcess updates the userFailedLoginMap with login count and last failed
// login time for users with failed login attempt
// If the user gets locked for current login attempt, it updates the storage entry too
Expand Down
64 changes: 64 additions & 0 deletions vault/request_handling_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
package vault

import (
"context"
"errors"
"strings"
"testing"
"time"
Expand All @@ -15,6 +17,9 @@ import (
credUserpass "github.com/hashicorp/vault/builtin/credential/userpass"
"github.com/hashicorp/vault/helper/namespace"
"github.com/hashicorp/vault/sdk/logical"
"github.com/stretchr/testify/require"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

func TestRequestHandling_Wrapping(t *testing.T) {
Expand Down Expand Up @@ -478,3 +483,62 @@ func TestRequestHandling_SecretLeaseMetric(t *testing.T) {
},
)
}

// TestRequestHandling_isRetryableRPCError tests that a retryable RPC error
// can be distinguished from a normal error
func TestRequestHandling_isRetryableRPCError(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
cancel()
testCases := []struct {
name string
ctx context.Context
err error
want bool
}{
{
name: "req context canceled",
ctx: ctx,
err: status.Error(codes.Canceled, "context canceled"),
want: false,
},
{
name: "server context canceled",
err: status.Error(codes.Canceled, "context canceled"),
want: true,
},
{
name: "unavailable",
err: status.Error(codes.Unavailable, "unavailable"),
want: true,
},
{
name: "other status",
err: status.Error(codes.FailedPrecondition, "failed"),
want: false,
},
{
name: "other unknown",
err: status.Error(codes.Unknown, "unknown"),
want: false,
},
{
name: "malformed header unknown",
err: status.Error(codes.Unknown, "malformed header: missing HTTP content-type"),
want: true,
},
{
name: "other error",
err: errors.New("other type of error"),
want: false,
},
}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
useCtx := tc.ctx
if tc.ctx == nil {
useCtx = context.Background()
}
require.Equal(t, tc.want, isRetryableRPCError(useCtx, tc.err))
})
}
}
Loading