Add HTTP/1 routing to the proxy about linkerd2 HOT 15 CLOSED

How do we want to handle HTTP/1.0 requests?

If there is a Host header, I think we should try to route it anyway (unless that's especially hard, codec-wise). Otherwise, I agree that we would ideally try to handle it as TCP.

Based on our experience with linkerd, there are many dumb clients in the wild. We should aim to transit the traffic if at all possible. Failing with a 502 is a fair last resort.

from linkerd2.

from linkerd2.

Having gotten a proof-of-concept working here, I'm now moving on to error handling, which leads to some implementation questions:

• How do we want to handle HTTP/1.0 requests? They do not have to have a Host header (that was added in HTTP/1.1), and so in the outbound case, we could have an instance of not knowing where to route request. Do we want to assume a target of just the SO_ORIGINAL_DST in that case? Or do we return a 400 error?
• As a blanket case, we map all errors into a 500 http response. In the case of a server not being able to handle h1 or h2, and closing the connection, do we stick to calling that a 500 error, or try to find cases where it was the user's fault for using the wrong protocol and send a 400 response?

from linkerd2.

My gut reactions:

• Forwarding to SO_ORIGINAL_DST for HTTP/1.0 seems like the right thing to do since this is what we would do for TCP anyway.
• In these cases if Conduit wasn't in the picture, the client would just see a closed connection, right? I'm not sure why, but it seems weird to map this case to a 4XX even if it is technically the client's fault. Returning a 5XX feels like a more natural representation of a closed connection for some reason.

from linkerd2.

How do we want to handle HTTP/1.0 requests?

Immediately, I think it's fine to punt on HTTP/1.0 until the rest of the work is done, just returning a 400. Or, we could even treat them as TCP (non-HTTP) requests in the immediate term since we cannot meaningfully transform the request, and we (potentially) have to wait for the service to close the connection and then close the incoming connection after forwarding, which are basically the TCP semantics. It would be nice to have HTTP-level telemetry for HTTP/1.0 but that could come later.

from linkerd2.

As a blanket case, we map all errors into a 500 http response. In the case of a server not being able to handle h1 or h2, and closing the connection, do we stick to calling that a 500 error, or try to find cases where it was the user's fault for using the wrong protocol and send a 400 response?

Isn't this a 502?

The server was acting as a gateway or proxy and received an invalid response from the upstream server.

from linkerd2.

☝️ yeah, that definitely sounds like Bad Gateway to me

from linkerd2.

I really like @briansmith 's suggestion to treat it as opaque TCP/IP. Are there any drawbacks aside from us not augmenting the connection with Conduit ✨ ?

from linkerd2.

Isn't this a 502?

Yep . I should spec more.

If there is a Host header, I think we should try to route it anyway (unless that's especially hard, codec-wise). Otherwise, I agree that we would ideally try to handle it as TCP.

At the moment, it'd be somewhat tricky to maybe route on HTTP/1.0. When peeking the connection, we might be able to notice it's HTTP/1.0, or the path might be really long and we don't actually know whether it's 1.0 or 1.1 yet. At that stage, it either goes into the TCP pipe flow, or the HTTP service flow. Only in the HTTP service flow, after the headers have been parsed, can we tell if it is HTTP/1.0 without a Host header.

At discovery time, we could make the "authority" of the request be the SO_ORIGINAL_DST iff the request is 1.0 and there is no Host header. Would it make sense to send the original socket address to the controller's destination service? Or should it completely skip asking the controller in that case?

from linkerd2.

I think my preferences are the following:

1. try to route HTTP/1.0 as HTTP -- if there is no host header, we use the original dst ip.
2. try to detect HTTP/1.0 and treat it as TCP; if an HTTP/1.0 request sneaks through, we fail it with a 502.
3. fail it with a 502.

Treating the the connection as TCP doesn't seem desirable from the product point of view -- we're preventing the user from getting access to data that we should be able to know about.

Would it make sense to send the original socket address to the controller's destination service? Or should it completely skip asking the controller in that case?

With the first approach i describe above, i could see it being much simpler from the proxy's perspective for us to attempt to discover and balance ip names through the controller. If this is indeed the case, we could have the controller simply echo back the requested IP in the short term (and later apply smarter pod-mapping stuff).

It's not required that we resolve original dst ips through the controller, though.

from linkerd2.

The easiest from an implementation side is to return the original dst as the "fully qualified authority" in discovery if there is no Host header. That would mean some work is also needed in the controller to echo the address back?

from linkerd2.

@seanmonstar yes; though i'd imagine that is a pretty simple change.

from linkerd2.

Treating the the connection as TCP doesn't seem desirable from the product point of view -- we're preventing the user from getting access to data that we should be able to know about.

To clarify my suggestion, I was saying that in the short term, it would be OK to treat HTTP/1.0 as TCP if that unblocks more important work, if it is complicated to properly handle HTTP/1.0, e.g. because the request might not have a Host header and we won't know that until after we have to make a decision about how to treat the request.

I definitely agree that eventually and even soon we should treat HTTP/1.0 as HTTP as much as possible, especially when there is a Host header.

from linkerd2.

Would it make sense to send the original socket address to the controller's destination service? Or should it completely skip asking the controller in that case?

With the first approach i describe above, i could see it being much simpler from the proxy's perspective for us to attempt to discover and balance ip names through the controller. If this is indeed the case, we could have the controller simply echo back the requested IP in the short term (and later apply smarter pod-mapping stuff).

I think that we should avoid that, if practical. In general, there is a N:M relationship between IP addresses and names. In some cases we might be able to do the equivalent of a reverse IP lookup to go from IP -> name and then, if there is only one element in the result array, go from names[0] -> [IP]. However, whatever benefit we get from that that seems complicated, error-prone, and not super high value compared to other things we could be doing. I think it would be useful for us to collect telemetry on how common HTTP/1.0 without a Host header actually is, and then ask users to send us that telemetry, before we put work into it.

Instead, I suggest that when there is no Host, we simply skip asking the controller anything and just send the request to the SO_ORIGINAL_DST SocketAddr (IP address and port).

The easiest from an implementation side is to return the original dst as the "fully qualified authority" in discovery if there is no Host header. That would mean some work is also needed in the controller to echo the address back?

I don't understand why using the SO_ORIGINAL_DST SocketAddr would require any interaction with the controller. Maybe because the FullyQualifiedAuthority requires a DNS name? Currently FullyQualifiedAuthority requires a DNS name to support the upcoming TLS work. Importantly, for TLS, the FullyQualifiedAuthority can't, in general, be sourced from the Destinations service, because we don't consider the Destinations service to be trustworthy.

I suggest instead that we create something like this, in the proxy:

enum Destination {
    // Named destinations. These potentially can be upgraded to
    // TLS.
    Named(FullyQualifiedAuthority),

    // Only for HTTP requests without a Host header. These
    // cannot be upgraded to TLS because we don't know the
    // DNS name to use to validate the certificate.
    Unnamed(SocketAddr),
}

Then change Connect to use Destination instead of FullyQualifiedAuthority.

Later on, we can come up with ways to deal with unnamed destinations. And in the interim we can document that the Host header is required for Conduit and if it isn't provided there will be some limitations.

Note also that, while the HTTP/1.1 spec requires the Host header field to be present, it is not unheard of for a HTTP/1.1 request to be lacking the Host header. I think we should treat an HTTP/1.1 request without a Host header as though it really were a HTTP/1.0 request and forward it the same way, to break as little as possible.

from linkerd2.

I don't understand why using the SO_ORIGINAL_DST SocketAddr would require any interaction with the controller.

Oh, it's not required. I was saying that's the absolute easiest, as then the recognize stuff just returns the original dst instead of a value from the Host header. We certainly can augment our Discover system to receive an enum and either ask the controller, or skip and just bind directly to the address.

from linkerd2.