-
Notifications
You must be signed in to change notification settings - Fork 619
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
address concerns raised while troubleshooting #524 #581
Conversation
@pschultz @magiconair PR to address #524 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor nitpicks. Otherwise, LGTM.
Is it possible to have multiple authors on a single commit? Then you could attribute @pschultz as well. More curious than asking. |
I'm not sure about the commit itself ... I don't know. I fully agree though that @pschultz should get credit in the changelog and release notes at a minimum. |
@magiconair Any more thoughts on changing the default state of proxy-protocol on the listener? Do we want to keep it false as-is in the current PR? I also noticed that in the updated |
@sean- would disabling PROXY by default affect you? |
@magiconair per earlier question...you definitely can with GitHub: |
You'll learn something new every day. Thx for looking. |
d9b647b
to
86abe1c
Compare
@magiconair, no it wouldn't. We're not using the TCP |
Lets do this. Thank you all! |
@leprechau shoot, we forgot something. Can you provide another PR with an updated |
@magiconair absolutely. |
Also, fabio users: if you find this change and think disabling PROXY by default was a grave mistake please let us know. Thx |
@leprechau does this disable proxy for tcp only or also for http? |
This disables PROXY protocol on all listeners by default unless enabled with |
I'm having second thoughts on disabling by default. Will sleep on it a night. :) |
Okay, I have the documentation done. I'll push it to a new branch and create a PR. For what it's worth HAProxy has it disabled by default unless specified on the listener as well. |
Updated documentation here: #583 |
This PR, when merged, should resolve #524 and restore proper TCP proxy functionality with an out-of-box configuration.