From deb2d773ead1b999fef5a20711a2ca4de47194be Mon Sep 17 00:00:00 2001 From: Nicolas De Loof Date: Tue, 25 Aug 2026 16:40:37 +0200 Subject: [PATCH] refactor: NewGraph stops mutating the project MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Building the dependency graph silently deleted unresolvable optional depends_on references from the caller's project — and since DependsOn is a map, even from projects sharing it. What later readers observed depended on how many graphs had been built before them. The pruning becomes an explicit step, applied once where the project consumed by the engines is established — create and start — before the graph, the dependency waits and the com.docker.compose.depends_on label read the model, using Project.WithoutUnresolvedOptionalDependencies introduced by compose-spec/compose-go#922 (go.mod bumped to the post-merge commit). NewGraph now just skips the edge; required dangling references still flow to its existing errors (naming the profiles that would enable the dependency). No behavior change; epic #14081, Lot 0 step 4 — precondition for a canonical project object across plan phases. Signed-off-by: Nicolas De Loof --- go.mod | 2 +- go.sum | 4 ++-- pkg/compose/create.go | 5 +++++ pkg/compose/dependencies.go | 10 ++++++---- pkg/compose/dependencies_test.go | 34 ++++++++++++++++++++++++++++++++ pkg/compose/start.go | 4 ++++ 6 files changed, 52 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index bd1f292cd7f..4867710b128 100644 --- a/go.mod +++ b/go.mod @@ -8,7 +8,7 @@ require ( github.com/Microsoft/go-winio v0.6.3-0.20251027160822-ad3df93bed29 github.com/acarl005/stripansi v0.0.0-20180116102854-5a71ef0e047d github.com/buger/goterm v1.0.4 - github.com/compose-spec/compose-go/v2 v2.14.0 + github.com/compose-spec/compose-go/v2 v2.14.1-0.20260825154407-6c1c2d727681 github.com/containerd/console v1.0.5 github.com/containerd/containerd/v2 v2.3.3 github.com/containerd/errdefs v1.0.0 diff --git a/go.sum b/go.sum index 97d942f32c0..5266c37f7de 100644 --- a/go.sum +++ b/go.sum @@ -110,8 +110,8 @@ github.com/cloudflare/circl v1.6.3 h1:9GPOhQGF9MCYUeXyMYlqTR6a5gTrgR/fBLXvUgtVcg github.com/cloudflare/circl v1.6.3/go.mod h1:2eXP6Qfat4O/Yhh8BznvKnJ+uzEoTQ6jVKJRn81BiS4= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb h1:EDmT6Q9Zs+SbUoc7Ik9EfrFqcylYqgPZ9ANSbTAntnE= github.com/codahale/rfc6979 v0.0.0-20141003034818-6a90f24967eb/go.mod h1:ZjrT6AXHbDs86ZSdt/osfBi5qfexBrKUdONk989Wnk4= -github.com/compose-spec/compose-go/v2 v2.14.0 h1:uaJeo5B3+OVlu+Rx2qLBcAdXPEUUzm5nQrRiGJafRAQ= -github.com/compose-spec/compose-go/v2 v2.14.0/go.mod h1:ZU6zlcweCZKyiB7BVfCizQT9XmkEIMFE+PRZydVcsZg= +github.com/compose-spec/compose-go/v2 v2.14.1-0.20260825154407-6c1c2d727681 h1:jqD2pgesJ/zRJwgaefxuSuT+/TGUlarjB93Zde40UtI= +github.com/compose-spec/compose-go/v2 v2.14.1-0.20260825154407-6c1c2d727681/go.mod h1:ZU6zlcweCZKyiB7BVfCizQT9XmkEIMFE+PRZydVcsZg= github.com/containerd/cgroups/v3 v3.1.3 h1:eUNflyMddm18+yrDmZPn3jI7C5hJ9ahABE5q6dyLYXQ= github.com/containerd/cgroups/v3 v3.1.3/go.mod h1:PKZ2AcWmSBsY/tJUVhtS/rluX0b1uq1GmPO1ElCmbOw= github.com/containerd/console v1.0.5 h1:R0ymNeydRqH2DmakFNdmjR2k0t7UPuiOV/N/27/qqsc= diff --git a/pkg/compose/create.go b/pkg/compose/create.go index c52c2e5863a..a8c03718126 100644 --- a/pkg/compose/create.go +++ b/pkg/compose/create.go @@ -70,6 +70,11 @@ func (s *composeService) create(ctx context.Context, project *types.Project, opt options.Services = project.ServiceNames() } + // resolve the model once: optional depends_on references left dangling by + // profiles or service selection are pruned before anything (dependency + // graph, container labels) reads them + project = project.WithoutUnresolvedOptionalDependencies() + err := project.CheckContainerNameUnicity() if err != nil { return err diff --git a/pkg/compose/dependencies.go b/pkg/compose/dependencies.go index 215e92a3c0f..a502b4645cf 100644 --- a/pkg/compose/dependencies.go +++ b/pkg/compose/dependencies.go @@ -247,7 +247,11 @@ func (v *Vertex) GetChildren() []*Vertex { return res } -// NewGraph returns the dependency graph of the services +// NewGraph returns the dependency graph of the services. It never modifies +// the project: an optional (required: false) dependency on a service absent +// from the model simply contributes no edge; pruning such references from +// the model itself is the caller's explicit decision — see +// Project.WithoutUnresolvedOptionalDependencies. func NewGraph(project *types.Project, initialStatus ServiceStatus) (*Graph, error) { graph := &Graph{ lock: sync.RWMutex{}, @@ -258,13 +262,11 @@ func NewGraph(project *types.Project, initialStatus ServiceStatus) (*Graph, erro graph.AddVertex(s.Name, s.Name, initialStatus) } - for index, s := range project.Services { + for _, s := range project.Services { for _, name := range s.GetDependencies() { err := graph.AddEdge(s.Name, name) if err != nil { if !s.DependsOn[name].Required { - delete(s.DependsOn, name) - project.Services[index] = s continue } if api.IsNotFoundError(err) { diff --git a/pkg/compose/dependencies_test.go b/pkg/compose/dependencies_test.go index 947a9bd4a3d..74b22f583b1 100644 --- a/pkg/compose/dependencies_test.go +++ b/pkg/compose/dependencies_test.go @@ -323,6 +323,40 @@ func TestBuildGraphDependsOn(t *testing.T) { } } +// NewGraph must never rewrite the project it reads: pruning unresolved +// optional dependencies is an explicit, separate step. Building a graph any +// number of times leaves the model byte-identical, so what later readers +// (dependency waits, container labels) observe no longer depends on how many +// graphs were built before them. +func TestNewGraphDoesNotMutateProject(t *testing.T) { + project := &types.Project{ + Services: types.Services{ + "app": { + Name: "app", + DependsOn: types.DependsOnConfig{ + "db": {Condition: types.ServiceConditionStarted, Required: true}, + "debug": {Condition: types.ServiceConditionStarted, Required: false}, + }, + }, + "db": {Name: "db"}, + }, + DisabledServices: types.Services{ + "debug": {Name: "debug", Profiles: []string{"debug"}}, + }, + } + + for range 2 { + graph, err := NewGraph(project, ServiceStopped) + assert.NilError(t, err) + // the unresolved optional dependency contributes no edge... + assert.Equal(t, len(graph.Vertices["app"].Children), 1) + // ...but stays in the model + assert.Equal(t, len(project.Services["app"].DependsOn), 2) + _, ok := project.Services["app"].DependsOn["debug"] + assert.Check(t, ok) + } +} + func isVertexEqual(a, b Vertex) bool { childrenEquality := true for c := range a.Children { diff --git a/pkg/compose/start.go b/pkg/compose/start.go index 8af52e97c13..302fb5885b5 100644 --- a/pkg/compose/start.go +++ b/pkg/compose/start.go @@ -48,6 +48,10 @@ func (s *composeService) start(ctx context.Context, projectName string, options return err } } + // resolve the model once: optional depends_on references left dangling by + // profiles or service selection are pruned before the dependency graph + // and the dependency waits read them + project = project.WithoutUnresolvedOptionalDependencies() res, err := s.apiClient().ContainerList(ctx, client.ContainerListOptions{ Filters: projectFilter(project.Name).Add("label", oneOffFilter(false)),