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

Kafka Scaler with both SASL and SSL(TLS) #1043

Closed
zroubalik opened this issue Aug 25, 2020 · 3 comments · Fixed by #1074
Closed

Kafka Scaler with both SASL and SSL(TLS) #1043

zroubalik opened this issue Aug 25, 2020 · 3 comments · Fixed by #1074
Assignees
Milestone

Comments

@zroubalik
Copy link
Member

zroubalik commented Aug 25, 2020

First of all I'd like to say, that I am not a Kafka expert.

Currently there's no way, how to set both SSL (cert, key, cacert) and SASL (username, password) for client in Kafka scaler, look at the code below:

if meta.authMode != kafkaAuthModeForNone && meta.authMode != kafkaAuthModeForSaslSSL {
if authParams["username"] == "" {
return meta, errors.New("no username given")
}
meta.username = strings.TrimSpace(authParams["username"])
if authParams["password"] == "" {
return meta, errors.New("no password given")
}
meta.password = strings.TrimSpace(authParams["password"])
}
if meta.authMode == kafkaAuthModeForSaslSSL {
if authParams["ca"] == "" {
return meta, errors.New("no ca given")
}
meta.ca = authParams["ca"]
if authParams["cert"] == "" {
return meta, errors.New("no cert given")
}
meta.cert = authParams["cert"]
if authParams["key"] == "" {
return meta, errors.New("no key given")
}
meta.key = authParams["key"]
}
return meta, nil

There are several options for Authentication of Kafka Scaler, but I haven't found a one, that allows me to specify both.

Is this a correct behavior? IMHO it should be possible to specify both.

@zroubalik
Copy link
Member Author

FYI^ @tbickford @iyacontrol @ppatierno @grassiale @rasavant-ms @ahmelsayed @flecno

(sorry for the spam, but wasn't sure whom to ask, so I simply look at the contribution history on the Kafka Scaler 😅)

@ppatierno
Copy link
Contributor

@zroubalik from what I see here

func getKafkaClients(metadata kafkaMetadata) (sarama.Client, sarama.ClusterAdmin, error) {

it's supported by mixing SSL with other SASL mechanisms for authentication like PLAINTEXT and SCRAM-SHA with both username and password. It's also possible to use SSL client authentication using the certificates.
Said that ... I really think that both codes need a refactoring because I can understand that it's not clear.
Enabling SSL is a different thing than enabling a specific SASL authentication mechanism.
At scaler level configuration I would differentiate them in two different parameters (encryption/tls and authentication) instead of having these kafkaAuthModeForXXXXX const trying to mix every single way.

@zroubalik
Copy link
Member Author

zroubalik commented Aug 31, 2020

At scaler level configuration I would differentiate them in two different parameters (encryption/tls and authentication) instead of having these kafkaAuthModeForXXXXX const trying to mix every single way.

+1000 that code is a mess

@ppatierno OK, but while looking at the code, the only authmode where you can specify certificates is in this if block metadata.authMode == kafkaAuthModeForSaslSSL, but for this authmode, you can't specify username and password.

if metadata.authMode == kafkaAuthModeForSaslSSL {

@zroubalik zroubalik added the help wanted Looking for support from community label Sep 2, 2020
@zroubalik zroubalik self-assigned this Sep 3, 2020
@zroubalik zroubalik added this to the v2.0 milestone Sep 3, 2020
@zroubalik zroubalik removed the help wanted Looking for support from community label Sep 3, 2020
SpiritZhou pushed a commit to SpiritZhou/keda that referenced this issue Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants