-
Notifications
You must be signed in to change notification settings - Fork 1.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update updated OpenId access and refresh token in memory config #5888
Conversation
@ttbadr : Could you please run |
@rohanKanojia fixed |
...netes-client-api/src/main/java/io/fabric8/kubernetes/client/utils/OpenIDConnectionUtils.java
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
import java.util.Collections; | ||
import java.util.LinkedHashMap; | ||
import java.util.Map; | ||
import java.util.*; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please revert this import related change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thx!
@@ -242,6 +240,12 @@ static boolean persistKubeConfigWithUpdatedToken(Config currentConfig, Map<Strin | |||
*/ | |||
public static boolean persistKubeConfigWithUpdatedAuthInfo(Config currentConfig, Consumer<AuthInfo> updateAction) | |||
throws IOException { | |||
AuthInfo authInfo = new AuthInfo(); | |||
authInfo.setAuthProvider(new AuthProviderConfig(new HashMap<>(2), currentConfig.getAuthProvider().getName())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This leads to a NPE if the current config has no auth provider set
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently working on some refactors and fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This leads to a NPE if the current config has no auth provider set
I think the OpenIDConnectionUtils wouldn't be invoked if no auth provider set, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not, but that's unclear in tests.
Also the method is reused elsewhere (i.e. this one is called from the OpenShiftOAuthInterceptor) because it was public.
Anyway, this might have some ramifications. I'll work on refactoring some of this stuff and maybe moving it elsewhere to avoid latent bugs.
Description
Fix #5880
Type of change
test, version modification, documentation, etc.)
Checklist