diff --git a/cmd/agent/main.go b/cmd/agent/main.go index 8d4ee12..c4b5b1d 100644 --- a/cmd/agent/main.go +++ b/cmd/agent/main.go @@ -49,12 +49,13 @@ func run() error { } resolver := updater.NewResolver(dockerCli, auth) exec := updater.NewDispatch( - updater.NewComposeExecutor(dockerCli, resolver, cfg.SelfID, updater.ExecDocker), + updater.NewComposeExecutor(dockerCli, resolver, cfg.SelfID, updater.ExecDocker, updater.ExecDockerOutput), updater.NewSwarmExecutor(dockerCli, resolver, auth), ) runner := updater.NewRunner(exec, cfg.UpdateTimeout) - collector := inventory.New(dockerCli, cfg.SelfID) + collector := inventory.New(dockerCli, cfg.SelfID). + WithBuildCheck(updater.NewBuildChecker(dockerCli, cfg.SelfID, updater.ExecDockerOutput), logger) agent := conn.New(conn.Config{ URL: cfg.MasterURL, Token: cfg.Token, Fingerprint: cfg.MasterKey, Version: version, }, collector, runner, logger) diff --git a/internal/agent/inventory/collector.go b/internal/agent/inventory/collector.go index fbe36bf..120c411 100644 --- a/internal/agent/inventory/collector.go +++ b/internal/agent/inventory/collector.go @@ -5,8 +5,12 @@ package inventory import ( "context" "fmt" + "log/slog" + "os" "sort" "strings" + "sync" + "time" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/image" @@ -22,6 +26,7 @@ const ( labelService = "com.docker.compose.service" labelWorkingDir = "com.docker.compose.project.working_dir" labelConfigFiles = "com.docker.compose.project.config_files" + labelEnvFile = "com.docker.compose.project.environment_file" labelSwarmTask = "com.docker.swarm.service.id" ) @@ -35,16 +40,56 @@ type DockerClient interface { Info(ctx context.Context) (system.Info, error) } +// BuildCheck finds the services of a Compose project that are built from +// source. The arguments are labels of one of the project's containers. +// *updater.BuildChecker satisfies it. +type BuildCheck interface { + BuiltServices(ctx context.Context, project, workingDir, configFilesLabel, envFileLabel string) (map[string]bool, error) +} + +// buildRetry is how long a failed build check is not repeated while the +// project's files stay the same. +const buildRetry = 5 * time.Minute + // Collector reads the local daemon's inventory. type Collector struct { cli DockerClient selfID string + + check BuildCheck + log *slog.Logger + now func() time.Time + + mu sync.Mutex + cache map[string]buildEntry // by Compose project + warned map[string]bool // projects whose check failure was logged } +// buildEntry is the cached outcome of a build check. It is valid as long as +// key (the project's labels plus its files' sizes and mtimes) is unchanged. +type buildEntry struct { + key string + built map[string]bool + err error + at time.Time +} + +// stackLabels are the Compose labels a build check needs. +type stackLabels struct{ workingDir, configFiles, envFile string } + // New returns a Collector. selfID is a prefix of the agent's own container // ID (inside a container, the hostname); that container is marked Self. func New(cli DockerClient, selfID string) *Collector { - return &Collector{cli: cli, selfID: selfID} + return &Collector{cli: cli, selfID: selfID, now: time.Now, cache: map[string]buildEntry{}, warned: map[string]bool{}} +} + +// WithBuildCheck makes the collector mark Compose services that have a +// build: section as Built. The check re-runs only when a project's files +// change. When it fails, the project's services are reported as not built and +// the failure is logged once. It returns c. +func (c *Collector) WithBuildCheck(check BuildCheck, log *slog.Logger) *Collector { + c.check, c.log = check, log + return c } // Hello returns the machine facts sent on connect. AgentVersion is left for @@ -81,6 +126,7 @@ func (c *Collector) Snapshot(ctx context.Context) ([]protocol.Service, error) { } byKey := make(map[string]protocol.Service) + stacks := make(map[string]stackLabels) for _, ct := range containers { if _, isTask := ct.Labels[labelSwarmTask]; isTask { continue @@ -90,12 +136,19 @@ func (c *Collector) Snapshot(ctx context.Context) ([]protocol.Service, error) { return nil, err } svc := c.fromContainer(ct, ref, repoDigests[ct.ImageID]) + if svc.Kind == protocol.KindCompose { + if _, ok := stacks[svc.ComposeProject]; !ok { + stacks[svc.ComposeProject] = stackLabels{svc.WorkingDir, svc.ConfigFiles, ct.Labels[labelEnvFile]} + } + } if prev, ok := byKey[svc.Key()]; ok { svc = mergeReplicas(prev, svc) } byKey[svc.Key()] = svc } + c.markBuilt(ctx, byKey, stacks) + if info.Swarm.ControlAvailable { services, err := c.cli.ServiceList(ctx, swarm.ServiceListOptions{Status: true}) if err != nil { @@ -216,3 +269,59 @@ func swarmStatus(st *swarm.ServiceStatus) string { return "degraded" } } + +// markBuilt sets Built on Compose services that have a build: section. +func (c *Collector) markBuilt(ctx context.Context, byKey map[string]protocol.Service, stacks map[string]stackLabels) { + if c.check == nil { + return + } + for key, svc := range byKey { + if svc.Kind != protocol.KindCompose { + continue + } + if c.builtServices(ctx, svc.ComposeProject, stacks[svc.ComposeProject])[svc.ComposeService] { + svc.Built = true + byKey[key] = svc + } + } +} + +// 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 { + key := l.workingDir + "|" + l.configFiles + "|" + l.envFile + "|" + fileStamps(l.configFiles+","+l.envFile) + c.mu.Lock() + defer c.mu.Unlock() + if e, ok := c.cache[project]; ok && e.key == key && (e.err == nil || c.now().Sub(e.at) < buildRetry) { + return e.built + } + 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: + 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) + } + } + return built +} + +// fileStamps summarises size and mtime of the comma-separated files, so a +// changed file changes the string. +func fileStamps(list string) string { + var b strings.Builder + for _, f := range strings.Split(list, ",") { + if f = strings.TrimSpace(f); f == "" { + continue + } + if fi, err := os.Stat(f); err == nil { + fmt.Fprintf(&b, "%s:%d:%d;", f, fi.Size(), fi.ModTime().UnixNano()) + } else { + fmt.Fprintf(&b, "%s:missing;", f) + } + } + return b.String() +} diff --git a/internal/agent/inventory/collector_test.go b/internal/agent/inventory/collector_test.go index ff464ff..a0ca78e 100644 --- a/internal/agent/inventory/collector_test.go +++ b/internal/agent/inventory/collector_test.go @@ -1,9 +1,16 @@ package inventory import ( + "bytes" "context" "errors" + "io" + "log/slog" + "os" + "path/filepath" + "strings" "testing" + "time" "github.com/docker/docker/api/types/container" "github.com/docker/docker/api/types/image" @@ -209,3 +216,77 @@ func TestSnapshot_ComposeConfigFilesIsRawLabel(t *testing.T) { require.Len(t, got, 1) assert.Equal(t, "/s/a/compose.yaml,/s/a/compose.override.yaml", got[0].ConfigFiles) } + +type fakeBuildCheck struct { + built map[string]bool + err error + calls int +} + +func (f *fakeBuildCheck) BuiltServices(context.Context, string, string, string, string) (map[string]bool, error) { + f.calls++ + return f.built, f.err +} + +func buildCollector(t *testing.T, check BuildCheck, files string) (*Collector, *fakeDocker) { + t.Helper() + ct := composeContainer("abc", "app", "web", "app-web", "sha256:img1", "running") + ct.Labels[labelConfigFiles] = files + other := composeContainer("def", "app", "db", "pg:16", "sha256:img2", "running") + other.Labels[labelConfigFiles] = files + cli := &fakeDocker{containers: []container.Summary{ct, other}} + return New(cli, "").WithBuildCheck(check, slog.New(slog.NewTextHandler(io.Discard, nil))), cli +} + +func TestSnapshot_BuiltServiceIsMarked(t *testing.T) { + check := &fakeBuildCheck{built: map[string]bool{"web": true, "db": false}} + c, _ := buildCollector(t, check, "/s/app/compose.yaml") + 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) + assert.False(t, byName["db"].Built) +} + +func TestSnapshot_BuildCheckIsCachedUntilFilesChange(t *testing.T) { + file := filepath.Join(t.TempDir(), "compose.yaml") + require.NoError(t, os.WriteFile(file, []byte("services: {}\n"), 0o644)) + check := &fakeBuildCheck{built: map[string]bool{"web": true}} + c, _ := buildCollector(t, check, file) + + for range 3 { + _, err := c.Snapshot(context.Background()) + require.NoError(t, err) + } + assert.Equal(t, 1, check.calls, "one check per project while files are unchanged") + + require.NoError(t, os.WriteFile(file, []byte("services:\n web: {}\n"), 0o644)) + _, err := c.Snapshot(context.Background()) + require.NoError(t, err) + assert.Equal(t, 2, check.calls, "a changed file re-runs the check") +} + +func TestSnapshot_BuildCheckFailureIsNotFatalAndRetriesLater(t *testing.T) { + check := &fakeBuildCheck{err: errors.New("no mounts")} + c, _ := buildCollector(t, check, "/s/app/compose.yaml") + now := time.Now() + c.now = func() time.Time { return now } + var logs bytes.Buffer + c.log = slog.New(slog.NewTextHandler(&logs, nil)) + + got, err := c.Snapshot(context.Background()) + require.NoError(t, err) + for _, s := range got { + assert.False(t, s.Built) + } + _, _ = c.Snapshot(context.Background()) + assert.Equal(t, 1, check.calls, "a failure is not retried on every snapshot") + + now = now.Add(buildRetry + time.Second) + _, _ = c.Snapshot(context.Background()) + assert.Equal(t, 2, check.calls) + assert.Equal(t, 1, strings.Count(logs.String(), "compose_build_check_failed"), "logged once per project") +} diff --git a/internal/agent/updater/build.go b/internal/agent/updater/build.go new file mode 100644 index 0000000..81798ce --- /dev/null +++ b/internal/agent/updater/build.go @@ -0,0 +1,113 @@ +package updater + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "os/exec" + "strings" + "time" +) + +// OutputRunner runs "docker " in dir and returns its stdout. A failure +// carries the tail of stderr in the error. +type OutputRunner func(ctx context.Context, dir string, args ...string) ([]byte, error) + +// ExecDockerOutput is the real OutputRunner. It uses the same minimal +// environment as ExecDocker. +func ExecDockerOutput(ctx context.Context, dir string, args ...string) ([]byte, error) { + cmd := exec.CommandContext(ctx, "docker", args...) + cmd.Dir = dir + cmd.Env = dockerEnv() + cmd.WaitDelay = 10 * time.Second + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + if err := cmd.Run(); err != nil { + tail := &tailBuffer{max: errorOutputTail} + tail.Write(stderr.Bytes()) + return nil, fmt.Errorf("%w (output: %s)", err, strings.TrimSpace(tail.String())) + } + return stdout.Bytes(), nil +} + +// BuildChecker finds out which services of a Compose project are built from +// source, by asking `docker compose config`. Such a service has a local +// image that an update would replace with the upstream one, so it is never +// updated. +type BuildChecker struct { + cli SelfInspector + selfID string + run OutputRunner +} + +// NewBuildChecker returns a BuildChecker. cli and selfID name the agent's own +// container, whose mounts decide which files may be read; run is +// ExecDockerOutput outside tests. +func NewBuildChecker(cli SelfInspector, selfID string, run OutputRunner) *BuildChecker { + return &BuildChecker{cli: cli, selfID: selfID, run: run} +} + +// BuiltServices returns the services of project that have a build: section. +// 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) { + roots, err := MountRoots(ctx, b.cli, b.selfID) + if err != nil { + return nil, err + } + files, dir, err := stackFiles(roots, configFilesLabel) + if err != nil { + return nil, err + } + if workingDir == "" { + workingDir = dir + } else if !insideAny(roots, workingDir) { + return nil, notMounted(workingDir) + } + envs, err := envFiles(roots, envFileLabel) + if err != nil { + return nil, err + } + return b.builtServices(ctx, project, workingDir, files, envs) +} + +// builtServices runs `docker compose config --format json` on files that are +// 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") + raw, err := b.run(ctx, workingDir, args...) + if err != nil { + return nil, fmt.Errorf("docker compose config: %w", err) + } + var cfg struct { + Services map[string]struct { + Build json.RawMessage `json:"build"` + } `json:"services"` + } + if err := json.Unmarshal(raw, &cfg); err != nil { + return nil, fmt.Errorf("parse docker compose config: %w", err) + } + built := make(map[string]bool, len(cfg.Services)) + for name, svc := range cfg.Services { + built[name] = len(svc.Build) > 0 && string(svc.Build) != "null" + } + return built, nil +} + +// composeBase returns the leading `docker compose` arguments every call +// shares. --project-directory is explicit because, with -f, compose would +// take it from the first file, which differs for stacks started with the +// flag. +func composeBase(project, workingDir string, files, envs []string) []string { + args := []string{"compose", "-p", project, "--project-directory", workingDir} + for _, e := range envs { + args = append(args, "--env-file", e) + } + for _, f := range files { + args = append(args, "-f", f) + } + return args +} diff --git a/internal/agent/updater/build_test.go b/internal/agent/updater/build_test.go new file mode 100644 index 0000000..273e7ee --- /dev/null +++ b/internal/agent/updater/build_test.go @@ -0,0 +1,44 @@ +package updater + +import ( + "context" + "errors" + "os" + "path/filepath" + "testing" + + "github.com/docker/docker/api/types/container" + "github.com/docker/docker/api/types/mount" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestBuildChecker_BuiltServices(t *testing.T) { + root, _ := filepath.EvalSymlinks(t.TempDir()) + stack := filepath.Join(root, "app") + require.NoError(t, os.MkdirAll(stack, 0o755)) + self := fakeSelf{mounts: []container.MountPoint{{Type: mount.TypeBind, Source: root, Destination: root, RW: true}}} + files := filepath.Join(stack, "compose.yaml") + "," + filepath.Join(stack, OverrideName) + + t.Run("reports only services with build", func(t *testing.T) { + o := &outRunner{json: `{"services":{"web":{"image":"nginx","build":{"context":"."}},"db":{"image":"pg"},"x":{"build":null}}}`} + got, err := NewBuildChecker(self, "me", o.run).BuiltServices(context.Background(), "app", stack, files, "") + require.NoError(t, err) + 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) + }) + t.Run("files outside the roots are refused without running docker", func(t *testing.T) { + o := &outRunner{json: plainConfig} + _, err := NewBuildChecker(self, "me", o.run).BuiltServices(context.Background(), "app", "", "/elsewhere/compose.yaml", "") + assert.ErrorContains(t, err, "not mounted") + assert.Empty(t, o.cmds) + }) + t.Run("runner and parse errors", func(t *testing.T) { + _, err := NewBuildChecker(self, "me", (&outRunner{err: errors.New("boom")}).run).BuiltServices(context.Background(), "app", stack, files, "") + assert.ErrorContains(t, err, "boom") + _, err = NewBuildChecker(self, "me", (&outRunner{json: "not json"}).run).BuiltServices(context.Background(), "app", stack, files, "") + assert.ErrorContains(t, err, "parse") + }) +} diff --git a/internal/agent/updater/compose.go b/internal/agent/updater/compose.go index a6a8ef9..aebbb4d 100644 --- a/internal/agent/updater/compose.go +++ b/internal/agent/updater/compose.go @@ -20,6 +20,7 @@ const ( labelComposeService = "com.docker.compose.service" labelWorkingDir = "com.docker.compose.project.working_dir" labelConfigFiles = "com.docker.compose.project.config_files" + labelEnvFile = "com.docker.compose.project.environment_file" ) // errorOutputTail is how much of a failed command's output goes into the @@ -80,13 +81,14 @@ type ComposeExecutor struct { resolve *Resolver selfID string run CommandRunner + build *BuildChecker } // NewComposeExecutor returns an executor for Compose jobs. selfID names the // agent's own container, whose mounts decide which directories may be -// touched; run is ExecDocker outside tests. -func NewComposeExecutor(cli ComposeDocker, resolve *Resolver, selfID string, run CommandRunner) *ComposeExecutor { - return &ComposeExecutor{cli: cli, resolve: resolve, selfID: selfID, run: run} +// touched; run and runOut are ExecDocker and ExecDockerOutput outside tests. +func NewComposeExecutor(cli ComposeDocker, resolve *Resolver, selfID string, run CommandRunner, runOut OutputRunner) *ComposeExecutor { + return &ComposeExecutor{cli: cli, resolve: resolve, selfID: selfID, run: run, build: NewBuildChecker(cli, selfID, runOut)} } // Execute pins job.Ref to its current digest, writes it to the stack's @@ -110,6 +112,7 @@ func (e *ComposeExecutor) Execute(ctx context.Context, job protocol.JobStart, ou } workingDir := cts[0].Labels[labelWorkingDir] configFiles := cts[0].Labels[labelConfigFiles] + envFileLabel := cts[0].Labels[labelEnvFile] roots, err := MountRoots(ctx, e.cli, e.selfID) if err != nil { @@ -125,6 +128,20 @@ func (e *ComposeExecutor) Execute(ctx context.Context, job protocol.JobStart, ou return "", notMounted(workingDir) } + envs, err := envFiles(roots, envFileLabel) + if err != nil { + return "", err + } + // Fresh check, not the inventory's cache: a build: section may have been + // added since. Nothing is written for such a service. + built, err := e.build.builtServices(ctx, project, workingDir, files[:len(files)-1], envs) + if err != nil { + return "", err + } + if built[service] { + return "", fmt.Errorf("service %s is built from source (build:), not updated", service) + } + pinned, err := e.resolve.Resolve(ctx, job.Ref) if err != nil { return "", err @@ -135,12 +152,7 @@ func (e *ComposeExecutor) Execute(ctx context.Context, job protocol.JobStart, ou return "", err } - // --project-directory is explicit because, with -f, compose would take it - // from the first file, which differs for stacks started with the flag. - base := []string{"compose", "-p", project, "--project-directory", workingDir} - for _, f := range files { - base = append(base, "-f", f) - } + base := composeBase(project, workingDir, files, envs) for _, step := range [][]string{{"pull", service}, {"up", "-d", service}} { args := append(append([]string(nil), base...), step...) fmt.Fprintf(out, "$ docker %s\n", strings.Join(args, " ")) diff --git a/internal/agent/updater/compose_test.go b/internal/agent/updater/compose_test.go index 96d3c6a..b4172c9 100644 --- a/internal/agent/updater/compose_test.go +++ b/internal/agent/updater/compose_test.go @@ -26,6 +26,20 @@ func (f fakeCompose) ContainerList(context.Context, container.ListOptions) ([]co return f.containers, nil } +// outRunner is a fake OutputRunner for `docker compose config`. +type outRunner struct { + json string + err error + cmds []ranCmd +} + +func (o *outRunner) run(_ context.Context, dir string, args ...string) ([]byte, error) { + o.cmds = append(o.cmds, ranCmd{dir, args}) + return []byte(o.json), o.err +} + +const plainConfig = `{"services":{"web":{"image":"x"},"db":{"image":"y"}}}` + type ranCmd struct { dir string args []string @@ -71,8 +85,10 @@ func TestComposeExecutor(t *testing.T) { overridePath := filepath.Join(stack, OverrideName) cleanOverride := func() { os.Remove(overridePath) } + outr := &outRunner{json: plainConfig} setup := func(cli ComposeDocker, dist *fakeDist, rec *recorder) *ComposeExecutor { - return NewComposeExecutor(cli, NewResolver(dist, nil), "me", rec.run) + *outr = outRunner{json: plainConfig} + return NewComposeExecutor(cli, NewResolver(dist, nil), "me", rec.run, outr.run) } t.Run("happy path", func(t *testing.T) { @@ -92,6 +108,80 @@ func TestComposeExecutor(t *testing.T) { assert.Equal(t, pinned, serviceImage(doc, "web")) assert.Contains(t, out.String(), "resolved "+pinned) }) + t.Run("built from source is refused before anything is written", func(t *testing.T) { + t.Cleanup(cleanOverride) + rec := &recorder{} + e := setup(fakeCompose{rw, []container.Summary{ct}}, &fakeDist{digest: d}, rec) + outr.json = `{"services":{"web":{"image":"nginx","build":{"context":"."}}}}` + _, err := e.Execute(context.Background(), job, io.Discard) + assert.EqualError(t, err, "service web is built from source (build:), not updated") + assert.Empty(t, rec.cmds, "no pull/up") + assert.NoFileExists(t, overridePath) + }) + t.Run("build check uses the job's files without the override", func(t *testing.T) { + t.Cleanup(cleanOverride) + rec := &recorder{} + _, 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]) + }) + t.Run("build check failure stops the job", func(t *testing.T) { + t.Cleanup(cleanOverride) + rec := &recorder{} + e := setup(fakeCompose{rw, []container.Summary{ct}}, &fakeDist{digest: d}, rec) + outr.err = errors.New("invalid compose file") + _, err := e.Execute(context.Background(), job, io.Discard) + assert.ErrorContains(t, err, "invalid compose file") + assert.Empty(t, rec.cmds) + assert.NoFileExists(t, overridePath) + }) + t.Run("env files are passed to every compose call", func(t *testing.T) { + t.Cleanup(cleanOverride) + envFile := filepath.Join(stack, ".env.prod") + require.NoError(t, os.WriteFile(envFile, []byte("A=1\n"), 0o644)) + withEnv := container.Summary{Labels: map[string]string{ + labelWorkingDir: stack, labelConfigFiles: filepath.Join(stack, "compose.yaml"), labelEnvFile: envFile, + }} + 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} + require.Len(t, outr.cmds, 1) + assert.Equal(t, prefix, outr.cmds[0].args[:len(prefix)]) + require.Len(t, rec.cmds, 2) + assert.Equal(t, prefix, rec.cmds[0].args[:len(prefix)]) + assert.Equal(t, prefix, rec.cmds[1].args[:len(prefix)]) + }) + t.Run("env file outside the roots is refused", func(t *testing.T) { + t.Cleanup(cleanOverride) + outside := filepath.Join(t.TempDir(), "secrets.env") + require.NoError(t, os.WriteFile(outside, []byte("A=1\n"), 0o644)) + bad := container.Summary{Labels: map[string]string{ + labelWorkingDir: stack, labelConfigFiles: filepath.Join(stack, "compose.yaml"), labelEnvFile: outside, + }} + rec := &recorder{} + _, err := setup(fakeCompose{rw, []container.Summary{bad}}, &fakeDist{digest: d}, rec).Execute(context.Background(), job, io.Discard) + assert.ErrorContains(t, err, "env file") + assert.Empty(t, rec.cmds) + assert.Empty(t, outr.cmds) + assert.NoFileExists(t, overridePath) + }) + t.Run("env file symlinked outside the roots is refused", func(t *testing.T) { + t.Cleanup(cleanOverride) + outside := filepath.Join(t.TempDir(), "secrets.env") + require.NoError(t, os.WriteFile(outside, []byte("A=1\n"), 0o644)) + link := filepath.Join(stack, ".env.link") + require.NoError(t, os.Symlink(outside, link)) + t.Cleanup(func() { os.Remove(link) }) + bad := container.Summary{Labels: map[string]string{ + labelWorkingDir: stack, labelConfigFiles: filepath.Join(stack, "compose.yaml"), labelEnvFile: link, + }} + rec := &recorder{} + _, err := setup(fakeCompose{rw, []container.Summary{bad}}, &fakeDist{digest: d}, rec).Execute(context.Background(), job, io.Discard) + assert.ErrorContains(t, err, "env file") + assert.Empty(t, rec.cmds) + }) t.Run("no matching container", func(t *testing.T) { rec := &recorder{} _, err := setup(fakeCompose{rw, nil}, &fakeDist{digest: d}, rec).Execute(context.Background(), job, io.Discard) @@ -163,3 +253,21 @@ func TestDockerEnv(t *testing.T) { assert.False(t, strings.HasPrefix(e, "DOCKER_CERT_PATH="), "unset variables are not passed") } } + +func TestExecDocker_UsesMinimalEnv(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "docker"), []byte("#!/bin/sh\nenv\n"), 0o755)) + t.Setenv("PATH", dir+":/usr/bin:/bin") + t.Setenv("AGENT_TOKEN", "secret") + t.Setenv("DOCKER_CONFIG", "/docker-config") + + var out strings.Builder + require.NoError(t, ExecDocker(context.Background(), dir, &out, "compose", "ps")) + assert.NotContains(t, out.String(), "AGENT_TOKEN") + assert.Contains(t, out.String(), "DOCKER_CONFIG=/docker-config") + + raw, err := ExecDockerOutput(context.Background(), dir, "compose", "config") + require.NoError(t, err) + assert.NotContains(t, string(raw), "AGENT_TOKEN") + assert.Contains(t, string(raw), "DOCKER_CONFIG=/docker-config") +} diff --git a/internal/agent/updater/override.go b/internal/agent/updater/override.go index 01d9490..e1ad899 100644 --- a/internal/agent/updater/override.go +++ b/internal/agent/updater/override.go @@ -136,6 +136,16 @@ func MountRoots(ctx context.Context, cli SelfInspector, selfID string) ([]string // override last so it wins. Every file, and the real location of its // directory after resolving symlinks, must be inside one of roots. func composeFiles(roots []string, configFilesLabel string) (files []string, dir string, err error) { + files, dir, err = stackFiles(roots, configFilesLabel) + if err != nil { + return nil, "", err + } + return append(files, filepath.Join(dir, OverrideName)), dir, nil +} + +// stackFiles is composeFiles without the override: the operator's own config +// files, checked against roots. dir is the first file's directory. +func stackFiles(roots []string, configFilesLabel string) (files []string, dir string, err error) { if len(roots) == 0 { return nil, "", errNoRoots } @@ -150,16 +160,7 @@ func composeFiles(roots []string, configFilesLabel string) (files []string, dir dir = filepath.Dir(files[0]) override := filepath.Join(dir, OverrideName) - // Only directories count once resolved: this drops a same-path mount of - // a single file such as /var/run/docker.sock. - var realRoots []string - for _, r := range roots { - if rr, err := filepath.EvalSymlinks(r); err == nil { - if fi, err := os.Stat(rr); err == nil && fi.IsDir() { - realRoots = append(realRoots, rr) - } - } - } + realRoots := realDirs(roots) out := files[:0] for _, f := range files { if !filepath.IsAbs(f) || !insideAny(roots, f) { @@ -176,7 +177,48 @@ func composeFiles(roots []string, configFilesLabel string) (files []string, dir out = append(out, f) } } - return append(out, override), dir, nil + return out, dir, nil +} + +// envFiles returns the --env-file paths from the container's +// com.docker.compose.project.environment_file label (empty label: none). +// Like config files, each must be inside one of roots, also after resolving +// symlinks of the file itself. +func envFiles(roots []string, label string) ([]string, error) { + var files []string + realRoots := realDirs(roots) + for _, f := range strings.Split(label, ",") { + if f = strings.TrimSpace(f); f == "" { + continue + } + if !filepath.IsAbs(f) || !insideAny(roots, f) { + return nil, fmt.Errorf("env file %s is not inside a directory mounted into the agent at the same path (read-write)", f) + } + real, err := filepath.EvalSymlinks(f) + if err != nil { + return nil, fmt.Errorf("resolve env file %s: %w", f, err) + } + if !insideAny(realRoots, real) { + return nil, fmt.Errorf("env file %s is not inside a directory mounted into the agent at the same path (read-write)", f) + } + files = append(files, f) + } + return files, nil +} + +// realDirs returns roots with symlinks resolved. Only directories count once +// resolved: this drops a same-path mount of a single file such as +// /var/run/docker.sock. +func realDirs(roots []string) []string { + var out []string + for _, r := range roots { + if rr, err := filepath.EvalSymlinks(r); err == nil { + if fi, err := os.Stat(rr); err == nil && fi.IsDir() { + out = append(out, rr) + } + } + } + return out } func insideAny(roots []string, path string) bool {