outbound: change push_discover to take a Service#2291
Merged
Conversation
Currently, the `push_discover` method in `linkerd_app_outbound` takes a `GetProfile` implementation, which is used to discover ServiceProfiles. In order to change this function to discover both ServiceProfiles and OutboundPolicies, PR #2265 will change this function to take an arbitrary `Service` that may discover ServiceProfiles, OutboundPolicies, or both. In order to make reviewing the change adding client policy discovery easier, the change to `push_discover` was factored out into this PR. Since `push_discover` now takes an arbitrary discovery service, and since we only want the configured profile discovery allowlist to apply to ServiceProfiles (and not to OutboundPolicies), this change also pulls out the filtering of addresses not in the allowlist from a layer pushed in `push_discover` into a middleware which is added to the profile discovery `Service`.
olix0r
reviewed
Mar 7, 2023
olix0r
reviewed
Mar 7, 2023
olix0r
reviewed
Mar 7, 2023
olix0r
requested changes
Mar 7, 2023
Member
There was a problem hiding this comment.
I think the best course of action is probably to move the allowlist stuff directly into the service profile crate. It's not an outbound-specific concern. There's not enough value in it being generic/decoupled from its use case. It's fine.
We shouldn't have to care about the type in tests, though; and it should just be part of how a profile service is built for each proxy variant (including inbound!)
olix0r
reviewed
Mar 7, 2023
olix0r
reviewed
Mar 7, 2023
olix0r
approved these changes
Mar 7, 2023
Member
olix0r
left a comment
There was a problem hiding this comment.
lgtm. Would generally prefer that we continue to export the Match types from core (as we do for the Addr types).
Contributor
Author
|
sounds good, i'll put it back |
Co-authored-by: Oliver Gould <ver@buoyant.io>
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Mar 11, 2023
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>
olix0r
added a commit
to linkerd/linkerd2
that referenced
this pull request
Mar 13, 2023
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>
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
2 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.
Currently, the
push_discovermethod inlinkerd_app_outboundtakes aGetProfileimplementation, which is used to discover ServiceProfiles.In order to change this function to discover both ServiceProfiles and
OutboundPolicies, PR #2265 will change this function to take an
arbitrary
Servicethat may discover ServiceProfiles, OutboundPolicies,or both. In order to make reviewing the change adding client policy
discovery easier, the change to
push_discoverwas factored out intothis PR.
Since
push_discovernow takes an arbitrary discovery service, andsince we only want the configured profile discovery allowlist to apply
to ServiceProfiles (and not to OutboundPolicies), this change also pulls
out the filtering of addresses not in the allowlist from a layer pushed
in
push_discoverinto a middleware which is added to the profilediscovery
Service.