Skip to content

Make proxy routing logic reusable#18370

Merged
rosstimothy merged 3 commits intomasterfrom
tross/reusable_proxy_routing
Nov 18, 2022
Merged

Make proxy routing logic reusable#18370
rosstimothy merged 3 commits intomasterfrom
tross/reusable_proxy_routing

Conversation

@rosstimothy
Copy link
Copy Markdown
Contributor

Routing logic existed within an unexported handler of ssh subsystem
requests, which prevented it from being reused by other components
within the proxy, like the webapi server. This causes significant
latency issues for web sessions because the web apiserver is required
to dial the proxy ssh server to determine how to route to the host.
Since the web apiserver and the proxy ssh server exist in the same
process this is an entirely unnecesarry step that could be avoided
if the routing and ability to established connections were shared
throughout the proxy.

A new proxy.Router object is introduced which contains all the
logic that used to exist in regular.proxySubsys for determining
how to connect to servers and clusters. All routing within the
regular.proxySubsys now leverages the proxy.Router to dial
the target.

This is step 1 in addressing #15167. Now that the proxy.Router
exists web.APIServer will be able to make use of it to avoid
dialing the same process to establish connections.

Note to reviewers:

This PR will be easier to review commit by commit as the first commit refactors lib/proxy so that all proxy peering code resides in lib/proxy/peer, which adds a bit of noise, but ultimately makes lib/proxy a bit tidier. The second commit adds proxy.Router, replaces the existing logic within regular.proxySubsys to use it, and updates all initializations of regular.Server which were supplying the SetProxyMode option to pass in a proxy.Router.

@rosstimothy rosstimothy changed the title Refactor proxy routing logic into a reusable object Make proxy routing logic reusable Nov 10, 2022
@rosstimothy rosstimothy force-pushed the tross/reusable_proxy_routing branch 3 times, most recently from cb6423f to 51a0977 Compare November 11, 2022 14:22
@rosstimothy rosstimothy marked this pull request as ready for review November 11, 2022 14:51
@rosstimothy
Copy link
Copy Markdown
Contributor Author

friendly ping @alistanis @fspmarshall

Comment thread lib/proxy/router.go Outdated
Comment thread lib/proxy/router.go
@rosstimothy rosstimothy force-pushed the tross/reusable_proxy_routing branch from bf91ea2 to 140f822 Compare November 17, 2022 16:37
Comment thread lib/srv/regular/proxy.go Outdated
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I thought you didn't like doing this in init?

(I actually prefer it this way, FWIW)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Lesser or two evils. Doing it on every proxy subsystem request seemed a bit excessive.

func newProxySubsys(ctx *srv.ServerContext, srv *Server, req proxySubsysRequest) (*proxySubsys, error) {
err := metrics.RegisterPrometheusCollectors(prometheusCollectors...)
if err != nil {
return nil, trace.Wrap(err)
}

Routing logic existed within an unexported handler of ssh subsystem
requests, which prevented it from being reused by other components
within the proxy, like the webapi server. This causes significant
latency issues for web sessions because the web apiserver is required
to dial the proxy ssh server to determine how to route to the host.
Since the web apiserver and the proxy ssh server exist in the same
process this is an entirely unnecesarry step that could be avoided
if the routing and ability to established connections were shared
throughout the proxy.

A new `proxy.Router` object is introduced which contains all the
logic that used to exist in `regular.proxySubsys` for determining
how to connect to servers and clusters. All routing within the
`regular.proxySubsys` now leverages the `proxy.Router` to dial
the target.

This is step 1 in addressing #15167. Now that the `proxy.Router`
exists `web.APIServer` will be able to make use of it to avoid
dialing the same process to establish connections.
@rosstimothy rosstimothy force-pushed the tross/reusable_proxy_routing branch from 140f822 to a278ef2 Compare November 18, 2022 12:36
@github-actions github-actions Bot removed the request for review from alistanis November 18, 2022 12:37
@rosstimothy rosstimothy enabled auto-merge (squash) November 18, 2022 12:39
@rosstimothy rosstimothy merged commit e4e7f53 into master Nov 18, 2022
@github-actions
Copy link
Copy Markdown
Contributor

@rosstimothy See the table below for backport results.

Branch Result
branch/v10 Failed
branch/v11 Failed
branch/v9 Failed

@rosstimothy rosstimothy deleted the tross/reusable_proxy_routing branch November 18, 2022 13:13
rosstimothy added a commit that referenced this pull request Nov 18, 2022
Routing logic existed within an unexported handler of ssh subsystem
requests, which prevented it from being reused by other components
within the proxy, like the webapi server. This causes significant
latency issues for web sessions because the web apiserver is required
to dial the proxy ssh server to determine how to route to the host.
Since the web apiserver and the proxy ssh server exist in the same
process this is an entirely unnecesarry step that could be avoided
if the routing and ability to established connections were shared
throughout the proxy.

A new `proxy.Router` object is introduced which contains all the
logic that used to exist in `regular.proxySubsys` for determining
how to connect to servers and clusters. All routing within the
`regular.proxySubsys` now leverages the `proxy.Router` to dial
the target.

This is step 1 in addressing #15167. Now that the `proxy.Router`
exists `web.APIServer` will be able to make use of it to avoid
dialing the same process to establish connections.
rosstimothy added a commit that referenced this pull request Nov 21, 2022
Routing logic existed within an unexported handler of ssh subsystem
requests, which prevented it from being reused by other components
within the proxy, like the webapi server. This causes significant
latency issues for web sessions because the web apiserver is required
to dial the proxy ssh server to determine how to route to the host.
Since the web apiserver and the proxy ssh server exist in the same
process this is an entirely unnecesarry step that could be avoided
if the routing and ability to established connections were shared
throughout the proxy.

A new `proxy.Router` object is introduced which contains all the
logic that used to exist in `regular.proxySubsys` for determining
how to connect to servers and clusters. All routing within the
`regular.proxySubsys` now leverages the `proxy.Router` to dial
the target.

This is step 1 in addressing #15167. Now that the `proxy.Router`
exists `web.APIServer` will be able to make use of it to avoid
dialing the same process to establish connections.
rosstimothy added a commit that referenced this pull request Nov 30, 2022
* create  package to contain proxy peering code

* Refactor proxy routing logic into a reusable object

Routing logic existed within an unexported handler of ssh subsystem
requests, which prevented it from being reused by other components
within the proxy, like the webapi server. This causes significant
latency issues for web sessions because the web apiserver is required
to dial the proxy ssh server to determine how to route to the host.
Since the web apiserver and the proxy ssh server exist in the same
process this is an entirely unnecesarry step that could be avoided
if the routing and ability to established connections were shared
throughout the proxy.

A new `proxy.Router` object is introduced which contains all the
logic that used to exist in `regular.proxySubsys` for determining
how to connect to servers and clusters. All routing within the
`regular.proxySubsys` now leverages the `proxy.Router` to dial
the target.

This is step 1 in addressing #15167. Now that the `proxy.Router`
exists `web.APIServer` will be able to make use of it to avoid
dialing the same process to establish connections.
rosstimothy added a commit that referenced this pull request Dec 6, 2022
* create  package to contain proxy peering code

* Refactor proxy routing logic into a reusable object

Routing logic existed within an unexported handler of ssh subsystem
requests, which prevented it from being reused by other components
within the proxy, like the webapi server. This causes significant
latency issues for web sessions because the web apiserver is required
to dial the proxy ssh server to determine how to route to the host.
Since the web apiserver and the proxy ssh server exist in the same
process this is an entirely unnecesarry step that could be avoided
if the routing and ability to established connections were shared
throughout the proxy.

A new `proxy.Router` object is introduced which contains all the
logic that used to exist in `regular.proxySubsys` for determining
how to connect to servers and clusters. All routing within the
`regular.proxySubsys` now leverages the `proxy.Router` to dial
the target.

This is step 1 in addressing #15167. Now that the `proxy.Router`
exists `web.APIServer` will be able to make use of it to avoid
dialing the same process to establish connections.
rosstimothy added a commit that referenced this pull request Dec 6, 2022
* Make proxy routing logic reusable (#18370)

* create  package to contain proxy peering code

* Refactor proxy routing logic into a reusable object

Routing logic existed within an unexported handler of ssh subsystem
requests, which prevented it from being reused by other components
within the proxy, like the webapi server. This causes significant
latency issues for web sessions because the web apiserver is required
to dial the proxy ssh server to determine how to route to the host.
Since the web apiserver and the proxy ssh server exist in the same
process this is an entirely unnecesarry step that could be avoided
if the routing and ability to established connections were shared
throughout the proxy.

A new `proxy.Router` object is introduced which contains all the
logic that used to exist in `regular.proxySubsys` for determining
how to connect to servers and clusters. All routing within the
`regular.proxySubsys` now leverages the `proxy.Router` to dial
the target.

This is step 1 in addressing #15167. Now that the `proxy.Router`
exists `web.APIServer` will be able to make use of it to avoid
dialing the same process to establish connections.

* Make session control logic reusable (#18565)

Session control logic existed within `HandleNewConn` of `srv/regular.Server`.
This prevented any of it from being used by other components that
also needed to enforce session control.

All the logic from within `HandleNewConn` was refactored to a new
`srv.SessionController` object which the `regular.Server` now uses
to perform session control. There were a few additional changes
needed to accomadate that session control now exists outside
the server and to make tests easier to write. Namely, altering
`srv.ComputLockTargets` to not take a `Server` as a parameter and
leveraging a clock within `services.SemaphoreLock`.

This is step 2 in addressing #15167. Before the web apiserver can
leverage the newly introduced proxy.Router and bypass making ssh
connections to the proxy ssh server it needs to be able to perfrom
session control.
rosstimothy added a commit that referenced this pull request Jan 12, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
github-merge-queue Bot pushed a commit that referenced this pull request Jan 16, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
github-actions Bot pushed a commit that referenced this pull request Jan 16, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
github-actions Bot pushed a commit that referenced this pull request Jan 16, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
github-actions Bot pushed a commit that referenced this pull request Jan 16, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
github-actions Bot pushed a commit that referenced this pull request Jan 16, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
github-merge-queue Bot pushed a commit that referenced this pull request Jan 19, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
github-merge-queue Bot pushed a commit that referenced this pull request Jan 19, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
github-merge-queue Bot pushed a commit that referenced this pull request Jan 19, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
github-merge-queue Bot pushed a commit that referenced this pull request Jan 19, 2024
During refactoring performed in #18370 the counter was moved but
never used outside of registration. This restores incrementing the
counter for dial attempts and improves error reporting in traces
in the router.
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