seldon-core
seldon-core copied to clipboard
Add Prefix for Kafka SSL Elements
When someone is using Kafka over SSL for logging messages to Kafka or consuming messages from Kafka to run predictions, we need to add the different certificates needed. While the current implementation works well for that, it only works for a single cluster.
In some cases we want to consume messages from a topic in a specific cluster and then log the responses of the model in a different topic in another cluster, it is not possible for now as we except the certificates to be the same when consuming and logging.
By adding a prefix to the environment variables used to get the different certificates, we could make it possible to use multiple clusters.
I suggest we only change the function GetSslElements() https://github.com/SeldonIO/seldon-core/blob/master/executor/api/util/utils.go#L164-L178
Instead of having
func GetSslElements() *SslKakfa {
sslElements := SslKakfa{
ClientCert: GetEnv("KAFKA_SSL_CLIENT_CERT", ""),
ClientKey: GetEnv("KAFKA_SSL_CLIENT_KEY", ""),
CACert: GetEnv("KAFKA_SSL_CA_CERT", ""),
// If we use path to files instead of string
ClientCertFile: GetEnv("KAFKA_SSL_CLIENT_CERT_FILE", ""),
ClientKeyFile: GetEnv("KAFKA_SSL_CLIENT_KEY_FILE", ""),
CACertFile: GetEnv("KAFKA_SSL_CA_CERT_FILE", ""),
// Optional password
ClientKeyPass: GetEnv("KAFKA_SSL_CLIENT_KEY_PASS", ""),
}
return &sslElements
}
We could have
func CheckPrefixSSLLogger() bool {
for _, envStr := range os.Environ() {
if strings.HasPrefix(envStr, 'LOGGER') {
return true
}
}
return false
}
func GetSslElements() *SslKakfa {
if CheckPrefixSSLLogger() {
sslElements := SslKakfa{
ClientCert: GetEnv("LOGGER_KAFKA_SSL_CLIENT_CERT", ""),
ClientKey: GetEnv("LOGGER_KAFKA_SSL_CLIENT_KEY", ""),
CACert: GetEnv("LOGGER_KAFKA_SSL_CA_CERT", ""),
}
return &sslElements
}
if CheckPrefixSSLExecutor() {
...
return &sslElements
}
sslElements := SslKakfa{
ClientCert: GetEnv("KAFKA_SSL_CLIENT_CERT", ""),
ClientKey: GetEnv("KAFKA_SSL_CLIENT_KEY", ""),
CACert: GetEnv("KAFKA_SSL_CA_CERT", ""),
// If we use path to files instead of string
ClientCertFile: GetEnv("KAFKA_SSL_CLIENT_CERT_FILE", ""),
ClientKeyFile: GetEnv("KAFKA_SSL_CLIENT_KEY_FILE", ""),
CACertFile: GetEnv("KAFKA_SSL_CA_CERT_FILE", ""),
// Optional password
ClientKeyPass: GetEnv("KAFKA_SSL_CLIENT_KEY_PASS", ""),
}
return &sslElements
}
N.B.: The code will be a bit different, it's only here to show what I have in mind.
I am available to write the code :D
This seems reasonable if we can make the current default easy so people don't need to specify both
A PR would indeed be welcome!
This issue is stale because it has been open 10 days with no activity. Remove stale label or comment or this will be closed in 5 days.
Still planned, I've been struggling because of the M1 processor on my mac and the different dependencies but it's still on my backlog.
Any update if this is still a priority for you?
Closing this. Please reopen if still needed.