Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inbound: remove default policies #2204

Merged
merged 19 commits into from
Feb 24, 2023
Merged

Conversation

hawkw
Copy link
Member

@hawkw hawkw commented Feb 1, 2023

Motivation

Currently, the inbound proxy has a notion of default ServerPolicies,
which are configured through an environment variable which selects the
default policy, and other environment variables that set properties for
lists of ports (requiring TLS, requiring mesh identity, and skipping
protocol detection). These are used in two cases:

  • When the proxy is configured without a policy controller address. In
    this case, the default policy is always used. In practice, this only
    occurs in the proxy's own integration tests.
  • When the proxy is configured with a policy controller, default
    policies are used when a ServerPolicy has yet to be resolved from the
    control plane for a specific port.

The default ServerPolicies, and the logic for determining the default
policy for a given port based on multiple environment variables,
introduces complexity into the proxy that make its behavior harder to
reason about. In addition, the proxy injector must currently inject the
proxy with an environment variable containing every opaque port, which
can cause issues when large port ranges are marked as opaque. See
linkerd/linkerd2#9803 for details.

Solution

In order to simplify the proxy's configuration surface area, this branch
removes the default policy configuration.

PR #2186 changed the inbound proxy to wait for policies to be resolved
before handling connections on those ports, rather than using the
default policy until a result is returned by the policy controller, so
the default policy can be removed in that case.

In addition, this branch removes the mode of operation where no policy
controller address is configured, removing the other case where default
policies are used. The policy-controller-less mode is only used for the
proxy's integration tests, and maintaining it introduces additional
complexity. Since the policy controller is now required in all cases, it
was necessary to add a mock policy controller implementation for use in
integration tests which previously relied on default policies.

The LINKERD2_PROXY_INBOUND_PORTS environment variable, which
configures the set of ports to eagerly look up policies for, and which
are never evicted from the cache once looked up, is still honored as it
was previously.

Implications

Critically, this change now means that the policy controller is now
required for the inbound proxy to operate. The proxy will not evict
the pre-configured list of inbound ports from its cache, so connections
to those ports will still function if communication to the policy
controller is interrupted. However, ports not in
LINKERD2_PROXY_INBOUND_PORTS may be evicted from the cache if they
exceed the idle timeout, and subsequent connections on those ports will
fail if the policy controller cannot be reached. Furthermore, the proxy
will need to communicate with the policy controller on startup to
populate the pre-configured list of inbound ports.

This PR will allow changing the proxy injector to avoid injecting
proxies with a list of every opaque port, since opaque ports are now
always configured by the policy controller, which can handle port
ranges, and the proxy no longer needs to be aware of every individual
port in a range until it actually receives a connection on that port.

Depends on #2186
Closes #2076
Closes #2079
Closes #2080

@hawkw hawkw marked this pull request as ready for review February 1, 2023 23:32
@hawkw hawkw requested a review from a team as a code owner February 1, 2023 23:32
@hawkw hawkw requested a review from olix0r February 1, 2023 23:32
@hawkw hawkw force-pushed the eliza/actually-no-default-policy branch from b725fea to c841a05 Compare February 22, 2023 19:30
@hawkw hawkw force-pushed the eliza/actually-no-default-policy branch from c841a05 to 27c7424 Compare February 23, 2023 21:40
@hawkw
Copy link
Member Author

hawkw commented Feb 23, 2023

Rebased now that #2186 has merged

`Store::for_test` is stashed in the tcp test, so it's awkward to
reference its type. This change adds a new `test_util`
`Store` currently depends on tonic grpc message types, so it's awkward
to mock it for testing.

This change relaxes the type constraints on `Store` so we can provide an
aribitrary service that provides a `ServerPolicy` watch.
@olix0r olix0r self-assigned this Feb 24, 2023
@olix0r olix0r enabled auto-merge (squash) February 24, 2023 02:11
@olix0r olix0r merged commit 93b06e6 into main Feb 24, 2023
@olix0r olix0r deleted the eliza/actually-no-default-policy branch February 24, 2023 02:11
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Feb 24, 2023
This release updates the proxy to require control-plane-discovered
policies for all inbound connections. Previously, the proxy would
use a locally-configured default policy while it synced policies from
the control plane.

---

* trace: never log access log spans to stdout (linkerd/linkerd2-proxy#2244)
* build(deps): bump thread_local from 1.1.4 to 1.1.7 (linkerd/linkerd2-proxy#2243)
* build(deps): bump tj-actions/changed-files from 35.5.4 to 35.5.5 (linkerd/linkerd2-proxy#2242)
* build(deps): bump serde_json from 1.0.91 to 1.0.93 (linkerd/linkerd2-proxy#2217)
* dev: Remove docker config mount (linkerd/linkerd2-proxy#2246)
* build(deps): bump anyhow from 1.0.68 to 1.0.69 (linkerd/linkerd2-proxy#2216)
* build(deps): bump actions/checkout from 3.2.0 to 3.3.0 (linkerd/linkerd2-proxy#2130)
* http: Use `ExtractParam` in `NewNormalizeUri` (linkerd/linkerd2-proxy#2245)
* http: Replace `classify::CanClassify` with `Param` (linkerd/linkerd2-proxy#2247)
* http: Improve timeout module (linkerd/linkerd2-proxy#2248)
* outbound: Decouple outbound HTTP server from logical target (linkerd/linkerd2-proxy#2249)
* build(deps): bump http from 0.2.8 to 0.2.9 (linkerd/linkerd2-proxy#2253)
* build(deps): bump fastrand from 1.8.0 to 1.9.0 (linkerd/linkerd2-proxy#2252)
* build(deps): bump signal-hook-registry from 1.4.0 to 1.4.1 (linkerd/linkerd2-proxy#2254)
* Use `ExtractParam` in `http::NewHeaderFromTarget` (linkerd/linkerd2-proxy#2255)
* outbound: Introduce a new outbound-route watch type (linkerd/linkerd2-proxy#2250)
* build(deps): bump tj-actions/changed-files from 35.5.5 to 35.5.6 (linkerd/linkerd2-proxy#2256)
* build(deps): bump hyper from 0.14.23 to 0.14.24 (linkerd/linkerd2-proxy#2257)
* build(deps): bump once_cell from 1.17.0 to 1.17.1 (linkerd/linkerd2-proxy#2258)
* build(deps): bump clang-sys from 1.4.0 to 1.6.0 (linkerd/linkerd2-proxy#2259)
* outbound: Add a policy router (linkerd/linkerd2-proxy#2251)
* inbound: connections wait for ServerPolicy discovery (linkerd/linkerd2-proxy#2186)
* inbound: Remove default policies (linkerd/linkerd2-proxy#2204)
* inbound: Introduce a `policy::LookupAddr` type (linkerd/linkerd2-proxy#2264)
* build(deps): bump futures from 0.3.25 to 0.3.26 (linkerd/linkerd2-proxy#2263)
* build(deps): bump proc-macro2 from 1.0.50 to 1.0.51 (linkerd/linkerd2-proxy#2261)
* build(deps): bump tinyvec_macros from 0.1.0 to 0.1.1 (linkerd/linkerd2-proxy#2262)

Signed-off-by: Oliver Gould <ver@buoyant.io>
olix0r added a commit that referenced this pull request Feb 25, 2023
The changes--specifically those in 93b06e6--prevent control plane
boot-strapping: the identity controller is unable to serve requests
because its proxy can't contact the destination pod for policy because
the destination pod doesn't have identity yet.

Ultimately, we probably want to change Linkerd's control plane
deployment topology to avoid these bootstrapping issues. In the
meantime, we'll need to revisit our approach to these changes.

* Revert 89ee318 inbound: Introduce a `policy::LookupAddr` type (#2264)
* Revert 93b06e6 inbound: Remove default policies (#2204)
* Revert c186d88 inbound: connections wait for ServerPolicy discovery (#2186)
olix0r added a commit that referenced this pull request Feb 25, 2023
The changes--specifically those in 93b06e6--prevent control plane
boot-strapping: the identity controller is unable to serve requests
because its proxy can't contact the destination pod for policy because
the destination pod doesn't have identity yet.

Ultimately, we probably want to change Linkerd's control plane
deployment topology to avoid these bootstrapping issues. In the
meantime, we'll need to revisit our approach to these changes.

* Revert 89ee318 inbound: Introduce a `policy::LookupAddr` type (#2264)
* Revert 93b06e6 inbound: Remove default policies (#2204)
* Revert c186d88 inbound: connections wait for ServerPolicy discovery (#2186)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Mar 1, 2023
* proxy: v2.191.0

This release updates the proxy to require control-plane-discovered
policies for all inbound connections. Previously, the proxy would
use a locally-configured default policy while it synced policies from
the control plane.

---

* trace: never log access log spans to stdout (linkerd/linkerd2-proxy#2244)
* build(deps): bump thread_local from 1.1.4 to 1.1.7 (linkerd/linkerd2-proxy#2243)
* build(deps): bump tj-actions/changed-files from 35.5.4 to 35.5.5 (linkerd/linkerd2-proxy#2242)
* build(deps): bump serde_json from 1.0.91 to 1.0.93 (linkerd/linkerd2-proxy#2217)
* dev: Remove docker config mount (linkerd/linkerd2-proxy#2246)
* build(deps): bump anyhow from 1.0.68 to 1.0.69 (linkerd/linkerd2-proxy#2216)
* build(deps): bump actions/checkout from 3.2.0 to 3.3.0 (linkerd/linkerd2-proxy#2130)
* http: Use `ExtractParam` in `NewNormalizeUri` (linkerd/linkerd2-proxy#2245)
* http: Replace `classify::CanClassify` with `Param` (linkerd/linkerd2-proxy#2247)
* http: Improve timeout module (linkerd/linkerd2-proxy#2248)
* outbound: Decouple outbound HTTP server from logical target (linkerd/linkerd2-proxy#2249)
* build(deps): bump http from 0.2.8 to 0.2.9 (linkerd/linkerd2-proxy#2253)
* build(deps): bump fastrand from 1.8.0 to 1.9.0 (linkerd/linkerd2-proxy#2252)
* build(deps): bump signal-hook-registry from 1.4.0 to 1.4.1 (linkerd/linkerd2-proxy#2254)
* Use `ExtractParam` in `http::NewHeaderFromTarget` (linkerd/linkerd2-proxy#2255)
* outbound: Introduce a new outbound-route watch type (linkerd/linkerd2-proxy#2250)
* build(deps): bump tj-actions/changed-files from 35.5.5 to 35.5.6 (linkerd/linkerd2-proxy#2256)
* build(deps): bump hyper from 0.14.23 to 0.14.24 (linkerd/linkerd2-proxy#2257)
* build(deps): bump once_cell from 1.17.0 to 1.17.1 (linkerd/linkerd2-proxy#2258)
* build(deps): bump clang-sys from 1.4.0 to 1.6.0 (linkerd/linkerd2-proxy#2259)
* outbound: Add a policy router (linkerd/linkerd2-proxy#2251)
* inbound: connections wait for ServerPolicy discovery (linkerd/linkerd2-proxy#2186)
* inbound: Remove default policies (linkerd/linkerd2-proxy#2204)
* inbound: Introduce a `policy::LookupAddr` type (linkerd/linkerd2-proxy#2264)
* build(deps): bump futures from 0.3.25 to 0.3.26 (linkerd/linkerd2-proxy#2263)
* build(deps): bump proc-macro2 from 1.0.50 to 1.0.51 (linkerd/linkerd2-proxy#2261)
* build(deps): bump tinyvec_macros from 0.1.0 to 0.1.1 (linkerd/linkerd2-proxy#2262)

Signed-off-by: Oliver Gould <ver@buoyant.io>

* proxy: v2.191.2

Revert inbound policy discovery behavior changes (from v2.191.0) and
update dependencies, especially h2.

---

* Revert inbound policy discovery changes (linkerd/linkerd2-proxy#2267)
* Bump v38 to v39 (linkerd/linkerd2-proxy#2269)
* dev: override CXX for rust-analyzer (linkerd/linkerd2-proxy#2270)
* build(deps): bump syn from 1.0.107 to 1.0.109 (linkerd/linkerd2-proxy#2274)
* build(deps): bump tokio-util from 0.7.4 to 0.7.7 (linkerd/linkerd2-proxy#2272)
* build(deps): bump tj-actions/changed-files from 35.5.6 to 35.6.0 (linkerd/linkerd2-proxy#2271)
* build(deps): bump prost-build from 0.11.6 to 0.11.8 (linkerd/linkerd2-proxy#2273)
* ci: Add a linkerd install workflow (linkerd/linkerd2-proxy#2268)
* allow `opaque_hidden_inferred_bound` warning on nightly (linkerd/linkerd2-proxy#2275)
* build(deps): bump h2 from 0.3.15 to 0.3.16 (linkerd/linkerd2-proxy#2278)
* build(deps): bump tempfile from 3.3.0 to 3.4.0 (linkerd/linkerd2-proxy#2277)
* build(deps): bump tokio-stream from 0.1.11 to 0.1.12 (linkerd/linkerd2-proxy#2276)
* stack: Make `failfast::Gate` general purpose (linkerd/linkerd2-proxy#2279)
* build(deps): bump slab from 0.4.7 to 0.4.8 (linkerd/linkerd2-proxy#2283)
* build(deps): bump async-stream from 0.3.3 to 0.3.4 (linkerd/linkerd2-proxy#2282)
* build(deps): bump jobserver from 0.1.25 to 0.1.26 (linkerd/linkerd2-proxy#2281)
* build(deps): bump tj-actions/changed-files from 35.6.0 to 35.6.1 (linkerd/linkerd2-proxy#2280)

Signed-off-by: Oliver Gould <ver@buoyant.io>

---------

Signed-off-by: Oliver Gould <ver@buoyant.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants