Conversation
Currently, the proxy integration tests run without a policy controller, and instead use environment variables to configure default inbound server policies. However, we do not intend to add a similar environment based defaulting mechanism for outbound client policies. Therefore, when the proxy starts consuming the outbound client policy API, we will no longer be able to run the integration tests without a policy controller. This branch adds an implementation of a mock policy controller for the integration tests. This was factored out from PR #2265, which implements outbound policy discovery and therefore requires a policy controller. Note that the mock policy controller added in #2265 serves both the inbound and outbound APIs, while this PR implements only the inbound API. This is because the a version of `linkerd2-proxy-api` that includes the outbound policy API has not yet been released, and I wanted to be able to merge this branch without waiting for a proxy API release. However, the mock policy controller implementation here was taken directly from #2265 with the outbound half removed. Some aspects of the implementation may seem somewhat more complex than necessary to implement only the inbound policy API, but I thought it was nicer to have a smaller diff when the outbound half is added in the near future. One small complexity here is that the inbound proxy will use default policies when it has not yet resolved. This means there is a potential for racy behavior in tests when a request is sent to an inbound proxy, if it has not yet started a watch on the policy for a port (or is in the process of starting that watch). This can result in flaky test failures: for example, in metrics tests, the name of the policy can change between requests if one request is handled with the default and the next request is handled with one resolved from the mock policy controller, resulting in two sets of labels rather than one. To avoid the test proxy flapping between the default policy and one resolved from the mock policy controller, I've also changed the proxy test harness so that any configured inbound ports in the test are added to `LINKERD2_PROXY_INBOUND_PORTS`, so that the proxy will eagerly resolve them. The test harness now waits for the mock policy controller to tell it that the proxy has started watches on those ports before actually running the test code.
olix0r
reviewed
Mar 7, 2023
olix0r
approved these changes
Mar 7, 2023
hawkw
added a commit
that referenced
this pull request
Mar 9, 2023
Currently, configuration for the policy controller client is part of the `linkerd_app_inbound` crate's `Config` struct. This is because only the inbound proxy currently communicates with the policy controller. In order to change the outbound side of the proxy to resolve client policies from the new `OutboundPolicy` API, the outbound proxy will now also need a policy controller client. Preferrably, both the client configuration and the actual client connection(s) should be shared between both halves of the proxy, similar to how the destination controller client is shared by both the inbound and outbound proxies. Therefore, this branch pulls the shared policy client configuration out of the inbound config and onto the top-level `linkerd_app` crate's `Config` struct. The shape of the `Inbound::build_policies` function is changed somewhat, as it now takes a client service and wraps it with the gRPC API client for the inbound policy API. This looks a bit different from how the destination controller client is shared between the inbound and outbound proxies, because in this case, the two halves of the proxy are using different gRPC APIs served by the same controller, rather than the same API. Additionally, the policy controller address and name environment variables are now required, and starting the proxy without them is a hard error. In practice, these are always set, and running without a policy controller was only used in proxy integration tests. PR #2288 changed the integration tests to run with a mock policy controller, so running the proxy without a policy controller is no longer necessary. Removing this mode simplifies the config-parsing code somewhat. This change was factored out from PR #2265.
olix0r
pushed a commit
that referenced
this pull request
Mar 9, 2023
Currently, configuration for the policy controller client is part of the `linkerd_app_inbound` crate's `Config` struct. This is because only the inbound proxy currently communicates with the policy controller. In order to change the outbound side of the proxy to resolve client policies from the new `OutboundPolicy` API, the outbound proxy will now also need a policy controller client. Preferrably, both the client configuration and the actual client connection(s) should be shared between both halves of the proxy, similar to how the destination controller client is shared by both the inbound and outbound proxies. Therefore, this branch pulls the shared policy client configuration out of the inbound config and onto the top-level `linkerd_app` crate's `Config` struct. The shape of the `Inbound::build_policies` function is changed somewhat, as it now takes a client service and wraps it with the gRPC API client for the inbound policy API. This looks a bit different from how the destination controller client is shared between the inbound and outbound proxies, because in this case, the two halves of the proxy are using different gRPC APIs served by the same controller, rather than the same API. Additionally, the policy controller address and name environment variables are now required, and starting the proxy without them is a hard error. In practice, these are always set, and running without a policy controller was only used in proxy integration tests. PR #2288 changed the integration tests to run with a mock policy controller, so running the proxy without a policy controller is no longer necessary. Removing this mode simplifies the config-parsing code somewhat. This change was factored out from PR #2265.
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 proxy integration tests run without a policy controller, and instead use environment variables to configure default inbound server policies. However, we do not intend to add a similar environment based defaulting mechanism for outbound client policies. Therefore, when the proxy starts consuming the outbound client policy API, we will no longer be able to run the integration tests without a policy controller.
This branch adds an implementation of a mock policy controller for the integration tests. This was factored out from PR #2265, which implements outbound policy discovery and therefore requires a policy controller.
Note that the mock policy controller added in #2265 serves both the inbound and outbound APIs, while this PR implements only the inbound API. This is because the a version of
linkerd2-proxy-apithat includes the outbound policy API has not yet been released, and I wanted to be able to merge this branch without waiting for a proxy API release. However, the mock policy controller implementation here was taken directly from #2265 with the outbound half removed. Some aspects of the implementation may seem somewhat more complex than necessary to implement only the inbound policy API, but I thought it was nicer to have a smaller diff when the outbound half is added in the near future.One small complexity here is that the inbound proxy will use default policies when it has not yet resolved. This means there is a potential for racy behavior in tests when a request is sent to an inbound proxy, if it has not yet started a watch on the policy for a port (or is in the process of starting that watch). This can result in flaky test failures: for example, in metrics tests, the name of the policy can change between requests if one request is handled with the default and the next request is handled with one resolved from the mock policy controller, resulting in two sets of labels rather than one. To avoid the test proxy flapping between the default policy and one resolved from the mock policy controller, I've also changed the proxy test harness so that any configured inbound ports in the test are added to
LINKERD2_PROXY_INBOUND_PORTS, so that the proxy will eagerly resolve them. The test harness now waits for the mock policy controller to tell it that the proxy has started watches on those ports before actually running the test code.