Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitc9535de

Browse files
committed
fix(agent/agentcontainers): use correct env execer commands
Fixescoder/internal#707
1 parent0a12ec5 commitc9535de

File tree

6 files changed

+235
-33
lines changed

6 files changed

+235
-33
lines changed

‎agent/agentcontainers/api.go

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import (
2323
"cdr.dev/slog"
2424
"github.com/coder/coder/v2/agent/agentcontainers/watcher"
2525
"github.com/coder/coder/v2/agent/agentexec"
26+
"github.com/coder/coder/v2/agent/usershell"
2627
"github.com/coder/coder/v2/coderd/httpapi"
2728
"github.com/coder/coder/v2/codersdk"
2829
"github.com/coder/coder/v2/codersdk/agentsdk"
@@ -54,6 +55,7 @@ type API struct {
5455
logger slog.Logger
5556
watcher watcher.Watcher
5657
execer agentexec.Execer
58+
commandEnvCommandEnv
5759
ccliContainerCLI
5860
containerLabelIncludeFiltermap[string]string// Labels to filter containers by.
5961
dccliDevcontainerCLI
@@ -106,6 +108,29 @@ func WithExecer(execer agentexec.Execer) Option {
106108
}
107109
}
108110

111+
// WithCommandEnv sets the CommandEnv implementation to use.
112+
funcWithCommandEnv(ceCommandEnv)Option {
113+
returnfunc(api*API) {
114+
api.commandEnv=func(ei usershell.EnvInfoer,preEnv []string) (string,string, []string,error) {
115+
shell,dir,env,err:=ce(ei,preEnv)
116+
iferr!=nil {
117+
returnshell,dir,env,err
118+
}
119+
env=slices.DeleteFunc(env,func(sstring)bool {
120+
// Ensure we filter out environment variables that come
121+
// from the parent agent and are incorrect or not
122+
// relevant for the devcontainer.
123+
returnstrings.HasPrefix(s,"CODER_WORKSPACE_AGENT_NAME=")||
124+
strings.HasPrefix(s,"CODER_WORKSPACE_AGENT_URL=")||
125+
strings.HasPrefix(s,"CODER_AGENT_TOKEN=")||
126+
strings.HasPrefix(s,"CODER_AGENT_AUTH=")||
127+
strings.HasPrefix(s,"CODER_AGENT_DEVCONTAINERS_ENABLE=")
128+
})
129+
returnshell,dir,env,nil
130+
}
131+
}
132+
}
133+
109134
// WithContainerCLI sets the agentcontainers.ContainerCLI implementation
110135
// to use. The default implementation uses the Docker CLI.
111136
funcWithContainerCLI(ccliContainerCLI)Option {
@@ -148,7 +173,7 @@ func WithSubAgentURL(url string) Option {
148173
}
149174
}
150175

151-
//WithSubAgent sets the environment variables for the sub-agent.
176+
//WithSubAgentEnv sets the environment variables for the sub-agent.
152177
funcWithSubAgentEnv(env...string)Option {
153178
returnfunc(api*API) {
154179
api.subAgentEnv=env
@@ -256,6 +281,13 @@ func NewAPI(logger slog.Logger, options ...Option) *API {
256281
for_,opt:=rangeoptions {
257282
opt(api)
258283
}
284+
ifapi.commandEnv!=nil {
285+
api.execer=newCommandEnvExecer(
286+
api.logger,
287+
api.commandEnv,
288+
api.execer,
289+
)
290+
}
259291
ifapi.ccli==nil {
260292
api.ccli=NewDockerCLI(api.execer)
261293
}

‎agent/agentcontainers/api_test.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import (
77
"net/http"
88
"net/http/httptest"
99
"os"
10+
"os/exec"
1011
"runtime"
1112
"strings"
1213
"sync"
@@ -26,7 +27,9 @@ import (
2627
"github.com/coder/coder/v2/agent/agentcontainers"
2728
"github.com/coder/coder/v2/agent/agentcontainers/acmock"
2829
"github.com/coder/coder/v2/agent/agentcontainers/watcher"
30+
"github.com/coder/coder/v2/agent/usershell"
2931
"github.com/coder/coder/v2/codersdk"
32+
"github.com/coder/coder/v2/pty"
3033
"github.com/coder/coder/v2/testutil"
3134
"github.com/coder/quartz"
3235
)
@@ -291,6 +294,38 @@ func (m *fakeSubAgentClient) Delete(ctx context.Context, id uuid.UUID) error {
291294
returnnil
292295
}
293296

297+
// fakeExecer implements agentexec.Execer for testing and tracks execution details
298+
typefakeExecerstruct {
299+
commands [][]string
300+
createdCommands []*exec.Cmd
301+
}
302+
303+
func (f*fakeExecer)CommandContext(ctx context.Context,cmdstring,args...string)*exec.Cmd {
304+
f.commands=append(f.commands,append([]string{cmd},args...))
305+
// Create a command that returns empty JSON for docker commands
306+
c:=exec.CommandContext(ctx,"echo","[]")
307+
f.createdCommands=append(f.createdCommands,c)
308+
returnc
309+
}
310+
311+
func (f*fakeExecer)PTYCommandContext(ctx context.Context,cmdstring,args...string)*pty.Cmd {
312+
f.commands=append(f.commands,append([]string{cmd},args...))
313+
return&pty.Cmd{
314+
Context:ctx,
315+
Path:cmd,
316+
Args:append([]string{cmd},args...),
317+
Env: []string{},
318+
Dir:"",
319+
}
320+
}
321+
322+
func (f*fakeExecer)getLastCommand()*exec.Cmd {
323+
iflen(f.createdCommands)==0 {
324+
returnnil
325+
}
326+
returnf.createdCommands[len(f.createdCommands)-1]
327+
}
328+
294329
funcTestAPI(t*testing.T) {
295330
t.Parallel()
296331

@@ -1970,6 +2005,48 @@ func TestAPI(t *testing.T) {
19702005
// Then: We expected it to succeed
19712006
require.Len(t,fSAC.created,1)
19722007
})
2008+
2009+
t.Run("CommandEnv",func(t*testing.T) {
2010+
t.Parallel()
2011+
2012+
ctx:=testutil.Context(t,testutil.WaitShort)
2013+
logger:=slogtest.Make(t,&slogtest.Options{IgnoreErrors:true}).Leveled(slog.LevelDebug)
2014+
2015+
// Create fake execer to track execution details.
2016+
fakeExec:=&fakeExecer{}
2017+
2018+
// Custom CommandEnv that returns specific values.
2019+
testShell:="/bin/custom-shell"
2020+
testDir:=t.TempDir()
2021+
testEnv:= []string{"CUSTOM_VAR=test_value","PATH=/custom/path"}
2022+
2023+
commandEnv:=func(ei usershell.EnvInfoer,addEnv []string) (shell,dirstring,env []string,errerror) {
2024+
returntestShell,testDir,testEnv,nil
2025+
}
2026+
2027+
// Create API with CommandEnv.
2028+
api:=agentcontainers.NewAPI(logger,
2029+
agentcontainers.WithExecer(fakeExec),
2030+
agentcontainers.WithCommandEnv(commandEnv),
2031+
)
2032+
deferapi.Close()
2033+
2034+
// Call RefreshContainers directly to trigger CommandEnv usage.
2035+
_=api.RefreshContainers(ctx)// Ignore error since docker commands will fail.
2036+
2037+
// Verify commands were executed through the custom shell and environment.
2038+
require.NotEmpty(t,fakeExec.commands,"commands should be executed")
2039+
2040+
// The first command should be executed through the custom shell with -c.
2041+
require.Equal(t,testShell,fakeExec.commands[0][0],"custom shell should be used")
2042+
require.Equal(t,"-c",fakeExec.commands[0][1],"shell should be called with -c")
2043+
2044+
// Verify the environment was set on the command.
2045+
lastCmd:=fakeExec.getLastCommand()
2046+
require.NotNil(t,lastCmd,"command should be created")
2047+
require.Equal(t,testDir,lastCmd.Dir,"custom directory should be used")
2048+
require.Equal(t,testEnv,lastCmd.Env,"custom environment should be used")
2049+
})
19732050
}
19742051

19752052
// mustFindDevcontainerByPath returns the devcontainer with the given workspace

‎agent/agentcontainers/devcontainercli.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"encoding/json"
88
"errors"
99
"io"
10-
"os"
1110

1211
"golang.org/x/xerrors"
1312

@@ -280,7 +279,6 @@ func (d *devcontainerCLI) ReadConfig(ctx context.Context, workspaceFolder, confi
280279
}
281280

282281
c:=d.execer.CommandContext(ctx,"devcontainer",args...)
283-
c.Env=append(c.Env,"PATH="+os.Getenv("PATH"))
284282
c.Env=append(c.Env,env...)
285283

286284
varstdoutBuf bytes.Buffer

‎agent/agentcontainers/execer.go

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package agentcontainers
2+
3+
import (
4+
"context"
5+
"os/exec"
6+
"runtime"
7+
8+
"github.com/kballard/go-shellquote"
9+
10+
"cdr.dev/slog"
11+
"github.com/coder/coder/v2/agent/agentexec"
12+
"github.com/coder/coder/v2/agent/usershell"
13+
"github.com/coder/coder/v2/pty"
14+
)
15+
16+
// CommandEnv is a function that returns the shell, working directory,
17+
// and environment variables to use when executing a command. It takes
18+
// an EnvInfoer and a pre-existing environment slice as arguments.
19+
// This signature matches agentssh.Server.CommandEnv.
20+
typeCommandEnvfunc(ei usershell.EnvInfoer,addEnv []string) (shell,dirstring,env []string,errerror)
21+
22+
// commandEnvExecer is an agentexec.Execer that uses a CommandEnv to
23+
// determine the shell, working directory, and environment variables
24+
// for commands. It wraps another agentexec.Execer to provide the
25+
// necessary context.
26+
typecommandEnvExecerstruct {
27+
logger slog.Logger
28+
commandEnvCommandEnv
29+
execer agentexec.Execer
30+
}
31+
32+
funcnewCommandEnvExecer(
33+
logger slog.Logger,
34+
commandEnvCommandEnv,
35+
execer agentexec.Execer,
36+
)*commandEnvExecer {
37+
return&commandEnvExecer{
38+
logger:logger,
39+
commandEnv:commandEnv,
40+
execer:execer,
41+
}
42+
}
43+
44+
// Ensure commandEnvExecer implements agentexec.Execer.
45+
var_ agentexec.Execer= (*commandEnvExecer)(nil)
46+
47+
func (e*commandEnvExecer)prepare(ctx context.Context,inNamestring,inArgs...string) (namestring,args []string,dirstring,env []string) {
48+
shell,dir,env,err:=e.commandEnv(nil,nil)
49+
iferr!=nil {
50+
e.logger.Error(ctx,"get command environment failed",slog.Error(err))
51+
returninName,inArgs,"",nil
52+
}
53+
54+
caller:="-c"
55+
ifruntime.GOOS=="windows" {
56+
caller="/c"
57+
}
58+
name=shell
59+
args= []string{caller,shellquote.Join(append([]string{inName},args...)...)}
60+
returnname,args,dir,env
61+
}
62+
63+
func (e*commandEnvExecer)CommandContext(ctx context.Context,cmdstring,args...string)*exec.Cmd {
64+
name,args,dir,env:=e.prepare(ctx,cmd,args...)
65+
c:=e.execer.CommandContext(ctx,name,args...)
66+
c.Dir=dir
67+
c.Env=env
68+
returnc
69+
}
70+
71+
func (e*commandEnvExecer)PTYCommandContext(ctx context.Context,cmdstring,args...string)*pty.Cmd {
72+
name,args,dir,env:=e.prepare(ctx,cmd,args...)
73+
c:=e.execer.PTYCommandContext(ctx,name,args...)
74+
c.Dir=dir
75+
c.Env=env
76+
returnc
77+
}

‎agent/agentssh/agentssh.go

Lines changed: 47 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,49 @@ func (s *Server) sftpHandler(logger slog.Logger, session ssh.Session) error {
816816
returnxerrors.Errorf("sftp server closed with error: %w",err)
817817
}
818818

819+
func (s*Server)CommandEnv(ei usershell.EnvInfoer,addEnv []string) (shell,dirstring,env []string,errerror) {
820+
ifei==nil {
821+
ei=&usershell.SystemEnvInfo{}
822+
}
823+
824+
currentUser,err:=ei.User()
825+
iferr!=nil {
826+
return"","",nil,xerrors.Errorf("get current user: %w",err)
827+
}
828+
username:=currentUser.Username
829+
830+
shell,err=ei.Shell(username)
831+
iferr!=nil {
832+
return"","",nil,xerrors.Errorf("get user shell: %w",err)
833+
}
834+
835+
dir=s.config.WorkingDirectory()
836+
837+
// If the metadata directory doesn't exist, we run the command
838+
// in the users home directory.
839+
_,err=os.Stat(dir)
840+
ifdir==""||err!=nil {
841+
// Default to user home if a directory is not set.
842+
homedir,err:=ei.HomeDir()
843+
iferr!=nil {
844+
return"","",nil,xerrors.Errorf("get home dir: %w",err)
845+
}
846+
dir=homedir
847+
}
848+
env=append(ei.Environ(),addEnv...)
849+
// Set login variables (see `man login`).
850+
env=append(env,fmt.Sprintf("USER=%s",username))
851+
env=append(env,fmt.Sprintf("LOGNAME=%s",username))
852+
env=append(env,fmt.Sprintf("SHELL=%s",shell))
853+
854+
env,err=s.config.UpdateEnv(env)
855+
iferr!=nil {
856+
return"","",nil,xerrors.Errorf("apply env: %w",err)
857+
}
858+
859+
returnshell,dir,env,nil
860+
}
861+
819862
// CreateCommand processes raw command input with OpenSSH-like behavior.
820863
// If the script provided is empty, it will default to the users shell.
821864
// This injects environment variables specified by the user at launch too.
@@ -827,15 +870,10 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
827870
ifei==nil {
828871
ei=&usershell.SystemEnvInfo{}
829872
}
830-
currentUser,err:=ei.User()
831-
iferr!=nil {
832-
returnnil,xerrors.Errorf("get current user: %w",err)
833-
}
834-
username:=currentUser.Username
835873

836-
shell,err:=ei.Shell(username)
874+
shell,dir,env,err:=s.CommandEnv(ei,env)
837875
iferr!=nil {
838-
returnnil,xerrors.Errorf("get user shell: %w",err)
876+
returnnil,xerrors.Errorf("prepare command env: %w",err)
839877
}
840878

841879
// OpenSSH executes all commands with the users current shell.
@@ -893,24 +931,8 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
893931
)
894932
}
895933
cmd:=s.Execer.PTYCommandContext(ctx,modifiedName,modifiedArgs...)
896-
cmd.Dir=s.config.WorkingDirectory()
897-
898-
// If the metadata directory doesn't exist, we run the command
899-
// in the users home directory.
900-
_,err=os.Stat(cmd.Dir)
901-
ifcmd.Dir==""||err!=nil {
902-
// Default to user home if a directory is not set.
903-
homedir,err:=ei.HomeDir()
904-
iferr!=nil {
905-
returnnil,xerrors.Errorf("get home dir: %w",err)
906-
}
907-
cmd.Dir=homedir
908-
}
909-
cmd.Env=append(ei.Environ(),env...)
910-
// Set login variables (see `man login`).
911-
cmd.Env=append(cmd.Env,fmt.Sprintf("USER=%s",username))
912-
cmd.Env=append(cmd.Env,fmt.Sprintf("LOGNAME=%s",username))
913-
cmd.Env=append(cmd.Env,fmt.Sprintf("SHELL=%s",shell))
934+
cmd.Dir=dir
935+
cmd.Env=env
914936

915937
// Set SSH connection environment variables (these are also set by OpenSSH
916938
// and thus expected to be present by SSH clients). Since the agent does
@@ -921,11 +943,6 @@ func (s *Server) CreateCommand(ctx context.Context, script string, env []string,
921943
cmd.Env=append(cmd.Env,fmt.Sprintf("SSH_CLIENT=%s %s %s",srcAddr,srcPort,dstPort))
922944
cmd.Env=append(cmd.Env,fmt.Sprintf("SSH_CONNECTION=%s %s %s %s",srcAddr,srcPort,dstAddr,dstPort))
923945

924-
cmd.Env,err=s.config.UpdateEnv(cmd.Env)
925-
iferr!=nil {
926-
returnnil,xerrors.Errorf("apply env: %w",err)
927-
}
928-
929946
returncmd,nil
930947
}
931948

‎agent/api.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ func (a *agent) apiHandler(aAPI proto.DRPCAgentClient26) (http.Handler, func() e
4343
ifa.experimentalDevcontainersEnabled {
4444
containerAPIOpts:= []agentcontainers.Option{
4545
agentcontainers.WithExecer(a.execer),
46+
agentcontainers.WithCommandEnv(a.sshServer.CommandEnv),
4647
agentcontainers.WithScriptLogger(func(logSourceID uuid.UUID) agentcontainers.ScriptLogger {
4748
returna.logSender.GetScriptLogger(logSourceID)
4849
}),

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp