Skip to content

Commit

Permalink
dashboard/app: make dashapi.BugReport more self-contained
Browse files Browse the repository at this point in the history 
Extend BugReport with few fields that are needed by all reportings anyway.
This allows to not create and fill an almost identical object to pass to template.

Update #501
  • Loading branch information
@dvyukov
dvyukov committed Mar 17, 2019
1 parent ded8ff1 commit 5900c53
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 79 deletions.
3 changes: 3 additions & 0 deletions dashboard/app/app_test.go
View file
Expand Up @@ -235,6 +235,9 @@ func testBuild(id int) *dashapi.Build {
return &dashapi.Build{
Manager: fmt.Sprintf("manager%v", id),
ID: fmt.Sprintf("build%v", id),
OS: "linux",
Arch: "amd64",
VMArch: "amd64",
SyzkallerCommit: fmt.Sprintf("syzkaller_commit%v", id),
CompilerID: fmt.Sprintf("compiler%v", id),
KernelRepo: fmt.Sprintf("repo%v", id),
Expand Down
4 changes: 4 additions & 0 deletions dashboard/app/email_test.go
View file
Expand Up @@ -111,6 +111,7 @@ For more options, visit https://groups.google.com/d/optout.

// Now report syz reproducer and check updated email.
build2 := testBuild(10)
build2.Arch = "386"
build2.KernelRepo = testConfig.Namespaces["test2"].Repos[0].URL
build2.KernelBranch = testConfig.Namespaces["test2"].Repos[0].Branch
build2.KernelCommitTitle = "a really long title, longer than 80 chars, really long-long-long-long-long-long title"
Expand Down Expand Up @@ -149,6 +150,7 @@ console output: %[3]v
kernel config: %[4]v
dashboard link: https://testapp.appspot.com/bug?extid=%[1]v
compiler: compiler10
userspace arch: i386
syz repro: %[2]v
CC: [bar@foo.com foo@bar.com maintainers@repo10.org bugs@repo10.org]
Expand Down Expand Up @@ -193,6 +195,7 @@ console output: %[3]v
kernel config: %[4]v
dashboard link: https://testapp.appspot.com/bug?extid=%[1]v
compiler: compiler10
userspace arch: i386
syz repro: %[2]v
CC: [bar@foo.com foo@bar.com maintainers@repo10.org bugs@repo10.org]
Expand Down Expand Up @@ -261,6 +264,7 @@ console output: %[4]v
kernel config: %[5]v
dashboard link: https://testapp.appspot.com/bug?extid=%[1]v
compiler: compiler10
userspace arch: i386
syz repro: %[3]v
C reproducer: %[2]v
CC: [qux@qux.com maintainers@repo10.org bugs@repo10.org]
Expand Down
18 changes: 13 additions & 5 deletions dashboard/app/jobs.go
View file
Expand Up @@ -354,10 +354,6 @@ func createBugReportForJob(c context.Context, job *Job, jobKey *datastore.Key, c
if err != nil {
return nil, err
}
patch, _, err := getText(c, textPatch, job.Patch)
if err != nil {
return nil, err
}
build, err := loadBuild(c, job.Namespace, job.BuildID)
if err != nil {
return nil, err
Expand All @@ -374,13 +370,20 @@ func createBugReportForJob(c context.Context, job *Job, jobKey *datastore.Key, c
if bugReporting == nil {
return nil, fmt.Errorf("job bug has no reporting %q", job.Reporting)
}
creditEmail, err := email.AddAddrContext(ownEmail(c), bugReporting.ID)
if err != nil {
return nil, err
}
rep := &dashapi.BugReport{
Type: dashapi.ReportTestPatch,
Namespace: job.Namespace,
Config: reportingConfig,
ID: bugReporting.ID,
JobID: extJobID(jobKey),
ExtID: job.ExtID,
Title: bug.displayTitle(),
Link: fmt.Sprintf("%v/bug?extid=%v", appURL(c), bugReporting.ID),
CreditEmail: creditEmail,
CC: job.CC,
Log: crashLog,
LogLink: externalLink(c, textCrashLog, job.CrashLog),
Expand All @@ -389,6 +392,7 @@ func createBugReportForJob(c context.Context, job *Job, jobKey *datastore.Key, c
OS: build.OS,
Arch: build.Arch,
VMArch: build.VMArch,
UserSpaceArch: kernelArch(build.Arch),
CompilerID: build.CompilerID,
KernelRepo: build.KernelRepo,
KernelRepoAlias: kernelRepoInfo(build).Alias,
Expand All @@ -401,9 +405,13 @@ func createBugReportForJob(c context.Context, job *Job, jobKey *datastore.Key, c
CrashTitle: job.CrashTitle,
Error: jobError,
ErrorLink: externalLink(c, textError, job.Error),
Patch: patch,
PatchLink: externalLink(c, textPatch, job.Patch),
}
// 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:]
rep.ErrorTruncated = true
}
return rep, nil
}

Expand Down
2 changes: 1 addition & 1 deletion dashboard/app/mail_bug.txt
View file
Expand Up @@ -5,7 +5,7 @@ Hello,
syzbot {{if .First}}found{{else}}has found a reproducer for{{end}} the following crash on:

HEAD commit: {{formatShortHash .KernelCommit}} {{formatCommitTableTitle .KernelCommitTitle}}
git tree: {{.KernelRepo}}
git tree: {{.KernelRepoAlias}}
{{if .LogLink}}console output: {{.LogLink}}
{{end}}{{if .KernelConfigLink}}kernel config: {{.KernelConfigLink}}
{{end}}dashboard link: {{.Link}}
Expand Down
4 changes: 2 additions & 2 deletions dashboard/app/mail_test_result.txt
View file
Expand Up @@ -8,7 +8,7 @@ syzbot has tested the proposed patch but the reproducer still triggered crash:
syzbot tried to test the proposed patch but build/boot failed:

{{printf "%s" .Error}}
{{if .ErrorLink}}
{{if .ErrorTruncated}}
Error text is too large and was truncated, full error text is at:
{{.ErrorLink}}
{{end}}
Expand All @@ -20,7 +20,7 @@ Reported-and-tested-by: {{.CreditEmail}}
Tested on:

commit: {{formatShortHash .KernelCommit}} {{formatCommitTableTitle .KernelCommitTitle}}
git tree: {{.KernelRepo}}
git tree: {{.KernelRepoAlias}}
{{if .LogLink}}console output: {{.LogLink}}
{{end}}{{if .KernelConfigLink}}kernel config: {{.KernelConfigLink}}
{{end}}compiler: {{.CompilerID}}
Expand Down
25 changes: 25 additions & 0 deletions dashboard/app/reporting.go
View file
Expand Up @@ -338,16 +338,27 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *datast
if err != nil {
return nil, err
}
creditEmail, err := email.AddAddrContext(ownEmail(c), bugReporting.ID)
if err != nil {
return nil, err
}
typ := dashapi.ReportNew
if !bugReporting.Reported.IsZero() {
typ = dashapi.ReportRepro
}

kernelRepo := kernelRepoInfo(build)
rep := &dashapi.BugReport{
Type: typ,
Namespace: bug.Namespace,
Config: reportingConfig,
ID: bugReporting.ID,
ExtID: bugReporting.ExtID,
First: bugReporting.Reported.IsZero(),
Moderation: reporting.moderation,
Title: bug.displayTitle(),
Link: fmt.Sprintf("%v/bug?extid=%v", appURL(c), bugReporting.ID),
CreditEmail: creditEmail,
Log: crashLog,
LogLink: externalLink(c, textCrashLog, crash.Log),
Report: report,
Expand All @@ -356,6 +367,7 @@ func createBugReport(c context.Context, bug *Bug, crash *Crash, crashKey *datast
OS: build.OS,
Arch: build.Arch,
VMArch: build.VMArch,
UserSpaceArch: kernelArch(build.Arch),
CompilerID: build.CompilerID,
KernelRepo: build.KernelRepo,
KernelRepoAlias: kernelRepo.Alias,
Expand Down Expand Up @@ -912,3 +924,16 @@ func (a bugReportSorter) Less(i, j int) bool {
}
return a[i].FirstTime.Before(a[j].FirstTime)
}

// kernelArch returns arch as kernel developers know it (rather than Go names).
// Currently Linux-specific.
func kernelArch(arch string) string {
switch arch {
case "386":
return "i386"
case "amd64":
return "" // this is kinda the default, so we don't notify about it
default:
return arch
}
}
66 changes: 1 addition & 65 deletions dashboard/app/reporting_email.go
View file
Expand Up @@ -230,76 +230,12 @@ func emailReport(c context.Context, rep *dashapi.BugReport, templ string) error
return fmt.Errorf("failed to unmarshal email config: %v", err)
}
to := email.MergeEmailLists([]string{cfg.Email}, rep.CC)
// 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:]
} else {
rep.ErrorLink = ""
}
from, err := email.AddAddrContext(fromAddr(c), rep.ID)
if err != nil {
return err
}
creditEmail, err := email.AddAddrContext(ownEmail(c), rep.ID)
if err != nil {
return err
}
userspaceArch := ""
if rep.Arch == "386" {
userspaceArch = "i386"
}
link := fmt.Sprintf("%v/bug?extid=%v", appURL(c), rep.ID)
// Data passed to the template.
type BugReportData struct {
First bool
Link string
CreditEmail string
Moderation bool
Maintainers []string
CompilerID string
KernelRepo string
KernelCommit string
KernelCommitTitle string
KernelCommitDate time.Time
UserSpaceArch string
CrashTitle string
Report []byte
Error []byte
ErrorLink string
LogLink string
KernelConfigLink string
ReproSyzLink string
ReproCLink string
NumCrashes int64
HappenedOn []string
PatchLink string
}
data := &BugReportData{
First: rep.First,
Link: link,
CreditEmail: creditEmail,
Moderation: rep.Moderation,
Maintainers: rep.Maintainers,
CompilerID: rep.CompilerID,
KernelRepo: rep.KernelRepoAlias,
KernelCommit: rep.KernelCommit,
KernelCommitTitle: rep.KernelCommitTitle,
KernelCommitDate: rep.KernelCommitDate,
UserSpaceArch: userspaceArch,
CrashTitle: rep.CrashTitle,
Report: rep.Report,
Error: rep.Error,
ErrorLink: rep.ErrorLink,
LogLink: rep.LogLink,
KernelConfigLink: rep.KernelConfigLink,
ReproSyzLink: rep.ReproSyzLink,
ReproCLink: rep.ReproCLink,
NumCrashes: rep.NumCrashes,
HappenedOn: rep.HappenedOn,
PatchLink: rep.PatchLink,
}
log.Infof(c, "sending email %q to %q", rep.Title, to)
return sendMailTemplate(c, rep.Title, from, to, rep.ExtID, nil, templ, data)
return sendMailTemplate(c, rep.Title, from, to, rep.ExtID, nil, templ, rep)
}

// handleIncomingMail is the entry point for incoming emails.
Expand Down
17 changes: 17 additions & 0 deletions dashboard/app/reporting_test.go
View file
Expand Up @@ -6,6 +6,7 @@
package dash

import (
"fmt"
"testing"
"time"

Expand Down Expand Up @@ -39,12 +40,18 @@ func TestReportBug(t *testing.T) {
c.expectNE(rep.ID, "")
_, dbCrash, dbBuild := c.loadBug(rep.ID)
want := &dashapi.BugReport{
Type: dashapi.ReportNew,
Namespace: "test1",
Config: []byte(`{"Index":1}`),
ID: rep.ID,
OS: "linux",
Arch: "amd64",
VMArch: "amd64",
First: true,
Moderation: true,
Title: "title1",
Link: fmt.Sprintf("https://testapp.appspot.com/bug?extid=%v", rep.ID),
CreditEmail: fmt.Sprintf("syzbot+%v@testapp.appspotmail.com", rep.ID),
Maintainers: []string{"bar@foo.com", "foo@bar.com"},
CompilerID: "compiler1",
KernelRepo: "repo1",
Expand All @@ -71,6 +78,7 @@ func TestReportBug(t *testing.T) {
// Now add syz repro and check that we get another bug report.
crash1.ReproOpts = []byte("some opts")
crash1.ReproSyz = []byte("getpid()")
want.Type = dashapi.ReportRepro
want.First = false
want.ReproSyz = []byte(syzReproPrefix + "#some opts\ngetpid()")
c.client.ReportCrash(crash1)
Expand Down Expand Up @@ -111,7 +119,10 @@ func TestReportBug(t *testing.T) {
rep2 := c.client.pollBug()
c.expectNE(rep2.ID, "")
c.expectNE(rep2.ID, rep.ID)
want.Type = dashapi.ReportNew
want.ID = rep2.ID
want.Link = fmt.Sprintf("https://testapp.appspot.com/bug?extid=%v", rep2.ID)
want.CreditEmail = fmt.Sprintf("syzbot+%v@testapp.appspotmail.com", rep2.ID)
want.First = true
want.Moderation = false
want.Config = []byte(`{"Index":2}`)
Expand Down Expand Up @@ -178,12 +189,18 @@ func TestInvalidBug(t *testing.T) {
c.expectNE(rep.ID, "")
_, dbCrash, dbBuild := c.loadBug(rep.ID)
want := &dashapi.BugReport{
Type: dashapi.ReportNew,
Namespace: "test1",
Config: []byte(`{"Index":1}`),
ID: rep.ID,
OS: "linux",
Arch: "amd64",
VMArch: "amd64",
First: true,
Moderation: true,
Title: "title1 (2)",
Link: fmt.Sprintf("https://testapp.appspot.com/bug?extid=%v", rep.ID),
CreditEmail: fmt.Sprintf("syzbot+%v@testapp.appspotmail.com", rep.ID),
CompilerID: "compiler1",
KernelRepo: "repo1",
KernelRepoAlias: "repo1 branch1",
Expand Down
23 changes: 17 additions & 6 deletions dashboard/dashapi/dashapi.go
View file
Expand Up @@ -256,19 +256,23 @@ func (dash *Dashboard) LogError(name, msg string, args ...interface{}) {
// BugReport describes a single bug.
// Used by dashboard external reporting.
type BugReport struct {
Type ReportType
Namespace string
Config []byte
ID string
JobID string
ExtID string // arbitrary reporting ID forwarded from BugUpdate.ExtID
First bool // Set for first report for this bug.
First bool // Set for first report for this bug (Type == ReportNew).
Moderation bool
Title string
Link string // link to the bug on dashboard
CreditEmail string // email for the Reported-by tag
Maintainers []string
CC []string // additional CC emails
OS string
Arch string
VMArch string
UserSpaceArch string // user-space arch as kernel developers know it (rather than Go names)
CompilerID string
KernelRepo string
KernelRepoAlias string
Expand All @@ -290,11 +294,11 @@ type BugReport struct {
NumCrashes int64
HappenedOn []string // list of kernel repo aliases

CrashTitle string // job execution crash title
Error []byte // job execution error
ErrorLink string
Patch []byte // testing job patch
PatchLink string
CrashTitle string // job execution crash title
Error []byte // job execution error
ErrorLink string
ErrorTruncated bool // full Error text is too large and was truncated
PatchLink string
}

type BugUpdate struct {
Expand Down Expand Up @@ -425,6 +429,7 @@ type (
BugStatus int
BugNotif int
ReproLevel int
ReportType int
)

const (
Expand Down Expand Up @@ -452,6 +457,12 @@ const (
ReproLevelC
)

const (
ReportNew ReportType = iota // First report for this bug in the reporting stage.
ReportRepro // Found repro for an already reported bug.
ReportTestPatch // Patch testing result.
)

func (dash *Dashboard) Query(method string, req, reply interface{}) error {
if dash.logger != nil {
dash.logger("API(%v): %#v", method, req)
Expand Down

0 comments on commit 5900c53

Please sign in to comment.