outbound: discover client policies from the `OutboundPolicy` API by hawkw · Pull Request #2265 · linkerd/linkerd2-proxy

hawkw · 2023-02-24T19:58:57Z

This branch changes the outbound proxy to resolve client policies using
the OutboundPolicy proxy-api service (added in
linkerd/linkerd2-proxy-api#165). The proxy will preferentially use an
OutboundPolicy resolution over a ServiceProfile resolution in the
case that both are resolved and the OutboundPolicy is not a default
policy. If the OutboundPolicy is a default policy, it is used only
when no ServiceProfile is resolved. The ingress-mode proxy also
performs OutboundPolicy resolutions.

This branch also adds an implementation of the OutboundPolicy service
to the mock policy controller in linkerd-app-integration, and some
integration tests for client policies (including header-based routing).

Some notes:

The policy controller address, policy controller name, and workload
identifier environment variables are now mandatory, since the proxy
will always attempt to resolve outbound client policies.
Currently, targets constructed from an OutboundPolicy will report
the original destination address as their LogicalAddr, rather than
an authority. In the future, we will change the way metrics, tap, and
error labels work to obviate the need for LogicalAddrs, and use
structured metadata here instead.
We probably ought to add additional tests in follow-up branches.
I don't love that the TargetAddr type, which is the target
address for both ServiceProfile and OutboundPolicy resolutions,
has to live in the linkerd-client-policy crate. But unfortunately,
it cannot be defined in linkerd-app-outbound, as the direct stack in
linkerd-app-inbound must also implement Param<TargetAddr>.
The gateway doesn't currently resolve client policies, but I'll have a
follow-up PR for that ready shortly.

And, pull the discovery allowlist out into a wrapper around `GetProfile`. This way, a discovery service performing profile discovery, policy discovery, or both can be passed into `push_discover`, depending on the stack.

This reverts commit 04f4dcf.

; Conflicts: ; linkerd/app/inbound/src/policy/config.rs ; linkerd/app/inbound/src/policy/store.rs ; linkerd/app/inbound/src/server.rs ; linkerd/app/inbound/src/test_util.rs ; linkerd/app/src/env.rs

hawkw · 2023-02-27T18:53:57Z

pushed test docker image mycoliza/l2-proxy:eliza.policy-discovery.744bd98c7

linkerd/app/outbound/src/discover.rs

* Split out synthetic policies for readability * wi * fix discovery integration tests * fixup integration * golf * clarity * Differentiate forward metadata * Update linkerd/app/outbound/src/discover.rs --------- Co-authored-by: Eliza Weisman <eliza@buoyant.io>

linkerd/proxy/client-policy/src/lib.rs

olix0r · 2023-03-11T18:20:44Z

linkerd/app/outbound/src/discover/tests.rs

+        let profiles = support::profile::resolver().profile(addr, profiles::Profile::default());
+        svc::mk(move |OrigDstAddr(addr)| {
+            // TODO(eliza): policy!
+            profiles.clone().oneshot(profiles::LookupAddr(addr.into()))


does this test care about profiles or policies at all, really? What are we testing?

i don't think this test actually cares besides that it needs something it can construct an inner stack from. if we remove theFrom<(Option<profile::Receiver>, T)> impl for Discovery<T> (making both a policy rx mandatory), we will need to return both a policy and profile rx here as well.

i believe this test is testing the behavior of the push_discover stack, so we can't just remove that entirely.

outdated

linkerd/app/outbound/src/sidecar.rs

Co-authored-by: Oliver Gould <ver@buoyant.io>

linkerd/app/outbound/src/sidecar.rs

Make clearer assertions about supported routing states.

To support Gateway API-style routes in the outbound proxy, we need to begin discovering this route configuration from the control plane (via the new `OutboundPolicies` API). This change updates the proxy as follows: 1. Policy controller configuration is now required for the proxy. Previously, the policy API was optionally configured for the inbound proxy. 2. The sidecar and ingress proxies are updated to use client policies. Service profile configurations continue to be used when they include HTTP routes and/or traffic split. Otherwise, a client policy is used to route traffic. Outbound policies are currently discovered for *all* outbound IP addresses. Over time, the policy controller will assume responsibility to make *all* routing decisions. It does not yet serve responses for all cases, however, so some fallback behavior exists to use endpoint metadata from profile discovery, if it exists. The multi-cluster gateway configuration does not yet use policies for outbound routing. Furthermore, the proxy reports an IP logical address for policy routes (instead of a named address, as is done with profiles). There are no new metrics or labels introduced in this PR. Metrics changes will be made in follow-up changes. --- * outbound: Decouple backend caching from request distribution (linkerd/linkerd2-proxy#2284) * build(deps): bump socket2 from 0.4.7 to 0.4.9 (linkerd/linkerd2-proxy#2290) * README: comment just-cargo and make it more clear (linkerd/linkerd2-proxy#2292) * build(deps): bump prettyplease from 0.1.23 to 0.1.24 (linkerd/linkerd2-proxy#2293) * build(deps): bump tokio from 1.25.0 to 1.26.0 (linkerd/linkerd2-proxy#2286) * build(deps): bump petgraph from 0.6.2 to 0.6.3 (linkerd/linkerd2-proxy#2285) * client-policy: add protobuf conversion (linkerd/linkerd2-proxy#2289) * integration: add test policy controller (linkerd/linkerd2-proxy#2288) * outbound: change `push_discover` to take a `Service` (linkerd/linkerd2-proxy#2291) * build(deps): bump rustix from 0.36.7 to 0.36.9 (linkerd/linkerd2-proxy#2295) * build(deps): bump serde_json from 1.0.93 to 1.0.94 (linkerd/linkerd2-proxy#2296) * build(deps): bump async-trait from 0.1.64 to 0.1.66 (linkerd/linkerd2-proxy#2297) * build(deps): bump thiserror from 1.0.38 to 1.0.39 (linkerd/linkerd2-proxy#2298) * build(deps): bump mio from 0.8.5 to 0.8.6 (linkerd/linkerd2-proxy#2299) * separate policy client config from `inbound::Config` (linkerd/linkerd2-proxy#2307) * outbound: Require ClientPolicy discovery (linkerd/linkerd2-proxy#2265) Signed-off-by: Oliver Gould <ver@buoyant.io>

To support Gateway API-style routes in the outbound proxy, we need to begin discovering this route configuration from the control plane (via the new `OutboundPolicies` API). This change updates the proxy as follows: 1. Policy controller configuration is now required for the proxy. Previously, the policy API was optionally configured for the inbound proxy. 2. The sidecar and ingress proxies are updated to use client policies. Service profile configurations continue to be used when they include HTTP routes and/or traffic split. Otherwise, a client policy is used to route traffic. Outbound policies are currently discovered for *all* outbound IP addresses. Over time, the policy controller will assume responsibility to make *all* routing decisions. It does not yet serve responses for all cases, however, so some fallback behavior exists to use endpoint metadata from profile discovery, if it exists. The multi-cluster gateway configuration does not yet use policies for outbound routing. Furthermore, the proxy reports an IP logical address for policy routes (instead of a named address, as is done with profiles). There are no new metrics or labels introduced in this PR. Metrics changes will be made in follow-up changes. --- * outbound: Decouple backend caching from request distribution (linkerd/linkerd2-proxy#2284) * build(deps): bump socket2 from 0.4.7 to 0.4.9 (linkerd/linkerd2-proxy#2290) * README: comment just-cargo and make it more clear (linkerd/linkerd2-proxy#2292) * build(deps): bump prettyplease from 0.1.23 to 0.1.24 (linkerd/linkerd2-proxy#2293) * build(deps): bump tokio from 1.25.0 to 1.26.0 (linkerd/linkerd2-proxy#2286) * build(deps): bump petgraph from 0.6.2 to 0.6.3 (linkerd/linkerd2-proxy#2285) * client-policy: add protobuf conversion (linkerd/linkerd2-proxy#2289) * integration: add test policy controller (linkerd/linkerd2-proxy#2288) * outbound: change `push_discover` to take a `Service` (linkerd/linkerd2-proxy#2291) * build(deps): bump rustix from 0.36.7 to 0.36.9 (linkerd/linkerd2-proxy#2295) * build(deps): bump serde_json from 1.0.93 to 1.0.94 (linkerd/linkerd2-proxy#2296) * build(deps): bump async-trait from 0.1.64 to 0.1.66 (linkerd/linkerd2-proxy#2297) * build(deps): bump thiserror from 1.0.38 to 1.0.39 (linkerd/linkerd2-proxy#2298) * build(deps): bump mio from 0.8.5 to 0.8.6 (linkerd/linkerd2-proxy#2299) * separate policy client config from `inbound::Config` (linkerd/linkerd2-proxy#2307) * outbound: Require ClientPolicy discovery (linkerd/linkerd2-proxy#2265) * just: Fix docker tag formatting (linkerd/linkerd2-proxy#2312) * outbound: Report concrete authorities for policies (linkerd/linkerd2-proxy#2313) Signed-off-by: Oliver Gould <ver@buoyant.io>

hawkw added 11 commits February 24, 2023 12:01

use proxy-api with client policy

ed0afe6

make push_discover generic over any discovery service

6b812a2

And, pull the discovery allowlist out into a wrapper around `GetProfile`. This way, a discovery service performing profile discovery, policy discovery, or both can be passed into `push_discover`, depending on the stack.

plumbing

e470504

wip: extractparam

b77c865

Revert "wip: extractparam"

ec9cf76

This reverts commit 04f4dcf.

add target addr newtype

9f93fee

more wip

ae1c898

wip protobuf conversion

1272132

blergh

3e21fa5

first pass on protobuf conversion

1cf0637

+lockfile

0e1cbc8

hawkw force-pushed the eliza/policy-discovery branch from 6602162 to 0e1cbc8 Compare February 24, 2023 20:01

hawkw added 5 commits February 24, 2023 12:18

add API client

64bf4c8

pull policy client out of inbound

035aa11

almost works except addr types

9d2355b

make worky

a8b1f1a

Merge branch 'main' into eliza/policy-discovery

744bd98

; Conflicts: ; linkerd/app/inbound/src/policy/config.rs ; linkerd/app/inbound/src/policy/store.rs ; linkerd/app/inbound/src/server.rs ; linkerd/app/inbound/src/test_util.rs ; linkerd/app/src/env.rs

hawkw added 12 commits February 27, 2023 12:57

update proxy-api branch

abb4c17

make tests compiley

dadcc1f

why would eliza put that there

9fbaecd

wire up integration test policy controller

c95fff7

implement test client policy controller

e491ae5

treat NotFound as no policy

788393c

placate clippy

a275a56

start on client policy integration tests

cfcd3d1

wip actually wire up routes (whoops)

13720fc

start actually using routes

9af725d

select correct version routes

3de6910

handle policy protocol hints slightly nicer

a9376ec

hawkw added 2 commits March 10, 2023 13:23

cleanup

224d353

clippy

dd2e899

olix0r reviewed Mar 11, 2023

View reviewed changes

linkerd/app/outbound/src/discover.rs Show resolved Hide resolved

olix0r previously approved these changes Mar 11, 2023

View reviewed changes

olix0r reviewed Mar 11, 2023

View reviewed changes

linkerd/proxy/client-policy/src/lib.rs Outdated Show resolved Hide resolved

hawkw added 4 commits March 11, 2023 10:17

singleton-ize metadata

0b2aa15

add span to spawned policy synthesis future

9d5039f

rm mut

6778fd1

+clone

8242006

olix0r reviewed Mar 11, 2023

View reviewed changes

less brittle error cause extraction

9d4a7f8

olix0r reviewed Mar 11, 2023

View reviewed changes

linkerd/app/outbound/src/sidecar.rs Outdated Show resolved Hide resolved

olix0r reviewed Mar 11, 2023

View reviewed changes

linkerd/app/outbound/src/sidecar.rs Outdated Show resolved Hide resolved

Update linkerd/proxy/client-policy/src/lib.rs

8d62e92

Co-authored-by: Oliver Gould <ver@buoyant.io>

olix0r reviewed Mar 11, 2023

View reviewed changes

linkerd/app/outbound/src/sidecar.rs Outdated Show resolved Hide resolved

hawkw added 3 commits March 11, 2023 10:34

Update linkerd/app/outbound/src/sidecar.rs

97f5387

wrong queue

98ace90

try to make comment clearer

584fd77

olix0r reviewed Mar 11, 2023

View reviewed changes

linkerd/app/outbound/src/sidecar.rs Outdated Show resolved Hide resolved

whoops actually turn it into an addr

287f360

olix0r reviewed Mar 11, 2023

View reviewed changes

linkerd/app/outbound/src/sidecar.rs Outdated Show resolved Hide resolved

hawkw and others added 2 commits March 11, 2023 10:55

another pass at logicaladdr comment

ebb5105

Clarify sidecar routing behavior (#2311)

bf0e0b8

Make clearer assertions about supported routing states.

olix0r merged commit 3dec105 into main Mar 11, 2023

olix0r deleted the eliza/policy-discovery branch March 11, 2023 22:41

olix0r mentioned this pull request Mar 11, 2023

proxy: v2.192.1 linkerd/linkerd2#10498

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

outbound: discover client policies from the `OutboundPolicy` API#2265

outbound: discover client policies from the `OutboundPolicy` API#2265
olix0r merged 86 commits intomainfrom
eliza/policy-discovery

hawkw commented Feb 24, 2023 •

edited

Loading

Uh oh!

hawkw commented Feb 27, 2023

Uh oh!

Uh oh!

Uh oh!

olix0r Mar 11, 2023

Uh oh!

hawkw Mar 11, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

hawkw commented Feb 24, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hawkw commented Feb 27, 2023

Uh oh!

Uh oh!

Uh oh!

olix0r Mar 11, 2023

Choose a reason for hiding this comment

Uh oh!

hawkw Mar 11, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

hawkw commented Feb 24, 2023 •

edited

Loading