Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pkg/compose/watch.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,10 @@ func (s *composeService) Watch(ctx context.Context, project *types.Project, opti
return wait()
}

func selectWatchServices(project *types.Project, services []string) (*types.Project, error) {
return project.WithSelectedServices(services, types.IgnoreDependencies)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM — LIKELY] selectWatchServices with IgnoreDependencies may strip dependency metadata needed by create/start

selectWatchServices now calls project.WithSelectedServices(services, types.IgnoreDependencies), which removes all dependency services from the in-memory project object used throughout the entire watch session.

Previously the default mode (IncludeDependencies) kept dependency service configs in scope. With IgnoreDependencies, if a user watches only frontend (which depends_on: backend), backend's service config is stripped from the project variable early on (line 195). Later in rebuild(), the create and start calls operate on this stripped project:

p, err := project.WithSelectedServices(services, types.IncludeDependents)
err = s.start(ctx, project.Name, api.StartOptions{
    Project:  p,      // p derived from the already-stripped project
    Services: services,
}, nil)

If s.create() (line 669) or s.start() needs to resolve network membership, shared volumes, or environment interpolation from a dependency service's config, it will find it absent from the project. This is a broader change than the targeted Build.Deps = false fix and may introduce regressions for setups where the dependency service config is needed during container re-creation.

The Build.Deps = false fix on lines 643 is sufficient to prevent unnecessary dependency rebuilds. It's worth verifying that IgnoreDependencies here is intentional and that create/start do not rely on the full project graph.

}

type watchRule struct {
types.Trigger
include watch.PathMatcher
Expand Down Expand Up @@ -188,7 +192,7 @@ func (r watchRule) Matches(event watch.FileEvent) *sync.PathMapping {

func (s *composeService) watch(ctx context.Context, project *types.Project, options api.WatchOptions) (func() error, error) { //nolint: gocyclo
var err error
if project, err = project.WithSelectedServices(options.Services); err != nil {
if project, err = selectWatchServices(project, options.Services); err != nil {
return nil, err
}
syncer, err := s.getSyncImplementation(project)
Expand Down Expand Up @@ -636,6 +640,7 @@ func (s *composeService) rebuild(ctx context.Context, project *types.Project, se
options.LogTo.Log(api.WatchLogger, fmt.Sprintf("Rebuilding service(s) %q after changes were detected...", services))
// restrict the build to ONLY this service, not any of its dependencies
options.Build.Services = services

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[MEDIUM] Data race and shared-state mutation via *BuildOptions pointer in rebuild()

rebuild() receives options api.WatchOptions by value, but options.Build is a *BuildOptions pointer that still references the same underlying struct allocated in NewWatcher / up. The three mutations on lines 642–645:

options.Build.Services = services
options.Build.Deps = false
options.Build.Progress = string(progressui.PlainMode)
options.Build.Out = cutils.GetWriter(...)

…all write directly through the shared pointer to the original BuildOptions. This creates two problems:

  1. Data race: If two file-watch events fire concurrently (e.g., both frontend and another watched service change at the same time), two goroutines will concurrently write to options.Build.Services and options.Build.Out on the same struct — an unsynchronised concurrent write in Go, detectable by the race detector.

  2. Stale shared state: Build.Services is permanently overwritten after each rebuild on the shared struct referenced by w.options. While this is harmless for Deps (it's always forced to false), it is a structural side-effect on shared state rather than an intentional isolated change.

Suggested fix — make a local copy of BuildOptions before mutating it:

bo := *options.Build  // shallow copy of the value struct
bo.Services = services
bo.Deps = false
bo.Progress = string(progressui.PlainMode)
bo.Out = cutils.GetWriter(...)
options.Build = &bo   // point to local copy, not the shared one

options.Build.Deps = false
options.Build.Progress = string(progressui.PlainMode)
options.Build.Out = cutils.GetWriter(func(line string) {
options.LogTo.Log(api.WatchLogger, line)
Expand Down
31 changes: 31 additions & 0 deletions pkg/compose/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,37 @@ func TestWatch_Sync(t *testing.T) {
// TODO: there's not a great way to assert that the rebuild attempt happened
}

func TestSelectWatchServicesIgnoresDependencies(t *testing.T) {
project := &types.Project{
Name: "myProjectName",
Services: types.Services{
"backend": {
Name: "backend",
},
"stats": {
Name: "stats",
DependsOn: types.DependsOnConfig{
"backend": {
Condition: types.ServiceConditionStarted,
Restart: true,
Required: true,
},
},
},
},
}

selected, err := selectWatchServices(project, []string{"stats"})
assert.NilError(t, err)

_, ok := selected.Services["stats"]
assert.Assert(t, ok)
_, ok = selected.Services["backend"]
assert.Assert(t, !ok)
assert.Assert(t, len(selected.Services["stats"].DependsOn) == 0)
assert.Assert(t, len(project.Services["stats"].DependsOn) != 0)
}

type fakeSyncer struct {
synced chan []*sync.PathMapping
}
Expand Down
19 changes: 19 additions & 0 deletions pkg/e2e/fixtures/watch/rebuild-deps.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
services:
backend:
build:
dockerfile_inline: |
FROM nginx
RUN mkdir /data
COPY backend /data/backend
frontend:
build:
dockerfile_inline: |
FROM nginx
RUN mkdir /data
COPY frontend /data/frontend
depends_on:
- backend
develop:
watch:
- path: frontend
action: rebuild
55 changes: 55 additions & 0 deletions pkg/e2e/watch_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,61 @@ func TestWatchMultiServices(t *testing.T) {
c.RunDockerComposeCmdNoCheck(t, "-p", projectName, "kill", "-s", "9")
}

func TestWatchRebuildIgnoresDependencies(t *testing.T) {
c := NewCLI(t)
const projectName = "test_watch_rebuild_deps"

defer c.cleanupWithDown(t, projectName)

tmpdir := t.TempDir()
composeFilePath := filepath.Join(tmpdir, "compose.yaml")
CopyFile(t, filepath.Join("fixtures", "watch", "rebuild-deps.yaml"), composeFilePath)

for _, svc := range []string{"backend", "frontend"} {
assert.NilError(t, os.WriteFile(filepath.Join(tmpdir, svc), []byte("v1"), 0o600))
}

cmd := c.NewDockerComposeCmd(t, "-p", projectName, "-f", composeFilePath, "up", "--build", "--watch")
buffer := bytes.NewBuffer(nil)
cmd.Stdout = buffer
watch := icmd.StartCmd(cmd)
t.Cleanup(func() {
if watch.Cmd.Process != nil {
_ = watch.Cmd.Process.Kill()
}
})
Comment on lines +388 to +393

poll.WaitOn(t, func(l poll.LogT) poll.Result {
if strings.Contains(watch.Stdout(), "Attaching to ") {
return poll.Success()
}
return poll.Continue("%v", watch.Stdout())
}, poll.WithTimeout(90*time.Second))

containerID := func(service string) string {
res := c.RunDockerComposeCmd(t, "-p", projectName, "ps", "-q", service)
return strings.TrimSpace(res.Stdout())
}
backendID := containerID("backend")
assert.Assert(t, backendID != "")

t.Log("editing frontend code only")
assert.NilError(t, os.WriteFile(filepath.Join(tmpdir, "frontend"), []byte("v2"), 0o600))

poll.WaitOn(t, func(l poll.LogT) poll.Result {
cat := c.RunDockerComposeCmdNoCheck(t, "-p", projectName, "exec", "frontend", "cat", "/data/frontend")
if strings.Contains(cat.Stdout(), "v2") {
return poll.Success()
}
return poll.Continue("%v", cat.Combined())
}, poll.WithTimeout(90*time.Second))

t.Log("backend must not be rebuilt nor recreated")
assert.Equal(t, backendID, containerID("backend"))
Comment on lines +402 to +421

c.RunDockerComposeCmdNoCheck(t, "-p", projectName, "kill", "-s", "9")
}

func TestWatchIncludes(t *testing.T) {
c := NewCLI(t)
const projectName = "test_watch_includes"
Expand Down
Loading