internal/sandbox: generalize the API
Change the sandbox API so it is more like os/exec, where
one constructs a Cmd and then calls Output to get the output.
This not only makes the API more familiar, it also generalizes it:
now it is possible to set the working directory and environment
for binaries run in the sandbox.
Change-Id: I12972b5a553d98c002c61ef98b0eb690efeb9257
Reviewed-on: https://go-review.googlesource.com/c/pkgsite-metrics/+/471416
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Maceo Thompson <maceothompson@google.com>
Run-TryBot: Jonathan Amsterdam <jba@google.com>
diff --git a/internal/sandbox/runner.go b/internal/sandbox/runner.go
index d76b88f..77fa922 100644
--- a/internal/sandbox/runner.go
+++ b/internal/sandbox/runner.go
@@ -14,12 +14,12 @@
import (
"bytes"
+ "encoding/json"
"errors"
"io"
"log"
"os"
"os/exec"
- "strings"
)
func main() {
@@ -31,11 +31,11 @@
log.Fatal(err)
}
log.Printf("read %q", in)
- args := strings.Fields(string(in))
- if len(args) == 0 {
- log.Fatal("no args")
+ var cmd exec.Cmd
+ if err := json.Unmarshal(in, &cmd); err != nil {
+ log.Fatal(err)
}
- cmd := exec.Command(args[0], args[1:]...)
+ log.Printf("cmd: %+v", cmd)
out, err := cmd.Output()
if err != nil {
s := err.Error()
@@ -43,7 +43,7 @@
if errors.As(err, &eerr) {
s += ": " + string(bytes.TrimSpace(eerr.Stderr))
}
- log.Fatalf("%s failed with %s", args[0], s)
+ log.Fatalf("%v failed with %s", cmd.Args, s)
}
if _, err := os.Stdout.Write(out); err != nil {
log.Fatal(err)
diff --git a/internal/sandbox/sandbox.go b/internal/sandbox/sandbox.go
index 0625d1d..c2b0855 100644
--- a/internal/sandbox/sandbox.go
+++ b/internal/sandbox/sandbox.go
@@ -7,12 +7,9 @@
import (
"bytes"
- "context"
+ "encoding/json"
"fmt"
- "io"
"os/exec"
- "strings"
- "unicode"
"golang.org/x/pkgsite-metrics/internal/derrors"
)
@@ -37,44 +34,74 @@
}
}
-// Run runs program with args in a sandbox.
-// The program argument is the absolute path to the program from
-// within the sandbox.
-// It is invoked directly, as with [exec.Command]; no shell
-// interpretation is performed.
-// Its working directory is the bundle filesystem root.
-// The program is passed the given arguments, which must not contain whitespace.
-//
-// If the program succeeds (exits with code 0), its standard output is returned.
-// If it fails, the first return value is empty and the error comes from [exec.Command.Output].
-func (s *Sandbox) Run(ctx context.Context, program string, args ...string) (stdout []byte, err error) {
- defer derrors.Wrap(&err, "Run(%s, %q)", program, args)
- for _, a := range args {
- if strings.IndexFunc(a, unicode.IsSpace) >= 0 {
- return nil, fmt.Errorf("arg %q contains whitespace", a)
- }
- }
+// Cmd's exported fields must be a subset of the exported fields of exec.Cmd.
+// runner.go must be able to unmarshal a sandbox.Cmd into an exec.Cmd.
+// Cmd describes how to run a binary in a sandbox.
+type Cmd struct {
+ sb *Sandbox
+
+ // Path is the path of the command to run.
+ //
+ // This is the only field that must be set to a non-zero
+ // value. If Path is relative, it is evaluated relative
+ // to Dir.
+ Path string
+
+ // Args holds command line arguments, including the command as Args[0].
+ // If the Args field is empty or nil, Run uses {Path}.
+ //
+ // In typical use, both Path and Args are set by calling Command.
+ Args []string
+
+ // Env specifies the environment of the process.
+ // Each entry is of the form "key=value".
+ // If Env is nil, the new process uses whatever environment
+ // runsc provides by default.
+ Env []string
+
+ // Dir specifies the working directory of the command.
+ // If Dir is the empty string, Run runs the command in the
+ // root of the sandbox filesystem.
+ Dir string
+}
+
+// Command creates a *Cmd to run path in the sandbox.
+// It behaves like [os/exec.Command].
+func (s *Sandbox) Command(path string, arg ...string) *Cmd {
+ return &Cmd{
+ sb: s,
+ Path: path,
+ Args: append([]string{path}, arg...),
+ }
+}
+
+// Output runs Cmd in the sandbox used to create it, and returns its standard output.
+func (c *Cmd) Output() (_ []byte, err error) {
+ defer derrors.Wrap(&err, "Cmd.Output %q", c.Args)
// -ignore-cgroups is needed to avoid this error from runsc:
// cannot set up cgroup for root: configuring cgroup: write /sys/fs/cgroup/cgroup.subtree_control: device or resource busy
- cmd := exec.CommandContext(ctx, s.Runsc, "-ignore-cgroups", "-network=none", "run", "sandbox")
- cmd.Dir = s.bundleDir
+ cmd := exec.Command(c.sb.Runsc, "-ignore-cgroups", "-network=none", "run", "sandbox")
+ cmd.Dir = c.sb.bundleDir
stdinPipe, err := cmd.StdinPipe()
if err != nil {
return nil, err
}
- stdin := program + " " + strings.Join(args, " ")
- c := make(chan error, 1)
+ stdin, err := json.Marshal(c)
+ if err != nil {
+ return nil, err
+ }
+ ch := make(chan error, 1)
go func() {
- _, err := io.WriteString(stdinPipe, stdin)
+ _, err := stdinPipe.Write(stdin)
stdinPipe.Close()
- c <- err
+ ch <- err
}()
out, err := cmd.Output()
if err != nil {
return nil, err
}
- if err := <-c; err != nil {
+ if err := <-ch; err != nil {
return nil, fmt.Errorf("writing stdin: %w", err)
}
return bytes.TrimSpace(out), nil
diff --git a/internal/sandbox/sandbox_test.go b/internal/sandbox/sandbox_test.go
index 536ca9f..60fa51f 100644
--- a/internal/sandbox/sandbox_test.go
+++ b/internal/sandbox/sandbox_test.go
@@ -5,7 +5,6 @@
package sandbox
import (
- "context"
"errors"
"os"
"os/exec"
@@ -23,12 +22,11 @@
if os.Getenv("RUN_FROM_MAKE") != "1" {
t.Skip("skipping; must run with 'make'.")
}
- ctx := context.Background()
sb := New("testdata/bundle")
sb.Runsc = "/usr/local/bin/runsc" // must match path in Makefile
t.Run("printargs", func(t *testing.T) {
- out, err := sb.Run(ctx, "/printargs", "a", "b")
+ out, err := sb.Command("printargs", "a", "b").Output()
if err != nil {
t.Fatal(derrors.IncludeStderr(err))
}
@@ -43,17 +41,20 @@
})
t.Run("space in arg", func(t *testing.T) {
- _, err := sb.Run(ctx, "foo", "a b c")
- if err == nil {
- t.Fatal("got nil, want error")
+ out, err := sb.Command("printargs", "a", "b c\td").Output()
+ if err != nil {
+ t.Fatal(derrors.IncludeStderr(err))
}
- if g, w := err.Error(), "contains whitespace"; !strings.Contains(g, w) {
- t.Fatalf("got\n%q\nwhich does not contain %q", g, w)
+ want := `args:
+0: "a"
+1: "b c\td"`
+ got := string(out)
+ if got != want {
+ t.Fatalf("got\n%q\nwant\n%q", got, want)
}
})
-
t.Run("no program", func(t *testing.T) {
- _, err := sb.Run(ctx, "foo")
+ _, err := sb.Command("foo").Output()
var eerr *exec.ExitError
if !errors.As(err, &eerr) {
t.Fatalf("got %T, wanted *exec.ExitError", err)
@@ -61,7 +62,7 @@
if g, w := eerr.ExitCode(), 1; g != w {
t.Fatalf("got exit code %d, wanted %d", g, w)
}
- if g, w := string(eerr.Stderr), "executable file not found"; !strings.Contains(g, w) {
+ if g, w := string(eerr.Stderr), "no such file"; !strings.Contains(g, w) {
t.Fatalf("got\n%q\nwhich does not contain %q", g, w)
}
})
diff --git a/internal/worker/vulncheck_scan.go b/internal/worker/vulncheck_scan.go
index c509b04..066446b 100644
--- a/internal/worker/vulncheck_scan.go
+++ b/internal/worker/vulncheck_scan.go
@@ -345,7 +345,7 @@
}
log.Infof(ctx, "go mod download succeeded")
log.Infof(ctx, "%s@%s: running vulncheck in sandbox", modulePath, version)
- stdout, err := sbox.Run(ctx, "/binaries/vulncheck_sandbox", mode, sandboxDir)
+ stdout, err := sbox.Command("/binaries/vulncheck_sandbox", mode, sandboxDir).Output()
if err != nil {
return nil, errors.New(derrors.IncludeStderr(err))
}
@@ -374,7 +374,7 @@
return nil, err
}
log.Infof(ctx, "%s@%s/%s: running vulncheck in sandbox on %s", modulePath, version, binDir, destf.Name())
- stdout, err := s.sbox.Run(ctx, "/binaries/vulncheck_sandbox", ModeBinary, destf.Name())
+ stdout, err := s.sbox.Command("/binaries/vulncheck_sandbox", ModeBinary, destf.Name()).Output()
if err != nil {
return nil, errors.New(derrors.IncludeStderr(err))
}
@@ -762,7 +762,7 @@
out, err = exec.Command("go", "clean", "-cache", "-modcache").CombinedOutput()
} else {
logDiskUsage("before")
- out, err = s.sbox.Run(ctx, "go", "clean", "-cache", "-modcache")
+ out, err = s.sbox.Command("go", "clean", "-cache", "-modcache").Output()
if err == nil {
logDiskUsage("after")
}
