dc10160221 · krz/gitbay

cmd/gitbay/main.go +1

		@@ -618,6 +618,7 @@ func repoCmd() *cobra.Command {
618	618	pass("require-resolved", passOpts{server: []string{"repo", "settings", "require-resolved"}, needsRepo: true}),
619	619	pass("require-codeowners", passOpts{server: []string{"repo", "settings", "require-codeowners"}, needsRepo: true}),
620	620	pass("require-checks", passOpts{server: []string{"repo", "settings", "require-checks"}, needsRepo: true}),
	621	pass("require-contexts", passOpts{server: []string{"repo", "settings", "require-contexts"}, needsRepo: true}),
621	622	pass("visibility", passOpts{server: []string{"repo", "settings", "visibility"}, needsRepo: true}),
622	623	pass("require-signed", passOpts{server: []string{"repo", "settings", "require-signed"}, needsRepo: true}),
623	624	pass("require-mr", passOpts{server: []string{"repo", "settings", "require-mr"}, needsRepo: true}),

cmd/gitbay/summaries_gen.go +1

		@@ -187,6 +187,7 @@ var summaries = map[string]string{
187	187	"repo settings require-approvals": "require N fresh approvals to merge",
188	188	"repo settings require-checks": "gate merges on green statuses",
189	189	"repo settings require-codeowners": "require an owner's approval for every file CODEOWNERS covers",
	190	"repo settings require-contexts": "name the statuses the checks gate waits for, and turn the gate on",
190	191	"repo settings require-mr": "protected branches take changes through merge requests only",
191	192	"repo settings require-resolved": "require all review threads resolved to merge",
192	193	"repo settings require-signed": "require verified commit signatures",

e2e/settingsweb_test.go +7 −2

		@@ -55,7 +55,12 @@ func TestRepoSettingsWeb(t *testing.T) {
55	55
56	56	post(url.Values{"field": {"description"}, "description": {"a fine tool"}})
57	57	post(url.Values{"field": {"website"}, "website": {"https://tool.example"}})
58		post(url.Values{"field": {"require-checks"}, "require-checks": {"on"}})
	58	// Saving required contexts turns the checks gate on, and the page
	59	// shows it ticked with the contexts in its hint (#258).
	60	if body := post(url.Values{"field": {"require-contexts"}, "contexts": {"ext/deploy lint"}}); !strings.Contains(body, `id="require-checks" name="require-checks" value="on" checked`) \|\|
	61	!strings.Contains(body, `Also waits for <code>ext/deploy</code>, <code>lint</code> until they report.`) {
	62	t.Fatalf("required contexts did not show as turning required checks on:\n%s", body)
	63	}
59	64	post(url.Values{"field": {"require-approvals"}, "approvals": {"2"}})
60	65	post(url.Values{"field": {"protect"}, "branch": {"main"}})
61	66
		@@ -66,7 +71,7 @@ func TestRepoSettingsWeb(t *testing.T) {
66	71	}
67	72	}
68	73	out, _, _ = inst.ssh(t, aliceKey, "", "repo", "settings", "show", "alice/app", "--json")
69		for _, want := range []string{`"require_checks":true`, `"require_approvals":2`, `"main"`} {
	74	for _, want := range []string{`"require_checks":true`, `"required_contexts":["ext/deploy","lint"]`, `"require_approvals":2`, `"main"`} {
70	75	if !strings.Contains(out, want) {
71	76	t.Fatalf("settings show missing %q:\n%s", want, out)
72	77	}

internal/control/checksgate_test.go +55 −7

		@@ -3,6 +3,7 @@ package control
3	3	import (
4	4	"os"
5	5	"path/filepath"
	6	"slices"
6	7	"strings"
7	8	"testing"
8	9
		@@ -18,9 +19,30 @@ func gatesForHead(t *testing.T, ciYML string) GatesOut {
18	19	}
19	20
20	21	func gatesForHeadSeeded(t *testing.T, ciYML string, seed bool) GatesOut {
	22	return gatesFor(t, ciYML, nil, func(st *store.Store, repoID, uid int64, targetSHA, _ string) {
	23	if !seed {
	24	return
	25	}
	26	if err := st.SetCommitStatus(repoID, targetSHA, "lint", "success", "", "", uid); err != nil {
	27	t.Fatal(err)
	28	}
	29	})
	30	}
	31
	32	// gatesFor builds a repository with require_checks on and set applied to
	33	// its settings, a bare dir holding the given .gitbay/ci.yml (empty
	34	// string for none), and one MR; seed records statuses before the gates
	35	// are computed.
	36	func gatesFor(t testing.T, ciYML string, set func(store.RepoSettings),
	37	seed func(st *store.Store, repoID, uid int64, targetSHA, headSHA string)) GatesOut {
21	38	t.Helper()
22	39	st, repo, uid := newQueueTestRepo(t)
23		if _, err := st.UpdateRepoSettings(repo.ID, func(set *store.RepoSettings) { set.RequireChecks = true }); err != nil {
	40	if _, err := st.UpdateRepoSettings(repo.ID, func(s *store.RepoSettings) {
	41	s.RequireChecks = true
	42	if set != nil {
	43	set(s)
	44	}
	45	}); err != nil {
24	46	t.Fatal(err)
25	47	}
26	48	repo, err := st.RepoByID(repo.ID)
		@@ -58,13 +80,9 @@ func gatesForHeadSeeded(t *testing.T, ciYML string, seed bool) GatesOut {
58	80	if err != nil {
59	81	t.Fatal(err)
60	82	}
61
62		if seed {
63		if err := st.SetCommitStatus(repo.ID, targetSHA, "lint", "success", "", "", uid); err != nil {
64		t.Fatal(err)
65		}
	83	if seed != nil {
	84	seed(st, repo.ID, uid, targetSHA, headSHA)
66	85	}
67
68	86	g, err := MergeGates(st, repo, mr, dir, targetSHA, headSHA)
69	87	if err != nil {
70	88	t.Fatal(err)
		@@ -118,3 +136,33 @@ func TestRequireChecksRefusesSilentHeadInReportingRepo(t *testing.T) {
118	136	t.Fatalf("allowed a silent head in a repository that reports statuses: %v", g.Unmet)
119	137	}
120	138	}
	139
	140	// A required context that has not reported holds the merge as pending,
	141	// even when every status that did report is green (#258).
	142	func TestRequiredContextMissingIsPending(t *testing.T) {
	143	g := gatesFor(t, "", func(s *store.RepoSettings) { s.RequiredContexts = []string{"ext/deploy", "lint"} },
	144	func(st *store.Store, repoID, uid int64, _, headSHA string) {
	145	if err := st.SetCommitStatus(repoID, headSHA, "lint", "success", "", "", uid); err != nil {
	146	t.Fatal(err)
	147	}
	148	})
	149	if g.Checks != "pending" \|\| !slices.Equal(g.ChecksMissing, []string{"ext/deploy"}) {
	150	t.Fatalf("checks %q, missing %v", g.Checks, g.ChecksMissing)
	151	}
	152	if !checksUnmet(g) \|\| !strings.Contains(strings.Join(g.Unmet, "\n"), "ext/deploy=missing") {
	153	t.Fatalf("unmet: %v", g.Unmet)
	154	}
	155	}
	156
	157	// Every required context reported green: nothing is held.
	158	func TestRequiredContextsReportedPass(t *testing.T) {
	159	g := gatesFor(t, "", func(s *store.RepoSettings) { s.RequiredContexts = []string{"lint"} },
	160	func(st *store.Store, repoID, uid int64, _, headSHA string) {
	161	if err := st.SetCommitStatus(repoID, headSHA, "lint", "success", "", "", uid); err != nil {
	162	t.Fatal(err)
	163	}
	164	})
	165	if checksUnmet(g) \|\| len(g.ChecksMissing) != 0 \|\| g.Checks != "success" {
	166	t.Fatalf("checks %q, missing %v, unmet %v", g.Checks, g.ChecksMissing, g.Unmet)
	167	}
	168	}

internal/control/mr.go +72 −1

		@@ -47,6 +47,11 @@ func init() {
47	47	Usage: "repo settings require-checks <owner/name> on\|off",
48	48	Examples: []string{"repo settings require-checks krz/gitbay on"},
49	49	Run: runRequireChecks})
	50	register(Command{Path: []string{"repo", "settings", "require-contexts"},
	51	Summary: "name the statuses the checks gate waits for, and turn the gate on",
	52	Usage: "repo settings require-contexts <owner/name> [<context>...] (none clears the list)",
	53	Examples: []string{"repo settings require-contexts krz/gitbay ci/build ci/test"},
	54	Run: runRequireContexts})
50	55	register(Command{Path: []string{"repo", "settings", "require-mr"},
51	56	Summary: "protected branches take changes through merge requests only",
52	57	Usage: "repo settings require-mr <owner/name> on\|off",
		@@ -340,6 +345,54 @@ func runRequireChecks(c *Ctx, args []string) int {
340	345	})
341	346	}
342	347
	348	// maxRequiredContexts bounds the list: a gate naming more checks than
	349	// this is a configuration mistake.
	350	const maxRequiredContexts = 20
	351
	352	func runRequireContexts(c *Ctx, args []string) int {
	353	if len(args) < 1 {
	354	return c.usage()
	355	}
	356	var contexts []string
	357	for _, ctx := range args[1:] {
	358	if ctx == "" \|\| len(ctx) > 100 \|\| strings.ContainsAny(ctx, " \t\r\n") {
	359	return c.fail(protocol.ExitUsage, "a context is 1 to 100 characters with no whitespace: %q", ctx)
	360	}
	361	if !slices.Contains(contexts, ctx) {
	362	contexts = append(contexts, ctx)
	363	}
	364	}
	365	if len(contexts) > maxRequiredContexts {
	366	return c.fail(protocol.ExitUsage, "at most %d required contexts", maxRequiredContexts)
	367	}
	368	repo, code := resolveRepo(c, args[0], policy.CanAdmin)
	369	if code >= 0 {
	370	return code
	371	}
	372	// Naming contexts asks for the gate, so it turns require_checks on in
	373	// the same update. Clearing the list leaves the gate as it was.
	374	s, err := c.Store.UpdateRepoSettings(repo.ID, func(s *store.RepoSettings) {
	375	s.RequiredContexts = contexts
	376	if len(contexts) > 0 {
	377	s.RequireChecks = true
	378	}
	379	})
	380	if err != nil {
	381	return c.fail(protocol.ExitFailure, "%v", err)
	382	}
	383	return c.emit(s, func(w io.Writer) {
	384	if len(contexts) > 0 {
	385	fmt.Fprintf(w, "required contexts on %s: %s; require_checks on\n", repo.Path(), strings.Join(contexts, ", "))
	386	return
	387	}
	388	gate := "off"
	389	if s.RequireChecks {
	390	gate = "on"
	391	}
	392	fmt.Fprintf(w, "required contexts cleared on %s; require_checks %s\n", repo.Path(), gate)
	393	})
	394	}
	395
343	396	func runRequireMR(c *Ctx, args []string) int {
344	397	if len(args) != 2 \|\| (args[1] != "on" && args[1] != "off") {
345	398	return c.usage()
		@@ -1577,13 +1630,28 @@ func MergeGates(st *store.Store, repo store.Repo, mr store.MR, dir, targetSHA, h
1577	1630	}
1578	1631
1579	1632	// Checks: with require_checks, every status the head carries must be
1580		// green, and a head something was going to report on must carry some.
	1633	// green, a head something was going to report on must carry some, and
	1634	// every required context must have reported: one that has not is
	1635	// pending whatever the others say (#258). Setting contexts turns
	1636	// require_checks on; turned off again, the list is kept and unread.
1581	1637	statuses, err := st.ListCommitStatuses(repo.ID, headSHA)
1582	1638	if err != nil {
1583	1639	return g, err
1584	1640	}
1585	1641	g.Checks = store.CombinedStatus(statuses)
1586	1642	if set.RequireChecks {
	1643	reported := map[string]bool{}
	1644	for _, s := range statuses {
	1645	reported[s.Context] = true
	1646	}
	1647	for _, want := range set.RequiredContexts {
	1648	if !reported[want] {
	1649	g.ChecksMissing = append(g.ChecksMissing, want)
	1650	}
	1651	}
	1652	if len(g.ChecksMissing) > 0 && (g.Checks == "" \|\| g.Checks == "success") {
	1653	g.Checks = "pending"
	1654	}
1587	1655	switch g.Checks {
1588	1656	case "success":
1589	1657	case "":
		@@ -1597,6 +1665,9 @@ func MergeGates(st *store.Store, repo store.Repo, mr store.MR, dir, targetSHA, h
1597	1665	bad = append(bad, st.Context+"="+st.State)
1598	1666	}
1599	1667	}
	1668	for _, m := range g.ChecksMissing {
	1669	bad = append(bad, m+"=missing")
	1670	}
1600	1671	g.Unmet = append(g.Unmet, fmt.Sprintf("%s requires green checks; %.10s has %s", repo.Path(), headSHA, strings.Join(bad, ", ")))
1601	1672	}
1602	1673	}

internal/control/mr_test.go +75

		@@ -5,6 +5,7 @@ import (
5	5	"encoding/json"
6	6	"os"
7	7	"path/filepath"
	8	"slices"
8	9	"strconv"
9	10	"strings"
10	11	"testing"
		@@ -262,3 +263,77 @@ func TestMRShowPluralizesMultiRowSections(t *testing.T) {
262	263	}
263	264	}
264	265	}
	266
	267	// require-contexts stores a deduplicated list and turns require_checks
	268	// on with it; an empty list clears the contexts and leaves
	269	// require_checks as it was. A context with whitespace is refused (#258).
	270	func TestRequireContextsSetsAndClears(t *testing.T) {
	271	st, repo, uid := newQueueTestRepo(t)
	272	alice := store.User{ID: uid, Username: "alice"}
	273	dispatch := func(args ...string) int {
	274	t.Helper()
	275	c, _, _ := mrTestCtx(st, alice)
	276	return Dispatch(c, args)
	277	}
	278	contexts := func(names ...string) int {
	279	t.Helper()
	280	return dispatch(append([]string{"repo", "settings", "require-contexts", repo.Path()}, names...)...)
	281	}
	282	settings := func() store.RepoSettings {
	283	t.Helper()
	284	got, err := st.RepoByID(repo.ID)
	285	if err != nil {
	286	t.Fatal(err)
	287	}
	288	return got.Settings
	289	}
	290
	291	if settings().RequireChecks {
	292	t.Fatal("require_checks on in a new repository")
	293	}
	294	if code := contexts("lint", "ext/deploy", "lint"); code != protocol.ExitOK {
	295	t.Fatalf("set: exit %d", code)
	296	}
	297	if s := settings(); !slices.Equal(s.RequiredContexts, []string{"lint", "ext/deploy"}) \|\| !s.RequireChecks {
	298	t.Fatalf("stored %v, require_checks %v; want [lint ext/deploy], on", s.RequiredContexts, s.RequireChecks)
	299	}
	300	if code := contexts("bad context"); code != protocol.ExitUsage {
	301	t.Fatalf("a context with a space: exit %d", code)
	302	}
	303	if code := contexts(); code != protocol.ExitOK {
	304	t.Fatalf("clear: exit %d", code)
	305	}
	306	if s := settings(); len(s.RequiredContexts) != 0 \|\| !s.RequireChecks {
	307	t.Fatalf("after clearing: contexts %v, require_checks %v; want none, still on", s.RequiredContexts, s.RequireChecks)
	308	}
	309	if code := dispatch("repo", "settings", "require-checks", repo.Path(), "off"); code != protocol.ExitOK {
	310	t.Fatalf("require-checks off: exit %d", code)
	311	}
	312	if code := contexts(); code != protocol.ExitOK {
	313	t.Fatalf("clear again: exit %d", code)
	314	}
	315	if settings().RequireChecks {
	316	t.Fatal("clearing the list turned require_checks on")
	317	}
	318	}
	319
	320	// settings show prints the checks gate beside the contexts it waits for,
	321	// so a list that turned the gate on is visible where the gate is (#258).
	322	func TestSettingsShowRequiredContexts(t *testing.T) {
	323	st, repo, uid := newQueueTestRepo(t)
	324	alice := store.User{ID: uid, Username: "alice"}
	325	c, _, _ := mrTestCtx(st, alice)
	326	if code := Dispatch(c, []string{"repo", "settings", "require-contexts", repo.Path(), "ext/deploy", "lint"}); code != protocol.ExitOK {
	327	t.Fatalf("require-contexts: exit %d", code)
	328	}
	329	c, out, _ := mrTestCtx(st, alice)
	330	if code := Dispatch(c, []string{"repo", "settings", "show", repo.Path()}); code != protocol.ExitOK {
	331	t.Fatalf("settings show: exit %d", code)
	332	}
	333	got := strings.Join(strings.Fields(out.String()), " ")
	334	for _, want := range []string{"require checks true", "required contexts ext/deploy, lint"} {
	335	if !strings.Contains(got, want) {
	336	t.Errorf("settings show lacks %q:\n%s", want, out.String())
	337	}
	338	}
	339	}

internal/control/output.go +2 −1

		@@ -57,7 +57,8 @@ type GatesOut struct {
57	57	ResolvedRequired bool `json:"resolved_required"`
58	58	OpenThreads int `json:"open_threads"`
59	59	ChecksRequired bool `json:"checks_required"`
60		Checks string `json:"checks,omitempty"` // combined status; "" when none reported
	60	Checks string `json:"checks,omitempty"` // combined status; "" when none reported
	61	ChecksMissing []string `json:"checks_missing,omitempty"` // required contexts not reported
61	62	FastForward bool `json:"fast_forward"`
62	63	Unmet []string `json:"unmet,omitempty"`
63	64	}

internal/control/repo.go +2

		@@ -711,6 +711,8 @@ func runSettingsShow(c *Ctx, args []string) int {
711	711	"protected branches", strings.Join(repo.Settings.ProtectedBranches, ", "),
712	712	"protected tags", strings.Join(repo.Settings.ProtectedTags, ", "),
713	713	"require mr", strconv.FormatBool(repo.Settings.RequireMR),
	714	"require checks", strconv.FormatBool(repo.Settings.RequireChecks),
	715	"required contexts", strings.Join(repo.Settings.RequiredContexts, ", "),
714	716	"require signed commits", strconv.FormatBool(repo.Settings.RequireSignedCommits),
715	717	"git daemon", strconv.FormatBool(repo.Settings.GitDaemon),
716	718	"archived", strconv.FormatBool(repo.Settings.Archived),

internal/httpd/mrpage_test.go +3

		@@ -195,6 +195,9 @@ func TestMRGatesRender(t *testing.T) {
195	195	if out := render(&control.GatesOut{FastForward: true}); !strings.Contains(out, "All gates met") \|\| !strings.Contains(out, "fast-forward possible") {
196	196	t.Errorf("met gates not rendered:\n%s", out)
197	197	}
	198	if out := render(&control.GatesOut{Checks: "pending", ChecksMissing: []string{"ext/deploy"}}); !strings.Contains(out, "waiting on <code>ext/deploy</code>") {
	199	t.Errorf("missing required context not rendered:\n%s", out)
	200	}
198	201	if out := render(nil); strings.Contains(out, "Merge gates") {
199	202	t.Errorf("gates block on a merge request without gates:\n%s", out)
200	203	}

internal/httpd/settings.go +4

		@@ -105,6 +105,8 @@ func (s Server) settingsSubmit(w http.ResponseWriter, r http.Request, u store.
105	105	argv = []string{"repo", "settings", "git-daemon", repo, onOff(v("git-daemon"))}
106	106	case "require-checks":
107	107	argv = []string{"repo", "settings", "require-checks", repo, onOff(v("require-checks"))}
	108	case "require-contexts":
	109	argv = append([]string{"repo", "settings", "require-contexts", repo}, strings.Fields(v("contexts"))...)
108	110	case "require-resolved":
109	111	argv = []string{"repo", "settings", "require-resolved", repo, onOff(v("require-resolved"))}
110	112	case "require-codeowners":
		@@ -227,6 +229,8 @@ func fieldLabel(field string) string {
227	229	return "git:// serving"
228	230	case "require-checks":
229	231	return "required checks"
	232	case "require-contexts":
	233	return "required contexts"
230	234	case "require-approvals":
231	235	return "approvals"
232	236	case "require-resolved":

internal/store/repos.go +11 −7

		@@ -27,13 +27,17 @@ type RepoSettings struct {
27	27	ProtectedTags []string `json:"protected_tags,omitempty"` // path.Match globs
28	28	RequireSignedCommits bool `json:"require_signed_commits,omitempty"`
29	29	RequireChecks bool `json:"require_checks,omitempty"`
30		RequireApprovals int `json:"require_approvals,omitempty"`
31		RequireResolved bool `json:"require_resolved,omitempty"`
32		RequireCodeowners bool `json:"require_codeowners,omitempty"`
33		RequireMR bool `json:"require_mr,omitempty"`
34		GitDaemon bool `json:"git_daemon,omitempty"`
35		Archived bool `json:"archived,omitempty"`
36		Website string `json:"website,omitempty"`
	30	// RequiredContexts are statuses require_checks waits for whether or
	31	// not they have reported; one that has not is pending. Setting a
	32	// non-empty list turns RequireChecks on (#258).
	33	RequiredContexts []string `json:"required_contexts,omitempty"`
	34	RequireApprovals int `json:"require_approvals,omitempty"`
	35	RequireResolved bool `json:"require_resolved,omitempty"`
	36	RequireCodeowners bool `json:"require_codeowners,omitempty"`
	37	RequireMR bool `json:"require_mr,omitempty"`
	38	GitDaemon bool `json:"git_daemon,omitempty"`
	39	Archived bool `json:"archived,omitempty"`
	40	Website string `json:"website,omitempty"`
37	41	}
38	42
39	43	// Path returns the canonical owner/name form.

internal/web/templates/mr.html +1

		@@ -122,6 +122,7 @@
122	122	{{else}}<p class="row"><span class="dot ok"></span>All gates met</p>{{end}}
123	123	{{if .ApprovalsRequired}}<p class="row none">approvals: {{len .Approvals}} of {{.ApprovalsRequired}}{{if .Approvals}} ({{range $i, $a := .Approvals}}{{if $i}}, {{end}}{{$a}}{{end}}){{end}}</p>{{end}}
124	124	{{range .OwnersOutstanding}}<p class="row none">waiting on {{range $i, $o := .Owners}}{{if $i}} or {{end}}<a href="/{{$o}}">{{$o}}</a>{{end}} for {{range $i, $f := .Files}}{{if $i}}, {{end}}<code>{{$f}}</code>{{end}}</p>{{end}}
	125	{{range .ChecksMissing}}<p class="row none">waiting on <code>{{.}}</code>, not yet reported</p>{{end}}
125	126	<p class="row none">{{if .FastForward}}fast-forward possible{{else}}not a fast-forward: rebase, or merge with a merge commit{{end}}</p>
126	127	</div>{{end}}
127	128	<div class="grp">

internal/web/templates/settings.html +7 −1

		@@ -72,10 +72,16 @@
72	72	<p class="meta">Checked before a merge, in this order: checks, approvals, resolved threads, signatures.</p>
73	73	<form method="post" action="{{$base}}" class="setform">
74	74	<input type="hidden" name="field" value="require-checks">
75		<div><label for="require-checks">Required checks</label><p class="hint">Requires CI to succeed.</p></div>
	75	<div><label for="require-checks">Required checks</label><p class="hint">Requires CI to succeed.{{with .Repo.Settings.RequiredContexts}} Also waits for {{range $i, $c := .}}{{if $i}}, {{end}}<code>{{$c}}</code>{{end}} until they report{{if not $.Repo.Settings.RequireChecks}}, once this is on{{end}}.{{end}}</p></div>
76	76	<div class="check"><input type="checkbox" id="require-checks" name="require-checks" value="on"{{if .Repo.Settings.RequireChecks}} checked{{end}}></div>
77	77	<div><button type="submit" class="btn">Save</button></div>
78	78	</form>
	79	<form method="post" action="{{$base}}" class="setform">
	80	<input type="hidden" name="field" value="require-contexts">
	81	<div><label for="contexts">Required contexts</label><p class="hint">Statuses the checks gate waits for until they report, separated by spaces. Saving any turns required checks on; saving none leaves it as it is.</p></div>
	82	<div><input type="text" id="contexts" name="contexts" value="{{range $i, $c := .Repo.Settings.RequiredContexts}}{{if $i}} {{end}}{{$c}}{{end}}" autocomplete="off"></div>
	83	<div><button type="submit" class="btn">Save</button></div>
	84	</form>
79	85	<form method="post" action="{{$base}}" class="setform">
80	86	<input type="hidden" name="field" value="require-approvals">
81	87	<div><label for="approvals">Approvals</label><p class="hint">Approvals from anyone with write access. Zero means none required.</p></div>

Commit dc10160221

Commit `dc10160221`