java
java copied to clipboard
fix: AuthenticationRefresher wrapper solution
This fixes/relates to #2438 and #290
This commit introduces the AuthenticationRefresher as an implementation of Authentication. It's purpose is to warp another Authentication instance and refresh it very n seconds by calling provide on the wrapped Authentication object.
This code is intedned to be used a solution to issue #2438.
Any Authentication object that provides credentials, that must be refreshed, to the ApiClient do not have a way to do so.
There are two ways to use the AuthenticationRefresher with ClientBuilder. The first option is to use the ClientBuilder.setAuthentication(...) method. An Authentication instance can be wrapped in an AuthenticationRefresher and passed into the setAuthentication method. For example,
ClientBuilder.standard().setAuthentication(
//wrap an auth and refresh it every 15 minutes (900s)
new Authentication(someDelegateAuthenticationInstance, 900)
);
This is integrated up into the ClientBuilder.build() method by adding a new authenticationRefreshSeconds field and getter/setter pair. If a refresh interval is set, the Authentication object used by ClientBuilder will be wrapped with an AuthenticationRefresher.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: raykrueger Once this PR has been reviewed and has the lgtm label, please assign yue9944882 for approval. For more information see the Kubernetes Code Review Process.
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
Welcome @raykrueger!
It looks like this is your first PR to kubernetes-client/java 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.
You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.
You can also check if kubernetes-client/java has its own contribution guidelines.
You may want to refer to our testing guide if you run into trouble with your tests not passing.
If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!
Thank you, and welcome to Kubernetes. :smiley:
@raykrueger is there a way/need to be able to cancel the task running in the executor?
@raykrueger is there a way/need to be able to cancel the task running in the executor?
Yep! I'm combining Brendan's concern with some WeakReference foo to shut the timerdown when the APIClient gets GC'd. Commit incoming!
I started the code review thing on accident and don't know what to do about that heh.
(Sorry for spamming, wanted a new line in the previous message). Please have a look at the latest. I've added some safety around time timer and moved it to a field. The timer is now cancelled and nulled out if the APICilent is garbage collected. Additionally I removed the flaky Thread.sleep timing from the tests in favor of Countdown Latches.
Few more comments. Thanks.
Ok, I've simplified things a bit. Let me know if this makes better sense. Thanks!
I pushed some code a few days ago, but didn’t reach out here. Back to you sir!
/ok-to-test
Sorry, I feel like I'm dragging this code review on and on, but I think this code could be simpler. Added a comment.
any update on this - or is this comment still waiting to be resolved?
@bryantbiggs still waiting afaik.
btw, I dug into this deeper, this won't work correctly because when we construct the ApiClient the token is set once and it is never revisited.
Making token refresh work will require changes in the upstream code generator, or a patch to ApiClient.
Closing this for now since I think we will need to start with a fresh solution.
See: https://github.com/OpenAPITools/openapi-generator/pull/6036