-
Notifications
You must be signed in to change notification settings - Fork 268
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Handle HTTP traffic over opaque transport connections #1416
Conversation
If an endpoint is set as opaque, but its logical service is not marked as opaque, the connection will error out. The `TransportHeader` in this case will not contain the logical name of the service, but it will still do protocol detection. Through this change, when an endpoint is marked as opaque and we connect to the proxy's inbound port, if the `TransportHeader` has a protocol, we go through the inbound HTTP stack instead of proxying TCP. Add session protocol to local Signed-off-by: Matei David <matei@buoyant.io>
Signed-off-by: Matei David <matei@buoyant.io>
Signed-off-by: Matei David <matei@buoyant.io>
Signed-off-by: Matei David <matei@buoyant.io>
let permit = | ||
allow.check_authorized(client.client_addr, &tls)?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, we are no longer checking the policy on opaque connections?
It's probably better to introduce a new target type--LocalHttp
or something (and rename Local
to LocalTcp
?). This switch could return an Either<Either<LocalTcp, LocalHttp>, GatewayTransportHeader>
and then the inner switch predicate can simply return the target... I can probably put a suggestion up to this effect.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* Introduce a dedicated direct::LocalHttp target type * -pub * - needless borrow * remove redundant check_policy
In addition to dependency updates, this change updates the inbound proxy to handle opaquely transported HTTP traffic. This fixes an issue encountered when a `Service`'s opaque ports annotation does not match that of the pods in the service. This situation should be rare. --- * Handle HTTP traffic over opaque transport connections (linkerd/linkerd2-proxy#1416) * build(deps): bump tracing-subscriber from 0.3.3 to 0.3.4 (linkerd/linkerd2-proxy#1421) * build(deps): bump pin-project from 1.0.8 to 1.0.9 (linkerd/linkerd2-proxy#1422) * build(deps): bump tracing-subscriber from 0.3.4 to 0.3.5 (linkerd/linkerd2-proxy#1423) * build(deps): bump pin-project from 1.0.9 to 1.0.10 (linkerd/linkerd2-proxy#1425) * build(deps): bump http from 0.2.5 to 0.2.6 (linkerd/linkerd2-proxy#1424) * build(deps): bump serde_json from 1.0.73 to 1.0.74 (linkerd/linkerd2-proxy#1427) * Decouple client connection metadata from the I/O type (linkerd/linkerd2-proxy#1426) * tests: rename 'metrics' addr to 'admin' (linkerd/linkerd2-proxy#1429)
In addition to dependency updates, this change updates the inbound proxy to handle opaquely transported HTTP traffic. This fixes an issue encountered when a `Service`'s opaque ports annotation does not match that of the pods in the service. This situation should be rare. --- * Handle HTTP traffic over opaque transport connections (linkerd/linkerd2-proxy#1416) * build(deps): bump tracing-subscriber from 0.3.3 to 0.3.4 (linkerd/linkerd2-proxy#1421) * build(deps): bump pin-project from 1.0.8 to 1.0.9 (linkerd/linkerd2-proxy#1422) * build(deps): bump tracing-subscriber from 0.3.4 to 0.3.5 (linkerd/linkerd2-proxy#1423) * build(deps): bump pin-project from 1.0.9 to 1.0.10 (linkerd/linkerd2-proxy#1425) * build(deps): bump http from 0.2.5 to 0.2.6 (linkerd/linkerd2-proxy#1424) * build(deps): bump serde_json from 1.0.73 to 1.0.74 (linkerd/linkerd2-proxy#1427) * Decouple client connection metadata from the I/O type (linkerd/linkerd2-proxy#1426) * tests: rename 'metrics' addr to 'admin' (linkerd/linkerd2-proxy#1429)
Closes linkerd/linkerd2#6178
When an endpoint is marked as opaque, but its logical service does not have an opaque annotation, the
TransportHeader
will not an include an alternate name, but it will the connection protocol:The connection will be closed with an error. Through this change, we can handle HTTP traffic over opaque connections a bit more gracefully. When a
TransportHeader
has a protocol but no alternate name for the target, instead of rejecting the connection, we go through the inbound http stack. The full set of logs are attached in a spoiler tag below.Full set of logs from k3d test
This seems to work in k3d. To get this to work, I had to implement a bunch of param traits on
Local
so that it can be used with the HTTP stack. In the process, I refactored some of the existingparam
implementations; would appreciate a more thorough look there to make sure we don't have redundancies or obvious failures.Edit: I see there are some leftover compiler assertions we used for the target type. Do we generally want those removed when the PR is submitted? Probably an obvious question but thought I'd ask anyway.