[vnet] feat: add SSH configuration diagnostic#55594
Conversation
7f553e9 to
03c572b
Compare
ravicious
left a comment
There was a problem hiding this comment.
The UI for this looks good, I added some small improvements. I'll take a look at the Go part tomorrow.
| ], | ||
| }, | ||
| }; | ||
| const sshConfigReport = makeCheckReport({ |
There was a problem hiding this comment.
Could you add the SSH report to the story for DocumentVnetDiagReport at web/packages/teleterm/src/ui/Vnet/DocumentVnetDiagReport.story.tsx? It boils down to adding some controls so that it's easier to see the story in different states.
pnpm storybook then https://localhost:9002/?path=/story/teleterm-vnet-documentvnetdiagreport--document-vnet-diag-report the controls should be at the bottom, if not there's a button in the top right to show them.
There was a problem hiding this comment.
yeah that storybook is pretty useful, didn't really know about it. Added in the latest commit.
There was a problem hiding this comment.
Oh yeah that escaped my mind. I was reviewing with my (incorrect) assumption that VNet for SSH will be something that you need to opt-in to separately. But I just looked again at the conversation we had under the RFD #53370 (comment) and Nic explicitly mentions that everything will be enabled by default but there should be an option in Connect to add the Include directive to ~/.ssh/config.
As a user, if I saw a green icon and then "No issues found" below, I'd think that everything's working and it wouldn't necessarily cross my mind to click on "Open Diag Report" to figure out why SSH connections aren't going through.
At the same time, VNet for SSH probably isn't something that everyone's going to use, so we cannot report this diag check as an error because it'd constantly show a notification in Connect.
I wonder if as a middle ground we could do something like this:
- There's a separate RPC for checking the status of the default SSH config, or maybe
VnetService.Startreturns this in the response. - When VNet is running and Connect knows that the default SSH config doesn't include VNet's config, the VNet panel says something like this below "Proxying TCP connections…":
-
VNet is not included in the default SSH config. [See More]
- [See More] opens the diag report.
-
- The SSH diag check is included in the report, but we extend
CheckReportStatusto include a new status, something like mayberecommendationoradvice. It'd signify that everything is okay but we recommend changing some external configs for seamless experience.- It'd be best if adding that
Includewas a single button click, but I suppose pointing the users to the diag report would be okay for now.
- It'd be best if adding that
How does that sound to you? Any other ideas?
There was a problem hiding this comment.
Sounds good to me. I'm only not sure if we need a new RPC just for the VNet SSH status, maybe we could read that from the existing diag response?
One more thing that comes to my mind is that we could show the same information in the Connect with any SSH client to ... notification, so that users can immediately see that something might not be working.
There was a problem hiding this comment.
It'd be best if adding that Include was a single button click
It will be very soon, I just haven't gotten there yet. it's still ugly but I'm working on something like this

This currently just uses the diag report which it already has access to here. Anyway I don't think I'll change this slider panel in this PR, it will be in another one.
There was a problem hiding this comment.
Cool yeah, the screenshot looks pretty much how I imagined it could work. We'd just need to find a way to display this without showing a big button. I still think the panel could use a very short explanation of the problem with the default config + a button that leads either to the diag report with a longer explanation and a button to do this or to a modal that includes a longer explanation and a button.
I'm only not sure if we need a new RPC just for the VNet SSH status, maybe we could read that from the existing diag response?
This currently just uses the diag report which it already has access to here.
I guess that's fine, can't really think of any downsides right now. When I was originally adding the "Proxying TCP connections" part, there was no diagnostics so I had to make a separate endpoint to get this data. It's also updated any time you open the panel whereas the diag report gets updated only every 30 seconds.
There was a problem hiding this comment.
alright here's what I've got in the latest PR, it tells you to check the diag report but i didn't think we needed 2 buttons to open it right next to each other, and i still plan to add the one-click "add to ~/.ssh/config button in the diag report page

There was a problem hiding this comment.
@ravicious do you think this PR adding the diagnostic is fine and we can discuss the status UX in #55755?
There was a problem hiding this comment.
I'd maybe move the discussion there since it's hard to talk about one without the other.
4a583b3 to
9abc8ad
Compare
9ad9ad1 to
b1fa221
Compare
7ece253 to
c7a3bab
Compare
This commit adds a VNet diagnostic that reports whether the default user OpenSSH config file (`~/.ssh/config`) includes VNet's generated SSH config file.
c7a3bab to
593266c
Compare
| // which may be helpful for debugging. | ||
| func (d *SSHDiag) Commands(ctx context.Context) []*exec.Cmd { | ||
| return []*exec.Cmd{ | ||
| exec.CommandContext(ctx, "cat", d.userOpenSSHConfigPath), |
There was a problem hiding this comment.
Windows doesn't have cat, it works on our setups only because we have Git Bash (or equivalent) installed. 😏 It looks like we need to use type instead.
type is weird though since it seems to be a built-in command which doesn't have a corresponding .exe. Because of this I'm not sure how BadBatBut applies to it. But I think we should be safe as we don't pass any user input to it.
Another thing is that on Windows I think such command will always error anyway, as I believe by default there's no ~/.ssh/config unless the user creates it (tbh I don't even remember if it's there by default on macOS).
I suppose we're fine with the error about cat/type for now? We'd have to rewrite how the diag package executes commands to account for this case.
There was a problem hiding this comment.
Yeah type is not even a real "command" but a shell built-in we'd have to run through cmd.exe, and then escape the path... using a command to just print the file was always a hack, in the latest commit switched to reading and returning the file contents directly as part of the diag report
| // This intentionally always returns CHECK_REPORT_STATUS_OK even if | ||
| // ~/.ssh/config does not include the VNet generated SSH config. It is | ||
| // not mandatory to configure SSH and returning an error status would | ||
| // case an alert and notification in Connect. | ||
| Status: diagv1.CheckReportStatus_CHECK_REPORT_STATUS_OK, |
There was a problem hiding this comment.
I don't love this but I guess it's an acceptable piece of tech debt for now.
I'm not 100% sure what would be the best way to represent this, so perhaps it's actually best to keep it this way for now until we have add another check which might return a "warning".
At the moment I just can't decide if we want checks to be able to return Ok/Error/Warning or if we want to have two kinds of checks, ones that return Ok/Error and ones that return Ok/Warning.
| // UserOpensshConfigPath is the full path to the user's default OpenSSH config | ||
| // file (~/.ssh/config). | ||
| string user_openssh_config_path = 1; |
There was a problem hiding this comment.
I don't know if we have an agreed convention at Teleport wrt proto comments, but I've been starting my comments with how the field is named in the proto file rather than how it'll end up in Go.
Tbh it feels like something the protoc Go plugin should handle automatically since it converts snake case to camel case anyway. 😏
There was a problem hiding this comment.
switched the comment style to at least be consistent within this file
|
Leaving a preemptive approval since only the cat/type issue needs to be addressed. |
Backport #55594 to branch/v18 Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
Backport #55594 to branch/v17 Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com>
* [v17][vnet] feat: TCP dial to SSH targets Backport #55087 to branch/v17 * [v17][vnet] feat: accept incoming SSH connections Backport #55155 to branch/v17 * [v17][vnet] feat: forward SSH connections to target Backport #55156 to branch/v17 * [v17][vnet] feat: write VNet SSH keys to TELEPORT_HOME Backport #55228 to branch/v17 * [v17][vnet] feat: write OpenSSH-compatible config file for VNet SSH Backport #55239 to branch/v17 * [v17][vnet] fix: support <hostname>.<leaf-cluster> for VNet SSH Backport #55688 to branch/v17 * fix BlockUntil API for backport * [v17][vnet] feat: add "Connect with VNet" button to SSH servers Backport #55623 to branch/v17 * [v17][vnet] feat: support VNet SSH when cluster name does not match proxy public addr Backport #55655 to branch/v17 * [v17][vnet] feat: add SSH configuration diagnostic Backport #55594 to branch/v17 Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> * [v17][vnet] feat: show SSH status in VNet slider Backport #55755 to branch/v17 Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> * [v17][vnet] feat: support proxy recording mode with VNet SSH Backport #55788 to branch/v17 * [v17][vnet] feat: support diag checks on windows Backport #55856 to branch/v17 * [v17] fix: data race in vnet.TestSSH Backport #55980 to branch/v17 * [v17][vnet] feat: mention SSH on VNet info page Backport #55973 to branch/v17 * [v17][vnet] feat: serve DNS on IPv4 Backport #55539 to branch/v17 * [v17][vnet] fix: close proxied channel only after data and requests are complete Backport #56020 to branch/v17 * [v17][vnet] feat: automatic SSH client configuration Backport #55923 to branch/v17 * VNet docs: Provide clear instructions for getting debug logs (#56068) * VNet diag notification: Do not show button to open report if there's no workspace selected (#56067) * VNet diag report: Don't show button in notification if there's no workspace * Replace deprecated MutableRefObject with RefObject * Make openReport not depend on value of rootClusterUri Otherwise the effect that uses setInterval re-runs whenever the user switches to another workspace. * [v17][vnet] feat: automatic SSH client configuration in Connect Backport #55924 to branch/v17 Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com> * [v17][vnet] fix: avoid empty host matchers in generated SSH config Backport #56103 to branch/v17 * avoid t.Context() pre go1.24 * fix cspell lint * [v17][docs] VNet SSH Backport #56147 to branch/v17 * [v17][vnet] feat: SSH usage reporting Backport #56537 to branch/v17 * [v17][vnet] fix: mask default IP route on windows Backport #56957 to branch/v17 --------- Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>
* [v18][vnet] feat: TCP dial to SSH targets Backport #55087 to branch/v18 * [v18][vnet] feat: accept incoming SSH connections Backport #55155 to branch/v18 * [v18][vnet] feat: forward SSH connections to target Backport #55156 to branch/v18 * [v18][vnet] feat: write VNet SSH keys to TELEPORT_HOME Backport #55228 to branch/v18 * [v18][vnet] feat: write OpenSSH-compatible config file for VNet SSH Backport #55239 to branch/v18 * [v18][vnet] fix: support <hostname>.<leaf-cluster> for VNet SSH Backport #55688 to branch/v18 * [v18][vnet] feat: add "Connect with VNet" button to SSH servers Backport #55623 to branch/v18 * fix test in backport * [v18][vnet] feat: support VNet SSH when cluster name does not match proxy public addr Backport #55655 to branch/v18 * [v18][vnet] feat: add SSH configuration diagnostic Backport #55594 to branch/v18 Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> * [v18][vnet] feat: show SSH status in VNet slider Backport #55755 to branch/v18 Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> * [v18][vnet] feat: support proxy recording mode with VNet SSH Backport #55788 to branch/v18 * [v18][vnet] feat: support diag checks on windows Backport #55856 to branch/v18 * [v18] fix: data race in vnet.TestSSH Backport #55980 to branch/v18 * [v18][vnet] feat: mention SSH on VNet info page Backport #55973 to branch/v18 * [v18][vnet] feat: serve DNS on IPv4 Backport #55539 to branch/v18 * [v18][vnet] fix: close proxied channel only after data and requests are complete Backport #56020 to branch/v18 * [v18][vnet] feat: automatic SSH client configuration Backport #55923 to branch/v18 * VNet diag notification: Do not show button to open report if there's no workspace selected (#56067) * VNet diag report: Don't show button in notification if there's no workspace * Replace deprecated MutableRefObject with RefObject * Make openReport not depend on value of rootClusterUri Otherwise the effect that uses setInterval re-runs whenever the user switches to another workspace. * [v18][vnet] feat: automatic SSH client configuration in Connect Backport #55924 to branch/v18 Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com> * [v18][vnet] fix: avoid empty host matchers in generated SSH config Backport #56103 to branch/v18 * [v18][docs] VNet SSH Backport #56147 to branch/v18 * [v18][docs] add VNet warnings Backport #56601 to branch/v18 * [v18][vnet] feat: SSH usage reporting Backport #56537 to branch/v18 * [v18][vnet] fix: mask default IP route on windows Backport #56957 to branch/v18 --------- Co-authored-by: Rafał Cieślak <rafal.cieslak@goteleport.com> Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>

This commit adds a VNet diagnostic that reports whether the default user OpenSSH config file (
~/.ssh/config) includes VNet's generated SSH config file.Part of RFD 207
Example diagnostics:
screenshots
Copied text with VNet not configured
VNet Diagnostic ReportCreated at: 2025-06-10 08:50:25 (Tue, 10 Jun 2025 15:50:25 GMT)
Network interface: utun5
IPv4 CIDR ranges: 100.64.0.0/10
IPv6 prefix: fdc3:29fa:bc6e::
DNS zones: one.private, nicklaassen.ca
✅ There are no network routes in conflict with VNet.
The user's default SSH configuration file does not include VNet's
generated configuration file and connections to VNet SSH hosts will
not work by default.
Copied text with VNet configured
VNet Diagnostic ReportCreated at: 2025-06-10 08:53:26 (Tue, 10 Jun 2025 15:53:26 GMT)
Network interface: utun5
IPv4 CIDR ranges: 100.64.0.0/10
IPv6 prefix: fdc3:29fa:bc6e::
DNS zones: one.private, nicklaassen.ca
✅ There are no network routes in conflict with VNet.
✅ VNet SSH is configured correctly.