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

stack: Introduce ExtractParam and InsertParam #1169

Merged
merged 1 commit into from
Jul 28, 2021
Merged

Conversation

olix0r
Copy link
Member

@olix0r olix0r commented Jul 27, 2021

Stack modules that take configuration take one of two approaches: they
either use the Param trait to extract a configuration from the target
or they take configuration at construction-time. For example, the TLS
server module takes its detection timeout as an argument when
constructing its layer. But we want to modify the inbound stack to
configure detection timeouts dynamically, as informed by discovery; so
it will no longer be appropriate to take this configuration when
constructing the layer.

This change introduces the stack::ExtactParam trait which lets us
provide a strategy to the TLS server layer. For now, strategy
implementations ignore the target type, continuing to use a fixed
timeout set during stack construction; but in an upcoming change we'll
be able to modify some of these stacks to use the target to configure
this value.

The ExtractParam trait decouples this decision from the stack module:
The TLS server module no longer cares whether the timeout is configured
statically or dynamically.

Furthermore, some stack modules may want to insert new paramters into
the target. For instance, the TLS server inserts the connections TLS
status into the target. We've generally managed this with tuples, but
this leads to some britleness around the shape of the target.

The InsertParam trait allows stacks to construct new target types
other than tuples, though the existing implementations continue to
construct tuples as before. This will support more complicated target
type construction.

This change updates the TLS server to use these new traits without
altering its prior functionality. While this incurs some param boilerplate,
this sets up followup changes that will use this flexbility to configure the
inbound stack based on per-port policies.

Stack modules that take configuration take one of two approaches: they
either use the `Param` trait to extract a configuration from the target
or they take configuration at construction-time. For example, the TLS
server module takes its detection timeout as an argument when
constructing its layer. But we want to modify the inbound stack to
configure detection timeouts dynamically, as informed by discovery; so
it will no longer be appropriate to take this configuration when
constructing the layer.

This change introduces the `stack::ExtactParam` trait which lets us
provide a _strategy_ to the TLS server layer. For now, strategy
implementations ignore the target type, continuing to use a fixed
timeout set during stack construction; but in an upcoming change we'll
be able to modify some of these stacks to use the target to configure
this value.

The `ExtractParam` trait decouples this decision from the stack module:
The TLS server module no longer cares whether the timeout is configured
statically or dynamically.

Furthermore, some stack modules may want to insert new paramters into
the target. For instance, the TLS server inserts the connections TLS
status into the target. We've generally managed this with tuples, but
this leads to some britleness around the shape of the target.

The `InsertParam` trait allows stacks to construct new target types
other than tuples, though the existing implementations continue to
construct tuples as before. This will support more complicated target
type construction.

This change updates the TLS server to use these new traits without
altering its prior functionality. This sets up followup changes that
will use this flexbility to configure the inbound stack based on
per-port policies.
@olix0r olix0r requested a review from a team July 27, 2021 21:00
@codecov-commenter
Copy link

Codecov Report

Merging #1169 (843d948) into main (3b097f3) will decrease coverage by 0.02%.
The diff coverage is 76.11%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1169      +/-   ##
==========================================
- Coverage   75.13%   75.11%   -0.03%     
==========================================
  Files         235      235              
  Lines       15055    15095      +40     
==========================================
+ Hits        11311    11338      +27     
- Misses       3744     3757      +13     
Impacted Files Coverage Δ
linkerd/app/core/src/svc.rs 75.58% <ø> (ø)
linkerd/detect/src/lib.rs 76.59% <0.00%> (ø)
linkerd/proxy/tap/src/accept.rs 2.85% <ø> (ø)
linkerd/stack/src/lib.rs 20.00% <0.00%> (-80.00%) ⬇️
linkerd/app/src/tap.rs 60.00% <25.00%> (-12.23%) ⬇️
linkerd/app/admin/src/stack.rs 63.33% <88.88%> (+4.84%) ⬆️
linkerd/app/core/src/metrics/tcp_accept_errors.rs 96.42% <100.00%> (ø)
linkerd/app/inbound/src/direct.rs 37.07% <100.00%> (+5.75%) ⬆️
linkerd/app/inbound/src/lib.rs 88.98% <100.00%> (+1.48%) ⬆️
linkerd/app/inbound/src/target.rs 63.79% <100.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b097f3...843d948. Read the comment docs.

@olix0r
Copy link
Member Author

olix0r commented Jul 28, 2021

fuzzer compilation failures are due to rust-lang/rust#87455

Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the thorough description this helped the review. Changes look good and makes sense.

@olix0r olix0r merged commit 6f7968f into main Jul 28, 2021
@olix0r olix0r deleted the ver/extract-param branch July 28, 2021 18:40
olix0r added a commit that referenced this pull request Jul 30, 2021
Following #1169, this change modifies the `detect` middleware to use
`ExtractParam`. In doing this, it makes sense to modify the default
implementations of `ExtractParam` so that we can provide a static
configuration to the module that is cloned for all targets without
modification.

This change does not alter any functionality, but sets up to make the
HTTP detection layer configured per-target.
olix0r added a commit that referenced this pull request Aug 2, 2021
Following #1169, this change modifies the `detect` middleware to use
`ExtractParam`. In doing this, it makes sense to modify the default
implementations of `ExtractParam` so that we can provide a static
configuration to the module that is cloned for all targets without
modification.

This change does not alter any functionality, but sets up to make the
HTTP detection layer configured per-target.
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Aug 5, 2021
This release includes only dependency updates an internal changes to
support upcoming policy features. No user-facing changes are expected.

---

* build(deps): bump tokio from 1.8.2 to 1.9.0 (linkerd/linkerd2-proxy#1162)
* build(deps): bump hyper from 0.14.10 to 0.14.11 (linkerd/linkerd2-proxy#1163)
* build(deps): bump codecov/codecov-action from 2.0.1 to 2.0.2 (linkerd/linkerd2-proxy#1165)
* build(deps): bump futures from 0.3.15 to 0.3.16 (linkerd/linkerd2-proxy#1166)
* build(deps): bump softprops/action-gh-release (linkerd/linkerd2-proxy#1167)
* build(deps): bump softprops/action-gh-release from 0.1.6 to 1 (linkerd/linkerd2-proxy#1168)
* stack: Introduce ExtractParam and InsertParam (linkerd/linkerd2-proxy#1169)
* inbound: Consolidate port-based switching (linkerd/linkerd2-proxy#1170)
* build(deps): bump socket2 from 0.4.0 to 0.4.1 (linkerd/linkerd2-proxy#1171)
* build(deps): bump serde_json from 1.0.64 to 1.0.65 (linkerd/linkerd2-proxy#1172)
* build(deps): bump async-trait from 0.1.50 to 0.1.51 (linkerd/linkerd2-proxy#1173)
* Update to Rust v1.54.0 (linkerd/linkerd2-proxy#1175)
* build(deps): bump serde_json from 1.0.65 to 1.0.66 (linkerd/linkerd2-proxy#1176)
* Add BoxTcp and BoxNewTcp service type aliases (linkerd/linkerd2-proxy#1177)
* http: Use ExtractParam to support dynamic detect configuration (linkerd/linkerd2-proxy#1174)
* Split the inbound server into multiple modules (linkerd/linkerd2-proxy#1179)
olix0r added a commit to linkerd/linkerd2 that referenced this pull request Aug 5, 2021
This release includes only dependency updates an internal changes to
support upcoming policy features. No user-facing changes are expected.

---

* build(deps): bump tokio from 1.8.2 to 1.9.0 (linkerd/linkerd2-proxy#1162)
* build(deps): bump hyper from 0.14.10 to 0.14.11 (linkerd/linkerd2-proxy#1163)
* build(deps): bump codecov/codecov-action from 2.0.1 to 2.0.2 (linkerd/linkerd2-proxy#1165)
* build(deps): bump futures from 0.3.15 to 0.3.16 (linkerd/linkerd2-proxy#1166)
* build(deps): bump softprops/action-gh-release (linkerd/linkerd2-proxy#1167)
* build(deps): bump softprops/action-gh-release from 0.1.6 to 1 (linkerd/linkerd2-proxy#1168)
* stack: Introduce ExtractParam and InsertParam (linkerd/linkerd2-proxy#1169)
* inbound: Consolidate port-based switching (linkerd/linkerd2-proxy#1170)
* build(deps): bump socket2 from 0.4.0 to 0.4.1 (linkerd/linkerd2-proxy#1171)
* build(deps): bump serde_json from 1.0.64 to 1.0.65 (linkerd/linkerd2-proxy#1172)
* build(deps): bump async-trait from 0.1.50 to 0.1.51 (linkerd/linkerd2-proxy#1173)
* Update to Rust v1.54.0 (linkerd/linkerd2-proxy#1175)
* build(deps): bump serde_json from 1.0.65 to 1.0.66 (linkerd/linkerd2-proxy#1176)
* Add BoxTcp and BoxNewTcp service type aliases (linkerd/linkerd2-proxy#1177)
* http: Use ExtractParam to support dynamic detect configuration (linkerd/linkerd2-proxy#1174)
* Split the inbound server into multiple modules (linkerd/linkerd2-proxy#1179)
sannimichaelse pushed a commit to sannimichaelse/linkerd2 that referenced this pull request Aug 10, 2021
This release includes only dependency updates an internal changes to
support upcoming policy features. No user-facing changes are expected.

---

* build(deps): bump tokio from 1.8.2 to 1.9.0 (linkerd/linkerd2-proxy#1162)
* build(deps): bump hyper from 0.14.10 to 0.14.11 (linkerd/linkerd2-proxy#1163)
* build(deps): bump codecov/codecov-action from 2.0.1 to 2.0.2 (linkerd/linkerd2-proxy#1165)
* build(deps): bump futures from 0.3.15 to 0.3.16 (linkerd/linkerd2-proxy#1166)
* build(deps): bump softprops/action-gh-release (linkerd/linkerd2-proxy#1167)
* build(deps): bump softprops/action-gh-release from 0.1.6 to 1 (linkerd/linkerd2-proxy#1168)
* stack: Introduce ExtractParam and InsertParam (linkerd/linkerd2-proxy#1169)
* inbound: Consolidate port-based switching (linkerd/linkerd2-proxy#1170)
* build(deps): bump socket2 from 0.4.0 to 0.4.1 (linkerd/linkerd2-proxy#1171)
* build(deps): bump serde_json from 1.0.64 to 1.0.65 (linkerd/linkerd2-proxy#1172)
* build(deps): bump async-trait from 0.1.50 to 0.1.51 (linkerd/linkerd2-proxy#1173)
* Update to Rust v1.54.0 (linkerd/linkerd2-proxy#1175)
* build(deps): bump serde_json from 1.0.65 to 1.0.66 (linkerd/linkerd2-proxy#1176)
* Add BoxTcp and BoxNewTcp service type aliases (linkerd/linkerd2-proxy#1177)
* http: Use ExtractParam to support dynamic detect configuration (linkerd/linkerd2-proxy#1174)
* Split the inbound server into multiple modules (linkerd/linkerd2-proxy#1179)
sannimichaelse pushed a commit to sannimichaelse/linkerd2 that referenced this pull request Aug 13, 2021
This release includes only dependency updates an internal changes to
support upcoming policy features. No user-facing changes are expected.

---

* build(deps): bump tokio from 1.8.2 to 1.9.0 (linkerd/linkerd2-proxy#1162)
* build(deps): bump hyper from 0.14.10 to 0.14.11 (linkerd/linkerd2-proxy#1163)
* build(deps): bump codecov/codecov-action from 2.0.1 to 2.0.2 (linkerd/linkerd2-proxy#1165)
* build(deps): bump futures from 0.3.15 to 0.3.16 (linkerd/linkerd2-proxy#1166)
* build(deps): bump softprops/action-gh-release (linkerd/linkerd2-proxy#1167)
* build(deps): bump softprops/action-gh-release from 0.1.6 to 1 (linkerd/linkerd2-proxy#1168)
* stack: Introduce ExtractParam and InsertParam (linkerd/linkerd2-proxy#1169)
* inbound: Consolidate port-based switching (linkerd/linkerd2-proxy#1170)
* build(deps): bump socket2 from 0.4.0 to 0.4.1 (linkerd/linkerd2-proxy#1171)
* build(deps): bump serde_json from 1.0.64 to 1.0.65 (linkerd/linkerd2-proxy#1172)
* build(deps): bump async-trait from 0.1.50 to 0.1.51 (linkerd/linkerd2-proxy#1173)
* Update to Rust v1.54.0 (linkerd/linkerd2-proxy#1175)
* build(deps): bump serde_json from 1.0.65 to 1.0.66 (linkerd/linkerd2-proxy#1176)
* Add BoxTcp and BoxNewTcp service type aliases (linkerd/linkerd2-proxy#1177)
* http: Use ExtractParam to support dynamic detect configuration (linkerd/linkerd2-proxy#1174)
* Split the inbound server into multiple modules (linkerd/linkerd2-proxy#1179)

Signed-off-by: Sanni Michael <sannimichaelse@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants