From 703f643f91e8becd2495f1f102346a0b7fa63867 Mon Sep 17 00:00:00 2001 From: Aleksandr Nogikh Date: Mon, 24 Apr 2023 19:52:41 +0200 Subject: dashboard: support bug labels Let bug labels come in three flavours: 1) Bug labels with multiple values (e.g. `subsystems`). 2) Bug labels with only one value (e.g. `prio`). 3) Flags. Let users configure bug labels via email by issuing the following commands: #syz set subsystems: abc, def #syz set no-reminders #syz unset no-reminders Also let users set tags for invididual bugs in reported bug lists: #syz set <1> some-tag --- dashboard/app/api.go | 2 +- dashboard/app/bug.html | 8 +- dashboard/app/cache.go | 9 +- dashboard/app/email_test.go | 145 +++++++++++++++++++++----- dashboard/app/entities.go | 110 ++++++++++++++------ dashboard/app/entities_test.go | 62 +++++++++++ dashboard/app/index.yaml | 3 +- dashboard/app/label.go | 198 +++++++++++++++++++++++++++++++++++ dashboard/app/linux_reporting.go | 4 +- dashboard/app/main.go | 71 ++++++++----- dashboard/app/main_test.go | 6 +- dashboard/app/reporting.go | 8 +- dashboard/app/reporting_email.go | 220 +++++++++++++++++++++++++++++---------- dashboard/app/reporting_lists.go | 6 +- dashboard/app/subsystem.go | 34 +++--- dashboard/app/subsystem_test.go | 45 +++----- dashboard/app/templates.html | 10 +- pkg/email/parser.go | 5 +- pkg/email/parser_test.go | 10 ++ pkg/html/pages/style.css | 4 +- 20 files changed, 745 insertions(+), 215 deletions(-) create mode 100644 dashboard/app/entities_test.go create mode 100644 dashboard/app/label.go diff --git a/dashboard/app/api.go b/dashboard/app/api.go index 6fa9c2073..400540e12 100644 --- a/dashboard/app/api.go +++ b/dashboard/app/api.go @@ -801,7 +801,7 @@ func reportCrash(c context.Context, build *Build, req *dashapi.Crash) (*Bug, err bug.HasReport = true } if calculateSubsystems { - bug.SetAutoSubsystems(newSubsystems, now, getSubsystemRevision(c, ns)) + bug.SetAutoSubsystems(c, newSubsystems, now, getSubsystemRevision(c, ns)) } bug.increaseCrashStats(now) bug.HappenedOn = mergeString(bug.HappenedOn, build.Manager) diff --git a/dashboard/app/bug.html b/dashboard/app/bug.html index 141059358..fedcb0ff1 100644 --- a/dashboard/app/bug.html +++ b/dashboard/app/bug.html @@ -16,11 +16,11 @@ Page with details about a single bug. {{.Bug.Title}}

Status: {{if .Bug.ExternalLink}}{{.Bug.Status}}{{else}}{{.Bug.Status}}{{end}}
- {{if .Subsystems}} - Subsystems: {{range .Subsystems}} - {{link .Link .Name}} + {{if .Labels}} + Labels: {{range .Labels}} + {{link .Link .Name}} {{- end}} - (incorrect?)
+ (incorrect?)
{{- end}} {{if .Bug.CreditEmail}} Reported-by: {{.Bug.CreditEmail}}
diff --git a/dashboard/app/cache.go b/dashboard/app/cache.go index 2c8ca6376..dee2fb989 100644 --- a/dashboard/app/cache.go +++ b/dashboard/app/cache.go @@ -72,12 +72,13 @@ func buildAndStoreCached(c context.Context, bugs []*Bug, ns string, accessLevel continue } v.Total.Record(bug) - for _, subsystem := range bug.Tags.Subsystems { - stats := v.Subsystems[subsystem.Name] + subsystems := bug.LabelValues(SubsystemLabel) + for _, label := range subsystems { + stats := v.Subsystems[label.Value] stats.Record(bug) - v.Subsystems[subsystem.Name] = stats + v.Subsystems[label.Value] = stats } - if len(bug.Tags.Subsystems) == 0 { + if len(subsystems) == 0 { v.NoSubsystem.Record(bug) } } diff --git a/dashboard/app/email_test.go b/dashboard/app/email_test.go index 2de5236b6..edd86d787 100644 --- a/dashboard/app/email_test.go +++ b/dashboard/app/email_test.go @@ -12,6 +12,7 @@ import ( "github.com/google/syzkaller/dashboard/dashapi" "github.com/google/syzkaller/pkg/email" "github.com/google/syzkaller/sys/targets" + "github.com/stretchr/testify/assert" ) // nolint: funlen @@ -1073,7 +1074,7 @@ func TestEmailPatchTestingAccess(t *testing.T) { c.expectEQ(pollResp.ID, "") } -func TestEmailInvalidSetCommand(t *testing.T) { +func TestEmailSetInvalidSubsystems(t *testing.T) { c := NewCtx(t) defer c.Close() @@ -1089,32 +1090,23 @@ func TestEmailInvalidSetCommand(t *testing.T) { sender := c.pollEmailBug().Sender - // Fully invalid command. - c.incomingEmail(sender, "#syz set tag\n", - EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) - syzbotReply := c.pollEmailBug() - c.expectEQ(strings.Contains(syzbotReply.Body, "I've failed to parse your command"), true) - // Invalid subsystem name. c.incomingEmail(sender, "#syz set subsystems: non-existent", EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) c.expectEQ(c.pollEmailBug().Body, `> #syz set subsystems: non-existent -Please use subsystem names from the list of supported subsystems: -https://testapp.appspot.com/access-public-email/subsystems?all=true +The specified label value is incorrect. +"non-existent" is not among the allowed values. +Please use one of the supported label values. -If you believe that the subsystem list should be changed, please contact the bot's maintainers. +The following labels are suported: +no-reminders, prio: {low, normal, high}, subsystems: {.. see below ..} +The list of subsystems: https://testapp.appspot.com/access-public-email/subsystems?all=true `) - - // No subsystems. - c.incomingEmail(sender, "#syz set subsystems:\n", - EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) - c.expectEQ(strings.Contains(c.pollEmailBug().Body, - "You must specify at least one subsystem name"), true) } -func TestEmailSetCommand(t *testing.T) { +func TestEmailSetSubsystems(t *testing.T) { c := NewCtx(t) defer c.Close() @@ -1133,20 +1125,123 @@ func TestEmailSetCommand(t *testing.T) { c.expectOK(err) // At the beginning, there are no subsystems. - expectSubsystems(t, client, extBugID) + expectLabels(t, client, extBugID) // Set one subsystem. c.incomingEmail(sender, "#syz set subsystems: subsystemA\n", EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) - syzbotReply := c.pollEmailBug() - c.expectEQ(syzbotReply.To, []string{"test@requester.com"}) - c.expectEQ(syzbotReply.Cc, []string{"test@syzkaller.com"}) - c.expectEQ(strings.Contains(syzbotReply.Body, "I've successfully updated the bug's subsystems."), true) - expectSubsystems(t, client, extBugID, "subsystemA") + expectLabels(t, client, extBugID, "subsystems:subsystemA") // Set two subsystems. c.incomingEmail(sender, "#syz set subsystems: subsystemA, subsystemB\n", EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) - c.pollEmailBug() - expectSubsystems(t, client, extBugID, "subsystemA", "subsystemB") + expectLabels(t, client, extBugID, "subsystems:subsystemA", "subsystems:subsystemB") +} + +func TestEmailBugLabels(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + client := c.makeClient(clientPublicEmail, keyPublicEmail, true) + mailingList := config.Namespaces["access-public-email"].Reporting[0].Config.(*EmailConfig).Email + + build := testBuild(1) + client.UploadBuild(build) + + crash := testCrash(build, 1) + client.ReportCrash(crash) + + sender := c.pollEmailBug().Sender + _, extBugID, err := email.RemoveAddrContext(sender) + c.expectOK(err) + + // At the beginning, there are no tags. + expectLabels(t, client, extBugID) + + // Set a tag. + c.incomingEmail(sender, "#syz set prio: low\n", + EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) + expectLabels(t, client, extBugID, "prio:low") + + // Notice that medium prio supercedes low prio since they are of the oneOf type. + c.incomingEmail(sender, "#syz set prio: high\n", + EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) + expectLabels(t, client, extBugID, "prio:high") + + // Also set a flag label. + c.incomingEmail(sender, "#syz set no-reminders\n", + EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) + expectLabels(t, client, extBugID, "prio:high", "no-reminders") + + // Remove a tag. + c.incomingEmail(sender, "#syz unset prio\n", + EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) + expectLabels(t, client, extBugID, "no-reminders") + + // Remove another tag. + c.incomingEmail(sender, "#syz unset no-reminders\n", + EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) + expectLabels(t, client, extBugID) +} + +func TestInvalidEmailBugLabels(t *testing.T) { + c := NewCtx(t) + defer c.Close() + + client := c.makeClient(clientPublicEmail, keyPublicEmail, true) + mailingList := config.Namespaces["access-public-email"].Reporting[0].Config.(*EmailConfig).Email + + build := testBuild(1) + client.UploadBuild(build) + + crash := testCrash(build, 1) + client.ReportCrash(crash) + c.incomingEmail(c.pollEmailBug().Sender, "#syz upstream\n") + + sender := c.pollEmailBug().Sender + + // Non-existing label. + c.incomingEmail(sender, "#syz set label: tag", + EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) + body := c.pollEmailBug().Body + c.expectEQ(body, `> #syz set label: tag + +The specified label "label" is unknown. +Please use one of the supported labels. + +The following labels are suported: +no-reminders, prio: {low, normal, high}, subsystems: {.. see below ..} +The list of subsystems: https://testapp.appspot.com/access-public-email/subsystems?all=true + +`) + + // Existing label, wrong value. + c.incomingEmail(sender, "#syz set prio: unknown\n", + EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) + c.expectEQ(strings.Contains(c.pollEmailBug().Body, + `The specified label value is incorrect. +"unknown" is not among the allowed values`), true) + + // Existing label, too many values. + c.incomingEmail(sender, "#syz set prio: low, high\n", + EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) + c.expectEQ(strings.Contains(c.pollEmailBug().Body, + `The specified label value is incorrect. +You must specify only one of the allowed values.`), true) + + // Removing a non-existing label. + c.incomingEmail(sender, "#syz unset tag2\n", + EmailOptFrom("test@requester.com"), EmailOptCC([]string{mailingList})) + syzbotReply := c.pollEmailBug() + c.expectEQ(strings.Contains(syzbotReply.Body, "The following labels did not exist: tag2"), true) +} + +func expectLabels(t *testing.T, client *apiClient, extID string, labels ...string) { + t.Helper() + bug, _, _ := client.Ctx.loadBug(extID) + names := []string{} + for _, item := range bug.Labels { + names = append(names, item.String()) + } + assert.ElementsMatch(t, names, labels) } diff --git a/dashboard/app/entities.go b/dashboard/app/entities.go index 2ed019b1b..29d775921 100644 --- a/dashboard/app/entities.go +++ b/dashboard/app/entities.go @@ -7,6 +7,7 @@ import ( "fmt" "regexp" "strconv" + "strings" "time" "github.com/google/syzkaller/dashboard/dashapi" @@ -118,50 +119,60 @@ type Bug struct { // bit 1 - don't want to publish it (syzkaller build/test errors) KcidbStatus int64 DailyStats []BugDailyStats - Tags BugTags + Labels []BugLabel DiscussionInfo []BugDiscussionInfo } -type BugTags struct { - Subsystems []BugSubsystem -} +type BugLabelType string -type BugSubsystem struct { - // For now, let's keep the bare minimum number of fields. - // The subsystem names we use now are not stable and should not be relied upon. - Name string +type BugLabel struct { + Label BugLabelType + // Either empty (for flags) or contains the value. + Value string // The email of the user who manually set this subsystem tag. - // If empty, the subsystem was set automatically. + // If empty, the label was set automatically. SetBy string + // Link to the message. + Link string } -func (bug *Bug) SetAutoSubsystems(list []*subsystem.Subsystem, now time.Time, rev int) { - objects := []BugSubsystem{} - for _, item := range list { - objects = append(objects, BugSubsystem{Name: item.Name}) +func (label *BugLabel) String() string { + if label.Value == "" { + return string(label.Label) } - bug.SubsystemsRev = rev - bug.SetSubsystems(objects, now) + return string(label.Label) + ":" + label.Value } -func (bug *Bug) SetUserSubsystems(list []*subsystem.Subsystem, now time.Time, user string) { - objects := []BugSubsystem{} +func (bug *Bug) SetAutoSubsystems(c context.Context, list []*subsystem.Subsystem, now time.Time, rev int) { + bug.SubsystemsRev = rev + bug.SubsystemsTime = now + var objects []BugLabel for _, item := range list { - objects = append(objects, BugSubsystem{ - Name: item.Name, - SetBy: user, - }) + objects = append(objects, BugLabel{Label: SubsystemLabel, Value: item.Name}) } - bug.SetSubsystems(objects, now) + bug.SetLabels(makeLabelSet(c, bug.Namespace), objects) } -func (bug *Bug) SetSubsystems(list []BugSubsystem, now time.Time) { - bug.Tags.Subsystems = list - bug.SubsystemsTime = now +func updateSingleBug(c context.Context, bugKey *db.Key, transform func(*Bug) error) error { + tx := func(c context.Context) error { + bug := new(Bug) + if err := db.Get(c, bugKey, bug); err != nil { + return fmt.Errorf("failed to get bug: %v", err) + } + err := transform(bug) + if err != nil { + return err + } + if _, err := db.Put(c, bugKey, bug); err != nil { + return fmt.Errorf("failed to put bug: %v", err) + } + return nil + } + return db.RunInTransaction(c, tx, &db.TransactionOptions{Attempts: 10}) } func (bug *Bug) hasUserSubsystems() bool { - for _, item := range bug.Tags.Subsystems { + for _, item := range bug.LabelValues(SubsystemLabel) { if item.SetBy != "" { return true } @@ -169,19 +180,50 @@ func (bug *Bug) hasUserSubsystems() bool { return false } -func (bug *Bug) hasSubsystem(name string) bool { - for _, item := range bug.Tags.Subsystems { - if item.Name == name { - return true - } - } - return false +// Initially, subsystem labels were stored as Tags.Subsystems, but over time +// it turned out that we'd better store all labels together. +// Let's keep this conversion code until "Tags" are removed from all bugs. +// Then it can be removed. + +type Bug202304 struct { + Tags BugTags202304 +} + +type BugTags202304 struct { + Subsystems []BugTag202304 +} + +type BugTag202304 struct { + Name string + SetBy string } -func (bug *Bug) Load(ps []db.Property) error { +func (bug *Bug) Load(origProps []db.Property) error { + // First filer out Tag properties. + var tags, ps []db.Property + for _, p := range origProps { + if strings.HasPrefix(p.Name, "Tags.") { + tags = append(tags, p) + } else { + ps = append(ps, p) + } + } if err := db.LoadStruct(bug, ps); err != nil { return err } + if len(tags) > 0 { + old := Bug202304{} + if err := db.LoadStruct(&old, tags); err != nil { + return err + } + for _, entry := range old.Tags.Subsystems { + bug.Labels = append(bug.Labels, BugLabel{ + Label: SubsystemLabel, + SetBy: entry.SetBy, + Value: entry.Name, + }) + } + } headReproFound := false for _, p := range ps { if p.Name == "HeadReproLevel" { diff --git a/dashboard/app/entities_test.go b/dashboard/app/entities_test.go new file mode 100644 index 000000000..7db3d5951 --- /dev/null +++ b/dashboard/app/entities_test.go @@ -0,0 +1,62 @@ +// Copyright 2023 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. + +package main + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + db "google.golang.org/appengine/v2/datastore" +) + +func TestOldBugTagsConversion(t *testing.T) { + oldBug := &struct { + Namespace string + Title string + Tags BugTags202304 + }{ + Namespace: "some-ns", + Title: "some title", + Tags: BugTags202304{ + Subsystems: []BugTag202304{ + { + Name: "first", + SetBy: "user", + }, + { + Name: "second", + }, + }, + }, + } + + fields, err := db.SaveStruct(oldBug) + if err != nil { + t.Fatal(err) + } + + newBug := &Bug{} + err = newBug.Load(fields) + if err != nil { + t.Fatal(err) + } + + if diff := cmp.Diff(&Bug{ + Namespace: "some-ns", + Title: "some title", + Labels: []BugLabel{ + { + Value: "first", + SetBy: "user", + Label: SubsystemLabel, + }, + { + Value: "second", + Label: SubsystemLabel, + }, + }, + }, newBug); diff != "" { + t.Fatal(diff) + } +} diff --git a/dashboard/app/index.yaml b/dashboard/app/index.yaml index f101e59bd..7b18993f2 100644 --- a/dashboard/app/index.yaml +++ b/dashboard/app/index.yaml @@ -115,7 +115,8 @@ indexes: - kind: Bug properties: - name: Namespace - - name: Tags.Subsystems.Name + - name: Labels.Label + - name: Labels.Value - name: Status - kind: Build diff --git a/dashboard/app/label.go b/dashboard/app/label.go new file mode 100644 index 000000000..34ad0ccf5 --- /dev/null +++ b/dashboard/app/label.go @@ -0,0 +1,198 @@ +// Copyright 2023 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. + +package main + +import ( + "fmt" + "sort" + "strings" + + "golang.org/x/net/context" +) + +const ( + EmptyLabel BugLabelType = "" + SubsystemLabel BugLabelType = "subsystems" + PriorityLabel BugLabelType = "prio" + NoRemindersLabel BugLabelType = "no-reminders" +) + +type oneOf []string +type subsetOf []string +type trueFalse struct{} + +func makeLabelSet(c context.Context, ns string) *labelSet { + ret := map[BugLabelType]interface{}{ + PriorityLabel: oneOf([]string{"low", "normal", "high"}), + NoRemindersLabel: trueFalse{}, + } + service := getSubsystemService(c, ns) + if service != nil { + names := []string{} + for _, item := range service.List() { + names = append(names, item.Name) + } + ret[SubsystemLabel] = subsetOf(names) + } + return &labelSet{ + c: c, + ns: ns, + labels: ret, + } +} + +type labelSet struct { + c context.Context + ns string + labels map[BugLabelType]interface{} +} + +func (s *labelSet) FindLabel(label BugLabelType) bool { + _, ok := s.labels[label] + return ok +} + +func (s *labelSet) ValidateValues(label BugLabelType, values []BugLabel) string { + rules := s.labels[label] + if rules == nil { + return "" + } + switch v := rules.(type) { + case oneOf: + if len(values) != 1 { + return "You must specify only one of the allowed values" + } + if !stringInList([]string(v), values[0].Value) { + return fmt.Sprintf("%q is not among the allowed values", values[0].Value) + } + case subsetOf: + for _, item := range values { + if !stringInList([]string(v), item.Value) { + return fmt.Sprintf("%q is not among the allowed values", item.Value) + } + } + case trueFalse: + if len(values) != 1 || values[0].Value != "" { + return "This label does not support any values" + } + } + return "" +} + +func (s *labelSet) Help() string { + var sortedKeys []BugLabelType + for key := range s.labels { + sortedKeys = append(sortedKeys, key) + } + sort.Slice(sortedKeys, func(i, j int) bool { + return string(sortedKeys[i]) < string(sortedKeys[j]) + }) + + var help strings.Builder + for _, key := range sortedKeys { + if help.Len() > 0 { + help.WriteString(", ") + } + if key == SubsystemLabel { + help.WriteString(fmt.Sprintf("%s: {.. see below ..}", key)) + continue + } + switch v := s.labels[key].(type) { + case oneOf: + help.WriteString(string(key)) + help.WriteString(": {") + list := []string(v) + for i := range list { + if i > 0 { + help.WriteString(", ") + } + help.WriteString(list[i]) + } + help.WriteByte('}') + case trueFalse: + help.WriteString(string(key)) + } + } + + var sb strings.Builder + writeWrapped(&sb, help.String()) + if _, ok := s.labels[SubsystemLabel]; ok { + url := subsystemListURL(s.c, s.ns) + if url != "" { + sb.WriteString(fmt.Sprintf("\nThe list of subsystems: %s", url)) + } + } + return sb.String() +} + +func writeWrapped(sb *strings.Builder, str string) { + const limit = 80 + lineLen := 0 + for _, token := range strings.Fields(str) { + if lineLen >= limit || + lineLen > 0 && lineLen+len(token) >= limit { + sb.WriteByte('\n') + lineLen = 0 + } + if lineLen > 0 { + sb.WriteString(" ") + } + sb.WriteString(token) + lineLen += len(token) + } +} + +func (bug *Bug) HasLabel(label BugLabelType, value string) bool { + for _, item := range bug.Labels { + if item.Label == label && item.Value == value { + return true + } + } + return false +} + +func (bug *Bug) LabelValues(label BugLabelType) []BugLabel { + var ret []BugLabel + for _, item := range bug.Labels { + if item.Label == label { + ret = append(ret, item) + } + } + return ret +} + +func (bug *Bug) SetLabels(set *labelSet, values []BugLabel) error { + var label BugLabelType + for _, v := range values { + if label != EmptyLabel && label != v.Label { + return fmt.Errorf("values of the same label type are expected") + } + label = v.Label + } + if errStr := set.ValidateValues(label, values); errStr != "" { + return fmt.Errorf("%s", errStr) + } + bug.UnsetLabels(label) + bug.Labels = append(bug.Labels, values...) + return nil +} + +func (bug *Bug) UnsetLabels(labels ...BugLabelType) map[BugLabelType]struct{} { + toDelete := map[BugLabelType]struct{}{} + notFound := map[BugLabelType]struct{}{} + for _, name := range labels { + toDelete[name] = struct{}{} + notFound[name] = struct{}{} + } + var newList []BugLabel + for _, item := range bug.Labels { + if _, ok := toDelete[item.Label]; ok { + delete(notFound, item.Label) + continue + } + newList = append(newList, item) + } + bug.Labels = newList + return notFound +} diff --git a/dashboard/app/linux_reporting.go b/dashboard/app/linux_reporting.go index 98220194b..8ddffd433 100644 --- a/dashboard/app/linux_reporting.go +++ b/dashboard/app/linux_reporting.go @@ -8,9 +8,9 @@ package main // canBeVfsBug determines whether a bug could belong to the VFS subsystem itself. func canBeVfsBug(bug *Bug) bool { - for _, subsystem := range bug.Tags.Subsystems { + for _, subsystem := range bug.LabelValues(SubsystemLabel) { // The "vfs" one is left for compatibility with the older matching code. - if subsystem.Name == "vfs" || subsystem.Name == "fs" { + if subsystem.Value == "vfs" || subsystem.Value == "fs" { return true } } diff --git a/dashboard/app/main.go b/dashboard/app/main.go index 0f5d7d993..989bdcf5e 100644 --- a/dashboard/app/main.go +++ b/dashboard/app/main.go @@ -245,7 +245,7 @@ type uiBugPage struct { SampleReport template.HTML Crashes *uiCrashTable TestPatchJobs *uiJobList - Subsystems []*uiBugSubsystem + Labels []*uiBugLabel } const ( @@ -304,11 +304,11 @@ type uiBug struct { MissingOn []string NumManagers int LastActivity time.Time - Subsystems []*uiBugSubsystem + Labels []*uiBugLabel Discussions DiscussionSummary } -type uiBugSubsystem struct { +type uiBugLabel struct { Name string Link string } @@ -373,15 +373,16 @@ type userBugFilter struct { Manager string // show bugs that happened on the manager OnlyManager string // show bugs that happened ONLY on the manager Subsystem string // only show bugs belonging to the subsystem + Label string // TODO: support multiple. NoSubsystem bool } func MakeBugFilter(r *http.Request) *userBugFilter { return &userBugFilter{ - Subsystem: r.FormValue("subsystem"), NoSubsystem: r.FormValue("no_subsystem") != "", Manager: r.FormValue("manager"), OnlyManager: r.FormValue("only_manager"), + Label: r.FormValue("label"), } } @@ -410,20 +411,26 @@ func (filter *userBugFilter) MatchBug(bug *Bug) bool { if filter.Manager != "" && !stringInList(bug.HappenedOn, filter.Manager) { return false } - if filter.NoSubsystem && len(bug.Tags.Subsystems) > 0 { + if filter.NoSubsystem && len(bug.LabelValues(SubsystemLabel)) > 0 { return false } - if filter.Subsystem != "" && !bug.hasSubsystem(filter.Subsystem) { - return false + if filter.Label != "" { + label, value := splitLabel(filter.Label) + return bug.HasLabel(label, value) } return true } +func splitLabel(rawLabel string) (BugLabelType, string) { + label, value, _ := strings.Cut(rawLabel, ":") + return BugLabelType(label), value +} + func (filter *userBugFilter) Any() bool { if filter == nil { return false } - return filter.Subsystem != "" || filter.OnlyManager != "" || filter.Manager != "" || filter.NoSubsystem + return filter.Label != "" || filter.OnlyManager != "" || filter.Manager != "" || filter.NoSubsystem } // handleMain serves main page. @@ -784,8 +791,8 @@ func handleBug(c context.Context, w http.ResponseWriter, r *http.Request) error SampleReport: sampleReport, Crashes: crashesTable, } - for _, entry := range bug.Tags.Subsystems { - data.Subsystems = append(data.Subsystems, makeBugSubsystemUI(c, bug, entry)) + for _, entry := range bug.Labels { + data.Labels = append(data.Labels, makeBugLabelUI(c, bug, entry)) } // bug.BisectFix is set to BisectNot in two cases : // - no fix bisections have been performed on the bug @@ -815,19 +822,30 @@ func handleBug(c context.Context, w http.ResponseWriter, r *http.Request) error return serveTemplate(w, "bug.html", data) } -func makeBugSubsystemUI(c context.Context, bug *Bug, entry BugSubsystem) *uiBugSubsystem { +func makeBugLabelUI(c context.Context, bug *Bug, entry BugLabel) *uiBugLabel { url := getCurrentURL(c) - // By default let's point to the subsystem's page. - link := fmt.Sprintf("/%s/s/%s", bug.Namespace, entry.Name) - if strings.HasPrefix(url, "/"+bug.Namespace) && - !strings.Contains(url, "/s/") { - // If we're on a main or terminal page, let's amend the bug filter instead. - link = html.AmendURL(url, "subsystem", entry.Name) - } - return &uiBugSubsystem{ - Name: entry.Name, + filterValue := string(entry.Label) + ":" + entry.Value + + // If we're on a main/terminal/subsystem page, let's stay there. + link := url + if !strings.HasPrefix(url, "/"+bug.Namespace) { + link = fmt.Sprintf("/%s", bug.Namespace) + } + link = html.AmendURL(link, "label", filterValue) + + ret := &uiBugLabel{ + Name: entry.String(), Link: link, } + // Patch depending on the specific label type. + switch entry.Label { + case SubsystemLabel: + // Prefer link to the per-subsystem page. + if !strings.HasPrefix(url, "/"+bug.Namespace) || strings.Contains(url, "/s/") { + ret.Link = fmt.Sprintf("/%s/s/%s", bug.Namespace, entry.Value) + } + } + return ret } func getBugDiscussionsUI(c context.Context, bug *Bug) ([]*uiBugDiscussion, error) { @@ -1323,10 +1341,11 @@ func fetchTerminalBugs(c context.Context, accessLevel AccessLevel, } func applyBugFilter(query *db.Query, filter *userBugFilter) *db.Query { - manager, subsystem := filter.ManagerName(), filter.Subsystem - if subsystem != "" { - // Subsystem filter is more granular, so give it priority. - query = query.Filter("Tags.Subsystems.Name=", subsystem) + manager := filter.ManagerName() + label, value := splitLabel(filter.Label) + if label != EmptyLabel { + query = query.Filter("Labels.Label=", string(label)) + query = query.Filter("Labels.Value=", value) } else if manager != "" { query = query.Filter("HappenedOn=", manager) } @@ -1477,8 +1496,8 @@ func createUIBug(c context.Context, bug *Bug, state *ReportingState, managers [] LastActivity: bug.LastActivity, Discussions: bug.discussionSummary(), } - for _, entry := range bug.Tags.Subsystems { - uiBug.Subsystems = append(uiBug.Subsystems, makeBugSubsystemUI(c, bug, entry)) + for _, entry := range bug.Labels { + uiBug.Labels = append(uiBug.Labels, makeBugLabelUI(c, bug, entry)) } updateBugBadness(c, uiBug) if len(bug.Commits) != 0 { diff --git a/dashboard/app/main_test.go b/dashboard/app/main_test.go index 3a07f5040..be1b3e847 100644 --- a/dashboard/app/main_test.go +++ b/dashboard/app/main_test.go @@ -110,7 +110,7 @@ func TestSubsystemFilterMain(t *testing.T) { } } // Check that filtering on the main page works. - reply, err = c.AuthGET(AccessAdmin, "/test1?subsystem="+subsystemA) + reply, err = c.AuthGET(AccessAdmin, "/test1?label=subsystems:"+subsystemA) c.expectOK(err) for _, title := range []string{crash2.Title} { if bytes.Contains(reply, []byte(title)) { @@ -147,7 +147,7 @@ func TestSubsystemFilterTerminal(t *testing.T) { } // Verify that the filtering works on the invalid bugs page. - reply, err := c.AuthGET(AccessAdmin, "/test1/invalid?subsystem="+subsystemB) + reply, err := c.AuthGET(AccessAdmin, "/test1/invalid?label=subsystems:"+subsystemB) c.expectOK(err) for _, title := range []string{crash1.Title} { if bytes.Contains(reply, []byte(title)) { @@ -179,7 +179,7 @@ func TestMainBugFilters(t *testing.T) { assert.Contains(t, string(reply), build1.Manager) assert.NotContains(t, string(reply), "Applied filters") - reply, err = c.AuthGET(AccessAdmin, "/test1?subsystem=abcd") + reply, err = c.AuthGET(AccessAdmin, "/test1?label=subsystems:abcd") c.expectOK(err) assert.NotContains(t, string(reply), build1.Manager) // managers are hidden assert.Contains(t, string(reply), "Applied filters") // we're seeing a prompt to disable the filter diff --git a/dashboard/app/reporting.go b/dashboard/app/reporting.go index 63ae6d3a7..252a44cb8 100644 --- a/dashboard/app/reporting.go +++ b/dashboard/app/reporting.go @@ -556,14 +556,14 @@ func fillBugReport(c context.Context, rep *dashapi.BugReport, bug *Bug, bugRepor rep.KernelConfigLink = externalLink(c, textKernelConfig, build.KernelConfig) rep.SyzkallerCommit = build.SyzkallerCommit rep.NoRepro = build.Type == BuildFailed - for _, item := range bug.Tags.Subsystems { + for _, item := range bug.LabelValues(SubsystemLabel) { rep.Subsystems = append(rep.Subsystems, dashapi.BugSubsystem{ - Name: item.Name, + Name: item.Value, SetBy: item.SetBy, - Link: fmt.Sprintf("%v/%s/s/%s", appURL(c), bug.Namespace, item.Name), + Link: fmt.Sprintf("%v/%s/s/%s", appURL(c), bug.Namespace, item.Value), }) rep.Maintainers = email.MergeEmailLists(rep.Maintainers, - subsystemMaintainers(c, rep.Namespace, item.Name)) + subsystemMaintainers(c, rep.Namespace, item.Value)) } for _, addr := range bug.UNCC { rep.CC = email.RemoveFromEmailList(rep.CC, addr) diff --git a/dashboard/app/reporting_email.go b/dashboard/app/reporting_email.go index e76aa39e6..6038a8539 100644 --- a/dashboard/app/reporting_email.go +++ b/dashboard/app/reporting_email.go @@ -22,7 +22,6 @@ import ( "github.com/google/syzkaller/pkg/email" "github.com/google/syzkaller/pkg/email/lore" "github.com/google/syzkaller/pkg/html" - "github.com/google/syzkaller/pkg/subsystem" "golang.org/x/net/context" "google.golang.org/appengine/v2" db "google.golang.org/appengine/v2/datastore" @@ -584,7 +583,9 @@ func handleBugCommand(c context.Context, bugInfo *bugInfoResult, msg *email.Emai case email.CmdTest: return handleTestCommand(c, bugInfo, msg, command) case email.CmdSet: - return handleSetCommand(c, bugInfo, msg, command) + return handleSetCommand(c, bugInfo.bug, msg, command) + case email.CmdUnset: + return handleUnsetCommand(c, bugInfo.bug, msg, command) case email.CmdUpstream, email.CmdInvalid, email.CmdUnDup: case email.CmdFix: if command.Args == "" { @@ -695,59 +696,120 @@ func handleTestCommand(c context.Context, info *bugInfoResult, } var ( - // The format is `#syz set KEY: value(s)`. We also tolerate `#syz set KEY value(s)`. - setCmdRe = regexp.MustCompile(`^(\w+):?\s*(.*?)\s*$`) - setCmdArgSplitRe = regexp.MustCompile(`[\s,]+`) - cmdParseFailureFmt = `I've failed to parse your command. Please use the following format: -#syz set subsystems: some-subsystem-A, some-subsystem-B - -The list of subsystems can be found at %s` - cmdNoSubsystems = `Subsystems assignment is not yet supported for this kernel. + // The supported formats are: + // For bugs: + // #syz set LABEL[: value_1, [value_2, ....]] + // For bug lists: + // #syz set LABEL[: value_1, [value_2, ....]] + setCmdRe = regexp.MustCompile(`(?m)\s*([-\w]+)\s*(?:\:\s*([,\-\w\s]*?))?$`) + setCmdArgSplitRe = regexp.MustCompile(`[\s,]+`) + setBugCmdFormat = `I've failed to parse your command. Please use the following format(s): +#syz set some-flag +#syz set label: value +#syz set subsystems: one-subsystem, another-subsystem + +Or, for bug lists, +#syz set some-flag +#syz set label: value +#syz set subsystems: one-subsystem, another-subsystem + +The following labels are suported: +%s` + setCmdUnknownLabel = `The specified label %q is unknown. +Please use one of the supported labels. + +The following labels are suported: +%s` + setCmdUnknownValue = `The specified label value is incorrect. +%s. +Please use one of the supported label values. + +The following labels are suported: +%s` + cmdInternalErrorReply = `The command was not executed due to an internal error. Please contact the bot's maintainers.` - cmdUnknownSubsystemFmt = `Please use subsystem names from the list of supported subsystems: -%s - -If you believe that the subsystem list should be changed, please contact the bot's maintainers.` - cmdNoSubsystemsSetFmt = `You must specify at least one subsystem name. -The list of available subsystems can be found at %s` ) -func handleSetCommand(c context.Context, info *bugInfoResult, - msg *email.Email, command *email.SingleCommand) string { - subsystemsURL := fmt.Sprintf("%v/%v/subsystems?all=true", appURL(c), info.bug.Namespace) +func handleSetCommand(c context.Context, bug *Bug, msg *email.Email, + command *email.SingleCommand) string { + labelSet := makeLabelSet(c, bug.Namespace) + match := setCmdRe.FindStringSubmatch(command.Args) - if len(match) != 3 { - return fmt.Sprintf(cmdParseFailureFmt, subsystemsURL) - } - cmd, args := match[1], match[2] - // Now we only support setting bug's subsystems. - // Also let's tolerate both subsystem spellings. - if cmd != "subsystem" && cmd != "subsystems" { - return fmt.Sprintf(cmdParseFailureFmt, subsystemsURL) - } - service := getSubsystemService(c, info.bug.Namespace) - if service == nil { - return cmdNoSubsystems - } - if args == "" { - return fmt.Sprintf(cmdNoSubsystemsSetFmt, subsystemsURL) - } - subsystems := []*subsystem.Subsystem{} - for _, name := range setCmdArgSplitRe.Split(args, -1) { - item := service.ByName(name) - if item == nil { - return fmt.Sprintf(cmdUnknownSubsystemFmt, subsystemsURL) - } - subsystems = append(subsystems, item) + if match == nil { + return fmt.Sprintf(setBugCmdFormat, labelSet.Help()) + } + label, values := BugLabelType(match[1]), match[2] + log.Infof(c, "label=%s values=%s", label, values) + if !labelSet.FindLabel(label) { + return fmt.Sprintf(setCmdUnknownLabel, label, labelSet.Help()) + } + + log.Infof(c, "setting %#v values for %q (label %q)", values, bug.displayTitle(), label) + var labels []BugLabel + for _, value := range unique(setCmdArgSplitRe.Split(values, -1)) { + labels = append(labels, BugLabel{ + Label: label, + Value: value, + SetBy: msg.Author, + Link: msg.Link, + }) + } + var setError error + err := updateSingleBug(c, bug.key(c), func(bug *Bug) error { + setError = bug.SetLabels(labelSet, labels) + return setError + }) + if setError != nil { + return fmt.Sprintf(setCmdUnknownValue, setError, labelSet.Help()) } - // All the replies below will only be sent to the initiator and the mailing list. - msg.Cc = []string{info.reporting.Config.(*EmailConfig).Email} - err := updateBugSubsystems(c, info.bugKey, subsystems, userAssignment(msg.Author)) if err != nil { - log.Errorf(c, "failed to assign bug's subsystems: %s", err) - return "I've failed to update subsystems due to an internal error.\n" + log.Errorf(c, "failed to set bug tags: %s", err) + return cmdInternalErrorReply } - return "Thank you!\n\nI've successfully updated the bug's subsystems." + return "" +} + +var ( + unsetBugCmdFormat = `I've failed to parse your command. Please use the following format(s): +#syz unset any-label + +Or, for bug lists, +#syz unset any-label +` + unsetLabelsNotFound = `The following labels did not exist: %s` +) + +func handleUnsetCommand(c context.Context, bug *Bug, msg *email.Email, + command *email.SingleCommand) string { + match := setCmdRe.FindStringSubmatch(command.Args) + if match == nil { + return unsetBugCmdFormat + } + var labels []BugLabelType + for _, name := range unique(setCmdArgSplitRe.Split(command.Args, -1)) { + labels = append(labels, BugLabelType(name)) + } + + var notFound map[BugLabelType]struct{} + var notFoundErr = fmt.Errorf("some labels were not found") + err := updateSingleBug(c, bug.key(c), func(bug *Bug) error { + notFound = bug.UnsetLabels(labels...) + if len(notFound) > 0 { + return notFoundErr + } + return nil + }) + if err == notFoundErr { + var names []string + for label := range notFound { + names = append(names, string(label)) + } + return fmt.Sprintf(unsetLabelsNotFound, strings.Join(names, ", ")) + } else if err != nil { + log.Errorf(c, "failed to unset bug labels: %s", err) + return cmdInternalErrorReply + } + return "" } func handleEmailBounce(w http.ResponseWriter, r *http.Request) { @@ -765,6 +827,18 @@ func handleEmailBounce(w http.ResponseWriter, r *http.Request) { log.Infof(c, "%s", body) } +var ( + setGroupCmdRe = regexp.MustCompile(`(?m)\s*<(\d+)>\s*(.*)$`) + setGroupCmdFormat = `I've failed to parse your command. Please use the following format(s): +#syz set some-label, another-label +#syz set subsystems: one-subsystem, another-subsystem +#syz unset some-label +` + setGroupCmdBadRef = `The specified number is invalid. It must be one of the values +listed in the bug list table. +` +) + func handleBugListCommand(c context.Context, bugListInfo *bugListInfoResult, msg *email.Email, command *email.SingleCommand) string { upd := &dashapi.BugListUpdate{ @@ -772,18 +846,46 @@ func handleBugListCommand(c context.Context, bugListInfo *bugListInfoResult, ExtID: msg.MessageID, Link: msg.Link, } - if command.Command == email.CmdUpstream { + switch command.Command { + case email.CmdUpstream: upd.Command = dashapi.BugListUpstreamCmd - } else if command.Command == email.CmdRegenerate { + case email.CmdRegenerate: upd.Command = dashapi.BugListRegenerateCmd - } else { + case email.CmdSet, email.CmdUnset: + // Extract and cut the part. + match := setGroupCmdRe.FindStringSubmatch(command.Args) + if match == nil { + return setGroupCmdFormat + } + ref, args := match[1], match[2] + numRef, err := strconv.Atoi(ref) + if err != nil { + return setGroupCmdFormat + } + if numRef < 1 || numRef > len(bugListInfo.keys) { + return setGroupCmdBadRef + } + bugKey := bugListInfo.keys[numRef-1] + bug := new(Bug) + if err := db.Get(c, bugKey, bug); err != nil { + log.Errorf(c, "failed to fetch bug by key %s: %s", bugKey, err) + return cmdInternalErrorReply + } + command.Args = args + switch command.Command { + case email.CmdSet: + return handleSetCommand(c, bug, msg, command) + case email.CmdUnset: + return handleUnsetCommand(c, bug, msg, command) + } + default: upd.Command = dashapi.BugListUpdateCmd } log.Infof(c, "bug list update: id=%s, cmd=%v", upd.ID, upd.Command) reply, err := reportingBugListCommand(c, upd) if err != nil { log.Errorf(c, "bug list command failed: %s", err) - return "" + return cmdInternalErrorReply } return reply } @@ -794,6 +896,7 @@ var nonCriticalBounceRe = regexp.MustCompile(`\*\* Address not found \*\*|550 #5 type bugListInfoResult struct { id string config *EmailConfig + keys []*db.Key } func identifyEmail(c context.Context, msg *email.Email) (*bugInfoResult, *bugListInfoResult, *EmailConfig) { @@ -803,7 +906,7 @@ func identifyEmail(c context.Context, msg *email.Email) (*bugInfoResult, *bugLis bugID = msg.BugIDs[0] } if isBugListHash(bugID) { - subsystem, _, stage, err := findSubsystemReportByID(c, bugID) + subsystem, report, stage, err := findSubsystemReportByID(c, bugID) if err != nil { log.Errorf(c, "findBugListByID failed: %s", err) return nil, nil, nil @@ -822,7 +925,16 @@ func identifyEmail(c context.Context, msg *email.Email) (*bugInfoResult, *bugLis log.Errorf(c, "bug list's reporting config is not EmailConfig (id=%v)", bugID) return nil, nil, nil } - return nil, &bugListInfoResult{id: bugID, config: emailConfig}, emailConfig + keys, err := report.getBugKeys() + if err != nil { + log.Errorf(c, "failed to extract keys from bug list: %s", err) + return nil, nil, nil + } + return nil, &bugListInfoResult{ + id: bugID, + config: emailConfig, + keys: keys, + }, emailConfig } bugInfo := loadBugInfo(c, msg) if bugInfo == nil { diff --git a/dashboard/app/reporting_lists.go b/dashboard/app/reporting_lists.go index 0fe490f02..eb5f2b835 100644 --- a/dashboard/app/reporting_lists.go +++ b/dashboard/app/reporting_lists.go @@ -340,7 +340,8 @@ func queryMatchingBugs(c context.Context, ns, name string, accessLevel AccessLev allOpenBugs, _, err := loadAllBugs(c, func(query *db.Query) *db.Query { return query.Filter("Namespace=", ns). Filter("Status=", BugStatusOpen). - Filter("Tags.Subsystems.Name=", name) + Filter("Labels.Label=", SubsystemLabel). + Filter("Labels.Value=", name) }) if err != nil { return nil, nil, fmt.Errorf("failed to query open bugs for subsystem: %w", err) @@ -348,7 +349,8 @@ func queryMatchingBugs(c context.Context, ns, name string, accessLevel AccessLev allFixedBugs, _, err := loadAllBugs(c, func(query *db.Query) *db.Query { return query.Filter("Namespace=", ns). Filter("Status=", BugStatusFixed). - Filter("Tags.Subsystems.Name=", name) + Filter("Labels.Label=", SubsystemLabel). + Filter("Labels.Value=", name) }) if err != nil { return nil, nil, fmt.Errorf("failed to query fixed bugs for subsystem: %w", err) diff --git a/dashboard/app/subsystem.go b/dashboard/app/subsystem.go index 7b7b5f182..2478f09af 100644 --- a/dashboard/app/subsystem.go +++ b/dashboard/app/subsystem.go @@ -90,49 +90,38 @@ func bugsToUpdateSubsystems(c context.Context, ns string, count int) ([]*Bug, [] } func checkOutdatedSubsystems(c context.Context, service *subsystem.Service, bug *Bug) { - for _, item := range bug.Tags.Subsystems { - if service.ByName(item.Name) == nil { - log.Errorf(c, "ns=%s bug=%s subsystem %s no longer exists", bug.Namespace, bug.Title, item.Name) + for _, item := range bug.LabelValues(SubsystemLabel) { + if service.ByName(item.Value) == nil { + log.Errorf(c, "ns=%s bug=%s subsystem %s no longer exists", bug.Namespace, bug.Title, item.Value) } } } type ( autoInference int - userAssignment string updateRevision int ) func updateBugSubsystems(c context.Context, bugKey *db.Key, list []*subsystem.Subsystem, info interface{}) error { now := timeNow(c) - tx := func(c context.Context) error { - bug := new(Bug) - if err := db.Get(c, bugKey, bug); err != nil { - return fmt.Errorf("failed to get bug: %v", err) - } + return updateSingleBug(c, bugKey, func(bug *Bug) error { switch v := info.(type) { case autoInference: logSubsystemChange(c, bug, list) - bug.SetAutoSubsystems(list, now, int(v)) - case userAssignment: - bug.SetUserSubsystems(list, now, string(v)) + bug.SetAutoSubsystems(c, list, now, int(v)) case updateRevision: bug.SubsystemsRev = int(v) bug.SubsystemsTime = now } - if _, err := db.Put(c, bugKey, bug); err != nil { - return fmt.Errorf("failed to put bug: %v", err) - } return nil - } - return db.RunInTransaction(c, tx, &db.TransactionOptions{Attempts: 10}) + }) } func logSubsystemChange(c context.Context, bug *Bug, new []*subsystem.Subsystem) { var oldNames, newNames []string - for _, item := range bug.Tags.Subsystems { - oldNames = append(oldNames, item.Name) + for _, item := range bug.LabelValues(SubsystemLabel) { + oldNames = append(oldNames, item.Value) } for _, item := range new { newNames = append(newNames, item.Name) @@ -225,3 +214,10 @@ func getSubsystemRevision(c context.Context, ns string) int { } return config.Namespaces[ns].Subsystems.Revision } + +func subsystemListURL(c context.Context, ns string) string { + if getSubsystemService(c, ns) == nil { + return "" + } + return fmt.Sprintf("%v/%v/subsystems?all=true", appURL(c), ns) +} diff --git a/dashboard/app/subsystem_test.go b/dashboard/app/subsystem_test.go index 6edb9e704..8898d3fe0 100644 --- a/dashboard/app/subsystem_test.go +++ b/dashboard/app/subsystem_test.go @@ -51,7 +51,7 @@ func TestPeriodicSubsystemRefresh(t *testing.T) { extID := rep.ID // Initially there should be no subsystems. - expectSubsystems(t, client, extID) + expectLabels(t, client, extID) // Update subsystems. item := &subsystem.Subsystem{ @@ -65,14 +65,14 @@ func TestPeriodicSubsystemRefresh(t *testing.T) { c.advanceTime(time.Hour) _, err := c.AuthGET(AccessUser, "/cron/refresh_subsystems") c.expectOK(err) - expectSubsystems(t, client, extID) // Not enough time has passed yet. + expectLabels(t, client, extID) // Not enough time has passed yet. // Wait until the refresh period is over. c.advanceTime(openBugsUpdateTime) _, err = c.AuthGET(AccessUser, "/cron/refresh_subsystems") c.expectOK(err) - expectSubsystems(t, client, extID, "first") + expectLabels(t, client, extID, "subsystems:first") } func TestOpenBugRevRefresh(t *testing.T) { @@ -94,7 +94,7 @@ func TestOpenBugRevRefresh(t *testing.T) { extID := rep.ID // Initially there should be no subsystems. - expectSubsystems(t, client, extID) + expectLabels(t, client, extID) // Update subsystems. c.advanceTime(time.Hour) @@ -108,7 +108,7 @@ func TestOpenBugRevRefresh(t *testing.T) { // Refresh subsystems. _, err := c.AuthGET(AccessUser, "/cron/refresh_subsystems") c.expectOK(err) - expectSubsystems(t, client, extID, "first") + expectLabels(t, client, extID, "subsystems:first") } func TestClosedBugSubsystemRefresh(t *testing.T) { @@ -145,7 +145,7 @@ func TestClosedBugSubsystemRefresh(t *testing.T) { c.expectEQ(bug.Status, BugStatusFixed) // Initially there should be no subsystems. - expectSubsystems(t, client, extID) + expectLabels(t, client, extID) // Update subsystems. c.advanceTime(time.Hour) @@ -159,7 +159,7 @@ func TestClosedBugSubsystemRefresh(t *testing.T) { c.advanceTime(time.Hour) _, err := c.AuthGET(AccessUser, "/cron/refresh_subsystems") c.expectOK(err) - expectSubsystems(t, client, extID, "first") + expectLabels(t, client, extID, "subsystems:first") } func TestUserSubsystemsRefresh(t *testing.T) { @@ -183,21 +183,20 @@ func TestUserSubsystemsRefresh(t *testing.T) { c.expectOK(err) // Make sure we've set the right subsystem. - expectSubsystems(t, client, extID, "subsystemA") + expectLabels(t, client, extID, "subsystems:subsystemA") // Manually set another subsystem. c.incomingEmail(sender, "#syz set subsystems: subsystemB\n", EmailOptFrom("test@requester.com")) - c.pollEmailBug() - expectSubsystems(t, client, extID, "subsystemB") + expectLabels(t, client, extID, "subsystems:subsystemB") // Refresh subsystems. c.advanceTime(openBugsUpdateTime + time.Hour) _, err = c.AuthGET(AccessUser, "/cron/refresh_subsystems") c.expectOK(err) - // The subsystem must stay the same. - expectSubsystems(t, client, extID, "subsystemB") + // The subsystems must stay the same. + expectLabels(t, client, extID, "subsystems:subsystemB") // Bump the subsystem revision and refresh subsystems. c.setSubsystems(ns, testSubsystems, 2) @@ -206,17 +205,7 @@ func TestUserSubsystemsRefresh(t *testing.T) { c.expectOK(err) // The subsystem must still stay the same. - expectSubsystems(t, client, extID, "subsystemB") -} - -func expectSubsystems(t *testing.T, client *apiClient, extID string, subsystems ...string) { - t.Helper() - bug, _, _ := client.Ctx.loadBug(extID) - names := []string{} - for _, item := range bug.Tags.Subsystems { - names = append(names, item.Name) - } - assert.ElementsMatch(t, names, subsystems) + expectLabels(t, client, extID, "subsystems:subsystemB") } // nolint: goconst @@ -684,11 +673,11 @@ In total, 3 issues are still open. Some of the still happening issues: -Crashes Repro Title -2 No WARNING: a first - https://testapp.appspot.com/bug?extid=%[1]v -2 No WARNING: a third - https://testapp.appspot.com/bug?extid=%[2]v +Ref Crashes Repro Title +<1> 2 No WARNING: a first + https://testapp.appspot.com/bug?extid=%[1]v +<2> 2 No WARNING: a third + https://testapp.appspot.com/bug?extid=%[2]v The report will be sent to: [subsystemA@list.com subsystemA@person.com]. If the report looks fine to you, please send the "syz upstream" command. diff --git a/dashboard/app/templates.html b/dashboard/app/templates.html index 18371e94e..f65b1faca 100644 --- a/dashboard/app/templates.html +++ b/dashboard/app/templates.html @@ -99,12 +99,12 @@ Use of this source code is governed by Apache 2 LICENSE that can be found in the {{if .Filter.OnlyManager}} Only Manager={{.Filter.OnlyManager}} ({{link (call .DropURL "only_manager") "drop"}}) {{end}} - {{if .Filter.Subsystem}} - Subsystem={{.Filter.Subsystem}} ({{link (call .DropURL "subsystem") "drop"}}) - {{end}} {{if .Filter.NoSubsystem}} NoSubsystem={{.Filter.NoSubsystem}} ({{link (call .DropURL "no_subsystem") "drop"}}) {{end}} + {{if .Filter.Label}} + Label={{.Filter.Label}} ({{link (call .DropURL "label") "drop"}}) + {{end}}
{{end}} {{end}} @@ -160,8 +160,8 @@ Use of this source code is governed by Apache 2 LICENSE that can be found in the {{if $.ShowNamespace}}{{$b.Namespace}}{{end}} {{$b.Title}} - {{- range $b.Subsystems}} - {{link .Link .Name}} + {{- range $b.Labels}} + {{link .Link .Name}} {{- end}} {{formatReproLevel $b.ReproLevel}} diff --git a/pkg/email/parser.go b/pkg/email/parser.go index 152f5e555..cd454646c 100644 --- a/pkg/email/parser.go +++ b/pkg/email/parser.go @@ -53,6 +53,7 @@ const ( CmdInvalid CmdUnCC CmdSet + CmdUnset CmdRegenerate cmdTest5 @@ -296,7 +297,7 @@ func extractCommand(body string) (*SingleCommand, int) { switch cmd { case CmdTest: args = extractArgsTokens(body[cmdPos+cmdEnd:], 2) - case CmdSet: + case CmdSet, CmdUnset: args = extractArgsLine(body[cmdPos+cmdEnd:]) case cmdTest5: args = extractArgsTokens(body[cmdPos+cmdEnd:], 5) @@ -332,6 +333,8 @@ func strToCmd(str string) Command { return CmdUnCC case "set", "set:": return CmdSet + case "unset", "unset:": + return CmdUnset case "regenerate": return CmdRegenerate case "test_5_arg_cmd": diff --git a/pkg/email/parser_test.go b/pkg/email/parser_test.go index ee39d6164..0d3c54c87 100644 --- a/pkg/email/parser_test.go +++ b/pkg/email/parser_test.go @@ -375,6 +375,16 @@ baz }, { body: ` +#syz unset some tag +`, + cmd: &SingleCommand{ + Command: CmdUnset, + Str: "unset", + Args: "some tag", + }, + }, + { + body: ` #syz fix: abcd #syz fix: xyz `, diff --git a/pkg/html/pages/style.css b/pkg/html/pages/style.css index 661886e41..3bff3a9ed 100644 --- a/pkg/html/pages/style.css +++ b/pkg/html/pages/style.css @@ -207,7 +207,7 @@ table td, table th { font-size: 75%; } -.subsystem { +.bug-label { background: white; border: 1pt solid black; display: inline-block; @@ -217,7 +217,7 @@ table td, table th { font-size: small; } -.subsystem a { +.bug-label a { text-decoration: none; color: black; } -- cgit mrf-deployment