Adjust diagnostic messages of connection test for Connect My Computer#35308
Adjust diagnostic messages of connection test for Connect My Computer#35308
Conversation
There was a problem hiding this comment.
I feel like this could be considered controversial but I think it should be fine? Unless we want to explicitly include another field in the trace for those longer messages that would be shown in "Show details" along with the actual error.
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
ravicious
changed the title
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
2 similar comments
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
There was a problem hiding this comment.
idk if i'm just brain farting or is a dumb question... but what about non-linux login? same with systemctl, what about mac equivalent launctl? i know it was here before, and was just curious
There was a problem hiding this comment.
It doesn't look like this was handled. I think we could add yet another field to TestConnectionRequest which describes the platform used to differentiate between systemctl/launchctl. In this specific line we can just drop "Linux" from the message and say "system" or "OS" I suppose.
There was a problem hiding this comment.
I'll push a commit tomorrow which addresses this.
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
When showing multiple TextIcons one after another, like when showing connection diagnostic traces, it makes it easier to see where one trace starts and another ends. Especially if one of the traces spans multiple lines.
ravicious
force-pushed
the
ravicious/cmc-conn-test
branch
from
606cc64 to
41ec232
Compare
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
|
The PR changelog entry failed validation: Changelog entry not found in the PR body. Please add a "no-changelog" label to the PR, or changelog lines starting with |
|
@kimlisa Ping. |
|
The failing test made me realize that I didn't add any for the behavior expressed in |
In CI, it looks like we can absolutely spawn a new session as root, so the test is meaningless there. Instead, I'm going to add authhandlers tests and this test to the Connect My Computer test plan.
github-merge-queue
Bot
removed this pull request from the merge queue due to failed status checks
|
@ravicious See the table below for backport results.
|
3 participants
Closes #32206, closes #32185.
The messages had to be adjusted in two places:
lib/srv/authhandlers.go– this is on the side of the server agent. Here we use the fact thath.c.Server.GetInfo()returnstypes.Server, so to detect a Connect My Computer node we can check if the node hastypes.ConnectMyComputerNodeOwnerLabelin its labels.lib/client/conntest/ssh.go– this is on the side of the proxy and the request to perform a connection diagnostic. We don't have access to info about the target (unless we explicitly fetch it). But at the time of making the request, we know that we're inside the Connect My Computer flow, so we pass another argument throughTestConnectionRequest.I also included a couple of UI improvements, each one is in its own separate commit.
Changelog: Added a connection test when enrolling a new Connect My Computer resource in Web UI