From 4596823d5cea7cc8b7cb0da2ebc940e8e824a2bc Mon Sep 17 00:00:00 2001 From: bilby91 Date: Sat, 22 Aug 2026 23:13:44 -0300 Subject: [PATCH] fix(compose): dependency closure for restricted plans, carry namespace modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round-2 adversarial findings on the native backend switch in dap: 1. A Plan restricted to a service subset (devcontainer runServices) started exactly the named services — docker compose up starts their transitive dependency closure. makeKeepSet and the engine's sidecar-image builds now expand the selection through the new compose.ServiceClosure (depends_on plus service: namespace edges, the same edge set TopoSort orders by), so a dependency outside runServices is built and started again. 2. network_mode/pid/ipc were silently dropped: serviceToRunSpec always attached the project network and the runtime spec had no namespace fields, so `network_mode: none` — an explicit isolation request — received full project-network connectivity. RunSpec gains NetworkMode/PidMode/IpcMode (Docker HostConfig syntax), the docker backend maps them, service: resolves to the dependency's started container ID, and a service with a network mode skips the project network per Docker's API constraints. TopoSort now also treats pid:/ipc: service references as ordering edges. Backends without namespace sharing are unaffected: Plan.Validate already refuses these projects via Capabilities.NamespaceSharing. Co-Authored-By: Claude Fable 5 --- CHANGELOG.md | 17 ++++++ compose/graph.go | 76 ++++++++++++++++++++---- compose/orchestrator.go | 79 +++++++++++++++++++++++-- compose/orchestrator_test.go | 110 ++++++++++++++++++++++++++++++++++- runtime/docker/run.go | 12 ++++ runtime/runtime.go | 15 ++++- up.go | 15 +++-- up_compose_sidecar_test.go | 23 ++++++++ 8 files changed, 323 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2434108..a9a8269 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **compose (native)** — a plan restricted to a service subset + (devcontainer `runServices`) now starts and builds the transitive + dependency closure of the selection, matching `docker compose up + `. Previously a dependency outside the list was neither + built nor started, so the primary came up with its `depends_on` + absent. +- **compose (native)** — `network_mode:`, `pid:` and `ipc:` are now + carried through to the backend instead of being silently replaced by + the project network. `service:` references resolve to the + dependency's container (ordering already guaranteed by the + dependency graph); a service with a network mode is not attached to + the project network, matching Docker's API constraints. In + particular `network_mode: none` — an explicit isolation request — + previously received full project-network connectivity. + ## [0.4.1] - 2026-08-22 ### Fixed diff --git a/compose/graph.go b/compose/graph.go index 5161491..cb551dd 100644 --- a/compose/graph.go +++ b/compose/graph.go @@ -40,16 +40,9 @@ func TopoSort(project *composetypes.Project) ([]Level, error) { deps[name] = map[string]struct{}{} } for name, svc := range services { - for dep := range svc.DependsOn { - if _, ok := services[dep]; !ok { - continue - } - deps[name][dep] = struct{}{} - } - if nm := svc.NetworkMode; isServiceNetworkMode(nm) { - peer := nm[len("service:"):] - if _, ok := services[peer]; ok && peer != name { - deps[name][peer] = struct{}{} + for _, dep := range serviceEdges(svc) { + if _, ok := services[dep]; ok && dep != name { + deps[name][dep] = struct{}{} } } } @@ -145,6 +138,67 @@ func findCycle(deps map[string]map[string]struct{}, remaining map[string]struct{ // orchestrator surfaces the dep edge here so topo-sort respects the // ordering even though compose-go doesn't model it under DependsOn. func isServiceNetworkMode(nm string) bool { + return serviceRefTarget(nm) != "" +} + +// serviceRefTarget returns the service name a `service:` +// namespace-mode value points at, or "" when the value is anything +// else (empty, "host", "none", "container:", ...). +func serviceRefTarget(v string) string { const p = "service:" - return len(nm) > len(p) && nm[:len(p)] == p + if len(v) > len(p) && v[:len(p)] == p { + return v[len(p):] + } + return "" +} + +// serviceEdges lists the services svc depends on: depends_on entries +// plus the implicit edges from `network_mode: service:` and the +// pid/ipc equivalents — joining another service's namespace requires +// that service's container to exist first. +func serviceEdges(svc composetypes.ServiceConfig) []string { + var out []string + for dep := range svc.DependsOn { + out = append(out, dep) + } + for _, mode := range []string{svc.NetworkMode, svc.Pid, svc.Ipc} { + if peer := serviceRefTarget(mode); peer != "" { + out = append(out, peer) + } + } + return out +} + +// ServiceClosure returns names plus the transitive closure of their +// dependencies (the same edge set TopoSort orders by). `docker +// compose up ` starts the named services AND everything +// they depend on; callers restricting a Plan to a service subset use +// this to reproduce that contract. Names not present in the project +// are kept verbatim (Plan validation surfaces them); the result is +// sorted for determinism. +func ServiceClosure(project *composetypes.Project, names []string) []string { + if project == nil { + return append([]string(nil), names...) + } + seen := map[string]bool{} + queue := append([]string(nil), names...) + for len(queue) > 0 { + name := queue[0] + queue = queue[1:] + if seen[name] { + continue + } + seen[name] = true + svc, ok := project.Services[name] + if !ok { + continue + } + queue = append(queue, serviceEdges(svc)...) + } + out := make([]string, 0, len(seen)) + for name := range seen { + out = append(out, name) + } + sort.Strings(out) + return out } diff --git a/compose/orchestrator.go b/compose/orchestrator.go index bda0226..d6c62ce 100644 --- a/compose/orchestrator.go +++ b/compose/orchestrator.go @@ -169,6 +169,18 @@ func (o *Orchestrator) Up(ctx context.Context, plan *Plan) (UpResult, error) { // limited which services to bring up. keep := makeKeepSet(plan) + // resolveContainer maps a service name to its already-started + // container ID, for `service:` namespace-mode references. The + // target is always in an earlier level (serviceEdges makes it a + // TopoSort dependency), but same-level writes to the map are + // concurrent, so reads take the same lock. + var idsMu sync.Mutex + resolveContainer := func(name string) string { + idsMu.Lock() + defer idsMu.Unlock() + return res.ContainerIDs[name] + } + for _, level := range levels { var started []string var startMu sync.Mutex @@ -188,7 +200,7 @@ func (o *Orchestrator) Up(ctx context.Context, plan *Plan) (UpResult, error) { if !ok { return } - id, err := o.ensureService(ctx, plan, svc, projectLabels) + id, err := o.ensureService(ctx, plan, svc, projectLabels, resolveContainer) startMu.Lock() defer startMu.Unlock() if err != nil { @@ -198,7 +210,9 @@ func (o *Orchestrator) Up(ctx context.Context, plan *Plan) (UpResult, error) { } return } + idsMu.Lock() res.ContainerIDs[svcName] = id + idsMu.Unlock() started = append(started, svcName) }() } @@ -449,6 +463,7 @@ func (o *Orchestrator) ensureService( plan *Plan, svc composetypes.ServiceConfig, projectLabels map[string]string, + resolveContainer func(string) string, ) (string, error) { // Resolve the service's image to its digest before hashing. The // compose file carries a tag (e.g. "postgres:17-alpine") which is @@ -520,7 +535,10 @@ func (o *Orchestrator) ensureService( } } - spec := serviceToRunSpec(plan, svc, projectLabels, hash, imageDigest) + spec, err := serviceToRunSpec(plan, svc, projectLabels, hash, imageDigest, resolveContainer) + if err != nil { + return "", err + } if o.selfProbe { // The orchestrator probes health itself (see waitFor); explicitly // DISABLE the backend's native HEALTHCHECK. Nil would mean "inherit @@ -788,7 +806,8 @@ func serviceToRunSpec( svc composetypes.ServiceConfig, projectLabels map[string]string, hash, imageDigest string, -) runtime.RunSpec { + resolveContainer func(string) string, +) (runtime.RunSpec, error) { labels := copyLabels(plan.Labels) for k, v := range projectLabels { labels[k] = v @@ -824,6 +843,29 @@ func serviceToRunSpec( }) } + // Namespace modes, resolved to HostConfig syntax. A service that + // joins another container's network namespace (or opts out with + // "none"/"host") must not also get per-network endpoint config — + // Docker rejects the combination — so the project network is + // skipped for it. Service-name DNS is then the joined namespace's + // concern, matching `docker compose` semantics. + networkMode, err := resolveNamespaceMode(svc.NetworkMode, resolveContainer) + if err != nil { + return runtime.RunSpec{}, fmt.Errorf("service %q network_mode: %w", svc.Name, err) + } + pidMode, err := resolveNamespaceMode(svc.Pid, resolveContainer) + if err != nil { + return runtime.RunSpec{}, fmt.Errorf("service %q pid: %w", svc.Name, err) + } + ipcMode, err := resolveNamespaceMode(svc.Ipc, resolveContainer) + if err != nil { + return runtime.RunSpec{}, fmt.Errorf("service %q ipc: %w", svc.Name, err) + } + networks := []string{plan.ProjectName + "_default"} + if networkMode != "" { + networks = nil + } + memBytes, nanoCPUs := resourcesOf(svc) return runtime.RunSpec{ Image: svc.Image, @@ -835,7 +877,10 @@ func serviceToRunSpec( Env: env, Labels: labels, Mounts: mounts, - Networks: []string{plan.ProjectName + "_default"}, + Networks: networks, + NetworkMode: networkMode, + PidMode: pidMode, + IpcMode: ipcMode, Ports: portsOf(svc.Ports), RestartPolicy: restartPolicyOf(svc.Restart), HealthCheck: healthCheckOf(svc.HealthCheck), @@ -845,7 +890,25 @@ func serviceToRunSpec( SecurityOpt: svc.SecurityOpt, MemoryBytes: memBytes, NanoCPUs: nanoCPUs, - } + }, nil +} + +// resolveNamespaceMode translates a compose namespace-mode value into +// Docker HostConfig syntax. `service:` becomes `container:` +// via the dependency's already-started container (serviceEdges makes +// the target a TopoSort dependency, so it lives in an earlier level); +// everything else — "", "host", "none", "container:" — passes +// through verbatim. +func resolveNamespaceMode(v string, resolveContainer func(string) string) (string, error) { + target := serviceRefTarget(v) + if target == "" { + return v, nil + } + id := resolveContainer(target) + if id == "" { + return "", fmt.Errorf("mode %q: service %q has no started container to join", v, target) + } + return "container:" + id, nil } // resourcesOf extracts the memory + CPU limits from a compose service. @@ -992,7 +1055,11 @@ func makeKeepSet(plan *Plan) map[string]bool { } return keep } - for _, name := range plan.Services { + // `docker compose up ` starts the named services and + // their transitive dependencies; a restricted Plan keeps that + // contract, or a kept service would come up with its depends_on + // (or `service:` namespace target) absent. + for _, name := range ServiceClosure(plan.Project, plan.Services) { keep[name] = true } return keep diff --git a/compose/orchestrator_test.go b/compose/orchestrator_test.go index a8f9c23..6b4070e 100644 --- a/compose/orchestrator_test.go +++ b/compose/orchestrator_test.go @@ -313,7 +313,10 @@ func TestServiceToRunSpec_CarriesSecurityFields(t *testing.T) { CapAdd: []string{"SYS_ADMIN"}, SecurityOpt: []string{"seccomp=unconfined"}, } - spec := serviceToRunSpec(&Plan{ProjectName: "dc-x"}, svc, nil, "hash", "") + spec, err := serviceToRunSpec(&Plan{ProjectName: "dc-x"}, svc, nil, "hash", "", func(string) string { return "" }) + if err != nil { + t.Fatalf("serviceToRunSpec: %v", err) + } if !spec.Privileged { t.Error("Privileged not carried into RunSpec") @@ -967,3 +970,108 @@ func TestUp_AdoptsRunningForeignContainerWithoutStarting(t *testing.T) { rt.removeCalls, rt.runCalls, rt.startCalls) } } + +// Restricting a plan to a service subset must keep `docker compose up +// ` semantics: dependencies (including `service:` +// namespace targets) come up too. +func TestUp_RestrictedServicesStartDependencyClosure(t *testing.T) { + rt := newMockRuntime() + orch := NewOrchestrator(rt, "docker") + proj := newProject(t, map[string][]string{ + "db": nil, + "app": {"db"}, + "unwanted": nil, + }) + + res, err := orch.Up(context.Background(), &Plan{ + Project: proj, ProjectName: "dc-x", Services: []string{"app"}, + }) + if err != nil { + t.Fatalf("Up: %v", err) + } + if res.ContainerIDs["app"] == "" || res.ContainerIDs["db"] == "" { + t.Errorf("ContainerIDs = %+v, want app AND its dependency db", res.ContainerIDs) + } + if _, started := res.ContainerIDs["unwanted"]; started { + t.Error("service outside the closure was started") + } +} + +func TestServiceClosure_FollowsNamespaceEdges(t *testing.T) { + proj := newProject(t, map[string][]string{"proxy": nil, "app": nil, "other": nil}) + app := proj.Services["app"] + app.NetworkMode = "service:proxy" + proj.Services["app"] = app + + got := ServiceClosure(proj, []string{"app"}) + want := []string{"app", "proxy"} + if len(got) != len(want) || got[0] != want[0] || got[1] != want[1] { + t.Errorf("ServiceClosure = %v, want %v", got, want) + } +} + +// network_mode must reach the backend, not be silently replaced by +// the project network — `none` in particular is an isolation request. +func TestUp_NetworkModeCarriedAndProjectNetworkSkipped(t *testing.T) { + rt := newMockRuntime() + orch := NewOrchestrator(rt, "docker") + proj := newProject(t, map[string][]string{"app": nil, "sandboxed": nil}) + sandboxed := proj.Services["sandboxed"] + sandboxed.NetworkMode = "none" + sandboxed.Pid = "host" + proj.Services["sandboxed"] = sandboxed + + var mu sync.Mutex + specs := map[string]runtime.RunSpec{} + rt.OnRunContainer = func(spec runtime.RunSpec) (*runtime.Container, error) { + mu.Lock() + specs[spec.Labels[LabelComposeService]] = spec + mu.Unlock() + return nil, nil + } + + if _, err := orch.Up(context.Background(), &Plan{Project: proj, ProjectName: "dc-x"}); err != nil { + t.Fatalf("Up: %v", err) + } + if got := specs["sandboxed"].NetworkMode; got != "none" { + t.Errorf("sandboxed NetworkMode = %q, want none", got) + } + if got := specs["sandboxed"].PidMode; got != "host" { + t.Errorf("sandboxed PidMode = %q, want host", got) + } + if nets := specs["sandboxed"].Networks; len(nets) != 0 { + t.Errorf("sandboxed Networks = %v; a namespace mode excludes the project network", nets) + } + if nets := specs["app"].Networks; len(nets) != 1 { + t.Errorf("app Networks = %v, want the project network", nets) + } +} + +// `service:` resolves to the dependency's container ID, which the +// topo order guarantees exists by the time the dependent is created. +func TestUp_ServiceNetworkModeResolvesToContainer(t *testing.T) { + rt := newMockRuntime() + orch := NewOrchestrator(rt, "docker") + proj := newProject(t, map[string][]string{"proxy": nil, "app": nil}) + app := proj.Services["app"] + app.NetworkMode = "service:proxy" + proj.Services["app"] = app + + var mu sync.Mutex + specs := map[string]runtime.RunSpec{} + rt.OnRunContainer = func(spec runtime.RunSpec) (*runtime.Container, error) { + mu.Lock() + specs[spec.Labels[LabelComposeService]] = spec + mu.Unlock() + return nil, nil + } + + res, err := orch.Up(context.Background(), &Plan{Project: proj, ProjectName: "dc-x"}) + if err != nil { + t.Fatalf("Up: %v", err) + } + want := "container:" + res.ContainerIDs["proxy"] + if got := specs["app"].NetworkMode; got != want { + t.Errorf("app NetworkMode = %q, want %q", got, want) + } +} diff --git a/runtime/docker/run.go b/runtime/docker/run.go index 2c4f6d8..8734bfb 100644 --- a/runtime/docker/run.go +++ b/runtime/docker/run.go @@ -66,6 +66,18 @@ func (r *Runtime) RunContainer(ctx context.Context, spec runtime.RunSpec) (*runt t := true hostCfg.Init = &t } + // Namespace modes are exclusive with per-network endpoint config at + // the Docker API level; the orchestrator leaves Networks empty when + // NetworkMode is set, so toNetworkingConfig below returns nil. + if spec.NetworkMode != "" { + hostCfg.NetworkMode = container.NetworkMode(spec.NetworkMode) + } + if spec.PidMode != "" { + hostCfg.PidMode = container.PidMode(spec.PidMode) + } + if spec.IpcMode != "" { + hostCfg.IpcMode = container.IpcMode(spec.IpcMode) + } res, err := r.api.ContainerCreate(ctx, client.ContainerCreateOptions{ Name: spec.Name, diff --git a/runtime/runtime.go b/runtime/runtime.go index cf7015a..778c607 100644 --- a/runtime/runtime.go +++ b/runtime/runtime.go @@ -438,9 +438,22 @@ type RunSpec struct { // means "backend default" — docker assigns the default bridge; // apple assigns the built-in vmnet network. Used by the compose // orchestrator to attach services to the project network it - // just created via CreateNetwork. + // just created via CreateNetwork. Mutually exclusive with + // NetworkMode. Networks []string + // NetworkMode, PidMode and IpcMode carry the container's kernel + // namespace modes in Docker HostConfig syntax ("none", "host", + // "container:"). Empty means the backend default. + // The compose orchestrator translates `network_mode:` / `pid:` / + // `ipc:` directives here, resolving `service:` references to + // the dependency's container first. Backends without namespace + // sharing never see these: compose.Plan.Validate refuses such + // projects via Capabilities.NamespaceSharing. + NetworkMode string + PidMode string + IpcMode string + // Ports lists the ports this container publishes to the host. // Empty means no publishing (the container's ports are reachable // inside the project network but not from the host). Used by diff --git a/up.go b/up.go index 1675030..fe58192 100644 --- a/up.go +++ b/up.go @@ -779,11 +779,16 @@ func (e *Engine) buildComposeSidecarImages( projectName, workingDir string, opts UpOptions, ) error { - selected := func(name string) bool { - if len(src.RunServices) == 0 { - return true - } - return slices.Contains(src.RunServices, name) + // Selection mirrors `docker compose up `: + // the named services plus their transitive dependencies (the same + // closure the orchestrator keeps), so a dependency of a selected + // service gets its image built even when runServices doesn't name + // it directly. + selected := func(string) bool { return true } + if len(src.RunServices) > 0 { + closure := compose.ServiceClosure(project, + append(append([]string(nil), src.RunServices...), src.Service)) + selected = func(name string) bool { return slices.Contains(closure, name) } } // Deterministic build order: map iteration would shuffle build // output (and any failure) between runs. diff --git a/up_compose_sidecar_test.go b/up_compose_sidecar_test.go index ac993bd..a5755be 100644 --- a/up_compose_sidecar_test.go +++ b/up_compose_sidecar_test.go @@ -129,3 +129,26 @@ func TestBuildComposeSidecarImages_HonorsRunServices(t *testing.T) { t.Error("unselected service was mutated") } } + +// A dependency of a selected service must be built even when +// runServices doesn't name it directly — `docker compose up app` +// builds app's depends_on closure. +func TestBuildComposeSidecarImages_BuildsDependenciesOfSelection(t *testing.T) { + eng, rt := sidecarEngine(t) + + project := sidecarProject(map[string]composetypes.ServiceConfig{ + "app": {Image: "dc-final:latest", DependsOn: composetypes.DependsOnConfig{ + "db": composetypes.ServiceDependency{Condition: "service_started"}, + }}, + "db": {Build: &composetypes.BuildConfig{Context: "./db"}}, + "excluded": {Build: &composetypes.BuildConfig{Context: "./excluded"}}, + }) + src := &config.ComposeSource{Service: "app", RunServices: []string{"app"}} + + if err := eng.buildComposeSidecarImages(context.Background(), project, src, "dc-x", t.TempDir(), sidecarUpOptions()); err != nil { + t.Fatalf("buildComposeSidecarImages: %v", err) + } + if len(rt.builds) != 1 || rt.builds[0].Tag != "dc-x-db" { + t.Fatalf("builds = %+v, want app's dependency db and nothing else", rt.builds) + } +}