Commit 76668f2f38
Verified · cmc ci/build: success ci/test: success
Layout: unified · split
.gitbay/wiki/Admin.org +14 −9
| @@ -408,15 +408,20 @@ repos likewise need at least read for the clone. | |||
| 408 | 408 | ||
| 409 | ** Container isolation | 409 | ** Container isolation |
| 410 | 410 | ||
| 411 | Builds currently run as the runner's own user with no container; the | 411 | Builds run in a rootless podman container, one per job, with the |
| 412 | service drop-in and =-repos= are the controls (krz/gitbay#144). The step | 412 | workspace bind mounted and nothing else: the clone happens outside with |
| 413 | environment is constructed rather than inherited, so a build sees =PATH=, | 413 | the runner's key, so a step cannot read it. =-isolation none= keeps the |
| 414 | =HOME= (its workspace), =LANG=, =CI=, its =GITBAY_*= variables and its | 414 | old behaviour — steps on the host as the runner's user — for an instance |
| 415 | secrets and nothing else — but a step can still read what that user can | 415 | where every repository is trusted. There is no automatic fallback: a |
| 416 | read, and concurrent builds share a =-workdir=. | 416 | runner started with =-isolation podman= that cannot find a working |
| 417 | 417 | podman exits rather than running a build unsandboxed. | |
| 418 | Rootless podman is the chosen remedy; the host preparation ships ahead of | 418 | |
| 419 | the runner that uses it, so the order is fixed: | 419 | =-image= sets the default image for jobs that name none |
| 420 | (=docker.io/library/debian:stable-slim= if unset); a job overrides it | ||
| 421 | with =image:= in =.gitbay/ci.yml=, validated as a reference so a config | ||
| 422 | file cannot turn it into podman arguments. | ||
| 423 | |||
| 424 | Prepare a host before pointing an isolating runner at it: | ||
| 420 | 425 | ||
| 421 | #+begin_src sh | 426 | #+begin_src sh |
| 422 | ssh -p 2222 root@<host> 'sh -s' < deploy/runner-podman-setup.sh | 427 | ssh -p 2222 root@<host> 'sh -s' < deploy/runner-podman-setup.sh |
.gitbay/wiki/Parity.org +1
| @@ -153,6 +153,7 @@ column and are always markdown. | |||
| 153 | | build jobs | yes | yes | yes | | 153 | | build jobs | yes | yes | yes | |
| 154 | | build trigger | yes | yes | yes | | 154 | | build trigger | yes | yes | yes | |
| 155 | | build cancel | yes | yes | no | | 155 | | build cancel | yes | yes | no | |
| 156 | | job image (ci.yml) | yes | n/a | n/a | | ||
| 156 | | dependency checks on/off | yes | yes | no | | 157 | | dependency checks on/off | yes | yes | no | |
| 157 | | dependency status | yes | yes | no | | 158 | | dependency status | yes | yes | no | |
| 158 | | delete, transfer | yes | no | no | | 159 | | delete, transfer | yes | no | no | |
.gitbay/wiki/Threat-Model.org +23 −14
| @@ -137,17 +137,25 @@ runner, polling over SSH, clones the commit and runs its steps. | |||
| 137 | one build can poison a cache another reads, which is no more than | 137 | one build can poison a cache another reads, which is no more than |
| 138 | anything a step can already do as this user, and is what isolation | 138 | anything a step can already do as this user, and is what isolation |
| 139 | (krz/gitbay#144) is for. | 139 | (krz/gitbay#144) is for. |
| 140 | - *Where it runs.* Steps run as the runner's own user on the runner | 140 | - *Where it runs.* Steps run in a rootless podman container, one per |
| 141 | host, with no container; the systemd drop-in adds =NoNewPrivileges=, | 141 | job, with the workspace bind mounted and nothing else. The clone |
| 142 | =ProtectSystem=full= and the kernel and cgroup protections. =-repos= | 142 | happens outside it with the runner's key, so the container never sees |
| 143 | limits a runner to named repositories, which is the control that | 143 | =GIT_SSH_COMMAND=, the key, or the runner's environment. =-isolation |
| 144 | matters on an open instance: without it a runner builds whatever | 144 | none= runs steps on the host as before, for an instance where every |
| 145 | anyone pushes. | 145 | repository is trusted; there is no automatic fallback to it — a runner |
| 146 | 146 | configured for podman that cannot find one refuses to start, because | |
| 147 | Anything a step can do as the runner's user, a pushed =ci.yml= can do. | 147 | dropping isolation silently is worse than a stopped runner. The |
| 148 | Treat the runner host as executing untrusted code: keep it off the | 148 | systemd drop-in still adds =NoNewPrivileges=, =ProtectSystem=full= and |
| 149 | daemon's host where the database lives, or scope it to repositories | 149 | the kernel and cgroup protections, and =-repos= still limits a runner |
| 150 | whose writers you trust. | 150 | to named repositories. |
| 151 | |||
| 152 | Under =-isolation none=, anything a step can do as the runner's user a | ||
| 153 | pushed =ci.yml= can do. Under podman a step is confined to its | ||
| 154 | container and the bind-mounted workspace, but the build home's caches | ||
| 155 | are shared between builds, so one build can still leave something a | ||
| 156 | later build reads. Treat the runner host as executing untrusted code: | ||
| 157 | keep it off the daemon's host where the database lives, or scope it to | ||
| 158 | repositories whose writers you trust. | ||
| 151 | 159 | ||
| 152 | * What has not been audited | 160 | * What has not been audited |
| 153 | 161 | ||
| @@ -187,6 +195,7 @@ assume has been checked. | |||
| 187 | harmless (see [[Admin]]). | 195 | harmless (see [[Admin]]). |
| 188 | - A global signature-verification epoch over-invalidates the cache on any | 196 | - A global signature-verification epoch over-invalidates the cache on any |
| 189 | trust-input change. Correct, not a leak; a performance tradeoff. | 197 | trust-input change. Correct, not a leak; a performance tradeoff. |
| 190 | - Build steps run as the runner's user with no container. Isolation is | 198 | - A build's secrets are environment variables inside its container, so |
| 191 | the key scope, the sandboxing drop-in and =-repos=; containers are | 199 | they are visible to =podman inspect= as the runner's user — the same |
| 192 | future work. | 200 | user that already holds them in memory. They reach podman through a |
| 201 | 0600 env file rather than argv, since =/proc= is world-readable. | ||
cmd/gitbay-runner/isolate.go added +168
| @@ -0,0 +1,168 @@ | |||
| 1 | package main | ||
| 2 | |||
| 3 | import ( | ||
| 4 | "fmt" | ||
| 5 | "io" | ||
| 6 | "log" | ||
| 7 | "os" | ||
| 8 | "os/exec" | ||
| 9 | "path/filepath" | ||
| 10 | "strings" | ||
| 11 | "time" | ||
| 12 | |||
| 13 | "gitbay.org/gitbay/internal/toolpath" | ||
| 14 | ) | ||
| 15 | |||
| 16 | // Isolation modes. podman runs a job's steps in a container; none runs | ||
| 17 | // them on the host as the runner's user, which is what the runner did | ||
| 18 | // before #144 and what a private instance may still choose. | ||
| 19 | const ( | ||
| 20 | isolationPodman = "podman" | ||
| 21 | isolationNone = "none" | ||
| 22 | ) | ||
| 23 | |||
| 24 | // defaultImage is used when neither the job nor -image names one. Chosen | ||
| 25 | // for being small and having a shell; anything a build actually needs it | ||
| 26 | // declares with `image:`. | ||
| 27 | const defaultImage = "docker.io/library/debian:stable-slim" | ||
| 28 | |||
| 29 | // checkIsolation fails the runner at start-up rather than at the first | ||
| 30 | // build, and refuses anything it does not recognise. There is no silent | ||
| 31 | // fallback from podman to the host: dropping isolation without saying so | ||
| 32 | // is the failure mode this whole change exists to prevent (#144). | ||
| 33 | func (r *runner) checkIsolation() error { | ||
| 34 | switch r.isolation { | ||
| 35 | case isolationNone: | ||
| 36 | log.Printf("WARNING: -isolation none: build steps run on this host as %s, "+ | ||
| 37 | "with no container. Only do this where every repository is trusted.", currentUser()) | ||
| 38 | return nil | ||
| 39 | case isolationPodman: | ||
| 40 | bin := toolpath.Look("podman") | ||
| 41 | out, err := exec.Command(bin, "info", "--format", "{{.Host.Security.Rootless}}").CombinedOutput() | ||
| 42 | if err != nil { | ||
| 43 | return fmt.Errorf("podman is required by -isolation podman but does not work here: %v\n%s\n"+ | ||
| 44 | "prepare the host with deploy/runner-podman-setup.sh, or pass -isolation none "+ | ||
| 45 | "if every repository on this instance is trusted", err, strings.TrimSpace(string(out))) | ||
| 46 | } | ||
| 47 | if r.image == "" { | ||
| 48 | r.image = defaultImage | ||
| 49 | } | ||
| 50 | log.Printf("isolation: podman (rootless=%s), default image %s", | ||
| 51 | strings.TrimSpace(string(out)), r.image) | ||
| 52 | return nil | ||
| 53 | default: | ||
| 54 | return fmt.Errorf("unknown -isolation %q: podman or none", r.isolation) | ||
| 55 | } | ||
| 56 | } | ||
| 57 | |||
| 58 | // runSteps executes a job's steps and reports whether all succeeded. The | ||
| 59 | // clone has already happened, outside any container and with the runner's | ||
| 60 | // key: the container never sees GIT_SSH_COMMAND, the key, or the runner's | ||
| 61 | // environment — it gets the workspace and nothing else. | ||
| 62 | type stepRunner func(cmd *exec.Cmd, deadline time.Time) (bool, string) | ||
| 63 | |||
| 64 | func (r *runner) runSteps(j job, dir string, env []string, sink io.Writer, deadline time.Time, runStep stepRunner) bool { | ||
| 65 | if r.isolation == isolationNone { | ||
| 66 | for _, step := range j.Steps { | ||
| 67 | fmt.Fprintf(sink, "$ %s\n", step) | ||
| 68 | cmd := exec.Command(toolpath.Look("sh"), "-c", step) | ||
| 69 | cmd.Dir, cmd.Env = dir, env | ||
| 70 | cmd.Stdout, cmd.Stderr = sink, sink | ||
| 71 | if ok, why := runStep(cmd, deadline); !ok { | ||
| 72 | fmt.Fprintf(sink, "%s\n", why) | ||
| 73 | return false | ||
| 74 | } | ||
| 75 | } | ||
| 76 | return true | ||
| 77 | } | ||
| 78 | return r.runStepsPodman(j, dir, env, sink, deadline, runStep) | ||
| 79 | } | ||
| 80 | |||
| 81 | // runStepsPodman starts one container for the whole job and runs each | ||
| 82 | // step in it with `podman exec`. One container per job, not per step, | ||
| 83 | // because steps share state — a build step writes what a test step reads | ||
| 84 | // — and per-step containers would break that. | ||
| 85 | func (r *runner) runStepsPodman(j job, dir string, env []string, sink io.Writer, deadline time.Time, runStep stepRunner) bool { | ||
| 86 | podman := toolpath.Look("podman") | ||
| 87 | image := j.Image | ||
| 88 | if image == "" { | ||
| 89 | image = r.image | ||
| 90 | } | ||
| 91 | |||
| 92 | // Secrets must not reach argv: /proc is world-readable, and this | ||
| 93 | // codebase keeps them on stdin or in files everywhere else. An env | ||
| 94 | // file outside the workspace holds them instead — outside because the | ||
| 95 | // workspace is bind mounted, and a file of secrets sitting in the | ||
| 96 | // checkout is one `cat` from a build's own log. | ||
| 97 | envFile := filepath.Join(r.workdir, fmt.Sprintf("env-%d", j.ID)) | ||
| 98 | if err := writeEnvFile(envFile, env); err != nil { | ||
| 99 | fmt.Fprintf(sink, "preparing the build environment: %v\n", err) | ||
| 100 | return false | ||
| 101 | } | ||
| 102 | defer os.Remove(envFile) | ||
| 103 | |||
| 104 | name := fmt.Sprintf("gitbay-build-%d", j.ID) | ||
| 105 | // --rm so a container cannot outlive its build; the explicit rm below | ||
| 106 | // covers the case where the daemon-less run itself fails. | ||
| 107 | start := exec.Command(podman, "run", "--detach", "--rm", | ||
| 108 | "--name", name, | ||
| 109 | "--env-file", envFile, | ||
| 110 | "--volume", dir+":/workspace:rw", | ||
| 111 | "--workdir", "/workspace", | ||
| 112 | "--entrypoint", "sh", | ||
| 113 | image, "-c", "sleep infinity") | ||
| 114 | start.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + r.podmanHome()} | ||
| 115 | if out, err := start.CombinedOutput(); err != nil { | ||
| 116 | // A pull failure lands here. Fail the build with what podman | ||
| 117 | // said; do not retry and do not fall back to another image. | ||
| 118 | fmt.Fprintf(sink, "starting the build container from %s failed:\n%s\n", image, strings.TrimSpace(string(out))) | ||
| 119 | return false | ||
| 120 | } | ||
| 121 | defer exec.Command(podman, "rm", "--force", name).Run() | ||
| 122 | |||
| 123 | for _, step := range j.Steps { | ||
| 124 | fmt.Fprintf(sink, "$ %s\n", step) | ||
| 125 | cmd := exec.Command(podman, "exec", "--workdir", "/workspace", name, "sh", "-c", step) | ||
| 126 | cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + r.podmanHome()} | ||
| 127 | cmd.Stdout, cmd.Stderr = sink, sink | ||
| 128 | if ok, why := runStep(cmd, deadline); !ok { | ||
| 129 | fmt.Fprintf(sink, "%s\n", why) | ||
| 130 | return false | ||
| 131 | } | ||
| 132 | } | ||
| 133 | return true | ||
| 134 | } | ||
| 135 | |||
| 136 | // podmanHome is where podman keeps its own storage: the runner's home, | ||
| 137 | // not a build's. The container store is the runner's business, and a | ||
| 138 | // build never sees this path. | ||
| 139 | func (r *runner) podmanHome() string { | ||
| 140 | if h, err := os.UserHomeDir(); err == nil && h != "" { | ||
| 141 | return h | ||
| 142 | } | ||
| 143 | return "/var/lib/gitbay-runner" | ||
| 144 | } | ||
| 145 | |||
| 146 | // writeEnvFile writes KEY=VALUE lines for podman --env-file, readable | ||
| 147 | // only by this user. Values containing a newline are refused rather than | ||
| 148 | // silently truncated: the format has no escape for one, and a secret that | ||
| 149 | // half-arrives is worse than a failed build. | ||
| 150 | func writeEnvFile(path string, env []string) error { | ||
| 151 | var b strings.Builder | ||
| 152 | for _, e := range env { | ||
| 153 | if strings.ContainsAny(e, "\n\r") { | ||
| 154 | name, _, _ := strings.Cut(e, "=") | ||
| 155 | return fmt.Errorf("%s contains a newline, which an env file cannot carry", name) | ||
| 156 | } | ||
| 157 | b.WriteString(e) | ||
| 158 | b.WriteByte('\n') | ||
| 159 | } | ||
| 160 | return os.WriteFile(path, []byte(b.String()), 0o600) | ||
| 161 | } | ||
| 162 | |||
| 163 | func currentUser() string { | ||
| 164 | if u := os.Getenv("USER"); u != "" { | ||
| 165 | return u | ||
| 166 | } | ||
| 167 | return fmt.Sprintf("uid %d", os.Getuid()) | ||
| 168 | } | ||
cmd/gitbay-runner/main.go +18 −12
| @@ -35,6 +35,7 @@ type job struct { | |||
| 35 | SHA string `json:"sha"` | 35 | SHA string `json:"sha"` |
| 36 | Ref string `json:"ref"` | 36 | Ref string `json:"ref"` |
| 37 | Steps []string `json:"steps"` | 37 | Steps []string `json:"steps"` |
| 38 | Image string `json:"image"` | ||
| 38 | Secrets map[string]string `json:"secrets"` | 39 | Secrets map[string]string `json:"secrets"` |
| 39 | } | 40 | } |
| 40 | 41 | ||
| @@ -44,6 +45,10 @@ type runner struct { | |||
| 44 | cloneBase string // e.g. ssh://git@gitbay.org | 45 | cloneBase string // e.g. ssh://git@gitbay.org |
| 45 | workdir string | 46 | workdir string |
| 46 | timeout time.Duration | 47 | timeout time.Duration |
| 48 | // image is the container image for a job that names none, and | ||
| 49 | // isolation selects how steps run: "podman" or "none". | ||
| 50 | image string | ||
| 51 | isolation string | ||
| 47 | // repos limits which repositories this runner claims builds for. Empty | 52 | // repos limits which repositories this runner claims builds for. Empty |
| 48 | // means any, which is what a runner on the server itself wants; a runner | 53 | // means any, which is what a runner on the server itself wants; a runner |
| 49 | // somewhere that should not execute every repository's steps names them. | 54 | // somewhere that should not execute every repository's steps names them. |
| @@ -61,6 +66,8 @@ func main() { | |||
| 61 | repos = flag.String("repos", "", "only claim builds for these repositories, comma-separated owner/name (default: any)") | 66 | repos = flag.String("repos", "", "only claim builds for these repositories, comma-separated owner/name (default: any)") |
| 62 | once = flag.Bool("once", false, "process at most one build, then exit") | 67 | once = flag.Bool("once", false, "process at most one build, then exit") |
| 63 | jobs = flag.Int("jobs", 1, "builds to run at once") | 68 | jobs = flag.Int("jobs", 1, "builds to run at once") |
| 69 | image = flag.String("image", "", "default container image for jobs that name none") | ||
| 70 | isolation = flag.String("isolation", "podman", "how steps run: podman, or none for no container") | ||
| 64 | version = flag.Bool("version", false, "print the commit this binary was built from, then exit") | 71 | version = flag.Bool("version", false, "print the commit this binary was built from, then exit") |
| 65 | ) | 72 | ) |
| 66 | flag.Parse() | 73 | flag.Parse() |
| @@ -76,6 +83,16 @@ func main() { | |||
| 76 | cloneBase: *cloneBase, | 83 | cloneBase: *cloneBase, |
| 77 | workdir: *workdir, | 84 | workdir: *workdir, |
| 78 | timeout: *timeout, | 85 | timeout: *timeout, |
| 86 | image: *image, | ||
| 87 | isolation: *isolation, | ||
| 88 | } | ||
| 89 | if err := r.checkIsolation(); err != nil { | ||
| 90 | // Refusing to start is the point. A runner that quietly fell back | ||
| 91 | // to running repository code on the host would drop isolation | ||
| 92 | // with nothing to surface it, which is worse than a stopped | ||
| 93 | // runner: the operator sees a failed unit either way, but only | ||
| 94 | // one of them is honest about why (#144). | ||
| 95 | log.Fatalf("isolation: %v", err) | ||
| 79 | } | 96 | } |
| 80 | if *sshOpts != "" { | 97 | if *sshOpts != "" { |
| 81 | r.sshOpts = strings.Fields(*sshOpts) | 98 | r.sshOpts = strings.Fields(*sshOpts) |
| @@ -323,18 +340,7 @@ func (r *runner) run(j job) bool { | |||
| 323 | } | 340 | } |
| 324 | 341 | ||
| 325 | env := stepEnv(j, buildHome) | 342 | env := stepEnv(j, buildHome) |
| 326 | for _, step := range j.Steps { | 343 | return r.runSteps(j, dir, env, sink, deadline, runStep) |
| 327 | fmt.Fprintf(sink, "$ %s\n", step) | ||
| 328 | cmd := exec.Command(toolpath.Look("sh"), "-c", step) | ||
| 329 | cmd.Dir = dir | ||
| 330 | cmd.Env = env | ||
| 331 | cmd.Stdout, cmd.Stderr = sink, sink | ||
| 332 | if ok, why := runStep(cmd, deadline); !ok { | ||
| 333 | fmt.Fprintf(sink, "%s\n", why) | ||
| 334 | return false | ||
| 335 | } | ||
| 336 | } | ||
| 337 | return true | ||
| 338 | } | 344 | } |
| 339 | 345 | ||
| 340 | // stepEnv builds the environment a build step runs with. It is | 346 | // stepEnv builds the environment a build step runs with. It is |
deploy/gitbay-runner.override.conf +5 −1
| @@ -17,7 +17,11 @@ | |||
| 17 | # and read-only git, and the sandboxing below keeps a step from | 17 | # and read-only git, and the sandboxing below keeps a step from |
| 18 | # touching the system outside its workspace. | 18 | # touching the system outside its workspace. |
| 19 | # | 19 | # |
| 20 | # Delegate=yes and the storage path below are what rootless podman needs | 20 | # Delegate=yes |
| 21 | # The service unit's ExecStart carries -isolation; podman is the default, | ||
| 22 | # and a runner that cannot find one refuses to start rather than running | ||
| 23 | # repository code on the host. Prepare the host first | ||
| 24 | # (deploy/runner-podman-setup.sh). and the storage path below are what rootless podman needs | ||
| 21 | # (#144): it manages its own cgroups for a container, and its image and | 25 | # (#144): it manages its own cgroups for a container, and its image and |
| 22 | # container store lives under the runner's home, which ProtectSystem | 26 | # container store lives under the runner's home, which ProtectSystem |
| 23 | # would otherwise make read-only. Prepare the host with | 27 | # would otherwise make read-only. Prepare the host with |
e2e/build_cancel_test.go +1
| @@ -122,6 +122,7 @@ func TestBuildCancelRunning(t *testing.T) { | |||
| 122 | opts := fmt.Sprintf("-p %d -i %s -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=%s -o BatchMode=yes", | 122 | opts := fmt.Sprintf("-p %d -i %s -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=%s -o BatchMode=yes", |
| 123 | inst.port, runnerKey, filepath.Join(inst.sshDir, "known_hosts")) | 123 | inst.port, runnerKey, filepath.Join(inst.sshDir, "known_hosts")) |
| 124 | runner := exec.Command(inst.runner, "-once", "-remote", "git@127.0.0.1", "-ssh-opts", opts, | 124 | runner := exec.Command(inst.runner, "-once", "-remote", "git@127.0.0.1", "-ssh-opts", opts, |
| 125 | "-isolation", "none", | ||
| 125 | "-clone-base", fmt.Sprintf("ssh://git@127.0.0.1:%d", inst.port), "-workdir", t.TempDir()) | 126 | "-clone-base", fmt.Sprintf("ssh://git@127.0.0.1:%d", inst.port), "-workdir", t.TempDir()) |
| 126 | runner.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null") | 127 | runner.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null") |
| 127 | var runnerOut strings.Builder | 128 | var runnerOut strings.Builder |
e2e/ci_test.go +6
| @@ -27,9 +27,14 @@ func (i *instance) runnerOnce(t *testing.T, key string) string { | |||
| 27 | t.Helper() | 27 | t.Helper() |
| 28 | opts := fmt.Sprintf("-p %d -i %s -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=%s -o BatchMode=yes", | 28 | opts := fmt.Sprintf("-p %d -i %s -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=%s -o BatchMode=yes", |
| 29 | i.port, key, filepath.Join(i.sshDir, "known_hosts")) | 29 | i.port, key, filepath.Join(i.sshDir, "known_hosts")) |
| 30 | // -isolation none: these tests exercise claiming, logs, statuses and | ||
| 31 | // cancellation, not the sandbox, and the suite must run on a machine | ||
| 32 | // without podman. The isolation tests are in isolation_podman_test.go | ||
| 33 | // and skip visibly when it is absent (#144). | ||
| 30 | cmd := exec.Command(i.runner, "-once", | 34 | cmd := exec.Command(i.runner, "-once", |
| 31 | "-remote", "git@127.0.0.1", | 35 | "-remote", "git@127.0.0.1", |
| 32 | "-ssh-opts", opts, | 36 | "-ssh-opts", opts, |
| 37 | "-isolation", "none", | ||
| 33 | "-clone-base", fmt.Sprintf("ssh://git@127.0.0.1:%d", i.port), | 38 | "-clone-base", fmt.Sprintf("ssh://git@127.0.0.1:%d", i.port), |
| 34 | "-workdir", t.TempDir()) | 39 | "-workdir", t.TempDir()) |
| 35 | cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null") | 40 | cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null") |
| @@ -281,6 +286,7 @@ func (i *instance) runnerJobs(t *testing.T, key, repo string, jobs int) string { | |||
| 281 | cmd := exec.Command(i.runner, | 286 | cmd := exec.Command(i.runner, |
| 282 | "-jobs", fmt.Sprint(jobs), | 287 | "-jobs", fmt.Sprint(jobs), |
| 283 | "-poll", "200ms", | 288 | "-poll", "200ms", |
| 289 | "-isolation", "none", | ||
| 284 | "-remote", "git@127.0.0.1", | 290 | "-remote", "git@127.0.0.1", |
| 285 | "-ssh-opts", opts, | 291 | "-ssh-opts", opts, |
| 286 | "-clone-base", fmt.Sprintf("ssh://git@127.0.0.1:%d", i.port), | 292 | "-clone-base", fmt.Sprintf("ssh://git@127.0.0.1:%d", i.port), |
e2e/isolation_podman_test.go added +163
| @@ -0,0 +1,163 @@ | |||
| 1 | package e2e | ||
| 2 | |||
| 3 | import ( | ||
| 4 | "fmt" | ||
| 5 | "os" | ||
| 6 | "os/exec" | ||
| 7 | "path/filepath" | ||
| 8 | "strings" | ||
| 9 | "testing" | ||
| 10 | ) | ||
| 11 | |||
| 12 | // havePodman reports whether a working rootless podman is on this | ||
| 13 | // machine. The skip is loud on purpose: an isolation test that quietly | ||
| 14 | // does not run is how isolation regresses (#144). | ||
| 15 | func havePodman(t *testing.T) bool { | ||
| 16 | t.Helper() | ||
| 17 | if _, err := exec.LookPath("podman"); err != nil { | ||
| 18 | t.Log("SKIPPING ISOLATION TEST: podman is not installed on this machine. " + | ||
| 19 | "The container path is NOT covered by this run.") | ||
| 20 | return false | ||
| 21 | } | ||
| 22 | if out, err := exec.Command("podman", "info", "--format", "{{.Host.Security.Rootless}}").CombinedOutput(); err != nil { | ||
| 23 | t.Logf("SKIPPING ISOLATION TEST: podman does not work here: %v\n%s", err, out) | ||
| 24 | return false | ||
| 25 | } | ||
| 26 | return true | ||
| 27 | } | ||
| 28 | |||
| 29 | // The fallback that must not exist: with -isolation podman and no podman, | ||
| 30 | // the runner refuses to start rather than running a build on the host. | ||
| 31 | // This one needs no podman, so it runs everywhere. | ||
| 32 | func TestRunnerRefusesToStartWithoutPodman(t *testing.T) { | ||
| 33 | bin := buildRunner(t) | ||
| 34 | cmd := exec.Command(bin, "-once", "-remote", "git@127.0.0.1", | ||
| 35 | "-isolation", "podman", "-workdir", t.TempDir()) | ||
| 36 | // An empty PATH is the reliable way to make podman missing whether or | ||
| 37 | // not this machine has one. | ||
| 38 | cmd.Env = []string{"PATH=" + t.TempDir(), "HOME=" + t.TempDir()} | ||
| 39 | out, err := cmd.CombinedOutput() | ||
| 40 | if err == nil { | ||
| 41 | t.Fatalf("the runner started without podman:\n%s", out) | ||
| 42 | } | ||
| 43 | if !strings.Contains(string(out), "podman") { | ||
| 44 | t.Errorf("refusal does not say podman is the problem:\n%s", out) | ||
| 45 | } | ||
| 46 | if !strings.Contains(string(out), "runner-podman-setup.sh") { | ||
| 47 | t.Errorf("refusal does not say how to fix it:\n%s", out) | ||
| 48 | } | ||
| 49 | } | ||
| 50 | |||
| 51 | // An unknown mode is refused rather than guessed at. | ||
| 52 | func TestRunnerRefusesUnknownIsolation(t *testing.T) { | ||
| 53 | bin := buildRunner(t) | ||
| 54 | cmd := exec.Command(bin, "-once", "-remote", "git@127.0.0.1", | ||
| 55 | "-isolation", "chroot", "-workdir", t.TempDir()) | ||
| 56 | out, err := cmd.CombinedOutput() | ||
| 57 | if err == nil { | ||
| 58 | t.Fatalf("an unknown isolation mode started:\n%s", out) | ||
| 59 | } | ||
| 60 | if !strings.Contains(string(out), "podman or none") { | ||
| 61 | t.Errorf("refusal does not name the valid modes:\n%s", out) | ||
| 62 | } | ||
| 63 | } | ||
| 64 | |||
| 65 | // With podman, a step runs in a container: it cannot read the runner's | ||
| 66 | // SSH key, and it does not see the runner's home. | ||
| 67 | func TestPodmanStepCannotReachTheRunnersKey(t *testing.T) { | ||
| 68 | if !havePodman(t) { | ||
| 69 | t.Skip("no podman") | ||
| 70 | } | ||
| 71 | inst := startInstance(t) | ||
| 72 | inst.runner = buildRunner(t) | ||
| 73 | aliceKey := inst.newKey(t, "alice") | ||
| 74 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub") | ||
| 75 | runnerKey := inst.newKey(t, "ci") | ||
| 76 | inst.admin(t, "admin", "user", "create", "ci", "--key", runnerKey+".pub", "--admin") | ||
| 77 | inst.ssh(t, aliceKey, "", "repo", "create", "alice/app") | ||
| 78 | |||
| 79 | env := inst.gitEnv(aliceKey) | ||
| 80 | work := t.TempDir() | ||
| 81 | mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w") | ||
| 82 | dir := filepath.Join(work, "w") | ||
| 83 | os.MkdirAll(filepath.Join(dir, ".gitbay"), 0o755) | ||
| 84 | // The step tries to read the key the runner authenticates with, and | ||
| 85 | // to list the runner's home. Both must fail inside the container. | ||
| 86 | os.WriteFile(filepath.Join(dir, ".gitbay", "ci.yml"), []byte( | ||
| 87 | "jobs:\n peek:\n image: docker.io/library/debian:stable-slim\n steps:\n"+ | ||
| 88 | " - 'if cat "+runnerKey+" 2>/dev/null; then echo LEAKED-KEY; exit 1; fi; echo no-key'\n"+ | ||
| 89 | " - 'echo HOME=$HOME; ls /workspace'\n"), 0o644) | ||
| 90 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | ||
| 91 | mustGit(t, dir, env, "add", ".") | ||
| 92 | mustGit(t, dir, env, "commit", "-q", "-m", "base") | ||
| 93 | mustGit(t, dir, env, "push", "-q", "origin", "main") | ||
| 94 | |||
| 95 | runnerPodmanOnce(t, inst, runnerKey) | ||
| 96 | out, _, _ := inst.ssh(t, aliceKey, "", "build", "list", "alice/app") | ||
| 97 | if !strings.Contains(out, "success") { | ||
| 98 | t.Fatalf("the containerised build did not pass:\n%s", out) | ||
| 99 | } | ||
| 100 | log, _, _ := inst.ssh(t, aliceKey, "", "build", "log", "alice/app", "1") | ||
| 101 | if strings.Contains(log, "LEAKED-KEY") { | ||
| 102 | t.Errorf("a step read the runner's ssh key:\n%s", log) | ||
| 103 | } | ||
| 104 | if !strings.Contains(log, "no-key") { | ||
| 105 | t.Errorf("the step did not run as expected:\n%s", log) | ||
| 106 | } | ||
| 107 | } | ||
| 108 | |||
| 109 | // A pull failure fails the build and says why, rather than retrying or | ||
| 110 | // silently choosing another image. | ||
| 111 | func TestPodmanPullFailureFailsTheBuild(t *testing.T) { | ||
| 112 | if !havePodman(t) { | ||
| 113 | t.Skip("no podman") | ||
| 114 | } | ||
| 115 | inst := startInstance(t) | ||
| 116 | inst.runner = buildRunner(t) | ||
| 117 | aliceKey := inst.newKey(t, "alice") | ||
| 118 | inst.admin(t, "admin", "user", "create", "alice", "--key", aliceKey+".pub") | ||
| 119 | runnerKey := inst.newKey(t, "ci") | ||
| 120 | inst.admin(t, "admin", "user", "create", "ci", "--key", runnerKey+".pub", "--admin") | ||
| 121 | inst.ssh(t, aliceKey, "", "repo", "create", "alice/app") | ||
| 122 | |||
| 123 | env := inst.gitEnv(aliceKey) | ||
| 124 | work := t.TempDir() | ||
| 125 | mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w") | ||
| 126 | dir := filepath.Join(work, "w") | ||
| 127 | os.MkdirAll(filepath.Join(dir, ".gitbay"), 0o755) | ||
| 128 | os.WriteFile(filepath.Join(dir, ".gitbay", "ci.yml"), []byte( | ||
| 129 | "jobs:\n nope:\n image: localhost/gitbay-no-such-image:v0\n steps:\n - echo unreachable\n"), 0o644) | ||
| 130 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") | ||
| 131 | mustGit(t, dir, env, "add", ".") | ||
| 132 | mustGit(t, dir, env, "commit", "-q", "-m", "base") | ||
| 133 | mustGit(t, dir, env, "push", "-q", "origin", "main") | ||
| 134 | |||
| 135 | runnerPodmanOnce(t, inst, runnerKey) | ||
| 136 | out, _, _ := inst.ssh(t, aliceKey, "", "build", "list", "alice/app") | ||
| 137 | if !strings.Contains(out, "failure") { | ||
| 138 | t.Fatalf("a build with an unpullable image did not fail:\n%s", out) | ||
| 139 | } | ||
| 140 | log, _, _ := inst.ssh(t, aliceKey, "", "build", "log", "alice/app", "1") | ||
| 141 | if !strings.Contains(log, "gitbay-no-such-image") { | ||
| 142 | t.Errorf("the log does not name the image that could not be pulled:\n%s", log) | ||
| 143 | } | ||
| 144 | if strings.Contains(log, "unreachable") { | ||
| 145 | t.Error("a step ran despite the image failing to start") | ||
| 146 | } | ||
| 147 | } | ||
| 148 | |||
| 149 | func runnerPodmanOnce(t *testing.T, inst *instance, key string) { | ||
| 150 | t.Helper() | ||
| 151 | opts := fmt.Sprintf("-p %d -i %s -o IdentitiesOnly=yes -o StrictHostKeyChecking=no -o UserKnownHostsFile=%s -o BatchMode=yes", | ||
| 152 | inst.port, key, filepath.Join(inst.sshDir, "known_hosts")) | ||
| 153 | cmd := exec.Command(inst.runner, "-once", | ||
| 154 | "-remote", "git@127.0.0.1", | ||
| 155 | "-ssh-opts", opts, | ||
| 156 | "-isolation", "podman", | ||
| 157 | "-clone-base", fmt.Sprintf("ssh://git@127.0.0.1:%d", inst.port), | ||
| 158 | "-workdir", t.TempDir()) | ||
| 159 | cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null") | ||
| 160 | if out, err := cmd.CombinedOutput(); err != nil { | ||
| 161 | t.Fatalf("runner: %v\n%s", err, out) | ||
| 162 | } | ||
| 163 | } | ||
internal/ci/sched.go +1 −1
| @@ -101,7 +101,7 @@ func (s *Scheduler) RunDue(now time.Time) { | |||
| 101 | continue | 101 | continue |
| 102 | } | 102 | } |
| 103 | steps, _ := json.Marshal(job.Steps) | 103 | steps, _ := json.Marshal(job.Steps) |
| 104 | n, err := s.St.CreateBuild(repo.ID, job.Name, sha, repo.DefaultBranch, string(steps), true) | 104 | n, err := s.St.CreateBuild(repo.ID, job.Name, sha, repo.DefaultBranch, string(steps), job.Image, true) |
| 105 | if err != nil { | 105 | if err != nil { |
| 106 | slog.Error("scheduler: queueing build", "repo", repo.Path(), "job", job.Name, "err", err) | 106 | slog.Error("scheduler: queueing build", "repo", repo.Path(), "job", job.Name, "err", err) |
| 107 | continue | 107 | continue |
internal/control/build.go +4 −3
| @@ -227,7 +227,7 @@ func runBuildTrigger(c *Ctx, args []string) int { | |||
| 227 | continue | 227 | continue |
| 228 | } | 228 | } |
| 229 | steps, _ := json.Marshal(j.Steps) | 229 | steps, _ := json.Marshal(j.Steps) |
| 230 | n, err := c.Store.CreateBuild(repo.ID, j.Name, sha, repo.DefaultBranch, string(steps), true) | 230 | n, err := c.Store.CreateBuild(repo.ID, j.Name, sha, repo.DefaultBranch, string(steps), j.Image, true) |
| 231 | if err != nil { | 231 | if err != nil { |
| 232 | return c.fail(protocol.ExitFailure, "%v", err) | 232 | return c.fail(protocol.ExitFailure, "%v", err) |
| 233 | } | 233 | } |
| @@ -398,8 +398,9 @@ func runRunnerNext(c *Ctx, args []string) int { | |||
| 398 | SHA string `json:"sha"` | 398 | SHA string `json:"sha"` |
| 399 | Ref string `json:"ref"` | 399 | Ref string `json:"ref"` |
| 400 | Steps []string `json:"steps"` | 400 | Steps []string `json:"steps"` |
| 401 | Image string `json:"image,omitempty"` | ||
| 401 | Secrets map[string]string `json:"secrets,omitempty"` | 402 | Secrets map[string]string `json:"secrets,omitempty"` |
| 402 | }{b.ID, repo.Path(), b.Number, b.Job, b.SHA, b.Ref, steps, secrets} | 403 | }{b.ID, repo.Path(), b.Number, b.Job, b.SHA, b.Ref, steps, b.Image, secrets} |
| 403 | return c.emit(d, func(w io.Writer) { | 404 | return c.emit(d, func(w io.Writer) { |
| 404 | fmt.Fprintf(w, "build %d: %s %s @ %.10s\n", d.ID, d.Repo, d.Job, d.SHA) | 405 | fmt.Fprintf(w, "build %d: %s %s @ %.10s\n", d.ID, d.Repo, d.Job, d.SHA) |
| 405 | }) | 406 | }) |
| @@ -687,7 +688,7 @@ func queueJobs( | |||
| 687 | continue | 688 | continue |
| 688 | } | 689 | } |
| 689 | steps, _ := json.Marshal(j.Steps) | 690 | steps, _ := json.Marshal(j.Steps) |
| 690 | n, err := st.CreateBuild(repo.ID, j.Name, sha, ref, string(steps), trusted) | 691 | n, err := st.CreateBuild(repo.ID, j.Name, sha, ref, string(steps), j.Image, trusted) |
| 691 | if err != nil { | 692 | if err != nil { |
| 692 | slog.Error("queueing build", "repo", repo.Path(), "job", j.Name, "err", err) | 693 | slog.Error("queueing build", "repo", repo.Path(), "job", j.Name, "err", err) |
| 693 | continue | 694 | continue |
internal/control/build_test.go +1 −1
| @@ -432,7 +432,7 @@ func TestQueueBranchBuildsAlreadyBuiltJobRecordsNoSkippedStatus(t *testing.T) { | |||
| 432 | // The same commit already has a build for "unit" from another branch, | 432 | // The same commit already has a build for "unit" from another branch, |
| 433 | // still pending. Its filter would exclude this push too, so the only | 433 | // still pending. Its filter would exclude this push too, so the only |
| 434 | // way to tell the two paths apart is that this one must record nothing. | 434 | // way to tell the two paths apart is that this one must record nothing. |
| 435 | if _, err := st.CreateBuild(repo.ID, "unit", newSHA, "other", `["echo hi"]`, true); err != nil { | 435 | if _, err := st.CreateBuild(repo.ID, "unit", newSHA, "other", `["echo hi"]`, "", true); err != nil { |
| 436 | t.Fatal(err) | 436 | t.Fatal(err) |
| 437 | } | 437 | } |
| 438 | 438 | ||
internal/control/runnernext_test.go +5 −5
| @@ -60,14 +60,14 @@ func setupOrphanRepo(t *testing.T) (*store.Store, store.Repo, int64, string, str | |||
| 60 | func TestRunnerNextSkipsOrphanedBuildAndClaimsNext(t *testing.T) { | 60 | func TestRunnerNextSkipsOrphanedBuildAndClaimsNext(t *testing.T) { |
| 61 | st, repo, uid, root, baseSHA, orphanSHA := setupOrphanRepo(t) | 61 | st, repo, uid, root, baseSHA, orphanSHA := setupOrphanRepo(t) |
| 62 | 62 | ||
| 63 | orphanedID, err := st.CreateBuild(repo.ID, "unit", orphanSHA, "main", "[]", true) | 63 | orphanedID, err := st.CreateBuild(repo.ID, "unit", orphanSHA, "main", "[]", "", true) |
| 64 | if err != nil { | 64 | if err != nil { |
| 65 | t.Fatal(err) | 65 | t.Fatal(err) |
| 66 | } | 66 | } |
| 67 | if err := st.SetCommitStatus(repo.ID, orphanSHA, "ci/unit", "pending", "queued", "https://x.test", uid); err != nil { | 67 | if err := st.SetCommitStatus(repo.ID, orphanSHA, "ci/unit", "pending", "queued", "https://x.test", uid); err != nil { |
| 68 | t.Fatal(err) | 68 | t.Fatal(err) |
| 69 | } | 69 | } |
| 70 | realID, err := st.CreateBuild(repo.ID, "unit", baseSHA, "main", "[]", true) | 70 | realID, err := st.CreateBuild(repo.ID, "unit", baseSHA, "main", "[]", "", true) |
| 71 | if err != nil { | 71 | if err != nil { |
| 72 | t.Fatal(err) | 72 | t.Fatal(err) |
| 73 | } | 73 | } |
| @@ -113,7 +113,7 @@ func TestRunnerNextSkipsOrphanedBuildAndClaimsNext(t *testing.T) { | |||
| 113 | // the reachability check must never reject a healthy build. | 113 | // the reachability check must never reject a healthy build. |
| 114 | func TestRunnerNextClaimsReachableBuildNormally(t *testing.T) { | 114 | func TestRunnerNextClaimsReachableBuildNormally(t *testing.T) { |
| 115 | st, repo, uid, root, baseSHA, _ := setupOrphanRepo(t) | 115 | st, repo, uid, root, baseSHA, _ := setupOrphanRepo(t) |
| 116 | id, err := st.CreateBuild(repo.ID, "unit", baseSHA, "main", "[]", true) | 116 | id, err := st.CreateBuild(repo.ID, "unit", baseSHA, "main", "[]", "", true) |
| 117 | if err != nil { | 117 | if err != nil { |
| 118 | t.Fatal(err) | 118 | t.Fatal(err) |
| 119 | } | 119 | } |
| @@ -147,7 +147,7 @@ func TestRunnerNextOrphanedQueuePastCapReportsNoPendingBuilds(t *testing.T) { | |||
| 147 | st, repo, uid, root, _, orphanSHA := setupOrphanRepo(t) | 147 | st, repo, uid, root, _, orphanSHA := setupOrphanRepo(t) |
| 148 | total := maxOrphanSkip + 1 | 148 | total := maxOrphanSkip + 1 |
| 149 | for i := 0; i < total; i++ { | 149 | for i := 0; i < total; i++ { |
| 150 | if _, err := st.CreateBuild(repo.ID, fmt.Sprintf("job%d", i), orphanSHA, "main", "[]", true); err != nil { | 150 | if _, err := st.CreateBuild(repo.ID, fmt.Sprintf("job%d", i), orphanSHA, "main", "[]", "", true); err != nil { |
| 151 | t.Fatal(err) | 151 | t.Fatal(err) |
| 152 | } | 152 | } |
| 153 | } | 153 | } |
| @@ -194,7 +194,7 @@ func TestRunnerNextClaimsBuildWhenReachabilityCannotBeChecked(t *testing.T) { | |||
| 194 | // No RepoDir created on disk at all: Reachable will fail to even stat | 194 | // No RepoDir created on disk at all: Reachable will fail to even stat |
| 195 | // the repository, which must not be read as "orphaned". | 195 | // the repository, which must not be read as "orphaned". |
| 196 | root := t.TempDir() | 196 | root := t.TempDir() |
| 197 | id, err := st.CreateBuild(repo.ID, "unit", strings.Repeat("a", 40), "main", "[]", true) | 197 | id, err := st.CreateBuild(repo.ID, "unit", strings.Repeat("a", 40), "main", "[]", "", true) |
| 198 | if err != nil { | 198 | if err != nil { |
| 199 | t.Fatal(err) | 199 | t.Fatal(err) |
| 200 | } | 200 | } |
internal/hookd/hookd.go +1 −1
| @@ -302,7 +302,7 @@ func (s *Server) queueTagBuilds(repo store.Repo, userID int64, tag, pushed strin | |||
| 302 | continue | 302 | continue |
| 303 | } | 303 | } |
| 304 | steps, _ := json.Marshal(j.Steps) | 304 | steps, _ := json.Marshal(j.Steps) |
| 305 | n, err := s.st.CreateBuild(repo.ID, j.Name, sha, tag, string(steps), true) | 305 | n, err := s.st.CreateBuild(repo.ID, j.Name, sha, tag, string(steps), j.Image, true) |
| 306 | if err != nil { | 306 | if err != nil { |
| 307 | slog.Error("queueing tag build", "repo", repo.Path(), "job", j.Name, "err", err) | 307 | slog.Error("queueing tag build", "repo", repo.Path(), "job", j.Name, "err", err) |
| 308 | continue | 308 | continue |
internal/store/builds.go +6 −5
| @@ -18,6 +18,7 @@ type Build struct { | |||
| 18 | SHA string | 18 | SHA string |
| 19 | Ref string | 19 | Ref string |
| 20 | Steps string // JSON array of shell commands | 20 | Steps string // JSON array of shell commands |
| 21 | Image string // container image for the steps; "" means the runner default | ||
| 21 | Status string // pending|running|success|failure | 22 | Status string // pending|running|success|failure |
| 22 | CreatedAt string | 23 | CreatedAt string |
| 23 | StartedAt string | 24 | StartedAt string |
| @@ -38,7 +39,7 @@ var truncNotice = []byte("\n[log truncated: reached the " + | |||
| 38 | 39 | ||
| 39 | // CreateBuild allocates the per-repo build number in the same transaction | 40 | // CreateBuild allocates the per-repo build number in the same transaction |
| 40 | // as the insert, like issue and MR numbers. | 41 | // as the insert, like issue and MR numbers. |
| 41 | func (s *Store) CreateBuild(repoID int64, job, sha, ref, stepsJSON string, trusted bool) (int64, error) { | 42 | func (s *Store) CreateBuild(repoID int64, job, sha, ref, stepsJSON, image string, trusted bool) (int64, error) { |
| 42 | tx, err := s.DB.Begin() | 43 | tx, err := s.DB.Begin() |
| 43 | if err != nil { | 44 | if err != nil { |
| 44 | return 0, err | 45 | return 0, err |
| @@ -52,21 +53,21 @@ func (s *Store) CreateBuild(repoID int64, job, sha, ref, stepsJSON string, trust | |||
| 52 | return 0, err | 53 | return 0, err |
| 53 | } | 54 | } |
| 54 | if _, err := tx.Exec( | 55 | if _, err := tx.Exec( |
| 55 | "INSERT INTO builds (repo_id, number, job, sha, ref, steps, trusted) VALUES (?, ?, ?, ?, ?, ?, ?)", | 56 | "INSERT INTO builds (repo_id, number, job, sha, ref, steps, image, trusted) VALUES (?, ?, ?, ?, ?, ?, ?, ?)", |
| 56 | repoID, n, job, sha, ref, stepsJSON, trusted); err != nil { | 57 | repoID, n, job, sha, ref, stepsJSON, image, trusted); err != nil { |
| 57 | return 0, err | 58 | return 0, err |
| 58 | } | 59 | } |
| 59 | return n, tx.Commit() | 60 | return n, tx.Commit() |
| 60 | } | 61 | } |
| 61 | 62 | ||
| 62 | const buildSelect = ` | 63 | const buildSelect = ` |
| 63 | SELECT id, repo_id, number, job, sha, ref, steps, status, created_at, started_at, finished_at, trusted | 64 | SELECT id, repo_id, number, job, sha, ref, steps, image, status, created_at, started_at, finished_at, trusted |
| 64 | FROM builds` | 65 | FROM builds` |
| 65 | 66 | ||
| 66 | func scanBuild(row interface{ Scan(...any) error }) (Build, error) { | 67 | func scanBuild(row interface{ Scan(...any) error }) (Build, error) { |
| 67 | var b Build | 68 | var b Build |
| 68 | var trusted int | 69 | var trusted int |
| 69 | err := row.Scan(&b.ID, &b.RepoID, &b.Number, &b.Job, &b.SHA, &b.Ref, &b.Steps, | 70 | err := row.Scan(&b.ID, &b.RepoID, &b.Number, &b.Job, &b.SHA, &b.Ref, &b.Steps, &b.Image, |
| 70 | &b.Status, &b.CreatedAt, &b.StartedAt, &b.FinishedAt, &trusted) | 71 | &b.Status, &b.CreatedAt, &b.StartedAt, &b.FinishedAt, &trusted) |
| 71 | b.Trusted = trusted != 0 | 72 | b.Trusted = trusted != 0 |
| 72 | return b, err | 73 | return b, err |
internal/store/builds_test.go +7 −7
| @@ -21,11 +21,11 @@ func TestReapStaleBuilds(t *testing.T) { | |||
| 21 | t.Fatal(err) | 21 | t.Fatal(err) |
| 22 | } | 22 | } |
| 23 | 23 | ||
| 24 | stuck, err := s.CreateBuild(1, "test", "abc123", "main", `["true"]`, true) | 24 | stuck, err := s.CreateBuild(1, "test", "abc123", "main", `["true"]`, "", true) |
| 25 | if err != nil { | 25 | if err != nil { |
| 26 | t.Fatal(err) | 26 | t.Fatal(err) |
| 27 | } | 27 | } |
| 28 | fresh, err := s.CreateBuild(1, "pages", "abc123", "main", `["true"]`, true) | 28 | fresh, err := s.CreateBuild(1, "pages", "abc123", "main", `["true"]`, "", true) |
| 29 | if err != nil { | 29 | if err != nil { |
| 30 | t.Fatal(err) | 30 | t.Fatal(err) |
| 31 | } | 31 | } |
| @@ -87,11 +87,11 @@ func TestBuildsForCommitTiming(t *testing.T) { | |||
| 87 | } | 87 | } |
| 88 | // Two runs of the same job on one commit: the retry is what counts. | 88 | // Two runs of the same job on one commit: the retry is what counts. |
| 89 | for range 2 { | 89 | for range 2 { |
| 90 | if _, err := s.CreateBuild(repoID, "test", "abc123", "main", `["true"]`, true); err != nil { | 90 | if _, err := s.CreateBuild(repoID, "test", "abc123", "main", `["true"]`, "", true); err != nil { |
| 91 | t.Fatal(err) | 91 | t.Fatal(err) |
| 92 | } | 92 | } |
| 93 | } | 93 | } |
| 94 | if _, err := s.CreateBuild(repoID, "lint", "def456", "main", `["true"]`, true); err != nil { | 94 | if _, err := s.CreateBuild(repoID, "lint", "def456", "main", `["true"]`, "", true); err != nil { |
| 95 | t.Fatal(err) | 95 | t.Fatal(err) |
| 96 | } | 96 | } |
| 97 | if _, err := s.DB.Exec(`UPDATE builds SET started_at = '2026-08-28T04:42:54Z', | 97 | if _, err := s.DB.Exec(`UPDATE builds SET started_at = '2026-08-28T04:42:54Z', |
| @@ -140,10 +140,10 @@ func TestClaimBuildScopedToRepos(t *testing.T) { | |||
| 140 | t.Fatal(err) | 140 | t.Fatal(err) |
| 141 | } | 141 | } |
| 142 | // Queued first, so an unscoped claim would take it. | 142 | // Queued first, so an unscoped claim would take it. |
| 143 | if _, err := s.CreateBuild(theirs, "evil", "abc123", "main", `["true"]`, true); err != nil { | 143 | if _, err := s.CreateBuild(theirs, "evil", "abc123", "main", `["true"]`, "", true); err != nil { |
| 144 | t.Fatal(err) | 144 | t.Fatal(err) |
| 145 | } | 145 | } |
| 146 | wanted, err := s.CreateBuild(mine, "deploy", "def456", "main", `["true"]`, true) | 146 | wanted, err := s.CreateBuild(mine, "deploy", "def456", "main", `["true"]`, "", true) |
| 147 | if err != nil { | 147 | if err != nil { |
| 148 | t.Fatal(err) | 148 | t.Fatal(err) |
| 149 | } | 149 | } |
| @@ -182,7 +182,7 @@ func TestBuildLogSaysWhenItTruncates(t *testing.T) { | |||
| 182 | if _, err := s.CreateRepo("user", uid, "orgo", "public"); err != nil { | 182 | if _, err := s.CreateRepo("user", uid, "orgo", "public"); err != nil { |
| 183 | t.Fatal(err) | 183 | t.Fatal(err) |
| 184 | } | 184 | } |
| 185 | id, err := s.CreateBuild(1, "test", "abc123", "main", `["true"]`, true) | 185 | id, err := s.CreateBuild(1, "test", "abc123", "main", `["true"]`, "", true) |
| 186 | if err != nil { | 186 | if err != nil { |
| 187 | t.Fatal(err) | 187 | t.Fatal(err) |
| 188 | } | 188 | } |
internal/store/migrations/0044_build_image.down.sql added +1
| @@ -0,0 +1 @@ | |||
| 1 | ALTER TABLE builds DROP COLUMN image; | ||
internal/store/migrations/0044_build_image.up.sql added +4
| @@ -0,0 +1,4 @@ | |||
| 1 | -- The container image a build's steps run in, captured when the build is | ||
| 2 | -- queued so it is the image the config named at that commit (#144). Empty | ||
| 3 | -- means the runner's configured default. | ||
| 4 | ALTER TABLE builds ADD COLUMN image TEXT NOT NULL DEFAULT ''; | ||
internal/store/statuses_test.go +1 −1
| @@ -17,7 +17,7 @@ func TestChecksForCommit(t *testing.T) { | |||
| 17 | if err != nil { | 17 | if err != nil { |
| 18 | t.Fatal(err) | 18 | t.Fatal(err) |
| 19 | } | 19 | } |
| 20 | if _, err := s.CreateBuild(repoID, "test", "abc123", "main", `["true"]`, true); err != nil { | 20 | if _, err := s.CreateBuild(repoID, "test", "abc123", "main", `["true"]`, "", true); err != nil { |
| 21 | t.Fatal(err) | 21 | t.Fatal(err) |
| 22 | } | 22 | } |
| 23 | if _, err := s.DB.Exec(`UPDATE builds SET started_at = '2026-08-28T04:42:54Z', | 23 | if _, err := s.DB.Exec(`UPDATE builds SET started_at = '2026-08-28T04:42:54Z', |