Skip to content

http: refactory Http::Utility::resolveMostSpecificPerFilterConfig method#16673

Merged
alyssawilk merged 10 commits intoenvoyproxy:mainfrom
soulxu:use_resolveMostSpecificPerFilterConfig
Jun 1, 2021
Merged

http: refactory Http::Utility::resolveMostSpecificPerFilterConfig method#16673
alyssawilk merged 10 commits intoenvoyproxy:mainfrom
soulxu:use_resolveMostSpecificPerFilterConfig

Conversation

@soulxu
Copy link
Copy Markdown
Member

@soulxu soulxu commented May 26, 2021

Commit Message: http: refactory Http::Utility::resolveMostSpecificPerFilterConfig method
Additional Description:
The Http::Utility::resolveMostSpecificPerFilterConfig and RouteEntry::mostSpecificPerFilterConfigTyped are doing
the same thing. RouteEntry::mostSpecificPerFilterConfigTyped is more efficient, and Http::Utility::resolveMostSpecificPerFilterConfig can take care some of sanity checks. So converge them. Refactory Http::Utility::resolveMostSpecificPerFilterConfig to use
RouteEntry::mostSpecificPerFilterConfigTyped. And using Http::Utility::resolveMostSpecificPerFilterConfig for the filters consistently.
Risk Level: low
Testing: unittest
Docs Changes: n/a
Release Notes: n/a
Fixes #14894

soulxu added 6 commits May 26, 2021 06:48
…cPerFilterConfigTyped

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Copy link
Copy Markdown
Contributor

@alyssawilk alyssawilk left a comment

Choose a reason for hiding this comment

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

Nice find! Want to ping back when CI is happy and I'll take a look?

@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented May 26, 2021

Nice find! Want to ping back when CI is happy and I'll take a look?

thanks for having a look at, let me make the CI happy first!

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
alyssawilk
alyssawilk previously approved these changes May 27, 2021
@alyssawilk
Copy link
Copy Markdown
Contributor

looks like this needs a main merge though!

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
Signed-off-by: He Jie Xu <hejie.xu@intel.com>
@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented May 28, 2021

/retest

@repokitteh-read-only
Copy link
Copy Markdown

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

🐱

Caused by: a #16673 (comment) was created by @soulxu.

see: more, trace.

Copy link
Copy Markdown
Member

@htuch htuch left a comment

Choose a reason for hiding this comment

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

Makes sense to me; this is less confusing. Will defer to @alyssawilk for full review. Thanks!

@soulxu
Copy link
Copy Markdown
Member Author

soulxu commented May 31, 2021

emm...coverage test failed. probably due to I removed few lines code.

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
@alyssawilk alyssawilk merged commit e7abafd into envoyproxy:main Jun 1, 2021
leyao-daily pushed a commit to leyao-daily/envoy that referenced this pull request Sep 30, 2021
…hod (envoyproxy#16673)

Commit Message: http: refactory Http::Utility::resolveMostSpecificPerFilterConfig method
Additional Description:
The Http::Utility::resolveMostSpecificPerFilterConfig and RouteEntry::mostSpecificPerFilterConfigTyped are doing
the same thing. RouteEntry::mostSpecificPerFilterConfigTyped is more efficient, and Http::Utility::resolveMostSpecificPerFilterConfig can take care some of sanity checks. So converge them. Refactory Http::Utility::resolveMostSpecificPerFilterConfig to use
RouteEntry::mostSpecificPerFilterConfigTyped. And using Http::Utility::resolveMostSpecificPerFilterConfig for the filters consistently.
Risk Level: low
Testing: unittest
Docs Changes: n/a
Release Notes: n/a
Fixes envoyproxy#14894

Signed-off-by: He Jie Xu <hejie.xu@intel.com>
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.

Inconsistency in per-route filter config resolution across filters

3 participants