From 36707f9177cfb915646475701f85a9b87ab7eef1 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Wed, 19 Feb 2025 23:12:40 +0000 Subject: [PATCH 01/14] feat(agent): write up reconnectingpty server to container exec --- agent/agent_test.go | 73 +++++++++++++++++++++++++-- agent/agentssh/agentssh.go | 14 ++++- agent/agentssh/agentssh_test.go | 4 ++ agent/reconnectingpty/server.go | 14 ++++- codersdk/workspacesdk/agentconn.go | 28 ++++++++-- codersdk/workspacesdk/workspacesdk.go | 17 ++++++- 6 files changed, 139 insertions(+), 11 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index 834e0a3e68151..e44df749d31dd 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -25,8 +25,14 @@ import ( "testing" "time" + "go.uber.org/goleak" + "tailscale.com/net/speedtest" + "tailscale.com/tailcfg" + "github.com/bramvdbogaerde/go-scp" "github.com/google/uuid" + "github.com/ory/dockertest/v3" + "github.com/ory/dockertest/v3/docker" "github.com/pion/udp" "github.com/pkg/sftp" "github.com/prometheus/client_golang/prometheus" @@ -34,15 +40,13 @@ import ( "github.com/spf13/afero" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "go.uber.org/goleak" "golang.org/x/crypto/ssh" "golang.org/x/exp/slices" "golang.org/x/xerrors" - "tailscale.com/net/speedtest" - "tailscale.com/tailcfg" "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/agent" "github.com/coder/coder/v2/agent/agentssh" "github.com/coder/coder/v2/agent/agenttest" @@ -1761,6 +1765,69 @@ func TestAgent_ReconnectingPTY(t *testing.T) { } } +// This tests end-to-end functionality of connecting to a running container +// and executing a command. It creates a real Docker container and runs a +// command. As such, it does not run by default in CI. +// You can run it manually as follows: +// +// CODER_TEST_USE_DOCKER=1 go test -count=1 ./agent -run TestAgent_ReconnectingPTYContainer +func TestAgent_ReconnectingPTYContainer(t *testing.T) { + t.Parallel() + if ctud, ok := os.LookupEnv("CODER_TEST_USE_DOCKER"); !ok || ctud != "1" { + t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test") + } + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + pool, err := dockertest.NewPool("") + require.NoError(t, err, "Could not connect to docker") + ct, err := pool.RunWithOptions(&dockertest.RunOptions{ + Repository: "busybox", + Tag: "latest", + Cmd: []string{"sleep", "infnity"}, + }, func(config *docker.HostConfig) { + config.AutoRemove = true + config.RestartPolicy = docker.RestartPolicy{Name: "no"} + }) + require.NoError(t, err, "Could not start container") + // Wait for container to start + require.Eventually(t, func() bool { + ct, ok := pool.ContainerByName(ct.Container.Name) + return ok && ct.Container.State.Running + }, testutil.WaitShort, testutil.IntervalSlow, "Container did not start in time") + + // nolint: dogsled + conn, _, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0) + ac, err := conn.ReconnectingPTY(ctx, uuid.New(), 80, 80, "/bin/sh", func(arp *workspacesdk.AgentReconnectingPTYInit) { + arp.Container = ct.Container.ID + }) + require.NoError(t, err, "failed to create ReconnectingPTY") + defer ac.Close() + tr := testutil.NewTerminalReader(t, ac) + + require.NoError(t, tr.ReadUntil(ctx, func(line string) bool { + return strings.Contains(line, "#") || strings.Contains(line, "$") + }), "find prompt") + + require.NoError(t, json.NewEncoder(ac).Encode(workspacesdk.ReconnectingPTYRequest{ + Data: "hostname\r", + }), "write hostname") + require.NoError(t, tr.ReadUntil(ctx, func(line string) bool { + return strings.Contains(line, "hostname") + }), "find hostname command") + + require.NoError(t, tr.ReadUntil(ctx, func(line string) bool { + return strings.Contains(line, ct.Container.Config.Hostname) + }), "find hostname output") + require.NoError(t, json.NewEncoder(ac).Encode(workspacesdk.ReconnectingPTYRequest{ + Data: "exit\r", + }), "write exit command") + + // Wait for the connection to close. + require.ErrorIs(t, tr.ReadUntil(ctx, nil), io.EOF) +} + func TestAgent_Dial(t *testing.T) { t.Parallel() diff --git a/agent/agentssh/agentssh.go b/agent/agentssh/agentssh.go index a7e028541aa6e..b125337bb6942 100644 --- a/agent/agentssh/agentssh.go +++ b/agent/agentssh/agentssh.go @@ -708,6 +708,8 @@ type EnvInfoer interface { UserHomeDir() (string, error) // UserShell returns the shell of the given user. UserShell(username string) (string, error) + // ModifyCommand modifies the command and arguments before execution. + ModifyCommand(name string, args ...string) (string, []string) } type systemEnvInfoer struct{} @@ -737,6 +739,10 @@ func (systemEnvInfoer) UserShell(username string) (string, error) { return usershell.Get(username) } +func (systemEnvInfoer) ModifyCommand(name string, args ...string) (string, []string) { + return name, args +} + // CreateCommand processes raw command input with OpenSSH-like behavior. // If the script provided is empty, it will default to the users shell. // This injects environment variables specified by the user at launch too. @@ -802,7 +808,13 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string, } } - cmd := s.Execer.PTYCommandContext(ctx, name, args...) + // Modify command prior to execution. This will usually be a no-op, but not always. + modifiedName, modifiedArgs := deps.ModifyCommand(name, args...) + s.logger.Info(ctx, "modified command", + slog.F("before", append([]string{name}, args...)), + slog.F("after", append([]string{modifiedName}, modifiedArgs...)), + ) + cmd := s.Execer.PTYCommandContext(ctx, modifiedName, modifiedArgs...) cmd.Dir = s.config.WorkingDirectory() // If the metadata directory doesn't exist, we run the command diff --git a/agent/agentssh/agentssh_test.go b/agent/agentssh/agentssh_test.go index 378657ebee5ad..543aed499776c 100644 --- a/agent/agentssh/agentssh_test.go +++ b/agent/agentssh/agentssh_test.go @@ -140,6 +140,10 @@ func (f *fakeEnvInfoer) UserShell(u string) (string, error) { return f.UserShellFn(u) } +func (*fakeEnvInfoer) ModifyCommand(cmd string, args ...string) (string, []string) { + return cmd, args +} + func TestNewServer_CloseActiveConnections(t *testing.T) { t.Parallel() diff --git a/agent/reconnectingpty/server.go b/agent/reconnectingpty/server.go index 465667c616180..25f09e7baee5b 100644 --- a/agent/reconnectingpty/server.go +++ b/agent/reconnectingpty/server.go @@ -14,6 +14,7 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agentssh" "github.com/coder/coder/v2/codersdk/workspacesdk" ) @@ -116,7 +117,7 @@ func (s *Server) handleConn(ctx context.Context, logger slog.Logger, conn net.Co } connectionID := uuid.NewString() - connLogger := logger.With(slog.F("message_id", msg.ID), slog.F("connection_id", connectionID)) + connLogger := logger.With(slog.F("message_id", msg.ID), slog.F("connection_id", connectionID), slog.F("container", msg.Container), slog.F("container_user", msg.ContainerUser)) connLogger.Debug(ctx, "starting handler") defer func() { @@ -158,8 +159,17 @@ func (s *Server) handleConn(ctx context.Context, logger slog.Logger, conn net.Co } }() + var ei agentssh.EnvInfoer + if 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) + } + ei = dei + s.logger.Info(ctx, "got container env info", slog.F("container", msg.Container)) + } // Empty command will default to the users shell! - cmd, err := s.commandCreator.CreateCommand(ctx, msg.Command, nil, nil) + cmd, err := s.commandCreator.CreateCommand(ctx, msg.Command, nil, ei) if err != nil { s.errorsTotal.WithLabelValues("create_command").Add(1) return xerrors.Errorf("create command: %w", err) diff --git a/codersdk/workspacesdk/agentconn.go b/codersdk/workspacesdk/agentconn.go index f803f8736a6fa..6fa06c0ab5bd6 100644 --- a/codersdk/workspacesdk/agentconn.go +++ b/codersdk/workspacesdk/agentconn.go @@ -93,6 +93,24 @@ type AgentReconnectingPTYInit struct { Height uint16 Width uint16 Command string + // Container, if set, will attempt to exec into a running container visible to the agent. + // This should be a unique container ID (implementation-dependent). + Container string + // ContainerUser, if set, will set the target user when execing into a container. + // This can be a username or UID, depending on the underlying implementation. + // This is ignored if Container is not set. + ContainerUser string +} + +// AgentReconnectingPTYInitOption is a functional option for AgentReconnectingPTYInit. +type AgentReconnectingPTYInitOption func(*AgentReconnectingPTYInit) + +// AgentReconnectingPTYInitWithContainer sets the container and container user for the reconnecting PTY session. +func AgentReconnectingPTYInitWithContainer(container, containerUser string) AgentReconnectingPTYInitOption { + return func(init *AgentReconnectingPTYInit) { + init.Container = container + init.ContainerUser = containerUser + } } // ReconnectingPTYRequest is sent from the client to the server @@ -107,7 +125,7 @@ type ReconnectingPTYRequest struct { // ReconnectingPTY spawns a new reconnecting terminal session. // `ReconnectingPTYRequest` should be JSON marshaled and written to the returned net.Conn. // Raw terminal output will be read from the returned net.Conn. -func (c *AgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, width uint16, command string) (net.Conn, error) { +func (c *AgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, width uint16, command string, initOpts ...AgentReconnectingPTYInitOption) (net.Conn, error) { ctx, span := tracing.StartSpan(ctx) defer span.End() @@ -119,12 +137,16 @@ func (c *AgentConn) ReconnectingPTY(ctx context.Context, id uuid.UUID, height, w if err != nil { return nil, err } - data, err := json.Marshal(AgentReconnectingPTYInit{ + rptyInit := AgentReconnectingPTYInit{ ID: id, Height: height, Width: width, Command: command, - }) + } + for _, o := range initOpts { + o(&rptyInit) + } + data, err := json.Marshal(rptyInit) if err != nil { _ = conn.Close() return nil, err diff --git a/codersdk/workspacesdk/workspacesdk.go b/codersdk/workspacesdk/workspacesdk.go index 17b22a363d6a0..c74f95e2ef4e0 100644 --- a/codersdk/workspacesdk/workspacesdk.go +++ b/codersdk/workspacesdk/workspacesdk.go @@ -12,12 +12,14 @@ import ( "strconv" "strings" - "github.com/google/uuid" - "golang.org/x/xerrors" "tailscale.com/tailcfg" "tailscale.com/wgengine/capture" + "github.com/google/uuid" + "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/tailnet" "github.com/coder/coder/v2/tailnet/proto" @@ -305,6 +307,11 @@ type WorkspaceAgentReconnectingPTYOpts struct { // issue-reconnecting-pty-signed-token endpoint. If set, the session token // on the client will not be sent. SignedToken string + + // Container, if set, will attempt to exec into a running container visible to the agent. + // This should be a unique container ID (implementation-dependent). + Container string + ContainerUser string } // AgentReconnectingPTY spawns a PTY that reconnects using the token provided. @@ -320,6 +327,12 @@ func (c *Client) AgentReconnectingPTY(ctx context.Context, opts WorkspaceAgentRe q.Set("width", strconv.Itoa(int(opts.Width))) q.Set("height", strconv.Itoa(int(opts.Height))) q.Set("command", opts.Command) + if opts.Container != "" { + q.Set("container", opts.Container) + } + if opts.ContainerUser != "" { + q.Set("container_user", opts.ContainerUser) + } // If we're using a signed token, set the query parameter. if opts.SignedToken != "" { q.Set(codersdk.SignedAppTokenQueryParameter, opts.SignedToken) From 690e91cc263d5a8e35068328092a7e9d4d248270 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 11:52:14 +0000 Subject: [PATCH 02/14] refactor: move SystemEnvInfo to usershell --- agent/agentcontainers/containers_dockercli.go | 21 ++---- agent/agentssh/agentssh.go | 61 +++-------------- agent/reconnectingpty/server.go | 3 +- agent/usershell/usershell.go | 65 +++++++++++++++++++ agent/usershell/usershell_darwin.go | 1 + ...ell_test.go => usershell_internal_test.go} | 10 ++- agent/usershell/usershell_other.go | 1 + agent/usershell/usershell_windows.go | 1 + 8 files changed, 89 insertions(+), 74 deletions(-) create mode 100644 agent/usershell/usershell.go rename agent/usershell/{usershell_test.go => usershell_internal_test.go} (80%) diff --git a/agent/agentcontainers/containers_dockercli.go b/agent/agentcontainers/containers_dockercli.go index 64f264c1ba730..6299cad79dbc0 100644 --- a/agent/agentcontainers/containers_dockercli.go +++ b/agent/agentcontainers/containers_dockercli.go @@ -6,7 +6,6 @@ import ( "context" "encoding/json" "fmt" - "os" "os/user" "slices" "sort" @@ -15,12 +14,18 @@ import ( "time" "github.com/coder/coder/v2/agent/agentexec" + "github.com/coder/coder/v2/agent/usershell" "github.com/coder/coder/v2/codersdk" "golang.org/x/exp/maps" "golang.org/x/xerrors" ) +const ( + RuntimeSystem int = iota + RuntimeDocker +) + // DockerCLILister is a ContainerLister that lists containers using the docker CLI type DockerCLILister struct { execer agentexec.Execer @@ -37,6 +42,7 @@ func NewDocker(execer agentexec.Execer) Lister { // DockerEnvInfoer is an implementation of agentssh.EnvInfoer that returns // information about a container. type DockerEnvInfoer struct { + usershell.SystemEnvInfo container string user *user.User userShell string @@ -128,19 +134,6 @@ func (dei *DockerEnvInfoer) CurrentUser() (*user.User, error) { return &u, nil } -func (*DockerEnvInfoer) Environ() []string { - // Return a clone of the environment so that the caller can't modify it - return os.Environ() -} - -func (*DockerEnvInfoer) UserHomeDir() (string, error) { - // We default the working directory of the command to the user's home - // directory. Since this came from inside the container, we cannot guarantee - // that this exists on the host. Return the "real" home directory of the user - // instead. - return os.UserHomeDir() -} - func (dei *DockerEnvInfoer) UserShell(string) (string, error) { return dei.userShell, nil } diff --git a/agent/agentssh/agentssh.go b/agent/agentssh/agentssh.go index b125337bb6942..d4a93d1c956c6 100644 --- a/agent/agentssh/agentssh.go +++ b/agent/agentssh/agentssh.go @@ -698,51 +698,6 @@ func (s *Server) sftpHandler(logger slog.Logger, session ssh.Session) { _ = session.Exit(1) } -// EnvInfoer encapsulates external information required by CreateCommand. -type EnvInfoer interface { - // CurrentUser returns the current user. - CurrentUser() (*user.User, error) - // Environ returns the environment variables of the current process. - Environ() []string - // UserHomeDir returns the home directory of the current user. - UserHomeDir() (string, error) - // UserShell returns the shell of the given user. - UserShell(username string) (string, error) - // ModifyCommand modifies the command and arguments before execution. - ModifyCommand(name string, args ...string) (string, []string) -} - -type systemEnvInfoer struct{} - -var defaultEnvInfoer EnvInfoer = &systemEnvInfoer{} - -// DefaultEnvInfoer returns a default implementation of -// EnvInfoer. This reads information using the default Go -// implementations. -func DefaultEnvInfoer() EnvInfoer { - return defaultEnvInfoer -} - -func (systemEnvInfoer) CurrentUser() (*user.User, error) { - return user.Current() -} - -func (systemEnvInfoer) Environ() []string { - return os.Environ() -} - -func (systemEnvInfoer) UserHomeDir() (string, error) { - return userHomeDir() -} - -func (systemEnvInfoer) UserShell(username string) (string, error) { - return usershell.Get(username) -} - -func (systemEnvInfoer) ModifyCommand(name string, args ...string) (string, []string) { - return name, args -} - // CreateCommand processes raw command input with OpenSSH-like behavior. // If the script provided is empty, it will default to the users shell. // This injects environment variables specified by the user at launch too. @@ -750,17 +705,17 @@ func (systemEnvInfoer) ModifyCommand(name string, args ...string) (string, []str // alternative implementations for the dependencies of CreateCommand. // This is useful when creating a command to be run in a separate environment // (for example, a Docker container). Pass in nil to use the default. -func (s *Server) CreateCommand(ctx context.Context, script string, env []string, deps EnvInfoer) (*pty.Cmd, error) { - if deps == nil { - deps = DefaultEnvInfoer() +func (s *Server) CreateCommand(ctx context.Context, script string, env []string, ei usershell.EnvInfoer) (*pty.Cmd, error) { + if ei == nil { + ei = &usershell.SystemEnvInfo{} } - currentUser, err := deps.CurrentUser() + currentUser, err := ei.CurrentUser() if err != nil { return nil, xerrors.Errorf("get current user: %w", err) } username := currentUser.Username - shell, err := deps.UserShell(username) + shell, err := ei.UserShell(username) if err != nil { return nil, xerrors.Errorf("get user shell: %w", err) } @@ -809,7 +764,7 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string, } // Modify command prior to execution. This will usually be a no-op, but not always. - modifiedName, modifiedArgs := deps.ModifyCommand(name, args...) + modifiedName, modifiedArgs := ei.ModifyCommand(name, args...) s.logger.Info(ctx, "modified command", slog.F("before", append([]string{name}, args...)), slog.F("after", append([]string{modifiedName}, modifiedArgs...)), @@ -822,13 +777,13 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string, _, err = os.Stat(cmd.Dir) if cmd.Dir == "" || err != nil { // Default to user home if a directory is not set. - homedir, err := deps.UserHomeDir() + homedir, err := ei.UserHomeDir() if err != nil { return nil, xerrors.Errorf("get home dir: %w", err) } cmd.Dir = homedir } - cmd.Env = append(deps.Environ(), env...) + cmd.Env = append(ei.Environ(), env...) cmd.Env = append(cmd.Env, fmt.Sprintf("USER=%s", username)) // Set SSH connection environment variables (these are also set by OpenSSH diff --git a/agent/reconnectingpty/server.go b/agent/reconnectingpty/server.go index 25f09e7baee5b..264f9174f12b0 100644 --- a/agent/reconnectingpty/server.go +++ b/agent/reconnectingpty/server.go @@ -16,6 +16,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/agent/agentcontainers" "github.com/coder/coder/v2/agent/agentssh" + "github.com/coder/coder/v2/agent/usershell" "github.com/coder/coder/v2/codersdk/workspacesdk" ) @@ -159,7 +160,7 @@ func (s *Server) handleConn(ctx context.Context, logger slog.Logger, conn net.Co } }() - var ei agentssh.EnvInfoer + var ei usershell.EnvInfoer if msg.Container != "" { dei, err := agentcontainers.EnvInfo(ctx, s.commandCreator.Execer, msg.Container, msg.ContainerUser) if err != nil { diff --git a/agent/usershell/usershell.go b/agent/usershell/usershell.go new file mode 100644 index 0000000000000..7dff54e7c8db8 --- /dev/null +++ b/agent/usershell/usershell.go @@ -0,0 +1,65 @@ +package usershell + +import ( + "os" + "os/user" + + "golang.org/x/xerrors" +) + +// HomeDir returns the home directory of the current user, giving +// priority to the $HOME environment variable. +func HomeDir() (string, error) { + // First we check the environment. + homedir, err := os.UserHomeDir() + if err == nil { + return homedir, nil + } + + // As a fallback, we try the user information. + u, err := user.Current() + if err != nil { + return "", xerrors.Errorf("current user: %w", err) + } + return u.HomeDir, nil +} + +// EnvInfoer encapsulates external information about the environment. +type EnvInfoer interface { + // CurrentUser returns the current user. + CurrentUser() (*user.User, error) + // Environ returns the environment variables of the current process. + Environ() []string + // UserHomeDir returns the home directory of the current user. + UserHomeDir() (string, error) + // UserShell returns the shell of the given user. + UserShell(username string) (string, error) + // ModifyCommand modifies the command and arguments before execution based on + // the environment. This is useful for executing a command inside a container. + // In the default case, the command and arguments are returned unchanged. + ModifyCommand(name string, args ...string) (string, []string) +} + +// SystemEnvInfo encapsulates the information about the environment +// just using the default Go implementations. +type SystemEnvInfo struct{} + +func (SystemEnvInfo) CurrentUser() (*user.User, error) { + return user.Current() +} + +func (SystemEnvInfo) Environ() []string { + return os.Environ() +} + +func (SystemEnvInfo) UserHomeDir() (string, error) { + return HomeDir() +} + +func (SystemEnvInfo) UserShell(username string) (string, error) { + return Get(username) +} + +func (SystemEnvInfo) ModifyCommand(name string, args ...string) (string, []string) { + return name, args +} diff --git a/agent/usershell/usershell_darwin.go b/agent/usershell/usershell_darwin.go index 0f5be08f82631..5f221bc43ed39 100644 --- a/agent/usershell/usershell_darwin.go +++ b/agent/usershell/usershell_darwin.go @@ -10,6 +10,7 @@ import ( ) // Get returns the $SHELL environment variable. +// Deprecated: use SystemEnvInfo.UserShell instead. func Get(username string) (string, error) { // This command will output "UserShell: /bin/zsh" if successful, we // can ignore the error since we have fallback behavior. diff --git a/agent/usershell/usershell_test.go b/agent/usershell/usershell_internal_test.go similarity index 80% rename from agent/usershell/usershell_test.go rename to agent/usershell/usershell_internal_test.go index ee49afcb14412..fc5b0b82198e0 100644 --- a/agent/usershell/usershell_test.go +++ b/agent/usershell/usershell_internal_test.go @@ -1,4 +1,4 @@ -package usershell_test +package usershell import ( "os/user" @@ -6,8 +6,6 @@ import ( "testing" "github.com/stretchr/testify/require" - - "github.com/coder/coder/v2/agent/usershell" ) //nolint:paralleltest,tparallel // This test sets an environment variable. @@ -20,7 +18,7 @@ func TestGet(t *testing.T) { t.Setenv("SHELL", "/bin/sh") t.Run("NonExistentUser", func(t *testing.T) { - shell, err := usershell.Get("notauser") + shell, err := Get("notauser") require.NoError(t, err) require.Equal(t, "/bin/sh", shell) }) @@ -31,14 +29,14 @@ func TestGet(t *testing.T) { t.Setenv("SHELL", "") t.Run("NotFound", func(t *testing.T) { - _, err := usershell.Get("notauser") + _, err := Get("notauser") require.Error(t, err) }) t.Run("User", func(t *testing.T) { u, err := user.Current() require.NoError(t, err) - shell, err := usershell.Get(u.Username) + shell, err := Get(u.Username) require.NoError(t, err) require.NotEmpty(t, shell) }) diff --git a/agent/usershell/usershell_other.go b/agent/usershell/usershell_other.go index d015b7ebf4111..6ee3ad2368faf 100644 --- a/agent/usershell/usershell_other.go +++ b/agent/usershell/usershell_other.go @@ -11,6 +11,7 @@ import ( ) // Get returns the /etc/passwd entry for the username provided. +// Deprecated: use SystemEnvInfo.UserShell instead. func Get(username string) (string, error) { contents, err := os.ReadFile("/etc/passwd") if err != nil { diff --git a/agent/usershell/usershell_windows.go b/agent/usershell/usershell_windows.go index e12537bf3a99f..52823d900de99 100644 --- a/agent/usershell/usershell_windows.go +++ b/agent/usershell/usershell_windows.go @@ -3,6 +3,7 @@ package usershell import "os/exec" // Get returns the command prompt binary name. +// Deprecated: use SystemEnvInfo.UserShell instead. func Get(username string) (string, error) { _, err := exec.LookPath("pwsh.exe") if err == nil { From 92130fecc4dc5b84c8e5aed6090179c02e7646ab Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 15:14:48 +0000 Subject: [PATCH 03/14] Apply suggestions from code review Co-authored-by: Mathias Fredriksson Co-authored-by: Danny Kopping --- agent/agent_test.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index e44df749d31dd..51cda428caf67 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1773,12 +1773,11 @@ func TestAgent_ReconnectingPTY(t *testing.T) { // CODER_TEST_USE_DOCKER=1 go test -count=1 ./agent -run TestAgent_ReconnectingPTYContainer func TestAgent_ReconnectingPTYContainer(t *testing.T) { t.Parallel() - if ctud, ok := os.LookupEnv("CODER_TEST_USE_DOCKER"); !ok || ctud != "1" { + if os.Getenv("CODER_TEST_USE_DOCKER") != "1" { t.Skip("Set CODER_TEST_USE_DOCKER=1 to run this test") } - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() + ctx := testutil.Context(t, testutil.WaitLong) pool, err := dockertest.NewPool("") require.NoError(t, err, "Could not connect to docker") From 50f1c7f4b5a33e79f136596f1526372b22258d2a Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 15:27:52 +0000 Subject: [PATCH 04/14] hide behind experimental flag --- agent/agent.go | 60 ++++++++++++++++++--------------- agent/agent_test.go | 4 ++- agent/reconnectingpty/server.go | 25 ++++++++------ cli/agent.go | 9 ++--- 4 files changed, 56 insertions(+), 42 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 0b3a6b3ecd2cf..35ba008076cfd 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -68,26 +68,27 @@ const ( ) type Options struct { - Filesystem afero.Fs - LogDir string - TempDir string - ScriptDataDir string - ExchangeToken func(ctx context.Context) (string, error) - Client Client - ReconnectingPTYTimeout time.Duration - EnvironmentVariables map[string]string - Logger slog.Logger - IgnorePorts map[int]string - PortCacheDuration time.Duration - SSHMaxTimeout time.Duration - TailnetListenPort uint16 - Subsystems []codersdk.AgentSubsystem - PrometheusRegistry *prometheus.Registry - ReportMetadataInterval time.Duration - ServiceBannerRefreshInterval time.Duration - BlockFileTransfer bool - Execer agentexec.Execer - ContainerLister agentcontainers.Lister + Filesystem afero.Fs + LogDir string + TempDir string + ScriptDataDir string + ExchangeToken func(ctx context.Context) (string, error) + Client Client + ReconnectingPTYTimeout time.Duration + EnvironmentVariables map[string]string + Logger slog.Logger + IgnorePorts map[int]string + PortCacheDuration time.Duration + SSHMaxTimeout time.Duration + TailnetListenPort uint16 + Subsystems []codersdk.AgentSubsystem + PrometheusRegistry *prometheus.Registry + ReportMetadataInterval time.Duration + ServiceBannerRefreshInterval time.Duration + BlockFileTransfer bool + Execer agentexec.Execer + ContainerLister agentcontainers.Lister + ExperimentalContainersEnabled bool } type Client interface { @@ -184,10 +185,11 @@ func New(options Options) Agent { logSender: agentsdk.NewLogSender(options.Logger), blockFileTransfer: options.BlockFileTransfer, - prometheusRegistry: prometheusRegistry, - metrics: newAgentMetrics(prometheusRegistry), - execer: options.Execer, - lister: options.ContainerLister, + prometheusRegistry: prometheusRegistry, + metrics: newAgentMetrics(prometheusRegistry), + execer: options.Execer, + lister: options.ContainerLister, + experimentalDevcontainersEnabled: options.ExperimentalContainersEnabled, } // Initially, we have a closed channel, reflecting the fact that we are not initially connected. // Each time we connect we replace the channel (while holding the closeMutex) with a new one @@ -255,9 +257,10 @@ type agent struct { prometheusRegistry *prometheus.Registry // metrics are prometheus registered metrics that will be collected and // labeled in Coder with the agent + workspace. - metrics *agentMetrics - execer agentexec.Execer - lister agentcontainers.Lister + metrics *agentMetrics + execer agentexec.Execer + lister agentcontainers.Lister + experimentalDevcontainersEnabled bool } func (a *agent) TailnetConn() *tailnet.Conn { @@ -297,6 +300,9 @@ func (a *agent) init() { a.sshServer, a.metrics.connectionsTotal, a.metrics.reconnectingPTYErrors, a.reconnectingPTYTimeout, + func(s *reconnectingpty.Server) { + s.ExperimentalContainersEnabled = a.experimentalDevcontainersEnabled + }, ) go a.runLoop() } diff --git a/agent/agent_test.go b/agent/agent_test.go index 51cda428caf67..de6fccc6a6328 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1797,7 +1797,9 @@ func TestAgent_ReconnectingPTYContainer(t *testing.T) { }, testutil.WaitShort, testutil.IntervalSlow, "Container did not start in time") // nolint: dogsled - conn, _, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0) + conn, _, _, _, _ := setupAgent(t, agentsdk.Manifest{}, 0, func(_ *agenttest.Client, o *agent.Options) { + o.ExperimentalContainersEnabled = true + }) ac, err := conn.ReconnectingPTY(ctx, uuid.New(), 80, 80, "/bin/sh", func(arp *workspacesdk.AgentReconnectingPTYInit) { arp.Container = ct.Container.ID }) diff --git a/agent/reconnectingpty/server.go b/agent/reconnectingpty/server.go index 264f9174f12b0..7b4596a122458 100644 --- a/agent/reconnectingpty/server.go +++ b/agent/reconnectingpty/server.go @@ -21,27 +21,32 @@ import ( ) type Server struct { - logger slog.Logger - connectionsTotal prometheus.Counter - errorsTotal *prometheus.CounterVec - commandCreator *agentssh.Server - connCount atomic.Int64 - reconnectingPTYs sync.Map - timeout time.Duration + logger slog.Logger + connectionsTotal prometheus.Counter + errorsTotal *prometheus.CounterVec + commandCreator *agentssh.Server + connCount atomic.Int64 + reconnectingPTYs sync.Map + timeout time.Duration + ExperimentalContainersEnabled bool } // NewServer returns a new ReconnectingPTY server func NewServer(logger slog.Logger, commandCreator *agentssh.Server, connectionsTotal prometheus.Counter, errorsTotal *prometheus.CounterVec, - timeout time.Duration, + timeout time.Duration, opts ...func(*Server), ) *Server { - return &Server{ + s := &Server{ logger: logger, commandCreator: commandCreator, connectionsTotal: connectionsTotal, errorsTotal: errorsTotal, timeout: timeout, } + for _, o := range opts { + o(s) + } + return s } func (s *Server) Serve(ctx, hardCtx context.Context, l net.Listener) (retErr error) { @@ -161,7 +166,7 @@ func (s *Server) handleConn(ctx context.Context, logger slog.Logger, conn net.Co }() var ei usershell.EnvInfoer - if msg.Container != "" { + if s.ExperimentalContainersEnabled && 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) diff --git a/cli/agent.go b/cli/agent.go index e8a46a84e071c..6c1a25db02383 100644 --- a/cli/agent.go +++ b/cli/agent.go @@ -347,10 +347,11 @@ func (r *RootCmd) workspaceAgent() *serpent.Command { SSHMaxTimeout: sshMaxTimeout, Subsystems: subsystems, - PrometheusRegistry: prometheusRegistry, - BlockFileTransfer: blockFileTransfer, - Execer: execer, - ContainerLister: containerLister, + PrometheusRegistry: prometheusRegistry, + BlockFileTransfer: blockFileTransfer, + Execer: execer, + ContainerLister: containerLister, + ExperimentalContainersEnabled: devcontainersEnabled, }) promHandler := agent.PrometheusMetricsHandler(prometheusRegistry, logger) From 46a6b7d1c949f14e280094914a0de165bcb951dd Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 15:29:20 +0000 Subject: [PATCH 05/14] simply naming of interface methods --- agent/agentcontainers/containers_dockercli.go | 4 ++-- .../containers_internal_test.go | 6 +++--- agent/agentssh/agentssh.go | 6 +++--- agent/agentssh/agentssh_test.go | 6 +++--- agent/usershell/usershell.go | 18 +++++++++--------- 5 files changed, 20 insertions(+), 20 deletions(-) diff --git a/agent/agentcontainers/containers_dockercli.go b/agent/agentcontainers/containers_dockercli.go index 6299cad79dbc0..04b602f458e5b 100644 --- a/agent/agentcontainers/containers_dockercli.go +++ b/agent/agentcontainers/containers_dockercli.go @@ -128,13 +128,13 @@ func EnvInfo(ctx context.Context, execer agentexec.Execer, container, containerU return &dei, nil } -func (dei *DockerEnvInfoer) CurrentUser() (*user.User, error) { +func (dei *DockerEnvInfoer) User() (*user.User, error) { // Clone the user so that the caller can't modify it u := *dei.user return &u, nil } -func (dei *DockerEnvInfoer) UserShell(string) (string, error) { +func (dei *DockerEnvInfoer) Shell(string) (string, error) { return dei.userShell, nil } diff --git a/agent/agentcontainers/containers_internal_test.go b/agent/agentcontainers/containers_internal_test.go index cdda03f9c8200..d48b95ebd74a6 100644 --- a/agent/agentcontainers/containers_internal_test.go +++ b/agent/agentcontainers/containers_internal_test.go @@ -502,15 +502,15 @@ func TestDockerEnvInfoer(t *testing.T) { dei, err := EnvInfo(ctx, agentexec.DefaultExecer, ct.Container.ID, tt.containerUser) require.NoError(t, err, "Expected no error from DockerEnvInfo()") - u, err := dei.CurrentUser() + u, err := dei.User() require.NoError(t, err, "Expected no error from CurrentUser()") require.Equal(t, tt.expectedUsername, u.Username, "Expected username to match") - hd, err := dei.UserHomeDir() + hd, err := dei.HomeDir() require.NoError(t, err, "Expected no error from UserHomeDir()") require.NotEmpty(t, hd, "Expected user homedir to be non-empty") - sh, err := dei.UserShell(tt.containerUser) + sh, err := dei.Shell(tt.containerUser) require.NoError(t, err, "Expected no error from UserShell()") require.Equal(t, tt.expectedUserShell, sh, "Expected user shell to match") diff --git a/agent/agentssh/agentssh.go b/agent/agentssh/agentssh.go index d4a93d1c956c6..3d91100eda390 100644 --- a/agent/agentssh/agentssh.go +++ b/agent/agentssh/agentssh.go @@ -709,13 +709,13 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string, if ei == nil { ei = &usershell.SystemEnvInfo{} } - currentUser, err := ei.CurrentUser() + currentUser, err := ei.User() if err != nil { return nil, xerrors.Errorf("get current user: %w", err) } username := currentUser.Username - shell, err := ei.UserShell(username) + shell, err := ei.Shell(username) if err != nil { return nil, xerrors.Errorf("get user shell: %w", err) } @@ -777,7 +777,7 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string, _, err = os.Stat(cmd.Dir) if cmd.Dir == "" || err != nil { // Default to user home if a directory is not set. - homedir, err := ei.UserHomeDir() + homedir, err := ei.HomeDir() if err != nil { return nil, xerrors.Errorf("get home dir: %w", err) } diff --git a/agent/agentssh/agentssh_test.go b/agent/agentssh/agentssh_test.go index 543aed499776c..6b0706e95db44 100644 --- a/agent/agentssh/agentssh_test.go +++ b/agent/agentssh/agentssh_test.go @@ -124,7 +124,7 @@ type fakeEnvInfoer struct { UserShellFn func(string) (string, error) } -func (f *fakeEnvInfoer) CurrentUser() (u *user.User, err error) { +func (f *fakeEnvInfoer) User() (u *user.User, err error) { return f.CurrentUserFn() } @@ -132,11 +132,11 @@ func (f *fakeEnvInfoer) Environ() []string { return f.EnvironFn() } -func (f *fakeEnvInfoer) UserHomeDir() (string, error) { +func (f *fakeEnvInfoer) HomeDir() (string, error) { return f.UserHomeDirFn() } -func (f *fakeEnvInfoer) UserShell(u string) (string, error) { +func (f *fakeEnvInfoer) Shell(u string) (string, error) { return f.UserShellFn(u) } diff --git a/agent/usershell/usershell.go b/agent/usershell/usershell.go index 7dff54e7c8db8..081bf22f4292d 100644 --- a/agent/usershell/usershell.go +++ b/agent/usershell/usershell.go @@ -26,14 +26,14 @@ func HomeDir() (string, error) { // EnvInfoer encapsulates external information about the environment. type EnvInfoer interface { - // CurrentUser returns the current user. - CurrentUser() (*user.User, error) + // User returns the current user. + User() (*user.User, error) // Environ returns the environment variables of the current process. Environ() []string - // UserHomeDir returns the home directory of the current user. - UserHomeDir() (string, error) - // UserShell returns the shell of the given user. - UserShell(username string) (string, error) + // HomeDir returns the home directory of the current user. + HomeDir() (string, error) + // Shell returns the shell of the given user. + Shell(username string) (string, error) // ModifyCommand modifies the command and arguments before execution based on // the environment. This is useful for executing a command inside a container. // In the default case, the command and arguments are returned unchanged. @@ -44,7 +44,7 @@ type EnvInfoer interface { // just using the default Go implementations. type SystemEnvInfo struct{} -func (SystemEnvInfo) CurrentUser() (*user.User, error) { +func (SystemEnvInfo) User() (*user.User, error) { return user.Current() } @@ -52,11 +52,11 @@ func (SystemEnvInfo) Environ() []string { return os.Environ() } -func (SystemEnvInfo) UserHomeDir() (string, error) { +func (SystemEnvInfo) HomeDir() (string, error) { return HomeDir() } -func (SystemEnvInfo) UserShell(username string) (string, error) { +func (SystemEnvInfo) Shell(username string) (string, error) { return Get(username) } From 1eec238055b153ed8a6e3d3ffa3abe5cb367f2ac Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 15:30:52 +0000 Subject: [PATCH 06/14] make usershell tests external again --- .../{usershell_internal_test.go => usershell_test.go} | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) rename agent/usershell/{usershell_internal_test.go => usershell_test.go} (80%) diff --git a/agent/usershell/usershell_internal_test.go b/agent/usershell/usershell_test.go similarity index 80% rename from agent/usershell/usershell_internal_test.go rename to agent/usershell/usershell_test.go index fc5b0b82198e0..ee49afcb14412 100644 --- a/agent/usershell/usershell_internal_test.go +++ b/agent/usershell/usershell_test.go @@ -1,4 +1,4 @@ -package usershell +package usershell_test import ( "os/user" @@ -6,6 +6,8 @@ import ( "testing" "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/agent/usershell" ) //nolint:paralleltest,tparallel // This test sets an environment variable. @@ -18,7 +20,7 @@ func TestGet(t *testing.T) { t.Setenv("SHELL", "/bin/sh") t.Run("NonExistentUser", func(t *testing.T) { - shell, err := Get("notauser") + shell, err := usershell.Get("notauser") require.NoError(t, err) require.Equal(t, "/bin/sh", shell) }) @@ -29,14 +31,14 @@ func TestGet(t *testing.T) { t.Setenv("SHELL", "") t.Run("NotFound", func(t *testing.T) { - _, err := Get("notauser") + _, err := usershell.Get("notauser") require.Error(t, err) }) t.Run("User", func(t *testing.T) { u, err := user.Current() require.NoError(t, err) - shell, err := Get(u.Username) + shell, err := usershell.Get(u.Username) require.NoError(t, err) require.NotEmpty(t, shell) }) From 0aae37f8f60188b2ee9bdc1b8798d307ad6fe3d1 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 15:33:08 +0000 Subject: [PATCH 07/14] add experimental note --- codersdk/workspacesdk/workspacesdk.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/codersdk/workspacesdk/workspacesdk.go b/codersdk/workspacesdk/workspacesdk.go index c74f95e2ef4e0..9f50622635568 100644 --- a/codersdk/workspacesdk/workspacesdk.go +++ b/codersdk/workspacesdk/workspacesdk.go @@ -308,8 +308,13 @@ type WorkspaceAgentReconnectingPTYOpts struct { // on the client will not be sent. SignedToken string - // Container, if set, will attempt to exec into a running container visible to the agent. - // This should be a unique container ID (implementation-dependent). + // Experimental: Container, if set, will attempt to exec into a running container + // visible to the agent. This should be a unique container ID + // (implementation-dependent). + // ContainerUser is the user as which to exec into the container. + // NOTE: This feature is currently experimental and is currently "opt-in". + // In order to use this feature, the agent must have the environment variable + // CODER_AGENT_DEVCONTAINERS_ENABLE set to "true". Container string ContainerUser string } From 6fffa6b68854e0f63131c885c653d4ed4d164c29 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 15:34:03 +0000 Subject: [PATCH 08/14] mark usershell.HomeDir() as deprecated --- agent/usershell/usershell.go | 1 + 1 file changed, 1 insertion(+) diff --git a/agent/usershell/usershell.go b/agent/usershell/usershell.go index 081bf22f4292d..9400dc91679da 100644 --- a/agent/usershell/usershell.go +++ b/agent/usershell/usershell.go @@ -9,6 +9,7 @@ import ( // HomeDir returns the home directory of the current user, giving // priority to the $HOME environment variable. +// Deprecated: use EnvInfoer.HomeDir() instead. func HomeDir() (string, error) { // First we check the environment. homedir, err := os.UserHomeDir() From 3127cbac6d60539920c6d028caa6a0b757411d51 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 15:36:47 +0000 Subject: [PATCH 09/14] only log modified command at debug if changed --- agent/agentssh/agentssh.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/agent/agentssh/agentssh.go b/agent/agentssh/agentssh.go index 3d91100eda390..deceb95fa6d6d 100644 --- a/agent/agentssh/agentssh.go +++ b/agent/agentssh/agentssh.go @@ -765,10 +765,13 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string, // Modify command prior to execution. This will usually be a no-op, but not always. modifiedName, modifiedArgs := ei.ModifyCommand(name, args...) - s.logger.Info(ctx, "modified command", - slog.F("before", append([]string{name}, args...)), - slog.F("after", append([]string{modifiedName}, modifiedArgs...)), - ) + // Log if the command was modified. + if modifiedName != name && slices.Compare(modifiedArgs, args) != 0 { + s.logger.Debug(ctx, "modified command", + slog.F("before", append([]string{name}, args...)), + slog.F("after", append([]string{modifiedName}, modifiedArgs...)), + ) + } cmd := s.Execer.PTYCommandContext(ctx, modifiedName, modifiedArgs...) cmd.Dir = s.config.WorkingDirectory() From f4c0eaba7b26620b8825fc144c217e9be02245f5 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 15:37:26 +0000 Subject: [PATCH 10/14] clarify the why of ModifyCommand --- agent/agentssh/agentssh.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/agent/agentssh/agentssh.go b/agent/agentssh/agentssh.go index deceb95fa6d6d..d5fe945c49939 100644 --- a/agent/agentssh/agentssh.go +++ b/agent/agentssh/agentssh.go @@ -763,7 +763,9 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string, } } - // Modify command prior to execution. This will usually be a no-op, but not always. + // Modify command prior to execution. This will usually be a no-op, but not + // always. For example, to run a command in a Docker container, we need to + // modify the command to be `docker exec -it `. modifiedName, modifiedArgs := ei.ModifyCommand(name, args...) // Log if the command was modified. if modifiedName != name && slices.Compare(modifiedArgs, args) != 0 { From aafe13f137d0cacf9c3489bbb7440813bfdceac5 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 16:11:56 +0000 Subject: [PATCH 11/14] rm unused runtime iotas --- agent/agentcontainers/containers_dockercli.go | 5 ----- 1 file changed, 5 deletions(-) diff --git a/agent/agentcontainers/containers_dockercli.go b/agent/agentcontainers/containers_dockercli.go index 04b602f458e5b..27e5f835d5adb 100644 --- a/agent/agentcontainers/containers_dockercli.go +++ b/agent/agentcontainers/containers_dockercli.go @@ -21,11 +21,6 @@ import ( "golang.org/x/xerrors" ) -const ( - RuntimeSystem int = iota - RuntimeDocker -) - // DockerCLILister is a ContainerLister that lists containers using the docker CLI type DockerCLILister struct { execer agentexec.Execer From 12389d8a86c74e164b89d2d89d98cc8b2112ccad Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 17:00:14 +0000 Subject: [PATCH 12/14] reduce diff size --- agent/agent.go | 57 +++++++++++++++++---------------- agent/reconnectingpty/server.go | 15 +++++---- cli/agent.go | 9 +++--- 3 files changed, 43 insertions(+), 38 deletions(-) diff --git a/agent/agent.go b/agent/agent.go index 35ba008076cfd..285636cd31344 100644 --- a/agent/agent.go +++ b/agent/agent.go @@ -68,26 +68,27 @@ const ( ) type Options struct { - Filesystem afero.Fs - LogDir string - TempDir string - ScriptDataDir string - ExchangeToken func(ctx context.Context) (string, error) - Client Client - ReconnectingPTYTimeout time.Duration - EnvironmentVariables map[string]string - Logger slog.Logger - IgnorePorts map[int]string - PortCacheDuration time.Duration - SSHMaxTimeout time.Duration - TailnetListenPort uint16 - Subsystems []codersdk.AgentSubsystem - PrometheusRegistry *prometheus.Registry - ReportMetadataInterval time.Duration - ServiceBannerRefreshInterval time.Duration - BlockFileTransfer bool - Execer agentexec.Execer - ContainerLister agentcontainers.Lister + Filesystem afero.Fs + LogDir string + TempDir string + ScriptDataDir string + ExchangeToken func(ctx context.Context) (string, error) + Client Client + ReconnectingPTYTimeout time.Duration + EnvironmentVariables map[string]string + Logger slog.Logger + IgnorePorts map[int]string + PortCacheDuration time.Duration + SSHMaxTimeout time.Duration + TailnetListenPort uint16 + Subsystems []codersdk.AgentSubsystem + PrometheusRegistry *prometheus.Registry + ReportMetadataInterval time.Duration + ServiceBannerRefreshInterval time.Duration + BlockFileTransfer bool + Execer agentexec.Execer + ContainerLister agentcontainers.Lister + ExperimentalContainersEnabled bool } @@ -185,10 +186,11 @@ func New(options Options) Agent { logSender: agentsdk.NewLogSender(options.Logger), blockFileTransfer: options.BlockFileTransfer, - prometheusRegistry: prometheusRegistry, - metrics: newAgentMetrics(prometheusRegistry), - execer: options.Execer, - lister: options.ContainerLister, + prometheusRegistry: prometheusRegistry, + metrics: newAgentMetrics(prometheusRegistry), + execer: options.Execer, + lister: options.ContainerLister, + experimentalDevcontainersEnabled: options.ExperimentalContainersEnabled, } // Initially, we have a closed channel, reflecting the fact that we are not initially connected. @@ -257,9 +259,10 @@ type agent struct { prometheusRegistry *prometheus.Registry // metrics are prometheus registered metrics that will be collected and // labeled in Coder with the agent + workspace. - metrics *agentMetrics - execer agentexec.Execer - lister agentcontainers.Lister + metrics *agentMetrics + execer agentexec.Execer + lister agentcontainers.Lister + experimentalDevcontainersEnabled bool } diff --git a/agent/reconnectingpty/server.go b/agent/reconnectingpty/server.go index 7b4596a122458..ab4ce854c789c 100644 --- a/agent/reconnectingpty/server.go +++ b/agent/reconnectingpty/server.go @@ -21,13 +21,14 @@ import ( ) type Server struct { - logger slog.Logger - connectionsTotal prometheus.Counter - errorsTotal *prometheus.CounterVec - commandCreator *agentssh.Server - connCount atomic.Int64 - reconnectingPTYs sync.Map - timeout time.Duration + logger slog.Logger + connectionsTotal prometheus.Counter + errorsTotal *prometheus.CounterVec + commandCreator *agentssh.Server + connCount atomic.Int64 + reconnectingPTYs sync.Map + timeout time.Duration + ExperimentalContainersEnabled bool } diff --git a/cli/agent.go b/cli/agent.go index 6c1a25db02383..01d6c36f7a045 100644 --- a/cli/agent.go +++ b/cli/agent.go @@ -347,10 +347,11 @@ func (r *RootCmd) workspaceAgent() *serpent.Command { SSHMaxTimeout: sshMaxTimeout, Subsystems: subsystems, - PrometheusRegistry: prometheusRegistry, - BlockFileTransfer: blockFileTransfer, - Execer: execer, - ContainerLister: containerLister, + PrometheusRegistry: prometheusRegistry, + BlockFileTransfer: blockFileTransfer, + Execer: execer, + ContainerLister: containerLister, + ExperimentalContainersEnabled: devcontainersEnabled, }) From 72d3ff1ae381b0d5dd18f6e6e46703a5a6f930c0 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 20:34:24 +0000 Subject: [PATCH 13/14] purge container --- agent/agent_test.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/agent/agent_test.go b/agent/agent_test.go index de6fccc6a6328..935309e98d873 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -1790,6 +1790,10 @@ func TestAgent_ReconnectingPTYContainer(t *testing.T) { config.RestartPolicy = docker.RestartPolicy{Name: "no"} }) require.NoError(t, err, "Could not start container") + t.Cleanup(func() { + err := pool.Purge(ct) + require.NoError(t, err, "Could not stop container") + }) // Wait for container to start require.Eventually(t, func() bool { ct, ok := pool.ContainerByName(ct.Container.Name) From 40a61faa047bb429d8ad5f565b2e654556596c11 Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Tue, 25 Feb 2025 20:58:09 +0000 Subject: [PATCH 14/14] add missing parameters for container and container_user --- coderd/workspaceapps/proxy.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/coderd/workspaceapps/proxy.go b/coderd/workspaceapps/proxy.go index 04c3dec0c6c0d..ab67e6c260349 100644 --- a/coderd/workspaceapps/proxy.go +++ b/coderd/workspaceapps/proxy.go @@ -653,6 +653,8 @@ func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { reconnect := parser.RequiredNotEmpty("reconnect").UUID(values, uuid.New(), "reconnect") height := parser.UInt(values, 80, "height") width := parser.UInt(values, 80, "width") + container := parser.String(values, "", "container") + containerUser := parser.String(values, "", "container_user") if len(parser.Errors) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid query parameters.", @@ -690,7 +692,10 @@ func (s *Server) workspaceAgentPTY(rw http.ResponseWriter, r *http.Request) { } defer release() log.Debug(ctx, "dialed workspace agent") - ptNetConn, err := agentConn.ReconnectingPTY(ctx, reconnect, uint16(height), uint16(width), r.URL.Query().Get("command")) + ptNetConn, err := agentConn.ReconnectingPTY(ctx, reconnect, uint16(height), uint16(width), r.URL.Query().Get("command"), func(arp *workspacesdk.AgentReconnectingPTYInit) { + arp.Container = container + arp.ContainerUser = containerUser + }) if err != nil { log.Debug(ctx, "dial reconnecting pty server in workspace agent", slog.Error(err)) _ = conn.Close(websocket.StatusInternalError, httpapi.WebsocketCloseSprintf("dial: %s", err))