Comments (15)
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 aHost
header (that was added inHTTP/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 theSO_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
forHTTP/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:
- try to route HTTP/1.0 as HTTP -- if there is no host header, we use the original dst ip.
- 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.
- 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.
Related Issues (20)
- Intermittent routing failures with HTTPRoute HOT 12
- Linkerd-proxy logging full header contents of incoming http requests for log level debug and trace. HOT 3
- Allow port ranges in dynamic authorization policy resources
- Prometheus metrics scrapes of `linkerd-proxy` are not TLS protected (occassionally) HOT 6
- Change default `cr.l5d.io` to `ghcr.io`? HOT 1
- Linkerd Multi-Cluster service-mirroring to give option to mirror EndpointSlices as well HOT 3
- Helm upgrade always changing due to trust root? HOT 2
- Connection refused randomly for pairs of pods HOT 4
- Destination container in the linkerd-destination pod panics when using deployments with headless services
- Connection refused (os error 111) error.sources=[Connection refused (os error 111)] HOT 2
- [FR] - reduce endpoint added/removed logs to debug HOT 4
- `duplicate metrics` in destination controller
- Linkerd is giving 200 or 400 responses for the same un-encoded url request depending on the situation HOT 5
- Linkerd destination repeatedly logging endpoint profile translator errors HOT 3
- Linkerd proxy fails to connect to other proxy HOT 2
- duplicated copies of trust anchor certificate HOT 1
- IPv6 semantics differ from Kubernetes without Linkerd HOT 3
- Helm install documentation refers to incorrect repo HOT 1
- Traefik Router unable to communicate with meshed services when linkerd inbound policy is all-authenticated. HOT 5
- Policy controller fails to watch resources. HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from linkerd2.