From a2da6454f7e3464bc074ca38b9274b1293127cdb Mon Sep 17 00:00:00 2001 From: mhoennig Date: Tue, 1 Sep 2026 15:39:31 +0200 Subject: [PATCH] werkdock: ordered mounts with --tmpfs, images verb, :rw accepted Session C groundwork: Werkator's git-metadata mask needs a tmpfs BETWEEN binds (ro-bind .git, tmpfs .git/werkator, bind workspace), so -v and --tmpfs now collect into one ordered mount list and RunSpec carries Mounts instead of Binds. --tmpfs DEST is the docker flag of the same name. `werkdock images` lists loaded image names one per line, so a consumer can check existence through the CLI. -v accepts the explicit :rw docker default instead of refusing it. Co-Authored-By: Claude Fable 5 --- internal/cli/cli.go | 6 ++- internal/cli/images.go | 35 ++++++++++++++ internal/cli/run.go | 86 +++++++++++++++++++++++------------ internal/cli/run_test.go | 46 ++++++++++++++++--- internal/engine/bwrap.go | 33 +++++++++----- internal/engine/bwrap_test.go | 29 +++++++----- internal/engine/engine.go | 27 ++++++++--- internal/store/store.go | 21 +++++++++ internal/store/store_test.go | 19 ++++++++ 9 files changed, 236 insertions(+), 66 deletions(-) create mode 100644 internal/cli/images.go diff --git a/internal/cli/cli.go b/internal/cli/cli.go index 903e898..e668f9d 100644 --- a/internal/cli/cli.go +++ b/internal/cli/cli.go @@ -27,6 +27,8 @@ func Main(args []string) int { return runCmd(args[1:]) case "load": return loadCmd(args[1:]) + case "images": + return imagesCmd(args[1:]) case "doctor": return doctorCmd(args[1:]) case "version", "--version": @@ -49,11 +51,13 @@ Network, uid, /proc, /dev, and /tmp come from the host by contract. Usage: werkdock run [flags] IMAGE COMMAND [ARG...] run a command in a sandbox werkdock load -i ARCHIVE [--name NAME] import a rootfs archive as an image + werkdock images list loaded images, one name per line werkdock doctor [TARGET_DIR] check whether this host can run sandboxes werkdock version print the version Run flags: - -v, --volume SRC:DEST[:ro] bind mount (repeatable, applied in order) + -v, --volume SRC:DEST[:ro] bind mount (repeatable; -v and --tmpfs apply in flag order) + --tmpfs DEST empty tmpfs at DEST (repeatable) -e, --env KEY=VALUE set an environment variable (KEY alone copies it from the host) -w, --workdir DIR working directory inside the sandbox (default /) --rm remove the instance afterwards (currently required) diff --git a/internal/cli/images.go b/internal/cli/images.go new file mode 100644 index 0000000..9d8b944 --- /dev/null +++ b/internal/cli/images.go @@ -0,0 +1,35 @@ +package cli + +import ( + "flag" + "fmt" + "io" + + "werkdock/internal/store" +) + +// imagesCmd prints the loaded image names, one per line — machine-usable +// (Werkator checks image existence through it) and close enough to +// `docker images --format '{{.Repository}}'`. +func imagesCmd(args []string) int { + fs := flag.NewFlagSet("images", flag.ContinueOnError) + fs.SetOutput(io.Discard) + if err := fs.Parse(args); err != nil { + return fail(err) + } + if len(fs.Args()) != 0 { + return fail(fmt.Errorf("unexpected argument %q", fs.Args()[0])) + } + st, err := store.Default() + if err != nil { + return fail(err) + } + names, err := st.List() + if err != nil { + return fail(err) + } + for _, name := range names { + fmt.Println(name) + } + return 0 +} diff --git a/internal/cli/run.go b/internal/cli/run.go index 8168db6..50cb441 100644 --- a/internal/cli/run.go +++ b/internal/cli/run.go @@ -16,7 +16,7 @@ import ( // runOptions is the parsed form of `werkdock run` flags, separated from // execution so the parsing is testable and a later daemon can reuse it. type runOptions struct { - Volumes []engine.Bind + Mounts []engine.Mount Env []engine.EnvVar Workdir string Remove bool @@ -39,7 +39,7 @@ func runCmd(args []string) int { } spec := engine.RunSpec{ RootFS: rootfs, - Binds: hostBinds(opts.Volumes), + Mounts: hostMounts(opts.Mounts), Env: opts.Env, Workdir: opts.Workdir, Command: opts.Command, @@ -52,15 +52,15 @@ func runCmd(args []string) int { return code } -// hostBinds prepends the host mounts the contract prescribes: DNS comes +// hostMounts prepends the host mounts the contract prescribes: DNS comes // from the host, so /etc/resolv.conf is bound read-only when it exists — -// before the user binds, so an explicit bind over /etc wins. -func hostBinds(volumes []engine.Bind) []engine.Bind { - var binds []engine.Bind +// before the user mounts, so an explicit mount over /etc wins. +func hostMounts(mounts []engine.Mount) []engine.Mount { + var all []engine.Mount if fi, err := os.Stat("/etc/resolv.conf"); err == nil && fi.Mode().IsRegular() { - binds = append(binds, engine.Bind{Source: "/etc/resolv.conf", Dest: "/etc/resolv.conf", ReadOnly: true}) + all = append(all, engine.Mount{Mode: engine.MountRoBind, Source: "/etc/resolv.conf", Dest: "/etc/resolv.conf"}) } - return append(binds, volumes...) + return append(all, mounts...) } // parseRun parses the docker-shaped run flags. Docker flags whose @@ -69,10 +69,16 @@ func hostBinds(volumes []engine.Bind) []engine.Bind { func parseRun(args []string, getenv func(string) string) (*runOptions, error) { fs := flag.NewFlagSet("run", flag.ContinueOnError) fs.SetOutput(io.Discard) - var volumes, envs stringList + var envs stringList opts := &runOptions{} - fs.Var(&volumes, "v", "bind mount SRC:DEST[:ro]") - fs.Var(&volumes, "volume", "bind mount SRC:DEST[:ro]") + // -v and --tmpfs collect into ONE ordered list: bwrap layers mounts in + // order, so a tmpfs between two binds (the git-metadata mask) must stay + // between them. + volumes := &mountFlag{mounts: &opts.Mounts} + tmpfs := &mountFlag{mounts: &opts.Mounts, tmpfs: true} + fs.Var(volumes, "v", "bind mount SRC:DEST[:ro]") + fs.Var(volumes, "volume", "bind mount SRC:DEST[:ro]") + fs.Var(tmpfs, "tmpfs", "empty tmpfs at DEST") fs.Var(&envs, "e", "environment variable KEY=VALUE") fs.Var(&envs, "env", "environment variable KEY=VALUE") fs.StringVar(&opts.Workdir, "w", "", "working directory inside the sandbox") @@ -101,13 +107,6 @@ func parseRun(args []string, getenv func(string) string) (*runOptions, error) { } opts.Image = rest[0] opts.Command = rest[1:] - for _, v := range volumes { - bind, err := parseVolume(v) - if err != nil { - return nil, err - } - opts.Volumes = append(opts.Volumes, bind) - } for _, e := range envs { opts.Env = append(opts.Env, parseEnv(e, getenv)) } @@ -117,25 +116,54 @@ func parseRun(args []string, getenv func(string) string) (*runOptions, error) { return opts, nil } -func parseVolume(v string) (engine.Bind, error) { +func parseVolume(v string) (engine.Mount, error) { parts := strings.Split(v, ":") if len(parts) < 2 || len(parts) > 3 { - return engine.Bind{}, fmt.Errorf("invalid volume %q, expected SRC:DEST[:ro]", v) + return engine.Mount{}, fmt.Errorf("invalid volume %q, expected SRC:DEST[:ro]", v) } - bind := engine.Bind{Source: parts[0], Dest: parts[1]} + mount := engine.Mount{Mode: engine.MountBind, Source: parts[0], Dest: parts[1]} if len(parts) == 3 { - if parts[2] != "ro" { - return engine.Bind{}, fmt.Errorf("invalid volume option %q in %q, only 'ro' is supported", parts[2], v) + switch parts[2] { + case "ro": + mount.Mode = engine.MountRoBind + case "rw": + // docker accepts :rw as the explicit default; so do we + default: + return engine.Mount{}, fmt.Errorf("invalid volume option %q in %q, only 'ro' and 'rw' are supported", parts[2], v) } - bind.ReadOnly = true } - if !filepath.IsAbs(bind.Source) { - return engine.Bind{}, fmt.Errorf("volume source must be an absolute path: %s", bind.Source) + if !filepath.IsAbs(mount.Source) { + return engine.Mount{}, fmt.Errorf("volume source must be an absolute path: %s", mount.Source) } - if !filepath.IsAbs(bind.Dest) { - return engine.Bind{}, fmt.Errorf("volume destination must be an absolute path: %s", bind.Dest) + if !filepath.IsAbs(mount.Dest) { + return engine.Mount{}, fmt.Errorf("volume destination must be an absolute path: %s", mount.Dest) } - return bind, nil + return mount, nil +} + +// mountFlag appends -v/--volume and --tmpfs values to one shared, +// ordered mount list. +type mountFlag struct { + mounts *[]engine.Mount + tmpfs bool +} + +func (f *mountFlag) String() string { return "" } + +func (f *mountFlag) Set(v string) error { + if f.tmpfs { + if !filepath.IsAbs(v) { + return fmt.Errorf("tmpfs destination must be an absolute path: %s", v) + } + *f.mounts = append(*f.mounts, engine.Mount{Mode: engine.MountTmpfs, Dest: v}) + return nil + } + mount, err := parseVolume(v) + if err != nil { + return err + } + *f.mounts = append(*f.mounts, mount) + return nil } func parseEnv(e string, getenv func(string) string) engine.EnvVar { diff --git a/internal/cli/run_test.go b/internal/cli/run_test.go index d4364cd..d8d26b6 100644 --- a/internal/cli/run_test.go +++ b/internal/cli/run_test.go @@ -28,12 +28,12 @@ func TestParseRunSupportedFlags(t *testing.T) { if !reflect.DeepEqual(opts.Command, []string{"sh", "-c", "./gradlew build"}) { t.Errorf("command: got %q", opts.Command) } - wantVolumes := []engine.Bind{ - {Source: "/repo", Dest: "/repo"}, - {Source: "/cache", Dest: "/root/.gradle", ReadOnly: true}, + wantMounts := []engine.Mount{ + {Mode: engine.MountBind, Source: "/repo", Dest: "/repo"}, + {Mode: engine.MountRoBind, Source: "/cache", Dest: "/root/.gradle"}, } - if !reflect.DeepEqual(opts.Volumes, wantVolumes) { - t.Errorf("volumes: got %+v", opts.Volumes) + if !reflect.DeepEqual(opts.Mounts, wantMounts) { + t.Errorf("mounts: got %+v", opts.Mounts) } if !reflect.DeepEqual(opts.Env, []engine.EnvVar{{Key: "CI", Value: "true"}}) { t.Errorf("env: got %+v", opts.Env) @@ -96,7 +96,8 @@ func TestParseRunValidation(t *testing.T) { {"no image", []string{"--rm"}, "no image specified"}, {"no command", []string{"--rm", "img"}, "no command specified"}, {"volume without dest", []string{"--rm", "-v", "/only-src", "img", "true"}, "expected SRC:DEST"}, - {"volume with bad option", []string{"--rm", "-v", "/a:/b:rw", "img", "true"}, "only 'ro' is supported"}, + {"volume with bad option", []string{"--rm", "-v", "/a:/b:cached", "img", "true"}, "only 'ro' and 'rw' are supported"}, + {"relative tmpfs dest", []string{"--rm", "--tmpfs", "rel", "img", "true"}, "absolute"}, {"relative volume source", []string{"--rm", "-v", "rel:/b", "img", "true"}, "absolute"}, {"relative volume dest", []string{"--rm", "-v", "/a:rel", "img", "true"}, "absolute"}, {"relative workdir", []string{"--rm", "-w", "rel", "img", "true"}, "absolute"}, @@ -111,6 +112,39 @@ func TestParseRunValidation(t *testing.T) { } } +func TestParseRunKeepsMountFlagOrderAcrossVolumeAndTmpfs(t *testing.T) { + // The git-metadata mask depends on it: ro-bind .git, tmpfs over + // .git/werkator, then the workspace bind — in exactly this order. + opts, err := parseRun([]string{ + "--rm", + "-v", "/r/.git:/r/.git:ro", + "--tmpfs", "/r/.git/werkator", + "-v", "/r/ws:/r/ws", + "img", "true", + }, noEnv) + if err != nil { + t.Fatal(err) + } + want := []engine.Mount{ + {Mode: engine.MountRoBind, Source: "/r/.git", Dest: "/r/.git"}, + {Mode: engine.MountTmpfs, Dest: "/r/.git/werkator"}, + {Mode: engine.MountBind, Source: "/r/ws", Dest: "/r/ws"}, + } + if !reflect.DeepEqual(opts.Mounts, want) { + t.Errorf("mounts: got %+v", opts.Mounts) + } +} + +func TestParseRunAcceptsTheExplicitRwVolumeOption(t *testing.T) { + opts, err := parseRun([]string{"--rm", "-v", "/a:/b:rw", "img", "true"}, noEnv) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(opts.Mounts, []engine.Mount{{Mode: engine.MountBind, Source: "/a", Dest: "/b"}}) { + t.Errorf("mounts: got %+v", opts.Mounts) + } +} + func TestParseRunStopsFlagParsingAtTheImage(t *testing.T) { // Docker semantics: everything after the image belongs to the // command, even if it looks like a flag. diff --git a/internal/engine/bwrap.go b/internal/engine/bwrap.go index 2f278f5..7d47d0e 100644 --- a/internal/engine/bwrap.go +++ b/internal/engine/bwrap.go @@ -65,15 +65,20 @@ func (b *Bwrap) Argv(spec RunSpec) ([]string, error) { "--tmpfs", "/tmp", "--tmpfs", "/root", } - for _, bd := range spec.Binds { - if !filepath.IsAbs(bd.Dest) { - return nil, fmt.Errorf("bind destination must be an absolute path: %s", bd.Dest) + for _, m := range spec.Mounts { + if !filepath.IsAbs(m.Dest) { + return nil, fmt.Errorf("mount destination must be an absolute path: %s", m.Dest) } - flag := "--bind" - if bd.ReadOnly { - flag = "--ro-bind" + switch m.Mode { + case MountBind: + args = append(args, "--bind", m.Source, m.Dest) + case MountRoBind: + args = append(args, "--ro-bind", m.Source, m.Dest) + case MountTmpfs: + args = append(args, "--tmpfs", m.Dest) + default: + return nil, fmt.Errorf("unknown mount mode %d for %s", m.Mode, m.Dest) } - args = append(args, flag, bd.Source, bd.Dest) } args = append(args, "--clearenv", @@ -108,17 +113,23 @@ func EnsureMountpoints(spec RunSpec) error { return err } } - for _, bd := range spec.Binds { - target, err := rootfsPath(spec.RootFS, bd.Dest) + for _, m := range spec.Mounts { + target, err := rootfsPath(spec.RootFS, m.Dest) if err != nil { return err } if _, err := os.Lstat(target); err == nil { continue } - src, err := os.Stat(bd.Source) + if m.Mode == MountTmpfs { + if err := os.MkdirAll(target, 0o755); err != nil { + return err + } + continue + } + src, err := os.Stat(m.Source) if err != nil { - return fmt.Errorf("bind source %s: %w", bd.Source, err) + return fmt.Errorf("bind source %s: %w", m.Source, err) } if src.Mode().IsRegular() { if err := os.MkdirAll(filepath.Dir(target), 0o755); err != nil { diff --git a/internal/engine/bwrap_test.go b/internal/engine/bwrap_test.go index 0a18d91..256230a 100644 --- a/internal/engine/bwrap_test.go +++ b/internal/engine/bwrap_test.go @@ -14,10 +14,12 @@ func TestArgvAssemblesTheHardenedInvocation(t *testing.T) { b := &Bwrap{} spec := RunSpec{ RootFS: "/store/images/buildenv/rootfs", - Binds: []Bind{ - {Source: "/etc/resolv.conf", Dest: "/etc/resolv.conf", ReadOnly: true}, - {Source: "/repo", Dest: "/repo"}, - {Source: "/cache", Dest: "/root/.gradle"}, + Mounts: []Mount{ + {Mode: MountRoBind, Source: "/etc/resolv.conf", Dest: "/etc/resolv.conf"}, + {Mode: MountRoBind, Source: "/repo/.git", Dest: "/repo/.git"}, + {Mode: MountTmpfs, Dest: "/repo/.git/werkator"}, + {Mode: MountBind, Source: "/repo", Dest: "/repo"}, + {Mode: MountBind, Source: "/cache", Dest: "/root/.gradle"}, }, Env: []EnvVar{{Key: "CI", Value: "true"}, {Key: "TERM", Value: "dumb"}}, Workdir: "/repo", @@ -34,6 +36,8 @@ func TestArgvAssemblesTheHardenedInvocation(t *testing.T) { "--ro-bind", "/store/images/buildenv/rootfs", "/", "--proc", "/proc", "--dev", "/dev", "--tmpfs", "/tmp", "--tmpfs", "/root", "--ro-bind", "/etc/resolv.conf", "/etc/resolv.conf", + "--ro-bind", "/repo/.git", "/repo/.git", + "--tmpfs", "/repo/.git/werkator", "--bind", "/repo", "/repo", "--bind", "/cache", "/root/.gradle", "--clearenv", @@ -59,8 +63,8 @@ func TestArgvValidation(t *testing.T) { {"relative rootfs", RunSpec{RootFS: "rootfs", Command: []string{"true"}}, "absolute"}, {"missing command", RunSpec{RootFS: "/r"}, "no command specified"}, { - "relative bind dest", - RunSpec{RootFS: "/r", Binds: []Bind{{Source: "/s", Dest: "work"}}, Command: []string{"true"}}, + "relative mount dest", + RunSpec{RootFS: "/r", Mounts: []Mount{{Mode: MountBind, Source: "/s", Dest: "work"}}, Command: []string{"true"}}, "absolute", }, } @@ -103,17 +107,18 @@ func TestEnsureMountpointsCreatesMissingAndSkipsExisting(t *testing.T) { } spec := RunSpec{ RootFS: rootfs, - Binds: []Bind{ - {Source: "/etc", Dest: "/etc/resolv.conf", ReadOnly: true}, // exists: skipped (source type irrelevant) - {Source: srcDir, Dest: "/repo/workspace"}, // missing dir mountpoint - {Source: srcFile, Dest: "/etc/hosts.werkdock"}, // missing file mountpoint + Mounts: []Mount{ + {Mode: MountRoBind, Source: "/etc", Dest: "/etc/resolv.conf"}, // exists: skipped (source type irrelevant) + {Mode: MountBind, Source: srcDir, Dest: "/repo/workspace"}, // missing dir mountpoint + {Mode: MountBind, Source: srcFile, Dest: "/etc/hosts.werkdock"}, // missing file mountpoint + {Mode: MountTmpfs, Dest: "/repo/.git/werkator"}, // tmpfs mountpoint, no source }, Command: []string{"true"}, } if err := EnsureMountpoints(spec); err != nil { t.Fatal(err) } - for _, dir := range []string{"proc", "dev", "tmp", "root", "repo/workspace"} { + for _, dir := range []string{"proc", "dev", "tmp", "root", "repo/workspace", "repo/.git/werkator"} { fi, err := os.Stat(filepath.Join(rootfs, dir)) if err != nil || !fi.IsDir() { t.Errorf("expected directory mountpoint %s in the rootfs: %v", dir, err) @@ -132,7 +137,7 @@ func TestEnsureMountpointsCreatesMissingAndSkipsExisting(t *testing.T) { func TestEnsureMountpointsRefusesEscapingDestinations(t *testing.T) { spec := RunSpec{ RootFS: t.TempDir(), - Binds: []Bind{{Source: "/tmp", Dest: "/../outside"}}, + Mounts: []Mount{{Mode: MountBind, Source: "/tmp", Dest: "/../outside"}}, Command: []string{"true"}, } err := EnsureMountpoints(spec) diff --git a/internal/engine/engine.go b/internal/engine/engine.go index 5763509..323f0f6 100644 --- a/internal/engine/engine.go +++ b/internal/engine/engine.go @@ -3,12 +3,25 @@ // logic without duplicating it (RFC 0002). package engine -// Bind is one bind mount, applied in order; later mounts shadow earlier -// ones at their own path, exactly as bwrap layers them. -type Bind struct { - Source string - Dest string - ReadOnly bool +// MountMode distinguishes the mount kinds a RunSpec can carry. +type MountMode int + +const ( + // MountBind is a read-write bind mount. + MountBind MountMode = iota + // MountRoBind is a read-only bind mount. + MountRoBind + // MountTmpfs is an empty tmpfs at Dest; Source is unused. + MountTmpfs +) + +// Mount is one mount, applied in order; later mounts shadow earlier +// ones at their own path, exactly as bwrap layers them — the order of +// -v and --tmpfs flags is therefore significant and preserved. +type Mount struct { + Mode MountMode + Source string + Dest string } // EnvVar is one environment variable; order is preserved. @@ -23,7 +36,7 @@ type RunSpec struct { // RootFS is the absolute path to the unpacked image rootfs, // bound read-only at /. RootFS string - Binds []Bind + Mounts []Mount Env []EnvVar Workdir string Command []string diff --git a/internal/store/store.go b/internal/store/store.go index 871afa2..82d5ba7 100644 --- a/internal/store/store.go +++ b/internal/store/store.go @@ -11,6 +11,7 @@ import ( "os/exec" "path/filepath" "regexp" + "sort" "strings" "time" ) @@ -57,6 +58,26 @@ func (s Store) RootFS(name string) (string, error) { return rootfs, nil } +// List returns the names of all loaded images, sorted; half-written +// `.tmp` directories from an interrupted load are not images. +func (s Store) List() ([]string, error) { + entries, err := os.ReadDir(filepath.Join(s.Root, "images")) + if os.IsNotExist(err) { + return nil, nil + } + if err != nil { + return nil, err + } + var names []string + for _, e := range entries { + if e.IsDir() && nameRe.MatchString(e.Name()) { + names = append(names, e.Name()) + } + } + sort.Strings(names) + return names, nil +} + // Load imports a rootfs archive as an image. The archive is unpacked // with the tar CLI (compression auto-detected; .tar.zst needs the zstd // binary, which doctor checks) into a temporary directory and renamed diff --git a/internal/store/store_test.go b/internal/store/store_test.go index 4ff871e..759d493 100644 --- a/internal/store/store_test.go +++ b/internal/store/store_test.go @@ -115,6 +115,25 @@ func TestRootFSValidation(t *testing.T) { } } +func TestListNamesLoadedImagesAndIgnoresTmpLeftovers(t *testing.T) { + st := Store{Root: t.TempDir()} + if names, err := st.List(); err != nil || names != nil { + t.Fatalf("empty store: got %v, %v", names, err) + } + for _, dir := range []string{"beta", "alpha", "broken.tmp"} { + if err := os.MkdirAll(filepath.Join(st.Root, "images", dir), 0o755); err != nil { + t.Fatal(err) + } + } + names, err := st.List() + if err != nil { + t.Fatal(err) + } + if len(names) != 2 || names[0] != "alpha" || names[1] != "beta" { + t.Errorf("got %v, want [alpha beta]", names) + } +} + func TestImageNameFromArchive(t *testing.T) { tests := []struct{ in, want string }{ {"werkator-buildenv-trixie.tar.zst", "werkator-buildenv-trixie"},