Skip to content

Commit 95b156e

Browse files
johnstcnjohnstcn
committed
fix(agent/agentcontainers): create new WorkspaceAgentDevcontainerPort type, return container and host port
1 parent 393f6e9 commit 95b156e

File tree

9 files changed

+283
-57
lines changed

9 files changed

+283
-57
lines changed

‎agent/agentcontainers/containers_dockercli.go

+53-15
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"
@@ -379,6 +380,19 @@ func convertDockerInspect(raw []byte) ([]codersdk.WorkspaceAgentDevcontainer, []
379380
}
380381
outs := make([]codersdk.WorkspaceAgentDevcontainer, 0, len(ins))
381382

383+
// Say you have two containers:
384+
// - Container A with Host IP 127.0.0.1:8000 mapped to container port 8001
385+
// - Container B with Host IP [::1]:8000 mapped to container port 8001
386+
// A request to localhost:8000 may be routed to either container.
387+
// We don't know which one for sure, so we need to surface this to the user.
388+
// Keep track of all host ports we see. If we see the same host port
389+
// mapped to multiple containers on different host IPs, we need to
390+
// warn the user about this.
391+
// Note that we only do this for loopback or unspecified IPs.
392+
// We'll assume that the user knows what they're doing if they bind to
393+
// a specific IP address.
394+
hostPortContainers := make(map[int][]string)
395+
382396
for _, in := range ins {
383397
out := codersdk.WorkspaceAgentDevcontainer{
384398
CreatedAt: in.Created,
@@ -387,7 +401,7 @@ func convertDockerInspect(raw []byte) ([]codersdk.WorkspaceAgentDevcontainer, []
387401
ID: in.ID,
388402
Image: in.Config.Image,
389403
Labels: in.Config.Labels,
390-
Ports: make([]codersdk.WorkspaceAgentListeningPort, 0),
404+
Ports: make([]codersdk.WorkspaceAgentDevcontainerPort, 0),
391405
Running: in.State.Running,
392406
Status: in.State.String(),
393407
Volumes: make(map[string]string, len(in.Mounts)),
@@ -399,35 +413,43 @@ func convertDockerInspect(raw []byte) ([]codersdk.WorkspaceAgentDevcontainer, []
399413
portKeys := maps.Keys(in.NetworkSettings.Ports)
400414
// Sort the ports for deterministic output.
401415
sort.Strings(portKeys)
402-
// Each port binding may have multiple entries mapped to the same interface.
403-
// Keep track of the ports we've already seen.
404-
seen := make(map[int]struct{}, len(in.NetworkSettings.Ports))
416+
// If we see the same port bound to both ipv4 and ipv6 loopback or unspecified
417+
// interfaces to the same container port, there is no point in adding it multiple times.
418+
loopbackHostPortContainerPorts := make(map[int]uint16, 0)
405419
for _, pk := range portKeys {
406420
for _, p := range in.NetworkSettings.Ports[pk] {
407-
_, network, err := convertDockerPort(pk)
421+
cp, network, err := convertDockerPort(pk)
408422
if err != nil {
409423
warns = append(warns, fmt.Sprintf("convert docker port: %s", err.Error()))
410424
// Default network to "tcp" if we can't parse it.
411425
network = "tcp"
412426
}
413427
hp, err := strconv.Atoi(p.HostPort)
414428
if err != nil {
415-
warns = append(warns, fmt.Sprintf("convert docker port: %s", err.Error()))
429+
warns = append(warns, fmt.Sprintf("convert docker host port: %s", err.Error()))
416430
continue
417431
}
418-
if hp > 65535 || hp < 0 { // invalid port
419-
warns = append(warns, fmt.Sprintf("convert docker port: invalid host port %d", hp))
432+
if hp > 65535 || hp < 1 { // invalid port
433+
warns = append(warns, fmt.Sprintf("convert docker host port: invalid host port %d", hp))
420434
continue
421435
}
422-
if _, ok := seen[hp]; ok {
423-
// We've already seen this port, so skip it.
424-
continue
436+
437+
// Deduplicate host ports for loopback and unspecified IPs.
438+
if isLoopbackOrUnspecified(p.HostIP) {
439+
if found, ok := loopbackHostPortContainerPorts[hp]; ok && found == cp {
440+
// We've already seen this port, so skip it.
441+
continue
442+
}
443+
loopbackHostPortContainerPorts[hp] = cp
444+
// Also keep track of the host port and the container ID.
445+
hostPortContainers[hp] = append(hostPortContainers[hp], in.ID)
425446
}
426-
out.Ports = append(out.Ports, codersdk.WorkspaceAgentListeningPort{
427-
Network: network,
428-
Port: uint16(hp),
447+
out.Ports = append(out.Ports, codersdk.WorkspaceAgentDevcontainerPort{
448+
Network: network,
449+
Port: cp,
450+
HostPort: uint16(hp),
451+
HostIP: p.HostIP,
429452
})
430-
seen[hp] = struct{}{}
431453
}
432454
}
433455

@@ -444,6 +466,13 @@ func convertDockerInspect(raw []byte) ([]codersdk.WorkspaceAgentDevcontainer, []
444466
outs = append(outs, out)
445467
}
446468

469+
// Check if any host ports are mapped to multiple containers.
470+
for hp, ids := range hostPortContainers {
471+
if len(ids) > 1 {
472+
warns = append(warns, fmt.Sprintf("host port %d is mapped to multiple containers on different interfaces: %s", hp, strings.Join(ids, ", ")))
473+
}
474+
}
475+
447476
return outs, warns, nil
448477
}
449478

@@ -471,3 +500,12 @@ func convertDockerPort(in string) (uint16, string, error) {
471500
return 0, "", xerrors.Errorf("invalid port format: %s", in)
472501
}
473502
}
503+
504+
// convenience function to check if an IP address is loopback or unspecified
505+
func isLoopbackOrUnspecified(ips string) bool {
506+
nip := net.ParseIP(ips)
507+
if nip == nil {
508+
return false // technically correct, I suppose
509+
}
510+
return nip.IsLoopback() || nip.IsUnspecified()
511+
}

‎agent/agentcontainers/containers_internal_test.go

+70-19
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package agentcontainers
22

33
import (
44
"fmt"
5+
"math/rand"
56
"os"
67
"path/filepath"
78
"slices"
@@ -438,7 +439,7 @@ func TestConvertDockerInspect(t *testing.T) {
438439
Labels: map[string]string{},
439440
Running: true,
440441
Status: "running",
441-
Ports: []codersdk.WorkspaceAgentListeningPort{},
442+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{},
442443
Volumes: map[string]string{},
443444
},
444445
},
@@ -454,7 +455,7 @@ func TestConvertDockerInspect(t *testing.T) {
454455
Labels: map[string]string{"baz": "zap", "foo": "bar"},
455456
Running: true,
456457
Status: "running",
457-
Ports: []codersdk.WorkspaceAgentListeningPort{},
458+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{},
458459
Volumes: map[string]string{},
459460
},
460461
},
@@ -470,7 +471,7 @@ func TestConvertDockerInspect(t *testing.T) {
470471
Labels: map[string]string{},
471472
Running: true,
472473
Status: "running",
473-
Ports: []codersdk.WorkspaceAgentListeningPort{},
474+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{},
474475
Volumes: map[string]string{
475476
"/tmp/test/a": "/var/coder/a",
476477
"/tmp/test/b": "/var/coder/b",
@@ -489,10 +490,12 @@ func TestConvertDockerInspect(t *testing.T) {
489490
Labels: map[string]string{},
490491
Running: true,
491492
Status: "running",
492-
Ports: []codersdk.WorkspaceAgentListeningPort{
493+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{
493494
{
494-
Network: "tcp",
495-
Port: 12345,
495+
Network: "tcp",
496+
Port: 12345,
497+
HostPort: 12345,
498+
HostIP: "0.0.0.0",
496499
},
497500
},
498501
Volumes: map[string]string{},
@@ -510,16 +513,60 @@ func TestConvertDockerInspect(t *testing.T) {
510513
Labels: map[string]string{},
511514
Running: true,
512515
Status: "running",
513-
Ports: []codersdk.WorkspaceAgentListeningPort{
516+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{
514517
{
515-
Network: "tcp",
516-
Port: 12345,
518+
Network: "tcp",
519+
Port: 23456,
520+
HostPort: 12345,
521+
HostIP: "0.0.0.0",
517522
},
518523
},
519524
Volumes: map[string]string{},
520525
},
521526
},
522527
},
528+
{
529+
name: "container_sameportdiffip",
530+
expect: []codersdk.WorkspaceAgentDevcontainer{
531+
{
532+
CreatedAt: time.Date(2025, 3, 11, 17, 56, 34, 842164541, time.UTC),
533+
ID: "a",
534+
FriendlyName: "a",
535+
Image: "debian:bookworm",
536+
Labels: map[string]string{},
537+
Running: true,
538+
Status: "running",
539+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{
540+
{
541+
Network: "tcp",
542+
Port: 8001,
543+
HostPort: 8000,
544+
HostIP: "0.0.0.0",
545+
},
546+
},
547+
Volumes: map[string]string{},
548+
},
549+
{
550+
CreatedAt: time.Date(2025, 3, 11, 17, 56, 34, 842164541, time.UTC),
551+
ID: "b",
552+
FriendlyName: "b",
553+
Image: "debian:bookworm",
554+
Labels: map[string]string{},
555+
Running: true,
556+
Status: "running",
557+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{
558+
{
559+
Network: "tcp",
560+
Port: 8001,
561+
HostPort: 8000,
562+
HostIP: "::",
563+
},
564+
},
565+
Volumes: map[string]string{},
566+
},
567+
},
568+
expectWarns: []string{"host port 8000 is mapped to multiple containers on different interfaces: a, b"},
569+
},
523570
{
524571
name: "container_volume",
525572
expect: []codersdk.WorkspaceAgentDevcontainer{
@@ -531,7 +578,7 @@ func TestConvertDockerInspect(t *testing.T) {
531578
Labels: map[string]string{},
532579
Running: true,
533580
Status: "running",
534-
Ports: []codersdk.WorkspaceAgentListeningPort{},
581+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{},
535582
Volumes: map[string]string{
536583
"/var/lib/docker/volumes/testvol/_data": "/testvol",
537584
},
@@ -552,7 +599,7 @@ func TestConvertDockerInspect(t *testing.T) {
552599
},
553600
Running: true,
554601
Status: "running",
555-
Ports: []codersdk.WorkspaceAgentListeningPort{},
602+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{},
556603
Volumes: map[string]string{},
557604
},
558605
},
@@ -571,7 +618,7 @@ func TestConvertDockerInspect(t *testing.T) {
571618
},
572619
Running: true,
573620
Status: "running",
574-
Ports: []codersdk.WorkspaceAgentListeningPort{},
621+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{},
575622
Volumes: map[string]string{},
576623
},
577624
},
@@ -590,11 +637,12 @@ func TestConvertDockerInspect(t *testing.T) {
590637
},
591638
Running: true,
592639
Status: "running",
593-
Ports: []codersdk.WorkspaceAgentListeningPort{
640+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{
594641
{
595-
Network: "tcp",
596-
// Container port 8080 is mapped to 32768 on the host.
597-
Port: 32768,
642+
Network: "tcp",
643+
Port: 8080,
644+
HostPort: 32768,
645+
HostIP: "0.0.0.0",
598646
},
599647
},
600648
Volumes: map[string]string{},
@@ -771,10 +819,13 @@ func fakeContainer(t *testing.T, mut ...func(*codersdk.WorkspaceAgentDevcontaine
771819
testutil.GetRandomName(t): testutil.GetRandomName(t),
772820
},
773821
Running: true,
774-
Ports: []codersdk.WorkspaceAgentListeningPort{
822+
Ports: []codersdk.WorkspaceAgentDevcontainerPort{
775823
{
776-
Network: "tcp",
777-
Port: testutil.RandomPortNoListen(t),
824+
Network: "tcp",
825+
Port: testutil.RandomPortNoListen(t),
826+
HostPort: testutil.RandomPortNoListen(t),
827+
//nolint:gosec // this is a test
828+
HostIP: []string{"127.0.0.1", "[::1]", "localhost", "0.0.0.0", "[::]", testutil.GetRandomName(t)}[rand.Intn(6)],
778829
},
779830
},
780831
Status: testutil.MustRandString(t, 10),

‎agent/agentcontainers/testdata/container_sameportdiffip/docker_inspect.json

+51
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
[
2+
{
3+
"Id": "a",
4+
"Created": "2025-03-11T17:56:34.842164541Z",
5+
"State": {
6+
"Running": true,
7+
"ExitCode": 0,
8+
"Error": ""
9+
},
10+
"Name": "/a",
11+
"Mounts": [],
12+
"Config": {
13+
"Image": "debian:bookworm",
14+
"Labels": {}
15+
},
16+
"NetworkSettings": {
17+
"Ports": {
18+
"8001/tcp": [
19+
{
20+
"HostIp": "0.0.0.0",
21+
"HostPort": "8000"
22+
}
23+
]
24+
}
25+
}
26+
},
27+
{
28+
"Id": "b",
29+
"Created": "2025-03-11T17:56:34.842164541Z",
30+
"State": {
31+
"Running": true,
32+
"ExitCode": 0,
33+
"Error": ""
34+
},
35+
"Name": "/b",
36+
"Config": {
37+
"Image": "debian:bookworm",
38+
"Labels": {}
39+
},
40+
"NetworkSettings": {
41+
"Ports": {
42+
"8001/tcp": [
43+
{
44+
"HostIp": "::",
45+
"HostPort": "8000"
46+
}
47+
]
48+
}
49+
}
50+
}
51+
]

‎coderd/apidoc/docs.go

+22-1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)