Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: HeadersWithUnderscoreActions on ClientTrafficPolicy #3052

Conversation

davidalger
Copy link
Contributor

What type of PR is this?

Adds configuration to ClientTrafficPolicy to support overriding the default for HeadersWithUnderscoreActions in common HTTP protocol options.

What this PR does / why we need it:

Envoy defaults have headers_with_underscores_action set to ALLOW while Envoy Gateway defaults to REJECT_REQUEST despite the underscore character being allowed in header names by RFC-7230. This may be an ok default for EG to be more strict out of the box, but it breaks compatibility with preexisting API client configurations which send underscores in header names.

@davidalger davidalger requested a review from a team as a code owner March 28, 2024 18:57
Copy link

codecov bot commented Mar 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.90%. Comparing base (ceb697f) to head (219fb2e).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3052      +/-   ##
==========================================
+ Coverage   64.89%   64.90%   +0.01%     
==========================================
  Files         121      121              
  Lines       21382    21393      +11     
==========================================
+ Hits        13875    13886      +11     
  Misses       6639     6639              
  Partials      868      868              

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

@davidalger davidalger force-pushed the algerdev/feat-headers-with-underscores-action branch 2 times, most recently from 0c6b66e to 9b95cab Compare March 28, 2024 19:14
@davidalger davidalger force-pushed the algerdev/feat-headers-with-underscores-action branch from 9b95cab to bab490a Compare March 28, 2024 19:22
Signed-off-by: David Alger <[email protected]>
Xunzhuo
Xunzhuo previously approved these changes Mar 29, 2024
arkodg
arkodg previously approved these changes Mar 29, 2024
Copy link
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
Copy link
Contributor

arkodg commented Mar 29, 2024

looks like a rebase is needed @davidalger

# Conflicts:
#	site/content/en/latest/api/extension_types.md
@davidalger davidalger dismissed stale reviews from arkodg and Xunzhuo via 219fb2e March 29, 2024 13:33
@davidalger davidalger requested review from Xunzhuo and arkodg March 29, 2024 13:38
@davidalger
Copy link
Contributor Author

/retest

@arkodg arkodg merged commit 519eb70 into envoyproxy:main Mar 29, 2024
22 checks passed
@davidalger davidalger deleted the algerdev/feat-headers-with-underscores-action branch March 29, 2024 14:13
ShyunnY pushed a commit to ShyunnY/gateway that referenced this pull request Apr 1, 2024
…#3052)

* feat: HeadersWithUnderscoreActions on ClientTrafficPolicy

Signed-off-by: David Alger <[email protected]>

* Add XDS translator test

Signed-off-by: David Alger <[email protected]>

* Rename using preferred func prefix

Signed-off-by: David Alger <[email protected]>

---------

Signed-off-by: David Alger <[email protected]>
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