diff --git a/checks/branch_protection_test.go b/checks/branch_protection_test.go index d56a4cce2bd5..cb0216be7095 100644 --- a/checks/branch_protection_test.go +++ b/checks/branch_protection_test.go @@ -92,7 +92,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &trueVal, RequireCodeOwnerReviews: &trueVal, @@ -112,7 +112,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: nil, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -152,7 +152,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: nil, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -188,7 +188,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &trueVal, RequireCodeOwnerReviews: &trueVal, @@ -210,7 +210,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: nil, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -246,7 +246,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &trueVal, RequireCodeOwnerReviews: &trueVal, @@ -268,7 +268,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &trueVal, RequireCodeOwnerReviews: &trueVal, @@ -305,7 +305,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: nil, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -344,7 +344,7 @@ func TestReleaseAndDevBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: nil, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, diff --git a/checks/evaluation/branch_protection.go b/checks/evaluation/branch_protection.go index f5848d521dc8..76f280b1841a 100644 --- a/checks/evaluation/branch_protection.go +++ b/checks/evaluation/branch_protection.go @@ -285,7 +285,7 @@ func nonAdminReviewProtection(branch *clients.BranchRef) (int, int) { // Having at least 1 reviewer is twice as important as the other Tier 2 requirements. const reviewerWeight = 2 max += reviewerWeight - if valueOrZero(branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount) > 0 { + if valueOrZero(branch.BranchProtectionRule.PullRequestRule.RequiredApprovingReviewCount) > 0 { // We do not display anything here, it's done in nonAdminThoroughReviewProtection() score += reviewerWeight } @@ -327,7 +327,7 @@ func adminReviewProtection(branch *clients.BranchRef, dl checker.DetailLogger) ( } max++ - if valueOrZero(branch.BranchProtectionRule.RequiredPullRequestReviews.Required) { + if valueOrZero(branch.BranchProtectionRule.PullRequestRule.Required) { score++ info(dl, log, "PRs are required in order to make changes on branch '%s'", *branch.Name) } else { @@ -345,10 +345,10 @@ func adminThoroughReviewProtection(branch *clients.BranchRef, dl checker.DetailL // Only log information if the branch is protected. log := branch.Protected != nil && *branch.Protected - if branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews != nil { + if branch.BranchProtectionRule.PullRequestRule.DismissStaleReviews != nil { // Note: we don't increase max possible score for non-admin viewers. max++ - switch *branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews { + switch *branch.BranchProtectionRule.PullRequestRule.DismissStaleReviews { case true: info(dl, log, "stale review dismissal enabled on branch '%s'", *branch.Name) score++ @@ -386,7 +386,7 @@ func nonAdminThoroughReviewProtection(branch *clients.BranchRef, dl checker.Deta max++ - reviewers := valueOrZero(branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount) + reviewers := valueOrZero(branch.BranchProtectionRule.PullRequestRule.RequiredApprovingReviewCount) if reviewers >= minReviews { info(dl, log, "number of required reviewers is %d on branch '%s'", reviewers, *branch.Name) score++ @@ -406,8 +406,8 @@ func codeownerBranchProtection( log := branch.Protected != nil && *branch.Protected - if branch.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews != nil { - switch *branch.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews { + if branch.BranchProtectionRule.PullRequestRule.RequireCodeOwnerReviews != nil { + switch *branch.BranchProtectionRule.PullRequestRule.RequireCodeOwnerReviews { case true: info(dl, log, "codeowner review is required on branch '%s'", *branch.Name) if len(codeownersFiles) == 0 { diff --git a/checks/evaluation/branch_protection_test.go b/checks/evaluation/branch_protection_test.go index 0084e4e0a343..9b847e8e50ee 100644 --- a/checks/evaluation/branch_protection_test.go +++ b/checks/evaluation/branch_protection_test.go @@ -67,7 +67,7 @@ func TestIsBranchProtected(t *testing.T) { RequireLinearHistory: &falseVal, EnforceAdmins: &falseVal, RequireLastPushApproval: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, CheckRules: clients.StatusChecksRule{ @@ -105,7 +105,7 @@ func TestIsBranchProtected(t *testing.T) { Name: &branchVal, Protected: &trueVal, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -142,7 +142,7 @@ func TestIsBranchProtected(t *testing.T) { RequireLinearHistory: &falseVal, AllowForcePushes: &falseVal, AllowDeletions: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -179,7 +179,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: nil, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -208,7 +208,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -237,7 +237,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &trueVal, RequireCodeOwnerReviews: &trueVal, @@ -269,7 +269,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: nil, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -327,7 +327,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: nil, Contexts: nil, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: nil, RequireCodeOwnerReviews: &falseVal, @@ -359,7 +359,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -391,7 +391,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -424,7 +424,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -456,7 +456,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &falseVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &falseVal, RequireCodeOwnerReviews: &falseVal, @@ -488,7 +488,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &trueVal, RequireCodeOwnerReviews: &trueVal, @@ -520,7 +520,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &trueVal, RequireCodeOwnerReviews: &trueVal, @@ -553,7 +553,7 @@ func TestIsBranchProtected(t *testing.T) { UpToDateBeforeMerge: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &trueVal, RequireCodeOwnerReviews: &trueVal, diff --git a/clients/branch.go b/clients/branch.go index 9cabdbc31419..824b6cb271cc 100644 --- a/clients/branch.go +++ b/clients/branch.go @@ -23,7 +23,7 @@ type BranchRef struct { // BranchProtectionRule captures the settings enabled on a branch for security. type BranchProtectionRule struct { - RequiredPullRequestReviews PullRequestReviewRule + PullRequestRule PullRequestRule AllowDeletions *bool AllowForcePushes *bool RequireLinearHistory *bool @@ -39,8 +39,8 @@ type StatusChecksRule struct { Contexts []string } -// PullRequestReviewRule captures settings on a PullRequest. -type PullRequestReviewRule struct { +// PullRequestRule captures settings on a PullRequest. +type PullRequestRule struct { Required *bool // are PRs required RequiredApprovingReviewCount *int32 DismissStaleReviews *bool diff --git a/clients/githubrepo/branches.go b/clients/githubrepo/branches.go index 4c42c07e1fc5..16d2a2167bd7 100644 --- a/clients/githubrepo/branches.go +++ b/clients/githubrepo/branches.go @@ -327,7 +327,7 @@ func (handler *branchesHandler) getBranch(branch string) (*clients.BranchRef, er func copyAdminSettings(src *branchProtectionRule, dst *clients.BranchProtectionRule) { copyBoolPtr(src.IsAdminEnforced, &dst.EnforceAdmins) copyBoolPtr(src.RequireLastPushApproval, &dst.RequireLastPushApproval) - copyBoolPtr(src.DismissesStaleReviews, &dst.RequiredPullRequestReviews.DismissStaleReviews) + copyBoolPtr(src.DismissesStaleReviews, &dst.PullRequestRule.DismissStaleReviews) if src.RequiresStatusChecks != nil { copyBoolPtr(src.RequiresStatusChecks, &dst.CheckRules.RequiresStatusChecks) // TODO(#3255): Update when GitHub GraphQL bug is fixed @@ -342,12 +342,12 @@ func copyAdminSettings(src *branchProtectionRule, dst *clients.BranchProtectionR } } // we always have the data to know if PRs are required - if dst.RequiredPullRequestReviews.Required == nil { - dst.RequiredPullRequestReviews.Required = asPtr(false) + if dst.PullRequestRule.Required == nil { + dst.PullRequestRule.Required = asPtr(false) } // these values report as &false when PRs aren't required, so if they're true then PRs are required if valueOrZero(src.RequireLastPushApproval) || valueOrZero(src.DismissesStaleReviews) { - dst.RequiredPullRequestReviews.Required = asPtr(true) + dst.PullRequestRule.Required = asPtr(true) } } @@ -358,32 +358,32 @@ func copyNonAdminSettings(src interface{}, dst *clients.BranchProtectionRule) { copyBoolPtr(v.AllowsDeletions, &dst.AllowDeletions) copyBoolPtr(v.AllowsForcePushes, &dst.AllowForcePushes) copyBoolPtr(v.RequiresLinearHistory, &dst.RequireLinearHistory) - copyInt32Ptr(v.RequiredApprovingReviewCount, &dst.RequiredPullRequestReviews.RequiredApprovingReviewCount) - copyBoolPtr(v.RequiresCodeOwnerReviews, &dst.RequiredPullRequestReviews.RequireCodeOwnerReviews) + copyInt32Ptr(v.RequiredApprovingReviewCount, &dst.PullRequestRule.RequiredApprovingReviewCount) + copyBoolPtr(v.RequiresCodeOwnerReviews, &dst.PullRequestRule.RequireCodeOwnerReviews) copyStringSlice(v.RequiredStatusCheckContexts, &dst.CheckRules.Contexts) // we always have the data to know if PRs are required - if dst.RequiredPullRequestReviews.Required == nil { - dst.RequiredPullRequestReviews.Required = asPtr(false) + if dst.PullRequestRule.Required == nil { + dst.PullRequestRule.Required = asPtr(false) } // GitHub returns nil for RequiredApprovingReviewCount when PRs aren't required and non-nil when they are // RequiresCodeOwnerReviews is &false even if PRs aren't required, so we need it to be true if v.RequiredApprovingReviewCount != nil || valueOrZero(v.RequiresCodeOwnerReviews) { - dst.RequiredPullRequestReviews.Required = asPtr(true) + dst.PullRequestRule.Required = asPtr(true) } case *refUpdateRule: copyBoolPtr(v.AllowsDeletions, &dst.AllowDeletions) copyBoolPtr(v.AllowsForcePushes, &dst.AllowForcePushes) copyBoolPtr(v.RequiresLinearHistory, &dst.RequireLinearHistory) - copyInt32Ptr(v.RequiredApprovingReviewCount, &dst.RequiredPullRequestReviews.RequiredApprovingReviewCount) - copyBoolPtr(v.RequiresCodeOwnerReviews, &dst.RequiredPullRequestReviews.RequireCodeOwnerReviews) + copyInt32Ptr(v.RequiredApprovingReviewCount, &dst.PullRequestRule.RequiredApprovingReviewCount) + copyBoolPtr(v.RequiresCodeOwnerReviews, &dst.PullRequestRule.RequireCodeOwnerReviews) copyStringSlice(v.RequiredStatusCheckContexts, &dst.CheckRules.Contexts) // Evaluate if we have data to infer that the project requires PRs to make changes. If we don't have data, we let // Required stay nil if valueOrZero(v.RequiredApprovingReviewCount) > 0 || valueOrZero(v.RequiresCodeOwnerReviews) { - dst.RequiredPullRequestReviews.Required = asPtr(true) + dst.PullRequestRule.Required = asPtr(true) } } } @@ -508,7 +508,7 @@ func applyRepoRules(branchRef *clients.BranchRef, rules []*repoRuleSet) { AllowDeletions: asPtr(true), AllowForcePushes: asPtr(true), RequireLinearHistory: asPtr(false), - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: asPtr(false), }, } @@ -534,11 +534,11 @@ func applyRepoRules(branchRef *clients.BranchRef, rules []*repoRuleSet) { } func translatePullRequestRepoRule(base *clients.BranchProtectionRule, rule *repoRule) { - base.RequiredPullRequestReviews.Required = asPtr(true) - base.RequiredPullRequestReviews.DismissStaleReviews = rule.Parameters.PullRequestParameters.DismissStaleReviewsOnPush - base.RequiredPullRequestReviews.RequireCodeOwnerReviews = rule.Parameters.PullRequestParameters.RequireCodeOwnerReview + base.PullRequestRule.Required = asPtr(true) + base.PullRequestRule.DismissStaleReviews = rule.Parameters.PullRequestParameters.DismissStaleReviewsOnPush + base.PullRequestRule.RequireCodeOwnerReviews = rule.Parameters.PullRequestParameters.RequireCodeOwnerReview base.RequireLastPushApproval = rule.Parameters.PullRequestParameters.RequireLastPushApproval - base.RequiredPullRequestReviews.RequiredApprovingReviewCount = rule.Parameters.PullRequestParameters. + base.PullRequestRule.RequiredApprovingReviewCount = rule.Parameters.PullRequestParameters. RequiredApprovingReviewCount } @@ -582,7 +582,7 @@ func mergeBranchProtectionRules(base, translated *clients.BranchProtectionRule) base.RequireLinearHistory = translated.RequireLinearHistory } mergeCheckRules(&base.CheckRules, &translated.CheckRules) - mergePullRequestReviews(&base.RequiredPullRequestReviews, &translated.RequiredPullRequestReviews) + mergePullRequestReviews(&base.PullRequestRule, &translated.PullRequestRule) } func mergeCheckRules(base, translated *clients.StatusChecksRule) { @@ -600,7 +600,7 @@ func mergeCheckRules(base, translated *clients.StatusChecksRule) { } } -func mergePullRequestReviews(base, translated *clients.PullRequestReviewRule) { +func mergePullRequestReviews(base, translated *clients.PullRequestRule) { if base.Required == nil || valueOrZero(translated.Required) { base.Required = translated.Required } diff --git a/clients/githubrepo/branches_test.go b/clients/githubrepo/branches_test.go index f41316a7ae44..eec6415c59b7 100644 --- a/clients/githubrepo/branches_test.go +++ b/clients/githubrepo/branches_test.go @@ -209,7 +209,7 @@ func Test_applyRepoRules(t *testing.T) { EnforceAdmins: &trueVal, RequireLastPushApproval: nil, // this checkbox is enabled only if require status checks RequireLinearHistory: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -227,7 +227,7 @@ func Test_applyRepoRules(t *testing.T) { AllowForcePushes: &trueVal, RequireLinearHistory: &falseVal, EnforceAdmins: &trueVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -245,7 +245,7 @@ func Test_applyRepoRules(t *testing.T) { AllowForcePushes: &trueVal, RequireLinearHistory: &falseVal, EnforceAdmins: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -268,7 +268,7 @@ func Test_applyRepoRules(t *testing.T) { AllowForcePushes: &falseVal, EnforceAdmins: &falseVal, // Downgrade: deletion does not enforce admins RequireLinearHistory: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -292,7 +292,7 @@ func Test_applyRepoRules(t *testing.T) { AllowForcePushes: &falseVal, EnforceAdmins: &falseVal, // Maintain: deletion enforces but force-push does not RequireLinearHistory: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -315,7 +315,7 @@ func Test_applyRepoRules(t *testing.T) { AllowForcePushes: &falseVal, EnforceAdmins: &trueVal, // Maintain: base and rule are equal strictness RequireLinearHistory: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -333,7 +333,7 @@ func Test_applyRepoRules(t *testing.T) { AllowForcePushes: &falseVal, EnforceAdmins: &trueVal, RequireLinearHistory: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -351,7 +351,7 @@ func Test_applyRepoRules(t *testing.T) { AllowForcePushes: &trueVal, RequireLinearHistory: &trueVal, EnforceAdmins: &trueVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -378,7 +378,7 @@ func Test_applyRepoRules(t *testing.T) { EnforceAdmins: &trueVal, RequireLastPushApproval: &trueVal, RequireLinearHistory: &falseVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, RequiredApprovingReviewCount: &zeroVal, }, @@ -409,7 +409,7 @@ func Test_applyRepoRules(t *testing.T) { EnforceAdmins: &trueVal, RequireLinearHistory: &falseVal, RequireLastPushApproval: &trueVal, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, DismissStaleReviews: &trueVal, RequireCodeOwnerReviews: &trueVal, @@ -447,7 +447,7 @@ func Test_applyRepoRules(t *testing.T) { RequiresStatusChecks: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, }, }, @@ -515,7 +515,7 @@ func Test_applyRepoRules(t *testing.T) { RequiresStatusChecks: &trueVal, Contexts: []string{"foo"}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &trueVal, RequiredApprovingReviewCount: &twoVal, DismissStaleReviews: &trueVal, @@ -577,7 +577,7 @@ func Test_translationFromGithubAPIBranchProtectionData(t *testing.T) { RequiresStatusChecks: nil, Contexts: []string{}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: asPtr[int32](0), RequireCodeOwnerReviews: &falseVal, }, @@ -615,7 +615,7 @@ func Test_translationFromGithubAPIBranchProtectionData(t *testing.T) { RequiresStatusChecks: &falseVal, Contexts: []string{}, }, - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: &falseVal, RequireCodeOwnerReviews: &falseVal, DismissStaleReviews: &falseVal, diff --git a/clients/gitlabrepo/branches.go b/clients/gitlabrepo/branches.go index f50f5e3cdb07..7878a33d7fee 100644 --- a/clients/gitlabrepo/branches.go +++ b/clients/gitlabrepo/branches.go @@ -193,7 +193,7 @@ func makeBranchRefFrom(branch *gitlab.Branch, protectedBranch *gitlab.ProtectedB Contexts: makeContextsFromResp(projectStatusChecks), } - pullRequestReviewRule := clients.PullRequestReviewRule{ + pullRequestReviewRule := clients.PullRequestRule{ // TODO how do we know if they're required? DismissStaleReviews: newTrue(), RequireCodeOwnerReviews: &protectedBranch.CodeOwnerApprovalRequired, @@ -208,7 +208,7 @@ func makeBranchRefFrom(branch *gitlab.Branch, protectedBranch *gitlab.ProtectedB Name: &branch.Name, Protected: &branch.Protected, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: pullRequestReviewRule, + PullRequestRule: pullRequestReviewRule, AllowDeletions: newFalse(), AllowForcePushes: &protectedBranch.AllowForcePush, EnforceAdmins: newTrue(), diff --git a/cron/internal/format/json_raw_results.go b/cron/internal/format/json_raw_results.go index 5cf4b6dacf4e..0e3f03308cc7 100644 --- a/cron/internal/format/json_raw_results.go +++ b/cron/internal/format/json_raw_results.go @@ -210,13 +210,13 @@ func addBranchProtectionRawResults(r *jsonScorecardRawResult, bp *checker.Branch bp = &jsonBranchProtectionSettings{ AllowsDeletions: v.BranchProtectionRule.AllowDeletions, AllowsForcePushes: v.BranchProtectionRule.AllowForcePushes, - RequiresCodeOwnerReviews: v.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews, + RequiresCodeOwnerReviews: v.BranchProtectionRule.PullRequestRule.RequireCodeOwnerReviews, RequiresLinearHistory: v.BranchProtectionRule.RequireLinearHistory, - DismissesStaleReviews: v.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews, + DismissesStaleReviews: v.BranchProtectionRule.PullRequestRule.DismissStaleReviews, EnforcesAdmins: v.BranchProtectionRule.EnforceAdmins, RequiresStatusChecks: v.BranchProtectionRule.CheckRules.RequiresStatusChecks, RequiresUpToDateBranchBeforeMerging: v.BranchProtectionRule.CheckRules.UpToDateBeforeMerge, - RequiredApprovingReviewCount: v.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount, + RequiredApprovingReviewCount: v.BranchProtectionRule.PullRequestRule.RequiredApprovingReviewCount, StatusCheckContexts: v.BranchProtectionRule.CheckRules.Contexts, } } diff --git a/pkg/json_raw_results.go b/pkg/json_raw_results.go index 211071454044..14c6eabe9c3c 100644 --- a/pkg/json_raw_results.go +++ b/pkg/json_raw_results.go @@ -712,13 +712,13 @@ func (r *jsonScorecardRawResult) addBranchProtectionRawResults(bp *checker.Branc bp = &jsonBranchProtectionSettings{ AllowsDeletions: v.BranchProtectionRule.AllowDeletions, AllowsForcePushes: v.BranchProtectionRule.AllowForcePushes, - RequiresCodeOwnerReviews: v.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews, + RequiresCodeOwnerReviews: v.BranchProtectionRule.PullRequestRule.RequireCodeOwnerReviews, RequiresLinearHistory: v.BranchProtectionRule.RequireLinearHistory, - DismissesStaleReviews: v.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews, + DismissesStaleReviews: v.BranchProtectionRule.PullRequestRule.DismissStaleReviews, EnforcesAdmins: v.BranchProtectionRule.EnforceAdmins, RequiresStatusChecks: v.BranchProtectionRule.CheckRules.RequiresStatusChecks, RequiresUpToDateBranchBeforeMerging: v.BranchProtectionRule.CheckRules.UpToDateBeforeMerge, - RequiredApprovingReviewCount: v.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount, + RequiredApprovingReviewCount: v.BranchProtectionRule.PullRequestRule.RequiredApprovingReviewCount, StatusCheckContexts: v.BranchProtectionRule.CheckRules.Contexts, } } diff --git a/pkg/json_raw_results_test.go b/pkg/json_raw_results_test.go index 4af7c5fc088e..2c3709c63502 100644 --- a/pkg/json_raw_results_test.go +++ b/pkg/json_raw_results_test.go @@ -1114,7 +1114,7 @@ func TestJsonScorecardRawResult(t *testing.T) { BranchProtectionRule: clients.BranchProtectionRule{ AllowDeletions: boolPtr(true), AllowForcePushes: boolPtr(false), - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ Required: boolPtr(true), RequireCodeOwnerReviews: boolPtr(true), DismissStaleReviews: boolPtr(true), diff --git a/probes/dismissesStaleReviews/impl.go b/probes/dismissesStaleReviews/impl.go index fd63d7646da8..c301642d4c8c 100644 --- a/probes/dismissesStaleReviews/impl.go +++ b/probes/dismissesStaleReviews/impl.go @@ -44,7 +44,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Branches { branch := &r.Branches[i] - p := branch.BranchProtectionRule.RequiredPullRequestReviews.DismissStaleReviews + p := branch.BranchProtectionRule.PullRequestRule.DismissStaleReviews text, outcome, err := branchprotection.GetTextOutcomeFromBool(p, "stale review dismissal", *branch.Name) diff --git a/probes/dismissesStaleReviews/impl_test.go b/probes/dismissesStaleReviews/impl_test.go index d074ca52e1c8..c8ca4d4d891a 100644 --- a/probes/dismissesStaleReviews/impl_test.go +++ b/probes/dismissesStaleReviews/impl_test.go @@ -49,7 +49,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, }, @@ -69,7 +69,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, }, @@ -77,7 +77,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, }, @@ -97,7 +97,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, }, @@ -105,7 +105,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &falseVal, }, }, @@ -125,7 +125,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &falseVal, }, }, @@ -133,7 +133,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &trueVal, }, }, @@ -153,7 +153,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: &falseVal, }, }, @@ -161,7 +161,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ DismissStaleReviews: nil, }, }, diff --git a/probes/requiresApproversForPullRequests/impl.go b/probes/requiresApproversForPullRequests/impl.go index 14f114f2f5e8..ef642c3c6eae 100644 --- a/probes/requiresApproversForPullRequests/impl.go +++ b/probes/requiresApproversForPullRequests/impl.go @@ -51,7 +51,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { trueMsg := fmt.Sprintf("required approving review count on branch '%s'", *branch.Name) falseMsg := fmt.Sprintf("branch '%s' does not require approvers", *branch.Name) - p := branch.BranchProtectionRule.RequiredPullRequestReviews.RequiredApprovingReviewCount + p := branch.BranchProtectionRule.PullRequestRule.RequiredApprovingReviewCount f, err := finding.NewWith(fs, Probe, "", nil, finding.OutcomeNotAvailable) if err != nil { diff --git a/probes/requiresApproversForPullRequests/impl_test.go b/probes/requiresApproversForPullRequests/impl_test.go index c15a39cb8dd5..db5fa9c408aa 100644 --- a/probes/requiresApproversForPullRequests/impl_test.go +++ b/probes/requiresApproversForPullRequests/impl_test.go @@ -49,7 +49,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, }, @@ -69,7 +69,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, }, @@ -77,7 +77,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, }, @@ -97,7 +97,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, }, @@ -105,7 +105,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &zeroVal, }, }, @@ -125,7 +125,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &zeroVal, }, }, @@ -133,7 +133,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &oneVal, }, }, @@ -153,7 +153,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: &zeroVal, }, }, @@ -161,7 +161,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequiredApprovingReviewCount: nil, }, }, diff --git a/probes/requiresCodeOwnersReview/impl.go b/probes/requiresCodeOwnersReview/impl.go index b5ff4bc9143a..444164152ed1 100644 --- a/probes/requiresCodeOwnersReview/impl.go +++ b/probes/requiresCodeOwnersReview/impl.go @@ -42,7 +42,7 @@ func Run(raw *checker.RawResults) ([]finding.Finding, string, error) { for i := range r.Branches { branch := &r.Branches[i] - reqOwnerReviews := branch.BranchProtectionRule.RequiredPullRequestReviews.RequireCodeOwnerReviews + reqOwnerReviews := branch.BranchProtectionRule.PullRequestRule.RequireCodeOwnerReviews var text string var outcome finding.Outcome diff --git a/probes/requiresCodeOwnersReview/impl_test.go b/probes/requiresCodeOwnersReview/impl_test.go index b58a00c716a2..6f7a85a93c7f 100644 --- a/probes/requiresCodeOwnersReview/impl_test.go +++ b/probes/requiresCodeOwnersReview/impl_test.go @@ -48,7 +48,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -69,7 +69,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -90,7 +90,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -98,7 +98,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -119,7 +119,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -127,7 +127,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -148,7 +148,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -156,7 +156,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, }, @@ -177,7 +177,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -185,7 +185,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, }, @@ -206,7 +206,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, }, @@ -214,7 +214,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -235,7 +235,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, }, @@ -243,7 +243,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &trueVal, }, }, @@ -264,7 +264,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal1, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: &falseVal, }, }, @@ -272,7 +272,7 @@ func Test_Run(t *testing.T) { { Name: &branchVal2, BranchProtectionRule: clients.BranchProtectionRule{ - RequiredPullRequestReviews: clients.PullRequestReviewRule{ + PullRequestRule: clients.PullRequestRule{ RequireCodeOwnerReviews: nil, }, },