connector-proxy: move from runtime to connector docker

[mdibaiee]

Idea

Johnny brought up this idea, and I've been thinking about it, how we might approach it and its implications.

In a nutshell, the idea is to move the connector-proxy's protocol translation part into the connector that needs it, rather than having it in the runtime. This means if we have an Airbyte connector, the connector's docker image can include a binary as its entrypoint, which exposes an interface compatible with Flow protocol, but translates Flow protocol to Airbyte protocol and then passes it to the Airbyte connector in the docker image. In case of connectors that work directly with Flow protocol, there is no need for a proxy.

Implications

Some implications I can think of:

1. Runtime does not need to know about all the different protocols that need a compatibility layer with Flow protocol
2. The runtime needs to make less assumptions and be less restricted by all the different protocols that we may want to support, so less code to maintain, and less bugs in runtime. Currently we have a bunch of hacky code to handle Airbyte's flakiness / incompatibilities with our protocol (e.g. sending a missing checkpoint at the end, waiting for config files to be written to disk before starting, etc.).
3. We no longer need to handle "injecting" a binary into docker containers and hacking up the image in order to run that binary, and no need to translate flow commands into airbyte commands
4. We will now need to bundle a proxy process with Airbyte connectors, the only challenge here is to make sure it is up-to-date with our latest protocol. The issue I can see here is that if we decide to update Flow protocol, we will need a way of making sure that existing versions of Airbyte connectors (with their old translation layer) still work. This is not something out-of-the-ordinary though, we will need some sort of versioning for our protocol if we decide to make backward-incompatible changes.

Potential Pathway

A potential pathway is extracting connector-proxy's Airbyte translation layer to a separate component (perhaps in https://github.com/estuary/airbyte so it can be released in synchrony with the connectors). What will be left in the runtime is just handling network-tunnel and Flow protocol. I can think of potential positives of using Go for this layer, given the simplicity of working with stdio (in Rust we have many abstractions on top of stdio at the moment), but I'm not sure if it's justified given we already have working code in Rust.

The Airbyte translation layer will then have to handle translating gRPC into Airbyte messages as it already does.

Open Questions

1. Do we keep NetworkTunnel as part of connector-proxy in the runtime or do we want to also move NetworkTunnel to be an independent binary that connectors can pull into their docker image if needed?

[williamhbaker]

I really like the idea of the runtime not needing to inject the binary into connector containers. And along those lines, leaving it up to connectors to figure out how they are going to comply with the protocol in a more encapsulated way.

I'm not too familiar with NetworkTunnel, so a question there: Is it the case that every connector that needs to use the connector proxy also needs to use the network tunnel, and that connectors that don't need connector proxy don't need the tunnel? Or is it more complicated than that?

[mdibaiee]

Network tunnel applies specifically to connectors which have an address field at the moment, what happens is we automatically inject network tunnel as part of these connectors' endpoint_spec_schema, so users can enable network tunnel for these connectors.

If network tunnel is enabled through endpoint_spec of the connector, then connector-proxy picks that up, and runs an OpenSSH tunnel given the address of the connector, and the credentials of the bastion server...

Just describing this, it feels like this could also be a separate binary that does all of this on top of an existing connector, but it will be slightly more work. However, I can see the merit of moving network tunnel to be a separate binary, since at the moment we have a hardcoded address field, and I can imagine if it is a binary, this can be part of its arguments as a configuration. We have also previously discussed the need for somehow specifying a default port for connectors that use network tunnel #513 (comment) cc @willdonnelly , but this would work:

ENTRYPOINT network-tunnel --address-field "address" --default-port 5342
ENTRYPOINT network-tunnel --host-field "host" --port-field "port"

These binaries can potentially be stacked on top of each other. If network-tunnel works on connectors that already support Flow protocol, then it's a matter of stacking it on top of the translation layer (this would work well once we have sockets as well):

#/bin/bash

airbyte-translator --out-socket /a-grpc-socket &
network-tunnel --in-socket /a-grpc-socket --out-socket /flow-socket

[jgraettinger]

I think this is great - this kind of composition would give us more flexibility. Non-SaaS Airbyte connector? It uses airbyte-translator and network-tunnel. Our materialize-postgres? It needs network-tunnel but can talk directly to the runtime / doesn't need a proxy. And so on.

[willdonnelly]

Moving to this sort of model feels right to me, I especially like that it becomes more reasonable to integrate things like Google's Cloud SQL Auth Proxy into just those connector images for which that makes sense.

[mdibaiee]

This is how I'm imagining the parts necessary to implement this:

1. Create a new program called airbyte-to-flow in estuary/airbyte with this interface:
   • --connector-entrypoint="python main.py": airbyte connector entrypoint
   • --in-socket=/flow-in: path to unix socket to read from flow runtime
   • --out-socket=/connector-out: path to unix socket to write to flow runtime
2. Update runtime (specifically the code in what is currently known as connector-proxy) to use two unix sockets at special paths (e.g. /flow-in and /connector-out) in the docker container of the connector to communicate using flow protocol instead of stdin and stdout
3. Update flow protocol connectors (source-boilerplate) to use the unix socket instead of stdio for communicating with flow. All connectors must accept parameters --in-socket and --out-socket to be able to configure the socket on which they communicate. This is necessary to allow stacking of different layers on each other.
4. Move network-tunnel to be an independent binary which also accepts --in-socket and --out-socket as well as other necessary parameters for configuration. We can provide this binary alone in a docker image, so other images can easily do COPY --from=ghcr.io/estuary/network-tunnel:v1 /network-tunnel ..
5. Update existing connectors to include network-tunnel binary and use it on top of their own process

[mdibaiee]

@willdonnelly I thought about that, I wanted the interface of all of these layers to be the same, and so if there is a case where we might need two sockets (I think this applies to any layer that sits between two layers), I thought we make it uniform and just use two sockets for all of them

[willdonnelly]

Yeah, but my point is that currently the only "intermediary" layer in the gRPC communication flow would be network-tunnel. I agree it makes sense to have two sockets there.

But for the proposed airbyte-to-flow binary it doesn't make sense (since as far as Flow is concerned that's the endpoint of the communications, the fact that the actual work is done by a subprocess over stdin/stdout is an implementation detail), and likewise those connectors which support gRPC directly should only need a single socket over which the Flow runtime sends requests and receives responses.

edit: Another way of expressing my complaint is that if an ordinary connector requires two sockets, the network-tunnel binary would actually require four by this logic: two for communicating with the underlying connector implementation and two for communicating with Flow.

[jgraettinger]

A couple of thoughts:

• It's not clear that network-tunnel needs to be a proxy. It could, instead, take some configuration (as json over stdin?) and do its thing. It's then the responsibility of the connector to know that it's composing in network-tunnel and to start it with its sub-configuration. In the case of Airbyte connectors, airbyte-to-flow would have to implement this extract-config-and-shell-to-network-tunnel behavior.

• I'd recommend having some channel by which the started program tells its caller that it's listening at the agreed place and ready to accept work. Otherwise there's a startup race where the caller can't know when it should actually dial the started delegate. I've used a message like "READY https://localhost:12345" or "READY unix:///path/to/socket" for this kind of thing in the past (for example, the npm worker).

• We need flexibility for sockets to be either Unix or regular TCP. If we're running these inside of Firecracker, for example, we must dial into the connector via TCP rather than a UDS.

• I am also confused why two sockets would be needed. Rather than "--out-socket", is it more like "here's the socket to forward to and proxy back from?" "--forward-socket" or "--delegate-socket"? I suppose whether this is needed depends on whether network-tunnel is a sidecar or a MitM proxy.

[mdibaiee]

edit: Another way of expressing my complaint is that if an ordinary connector requires two sockets, the network-tunnel binary would actually require four by this logic: two for communicating with the underlying connector implementation and two for communicating with Flow.

This clarified your position for me, thanks. You are right we can do with just one socket for most of these layers and when necessary they can have two layers (which also now depends on whether we make network-tunnel actually a mitm proxy or follow Johnny's potential idea).

[mdibaiee]

We need flexibility for sockets to be either Unix or regular TCP. If we're running these inside of Firecracker, for example, we must dial into the connector via TCP rather than a UDS.

This is a good point, thanks for bringing it, I'll consider it during implementation.

I'd recommend having some channel by which the started program tells its caller that it's listening at the agreed place and ready to accept work. Otherwise there's a startup race where the caller can't know when it should actually dial the started delegate. I've used a message like "READY https://localhost:12345/" or "READY unix:///path/to/socket" for this kind of thing in the past (for example, the npm worker).

Also a good point 👍🏽

It's not clear that network-tunnel needs to be a proxy. It could, instead, take some configuration (as json over stdin?) and do its thing. It's then the responsibility of the connector to know that it's composing in network-tunnel and to start it with its sub-configuration. In the case of Airbyte connectors, airbyte-to-flow would have to implement this extract-config-and-shell-to-network-tunnel behavior.

This would entail that the connector is responsible for receiving configuration for the tunnel, which can be a good thing, or perhaps "bad" if we consider a non-uniform UX for configuring network tunnel to be bad.

I can see the merit of this in cases where the connector needs a specific kind of configuration of network-tunnel to work, or the fact that the connector will know best how to construct the local address (host:port), and even can provide defaults for the forward host and port appropriate to the connector.

I think I like this more than hijacking the endpoint_spec_schema and requiring a specific field to be named, etc. etc., it seems to be a better pattern.