From 69aace20c82cbc7388d72f62a885dcb6c77431d3 Mon Sep 17 00:00:00 2001 From: Benjamin Wang Date: Thu, 17 Nov 2022 05:54:52 +0800 Subject: [PATCH] clientv3: do not refresh token when using TLS CommonName based authentication When users use the TLS CommonName based authentication, the authTokenBundle is always nil. But it's possible for the clients to get `rpctypes.ErrAuthOldRevision` response when the clients concurrently modify auth data (e.g, addUser, deleteUser etc.). In this case, there is no need to refresh the token; instead the clients just need to retry the operations (e.g. Put, Delete etc). Signed-off-by: Benjamin Wang --- client/v3/retry_interceptor.go | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/client/v3/retry_interceptor.go b/client/v3/retry_interceptor.go index 04f157a1dcb..7dc5ddae0fd 100644 --- a/client/v3/retry_interceptor.go +++ b/client/v3/retry_interceptor.go @@ -74,13 +74,7 @@ func (c *Client) unaryClientInterceptor(optFuncs ...retryOption) grpc.UnaryClien continue } if c.shouldRefreshToken(lastErr, callOpts) { - // clear auth token before refreshing it. - // call c.Auth.Authenticate with an invalid token will always fail the auth check on the server-side, - // if the server has not apply the patch of pr #12165 (https://github.com/etcd-io/etcd/pull/12165) - // and a rpctypes.ErrInvalidAuthToken will recursively call c.getToken until system run out of resource. - c.authTokenBundle.UpdateAuthToken("") - - gterr := c.getToken(ctx) + gterr := c.refreshToken(ctx) if gterr != nil { c.GetLogger().Warn( "retrying of unary invoker failed to fetch new auth token", @@ -161,6 +155,24 @@ func (c *Client) shouldRefreshToken(err error, callOpts *options) bool { (rpctypes.Error(err) == rpctypes.ErrInvalidAuthToken || rpctypes.Error(err) == rpctypes.ErrAuthOldRevision) } +func (c *Client) refreshToken(ctx context.Context) error { + if c.authTokenBundle == nil { + // c.authTokenBundle will be initialized only when + // c.Username != "" && c.Password != "". + // + // When users use the TLS CommonName based authentication, the + // authTokenBundle is always nil. But it's possible for the clients + // to get `rpctypes.ErrAuthOldRevision` response when the clients + // concurrently modify auth data (e.g, addUser, deleteUser etc.). + // In this case, there is no need to refresh the token; instead the + // clients just need to retry the operations (e.g. Put, Delete etc). + return nil + } + // clear auth token before refreshing it. + c.authTokenBundle.UpdateAuthToken("") + return c.getToken(ctx) +} + // type serverStreamingRetryingStream is the implementation of grpc.ClientStream that acts as a // proxy to the underlying call. If any of the RecvMsg() calls fail, it will try to reestablish // a new ClientStream according to the retry policy. @@ -259,10 +271,7 @@ func (s *serverStreamingRetryingStream) receiveMsgAndIndicateRetry(m interface{} return true, err } if s.client.shouldRefreshToken(err, s.callOpts) { - // clear auth token to avoid failure when call getToken - s.client.authTokenBundle.UpdateAuthToken("") - - gterr := s.client.getToken(s.ctx) + gterr := s.client.refreshToken(s.ctx) if gterr != nil { s.client.lg.Warn("retry failed to fetch new auth token", zap.Error(gterr)) return false, err // return the original error for simplicity