clientv3: do not refresh token when users use CommonName based authentication #14770
Conversation
Codecov Report
@@ Coverage Diff @@
## main #14770 +/- ##
==========================================
- Coverage 75.64% 75.44% -0.21%
==========================================
Files 457 457
Lines 37372 37375 +3
==========================================
- Hits 28270 28197 -73
- Misses 7336 7402 +66
- Partials 1766 1776 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
ahrtr
changed the title
|
When users use CommonName based authentication, then no need to fresh token at all. And actually the authTokenBundle is always nil, so the client will crash when refreshing the token. We do not have any e2e test cases to cover the Common Name based authentication so far. So I added the first one in this PR. @mitake Please take a look at this PR. thx. cc @ptabor @serathius @spzala |
|
I just noticed that we already have 3 cases to cover the CommonName based authentication in ctl_v3_auth_no_proxy_test.go, it's good news. |
| // apply the certificate which has `root` CommonName, | ||
| // and reset the setting when the test case finishes. | ||
| t.Log("Apply certificate with root CommonName") | ||
| resetCert := applyTLSWithRootCommonName() | ||
| defer resetCert() |
There was a problem hiding this comment.
There should be a separate cert bundle and an option in e2e tests pick certs with CommonName.
There was a problem hiding this comment.
I thought about enhancing the e2e framework, but it needs more time. I will think about it in a separate PR. Let's get 3.5.6 out firstly.
There was a problem hiding this comment.
Just rebased this PR, and added a TODO comment. PTAL.
…tication 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 <wachao@vmware.com>
Signed-off-by: Benjamin Wang <wachao@vmware.com>
Refer to etcd-io#14764 Signed-off-by: Benjamin Wang <wachao@vmware.com>
|
@ahrtr Thanks a lot for fixing the issue, the change LGTM |
Fix #14764
Signed-off-by: Benjamin Wang wachao@vmware.com
Please read https://github.com/etcd-io/etcd/blob/main/CONTRIBUTING.md#contribution-flow.