-
Notifications
You must be signed in to change notification settings - Fork 271
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: refactor newtypes for policy addresses #2264
Merged
Merged
Conversation
This file contains 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
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)
b6ab13d
to
a18e924
Compare
olix0r
approved these changes
Feb 24, 2023
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
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.
Depends on #2186.
Currently, the
linkerd-app-inbound
crate'sGetPolicy
trait performs lookups forOrigDstAddr
s. This is not strictly correct: theOrigDstAddr
newtype should specifically represent an address that was returned by agetsockopt(..., SO_ORIGINAL_DST, ...)
call, and in the case of the inbound direct stack, the "original dst address" passed toGetPolicy
may actually be synthesized from a port override in theTransportHeader
. Therefore, it would be nicer if we had aLookupAddr
type specifically representing an address used as an argument for policy lookups, analogous to theprofiles::LookupAddr
newtype used on the outbound side. See this comment for details.Therefore, this branch makes the following changes:
ServerPolicy
,AllowPermit
, etc) which store addresses represent those addresses asServerAddr
rather thanOrigDstAddr
, and perform address matching onServerAddr
s.inbound::policy
module now defines aLookupAddr
newtype, which is passed as an argument toGetPolicy
.ExtractParam
impls to thepolicy::Discover
layer indicating howLookupAddr
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.