Skip to content

Commit 172e523

Browse files
johnstcnmafredridannykopping
authored
feat(agent): wire up agentssh server to allow exec into container (coder#16638)
Builds on top of coder#16623 and wires up the ReconnectingPTY server. This does nothing to wire up the web terminal yet but the added test demonstrates the functionality working. Other changes: * Refactors and moves the `SystemEnvInfo` interface to the `agent/usershell` package to address follow-up from coder#16623 (comment) * Marks `usershellinfo.Get` as deprecated. Consumers should use the `EnvInfoer` interface instead. --------- Co-authored-by: Mathias Fredriksson <mafredri@gmail.com> Co-authored-by: Danny Kopping <danny@coder.com>
1 parent a322339 commit 172e523

File tree

15 files changed

+260
-82
lines changed

15 files changed

+260
-82
lines changed

‎agent/agent.go

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,8 @@ type Options struct {
8888
BlockFileTransfer bool
8989
Execer agentexec.Execer
9090
ContainerLister agentcontainers.Lister
91+
92+
ExperimentalContainersEnabled bool
9193
}
9294

9395
type Client interface {
@@ -188,6 +190,8 @@ func New(options Options) Agent {
188190
metrics: newAgentMetrics(prometheusRegistry),
189191
execer: options.Execer,
190192
lister: options.ContainerLister,
193+
194+
experimentalDevcontainersEnabled: options.ExperimentalContainersEnabled,
191195
}
192196
// Initially, we have a closed channel, reflecting the fact that we are not initially connected.
193197
// Each time we connect we replace the channel (while holding the closeMutex) with a new one
@@ -258,6 +262,8 @@ type agent struct {
258262
metrics *agentMetrics
259263
execer agentexec.Execer
260264
lister agentcontainers.Lister
265+
266+
experimentalDevcontainersEnabled bool
261267
}
262268

263269
func (a *agent) TailnetConn() *tailnet.Conn {
@@ -297,6 +303,9 @@ func (a *agent) init() {
297303
a.sshServer,
298304
a.metrics.connectionsTotal, a.metrics.reconnectingPTYErrors,
299305
a.reconnectingPTYTimeout,
306+
func(s *reconnectingpty.Server) {
307+
s.ExperimentalContainersEnabled = a.experimentalDevcontainersEnabled
308+
},
300309
)
301310
go a.runLoop()
302311
}

‎agent/agent_test.go

Lines changed: 75 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,24 +25,28 @@ import (
2525
"testing"
2626
"time"
2727

28+
"go.uber.org/goleak"
29+
"tailscale.com/net/speedtest"
30+
"tailscale.com/tailcfg"
31+
2832
"github.com/bramvdbogaerde/go-scp"
2933
"github.com/google/uuid"
34+
"github.com/ory/dockertest/v3"
35+
"github.com/ory/dockertest/v3/docker"
3036
"github.com/pion/udp"
3137
"github.com/pkg/sftp"
3238
"github.com/prometheus/client_golang/prometheus"
3339
promgo "github.com/prometheus/client_model/go"
3440
"github.com/spf13/afero"
3541
"github.com/stretchr/testify/assert"
3642
"github.com/stretchr/testify/require"
37-
"go.uber.org/goleak"
3843
"golang.org/x/crypto/ssh"
3944
"golang.org/x/exp/slices"
4045
"golang.org/x/xerrors"
41-
"tailscale.com/net/speedtest"
42-
"tailscale.com/tailcfg"
4346

4447
"cdr.dev/slog"
4548
"cdr.dev/slog/sloggers/slogtest"
49+
4650
"github.com/coder/coder/v2/agent"
4751
"github.com/coder/coder/v2/agent/agentssh"
4852
"github.com/coder/coder/v2/agent/agenttest"
@@ -1761,6 +1765,74 @@ func TestAgent_ReconnectingPTY(t *testing.T) {
17611765
}
17621766
}
17631767

1768+
// This tests end-to-end functionality of connecting to a running container
1769+
// and executing a command. It creates a real Docker container and runs a
1770+
// command. As such, it does not run by default in CI.
1771+
// You can run it manually as follows:
1772+
//
1773+
// CODER_TEST_USE_DOCKER=1 go test -count=1 ./agent -run TestAgent_ReconnectingPTYContainer
1774+
func TestAgent_ReconnectingPTYContainer(t *testing.T) {
1775+
t.Parallel()
1776+
if os.Getenv("CODER_TEST_USE_DOCKER") != "1" {
1777+
t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test")
1778+
}
1779+
1780+
ctx := testutil.Context(t, testutil.WaitLong)
1781+
1782+
pool, err := dockertest.NewPool("")
1783+
require.NoError(t, err, "Could not connect to docker")
1784+
ct, err := pool.RunWithOptions(&dockertest.RunOptions{
1785+
Repository: "busybox",
1786+
Tag: "latest",
1787+
Cmd: []string{"sleep", "infnity"},
1788+
}, func(config *docker.HostConfig) {
1789+
config.AutoRemove = true
1790+
config.RestartPolicy = docker.RestartPolicy{Name: "no"}
1791+
})
1792+
require.NoError(t, err, "Could not start container")
1793+
t.Cleanup(func() {
1794+
err := pool.Purge(ct)
1795+
require.NoError(t, err, "Could not stop container")
1796+
})
1797+
// Wait for container to start
1798+
require.Eventually(t, func() bool {
1799+
ct, ok := pool.ContainerByName(ct.Container.Name)
1800+
return ok && ct.Container.State.Running
1801+
}, testutil.WaitShort, testutil.IntervalSlow, "Container did not start in time")
1802+
1803+
// nolint: dogsled
1804+
conn, _, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0, func(_ *agenttest.Client, o *agent.Options) {
1805+
o.ExperimentalContainersEnabled = true
1806+
})
1807+
ac, err := conn.ReconnectingPTY(ctx, uuid.New(), 80, 80, "/bin/sh", func(arp *workspacesdk.AgentReconnectingPTYInit) {
1808+
arp.Container = ct.Container.ID
1809+
})
1810+
require.NoError(t, err, "failed to create ReconnectingPTY")
1811+
defer ac.Close()
1812+
tr := testutil.NewTerminalReader(t, ac)
1813+
1814+
require.NoError(t, tr.ReadUntil(ctx, func(line string) bool {
1815+
return strings.Contains(line, "#") || strings.Contains(line, "$")
1816+
}), "find prompt")
1817+
1818+
require.NoError(t, json.NewEncoder(ac).Encode(workspacesdk.ReconnectingPTYRequest{
1819+
Data: "hostname\r",
1820+
}), "write hostname")
1821+
require.NoError(t, tr.ReadUntil(ctx, func(line string) bool {
1822+
return strings.Contains(line, "hostname")
1823+
}), "find hostname command")
1824+
1825+
require.NoError(t, tr.ReadUntil(ctx, func(line string) bool {
1826+
return strings.Contains(line, ct.Container.Config.Hostname)
1827+
}), "find hostname output")
1828+
require.NoError(t, json.NewEncoder(ac).Encode(workspacesdk.ReconnectingPTYRequest{
1829+
Data: "exit\r",
1830+
}), "write exit command")
1831+
1832+
// Wait for the connection to close.
1833+
require.ErrorIs(t, tr.ReadUntil(ctx, nil), io.EOF)
1834+
}
1835+
17641836
func TestAgent_Dial(t *testing.T) {
17651837
t.Parallel()
17661838

‎agent/agentcontainers/containers_dockercli.go

Lines changed: 4 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@ import (
66
"context"
77
"encoding/json"
88
"fmt"
9-
"os"
109
"os/user"
1110
"slices"
1211
"sort"
@@ -15,6 +14,7 @@ import (
1514
"time"
1615

1716
"github.com/coder/coder/v2/agent/agentexec"
17+
"github.com/coder/coder/v2/agent/usershell"
1818
"github.com/coder/coder/v2/codersdk"
1919

2020
"golang.org/x/exp/maps"
@@ -37,6 +37,7 @@ func NewDocker(execer agentexec.Execer) Lister {
3737
// DockerEnvInfoer is an implementation of agentssh.EnvInfoer that returns
3838
// information about a container.
3939
type DockerEnvInfoer struct {
40+
usershell.SystemEnvInfo
4041
container string
4142
user *user.User
4243
userShell string
@@ -122,26 +123,13 @@ func EnvInfo(ctx context.Context, execer agentexec.Execer, container, containerU
122123
return &dei, nil
123124
}
124125

125-
func (dei *DockerEnvInfoer) CurrentUser() (*user.User, error) {
126+
func (dei *DockerEnvInfoer) User() (*user.User, error) {
126127
// Clone the user so that the caller can't modify it
127128
u := *dei.user
128129
return &u, nil
129130
}
130131

131-
func (*DockerEnvInfoer) Environ() []string {
132-
// Return a clone of the environment so that the caller can't modify it
133-
return os.Environ()
134-
}
135-
136-
func (*DockerEnvInfoer) UserHomeDir() (string, error) {
137-
// We default the working directory of the command to the user's home
138-
// directory. Since this came from inside the container, we cannot guarantee
139-
// that this exists on the host. Return the "real" home directory of the user
140-
// instead.
141-
return os.UserHomeDir()
142-
}
143-
144-
func (dei *DockerEnvInfoer) UserShell(string) (string, error) {
132+
func (dei *DockerEnvInfoer) Shell(string) (string, error) {
145133
return dei.userShell, nil
146134
}
147135

‎agent/agentcontainers/containers_internal_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -502,15 +502,15 @@ func TestDockerEnvInfoer(t *testing.T) {
502502
dei, err := EnvInfo(ctx, agentexec.DefaultExecer, ct.Container.ID, tt.containerUser)
503503
require.NoError(t, err, "Expected no error from DockerEnvInfo()")
504504

505-
u, err := dei.CurrentUser()
505+
u, err := dei.User()
506506
require.NoError(t, err, "Expected no error from CurrentUser()")
507507
require.Equal(t, tt.expectedUsername, u.Username, "Expected username to match")
508508

509-
hd, err := dei.UserHomeDir()
509+
hd, err := dei.HomeDir()
510510
require.NoError(t, err, "Expected no error from UserHomeDir()")
511511
require.NotEmpty(t, hd, "Expected user homedir to be non-empty")
512512

513-
sh, err := dei.UserShell(tt.containerUser)
513+
sh, err := dei.Shell(tt.containerUser)
514514
require.NoError(t, err, "Expected no error from UserShell()")
515515
require.Equal(t, tt.expectedUserShell, sh, "Expected user shell to match")
516516

‎agent/agentssh/agentssh.go

Lines changed: 19 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -698,63 +698,24 @@ func (s *Server) sftpHandler(logger slog.Logger, session ssh.Session) {
698698
_ = session.Exit(1)
699699
}
700700

701-
// EnvInfoer encapsulates external information required by CreateCommand.
702-
type EnvInfoer interface {
703-
// CurrentUser returns the current user.
704-
CurrentUser() (*user.User, error)
705-
// Environ returns the environment variables of the current process.
706-
Environ() []string
707-
// UserHomeDir returns the home directory of the current user.
708-
UserHomeDir() (string, error)
709-
// UserShell returns the shell of the given user.
710-
UserShell(username string) (string, error)
711-
}
712-
713-
type systemEnvInfoer struct{}
714-
715-
var defaultEnvInfoer EnvInfoer = &systemEnvInfoer{}
716-
717-
// DefaultEnvInfoer returns a default implementation of
718-
// EnvInfoer. This reads information using the default Go
719-
// implementations.
720-
func DefaultEnvInfoer() EnvInfoer {
721-
return defaultEnvInfoer
722-
}
723-
724-
func (systemEnvInfoer) CurrentUser() (*user.User, error) {
725-
return user.Current()
726-
}
727-
728-
func (systemEnvInfoer) Environ() []string {
729-
return os.Environ()
730-
}
731-
732-
func (systemEnvInfoer) UserHomeDir() (string, error) {
733-
return userHomeDir()
734-
}
735-
736-
func (systemEnvInfoer) UserShell(username string) (string, error) {
737-
return usershell.Get(username)
738-
}
739-
740701
// CreateCommand processes raw command input with OpenSSH-like behavior.
741702
// If the script provided is empty, it will default to the users shell.
742703
// This injects environment variables specified by the user at launch too.
743704
// The final argument is an interface that allows the caller to provide
744705
// alternative implementations for the dependencies of CreateCommand.
745706
// This is useful when creating a command to be run in a separate environment
746707
// (for example, a Docker container). Pass in nil to use the default.
747-
func (s *Server) CreateCommand(ctx context.Context, script string, env []string, deps EnvInfoer) (*pty.Cmd, error) {
748-
if deps == nil {
749-
deps = DefaultEnvInfoer()
708+
func (s *Server) CreateCommand(ctx context.Context, script string, env []string, ei usershell.EnvInfoer) (*pty.Cmd, error) {
709+
if ei == nil {
710+
ei = &usershell.SystemEnvInfo{}
750711
}
751-
currentUser, err := deps.CurrentUser()
712+
currentUser, err := ei.User()
752713
if err != nil {
753714
return nil, xerrors.Errorf("get current user: %w", err)
754715
}
755716
username := currentUser.Username
756717

757-
shell, err := deps.UserShell(username)
718+
shell, err := ei.Shell(username)
758719
if err != nil {
759720
return nil, xerrors.Errorf("get user shell: %w", err)
760721
}
@@ -802,21 +763,32 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
802763
}
803764
}
804765

805-
cmd := s.Execer.PTYCommandContext(ctx, name, args...)
766+
// Modify command prior to execution. This will usually be a no-op, but not
767+
// always. For example, to run a command in a Docker container, we need to
768+
// modify the command to be `docker exec -it <container> <command>`.
769+
modifiedName, modifiedArgs := ei.ModifyCommand(name, args...)
770+
// Log if the command was modified.
771+
if modifiedName != name && slices.Compare(modifiedArgs, args) != 0 {
772+
s.logger.Debug(ctx, "modified command",
773+
slog.F("before", append([]string{name}, args...)),
774+
slog.F("after", append([]string{modifiedName}, modifiedArgs...)),
775+
)
776+
}
777+
cmd := s.Execer.PTYCommandContext(ctx, modifiedName, modifiedArgs...)
806778
cmd.Dir = s.config.WorkingDirectory()
807779

808780
// If the metadata directory doesn't exist, we run the command
809781
// in the users home directory.
810782
_, err = os.Stat(cmd.Dir)
811783
if cmd.Dir == "" || err != nil {
812784
// Default to user home if a directory is not set.
813-
homedir, err := deps.UserHomeDir()
785+
homedir, err := ei.HomeDir()
814786
if err != nil {
815787
return nil, xerrors.Errorf("get home dir: %w", err)
816788
}
817789
cmd.Dir = homedir
818790
}
819-
cmd.Env = append(deps.Environ(), env...)
791+
cmd.Env = append(ei.Environ(), env...)
820792
cmd.Env = append(cmd.Env, fmt.Sprintf("USER=%s", username))
821793

822794
// Set SSH connection environment variables (these are also set by OpenSSH

‎agent/agentssh/agentssh_test.go

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,22 +124,26 @@ type fakeEnvInfoer struct {
124124
UserShellFn func(string) (string, error)
125125
}
126126

127-
func (f *fakeEnvInfoer) CurrentUser() (u *user.User, err error) {
127+
func (f *fakeEnvInfoer) User() (u *user.User, err error) {
128128
return f.CurrentUserFn()
129129
}
130130

131131
func (f *fakeEnvInfoer) Environ() []string {
132132
return f.EnvironFn()
133133
}
134134

135-
func (f *fakeEnvInfoer) UserHomeDir() (string, error) {
135+
func (f *fakeEnvInfoer) HomeDir() (string, error) {
136136
return f.UserHomeDirFn()
137137
}
138138

139-
func (f *fakeEnvInfoer) UserShell(u string) (string, error) {
139+
func (f *fakeEnvInfoer) Shell(u string) (string, error) {
140140
return f.UserShellFn(u)
141141
}
142142

143+
func (*fakeEnvInfoer) ModifyCommand(cmd string, args ...string) (string, []string) {
144+
return cmd, args
145+
}
146+
143147
func TestNewServer_CloseActiveConnections(t *testing.T) {
144148
t.Parallel()
145149

0 commit comments

Comments
 (0)