Commit 28f6758983
Verified · cmc
Layout: unified · split
cmd/gitbay-runner/env_test.go +34 −13
| @@ -190,26 +190,47 @@ func TestSplitEnvKeepsMultilineOutOfTheFile(t *testing.T) { | |||
| 190 | } | 190 | } |
| 191 | } | 191 | } |
| 192 | 192 | ||
| 193 | // A build that talks back to the instance — releases, comments — needs an | 193 | // A build that talks back to the instance needs an address that works |
| 194 | // address that works from where it runs. GITBAY_SSH carries the runner's | 194 | // from where it runs. A runner polling over loopback keeps its podman |
| 195 | // remote; under podman a loopback remote is rewritten to the address at | 195 | // builds off the host's loopback, so they get the instance's public |
| 196 | // which pasta exposes the host, since the host's own addresses belong to | 196 | // destination from the claim, port included when it is not 22; any other |
| 197 | // the container inside it. | 197 | // remote is used as it is (#260). |
| 198 | func TestStepEnvCarriesInstanceAddress(t *testing.T) { | 198 | func TestStepEnvCarriesInstanceAddress(t *testing.T) { |
| 199 | env := stepEnv(job{}, "/tmp/buildhome", "git@gitbay.org") | 199 | env := stepEnv(job{}, "/tmp/buildhome", "git@gitbay.org") |
| 200 | if !containsEnv(env, "GITBAY_SSH=git@gitbay.org") { | 200 | if !containsEnv(env, "GITBAY_SSH=git@gitbay.org") { |
| 201 | t.Errorf("GITBAY_SSH missing: %q", env) | 201 | t.Errorf("GITBAY_SSH missing: %q", env) |
| 202 | } | 202 | } |
| 203 | for _, tc := range []struct{ remote, isolation, want string }{ | 203 | for _, tc := range []struct{ remote, isolation, public, want string }{ |
| 204 | {"git@127.0.0.1", isolationNone, "git@127.0.0.1"}, | 204 | {"git@127.0.0.1", isolationNone, "git@gitbay.org", "git@127.0.0.1"}, |
| 205 | {"git@127.0.0.1", isolationPodman, "git@169.254.1.2"}, | 205 | {"git@127.0.0.1", isolationPodman, "git@gitbay.org", "git@gitbay.org"}, |
| 206 | {"git@localhost", isolationPodman, "git@169.254.1.2"}, | 206 | {"git@127.0.0.1", isolationPodman, "git@gitbay.test:2022", "git@gitbay.test:2022"}, |
| 207 | {"git@gitbay.org", isolationPodman, "git@gitbay.org"}, | 207 | {"git@localhost", isolationPodman, "git@gitbay.org", "git@gitbay.org"}, |
| 208 | {"gitbay.org", isolationPodman, "gitbay.org"}, | 208 | {"git@127.0.0.1", isolationPodman, "", "git@127.0.0.1"}, |
| 209 | {"git@gitbay.org", isolationPodman, "git@other.test", "git@gitbay.org"}, | ||
| 210 | {"gitbay.org", isolationPodman, "git@gitbay.org", "gitbay.org"}, | ||
| 209 | } { | 211 | } { |
| 210 | r := &runner{remote: tc.remote, isolation: tc.isolation} | 212 | r := &runner{remote: tc.remote, isolation: tc.isolation} |
| 211 | if got := r.buildSSH(); got != tc.want { | 213 | if got := r.buildSSH(tc.public); got != tc.want { |
| 212 | t.Errorf("remote %s under %s: got %s want %s", tc.remote, tc.isolation, got, tc.want) | 214 | t.Errorf("remote %s under %s, public %q: got %s want %s", tc.remote, tc.isolation, tc.public, got, tc.want) |
| 215 | } | ||
| 216 | } | ||
| 217 | } | ||
| 218 | |||
| 219 | // Only a runner that polls over loopback shares an address a build could | ||
| 220 | // connect from, so only its builds lose the host-loopback mapping (#260). | ||
| 221 | func TestBuildNetworkKeepsLoopbackRunnersBuildsOff(t *testing.T) { | ||
| 222 | for _, tc := range []struct { | ||
| 223 | remote string | ||
| 224 | want []string | ||
| 225 | }{ | ||
| 226 | {"git@127.0.0.1", []string{"--network", "pasta:--no-map-gw"}}, | ||
| 227 | {"localhost", []string{"--network", "pasta:--no-map-gw"}}, | ||
| 228 | {"git@::1", []string{"--network", "pasta:--no-map-gw"}}, | ||
| 229 | {"git@gitbay.org", nil}, | ||
| 230 | } { | ||
| 231 | r := &runner{remote: tc.remote, isolation: isolationPodman} | ||
| 232 | if got := r.buildNetwork(); strings.Join(got, " ") != strings.Join(tc.want, " ") { | ||
| 233 | t.Errorf("remote %s: %q, want %q", tc.remote, got, tc.want) | ||
| 213 | } | 234 | } |
| 214 | } | 235 | } |
| 215 | } | 236 | } |
cmd/gitbay-runner/isolate.go +1
| @@ -151,6 +151,7 @@ func (r *runner) runStepsPodman(j job, dir string, env []string, sink io.Writer, | |||
| 151 | args = append(args, | 151 | args = append(args, |
| 152 | "--name", name, | 152 | "--name", name, |
| 153 | "--env-file", envFile) | 153 | "--env-file", envFile) |
| 154 | args = append(args, r.buildNetwork()...) | ||
| 154 | args = append(args, inheritArgs(inherit)...) | 155 | args = append(args, inheritArgs(inherit)...) |
| 155 | args = append(args, | 156 | args = append(args, |
| 156 | "--volume", dir+":/workspace:rw", | 157 | "--volume", dir+":/workspace:rw", |
cmd/gitbay-runner/main.go +43 −23
| @@ -42,7 +42,10 @@ type job struct { | |||
| 42 | // Trusted is false for a merge request head from a fork, and when the | 42 | // Trusted is false for a merge request head from a fork, and when the |
| 43 | // server did not say: such a build gets no secrets and a home of its | 43 | // server did not say: such a build gets no secrets and a home of its |
| 44 | // own (#255). | 44 | // own (#255). |
| 45 | Trusted bool `json:"trusted"` | 45 | Trusted bool `json:"trusted"` |
| 46 | // SSH is the instance's public ssh destination, for a build whose | ||
| 47 | // runner polls over loopback (#260). | ||
| 48 | SSH string `json:"ssh"` | ||
| 46 | Secrets map[string]string `json:"secrets"` | 49 | Secrets map[string]string `json:"secrets"` |
| 47 | } | 50 | } |
| 48 | 51 | ||
| @@ -425,7 +428,7 @@ func (r *runner) run(j job) bool { | |||
| 425 | } | 428 | } |
| 426 | } | 429 | } |
| 427 | 430 | ||
| 428 | env := stepEnv(j, home, r.buildSSH()) | 431 | env := stepEnv(j, home, r.buildSSH(j.SSH)) |
| 429 | return r.runSteps(j, dir, env, sink, deadline, runStep) | 432 | return r.runSteps(j, dir, env, sink, deadline, runStep) |
| 430 | } | 433 | } |
| 431 | 434 | ||
| @@ -483,27 +486,44 @@ func removeTree(dir string) error { | |||
| 483 | return os.RemoveAll(dir) | 486 | return os.RemoveAll(dir) |
| 484 | } | 487 | } |
| 485 | 488 | ||
| 486 | // buildSSH is the instance's ssh destination as a build reaches it. Under | 489 | // loopbackRemote reports whether the runner polls the daemon on its own |
| 487 | // podman, pasta gives the container the host's own addresses, so a | 490 | // host over loopback. |
| 488 | // loopback remote — the runner on the server itself — is unreachable by | 491 | func (r *runner) loopbackRemote() bool { |
| 489 | // that name; pasta exposes the host at 169.254.1.2, its | 492 | _, host, ok := strings.Cut(r.remote, "@") |
| 490 | // --map-host-loopback default. Any other remote is a real host elsewhere | 493 | if !ok { |
| 491 | // and works as it is. | 494 | host = r.remote |
| 492 | func (r *runner) buildSSH() string { | 495 | } |
| 493 | if r.isolation != isolationPodman { | 496 | return host == "127.0.0.1" || host == "localhost" || host == "::1" |
| 494 | return r.remote | 497 | } |
| 495 | } | 498 | |
| 496 | user, host, hasUser := strings.Cut(r.remote, "@") | 499 | // buildSSH is the instance's ssh destination as a build reaches it. A |
| 497 | if !hasUser { | 500 | // runner polling over loopback keeps its podman builds off the host's |
| 498 | user, host = "", user | 501 | // loopback (buildNetwork), so they get the instance's public destination |
| 499 | } | 502 | // from the claim. Any other remote is a real host elsewhere and works as |
| 500 | if host != "127.0.0.1" && host != "localhost" && host != "::1" { | 503 | // it is, and under -isolation none a build runs on the host itself. |
| 501 | return r.remote | 504 | func (r *runner) buildSSH(public string) string { |
| 502 | } | 505 | if r.isolation == isolationPodman && r.loopbackRemote() && public != "" { |
| 503 | if hasUser { | 506 | return public |
| 504 | return user + "@169.254.1.2" | 507 | } |
| 505 | } | 508 | return r.remote |
| 506 | return "169.254.1.2" | 509 | } |
| 510 | |||
| 511 | // buildNetwork is the podman network option for a build. pasta maps the | ||
| 512 | // container's gateway address to the host's loopback, and a build's | ||
| 513 | // connection through it arrives from 127.0.0.1 — the address a runner on | ||
| 514 | // the daemon's host polls from. The SSH auth limiter counts failures per | ||
| 515 | // source address, so a build sharing the runner's could throttle its | ||
| 516 | // polling (#260). --no-map-gw removes the mapping: the build reaches the | ||
| 517 | // host only at its public address, as any client on the internet does, | ||
| 518 | // and keeps its outbound access. The host's nftables table | ||
| 519 | // (deploy/gitbay-runner-egress.nft) then limits it to 22, 80 and 443 | ||
| 520 | // there; it cannot tell a build from the runner by uid, so it leaves | ||
| 521 | // 127.0.0.1:22 open, and this flag is what keeps builds off it. | ||
| 522 | func (r *runner) buildNetwork() []string { | ||
| 523 | if !r.loopbackRemote() { | ||
| 524 | return nil | ||
| 525 | } | ||
| 526 | return []string{"--network", "pasta:--no-map-gw"} | ||
| 507 | } | 527 | } |
| 508 | 528 | ||
| 509 | // stepEnv builds the environment a build step runs with. It is | 529 | // stepEnv builds the environment a build step runs with. It is |