amadan
ship: --check passes a process's variables through the runtime's env, never a file; reports a failed removal (rootfs Plan 1 Task 11, review fix round 1)
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
5 files changed,
+262
−116
ship_check.go+75 −55ship_check_test.go+137 −44ship_rootfs.go+7 −10ship_runtime.go+41 −5ship_runtime_test.go+2 −2
diff --git a/ship_check.go b/ship_check.go| index 5f3e00f75..064e118b4 100644 |
| --- a/ship_check.go |
| +++ b/ship_check.go |
| @@ -8,8 +8,6 @@ import ( |
| "fmt" |
| "io" |
| "maps" |
| - "os" |
| - "path/filepath" |
| "slices" |
| "strconv" |
| "strings" |
| @@ -80,7 +78,6 @@ type rootfsCheck struct { |
| image string |
| volumes []string // data, then run |
| containers []string |
| - envFiles map[string]string |
| } |
| // containerState is the part of a container's inspect State the check |
| @@ -104,13 +101,17 @@ type checkProbeResult struct { |
| // of its report to out as it goes. Every container, volume and image it |
| // made is removed before it returns, on every path; when ctx ends early the |
| // report is failed and says nothing more. |
| -func runRootfsCheck(ctx context.Context, rt containerRuntime, b *builtRootfs, probe string, out io.Writer) checkReport { |
| +func runRootfsCheck(ctx context.Context, rt containerRuntime, b *builtRootfs, probe string, out io.Writer) (r checkReport) { |
| var raw [4]byte |
| rand.Read(raw[:]) // never fails (crypto/rand, Go 1.24 on) |
| - c := &rootfsCheck{rt: rt, b: b, probe: probe, out: out, envFiles: map[string]string{}} |
| + c := &rootfsCheck{rt: rt, b: b, probe: probe, out: out} |
| c.id = hex.EncodeToString(raw[:]) |
| c.prefix = "carlos-check-" + c.id |
| - defer c.cleanup(ctx) |
| + // The cleanup's own lines (a removal that failed) are part of the report. |
| + defer func() { |
| + c.cleanup(ctx) |
| + r = c.report |
| + }() |
| c.check(ctx) |
| switch { |
| @@ -135,7 +136,7 @@ func (c *rootfsCheck) check(ctx context.Context) { |
| c.say(fmt.Sprintf("the %s process runs %s, which needs a JIT, and this release does not declare one. it passes here but stops at start on a box. ship it with --jit", name, prog)) |
| } |
| } |
| - if !c.checkArch(ctx) || !c.makeImageAndVolumes(ctx) || !c.writeEnvFiles() { |
| + if !c.checkArch(ctx) || !c.makeImageAndVolumes(ctx) { |
| return |
| } |
| if _, ok := c.b.procs[release.ProcessRelease]; ok && !c.runRelease(ctx) { |
| @@ -153,7 +154,7 @@ func (c *rootfsCheck) checkArch(ctx context.Context) bool { |
| if c.rt.name == "podman" { |
| format = "{{.Host.Arch}}" |
| } |
| - out, err := c.rt.run(ctx, nil, "info", "--format", format) |
| + out, err := c.rt.run(ctx, nil, nil, "info", "--format", format) |
| if err != nil { |
| return c.runtimeFailed(ctx, "info", err) |
| } |
| @@ -173,7 +174,7 @@ func (c *rootfsCheck) makeImageAndVolumes(ctx context.Context) bool { |
| } |
| for _, v := range []string{c.prefix + "-data", c.prefix + "-run"} { |
| c.volumes = append(c.volumes, v) |
| - _, err := c.rt.run(ctx, nil, "volume", "create", "--driver", "local", |
| + _, err := c.rt.run(ctx, nil, nil, "volume", "create", "--label", "carlos-check="+c.id, "--driver", "local", |
| "--opt", "type=tmpfs", "--opt", "device=tmpfs", |
| "--opt", fmt.Sprintf("o=uid=%d,gid=%d,mode=0700", checkUID, checkUID), v) |
| if err != nil { |
| @@ -183,37 +184,32 @@ func (c *rootfsCheck) makeImageAndVolumes(ctx context.Context) bool { |
| return true |
| } |
| -// writeEnvFiles writes one env file per process the check starts, 0600 in |
| -// the build's scratch: the process's env, then the instance's CARLOS_* |
| -// values, which win over nothing (validation keeps CARLOS_ names out of a |
| -// process's env). There is no CARLOS_CONTROL_SOCKET or CARLOS_PURGE: |
| -// nothing serves them here. An env file holds one variable per line, so a |
| -// value with a newline in it reaches the process cut at the newline. |
| -func (c *rootfsCheck) writeEnvFiles() bool { |
| - for _, name := range slices.Sorted(maps.Keys(c.b.procs)) { |
| - if name == release.ProcessConsole { |
| - continue |
| - } |
| - var buf strings.Builder |
| - env := c.b.procs[name].Env |
| - for _, k := range slices.Sorted(maps.Keys(env)) { |
| - fmt.Fprintf(&buf, "%s=%s\n", k, env[k]) |
| - } |
| - fmt.Fprintf(&buf, "CARLOS_SOCKET=%s\nCARLOS_DB=%s\nCARLOS_HOST=%s\nCARLOS_REGION=local\n", checkSocket, checkDB, checkHost) |
| - path := filepath.Join(c.b.scratch, "check-"+name+".env") |
| - if err := os.WriteFile(path, []byte(buf.String()), 0o600); err != nil { |
| - c.say(err.Error()) |
| - c.report.failed = true |
| - return false |
| - } |
| - c.envFiles[name] = path |
| - } |
| - return true |
| +// processVars is process name's environment in the check: its own env, |
| +// then the instance's CARLOS_* values (validation keeps CARLOS_ names out of |
| +// a process's env). There is no CARLOS_CONTROL_SOCKET or CARLOS_PURGE: |
| +// nothing serves them here. |
| +func (c *rootfsCheck) processVars(name string) map[string]string { |
| + vars := maps.Clone(c.b.procs[name].Env) |
| + if vars == nil { |
| + vars = map[string]string{} |
| + } |
| + vars["CARLOS_SOCKET"] = checkSocket |
| + vars["CARLOS_DB"] = checkDB |
| + vars["CARLOS_HOST"] = checkHost |
| + vars["CARLOS_REGION"] = "local" |
| + return vars |
| } |
| -// runArgs is the run argv of process name in container ctr: a box's launch |
| -// contract (spec §5.2) as a local runtime can give it. |
| -func (c *rootfsCheck) runArgs(ctr, name string) []string { |
| +// runArgs is the run argv of process name in container ctr, a box's launch |
| +// contract (spec §5.2) as a local runtime can give it, and the runtime |
| +// child's environment. Each variable is named on argv as -e KEY and its |
| +// value travels in the child's environment, which holds nothing else of |
| +// this process's (runtimeChildEnv): a value is never written to a file, a |
| +// newline in it stays in it, and a container cannot name a host variable it |
| +// does not declare. The exception is a variable the runtime CLI itself |
| +// reads (PATH, HOME, ...; an image's PATH is merged into its processes' |
| +// env): it goes on argv as -e KEY=VALUE, so the CLI keeps its own. |
| +func (c *rootfsCheck) runArgs(ctr, name string) (args, env []string) { |
| p := c.b.procs[name] |
| wd := p.WorkingDir |
| if wd == "" { |
| @@ -221,7 +217,7 @@ func (c *rootfsCheck) runArgs(ctr, name string) []string { |
| } |
| mem := strconv.FormatInt(checkMemory, 10) |
| user := fmt.Sprintf("%d:%d", checkUID, checkUID) |
| - args := []string{"run", "-d", "--name", ctr, |
| + args = []string{"run", "-d", "--name", ctr, |
| "--read-only", "--tmpfs", "/tmp", |
| "--memory", mem, "--memory-swap", mem, |
| "--cap-drop", "ALL", "--security-opt", "no-new-privileges", |
| @@ -229,13 +225,23 @@ func (c *rootfsCheck) runArgs(ctr, name string) []string { |
| "-v", c.volumes[0] + ":" + checkDataDir, |
| "-v", c.volumes[1] + ":" + checkRunDir, |
| "-v", c.probe + ":" + checkProbeMount + ":ro", |
| - "--env-file", c.envFiles[name], |
| + } |
| + vars := c.processVars(name) |
| + for _, k := range slices.Sorted(maps.Keys(vars)) { |
| + if slices.Contains(runtimeEnvNames, k) { |
| + args = append(args, "-e", k+"="+vars[k]) |
| + delete(vars, k) |
| + } else { |
| + args = append(args, "-e", k) |
| + } |
| + } |
| + args = append(args, |
| "-w", wd, |
| "--entrypoint", p.Command[0], |
| - "--label", "carlos-check=" + c.id, |
| + "--label", "carlos-check="+c.id, |
| c.image, |
| - } |
| - return append(args, p.Command[1:]...) |
| + ) |
| + return append(args, p.Command[1:]...), runtimeChildEnv(vars) |
| } |
| // start runs process name detached and answers its container. A run the |
| @@ -244,7 +250,8 @@ func (c *rootfsCheck) runArgs(ctr, name string) []string { |
| func (c *rootfsCheck) start(ctx context.Context, name string) (string, bool) { |
| ctr := c.prefix + "-" + name |
| c.containers = append(c.containers, ctr) |
| - if _, err := c.rt.run(ctx, nil, c.runArgs(ctr, name)...); err != nil { |
| + args, env := c.runArgs(ctr, name) |
| + if _, err := c.rt.run(ctx, nil, env, args...); err != nil { |
| if ctx.Err() == nil && execFormatError(err.Error()) { |
| return ctr, c.fail(checkNoEmulation) |
| } |
| @@ -266,7 +273,7 @@ func (c *rootfsCheck) runRelease(ctx context.Context) bool { |
| timeout = time.Duration(s) * time.Second |
| } |
| wctx, cancel := context.WithTimeout(ctx, timeout) |
| - out, err := c.rt.run(wctx, nil, "wait", ctr) |
| + out, err := c.rt.run(wctx, nil, nil, "wait", ctr) |
| timedOut := wctx.Err() != nil |
| cancel() |
| if err != nil { |
| @@ -301,7 +308,7 @@ func (c *rootfsCheck) runWeb(ctx context.Context) bool { |
| // The probe ends itself at its timeout; the margin bounds a runtime |
| // that never returns. |
| pctx, cancel := context.WithTimeout(ctx, release.RootfsHealthTimeout+30*time.Second) |
| - out, perr := c.rt.run(pctx, nil, "exec", ctr, checkProbeMount, "__rootfs-check-probe", |
| + out, perr := c.rt.run(pctx, nil, nil, "exec", ctr, checkProbeMount, "__rootfs-check-probe", |
| "--socket", checkSocket, "--timeout", fmt.Sprintf("%ds", int(release.RootfsHealthTimeout/time.Second))) |
| cancel() |
| if ctx.Err() != nil { |
| @@ -401,7 +408,7 @@ func (c *rootfsCheck) exited(name string, st containerState, logs []string) bool |
| func (c *rootfsCheck) inspect(ctx context.Context, ctr string) (containerState, error) { |
| var st containerState |
| - out, err := c.rt.run(ctx, nil, "inspect", "--format", "{{json .State}}", ctr) |
| + out, err := c.rt.run(ctx, nil, nil, "inspect", "--format", "{{json .State}}", ctr) |
| if err != nil { |
| return st, err |
| } |
| @@ -411,7 +418,7 @@ func (c *rootfsCheck) inspect(ctx context.Context, ctr string) (containerState, |
| // logs is the container's last checkLogLines lines, both streams (the |
| // runner joins them for logs), or none when the runtime cannot give them. |
| func (c *rootfsCheck) logs(ctx context.Context, ctr string) []string { |
| - out, err := c.rt.run(ctx, nil, "logs", "--tail", strconv.Itoa(checkLogLines), ctr) |
| + out, err := c.rt.run(ctx, nil, nil, "logs", "--tail", strconv.Itoa(checkLogLines), ctr) |
| if err != nil { |
| return nil |
| } |
| @@ -430,25 +437,38 @@ func (c *rootfsCheck) logs(ctx context.Context, ctr string) []string { |
| // cleanup removes, on every path, everything the check asked the runtime to |
| // make: containers, then volumes, then the image. It runs under its own |
| -// bound, not ctx, so a Ctrl-C still cleans up. A removal of something the |
| -// runtime never made fails harmlessly and is not reported. |
| +// bound, not ctx, so a Ctrl-C still cleans up. A removal that fails is |
| +// reported in the runtime's words, unless the runtime says the object does |
| +// not exist (a call cut short may never have made it); what is left carries |
| +// the carlos-check label, or the image's carlos-check- name, to find it by. |
| func (c *rootfsCheck) cleanup(ctx context.Context) { |
| ctx, cancel := context.WithTimeout(context.WithoutCancel(ctx), 30*time.Second) |
| defer cancel() |
| + var removals [][]string |
| for _, ctr := range c.containers { |
| - c.rt.run(ctx, nil, "rm", "-f", ctr) |
| + removals = append(removals, []string{"rm", "-f", ctr}) |
| } |
| for _, v := range c.volumes { |
| - c.rt.run(ctx, nil, "volume", "rm", v) |
| + removals = append(removals, []string{"volume", "rm", v}) |
| } |
| if c.image != "" { |
| - c.rt.run(ctx, nil, "rmi", c.image) |
| + removals = append(removals, []string{"rmi", c.image}) |
| } |
| - for _, p := range c.envFiles { |
| - os.Remove(p) |
| + for _, args := range removals { |
| + if _, err := c.rt.run(ctx, nil, nil, args...); err != nil && !notFoundError(err) { |
| + c.say(fmt.Sprintf("%s %s: %v", c.rt.name, strings.Join(args, " "), err)) |
| + } |
| } |
| } |
| +// notFoundError reports whether a runtime's refusal says the object does not |
| +// exist: docker's "No such container", "no such volume", "No such image", |
| +// podman's "no such container", "image not known". |
| +func notFoundError(err error) bool { |
| + msg := strings.ToLower(err.Error()) |
| + return strings.Contains(msg, "no such ") || strings.Contains(msg, "not known") |
| +} |
| + |
| func (c *rootfsCheck) say(line string) { |
| c.report.lines = append(c.report.lines, line) |
| fmt.Fprintln(c.out, line) |
diff --git a/ship_check_test.go b/ship_check_test.go| index f949241fe..cbe084b88 100644 |
| --- a/ship_check_test.go |
| +++ b/ship_check_test.go |
| @@ -5,14 +5,15 @@ import ( |
| "bytes" |
| "context" |
| "errors" |
| + "fmt" |
| "io" |
| "maps" |
| "os" |
| "path" |
| "path/filepath" |
| "regexp" |
| - "runtime" |
| "slices" |
| + "strconv" |
| "strings" |
| "testing" |
| "time" |
| @@ -48,11 +49,12 @@ type fakeCheck struct { |
| failAt, cancelAt int // the index of the call to fail or cancel at; -1 for none |
| cancel context.CancelFunc |
| + removeErr map[string]error // a removal's refusal, by object (summary's name) |
| out *bytes.Buffer |
| - atImport string // what the report had said when import ran |
| - env map[string]string // each process's env file, read at its run |
| - envMode map[string]os.FileMode |
| + atImport string // what the report had said when import ran |
| + env map[string][]string // each process's child environment, at its run |
| + envNil []string // the calls that ran in this process's environment |
| } |
| func newFakeCheck(t *testing.T) *fakeCheck { |
| @@ -61,7 +63,7 @@ func newFakeCheck(t *testing.T) *fakeCheck { |
| arch: "aarch64", waitOut: "0\n", probeOut: `{"status":200,"after_ms":42}` + "\n", |
| runErr: map[string]error{}, state: map[string]string{}, logs: map[string]string{}, |
| failAt: -1, cancelAt: -1, out: &bytes.Buffer{}, |
| - env: map[string]string{}, envMode: map[string]os.FileMode{}, |
| + removeErr: map[string]error{}, env: map[string][]string{}, |
| } |
| } |
| @@ -79,13 +81,16 @@ func flagValue(args []string, flag string) string { |
| return "" |
| } |
| -func (f *fakeCheck) run(ctx context.Context, stdin io.Reader, args ...string) ([]byte, error) { |
| +func (f *fakeCheck) run(ctx context.Context, stdin io.Reader, env []string, args ...string) ([]byte, error) { |
| if stdin != nil { |
| io.Copy(io.Discard, stdin) |
| } |
| i := len(f.calls) |
| f.calls = append(f.calls, slices.Clone(args)) |
| last := args[len(args)-1] |
| + if env == nil { |
| + f.envNil = append(f.envNil, args[0]) |
| + } |
| var removes string |
| switch { |
| @@ -104,6 +109,13 @@ func (f *fakeCheck) run(ctx context.Context, stdin io.Reader, args ...string) ([ |
| if !f.made[removes] { |
| f.strays = append(f.strays, removes) |
| } |
| + obj := procOf(last) |
| + if strings.HasSuffix(last, ":latest") { |
| + obj = "image" |
| + } |
| + if err := f.removeErr[obj]; err != nil { |
| + return nil, err |
| + } |
| delete(f.live, removes) |
| return nil, nil |
| } |
| @@ -120,13 +132,7 @@ func (f *fakeCheck) run(ctx context.Context, stdin io.Reader, args ...string) ([ |
| case args[0] == "run": |
| ctr := flagValue(args, "--name") |
| makes = "container:" + ctr |
| - proc := procOf(ctr) |
| - if b, err := os.ReadFile(flagValue(args, "--env-file")); err == nil { |
| - f.env[proc] = string(b) |
| - } |
| - if fi, err := os.Stat(flagValue(args, "--env-file")); err == nil { |
| - f.envMode[proc] = fi.Mode().Perm() |
| - } |
| + f.env[procOf(ctr)] = env |
| } |
| if makes != "" { |
| if name := makes[strings.IndexByte(makes, ':')+1:]; !checkName.MatchString(name) { |
| @@ -262,12 +268,16 @@ func checkTree(t *testing.T, files map[string]string) *rootfsimage.Tree { |
| return tree |
| } |
| +// shopCert is a multi-line value with a line shaped like a variable, which |
| +// an env file would have read as one. |
| +const shopCert = "-----BEGIN CERTIFICATE-----\nMIIB\nSHOP_MODE=injected\n-----END CERTIFICATE-----" |
| + |
| // shopProcs is a release, a web and one worker, jobs. |
| func shopProcs() map[string]release.Process { |
| return map[string]release.Process{ |
| release.ProcessRelease: {Command: []string{"/srv/migrate"}}, |
| release.ProcessWeb: {Command: []string{"/srv/server", "--port", "x"}, WorkingDir: "/srv", |
| - Env: map[string]string{"SHOP_MODE": "production", "A_FIRST": "1"}}, |
| + Env: map[string]string{"SHOP_MODE": "production", "A_FIRST": "1", "SHOP_CERT": shopCert, "PATH": "/srv/bin:/usr/bin"}}, |
| "jobs": {Command: []string{"/srv/jobs"}}, |
| } |
| } |
| @@ -275,7 +285,7 @@ func shopProcs() map[string]release.Process { |
| func shopBuild(t *testing.T, procs map[string]release.Process) *builtRootfs { |
| t.Helper() |
| tree := checkTree(t, map[string]string{"srv/server": "x", "srv/migrate": "x", "srv/jobs": "x"}) |
| - return &builtRootfs{tree: tree, procs: procs, scratch: t.TempDir()} |
| + return &builtRootfs{tree: tree, procs: procs} |
| } |
| const ( |
| @@ -340,6 +350,7 @@ func TestRootfsCheckWarnsOnAnotherArchitecture(t *testing.T) { |
| func TestRootfsCheckRunArgs(t *testing.T) { |
| noSettle(t) |
| + t.Setenv("SHOP_HOST_SECRET", "not the container's") |
| f := newFakeCheck(t) |
| r := runRootfsCheck(context.Background(), f.rt(), shopBuild(t, shopProcs()), "/probe/carlos", f.out) |
| if r.failed { |
| @@ -397,20 +408,53 @@ func TestRootfsCheckRunArgs(t *testing.T) { |
| t.Errorf("argv ends %q, want the image then the command's arguments", tail) |
| } |
| - wantEnv := "A_FIRST=1\nSHOP_MODE=production\n" + |
| - "CARLOS_SOCKET=/run/carlos/instances/check/app.sock\n" + |
| - "CARLOS_DB=/data/instances/check/shop.example.com.db\n" + |
| - "CARLOS_HOST=shop.example.com\nCARLOS_REGION=local\n" |
| - if got := f.env["web"]; got != wantEnv { |
| - t.Errorf("web's env file:\n%s\nwant:\n%s", got, wantEnv) |
| + // Each variable is named on argv; the values travel in the child's |
| + // environment, never in a file. A runtime's own name (PATH, from the |
| + // image) goes on argv whole, so the CLI keeps its own PATH. |
| + var named []string |
| + for i, a := range web { |
| + if a == "-e" { |
| + named = append(named, web[i+1]) |
| + } |
| + } |
| + wantNamed := []string{"A_FIRST", "CARLOS_DB", "CARLOS_HOST", "CARLOS_REGION", "CARLOS_SOCKET", "PATH=/srv/bin:/usr/bin", "SHOP_CERT", "SHOP_MODE"} |
| + if !slices.Equal(named, wantNamed) { |
| + t.Errorf("-e %q, want %q", named, wantNamed) |
| } |
| - for _, unwanted := range []string{"CARLOS_CONTROL_SOCKET", "CARLOS_PURGE"} { |
| - if strings.Contains(f.env["web"], unwanted) { |
| - t.Errorf("the env file sets %s", unwanted) |
| + if slices.Contains(web, "--env-file") { |
| + t.Error("the run still takes an env file") |
| + } |
| + env := f.env["web"] |
| + for _, kv := range []string{ |
| + "A_FIRST=1", "SHOP_MODE=production", "SHOP_CERT=" + shopCert, |
| + "CARLOS_SOCKET=/run/carlos/instances/check/app.sock", |
| + "CARLOS_DB=/data/instances/check/shop.example.com.db", |
| + "CARLOS_HOST=shop.example.com", "CARLOS_REGION=local", |
| + "PATH=" + os.Getenv("PATH"), |
| + } { |
| + if !slices.Contains(env, kv) { |
| + t.Errorf("the child's environment lacks %q: %q", kv, env) |
| } |
| } |
| - if runtime.GOOS != "windows" && f.envMode["web"] != 0o600 { |
| - t.Errorf("the env file's mode is %v, want 0600", f.envMode["web"]) |
| + for _, kv := range env { |
| + name, _, _ := strings.Cut(kv, "=") |
| + if !slices.Contains(runtimeEnvNames, name) && !slices.Contains(wantNamed, name) { |
| + t.Errorf("the child's environment carries %s, which the process does not declare", name) |
| + } |
| + if name == "PATH" && kv != "PATH="+os.Getenv("PATH") { |
| + t.Errorf("the runtime CLI got the image's %s", kv) |
| + } |
| + } |
| + for _, unwanted := range []string{"SHOP_HOST_SECRET", "CARLOS_CONTROL_SOCKET", "CARLOS_PURGE"} { |
| + for _, kv := range env { |
| + if strings.HasPrefix(kv, unwanted+"=") { |
| + t.Errorf("the child's environment sets %s", unwanted) |
| + } |
| + } |
| + } |
| + // The calls that pass no variables run in this process's environment. |
| + if want := []string{"info", "import", "volume", "volume", "wait", "exec", "inspect", "rm", "rm", "rm", "volume", "volume", "rmi"}; !slices.Equal(f.envNil, want) { |
| + t.Errorf("calls in this process's environment: %q, want %q", f.envNil, want) |
| } |
| // The probe runs inside web, from the mounted binary. |
| @@ -424,7 +468,7 @@ func TestRootfsCheckRunArgs(t *testing.T) { |
| t.Errorf("exec = %q, want %q", exec, want) |
| } |
| // Volumes are tmpfs, owned by the check's user. |
| - if want := []string{"volume", "create", "--driver", "local", "--opt", "type=tmpfs", "--opt", "device=tmpfs", "--opt", "o=uid=65534,gid=65534,mode=0700", prefix + "-data"}; !slices.Equal(f.calls[2], want) { |
| + if want := []string{"volume", "create", "--label", "carlos-check=" + id, "--driver", "local", "--opt", "type=tmpfs", "--opt", "device=tmpfs", "--opt", "o=uid=65534,gid=65534,mode=0700", prefix + "-data"}; !slices.Equal(f.calls[2], want) { |
| t.Errorf("volume create = %q, want %q", f.calls[2], want) |
| } |
| if want := []string{"inspect", "--format", "{{json .State}}", prefix + "-jobs"}; !slices.Equal(f.calls[9], want) { |
| @@ -559,11 +603,7 @@ func TestRootfsCheckNeverStartsConsole(t *testing.T) { |
| } |
| } |
| if _, ok := f.env["console"]; ok { |
| - t.Error("console got an env file") |
| - } |
| - // The env files go with the check. |
| - if ents, _ := os.ReadDir(b.scratch); len(ents) != 0 { |
| - t.Errorf("the scratch still holds %v", ents) |
| + t.Error("console was given an environment") |
| } |
| } |
| @@ -575,7 +615,7 @@ func TestRootfsCheckWarnsAboutAJITWithoutTheFlag(t *testing.T) { |
| procs := map[string]release.Process{ |
| release.ProcessWeb: {Command: []string{"node", "server.js"}, WorkingDir: "/srv"}, |
| } |
| - return &builtRootfs{tree: tree, procs: procs, scratch: t.TempDir(), jit: jit} |
| + return &builtRootfs{tree: tree, procs: procs, jit: jit} |
| } |
| f := newFakeCheck(t) |
| @@ -602,7 +642,6 @@ func TestRootfsCheckWarnsAboutAJITWithoutTheFlag(t *testing.T) { |
| procs: map[string]release.Process{ |
| release.ProcessWeb: {Command: []string{"bin/rails", "server"}, WorkingDir: "/rails"}, |
| }, |
| - scratch: t.TempDir(), |
| } |
| r = runRootfsCheck(context.Background(), f.rt(), rails, "/probe/carlos", f.out) |
| for _, l := range r.lines { |
| @@ -612,6 +651,34 @@ func TestRootfsCheckWarnsAboutAJITWithoutTheFlag(t *testing.T) { |
| } |
| } |
| +// A removal the runtime refuses is reported in its words, after the |
| +// verdict; one it refuses because the object does not exist is not. |
| +func TestRootfsCheckReportsAFailedRemoval(t *testing.T) { |
| + noSettle(t) |
| + f := newFakeCheck(t) |
| + f.removeErr["web"] = errors.New("Error response from daemon: cannot remove container: permission denied") |
| + f.removeErr["jobs"] = errors.New(`Error: no container with name or ID "x" found: no such container`) |
| + f.removeErr["data"] = errors.New("Error response from daemon: get x: no such volume") |
| + f.removeErr["image"] = errors.New("Error: x: image not known") |
| + r := runRootfsCheck(context.Background(), f.rt(), shopBuild(t, shopProcs()), "/probe/carlos", f.out) |
| + var web string |
| + for _, c := range f.calls { |
| + if c[0] == "rm" && procOf(c[len(c)-1]) == "web" { |
| + web = c[len(c)-1] |
| + } |
| + } |
| + want := []string{checkC74, "docker rm -f " + web + ": Error response from daemon: cannot remove container: permission denied"} |
| + if got := r.lines[len(r.lines)-2:]; !slices.Equal(got, want) { |
| + t.Errorf("report ends %q, want %q", got, want) |
| + } |
| + if r.failed { |
| + t.Error("a leftover container failed the image's check") |
| + } |
| + if !strings.HasSuffix(f.out.String(), want[1]+"\n") { |
| + t.Errorf("out = %q, want the removal failure last", f.out) |
| + } |
| +} |
| + |
| // Review Focus 5: a failure, or a stop, at any step removes everything made |
| // before it, and nothing else. |
| func TestRootfsCheckCleansUpOnEveryPath(t *testing.T) { |
| @@ -670,13 +737,14 @@ func TestRootfsCheckCleansUpOnEveryPath(t *testing.T) { |
| }) |
| } |
| -// The production runner keeps a failed command's stdout, and gives logs |
| -// both of the container's streams. |
| +// The production runner keeps a failed command's stdout, gives logs both |
| +// of the container's streams, and gives a child the env it is handed. |
| func TestExecRunnerKeepsStdoutAndJoinsLogs(t *testing.T) { |
| bin := t.TempDir() |
| stubRuntime(t, bin, "docker", `case "$1" in |
| exec) echo '{"status":0,"ports":[3000]}'; echo 'probe gave up' >&2; exit 1 ;; |
| logs) echo out-1; echo err-1 >&2; echo out-2 ;; |
| +run) printf '%s|%s' "$SHOP_CERT" "${SHOP_HOST_SECRET-unset}" ;; |
| esac |
| `) |
| onlyPath(t, bin) |
| @@ -684,14 +752,20 @@ esac |
| if err != nil { |
| t.Fatal(err) |
| } |
| - out, err := rt.run(context.Background(), nil, "exec", "x") |
| + out, err := rt.run(context.Background(), nil, nil, "exec", "x") |
| if err == nil || err.Error() != "probe gave up" || string(out) != `{"status":0,"ports":[3000]}`+"\n" { |
| t.Errorf("exec: %q, %v", out, err) |
| } |
| - out, err = rt.run(context.Background(), nil, "logs", "x") |
| + out, err = rt.run(context.Background(), nil, nil, "logs", "x") |
| if err != nil || string(out) != "out-1\nerr-1\nout-2\n" { |
| t.Errorf("logs: %q, %v", out, err) |
| } |
| + // Given an env, the child has that env and nothing else of ours. |
| + t.Setenv("SHOP_HOST_SECRET", "not the container's") |
| + out, err = rt.run(context.Background(), nil, runtimeChildEnv(map[string]string{"SHOP_CERT": shopCert}), "run", "x") |
| + if want := shopCert + "|unset"; err != nil || string(out) != want { |
| + t.Errorf("run: %q, %v; want %q", out, err, want) |
| + } |
| } |
| // Through the binary: carlos ship --rootfs --check with a stub docker, from |
| @@ -701,17 +775,30 @@ func TestShipRootfsCheckThroughTheBinary(t *testing.T) { |
| t.Skip("runs the carlos binary") |
| } |
| run, bucket := versioningCLI(t) |
| - dir, procs := rootfsRailsDir(t) |
| + dir, _ := rootfsRailsDir(t) |
| bin, work := t.TempDir(), t.TempDir() |
| - stubRuntime(t, bin, "docker", `echo "$@" >> "$FAKE_LOG" |
| + // A run's child has only the runtime's variables and the process's, so |
| + // the stub's paths are in the script, not its environment. At web's run |
| + // it keeps what the container would get and looks for an env file. |
| + logPath := filepath.Join(work, "log") |
| + stubRuntime(t, bin, "docker", fmt.Sprintf(`log='%s' |
| +work='%s' |
| +echo "$@" >> "$log" |
| case "$1" in |
| info) echo aarch64 ;; |
| import) cat > /dev/null ;; |
| +run) printf '%%s' "$SHOP_CERT" > "$work/cert" |
| + echo "host secret: ${SHOP_HOST_SECRET-unset}" >> "$log" |
| + find "$TMPDIR" -name '*.env' >> "$log" ;; |
| exec) echo "$FAKE_PROBE"; exit "$FAKE_PROBE_EXIT" ;; |
| inspect) echo '{"Running":true}' ;; |
| esac |
| exit 0 |
| -`) |
| +`, logPath, work)) |
| + procs := filepath.Join(work, "processes.json") |
| + if err := os.WriteFile(procs, []byte(`{"web": {"command": ["bin/rails", "server"], "working_dir": "/rails", "env": {"SHOP_CERT": `+strconv.Quote(shopCert)+`}}}`), 0o644); err != nil { |
| + t.Fatal(err) |
| + } |
| probe := filepath.Join(work, "probe") |
| if err := os.WriteFile(probe, []byte("x"), 0o755); err != nil { |
| t.Fatal(err) |
| @@ -720,7 +807,7 @@ exit 0 |
| t.Setenv("PATH", bin) |
| t.Setenv("CARLOS_CONTAINER_RUNTIME", "") |
| t.Setenv("CARLOS_CHECK_PROBE", probe) |
| - t.Setenv("FAKE_LOG", filepath.Join(work, "log")) |
| + t.Setenv("SHOP_HOST_SECRET", "not the container's") |
| t.Setenv("TMPDIR", tmp) |
| t.Setenv("FAKE_PROBE", `{"status":200,"after_ms":7}`) |
| @@ -734,12 +821,18 @@ exit 0 |
| t.Errorf("ship --check does not say %q:\n%s", want, out) |
| } |
| } |
| - log, _ := os.ReadFile(filepath.Join(work, "log")) |
| - for _, verb := range []string{"rm -f carlos-check-", "volume rm carlos-check-", "rmi carlos-check-"} { |
| + log, _ := os.ReadFile(logPath) |
| + for _, verb := range []string{"rm -f carlos-check-", "volume rm carlos-check-", "rmi carlos-check-", "host secret: unset\n"} { |
| if !strings.Contains(string(log), verb) { |
| t.Errorf("the runtime never saw %q:\n%s", verb, log) |
| } |
| } |
| + if strings.Contains(string(log), ".env") { |
| + t.Errorf("an env file was written:\n%s", log) |
| + } |
| + if cert, err := os.ReadFile(filepath.Join(work, "cert")); err != nil || string(cert) != shopCert { |
| + t.Errorf("web's SHOP_CERT reached the runtime as %q (%v), want %q", cert, err, shopCert) |
| + } |
| if strings.Contains(string(log), "-console") { |
| t.Errorf("the filled-in console was started:\n%s", log) |
| } |
diff --git a/ship_rootfs.go b/ship_rootfs.go| index 3f1436775..2b360fd9f 100644 |
| --- a/ship_rootfs.go |
| +++ b/ship_rootfs.go |
| @@ -89,16 +89,13 @@ type rootfsShipOpts struct { |
| // builtRootfs is a rootfs image built on this machine: the tree it was |
| // built from, the processes with the image's config merged in and console |
| // filled, the squashfs file (empty for a --check build, which writes none) |
| -// and its size, --jit, carried for the manifest and --check's warning, and |
| -// the scratch directory it was built in, where --check writes its env |
| -// files. |
| +// and its size, and --jit, carried for the manifest and --check's warning. |
| type builtRootfs struct { |
| - tree *rootfsimage.Tree |
| - procs map[string]release.Process |
| - image string |
| - size int64 |
| - jit bool |
| - scratch string |
| + tree *rootfsimage.Tree |
| + procs map[string]release.Process |
| + image string |
| + size int64 |
| + jit bool |
| } |
| var ( |
| @@ -365,7 +362,7 @@ func buildRootfs(ctx context.Context, src string, procs map[string]release.Proce |
| if err := rootfsimage.ResolveCommands(t, procs); err != nil { |
| return nil, err |
| } |
| - b := &builtRootfs{tree: t, procs: procs, scratch: scratch} |
| + b := &builtRootfs{tree: t, procs: procs} |
| if !write { |
| return b, nil |
| } |
diff --git a/ship_runtime.go b/ship_runtime.go| index e205b546b..a620557db 100644 |
| --- a/ship_runtime.go |
| +++ b/ship_runtime.go |
| @@ -6,10 +6,12 @@ import ( |
| "errors" |
| "fmt" |
| "io" |
| + "maps" |
| "os" |
| "os/exec" |
| "path/filepath" |
| "regexp" |
| + "slices" |
| "strings" |
| "github.com/carlosframework/platform/internal/rootfsimage" |
| @@ -29,8 +31,11 @@ var errNoRuntime = errors.New("--check runs the image with Docker or Podman, and |
| // none), and answers its stdout, also when it fails (--check's probe exits 1 |
| // with its findings on stdout). A failure carries the runtime's stderr. For |
| // logs, whose stdout and stderr are the container's two streams, the answer |
| -// is both streams in one. |
| -type runnerFunc func(ctx context.Context, stdin io.Reader, args ...string) ([]byte, error) |
| +// is both streams in one. env nil runs the binary in this process's |
| +// environment; otherwise env is the child's whole environment (--check's |
| +// run passes a container's variables there, named on argv as -e KEY, so no |
| +// value is written to a file or shown on a command line). |
| +type runnerFunc func(ctx context.Context, stdin io.Reader, env []string, args ...string) ([]byte, error) |
| // containerRuntime is the runtime found: name is "docker" or "podman", and |
| // run runs its binary. |
| @@ -61,9 +66,10 @@ func findContainerRuntime() (containerRuntime, error) { |
| // execRunner runs the binary at bin. Its error is the runtime's own |
| // message, the last of its stderr, so a caller can show why it failed. |
| func execRunner(bin string) runnerFunc { |
| - return func(ctx context.Context, stdin io.Reader, args ...string) ([]byte, error) { |
| + return func(ctx context.Context, stdin io.Reader, env []string, args ...string) ([]byte, error) { |
| cmd := exec.CommandContext(ctx, bin, args...) |
| cmd.Stdin = stdin |
| + cmd.Env = env |
| var stdout, stderr bytes.Buffer |
| cmd.Stdout, cmd.Stderr = &stdout, &stderr |
| if len(args) > 0 && args[0] == "logs" { |
| @@ -81,6 +87,36 @@ func execRunner(bin string) runnerFunc { |
| } |
| } |
| +// runtimeEnvNames are the variables the docker and podman CLIs read to |
| +// find their binaries, their config and their daemon or socket: the only |
| +// part of this process's environment a runtime child given its own env |
| +// keeps. |
| +var runtimeEnvNames = []string{ |
| + "PATH", "HOME", "USER", "LOGNAME", "TMPDIR", "LANG", |
| + "XDG_RUNTIME_DIR", "XDG_CONFIG_HOME", "XDG_DATA_HOME", "DBUS_SESSION_BUS_ADDRESS", |
| + "DOCKER_HOST", "DOCKER_CONTEXT", "DOCKER_CONFIG", "DOCKER_CERT_PATH", "DOCKER_TLS_VERIFY", "DOCKER_API_VERSION", |
| + "CONTAINER_HOST", "CONTAINER_CONNECTION", "CONTAINER_SSHKEY", |
| + "CONTAINERS_CONF", "CONTAINERS_STORAGE_CONF", "CONTAINERS_REGISTRIES_CONF", |
| + "SystemRoot", "SYSTEMROOT", "USERPROFILE", "APPDATA", "LOCALAPPDATA", "ProgramData", |
| +} |
| + |
| +// runtimeChildEnv is a runtime child's whole environment: the |
| +// runtimeEnvNames this process has, then vars, sorted by name. Nothing else |
| +// of this process's environment reaches it, so a container that names a |
| +// variable it does not declare gets nothing from the host. |
| +func runtimeChildEnv(vars map[string]string) []string { |
| + var env []string |
| + for _, name := range runtimeEnvNames { |
| + if v, ok := os.LookupEnv(name); ok { |
| + env = append(env, name+"="+v) |
| + } |
| + } |
| + for _, k := range slices.Sorted(maps.Keys(vars)) { |
| + env = append(env, k+"="+vars[k]) |
| + } |
| + return env |
| +} |
| + |
| // saveImage exports the local image ref to the archive file out: docker |
| // save writes the archive Docker makes, podman save an OCI archive. |
| // Failure is RFS-1-C59. |
| @@ -89,7 +125,7 @@ func (rt containerRuntime) saveImage(ctx context.Context, ref, out string) error |
| if rt.name == "podman" { |
| args = []string{"save", "--format", "oci-archive", "-o", out, ref} |
| } |
| - if _, err := rt.run(ctx, nil, args...); err != nil { |
| + if _, err := rt.run(ctx, nil, nil, args...); err != nil { |
| return fmt.Errorf("could not export %s with %s: %s", ref, rt.name, err) |
| } |
| return nil |
| @@ -110,7 +146,7 @@ func (rt containerRuntime) importTree(ctx context.Context, t *rootfsimage.Tree, |
| pw.CloseWithError(err) // nil closes it as EOF |
| done <- err |
| }() |
| - _, err := rt.run(ctx, pr, args...) |
| + _, err := rt.run(ctx, pr, nil, args...) |
| // A runtime that stopped reading must not leave the writer blocked. |
| pr.CloseWithError(io.ErrClosedPipe) |
| if werr := <-done; err == nil && werr != nil && !errors.Is(werr, io.ErrClosedPipe) { |
diff --git a/ship_runtime_test.go b/ship_runtime_test.go| index 43f90219b..43e6fd7c6 100644 |
| --- a/ship_runtime_test.go |
| +++ b/ship_runtime_test.go |
| @@ -308,7 +308,7 @@ func TestImportTreeArgs(t *testing.T) { |
| } |
| var gotArgs []string |
| var names []string |
| - rt := containerRuntime{name: c.name, run: func(ctx context.Context, stdin io.Reader, args ...string) ([]byte, error) { |
| + rt := containerRuntime{name: c.name, run: func(ctx context.Context, stdin io.Reader, _ []string, args ...string) ([]byte, error) { |
| gotArgs = args |
| tr := tar.NewReader(stdin) |
| for { |
| @@ -343,7 +343,7 @@ func TestImportTreeRuntimeFailsEarly(t *testing.T) { |
| t.Fatal(err) |
| } |
| boom := errors.New("boom") |
| - rt := containerRuntime{name: "docker", run: func(context.Context, io.Reader, ...string) ([]byte, error) { return nil, boom }} |
| + rt := containerRuntime{name: "docker", run: func(context.Context, io.Reader, []string, ...string) ([]byte, error) { return nil, boom }} |
| done := make(chan error, 1) |
| go func() { done <- rt.importTree(context.Background(), tree, "x") }() |
| select { |