-
Notifications
You must be signed in to change notification settings - Fork 175
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
Use Ruby Kafka's ssl_ca_cert_file_path parameter to feed the CA certs #410
Conversation
Signed-off-by: Sergey Yedrikov <syedriko@redhat.com>
6ed6879
to
e8a2e06
Compare
/cc @kenhys |
The dependency just got approved: zendesk/ruby-kafka#905 (review) |
zendesk/ruby-kafka#905 has merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
ruby-kafka 1.4.0 has been released: https://rubygems.org/gems/ruby-kafka/versions/1.4.0
Although it's not described in CHANGELOG.md, it seems that it ships this feature too. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ssl_ca_cert
option of this plugin accepts an array of paths but ruby-kafka 1.3.0 or before doesn't. So we should update the gemspec to require ruby-kafka 1.4.0 or later.
Array support of ssl_ca_cert_file_path is needed: zendesk/ruby-kafka#905 fluent#410 Signed-off-by: Takuro Ashie <ashie@clear-code.com>
I've added the commit to this pull request. |
Merged. Thanks for your contribution! |
This is another take on #390, with a more correct way of fixing #287, where the Fluentd Kafka plugin doesn't need to know how to parse PEM certificates. This PR depends on zendesk/ruby-kafka#905.
It is also in line with #287 (comment), which I completely agree with.