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

fix(connect/proxy): remove h2 from NextProtos #6235

Closed
wants to merge 1 commit into from
Closed

fix(connect/proxy): remove h2 from NextProtos #6235

wants to merge 1 commit into from

Conversation

sh0rez
Copy link

@sh0rez sh0rez commented Jul 29, 2019

So far, the proxy used to announce that it supports http/2, which is not really
the case. Especially it made the native SDK think that the upstream app also
supports it, which broke support for every http/1.1 or older apps.

This has been solved adding a function to the SDK to obtain a Service without http/2 enabled.
The proxy switches to this from now, disabling http/2 support. This is not configurable, because I was unable to find a place to put this, without mixing L4 and L7 handling.

Feedback welcome :)

Fixes #4466

So far, the proxy used to announce that it supports http/2, which is not really
the case. Especially it made the native SDK think that the upstream app also
supports it, which broke support for every http/1.1 or older apps.
@hashicorp-cla
Copy link

hashicorp-cla commented Jul 29, 2019

CLA assistant check
All committers have signed the CLA.

@sh0rez
Copy link
Author

sh0rez commented Aug 12, 2019

Hej, would be awesome if sb could take a look or hint me in another direction if needed.

@hanshasselberg
Copy link
Member

This is no longer an issue because the builtin proxy is no longer part of consul and envoy should be able to do the correct thing.

@hanshasselberg
Copy link
Member

Well, the managed proxies were removed, but not the builtin proxy. Sorry!

@hanshasselberg
Copy link
Member

Thanks for you work @sh0rez! I think the solution we have in mind is a little different: #4466 (comment). Would you be interested in giving that a try?

@hanshasselberg hanshasselberg self-assigned this Feb 3, 2020
@hanshasselberg hanshasselberg added the waiting-reply Waiting on response from Original Poster or another individual in the thread label Feb 3, 2020
@hanshasselberg
Copy link
Member

Closing because no reply. I am happy to reopen in case there are updates. Thanks!

@ghost ghost removed the waiting-reply Waiting on response from Original Poster or another individual in the thread label Feb 11, 2020
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.

Consul Connect SDK HTTPClient
3 participants