Skip to content

feat(cli): allow SSH command to connect to running container #16726

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Feb 28, 2025
Merged

feat(cli): allow SSH command to connect to running container #16726

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
3dc994a
feat(cli): add capability for SSH command to connect to a running con…
johnstcn Feb 26, 2025
56538a9
fixup! feat(cli): add capability for SSH command to connect to a runn…
johnstcn Feb 27, 2025
7437e49
address PR comments
johnstcn Feb 27, 2025
a1d0d00
fixup! address PR comments
johnstcn Feb 27, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fixup! feat(cli): add capability for SSH command to connect to a runn… 
…ing container
  • Loading branch information
@johnstcn
johnstcn committed Feb 27, 2025
commit 56538a975f8bff2dec0cc306087ce693441cbb5b
19 changes: 13 additions & 6 deletions agent/agentssh/agentssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,14 @@ const (
// MagicSessionTypeEnvironmentVariable is used to track the purpose behind an SSH connection.
// This is stripped from any commands being executed, and is counted towards connection stats.
MagicSessionTypeEnvironmentVariable = "CODER_SSH_SESSION_TYPE"
// ContainerEnvironmentVariable is used to specify the target container for an SSH connection.
// This is stripped from any commands being executed.
// Only available if CODER_AGENT_DEVCONTAINERS_ENABLE=true.
ContainerEnvironmentVariable = "CODER_CONTAINER"
Copy link
Member

Choose a reason for hiding this comment

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

Should we be more specific about the container here? Is it only Devcontainer?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not forcing it to be :) It can be any container but the UI and coder show should only show devcontainers.

Copy link
Member

Choose a reason for hiding this comment

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

ok, so in theory a smart Coder user can run any container this way?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep, in theory. This is still 'early access' so I'm choosing to not restrict things like this until it makes sense to do so.

// ContainerUserEnvironmentVariable is used to specify the container user for
// an SSH connection.
// Only available if CODER_AGENT_DEVCONTAINERS_ENABLE=true.
ContainerUserEnvironmentVariable = "CODER_CONTAINER_USER"
Copy link
Member

Choose a reason for hiding this comment

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

No need to change CODER_CONTAINER or CODER_CONTAINER_USER values, but should we prefix the varsity MagicSession like the other?

Copy link
Member Author

Choose a reason for hiding this comment

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

I personally dislike the MAGIC_ prefix as it doesn't really impart any information to the reader (except perhaps that it's mysterious and whimsical).

)

// MagicSessionType enums.
Expand Down Expand Up @@ -330,17 +338,17 @@ func (s *sessionCloseTracker) Close() error {

func extractContainerInfo(env []string) (container, containerUser string, filteredEnv []string) {
for _, kv := range env {
if strings.HasPrefix(kv, "CODER_CONTAINER=") {
container = strings.TrimPrefix(kv, "CODER_CONTAINER=")
if strings.HasPrefix(kv, ContainerEnvironmentVariable+"=") {
container = strings.TrimPrefix(kv, ContainerEnvironmentVariable+"=")
}

if strings.HasPrefix(kv, "CODER_CONTAINER_USER=") {
containerUser = strings.TrimPrefix(kv, "CODER_CONTAINER_USER=")
if strings.HasPrefix(kv, ContainerUserEnvironmentVariable+"=") {
containerUser = strings.TrimPrefix(kv, ContainerUserEnvironmentVariable+"=")
}
}

return container, containerUser, slices.DeleteFunc(env, func(kv string) bool {
return strings.HasPrefix(kv, "CODER_CONTAINER=") || strings.HasPrefix(kv, "CODER_CONTAINER_USER=")
return strings.HasPrefix(kv, ContainerEnvironmentVariable+"=") || strings.HasPrefix(kv, ContainerUserEnvironmentVariable+"=")
})
}

Expand Down Expand Up @@ -536,7 +544,6 @@ func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, env []str
ptyLabel = "yes"
}

// plumb in envinfoer here to modify command for container exec?
var ei usershell.EnvInfoer
var err error
if s.config.ExperimentalContainersEnabled && container != "" {
Expand Down
5 changes: 3 additions & 2 deletions cli/ssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (

"cdr.dev/slog"
"cdr.dev/slog/sloggers/sloghuman"
"github.com/coder/coder/v2/agent/agentssh"
"github.com/coder/coder/v2/cli/cliui"
"github.com/coder/coder/v2/cli/cliutil"
"github.com/coder/coder/v2/coderd/autobuild/notify"
Expand Down Expand Up @@ -476,8 +477,8 @@ func (r *RootCmd) ssh() *serpent.Command {

if container != "" {
for k, v := range map[string]string{
"CODER_CONTAINER": container,
"CODER_CONTAINER_USER": containerUser,
agentssh.ContainerEnvironmentVariable: container,
agentssh.ContainerUserEnvironmentVariable: containerUser,
} {
if err := sshSession.Setenv(k, v); err != nil {
return xerrors.Errorf("setenv: %w", err)
Expand Down
14 changes: 14 additions & 0 deletions cli/ssh_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1994,6 +1994,20 @@ func TestSSH_Container(t *testing.T) {
err := inv.WithContext(ctx).Run()
require.ErrorContains(t, err, "container not found:")
})

t.Run("NotEnabled", func(t *testing.T) {
t.Parallel()

ctx := testutil.Context(t, testutil.WaitShort)
client, workspace, agentToken := setupWorkspaceForAgent(t)
_ = agenttest.New(t, client.URL, agentToken)
_ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait()

inv, root := clitest.New(t, "ssh", workspace.Name, "-c", uuid.NewString())
clitest.SetupConfig(t, client, root)
err := inv.WithContext(ctx).Run()
require.ErrorContains(t, err, "container not found:")
})
}

// tGoContext runs fn in a goroutine passing a context that will be
Expand Down
Loading