From 88cb3e92ba25303ab67aaceb083fe7304fccd32f Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Sat, 9 May 2020 15:40:30 +0200 Subject: dashboard/app: add notion of per-repo build maintainers For context see: https://groups.google.com/d/msg/syzkaller/9HxkC_hRP4M/MzFl4laADgAJ Now if, say, linux-next, is build/boot-broken we can CC maintainers of the tree. Fixes #1667 --- dashboard/app/app_test.go | 34 ++++++++++--------- dashboard/app/config.go | 7 +++- dashboard/app/email_test.go | 79 ++++++++++++++++++++++++++++++++++++++++++-- dashboard/app/jobs.go | 6 ++-- dashboard/app/mail_bug.txt | 2 +- dashboard/app/reporting.go | 14 +++++--- dashboard/app/util_test.go | 39 ++++++++++++++-------- dashboard/dashapi/dashapi.go | 1 + 8 files changed, 141 insertions(+), 41 deletions(-) diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go index ce27b92d3..6bf4cb64b 100644 --- a/dashboard/app/app_test.go +++ b/dashboard/app/app_test.go @@ -59,16 +59,16 @@ var testConfig = &GlobalConfig{ }, Repos: []KernelRepo{ { - URL: "git://syzkaller.org", - Branch: "branch10", - Alias: "repo10alias", - CC: []string{"maintainers@repo10.org", "bugs@repo10.org"}, + URL: "git://syzkaller.org", + Branch: "branch10", + Alias: "repo10alias", + Maintainers: []string{"maintainers@repo10.org", "bugs@repo10.org"}, }, { - URL: "git://github.com/google/syzkaller", - Branch: "master", - Alias: "repo10alias", - CC: []string{"maintainers@repo10.org", "bugs@repo10.org"}, + URL: "git://github.com/google/syzkaller", + Branch: "master", + Alias: "repo10alias", + Maintainers: []string{"maintainers@repo10.org", "bugs@repo10.org"}, }, }, Managers: map[string]ConfigManager{ @@ -104,16 +104,18 @@ var testConfig = &GlobalConfig{ }, Repos: []KernelRepo{ { - URL: "git://syzkaller.org", - Branch: "branch10", - Alias: "repo10alias", - CC: []string{"maintainers@repo10.org", "bugs@repo10.org"}, + URL: "git://syzkaller.org", + Branch: "branch10", + Alias: "repo10alias", + CC: []string{"always@cc.me"}, + Maintainers: []string{"maintainers@repo10.org", "bugs@repo10.org"}, + BuildMaintainers: []string{"build-maintainers@repo10.org"}, }, { - URL: "git://syzkaller.org", - Branch: "branch20", - Alias: "repo20", - CC: []string{"maintainers@repo20.org", "bugs@repo20.org"}, + URL: "git://syzkaller.org", + Branch: "branch20", + Alias: "repo20", + Maintainers: []string{"maintainers@repo20.org", "bugs@repo20.org"}, }, }, Managers: map[string]ConfigManager{ diff --git a/dashboard/app/config.go b/dashboard/app/config.go index 78855a530..ad295fc98 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -161,6 +161,10 @@ type KernelRepo struct { ReportingPriority int // Additional CC list to add to all bugs reported on this repo. CC []string + // Additional CC list to add to bugs if we are mailing maintainers. + Maintainers []string + // Additional CC list to add to build/boot bugs if we are mailing maintainers. + BuildMaintainers []string } var ( @@ -316,7 +320,8 @@ func checkKernelRepos(ns string, cfg *Config) { if prio := repo.ReportingPriority; prio < 0 || prio > 9 { panic(fmt.Sprintf("%v: bad kernel repo reporting priority %v for %q", ns, prio, repo.Alias)) } - for _, email := range repo.CC { + emails := append(append(append([]string{}, repo.CC...), repo.Maintainers...), repo.BuildMaintainers...) + for _, email := range emails { if _, err := mail.ParseAddress(email); err != nil { panic(fmt.Sprintf("bad email address %q: %v", email, err)) } diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index 9c8dfd711..b7414369e 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -9,6 +9,7 @@ import ( "testing" "time" + "github.com/google/syzkaller/dashboard/dashapi" "github.com/google/syzkaller/pkg/email" ) @@ -142,6 +143,7 @@ For more options, visit https://groups.google.com/d/optout. kernelConfigLink := externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig) c.expectEQ(sender, fromAddr(c.ctx)) to := []string{ + "always@cc.me", "bugs2@syzkaller.com", "bugs@syzkaller.com", // This is from incomingEmail. "default@sender.com", // This is from incomingEmail. @@ -191,8 +193,14 @@ report1 kernelConfigLink := externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig) c.expectEQ(sender, fromAddr(c.ctx)) c.expectEQ(msg.To, []string{ - "bar@foo.com", "bugs@repo10.org", "bugs@syzkaller.com", - "default@maintainers.com", "foo@bar.com", "maintainers@repo10.org"}) + "always@cc.me", + "bar@foo.com", + "bugs@repo10.org", + "bugs@syzkaller.com", + "default@maintainers.com", + "foo@bar.com", + "maintainers@repo10.org", + }) c.expectEQ(msg.Subject, crash.Title) c.expectEQ(len(msg.Attachments), 0) c.expectEQ(msg.Body, fmt.Sprintf(`Hello, @@ -263,6 +271,7 @@ Content-Type: text/plain kernelConfigLink := externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig) c.expectEQ(sender, fromAddr(c.ctx)) c.expectEQ(msg.To, []string{ + "always@cc.me", "another@another.com", "bar@foo.com", "bugs@repo10.org", "bugs@syzkaller.com", "default@maintainers.com", "foo@bar.com", "maintainers@repo10.org", "new@new.com", "qux@qux.com"}) @@ -543,3 +552,69 @@ Please double check the address. `) } + +func TestEmailFailedBuild(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build := testBuild(1) + c.client2.UploadBuild(build) + + failedBuild := testBuild(10) + failedBuild.KernelRepo = testConfig.Namespaces["test2"].Repos[0].URL + failedBuild.KernelBranch = testConfig.Namespaces["test2"].Repos[0].Branch + failedBuild.KernelCommit = "kern2" + failedBuild.KernelCommitTitle = "failed build 1" + failedBuild.SyzkallerCommit = "syz2" + buildErrorReq := &dashapi.BuildErrorReq{ + Build: *failedBuild, + Crash: dashapi.Crash{ + Title: "failed build 1", + Report: []byte("report line 1\nreport line 2\n"), + Log: []byte("log line 1\nlog line 2\n"), + Maintainers: []string{"maintainer@crash"}, + }, + } + c.expectOK(c.client2.ReportBuildError(buildErrorReq)) + + msg := c.pollEmailBug() + sender, extBugID, err := email.RemoveAddrContext(msg.Sender) + c.expectOK(err) + _, dbCrash, dbBuild := c.loadBug(extBugID) + crashLogLink := externalLink(c.ctx, textCrashLog, dbCrash.Log) + kernelConfigLink := externalLink(c.ctx, textKernelConfig, dbBuild.KernelConfig) + c.expectEQ(sender, fromAddr(c.ctx)) + c.expectEQ(msg.To, []string{ + "always@cc.me", + "test@syzkaller.com", + }) + c.expectEQ(msg.Subject, buildErrorReq.Crash.Title) + c.expectEQ(len(msg.Attachments), 0) + c.expectEQ(msg.Body, fmt.Sprintf(`Hello, + +syzbot found the following crash on: + +HEAD commit: kern2 failed build 1 +git tree: repo10alias +console output: %[2]v +kernel config: %[3]v +dashboard link: https://testapp.appspot.com/bug?extid=%[1]v +compiler: compiler10 +CC: [maintainer@crash maintainers@repo10.org bugs@repo10.org build-maintainers@repo10.org] + +IMPORTANT: if you fix the bug, please add the following tag to the commit: +Reported-by: syzbot+%[1]v@testapp.appspotmail.com + +report line 1 +report line 2 + + +--- +This bug is generated by a bot. It may contain errors. +See https://goo.gl/tpsmEJ for more information about syzbot. +syzbot engineers can be reached at syzkaller@googlegroups.com. + +syzbot will keep track of this bug report. See: +https://goo.gl/tpsmEJ#status for how to communicate with syzbot.`, + extBugID, crashLogLink, kernelConfigLink)) +} diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go index 51d5c8cd4..7e9e93e0b 100644 --- a/dashboard/app/jobs.go +++ b/dashboard/app/jobs.go @@ -663,12 +663,13 @@ func createBugReportForJob(c context.Context, job *Job, jobKey *db.Key, config i default: return nil, fmt.Errorf("unknown job type %v", job.Type) } + kernelRepo := kernelRepoInfo(build) rep := &dashapi.BugReport{ Type: typ, Config: reportingConfig, JobID: extJobID(jobKey), ExtID: job.ExtID, - CC: job.CC, + CC: append(job.CC, kernelRepo.CC...), Log: crashLog, LogLink: externalLink(c, textCrashLog, job.CrashLog), Report: report, @@ -681,8 +682,7 @@ func createBugReportForJob(c context.Context, job *Job, jobKey *db.Key, config i PatchLink: externalLink(c, textPatch, job.Patch), } if job.Type == JobBisectCause || job.Type == JobBisectFix { - kernelRepo := kernelRepoInfo(build) - rep.Maintainers = append(crash.Maintainers, kernelRepo.CC...) + rep.Maintainers = append(crash.Maintainers, kernelRepo.Maintainers...) rep.ExtID = bugReporting.ExtID if bugReporting.CC != "" { rep.CC = strings.Split(bugReporting.CC, "|") diff --git a/dashboard/app/mail_bug.txt b/dashboard/app/mail_bug.txt index 7b0dde9b6..e6f80da63 100644 --- a/dashboard/app/mail_bug.txt +++ b/dashboard/app/mail_bug.txt @@ -14,7 +14,7 @@ git tree: {{.KernelRepoAlias}} {{end}}{{if .ReproSyzLink}}syz repro: {{.ReproSyzLink}} {{end}}{{if .ReproCLink}}C reproducer: {{.ReproCLink}} {{end}}{{if and .Moderation .Maintainers}}CC: {{.Maintainers}} -{{end}}{{if and (not .ReproCLink) (not .ReproSyzLink)}} +{{end}}{{if and (not .NoRepro) (not .ReproCLink) (not .ReproSyzLink)}} Unfortunately, I don't have any reproducer for this crash yet. {{end}} {{if .BisectCause}}{{if .BisectCause.Commit}}The bug was bisected to: diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go index 7892b0b34..c1d37a251 100644 --- a/dashboard/app/reporting.go +++ b/dashboard/app/reporting.go @@ -304,12 +304,13 @@ func createNotification(c context.Context, typ dashapi.BugNotif, public bool, te Title: bug.displayTitle(), Text: text, Public: public, + CC: kernelRepo.CC, } if public { - notif.Maintainers = append(crash.Maintainers, kernelRepo.CC...) + notif.Maintainers = append(crash.Maintainers, kernelRepo.Maintainers...) } if (public || reporting.moderation) && bugReporting.CC != "" { - notif.CC = strings.Split(bugReporting.CC, "|") + notif.CC = append(notif.CC, strings.Split(bugReporting.CC, "|")...) } return notif, nil } @@ -421,7 +422,8 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key LogLink: externalLink(c, textCrashLog, crash.Log), Report: report, ReportLink: externalLink(c, textCrashReport, crash.Report), - Maintainers: append(crash.Maintainers, kernelRepo.CC...), + CC: kernelRepo.CC, + Maintainers: append(crash.Maintainers, kernelRepo.Maintainers...), ReproC: reproC, ReproCLink: externalLink(c, textReproC, crash.ReproC), ReproSyz: reproSyz, @@ -431,7 +433,10 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key HappenedOn: managersToRepos(c, bug.Namespace, bug.HappenedOn), } if bugReporting.CC != "" { - rep.CC = strings.Split(bugReporting.CC, "|") + rep.CC = append(rep.CC, strings.Split(bugReporting.CC, "|")...) + } + if build.Type == BuildFailed { + rep.Maintainers = append(rep.Maintainers, kernelRepo.BuildMaintainers...) } if bug.BisectCause == BisectYes && !job.isUnreliableBisect() { rep.BisectCause = bisectFromJob(c, rep, job) @@ -471,6 +476,7 @@ func fillBugReport(c context.Context, rep *dashapi.BugReport, bug *Bug, bugRepor rep.KernelCommitDate = build.KernelCommitDate rep.KernelConfig = kernelConfig rep.KernelConfigLink = externalLink(c, textKernelConfig, build.KernelConfig) + rep.NoRepro = build.Type == BuildFailed for _, addr := range bug.UNCC { rep.CC = email.RemoveFromEmailList(rep.CC, addr) rep.Maintainers = email.RemoveFromEmailList(rep.Maintainers, addr) diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index d7ac5970a..092ad3d17 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -73,44 +73,50 @@ func NewCtx(t *testing.T) *Ctx { func (c *Ctx) expectOK(err error) { if err != nil { - c.t.Fatalf("\n%v: %v", caller(0), err) + c.t.Helper() + c.t.Fatal(err) } } func (c *Ctx) expectFail(msg string, err error) { + c.t.Helper() if err == nil { - c.t.Fatalf("\n%v: expected to fail, but it does not", caller(0)) + c.t.Fatalf("expected to fail, but it does not") } if !strings.Contains(err.Error(), msg) { - c.t.Fatalf("\n%v: expected to fail with %q, but failed with %q", caller(0), msg, err) + c.t.Fatalf("expected to fail with %q, but failed with %q", msg, err) } } func (c *Ctx) expectForbidden(err error) { + c.t.Helper() if err == nil { - c.t.Fatalf("\n%v: expected to fail as 403, but it does not", caller(0)) + c.t.Fatalf("expected to fail as 403, but it does not") } httpErr, ok := err.(HTTPError) if !ok || httpErr.Code != http.StatusForbidden { - c.t.Fatalf("\n%v: expected to fail as 403, but it failed as %v", caller(0), err) + c.t.Fatalf("expected to fail as 403, but it failed as %v", err) } } func (c *Ctx) expectEQ(got, want interface{}) { if diff := cmp.Diff(got, want); diff != "" { - c.t.Fatalf("\n%v: %v", caller(0), diff) + c.t.Helper() + c.t.Fatal(diff) } } func (c *Ctx) expectNE(got, want interface{}) { if reflect.DeepEqual(got, want) { - c.t.Fatalf("\n%v: equal: %#v", caller(0), got) + c.t.Helper() + c.t.Fatalf("equal: %#v", got) } } func (c *Ctx) expectTrue(v bool) { if !v { - c.t.Fatalf("\n%v: failed", caller(0)) + c.t.Helper() + c.t.Fatal("failed") } } @@ -280,19 +286,21 @@ func (c *Ctx) loadManager(ns, name string) (*Manager, *Build) { } func (c *Ctx) checkURLContents(url string, want []byte) { + c.t.Helper() got, err := c.AuthGET(AccessAdmin, url) if err != nil { - c.t.Fatalf("\n%v: %v request failed: %v", caller(0), url, err) + c.t.Fatalf("%v request failed: %v", url, err) } if !bytes.Equal(got, want) { - c.t.Fatalf("\n%v: url %v: got:\n%s\nwant:\n%s\n", caller(0), url, got, want) + c.t.Fatalf("url %v: got:\n%s\nwant:\n%s\n", url, got, want) } } func (c *Ctx) pollEmailBug() *aemail.Message { c.expectOK(c.GET("/email_poll")) if len(c.emailSink) == 0 { - c.t.Fatalf("\n%v: got no emails", caller(0)) + c.t.Helper() + c.t.Fatal("got no emails") } return <-c.emailSink } @@ -301,7 +309,8 @@ func (c *Ctx) expectNoEmail() { c.expectOK(c.GET("/email_poll")) if len(c.emailSink) != 0 { msg := <-c.emailSink - c.t.Fatalf("\n%v: got unexpected email: %v\n%s", caller(0), msg.Subject, msg.Body) + c.t.Helper() + c.t.Fatalf("got unexpected email: %v\n%s", msg.Subject, msg.Body) } } @@ -344,7 +353,8 @@ func (c *Ctx) makeClient(client, key string, failOnErrors bool) *apiClient { func (client *apiClient) pollBugs(expect int) []*dashapi.BugReport { resp, _ := client.ReportingPollBugs("test") if len(resp.Reports) != expect { - client.t.Fatalf("\n%v: want %v reports, got %v", caller(0), expect, len(resp.Reports)) + client.t.Helper() + client.t.Fatalf("want %v reports, got %v", expect, len(resp.Reports)) } for _, rep := range resp.Reports { reproLevel := dashapi.ReproLevelNone @@ -373,7 +383,8 @@ func (client *apiClient) pollBug() *dashapi.BugReport { func (client *apiClient) pollNotifs(expect int) []*dashapi.BugNotification { resp, _ := client.ReportingPollNotifications("test") if len(resp.Notifications) != expect { - client.t.Fatalf("\n%v: want %v notifs, got %v", caller(0), expect, len(resp.Notifications)) + client.t.Helper() + client.t.Fatalf("want %v notifs, got %v", expect, len(resp.Notifications)) } return resp.Notifications } diff --git a/dashboard/dashapi/dashapi.go b/dashboard/dashapi/dashapi.go index f6678a013..fe9a30e19 100644 --- a/dashboard/dashapi/dashapi.go +++ b/dashboard/dashapi/dashapi.go @@ -301,6 +301,7 @@ type BugReport struct { ExtID string // arbitrary reporting ID forwarded from BugUpdate.ExtID First bool // Set for first report for this bug (Type == ReportNew). Moderation bool + NoRepro bool // We don't expect repro (e.g. for build/boot errors). Title string Link string // link to the bug on dashboard CreditEmail string // email for the Reported-by tag -- cgit mrf-deployment