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

Recommended way to deal with proxies? #105

Open
chris05atm opened this issue Sep 6, 2019 · 4 comments
Open

Recommended way to deal with proxies? #105

chris05atm opened this issue Sep 6, 2019 · 4 comments
Labels
backlog A good idea, but not one we're planning on implementing in the near future enhancement New feature or request question Further information is requested

Comments

@chris05atm
Copy link

We would like to deploy a service that sits behind an egress proxy. All https communication outside of our AWS VPC requires egress through this proxy. I do not see a documented way to set a proxy for outgoing calls to Slack.

Describe the solution you'd like

The recommended way to set a https proxy.

Describe alternatives you've considered

I went diving through the code and suspect setting properties works.

System.setProperty("http.proxyHost", host);
System.setProperty("http.proxyPort", port);

Is this the recommended way to handle proxies for the foreseeable future? Can we document that and make it part of the API?

Additional context

Happy to submit PRs back once given a +1 that properties are the best way to handle this for now.

@chris05atm
Copy link
Author

I tried to update slack-client against the latest version of Horizon but hit shading issues around io.netty and DefaultAsyncHttpClientConfig:

'org.asynchttpclient.DefaultAsyncHttpClientConfig$Builder org.asynchttpclient.DefaultAsyncHttpClientConfig$Builder.setSslContext(com.hubspot.horizon.shaded.io.netty.handler.ssl.SslContext)')
java.lang.NoSuchMethodError: 'org.asynchttpclient.DefaultAsyncHttpClientConfig$Builder org.asynchttpclient.DefaultAsyncHttpClientConfig$Builder.setSslContext(com.hubspot.horizon.shaded.io.netty.handler.ssl.SslContext)'
        at com.hubspot.horizon.ning.NingAsyncHttpClient.<init>(NingAsyncHttpClient.java:68)

When using the shaded jars from slack-client things seem to break in ways I don't quite understand. I bumped the basebom to the latest published release and updated the io.netty version to be consistent among Horizon, basebom, and slack-client. I put a few hours into this this evening but unfortunately had to give up.

I've created a fork off Horizon 0.1.1 with proxy feature work but it isn't clear how to submit that back to the tip of master. Reading through git blame I see quite a few comments around trying to get shading to work so I imagine this isn't the first time shading has caused friction.

If anyone could lend advice I'm happy to take another crack at it.

Using the System.setProperty workaround above did not work for us in the end.

@johnnyleitrim johnnyleitrim added the question Further information is requested label Nov 13, 2019
@szabowexler
Copy link
Collaborator

Hmm -- @jhaber or @stevegutz before I dig too deep into this, do you guys have active knowledge of the best way to do a Horizon-based proxy setup?

@jhaber
Copy link
Member

jhaber commented Jan 8, 2020

I think we'd want to update HttpConfig to accept some proxy configuration, which the ApacheHttpClient and NingHttpClient would then be updated to respect

@szabowexler
Copy link
Collaborator

This may also tie into #141 - if we expose an API for configuring the backing http service, that would probably also improve support for proxies.

@szabowexler szabowexler added backlog A good idea, but not one we're planning on implementing in the near future enhancement New feature or request labels Jan 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog A good idea, but not one we're planning on implementing in the near future enhancement New feature or request question Further information is requested
Projects
None yet
Development

No branches or pull requests

4 participants