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

Error with TLS and require_ack_response #1554

Closed
mpeltonen opened this issue Apr 27, 2017 · 6 comments
Closed

Error with TLS and require_ack_response #1554

mpeltonen opened this issue Apr 27, 2017 · 6 comments

Comments

@mpeltonen
Copy link

Trying to setup (fluentd 14.14) out_forward plugin with TLS and at-least-once semantics. When I set require_ack_response true, I get this:

2017-04-27 14:51:15 +0300 [error]: #0 [aggregator_output] unexpected error while receiving ack message error_class=NoMethodError error="undefined method 'recv' for #<Fluent::PluginHelper::Socket::WrappedSocket::TLS:0x007f6849b06108>"
2017-04-27 14:51:15 +0300 [error]: #0 /opt/td-agent/embedded/lib/ruby/gems/2.1.0/gems/fluentd-0.14.14/lib/fluent/plugin/out_forward.rb:426:in 'read_ack_from_sock'
2017-04-27 14:51:15 +0300 [error]: #0 /opt/td-agent/embedded/lib/ruby/gems/2.1.0/gems/fluentd-0.14.14/lib/fluent/plugin/out_forward.rb:498:in 'block in ack_reader'
2017-04-27 14:51:15 +0300 [error]: #0 /opt/td-agent/embedded/lib/ruby/gems/2.1.0/gems/fluentd-0.14.14/lib/fluent/plugin/out_forward.rb:497:in 'each'
2017-04-27 14:51:15 +0300 [error]: #0 /opt/td-agent/embedded/lib/ruby/gems/2.1.0/gems/fluentd-0.14.14/lib/fluent/plugin/out_forward.rb:497:in 'ack_reader'
2017-04-27 14:51:15 +0300 [error]: #0 /opt/td-agent/embedded/lib/ruby/gems/2.1.0/gems/fluentd-0.14.14/lib/fluent/plugin_helper/thread.rb:78:in 'block in thread_create'

It looks like the reason is that Ruby's SSLSocket interface is different from TCPSocket (See for example https://bugs.ruby-lang.org/issues/12077 and https://bugs.ruby-lang.org/issues/8126).

There is probably a better way to fix this, but this seems to fix the issue for my case at least:

diff --git a/lib/fluent/plugin/out_forward.rb b/lib/fluent/plugin/out_forward.rb
index fb9d642..2eaf4c4 100644
--- a/lib/fluent/plugin/out_forward.rb
+++ b/lib/fluent/plugin/out_forward.rb
@@ -420,7 +420,7 @@ module Fluent::Plugin
     # return chunk id to be committed
     def read_ack_from_sock(sock, unpacker)
       begin
-        raw_data = sock.recv(@read_length)
+        raw_data = sock.instance_of? Fluent::PluginHelper::Socket::WrappedSocket::TLS ? sock.read(@read_length) : sock.recv(@read_length)
       rescue Errno::ECONNRESET
         raw_data = ""
       end
@repeatedly
Copy link
Member

@nurse How about this? Do we need to consider more points?

@nurse
Copy link
Contributor

nurse commented Apr 27, 2017

@repeatedly This code doesn't use socket specific options for recv. Therefore just raw_data = sock.read(@read_length) is also ok. (FIXED TYPO)

@mpeltonen
Copy link
Author

Therefore just raw_data = sock.recv(@read_length) is also ok.

Hmm, what do you mean by that? 'recv' is the method that is missing from SSLSocket and what causes the error with TLS transport?

@nurse
Copy link
Contributor

nurse commented Apr 27, 2017

@mpeltonen Oops sorry, it is typo. It must be raw_data = sock.read(@read_length)

@mpeltonen
Copy link
Author

Ah, right! I was also thinking that maybe just recv -> read would work, but was not sure about the semantics.

Would appreciate 0.14.16 soon with this change :)

@repeatedly
Copy link
Member

We will merge the patch soon: #1560

repeatedly added a commit that referenced this issue May 12, 2017
Call proper method for each connection type. fix #1554
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

No branches or pull requests

3 participants