fix(agent): give the override file the owner of the stack (#24)
This commit is contained in:
@@ -98,6 +98,9 @@ DATABASE_URL=postgres://... go test -tags integration ./internal/master/store/
|
|||||||
without the other has caused a fix commit already.
|
without the other has caused a fix commit already.
|
||||||
- **`<summary>` C# convention from global CLAUDE.md does not apply here** — this
|
- **`<summary>` C# convention from global CLAUDE.md does not apply here** — this
|
||||||
is Go. Use idiomatic GoDoc (`// FuncName does X.`).
|
is Go. Use idiomatic GoDoc (`// FuncName does X.`).
|
||||||
|
- **The override file gets the owner of the existing override, else of the stack's first
|
||||||
|
compose file.** The agent runs as root and renames a new file into place; it chowns it
|
||||||
|
afterwards. A failure is a `warning:` line in the job output, never a failed job.
|
||||||
- **Compose loads only one override file.** Of `compose.override.yml`,
|
- **Compose loads only one override file.** Of `compose.override.yml`,
|
||||||
`compose.override.yaml`, `docker-compose.override.yml`, `docker-compose.override.yaml`
|
`compose.override.yaml`, `docker-compose.override.yml`, `docker-compose.override.yaml`
|
||||||
only the first that exists is read. Hence the refusal described above.
|
only the first that exists is read. Hence the refusal described above.
|
||||||
|
|||||||
@@ -2,11 +2,15 @@ package updater
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
|
"io/fs"
|
||||||
"os"
|
"os"
|
||||||
"os/exec"
|
"os/exec"
|
||||||
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
"syscall"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
"github.com/docker/docker/api/types/container"
|
"github.com/docker/docker/api/types/container"
|
||||||
@@ -27,6 +31,26 @@ const (
|
|||||||
// job's error message; the full output is in the job log.
|
// job's error message; the full output is in the job log.
|
||||||
const errorOutputTail = 2 << 10
|
const errorOutputTail = 2 << 10
|
||||||
|
|
||||||
|
// chown is os.Chown; tests replace it.
|
||||||
|
var chown = os.Chown
|
||||||
|
|
||||||
|
// stackOwner returns the uid and gid the override file should have: those of
|
||||||
|
// the existing override if there is one, else those of the stack's compose file.
|
||||||
|
func stackOwner(overridePath, composeFile string) (uid, gid int, err error) {
|
||||||
|
fi, err := os.Stat(overridePath)
|
||||||
|
if errors.Is(err, fs.ErrNotExist) {
|
||||||
|
fi, err = os.Stat(composeFile)
|
||||||
|
}
|
||||||
|
if err != nil {
|
||||||
|
return 0, 0, err
|
||||||
|
}
|
||||||
|
st, ok := fi.Sys().(*syscall.Stat_t)
|
||||||
|
if !ok {
|
||||||
|
return 0, 0, errors.New("file owner is not available on this platform")
|
||||||
|
}
|
||||||
|
return int(st.Uid), int(st.Gid), nil
|
||||||
|
}
|
||||||
|
|
||||||
// tailWriter adapts a tailBuffer to io.Writer.
|
// tailWriter adapts a tailBuffer to io.Writer.
|
||||||
type tailWriter struct{ b *tailBuffer }
|
type tailWriter struct{ b *tailBuffer }
|
||||||
|
|
||||||
@@ -152,9 +176,19 @@ func (e *ComposeExecutor) Execute(ctx context.Context, job protocol.JobStart, ou
|
|||||||
}
|
}
|
||||||
fmt.Fprintln(out, resolveNote(job.Ref, pinned))
|
fmt.Fprintln(out, resolveNote(job.Ref, pinned))
|
||||||
|
|
||||||
|
// The agent runs as root and WriteOverride renames a new file into place,
|
||||||
|
// so the override would end up root-owned. Hand it back to the stack's
|
||||||
|
// owner. A failure here is only a warning: the update itself is fine.
|
||||||
|
overridePath := filepath.Join(dir, OverrideName)
|
||||||
|
uid, gid, ownerErr := stackOwner(overridePath, files[0])
|
||||||
if _, err := WriteOverride(dir, service, pinned); err != nil {
|
if _, err := WriteOverride(dir, service, pinned); err != nil {
|
||||||
return "", err
|
return "", err
|
||||||
}
|
}
|
||||||
|
if ownerErr != nil {
|
||||||
|
fmt.Fprintf(out, "warning: could not read the owner of %s or %s to give it to %s: %v\n", overridePath, files[0], overridePath, ownerErr)
|
||||||
|
} else if err := chown(overridePath, uid, gid); err != nil {
|
||||||
|
fmt.Fprintf(out, "warning: could not give %s the owner %d:%d: %v\n", overridePath, uid, gid, err)
|
||||||
|
}
|
||||||
|
|
||||||
base := composeBase(project, workingDir, files, envs)
|
base := composeBase(project, workingDir, files, envs)
|
||||||
for _, step := range [][]string{{"pull", service}, {"up", "-d", service}} {
|
for _, step := range [][]string{{"pull", service}, {"up", "-d", service}} {
|
||||||
|
|||||||
@@ -7,6 +7,7 @@ import (
|
|||||||
"os"
|
"os"
|
||||||
"path/filepath"
|
"path/filepath"
|
||||||
"strings"
|
"strings"
|
||||||
|
"syscall"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/docker/docker/api/types/container"
|
"github.com/docker/docker/api/types/container"
|
||||||
@@ -64,6 +65,48 @@ func (r *recorder) run(_ context.Context, dir string, out io.Writer, args ...str
|
|||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// stubChown replaces chown for the test. Tests that use it must not run in parallel.
|
||||||
|
func stubChown(t *testing.T, f func(path string, uid, gid int) error) {
|
||||||
|
t.Helper()
|
||||||
|
orig := chown
|
||||||
|
chown = f
|
||||||
|
t.Cleanup(func() { chown = orig })
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestStackOwner(t *testing.T) {
|
||||||
|
dir := t.TempDir()
|
||||||
|
override := filepath.Join(dir, OverrideName)
|
||||||
|
compose := filepath.Join(dir, "compose.yaml")
|
||||||
|
|
||||||
|
t.Run("falls back to the compose file when there is no override", func(t *testing.T) {
|
||||||
|
require.NoError(t, os.WriteFile(compose, nil, 0o644))
|
||||||
|
uid, gid, err := stackOwner(override, compose)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, os.Getuid(), uid)
|
||||||
|
assert.Equal(t, os.Getgid(), gid)
|
||||||
|
})
|
||||||
|
t.Run("prefers the existing override", func(t *testing.T) {
|
||||||
|
require.NoError(t, os.WriteFile(override, nil, 0o644))
|
||||||
|
require.NoError(t, os.Remove(compose))
|
||||||
|
uid, _, err := stackOwner(override, compose)
|
||||||
|
require.NoError(t, err, "the compose file is not consulted")
|
||||||
|
assert.Equal(t, os.Getuid(), uid)
|
||||||
|
})
|
||||||
|
t.Run("fails when neither file exists", func(t *testing.T) {
|
||||||
|
require.NoError(t, os.Remove(override))
|
||||||
|
_, _, err := stackOwner(override, compose)
|
||||||
|
assert.Error(t, err)
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestChown_RealToOwnIDs(t *testing.T) {
|
||||||
|
f := filepath.Join(t.TempDir(), "f")
|
||||||
|
require.NoError(t, os.WriteFile(f, nil, 0o644))
|
||||||
|
uid, gid, err := stackOwner(f, f)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.NoError(t, chown(f, uid, gid))
|
||||||
|
}
|
||||||
|
|
||||||
func TestComposeExecutor(t *testing.T) {
|
func TestComposeExecutor(t *testing.T) {
|
||||||
const d = "sha256:d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1"
|
const d = "sha256:d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1d1"
|
||||||
root, _ := filepath.EvalSymlinks(t.TempDir())
|
root, _ := filepath.EvalSymlinks(t.TempDir())
|
||||||
@@ -108,6 +151,43 @@ func TestComposeExecutor(t *testing.T) {
|
|||||||
assert.Equal(t, pinned, serviceImage(doc, "web"))
|
assert.Equal(t, pinned, serviceImage(doc, "web"))
|
||||||
assert.Contains(t, out.String(), "resolved "+pinned)
|
assert.Contains(t, out.String(), "resolved "+pinned)
|
||||||
})
|
})
|
||||||
|
t.Run("the override is chowned to the owner of the compose file", func(t *testing.T) {
|
||||||
|
t.Cleanup(cleanOverride)
|
||||||
|
composeFile := filepath.Join(stack, "compose.yaml")
|
||||||
|
require.NoError(t, os.WriteFile(composeFile, []byte("services: {}\n"), 0o644))
|
||||||
|
t.Cleanup(func() { os.Remove(composeFile) })
|
||||||
|
fi, err := os.Stat(composeFile)
|
||||||
|
require.NoError(t, err)
|
||||||
|
st := fi.Sys().(*syscall.Stat_t)
|
||||||
|
var gotPath string
|
||||||
|
var gotUID, gotGID int
|
||||||
|
stubChown(t, func(path string, uid, gid int) error {
|
||||||
|
gotPath, gotUID, gotGID = path, uid, gid
|
||||||
|
return nil
|
||||||
|
})
|
||||||
|
var out strings.Builder
|
||||||
|
_, err = setup(fakeCompose{rw, []container.Summary{ct}}, &fakeDist{digest: d}, &recorder{}).Execute(context.Background(), job, &out)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, overridePath, gotPath)
|
||||||
|
assert.Equal(t, int(st.Uid), gotUID)
|
||||||
|
assert.Equal(t, int(st.Gid), gotGID)
|
||||||
|
assert.NotContains(t, out.String(), "warning")
|
||||||
|
})
|
||||||
|
t.Run("a chown failure is a warning and the job still succeeds", func(t *testing.T) {
|
||||||
|
t.Cleanup(cleanOverride)
|
||||||
|
composeFile := filepath.Join(stack, "compose.yaml")
|
||||||
|
require.NoError(t, os.WriteFile(composeFile, []byte("services: {}\n"), 0o644))
|
||||||
|
t.Cleanup(func() { os.Remove(composeFile) })
|
||||||
|
stubChown(t, func(string, int, int) error { return errors.New("operation not permitted") })
|
||||||
|
var out strings.Builder
|
||||||
|
got, err := setup(fakeCompose{rw, []container.Summary{ct}}, &fakeDist{digest: d}, &recorder{}).Execute(context.Background(), job, &out)
|
||||||
|
require.NoError(t, err)
|
||||||
|
assert.Equal(t, pinned, got)
|
||||||
|
assert.Contains(t, out.String(), "warning: could not give "+overridePath+" the owner ")
|
||||||
|
assert.Contains(t, out.String(), "operation not permitted")
|
||||||
|
_, doc := readOverride(t, stack)
|
||||||
|
assert.Equal(t, pinned, serviceImage(doc, "web"))
|
||||||
|
})
|
||||||
t.Run("a pinned ref is written verbatim and not resolved", func(t *testing.T) {
|
t.Run("a pinned ref is written verbatim and not resolved", func(t *testing.T) {
|
||||||
t.Cleanup(cleanOverride)
|
t.Cleanup(cleanOverride)
|
||||||
for _, ref := range []string{"gitea.shcizo.se/shcizo/slusk:1.3@" + d, "gitea.shcizo.se/shcizo/slusk@" + d} {
|
for _, ref := range []string{"gitea.shcizo.se/shcizo/slusk:1.3@" + d, "gitea.shcizo.se/shcizo/slusk@" + d} {
|
||||||
|
|||||||
Reference in New Issue
Block a user