Skip to content
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

support sasl auth properties options to kafka connector #6439

Closed
davidkl97 opened this issue Dec 25, 2020 · 36 comments · Fixed by #8743
Closed

support sasl auth properties options to kafka connector #6439

davidkl97 opened this issue Dec 25, 2020 · 36 comments · Fixed by #8743
Assignees
Labels
enhancement New feature or request

Comments

@davidkl97
Copy link

Most of the production deployments of kafka clusters are using sasl authentication mechanism.
it would be nice to add this properties to the connector for production usage.

The properties should contain the mechanism usage, username and password etc for both the Client, Producer and Conumer.

Thanks.

@kokosing
Copy link
Member

Is this duplicate of #1184? Or do you mean different SASL mechanisms?

@davidkl97
Copy link
Author

I meant SASL Plaintext mechanism. Those mentioned issues relate to Kerbereos or SSL mechanisms...

I guess all of them are relevant for production usage and should be taken for consideration in the end.

@klDen klDen self-assigned this Apr 9, 2021
@klDen klDen added the enhancement New feature or request label Apr 13, 2021
@ashishmgofficial
Copy link

ashishmgofficial commented Jun 4, 2021

Hi @klDen , Is this still in active development ? We also had this requirement for SASL_SSL configurations . I have also raised a ticket for the same with expected couple of configs :

kafka.sasl.jaas.config
kafka.security.protocol=SASL_SSL
kafka.sasl.mechanism=PLAIN
kafka.sasl.username
kafka.sasl.password
kafka.group.id
basic.auth.credentials.source
basic.auth.user.info

#8197
My Organization is looking for introducing Trino to our Infra as Federated Query Engine and im doing this POC as part of its compatibility for our infra

@klDen
Copy link
Member

klDen commented Jun 4, 2021

Hello @ashishmgofficial ,

I was planning on working on this, but according to @lukasz-walkiewicz , a security team member at Starburst will be taking care of this :-)!

@ashishmgofficial
Copy link

@lukasz-walkiewicz Can you share any info on this ? Wanted to know the expected time this feature would be available to Trino and Starburst

@kokosing
Copy link
Member

One of the ideas would be to implement kafka.config.resources (like hive.config.resources) where user could pass their own properties that could be then used when connecting to Kafka services.

@klDen
Copy link
Member

klDen commented Jun 21, 2021

Hello @kokosing , does this idea means Trino won't be validating all configurations explicitely?

@kokosing
Copy link
Member

Yes. However Trino might override them. For example if you configure SSL in such file and SSL specific properties then they can be overriden.

@klDen
Copy link
Member

klDen commented Jun 21, 2021

Great. If this is the chosen solution, I'd be happy to help out on this.

@kokosing
Copy link
Member

Please do.

@vipinshreyaskumar
Copy link

@kokosing would you be able to point to an implementation snippet of kafka.config.resources usage with Trino. Our kafka cluster is configured with SASL_SSL at the moment.

@devsinghnaini
Copy link

yes we are also facing issue while connecting to kafka cluster which is SASL_SSL enabled from Trino.
@kokosing or @klDen Can you please help us on this configuration for kafka.config.resources or when will this be implemented in Trino.

@kokosing
Copy link
Member

It was an idea how this could be implemented. It is not yet implemented.

@klDen
Copy link
Member

klDen commented Jul 30, 2021

I'll start implementing this feature this weekend!

@gsvlad
Copy link

gsvlad commented Nov 8, 2021

hi guys. I would like to follow up a discussion.
In our project we want to stream data into Confluent topics from Trino, and Confluent uses SASL authentication method.
What is the current status of implementation of this feature? thanks

@klDen
Copy link
Member

klDen commented Nov 8, 2021

Hey @gsvlad , I have been busy on my end lately and didn't spend more time on this feature implementation.
What's currently missing are some Unit tests as well as fixing the CI build.
If there are still interest in this feature, I'll try resuming my work on this asap.

@gsvlad
Copy link

gsvlad commented Nov 8, 2021

This is much appreciated. I confirm the need as currently SASL authentication is the only supported by Confluent. Thank you!

@pichlerpa
Copy link

Hi everyone, I would also be interested in that feature, seems to be part of Starburst already? https://docs.starburst.io/latest/connector/starburst-kafka.html#sasl-authentication

@gsvlad
Copy link

gsvlad commented Nov 22, 2021

Hey @klDen, are there any updates? We are planning our further activities and some are related to this functionality. I would appreciate if you share your plans about it. Thank you

@klDen
Copy link
Member

klDen commented Nov 22, 2021

Hey @gsvlad , I'll resume my work here this week!

@whatsupbros
Copy link

whatsupbros commented Nov 29, 2021

@klDen, @kokosing, thanks for taking care of this feature!

Just wanted to point out that we also are waiting for it (being able to either propagate our own security configuration for the Kafka connection or being able to specify SASL_PLAIN as a security mechanism together with username/password/JAAS config).

And I can confirm that in my experience most Kafka clusters in Productive environments used SASL_PLAIN or SASL_SSL as their security mechanisms.

Also, Schema Registry connection usually requires SSL and authentication in prod envs, so, as @ashishmgofficial proposed, having configuration properties like these ones is also necessary (or an ability to provide them as a separate Schema Registry config file):

kafka.confluent-schema-registry-use-ssl=[true|false]
kafka.basic-auth-credentials-source=[URL|USER_INFO|SASL_INHERIT]
kafka.basic-auth-user-info=[username:password]

Looks like both things are already available in Starburst:

Thanks again!

@matt12eagles
Copy link

Someone posted in 2019 the ability to do this in the slack. (that and dynamically pull over topics).

Would still enjoy this functionality as well.

Thanks!

@whatsupbros
Copy link

whatsupbros commented Dec 1, 2021

Someone posted in 2019 the ability to do this in the slack.

@matt12eagles, you mean, this should be already possible to propagate custom security configuration to kafka connector (for both brokers and schema registry connections)? Would you be able to elaborate on it probably?

@matt12eagles
Copy link

I had read in slack that someone had got it working w/ a custom fork.. but I dont believe he/she had shared the code :(
I think they had SASL + SSL (schema registry) working. @Zza

@whatsupbros
Copy link

@matt12eagles:

I had read in slack that someone had got it working w/ a custom fork..

Please share the link to this fork, if you happen to find it here on GitHub.. This would be useful probably also for @klDen to simplify the effort.

@matt12eagles
Copy link

I don't have a link available, but any update if this is working or if anyone is working on this? May be able to take a shot at it if no one is working on this at the moment

@klDen
Copy link
Member

klDen commented Dec 9, 2021

Hey! Please try building from my PR branch. Im only missing product test in the PR, but it should be working and allow users to provide more custom Kafka configs. I'll work on the product tests very soon

@whatsupbros
Copy link

Greetings in the new year, @klDen! Any updates on including this enhancement in the official build probably?

@klDen
Copy link
Member

klDen commented Jan 11, 2022

Hello! I'll be resolving the comments this weekend! :)

@whatsupbros
Copy link

@klDen, any good/bad news probably? Sorry for bothering you..

@klDen
Copy link
Member

klDen commented Jan 25, 2022

@klDen, any good/bad news probably? Sorry for bothering you..

Hey! There's no news ATM. I'm waiting for the maintainers to review my PR.

@whatsupbros
Copy link

@klDen, thanks for the info, didn't notice the PR at first: #8743

Could we link it with this issue, probably?

@klDen
Copy link
Member

klDen commented Jan 26, 2022

I don't have the permission to link the PR here. People can refer to your comment and see the PR though 👍

@kokosing
Copy link
Member

I added link.

@whatsupbros
Copy link

@klDen hooray, the review was done! :)

@klDen
Copy link
Member

klDen commented Mar 1, 2022

@klDen hooray, the review was done! :)

Hey! I'll work on this end of next week. I'm currently AFK 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging a pull request may close this issue.

10 participants