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

Add SSL support #47

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add SSL support #47

wants to merge 2 commits into from

Conversation

skwashd
Copy link

@skwashd skwashd commented Mar 14, 2016

Adds initial support for SSL. The implementation supports the following
features:

  • Remote certificate validation
  • Custom CA bundles
  • Client side certs

SSL is now on by default.

Fixes #28

@vellamike
Copy link

Is this feature ready to be merged in? Would be very useful.

@vellamike
Copy link

Am I correct in assuming that this PR adds support for the lumberjack protocol?

@skwashd
Copy link
Author

skwashd commented Jun 7, 2016

@vellamike the library already implements the logstash json over HTTP protocol. This patch adds HTTPS support to the transport component.

@vellamike
Copy link

@skwashd my understanding is that the communication here is done over TCP, with no HTTP layer, and that logstash itself supports a custom TCP-like protocol called lumberjack which there is no python support for (though it is supported in node)

@dickolsson
Copy link

Code looks good. Fairly simple changes for such a major feature addition.

I would love for this to get merged. It's something we really need for a project.

@skwashd
Copy link
Author

skwashd commented Jun 28, 2016

@vellamike sorry you're right it is over TCP (now with optional SSL support).

See vklochan#28

Adds initial support for SSL. The implementation supports the following
features:

* Remote certificate validation
* Custom CA bundles
* Client side certs

SSL is now on by default.
@skwashd
Copy link
Author

skwashd commented Aug 10, 2016

@vklochan I've rebased current master into the patch. Is there anything else I need to do in order to get this merged?

@brenoarosa
Copy link

Can we have any update in this PR? This package does an amazing job for me but I need the SSL suport to use it.

@marji
Copy link

marji commented Oct 24, 2016

Could we please get this merged? It would be very useful.

@kramarz
Copy link

kramarz commented Feb 17, 2017

Can you make it compatible with python3? it complains that makeSocket does not accepts any arguments.
Change:

def makeSocket(self, timeout=1):
        s = super(TCPLogstashHandler, self).makeSocket(timeout)

To:

def makeSocket(self, *args, **kwargs):
        s = super(TCPLogstashHandler, self).makeSocket(*args, **kwargs)

@skwashd
Copy link
Author

skwashd commented Oct 5, 2017

@vklochan is there any thing I can do to help get this merged? Do you need a comaintainer to assist with cleaning up the backlog of issues and PRs?

@SalahAdDin
Copy link

@skwashd Finally, what did you do?

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

Successfully merging this pull request may close these issues.

7 participants