Skip to content

Refactor tracessh.Client to cover untraced paths - session requests#59291

Merged
Joerger merged 12 commits intomasterfrom
joerger/ssh-session-client
Sep 30, 2025
Merged

Refactor tracessh.Client to cover untraced paths - session requests#59291
Joerger merged 12 commits intomasterfrom
joerger/ssh-session-client

Conversation

@Joerger
Copy link
Copy Markdown
Contributor

@Joerger Joerger commented Sep 17, 2025

This PR replaces the ChanRequestCallback logic added in #42123 with a more x/crypto/ssh-like request handler registration flow, mirroring the signature of ssh.Client.HandleChannelOpen. Rather than wiring the callback through several layers of session creation methods, you can just register request handlers the same way you would register channel handlers at the top level.

The primary functional change is that the tracing ssh.Client can wrap the request handlers in new spans so that these requests can be traced with full context.

@github-actions github-actions bot requested a review from ryanclark September 17, 2025 22:31
@Joerger Joerger added no-changelog Indicates that a PR does not require a changelog entry backport/branch/v16 backport/branch/v17 backport/branch/v18 labels Sep 17, 2025
@Joerger Joerger force-pushed the joerger/ssh-session-client branch from 8e22e53 to 529b47a Compare September 17, 2025 22:51
Copy link
Copy Markdown
Contributor

@capnspacehook capnspacehook left a comment

Choose a reason for hiding this comment

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

LGTM, makes more sense to do it this way

@Joerger Joerger force-pushed the joerger/ssh-session-client branch from b9c06c3 to 9816928 Compare September 24, 2025 00:14
@Joerger Joerger force-pushed the joerger/ssh-session-client branch from 9816928 to b6d1e16 Compare September 29, 2025 17:21
@Joerger Joerger requested a review from rosstimothy September 30, 2025 01:01
@Joerger Joerger requested a review from rosstimothy September 30, 2025 16:46
@public-teleport-github-review-bot public-teleport-github-review-bot bot removed the request for review from ryanclark September 30, 2025 19:25
@Joerger Joerger added this pull request to the merge queue Sep 30, 2025
Merged via the queue into master with commit bafb299 Sep 30, 2025
42 checks passed
@Joerger Joerger deleted the joerger/ssh-session-client branch September 30, 2025 20:18
@backport-bot-workflows
Copy link
Copy Markdown
Contributor

@Joerger See the table below for backport results.

Branch Result
branch/v16 Failed
branch/v17 Failed
branch/v18 Create PR

Joerger added a commit that referenced this pull request Sep 30, 2025
…#59291)

* Move ssh session helper into a separate file.

* Replace ssh session ChannelRequestCallback with a more x/crypto/ssh-like approach to request channel handling.

* Update comments.

* Move new request handling logic into tracessh client.

* Fix goroutine leaks with refactors.

* Add test.

* Cleanup.

* Rename methods.

* Fix lint.

* Address comments; Fix error and cleanup handling issues in tracing ssh tests.

* Increase test timeouts.

* Fix test data race.
Joerger added a commit that referenced this pull request Sep 30, 2025
…#59291)

* Move ssh session helper into a separate file.

* Replace ssh session ChannelRequestCallback with a more x/crypto/ssh-like approach to request channel handling.

* Update comments.

* Move new request handling logic into tracessh client.

* Fix goroutine leaks with refactors.

* Add test.

* Cleanup.

* Rename methods.

* Fix lint.

* Address comments; Fix error and cleanup handling issues in tracing ssh tests.

* Increase test timeouts.

* Fix test data race.
github-merge-queue bot pushed a commit that referenced this pull request Oct 6, 2025
…ests (#59789)

* Refactor `tracessh.Client` to cover untraced paths - session requests (#59291)

* Move ssh session helper into a separate file.

* Replace ssh session ChannelRequestCallback with a more x/crypto/ssh-like approach to request channel handling.

* Update comments.

* Move new request handling logic into tracessh client.

* Fix goroutine leaks with refactors.

* Add test.

* Cleanup.

* Rename methods.

* Fix lint.

* Address comments; Fix error and cleanup handling issues in tracing ssh tests.

* Increase test timeouts.

* Fix test data race.

* Fix go version conflict.

* Fix merge conflict.
atburke pushed a commit that referenced this pull request Oct 7, 2025
…ests (#59788)

* Refactor `tracessh.Client` to cover untraced paths - session requests (#59291)

* Move ssh session helper into a separate file.

* Replace ssh session ChannelRequestCallback with a more x/crypto/ssh-like approach to request channel handling.

* Update comments.

* Move new request handling logic into tracessh client.

* Fix goroutine leaks with refactors.

* Add test.

* Cleanup.

* Rename methods.

* Fix lint.

* Address comments; Fix error and cleanup handling issues in tracing ssh tests.

* Increase test timeouts.

* Fix test data race.

* Fix go version conflict.

* Fix merge conflict.
rhammonds-teleport pushed a commit that referenced this pull request Nov 6, 2025
…#59291)

* Move ssh session helper into a separate file.

* Replace ssh session ChannelRequestCallback with a more x/crypto/ssh-like approach to request channel handling.

* Update comments.

* Move new request handling logic into tracessh client.

* Fix goroutine leaks with refactors.

* Add test.

* Cleanup.

* Rename methods.

* Fix lint.

* Address comments; Fix error and cleanup handling issues in tracing ssh tests.

* Increase test timeouts.

* Fix test data race.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport/branch/v17 backport/branch/v18 no-changelog Indicates that a PR does not require a changelog entry size/md

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants