Skip to content

outbound: move target types into stack modules#971

Merged
hawkw merged 1 commit intomainfrom
eliza/require-profile-iv
Apr 13, 2021
Merged

outbound: move target types into stack modules#971
hawkw merged 1 commit intomainfrom
eliza/require-profile-iv

Conversation

@hawkw
Copy link
Contributor

@hawkw hawkw commented Apr 9, 2021

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 hawkw requested review from a team and olix0r April 9, 2021 18:47
@hawkw hawkw self-assigned this Apr 9, 2021
Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@hawkw hawkw force-pushed the eliza/require-profile-iii branch 2 times, most recently from 397a7e6 to c05b0f3 Compare April 13, 2021 22:03
Base automatically changed from eliza/require-profile-iii to main April 13, 2021 22:19
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 hawkw force-pushed the eliza/require-profile-iv branch from 913cae3 to c3f5d48 Compare April 13, 2021 22:25
@hawkw hawkw merged commit 576d720 into main Apr 13, 2021
@hawkw hawkw deleted the eliza/require-profile-iv branch April 13, 2021 22:46
hawkw added a commit that referenced this pull request Apr 14, 2021
…972)

Depends on #971

Currently, the proxy should only perform `Destination.Get` resolutions
for logical (named) addresses provided by `Destination.GetProfile`
lookups, and never for IP addresses. However, this is not *guaranteed*;
as the destination resolver still accepts a `LookupAddr` type which may
be either a logical named address *or* an IP address.

In order to remove support for IP lookups from the control plane's
`Destination.Get` implementation, we should ensure that the proxy will
*never* attempt to resolve an IP (see linkerd/linkerd2#5246). Now that
the `Logical` target type and the logical stacks are only constructed
when 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 #963), the `Logical` target type has an
  `Option<LogicalAddr>`. We attempt to unwrap the `Option<Receiver>`
  returned by a profile lookup, and build a logical stack when there is
  a profile. The `Logical` target is with `Option<LogicalAddr>` from
  that 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_logical` function added in #963 to use a custom
  filter predicate that only constructs the `Logical` target when the
  profile resolution is `Some` _and_ the profile's `addr` field is
  `Some`. Now, `Logical` targets are only built when there is a known
  logical address.

* The `Destination.Get` resolver requires a target type that implements
  `Param<ConcreteAddr>`. Currently, a `ConcreteAddr` is a newtype around
  `Addr`, which may be either a named address or an IP address. This
  branch changes `ConcreteAddr` to a newtype around `NameAddr`. This
  means that the resolver now ensures `Destination.Get` queries are only
  performed for named addresses at the type level.

* Similarly, the `Target` type in `linkerd-service-profiles` is changed
  to store a `NameAddr` rather than an `Addr`.

* 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::Debug` output for some of the target types that include
  `NameAddr`s was a little unnecesarily verbose. I added new custom
  `Debug` impls that should make these a bit more concise.

This is the proxy component of linkerd/linkerd2#5246.

Signed-off-by: Eliza Weisman <[email protected]>
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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants