8000 chore(agent/agentssh): extract EnvInfoer by johnstcn · Pull Request #16603 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content

chore(agent/agentssh): extract EnvInfoer #16603

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 19, 2025
Merged

chore(agent/agentssh): extract EnvInfoer #16603

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7b28cfa
chore(agent/agentssh): extract CreateCommandDeps
johnstcn Feb 18, 2025
251d1d2
make fmt
johnstcn Feb 18, 2025
1259518
EnvInfoer is so Go
johnstcn Feb 18, 2025
c5f45e4
add test for custom implementation
johnstcn Feb 18, 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
EnvInfoer is so Go
  • Loading branch information
@johnstcn
johnstcn committed Feb 18, 2025
commit 1259518199ec3e7521d607bf425874dc29f074e3
28 changes: 14 additions & 14 deletions agent/agentssh/agentssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -670,8 +670,8 @@ func (s *Server) sftpHandler(logger slog.Logger, session ssh.Session) {
_ = session.Exit(1)
}

// CreateCommandDeps encapsulates external information required by CreateCommand.
type CreateCommandDeps interface {
// 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.
Expand All @@ -682,30 +682,30 @@ type CreateCommandDeps interface {
UserShell(username string) (string, error)
Copy link
Member

Choose a reason for hiding this comment

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

I feel like all these methods are related to the environment, figuring out the user, env, home and shell, so EnvInfo sounds like a pretty good name to me. Deps is a bit vague IMO and makes me think of dependency injection. 😄

We could also simplify this interface somewhat. All we really need are:

  • User()
  • Environ()

We'd just make sure the returned user has the correct home directory and shell populated. Not sure if that'd be worth the lift, though.

Copy link
Member Author

Choose a reason for hiding this comment

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

EnvInfoer it is!

I'd prefer to do the bare minimum here, but we can consolidate + simplify down the line!

}

type systemCreateCommandDeps struct{}
type systemEnvInfoer struct{}

var defaultCreateCommandDeps CreateCommandDeps = &systemCreateCommandDeps{}
var defaultEnvInfoer EnvInfoer = &systemEnvInfoer{}

// DefaultCreateCommandDeps returns a default implementation of
// CreateCommandDeps. This reads information using the default Go
// DefaultEnvInfoer returns a default implementation of
// EnvInfoer. This reads information using the default Go
// implementations.
func DefaultCreateCommandDeps() CreateCommandDeps {
return defaultCreateCommandDeps
func DefaultEnvInfoer() EnvInfoer {
return defaultEnvInfoer
}

func (systemCreateCommandDeps) CurrentUser() (*user.User, error) {
func (systemEnvInfoer) CurrentUser() (*user.User, error) {
return user.Current()
}

func (systemCreateCommandDeps) Environ() []string {
func (systemEnvInfoer) Environ() []string {
return os.Environ()
}

func (systemCreateCommandDeps) UserHomeDir() (string, error) {
func (systemEnvInfoer) UserHomeDir() (string, error) {
return userHomeDir()
}

func (systemCreateCommandDeps) UserShell(username string) (string, error) {
func (systemEnvInfoer) UserShell(username string) (string, error) {
return usershell.Get(username)
}

Expand All @@ -716,9 +716,9 @@ func (systemCreateCommandDeps) UserShell(username string) (string, error) {
// 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 CreateCommandDeps) (*pty.Cmd, error) {
func (s *Server) CreateCommand(ctx context.Context, script string, env []string, deps EnvInfoer) (*pty.Cmd, error) {
if deps == nil {
deps = DefaultCreateCommandDeps()
deps = DefaultEnvInfoer()
}
currentUser, err := deps.CurrentUser()
if err != nil {
Expand Down
0