Commit 44c2c11f58
Verified · cmc ci/build: success ci/test: failure
Layout: unified · split
.gitbay/wiki/Admin.org +19 −4
| @@ -432,10 +432,25 @@ The tag is deliberate rather than =:latest=: changing the file means | ||
| 432 | 432 | bumping the tag in =.gitbay/ci.yml=, so a running branch's image does not |
| 433 | 433 | change under it. |
| 434 | 434 | |
| 435 | =-image= sets the default image for jobs that name none | |
| 436 | (=docker.io/library/debian:stable-slim= if unset); a job overrides it | |
| 437 | with =image:= in =.gitbay/ci.yml=, validated as a reference so a config | |
| 438 | file cannot turn it into podman arguments. | |
| 435 | =-image= names the image a job runs in when it declares none, and is | |
| 436 | required under =-isolation podman=: there is no built-in default, | |
| 437 | because an image this host does not have would fail every build. A job | |
| 438 | overrides it with =image:= in =.gitbay/ci.yml=, validated as a reference | |
| 439 | so a config file cannot turn it into podman arguments. | |
| 440 | ||
| 441 | *Images are provisioned, never pulled by a build.* The runner passes | |
| 442 | =--pull=never=. Two reasons, and the second is the better one: the | |
| 443 | service runs with =RestrictSUIDSGID=yes= so podman cannot unpack a layer | |
| 444 | holding a setuid file, which is nearly every distribution image; and on | |
| 445 | an instance where anyone can push a =ci.yml=, =image:= would otherwise | |
| 446 | mean "fetch and run anything from the internet". An operator pulls or | |
| 447 | builds what is allowed and a build picks among those. A job naming an | |
| 448 | image the host does not have fails with a message saying so. | |
| 449 | ||
| 450 | #+begin_src sh | |
| 451 | su - ci-runner -s /bin/sh -c "podman pull docker.io/library/alpine:3.20" | |
| 452 | su - ci-runner -s /bin/sh -c "podman images" | |
| 453 | #+end_src | |
| 439 | 454 | |
| 440 | 455 | Prepare a host before pointing an isolating runner at it: |
| 441 | 456 | |
.gitbay/wiki/Threat-Model.org +5
| @@ -154,6 +154,11 @@ runner, polling over SSH, clones the commit and runs its steps. | ||
| 154 | 154 | arbitrary repository code, and under podman that code no longer runs in |
| 155 | 155 | the runner's process context. Under =-isolation none= there is no |
| 156 | 156 | container and the flag should be on. |
| 157 | - *Images are provisioned by the operator, not fetched by a build.* The | |
| 158 | runner passes =--pull=never=, so =image:= chooses among what the host | |
| 159 | already has rather than naming anything on the internet. On an | |
| 160 | instance with open registration that is the difference between a | |
| 161 | curated set and arbitrary code from a registry nobody vetted. | |
| 157 | 162 | |
| 158 | 163 | Under =-isolation none=, anything a step can do as the runner's user a |
| 159 | 164 | pushed =ci.yml= can do. Under podman a step is confined to its |
cmd/gitbay-runner/env_test.go +2 −1
| @@ -93,7 +93,8 @@ func TestStepEnvHomeIsNotTheWorkspace(t *testing.T) { | ||
| 93 | 93 | // has no user slice to work in. Every invocation must say so, or crun |
| 94 | 94 | // fails creating the container's scope (#144). |
| 95 | 95 | func TestPodmanUsesCgroupfs(t *testing.T) { |
| 96 | got := podmanGlobal() | |
| 96 | r := &runner{} | |
| 97 | got := r.podmanGlobal() | |
| 97 | 98 | found := false |
| 98 | 99 | for _, f := range got { |
| 99 | 100 | if f == "--cgroup-manager=cgroupfs" { |
cmd/gitbay-runner/isolate.go +50 −15
| @@ -21,10 +21,21 @@ const ( | ||
| 21 | 21 | isolationNone = "none" |
| 22 | 22 | ) |
| 23 | 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" | |
| 24 | // Images are provisioned, never pulled at build time. | |
| 25 | // | |
| 26 | // The service runs with RestrictSUIDSGID=yes, so podman cannot unpack an | |
| 27 | // image layer containing a setuid or setgid file — which is almost every | |
| 28 | // distribution image (chage, passwd, su). A pull from inside the service | |
| 29 | // fails deep in the unpack with "operation not permitted" on some file | |
| 30 | // nobody has heard of. | |
| 31 | // | |
| 32 | // Keeping that flag and provisioning images deliberately is the better | |
| 33 | // half of the trade, and not only because it is one less hardening | |
| 34 | // concession: on an instance where anyone can push a ci.yml, `image:` | |
| 35 | // would otherwise be "fetch and run this arbitrary image from the | |
| 36 | // internet". An operator pulls or builds what they will allow, and a | |
| 37 | // build chooses among those. --pull=never makes that explicit rather | |
| 38 | // than leaving it to whether a pull happens to fail (#144). | |
| 28 | 39 | |
| 29 | 40 | // checkIsolation fails the runner at start-up rather than at the first |
| 30 | 41 | // build, and refuses anything it does not recognise. There is no silent |
| @@ -37,6 +48,19 @@ func (r *runner) checkIsolation() error { | ||
| 37 | 48 | "with no container. Only do this where every repository is trusted.", currentUser()) |
| 38 | 49 | return nil |
| 39 | 50 | case isolationPodman: |
| 51 | // Configuration before environment: a missing -image is the | |
| 52 | // operator's to fix whatever the host looks like, and saying so | |
| 53 | // first means the message does not depend on which machine this | |
| 54 | // is. | |
| 55 | // | |
| 56 | // No built-in default image: one that is not provisioned here | |
| 57 | // would fail every build with --pull=never, and guessing which | |
| 58 | // image an operator has is worse than asking. | |
| 59 | if r.image == "" { | |
| 60 | return fmt.Errorf("-isolation podman needs -image <ref>, the image a job runs in " + | |
| 61 | "when it names none. It must already be present on this host: " + | |
| 62 | "pull or build it as the runner's user, since the service cannot unpack images") | |
| 63 | } | |
| 40 | 64 | bin := toolpath.Look("podman") |
| 41 | 65 | out, err := exec.Command(bin, "info", "--format", "{{.Host.Security.Rootless}}").CombinedOutput() |
| 42 | 66 | if err != nil { |
| @@ -44,10 +68,7 @@ func (r *runner) checkIsolation() error { | ||
| 44 | 68 | "prepare the host with deploy/runner-podman-setup.sh, or pass -isolation none "+ |
| 45 | 69 | "if every repository on this instance is trusted", err, strings.TrimSpace(string(out))) |
| 46 | 70 | } |
| 47 | if r.image == "" { | |
| 48 | r.image = defaultImage | |
| 49 | } | |
| 50 | log.Printf("isolation: podman (rootless=%s), default image %s", | |
| 71 | log.Printf("isolation: podman (rootless=%s), default image %s, images must be provisioned locally", | |
| 51 | 72 | strings.TrimSpace(string(out)), r.image) |
| 52 | 73 | return nil |
| 53 | 74 | default: |
| @@ -104,7 +125,8 @@ func (r *runner) runStepsPodman(j job, dir string, env []string, sink io.Writer, | ||
| 104 | 125 | name := fmt.Sprintf("gitbay-build-%d", j.ID) |
| 105 | 126 | // --rm so a container cannot outlive its build; the explicit rm below |
| 106 | 127 | // covers the case where the daemon-less run itself fails. |
| 107 | start := exec.Command(podman, append(podmanGlobal(), "run", "--detach", "--rm", | |
| 128 | start := exec.Command(podman, append(r.podmanGlobal(), "run", "--detach", "--rm", | |
| 129 | "--pull=never", | |
| 108 | 130 | "--name", name, |
| 109 | 131 | "--env-file", envFile, |
| 110 | 132 | "--volume", dir+":/workspace:rw", |
| @@ -113,16 +135,24 @@ func (r *runner) runStepsPodman(j job, dir string, env []string, sink io.Writer, | ||
| 113 | 135 | image, "-c", "sleep infinity")...) |
| 114 | 136 | start.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + r.podmanHome()} |
| 115 | 137 | 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))) | |
| 138 | // A missing image lands here, and it is the common case worth | |
| 139 | // explaining: this runner never pulls, so an image it does not | |
| 140 | // have is an operator's job to provision, not a transient error | |
| 141 | // to retry. | |
| 142 | msg := strings.TrimSpace(string(out)) | |
| 143 | fmt.Fprintf(sink, "starting the build container from %s failed:\n%s\n", image, msg) | |
| 144 | if strings.Contains(msg, "no such image") || strings.Contains(msg, "image not known") || | |
| 145 | strings.Contains(msg, "unable to find") { | |
| 146 | fmt.Fprintf(sink, "\nThis runner does not pull images. Ask an operator to provision %s "+ | |
| 147 | "on the runner host (podman pull, or podman build) before a job names it.\n", image) | |
| 148 | } | |
| 119 | 149 | return false |
| 120 | 150 | } |
| 121 | defer exec.Command(podman, append(podmanGlobal(), "rm", "--force", name)...).Run() | |
| 151 | defer exec.Command(podman, append(r.podmanGlobal(), "rm", "--force", name)...).Run() | |
| 122 | 152 | |
| 123 | 153 | for _, step := range j.Steps { |
| 124 | 154 | fmt.Fprintf(sink, "$ %s\n", step) |
| 125 | cmd := exec.Command(podman, append(podmanGlobal(), "exec", "--workdir", "/workspace", name, "sh", "-c", step)...) | |
| 155 | cmd := exec.Command(podman, append(r.podmanGlobal(), "exec", "--workdir", "/workspace", name, "sh", "-c", step)...) | |
| 126 | 156 | cmd.Env = []string{"PATH=" + os.Getenv("PATH"), "HOME=" + r.podmanHome()} |
| 127 | 157 | cmd.Stdout, cmd.Stderr = sink, sink |
| 128 | 158 | if ok, why := runStep(cmd, deadline); !ok { |
| @@ -142,7 +172,12 @@ func (r *runner) runStepsPodman(j job, dir string, env []string, sink io.Writer, | ||
| 142 | 172 | // fails with "create directory .../libpod-<id>.scope/container: No such |
| 143 | 173 | // file or directory". The service's own cgroup is delegated |
| 144 | 174 | // (Delegate=yes in the drop-in), which is what cgroupfs needs (#144). |
| 145 | func podmanGlobal() []string { | |
| 175 | func (r *runner) podmanGlobal() []string { | |
| 176 | // Storage paths are left to podman. They are recorded in its | |
| 177 | // database at first use, so passing --root or --runroot later fails | |
| 178 | // with "database configuration mismatch" — as does introducing an | |
| 179 | // XDG_RUNTIME_DIR the database was not initialised with. Changing | |
| 180 | // either means `podman system reset` and rebuilding the images. | |
| 146 | 181 | return []string{"--cgroup-manager=cgroupfs"} |
| 147 | 182 | } |
| 148 | 183 | |
deploy/gitbay-runner.override.conf +12 −5
| @@ -17,16 +17,23 @@ | ||
| 17 | 17 | # and read-only git, and the sandboxing below keeps a step from |
| 18 | 18 | # touching the system outside its workspace. |
| 19 | 19 | # |
| 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 | |
| 20 | # Delegate=yes and the storage path below are what rootless podman needs | |
| 25 | 21 | # (#144): it manages its own cgroups for a container, and its image and |
| 26 | 22 | # container store lives under the runner's home, which ProtectSystem |
| 27 | 23 | # would otherwise make read-only. Prepare the host with |
| 28 | 24 | # deploy/runner-podman-setup.sh before deploying a runner that isolates. |
| 29 | 25 | [Service] |
| 26 | # ExecStart is overridden here rather than left in the unit so the flags | |
| 27 | # and the sandboxing that has to match them live in one file: -isolation | |
| 28 | # podman needs NoNewPrivileges=no below, and -image needs an image the | |
| 29 | # host has been given (deploy/runner-podman-setup.sh, Containerfile.ci). | |
| 30 | # Deliberately no XDG_RUNTIME_DIR. podman records its run root in its | |
| 31 | # database at first use, so setting one later fails with "database | |
| 32 | # configuration mismatch"; the runner's storage was initialised without | |
| 33 | # it and works. Change it only together with `podman system reset` and a | |
| 34 | # rebuild of the images (#144). | |
| 35 | ExecStart= | |
| 36 | ExecStart=/usr/local/bin/gitbay-runner -remote git@127.0.0.1 -workdir /var/lib/gitbay-runner/work -poll 5s -timeout 45m -repos krz/gitbay -isolation podman -image localhost/gitbay-ci:1 | |
| 30 | 37 | Nice=10 |
| 31 | 38 | CPUWeight=30 |
| 32 | 39 | IOWeight=30 |
e2e/isolation_podman_test.go +50 −8
| @@ -12,6 +12,22 @@ import ( | ||
| 12 | 12 | // havePodman reports whether a working rootless podman is on this |
| 13 | 13 | // machine. The skip is loud on purpose: an isolation test that quietly |
| 14 | 14 | // does not run is how isolation regresses (#144). |
| 15 | // provisionedImage returns an image present on this host, since the | |
| 16 | // runner never pulls one (#144). Tests must use what is provisioned, the | |
| 17 | // same rule builds follow. | |
| 18 | func provisionedImage(t *testing.T) string { | |
| 19 | t.Helper() | |
| 20 | for _, img := range []string{"localhost/gitbay-ci:1", "docker.io/library/debian:stable-slim", "docker.io/library/alpine:latest"} { | |
| 21 | if err := exec.Command("podman", "image", "exists", img).Run(); err == nil { | |
| 22 | return img | |
| 23 | } | |
| 24 | } | |
| 25 | t.Log("SKIPPING ISOLATION TEST: podman has no image this test can use. " + | |
| 26 | "Provision one (podman build -t localhost/gitbay-ci:1 -f deploy/Containerfile.ci). " + | |
| 27 | "The container path is NOT covered by this run.") | |
| 28 | return "" | |
| 29 | } | |
| 30 | ||
| 15 | 31 | func havePodman(t *testing.T) bool { |
| 16 | 32 | t.Helper() |
| 17 | 33 | if _, err := exec.LookPath("podman"); err != nil { |
| @@ -32,7 +48,7 @@ func havePodman(t *testing.T) bool { | ||
| 32 | 48 | func TestRunnerRefusesToStartWithoutPodman(t *testing.T) { |
| 33 | 49 | bin := buildRunner(t) |
| 34 | 50 | cmd := exec.Command(bin, "-once", "-remote", "git@127.0.0.1", |
| 35 | "-isolation", "podman", "-workdir", t.TempDir()) | |
| 51 | "-isolation", "podman", "-image", "localhost/whatever:1", "-workdir", t.TempDir()) | |
| 36 | 52 | // An empty PATH is the reliable way to make podman missing whether or |
| 37 | 53 | // not this machine has one. |
| 38 | 54 | cmd.Env = []string{"PATH=" + t.TempDir(), "HOME=" + t.TempDir()} |
| @@ -76,6 +92,10 @@ func TestPodmanStepCannotReachTheRunnersKey(t *testing.T) { | ||
| 76 | 92 | inst.admin(t, "admin", "user", "create", "ci", "--key", runnerKey+".pub", "--admin") |
| 77 | 93 | inst.ssh(t, aliceKey, "", "repo", "create", "alice/app") |
| 78 | 94 | |
| 95 | image := provisionedImage(t) | |
| 96 | if image == "" { | |
| 97 | t.Skip("no provisioned image") | |
| 98 | } | |
| 79 | 99 | env := inst.gitEnv(aliceKey) |
| 80 | 100 | work := t.TempDir() |
| 81 | 101 | mustGit(t, work, env, "clone", inst.sshURL("alice/app"), "w") |
| @@ -84,7 +104,7 @@ func TestPodmanStepCannotReachTheRunnersKey(t *testing.T) { | ||
| 84 | 104 | // The step tries to read the key the runner authenticates with, and |
| 85 | 105 | // to list the runner's home. Both must fail inside the container. |
| 86 | 106 | os.WriteFile(filepath.Join(dir, ".gitbay", "ci.yml"), []byte( |
| 87 | "jobs:\n peek:\n image: docker.io/library/debian:stable-slim\n steps:\n"+ | |
| 107 | "jobs:\n peek:\n image: "+image+"\n steps:\n"+ | |
| 88 | 108 | " - 'if cat "+runnerKey+" 2>/dev/null; then echo LEAKED-KEY; exit 1; fi; echo no-key'\n"+ |
| 89 | 109 | " - 'echo HOME=$HOME; ls /workspace'\n"), 0o644) |
| 90 | 110 | mustGit(t, dir, env, "checkout", "-q", "-b", "main") |
| @@ -94,10 +114,12 @@ func TestPodmanStepCannotReachTheRunnersKey(t *testing.T) { | ||
| 94 | 114 | |
| 95 | 115 | runnerPodmanOnce(t, inst, runnerKey) |
| 96 | 116 | out, _, _ := inst.ssh(t, aliceKey, "", "build", "list", "alice/app") |
| 117 | log, _, _ := inst.ssh(t, aliceKey, "", "build", "log", "alice/app", "1") | |
| 97 | 118 | if !strings.Contains(out, "success") { |
| 98 | t.Fatalf("the containerised build did not pass:\n%s", out) | |
| 119 | // Without the log this says only "it failed", which cost two CI | |
| 120 | // rounds to diagnose the first time. | |
| 121 | t.Fatalf("the containerised build did not pass:\n%s\nbuild log:\n%s", out, log) | |
| 99 | 122 | } |
| 100 | log, _, _ := inst.ssh(t, aliceKey, "", "build", "log", "alice/app", "1") | |
| 101 | 123 | if strings.Contains(log, "LEAKED-KEY") { |
| 102 | 124 | t.Errorf("a step read the runner's ssh key:\n%s", log) |
| 103 | 125 | } |
| @@ -106,9 +128,9 @@ func TestPodmanStepCannotReachTheRunnersKey(t *testing.T) { | ||
| 106 | 128 | } |
| 107 | 129 | } |
| 108 | 130 | |
| 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) { | |
| 131 | // An image this runner does not have fails the build and says an | |
| 132 | // operator must provision it, rather than pulling it. | |
| 133 | func TestPodmanMissingImageFailsTheBuild(t *testing.T) { | |
| 112 | 134 | if !havePodman(t) { |
| 113 | 135 | t.Skip("no podman") |
| 114 | 136 | } |
| @@ -139,7 +161,10 @@ func TestPodmanPullFailureFailsTheBuild(t *testing.T) { | ||
| 139 | 161 | } |
| 140 | 162 | log, _, _ := inst.ssh(t, aliceKey, "", "build", "log", "alice/app", "1") |
| 141 | 163 | 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) | |
| 164 | t.Errorf("the log does not name the missing image:\n%s", log) | |
| 165 | } | |
| 166 | if !strings.Contains(log, "does not pull images") { | |
| 167 | t.Errorf("the log does not say an operator must provision it:\n%s", log) | |
| 143 | 168 | } |
| 144 | 169 | if strings.Contains(log, "unreachable") { |
| 145 | 170 | t.Error("a step ran despite the image failing to start") |
| @@ -154,6 +179,7 @@ func runnerPodmanOnce(t *testing.T, inst *instance, key string) { | ||
| 154 | 179 | "-remote", "git@127.0.0.1", |
| 155 | 180 | "-ssh-opts", opts, |
| 156 | 181 | "-isolation", "podman", |
| 182 | "-image", "localhost/gitbay-ci:1", | |
| 157 | 183 | "-clone-base", fmt.Sprintf("ssh://git@127.0.0.1:%d", inst.port), |
| 158 | 184 | "-workdir", t.TempDir()) |
| 159 | 185 | cmd.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null") |
| @@ -161,3 +187,19 @@ func runnerPodmanOnce(t *testing.T, inst *instance, key string) { | ||
| 161 | 187 | t.Fatalf("runner: %v\n%s", err, out) |
| 162 | 188 | } |
| 163 | 189 | } |
| 190 | ||
| 191 | // Under podman the runner insists on a default image rather than | |
| 192 | // guessing one: with --pull=never an image the host does not have fails | |
| 193 | // every job that names none. | |
| 194 | func TestRunnerRefusesPodmanWithoutAnImage(t *testing.T) { | |
| 195 | bin := buildRunner(t) | |
| 196 | cmd := exec.Command(bin, "-once", "-remote", "git@127.0.0.1", | |
| 197 | "-isolation", "podman", "-workdir", t.TempDir()) | |
| 198 | out, err := cmd.CombinedOutput() | |
| 199 | if err == nil { | |
| 200 | t.Fatalf("the runner started in podman mode with no -image:\n%s", out) | |
| 201 | } | |
| 202 | if !strings.Contains(string(out), "-image") { | |
| 203 | t.Errorf("refusal does not name the missing flag:\n%s", out) | |
| 204 | } | |
| 205 | } | |