api: Enable controlling the HTTP version used to connect to the backend #2433

liorokman · 2024-01-11T14:44:57Z

What this PR does / why we need it:
This PR proposes adding an API to control the HTTP version that will be used when connecting to a backend configured via an HTTPRoute.

kind: BackendTrafficPolicy
spec:
      useClientProtocol: true

The current behavior is that HTTP/2 is explicitly used if the route is a GRPC route, and it's possible to force HTTP/2-over-cleartext (h2c) by setting the appProtocol field in the backend Kubernetes Service to kubernetes.io/h2c. In all other cases EnvoyGateway generates configuration that forces HTTP/1.1 to the upstream.

The proposed API would allow configuring Envoy Proxy to use the same HTTP protocol version when connecting to the backend as was used by the downstream client.

codecov · 2024-01-11T14:49:34Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 66.82%. Comparing base (29946b0) to head (098c989).
Report is 84 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2433      +/-   ##
==========================================
+ Coverage   66.51%   66.82%   +0.31%     
==========================================
  Files         161      163       +2     
  Lines       22673    23274     +601     
==========================================
+ Hits        15080    15554     +474     
- Misses       6720     6812      +92     
- Partials      873      908      +35

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

arkodg · 2024-01-11T23:09:00Z

hey @liorokman can you raise a GH issue to define the use case ? would like to get input from the community to be understand if this is needed or not

liorokman · 2024-01-12T19:23:54Z

hey @liorokman can you raise a GH issue to define the use case ? would like to get input from the community to be understand if this is needed or not

Opened #2437

github-actions · 2024-02-11T20:02:00Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions!

zirain · 2024-02-12T12:28:58Z

/retest

github-actions · 2024-03-13T16:04:50Z

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. Please feel free to give a status update now, ping for review, when it's ready. Thank you for your contributions!

guydc · 2024-04-10T20:45:41Z

LGTM. Maybe we need to update the description with an up-to-date example of the API.

liorokman · 2024-04-11T10:28:01Z

Maybe we need to update the description with an up-to-date example of the API.

Done

arkodg · 2024-04-12T05:22:33Z

api/v1alpha1/backendtrafficpolicy_types.go

@@ -98,6 +103,15 @@ type BackendTrafficPolicySpec struct {
 	Compression []*Compression `json:"compression,omitempty"`
 }

+type HTTPOptions struct {
+	// UseClientProtocol configures Envoy to prefer sending requests to TLS enabled backends using


can you elaborate on why preference this given to TLS enabled backend ?

The goal is to use Envoy Proxy's use_downstream_protocol_config feature.

If the underlying transport socket supports TLS, it also supports ALPN. This makes it possible for Envoy to try to use both HTTP/1.1 and HTTP/2 as needed when connecting to it. If TLS is not enabled, then Envoy Proxy will either use HTTP/1.1 or HTTP/2. This means that if the downstream client is using the "wrong" protocol, the connection will simply fail.

It's easier to restrict this feature to TLS enabled upstreams where selection can be done based on ALPN than to try to force the downstream to have the right protocol. Fixing the client protocol selection is hard because:

If the Gateway has multiple HTTPRoutes, each one to a different upstream cluster with different supported protocols, then it's not possible to fix the Listener level ALPN array correctly. The route to be used is based on things that come later (e.g. the request path), so it's possible that one HTTPRoute has an upstream that supports only HTTP/2 and is matched on the /first path, while the other HTTPRoute attached to the same Gateway supports only HTTP/1.1 and is matched on the /second path. Envoy can't know which path is going to be used before the protocol is selected and the request actually sent to Envoy, and by that point it's too late to change the protocol.

If the downstream socket (the Gateway) is not TLS enabled, then there's no ALPN available to affect the client's selection anyways, and there would be a large chance of a misconfiguration that makes some HTTPRoutes unusable.

arkodg · 2024-04-12T05:29:57Z

api/v1alpha1/backendtrafficpolicy_types.go

@@ -98,6 +103,15 @@ type BackendTrafficPolicySpec struct {
 	Compression []*Compression `json:"compression,omitempty"`
 }

+type HTTPOptions struct {


there are already some HTTP only fields at the top level like FaultInjection, so vote to keep this at a top level
will bring this up in the next envoy gateway community meeting

OK. Will wait for the decision before changing this PR.

arkodg · 2024-04-12T05:51:43Z

api/v1alpha1/backendtrafficpolicy_types.go

@@ -98,6 +103,15 @@ type BackendTrafficPolicySpec struct {
 	Compression []*Compression `json:"compression,omitempty"`
 }

+type HTTPOptions struct {
+	// UseClientProtocol configures Envoy to prefer sending requests to TLS enabled backends using
+	// the same HTTP protocol that the incoming request used. Defaults to false, which means


prefer if we added a line in here saying something along the lines of - this is especially useful when the client (connecting to the gateway) and backend support multiple HTTP versions (e.g. http/1.1 and http/2), and users would like to force the backend to use the same version the client is using

OK. Effectively this means that the backend must be TLS enabled, or it can't support multiple HTTP versions.

can you elaborate on how TLS / ALPN is related here ?
as far as in understand, this knob is proactively deciding the upstream protocol version to use based on downstream connection, so it even applies to non TLS (like h2c to upstream, if downstream is h2c or h2)

A backend without TLS will either support HTTP/1.1 or HTTP/2, but never both. That's the point of h2c - it requires pre-knowledge on the part of the client that the server requires HTTP/2.

Since HTTP/2 is a binary protocol, if a client like Envoy Proxy starts a connection by sending a textual HTTP/1.1 request to an HTTP/2 over clear-text server, the server will disconnect it.
Since HTTP/1.1 is a textual protocol, if a client like Envoy Proxy starts a connection by sending an HTTP/2 cleartext request to an HTTP/1.1 server, the server will disconnect it.
Without TLS + ALPN, there is no way for a client like Envoy Proxy to negotiate with the server which protocol it can use, and this means that effectively upstream can't support multiple versions of HTTP without TLS+ALPN.

(like h2c to upstream, if downstream is h2c or h2)

We can configure the Envoy Proxy client to try to connect to upstream with the downstream protocol even without TLS on the upstream protocol. What it would do is automatically fail all requests where the downstream protocol doesn't match the single upstream protocol that is possible without upstream TLS.

Signed-off-by: Lior Okman <lior.okman@sap.com>

community meetings. Signed-off-by: Lior Okman <lior.okman@sap.com>

API. Signed-off-by: Lior Okman <lior.okman@sap.com>

upstream TLS backend. Signed-off-by: Lior Okman <lior.okman@sap.com>

liorokman · 2024-04-20T06:44:29Z

@guydc @arkodg PTAL

arkodg

LGTM thanks !

liorokman requested a review from a team as a code owner January 11, 2024 14:44

github-actions bot added the stale label Feb 11, 2024

github-actions bot removed the stale label Feb 12, 2024

github-actions bot added stale and removed stale labels Mar 13, 2024

liorokman force-pushed the btp-preserve-api branch from b0442cf to 6804cf9 Compare April 10, 2024 05:46

guydc previously approved these changes Apr 10, 2024

View reviewed changes

arkodg reviewed Apr 12, 2024

View reviewed changes

liorokman added 4 commits April 18, 2024 21:22

api: Enable controlling the HTTP version used to connect to the backend

f64efef

Signed-off-by: Lior Okman <lior.okman@sap.com>

Modified the proposed API as per the discussions in Slack and the

bf814e7

community meetings. Signed-off-by: Lior Okman <lior.okman@sap.com>

Removed an old CEL validation that was relevant for the older proposed

3f1c90c

API. Signed-off-by: Lior Okman <lior.okman@sap.com>

Remove the intermediate http struct and remove the requirement for an

2f20364

upstream TLS backend. Signed-off-by: Lior Okman <lior.okman@sap.com>

liorokman dismissed guydc’s stale review via 2f20364 April 18, 2024 18:24

liorokman force-pushed the btp-preserve-api branch from 02a6dd1 to 2f20364 Compare April 18, 2024 18:24

arkodg approved these changes Apr 23, 2024

View reviewed changes

arkodg requested a review from guydc April 23, 2024 02:02

arkodg added this to the v1.1.0-rc1 milestone Apr 23, 2024

Merge branch 'main' into btp-preserve-api

098c989

guydc approved these changes Apr 23, 2024

View reviewed changes

guydc merged commit aacf359 into envoyproxy:main Apr 23, 2024
22 checks passed

liorokman mentioned this pull request Apr 23, 2024

feat: Implement useClientProtocol #3254

Merged

liorokman mentioned this pull request May 6, 2024

Support configuring backend requests to use the same HTTP protocol as the client request #2437

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

api: Enable controlling the HTTP version used to connect to the backend #2433

api: Enable controlling the HTTP version used to connect to the backend #2433

liorokman commented Jan 11, 2024 •

edited

Loading

codecov bot commented Jan 11, 2024 •

edited

Loading

arkodg commented Jan 11, 2024

liorokman commented Jan 12, 2024

github-actions bot commented Feb 11, 2024

zirain commented Feb 12, 2024

github-actions bot commented Mar 13, 2024

guydc commented Apr 10, 2024

liorokman commented Apr 11, 2024

arkodg Apr 12, 2024

liorokman Apr 12, 2024

arkodg Apr 12, 2024

liorokman Apr 12, 2024

arkodg Apr 12, 2024

liorokman Apr 12, 2024

arkodg Apr 15, 2024 •

edited

Loading

liorokman Apr 15, 2024 •

edited

Loading

liorokman commented Apr 20, 2024

arkodg left a comment

api: Enable controlling the HTTP version used to connect to the backend #2433

api: Enable controlling the HTTP version used to connect to the backend #2433

Conversation

liorokman commented Jan 11, 2024 • edited Loading

codecov bot commented Jan 11, 2024 • edited Loading

Codecov Report

arkodg commented Jan 11, 2024

liorokman commented Jan 12, 2024

github-actions bot commented Feb 11, 2024

zirain commented Feb 12, 2024

github-actions bot commented Mar 13, 2024

guydc commented Apr 10, 2024

liorokman commented Apr 11, 2024

arkodg Apr 12, 2024

Choose a reason for hiding this comment

liorokman Apr 12, 2024

Choose a reason for hiding this comment

arkodg Apr 12, 2024

Choose a reason for hiding this comment

liorokman Apr 12, 2024

Choose a reason for hiding this comment

arkodg Apr 12, 2024

Choose a reason for hiding this comment

liorokman Apr 12, 2024

Choose a reason for hiding this comment

arkodg Apr 15, 2024 • edited Loading

Choose a reason for hiding this comment

liorokman Apr 15, 2024 • edited Loading

Choose a reason for hiding this comment

liorokman commented Apr 20, 2024

arkodg left a comment

Choose a reason for hiding this comment

liorokman commented Jan 11, 2024 •

edited

Loading

codecov bot commented Jan 11, 2024 •

edited

Loading

arkodg Apr 15, 2024 •

edited

Loading

liorokman Apr 15, 2024 •

edited

Loading