diff options
| author | Dmitry Vyukov <dvyukov@google.com> | 2019-02-21 18:18:18 +0100 |
|---|---|---|
| committer | Dmitry Vyukov <dvyukov@google.com> | 2019-02-22 11:42:54 +0100 |
| commit | 6a5fcca423a42e14346de8637cc30d79530bf034 (patch) | |
| tree | 3ba2ed78cc94936b42e2b4e36898947a3d1869c3 | |
| parent | 7ff74a98320355d2a8c097333827b6565dbf64b9 (diff) | |
dashboard/app: implement bug notifications/actions
Currently dashboard can only report new bugs and add reproducers
to already reported bugs.
This change adds infrastructure for the dashboard to actively act
on existing bugs in different ways. 4 new notifications (actions) added:
- dashboard can auto-upstream bugs from moderation after an embargo period
- dashboard can auto-upstream bugs if reporting criteria changes
(e.g. it reported a bug into moderation because there was no repro,
but then repro appears and the bug is automatically sent upstream)
- dashboard detects when a fixing commit does not appear in any tested trees
for too long and sends a notification about this
- dashboard detects stale bugs (last happened monts ago, no repro, no activity)
and auto-invalidates them
This will also be useful to send pings for old bugs and do other automation.
| -rw-r--r-- | dashboard/app/api.go | 1 | ||||
| -rw-r--r-- | dashboard/app/app_test.go | 22 | ||||
| -rw-r--r-- | dashboard/app/config.go | 9 | ||||
| -rw-r--r-- | dashboard/app/email_test.go | 10 | ||||
| -rw-r--r-- | dashboard/app/entities.go | 2 | ||||
| -rw-r--r-- | dashboard/app/main.go | 3 | ||||
| -rw-r--r-- | dashboard/app/notifications_test.go | 269 | ||||
| -rw-r--r-- | dashboard/app/reporting.go | 138 | ||||
| -rw-r--r-- | dashboard/app/reporting_email.go | 141 | ||||
| -rw-r--r-- | dashboard/app/reporting_external.go | 12 | ||||
| -rw-r--r-- | dashboard/app/reporting_test.go | 7 | ||||
| -rw-r--r-- | dashboard/app/util_test.go | 21 | ||||
| -rw-r--r-- | dashboard/dashapi/dashapi.go | 69 |
13 files changed, 630 insertions, 74 deletions
diff --git a/dashboard/app/api.go b/dashboard/app/api.go index c37afb404..022e94963 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -34,6 +34,7 @@ var apiHandlers = map[string]APIHandler{ "job_poll": apiJobPoll, "job_done": apiJobDone, "reporting_poll_bugs": apiReportingPollBugs, + "reporting_poll_notifs": apiReportingPollNotifications, "reporting_poll_closed": apiReportingPollClosed, "reporting_update": apiReportingUpdate, } diff --git a/dashboard/app/app_test.go b/dashboard/app/app_test.go index 63822d9e4..3987b024d 100644 --- a/dashboard/app/app_test.go +++ b/dashboard/app/app_test.go @@ -55,16 +55,11 @@ var testConfig = &GlobalConfig{ { Name: "reporting1", DailyLimit: 3, + Embargo: 14 * 24 * time.Hour, + Filter: skipWithRepro, Config: &TestConfig{ Index: 1, }, - Filter: func(bug *Bug) FilterResult { - if strings.HasPrefix(bug.Title, "skip without repro") && - bug.ReproLevel != dashapi.ReproLevelNone { - return FilterSkip - } - return FilterReport - }, }, { Name: "reporting2", @@ -99,9 +94,10 @@ var testConfig = &GlobalConfig{ { Name: "reporting1", DailyLimit: 5, + Embargo: 14 * 24 * time.Hour, + Filter: skipWithRepro, Config: &EmailConfig{ - Email: "test@syzkaller.com", - Moderation: true, + Email: "test@syzkaller.com", }, }, { @@ -215,6 +211,14 @@ const ( keyPublic = "clientpublickeyclientpublickey" ) +func skipWithRepro(bug *Bug) FilterResult { + if strings.HasPrefix(bug.Title, "skip with repro") && + bug.ReproLevel != dashapi.ReproLevelNone { + return FilterSkip + } + return FilterReport +} + type TestConfig struct { Index int } diff --git a/dashboard/app/config.go b/dashboard/app/config.go index b72c180c7..4b8a70a09 100644 --- a/dashboard/app/config.go +++ b/dashboard/app/config.go @@ -102,10 +102,15 @@ type Reporting struct { Filter ReportingFilter // How many new bugs report per day. DailyLimit int + // Upstream reports into next reporting after this period. + Embargo time.Duration // Type of reporting and its configuration. // The app has one built-in type, EmailConfig, which reports bugs by email. // And ExternalConfig which can be used to attach any external reporting system (e.g. Bugzilla). Config ReportingType + + // Set for all but last reporting stages. + moderation bool } type ReportingType interface { @@ -270,6 +275,10 @@ func checkNamespaceReporting(ns string, cfg *Config) { if reporting.DisplayTitle == "" { reporting.DisplayTitle = reporting.Name } + reporting.moderation = ri < len(cfg.Reporting)-1 + if !reporting.moderation && reporting.Embargo != 0 { + panic(fmt.Sprintf("embargo in the last reporting %v", reporting.Name)) + } checkConfigAccessLevel(&reporting.AccessLevel, parentAccessLevel, fmt.Sprintf("reporting %q/%q", ns, reporting.Name)) parentAccessLevel = reporting.AccessLevel diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index 557c4cccf..3cc0cb5f4 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -214,6 +214,7 @@ kernel config: %[4]v dashboard link: https://testapp.appspot.com/bug?extid=%[1]v compiler: compiler10 syz repro: %[2]v +CC: [bar@foo.com foo@bar.com maintainers@repo10.org bugs@repo10.org] IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+%[1]v@testapp.appspotmail.com @@ -289,6 +290,7 @@ dashboard link: https://testapp.appspot.com/bug?extid=%[1]v compiler: compiler10 syz repro: %[3]v C reproducer: %[2]v +CC: [qux@qux.com maintainers@repo10.org bugs@repo10.org] IMPORTANT: if you fix the bug, please add the following tag to the commit: Reported-by: syzbot+%[1]v@testapp.appspotmail.com @@ -508,19 +510,19 @@ func TestEmailCrossReportingDup(t *testing.T) { crash1 := testCrash(build, 1) crash1.Title = fmt.Sprintf("bug_%v", i) c.client2.ReportCrash(crash1) - bugSender := c.pollEmailBug() + bugSender := c.pollEmailBug().Sender for j := 0; j < test.bug; j++ { c.incomingEmail(bugSender, "#syz upstream") - bugSender = c.pollEmailBug() + bugSender = c.pollEmailBug().Sender } crash2 := testCrash(build, 2) crash2.Title = fmt.Sprintf("dup_%v", i) c.client2.ReportCrash(crash2) - dupSender := c.pollEmailBug() + dupSender := c.pollEmailBug().Sender for j := 0; j < test.dup; j++ { c.incomingEmail(dupSender, "#syz upstream") - dupSender = c.pollEmailBug() + dupSender = c.pollEmailBug().Sender } c.incomingEmail(bugSender, "#syz dup: "+crash2.Title) diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go index 705f4a26d..f68b641a9 100644 --- a/dashboard/app/entities.go +++ b/dashboard/app/entities.go @@ -102,7 +102,9 @@ type BugReporting struct { Link string CC string // additional emails added to CC list (|-delimited list) CrashID int64 // crash that we've last reported in this reporting + Auto bool // was it auto-upstreamed/obsoleted? ReproLevel dashapi.ReproLevel + OnHold time.Time // if set, the bug must not be upstreamed Reported time.Time Closed time.Time } diff --git a/dashboard/app/main.go b/dashboard/app/main.go index 8c3f624c8..3d704fc3a 100644 --- a/dashboard/app/main.go +++ b/dashboard/app/main.go @@ -598,6 +598,9 @@ func createUIBug(c context.Context, bug *Bug, state *ReportingState, managers [] switch bug.Status { case BugStatusInvalid: status = "closed as invalid" + if bugReporting.Auto { + status = "auto-" + status + } case BugStatusFixed: status = "fixed" case BugStatusDup: diff --git a/dashboard/app/notifications_test.go b/dashboard/app/notifications_test.go new file mode 100644 index 000000000..e6a7b6ba6 --- /dev/null +++ b/dashboard/app/notifications_test.go @@ -0,0 +1,269 @@ +// Copyright 2019 syzkaller project authors. All rights reserved. +// Use of this source code is governed by Apache 2 LICENSE that can be found in the LICENSE file. + +// +build aetest + +package dash + +import ( + "strings" + "testing" + "time" + + "github.com/google/syzkaller/dashboard/dashapi" +) + +func TestEmailNotifUpstreamEmbargo(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build := testBuild(1) + c.client2.UploadBuild(build) + + crash := testCrash(build, 1) + c.client2.ReportCrash(crash) + report := c.pollEmailBug() + c.expectEQ(report.To, []string{"test@syzkaller.com"}) + + // Upstreaming happens after 14 days, so no emails yet. + c.advanceTime(13 * 24 * time.Hour) + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + + // Now we should get notification about upstreaming and upstream report: + c.advanceTime(2 * 24 * time.Hour) + notifUpstream := c.pollEmailBug() + upstreamReport := c.pollEmailBug() + c.expectEQ(notifUpstream.Sender, report.Sender) + c.expectEQ(notifUpstream.Body, "Sending this report upstream.") + if upstreamReport.Sender == report.Sender { + t.Fatalf("upstream report from the same sender") + } + c.expectEQ(upstreamReport.To, []string{"bugs@syzkaller.com", "default@maintainers.com"}) +} + +func TestEmailNotifUpstreamSkip(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build := testBuild(1) + c.client2.UploadBuild(build) + + crash := testCrash(build, 1) + crash.Title = "skip with repro 1" + c.client2.ReportCrash(crash) + report := c.pollEmailBug() + c.expectEQ(report.To, []string{"test@syzkaller.com"}) + + // No emails yet. + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + + // Now upload repro and it should be auto-upstreamed. + crash.ReproOpts = []byte("repro opts") + crash.ReproSyz = []byte("getpid()") + c.client2.ReportCrash(crash) + notifUpstream := c.pollEmailBug() + upstreamReport := c.pollEmailBug() + c.expectEQ(notifUpstream.Sender, report.Sender) + c.expectEQ(notifUpstream.Body, "Sending this report upstream.") + if upstreamReport.Sender == report.Sender { + t.Fatalf("upstream report from the same sender") + } + c.expectEQ(upstreamReport.To, []string{"bugs@syzkaller.com", "default@maintainers.com"}) +} + +func TestEmailNotifBadFix(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build := testBuild(1) + c.client2.UploadBuild(build) + + crash := testCrash(build, 1) + c.client2.ReportCrash(crash) + report := c.pollEmailBug() + c.expectEQ(report.To, []string{"test@syzkaller.com"}) + + c.incomingEmail(report.Sender, "#syz fix: some: commit title") + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + + // Notification about bad fixing commit should be send after 90 days. + c.advanceTime(50 * 24 * time.Hour) + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + c.advanceTime(35 * 24 * time.Hour) + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + c.advanceTime(10 * 24 * time.Hour) + notif := c.pollEmailBug() + if !strings.Contains(notif.Body, "This bug is marked as fixed by commit:\nsome: commit title\n") { + t.Fatalf("bad notification text: %q", notif.Body) + } + // No notifications for another 14 days, then another one. + c.advanceTime(13 * 24 * time.Hour) + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + c.advanceTime(2 * 24 * time.Hour) + notif = c.pollEmailBug() + if !strings.Contains(notif.Body, "This bug is marked as fixed by commit:\nsome: commit title\n") { + t.Fatalf("bad notification text: %q", notif.Body) + } +} + +func TestEmailNotifObsoleted(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build := testBuild(1) + c.client2.UploadBuild(build) + + crash := testCrash(build, 1) + crash.Maintainers = []string{"maintainer@syzkaller.com"} + c.client2.ReportCrash(crash) + report := c.pollEmailBug() + // Need to upstream so that it's not auto-upstreamed before obsoleted. + c.incomingEmail(report.Sender, "#syz upstream") + report = c.pollEmailBug() + // Add more people to bug CC. + c.incomingEmail(report.Sender, "wow", EmailOptCC([]string{"somebody@else.com"})) + + // Bug is open, new crashes don't create new bug. + c.client2.ReportCrash(crash) + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + + // Not yet. + c.advanceTime(179 * 24 * time.Hour) + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + + // Now! + c.advanceTime(2 * 24 * time.Hour) + notif := c.pollEmailBug() + if !strings.Contains(notif.Body, "Auto-closing this bug as obsolete") { + t.Fatalf("bad notification text: %q", notif.Body) + } + c.expectEQ(notif.To, []string{"bugs@syzkaller.com", "default@sender.com", "somebody@else.com"}) + + // New crash must create new bug. + c.client2.ReportCrash(crash) + report = c.pollEmailBug() + c.expectEQ(report.Subject, "title1 (2)") + // Now the same, but for the last reporting (must have smaller CC list). + c.incomingEmail(report.Sender, "#syz upstream") + report = c.pollEmailBug() + c.incomingEmail(report.Sender, "#syz upstream") + report = c.pollEmailBug() + + c.advanceTime(181 * 24 * time.Hour) + notif = c.pollEmailBug() + if !strings.Contains(notif.Body, "Auto-closing this bug as obsolete") { + t.Fatalf("bad notification text: %q", notif.Body) + } + c.expectEQ(notif.To, []string{"bugs@syzkaller.com"}) +} + +func TestEmailNotifNotObsoleted(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build := testBuild(1) + c.client2.UploadBuild(build) + + // Crashes with repro are not auto-obsoleted. + crash1 := testCrash(build, 1) + crash1.ReproSyz = []byte("repro") + c.client2.ReportCrash(crash1) + report1 := c.pollEmailBug() + c.incomingEmail(report1.Sender, "#syz upstream") + report1 = c.pollEmailBug() + + // This crash will get another crash later. + crash2 := testCrash(build, 2) + c.client2.ReportCrash(crash2) + report2 := c.pollEmailBug() + c.incomingEmail(report2.Sender, "#syz upstream") + report2 = c.pollEmailBug() + + // This crash will get some activity later. + crash3 := testCrash(build, 3) + c.client2.ReportCrash(crash3) + report3 := c.pollEmailBug() + c.incomingEmail(report3.Sender, "#syz upstream") + report3 = c.pollEmailBug() + + // This will be obsoleted (just to check that we have timings right). + c.advanceTime(24 * time.Hour) + crash4 := testCrash(build, 4) + c.client2.ReportCrash(crash4) + report4 := c.pollEmailBug() + c.incomingEmail(report4.Sender, "#syz upstream") + report4 = c.pollEmailBug() + + c.advanceTime(179 * 24 * time.Hour) + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + + c.client2.ReportCrash(crash2) + c.incomingEmail(report3.Sender, "I am looking at it") + + c.advanceTime(5 * 24 * time.Hour) + // Only crash 4 is obsoleted. + notif := c.pollEmailBug() + c.expectEQ(notif.Sender, report4.Sender) + c.expectOK(c.GET("/email_poll")) + c.expectEQ(len(c.emailSink), 0) + + // Crash 3 also obsoleted after some time. + c.advanceTime(20 * 24 * time.Hour) + notif = c.pollEmailBug() + c.expectEQ(notif.Sender, report3.Sender) +} + +func TestExtNotifUpstreamEmbargo(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build1 := testBuild(1) + c.client.UploadBuild(build1) + + crash1 := testCrash(build1, 1) + c.client.ReportCrash(crash1) + rep := c.client.pollBug() + + // Specify fixing commit for the bug. + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ + ID: rep.ID, + Status: dashapi.BugStatusOpen, + }) + c.expectEQ(reply.OK, true) + c.client.pollNotifs(0) + c.advanceTime(20 * 24 * time.Hour) + notif := c.client.pollNotifs(1)[0] + c.expectEQ(notif.ID, rep.ID) + c.expectEQ(notif.Type, dashapi.BugNotifUpstream) +} + +func TestExtNotifUpstreamOnHold(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + build1 := testBuild(1) + c.client.UploadBuild(build1) + + crash1 := testCrash(build1, 1) + c.client.ReportCrash(crash1) + rep := c.client.pollBug() + + // Specify fixing commit for the bug. + reply, _ := c.client.ReportingUpdate(&dashapi.BugUpdate{ + ID: rep.ID, + Status: dashapi.BugStatusOpen, + OnHold: true, + }) + c.expectEQ(reply.OK, true) + c.advanceTime(20 * 24 * time.Hour) + c.client.pollNotifs(0) +} diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go index 183ff5a75..a75372413 100644 --- a/dashboard/app/reporting.go +++ b/dashboard/app/reporting.go @@ -26,10 +26,13 @@ import ( // - incomingCommand is called by backends to update bug statuses. const ( - maxMailLogLen = 1 << 20 - maxMailReportLen = 64 << 10 - maxInlineError = 16 << 10 - internalError = "internal error" + maxMailLogLen = 1 << 20 + maxMailReportLen = 64 << 10 + maxInlineError = 16 << 10 + notifyResendPeriod = 14 * 24 * time.Hour + notifyAboutBadCommitPeriod = 90 * 24 * time.Hour + autoObsoletePeriod = 180 * 24 * time.Hour + internalError = "internal error" // This is embedded as first line of syzkaller reproducer files. syzReproPrefix = "# See https://goo.gl/kgGztJ for information about syzkaller reproducers.\n" ) @@ -62,9 +65,6 @@ func reportingPollBugs(c context.Context, typ string) []*dashapi.BugReport { continue } reports = append(reports, rep) - if len(reports) > 50 { - break // temp measure during the jam - } } return reports } @@ -74,7 +74,7 @@ func handleReportBug(c context.Context, typ string, state *ReportingState, bug * if err != nil || reporting == nil { return nil, err } - rep, err := createBugReport(c, bug, crash, crashKey, bugReporting, reporting.Config) + rep, err := createBugReport(c, bug, crash, crashKey, bugReporting, reporting) if err != nil { return nil, err } @@ -150,6 +150,113 @@ func needReport(c context.Context, typ string, state *ReportingState, bug *Bug) return } +func reportingPollNotifications(c context.Context, typ string) []*dashapi.BugNotification { + var bugs []*Bug + _, err := datastore.NewQuery("Bug"). + Filter("Status<", BugStatusFixed). + GetAll(c, &bugs) + if err != nil { + log.Errorf(c, "%v", err) + return nil + } + log.Infof(c, "fetched %v bugs", len(bugs)) + var notifs []*dashapi.BugNotification + for _, bug := range bugs { + notif, err := handleReportNotif(c, typ, bug) + if err != nil { + log.Errorf(c, "%v: failed to create bug notif %v: %v", bug.Namespace, bug.Title, err) + continue + } + if notif == nil { + continue + } + notifs = append(notifs, notif) + if len(notifs) >= 10 { + break // don't send too many at once just in case + } + } + return notifs +} + +func handleReportNotif(c context.Context, typ string, bug *Bug) (*dashapi.BugNotification, error) { + reporting, bugReporting, _, _, err := currentReporting(c, bug) + if err != nil || reporting == nil { + return nil, nil + } + if typ != "" && typ != reporting.Config.Type() { + return nil, nil + } + if bug.Status != BugStatusOpen || bugReporting.Reported.IsZero() { + return nil, nil + } + + if reporting.moderation && + reporting.Embargo != 0 && + len(bug.Commits) == 0 && + bugReporting.OnHold.IsZero() && + timeSince(c, bugReporting.Reported) > reporting.Embargo { + log.Infof(c, "%v: upstreaming (embargo): %v", bug.Namespace, bug.Title) + return createNotification(c, dashapi.BugNotifUpstream, true, "", bug, reporting, bugReporting) + } + if reporting.moderation && + len(bug.Commits) == 0 && + bugReporting.OnHold.IsZero() && + reporting.Filter(bug) == FilterSkip { + log.Infof(c, "%v: upstreaming (skip): %v", bug.Namespace, bug.Title) + return createNotification(c, dashapi.BugNotifUpstream, true, "", bug, reporting, bugReporting) + } + if len(bug.Commits) == 0 && + bug.ReproLevel == ReproLevelNone && + timeSince(c, bug.LastActivity) > notifyResendPeriod && + timeSince(c, bug.LastTime) > autoObsoletePeriod { + log.Infof(c, "%v: obsoleting: %v", bug.Namespace, bug.Title) + return createNotification(c, dashapi.BugNotifObsoleted, false, "", bug, reporting, bugReporting) + } + if len(bug.Commits) > 0 && + len(bug.PatchedOn) == 0 && + timeSince(c, bug.LastActivity) > notifyResendPeriod && + timeSince(c, bug.FixTime) > notifyAboutBadCommitPeriod { + log.Infof(c, "%v: bad fix commit: %v", bug.Namespace, bug.Title) + commits := strings.Join(bug.Commits, "\n") + return createNotification(c, dashapi.BugNotifBadCommit, true, commits, bug, reporting, bugReporting) + } + return nil, nil +} + +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) + if err != nil { + return nil, err + } + crash, _, err := findCrashForBug(c, bug) + if err != nil { + return nil, fmt.Errorf("no crashes for bug") + } + build, err := loadBuild(c, bug.Namespace, crash.BuildID) + if err != nil { + return nil, err + } + kernelRepo := kernelRepoInfo(build) + notif := &dashapi.BugNotification{ + Type: typ, + Namespace: bug.Namespace, + Config: reportingConfig, + ID: bugReporting.ID, + ExtID: bugReporting.ExtID, + Title: bug.displayTitle(), + Text: text, + Public: public, + } + if public { + notif.Maintainers = append(crash.Maintainers, kernelRepo.CC...) + } + if (public || reporting.moderation) && bugReporting.CC != "" { + notif.CC = strings.Split(bugReporting.CC, "|") + } + return notif, nil +} + func currentReporting(c context.Context, bug *Bug) (*Reporting, *BugReporting, int, string, error) { for i := range bug.Reporting { bugReporting := &bug.Reporting[i] @@ -187,8 +294,8 @@ func reproStr(level dashapi.ReproLevel) string { } func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *datastore.Key, - bugReporting *BugReporting, config interface{}) (*dashapi.BugReport, error) { - reportingConfig, err := json.Marshal(config) + bugReporting *BugReporting, reporting *Reporting) (*dashapi.BugReport, error) { + reportingConfig, err := json.Marshal(reporting.Config) if err != nil { return nil, err } @@ -239,6 +346,7 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *datast ID: bugReporting.ID, ExtID: bugReporting.ExtID, First: bugReporting.Reported.IsZero(), + Moderation: reporting.moderation, Title: bug.displayTitle(), Log: crashLog, LogLink: externalLink(c, textCrashLog, crash.Log), @@ -540,6 +648,9 @@ func incomingCommandTx(c context.Context, now time.Time, cmd *dashapi.BugUpdate, if bug.Status != BugStatusDup { bug.DupOf = "" } + if cmd.Status != dashapi.BugStatusOpen || !cmd.OnHold { + bugReporting.OnHold = time.Time{} + } bug.LastActivity = now if _, err := datastore.Put(c, bugKey, bug); err != nil { return false, internalError, fmt.Errorf("failed to put bug: %v", err) @@ -561,6 +672,9 @@ func incomingCommandCmd(c context.Context, now time.Time, cmd *dashapi.BugUpdate bugReporting.Reported = now stateEnt.Sent++ // sending repro does not count against the quota } + if bugReporting.OnHold.IsZero() && cmd.OnHold { + bugReporting.OnHold = now + } // Close all previous reporting if they are not closed yet // (can happen due to Status == ReportingDisabled). for i := range bug.Reporting { @@ -588,10 +702,12 @@ func incomingCommandCmd(c context.Context, now time.Time, cmd *dashapi.BugUpdate bug.Status = BugStatusOpen bug.Closed = time.Time{} bugReporting.Closed = now + bugReporting.Auto = cmd.Notification case dashapi.BugStatusInvalid: - bugReporting.Closed = now bug.Closed = now bug.Status = BugStatusInvalid + bugReporting.Closed = now + bugReporting.Auto = cmd.Notification case dashapi.BugStatusDup: bug.Status = BugStatusDup bug.Closed = now diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go index 881767a8b..9707b9e03 100644 --- a/dashboard/app/reporting_email.go +++ b/dashboard/app/reporting_email.go @@ -57,7 +57,6 @@ var mailingLists map[string]bool type EmailConfig struct { Email string - Moderation bool MailMaintainers bool DefaultMaintainers []string } @@ -75,9 +74,6 @@ func (cfg *EmailConfig) Validate() error { return fmt.Errorf("bad email address %q: %v", email, err) } } - if cfg.Moderation && cfg.MailMaintainers { - return fmt.Errorf("both Moderation and MailMaintainers set") - } if cfg.MailMaintainers && len(cfg.DefaultMaintainers) == 0 { return fmt.Errorf("MailMaintainers is set but no DefaultMaintainers") } @@ -87,13 +83,18 @@ func (cfg *EmailConfig) Validate() error { // handleEmailPoll is called by cron and sends emails for new bugs, if any. func handleEmailPoll(w http.ResponseWriter, r *http.Request) { c := appengine.NewContext(r) - if err := emailPollBugs(c); err != nil { - log.Errorf(c, "bug poll failed: %v", err) + if err := emailPollJobs(c); err != nil { + log.Errorf(c, "job poll failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } - if err := emailPollJobs(c); err != nil { - log.Errorf(c, "job poll failed: %v", err) + if err := emailPollNotifications(c); err != nil { + log.Errorf(c, "notif poll failed: %v", err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + if err := emailPollBugs(c); err != nil { + log.Errorf(c, "bug poll failed: %v", err) http.Error(w, err.Error(), http.StatusInternalServerError) return } @@ -103,37 +104,102 @@ func handleEmailPoll(w http.ResponseWriter, r *http.Request) { func emailPollBugs(c context.Context) error { reports := reportingPollBugs(c, emailType) for _, rep := range reports { - cfg := new(EmailConfig) - if err := json.Unmarshal(rep.Config, cfg); err != nil { - log.Errorf(c, "failed to unmarshal email config: %v", err) - continue - } - if cfg.MailMaintainers { - rep.CC = email.MergeEmailLists(rep.CC, rep.Maintainers, cfg.DefaultMaintainers) + if err := emailSendBugReport(c, rep); err != nil { + log.Errorf(c, "%v", err) } - if err := emailReport(c, rep, "mail_bug.txt"); err != nil { - log.Errorf(c, "failed to report bug: %v", err) - continue - } - cmd := &dashapi.BugUpdate{ - ID: rep.ID, - Status: dashapi.BugStatusOpen, - ReproLevel: dashapi.ReproLevelNone, - CrashID: rep.CrashID, - } - if len(rep.ReproC) != 0 { - cmd.ReproLevel = dashapi.ReproLevelC - } else if len(rep.ReproSyz) != 0 { - cmd.ReproLevel = dashapi.ReproLevelSyz - } - ok, reason, err := incomingCommand(c, cmd) - if !ok || err != nil { - log.Errorf(c, "failed to update reported bug: ok=%v reason=%v err=%v", ok, reason, err) + } + return nil +} + +func emailSendBugReport(c context.Context, rep *dashapi.BugReport) error { + cfg := new(EmailConfig) + if err := json.Unmarshal(rep.Config, cfg); err != nil { + return fmt.Errorf("failed to unmarshal email config: %v", err) + } + if cfg.MailMaintainers { + rep.CC = email.MergeEmailLists(rep.CC, rep.Maintainers, cfg.DefaultMaintainers) + } + if err := emailReport(c, rep, "mail_bug.txt"); err != nil { + return fmt.Errorf("failed to report bug: %v", err) + } + cmd := &dashapi.BugUpdate{ + ID: rep.ID, + Status: dashapi.BugStatusOpen, + ReproLevel: dashapi.ReproLevelNone, + CrashID: rep.CrashID, + } + if len(rep.ReproC) != 0 { + cmd.ReproLevel = dashapi.ReproLevelC + } else if len(rep.ReproSyz) != 0 { + cmd.ReproLevel = dashapi.ReproLevelSyz + } + ok, reason, err := incomingCommand(c, cmd) + if !ok || err != nil { + return fmt.Errorf("failed to update reported bug: ok=%v reason=%v err=%v", ok, reason, err) + } + return nil +} + +func emailPollNotifications(c context.Context) error { + notifs := reportingPollNotifications(c, emailType) + for _, notif := range notifs { + if err := emailSendBugNotif(c, notif); err != nil { + log.Errorf(c, "%v", err) } } return nil } +func emailSendBugNotif(c context.Context, notif *dashapi.BugNotification) error { + status, body := dashapi.BugStatusOpen, "" + switch notif.Type { + case dashapi.BugNotifUpstream: + body = "Sending this report upstream." + status = dashapi.BugStatusUpstream + case dashapi.BugNotifBadCommit: + days := int(notifyAboutBadCommitPeriod / time.Hour / 24) + body = fmt.Sprintf("This bug is marked as fixed by commit:\n%v\n"+ + "But I can't find it in any tested tree for more than %v days.\n"+ + "Is it a correct commit? Please update it by replying:\n"+ + "#syz fix: exact-commit-title\n"+ + "Until then the bug is still considered open and\n"+ + "new crashes with the same signature are ignored.\n", + notif.Text, days) + case dashapi.BugNotifObsoleted: + body = "Auto-closing this bug as obsolete.\n" + + "Crashes did not happen for a while, no reproducer and no activity." + status = dashapi.BugStatusInvalid + default: + return fmt.Errorf("bad notification type %v", notif.Type) + } + cfg := new(EmailConfig) + if err := json.Unmarshal(notif.Config, cfg); err != nil { + return fmt.Errorf("failed to unmarshal email config: %v", err) + } + to := email.MergeEmailLists([]string{cfg.Email}, notif.CC) + if cfg.MailMaintainers && notif.Public { + to = email.MergeEmailLists(to, notif.Maintainers, cfg.DefaultMaintainers) + } + from, err := email.AddAddrContext(fromAddr(c), notif.ID) + if err != nil { + return err + } + log.Infof(c, "sending notif %v for %q to %q: %v", notif.Type, notif.Title, to, body) + if err := sendMailText(c, notif.Title, from, to, notif.ExtID, nil, body); err != nil { + return err + } + cmd := &dashapi.BugUpdate{ + ID: notif.ID, + Status: status, + Notification: true, + } + ok, reason, err := incomingCommand(c, cmd) + if !ok || err != nil { + return fmt.Errorf("notif update failed: ok=%v reason=%v err=%v", ok, reason, err) + } + return nil +} + func emailPollJobs(c context.Context) error { jobs, err := pollCompletedJobs(c, emailType) if err != nil { @@ -206,7 +272,7 @@ func emailReport(c context.Context, rep *dashapi.BugReport, templ string) error First: rep.First, Link: link, CreditEmail: creditEmail, - Moderation: cfg.Moderation, + Moderation: rep.Moderation, Maintainers: rep.Maintainers, CompilerID: rep.CompilerID, KernelRepo: rep.KernelRepoAlias, @@ -416,11 +482,16 @@ func sendMailTemplate(c context.Context, subject, from string, to []string, repl if err := mailTemplates.ExecuteTemplate(body, template, data); err != nil { return fmt.Errorf("failed to execute %v template: %v", template, err) } + return sendMailText(c, subject, from, to, replyTo, attachments, body.String()) +} + +func sendMailText(c context.Context, subject, from string, to []string, replyTo string, + attachments []aemail.Attachment, body string) error { msg := &aemail.Message{ Sender: from, To: to, Subject: subject, - Body: body.String(), + Body: body, Attachments: attachments, } if replyTo != "" { diff --git a/dashboard/app/reporting_external.go b/dashboard/app/reporting_external.go index 85e6b501e..4dd85e87f 100644 --- a/dashboard/app/reporting_external.go +++ b/dashboard/app/reporting_external.go @@ -37,6 +37,18 @@ func apiReportingPollBugs(c context.Context, r *http.Request, payload []byte) (i return resp, nil } +func apiReportingPollNotifications(c context.Context, r *http.Request, payload []byte) (interface{}, error) { + req := new(dashapi.PollNotificationsRequest) + if err := json.Unmarshal(payload, req); err != nil { + return nil, fmt.Errorf("failed to unmarshal request: %v", err) + } + notifs := reportingPollNotifications(c, req.Type) + resp := &dashapi.PollNotificationsResponse{ + Notifications: notifs, + } + return resp, nil +} + func apiReportingPollClosed(c context.Context, r *http.Request, payload []byte) (interface{}, error) { req := new(dashapi.PollClosedRequest) if err := json.Unmarshal(payload, req); err != nil { diff --git a/dashboard/app/reporting_test.go b/dashboard/app/reporting_test.go index ffbd4bce5..655bca8a3 100644 --- a/dashboard/app/reporting_test.go +++ b/dashboard/app/reporting_test.go @@ -46,6 +46,7 @@ func TestReportBug(t *testing.T) { Config: []byte(`{"Index":1}`), ID: rep.ID, First: true, + Moderation: true, Title: "title1", Maintainers: []string{"bar@foo.com", "foo@bar.com"}, CompilerID: "compiler1", @@ -122,6 +123,7 @@ func TestReportBug(t *testing.T) { } want.ID = rep2.ID want.First = true + want.Moderation = false want.Config = []byte(`{"Index":2}`) want.NumCrashes = 3 if diff := cmp.Diff(want, rep2); diff != "" { @@ -194,6 +196,7 @@ func TestInvalidBug(t *testing.T) { Config: []byte(`{"Index":1}`), ID: rep.ID, First: true, + Moderation: true, Title: "title1 (2)", CompilerID: "compiler1", KernelRepo: "repo1", @@ -387,7 +390,7 @@ func TestReportingFilter(t *testing.T) { c.client.UploadBuild(build) crash1 := testCrash(build, 1) - crash1.Title = "skip without repro 1" + crash1.Title = "skip with repro 1" c.client.ReportCrash(crash1) // This does not skip first reporting, because it does not have repro. @@ -410,7 +413,7 @@ func TestReportingFilter(t *testing.T) { // Now report a bug that must go to the second reporting right away. crash2 := testCrash(build, 2) - crash2.Title = "skip without repro 2" + crash2.Title = "skip with repro 2" crash2.ReproSyz = []byte("getpid()") c.client.ReportCrash(crash2) diff --git a/dashboard/app/util_test.go b/dashboard/app/util_test.go index d3481b2f1..14722c90f 100644 --- a/dashboard/app/util_test.go +++ b/dashboard/app/util_test.go @@ -231,11 +231,12 @@ func (c *Ctx) checkURLContents(url string, want []byte) { } } -func (c *Ctx) pollEmailBug() string { +func (c *Ctx) pollEmailBug() *aemail.Message { c.expectOK(c.GET("/email_poll")) - c.expectEQ(len(c.emailSink), 1) - msg := <-c.emailSink - return msg.Sender + if len(c.emailSink) == 0 { + c.t.Fatalf("\n%v: got no emails", caller(0)) + } + return <-c.emailSink } type apiClient struct { @@ -302,6 +303,18 @@ func (client *apiClient) pollBug() *dashapi.BugReport { return client.pollBugs(1)[0] } +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)) + } + return resp.Notifications +} + +func (client *apiClient) pollNotif() *dashapi.BugNotification { + return client.pollNotifs(1)[0] +} + func (client *apiClient) updateBug(extID string, status dashapi.BugStatus, dup string) { reply, _ := client.ReportingUpdate(&dashapi.BugUpdate{ ID: extID, diff --git a/dashboard/dashapi/dashapi.go b/dashboard/dashapi/dashapi.go index 02d7ea132..ed95d0f38 100644 --- a/dashboard/dashapi/dashapi.go +++ b/dashboard/dashapi/dashapi.go @@ -262,6 +262,7 @@ type BugReport struct { JobID string ExtID string // arbitrary reporting ID forwarded from BugUpdate.ExtID First bool // Set for first report for this bug. + Moderation bool Title string Maintainers []string CC []string // additional CC emails @@ -297,15 +298,17 @@ type BugReport struct { } type BugUpdate struct { - ID string - ExtID string - Link string - Status BugStatus - ReproLevel ReproLevel - DupOf string - FixCommits []string // Titles of commits that fix this bug. - CC []string // Additional emails to add to CC list in future emails. - CrashID int64 + ID string + ExtID string + Link string + Status BugStatus + ReproLevel ReproLevel + DupOf string + OnHold bool // If set for open bugs, don't upstream this bug. + Notification bool // Reply to a notification. + FixCommits []string // Titles of commits that fix this bug. + CC []string // Additional emails to add to CC list in future emails. + CrashID int64 } type BugUpdateReply struct { @@ -325,6 +328,31 @@ type PollBugsResponse struct { Reports []*BugReport } +type BugNotification struct { + Type BugNotif + Namespace string + Config []byte + ID string + ExtID string // arbitrary reporting ID forwarded from BugUpdate.ExtID + Title string + Text string // meaning depends on Type + CC []string // additional CC emails + Maintainers []string + // Public is what we want all involved people to see (e.g. if we notify about a wrong commit title, + // people need to see it and provide the right title). Not public is what we want to send only + // to a minimal set of recipients (our mailing list) (e.g. notification about an obsoleted bug + // is mostly "for the record"). + Public bool +} + +type PollNotificationsRequest struct { + Type string +} + +type PollNotificationsResponse struct { + Notifications []*BugNotification +} + type PollClosedRequest struct { IDs []string } @@ -344,6 +372,17 @@ func (dash *Dashboard) ReportingPollBugs(typ string) (*PollBugsResponse, error) return resp, nil } +func (dash *Dashboard) ReportingPollNotifications(typ string) (*PollNotificationsResponse, error) { + req := &PollNotificationsRequest{ + Type: typ, + } + resp := new(PollNotificationsResponse) + if err := dash.Query("reporting_poll_notifs", req, resp); err != nil { + return nil, err + } + return resp, nil +} + func (dash *Dashboard) ReportingPollClosed(ids []string) ([]string, error) { req := &PollClosedRequest{ IDs: ids, @@ -384,6 +423,7 @@ func (dash *Dashboard) UploadManagerStats(req *ManagerStatsReq) error { type ( BugStatus int + BugNotif int ReproLevel int ) @@ -396,6 +436,17 @@ const ( ) const ( + // Upstream bug into next reporting. + // If the action succeeds, reporting sends BugStatusUpstream update. + BugNotifUpstream BugNotif = iota + // Bug needs to be closed as obsoleted. + // If the action succeeds, reporting sends BugStatusInvalid update. + BugNotifObsoleted + // Bug fixing commit can't be discovered (wrong commit title). + BugNotifBadCommit +) + +const ( ReproLevelNone ReproLevel = iota ReproLevelSyz ReproLevelC |
