set_filter_state: enable per-route configuration override #37507
Conversation
|
@kyessenov will be OOO for a while, fine, let me review this. |
|
I think some tests for this new feature is necessary. /wait |
|
TBH I have no idea how to write tests, this simple change already took me nearly 10hrs to understanding |
😂 but it's required or we cannot merge this. Is there anyone is familiar to envoy in your colleagues? |
| config_.get()->updateFilterState({&headers}, decoder_callbacks_->streamInfo()); | ||
|
|
||
| // If configured, apply route level configuration next. | ||
| auto per_route_config = | ||
| Http::Utility::resolveMostSpecificPerFilterConfig<Filters::Common::SetFilterState::Config>( | ||
| decoder_callbacks_); | ||
| if (per_route_config) { | ||
| per_route_config->updateFilterState({&headers}, decoder_callbacks_->streamInfo()); | ||
| } |
There was a problem hiding this comment.
Is there a reason you run both, typically a route level config will override the listener level
There was a problem hiding this comment.
I was thinking it would be nice to have common listener level ones and then have additional route specific ones. You can override by applying both (route is last so it wins).
We do have this use case - but it could also be done by just copying the listener ones into every route of course.
There was a problem hiding this comment.
I would say in that case you should have explicit options most likely, as in override, merge, etc. Also acceptable at first of course to just say there's only one behavior, but this would be different from most filters today afaik
howardjohn
force-pushed
the
sfs/support-per-route
branch
2 times, most recently
from
beb9ae9 to
7d55bb1
Compare
This extends this filter to support usage in route configuration. When present, both the listener and route level settings are applied (listener first, then route). Signed-off-by: John Howard <[email protected]>
howardjohn
force-pushed
the
sfs/support-per-route
branch
from
7d55bb1 to
5c8a97f
Compare
Signed-off-by: John Howard <[email protected]>
howardjohn
force-pushed
the
sfs/support-per-route
branch
from
5c8a97f to
46a6a0c
Compare
Commit Message: set_filter_state: enable per-route configuration override
Additional Description:
This extends this filter to support usage in route configuration. When present, both the listener and route level settings are applied (listener first, then route).
Risk Level: Low
Testing:
Docs Changes:
Release Notes:
Platform Specific Features:
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Fixes commit #PR or SHA]
[Optional Deprecated:]
[Optional API Considerations:]