OAuthbearer support #74
Conversation
|
Hello, The implementation looks ok. But please fix the duplicate code issues I mentioned in the review. |
|
Hi! I can't see the review. Maybe you forgot to submit it. Thanks |
|
You cannot see the above change requests ? |
|
No, I can't. I think you have to submit the review (it's in 'pending' state) |
c_src/erlkaf_producer.cc
Outdated
| @@ -417,3 +444,105 @@ ERL_NIF_TERM enif_produce(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) | |||
|
|
|||
| return ATOMS.atomOk; | |||
| } | |||
|
|
|||
| char** split_producer_extensions(const std::string extensions_str, size_t* length) | |||
There was a problem hiding this comment.
Seems duplicate code with split_consumer_extensions . Move them into a dedicated file and include it in both consumer and producer modules.
There was a problem hiding this comment.
I have moved this function to a new file called erlkaf_oauthbearer.cc
c_src/erlkaf_producer.cc
Outdated
| return extensions; | ||
| } | ||
|
|
||
| void free_producer_extensions(char** extensions, size_t length) |
There was a problem hiding this comment.
Same as above. duplicated code
There was a problem hiding this comment.
I have moved this function to a new file called erlkaf_oauthbearer.cc
| } | ||
| } | ||
|
|
||
| ERL_NIF_TERM enif_producer_oauthbearer_set_token(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) |
There was a problem hiding this comment.
Lot of duplicate code with enif_consumer_oauthbearer_set_token. For sure you can refactor to have same function in both.
There was a problem hiding this comment.
I have moved some of this functionality (all but parsing the arguments) to a new file called erlkaf_oauthbearer.cc
| } | ||
| } | ||
|
|
||
| ERL_NIF_TERM enif_producer_oauthbearer_set_token_failure(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[]) |
There was a problem hiding this comment.
Lot of duplicate code with enif_consumer_oauthbearer_set_token_failure. For sure you can refactor to have same function in both.
There was a problem hiding this comment.
I have moved some of this functionality (all but parsing the arguments) to a new file called erlkaf_oauthbearer.cc
|
Now you can see it ? |
Hi!
I have opened this PR to include the necessary functionality to authenticate Erlkaf consumers and producers using the OAuthbearer protocol:
OIDC authentication passing the required parameters to librdkafka:
sasl_oauthbearer_method, sasl_oauthbearer_client_id, sasl_oauthbearer_client_secret, sasl_oauthbearer_scope sasl.oauthbearer.extensions, sasl_oauthbearer_token_endpoint_urlCustom refresh token callback to provide users a way to implement their retrieve access token function and pass it to librdkafka
I have needed to enable this options in order to retrieve the first token before trying to establish the connection with the brokers:
This way, the refresh callbacks are triggered automatically by librdkafka in a background thread.
Any suggestions or changes you think I should make, please let me know.