You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by GitBox <gi...@apache.org> on 2022/07/09 19:13:40 UTC
[GitHub] [kafka] kirktrue opened a new pull request, #12398: KAFKA-14062: Remove SaslExtensions overridden equals() and hashCode() methods
kirktrue opened a new pull request, #12398:
URL: https://github.com/apache/kafka/pull/12398
What
----
Kafka client is adding and removing the SASL extensions alternatively at the time of token refresh. During the window when the extensions are not present in the subject. If a connection to a broker is reattempted, it fails with the error that the extensions are missing.
Why
----
In clients, a Subject object is maintained which contains two sets each for Private and Public Credentials. Public Credentials includes the extensions. These values are stored in a `SaslExtensions` object which internally maintains these in a HashMap.
At the time of token refresh, a SaslExtensions object with these extensions is added to the public credentials set. As a next step, the refresh thread tries to logout the client for the older credentials. So it tries to remove the older token (private credential) and older SaslExtensions object (public credential) from the sets maintained in the Subject object.
SaslExtensions Class overrides the `equals` and `hashcode` functions and directly calls the `equals` and `hashcode` functions of HashMap. So at the time refresh when a new SaslExtensions object is added, because the extension values don't change, it results in a no-op because the hashes of the existing SaslExtensions object and the new object will be equals. But in the logout step, the only SaslExtensions object present in the set gets removed.
After removing the extensions in 1st refresh, the extensions will get added again at the time of 2nd refresh. So, this addition and removal keep happening alternatively.
The addition and removal of private credentials (tokens) from Subject work just fine because the tokens are always different.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions
Posted by GitBox <gi...@apache.org>.
kirktrue commented on PR #12398:
URL: https://github.com/apache/kafka/pull/12398#issuecomment-1179833908
The issue really does seem to boil down to how the `Subject` class implicitly expects the objects in its public credentials set to behave. They seem to really be expected to be unique. I'm not sure if we're violating some assumption or tenet in `OAuthBearerLoginModule`, SaslExtensions`, or not.
I don't see a way to change `OAuthBearerLoginModule` to account for the case when two instances in the set evaluated to being equal.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] omkreddy merged pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions
Posted by GitBox <gi...@apache.org>.
omkreddy merged PR #12398:
URL: https://github.com/apache/kafka/pull/12398
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions
Posted by GitBox <gi...@apache.org>.
kirktrue commented on PR #12398:
URL: https://github.com/apache/kafka/pull/12398#issuecomment-1179836722
Looking at this a little more, I see two classes (`OAuthBearerLoginModule` and `OAuthBearerRefreshingLogin`) that both seem to play a part in the login process. Given that `OAuthBearerRefreshingLogin` maintains a separate thread that modifies the `Subject`, I'm still wondering if there's some kind of race condition here 🤔
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions
Posted by GitBox <gi...@apache.org>.
kirktrue commented on PR #12398:
URL: https://github.com/apache/kafka/pull/12398#issuecomment-1179834093
cc @rondagostino
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions
Posted by GitBox <gi...@apache.org>.
kirktrue commented on PR #12398:
URL: https://github.com/apache/kafka/pull/12398#issuecomment-1179594826
cc @omkreddy @YiDing-Duke @stanislavkozlovski @rite2nikhil @emissionnebula @ijuma
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions
Posted by GitBox <gi...@apache.org>.
kirktrue commented on PR #12398:
URL: https://github.com/apache/kafka/pull/12398#issuecomment-1179832210
I've modified some existing unit tests in `OAuthBearerLoginModuleTest` that appear to exercise/mimic the refresh logic.
If the `equals` and `hashCode` method implementations in the `SaslExtensions` class are left in, the tests fail. When I remove the methods (as per @emissionnebula's change), the unit tests pass as expected.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions
Posted by GitBox <gi...@apache.org>.
kirktrue commented on PR #12398:
URL: https://github.com/apache/kafka/pull/12398#issuecomment-1179598843
Thanks for the feedback, @dajac! 😄
> Could we add a unit test?
Yes, I'd like to see that too.
@emissionnebula: has the testing for this been mostly manual up to this point?
I am willing to try to come up with one or more tests around this, but I can't commit to it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [kafka] dajac commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions
Posted by GitBox <gi...@apache.org>.
dajac commented on PR #12398:
URL: https://github.com/apache/kafka/pull/12398#issuecomment-1179598266
Could we add a unit test?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org