feat(kafka): add support for custom certificates #3321
Conversation
Hello there! In our company we use custom certificates to authenticate to our Kafka instances. Unfortunately PeerDB doesn't support uploading custom certificates for Kafka peers. This PR adds the possibility. The PR reuses the same logic as used for Clickhouse peers. Thank you for your work, review, feedback & eventual merge ❤️🙏
|
|
| } | ||
| tlsSetting.Certificates = []tls.Certificate{cert} | ||
| } | ||
| if config.RootCa != nil { |
There was a problem hiding this comment.
consider setting up the root CAs with the shared.CreateTlsConfig helper
There was a problem hiding this comment.
I wanted to use it, but I've ran into the issue that for Kafka peer you may use multiple hosts split by comma. And shared.CreateTlsConfig() helper is designed to work with single host only.
So I can either refactor the shared.CreateTlsConfig() helper method to work with multiple hosts or keep it as it is. ClickHouse connector uses this approach as well.
What would you prefer? 🙏 Thank you for your time!
There was a problem hiding this comment.
ah that's fair, I think it would be nice to refactor that function to accept a slice of hosts and then change both kafka and clickhouse to use. Not a blocker for merging though
There was a problem hiding this comment.
OK, thanks @heavycrystal! So I'm keeping it as it is now and won't refactor.
flow/connectors/kafka/kafka.go
Outdated
| @@ -75,7 +76,26 @@ func NewKafkaConnector( | |||
| kgo.WithLogger(kgoLogger(logger)), | |||
| ) | |||
| if !config.DisableTls { | |||
| optionalOpts = append(optionalOpts, kgo.DialTLSConfig(&tls.Config{MinVersion: tls.VersionTLS13})) | |||
| tlsSetting := &tls.Config{MinVersion: tls.VersionTLS13} | |||
There was a problem hiding this comment.
is TLS1.3 too bleeding edge? I'm not too familiar with the Kafka ecosystem but iirc Confluent may not always support TLS 1.3
There was a problem hiding this comment.
Good point! I lowered the version to tls.VersionTLS12
Hello there!
In our company we use custom certificates to authenticate to our Kafka instances. Unfortunately PeerDB doesn't support uploading custom certificates for Kafka peers.
This PR adds the possibility. The PR reuses the same logic as used for Clickhouse peers.
Thank you for your work, review, feedback & eventual merge ❤️🙏