Skip to content

Commit

Permalink
Merge pull request #13106 from stevekuznetsov/skuznets/enforce-curren…
Browse files Browse the repository at this point in the history 
…t-bugzilla-state

Allow validating Bugzilla bugs on state
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Jun 24, 2019
2 parents f049db9 + 1428dec commit 0b17ca8
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 38 deletions.
1 change: 1 addition & 0 deletions prow/plugins/bugzilla/BUILD.bazel
View file
Expand Up @@ -12,6 +12,7 @@ go_library(
"//prow/pluginhelp:go_default_library",
"//prow/plugins:go_default_library",
"//vendor/github.com/sirupsen/logrus:go_default_library",
"//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
],
)

Expand Down
34 changes: 24 additions & 10 deletions prow/plugins/bugzilla/bugzilla.go
View file
Expand Up @@ -26,6 +26,7 @@ import (
"strings"

"github.com/sirupsen/logrus"
"k8s.io/apimachinery/pkg/util/sets"

"k8s.io/test-infra/prow/bugzilla"
"k8s.io/test-infra/prow/github"
Expand Down Expand Up @@ -79,24 +80,30 @@ func helpProvider(config *plugins.Configuration, enabledRepos []string) (*plugin
message = fmt.Sprintf("on the %q branch, ", branch)
}
message += "valid bugs must "
var conditionsExist bool
var conditions []string
if opts[branch].IsOpen != nil {
conditionsExist = true
if *opts[branch].IsOpen {
message += "be open"
conditions = append(conditions, "be open")
} else {
message += "be closed"
conditions = append(conditions, "be closed")
}
}
if opts[branch].TargetRelease != nil {
conditionsExist = true
if opts[branch].IsOpen != nil {
message += " and "
}
message += fmt.Sprintf("target the %q release", *opts[branch].TargetRelease)
conditions = append(conditions, fmt.Sprintf("target the %q release", *opts[branch].TargetRelease))
}
if opts[branch].Statuses != nil {
conditions = append(conditions, fmt.Sprintf("be in one of the following states: %s", strings.Join(*opts[branch].Statuses, ", ")))
}
if !conditionsExist {
switch len(conditions) {
case 0:
message += "exist"
case 1:
message += conditions[0]
case 2:
message += fmt.Sprintf("%s and %s", conditions[0], conditions[1])
default:
conditions[len(conditions)-1] = fmt.Sprintf("and %s", conditions[len(conditions)-1])
message += strings.Join(conditions, ", ")
}
configInfoStrings = append(configInfoStrings, "<li>"+message+"</li>")
}
Expand Down Expand Up @@ -392,5 +399,12 @@ func validateBug(bug bugzilla.Bug, options plugins.BugzillaBranchOptions) (bool,
}
}

if options.Statuses != nil {
if !sets.NewString(*options.Statuses...).Has(bug.Status) {
valid = false
errors = append(errors, fmt.Sprintf("expected the bug to be in one of the following states: %s, but it is %s instead", strings.Join(*options.Statuses, ", "), bug.Status))
}
}

return valid, errors
}
37 changes: 28 additions & 9 deletions prow/plugins/bugzilla/bugzilla_test.go
View file
Expand Up @@ -55,8 +55,12 @@ orgs:
"*":
is_open: false
target_release: my-repo-default
statuses:
- VALIDATED
"my-repo-branch":
target_release: my-repo-branch`
target_release: my-repo-branch
statuses:
- MODIFIED`
var config plugins.Bugzilla
if err := yaml.Unmarshal([]byte(rawConfig), &config); err != nil {
t.Fatalf("couldn't unmarshal config: %v", err)
Expand All @@ -80,8 +84,8 @@ orgs:
<li>on the "my-org-branch" branch, valid bugs must be open and target the "my-org-branch-default" release</li>
</ul>`,
"my-org/my-repo": `The plugin has the following configuration:<ul>
<li>by default, valid bugs must be closed and target the "my-repo-default" release</li>
<li>on the "my-repo-branch" branch, valid bugs must be closed and target the "my-repo-branch" release</li>
<li>by default, valid bugs must be closed, target the "my-repo-default" release, and be in one of the following states: VALIDATED</li>
<li>on the "my-repo-branch" branch, valid bugs must be closed, target the "my-repo-branch" release, and be in one of the following states: MODIFIED</li>
</ul>`,
},
Commands: []pluginhelp.Command{{
Expand Down Expand Up @@ -643,6 +647,7 @@ func TestTitleMatch(t *testing.T) {
func TestValidateBug(t *testing.T) {
open, closed := true, false
one, two := "v1", "v2"
verified, modified := []string{"VERIFIED"}, []string{"MODIFIED"}
var testCases = []struct {
name string
bug bugzilla.Bug
Expand Down Expand Up @@ -703,19 +708,33 @@ func TestValidateBug(t *testing.T) {
why: []string{"expected the bug to target the \"v1\" release, but no target release was set"},
},
{
name: "matching both requirements means a valid bug",
bug: bugzilla.Bug{IsOpen: false, TargetRelease: []string{"v1"}},
options: plugins.BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &one},
name: "matching status requirement means a valid bug",
bug: bugzilla.Bug{Status: "MODIFIED"},
options: plugins.BugzillaBranchOptions{Statuses: &modified},
valid: true,
},
{
name: "matching neither requirements means an invalid bug",
bug: bugzilla.Bug{IsOpen: false, TargetRelease: []string{"v1"}},
options: plugins.BugzillaBranchOptions{IsOpen: &open, TargetRelease: &two},
name: "not matching status requirement means an invalid bug",
bug: bugzilla.Bug{Status: "MODIFIED"},
options: plugins.BugzillaBranchOptions{Statuses: &verified},
valid: false,
why: []string{"expected the bug to be in one of the following states: VERIFIED, but it is MODIFIED instead"},
},
{
name: "matching all requirements means a valid bug",
bug: bugzilla.Bug{IsOpen: false, TargetRelease: []string{"v1"}, Status: "MODIFIED"},
options: plugins.BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &one, Statuses: &modified},
valid: true,
},
{
name: "matching no requirements means an invalid bug",
bug: bugzilla.Bug{IsOpen: false, TargetRelease: []string{"v1"}, Status: "MODIFIED"},
options: plugins.BugzillaBranchOptions{IsOpen: &open, TargetRelease: &two, Statuses: &verified},
valid: false,
why: []string{
"expected the bug to be open, but it isn't",
"expected the bug to target the \"v2\" release, but it targets \"v1\" instead",
"expected the bug to be in one of the following states: VERIFIED, but it is MODIFIED instead",
},
},
}
Expand Down
15 changes: 12 additions & 3 deletions prow/plugins/config.go
View file
Expand Up @@ -1011,16 +1011,19 @@ type BugzillaRepoOptions struct {

// BugzillaBranchOptions describes how to check if a Bugzilla bug is valid or not.
type BugzillaBranchOptions struct {
IsOpen *bool `json:"is_open,omitempty"`
TargetRelease *string `json:"target_release,omitempty"`
IsOpen *bool `json:"is_open,omitempty"`
TargetRelease *string `json:"target_release,omitempty"`
Statuses *[]string `json:"statuses,omitempty"`
}

func (o BugzillaBranchOptions) matches(other BugzillaBranchOptions) bool {
isOpenMatch := o.IsOpen == nil && other.IsOpen == nil ||
(o.IsOpen != nil && other.IsOpen != nil && *o.IsOpen == *other.IsOpen)
targetReleaseMatch := o.TargetRelease == nil && other.TargetRelease == nil ||
(o.TargetRelease != nil && other.TargetRelease != nil && *o.TargetRelease == *other.TargetRelease)
return isOpenMatch && targetReleaseMatch
statusesMatch := o.Statuses == nil && other.Statuses == nil ||
(o.Statuses != nil && other.Statuses != nil && sets.NewString(*o.Statuses...).Equal(sets.NewString(*other.Statuses...)))
return isOpenMatch && targetReleaseMatch && statusesMatch
}

const BugzillaOptionsWildcard = `*`
Expand All @@ -1044,6 +1047,9 @@ func ResolveBugzillaOptions(parent, child BugzillaBranchOptions) BugzillaBranchO
if parent.TargetRelease != nil {
output.TargetRelease = parent.TargetRelease
}
if parent.Statuses != nil {
output.Statuses = parent.Statuses
}

//override with the child
if child.IsOpen != nil {
Expand All @@ -1052,6 +1058,9 @@ func ResolveBugzillaOptions(parent, child BugzillaBranchOptions) BugzillaBranchO
if child.TargetRelease != nil {
output.TargetRelease = child.TargetRelease
}
if child.Statuses != nil {
output.Statuses = child.Statuses
}

return output
}
Expand Down
44 changes: 28 additions & 16 deletions prow/plugins/config_test.go
View file
Expand Up @@ -377,6 +377,7 @@ func TestOptionsForItem(t *testing.T) {
func TestResolveBugzillaOptions(t *testing.T) {
open, closed := true, false
one, two := "v1", "v2"
modified, verified := []string{"VERIFIED"}, []string{"MODIFIED"}
var testCases = []struct {
name string
parent, child BugzillaBranchOptions
Expand All @@ -387,31 +388,37 @@ func TestResolveBugzillaOptions(t *testing.T) {
},
{
name: "no child means a copy of parent is the output",
parent: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one},
expected: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one},
parent: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one, Statuses: &modified},
expected: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one, Statuses: &modified},
},
{
name: "no parent means a copy of child is the output",
child: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one},
expected: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one},
child: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one, Statuses: &modified},
expected: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one, Statuses: &modified},
},
{
name: "child overrides parent on IsOpen",
parent: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one},
parent: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one, Statuses: &modified},
child: BugzillaBranchOptions{IsOpen: &closed},
expected: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &one},
expected: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &one, Statuses: &modified},
},
{
name: "child overrides parent on target release",
parent: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one},
parent: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one, Statuses: &modified},
child: BugzillaBranchOptions{TargetRelease: &two},
expected: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &two},
expected: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &two, Statuses: &modified},
},
{
name: "child overrides parent on statuses",
parent: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one, Statuses: &modified},
child: BugzillaBranchOptions{Statuses: &verified},
expected: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one, Statuses: &verified},
},
{
name: "child overrides parent on all fields",
parent: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one},
child: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &two},
expected: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &two},
parent: BugzillaBranchOptions{IsOpen: &open, TargetRelease: &one, Statuses: &verified},
child: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &two, Statuses: &modified},
expected: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &two, Statuses: &modified},
},
}
for _, testCase := range testCases {
Expand All @@ -426,6 +433,7 @@ func TestResolveBugzillaOptions(t *testing.T) {
func TestOptionsForBranch(t *testing.T) {
open, closed := true, false
globalDefault, globalBranchDefault, orgDefault, orgBranchDefault, repoDefault, repoBranch := "global-default", "global-branch-default", "my-org-default", "my-org-branch-default", "my-repo-default", "my-repo-branch"
verified, modified := []string{"VERIFIED"}, []string{"MODIFIED"}

rawConfig := `default:
"*":
Expand All @@ -447,8 +455,12 @@ orgs:
"*":
is_open: false
target_release: my-repo-default
statuses:
- VERIFIED
"my-repo-branch":
target_release: my-repo-branch`
target_release: my-repo-branch
statuses:
- MODIFIED`
var config Bugzilla
if err := yaml.Unmarshal([]byte(rawConfig), &config); err != nil {
t.Fatalf("couldn't unmarshal config: %v", err)
Expand Down Expand Up @@ -492,14 +504,14 @@ orgs:
org: "my-org",
repo: "my-repo",
branch: "some-branch",
expected: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &repoDefault},
expected: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &repoDefault, Statuses: &verified},
},
{
name: "branch on configured org and repo gets branch config",
org: "my-org",
repo: "my-repo",
branch: "my-repo-branch",
expected: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &repoBranch},
expected: BugzillaBranchOptions{IsOpen: &closed, TargetRelease: &repoBranch, Statuses: &modified},
},
}
for _, testCase := range testCases {
Expand Down Expand Up @@ -538,8 +550,8 @@ orgs:
org: "my-org",
repo: "my-repo",
expected: map[string]BugzillaBranchOptions{
"*": {IsOpen: &closed, TargetRelease: &repoDefault},
"my-repo-branch": {IsOpen: &closed, TargetRelease: &repoBranch},
"*": {IsOpen: &closed, TargetRelease: &repoDefault, Statuses: &verified},
"my-repo-branch": {IsOpen: &closed, TargetRelease: &repoBranch, Statuses: &modified},
},
},
}
Expand Down

0 comments on commit 0b17ca8

Please sign in to comment.