Closed bachmanity1 closed 1 month ago
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 oauthbearer_token_refresh_callback
and see if it is called and if it is, just follow the code docs to implement your own flow for this.
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.
I looked at your code and most likely it is you relying on a undocumented behavior of implicit callback execution.
Yes, you're right. My code relies on implicit callback execution.
This will not work since in order to support all flows an explicit one had to be included.
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).
Also your example does not show, that your version works after token validity time expires.
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 https://github.com/confluentinc/librdkafka/pull/3560 already includes logic to refresh the token before it expires. All the other wrappers around librdkafka
rely on this logic too.
This seems valid, but can you explain a bit more about this?
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.
Is it possible to change the code to fall back to implicit callbacks when explicit callbacks aren't passed?
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.
I think this would make users' lives a lot easier because implicit callbacks would probably work with most OIDC providers (as per KIP-768).
Not true. The whole reason why we replaced the implicit with explicit was to support librdkafka explicit flow for any token flow.
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.
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:
Retrieve jwt from token provider and forward it to the broker
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.
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 https://github.com/confluentinc/librdkafka/pull/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.