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

Commit75b27e8

Browse files
authored
fix(agent/agentcontainers): improve testing of convertDockerInspect, return correct host port (#16887)
* Improves separation of concerns between `runDockerInspect` and`convertDockerInspect`: `runDockerInspect` now just runs the command andreturns the output, while `convertDockerInspect` now does all of theconversion and parsing logic.* Improves testing of `convertDockerInspect` using real test fixtures.* Fixes issue where the container port is returned instead of the hostport.* Updates UI to link to correct host port. Container port is stilldisplayed in the button text, but the HostIP:HostPort is shown in apopover.* Adds stories for workspace agent UI
1 parent13d0dac commit75b27e8

File tree

22 files changed

+2612
-114
lines changed

22 files changed

+2612
-114
lines changed

‎agent/agentcontainers/containers_dockercli.go

Lines changed: 142 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"context"
77
"encoding/json"
88
"fmt"
9+
"net"
910
"os/user"
1011
"slices"
1112
"sort"
@@ -164,23 +165,28 @@ func (dei *DockerEnvInfoer) ModifyCommand(cmd string, args ...string) (string, [
164165
// devcontainerEnv is a helper function that inspects the container labels to
165166
// find the required environment variables for running a command in the container.
166167
funcdevcontainerEnv(ctx context.Context,execer agentexec.Execer,containerstring) ([]string,error) {
167-
ins,stderr,err:=runDockerInspect(ctx,execer,container)
168+
stdout,stderr,err:=runDockerInspect(ctx,execer,container)
168169
iferr!=nil {
169170
returnnil,xerrors.Errorf("inspect container: %w: %q",err,stderr)
170171
}
171172

173+
ins,_,err:=convertDockerInspect(stdout)
174+
iferr!=nil {
175+
returnnil,xerrors.Errorf("inspect container: %w",err)
176+
}
177+
172178
iflen(ins)!=1 {
173179
returnnil,xerrors.Errorf("inspect container: expected 1 container, got %d",len(ins))
174180
}
175181

176182
in:=ins[0]
177-
ifin.Config.Labels==nil {
183+
ifin.Labels==nil {
178184
returnnil,nil
179185
}
180186

181187
// We want to look for the devcontainer metadata, which is in the
182188
// value of the label `devcontainer.metadata`.
183-
rawMeta,ok:=in.Config.Labels["devcontainer.metadata"]
189+
rawMeta,ok:=in.Labels["devcontainer.metadata"]
184190
if!ok {
185191
returnnil,nil
186192
}
@@ -282,68 +288,63 @@ func (dcl *DockerCLILister) List(ctx context.Context) (codersdk.WorkspaceAgentLi
282288
// will still contain valid JSON. We will just end up missing
283289
// information about the removed container. We could potentially
284290
// log this error, but I'm not sure it's worth it.
285-
ins,dockerInspectStderr,err:=runDockerInspect(ctx,dcl.execer,ids...)
291+
dockerInspectStdout,dockerInspectStderr,err:=runDockerInspect(ctx,dcl.execer,ids...)
286292
iferr!=nil {
287293
return codersdk.WorkspaceAgentListContainersResponse{},xerrors.Errorf("run docker inspect: %w: %s",err,dockerInspectStderr)
288294
}
289295

290-
for_,in:=rangeins {
291-
out,warns:=convertDockerInspect(in)
292-
res.Warnings=append(res.Warnings,warns...)
293-
res.Containers=append(res.Containers,out)
296+
iflen(dockerInspectStderr)>0 {
297+
res.Warnings=append(res.Warnings,string(dockerInspectStderr))
294298
}
295299

296-
ifdockerPsStderr!="" {
297-
res.Warnings=append(res.Warnings,dockerPsStderr)
298-
}
299-
ifdockerInspectStderr!="" {
300-
res.Warnings=append(res.Warnings,dockerInspectStderr)
300+
outs,warns,err:=convertDockerInspect(dockerInspectStdout)
301+
iferr!=nil {
302+
return codersdk.WorkspaceAgentListContainersResponse{},xerrors.Errorf("convert docker inspect output: %w",err)
301303
}
304+
res.Warnings=append(res.Warnings,warns...)
305+
res.Containers=append(res.Containers,outs...)
302306

303307
returnres,nil
304308
}
305309

306310
// runDockerInspect is a helper function that runs `docker inspect` on the given
307311
// container IDs and returns the parsed output.
308312
// The stderr output is also returned for logging purposes.
309-
funcrunDockerInspect(ctx context.Context,execer agentexec.Execer,ids...string) ([]dockerInspect,string,error) {
313+
funcrunDockerInspect(ctx context.Context,execer agentexec.Execer,ids...string) (stdout,stderr []byte,errerror) {
310314
varstdoutBuf,stderrBuf bytes.Buffer
311315
cmd:=execer.CommandContext(ctx,"docker",append([]string{"inspect"},ids...)...)
312316
cmd.Stdout=&stdoutBuf
313317
cmd.Stderr=&stderrBuf
314-
err:=cmd.Run()
315-
stderr:=strings.TrimSpace(stderrBuf.String())
318+
err=cmd.Run()
319+
stdout=bytes.TrimSpace(stdoutBuf.Bytes())
320+
stderr=bytes.TrimSpace(stderrBuf.Bytes())
316321
iferr!=nil {
317-
returnnil,stderr,err
318-
}
319-
320-
varins []dockerInspect
321-
iferr:=json.NewDecoder(&stdoutBuf).Decode(&ins);err!=nil {
322-
returnnil,stderr,xerrors.Errorf("decode docker inspect output: %w",err)
322+
returnstdout,stderr,err
323323
}
324324

325-
returnins,stderr,nil
325+
returnstdout,stderr,nil
326326
}
327327

328328
// To avoid a direct dependency on the Docker API, we use the docker CLI
329329
// to fetch information about containers.
330330
typedockerInspectstruct {
331-
IDstring`json:"Id"`
332-
Created time.Time`json:"Created"`
333-
ConfigdockerInspectConfig`json:"Config"`
334-
HostConfigdockerInspectHostConfig`json:"HostConfig"`
335-
Namestring`json:"Name"`
336-
Mounts[]dockerInspectMount`json:"Mounts"`
337-
StatedockerInspectState`json:"State"`
331+
IDstring`json:"Id"`
332+
Createdtime.Time`json:"Created"`
333+
ConfigdockerInspectConfig`json:"Config"`
334+
Namestring`json:"Name"`
335+
Mounts[]dockerInspectMount`json:"Mounts"`
336+
StatedockerInspectState`json:"State"`
337+
NetworkSettingsdockerInspectNetworkSettings`json:"NetworkSettings"`
338338
}
339339

340340
typedockerInspectConfigstruct {
341341
Imagestring`json:"Image"`
342342
Labelsmap[string]string`json:"Labels"`
343343
}
344344

345-
typedockerInspectHostConfigstruct {
346-
PortBindingsmap[string]any`json:"PortBindings"`
345+
typedockerInspectPortstruct {
346+
HostIPstring`json:"HostIp"`
347+
HostPortstring`json:"HostPort"`
347348
}
348349

349350
typedockerInspectMountstruct {
@@ -358,6 +359,10 @@ type dockerInspectState struct {
358359
Errorstring`json:"Error"`
359360
}
360361

362+
typedockerInspectNetworkSettingsstruct {
363+
Portsmap[string][]dockerInspectPort`json:"Ports"`
364+
}
365+
361366
func (disdockerInspectState)String()string {
362367
ifdis.Running {
363368
return"running"
@@ -375,50 +380,108 @@ func (dis dockerInspectState) String() string {
375380
returnsb.String()
376381
}
377382

378-
funcconvertDockerInspect(indockerInspect) (codersdk.WorkspaceAgentDevcontainer, []string) {
383+
funcconvertDockerInspect(raw []byte) ([]codersdk.WorkspaceAgentDevcontainer, []string,error) {
379384
varwarns []string
380-
out:= codersdk.WorkspaceAgentDevcontainer{
381-
CreatedAt:in.Created,
382-
// Remove the leading slash from the container name
383-
FriendlyName:strings.TrimPrefix(in.Name,"/"),
384-
ID:in.ID,
385-
Image:in.Config.Image,
386-
Labels:in.Config.Labels,
387-
Ports:make([]codersdk.WorkspaceAgentListeningPort,0),
388-
Running:in.State.Running,
389-
Status:in.State.String(),
390-
Volumes:make(map[string]string,len(in.Mounts)),
391-
}
392-
393-
ifin.HostConfig.PortBindings==nil {
394-
in.HostConfig.PortBindings=make(map[string]any)
395-
}
396-
portKeys:=maps.Keys(in.HostConfig.PortBindings)
397-
// Sort the ports for deterministic output.
398-
sort.Strings(portKeys)
399-
for_,p:=rangeportKeys {
400-
ifport,network,err:=convertDockerPort(p);err!=nil {
401-
warns=append(warns,err.Error())
402-
}else {
403-
out.Ports=append(out.Ports, codersdk.WorkspaceAgentListeningPort{
404-
Network:network,
405-
Port:port,
406-
})
385+
varins []dockerInspect
386+
iferr:=json.NewDecoder(bytes.NewReader(raw)).Decode(&ins);err!=nil {
387+
returnnil,nil,xerrors.Errorf("decode docker inspect output: %w",err)
388+
}
389+
outs:=make([]codersdk.WorkspaceAgentDevcontainer,0,len(ins))
390+
391+
// Say you have two containers:
392+
// - Container A with Host IP 127.0.0.1:8000 mapped to container port 8001
393+
// - Container B with Host IP [::1]:8000 mapped to container port 8001
394+
// A request to localhost:8000 may be routed to either container.
395+
// We don't know which one for sure, so we need to surface this to the user.
396+
// Keep track of all host ports we see. If we see the same host port
397+
// mapped to multiple containers on different host IPs, we need to
398+
// warn the user about this.
399+
// Note that we only do this for loopback or unspecified IPs.
400+
// We'll assume that the user knows what they're doing if they bind to
401+
// a specific IP address.
402+
hostPortContainers:=make(map[int][]string)
403+
404+
for_,in:=rangeins {
405+
out:= codersdk.WorkspaceAgentDevcontainer{
406+
CreatedAt:in.Created,
407+
// Remove the leading slash from the container name
408+
FriendlyName:strings.TrimPrefix(in.Name,"/"),
409+
ID:in.ID,
410+
Image:in.Config.Image,
411+
Labels:in.Config.Labels,
412+
Ports:make([]codersdk.WorkspaceAgentDevcontainerPort,0),
413+
Running:in.State.Running,
414+
Status:in.State.String(),
415+
Volumes:make(map[string]string,len(in.Mounts)),
416+
}
417+
418+
ifin.NetworkSettings.Ports==nil {
419+
in.NetworkSettings.Ports=make(map[string][]dockerInspectPort)
420+
}
421+
portKeys:=maps.Keys(in.NetworkSettings.Ports)
422+
// Sort the ports for deterministic output.
423+
sort.Strings(portKeys)
424+
// If we see the same port bound to both ipv4 and ipv6 loopback or unspecified
425+
// interfaces to the same container port, there is no point in adding it multiple times.
426+
loopbackHostPortContainerPorts:=make(map[int]uint16,0)
427+
for_,pk:=rangeportKeys {
428+
for_,p:=rangein.NetworkSettings.Ports[pk] {
429+
cp,network,err:=convertDockerPort(pk)
430+
iferr!=nil {
431+
warns=append(warns,fmt.Sprintf("convert docker port: %s",err.Error()))
432+
// Default network to "tcp" if we can't parse it.
433+
network="tcp"
434+
}
435+
hp,err:=strconv.Atoi(p.HostPort)
436+
iferr!=nil {
437+
warns=append(warns,fmt.Sprintf("convert docker host port: %s",err.Error()))
438+
continue
439+
}
440+
ifhp>65535||hp<1 {// invalid port
441+
warns=append(warns,fmt.Sprintf("convert docker host port: invalid host port %d",hp))
442+
continue
443+
}
444+
445+
// Deduplicate host ports for loopback and unspecified IPs.
446+
ifisLoopbackOrUnspecified(p.HostIP) {
447+
iffound,ok:=loopbackHostPortContainerPorts[hp];ok&&found==cp {
448+
// We've already seen this port, so skip it.
449+
continue
450+
}
451+
loopbackHostPortContainerPorts[hp]=cp
452+
// Also keep track of the host port and the container ID.
453+
hostPortContainers[hp]=append(hostPortContainers[hp],in.ID)
454+
}
455+
out.Ports=append(out.Ports, codersdk.WorkspaceAgentDevcontainerPort{
456+
Network:network,
457+
Port:cp,
458+
HostPort:uint16(hp),
459+
HostIP:p.HostIP,
460+
})
461+
}
407462
}
408-
}
409463

410-
ifin.Mounts==nil {
411-
in.Mounts= []dockerInspectMount{}
464+
ifin.Mounts==nil {
465+
in.Mounts= []dockerInspectMount{}
466+
}
467+
// Sort the mounts for deterministic output.
468+
sort.Slice(in.Mounts,func(i,jint)bool {
469+
returnin.Mounts[i].Source<in.Mounts[j].Source
470+
})
471+
for_,k:=rangein.Mounts {
472+
out.Volumes[k.Source]=k.Destination
473+
}
474+
outs=append(outs,out)
412475
}
413-
// Sort the mounts for deterministic output.
414-
sort.Slice(in.Mounts,func(i,jint)bool {
415-
returnin.Mounts[i].Source<in.Mounts[j].Source
416-
})
417-
for_,k:=rangein.Mounts {
418-
out.Volumes[k.Source]=k.Destination
476+
477+
// Check if any host ports are mapped to multiple containers.
478+
forhp,ids:=rangehostPortContainers {
479+
iflen(ids)>1 {
480+
warns=append(warns,fmt.Sprintf("host port %d is mapped to multiple containers on different interfaces: %s",hp,strings.Join(ids,", ")))
481+
}
419482
}
420483

421-
returnout,warns
484+
returnouts,warns,nil
422485
}
423486

424487
// convertDockerPort converts a Docker port string to a port number and network
@@ -445,3 +508,12 @@ func convertDockerPort(in string) (uint16, string, error) {
445508
return0,"",xerrors.Errorf("invalid port format: %s",in)
446509
}
447510
}
511+
512+
// convenience function to check if an IP address is loopback or unspecified
513+
funcisLoopbackOrUnspecified(ipsstring)bool {
514+
nip:=net.ParseIP(ips)
515+
ifnip==nil {
516+
returnfalse// technically correct, I suppose
517+
}
518+
returnnip.IsLoopback()||nip.IsUnspecified()
519+
}

0 commit comments

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