-
-
Notifications
You must be signed in to change notification settings - Fork 120
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
OAUTHBEARER OIDC support is not working since v0.16.0 #505
Comments
The fact that you do not have the background thread is probably because the callback now needs to be explicitly handled to handle oauthbearer token refresh requests in a periodic fashion (at 80% of the token validity time). It might have worked before because without explicit callback librdkafka would apply its own logic for it and not the one from the user (which allows for explicit token refreshes). I would not consider it a bug. You can try implementing the your own If you can prove that with the explicit callback usage this does not work I will consider this a bug but of a low priority unless more people are affected. Maybe @bruce-szalwinski-he can check it out if he wants but this is not something I am willing to invest time at the moment as the oauthbearer flows I use work and had no reports about them since they were released. If you consider this critical to your software you can support me and then I can prioritize it. If not, you will have to wait. |
Update to above: I looked at your code and most likely it is you relying on a undocumented behavior of implicit callback execution. This will not work since in order to support all flows an explicit one had to be included. Also your example does not show, that your version works after token validity time expires. |
Yes, you're right. My code relies on implicit callback execution.
This seems valid, but can you explain a bit more about this? Is it possible to change the code to fall back to implicit callbacks when explicit callbacks aren't passed? I think this would make users' lives a lot easier because implicit callbacks would probably work with most OIDC providers (as per KIP-768).
Yeah, you're right. My example doesn't show that it works after the token validity time expires, but I don't think we need to worry about that. The pull request confluentinc/librdkafka#3560 already includes logic to refresh the token before it expires. All the other wrappers around |
This change was made to allow any oauth flow possible including once based on IAM, etc. It allows for time sensitive tokens regeneration with an external provider code.
Probably yes but I am not willing to investigate nor work on this. I may accept a PR if someone else is willing. The current (new) solution is explicit and handles several more cases for many vendors.
Not true. The whole reason why we replaced the implicit with explicit was to support librdkafka explicit flow for any token flow.
This is exactly what we need to worry about. Implicit trigger on initial token set does not revalidate not re-update it. The librdkafka PR flow clearly states:
Meaning there is external action happening that has to be in sync with librdkafka. It may sound easy but it is not. You would have to setup timer for tracking, refresh flow + update flow. Update flow would have to happen when no polling is happening (or event queue split would have to be done). It is absolutely not user-friendly compared to explicit token refresh flow that can be implemented in rdkafka and that is part of karafka and waterdrop. With the explicit flow, there is no need to manage any timing or other details beyond token obtaining and updating as states in the docs. |
I'm not trying to be rude, but I don't really understand what you're getting at. From what I understand, it seems like you think librdkafka can only retrieve the token during initialization and that users need to explicitly refresh the token when it expires. However, librdkafka actually supports automatic token refresh out of the box, so users don't need to handle the timing manually. You can check the code here: link. |
You are not rude. Your understanding is correct and I did simplify things a bit. Some flows do have token refresh and librdkafka invokes the callback we (rdkafka) uses at the 80% of time. You can also do it fully manually. The flow that we have allows for invocation of token retrieval logic that is fully independent from librdkafka, allowing for third party token management that is unknown for librdkafka, for example such as https://github.com/bruce-szalwinski-he/aws-msk-iam-sasl-signer-ruby The explicit invocation of callback allows to support any token management as long as it is time sensitive. This allows for support of things like the one I linked. |
FYI there are several ways to manage token refreshes in regards to polling and event polling in librdkafka that with explicit callbacks make things like independent event handling possible when refresh is needed during long processing (aka bypassing max poll interval) and several other things. As said, I am not willing to invest my time into simplifying this flow unless there is a bigger community pressure of people wanting to deal with this stuff. My time is fairly limited and I do my best to pick up things to work on that will benefit the community the most. |
I am going to close it. As said, if you are willing to rework this to support the old case that would work for some in an automatic way (which should not be hard I think because it only requires a callback deregistration / lack of registration to default) I am willing to accept a PR :) |
The OAUTHBEARER OIDC support that was introduced in confluentinc/librdkafka#3560 is working fine in versions up to v0.15.0, but it's broken since version v0.16.0.
I'm using the code shown below:
Click to expand logs
When exactly same code is used with rdkafka v0.16.0+ it doesn't work.
Click to expand logs
As you can see from the logs, both versions v0.15.0 and v0.16.0 use the same version of librdkafka (v2.3.0), so this issue doesn’t seem to stem from the C code. I’ve also tested the same configuration with librdkafka v2.3.0 and confirmed that it works fine.
My guess is that this issue is related to the token refresh callbacks. Looking at the logs, it seems like the default callbacks provided by the C code are never called starting from version v0.16.0. Also, there are no logs from the background thread in v0.16.0 (those that start with
[thrd:background]
). When I checked the diff between v0.15.0 and v0.16.0, it looks like #410 touched the OAuthBearer-related code, so maybe something got broken there.The text was updated successfully, but these errors were encountered: