dst: add Opaque hint to destination.ProtocolHint
#197
Merged
Conversation
This file contains 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
This indicates that the destination will handle the connection as an opaque TCP byte stream, and that HTTP/2 upgrades should not be performed. If the `opaque_transport` flag is also set for this destination, then the proxy should not send a session protocol in its transport header.
adleong
approved these changes
Feb 6, 2023
kleimkuhler
approved these changes
Feb 7, 2023
This was referenced Feb 7, 2023
hawkw
added a commit
to linkerd/linkerd2
that referenced
this pull request
Feb 9, 2023
The outbound proxy handles endpoints with the `opaque_transport` flag by opening a direct connection to the inbound proxy's inbound listener port, and sending a ProtoBuf `TransportHeader` including the target port of the originating outbound connection and an (optional) `SessionProtocol` describing the protocol used on that connection. Currently, outbound proxies initiating direct connections will *always* send `SessionProtocol` values communicating the protocol as understood by the outbound proxy. However, this is not always the desired behavior. Direct connections with `TransportHeader`s are used in two cases: for gateway connections, and for ports which are marked as opaque. When the inbound port is marked as opaque, the presence of a `SessionProtocol` tells the inbound proxy to handle that connection as the indicated protocol, which results in incorrect behavior when the inbound proxy's ServerPolicy configures the target port as opaque (see #9888). Therefore, the `Destination` proxy API has been updated to add a new `ProtocolHint`, `Opaque`, which indicates that an outbound proxy should _not_ send a `SessionProtocol` when initiating a direct connection, even if the outbound proxy handled the connection as HTTP. This hint was added to the proxy API in linkerd/linkerd2-proxy-api#197, and released in `linkerd2-proxy-api` v0.8.0. This branch updates the Destination controller's dependency on `linkerd2-proxy-api` to v0.8.0, and changes the controller to send an `Opaque` protocol hint when the target port is marked as opaque on the destination pod. This should override the `H2` protocol hint that is added when the destination is meshed. I've also added a new test for this behavior. Fixes #9888 (along with linkerd/linkerd2-proxy#2209, which changes the proxy to actually handle the `Opaque` protocol hint).
This was referenced Feb 9, 2023
hawkw
added a commit
to linkerd/linkerd2-proxy
that referenced
this pull request
Apr 13, 2023
Currently, when the outbound proxy makes a direct connection prefixed with a `TransportHeader` in order to send HTTP traffic, it will always send a `SessionProtocol` hint with the HTTP version as part of the header. This instructs the inbound proxy to use that protocol, even if the target port has a ServerPolicy that marks that port as opaque, which can result in incorrect handling of that connection. See linkerd/linkerd2#9888 for details. In order to prevent this, linkerd/linkerd2-proxy-api#197 adds a new `ProtocolHint` value to the protobuf endpoint metadata message. This will allow the Destination controller to explicitly indicate to the outbound proxy that a given endpoint is known to handle all connections to a port as an opaque TCP stream, and that the proxy should not perform a protocol upgrade or send a `SessionProtocol` in the transport header. This branch updates the proxy to handle this new hint value, and adds tests that the outbound proxy behaves as expected. Along with linkerd/linkerd2#10301, this will fix linkerd/linkerd2#9888. I opened a new PR for this change rather than attempting to rebase my previous PR #2209, as it felt a bit easier to start with a new branch and just make the changes that were still relevant. Therefore, this closes #2209.
hawkw
added a commit
to linkerd/linkerd2
that referenced
this pull request
Apr 14, 2023
The outbound proxy handles endpoints with the `opaque_transport` flag by opening a direct connection to the inbound proxy's inbound listener port, and sending a ProtoBuf `TransportHeader` including the target port of the originating outbound connection and an (optional) `SessionProtocol` describing the protocol used on that connection. Currently, outbound proxies initiating direct connections will *always* send `SessionProtocol` values communicating the protocol as understood by the outbound proxy. However, this is not always the desired behavior. Direct connections with `TransportHeader`s are used in two cases: for gateway connections, and for ports which are marked as opaque. When the inbound port is marked as opaque, the presence of a `SessionProtocol` tells the inbound proxy to handle that connection as the indicated protocol, which results in incorrect behavior when the inbound proxy's ServerPolicy configures the target port as opaque (see #9888). Therefore, the `Destination` proxy API has been updated to add a new `ProtocolHint`, `Opaque`, which indicates that an outbound proxy should _not_ send a `SessionProtocol` when initiating a direct connection, even if the outbound proxy handled the connection as HTTP. This hint was added to the proxy API in linkerd/linkerd2-proxy-api#197, and released in `linkerd2-proxy-api` v0.8.0. This branch updates the Destination controller's dependency on `linkerd2-proxy-api` to v0.8.0, and changes the controller to send an `Opaque` protocol hint when the target port is marked as opaque on the destination pod. This should override the `H2` protocol hint that is added when the destination is meshed. I've also added a new test for this behavior. Fixes #9888 (along with linkerd/linkerd2-proxy#2209, which changes the proxy to actually handle the `Opaque` protocol hint).
risingspiral
pushed a commit
to linkerd/linkerd2
that referenced
this pull request
May 4, 2023
The outbound proxy handles endpoints with the `opaque_transport` flag by opening a direct connection to the inbound proxy's inbound listener port, and sending a ProtoBuf `TransportHeader` including the target port of the originating outbound connection and an (optional) `SessionProtocol` describing the protocol used on that connection. Currently, outbound proxies initiating direct connections will *always* send `SessionProtocol` values communicating the protocol as understood by the outbound proxy. However, this is not always the desired behavior. Direct connections with `TransportHeader`s are used in two cases: for gateway connections, and for ports which are marked as opaque. When the inbound port is marked as opaque, the presence of a `SessionProtocol` tells the inbound proxy to handle that connection as the indicated protocol, which results in incorrect behavior when the inbound proxy's ServerPolicy configures the target port as opaque (see #9888). Therefore, the `Destination` proxy API has been updated to add a new `ProtocolHint`, `Opaque`, which indicates that an outbound proxy should _not_ send a `SessionProtocol` when initiating a direct connection, even if the outbound proxy handled the connection as HTTP. This hint was added to the proxy API in linkerd/linkerd2-proxy-api#197, and released in `linkerd2-proxy-api` v0.8.0. This branch updates the Destination controller's dependency on `linkerd2-proxy-api` to v0.8.0, and changes the controller to send an `Opaque` protocol hint when the target port is marked as opaque on the destination pod. This should override the `H2` protocol hint that is added when the destination is meshed. I've also added a new test for this behavior. Fixes #9888 (along with linkerd/linkerd2-proxy#2209, which changes the proxy to actually handle the `Opaque` protocol hint).
risingspiral
pushed a commit
to linkerd/linkerd2
that referenced
this pull request
May 5, 2023
The outbound proxy handles endpoints with the `opaque_transport` flag by opening a direct connection to the inbound proxy's inbound listener port, and sending a ProtoBuf `TransportHeader` including the target port of the originating outbound connection and an (optional) `SessionProtocol` describing the protocol used on that connection. Currently, outbound proxies initiating direct connections will *always* send `SessionProtocol` values communicating the protocol as understood by the outbound proxy. However, this is not always the desired behavior. Direct connections with `TransportHeader`s are used in two cases: for gateway connections, and for ports which are marked as opaque. When the inbound port is marked as opaque, the presence of a `SessionProtocol` tells the inbound proxy to handle that connection as the indicated protocol, which results in incorrect behavior when the inbound proxy's ServerPolicy configures the target port as opaque (see #9888). Therefore, the `Destination` proxy API has been updated to add a new `ProtocolHint`, `Opaque`, which indicates that an outbound proxy should _not_ send a `SessionProtocol` when initiating a direct connection, even if the outbound proxy handled the connection as HTTP. This hint was added to the proxy API in linkerd/linkerd2-proxy-api#197, and released in `linkerd2-proxy-api` v0.8.0. This branch updates the Destination controller's dependency on `linkerd2-proxy-api` to v0.8.0, and changes the controller to send an `Opaque` protocol hint when the target port is marked as opaque on the destination pod. This should override the `H2` protocol hint that is added when the destination is meshed. I've also added a new test for this behavior. Fixes #9888 (along with linkerd/linkerd2-proxy#2209, which changes the proxy to actually handle the `Opaque` protocol hint). Signed-off-by: Eric Anderson <eric@buoyant.io>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This indicates that the destination will handle the connection as an
opaque TCP byte stream, and that HTTP/2 upgrades should not be
performed. If the
opaque_transportflag is also set for thisdestination, then the proxy should not send a session protocol in its
transport header.
See linkerd/linkerd2#9888 for details.