From 594e4b568bed4b4054982d81449103009e49d2e0 Mon Sep 17 00:00:00 2001 From: Brett Kolodny Date: Mon, 12 May 2025 21:24:36 +0000 Subject: [PATCH 1/7] feat: allow one shot commands using coder ssh --- cli/ssh.go | 81 ++++++++++++++++++++------------ cli/ssh_test.go | 122 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 172 insertions(+), 31 deletions(-) diff --git a/cli/ssh.go b/cli/ssh.go index 7c5bda073f973..0157c60650b89 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -90,14 +90,26 @@ func (r *RootCmd) ssh() *serpent.Command { wsClient := workspacesdk.New(client) cmd := &serpent.Command{ Annotations: workspaceCommand, - Use: "ssh ", - Short: "Start a shell into a workspace", + Use: "ssh [command]", + Short: "Start a shell into a workspace or run a command", Middleware: serpent.Chain( - serpent.RequireNArgs(1), + // Require at least one arg for the workspace name + func(next serpent.HandlerFunc) serpent.HandlerFunc { + return func(i *serpent.Invocation) error { + got := len(i.Args) + if got < 1 { + return xerrors.Errorf("expected at least 1 arg but got %v %v", got, i.Args) + } + + return next(i) + } + }, r.InitClient(client), initAppearance(client, &appearanceConfig), ), Handler: func(inv *serpent.Invocation) (retErr error) { + command := strings.Join(inv.Args[1:], " ") + // Before dialing the SSH server over TCP, capture Interrupt signals // so that if we are interrupted, we have a chance to tear down the // TCP session cleanly before exiting. If we don't, then the TCP @@ -547,40 +559,47 @@ func (r *RootCmd) ssh() *serpent.Command { sshSession.Stdout = inv.Stdout sshSession.Stderr = inv.Stderr - err = sshSession.Shell() - if err != nil { - return xerrors.Errorf("start shell: %w", err) - } + if command != "" { + err := sshSession.Run(command) + if err != nil { + return xerrors.Errorf("run command: %w", err) + } + } else { + err = sshSession.Shell() + if err != nil { + return xerrors.Errorf("start shell: %w", err) + } - // Put cancel at the top of the defer stack to initiate - // shutdown of services. - defer cancel() + // Put cancel at the top of the defer stack to initiate + // shutdown of services. + defer cancel() - if validOut { - // Set initial window size. - width, height, err := term.GetSize(int(stdoutFile.Fd())) - if err == nil { - _ = sshSession.WindowChange(height, width) + if validOut { + // Set initial window size. + width, height, err := term.GetSize(int(stdoutFile.Fd())) + if err == nil { + _ = sshSession.WindowChange(height, width) + } } - } - err = sshSession.Wait() - conn.SendDisconnectedTelemetry() - if err != nil { - if exitErr := (&gossh.ExitError{}); errors.As(err, &exitErr) { - // Clear the error since it's not useful beyond - // reporting status. - return ExitError(exitErr.ExitStatus(), nil) - } - // If the connection drops unexpectedly, we get an - // ExitMissingError but no other error details, so try to at - // least give the user a better message - if errors.Is(err, &gossh.ExitMissingError{}) { - return ExitError(255, xerrors.New("SSH connection ended unexpectedly")) + err = sshSession.Wait() + conn.SendDisconnectedTelemetry() + if err != nil { + if exitErr := (&gossh.ExitError{}); errors.As(err, &exitErr) { + // Clear the error since it's not useful beyond + // reporting status. + return ExitError(exitErr.ExitStatus(), nil) + } + // If the connection drops unexpectedly, we get an + // ExitMissingError but no other error details, so try to at + // least give the user a better message + if errors.Is(err, &gossh.ExitMissingError{}) { + return ExitError(255, xerrors.New("SSH connection ended unexpectedly")) + } + return xerrors.Errorf("session ended: %w", err) } - return xerrors.Errorf("session ended: %w", err) - } + } return nil }, } diff --git a/cli/ssh_test.go b/cli/ssh_test.go index 5fcb6205d5e45..e66cf90aeb173 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -31,6 +31,7 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" "golang.org/x/crypto/ssh" + gossh "golang.org/x/crypto/ssh" gosshagent "golang.org/x/crypto/ssh/agent" "golang.org/x/sync/errgroup" "golang.org/x/xerrors" @@ -2200,6 +2201,127 @@ func TestSSH_CoderConnect(t *testing.T) { <-cmdDone }) + + t.Run("OneShot", func(t *testing.T) { + t.Parallel() + + client, workspace, agentToken := setupWorkspaceForAgent(t) + inv, root := clitest.New(t, "ssh", workspace.Name, "echo hello world") + clitest.SetupConfig(t, client, root) + + // Capture command output + output := new(bytes.Buffer) + inv.Stdout = output + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + cmdDone := tGo(t, func() { + err := inv.WithContext(ctx).Run() + assert.NoError(t, err) + }) + + _ = agenttest.New(t, client.URL, agentToken) + coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) + + <-cmdDone + + // Verify command output + assert.Contains(t, output.String(), "hello world") + }) + + t.Run("OneShotExitCode", func(t *testing.T) { + t.Parallel() + + client, workspace, agentToken := setupWorkspaceForAgent(t) + + // Setup agent first to avoid race conditions + _ = agenttest.New(t, client.URL, agentToken) + coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + // Test successful exit code + t.Run("Success", func(t *testing.T) { + inv, root := clitest.New(t, "ssh", workspace.Name, "exit 0") + clitest.SetupConfig(t, client, root) + + err := inv.WithContext(ctx).Run() + assert.NoError(t, err) + }) + + // Test error exit code + t.Run("Error", func(t *testing.T) { + inv, root := clitest.New(t, "ssh", workspace.Name, "exit 1") + clitest.SetupConfig(t, client, root) + + err := inv.WithContext(ctx).Run() + assert.Error(t, err) + var exitErr *gossh.ExitError + assert.True(t, errors.As(err, &exitErr)) + assert.Equal(t, 1, exitErr.ExitStatus()) + }) + }) + + t.Run("OneShotStdio", func(t *testing.T) { + t.Parallel() + client, workspace, agentToken := setupWorkspaceForAgent(t) + _, _ = tGoContext(t, func(ctx context.Context) { + // Run this async so the SSH command has to wait for + // the build and agent to connect! + _ = agenttest.New(t, client.URL, agentToken) + <-ctx.Done() + }) + + clientOutput, clientInput := io.Pipe() + serverOutput, serverInput := io.Pipe() + defer func() { + for _, c := range []io.Closer{clientOutput, clientInput, serverOutput, serverInput} { + _ = c.Close() + } + }() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + inv, root := clitest.New(t, "ssh", "--stdio", workspace.Name, "echo 'hello stdio'") + clitest.SetupConfig(t, client, root) + inv.Stdin = clientOutput + inv.Stdout = serverInput + inv.Stderr = io.Discard + + cmdDone := tGo(t, func() { + err := inv.WithContext(ctx).Run() + assert.NoError(t, err) + }) + + conn, channels, requests, err := ssh.NewClientConn(&testutil.ReaderWriterConn{ + Reader: serverOutput, + Writer: clientInput, + }, "", &ssh.ClientConfig{ + // #nosec + HostKeyCallback: ssh.InsecureIgnoreHostKey(), + }) + require.NoError(t, err) + defer conn.Close() + + sshClient := ssh.NewClient(conn, channels, requests) + session, err := sshClient.NewSession() + require.NoError(t, err) + defer session.Close() + + // Capture and verify command output + output, err := session.Output("echo hello back") + require.NoError(t, err) + assert.Contains(t, string(output), "hello back") + + err = sshClient.Close() + require.NoError(t, err) + _ = clientOutput.Close() + + <-cmdDone + }) } type fakeCoderConnectDialer struct{} From bed4103c187631efc284a697a0ca63a0947e20eb Mon Sep 17 00:00:00 2001 From: Brett Kolodny Date: Mon, 12 May 2025 21:28:37 +0000 Subject: [PATCH 2/7] chore: improve ssh missing args error message --- cli/ssh.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/ssh.go b/cli/ssh.go index 0157c60650b89..0e5f7bfc7399f 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -98,7 +98,7 @@ func (r *RootCmd) ssh() *serpent.Command { return func(i *serpent.Invocation) error { got := len(i.Args) if got < 1 { - return xerrors.Errorf("expected at least 1 arg but got %v %v", got, i.Args) + return xerrors.New("expected the name of a workspace") } return next(i) From 4026c077dd4016d03e4ff16bd1999eda70151045 Mon Sep 17 00:00:00 2001 From: Brett Kolodny Date: Mon, 12 May 2025 21:37:52 +0000 Subject: [PATCH 3/7] fix: lint --- cli/ssh.go | 1 - cli/ssh_test.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/cli/ssh.go b/cli/ssh.go index 0e5f7bfc7399f..df0deb5d01a51 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -598,7 +598,6 @@ func (r *RootCmd) ssh() *serpent.Command { } return xerrors.Errorf("session ended: %w", err) } - } return nil }, diff --git a/cli/ssh_test.go b/cli/ssh_test.go index e66cf90aeb173..13e3361fc6c28 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -31,7 +31,6 @@ import ( "github.com/stretchr/testify/require" "go.uber.org/mock/gomock" "golang.org/x/crypto/ssh" - gossh "golang.org/x/crypto/ssh" gosshagent "golang.org/x/crypto/ssh/agent" "golang.org/x/sync/errgroup" "golang.org/x/xerrors" @@ -2258,7 +2257,7 @@ func TestSSH_CoderConnect(t *testing.T) { err := inv.WithContext(ctx).Run() assert.Error(t, err) - var exitErr *gossh.ExitError + var exitErr *ssh.ExitError assert.True(t, errors.As(err, &exitErr)) assert.Equal(t, 1, exitErr.ExitStatus()) }) From 55735fcd0a97658a3d5a7b37698af53b6badaa74 Mon Sep 17 00:00:00 2001 From: Brett Kolodny Date: Mon, 12 May 2025 22:02:27 +0000 Subject: [PATCH 4/7] fix: gen --- cli/testdata/coder_--help.golden | 2 +- cli/testdata/coder_ssh_--help.golden | 4 ++-- docs/manifest.json | 2 +- docs/reference/cli/index.md | 2 +- docs/reference/cli/ssh.md | 4 ++-- 5 files changed, 7 insertions(+), 7 deletions(-) diff --git a/cli/testdata/coder_--help.golden b/cli/testdata/coder_--help.golden index 5a3ad462cdae8..f3c6f56a7a191 100644 --- a/cli/testdata/coder_--help.golden +++ b/cli/testdata/coder_--help.golden @@ -46,7 +46,7 @@ SUBCOMMANDS: show Display details of a workspace's resources and agents speedtest Run upload and download tests from your machine to a workspace - ssh Start a shell into a workspace + ssh Start a shell into a workspace or run a command start Start a workspace stat Show resource usage for the current workspace. state Manually manage Terraform state to fix broken workspaces diff --git a/cli/testdata/coder_ssh_--help.golden b/cli/testdata/coder_ssh_--help.golden index 1f7122dd655a2..27c39fce8ff7f 100644 --- a/cli/testdata/coder_ssh_--help.golden +++ b/cli/testdata/coder_ssh_--help.golden @@ -1,9 +1,9 @@ coder v0.0.0-devel USAGE: - coder ssh [flags] + coder ssh [flags] [command] - Start a shell into a workspace + Start a shell into a workspace or run a command OPTIONS: --disable-autostart bool, $CODER_SSH_DISABLE_AUTOSTART (default: false) diff --git a/docs/manifest.json b/docs/manifest.json index 4519767b071dd..cd85407b3d5ab 100644 --- a/docs/manifest.json +++ b/docs/manifest.json @@ -1455,7 +1455,7 @@ }, { "title": "ssh", - "description": "Start a shell into a workspace", + "description": "Start a shell into a workspace or run a command", "path": "reference/cli/ssh.md" }, { diff --git a/docs/reference/cli/index.md b/docs/reference/cli/index.md index 1803fd460c65b..2106374eba150 100644 --- a/docs/reference/cli/index.md +++ b/docs/reference/cli/index.md @@ -53,7 +53,7 @@ Coder — A tool for provisioning self-hosted development environments with Terr | [schedule](./schedule.md) | Schedule automated start and stop times for workspaces | | [show](./show.md) | Display details of a workspace's resources and agents | | [speedtest](./speedtest.md) | Run upload and download tests from your machine to a workspace | -| [ssh](./ssh.md) | Start a shell into a workspace | +| [ssh](./ssh.md) | Start a shell into a workspace or run a command | | [start](./start.md) | Start a workspace | | [stat](./stat.md) | Show resource usage for the current workspace. | | [stop](./stop.md) | Stop a workspace | diff --git a/docs/reference/cli/ssh.md b/docs/reference/cli/ssh.md index c5bae755c8419..d68eb4fbf3f14 100644 --- a/docs/reference/cli/ssh.md +++ b/docs/reference/cli/ssh.md @@ -1,12 +1,12 @@ # ssh -Start a shell into a workspace +Start a shell into a workspace or run a command ## Usage ```console -coder ssh [flags] +coder ssh [flags] [command] ``` ## Options From aaaab01950383204ee8244f40bfc389f36b3361d Mon Sep 17 00:00:00 2001 From: Brett Kolodny Date: Tue, 13 May 2025 14:16:07 +0000 Subject: [PATCH 5/7] fix: update command to work on both unix and windows --- cli/ssh_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/cli/ssh_test.go b/cli/ssh_test.go index 13e3361fc6c28..49f83daa0612a 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -2205,7 +2205,7 @@ func TestSSH_CoderConnect(t *testing.T) { t.Parallel() client, workspace, agentToken := setupWorkspaceForAgent(t) - inv, root := clitest.New(t, "ssh", workspace.Name, "echo hello world") + inv, root := clitest.New(t, "ssh", workspace.Name, "echo 'hello world'") clitest.SetupConfig(t, client, root) // Capture command output @@ -2311,7 +2311,7 @@ func TestSSH_CoderConnect(t *testing.T) { defer session.Close() // Capture and verify command output - output, err := session.Output("echo hello back") + output, err := session.Output("echo 'hello back'") require.NoError(t, err) assert.Contains(t, string(output), "hello back") From ca640b5f4388dd7ed623bdabadbe23d085c82262 Mon Sep 17 00:00:00 2001 From: Brett Kolodny Date: Fri, 16 May 2025 13:33:27 +0000 Subject: [PATCH 6/7] chore: add example to ssh help documentation --- cli/ssh.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/cli/ssh.go b/cli/ssh.go index 2eecaac86ed9a..5cc81284ca317 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -92,7 +92,13 @@ func (r *RootCmd) ssh() *serpent.Command { Annotations: workspaceCommand, Use: "ssh [command]", Short: "Start a shell into a workspace or run a command", - Long: "This command does not have full parity with the standard SSH command. For users who need the full functionality of SSH, create an ssh configuration with `coder config-ssh`.", + Long: "This command does not have full parity with the standard SSH command. For users who need the full functionality of SSH, create an ssh configuration with `coder config-ssh`.\n\n" + + FormatExamples( + Example{ + Description: "Use `--` to separate and pass flags directly to the command executed via SSH.", + Command: "coder ssh -- ls -la", + }, + ), Middleware: serpent.Chain( // Require at least one arg for the workspace name func(next serpent.HandlerFunc) serpent.HandlerFunc { From e0a3e7a0d49a361efd3b4b84dc216487fb725cc4 Mon Sep 17 00:00:00 2001 From: Brett Kolodny Date: Fri, 16 May 2025 13:47:15 +0000 Subject: [PATCH 7/7] chore: make gen --- cli/testdata/coder_ssh_--help.golden | 5 +++++ docs/reference/cli/ssh.md | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/cli/testdata/coder_ssh_--help.golden b/cli/testdata/coder_ssh_--help.golden index ad45226fc6579..8019dbdc2a4a4 100644 --- a/cli/testdata/coder_ssh_--help.golden +++ b/cli/testdata/coder_ssh_--help.golden @@ -8,6 +8,11 @@ USAGE: This command does not have full parity with the standard SSH command. For users who need the full functionality of SSH, create an ssh configuration with `coder config-ssh`. + + - Use `--` to separate and pass flags directly to the command executed via + SSH.: + + $ coder ssh -- ls -la OPTIONS: --disable-autostart bool, $CODER_SSH_DISABLE_AUTOSTART (default: false) diff --git a/docs/reference/cli/ssh.md b/docs/reference/cli/ssh.md index f62e2e57e99be..aaa76bd256e9e 100644 --- a/docs/reference/cli/ssh.md +++ b/docs/reference/cli/ssh.md @@ -13,6 +13,10 @@ coder ssh [flags] [command] ```console This command does not have full parity with the standard SSH command. For users who need the full functionality of SSH, create an ssh configuration with `coder config-ssh`. + + - Use `--` to separate and pass flags directly to the command executed via SSH.: + + $ coder ssh -- ls -la ``` ## Options