clientv3: get AuthToken automatically when clientConn is ready. #12264
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mitake
suggested changes
Sep 13, 2020
clientv3/retry_interceptor.go
Outdated
| } | ||
| creds = c.authTokenBundle.PerRPCCredentials() | ||
| } | ||
| if creds != nil { |
There was a problem hiding this comment.
I think logging metadata wouldn't be good, it can expose a credential in client side logs, right?
There was a problem hiding this comment.
Thanks, But I think that no other credential info in client side logs in
etcd/clientv3/credentials/credentials.go
Lines 110 to 115 in 76e769c
authToken only, there is not other keyword. Logging metadata for debug .
There was a problem hiding this comment.
yeah, but if the token can be leaked and malicious person can get it, it can result security issue. I think it shouldn't be logged. How do you think?
There was a problem hiding this comment.
emmm, you are right.
I fixed it at 0649f91 .
but CI was failed at https://travis-ci.com/github/etcd-io/etcd/jobs/385106741#L2064 .
=== RUN TestBalancerUnderNetworkPartitionWatchLeader
TestBalancerUnderNetworkPartitionWatchLeader: network_partition_test.go:266: took too long to detect leader lost
--- FAIL: TestBalancerUnderNetworkPartitionWatchLeader (3.35s)
But ,I test passed on my macbook like this:
$ PASSES=integration TESTCASE=TestBalancerUnderNetworkPartitionWatchLeader ./test [015eab45e]
Running with TEST_CPUS: 1,2,4
Starting 'integration' pass at 2020年 9月16日 星期三 12时26分27秒 CST
Running integration tests...
testing: warning: no tests to run
PASS
ok go.etcd.io/etcd/v3/integration 0.017s [no tests to run]
testing: warning: no tests to run
PASS
ok go.etcd.io/etcd/v3/client/integration 0.026s [no tests to run]
=== RUN TestBalancerUnderNetworkPartitionWatchLeader
--- PASS: TestBalancerUnderNetworkPartitionWatchLeader (1.65s)
=== RUN TestBalancerUnderNetworkPartitionWatchLeader
--- PASS: TestBalancerUnderNetworkPartitionWatchLeader (1.40s)
=== RUN TestBalancerUnderNetworkPartitionWatchLeader
--- PASS: TestBalancerUnderNetworkPartitionWatchLeader (1.23s)
PASS
ok go.etcd.io/etcd/v3/clientv3/integration 4.306s
testing: warning: no tests to run
PASS
ok go.etcd.io/etcd/v3/contrib/raftexample 0.013s [no tests to run]
Finished 'integration' pass at 2020年 9月16日 星期三 12时26分45秒 CST
Success
There was a problem hiding this comment.
CI works. thank you. @mitake
This was referenced Sep 30, 2020
gyuho
added a commit
that referenced
this pull request
Oct 12, 2020
…upstream-release-3.4 Automated cherry pick of #12264
jingyih
added a commit
that referenced
this pull request
Nov 16, 2020
…upstream-release-3.3 Automated cherry pick of #12264
bbiao
added a commit
to bbiao/etcd
that referenced
this pull request
Dec 14, 2020
Old etcdserver which have not apply pr of etcd-io#12165 will check auth token even if the request is a 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
added a commit
to bbiao/etcd
that referenced
this pull request
Dec 14, 2020
Old etcdserver which have not apply pr of etcd-io#12165 will check auth token even if the request is a 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
added a commit
to bbiao/etcd
that referenced
this pull request
Dec 14, 2020
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
added a commit
to bbiao/etcd
that referenced
this pull request
Dec 27, 2020
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
agargi
pushed a commit
to agargi/etcd
that referenced
this pull request
Jan 23, 2021
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
clientv3: get AuthToken automatically when clientConn is ready.
fixes: #11954