filters: allow filter installation for upstream connections

[mattklein123]

Needs some thinking around actual use cases but shouldn't be too difficult to write upstream filters and install them if needed.

[alanconway]

#3415 requires upstream L3/L4 filter chains attached to connections opened by clusters.
I propose to add a simplified version of the listener filter-chain configuration on the cluster configuration. I think there's no need for filter matching logic in this case, since we know where the connection is going - just a simple list of filters. It looks like I could have the cluster manager pass the configured filters when creating a HostImpl, which would create the filter chain on each client connection.

[mattklein123]

@alanconway yes that makes sense to me. Hopefully, you can reuse https://github.com/envoyproxy/envoy/blob/master/source/common/network/filter_manager_impl.h.

[alanconway]

I'm am looking at copying simple filter list config from listener_manager_impl to upstream_manager_impl, and from server/connection_handler_impl.cc newConnection() to upstream_impl.cc createConnection(). I think re-use of filter_manager_impl is implied, because it is a member of ConnectionImpl, which is used by both. It isn't complex, I just need to study how things are structured so I put it in the right place. So far the layout of the code seems pretty clear and logical, hope to have something working today.

[alanconway]

@mattklein123 question - re filter config:

I could move the listener.Filter proto type into the core package and import it into listener.proto and cds.proto. That would let me share all the code for building filter lists from the common config, upstream and downstream. However I'm not sure if that constitutes a breaking change for your config system.

Alternatively I can create a separate Cluster.Filter type in cds.proto with the same fields as listener.Filter. That will involve minor code duplication but it definitely won't break anything.

The choice also depends on whether upstream/downstream filters are the same type of thing or not. I lean towards treating them the same - there will be filter implementations that are only designed to work upstream or downstream and not both, but I don't think that requires they be different types for configuration or programming, and a single type means less to learn.

Preference?

[mattklein123]

Technically we would allow that kind of change. It's a breaking compile change not a breaking wire change. I'm fine with it. @htuch any thoughts?

[alanconway]

For now I am leaving everything where it is and creating some dubious dependencies from upstream to server code, I'll mark them all clearly. Once I have something working I'll let you folks review it and decide what should be pulled up for re use and where it should live. That's probably more efficient than me guessing at the re-org and then having to do it again if I get it wrong.

[ggreenway]

Another use case I'll probably need in the near to medium term: upstream proxy protocol. This would be straightforward with upstream network filters.

[alanconway]

Here's what I've got so far, not complete, untested etc (it compiles!) https://github.com/alanconway/envoy/tree/upstream-filter Here's how it looks in YAML config: clusters: - name: upstream_amqp filters: - name: envoy.filters.network.amqp_bridge connect_timeout: 0.25s type: STATIC hosts: [{ socket_address: { address: 127.0.0.1, port_value: 5672 }}]

…

On Fri, Jun 1, 2018 at 12:16 PM, Greg Greenway ***@***.***> wrote: Another use case I'll probably need in the near to medium term: upstream proxy protocol. This would be straightforward with upstream network filters. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#173 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AHa6XjfpKYdzgvwM9hY6ViZOTowthK2Rks5t4Wj5gaJpZM4KhTod> .

[alanconway]

Initial pull request #3571 - this works for #3415 (AMQP/HTTP bridge) but is not complete. I'd like a review of the direction before continuing.

[alanconway]

The client (cluster) and server (listener) filters are working, but I have an additional use case:

AMQP is a symmetric protocol, unlike HTTP. Since envoy is often a sidecar, I'd like many envoy sidecars to be able to connect to a single well-known dispatch rather than vice-versa. This trick is I'd like envoy to be able to receive requests from dispatch over those outbound connections. My amqp_client filter can do the translation and reverse the direction but I'm not sure how to route the resulting traffic in envoy.

I thought about a "reverse listener", a special address type for Listener which names a Cluster. Instead of listening, a reverse-listener would connect the cluster, flip the read/write sense of the connection, and present it to the rest of envoy as a downstream connection.

Thoughts?

[mattklein123]

@alanconway I don't think I fully follow your previous comment. Can you describe in more detail?

[alanconway]

In HTTP, the process (client) that opens the connection sends requests, the process (server) that listens for connections receives them - the direction of requests is always the same as the direction of establishing the connection.

In AMQP, once a connection is open, either side can create links and send messagse. Typically this is used in a client/server manner - the client sends requests, publishes to queues or subscribes to topics. However with Qpid Dispatch talking to Envoy, it's not really a client/server relationship - more a router-router peer relationship. The direction of making connections need not always match the direction of sending requests.

Specifically I anticipate deployments where there are a lot of Envoy sidecars sitting beside HTTP services and fewer dispatch routers, so it makes sense to have the sidecars "announce" themselves by making a connection to Dispatch, but then allow that connection to be used as if it were a listening connection - to receive requests from Dispatch that will be forwarded to local HTTP services.

This means reversing the sense of the connection since it was opened by envoy like a client but then is used as if it had been accepted like a server, which doesn't fit in Envoy's existing configuration - I'm wondering if there's a way it could be made to work. Does that make any sense?

[mattklein123]

This means reversing the sense of the connection since it was opened by envoy like a client but then is used as if it had been accepted like a server, which doesn't fit in Envoy's existing configuration - I'm wondering if there's a way it could be made to work. Does that make any sense?

Kind of. How is this connection going to be opened in the first place? I.e., how will Envoy know to make a reverse connection? Can this reverse connection then just do dispatch and loopback to a normal listener?

[alanconway]

Kind of. How is this connection going to be opened in the first place? I.e., how will Envoy know to make a reverse connection? Can this reverse connection then just do dispatch and loopback to a normal listener?
That is probably the best way to start: can we configure a Cluster to make a connection even if there's no HTTP traffic to prompt it? Then we can have an amqp_reverse_server filter on the cluster which is configured to make it's own loopback connection to a normal listener. Or something - I'll have a think.

[mattklein123]

That is probably the best way to start: can we configure a Cluster to make a connection even if there's no HTTP traffic to prompt it? Then we can have an amqp_reverse_server filter on the cluster which is configured to make it's own loopback connection to a normal listener. Or something - I'll have a think.

Your filter can do singleton tasks on the main thread (there are examples of ones that do this to pull in data, perform computations, etc.), though this is not on the datapath/worker so not sure if this will meet your needs or not. I'm not familiar enough with the protocol to have more concrete ideas right now. If you want to discuss more design stuff can you potentially put together a drawing/flow chart that shows the required flows? That might help.

[ggreenway]

@alanconway Are you planning ton continue work on this anytime soon? If not, I may pick this up in order to implement upstream proxy_protocol.

[alanconway]

@ggreenway that would be great; I want to get back to this but am juggling a bunch of other stuff so can't reliably promise to do this soon.

The pull request #3571 is working and tested by my AMQP prototype, but it lacks independent tests in the envoy test harness. There are probably a bunch of other rough edges, the comment on #3571 mentions the ones that are obvious to me.

[alanconway]

FYI I am working on finishing my initial pull request #3571, I'm adding tests, fixing the filter context and moving configuration to the proper place. If anyone knows of relevant changes or work that have happened since the pull request pop a note on here.

[alanconway]

I've got PR #6173 in for this issue, needs review.

[kyessenov]

We would like to pass additional metadata to the upstream endpoint per individual connections. Ideally, we want to share the work between HTTP and TCP filters. HTTP filters apply per individual requests, so we don't have a good way to inject extra stuff per upstream connection for HTTP filters. For TCP, we have a similar problem of re-using upstream connections for downstream connections which prevents us to apply stuff per upstream connection within downstream network filters.

What's the current work-around for these issues?

[alanconway]

My workaround is #6173. I feel that is still a viable PR but it needs to be updated. I've had a change in circumstances so don't have much time to give to it.

[mattklein123]

Complete