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

I like it, CI are all green.

From implementation perspective, should probably validate that modifying the filter-chain contents (essentially ensuring that all the referenced filters are still alive mid-request and don't go away if the list changes) isn't problematic.

The used filters list will be cached by the stream and never change when initializing the filter chain. And after that, we don't care the chain update for single request and every filter instance will cache related filter configuration.

I'm not sure if filter-chain and named-filters should be coupled together, seems to me that chaining is one feature and named-filters is another.

map<string, FilterChain> filter_chains = 2; // The value is a chain here

Hmm, i think there may be an information gap. we actual have no named-filters support here but a named-chains support? In the map, the key is unique key, the value must be a chain? It's designed to avoid we configure same chain at every route again and again. We treat the chain as the minimum unit and it's similar to named lua scripts, named route cluster specifier and so on.

BTW, we don't support to configure a filter_chain filter in the filter_chain filter's configuration recursively.

The composite filter can now support configuring a whole chain of filters (see #40885 and #42724). Given that, why do we need this filter? It seems like a second API to do the same thing we already have.

The composite filter can now support configuring a whole chain of filters (see #40885 and #42724). Given that, why do we need this filter? It seems like a second API to do the same thing we already have.

Composite‘s filter chain cannot works well in Envoy actually. Because composite works as a wrapper. From main filter chain's perspective, it's only one single filter. Because this reason, if any filters return a non-continue status, there would be a problem. That's say the composite filter chain could only be used for cases where all sub filters only return continue.

It will bring huge complexity to refactor the composite or filter chain to make composite filter chain works well in Envoy. And because inherent design of composite filter implementation, it's no way to let composite works like this new filter.

This new L7 filter (this PR) is much simple and easy to maintain, designed for per route filter chain natively. although they have similar API, but they actually have completely different underlying mechanism.
But in another way, this filter is less flexibility rather than composite filter, they are designed to meet different scenarios. (Composite is used to execute conditional logic based on generic matcher which even could match to previous's filters' modification, and this new filter only focus on per route filter chain. For scenarios where only simple per route filter chain is necessary, this new filter would be more efficient and robust. For scenarios where require more flexible control and conditional branch, composite is more powerful.)

Composite‘s filter chain cannot works well in Envoy actually. Because composite works as a wrapper. From main filter chain's perspective, it's only one single filter. Because this reason, if any filters return a non-continue status, there would be a problem. That's say the composite filter chain could only be used for cases where all sub filters only return continue.

I'm not super familiar with Envoy's filter API itself. What does "continue" mean in this context? Can you give me an example of a situation in which a filter might return a non-continue status? I'm trying to understand what limitations of the composite filter we're trying to overcome here in terms of practical examples.

It will bring huge complexity to refactor the composite or filter chain to make composite filter chain works well in Envoy. And because inherent design of composite filter implementation, it's no way to let composite works like this new filter.

This argument seems to be about Envoy's implementation, not about xDS as a multi-data plane API. From an API perspective, it seems extremely undesirable to have two different APIs to do essentially the same thing. I would strongly prefer to find a way to allow the existing API to do what you need here. For example, perhaps we can modify the composite filter implementation in Envoy such that if the matcher has only an on_no_match -- meaning that there is only one possible filter chain to use -- the implementation can provide exactly the behavior you are implementing here.

@markdroth @wbpcode Thanks for the thoughtful discussion on this one. I think the verbosity problem with Composite filters is very real. With the current matcher API, there are 3-4 levels of typed_config nesting and multiple @type declarations just to express "run this filter chain." The cognitive overhead is substantial.

As @markdroth you noted, the ExtensionWithMatcher wrapper is a case where Envoy has leaked its implementation details into the API in a rather bizarre way. Good API should follows the design principle where complexity scales with the use case. Right now, the composite filter is optimized for complex scenarios like conditional matcher trees with multiple branches, while making the common case like static per-route filter chains, unnecessarily verbose.

I think that having both APIs isn't duplication but layering which provides a simple API for the 80% of the use cases, and a powerful API for rest 20% who need conditional logic.

As @wbpcode mentioned, Envoy Gateway has been using similar patterns (the disabled flag at route level) without user complaints. I do see a lot of value in having simpler per-route filter chains and think that putting this in contrib would be counterproductive. Projects like Envoy Gateway which don't use contrib images wouldn't be able to leverage it and we'd force them to work around this complexity in other ways.

I also think that the inline matcher simplification that @wbpcode proposed here is in the right direction. It'll allow Composite Filter to directly include the matcher field and would help eliminate the ExtensionWithMatcher wrapper. If we combine it with this new filter, users would have a clear choice based on their needs.

I am still unhappy with having two different filters that do the same thing.

How about a compromise: We simplify the composite filter as @wbpcode is doing in #43227, and we also add a top-level filter chain field to the config, which can be set instead of the matcher. That way, we're still providing two APIs, but at least they're both in the same filter, so it will be clear to users that that's the right filter to use to solve that problem.

Thanks so much @markdroth.

With that way, then at least we can configure per route filter chain in easier way, that will make big sense for Envoy based gateway products 🙇 🎉 . Some points I am thinking about your proposed way (open for discussion):

1. As @agrawroh said, they are actually layered APIs (and essentially have different target scenarios), will it more friendly to users to put these API in different folders? they can check on demand and needn't to understand whole composite at start.
2. Because they are designed for different scenarios, that's we basically will only use one of them in a specific scenario. will folding them brings chaos to non-envoy data plane? For example, for gRPC, you actually only want current composite. But if we compress this new filter API into composite, the gRPC may also need to care the new API of this PR. Seems unnecessary burden to gRPC because if the API is an another filter, gRPC actually can ignore it directly, just like gRPC have ignored lots of L7 filters of Envoy? And in reverse, for non-envoy data plane who only want this new API/extension, they also needn't to care composite?
3. In other cases, for filters have similar (but different) features, we will give an meaningful name to show what it does, like local_ratelimit, ratelimit, ratelimit_quota, cache, cache_v2 and so on. If we do think these two filters are similar thing, using different but correlated name should be a mature way to go? Although I prefer current filter_chain also, because it literally show what the filter is doing: bridging a filter chain. This name is much easier to understand for users who know nothing about composite. But I am open to this. And new better name is welcome.

Anyway, my core point is always that we should be open to new extensions if they have explicit target scenarios and maintainers are willing to maintain. As @zirain said, we already have different filters could so similar things, and if we check Kong Gateway plugin hub (popular gateway product based on Nginx and OpenResty), there are more similar but different plugins in their community. A rich library of extensions and extensibility are the lifeblood of gateway/proxy. As an engineer need to support consumers from various different companies, I only hear complain about why Envoy doesn't support this or that, but I never hear a complain about why there are three different rate limits, or why there are two different cache filters. So, we may needn't to worry too much for adding new extension, if the extension is useful and maintainable.

Also cc @agrawroh for thoughts from perspective of both user and maintainer.

As Envoy Gateway maintainer, I would like to see a simpler API to enchane the per route level filter.

xref: compisite filter

The composite filter allows delegating filter actions to a filter specified by a match result. The purpose of this is to allow different filters or filter configurations to be selected based on the incoming request, allowing for more dynamic configuration that could become prohibitive when making use of per route configurations (e.g. because the cardinality would cause a route table explosion).

If composite could provide more ability based on the existing filters, that would be great. It should not being a blocker for introuding a new filter which make life easier.

@markdroth Thanks for being open to finding a middle ground here.

I do want to echo some of @wbpcode's points though. One thing that comes to mind is the xDS multi-data-plane story. If we fold this into composite, data planes like gRPC that only care about conditional matching would still need to understand or explicitly ignore the filter-chain-only mode. Keeping them separate lets each data plane adopt what's relevant. gRPC can just ignore filter_chain entirely, the same way it ignores many other L7 filters today.

There's also a discoverability aspect. Users looking for "simple per-route filter chain" might not intuitively reach for something called "composite" as the name itself implies certain complexity. A separate filter with a clear name does signal the intent immediately. We already do this with local_ratelimit vs ratelimit vs ratelimit_quota. They are all related but act as distinct tools for distinct scenarios.

That said, I do understand the concern about API surface area. If the consensus lands on keeping them in one filter, I think that's workable as long as we have clear documentation guiding users to the right pattern for their use case.

Not to mention that internally 8/10 people who I spoke to find Composite and the Matchers API in general too complicated to use.

Not to mention that internally 8/10 people who I spoke to find Composite and the Matchers API in general too complicated to use.

Completely same experience when I worked at NetEase 🤣 . I used ~40 min to explain everything to PM and gateway teams, then they said, we got it, then they rejected my proposal and ask me to provide a custom patch to data plane to support their requirement. And when I worked at ByteDance, they never never take the composite into their consideration, they always want keep it simple, keep it works.

I think these work background and feedbacks from others make me always think the composite and generic matcher is expert-only tool for tricky scenarios and not a handy solution for non-expert users. And in most cases, it's these non-expert engineers to distribute and support Envoy in various different companies. Also this background makes me want to separate them independent extensions for different scenarios to avoid additional mental burden for non-expert engineers. And, TBH, I didn't get an obvious side effect of keeping independent extensions. 😂
(Mark may being worry the possible explosion of the API surface/extension list with time pasted. This should be resolved by a reasonable deprecation mechanism. For example, we can mark the API out-of-maintain/out-of-time as deprecated, and remove it after 6-8 release cycles. And as @zirain said, it's hard to be a reason to block new extension.)

adding some more history from Envoy Gateway

• a common use case for north south is applying intent (defined as policies in Envoy Gateway, examples are BackendTrafficPolicy and SecurityPolicy) per route, which maps to many product use cases of applying different intent to different paths for the same hostname
• very early on we found out that PerRoute support didnt exist for a lot of filters which blocked many features, until @wbpcode introduced http: route level filter disabled support #27210
• this unblocked the project and we were able to land many features like OAuth and WASM which dont have PerRoute types
• the implementation is non trivial, see - listener config and route config
• even after pointing users to the above features, you can see many users finding a hard time to implement this
• tbh even I dread implementing features that need this incantation and rely on @zhaohuabing to get it right
• not only is it hard to type it right, there are many other config instantiations that may need to be set correctly to make sure drains are avoided
• I see this new API, as a second attempt to improve this feature which will
  • improve readability
  • improve writability
  • better performance (e.g. eliminate drains)

+1 on a simpler per-route API. Even though Envoy is mostly configured via the control plane, I still need to look at the Envoy configuration from time to time when troubleshooting. A more intuitive per-route API would really help 🙂

thanks for fixing xds/api include - hopefully this will remove my request change

Take it easy. This is still waiting a LGTM from api shepherd

Gently ping for another check or LGTM cc @adisuissa @markdroth

I think Envoy maintainers and Envoy gateway maintainers here are all good to continue with this new extension? Especially this not change any core API (route/listener/cluster/HCM/TCP), we may shouldn't stop here?

Sorry, I still don't understand why it's better to have a second filter for this, rather than just add a new field to the composite filter's config to set the filter chain without a matcher. I think that from an API perspective, the important thing that users are going to be thinking about is "I need to configure a separate filter chain", not how the filter chain is specified. I think it's going to cause confusion if we need to explain to everyone how to decide which of the two filters to use. I think the story is much easier to understand if we say that the composite filter is the one place to do this, and it has a few knobs for how the nested filter chain can be selected.

Can someone please tell me what use-case wouldn't be handled by that approach? Because as far as I can see, it would provide exactly the same functionality as would be provided here.

BTW, there is no multi-data-plane issue here: gRPC is free to support or not support the new non-matcher filter chain field, and that's true regardless of whether it's a new field in the composite filter config or a new filter.

Sorry, I still don't understand why it's better to have a second filter for this, rather than just add a new field to the composite filter's config to set the filter chain without a matcher. I think that from an API perspective, the important thing that users are going to be thinking about is "I need to configure a separate filter chain", not how the filter chain is specified. I think it's going to cause confusion if we need to explain to everyone how to decide which of the two filters to use. I think the story is much easier to understand if we say that the composite filter is the one place to do this, and it has a few knobs for how the nested filter chain can be selected.

Can someone please tell me what use-case wouldn't be handled by that approach? Because as far as I can see, it would provide exactly the same functionality as would be provided here.

Thanks for the response, Mark.

All things are about to make API more friendly (which is I strongly think is very very important from practices and user feedbacks). Our understanding is:

1. they are layered APIs and will cover different scenarios and even different group of developers. For any scenario where the new extension could cover, they needn't care composite. For any scenario require the composite (this expert tool), they should be easily to handle both filters. For a non expert users/developers, they only need to understand the new extension: oh, this is list, can configure multiple filters. That's all. They needn't to understand what is matcher tree and how it works until one day they find the simple extension cannot meet their requirements. So I don't want to hybrid them.
2. As Rohit said, naming self has complexity. For developers/users who will be confused by these two filters, the best design is give them the new simple one only, the composite will not enter their sight and list. For experts, they can handle by themselves.

The new extension should make most people happy (Maintainers, EG maintainers, most non-expert users).
So, from API's perspective, what is the block point?

BTW, there is no multi-data-plane issue here: gRPC is free to support or not support the new non-matcher filter chain field, and that's true regardless of whether it's a new field in the composite filter config or a new filter.

I think Rohit meant, combining APIs will result in that gRPC or similar clients will need to ignore specific fields in a supported extension rather than to ignore the whole extension. This may brings additional cognitive burden. Completely ignoring an extension should be much better to support an extension partially.
I have similar thoughts.

This is not a very core point anyway 🤣.

I hear what you're saying, but I just don't agree. I think that having a second filter to do the same thing is going to cause a lot more confusion than simply adding a new field to the composite filter.

I don't think the composite filter has to be only an "expert" filter. Once we have the new API in #43227, if we add a config field that allows the composite filter to set a filter chain without a matcher, then from the API perspective, it's exactly the same as what you're proposing here. Users who don't need to select the filter chain conditionally can just set the hard-coded filter chain field without a matcher, and they still don't need to know or care about the matcher.

I agree that "composite" may not be the best name for this filter, but I don't think that's a good justification for adding a second filter to do the same thing.

Stepping back a bit, I think we've gotten to the point in this discussion where we're both just repeating what we've already said and not actually offering any new arguments, and I don't think that's a productive use of either of our time. I have heard your argument, and I believe that you have heard mine, and I don't think we're going to agree. So we need to figure out how to move forward.

I still don't really think any of this is necessary, since essentially the same thing can be achieved via the composite filter with a matcher that matches everything. But I've offered a compromise of adding a new field to the composite filter to set the filter chain without a matcher, which I think addresses the main concern here. Like any compromise, it's not perfect, but I think it's something we can all live with. Can we agree to disagree and move forward with that compromise?

I think Rohit meant, combining APIs will result in that gRPC or similar clients will need to ignore specific fields in a supported extension rather than to ignore the whole extension. This may brings additional cognitive burden. Completely ignoring an extension should be much better to support an extension partially.

FWIW, there are already lots of cases where gRPC doesn't support every field in a given extension, and there are even a few cases where Envoy doesn't support a field that gRPC has added. So this really isn't anything new.

This pull request has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in 7 days if no further activity occurs. Please feel free to give a status update now, ping for review, or re-open when it's ready. Thank you for your contributions!