fix(agent/agentssh): allow remote forwarding a socket multiple times #11631
Conversation
5ce1baf to
96c0724
Compare
96c0724 to
dab6034
Compare
| // Ensure the SSH connection is ready by testing the shell | ||
| // input/output. | ||
| pty.WriteLine("echo ping' 'pong") | ||
| pty.ExpectMatchContext(ctx, "ping pong") |
There was a problem hiding this comment.
using the CLI to test the agent is awkward and this is an example. This test would be easier to write and easier to understand if were in agent_test.go and we could use the go ssh client.
There was a problem hiding this comment.
I do somewhat agree, but I'm not sure agent_test.go is a much better place since the actual forwarding is implemented here, in cli/remoteforward.go. This test should ensure that the behavior we want works with the actual coder ssh client.
There was a problem hiding this comment.
We already have tests for coder ssh in terms of making a unix socket remote forward and verifying it works. None of that behavior is changing in this PR.
What's changing is on the agent side of the connection. And it's not just coder ssh that talks to that SSH server: it could be OpenSSH or JetBrains Gateway or VSCode (via coder ssh --stdio), or even a custom SSH client written in go using codersdk. Therefore, we need to think about the required behavior in terms of what the SSH server is supposed to do, not just end-to-end with a specific client.
There was a problem hiding this comment.
Yeah, I'm not opposed tbh. I think the best place for most of this testing is within the agentssh package itself. I'll create an issue for this since if we go this route, there's some other restructuring that needs to be done as well. Like moving the forwarding logic from cli to agentssh, etc. Otherwise I think we'll end up with "many implementations of an SSH client".
Fixes #11198
Fixes https://github.com/coder/customers/issues/407
For this PR, I implemented a new test in
cli/ssh_test.go, however, in future, we should consider doing more extensive testing for forwarding within theagent/agentsshpackage.