client/v3: clear auth token when encounter ErrInvalidAuthToken #12549
Conversation
| // 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("") |
There was a problem hiding this comment.
Thanks a lot for the fix @bbiao , I think it would work fine but let me make sure it works as expected.
client/v3/retry_interceptor.go
Outdated
| @@ -74,6 +74,12 @@ func (c *Client) unaryClientInterceptor(logger *zap.Logger, optFuncs ...retryOpt | |||
| continue | |||
| } | |||
| if callOpts.retryAuth && rpctypes.Error(lastErr) == rpctypes.ErrInvalidAuthToken { | |||
| // clear auth token befault refreshing it. | |||
There was a problem hiding this comment.
Nit, typo "befault". Also, don't we need a similar update for https://github.com/etcd-io/etcd/blob/master/client/v3/retry_interceptor.go#L242 for a similar call to getToken when ErrInvalidAuthToken? Thanks! /cc @mitake @cfc4n
|
@bbiao thanks for addressing my one comment, how about the second comment/question? |
It's ok not to clear auth token when encounter the ErrInvalidAuthToken at https://github.com/etcd-io/etcd/blob/master/client/v3/retry_interceptor.go#L242, since the getToken itself is a unary rpc call, when failed it will retry, and at that moment the unary interceptor will clean the auth token. But I think it's better to clean the auth token too at https://github.com/etcd-io/etcd/blob/master/client/v3/retry_interceptor.go#L242, this will avoid a never-success rpc call. |
@bbiao lgtm, thanks for addressing the comments, can you please squash commits now? |
Old etcdserver which have not apply pr of etcd-io#12165 will check auth token even if the request is an Authenticate request. If the client has a invalid auth token, it will not able to update it's token, since the Authenticate has a invalid auth token. This fix clear the auth token when encounter an ErrInvalidAuthToken to talk with old version etcd servers. Fix etcd-io#12385 with etcd-io#12165 and etcd-io#12264
bbiao
force-pushed
the
bugfix/refresh_token
branch
from
b093a59 to
af4ef4e
Compare
|
@spzala commits have been squashed ;-) |
|
Travis is failing because of |
|
@spzala do you know how to rerun semaphore CI? I found that I cannot do that... |
|
@mitake there used to be a rerun option but I also don't see it. Need to investigate it, for now, I think closing/reopening PR is one quick option. As you said, failure seems not related to the changes. |
|
@spzala yeah that's smart, thanks for handling! |
Old etcdserver which have not apply pr of #12165 will check auth token
even if the request is an Authenticate request.
If the client has a invalid auth token, it will not able to update it's
token, since the Authenticate has a invalid auth token.
This fix clear the auth token when encounter an ErrInvalidAuthToken to
talk with old version etcd servers.
With pr #12165 and #12264 this will fix the problem in #12385
Fix #12385