-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add fine tuned oath/sasl configuration to kafka client #189
Add fine tuned oath/sasl configuration to kafka client #189
Conversation
Add changelog entry and increment minor version for new configuration features.
This commit updates the plugin defaults to align with the default (or lack of default) values in the underlying kafka client library. https://github.com/apache/kafka/blob/3.8.1/clients/src/main/java/org/apache/kafka/common/TopicIdPartition.java
Co-authored-by: Mashhur <[email protected]>
Co-authored-by: Mashhur <[email protected]>
Co-authored-by: Mashhur <[email protected]>
Co-authored-by: Mashhur <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Thank you so much Cas.
The last failed CI seems to me transient.
Reload yeilded a green run. Going to merge and release. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left suggestion inline to use order options alphabetically.
| <<plugins-{type}s-{plugin}-sasl_oauthbearer_token_endpoint_url>> |<<string,string>>|No | ||
| <<plugins-{type}s-{plugin}-sasl_oauthbearer_scope_claim_name>> |<<string,string>>|No | ||
| <<plugins-{type}s-{plugin}-sasl_login_callback_handler_class>> |<<string,string>>|No | ||
| <<plugins-{type}s-{plugin}-sasl_login_connect_timeout_ms>> |<<number,number>>|No | ||
| <<plugins-{type}s-{plugin}-sasl_login_read_timeout_ms>> |<<number,number>>|No | ||
| <<plugins-{type}s-{plugin}-sasl_login_retry_backoff_ms>> |<<number,number>>|No | ||
| <<plugins-{type}s-{plugin}-sasl_login_retry_backoff_max_ms>> |<<number,number>>|No |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The one got by me.
For next time: Options should be in alpha order in both option list and descriptions.
Add plugin configuration options for both input/output to support oauth and sasl authentication.
This PR aggregates #183 and #181 and adds some unit tests.
I found it difficult to spin up a valid test environment to fully test the integrations, but looking at the options in https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/common/config/SaslConfigs.java I think the added unit tests show that we are piping through the requested configuration to the kafka clients.
Closes #180