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 all commits
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
12 changes: 7 additions & 5 deletions agent/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,8 @@ type Options struct {
Execer agentexec.Execer
ContainerLister agentcontainers.Lister

ExperimentalContainersEnabled bool
ExperimentalConnectionReports bool
ExperimentalConnectionReports bool
ExperimentalDevcontainersEnabled bool
}

type Client interface {
Expand Down Expand Up @@ -156,7 +156,7 @@ func New(options Options) Agent {
options.Execer = agentexec.DefaultExecer
}
if options.ContainerLister == nil {
options.ContainerLister = agentcontainers.NewDocker(options.Execer)
options.ContainerLister = agentcontainers.NoopLister{}
}

hardCtx, hardCancel := context.WithCancel(context.Background())
Expand Down Expand Up @@ -195,7 +195,7 @@ func New(options Options) Agent {
execer: options.Execer,
lister: options.ContainerLister,

experimentalDevcontainersEnabled: options.ExperimentalContainersEnabled,
experimentalDevcontainersEnabled: options.ExperimentalDevcontainersEnabled,
experimentalConnectionReports: options.ExperimentalConnectionReports,
}
// Initially, we have a closed channel, reflecting the fact that we are not initially connected.
Expand Down Expand Up @@ -307,6 +307,8 @@ func (a *agent) init() {

return a.reportConnection(id, connectionType, ip)
},

ExperimentalDevContainersEnabled: a.experimentalDevcontainersEnabled,
})
if err != nil {
panic(err)
Expand Down Expand Up @@ -335,7 +337,7 @@ func (a *agent) init() {
a.metrics.connectionsTotal, a.metrics.reconnectingPTYErrors,
a.reconnectingPTYTimeout,
func(s *reconnectingpty.Server) {
s.ExperimentalContainersEnabled = a.experimentalDevcontainersEnabled
s.ExperimentalDevcontainersEnabled = a.experimentalDevcontainersEnabled
},
)
go a.runLoop()
Expand Down
2 changes: 1 addition & 1 deletion agent/agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1841,7 +1841,7 @@ func TestAgent_ReconnectingPTYContainer(t *testing.T) {

// nolint: dogsled
conn, _, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0, func(_ *agenttest.Client, o *agent.Options) {
o.ExperimentalContainersEnabled = true
o.ExperimentalDevcontainersEnabled = true
})
ac, err := conn.ReconnectingPTY(ctx, uuid.New(), 80, 80, "/bin/sh", func(arp *workspacesdk.AgentReconnectingPTYInit) {
arp.Container = ct.Container.ID
Expand Down
70 changes: 58 additions & 12 deletions agent/agentssh/agentssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import (

"cdr.dev/slog"

"github.com/coder/coder/v2/agent/agentcontainers"
"github.com/coder/coder/v2/agent/agentexec"
"github.com/coder/coder/v2/agent/agentrsa"
"github.com/coder/coder/v2/agent/usershell"
Expand Down Expand Up @@ -60,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 @@ -104,6 +113,9 @@ type Config struct {
BlockFileTransfer bool
// ReportConnection.
ReportConnection reportConnectionFunc
// Experimental: allow connecting to running containers if
// CODER_AGENT_DEVCONTAINERS_ENABLE=true.
ExperimentalDevContainersEnabled bool
}

type Server struct {
Expand Down Expand Up @@ -324,6 +336,22 @@ func (s *sessionCloseTracker) Close() error {
return s.Session.Close()
}

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

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

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

func (s *Server) sessionHandler(session ssh.Session) {
ctx := session.Context()
id := uuid.New()
Expand Down Expand Up @@ -353,6 +381,7 @@ func (s *Server) sessionHandler(session ssh.Session) {
defer s.trackSession(session, false)

reportSession := true

switch magicType {
case MagicSessionTypeVSCode:
s.connCountVSCode.Add(1)
Expand Down Expand Up @@ -395,9 +424,22 @@ func (s *Server) sessionHandler(session ssh.Session) {
return
}

container, containerUser, env := extractContainerInfo(env)
if container != "" {
s.logger.Debug(ctx, "container info",
slog.F("container", container),
slog.F("container_user", containerUser),
)
}

switch ss := session.Subsystem(); ss {
case "":
case "sftp":
if s.config.ExperimentalDevContainersEnabled && container != "" {
closeCause("sftp not yet supported with containers")
_ = session.Exit(1)
return
}
err := s.sftpHandler(logger, session)
if err != nil {
closeCause(err.Error())
Expand All @@ -422,7 +464,7 @@ func (s *Server) sessionHandler(session ssh.Session) {
env = append(env, fmt.Sprintf("DISPLAY=localhost:%d.%d", display, x11.ScreenNumber))
}

err := s.sessionStart(logger, session, env, magicType)
err := s.sessionStart(logger, session, env, magicType, container, containerUser)
var exitError *exec.ExitError
if xerrors.As(err, &exitError) {
code := exitError.ExitCode()
Expand Down Expand Up @@ -495,30 +537,34 @@ func (s *Server) fileTransferBlocked(session ssh.Session) bool {
return false
}

func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, env []string, magicType MagicSessionType) (retErr error) {
func (s *Server) sessionStart(logger slog.Logger, session ssh.Session, env []string, magicType MagicSessionType, container, containerUser string) (retErr error) {
Copy link
Member Author

Choose a reason for hiding this comment

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

review: think it might be time to consider an opts struct?

Copy link
Member

Choose a reason for hiding this comment

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

It could be done, but no need to do that yet IMO. If this grows more then probably.

I'd actually want us to refactor the whole session handling and manage our own *session in session.go or some such. But that's a story for another time.

ctx := session.Context()

magicTypeLabel := magicTypeMetricLabel(magicType)
sshPty, windowSize, isPty := session.Pty()
ptyLabel := "no"
if isPty {
ptyLabel = "yes"
}

cmd, err := s.CreateCommand(ctx, session.RawCommand(), env, nil)
if err != nil {
ptyLabel := "no"
if isPty {
ptyLabel = "yes"
var ei usershell.EnvInfoer
var err error
if s.config.ExperimentalDevContainersEnabled && container != "" {
ei, err = agentcontainers.EnvInfo(ctx, s.Execer, container, containerUser)
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, ptyLabel, "container_env_info").Add(1)
return err
}
}
cmd, err := s.CreateCommand(ctx, session.RawCommand(), env, ei)
if err != nil {
s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, ptyLabel, "create_command").Add(1)
return err
}

if ssh.AgentRequested(session) {
l, err := ssh.NewAgentListener()
if err != nil {
ptyLabel := "no"
if isPty {
ptyLabel = "yes"
}

s.metrics.sessionErrors.WithLabelValues(magicTypeLabel, ptyLabel, "listener").Add(1)
return xerrors.Errorf("new agent listener: %w", err)
}
Expand Down
4 changes: 2 additions & 2 deletions agent/reconnectingpty/server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ type Server struct {
reconnectingPTYs sync.Map
timeout time.Duration

ExperimentalContainersEnabled bool
ExperimentalDevcontainersEnabled bool
}

// NewServer returns a new ReconnectingPTY server
Expand Down Expand Up @@ -187,7 +187,7 @@ func (s *Server) handleConn(ctx context.Context, logger slog.Logger, conn net.Co
}()

var ei usershell.EnvInfoer
if s.ExperimentalContainersEnabled && msg.Container != "" {
if s.ExperimentalDevcontainersEnabled && msg.Container != "" {
dei, err := agentcontainers.EnvInfo(ctx, s.commandCreator.Execer, msg.Container, msg.ContainerUser)
if err != nil {
return xerrors.Errorf("get container env info: %w", err)
Expand Down
44 changes: 22 additions & 22 deletions cli/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,24 +38,24 @@ import (

func (r *RootCmd) workspaceAgent() *serpent.Command {
var (
auth string
logDir string
scriptDataDir string
pprofAddress string
noReap bool
sshMaxTimeout time.Duration
tailnetListenPort int64
prometheusAddress string
debugAddress string
slogHumanPath string
slogJSONPath string
slogStackdriverPath string
blockFileTransfer bool
agentHeaderCommand string
agentHeader []string
devcontainersEnabled bool

experimentalConnectionReports bool
auth string
logDir string
scriptDataDir string
pprofAddress string
noReap bool
sshMaxTimeout time.Duration
tailnetListenPort int64
prometheusAddress string
debugAddress string
slogHumanPath string
slogJSONPath string
slogStackdriverPath string
blockFileTransfer bool
agentHeaderCommand string
agentHeader []string

experimentalConnectionReports bool
experimentalDevcontainersEnabled bool
)
cmd := &serpent.Command{
Use: "agent",
Expand Down Expand Up @@ -319,7 +319,7 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
}

var containerLister agentcontainers.Lister
if !devcontainersEnabled {
if !experimentalDevcontainersEnabled {
logger.Info(ctx, "agent devcontainer detection not enabled")
containerLister = &agentcontainers.NoopLister{}
} else {
Expand Down Expand Up @@ -358,8 +358,8 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
Execer: execer,
ContainerLister: containerLister,

ExperimentalContainersEnabled: devcontainersEnabled,
ExperimentalConnectionReports: experimentalConnectionReports,
ExperimentalDevcontainersEnabled: experimentalDevcontainersEnabled,
ExperimentalConnectionReports: experimentalConnectionReports,
})

promHandler := agent.PrometheusMetricsHandler(prometheusRegistry, logger)
Expand Down Expand Up @@ -487,7 +487,7 @@ func (r *RootCmd) workspaceAgent() *serpent.Command {
Default: "false",
Env: "CODER_AGENT_DEVCONTAINERS_ENABLE",
Description: "Allow the agent to automatically detect running devcontainers.",
Value: serpent.BoolOf(&devcontainersEnabled),
Value: serpent.BoolOf(&experimentalDevcontainersEnabled),
},
{
Flag: "experimental-connection-reports-enable",
Expand Down
4 changes: 3 additions & 1 deletion cli/exp_rpty_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/ory/dockertest/v3/docker"

"github.com/coder/coder/v2/agent"
"github.com/coder/coder/v2/agent/agentcontainers"
"github.com/coder/coder/v2/agent/agenttest"
"github.com/coder/coder/v2/cli/clitest"
"github.com/coder/coder/v2/coderd/coderdtest"
Expand Down Expand Up @@ -88,7 +89,8 @@ func TestExpRpty(t *testing.T) {
})

_ = agenttest.New(t, client.URL, agentToken, func(o *agent.Options) {
o.ExperimentalContainersEnabled = true
o.ExperimentalDevcontainersEnabled = true
o.ContainerLister = agentcontainers.NewDocker(o.Execer)
})
_ = coderdtest.NewWorkspaceAgentWaiter(t, client, workspace.ID).Wait()

Expand Down
56 changes: 56 additions & 0 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 @@ -76,6 +77,9 @@ func (r *RootCmd) ssh() *serpent.Command {
appearanceConfig codersdk.AppearanceConfig
networkInfoDir string
networkInfoInterval time.Duration

containerName string
containerUser string
)
client := new(codersdk.Client)
cmd := &serpent.Command{
Expand Down Expand Up @@ -282,6 +286,34 @@ func (r *RootCmd) ssh() *serpent.Command {
}
conn.AwaitReachable(ctx)

if containerName != "" {
cts, err := client.WorkspaceAgentListContainers(ctx, workspaceAgent.ID, nil)
if err != nil {
return xerrors.Errorf("list containers: %w", err)
}
if len(cts.Containers) == 0 {
cliui.Info(inv.Stderr, "No containers found!")
cliui.Info(inv.Stderr, "Tip: Agent container integration is experimental and not enabled by default.")
cliui.Info(inv.Stderr, " To enable it, set CODER_AGENT_DEVCONTAINERS_ENABLE=true in your template.")
return nil
}
var found bool
for _, c := range cts.Containers {
if c.FriendlyName == containerName || c.ID == containerName {
found = true
break
}
}
if !found {
availableContainers := make([]string, len(cts.Containers))
for i, c := range cts.Containers {
availableContainers[i] = c.FriendlyName
}
cliui.Errorf(inv.Stderr, "Container not found: %q\nAvailable containers: %v", containerName, availableContainers)
return nil
}
}

stopPolling := tryPollWorkspaceAutostop(ctx, client, workspace)
defer stopPolling()

Expand Down Expand Up @@ -454,6 +486,17 @@ func (r *RootCmd) ssh() *serpent.Command {
}
}

if containerName != "" {
for k, v := range map[string]string{
agentssh.ContainerEnvironmentVariable: containerName,
agentssh.ContainerUserEnvironmentVariable: containerUser,
} {
if err := sshSession.Setenv(k, v); err != nil {
return xerrors.Errorf("setenv: %w", err)
}
}
}

err = sshSession.RequestPty("xterm-256color", 128, 128, gossh.TerminalModes{})
if err != nil {
return xerrors.Errorf("request pty: %w", err)
Expand Down Expand Up @@ -594,6 +637,19 @@ func (r *RootCmd) ssh() *serpent.Command {
Default: "5s",
Value: serpent.DurationOf(&networkInfoInterval),
},
{
Flag: "container",
FlagShorthand: "c",
Description: "Specifies a container inside the workspace to connect to.",
Value: serpent.StringOf(&containerName),
Hidden: true, // Hidden until this features is at least in beta.
},
{
Flag: "container-user",
Description: "When connecting to a container, specifies the user to connect as.",
Value: serpent.StringOf(&containerUser),
Hidden: true, // Hidden until this features is at least in beta.
},
sshDisableAutostartOption(serpent.BoolOf(&disableAutostart)),
}
return cmd
Expand Down
Loading
Loading