Conversation
Signed-off-by: Kevin Leimkuhler <[email protected]>
hawkw
approved these changes
Apr 6, 2021
Contributor
hawkw
left a comment
hawkw
left a comment
There was a problem hiding this comment.
looks good to me!
for the tests, it might be nice to have a function for constructing Logicals to make that code a little more concise, but that's not a hard blocker.
hawkw
reviewed
Apr 6, 2021
Signed-off-by: Kevin Leimkuhler <[email protected]>
Contributor
Author
|
I may rename |
Signed-off-by: Kevin Leimkuhler <[email protected]>
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Apr 7, 2021
This release fixes a caching issue in the outbound proxy's "ingress mode" that could cause the incorrect client to be used for requests. This caching has been fixed so that clients cannot be incorrectly reused across logical destinations. --- * transport: introduce Bind trait and move orig dst to the type level (linkerd/linkerd2-proxy#957) * Initial fuzzer integration (linkerd/linkerd2-proxy#961) * Fix caching in ingress mode (linkerd/linkerd2-proxy#965)
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Apr 7, 2021
This release fixes a caching issue in the outbound proxy's "ingress mode" that could cause the incorrect client to be used for requests. This caching has been fixed so that clients cannot be incorrectly reused across logical destinations. --- * transport: introduce Bind trait and move orig dst to the type level (linkerd/linkerd2-proxy#957) * Initial fuzzer integration (linkerd/linkerd2-proxy#961) * Fix caching in ingress mode (linkerd/linkerd2-proxy#965)
Pothulapati
pushed a commit
to linkerd/linkerd2
that referenced
this pull request
Apr 14, 2021
This release fixes a caching issue in the outbound proxy's "ingress mode" that could cause the incorrect client to be used for requests. This caching has been fixed so that clients cannot be incorrectly reused across logical destinations. --- * transport: introduce Bind trait and move orig dst to the type level (linkerd/linkerd2-proxy#957) * Initial fuzzer integration (linkerd/linkerd2-proxy#961) * Fix caching in ingress mode (linkerd/linkerd2-proxy#965)
jijeesh
pushed a commit
to jijeesh/linkerd2
that referenced
this pull request
Apr 21, 2021
This release fixes a caching issue in the outbound proxy's "ingress mode" that could cause the incorrect client to be used for requests. This caching has been fixed so that clients cannot be incorrectly reused across logical destinations. --- * transport: introduce Bind trait and move orig dst to the type level (linkerd/linkerd2-proxy#957) * Initial fuzzer integration (linkerd/linkerd2-proxy#961) * Fix caching in ingress mode (linkerd/linkerd2-proxy#965) Signed-off-by: Jijeesh <[email protected]>
kleimkuhler
pushed a commit
to linkerd/linkerd2
that referenced
this pull request
May 12, 2021
This release fixes a caching issue in the outbound proxy's "ingress mode" that could cause the incorrect client to be used for requests. This caching has been fixed so that clients cannot be incorrectly reused across logical destinations. --- * transport: introduce Bind trait and move orig dst to the type level (linkerd/linkerd2-proxy#957) * Initial fuzzer integration (linkerd/linkerd2-proxy#961) * Fix caching in ingress mode (linkerd/linkerd2-proxy#965)
kleimkuhler
pushed a commit
to linkerd/linkerd2
that referenced
this pull request
May 12, 2021
This release fixes a caching issue in the outbound proxy's "ingress mode" that could cause the incorrect client to be used for requests. This caching has been fixed so that clients cannot be incorrectly reused across logical destinations. --- * transport: introduce Bind trait and move orig dst to the type level (linkerd/linkerd2-proxy#957) * Initial fuzzer integration (linkerd/linkerd2-proxy#961) * Fix caching in ingress mode (linkerd/linkerd2-proxy#965)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When the proxy is in
ingressmode, requests with differentl5d-dst-overrideheaders but the same address share the same key in the cache. This causes consecutive requests to the same address to all go to the value of the firstl5d-dst-overrideheader, ignoring the header values of later requests—unless the services are evicted from the cache.This occurs because each key into the cache is calculated based off
LogicalAddr'sorig_dstandprotocolfields (source). When the proxy is iningressmode, the value oforig_dstis overridden to0.0.0.0:port(source). This results in an easy key conflict: two different requests with the same port and protocol with result in the same key.To fix this, the
logical_addrfield is added toLogicalAddrand is used as a third field in calculating the cache key.logical_addris the value of thel5d-dst-overrideheader which means requests to the same address but with different header values will result in different keys.Fixes linkerd/linkerd2#5975
Testing
For manual testing, Linkerd can be installed with this version of the proxy:
Then, apply this manifest which creates two 3 deployments and 2 services:
injected nginx server and service
injected nginx server and service
injected
ingresspod withcurlused to send requests to1and2From the
curlpod, send requests with thel5d-dst-overrideheader set and observer withlinkerd viz tapthat the requests are received by the correct pods:Proxy tests
There are currently no tests for the
l5d-dst-overrideheader. I'll keep working on adding some additional tests for this, but those may come as a follow-up