[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions

2022-07-10 Thread GitBox


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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions

2022-07-10 Thread GitBox


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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions

2022-07-10 Thread GitBox


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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions

2022-07-10 Thread GitBox


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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions

2022-07-09 Thread GitBox


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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



[GitHub] [kafka] kirktrue commented on pull request #12398: KAFKA-14062: OAuth client token refresh fails with SASL extensions

2022-07-09 Thread GitBox


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-unsubscr...@kafka.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org