router: Added alt_header_name to set an arbitrary header for populating SNI by agrawroh · Pull Request #17995 · envoyproxy/envoy

agrawroh · 2021-09-06T04:25:16Z

This PR adds a new optional param called override_auto_sni_header in UpstreamHttpProtocolOptions which can be used to populate the SNI value from an arbitrary header other than Host/Authority.

We have a use-case to use a different header for setting the SNI while sending requests to the upstream clusters and it's currently not possible.

Commit Message: added an optional override_auto_sni_header to set an arbitrary header for populating SNI.
Additional Description: Adds a new optional param called override_auto_sni_header which can be used to populate the SNI value from an arbitrary header other than Host/Authority.
Risk Level: Low
Testing: Unit Tests, Integration Tests
Docs Changes: Added description on override_auto_sni_header in the Docs.
Release Notes: Added
Platform Specific Features: N/A

Signed-off-by: Rohit Agrawal rohit.agrawal@databricks.com

repokitteh-read-only · 2021-09-06T04:25:19Z

Hi @agrawroh, welcome and thank you for your contribution.

We will try to review your Pull Request as quickly as possible.

In the meantime, please take a look at the contribution guidelines if you have not done so already.

🐱

Caused by: #17995 was opened by agrawroh.

see: more, trace.

repokitteh-read-only · 2021-09-06T04:25:24Z

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto).
CC @envoyproxy/api-shepherds: Your approval is needed for changes made to api/envoy/.
envoyproxy/api-shepherds assignee is @markdroth
CC @envoyproxy/api-watchers: FYI only for changes made to api/envoy/.

🐱

Caused by: #17995 was opened by agrawroh.

see: more, trace.

api/envoy/api/v2/core/protocol.proto

test/common/router/router_test.cc

Shikugawa

Thanks! LGTM with minor comments.

Shikugawa

LGTM! Take over @envoyproxy/envoy-maintainers

zuercher · 2021-09-07T16:39:03Z

/assign @envoyproxy/api-shepherds

repokitteh-read-only · 2021-09-07T16:39:05Z

@envoyproxy/api-shepherds cannot be assigned to this issue.

🐱

Caused by: a #17995 (comment) was created by @zuercher.

see: more, trace.

zuercher · 2021-09-07T16:39:29Z

/assign-from @envoyproxy/api-shepherds

repokitteh-read-only · 2021-09-07T16:39:33Z

@envoyproxy/api-shepherds assignee is @markdroth

🐱

Caused by: a #17995 (comment) was created by @zuercher.

see: more, trace.

markdroth · 2021-09-08T14:45:06Z

/lgtm api

Shikugawa · 2021-09-08T15:36:17Z

/assign-from @envoyproxy/envoy-maintainers

repokitteh-read-only · 2021-09-08T15:36:20Z

@envoyproxy/envoy-maintainers assignee is @ggreenway

🐱

Caused by: a #17995 (comment) was created by @Shikugawa.

see: more, trace.

source/common/router/router.cc

api/envoy/config/core/v3/protocol.proto

soulxu

LGTM

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>

agrawroh · 2021-09-10T16:43:47Z

@ggreenway Would you please take another look whenever you get some free time?

ggreenway

Looks good!

test/common/router/router_test.cc

ggreenway · 2021-09-13T15:40:20Z

/wait

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>

agrawroh · 2021-09-13T19:07:48Z

@ggreenway @markdroth @lizan There were merge conflicts in one file so I had to rebase. Could you please kick-off the CI workflow again and also leave an LGTM?

agrawroh · 2021-09-13T21:51:03Z

/retest

repokitteh-read-only · 2021-09-13T21:51:06Z

Retrying Azure Pipelines:
Check envoy-presubmit isn't fully completed, but will still attempt retrying.
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17995 (comment) was created by @agrawroh.

see: more, trace.

agrawroh · 2021-09-13T22:48:31Z

/retest

repokitteh-read-only · 2021-09-13T22:48:34Z

Retrying Azure Pipelines:
Retried failed jobs in: envoy-presubmit

🐱

Caused by: a #17995 (comment) was created by @agrawroh.

see: more, trace.

agrawroh requested review from alyssawilk, mattklein123 and snowp as code owners September 6, 2021 04:25

repokitteh-read-only bot added v2-freeze api labels Sep 6, 2021

repokitteh-read-only bot assigned markdroth Sep 6, 2021

agrawroh mentioned this pull request Sep 6, 2021

filter: exposed setSni() function to Lua for setting SNI value for upstream connections #17991

Closed

agrawroh force-pushed the alt_sni_header branch 4 times, most recently from b34e701 to efca849 Compare September 6, 2021 07:50

Shikugawa reviewed Sep 7, 2021

View reviewed changes

api/envoy/api/v2/core/protocol.proto Outdated Show resolved Hide resolved

Shikugawa reviewed Sep 7, 2021

View reviewed changes

api/envoy/api/v2/core/protocol.proto Outdated Show resolved Hide resolved

Shikugawa reviewed Sep 7, 2021

View reviewed changes

test/common/router/router_test.cc Outdated Show resolved Hide resolved

Shikugawa reviewed Sep 7, 2021

View reviewed changes

agrawroh force-pushed the alt_sni_header branch from efca849 to 07f099e Compare September 7, 2021 02:33

Shikugawa previously approved these changes Sep 7, 2021

View reviewed changes

repokitteh-read-only bot removed the api label Sep 8, 2021

repokitteh-read-only bot assigned ggreenway Sep 8, 2021

lizan suggested changes Sep 8, 2021

View reviewed changes

source/common/router/router.cc Outdated Show resolved Hide resolved

api/envoy/config/core/v3/protocol.proto Outdated Show resolved Hide resolved

agrawroh dismissed Shikugawa’s stale review via ec0932d September 8, 2021 21:12

soulxu previously approved these changes Sep 10, 2021

View reviewed changes

agrawroh dismissed soulxu’s stale review via 61c44b8 September 10, 2021 00:25

agrawroh requested review from Shikugawa and soulxu September 10, 2021 01:13

agrawroh force-pushed the alt_sni_header branch from 61c44b8 to fbf64cd Compare September 10, 2021 02:11

Remove Test (Hanging on IPv6)

1bc69d4

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>

agrawroh force-pushed the alt_sni_header branch from fbf64cd to 1bc69d4 Compare September 10, 2021 02:48

lizan previously approved these changes Sep 10, 2021

View reviewed changes

repokitteh-read-only bot removed the api label Sep 10, 2021

ggreenway requested changes Sep 13, 2021

View reviewed changes

test/common/router/router_test.cc Outdated Show resolved Hide resolved

repokitteh-read-only bot added the waiting label Sep 13, 2021

Fixed Typo

a0aeb85

Signed-off-by: Rohit Agrawal <rohit.agrawal@databricks.com>

agrawroh dismissed lizan’s stale review via a0aeb85 September 13, 2021 15:58

repokitteh-read-only bot added api and removed waiting labels Sep 13, 2021

ggreenway previously approved these changes Sep 13, 2021

View reviewed changes

agrawroh requested a review from lizan September 13, 2021 16:17

Merge branch 'main' of github.com:envoyproxy/envoy into alt_sni_header

76e14e1

agrawroh dismissed ggreenway’s stale review via 76e14e1 September 13, 2021 19:05

ggreenway approved these changes Sep 13, 2021

View reviewed changes

yanavlasov self-assigned this Sep 14, 2021

ggreenway merged commit 5c14669 into envoyproxy:main Sep 14, 2021

lizan mentioned this pull request Sep 17, 2021

header-to-metadata: add option to apply an upstream TLS context override #18112

Closed

Conversation

agrawroh commented Sep 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

repokitteh-read-only bot commented Sep 6, 2021

Uh oh!

repokitteh-read-only bot commented Sep 6, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Shikugawa left a comment

Choose a reason for hiding this comment

Uh oh!

Shikugawa left a comment

Choose a reason for hiding this comment

Uh oh!

zuercher commented Sep 7, 2021

Uh oh!

repokitteh-read-only bot commented Sep 7, 2021

Uh oh!

zuercher commented Sep 7, 2021

Uh oh!

repokitteh-read-only bot commented Sep 7, 2021

Uh oh!

markdroth commented Sep 8, 2021

Uh oh!

Shikugawa commented Sep 8, 2021

Uh oh!

repokitteh-read-only bot commented Sep 8, 2021

Uh oh!

Uh oh!

Uh oh!

soulxu left a comment

Choose a reason for hiding this comment

Uh oh!

agrawroh commented Sep 10, 2021

Uh oh!

ggreenway left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ggreenway commented Sep 13, 2021

Uh oh!

agrawroh commented Sep 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

agrawroh commented Sep 13, 2021

Uh oh!

repokitteh-read-only bot commented Sep 13, 2021

Uh oh!

agrawroh commented Sep 13, 2021

Uh oh!

repokitteh-read-only bot commented Sep 13, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

agrawroh commented Sep 6, 2021 •

edited

Loading

agrawroh commented Sep 13, 2021 •

edited

Loading