outbound: only build logical stacks for profiles with logical addrs#972
Merged
outbound: only build logical stacks for profiles with logical addrs#972
Conversation
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
(thanks @kleimkuhler) Signed-off-by: Eliza Weisman <[email protected]>
sorry kevin it doesn't compile Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
This branch builds on #963 by changing the traffic split service to require its target type implement `Param<profiles::Receiver>` rather than requiring `Param<Option<profiles::Receiver>>`. This lets us simplify the implementation significantly by removing the "default" case that's built when no profile was discovered. Depends on #963 Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
This branch builds on #963 by changing the traffic split service to require its target type implement `Param<profiles::Receiver>` rather than requiring `Param<Option<profiles::Receiver>>`. This lets us simplify the implementation significantly by removing the "default" case that's built when no profile was discovered. Depends on #963 Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
This is similar to PR #963 but on the inbound side. Since the inbound proxy only requires logical targets in the HTTP profile route layers, this change is much simpler --- protocol detection etc are all above the profile resolution layer. We can simply add an `UnwrapOr` layer that skips the profile route layers. This change is much less important than the corresponding outbound change, but the two changes together will permit us to simplify the profile route layer by taking a `Param<profiles::Receiver>` rather than a `Param<Option<profiles::Receiver>>`. This does also require adding an additional `BoxResponse` layer to erase differing response body types based on whether or not profile route metrics are recorded.
This branch changes the `linkerd_service_profiles::http::route_request` layer to require a `Param<profiles::Receiver>` rather than a `Param<Option<profiles::Receiver>>` target type. This is possible because the inbound and outbound logical destination stacks are now only built when a profile is resolved (as of PRs #963 and #966). Depends on #963 Depends on #966
Depends on #967 This branch refactors the `linkerd-app-outbound` crate so that each stack target type is now defined in the module for the stack that uses it, rather than in one big `target` module. For example, `Logical` is now defined in `logical.rs` and `Endpoint` is defined in `endpoint.rs`. There should be no functional change in this PR --- it just moves code around.
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
IMO this is nicer even if it does involve a `From` impl with a 3-tuple in it :/ Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Pothulapati
added a commit
to linkerd/linkerd2
that referenced
this pull request
Apr 12, 2021
Fixes #5246 This PR updates the destination to report an error when `Get` is called for IP Queries. As the issue mentions, The proxies are not using this API anymore and it helps to simplify and remove unecessary logic. This change is aimed for `stable-2.11`, and depends on linkerd/linkerd2-proxy#972 Signed-off-by: Tarun Pothulapati <[email protected]>
kleimkuhler
approved these changes
Apr 12, 2021
Contributor
kleimkuhler
left a comment
kleimkuhler
left a comment
There was a problem hiding this comment.
Few small comments but otherwise looks good.
Depends on #967 This branch refactors the `linkerd-app-outbound` crate so that each stack target type is now defined in the module for the stack that uses it, rather than in one big `target` module. For example, `Logical` is now defined in `logical.rs` and `Endpoint` is defined in `endpoint.rs`. There should be no functional change in this PR --- it just moves code around.
hawkw
force-pushed
the
eliza/require-profile-iv
branch
from
913cae3 to
c3f5d48
Compare
olix0r
reviewed
Apr 14, 2021
Member
olix0r
left a comment
olix0r
left a comment
There was a problem hiding this comment.
Looking pretty good. A few comments
Co-authored-by: Oliver Gould <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
Signed-off-by: Eliza Weisman <[email protected]>
olix0r
approved these changes
Apr 14, 2021
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Apr 21, 2021
This release primarily improves protocol detection error messages in the admin server so that logs clearly indicate when the client expected a different TLS server identity than that of the running proxy. A number of internal improvements have been made, especially eliminating some potential runtime panics detected by oss-fuzz. It is not expected that these panics could be triggered in typical cluster configurations. --- * discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969) * replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970) * concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968) * http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973) * transport: Fix orig-dst compilation on non-Linux targets (linkerd/linkerd2-proxy#974) * Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975) * addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976) * outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963) * split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964) * inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966) * profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967) * outbound: move target types into stack modules (linkerd/linkerd2-proxy#971) * outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972) * app: inbound: add fuzzer (linkerd/linkerd2-proxy#977) * admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979) * reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980) * app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978) * admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982) * chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983)
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
May 12, 2021
This release simplifies internals so that endpoint-forwarding logic is completely distinct from handling of load balanced services. The ingress-mode outbound proxy has been simplified to *require* the `l5d-dst-override` header and to fail non-HTTP communication. This ensures that the ingress-mode proxy does not unexpectedly revert to insecure communication. Finally, a regression was recently introduced that caused all proxy logs to be output with ANSI control characters. Logs are now output in plaintext by default --- * discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969) * replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970) * concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968) * http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973) * transport: Fix orig-dst compilation on non-Linux targets (linkerd/linkerd2-proxy#974) * Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975) * addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976) * outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963) * split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964) * inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966) * profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967) * outbound: move target types into stack modules (linkerd/linkerd2-proxy#971) * outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972) * app: inbound: add fuzzer (linkerd/linkerd2-proxy#977) * admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979) * reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980) * app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978) * admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982) * chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983) * fuzz: Add logging to fuzz targets (linkerd/linkerd2-proxy#985) * fuzz: don't panic when the proxy closes the conn (linkerd/linkerd2-proxy#986) * Commit lock files for fuzzers (linkerd/linkerd2-proxy#984) * fuzz: allow client requests to fail (linkerd/linkerd2-proxy#989) * tower: update dependency to 0.4.7 (linkerd/linkerd2-proxy#990) * outbound: Make the Endpoint::logical_addr field optional (linkerd/linkerd2-proxy#991) * trace: explicitly disable ANSI terminal colors (linkerd/linkerd2-proxy#994) * ingress: Require the l5d-dst-override header (linkerd/linkerd2-proxy#992) * outbound: Do not support TCP-forwarding in ingress-mode (linkerd/linkerd2-proxy#995) * Decouple tcp forward stack from Endpoint target (linkerd/linkerd2-proxy#996) * Pickup linkerd-await wrapper in docker build (linkerd/linkerd2-proxy#999) * docs: Add fuzzing report (linkerd/linkerd2-proxy#1000) * tests: Use io::Error in mocked connector (linkerd/linkerd2-proxy#1001) * outbound: Decouple endpoint & logical stacks (linkerd/linkerd2-proxy#1002)
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
May 18, 2021
This release simplifies internals so that endpoint-forwarding logic is completely distinct from handling of load balanced services. The ingress-mode outbound proxy has been simplified to *require* the `l5d-dst-override` header and to fail non-HTTP communication. This ensures that the ingress-mode proxy does not unexpectedly revert to insecure communication. Finally, a regression was recently introduced that caused all proxy logs to be output with ANSI control characters. Logs are now output in plaintext by default --- * discover: replace `linkerd-channel` with `tokio-util` `PollSender` (linkerd/linkerd2-proxy#969) * replace `linkerd-channel` with `tokio-stream` (linkerd/linkerd2-proxy#970) * concurrency-limit: use `tokio-util`'s `PollSemaphore` (linkerd/linkerd2-proxy#968) * http: Do not fail fuzz tests when all IO is not read (linkerd/linkerd2-proxy#973) * transport: Fix orig-dst compilation on non-Linux targets (linkerd/linkerd2-proxy#974) * Update trust-dns to fix possible panic (linkerd/linkerd2-proxy#975) * addr: fix `to_http_authority` panic with IPv6 (linkerd/linkerd2-proxy#976) * outbound: skip logical stacks when no profile is discovered (linkerd/linkerd2-proxy#963) * split: change traffic splits to require a profile (linkerd/linkerd2-proxy#964) * inbound: only build profile route stacks when a profile is resolved (linkerd/linkerd2-proxy#966) * profiles: make receiver param in `route_request` non-optional (linkerd/linkerd2-proxy#967) * outbound: move target types into stack modules (linkerd/linkerd2-proxy#971) * outbound: only build logical stacks for profiles with logical addrs (linkerd/linkerd2-proxy#972) * app: inbound: add fuzzer (linkerd/linkerd2-proxy#977) * admin: Fail connections when HTTP detection fails (linkerd/linkerd2-proxy#979) * reduce error boilerplate with `thiserror` (linkerd/linkerd2-proxy#980) * app: inbound: fuzzer: generalise fuzzers and solve coverage build (linkerd/linkerd2-proxy#978) * admin: Assume meshed connections are HTTP/2 (linkerd/linkerd2-proxy#982) * chore: Fix deprecations on nightly (linkerd/linkerd2-proxy#983) * fuzz: Add logging to fuzz targets (linkerd/linkerd2-proxy#985) * fuzz: don't panic when the proxy closes the conn (linkerd/linkerd2-proxy#986) * Commit lock files for fuzzers (linkerd/linkerd2-proxy#984) * fuzz: allow client requests to fail (linkerd/linkerd2-proxy#989) * tower: update dependency to 0.4.7 (linkerd/linkerd2-proxy#990) * outbound: Make the Endpoint::logical_addr field optional (linkerd/linkerd2-proxy#991) * trace: explicitly disable ANSI terminal colors (linkerd/linkerd2-proxy#994) * ingress: Require the l5d-dst-override header (linkerd/linkerd2-proxy#992) * outbound: Do not support TCP-forwarding in ingress-mode (linkerd/linkerd2-proxy#995) * Decouple tcp forward stack from Endpoint target (linkerd/linkerd2-proxy#996) * Pickup linkerd-await wrapper in docker build (linkerd/linkerd2-proxy#999) * docs: Add fuzzing report (linkerd/linkerd2-proxy#1000) * tests: Use io::Error in mocked connector (linkerd/linkerd2-proxy#1001) * outbound: Decouple endpoint & logical stacks (linkerd/linkerd2-proxy#1002)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Depends on #971
Currently, the proxy should only perform
Destination.Getresolutionsfor logical (named) addresses provided by
Destination.GetProfilelookups, and never for IP addresses. However, this is not guaranteed;
as the destination resolver still accepts a
LookupAddrtype which maybe either a logical named address or an IP address.
In order to remove support for IP lookups from the control plane's
Destination.Getimplementation, we should ensure that the proxy willnever attempt to resolve an IP (see linkerd/linkerd2#5246). Now that
the
Logicaltarget type and the logical stacks are only constructedwhen a profile is resolved (as of PR #963), we can now change the proxy
to ensure statically that this never happens.
This branch makes the following changes:
Currently (as of outbound: skip logical stacks when no profile is discovered #963), the
Logicaltarget type has anOption<LogicalAddr>. We attempt to unwrap theOption<Receiver>returned by a profile lookup, and build a logical stack when there is
a profile. The
Logicaltarget is withOption<LogicalAddr>fromthat profile.
However, this is not actually the ideal behavior: ideally, we would
only contruct the logical stack when there is a resolved profile and
the profile includes a logical address. Therefore, this branch changes
the
push_unwrap_logicalfunction added in outbound: skip logical stacks when no profile is discovered #963 to use a customfilter predicate that only constructs the
Logicaltarget when theprofile resolution is
Someand the profile'saddrfield isSome. Now,Logicaltargets are only built when there is a knownlogical address.
The
Destination.Getresolver requires a target type that implementsParam<ConcreteAddr>. Currently, aConcreteAddris a newtype aroundAddr, which may be either a named address or an IP address. Thisbranch changes
ConcreteAddrto a newtype aroundNameAddr. Thismeans that the resolver now ensures
Destination.Getqueries are onlyperformed for named addresses at the type level.
Similarly, the
Targettype inlinkerd-service-profilesis changedto store a
NameAddrrather than anAddr.Several of the outbound stack tests now test invalid behavior that
should no longer happen --- building a logical stack and calling it
with a target that doesn't include a logical address. I rewrote these
tests so that the tests without logical addresses don't build a
logical stack, and added a new test for calling the logical stack
with a logical address. In the process, I did some cleanup and
refactoring in the outbound TCP stack tests.
Finally, while debugging test failures, I noticed that the
fmt::Debugoutput for some of the target types that includeNameAddrs was a little unnecesarily verbose. I added new customDebugimpls that should make these a bit more concise.This is the proxy component of linkerd/linkerd2#5246.
Signed-off-by: Eliza Weisman [email protected]