http: fixing UPSTREAM_REMOTE_ADDRESS for happy eyeballs, proxied connections#23139
http: fixing UPSTREAM_REMOTE_ADDRESS for happy eyeballs, proxied connections#23139yanavlasov merged 4 commits intoenvoyproxy:mainfrom
Conversation
|
@Augustyniak is having this in stream info sufficient for you folks, or do you want it in access logs as well? |
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
alyssawilk
force-pushed
the
upstream_address
branch
from
115d326 to
60cd202
Compare
adisuissa
left a comment
adisuissa
left a comment
There was a problem hiding this comment.
Thanks for refactoring the connection_info.
LGTM.
/assign-from @envoyproxy/senior-maintainers
|
@envoyproxy/senior-maintainers assignee is @yanavlasov |
|
/wait on CI |
Signed-off-by: Alyssa Wilk <alyssar@chromium.org>
Augustyniak
left a comment
Augustyniak
left a comment
There was a problem hiding this comment.
It works for us - It will be handy to have this in our proxy tests - thank you!.
One comment - not sure about the name of added headers it self - remote_address. I think that Envoy usually prefixes its headers with x-envoy and uses - as opposed to _ to separate words
|
I would rename |
|
Ah, looks like that can be added to response headers via UPSTREAM_REMOTE_ADDRESS only it's arguably broken for H-A and proxying so I'll go fix that. Thanks! |
|
/wait |
alyssawilk
changed the title
| # *Changes that may cause incompatibilities for some users, but should not for most* | ||
| - area: logging | ||
| change: | | ||
| changed the ``UPSTREAM_REMOTE_ADDRESS``, ``UPSTREAM_REMOTE_ADDRESS_WITHOUT_PORT``, and ``UPSTREAM_REMOTE_PORT`` fields to log based on the actual upstream connection rather than the upstream host. This fixes a bug where the address components were not consistently correct for Happy Eyeballs connections and proxied connections, but also means in cases where the host was selected but a connection was not established, the fields will be absent. This change can be temporarily reverted by setting the runtime guard ``envoy.reloadable_features.correct_upstream_address`` to false. |
There was a problem hiding this comment.
correct_upstream_address -> correct_remote_address
4 participants
Upstream host is already accessible, but in the case of proxying or happy eyeballs, one can't always infer the address from the host, so adding the remote address to the UpstreamStreamInfo and using it in place of the host's first address.
This also changes the default IPv4 upstream address in integration tests from 0.0.0.0 to 127.0.0.1 rather than snag the port across all addresses.
Risk Level: medium but guarded
Testing: integration test
Docs Changes: n/a
Release Notes: inline