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

SOCKS5 proxy for the Logstash connection #1106

Merged
merged 1 commit into from Mar 8, 2016

Conversation

Projects
None yet
5 participants
@andrewkroh
Copy link
Member

commented Mar 4, 2016

Adds the ability to use a SOCKS5 proxy on the Logstash output.

output:
  logstash:
    hosts: ["remote-host:5044"]
    proxy_url: socks5://user:password@socks5-proxy:2233
    #proxy_use_local_resolver: false

The documentation hasn't been updated yet. I'll do that in a separate PR.

@andrewkroh andrewkroh added the review label Mar 4, 2016

@@ -10,6 +10,7 @@ import (
"time"

"github.com/elastic/beats/libbeat/logp"
"golang.org/x/net/proxy"

This comment has been minimized.

Copy link
@ruflin

ruflin Mar 4, 2016

Collaborator

Small formatting issue

This comment has been minimized.

Copy link
@andrewkroh

andrewkroh Mar 4, 2016

Author Member

I separated the elastic packages from the third-party packages.

@ruflin

This comment has been minimized.

Copy link
Collaborator

commented Mar 4, 2016

Change LGTM but build is failing on travis (for libbeat). I bugs me that with 400 files the dependency is quite big, but I assume there is no way around it?

@andrewkroh andrewkroh force-pushed the andrewkroh:feature/logstash-socks5 branch 2 times, most recently from a33a147 to 4fba449 Mar 4, 2016

@andrewkroh

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2016

I removed the unused packages from golang.org/x/net. 432 files to 40. Let's see if it still builds.

@ruflin

This comment has been minimized.

Copy link
Collaborator

commented Mar 4, 2016

@andrewkroh Looks good
@urso Could you also have a look at this one and merge it?

@monicasarbu

This comment has been minimized.

Copy link
Contributor

commented Mar 4, 2016

Nice!

@urso

This comment has been minimized.

Copy link
Collaborator

commented Mar 4, 2016

we want to keep the test files?

e.g. vendor/golang.org/x/net/publicsuffix/table_test.go ~15k lines...

@andrewkroh andrewkroh force-pushed the andrewkroh:feature/logstash-socks5 branch from 4fba449 to 75a9545 Mar 4, 2016

@andrewkroh

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2016

@urso Good point, I removed the _test.go files.

@ruflin

This comment has been minimized.

Copy link
Collaborator

commented Mar 4, 2016

@andrewkroh Sounds like something we should do for all dependencies? Did you use a flag to do this?

@andrewkroh

This comment has been minimized.

Copy link
Member Author

commented Mar 4, 2016

No glide flags for this. I just used rm *_test.go.

@ruflin

This comment has been minimized.

Copy link
Collaborator

commented Mar 4, 2016

Ok, that means we have to "manually" remove then again when we use glide update.

@urso

This comment has been minimized.

Copy link
Collaborator

commented Mar 4, 2016

I kinda started to use glide.yml mostly to mention the repo + pin the version, but copy required files myself.

// socksProxyConfig holds the configuration information required to proxy
// Logstash connections through a SOCKS5 server.
type socksProxyConfig struct {
HostPort string `config:"hostport"` // host:port of the SOCKS5 server.

This comment has been minimized.

Copy link
@urso

urso Mar 4, 2016

Collaborator

maybe change config to 'host' + add ability to set username/password right in address (like we do support for other outputs)

This comment has been minimized.

Copy link
@andrewkroh

andrewkroh Mar 4, 2016

Author Member

How about I replace HostPort, Username, and Password with ProxyUrl (proxy_url in config)? This is more consistent with how you set the proxy for ES (it's also proxy_url). I'll use this method.

So you would write:

socks_proxy:
  proxy_url: socks5://username:password@mysocksproxy:1234
  remote_dns: true

This comment has been minimized.

Copy link
@andrewkroh

andrewkroh Mar 8, 2016

Author Member

I updated the PR. The config is now consistent with Elasticsearch (but requires a socks5:// URL).

output:
  logstash:
    hosts: ["remote-host:5044"]
    proxy_url: socks5://user:password@socks5-proxy:2233

@andrewkroh andrewkroh force-pushed the andrewkroh:feature/logstash-socks5 branch from 75a9545 to 848a2bb Mar 8, 2016

@andrewkroh

This comment has been minimized.

Copy link
Member Author

commented Mar 8, 2016

This updated this PR with documentation and added the proxy options to the configuration files.

tsg added a commit that referenced this pull request Mar 8, 2016

Merge pull request #1106 from andrewkroh/feature/logstash-socks5
SOCKS5 proxy for the Logstash connection

@tsg tsg merged commit e77b23e into elastic:master Mar 8, 2016

4 checks passed

CLA Commit author has signed the CLA
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
default Merged build finished.
Details
@tsg

This comment has been minimized.

Copy link
Collaborator

commented Mar 8, 2016

@elastic/beats (testing, testing, does this alias notifiy you?) It almost sounds like Godep was better for our needs? At least it did this cleanup automatically and we didn't have to manually delete the .git folders.

@ruflin

This comment has been minimized.

Copy link
Collaborator

commented Mar 8, 2016

@tsg For libbeat it is nice to have all the files inside, for the other dependencies agree. I like that we use now the vendor directory to follow the "golang" way, but lets see if something else then glide pops up. Its good that we know better and better what are requirements are.

@tsg

This comment has been minimized.

Copy link
Collaborator

commented Mar 8, 2016

@ruflin Godep now writes the dependencies in the vendor/ folder by default. I kind of prefer the cleaner yaml file, though.

Good point about libbeat, we need it with full files for the community beats. Not sure if any of the tools can do that selectively.

@andrewkroh andrewkroh deleted the andrewkroh:feature/logstash-socks5 branch Mar 8, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.