Conditional/Branching composite HTTP filters

[snowp]

Today filters can be configured in two ways: a per route configuration that is used when the incoming requests match a specific route entry and a default configuration that is used whenever no per route configuration is used.

One problem with this approach is that if you want to use a specific filter configuration for traffic that might be a sub match of many route entries, all of these route entries need to be duplicated to add the per route config. For example, you can imagine a large route table existing that primarily looks at :path, and wanting to add specific filter for GET requests. Today this would require injecting a route entry for each :path + :method == GET into the route table.

One way to make this easier would be introduce a filter (or a series of filters) that allows resolving the filter configuration + instantiation of a filter without relying on the route table. For example:

message CompositeFilter {
  message FilterMatch {
    config.common.matcher.v3.MatchPredicate match = 1;
    google.protobuf.Any typed_filter_config = 2;
  }

  repeated FilterMatch = 1
}

which follows the semantics that each FilterMatch is attempted. The first matching entry will indicate the filter that should be instantiated, while if nothing matches the filter does nothing.

Not only would this help simplify the route configuration, it would also make it possible for ECDS to configure different behavior based on the matching request without having to push this logic into each filter and without having to be accompanied by an RDS change to update the per route filter config.

[snowp]

@htuch @mattklein123 Thoughts on this from an API perspective? I think this would address some limitations of the current per filter config approach

[mattklein123]

@snowp in general I agree we should do this, and also include generic disable support. I think this is a dup though. I will try to find the duplicate(s) later. cc @kyessenov who I think has opened similar issues.

[kyessenov]

#11690 also wants different order of filters.

[snowp]

#11690 is similar but the key difference is that this issue proposed making this matching logic independent of the route selection. I think this approach is better for certain use cases (for more cross cutting concerns than route resolution) but there would be value in having a per route filter order as well.

Implementation wise this composite filter might be easier, since there are fewer edge cases around what happens when the resolved route changes.

[mattklein123]

Here is the generic disabling issue I was thinking about: #8669. I think it would be good to work that into this proposal.

[mattklein123]

(Basically I would make FilterMatch contain an embedded oneof that allows for: 1) inline config, 2) ecds config, 3) disable.

Perhaps (2) is not needed and could be added later.

[kyessenov]

I wonder if the filter match predicate and disablement is something that should be included inside ECDS, e.g. making the composite filter to be another filter. That's probably least disruptive. Route overrides with ECDS also need #12575 to be fixed.

[mattklein123]

Yeah I think there are multiple ways to go here. @snowp @kyessenov can you collaborate on a design to make sure we are covering as many cases as possible with the next set of changes?

[htuch]

Agree on the direction. Re: ECDS, I think this is a good idea or we need a real FCDS. What this proposal does is push a bunch of matching and routing logic into the filter chain, which today can only be loaded dynamically via LDS or (in the proposal) ECDS. LDS with delta updates might be some temporary relief here, but it's logically a bad fit, as request routing has nothing to do with listening.

[htuch]

Another structural approach that could be taken is to organize the filter chain as a tree, rather than as a list. In the proposal above, the branching applies only at a single level IIUC. We could have the branching also influence the rest of the tree.

[snowp]

If this was added as a regular filter, in theory they could be composed to create a tree structure, though we might want that to be first class rather than via a filter if that's the direction we want to go in.

The feature that I'm trying to unlock here is to have a management server be able to have a lot of flexibility to influence the request/response processing without being coupled with the management server that is providing routes/listeners/etc.

An example of this is connecting to an optional management server that might introduce faults/chaos/etc. into the stream. By having it only influence a single filter config, its uptime requirements are now much lower than that of the management server that provides more critical configuration updates.

[mattklein123]

IMO the use case makes sense to me. I don't feel very strongly about whether we build this into ECDS itself (it could have matching, disable, etc.) built in or it's a composite filter. I guess if you pushed me I would rather have this be built in.

Given that we all agree on the high level direction can @snowp @kyessenov collaborate offline on an API proposal?

[snowp]

Ok sounds good, I will get a doc started.

[snowp]

Here's a first draft I threw together: https://docs.google.com/document/d/1dOa_v1eMA4F61f0jdFkxtbcO-_TYD76oMfGoY7nzZhk/edit#

@kyessenov ptal

[htuch]

CC @penguingao re: selective filter enablement on internal redirects.

[wbpcode]

Use different filter configs for different requests in same route entry is a function that we've been hoping for for a long time.
We have a ugly implementation in our own Envoy branch. We just add a 'filters_config_switch_key' in HCM and compute a switch key by the config and incoming request.
Then we get per filter config from 'typed_per_filter_config' map by filter name and this switch key.
Our this scheme lacks design and has many problems. But it's simple and just work.

message FiltersConfigSwitchKey {
     KeyBuilder key_builder = 1;
}

typed_per_filter_config:
  <filter name1>:<runtime switch key1>: {}
  <filter name1>:<runtime switch key2>: {}
  <filter name2>:<runtime switch key1>: {}
  <filter name2>:<runtime switch key2>: {}

[snowp]

To update everyone on the current plan here: we're looking at splitting this proposal into a generic matching API proposal (#5569) and a dynamic filter creation proposal.

The dynamic creation or changing filter config doesn't have a dependency on the matching API, so it makes sense to split these up. This would also open up for arbitrary filters to do this without relying on the matching API.

[mattklein123]

@snowp per my comments in the doc, can we do a revamped proposal that covers the new plan (part HCM, part wrapper filter)? I think it will be easier to sort through.

The other major missing piece is the final form of the matcher config. I think the existing matcher we have is very powerful but not easily scalable (this has been mentioned elsewhere). This is a great opportunity to design a tree/trie with linear leaves matching system that can cover all the different use cases. I don't think this will be too terribly hard given what we know now and the code/config we already have. I'm happy to brainstorm more on this offline.

[markdroth]

I am concerned that this may be tying the xDS API more closely to Envoy's architecture. gRPC has started looking at how we can support HTTP filters in xDS, but it's difficult because the representation in the API does not match our interceptor architecture at all -- and in fact, one of the main mismatches is that we don't have an easy way for interceptors to affect the routing decision. I fear that the changes being discussed here may make that problem worse.

I'm just brainstorming here, but as an alternative, could we consider providing some sort of better abstraction here, like maybe separating out the filters that affect the routing decision from those that want to be run only in the case of a specific route? I'm thinking that the HCM config could include just those filters that affect the routing decision, and then the individual routes could contain a list of filters to be run only when that route is chosen. This would allow gRPC to support the filters in the route via something like our current interceptor API, and maybe we could provide some sort of new API for plugins that need to affect the routing decision. This approach might even eliminate the problem of having to override filter configs in routes to begin with; if all of the per-route filters are simply specified in the route itself, then there's no need to override a filter's config from multiple sources.

Another change that might help here would be to move the RouteAction out of Route so that the same RouteAction can be used in multiple Routes. This would be helpful not only for filter configuration but also for all of the other fields that are specified in RouteAction (e.g., retry policies), which today have to be repeated in multiple routes.

@ejona86 and @dfawley may want to weigh in here as well.

[snowp]

@markdroth Part of the original motivation behind this proposal was precisely to not have to rely on the route configuration, but instead to perform stream matching that is independent of the route resolution as a way to configure filters. The best example in my mind is wanting to apply some specific configuration to all POST requests without having to add a new POST entry for every single routing branch.

I would expect (based on my experience with grpc-java in the past) that gRPC interceptors would be able to keep an internal map of interceptors, picking which one to use based on the result of matching the incoming stream. I'm no expert on gRPC though, so I could be totally wrong. This would be conceptually similar to what I'm suggesting here, though in the Envoy case I was thinking of integrating it a bit more with the FilterManager (e.g. a filter can inject a filter after itself in the filter chain the stream hasn't passed the current filter).

[github-actions]

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or "no stalebot" or other activity occurs. Thank you for your contributions.