-
Notifications
You must be signed in to change notification settings - Fork 920
feat(agent): wire up agentssh server to allow exec into container #16638
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.
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
Changes from all commits
36707f9
690e91c
92130fe
50f1c7f
46a6b7d
1eec238
0aae37f
6fffa6b
3127cba
f4c0eab
aafe13f
12389d8
72d3ff1
40a61fa
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,7 +6,6 @@ import ( | |
"context" | ||
"encoding/json" | ||
"fmt" | ||
"os" | ||
"os/user" | ||
"slices" | ||
"sort" | ||
|
@@ -15,6 +14,7 @@ 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" | ||
|
@@ -37,6 +37,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 | ||
|
@@ -122,26 +123,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 (*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() | ||
} | ||
|
||
Comment on lines
-131
to
-143
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. review: we now use the embedded |
||
func (dei *DockerEnvInfoer) UserShell(string) (string, error) { | ||
func (dei *DockerEnvInfoer) Shell(string) (string, error) { | ||
return dei.userShell, nil | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -698,63 +698,24 @@ 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) | ||
} | ||
|
||
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) | ||
} | ||
|
||
Comment on lines
-701
to
-739
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. review: moved to |
||
// 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. | ||
// The final argument is an interface that allows the caller to provide | ||
// 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.User() | ||
if err != nil { | ||
return nil, xerrors.Errorf("get current user: %w", err) | ||
} | ||
username := currentUser.Username | ||
|
||
shell, err := deps.UserShell(username) | ||
shell, err := ei.Shell(username) | ||
if err != nil { | ||
return nil, xerrors.Errorf("get user shell: %w", err) | ||
} | ||
|
@@ -802,21 +763,32 @@ 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. For example, to run a command in a Docker container, we need to | ||
// modify the command to be `docker exec -it <container> <command>`. | ||
modifiedName, modifiedArgs := ei.ModifyCommand(name, args...) | ||
johnstcn marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// 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...) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Didn't we also have a docker execer? Did we end up not implementing that change, and are only relying on env infoer now? Or are we modifying commands from multiple angles? If former, all good, if latter, I'm wondering if we could unify the concepts (perhaps infoer can be applied on the execer level or vice-versa). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's all in the envinfoer now. |
||
cmd.Dir = s.config.WorkingDirectory() | ||
|
||
// If the metadata directory doesn't exist, we run the command | ||
// in the users home directory. | ||
_, 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.HomeDir() | ||
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 | ||
|
Uh oh!
There was an error while loading. Please reload this page.