feat(app/inbound): introduce Permitted<T> target
#4119
Merged
Conversation
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
this commit introduces a new target type to the inbound proxy's
policy enforcement middleware.
for context, this middleware is used by "lifting" a
`NewService<T, Service = S>` into an
`NewService<(HttpRoutePermit, T), Service = S>`,
where `S: svc::Service<http::Request<B>>`. this _lazily_ enforces
policy, allowing a connection to progress before eventually enforcing
authorization policy at the request-level, i.e. when a `S`-typed
`Service<Request<_>>` is yielded.
this is helpful in part because it allows us to implement generic
connection-level middleware across `T`-typed targets, while affixing
permits with information about the policy authorizing a request across
these connections.
this also allows us to inject policy enforcement in the stack before we
deal with routing traffic to `Logical` request targets.
in subsequent work, we intend to introduce `linkerd_http_prom` metrics
middleware to the inbound proxy at this level of the stack.
broadly, our middleware layers make use of our `Param<T>` and
`ExtractParam<P, T>` traits to abstract over field access from target
types. this is useful at the protocol- and connection-level layers of
our networking stack because it allows `NewService<T>` middleware like
`NewCountRequests` to be agnostic to its `T`-typed target, so long as it
(a) is provided an extractor to find a single time series from the
labeled metric family to increment given that `T`, and (b) wraps a
`NewService<T>` that yields a `Service<Request<B>`.
rust's type system does not support generic specialization, which means
that our `T: Param<U>`-shaped bounds do not play well with a `(_, T)`
tuple, which the outer layers of our inbound proxy's http router deals
with.
to facilitate the introduction of telemetry middleware wrapping our http
router, this commit introduces a `Permitted<T>` type to replace these
`(HttpRoutePermit, T)` tuples:
```rust
/// Describes an authorized `T`-typed target.
pub struct Permitted<T> {
pub permit: HttpRoutePermit,
pub target: T,
}
```
Signed-off-by: katelyn martin <[email protected]>
sfleen
approved these changes
Aug 29, 2025
Collaborator
sfleen
left a comment
sfleen
left a comment
There was a problem hiding this comment.
Awesome! This seems like a really clean solution to handling these type shenanigans, and honestly the fewer tuples we have in the service stack the better imo.
olix0r
approved these changes
Sep 2, 2025
cratelyn
added a commit
that referenced
this pull request
Sep 2, 2025
this is a follow-on to #4119. see #4119 (comment) for further background on this: > This is reaching unreadable levels of complexity imo. > > i'd look to trying to golf this down so that we don't have so many > wrapping lines, or otherwise extract this into a separate function so > it is, e.g. `push_filter(spawn_routes)` this commit performs that transformation, outlining our `push_filter(|| {})` into a standalone function rather than an inline closure. Signed-off-by: katelyn martin <[email protected]>
cratelyn
added a commit
that referenced
this pull request
Sep 3, 2025
this is a follow-on to #4119. see #4119 (comment) for further background on this: > This is reaching unreadable levels of complexity imo. > > i'd look to trying to golf this down so that we don't have so many > wrapping lines, or otherwise extract this into a separate function so > it is, e.g. `push_filter(spawn_routes)` this commit performs that transformation, outlining our `push_filter(|| {})` into a standalone function rather than an inline closure. Signed-off-by: katelyn martin <[email protected]>
cratelyn
added a commit
that referenced
this pull request
Sep 19, 2025
this commit makes a mechanical refactor in the interest of paving ground towards using the request body metrics middleware in the inbound proxy. because of minor differences in how we represent protocols like gRPC in the inbound and outbound proxies, we must account for: (a): distinct `NewService<T>`'s accepting `Grpc<T>` and `Http<T>` targets, respectively, each holding a single metrics `Family<L>`. (b): a single `NewService<T>` that matches on the `Permitted<T>` enum to select one of two metrics `Family<L>`s. see #4119 for more background on this `Permitted<T>` enum. so, we should make the `NewRecordBodyData` agnostic to its metrics, rather than holding a concrete `RequestBodyFamilies<L>`. this commit hoists the metrics out of the middleware, and into the `X` and `ReqX` extractors used in the `NewService<T>` and `Service<T>` layers, respectively. bear particular attention to this part of the diff: ```diff - ReqX: ExtractParam<L, Request<ReqB>>, - L: linkerd_metrics::prom::encoding::EncodeLabelSet - + std::fmt::Debug - + std::hash::Hash - + Eq - + Clone - + Send - + Sync - + 'static, + ReqX: ExtractParam<BodyDataMetrics, Request<ReqB>>, ``` in other words, rather than using `X` and subsequently `ReqX` to extract our labels `L`, we expect these extractors to yield a `BodyDataMetrics`: one time series that should be incremented accordingly by the instrumented request. in even simpler words: `ReqX` now calls `familes.metrics(&labels)` too. Signed-off-by: katelyn martin <[email protected]>
cratelyn
added a commit
that referenced
this pull request
Sep 22, 2025
this commit makes a mechanical refactor in the interest of paving ground towards using the request body metrics middleware in the inbound proxy. because of minor differences in how we represent protocols like gRPC in the inbound and outbound proxies, we must account for: (a): distinct `NewService<T>`'s accepting `Grpc<T>` and `Http<T>` targets, respectively, each holding a single metrics `Family<L>`. (b): a single `NewService<T>` that matches on the `Permitted<T>` enum to select one of two metrics `Family<L>`s. see #4119 for more background on this `Permitted<T>` enum. so, we should make the `NewRecordBodyData` agnostic to its metrics, rather than holding a concrete `RequestBodyFamilies<L>`. this commit hoists the metrics out of the middleware, and into the `X` and `ReqX` extractors used in the `NewService<T>` and `Service<T>` layers, respectively. bear particular attention to this part of the diff: ```diff - ReqX: ExtractParam<L, Request<ReqB>>, - L: linkerd_metrics::prom::encoding::EncodeLabelSet - + std::fmt::Debug - + std::hash::Hash - + Eq - + Clone - + Send - + Sync - + 'static, + ReqX: ExtractParam<BodyDataMetrics, Request<ReqB>>, ``` in other words, rather than using `X` and subsequently `ReqX` to extract our labels `L`, we expect these extractors to yield a `BodyDataMetrics`: one time series that should be incremented accordingly by the instrumented request. in even simpler words: `ReqX` now calls `familes.metrics(&labels)` too. Signed-off-by: katelyn martin <[email protected]>
cratelyn
added a commit
that referenced
this pull request
Sep 24, 2025
) #4119 introduced a `Permitted<T>` structure, to provide a more formal notion of a `(HttpRoutePermit, T)` tuple. during review of #4180, we had a discussion about how the inbound proxy's http router had metrics layers which were tightly coupled to the notion of a `Permitted<T>`: > This isn't a hard blocker--but this is unfortunate that we're coupling > this impl to the Permitted target type -- target types are usually an > _internal implementation detail_ and not a public api that is depended > on elsewhere. > > ... > > and then have our Permitted impl `Param<MetricsVariant> for Permitted<T>`. > > then this module remains decoupled from the concrete target types. > > That is, the metrics module is decoupled from _where it's called_--as > long as the caller provides a target that can give us what we need to > build our extractors, we're good to go. this branch performs a series of changes to decouple our metric layers' extractors from `Permitted<T>`. importantly, this has the effect of leaving the inbound metrics layers' extractors agnostic to their target: they now provide implementations like `svc::ExtractParam<BodyDataMetrics, T>` rather than `svc::ExtractParam<BodyDataMetrics, Permitted<T>>`. see #4180. --- * refactor(app/inbound): `Permitted<T>` is a struct this restructures `Permitted<T>` so that it is no longer an enum. we instead define a "dumb" enum that represents the grpc/http dichotomy, and return to a model in which `Permitted<T>` is a `struct`. Signed-off-by: katelyn martin <[email protected]> * refactor(app/inbound): `Permitted<T>` is not `T: Param<P>` in order to carve out space to allow us to define other `Param<P>` implementations for `Permitted<T>`, we remove this blanket deference to the inner `T` target. Signed-off-by: katelyn martin <[email protected]> * refactor(app/inbound): `Permitted<T>` exposes `Param<P>` impls we cannot expose `T` or any of its own parameters, but we can expose the permit, its variant, and its labels. Signed-off-by: katelyn martin <[email protected]> * refactor(app/inbound): `family` lookups accept `PermitVariant` these now accept specifically the variant, rather than the permitted target. Signed-off-by: katelyn martin <[email protected]> * refactor(app/inbound): `ExtractRecordBodyDataMetrics` is `T`-agnostic this type now can extract metrics from arbitrary targets. Signed-off-by: katelyn martin <[email protected]> * refactor(app/inbound): `ExtractRequestCount` is `T`-agnostic this type now can extract metrics from arbitrary targets. Signed-off-by: katelyn martin <[email protected]> * refactor(app/inbound): `LogicalPerRequest::from` is `T`-agnostic Signed-off-by: katelyn martin <[email protected]> --------- Signed-off-by: katelyn martin <[email protected]>
cratelyn
added a commit
that referenced
this pull request
Sep 24, 2025
this commit makes a mechanical refactor in the interest of paving ground towards using the request body metrics middleware in the inbound proxy. because of minor differences in how we represent protocols like gRPC in the inbound and outbound proxies, we must account for: (a): distinct `NewService<T>`'s accepting `Grpc<T>` and `Http<T>` targets, respectively, each holding a single metrics `Family<L>`. (b): a single `NewService<T>` that matches on the `Permitted<T>` enum to select one of two metrics `Family<L>`s. see #4119 for more background on this `Permitted<T>` enum. so, we should make the `NewRecordBodyData` agnostic to its metrics, rather than holding a concrete `RequestBodyFamilies<L>`. this commit hoists the metrics out of the middleware, and into the `X` and `ReqX` extractors used in the `NewService<T>` and `Service<T>` layers, respectively. bear particular attention to this part of the diff: ```diff - ReqX: ExtractParam<L, Request<ReqB>>, - L: linkerd_metrics::prom::encoding::EncodeLabelSet - + std::fmt::Debug - + std::hash::Hash - + Eq - + Clone - + Send - + Sync - + 'static, + ReqX: ExtractParam<BodyDataMetrics, Request<ReqB>>, ``` in other words, rather than using `X` and subsequently `ReqX` to extract our labels `L`, we expect these extractors to yield a `BodyDataMetrics`: one time series that should be incremented accordingly by the instrumented request. in even simpler words: `ReqX` now calls `familes.metrics(&labels)` too. Signed-off-by: katelyn martin <[email protected]>
cratelyn
added a commit
that referenced
this pull request
Sep 24, 2025
this commit makes a mechanical refactor in the interest of paving ground towards using the request body metrics middleware in the inbound proxy. because of minor differences in how we represent protocols like gRPC in the inbound and outbound proxies, we must account for: (a): distinct `NewService<T>`'s accepting `Grpc<T>` and `Http<T>` targets, respectively, each holding a single metrics `Family<L>`. (b): a single `NewService<T>` that matches on the `Permitted<T>` enum to select one of two metrics `Family<L>`s. see #4119 for more background on this `Permitted<T>` enum. so, we should make the `NewRecordBodyData` agnostic to its metrics, rather than holding a concrete `RequestBodyFamilies<L>`. this commit hoists the metrics out of the middleware, and into the `X` and `ReqX` extractors used in the `NewService<T>` and `Service<T>` layers, respectively. bear particular attention to this part of the diff: ```diff - ReqX: ExtractParam<L, Request<ReqB>>, - L: linkerd_metrics::prom::encoding::EncodeLabelSet - + std::fmt::Debug - + std::hash::Hash - + Eq - + Clone - + Send - + Sync - + 'static, + ReqX: ExtractParam<BodyDataMetrics, Request<ReqB>>, ``` in other words, rather than using `X` and subsequently `ReqX` to extract our labels `L`, we expect these extractors to yield a `BodyDataMetrics`: one time series that should be incremented accordingly by the instrumented request. in even simpler words: `ReqX` now calls `familes.metrics(&labels)` too. Signed-off-by: katelyn martin <[email protected]>
cratelyn
added a commit
that referenced
this pull request
Sep 24, 2025
this commit makes a mechanical refactor in the interest of paving ground towards using the request body metrics middleware in the inbound proxy. because of minor differences in how we represent protocols like gRPC in the inbound and outbound proxies, we must account for: (a): distinct `NewService<T>`'s accepting `Grpc<T>` and `Http<T>` targets, respectively, each holding a single metrics `Family<L>`. (b): a single `NewService<T>` that matches on the `Permitted<T>` enum to select one of two metrics `Family<L>`s. see #4119 for more background on this `Permitted<T>` enum. so, we should make the `NewRecordBodyData` agnostic to its metrics, rather than holding a concrete `RequestBodyFamilies<L>`. this commit hoists the metrics out of the middleware, and into the `X` and `ReqX` extractors used in the `NewService<T>` and `Service<T>` layers, respectively. bear particular attention to this part of the diff: ```diff - ReqX: ExtractParam<L, Request<ReqB>>, - L: linkerd_metrics::prom::encoding::EncodeLabelSet - + std::fmt::Debug - + std::hash::Hash - + Eq - + Clone - + Send - + Sync - + 'static, + ReqX: ExtractParam<BodyDataMetrics, Request<ReqB>>, ``` in other words, rather than using `X` and subsequently `ReqX` to extract our labels `L`, we expect these extractors to yield a `BodyDataMetrics`: one time series that should be incremented accordingly by the instrumented request. in even simpler words: `ReqX` now calls `familes.metrics(&labels)` too. Signed-off-by: katelyn martin <[email protected]>
cratelyn
added a commit
that referenced
this pull request
Sep 24, 2025
…#4174) this commit makes a mechanical refactor in the interest of paving ground towards using the request body metrics middleware in the inbound proxy. because of minor differences in how we represent protocols like gRPC in the inbound and outbound proxies, we must account for: (a): distinct `NewService<T>`'s accepting `Grpc<T>` and `Http<T>` targets, respectively, each holding a single metrics `Family<L>`. (b): a single `NewService<T>` that matches on the `Permitted<T>` enum to select one of two metrics `Family<L>`s. see #4119 for more background on this `Permitted<T>` enum. so, we should make the `NewRecordBodyData` agnostic to its metrics, rather than holding a concrete `RequestBodyFamilies<L>`. this commit hoists the metrics out of the middleware, and into the `X` and `ReqX` extractors used in the `NewService<T>` and `Service<T>` layers, respectively. bear particular attention to this part of the diff: ```diff - ReqX: ExtractParam<L, Request<ReqB>>, - L: linkerd_metrics::prom::encoding::EncodeLabelSet - + std::fmt::Debug - + std::hash::Hash - + Eq - + Clone - + Send - + Sync - + 'static, + ReqX: ExtractParam<BodyDataMetrics, Request<ReqB>>, ``` in other words, rather than using `X` and subsequently `ReqX` to extract our labels `L`, we expect these extractors to yield a `BodyDataMetrics`: one time series that should be incremented accordingly by the instrumented request. in even simpler words: `ReqX` now calls `familes.metrics(&labels)` too. Signed-off-by: katelyn martin <[email protected]>
cratelyn
added a commit
that referenced
this pull request
Dec 1, 2025
in #4298, we introduced a new metrics telemetry layer that can measure and report status codes, in a protocol-agnostic fashion. this commit integrates this status code telemtry into the inbound proxy, so that HTTP and gRPC traffic can be observed. a new family of metrics is introduced to the `InboundMetrics` structure, and the inbound http\* router's metrics layer is accordingly updated to thread this metrics family into an extractor, which is in turn provided to the `NewRecordStatusCode` layer. \* as a note for reviewers, the inbound proxy does not model the http and grpc protocols as distinct concepts in the network stack's type system, unlike the outbound proxy. this means that while target types in the outbound proxy like `Http<()>` are specific to HTTP, the inbound proxy's distinction of HTTP/gRPC is determined by obtaining and inspecting the `PermitVariant`. #### 🔗 related some previous pull requests related to this change: * #4298 * #4180 * #4203 * #4127 * #4119 Signed-off-by: katelyn martin <[email protected]>
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.
this commit introduces a new target type to the inbound proxy's
policy enforcement middleware.
for context, this middleware is used by "lifting" a
NewService<T, Service = S>into anNewService<(HttpRoutePermit, T), Service = S>,where
S: svc::Service<http::Request<B>>. this lazily enforcespolicy, allowing a connection to progress before eventually enforcing
authorization policy at the request-level, i.e. when a
S-typedService<Request<_>>is yielded.this is helpful in part because it allows us to implement generic
connection-level middleware across
T-typed targets, while affixingpermits with information about the policy authorizing a request across
these connections.
this also allows us to inject policy enforcement in the stack before we
deal with routing traffic to
Logicalrequest targets.in subsequent work, we intend to introduce
linkerd_http_prommetricsmiddleware to the inbound proxy at this level of the stack.
broadly, our middleware layers make use of our
Param<T>andExtractParam<P, T>traits to abstract over field access from targettypes. this is useful at the protocol- and connection-level layers of
our networking stack because it allows
NewService<T>middleware likeNewCountRequeststo be agnostic to itsT-typed target, so long as it(a) is provided an extractor to find a single time series from the
labeled metric family to increment given that
T, and (b) wraps aNewService<T>that yields aService<Request<B>.rust's type system does not support generic specialization, which means
that our
T: Param<U>-shaped bounds do not play well with a(_, T)tuple, which the outer layers of our inbound proxy's http router deals
with.
to facilitate the introduction of telemetry middleware wrapping our http
router, this commit introduces a
Permitted<T>type to replace these(HttpRoutePermit, T)tuples:Signed-off-by: katelyn martin [email protected]