Skip to content

http: route level filter disabled support#27210

Merged
wbpcode merged 15 commits intoenvoyproxy:mainfrom
wbpcode:dev-route-filter-config-support
Jun 12, 2023
Merged

http: route level filter disabled support#27210
wbpcode merged 15 commits intoenvoyproxy:mainfrom
wbpcode:dev-route-filter-config-support

Conversation

@wbpcode
Copy link
Copy Markdown
Member

@wbpcode wbpcode commented May 5, 2023

Commit Message: http: route level filter config support
Additional Description:

To close #15025, and implement of new API in the #25927. Partly related to #20867.

Specifically, is PR implement two features:
1. first, make the route/vh level per filter config could be optional. The old implementation has some problem and may need to be reverted.
2. second, a common way to disable a filter by the route/vh level per filter config.

Risk Level: mid.
Testing: wait.
Docs Changes: wait.
Release Notes: wait.
Platform Specific Features: wait.

@repokitteh-read-only
Copy link
Copy Markdown

CC @envoyproxy/api-shepherds: Your approval is needed for changes made to (api/envoy/|docs/root/api-docs/).
envoyproxy/api-shepherds assignee is @mattklein123
CC @envoyproxy/api-watchers: FYI only for changes made to (api/envoy/|docs/root/api-docs/).

🐱

Caused by: #27210 was synchronize by wbpcode.

see: more, trace.

@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented May 5, 2023

/wait

@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented May 6, 2023

/wait

This implementation will add additional hash searching for every filter instance creation. If there are lots filters need to be skipped, then this overhead should acceptable. But for the one who use lots of filters but needn't route level filter disable function, maybe this overhead is a little heavy.

Signed-off-by: wbpcode <[email protected]>
@wbpcode wbpcode force-pushed the dev-route-filter-config-support branch from 14f2e02 to 48a6e05 Compare May 7, 2023 07:43
@wbpcode wbpcode changed the title http: route level filter chain support http: route level filter config support May 7, 2023
Signed-off-by: wbpcode <[email protected]>
@wbpcode wbpcode marked this pull request as ready for review May 7, 2023 11:13
Signed-off-by: wbpcode <[email protected]>
Copy link
Copy Markdown
Contributor

@markdroth markdroth left a comment

Choose a reason for hiding this comment

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

Thanks for fixing this!

@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented May 18, 2023

/wait #27263

@kyessenov
Copy link
Copy Markdown
Contributor

Looking forward to seeing this land. I think this is super-useful as a generic per-route filter opt-in/opt-out, which is needed by k8s Gateway https://gateway-api.sigs.k8s.io/api-types/httproute/#filters-optional.

@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented May 26, 2023

Waiting for #27263 to be land first. But i think we almost get there. 🙂

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>
@wbpcode wbpcode requested a review from htuch as a code owner June 8, 2023 09:00
@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented Jun 8, 2023

cc @mattklein123
This is a common feature and I choose the header_mutation as an example. A new integration test is added to the header_mutation filter to ensure this new feature could work as expected. 😄

mattklein123
mattklein123 previously approved these changes Jun 8, 2023
Copy link
Copy Markdown
Member

@mattklein123 mattklein123 left a comment

Choose a reason for hiding this comment

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

Thanks, needs a main merge though.

@wbpcode
Copy link
Copy Markdown
Member Author

wbpcode commented Jun 8, 2023

cc @mattklein123 done, could you give a new stamp? thanks.

@wbpcode wbpcode enabled auto-merge (squash) June 8, 2023 22:09
@wbpcode wbpcode merged commit eec764f into envoyproxy:main Jun 12, 2023
@wbpcode wbpcode deleted the dev-route-filter-config-support branch June 12, 2023 15:07
asheryerm pushed a commit to asheryerm/envoy that referenced this pull request Jul 5, 2023
* refactor and fix tests

Signed-off-by: wbpcode <[email protected]>

* fix test

Signed-off-by: wbpcode <[email protected]>

* more detailed comments

Signed-off-by: wbpcode <[email protected]>

* resolve conflict

Signed-off-by: wbpcode <[email protected]>

* fix test

Signed-off-by: wbpcode <[email protected]>

* change log and docs

Signed-off-by: wbpcode <[email protected]>

* Update changelogs/current.yaml

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* Update changelogs/current.yaml

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* Update changelogs/current.yaml

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* Update envoy/http/filter_factory.h

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* Update source/common/http/filter_chain_helper.cc

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* add new integration test

Signed-off-by: wbpcode <[email protected]>

---------

Signed-off-by: wbpcode <[email protected]>
Signed-off-by: code <[email protected]>
Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: asheryer <[email protected]>
reskin89 pushed a commit to reskin89/envoy that referenced this pull request Jul 11, 2023
* refactor and fix tests

Signed-off-by: wbpcode <[email protected]>

* fix test

Signed-off-by: wbpcode <[email protected]>

* more detailed comments

Signed-off-by: wbpcode <[email protected]>

* resolve conflict

Signed-off-by: wbpcode <[email protected]>

* fix test

Signed-off-by: wbpcode <[email protected]>

* change log and docs

Signed-off-by: wbpcode <[email protected]>

* Update changelogs/current.yaml

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* Update changelogs/current.yaml

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* Update changelogs/current.yaml

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* Update envoy/http/filter_factory.h

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* Update source/common/http/filter_chain_helper.cc

Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: code <[email protected]>

* add new integration test

Signed-off-by: wbpcode <[email protected]>

---------

Signed-off-by: wbpcode <[email protected]>
Signed-off-by: code <[email protected]>
Co-authored-by: Matt Klein <[email protected]>
Signed-off-by: Ryan Eskin <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

allow HTTP filters to be marked as optional

5 participants