etcd: fix JWT renewal with user/pass authentication #6905
Merged
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
|
@klizhentas has shown concern about moving to a beta version of etcd for a production release #6183. However it doesn't seem clear when etcd plans on releasing 3.5 and this has pushed back a few other fixes as well. @awly Do you have a sense of how stable the current beta version is or how soon it'll be released officially? |
r0mant
reviewed
May 18, 2021
There is a data race in etcd that breaks the internal state in etcd client implementation for some server setups (user/pass authentication with JWTs).
|
You're right, let's wait for etcd 3.5 to be fully released before updating. |
Joerger
approved these changes
May 19, 2021
r0mant
approved these changes
May 19, 2021
awly
pushed a commit
that referenced
this pull request
May 19, 2021
There is a data race in etcd that breaks the internal state in etcd client implementation for some server setups (user/pass authentication with JWTs).
awly
pushed a commit
that referenced
this pull request
May 19, 2021
There is a data race in etcd that breaks the internal state in etcd client implementation for some server setups (user/pass authentication with JWTs).
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.
Two changes here:
the implementation of
b.client.Status(in the etcd library) internally causes the in-memory JWT store to be overwritten.this ends up breaking the JWT refresh logic (one store has the exired token but a different one gets the refreshed one).
I will send an upstream fix too
# 2 works without # 1.
Since this PR is against
master, I decided to keep # 1 anyway, for 7.0.I will cherry-pick only # 2 to 6.2, without dependency changes.
FYI @fspmarshall because you had some memory issues with the etcd client; check if this updated version of the library fixes them
Fixes #6881