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.

Sign up for GitHub

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

Jump to bottom

0.50.0-rc1 needs to correctly support use_proxy_proto #1050

Closed
kflynn opened this issue Dec 19, 2018 · 4 comments
Closed

0.50.0-rc1 needs to correctly support use_proxy_proto #1050

kflynn opened this issue Dec 19, 2018 · 4 comments
Assignees
@kflynn
Milestone
0.50.0 GA

Comments

@kflynn
Copy link
Member

kflynn commented Dec 19, 2018

If you have use_proxy_proto in your ambassador module, but you don't have any TLS termination contexts, we don't actually set use_proxy_proto in the Envoy config in 0.50.0-rc1 (and likely not in -ea7 either, as it appears that the bug was introduced with SNI support).

The fix is probably just to make sure that the base filter chains in v2listener.py, even without SNI, properly set use_proxy_proto. We also need to better test this (obviously).
@kflynn kflynn added this to the 0.50.0 GA milestone Dec 19, 2018
@kflynn
Copy link
Member Author

kflynn commented Dec 19, 2018

@containscafeine, if you can take a look at this during your day, that'd be wonderful! 😄

@kflynn
Copy link
Member Author

kflynn commented Dec 19, 2018

(Probably the fix is a change to the default filter chain at line 280 in v2listener.py... and testing. 😐 )

@kflynn kflynn assigned kflynn and unassigned concaf Dec 20, 2018
@iNoahNothing
Copy link
Contributor

This issue is reproduce-able with a TLS context configured.

https://gist.github.com/nbkrause/f9980d9a19d5b510b1e3e8bbeb75d1b9

@kflynn kflynn changed the title 0.50.0-rc1 doesn't support proxy proto without TLS 0.50.0-rc1 needs to correctly support use_proxy_proto Dec 20, 2018
kflynn pushed a commit that referenced this issue Dec 20, 2018
Configure use_proxy_proto correctly (fixes #1050).
9ab1f0f
@kflynn kflynn mentioned this issue Dec 20, 2018
Merged
@kflynn
Copy link
Member Author

kflynn commented Dec 24, 2018

Fixed in #1061.

@kflynn kflynn closed this as completed Dec 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kflynn kflynn

Labels
None yet
Projects
None yet
Milestone
0.50.0 GA
Development

No branches or pull requests
3 participants
@kflynn @concaf @iNoahNothing