From 5b13635e40c669852847af72891fa982fc4aa667 Mon Sep 17 00:00:00 2001 From: Dmitry Vyukov Date: Thu, 14 Jan 2021 12:05:38 +0100 Subject: dashboard/app: allow CCing people for bug on specific manager Currently we can CC specific people for bugs on a particular kernel repo (always, upstream, for build bugs). Add similar functionality for bugs on a particular manager. This will be needed to CC people on clang instances. --- dashboard/app/app_test.go | 59 +++++++++++++++++--------- dashboard/app/bisect_test.go | 5 ++- dashboard/app/config.go | 26 ++++++++---- dashboard/app/email_test.go | 84 +++++++++++++++++++++++++++++++++++-- dashboard/app/entities.go | 13 ++++++ dashboard/app/jobs.go | 25 +++++------ dashboard/app/jobs_test.go | 9 ++-- dashboard/app/notifications_test.go | 2 +- dashboard/app/reporting.go | 70 ++++++++++++++++++++----------- 9 files changed, 218 insertions(+), 75 deletions(-) diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go index 3f80e5a16..f70206f02 100644 --- a/dashboard/app/app_test.go +++ b/dashboard/app/app_test.go @@ -60,16 +60,20 @@ var testConfig = &GlobalConfig{ }, Repos: []KernelRepo{ { - URL: "git://syzkaller.org", - Branch: "branch10", - Alias: "repo10alias", - Maintainers: []string{"maintainers@repo10.org", "bugs@repo10.org"}, + URL: "git://syzkaller.org", + Branch: "branch10", + Alias: "repo10alias", + CC: CCConfig{ + Maintainers: []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"}, + URL: "git://github.com/google/syzkaller", + Branch: "master", + Alias: "repo10alias", + CC: CCConfig{ + Maintainers: []string{"maintainers@repo10.org", "bugs@repo10.org"}, + }, }, }, Managers: map[string]ConfigManager{ @@ -105,28 +109,39 @@ var testConfig = &GlobalConfig{ }, Repos: []KernelRepo{ { - 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: "branch10", + Alias: "repo10alias", + CC: CCConfig{ + Always: []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", - Maintainers: []string{"maintainers@repo20.org", "bugs@repo20.org"}, + URL: "git://syzkaller.org", + Branch: "branch20", + Alias: "repo20", + CC: CCConfig{ + Maintainers: []string{"maintainers@repo20.org", "bugs@repo20.org"}, + }, }, }, Managers: map[string]ConfigManager{ - "restricted-manager": { + restrictedManager: { RestrictedTestingRepo: "git://restricted.git/restricted.git", RestrictedTestingReason: "you should test only on restricted.git", }, - "no-fix-bisection-manager": { + noFixBisectionManager: { FixBisectionDisabled: true, }, + specialCCManager: { + CC: CCConfig{ + Always: []string{"always@manager.org"}, + Maintainers: []string{"maintainers@manager.org"}, + BuildMaintainers: []string{"build-maintainers@manager.org"}, + }, + }, }, Reporting: []Reporting{ { @@ -254,6 +269,10 @@ const ( keyUser = "clientuserkeyclientuserkey" clientPublic = "client-public" keyPublic = "clientpublickeyclientpublickey" + + restrictedManager = "restricted-manager" + noFixBisectionManager = "no-fix-bisection-manager" + specialCCManager = "special-cc-manager" ) func skipWithRepro(bug *Bug) FilterResult { diff --git a/dashboard/app/bisect_test.go b/dashboard/app/bisect_test.go index 8db11a0ba..4da13b77b 100644 --- a/dashboard/app/bisect_test.go +++ b/dashboard/app/bisect_test.go @@ -74,7 +74,10 @@ func TestBisectCause(t *testing.T) { c.expectEQ(pollResp.KernelConfig, build.KernelConfig) c.expectEQ(pollResp.SyzkallerCommit, build.SyzkallerCommit) c.expectEQ(pollResp.ReproOpts, []byte("repro opts 3")) - c.expectEQ(pollResp.ReproSyz, []byte("syncfs(3)")) + c.expectEQ(pollResp.ReproSyz, []byte( + "# See https://goo.gl/kgGztJ for information about syzkaller reproducers.\n"+ + "#repro opts 3\n"+ + "syncfs(3)")) c.expectEQ(pollResp.ReproC, []byte("int main() { return 3; }")) // Since we did not reply, we should get the same response. diff --git a/dashboard/app/config.go b/dashboard/app/config.go index eba1f29a1..f4f4d1ac9 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -130,6 +130,8 @@ type ConfigManager struct { ObsoletingMaxPeriod time.Duration // Determines if fix bisection should be disabled on this manager. FixBisectionDisabled bool + // CC for all bugs that happened only on this manager. + CC CCConfig } // One reporting stage. @@ -170,8 +172,13 @@ type KernelRepo struct { // ReportingPriority says if we need to prefer to report crashes in this // repo over crashes in repos with lower value. Must be in [0-9] range. ReportingPriority int - // Additional CC list to add to all bugs reported on this repo. - CC []string + // CC for all bugs reported on this repo. + CC CCConfig +} + +type CCConfig struct { + // Additional CC list to add to bugs unconditionally. + Always []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. @@ -346,11 +353,15 @@ 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)) } - 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)) - } + checkCC(&repo.CC) + } +} + +func checkCC(cc *CCConfig) { + emails := append(append(append([]string{}, cc.Always...), cc.Maintainers...), cc.BuildMaintainers...) + for _, email := range emails { + if _, err := mail.ParseAddress(email); err != nil { + panic(fmt.Sprintf("bad email address %q: %v", email, err)) } } } @@ -420,6 +431,7 @@ func checkManager(ns, name string, mgr ConfigManager) { if mgr.ObsoletingMinPeriod != 0 && mgr.ObsoletingMinPeriod < 24*time.Hour { panic(fmt.Sprintf("manager %v/%v obsoleting: too low MinPeriod", ns, name)) } + checkCC(&mgr.CC) } func checkKcidb(ns string, kcidb *KcidbConfig) { diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index e3eae3944..7e558a28a 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -406,7 +406,6 @@ func TestEmailDup(t *testing.T) { crash1.Title = "KASAN: another title" c.client2.ReportCrash(crash2) - c.expectOK(c.GET("/email_poll")) msg1 := c.pollEmailBug() msg2 := c.pollEmailBug() @@ -449,7 +448,6 @@ func TestEmailUndup(t *testing.T) { crash1.Title = "KASAN: another title" c.client2.ReportCrash(crash2) - c.expectOK(c.GET("/email_poll")) msg1 := c.pollEmailBug() msg2 := c.pollEmailBug() @@ -632,7 +630,6 @@ func TestEmailUnfix(t *testing.T) { crash := testCrash(build, 1) c.client2.ReportCrash(crash) - c.expectOK(c.GET("/email_poll")) msg := c.pollEmailBug() c.incomingEmail(msg.Sender, "#syz fix: some commit") @@ -649,3 +646,84 @@ func TestEmailUnfix(t *testing.T) { c.client2.ReportCrash(crash) c.expectNoEmail() } + +func TestEmailManagerCC(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + // Test that we add manager CC. + build1 := testBuild(1) + build1.Manager = specialCCManager + c.client2.UploadBuild(build1) + + crash := testCrash(build1, 1) + c.client2.ReportCrash(crash) + + msg := c.pollEmailBug() + c.expectEQ(msg.To, []string{ + "always@manager.org", + "test@syzkaller.com", + }) + + // Test that we add manager maintainers. + c.incomingEmail(msg.Sender, "#syz upstream") + msg = c.pollEmailBug() + c.expectEQ(msg.To, []string{ + "always@manager.org", + "bugs@syzkaller.com", + "default@maintainers.com", + "maintainers@manager.org", + }) + + // Test that we add manager build maintainers. + build2 := testBuild(2) + build2.Manager = specialCCManager + buildErrorReq := &dashapi.BuildErrorReq{ + Build: *build2, + Crash: dashapi.Crash{ + Title: "failed build 1", + Report: []byte("report\n"), + Log: []byte("log\n"), + }, + } + c.expectOK(c.client2.ReportBuildError(buildErrorReq)) + msg = c.pollEmailBug() + c.expectEQ(msg.To, []string{ + "always@manager.org", + "test@syzkaller.com", + }) + + c.incomingEmail(msg.Sender, "#syz upstream") + msg = c.pollEmailBug() + c.expectEQ(msg.To, []string{ + "always@manager.org", + "bugs@syzkaller.com", + "build-maintainers@manager.org", + "default@maintainers.com", + "maintainers@manager.org", + }) + + // Test that we don't add manager CC when the crash happened on 1+ managers. + build3 := testBuild(3) + build1.Manager = specialCCManager + c.client2.UploadBuild(build3) + crash = testCrash(build3, 2) + c.client2.ReportCrash(crash) + + build4 := testBuild(4) + c.client2.UploadBuild(build4) + crash = testCrash(build4, 2) + c.client2.ReportCrash(crash) + + msg = c.pollEmailBug() + c.expectEQ(msg.To, []string{ + "test@syzkaller.com", + }) + + c.incomingEmail(msg.Sender, "#syz upstream") + msg = c.pollEmailBug() + c.expectEQ(msg.To, []string{ + "bugs@syzkaller.com", + "default@maintainers.com", + }) +} diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go index d5558fde6..efb248987 100644 --- a/dashboard/app/entities.go +++ b/dashboard/app/entities.go @@ -204,6 +204,19 @@ const ( JobBisectFix ) +func (typ JobType) toDashapiReportType() dashapi.ReportType { + switch typ { + case JobTestPatch: + return dashapi.ReportTestPatch + case JobBisectCause: + return dashapi.ReportBisectCause + case JobBisectFix: + return dashapi.ReportBisectFix + default: + panic(fmt.Sprintf("unknown job type %v", typ)) + } +} + type JobFlags int64 const ( diff --git a/dashboard/app/jobs.go b/dashboard/app/jobs.go index af20f75e1..938444b89 100644 --- a/dashboard/app/jobs.go +++ b/dashboard/app/jobs.go @@ -393,7 +393,7 @@ func createJobResp(c context.Context, job *Job, jobKey *db.Key) (*dashapi.JobPol if err != nil { return nil, false, err } - reproSyz, _, err := getText(c, textReproSyz, crash.ReproSyz) + reproSyz, err := loadReproSyz(c, crash) if err != nil { return nil, false, err } @@ -661,24 +661,13 @@ func createBugReportForJob(c context.Context, job *Job, jobKey *db.Key, config i if bugReporting == nil { return nil, fmt.Errorf("job bug has no reporting %q", job.Reporting) } - var typ dashapi.ReportType - switch job.Type { - case JobTestPatch: - typ = dashapi.ReportTestPatch - case JobBisectCause: - typ = dashapi.ReportBisectCause - case JobBisectFix: - typ = dashapi.ReportBisectFix - default: - return nil, fmt.Errorf("unknown job type %v", job.Type) - } kernelRepo := kernelRepoInfo(build) rep := &dashapi.BugReport{ - Type: typ, + Type: job.Type.toDashapiReportType(), Config: reportingConfig, JobID: extJobID(jobKey), ExtID: job.ExtID, - CC: append(job.CC, kernelRepo.CC...), + CC: append(job.CC, kernelRepo.CC.Always...), Log: crashLog, LogLink: externalLink(c, textCrashLog, job.CrashLog), Report: report, @@ -693,7 +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 { - rep.Maintainers = append(crash.Maintainers, kernelRepo.Maintainers...) + rep.Maintainers = append(crash.Maintainers, kernelRepo.CC.Maintainers...) rep.ExtID = bugReporting.ExtID if bugReporting.CC != "" { rep.CC = strings.Split(bugReporting.CC, "|") @@ -705,6 +694,12 @@ func createBugReportForJob(c context.Context, job *Job, jobKey *db.Key, config i rep.BisectFix = bisectFromJob(c, rep, job) } } + if mgr := bug.managerConfig(); mgr != nil { + rep.CC = append(rep.CC, mgr.CC.Always...) + if job.Type == JobBisectCause || job.Type == JobBisectFix { + rep.Maintainers = append(rep.Maintainers, mgr.CC.Maintainers...) + } + } // Build error output and failing VM boot log can be way too long to inline. if len(rep.Error) > maxInlineError { rep.Error = rep.Error[len(rep.Error)-maxInlineError:] diff --git a/dashboard/app/jobs_test.go b/dashboard/app/jobs_test.go index 63827e360..902a4d152 100644 --- a/dashboard/app/jobs_test.go +++ b/dashboard/app/jobs_test.go @@ -103,7 +103,10 @@ func TestJob(t *testing.T) { c.expectEQ(pollResp.SyzkallerCommit, build.SyzkallerCommit) c.expectEQ(pollResp.Patch, []byte(patch)) c.expectEQ(pollResp.ReproOpts, []byte("repro opts")) - c.expectEQ(pollResp.ReproSyz, []byte("repro syz")) + c.expectEQ(pollResp.ReproSyz, []byte( + "# See https://goo.gl/kgGztJ for information about syzkaller reproducers.\n"+ + "#repro opts\n"+ + "repro syz")) c.expectEQ(pollResp.ReproC, []byte("repro C")) pollResp2 := c.client2.pollJobs(build.Manager) @@ -337,7 +340,7 @@ func TestJobRestrictedManager(t *testing.T) { defer c.Close() build := testBuild(1) - build.Manager = "restricted-manager" + build.Manager = restrictedManager c.client2.UploadBuild(build) crash := testCrash(build, 1) @@ -650,7 +653,7 @@ func TestFixBisectionsDisabled(t *testing.T) { // Upload a crash report. build := testBuild(1) - build.Manager = "no-fix-bisection-manager" + build.Manager = noFixBisectionManager c.client2.UploadBuild(build) crash := testCrashWithRepro(build, 20) c.client2.ReportCrash(crash) diff --git a/dashboard/app/notifications_test.go b/dashboard/app/notifications_test.go index 593f99c01..619b150ee 100644 --- a/dashboard/app/notifications_test.go +++ b/dashboard/app/notifications_test.go @@ -290,7 +290,7 @@ func TestEmailNotifObsoletedManager(t *testing.T) { defer c.Close() build := testBuild(1) - build.Manager = "no-fix-bisection-manager" + build.Manager = noFixBisectionManager c.client2.UploadBuild(build) crash := testCrashWithRepro(build, 1) c.client2.ReportCrash(crash) diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go index 9c8943993..0b6e5083a 100644 --- a/dashboard/app/reporting.go +++ b/dashboard/app/reporting.go @@ -267,11 +267,8 @@ func (bug *Bug) obsoletePeriod() time.Duration { bug.Reporting[len(bug.Reporting)-1].Reported.IsZero() { min, max = config.Obsoleting.NonFinalMinPeriod, config.Obsoleting.NonFinalMaxPeriod } - if len(bug.HappenedOn) == 1 { - mgr := config.Namespaces[bug.Namespace].Managers[bug.HappenedOn[0]] - if mgr.ObsoletingMinPeriod != 0 { - min, max = mgr.ObsoletingMinPeriod, mgr.ObsoletingMaxPeriod - } + if mgr := bug.managerConfig(); mgr != nil && mgr.ObsoletingMinPeriod != 0 { + min, max = mgr.ObsoletingMinPeriod, mgr.ObsoletingMaxPeriod } if period < min { period = min @@ -282,6 +279,14 @@ func (bug *Bug) obsoletePeriod() time.Duration { return period } +func (bug *Bug) managerConfig() *ConfigManager { + if len(bug.HappenedOn) != 1 { + return nil + } + mgr := config.Namespaces[bug.Namespace].Managers[bug.HappenedOn[0]] + return &mgr +} + func createNotification(c context.Context, typ dashapi.BugNotif, public bool, text string, bug *Bug, reporting *Reporting, bugReporting *BugReporting) (*dashapi.BugNotification, error) { reportingConfig, err := json.Marshal(reporting.Config) @@ -306,14 +311,20 @@ func createNotification(c context.Context, typ dashapi.BugNotif, public bool, te Title: bug.displayTitle(), Text: text, Public: public, - CC: kernelRepo.CC, + CC: kernelRepo.CC.Always, } if public { - notif.Maintainers = append(crash.Maintainers, kernelRepo.Maintainers...) + notif.Maintainers = append(crash.Maintainers, kernelRepo.CC.Maintainers...) } if (public || reporting.moderation) && bugReporting.CC != "" { notif.CC = append(notif.CC, strings.Split(bugReporting.CC, "|")...) } + if mgr := bug.managerConfig(); mgr != nil { + notif.CC = append(notif.CC, mgr.CC.Always...) + if public { + notif.Maintainers = append(notif.Maintainers, mgr.CC.Maintainers...) + } + } return notif, nil } @@ -363,10 +374,7 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key return nil, err } var job *Job - reportBisection := bug.BisectCause == BisectYes || - bug.BisectCause == BisectInconclusive || - bug.BisectCause == BisectHorizont - if reportBisection { + if bug.BisectCause == BisectYes || bug.BisectCause == BisectInconclusive || bug.BisectCause == BisectHorizont { // If we have bisection results, report the crash/repro used for bisection. job1, crash1, _, crashKey1, err := loadBisectJob(c, bug, JobBisectCause) if err != nil { @@ -397,7 +405,7 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key if err != nil { return nil, err } - reproSyz, _, err := getText(c, textReproSyz, crash.ReproSyz) + reproSyz, err := loadReproSyz(c, crash) if err != nil { return nil, err } @@ -405,15 +413,6 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key if err != nil { return nil, err } - if len(reproSyz) != 0 { - buf := new(bytes.Buffer) - buf.WriteString(syzReproPrefix) - if len(crash.ReproOpts) != 0 { - fmt.Fprintf(buf, "#%s\n", crash.ReproOpts) - } - buf.Write(reproSyz) - reproSyz = buf.Bytes() - } build, err := loadBuild(c, bug.Namespace, crash.BuildID) if err != nil { return nil, err @@ -434,8 +433,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), - CC: kernelRepo.CC, - Maintainers: append(crash.Maintainers, kernelRepo.Maintainers...), + CC: kernelRepo.CC.Always, + Maintainers: append(crash.Maintainers, kernelRepo.CC.Maintainers...), ReproC: reproC, ReproCLink: externalLink(c, textReproC, crash.ReproC), ReproSyz: reproSyz, @@ -452,9 +451,16 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key rep.CC = append(rep.CC, strings.Split(bugReporting.CC, "|")...) } if build.Type == BuildFailed { - rep.Maintainers = append(rep.Maintainers, kernelRepo.BuildMaintainers...) + rep.Maintainers = append(rep.Maintainers, kernelRepo.CC.BuildMaintainers...) + } + if mgr := bug.managerConfig(); mgr != nil { + rep.CC = append(rep.CC, mgr.CC.Always...) + rep.Maintainers = append(rep.Maintainers, mgr.CC.Maintainers...) + if build.Type == BuildFailed { + rep.Maintainers = append(rep.Maintainers, mgr.CC.BuildMaintainers...) + } } - if reportBisection { + if job != nil { rep.BisectCause = bisectFromJob(c, rep, job) } if err := fillBugReport(c, rep, bug, bugReporting, build); err != nil { @@ -463,6 +469,20 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *db.Key return rep, nil } +func loadReproSyz(c context.Context, crash *Crash) ([]byte, error) { + reproSyz, _, err := getText(c, textReproSyz, crash.ReproSyz) + if err != nil || len(reproSyz) == 0 { + return nil, err + } + buf := new(bytes.Buffer) + buf.WriteString(syzReproPrefix) + if len(crash.ReproOpts) != 0 { + fmt.Fprintf(buf, "#%s\n", crash.ReproOpts) + } + buf.Write(reproSyz) + return buf.Bytes(), nil +} + // fillBugReport fills common report fields for bug and job reports. func fillBugReport(c context.Context, rep *dashapi.BugReport, bug *Bug, bugReporting *BugReporting, build *Build) error { -- cgit mrf-deployment