Commit 79e1555258
Verified · cmc ci/build: success ci/test: success
cmd/gitbayd/system.go +1 −1
| @@ -93,7 +93,7 @@ func shellCmd() *cobra.Command { | ||
| 93 | 93 | fmt.Fprintf(os.Stderr, "gitbay control plane: interactive shells are not available.\nTry: ssh <host> help\n") |
| 94 | 94 | os.Exit(protocol.ExitUsage) |
| 95 | 95 | } |
| 96 | code := sshd.Exec(cfg, st, user, key.Scope, key.Fingerprint, cmdline, os.Stdin, os.Stdout, os.Stderr, nil) | |
| 96 | code := sshd.Exec(cfg, st, user, key.Scope, key.Fingerprint, cmdline, os.Stdin, os.Stdout, os.Stderr, nil, nil) | |
| 97 | 97 | st.Close() |
| 98 | 98 | os.Exit(code) |
| 99 | 99 | return nil |
e2e/shutdown_test.go +1 −1
| @@ -134,7 +134,7 @@ func TestShutdownEndsFollows(t *testing.T) { | ||
| 134 | 134 | if err := inst.proc.Process.Signal(syscall.SIGTERM); err != nil { |
| 135 | 135 | t.Fatal(err) |
| 136 | 136 | } |
| 137 | web.waitFor(t, "gitbay is restarting; reload in a moment") | |
| 137 | web.waitFor(t, `<p class="notice" role="status">gitbay is restarting`) | |
| 138 | 138 | web.waitFor(t, "</html>") |
| 139 | 139 | var exit *exec.ExitError |
| 140 | 140 | if err := cmd.Wait(); !errors.As(err, &exit) || exit.ExitCode() != 1 { |
internal/control/buildfollow.go +31 −5
| @@ -62,7 +62,8 @@ func dropFollow(uid int64) { | ||
| 62 | 62 | |
| 63 | 63 | // mayStillRead reports whether the follower can still read the build's |
| 64 | 64 | // repository. It looks the repository up by id, so a rename mid-follow |
| 65 | // does not end the follow. | |
| 65 | // does not end the follow, and reloads the account, so disabling it | |
| 66 | // does. | |
| 66 | 67 | func mayStillRead(c *Ctx, repoID int64) (bool, error) { |
| 67 | 68 | repo, err := c.Store.RepoByID(repoID) |
| 68 | 69 | if errors.Is(err, store.ErrNotFound) { |
| @@ -71,11 +72,36 @@ func mayStillRead(c *Ctx, repoID int64) (bool, error) { | ||
| 71 | 72 | if err != nil { |
| 72 | 73 | return false, err |
| 73 | 74 | } |
| 74 | grant, err := c.Store.AccessRole(repo.ID, c.User.ID) | |
| 75 | u := c.User | |
| 76 | if u.ID != 0 { | |
| 77 | u, err = c.Store.UserByID(u.ID) | |
| 78 | if errors.Is(err, store.ErrNotFound) { | |
| 79 | return false, nil | |
| 80 | } | |
| 81 | if err != nil { | |
| 82 | return false, err | |
| 83 | } | |
| 84 | if u.Disabled { | |
| 85 | return false, nil | |
| 86 | } | |
| 87 | } | |
| 88 | grant, err := c.Store.AccessRole(repo.ID, u.ID) | |
| 75 | 89 | if err != nil { |
| 76 | 90 | return false, err |
| 77 | 91 | } |
| 78 | return policy.CanRead(c.User, repo, grant), nil | |
| 92 | return policy.CanRead(u, repo, grant), nil | |
| 93 | } | |
| 94 | ||
| 95 | // ended is what a follow returns when its Done closes. A restart says | |
| 96 | // so, whatever the surface, so the reader knows to follow again; a | |
| 97 | // reader who left hears nothing. | |
| 98 | func ended(c *Ctx) int { | |
| 99 | select { | |
| 100 | case <-c.Stopping: | |
| 101 | fmt.Fprintln(c.Stderr, "gitbay is restarting; follow the build again in a moment") | |
| 102 | default: | |
| 103 | } | |
| 104 | return protocol.ExitFailure | |
| 79 | 105 | } |
| 80 | 106 | |
| 81 | 107 | // followBuildLog writes the build's log as it grows and returns once the |
| @@ -150,12 +176,12 @@ func followBuildLog(c *Ctx, repo store.Repo, b store.Build) int { | ||
| 150 | 176 | case <-t.C: |
| 151 | 177 | case <-c.Done: |
| 152 | 178 | t.Stop() |
| 153 | return protocol.ExitFailure | |
| 179 | return ended(c) | |
| 154 | 180 | } |
| 155 | 181 | case <-t.C: |
| 156 | 182 | case <-c.Done: |
| 157 | 183 | t.Stop() |
| 158 | return protocol.ExitFailure | |
| 184 | return ended(c) | |
| 159 | 185 | } |
| 160 | 186 | t.Stop() |
| 161 | 187 | } |
internal/control/buildfollow_test.go +58 −2
| @@ -33,6 +33,13 @@ func (b *syncBuffer) String() string { | ||
| 33 | 33 | // follow starts build log --follow on build 1 of repo and returns the |
| 34 | 34 | // buffers and a channel carrying the exit code. |
| 35 | 35 | func follow(t *testing.T, st *store.Store, uid int64, repo store.Repo, done <-chan struct{}) (*syncBuffer, *syncBuffer, chan int) { |
| 36 | t.Helper() | |
| 37 | return followStopping(t, st, uid, repo, done, nil) | |
| 38 | } | |
| 39 | ||
| 40 | // followStopping is follow on a surface that is being restarted when | |
| 41 | // stopping closes. | |
| 42 | func followStopping(t *testing.T, st *store.Store, uid int64, repo store.Repo, done, stopping <-chan struct{}) (*syncBuffer, *syncBuffer, chan int) { | |
| 36 | 43 | t.Helper() |
| 37 | 44 | u, err := st.UserByID(uid) |
| 38 | 45 | if err != nil { |
| @@ -40,7 +47,7 @@ func follow(t *testing.T, st *store.Store, uid int64, repo store.Repo, done <-ch | ||
| 40 | 47 | } |
| 41 | 48 | var out, errOut syncBuffer |
| 42 | 49 | c := &Ctx{User: u, Scope: "full", Store: st, Stdin: strings.NewReader(""), |
| 43 | Stdout: &out, Stderr: &errOut, Done: done} | |
| 50 | Stdout: &out, Stderr: &errOut, Done: done, Stopping: stopping} | |
| 44 | 51 | res := make(chan int, 1) |
| 45 | 52 | go func() { res <- Dispatch(c, []string{"build", "log", repo.Path(), "1", "--follow"}) }() |
| 46 | 53 | return &out, &errOut, res |
| @@ -146,13 +153,62 @@ func TestBuildLogFollowDone(t *testing.T) { | ||
| 146 | 153 | } |
| 147 | 154 | st.AppendBuildLog(id, []byte("step one\n")) |
| 148 | 155 | done := make(chan struct{}) |
| 149 | out, _, res := follow(t, st, uid, repo, done) | |
| 156 | out, errOut, res := follow(t, st, uid, repo, done) | |
| 150 | 157 | |
| 151 | 158 | waitOutput(t, out, "step one") |
| 152 | 159 | close(done) |
| 153 | 160 | if code := waitExit(t, res); code != protocol.ExitFailure { |
| 154 | 161 | t.Fatalf("exit %d, want %d", code, protocol.ExitFailure) |
| 155 | 162 | } |
| 163 | if errOut.String() != "" { | |
| 164 | t.Errorf("a follow whose reader left wrote %q", errOut) | |
| 165 | } | |
| 166 | } | |
| 167 | ||
| 168 | // A restart ends a follow and says so, so the reader knows to follow | |
| 169 | // again. | |
| 170 | func TestBuildLogFollowRestart(t *testing.T) { | |
| 171 | shortFollowTimers(t) | |
| 172 | st, repo, uid := newQueueTestRepo(t) | |
| 173 | id, err := st.CreateBuild(repo.ID, "unit", "abc", "main", `["true"]`, "", "", true) | |
| 174 | if err != nil { | |
| 175 | t.Fatal(err) | |
| 176 | } | |
| 177 | st.AppendBuildLog(id, []byte("step one\n")) | |
| 178 | stopping := make(chan struct{}) | |
| 179 | out, errOut, res := followStopping(t, st, uid, repo, stopping, stopping) | |
| 180 | waitOutput(t, out, "step one") | |
| 181 | close(stopping) | |
| 182 | if code := waitExit(t, res); code != protocol.ExitFailure { | |
| 183 | t.Fatalf("exit %d, want %d", code, protocol.ExitFailure) | |
| 184 | } | |
| 185 | if got := strings.TrimSpace(errOut.String()); got != "gitbay is restarting; follow the build again in a moment" { | |
| 186 | t.Errorf("stderr %q", got) | |
| 187 | } | |
| 188 | } | |
| 189 | ||
| 190 | // A follower whose account is disabled mid-follow is ended, even on a | |
| 191 | // public repository. | |
| 192 | func TestBuildLogFollowDisabled(t *testing.T) { | |
| 193 | shortFollowTimers(t) | |
| 194 | st, repo, _ := newQueueTestRepo(t) | |
| 195 | bob, err := st.CreateUser("bob", false) | |
| 196 | if err != nil { | |
| 197 | t.Fatal(err) | |
| 198 | } | |
| 199 | id, err := st.CreateBuild(repo.ID, "unit", "abc", "main", `["true"]`, "", "", true) | |
| 200 | if err != nil { | |
| 201 | t.Fatal(err) | |
| 202 | } | |
| 203 | st.AppendBuildLog(id, []byte("step one\n")) | |
| 204 | out, errOut, res := follow(t, st, bob, repo, nil) | |
| 205 | waitOutput(t, out, "step one") | |
| 206 | if err := st.SetUserDisabled(bob, true); err != nil { | |
| 207 | t.Fatal(err) | |
| 208 | } | |
| 209 | if code := waitExit(t, res); code != protocol.ExitNotFound { | |
| 210 | t.Fatalf("exit %d, want %d: %s", code, protocol.ExitNotFound, errOut) | |
| 211 | } | |
| 156 | 212 | } |
| 157 | 213 | |
| 158 | 214 | // A build that stays pending ends its own follow: nothing reaps a queued |
internal/control/control.go +4
| @@ -44,6 +44,10 @@ type Ctx struct { | ||
| 44 | 44 | // more: the SSH channel closed or the HTTP request ended. A command |
| 45 | 45 | // that runs until something happens (build log --follow) stops on it. |
| 46 | 46 | Done <-chan struct{} |
| 47 | // Stopping, when the surface has one, closes when the daemon is | |
| 48 | // restarting. It closes Done too; a command that ends on Done checks | |
| 49 | // it to say why. | |
| 50 | Stopping <-chan struct{} | |
| 47 | 51 | } |
| 48 | 52 | |
| 49 | 53 | // usage reports a bad invocation with the command's registered usage, |
internal/httpd/api.go +1
| @@ -74,6 +74,7 @@ func (s *Server) apiCmd(w http.ResponseWriter, r *http.Request) { | ||
| 74 | 74 | ViaAPI: true, |
| 75 | 75 | ReadOnly: scope == "read", |
| 76 | 76 | Done: s.until(r), |
| 77 | Stopping: s.stopping, | |
| 77 | 78 | } |
| 78 | 79 | code := control.Dispatch(ctx, req.Argv) |
| 79 | 80 | |
internal/httpd/apiread.go +1
| @@ -63,6 +63,7 @@ func (s *Server) apiRead(w http.ResponseWriter, r *http.Request) { | ||
| 63 | 63 | ViaAPI: true, |
| 64 | 64 | ReadOnly: true, |
| 65 | 65 | Done: s.until(r), |
| 66 | Stopping: s.stopping, | |
| 66 | 67 | } |
| 67 | 68 | code := control.Dispatch(ctx, argv) |
| 68 | 69 | |
internal/httpd/builds.go −5
| @@ -358,11 +358,6 @@ func (s *Server) streamBuild(w http.ResponseWriter, r *http.Request, v buildView | ||
| 358 | 358 | // The client left; nothing more to write. |
| 359 | 359 | return |
| 360 | 360 | } |
| 361 | if code != protocol.ExitOK && s.stopped() { | |
| 362 | io.WriteString(w, `</pre><p class="notice" role="status">gitbay is restarting; reload in a moment to pick the log up again.</p>`) | |
| 363 | io.WriteString(w, tail) | |
| 364 | return | |
| 365 | } | |
| 366 | 361 | if code == protocol.ExitDenied { |
| 367 | 362 | // The follow cap: the stored log once, and why it is not live. |
| 368 | 363 | log, _, _ := s.runControl(viewer, []string{"build", "log", path, n}) |
internal/httpd/control.go +11 −10
| @@ -57,16 +57,17 @@ func (s *Server) runControlCode(u store.User, argv []string) (out string, msg st | ||
| 57 | 57 | func (s *Server) runControlStream(u store.User, argv []string, out io.Writer, done <-chan struct{}) (msg string, code int) { |
| 58 | 58 | var stderr bytes.Buffer |
| 59 | 59 | ctx := &control.Ctx{ |
| 60 | User: u, | |
| 61 | Source: "web", | |
| 62 | Scope: "full", | |
| 63 | Store: s.st, | |
| 64 | Cfg: s.cfg, | |
| 65 | Stdin: strings.NewReader(""), | |
| 66 | Stdout: out, | |
| 67 | Stderr: &stderr, | |
| 68 | ViaAPI: true, | |
| 69 | Done: done, | |
| 60 | User: u, | |
| 61 | Source: "web", | |
| 62 | Scope: "full", | |
| 63 | Store: s.st, | |
| 64 | Cfg: s.cfg, | |
| 65 | Stdin: strings.NewReader(""), | |
| 66 | Stdout: out, | |
| 67 | Stderr: &stderr, | |
| 68 | ViaAPI: true, | |
| 69 | Done: done, | |
| 70 | Stopping: s.stopping, | |
| 70 | 71 | } |
| 71 | 72 | code = control.Dispatch(ctx, argv) |
| 72 | 73 | return strings.TrimSpace(stderr.String()), code |
internal/httpd/smart.go +1 −10
| @@ -26,7 +26,7 @@ type Server struct { | ||
| 26 | 26 | cfg config.Config |
| 27 | 27 | st *store.Store |
| 28 | 28 | apiLimit *apiLimiter |
| 29 | proxies []*net.IPNet // http.trusted_proxies, parsed once | |
| 29 | proxies []*net.IPNet // http.trusted_proxies, parsed once | |
| 30 | 30 | stopping chan struct{} // closed by Stop |
| 31 | 31 | stopOnce sync.Once |
| 32 | 32 | } |
| @@ -58,15 +58,6 @@ func (s *Server) until(r *http.Request) <-chan struct{} { | ||
| 58 | 58 | return done |
| 59 | 59 | } |
| 60 | 60 | |
| 61 | func (s *Server) stopped() bool { | |
| 62 | select { | |
| 63 | case <-s.stopping: | |
| 64 | return true | |
| 65 | default: | |
| 66 | return false | |
| 67 | } | |
| 68 | } | |
| 69 | ||
| 70 | 61 | // receivePackRefusal exists only to fail legibly if a client POSTs without |
| 71 | 62 | // reading the advertisement first. |
| 72 | 63 | func (s *Server) receivePackRefusal(w http.ResponseWriter, r *http.Request) { |
internal/sshd/sshd.go +12 −18
| @@ -268,13 +268,6 @@ func (s *Server) handleSession(sconn *ssh.ServerConn, ch ssh.Channel, reqs <-cha | ||
| 268 | 268 | close(done) |
| 269 | 269 | }() |
| 270 | 270 | code := s.runExec(sconn, ch, payload.Command, done) |
| 271 | select { | |
| 272 | case <-s.stopping: | |
| 273 | if code != protocol.ExitOK { | |
| 274 | fmt.Fprintln(ch.Stderr(), "gitbay is restarting; run the command again in a moment") | |
| 275 | } | |
| 276 | default: | |
| 277 | } | |
| 278 | 271 | sendExit(ch, code) |
| 279 | 272 | return |
| 280 | 273 | case "shell": |
| @@ -309,7 +302,7 @@ func (s *Server) runExec(sconn *ssh.ServerConn, ch ssh.Channel, cmdline string, | ||
| 309 | 302 | return protocol.ExitDenied |
| 310 | 303 | } |
| 311 | 304 | _ = s.st.TouchSSHKey(keyID) |
| 312 | return Exec(s.cfg, s.st, user, ext["scope"], ext["key-fp"], cmdline, ch, ch, ch.Stderr(), done) | |
| 305 | return Exec(s.cfg, s.st, user, ext["scope"], ext["key-fp"], cmdline, ch, ch, ch.Stderr(), done, s.stopping) | |
| 313 | 306 | } |
| 314 | 307 | |
| 315 | 308 | // runAnonymous handles a session from an unregistered key: the register |
| @@ -340,7 +333,7 @@ func (s *Server) runAnonymous(ch ssh.Channel, keyB64, cmdline string) int { | ||
| 340 | 333 | // single dispatch path shared by the embedded listener and the system-sshd |
| 341 | 334 | // forced command (gitbayd shell). |
| 342 | 335 | func Exec(cfg config.Config, st *store.Store, user store.User, scope, source, cmdline string, |
| 343 | stdin io.Reader, stdout, stderr io.Writer, done <-chan struct{}) int { | |
| 336 | stdin io.Reader, stdout, stderr io.Writer, done, stopping <-chan struct{}) int { | |
| 344 | 337 | if user.Disabled { |
| 345 | 338 | fmt.Fprintln(stderr, "this account is disabled; contact the instance admin") |
| 346 | 339 | return protocol.ExitDenied |
| @@ -369,15 +362,16 @@ func Exec(cfg config.Config, st *store.Store, user store.User, scope, source, cm | ||
| 369 | 362 | } |
| 370 | 363 | } |
| 371 | 364 | ctx := &control.Ctx{ |
| 372 | User: user, | |
| 373 | Scope: scope, | |
| 374 | Source: source, | |
| 375 | Store: st, | |
| 376 | Cfg: cfg, | |
| 377 | Stdin: stdin, | |
| 378 | Stdout: stdout, | |
| 379 | Stderr: stderr, | |
| 380 | Done: done, | |
| 365 | User: user, | |
| 366 | Scope: scope, | |
| 367 | Source: source, | |
| 368 | Store: st, | |
| 369 | Cfg: cfg, | |
| 370 | Stdin: stdin, | |
| 371 | Stdout: stdout, | |
| 372 | Stderr: stderr, | |
| 373 | Done: done, | |
| 374 | Stopping: stopping, | |
| 381 | 375 | } |
| 382 | 376 | return control.Dispatch(ctx, argv) |
| 383 | 377 | } |
internal/sshd/sshd_test.go +21
| @@ -153,6 +153,27 @@ func TestFollowEndsWhenChannelCloses(t *testing.T) { | ||
| 153 | 153 | } |
| 154 | 154 | } |
| 155 | 155 | |
| 156 | // A command that fails on its own while the server is stopping says | |
| 157 | // nothing about a restart: only a follow that Stop ended does. | |
| 158 | func TestStopLeavesOtherFailuresAlone(t *testing.T) { | |
| 159 | srv, client := followServer(t) | |
| 160 | srv.Stop() | |
| 161 | sess, err := client.NewSession() | |
| 162 | if err != nil { | |
| 163 | t.Fatal(err) | |
| 164 | } | |
| 165 | defer sess.Close() | |
| 166 | var stderr bytes.Buffer | |
| 167 | sess.Stderr = &stderr | |
| 168 | var exit *ssh.ExitError | |
| 169 | if err := sess.Run("repo show nosuch/repo"); !errors.As(err, &exit) || exit.ExitStatus() != 3 { | |
| 170 | t.Fatalf("repo show of a missing repository: %v, want exit 3", err) | |
| 171 | } | |
| 172 | if strings.Contains(stderr.String(), "restarting") { | |
| 173 | t.Errorf("stderr %q", stderr.String()) | |
| 174 | } | |
| 175 | } | |
| 176 | ||
| 156 | 177 | // Stop ends a follow with exit 1 and says why, without closing the |
| 157 | 178 | // connection. |
| 158 | 179 | func TestStopEndsFollow(t *testing.T) { |