http: add support for dynamically injecting filters into the chain

[snowp]

Adds support for a set of new filter callbacks that allows a filter to
inject a filter into the filter chain immediately following itself. This
allows a filter to inspect the incoming headers and dynamically adjust
what filters are being executed.

Filters can only be injected before headers have made it to the next
filter; this ensures that the injected filters will be treated exactly
as any other filter.

See https://github.com/envoyproxy/envoy/compare/master...snowp:composite-filter?expand=1 for a working example of a filter making use of this API.

Part of #12968

Signed-off-by: Snow Pettersen snowp@lyft.com

Risk Level: Medium, new unused feature but changes to FM
Testing: UTs
Docs Changes: n/a
Release Notes: n/a
Platform Specific Features: n/a
[Optional Runtime guard:]
[Optional Fixes #Issue]
[Optional Deprecated:]

[htuch]

@snowp can you explain a bit more on how the the lifecycle of filters and filter factories works in this setup? Would be helpful to have some documentation on this beyond just the patch linked.

[snowp]

Sure, let me add some comments to the callbacks that talks about expected usage

[snowp]

So to elaborate on the use case here, the goal behind this change is to allowing a remote server a lot of flexibility to influence the filter iteration (see https://docs.google.com/document/d/1dOa_v1eMA4F61f0jdFkxtbcO-_TYD76oMfGoY7nzZhk/edit#heading=h.5wabejoc1qk5 for the original doc).

The immediate use case for us is being able to express

match on header X:
if value == y => use filter config A
if value == z => use filter config B

as a way to facilitate fault injections based on a header identifying which experiment the current request belongs to.

Some possible solutions to this:

• extend the fault filter to support being passed fault configuration as part of a match action
• provide a filter that can create an inner filter based on the provided config, delegating HCM callbacks to the inner filter
• provide a hook into the FM that allows to inject filters on demand, using a filter to create the delegated filters but hand over management to the FM (which this PR is a part of)

Solution 1 works but ends up being very specific to the fault filter, which means that this flexibility is not afforded in a generic way.

Solution 2 & 3 are conceptually similar, the main difference being whether we have to manage the delegated filters ourselves and whether we can support injecting multiple filters (this becomes very tricky to do in a delegated manner).

They have the same implications when it comes to making the list of filters dynamic, though solution 3 ensures that the FM is aware of the final filter ordering. The addition of a filter callback to add a dynamic filter does increase the complexity of the filter API, but does open up for a lot of possibilities (e.g. #11690)

Let me know if you have any thoughts on whether this is the right approach here, totally open for suggestions! @htuch @alyssawilk

[htuch]

@snowp when I read the original doc, I had in mind the flow in https://docs.google.com/document/d/1dOa_v1eMA4F61f0jdFkxtbcO-_TYD76oMfGoY7nzZhk/edit#. I.e. we are just selectively enabling/disabling filters. That's super easy to understand but also limited for your use case.

In the flows above, who actually is doing the filter chain injecting? Is the fault filter injecting a new filter for config A or B? Or is some other special matcher filter making the choice and injecting different fault filters? This model I think gives a lot of rope; we can easily get into a place where the actual filter execution is very hard to predict. I don't know if that's actionable but just an observation.

[snowp]

@htuch My idea would be for a special filter to handle the injection of the filter: it would snap the filter factories for each possible configuration during its own creation (which means filter factories are created at the same time as regular filters), and then the match result would indicate which factory to use to create the filter to inject. This means that filters like the fault filter doesn't have to know anything about this, making it possible to mix and match filters if desired.

One of the problems with this approach (as you point out) is that the filter API surface is pretty huge: any enforcement of which filters to inject, when to inject them, etc. is left up to the filter doing the injection. As a result the possible usages of the API might be hard to predict.

So while I like the flexibility this gives us, I'd like to make sure that we'd be okay with supporting this API in the future (@alyssawilk might have thoughts here? or @mattklein123). I can imagine more elaborate APIs that would lock things down a bit (e.g. using the idea of declaring dynamic slots in the filter config that can be populated during filter chain iteration with a filter from an allowlist), but the alternatives comes with their own set of complexities.

The most conservative approach is one of the options I mentioned where we don't mess with the filter chain at all and just teach the fault filter (and any other filter that we want this kind of dynamic behavior on match results) to understand match filter-specific match actions.

[htuch]

I think what you have makes sense when it's used by these special filters implementing matchers. Definitely interested in hearing @alyssawilk and @mattklein123 take on this though.

Part of me wishes this was only available in allowlisted scenarios where we actually need this flexibility, but if we're adding it to the public interface, I'm not sure this is something that makes sense. I have a non-specific unease with self-modifying filter chains, but nothing I can pin down to say we shouldn't do this.

[mattklein123]

@snowp asked me to take a look at this and I should have time to look through tomorrow.

[mattklein123]

Paging all of this back in, I had thought that we were effectively going to go with your solution (1) here: #14722 (comment).

I agree it's not optimal in the sense that the each filter has to implement it's own logic, at the same time, it feels like a fairly natural pattern that we could embed filter specific config and have it be passed as part of the match action, much like how we allow route specific config today. We could probably even type check this in exactly the same way to make sure the config lines up with the filter it's being passed to and is already pre-loaded and verified.

IMO this is simpler to understand and still quite flexible. We could do something like this PR later but my preference would be to avoid it until if/when we really feel that we need it?

I'm happy to be swayed but that's my 2c.

[snowp]

IMO this is simpler to understand and still quite flexible. We could do something like this PR later but my preference would be to avoid it until if/when we really feel that we need it?

Yeah given the push back I'm inclined to go that direction. I'll close out this PR and then get something working using the delegated approach, and then we can revisit this if we need this flexibility down the line.