Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for allowing skipped checks in has_successful_status #789

26 changes: 19 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -322,13 +322,24 @@ if:
deletions: "> 100"
total: "> 200"

# DEPRECATED: Use "has_status" below instead, which is more flexible.
# "has_successful_status" is satisfied if the status checks that are specified
# are marked successful on the head commit of the pull request.
has_successful_status:
- "status-name-1"
- "status-name-2"
- "status-name-3"

# "has_status" is satisfied if the status checks that are specified are
# finished and concluded with one of the conclusions specified.
# "conclusions" is optional and defaults to ["success"].
has_status:
conclusions: ["success", "skipped"]
statuses:
- "status-name-1"
- "status-name-2"
- "status-name-3"

# "has_labels" is satisfied if the pull request has the specified labels
# applied
has_labels:
Expand Down Expand Up @@ -510,16 +521,17 @@ requires:
# count as approved. If present, conditions are an additional requirement
# beyond the approvals required by "count".
#
# For example, if "count" is 1 and "conditions" contains the "has_successful_status"
# For example, if "count" is 1 and "conditions" contains the "has_status"
# condition with the "build" status, the rule is only approved once the
# "build" status check passes and one authorized reviewer leaves a review.
conditions:
# The "conditions" block accepts all of the keys documented as part
# of the "if" block for predicates. The "has_successful_status" key is
# shown here as an example of one type of condition.
has_successful_status:
- "build"
- "vulnerability scan"
# The "conditions" block accepts all of the keys documented as part of the
# "if" block for predicates. The "has_status" key is shown here as an
# example of one type of condition.
has_status:
statuses:
- "build"
- "vulnerability scan"
```

### Approval Policies
Expand Down
10 changes: 5 additions & 5 deletions policy/approval/approve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ func TestIsApproved(t *testing.T) {
r := &Rule{
Requires: Requires{
Conditions: predicate.Predicates{
HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"deploy"},
HasStatus: &predicate.HasStatus{Statuses: []string{"deploy"}},
},
},
}
Expand All @@ -688,7 +688,7 @@ func TestIsApproved(t *testing.T) {
r := &Rule{
Requires: Requires{
Conditions: predicate.Predicates{
HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"build"},
HasStatus: &predicate.HasStatus{Statuses: []string{"build"}},
},
},
}
Expand All @@ -701,7 +701,7 @@ func TestIsApproved(t *testing.T) {
Requires: Requires{
Count: 1,
Conditions: predicate.Predicates{
HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"build"},
HasStatus: &predicate.HasStatus{Statuses: []string{"build"}},
},
},
}
Expand All @@ -717,7 +717,7 @@ func TestIsApproved(t *testing.T) {
Organizations: []string{"everyone"},
},
Conditions: predicate.Predicates{
HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"build"},
HasStatus: &predicate.HasStatus{Statuses: []string{"build"}},
},
},
}
Expand Down Expand Up @@ -825,7 +825,7 @@ func TestTrigger(t *testing.T) {
r := &Rule{
Requires: Requires{
Conditions: predicate.Predicates{
HasSuccessfulStatus: &predicate.HasSuccessfulStatus{"build"},
HasStatus: predicate.NewHasStatus([]string{"status1"}, []string{"skipped", "success"}),
},
},
}
Expand Down
22 changes: 22 additions & 0 deletions policy/approval/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ func TestParsePolicy(t *testing.T) {
- and:
- rule6
- rule7
- or:
- rule8
- rule9
`

ruleText := `
Expand Down Expand Up @@ -67,6 +70,15 @@ func TestParsePolicy(t *testing.T) {
enabled: true
requires:
admins: true
- name: rule8
if:
has_successful_status:
- status1
- name: rule9
if:
has_status:
conclusions: ["success", "skipped"]
statuses: [status2, status3]
`

var policy Policy
Expand Down Expand Up @@ -119,6 +131,16 @@ func TestParsePolicy(t *testing.T) {
&RuleRequirement{
rule: rules[6],
},
&OrRequirement{
requirements: []common.Evaluator{
&RuleRequirement{
rule: rules[7],
},
&RuleRequirement{
rule: rules[8],
},
},
},
},
},
},
Expand Down
8 changes: 8 additions & 0 deletions policy/predicate/predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ type Predicates struct {

ModifiedLines *ModifiedLines `yaml:"modified_lines"`

HasStatus *HasStatus `yaml:"has_status"`
// `has_successful_status` is a deprecated field that is kept for backwards
// compatibility. `has_status` replaces it, and can accept any conclusion
// rather than just "success".
HasSuccessfulStatus *HasSuccessfulStatus `yaml:"has_successful_status"`

HasLabels *HasLabels `yaml:"has_labels"`
Expand Down Expand Up @@ -78,6 +82,10 @@ func (p *Predicates) Predicates() []Predicate {
ps = append(ps, Predicate(p.ModifiedLines))
}

if p.HasStatus != nil {
ps = append(ps, Predicate(p.HasStatus))
}

if p.HasSuccessfulStatus != nil {
ps = append(ps, Predicate(p.HasSuccessfulStatus))
}
Expand Down
84 changes: 72 additions & 12 deletions policy/predicate/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,37 +16,55 @@ package predicate

import (
"context"
"fmt"
"slices"
"strings"

"github.com/palantir/policy-bot/policy/common"
"github.com/palantir/policy-bot/pull"
"github.com/pkg/errors"
)

type HasSuccessfulStatus []string

var _ Predicate = HasSuccessfulStatus([]string{})
type AllowedConclusions []string

func (pred HasSuccessfulStatus) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
statuses, err := prctx.LatestStatuses()
type HasStatus struct {
Conclusions AllowedConclusions `yaml:"conclusions"`
Statuses []string `yaml:"statuses"`
}

predicateResult := common.PredicateResult{
ValuePhrase: "status checks",
ConditionPhrase: "exist and pass",
func NewHasStatus(statuses []string, conclusions []string) *HasStatus {
return &HasStatus{
Conclusions: conclusions,
Statuses: statuses,
}
}

var _ Predicate = HasStatus{}

func (pred HasStatus) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
statuses, err := prctx.LatestStatuses()
if err != nil {
return nil, errors.Wrap(err, "failed to list commit statuses")
}

conclusions := pred.Conclusions
if len(conclusions) == 0 {
conclusions = AllowedConclusions{"success"}
}

predicateResult := common.PredicateResult{
ValuePhrase: "status checks",
ConditionPhrase: fmt.Sprintf("exist and have conclusion %s", conclusions.joinWithOr()),
}

var missingResults []string
var failingStatuses []string
for _, status := range pred {
for _, status := range pred.Statuses {
result, ok := statuses[status]
if !ok {
missingResults = append(missingResults, status)
}
if result != "success" {
if !slices.Contains(conclusions, result) {
failingStatuses = append(failingStatuses, status)
}
}
Expand All @@ -60,16 +78,58 @@ func (pred HasSuccessfulStatus) Evaluate(ctx context.Context, prctx pull.Context

if len(failingStatuses) > 0 {
predicateResult.Values = failingStatuses
predicateResult.Description = "One or more statuses has not passed: " + strings.Join(failingStatuses, ",")
predicateResult.Description = fmt.Sprintf("One or more statuses has not concluded with %s: %s", pred.Conclusions.joinWithOr(), strings.Join(failingStatuses, ","))
predicateResult.Satisfied = false
return &predicateResult, nil
}

predicateResult.Values = pred
predicateResult.Values = pred.Statuses
predicateResult.Satisfied = true
return &predicateResult, nil
}

func (pred HasStatus) Trigger() common.Trigger {
return common.TriggerStatus
}

// HasSuccessfulStatus checks that the specified statuses have a successful
// conclusion.
//
// Deprecated: use the more flexible `HasStatus` with `conclusions: ["success"]`
// instead.
type HasSuccessfulStatus []string

var _ Predicate = HasSuccessfulStatus{}

func (pred HasSuccessfulStatus) Evaluate(ctx context.Context, prctx pull.Context) (*common.PredicateResult, error) {
return HasStatus{
Statuses: pred,
}.Evaluate(ctx, prctx)
}

func (pred HasSuccessfulStatus) Trigger() common.Trigger {
return common.TriggerStatus
}

// joinWithOr returns a string that represents the allowed conclusions in a
// format that can be used in a sentence. For example, if the allowed
// conclusions are "success" and "failure", this will return "success or
// failure". If there are more than two conclusions, the first n-1 will be
// separated by commas.
func (c AllowedConclusions) joinWithOr() string {
slices.Sort(c)

length := len(c)
switch length {
case 0:
return ""
case 1:
return c[0]
case 2:
return c[0] + " or " + c[1]
}

head, tail := c[:length-1], c[length-1]

return strings.Join(head, ", ") + ", or " + tail
}
Loading