Skip to content

feat: Implement enabling HTTP/1.1 trailers#2492

Merged
zirain merged 2 commits intoenvoyproxy:mainfrom
liorokman:ctp-trailers-impl
Jan 24, 2024
Merged

feat: Implement enabling HTTP/1.1 trailers#2492
zirain merged 2 commits intoenvoyproxy:mainfrom
liorokman:ctp-trailers-impl

Conversation

@liorokman
Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

This PR implements #2487 .

I recommend that issue #2247 be updated so that if upstream TLS is enabled, then instead of forcing HTTP/1.1 if trailers are enabled by using HttpProtocolOptions_ExplicitHttpConfig in the cluster setup, ALPN should be allowed to choose the best upstream protocol by using AutoConfig on the cluster level and the trailers should be enabled in the correct place there.

Signed-off-by: Lior Okman <lior.okman@sap.com>
Signed-off-by: Lior Okman <lior.okman@sap.com>
@liorokman liorokman requested a review from a team as a code owner January 24, 2024 06:56
@codecov
Copy link
Copy Markdown

codecov bot commented Jan 24, 2024

Codecov Report

Attention: 20 lines in your changes are missing coverage. Please review.

Comparison is base (e4e2b68) 64.69% compared to head (f17df76) 64.63%.
Report is 2 commits behind head on main.

Files Patch % Lines
internal/ir/zz_generated.deepcopy.go 0.00% 18 Missing and 1 partial ⚠️
internal/xds/translator/listener.go 90.90% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2492      +/-   ##
==========================================
- Coverage   64.69%   64.63%   -0.06%     
==========================================
  Files         115      115              
  Lines       17472    17511      +39     
==========================================
+ Hits        11304    11319      +15     
- Misses       5447     5469      +22     
- Partials      721      723       +2     

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

Copy link
Copy Markdown
Member

@zirain zirain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to see a e2e test for new feature.

Copy link
Copy Markdown
Contributor

@arkodg arkodg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM thanks !

@arkodg arkodg requested review from a team and zirain January 24, 2024 18:21
@zirain zirain merged commit 8ce7914 into envoyproxy:main Jan 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants