mailgun / kafka-pixy

gRPC/REST proxy for Kafka
Apache License 2.0
768 stars 119 forks source link

Support TLS connection to Kafka #180

Closed ahamidi closed 4 years ago

ahamidi commented 4 years ago

This PR adds support for TLS connections to Kafka. It introduces the following new properties to the Kafka configuration:

Fixes #178

mailgun-ci commented 4 years ago

Can one of the admins verify this patch?

horkhe commented 4 years ago

@ahamidi so far so good :)

ahamidi commented 4 years ago

@horkhe I haven't had a chance to test it against a Kafka cluster yet, but hopefully will get a chance to in the next few days.

I noticed that not all configurations options were exposed directly as CLI flags. Was that a conscious decision to simplify the options? Should I trim the Kafka TLS options to the basics/most common?

horkhe commented 4 years ago

We only exposed bare minimum of command line parameters for simplest configuration option. Mostly to allow running kafka-pixy on a dev laptop for testing. In production kafka-pixy is supposed to run with only one cmd argument --config. Although I see no harm in exposing the TLS related command line arguments, if only to allow quick tests that TLS connections is working.

Please let me know how your testing goes. Also please squash all the commits with the commit message "Add support for TLS connection to Kafka".

And thanks again.

@thrawn01 do you have anything to add?

thrawn01 commented 4 years ago

LGTM!

ahamidi commented 4 years ago

I was able to test this against Amazon MSK with TLS authentication enabled and it works as expected.

I'm not sure why the test is failing though (fails in multiplexer_test.go)