Skip to content

Commit

Permalink
bugzilla: Fix bugs
Browse files Browse the repository at this point in the history
  • Loading branch information
AlexNPavel committed May 1, 2020
1 parent e41dc79 commit 519688c
Show file tree
Hide file tree
Showing 6 changed files with 212 additions and 20 deletions.
61 changes: 59 additions & 2 deletions prow/bugzilla/client.go
Expand Up @@ -39,6 +39,8 @@ type Client interface {
GetBug(id int) (*Bug, error)
GetComments(id int) ([]Comment, error)
GetExternalBugPRsOnBug(id int) ([]ExternalBug, error)
GetSubComponentsOnBug(id int) (map[string][]string, error)
GetClones(bug *Bug) ([]*Bug, error)
CreateBug(bug *BugCreate) (int, error)
CloneBug(bug *Bug) (int, error)
UpdateBug(id int, update BugUpdate) error
Expand Down Expand Up @@ -92,6 +94,54 @@ func (c *client) GetBug(id int) (*Bug, error) {
return parsedResponse.Bugs[0], nil
}

func getClones(c Client, bug *Bug) ([]*Bug, error) {
clones := []*Bug{}
for _, dependentID := range bug.Blocks {
dependent, err := c.GetBug(dependentID)
if err != nil {
return nil, fmt.Errorf("Failed to get dependent bug #%d: %v", dependentID, err)
}
if dependent.Summary == bug.Summary {
clones = append(clones, dependent)
}
}
return clones, nil
}

// GetClones gets the list of bugs that the provided bug blocks that also have a matching summary.
func (c *client) GetClones(bug *Bug) ([]*Bug, error) {
return getClones(c, bug)
}

// GetSubComponentsOnBug retrieves a the list of SubComponents of the bug.
// SubComponents are a Red Hat bugzilla specific extra field.
func (c *client) GetSubComponentsOnBug(id int) (map[string][]string, error) {
logger := c.logger.WithFields(logrus.Fields{methodField: "GetSubComponentsOnBug", "id": id})
req, err := http.NewRequest(http.MethodGet, fmt.Sprintf("%s/rest/bug/%d", c.endpoint, id), nil)
if err != nil {
return nil, err
}
values := req.URL.Query()
values.Add("include_fields", "sub_components")
req.URL.RawQuery = values.Encode()
raw, err := c.request(req, logger)
if err != nil {
return nil, err
}
var parsedResponse struct {
Bugs []struct {
SubComponents map[string][]string `json:"sub_components"`
} `json:"bugs"`
}
if err := json.Unmarshal(raw, &parsedResponse); err != nil {
return nil, fmt.Errorf("could not unmarshal response body: %v", err)
}
if len(parsedResponse.Bugs) != 1 {
return nil, fmt.Errorf("did not get one bug, but %d: %v", len(parsedResponse.Bugs), parsedResponse)
}
return parsedResponse.Bugs[0].SubComponents, nil
}

// GetExternalBugPRsOnBug retrieves external bugs on a Bug from the server
// and returns any that reference a Pull Request in GitHub
// https://bugzilla.readthedocs.io/en/latest/api/core/v1/bug.html#get-bug
Expand Down Expand Up @@ -188,7 +238,7 @@ func (c *client) CreateBug(bug *BugCreate) (int, error) {
return idStruct.ID, nil
}

func cloneBugStruct(bug *Bug, comments []Comment) *BugCreate {
func cloneBugStruct(bug *Bug, subcomponents map[string][]string, comments []Comment) *BugCreate {
newBug := &BugCreate{
Alias: bug.Alias,
AssignedTo: bug.AssignedTo,
Expand All @@ -207,6 +257,9 @@ func cloneBugStruct(bug *Bug, comments []Comment) *BugCreate {
TargetMilestone: bug.TargetMilestone,
Version: bug.Version,
}
if len(subcomponents) > 0 {
newBug.SubComponents = subcomponents
}
if len(comments) > 0 && comments[0].Count == 0 {
desc := comments[0]
newBug.Description = fmt.Sprintf("This is a clone of Bug #%d. This is the description of that bug:\n%s", bug.ID, desc.Text)
Expand All @@ -225,11 +278,15 @@ func cloneBugStruct(bug *Bug, comments []Comment) *BugCreate {
// clone handles the bz client calls for the bug cloning process and allows us to share the implementation
// between the real and fake client to prevent bugs from accidental discrepencies between the two.
func clone(c Client, bug *Bug) (int, error) {
subcomponents, err := c.GetSubComponentsOnBug(bug.ID)
if err != nil {
return 0, fmt.Errorf("failed to check if bug has subcomponents: %v", err)
}
comments, err := c.GetComments(bug.ID)
if err != nil {
return 0, fmt.Errorf("failed to get parent bug's comments: %v", err)
}
id, err := c.CreateBug(cloneBugStruct(bug, comments))
id, err := c.CreateBug(cloneBugStruct(bug, subcomponents, comments))
if err != nil {
return id, err
}
Expand Down
2 changes: 1 addition & 1 deletion prow/bugzilla/client_test.go
Expand Up @@ -340,7 +340,7 @@ func TestCloneBugStruct(t *testing.T) {
},
}}
for _, testCase := range testCases {
newBug := cloneBugStruct(&testCase.bug, testCase.comments)
newBug := cloneBugStruct(&testCase.bug, nil, testCase.comments)
if !reflect.DeepEqual(*newBug, testCase.expected) {
t.Errorf("%s: Difference in expected BugCreate and actual: %s", testCase.name, cmp.Diff(testCase.expected, *newBug))
}
Expand Down
23 changes: 23 additions & 0 deletions prow/bugzilla/fake.go
Expand Up @@ -31,6 +31,7 @@ type Fake struct {
BugErrors sets.Int
BugCreateErrors sets.String
ExternalBugs map[int][]ExternalBug
SubComponents map[int]map[string][]string
}

// Endpoint returns the endpoint for this fake
Expand Down Expand Up @@ -81,6 +82,11 @@ func (c *Fake) UpdateBug(id int, update BugUpdate) error {
} else {
bug.DependsOn = sets.NewInt(bug.DependsOn...).Insert(update.DependsOn.Add...).Delete(update.DependsOn.Remove...).List()
}
for _, blockerID := range bug.DependsOn {
blockerBug := c.Bugs[blockerID]
blockerBug.Blocks = append(blockerBug.Blocks, id)
c.Bugs[blockerID] = blockerBug
}
}
c.Bugs[id] = bug
return nil
Expand Down Expand Up @@ -165,6 +171,9 @@ func (c *Fake) CreateBug(bug *BugCreate) (int, error) {
Tags: bug.CommentTags,
}}
c.BugComments[newID] = newComments
if bug.SubComponents != nil {
c.SubComponents[newID] = bug.SubComponents
}
return newID, nil
}

Expand All @@ -185,5 +194,19 @@ func (c *Fake) CloneBug(bug *Bug) (int, error) {
return clone(c, bug)
}

func (c *Fake) GetSubComponentsOnBug(id int) (map[string][]string, error) {
if c.BugErrors.Has(id) {
return nil, errors.New("injected error getting bug subcomponents")
}
return c.SubComponents[id], nil
}

func (c *Fake) GetClones(bug *Bug) ([]*Bug, error) {
if c.BugErrors.Has(bug.ID) {
return nil, errors.New("injected error getting subcomponents")
}
return getClones(c, bug)
}

// the Fake is a Client
var _ Client = &Fake{}
3 changes: 3 additions & 0 deletions prow/bugzilla/types.go
Expand Up @@ -149,6 +149,9 @@ type BugCreate struct {
Severity string `json:"severity,omitempty"`
// Status is the current status of the bug.
Status string `json:"status,omitempty"`
// SubComponents are the subcomponents of the component for the bug. The key is the Component name, while the value is an array of length 1 containing the subcomponent name.
// This is a Red Hat bugzilla specific extra field.
SubComponents map[string][]string `json:"sub_components,omitempty"`
// Summary is the summary of this bug.
Summary string `json:"summary,omitempty"`
// TargetMilestone is the milestone that this bug is supposed to be fixed by, or for closed bugs, the milestone that it was fixed for.
Expand Down
16 changes: 13 additions & 3 deletions prow/plugins/bugzilla/bugzilla.go
Expand Up @@ -845,7 +845,7 @@ func handleCherrypick(e event, gc githubClient, bc bugzilla.Client, options plug
pr, err := gc.GetPullRequest(e.org, e.repo, e.cherrypickFromPRNum)
if err != nil {
log.WithError(err).Warn("Unexpected error getting title of pull request being cherrypicked from.")
return comment(formatError(fmt.Sprintf("creating a cherry-pick bug in Bugzilla: failed to check the state of cherrypicked pull request at https://github.com/%s/%s/pull/%d", e.org, e.repo, e.cherrypickFromPRNum), bc.Endpoint(), e.bugId, err))
return comment(fmt.Sprintf("Error creating a cherry-pick bug in Bugzilla: failed to check the state of cherrypicked pull request at https://github.com/%s/%s/pull/%d: %v.\nPlease contact an administrator to resolve this issue, then request a bug refresh with <code>/bugzilla refresh</code>.", e.org, e.repo, e.cherrypickFromPRNum, err))
}
// Attempt to identify bug from PR title
bugID, bugMissing, err := bugIDFromTitle(pr.Title)
Expand All @@ -856,7 +856,7 @@ func handleCherrypick(e event, gc githubClient, bc bugzilla.Client, options plug
} else if err != nil {
// should be impossible based on the regex
log.WithError(err).Debugf("Failed to get bug ID from PR title \"%s\"", pr.Title)
return comment(formatError(fmt.Sprintf("creating a cherry-pick bug in Bugzilla: could not get bug ID from PR title \"%s\"", pr.Title), bc.Endpoint(), 0, err))
return comment(fmt.Sprintf("Error creating a cherry-pick bug in Bugzilla: could not get bug ID from PR title \"%s\": %v", pr.Title, err))
}
oldLink := fmt.Sprintf(bugLink, bugID, bc.Endpoint(), bugID)
// Since getBug generates a comment itself, we have to add a prefix explaining that this was a cherrypick attempt to the comment
Expand All @@ -867,6 +867,16 @@ func handleCherrypick(e event, gc githubClient, bc bugzilla.Client, options plug
if err != nil || bug == nil {
return err
}
clones, err := bc.GetClones(bug)
if err != nil {
return comment(formatError(fmt.Sprintf("creating a cherry-pick bug in Bugzilla: could not get list of clones for %s", oldLink), bc.Endpoint(), bug.ID, err))
}
targetVersion := *options.TargetRelease
for _, clone := range clones {
if len(clone.Version) == 1 && clone.Version[0] == *options.TargetRelease {
return comment(fmt.Sprintf("Not creating new clone for %s as %s has been detected as a clone for the correct target version of this cherrypick. Running refresh:\n/bugzilla refresh", oldLink, fmt.Sprintf(bugLink, clone.ID, bc.Endpoint(), clone.ID)))
}
}
cloneID, err := bc.CloneBug(bug)
if err != nil {
log.WithError(err).Debugf("Failed to clone bug %d", bugID)
Expand All @@ -875,7 +885,7 @@ func handleCherrypick(e event, gc githubClient, bc bugzilla.Client, options plug
cloneLink := fmt.Sprintf(bugLink, cloneID, bc.Endpoint(), cloneID)
// Update the version of the bug to the target release
update := bugzilla.BugUpdate{
Version: *options.TargetRelease,
Version: targetVersion,
}
err = bc.UpdateBug(cloneID, update)
if err != nil {
Expand Down
127 changes: 113 additions & 14 deletions prow/plugins/bugzilla/bugzilla_test.go
Expand Up @@ -674,18 +674,20 @@ func TestHandle(t *testing.T) {
cherryPickFromPRNum int
cherryPickTo string
// the "e.body" for PRs is the PR title; this field can be used to replace the "body" for PR handles for cases where the body != description
body string
externalBugs []bugzilla.ExternalBug
prs []github.PullRequest
bugs []bugzilla.Bug
bugComments map[int][]bugzilla.Comment
bugErrors []int
bugCreateErrors []string
options plugins.BugzillaBranchOptions
expectedLabels []string
expectedComment string
expectedBug *bugzilla.Bug
expectedExternalBugs []bugzilla.ExternalBug
body string
externalBugs []bugzilla.ExternalBug
prs []github.PullRequest
bugs []bugzilla.Bug
bugComments map[int][]bugzilla.Comment
bugErrors []int
bugCreateErrors []string
subComponents map[int]map[string][]string
options plugins.BugzillaBranchOptions
expectedLabels []string
expectedComment string
expectedBug *bugzilla.Bug
expectedExternalBugs []bugzilla.ExternalBug
expectedSubComponents map[int]map[string][]string
}{
{
name: "no bug found leaves a comment",
Expand Down Expand Up @@ -1151,8 +1153,7 @@ Instructions for interacting with me using PR comments are available [here](http
cherryPickFromPRNum: 1,
cherryPickTo: "v1",
options: plugins.BugzillaBranchOptions{TargetRelease: &v1},
expectedComment: `org/repo#1:@user: An error was encountered creating a cherry-pick bug in Bugzilla: failed to check the state of cherrypicked pull request at https://github.com/org/repo/pull/1 for bug 123 on the Bugzilla server at www.bugzilla:
> pull request number 1 does not exist
expectedComment: `org/repo#1:@user: Error creating a cherry-pick bug in Bugzilla: failed to check the state of cherrypicked pull request at https://github.com/org/repo/pull/1: pull request number 1 does not exist.
Please contact an administrator to resolve this issue, then request a bug refresh with <code>/bugzilla refresh</code>.
<details>
Expand Down Expand Up @@ -1237,6 +1238,97 @@ In response to [this](http.com):
>[v1] Bug 123: fixed it!
Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>`,
}, {
name: "If bug clone with correct target version already exists, do not create new clone",
bugs: []bugzilla.Bug{
{Summary: "This is a test bug", Product: "Test", Component: []string{"TestComponent"}, Version: []string{"v2"}, ID: 123, Status: "CLOSED", Severity: "urgent", Blocks: []int{124}},
{Summary: "This is a test bug", Product: "Test", Component: []string{"TestComponent"}, Version: []string{"v1"}, ID: 124, Status: "NEW", Severity: "urgent", DependsOn: []int{123}},
},
bugComments: map[int][]bugzilla.Comment{123: {{BugID: 123, Count: 0, Text: "This is a bug"}}},
prs: []github.PullRequest{{Number: base.number, Body: base.body, Title: base.body}, {Number: 2, Body: "This is an automated cherry-pick of #1.\n\n/assign user", Title: "[v1] " + base.body}},
body: "[v1] " + base.body,
cherryPick: true,
cherryPickFromPRNum: 1,
cherryPickTo: "v1",
options: plugins.BugzillaBranchOptions{TargetRelease: &v1},
expectedComment: `org/repo#1:@user: Not creating new clone for [Bugzilla bug 123](www.bugzilla/show_bug.cgi?id=123) as [Bugzilla bug 124](www.bugzilla/show_bug.cgi?id=124) has been detected as a clone for the correct target version of this cherrypick. Running refresh:
/bugzilla refresh
<details>
In response to [this](http.com):
>[v1] Bug 123: fixed it!
Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>`,
}, {
name: "Clone for different version does not block creation of new clone",
bugs: []bugzilla.Bug{
{Summary: "This is a test bug", Product: "Test", Component: []string{"TestComponent"}, Version: []string{"v2"}, ID: 123, Status: "CLOSED", Severity: "urgent", Blocks: []int{124}},
{Summary: "This is a test bug", Product: "Test", Component: []string{"TestComponent"}, Version: []string{"v3"}, ID: 124, Status: "NEW", Severity: "urgent", DependsOn: []int{123}},
},
bugComments: map[int][]bugzilla.Comment{123: {{BugID: 123, Count: 0, Text: "This is a bug"}}},
prs: []github.PullRequest{{Number: base.number, Body: base.body, Title: base.body}, {Number: 2, Body: "This is an automated cherry-pick of #1.\n\n/assign user", Title: "[v1] " + base.body}},
body: "[v1] " + base.body,
cherryPick: true,
cherryPickFromPRNum: 1,
cherryPickTo: "v1",
options: plugins.BugzillaBranchOptions{TargetRelease: &v1},
expectedComment: `org/repo#1:@user: [Bugzilla bug 123](www.bugzilla/show_bug.cgi?id=123) has been cloned as [Bugzilla bug 125](www.bugzilla/show_bug.cgi?id=125). Retitling PR to link against new bug.
/retitle [v1] Bug 125: fixed it!
<details>
In response to [this](http.com):
>[v1] Bug 123: fixed it!
Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>`,
}, {
name: "Bug with SubComponents creates bug with correct subcomponents",
bugs: []bugzilla.Bug{{Product: "Test", Component: []string{"TestComponent"}, Version: []string{"v2"}, ID: 123, Status: "CLOSED", Severity: "urgent"}},
bugComments: map[int][]bugzilla.Comment{123: {{BugID: 123, Count: 0, Text: "This is a bug"}}},
subComponents: map[int]map[string][]string{
123: {
"TestComponent": {
"TestSubComponent",
},
},
},
prs: []github.PullRequest{{Number: base.number, Body: base.body, Title: base.body}, {Number: 2, Body: "This is an automated cherry-pick of #1.\n\n/assign user", Title: "[v1] " + base.body}},
body: "[v1] " + base.body,
cherryPick: true,
cherryPickFromPRNum: 1,
cherryPickTo: "v1",
options: plugins.BugzillaBranchOptions{TargetRelease: &v1},
expectedSubComponents: map[int]map[string][]string{
123: {
"TestComponent": {
"TestSubComponent",
},
},
124: {
"TestComponent": {
"TestSubComponent",
},
},
},
expectedComment: `org/repo#1:@user: [Bugzilla bug 123](www.bugzilla/show_bug.cgi?id=123) has been cloned as [Bugzilla bug 124](www.bugzilla/show_bug.cgi?id=124). Retitling PR to link against new bug.
/retitle [v1] Bug 124: fixed it!
<details>
In response to [this](http.com):
>[v1] Bug 123: fixed it!
Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
</details>`,
},
Expand All @@ -1259,6 +1351,7 @@ Instructions for interacting with me using PR comments are available [here](http
bc := bugzilla.Fake{
EndpointString: "www.bugzilla",
Bugs: map[int]bugzilla.Bug{},
SubComponents: map[int]map[string][]string{},
BugComments: testCase.bugComments,
BugErrors: sets.NewInt(),
BugCreateErrors: sets.NewString(),
Expand All @@ -1272,6 +1365,9 @@ Instructions for interacting with me using PR comments are available [here](http
for _, externalBug := range testCase.externalBugs {
bc.ExternalBugs[externalBug.BugzillaBugID] = append(bc.ExternalBugs[externalBug.BugzillaBugID], externalBug)
}
for id, subComponent := range testCase.subComponents {
bc.SubComponents[id] = subComponent
}
e.missing = testCase.missing
e.merged = testCase.merged
e.cherrypick = testCase.cherryPick
Expand Down Expand Up @@ -1313,6 +1409,9 @@ Instructions for interacting with me using PR comments are available [here](http
t.Errorf("%s: got incorrect external bugs after update: %s", testCase.name, cmp.Diff(actual, expected, allowEvent))
}
}
if testCase.expectedSubComponents != nil && !reflect.DeepEqual(bc.SubComponents, testCase.expectedSubComponents) {
t.Errorf("%s: got incorrect subcomponents after update: %s", testCase.name, cmp.Diff(actual, expected))
}
})
}
}
Expand Down

0 comments on commit 519688c

Please sign in to comment.