Skip to content

Conversation

hawkw
Copy link
Contributor

@hawkw hawkw commented Feb 23, 2023

Depends on #2186.

Currently, the linkerd-app-inbound crate's GetPolicy trait performs lookups for OrigDstAddrs. This is not strictly correct: the OrigDstAddr newtype should specifically represent an address that was returned by a getsockopt(..., SO_ORIGINAL_DST, ...) call, and in the case of the inbound direct stack, the "original dst address" passed to GetPolicy may actually be synthesized from a port override in the TransportHeader. Therefore, it would be nicer if we had a LookupAddr type specifically representing an address used as an argument for policy lookups, analogous to the profiles::LookupAddr newtype used on the outbound side. See this comment for details.

Therefore, this branch makes the following changes:

  • Policy types (e.g. ServerPolicy, AllowPermit, etc) which store addresses represent those addresses as ServerAddr rather than OrigDstAddr, and perform address matching on ServerAddrs.
  • The inbound::policy module now defines a LookupAddr newtype, which is passed as an argument to GetPolicy.
  • The inbound accept and direct stacks provide ExtractParam impls to the policy::Discover layer indicating how LookupAddrs are produced from targets in that stack. This makes the different behaviors between the normal accept stack and the direct stack more explicit at the callsite.
@hawkw hawkw requested a review from olix0r February 23, 2023 18:53
@hawkw hawkw requested a review from a team as a code owner February 23, 2023 18:53
Base automatically changed from eliza/no-default-policy to main February 23, 2023 21:32
Depends on #2186. Currently, the `linkerd-app-inbound` crate's `GetPolicy` trait performs lookups for `OrigDstAddr`s. This is not strictly correct: the `OrigDstAddr` newtype should specifically represent an address that was returned by a `getsockopt(..., SO_ORIGINAL_DST, ...)` call, and in the case of the inbound direct stack, the "original dst address" passed to `GetPolicy` may actually be synthesized from a port override in the `TransportHeader`. Therefore, it would be nicer if we had a `LookupAddr` type specifically representing an address used as an argument for policy lookups, analogous to the `profiles::LookupAddr` newtype used on the outbound side. See [this comment][1] for details. Therefore, this branch makes the following changes: - Policy types (e.g. `ServerPolicy`, `AllowPermit`, etc) which store addresses represent those addresses as `ServerAddr` rather than `OrigDstAddr`, and perform address matching on `ServerAddr`s. - The `inbound::policy` module now defines a `LookupAddr` newtype, which is passed as an argument to `GetPolicy`. - The inbound accept and direct stacks provide `ExtractParam` impls to the `policy::Discover` layer indicating how `LookupAddr`s are produced from targets in that stack. This makes the different behaviors between the normal accept stack and the direct stack more explicit at the callsite. [1]: #2186 (comment)
@hawkw hawkw force-pushed the eliza/policy-lookupaddr branch from b6ab13d to a18e924 Compare February 23, 2023 21:50
@olix0r olix0r enabled auto-merge (squash) February 24, 2023 16:01
@olix0r olix0r merged commit 89ee318 into main Feb 24, 2023
@olix0r olix0r deleted the eliza/policy-lookupaddr branch February 24, 2023 16:03
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

2 participants