Skip to content

Commit 3e895f3

Browse files
bilby91claude
andauthored
feat(engine): option to drop compose host port publishing (#137)
EngineOptions.DisableHostPortPublishing drops the host side of compose `ports:` entries. Services still start and nothing else about them changes; each dropped entry raises an engine.warn event (compose_host_port_publish_skipped) naming the service and where it is reachable instead. An embedder whose daemon "host" is a namespace shared with other workloads — the engine running inside a Kubernetes pod next to sidecars — cannot let a project bind there: a service publishing 8080 loses the race against a sidecar already listening and the boot fails on a raw `bind: address already in use`. Nothing in that shape consumes the publish anyway, since service-to-service traffic goes over the compose network by service name and an embedder forwarding a port out of the namespace dials the container on that network — which is what makes dropping it safe, and a silent default acceptable. It still warns, and the message says where the service is reachable rather than only that something was ignored. The entries are removed from the in-memory project via a new compose.ApplyDropHostPorts, alongside the existing Apply*Override helpers, rather than inside the orchestrator's portsOf: ConfigHash reads the project's ServiceConfig, so a container created before the option was turned on drifts and is recreated instead of being reused with its publishes intact. Native backend only. The shellout path hands `ports:` to `docker compose`, which publishes them regardless, and stripping them there would mean re-implementing merge/extends/override resolution against the YAML — so a compose-source Up under ComposeBackendShellout is refused rather than run with the option silently ignored. `network_mode: host` has the same collision but needs a refusal, not a drop: moving the service to a bridge network would change reachability. Filed as #136. Closes #135 Co-authored-by: bilby91 <2201079+bilby91@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent eb4c0c8 commit 3e895f3

8 files changed

Lines changed: 554 additions & 0 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,28 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Added
11+
12+
- **`EngineOptions.DisableHostPortPublishing`** — drops the host side of compose
13+
`ports:` entries. Services still start and nothing else about them changes; each
14+
dropped entry raises an `engine.warn` event (`compose_host_port_publish_skipped`)
15+
naming the service and where it is reachable instead. Set it when the daemon's "host"
16+
is a namespace shared with other workloads — the engine running inside a Kubernetes
17+
pod next to sidecars — where a service publishing a common port (`8080`) loses the
18+
race against a sidecar already listening there and the boot fails on a raw
19+
`bind: address already in use`. Nothing in that shape consumes the publish anyway:
20+
service-to-service traffic goes over the compose network by service name, and an
21+
embedder forwarding a port out of the namespace dials the container on that network,
22+
which is why dropping it changes no connectivity. It is an `EngineOptions` field
23+
rather than a `devcontainer.json` one because it describes where the engine is
24+
deployed, not the project. Entries are dropped from the in-memory project, so
25+
`ConfigHash` sees them go and a container created before the option was turned on is
26+
recreated rather than reused with its publishes intact. `ComposeBackendNative` only —
27+
under `ComposeBackendShellout`, `docker compose` owns the publish, so a compose-source
28+
`Up` is refused rather than run with the option silently ignored. `network_mode: host`
29+
has the same collision and is not covered here; it needs a refusal rather than a drop
30+
([#136](https://github.com/crunchloop/devcontainer/issues/136)).
31+
1032
### Removed
1133

1234
- **BREAKING — `runtime.Capabilities` is cut from six fields to three.** The struct

‎README.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ field/behavior the library covers. Legend: ✅ acted on · ⚠️ parsed but not
100100
| `forwardPorts` | ⚠️ parsed | Not actuated; [#7](https://github.com/crunchloop/devcontainer/issues/7) |
101101
| `portsAttributes`, `otherPortsAttributes` | ⚠️ parsed | Surfaced on `ResolvedConfig`; not enforced |
102102
| `appPort` (deprecated) | ✅ translated | Folded into `forwardPorts` (skipping container ports already declared); deprecation warning still emitted |
103+
| compose `ports:` | ✅ published | Host publishes come from the compose file, not `devcontainer.json`. `EngineOptions.DisableHostPortPublishing` drops the host side of every entry — for an embedder whose daemon host is a namespace shared with other workloads (the engine inside a Kubernetes pod), where the binding collides with a sidecar and nothing consumes it. Each dropped entry raises an `engine.warn` event. Requires `ComposeBackendNative`. |
103104

104105
**Other**
105106

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
package compose
2+
3+
import (
4+
"testing"
5+
6+
composetypes "github.com/compose-spec/compose-go/v2/types"
7+
)
8+
9+
func portProject() *composetypes.Project {
10+
return &composetypes.Project{
11+
Services: composetypes.Services{
12+
"app": {
13+
Name: "app",
14+
Image: "app:dev",
15+
Ports: []composetypes.ServicePortConfig{
16+
{Target: 8080, Published: "8080", Protocol: "tcp"},
17+
{Target: 9229, Published: "9229", HostIP: "127.0.0.1", Protocol: "tcp"},
18+
},
19+
Environment: composetypes.MappingWithEquals{},
20+
},
21+
"db": {
22+
Name: "db",
23+
Image: "postgres:17",
24+
Ports: []composetypes.ServicePortConfig{
25+
// `ports: ["5432"]` — no published side; the daemon
26+
// picks an ephemeral host port, in the same namespace.
27+
{Target: 5432},
28+
},
29+
},
30+
"cache": {
31+
Name: "cache",
32+
Image: "redis:7",
33+
},
34+
},
35+
}
36+
}
37+
38+
// Every entry goes, on every service, and the caller learns what was
39+
// taken so it can say so. Service order is sorted: the engine turns
40+
// each entry into a warning, and warning order must not depend on map
41+
// iteration.
42+
func TestApplyDropHostPorts_DropsEveryEntry(t *testing.T) {
43+
proj := portProject()
44+
45+
dropped := ApplyDropHostPorts(proj)
46+
47+
for name, svc := range proj.Services {
48+
if len(svc.Ports) != 0 {
49+
t.Errorf("service %q kept %d ports, want 0", name, len(svc.Ports))
50+
}
51+
}
52+
if len(dropped) != 3 {
53+
t.Fatalf("dropped = %d entries, want 3; got %+v", len(dropped), dropped)
54+
}
55+
want := []string{"app", "app", "db"}
56+
for i, svc := range want {
57+
if dropped[i].Service != svc {
58+
t.Errorf("dropped[%d].Service = %q, want %q (sorted by service name)", i, dropped[i].Service, svc)
59+
}
60+
}
61+
if got := dropped[0].String(); got != "8080:8080/tcp" {
62+
t.Errorf("dropped[0] = %q, want 8080:8080/tcp", got)
63+
}
64+
if got := dropped[1].String(); got != "127.0.0.1:9229:9229/tcp" {
65+
t.Errorf("dropped[1] = %q, want 127.0.0.1:9229:9229/tcp", got)
66+
}
67+
// Published unset: still dropped, and rendered without a host side
68+
// rather than as ":5432/tcp".
69+
if got := dropped[2].String(); got != "5432/tcp" {
70+
t.Errorf("dropped[2] = %q, want 5432/tcp", got)
71+
}
72+
}
73+
74+
// The drop is surgical: nothing but ports: moves. A service that
75+
// declared none is not rewritten at all.
76+
func TestApplyDropHostPorts_LeavesTheRestOfTheServiceAlone(t *testing.T) {
77+
proj := portProject()
78+
79+
ApplyDropHostPorts(proj)
80+
81+
if got := proj.Services["app"].Image; got != "app:dev" {
82+
t.Errorf("app image = %q, want app:dev", got)
83+
}
84+
if got := proj.Services["cache"].Image; got != "redis:7" {
85+
t.Errorf("cache image = %q, want redis:7", got)
86+
}
87+
if len(proj.Services) != 3 {
88+
t.Errorf("services = %d, want 3", len(proj.Services))
89+
}
90+
}
91+
92+
func TestApplyDropHostPorts_NoPortsIsANoOp(t *testing.T) {
93+
proj := &composetypes.Project{
94+
Services: composetypes.Services{"app": {Name: "app", Image: "app:dev"}},
95+
}
96+
if dropped := ApplyDropHostPorts(proj); dropped != nil {
97+
t.Errorf("dropped = %+v, want nil", dropped)
98+
}
99+
if dropped := ApplyDropHostPorts(nil); dropped != nil {
100+
t.Errorf("nil project: dropped = %+v, want nil", dropped)
101+
}
102+
}
103+
104+
// The reason the drop happens here and not inside the orchestrator's
105+
// portsOf: ConfigHash reads the project's ServiceConfig, so removing
106+
// the entries is visible to the recreate check. A container created
107+
// before the option was turned on published the ports and must be
108+
// replaced, not reused.
109+
func TestApplyDropHostPorts_ChangesConfigHash(t *testing.T) {
110+
proj := portProject()
111+
before := ConfigHash("sha256:x", proj.Services["app"])
112+
113+
ApplyDropHostPorts(proj)
114+
115+
after := ConfigHash("sha256:x", proj.Services["app"])
116+
if before == after {
117+
t.Error("ApplyDropHostPorts did not change ConfigHash; a container created with the publishes would be reused with them intact")
118+
}
119+
}

‎compose/apply_override.go‎

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ package compose
22

33
import (
44
"fmt"
5+
"sort"
6+
"strconv"
57

68
composetypes "github.com/compose-spec/compose-go/v2/types"
79
)
@@ -135,3 +137,94 @@ func ApplyRunOverride(project *composetypes.Project, primaryService string, ov O
135137
project.Services[primaryService] = svc
136138
return nil
137139
}
140+
141+
// DroppedHostPort records one `ports:` entry removed by
142+
// ApplyDropHostPorts, so the caller can report what it took away.
143+
type DroppedHostPort struct {
144+
// Service is the compose service that declared the entry.
145+
Service string
146+
147+
// HostIP is the address the entry asked to bind, empty for
148+
// compose's all-interfaces default.
149+
HostIP string
150+
151+
// Published is the host-side port, empty when the entry left it
152+
// to the daemon (`ports: ["8080"]` — an ephemeral host port).
153+
Published string
154+
155+
// Target is the container-side port the entry published.
156+
Target int
157+
158+
// Protocol is "tcp" or "udp", empty for compose's tcp default.
159+
Protocol string
160+
}
161+
162+
// String renders the entry in compose's own short syntax
163+
// (`127.0.0.1:8080:8080/tcp`), so a diagnostic can quote the line the
164+
// user wrote rather than a shape only this package knows.
165+
func (d DroppedHostPort) String() string {
166+
proto := d.Protocol
167+
if proto == "" {
168+
proto = "tcp"
169+
}
170+
out := strconv.Itoa(d.Target) + "/" + proto
171+
if d.Published != "" {
172+
out = d.Published + ":" + out
173+
}
174+
if d.HostIP != "" {
175+
out = d.HostIP + ":" + out
176+
}
177+
return out
178+
}
179+
180+
// ApplyDropHostPorts mutates project so no service publishes a host
181+
// port, and returns every entry it removed in service-name order.
182+
//
183+
// For an engine whose daemon's "host" is a namespace shared with
184+
// workloads it does not own — the engine running inside a Kubernetes
185+
// pod next to sidecars — a published port is not the project's to
186+
// take: it collides with whatever else already listens there, and
187+
// nothing consumes it, because service-to-service traffic goes over
188+
// the compose network by service name and an embedder forwarding a
189+
// port out of the namespace dials the container on that network.
190+
//
191+
// Dropping the entries here rather than inside the orchestrator is
192+
// deliberate: ConfigHash reads the project's ServiceConfig, so a
193+
// container created with the bindings drifts from one created
194+
// without them and is recreated, instead of being reused with its
195+
// publishes intact.
196+
//
197+
// Entries are removed whole, including ones that left `published`
198+
// unset — those bind an ephemeral host port, which is the same
199+
// namespace and the same problem.
200+
func ApplyDropHostPorts(project *composetypes.Project) []DroppedHostPort {
201+
if project == nil {
202+
return nil
203+
}
204+
205+
names := make([]string, 0, len(project.Services))
206+
for name := range project.Services {
207+
names = append(names, name)
208+
}
209+
sort.Strings(names)
210+
211+
var dropped []DroppedHostPort
212+
for _, name := range names {
213+
svc := project.Services[name]
214+
if len(svc.Ports) == 0 {
215+
continue
216+
}
217+
for _, p := range svc.Ports {
218+
dropped = append(dropped, DroppedHostPort{
219+
Service: name,
220+
HostIP: p.HostIP,
221+
Published: p.Published,
222+
Target: int(p.Target),
223+
Protocol: p.Protocol,
224+
})
225+
}
226+
svc.Ports = nil
227+
project.Services[name] = svc
228+
}
229+
return dropped
230+
}

‎engine.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,38 @@ type EngineOptions struct {
8484
// Native; then the default flips and the shellout path is
8585
// deleted.
8686
ComposeBackend ComposeBackend
87+
88+
// DisableHostPortPublishing drops the host side of compose
89+
// `ports:` entries. Services still start and nothing else about
90+
// them changes; they are simply not published. Each dropped entry
91+
// is reported as an events.WarnEvent with code
92+
// "compose_host_port_publish_skipped".
93+
//
94+
// Set it when the daemon's "host" is a namespace shared with other
95+
// workloads — the engine running inside a Kubernetes pod next to
96+
// sidecars — where binding a host port is neither useful nor safe:
97+
// a service publishing a common port (8080) loses the race against
98+
// a sidecar already listening there and the boot fails on a raw
99+
// "bind: address already in use". Nothing consumes the publish in
100+
// that shape anyway, so the binding only takes a scarce shared
101+
// port.
102+
//
103+
// Dropping it leaves connectivity intact — service-to-service
104+
// traffic goes over the compose network by service name, and an
105+
// embedder forwarding a port out of the namespace dials the
106+
// container on that network, never the host publish.
107+
//
108+
// It lives here rather than in devcontainer.json because it is a
109+
// property of where the engine is deployed, not of the project: a
110+
// user cannot know what their compose stack is scheduled next to,
111+
// and a config field would let them opt back into a binding that
112+
// cannot work.
113+
//
114+
// ComposeBackendNative only. The shellout backend hands `ports:`
115+
// to `docker compose`, which publishes them regardless, so a
116+
// compose-source Up under ComposeBackendShellout is refused rather
117+
// than run with the option silently ignored.
118+
DisableHostPortPublishing bool
87119
}
88120

89121
// ComposeBackend selects between the legacy shellout and the new
Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
//go:build integration
2+
3+
package integration
4+
5+
import (
6+
"context"
7+
"path/filepath"
8+
"strings"
9+
"testing"
10+
"time"
11+
12+
devcontainer "github.com/crunchloop/devcontainer"
13+
)
14+
15+
// writeComposePortsWorkspace lays out a project whose primary service
16+
// publishes a host port and whose sidecar answers on the compose
17+
// network. The two together are what DisableHostPortPublishing
18+
// claims: the publish is removable, the service-name path is not.
19+
func writeComposePortsWorkspace(t *testing.T) string {
20+
t.Helper()
21+
dir := t.TempDir()
22+
23+
mustWrite(t, filepath.Join(dir, "docker-compose.yml"), `
24+
services:
25+
app:
26+
image: `+testImage+`
27+
command: ["sh", "-c", "while sleep 1000; do :; done"]
28+
ports:
29+
- "39080:8080"
30+
db:
31+
image: `+testImage+`
32+
command: ["sh", "-c", "while true; do echo db-reachable | nc -l -p 5000; done"]
33+
`)
34+
35+
mustWrite(t, filepath.Join(dir, ".devcontainer", "devcontainer.json"), `{
36+
"dockerComposeFile": "../docker-compose.yml",
37+
"service": "app",
38+
"runServices": ["app", "db"],
39+
"workspaceFolder": "/workspaces/proj"
40+
}`)
41+
return dir
42+
}
43+
44+
// The option's claim is that dropping the host side of `ports:` is
45+
// semantically safe. This is the half a fake runtime cannot make: the
46+
// project still boots against a real daemon, and the sidecar is still
47+
// reachable by service name over the compose network — which is how
48+
// an embedder reaches it, and why nothing is lost by not publishing.
49+
//
50+
// What the host binding itself does (or, here, does not do) is
51+
// asserted at the RunSpec boundary in the unit tests; the daemon-side
52+
// translation of RunSpec.Ports is covered in runtime/docker.
53+
func TestComposeSource_DisableHostPortPublishing_BootsAndKeepsServiceDNS(t *testing.T) {
54+
if testing.Short() {
55+
t.Skip()
56+
}
57+
58+
eng, rt := newEngineWith(t, devcontainer.EngineOptions{
59+
ComposeBackend: devcontainer.ComposeBackendNative,
60+
DisableHostPortPublishing: true,
61+
})
62+
defer rt.Close()
63+
64+
ws := writeComposePortsWorkspace(t)
65+
66+
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Minute)
67+
defer cancel()
68+
69+
wsObj, err := eng.Up(ctx, devcontainer.UpOptions{
70+
LocalWorkspaceFolder: ws,
71+
Recreate: true,
72+
SkipLifecycle: true,
73+
})
74+
if err != nil {
75+
t.Fatalf("Up: %v", err)
76+
}
77+
defer func() {
78+
_ = eng.Down(context.Background(), wsObj, devcontainer.DownOptions{
79+
Remove: true,
80+
RemoveVolumes: true,
81+
})
82+
}()
83+
84+
// Retry: nothing orders the sidecar's listener against this exec,
85+
// and busybox nc serves one connection per accept.
86+
res, err := eng.Exec(ctx, wsObj, devcontainer.ExecOptions{
87+
Cmd: []string{"sh", "-c", `for i in $(seq 1 30); do out=$(nc -w 5 db 5000); if [ -n "$out" ]; then echo "$out"; exit 0; fi; sleep 1; done; exit 1`},
88+
})
89+
if err != nil {
90+
t.Fatalf("Exec nc db: %v", err)
91+
}
92+
if !strings.Contains(res.Stdout, "db-reachable") {
93+
t.Errorf("sidecar unreachable by service name: stdout=%q stderr=%q", res.Stdout, res.Stderr)
94+
}
95+
}

0 commit comments

Comments
 (0)