diff --git a/internal/agent/inventory/collector.go b/internal/agent/inventory/collector.go index 120c411..832599a 100644 --- a/internal/agent/inventory/collector.go +++ b/internal/agent/inventory/collector.go @@ -279,7 +279,18 @@ func (c *Collector) markBuilt(ctx context.Context, byKey map[string]protocol.Ser if svc.Kind != protocol.KindCompose { continue } - if c.builtServices(ctx, svc.ComposeProject, stacks[svc.ComposeProject])[svc.ComposeService] { + built := c.builtServices(ctx, svc.ComposeProject, stacks[svc.ComposeProject]) + if built == nil { + continue // check failed; already logged + } + isBuilt, known := built[svc.ComposeService] + if !known { + // Fail closed: a service compose does not list cannot be shown to + // be free of build:, so it is not offered for update. + c.warnOnce(svc.ComposeProject+"/"+svc.ComposeService, "compose_service_not_in_config", + "project", svc.ComposeProject, "service", svc.ComposeService) + } + if isBuilt || !known { svc.Built = true byKey[key] = svc } @@ -289,6 +300,10 @@ func (c *Collector) markBuilt(ctx context.Context, byKey map[string]protocol.Ser // builtServices returns the project's built services, from the cache while // the project's files are unchanged. func (c *Collector) builtServices(ctx context.Context, project string, l stackLabels) map[string]bool { + // The key covers the files named in the labels. Changes in files they + // pull in (extends:, include:, the default .env) do not invalidate the + // cache, so Built can be stale until another change or an agent restart. + // The job-time check is always fresh, so jobs stay safe. key := l.workingDir + "|" + l.configFiles + "|" + l.envFile + "|" + fileStamps(l.configFiles+","+l.envFile) c.mu.Lock() defer c.mu.Unlock() @@ -297,18 +312,30 @@ func (c *Collector) builtServices(ctx context.Context, project string, l stackLa } built, err := c.check.BuiltServices(ctx, project, l.workingDir, l.configFiles, l.envFile) c.cache[project] = buildEntry{key: key, built: built, err: err, at: c.now()} - switch { - case err == nil: + if err == nil { delete(c.warned, project) - case !c.warned[project]: - c.warned[project] = true - if c.log != nil { - c.log.Warn("compose_build_check_failed", "project", project, "error", err) - } + } else { + c.warnOnceLocked(project, "compose_build_check_failed", "project", project, "error", err) } return built } +// warnOnce logs msg the first time it is called for key (until the next +// successful check of the project resets it). +func (c *Collector) warnOnce(key, msg string, args ...any) { + c.mu.Lock() + defer c.mu.Unlock() + c.warnOnceLocked(key, msg, args...) +} + +func (c *Collector) warnOnceLocked(key, msg string, args ...any) { + if c.warned[key] || c.log == nil { + return + } + c.warned[key] = true + c.log.Warn(msg, args...) +} + // fileStamps summarises size and mtime of the comma-separated files, so a // changed file changes the string. func fileStamps(list string) string { diff --git a/internal/agent/inventory/collector_test.go b/internal/agent/inventory/collector_test.go index a0ca78e..2feb70d 100644 --- a/internal/agent/inventory/collector_test.go +++ b/internal/agent/inventory/collector_test.go @@ -290,3 +290,22 @@ func TestSnapshot_BuildCheckFailureIsNotFatalAndRetriesLater(t *testing.T) { assert.Equal(t, 2, check.calls) assert.Equal(t, 1, strings.Count(logs.String(), "compose_build_check_failed"), "logged once per project") } + +func TestSnapshot_ServiceAbsentFromConfigFailsClosed(t *testing.T) { + check := &fakeBuildCheck{built: map[string]bool{"db": false}} // "web" is missing + c, _ := buildCollector(t, check, "/s/app/compose.yaml") + var logs bytes.Buffer + c.log = slog.New(slog.NewTextHandler(&logs, nil)) + + for range 2 { + got, err := c.Snapshot(context.Background()) + require.NoError(t, err) + byName := map[string]protocol.Service{} + for _, s := range got { + byName[s.ComposeService] = s + } + assert.True(t, byName["web"].Built, "an unknown service is not offered for update") + assert.False(t, byName["db"].Built) + } + assert.Equal(t, 1, strings.Count(logs.String(), "compose_service_not_in_config"), "logged once per project") +} diff --git a/internal/agent/updater/build.go b/internal/agent/updater/build.go index 81798ce..4433fbc 100644 --- a/internal/agent/updater/build.go +++ b/internal/agent/updater/build.go @@ -49,7 +49,9 @@ func NewBuildChecker(cli SelfInspector, selfID string, run OutputRunner) *BuildC return &BuildChecker{cli: cli, selfID: selfID, run: run} } -// BuiltServices returns the services of project that have a build: section. +// BuiltServices returns every service of project (all profiles) mapped to +// whether it has a build: section. A service that is not a key is not in the +// project's configuration. // The arguments are the labels of one of the project's containers. Files that // are not inside the agent's read-write same-path mounts are refused. func (b *BuildChecker) BuiltServices(ctx context.Context, project, workingDir, configFilesLabel, envFileLabel string) (map[string]bool, error) { @@ -77,7 +79,9 @@ func (b *BuildChecker) BuiltServices(ctx context.Context, project, workingDir, c // already checked. func (b *BuildChecker) builtServices(ctx context.Context, project, workingDir string, files, envs []string) (map[string]bool, error) { args := composeBase(project, workingDir, files, envs) - args = append(args, "config", "--format", "json") + // Without --profile * compose omits services whose profile is not active, + // and a built service behind a profile would read as absent. + args = append(args, "--profile", "*", "config", "--format", "json") raw, err := b.run(ctx, workingDir, args...) if err != nil { return nil, fmt.Errorf("docker compose config: %w", err) diff --git a/internal/agent/updater/build_test.go b/internal/agent/updater/build_test.go index 273e7ee..9c863c7 100644 --- a/internal/agent/updater/build_test.go +++ b/internal/agent/updater/build_test.go @@ -27,7 +27,7 @@ func TestBuildChecker_BuiltServices(t *testing.T) { assert.Equal(t, map[string]bool{"web": true, "db": false, "x": false}, got) require.Len(t, o.cmds, 1) assert.Equal(t, stack, o.cmds[0].dir) - assert.Equal(t, []string{"compose", "-p", "app", "--project-directory", stack, "-f", filepath.Join(stack, "compose.yaml"), "config", "--format", "json"}, o.cmds[0].args) + assert.Equal(t, []string{"compose", "-p", "app", "--project-directory", stack, "-f", filepath.Join(stack, "compose.yaml"), "--profile", "*", "config", "--format", "json"}, o.cmds[0].args) }) t.Run("files outside the roots are refused without running docker", func(t *testing.T) { o := &outRunner{json: plainConfig} diff --git a/internal/agent/updater/compose.go b/internal/agent/updater/compose.go index aebbb4d..1e20aac 100644 --- a/internal/agent/updater/compose.go +++ b/internal/agent/updater/compose.go @@ -138,7 +138,11 @@ func (e *ComposeExecutor) Execute(ctx context.Context, job protocol.JobStart, ou if err != nil { return "", err } - if built[service] { + isBuilt, known := built[service] + if !known { + return "", fmt.Errorf("service %s not found in compose config", service) + } + if isBuilt { return "", fmt.Errorf("service %s is built from source (build:), not updated", service) } diff --git a/internal/agent/updater/compose_test.go b/internal/agent/updater/compose_test.go index b4172c9..ab9b637 100644 --- a/internal/agent/updater/compose_test.go +++ b/internal/agent/updater/compose_test.go @@ -124,7 +124,20 @@ func TestComposeExecutor(t *testing.T) { _, err := setup(fakeCompose{rw, []container.Summary{ct}}, &fakeDist{digest: d}, rec).Execute(context.Background(), job, io.Discard) require.NoError(t, err) require.Len(t, outr.cmds, 1) - assert.Equal(t, ranCmd{stack, []string{"compose", "-p", "slusk", "--project-directory", stack, "-f", filepath.Join(stack, "compose.yaml"), "config", "--format", "json"}}, outr.cmds[0]) + assert.Equal(t, ranCmd{stack, []string{"compose", "-p", "slusk", "--project-directory", stack, "-f", filepath.Join(stack, "compose.yaml"), "--profile", "*", "config", "--format", "json"}}, outr.cmds[0]) + }) + t.Run("service missing from the config is refused", func(t *testing.T) { + t.Cleanup(cleanOverride) + rec := &recorder{} + e := setup(fakeCompose{rw, []container.Summary{ct}}, &fakeDist{digest: d}, rec) + outr.json = `{"services":{"db":{"image":"y"}}}` + dist := &fakeDist{digest: d} + e.resolve = NewResolver(dist, nil) + _, err := e.Execute(context.Background(), job, io.Discard) + assert.EqualError(t, err, "service web not found in compose config") + assert.Empty(t, rec.cmds) + assert.Empty(t, dist.refs, "nothing resolved") + assert.NoFileExists(t, overridePath) }) t.Run("build check failure stops the job", func(t *testing.T) { t.Cleanup(cleanOverride) @@ -140,13 +153,16 @@ func TestComposeExecutor(t *testing.T) { t.Cleanup(cleanOverride) envFile := filepath.Join(stack, ".env.prod") require.NoError(t, os.WriteFile(envFile, []byte("A=1\n"), 0o644)) + t.Cleanup(func() { os.Remove(envFile); os.Remove(filepath.Join(stack, ".env.local")) }) + envFile2 := filepath.Join(stack, ".env.local") + require.NoError(t, os.WriteFile(envFile2, []byte("B=2\n"), 0o644)) withEnv := container.Summary{Labels: map[string]string{ - labelWorkingDir: stack, labelConfigFiles: filepath.Join(stack, "compose.yaml"), labelEnvFile: envFile, + labelWorkingDir: stack, labelConfigFiles: filepath.Join(stack, "compose.yaml"), labelEnvFile: envFile + "," + envFile2, }} rec := &recorder{} _, err := setup(fakeCompose{rw, []container.Summary{withEnv}}, &fakeDist{digest: d}, rec).Execute(context.Background(), job, io.Discard) require.NoError(t, err) - prefix := []string{"compose", "-p", "slusk", "--project-directory", stack, "--env-file", envFile} + prefix := []string{"compose", "-p", "slusk", "--project-directory", stack, "--env-file", envFile, "--env-file", envFile2} require.Len(t, outr.cmds, 1) assert.Equal(t, prefix, outr.cmds[0].args[:len(prefix)]) require.Len(t, rec.cmds, 2)