chore(agent/agentssh): extract EnvInfoer#16603
Conversation
mafredri
left a comment
There was a problem hiding this comment.
Naming aside, changes look good so approving 👍🏻
| // 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) |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
EnvInfoer it is!
I'd prefer to do the bare minimum here, but we can consolidate + simplify down the line!
| t.Parallel() | ||
| cmd, err := s.CreateCommand(ctx, `#!/usr/bin/env bash | ||
| echo test`, nil) | ||
| echo test`, nil, nil) |
There was a problem hiding this comment.
Perhaps we should add a test for a custom implementation as well?
Extracts environment-level dependencies of
agentssh.Server.CreateCommand()to an interface to allow alternative implementations to be passed in.We'll need this so we can pass in an alternative implementation that can reference a running container instead of the agent's environment.
I'm not married to the naming convention and will rename the thing if anyone has a better name.