Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 43 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -372,14 +372,15 @@ if:
deletions: "> 100"
total: "> 200"

# DEPRECATED: Use "has_status" below instead, which is more flexible.
# DEPRECATED: Use "has_status_check" 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"

# DEPRECATED: Use "has_status_check" below instead, which is more flexible.
# "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"].
Expand All @@ -390,6 +391,29 @@ if:
- "status-name-2"
- "status-name-3"

# "has_status_check" is satisfied if the status checks that are specified are
# in one of the statuses specified. If statuses includes "completed" and the status check
# is in that state it must also concluded with one of the conclusions specified additionally to statify has_status_check.
# > "checks" are required and are evaluated as regular expressions.
# > "statuses" is optional and defaults to ["completed", "success"].
# Possible values are: ["completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting", "error", "success"].
# If the value "any" is included in "statuses", the statuses will be expanded to all possible values listed above.
# While ["completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting]
# represent possible statuses for a 'check',
# the statuses ["error", "failing", "pending", "success"] represent possible statuses for 'commit statuses'
# see https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/collaborating-on-repositories-with-code-quality-features/about-status-checks#types-of-status-checks-on-github
# > "conclusions" is optional and defaults to ["success"].
# Possible values are ["action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"].
# If the value "any" is included in "conclusions", the statuses will be expanded to all possible values listed above.
# Conclustions are only evaluated if the status is in the "completed" state so this must be included in the "statuses" list.
# Also it has no effect on 'commit statuses' since they do not have conclusions.
has_status_check:
statuses: ["any"]
conclusions: ["any"]
checks:
- status-name-[0-9]

# DEPRECATED: Use "has_workflow" below instead, which is more flexible.
# "has_workflow_result" is satisfied if the GitHub Actions workflow runs that
# are specified all finished and concluded with one of the conclusions
# specified. "conclusions" is optional and defaults to ["success"].
Expand All @@ -403,6 +427,24 @@ if:
- ".github/workflows/a.yml"
- ".github/workflows/b.yml"

# "has_workflow" is satisfied if the workflows that are specified are
# in one of the statuses specified. If statuses includes "completed" and the workflow
# is in that state it must also concluded with one of the conclusions specified additionally to statify has_workflow.
# "workflows" are required and are evaluated as regular expressions.
# "statuses" is optional and defaults to ["completed"].
# Possible values are: ["completed", "expected", "failure", "in_progress", "pending", "queued", "requested", "startup_failure", "waiting"].
# If the value "any" is included in "statuses", the statuses will be expanded to all possible values listed above.
# "conclusions" is optional and defaults to ["success"].
# Possible values are ["action_required", "cancelled", "failure", "neutral", "skipped", "stale", "success", "timed_out"].
# If the value "any" is included in "conclusions", the statuses will be expanded to all possible values listed above.
# Conclustions are only evaluated if the status is in the "completed" state so this must be included in the "statuses" list.
has_workflow:
statuses: ["any"]
conclusions: ["any"]
workflows:
- '\.github/workflows/test-dependencies\.yml'
- 'check.*\.yml'

# "has_labels" is satisfied if the pull request has the specified labels
# applied
has_labels:
Expand Down
35 changes: 31 additions & 4 deletions policy/approval/approve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"
"time"

"github.com/google/go-github/v70/github"
"github.com/palantir/policy-bot/policy/common"
"github.com/palantir/policy-bot/policy/predicate"
"github.com/palantir/policy-bot/pull"
Expand Down Expand Up @@ -139,10 +140,12 @@ func TestIsApproved(t *testing.T) {
"comment-approver": {"everyone", "cool-org"},
"review-approver": {"everyone", "even-cooler-org"},
},
LatestStatusesValue: map[string]string{
"build": "success",
"deploy": "pending",
"scan": "success",
LatestRepoStatusesValue: map[string]*github.RepoStatus{
"build": {State: github.Ptr("success")},
"deploy": {State: github.Ptr("pending")},
"scan": {State: github.Ptr("success")},
"flaky": {State: github.Ptr("failure")},
"kermit": {State: github.Ptr("error")},
},
}
}
Expand Down Expand Up @@ -689,6 +692,30 @@ func TestIsApproved(t *testing.T) {
assertPending(t, prctx, r, "0/1 required conditions")
})

t.Run("conditionsRequiredStatusFailure", func(t *testing.T) {
prctx := basePullContext()
r := &Rule{
Requires: Requires{
Conditions: predicate.Predicates{
HasStatus: &predicate.HasStatus{Statuses: []string{"flaky"}},
},
},
}
assertPending(t, prctx, r, "0/1 required conditions")
})

t.Run("conditionsRequiredStatusError", func(t *testing.T) {
prctx := basePullContext()
r := &Rule{
Requires: Requires{
Conditions: predicate.Predicates{
HasStatus: &predicate.HasStatus{Statuses: []string{"kermit"}},
},
},
}
assertPending(t, prctx, r, "0/1 required conditions")
})

t.Run("conditionsRequiredStatusSuccess", func(t *testing.T) {
prctx := basePullContext()
r := &Rule{
Expand Down
11 changes: 11 additions & 0 deletions policy/common/regexp.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,21 @@ func NewRegexp(pattern string) (Regexp, error) {
return Regexp{r: r}, nil
}

func NewMustCompileRegexp(pattern string) Regexp {
return Regexp{r: regexp.MustCompile(pattern)}
}

func NewCompiledRegexp(r *regexp.Regexp) Regexp {
return Regexp{r: r}
}

// UnquoteMeta reverses the effect of QuoteMeta
func UnquoteMeta(s string) string {
re := regexp.MustCompile(`\\([\\.?])`) // Match escaped characters \\, \., and \?
literalString := re.ReplaceAllString(s, "$1")
return literalString
}

func (r Regexp) Matches(s string) bool {
if r.r == nil {
return false
Expand Down
22 changes: 16 additions & 6 deletions policy/predicate/predicates.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,13 +33,9 @@ type Predicates struct {

ModifiedLines *ModifiedLines `yaml:"modified_lines,omitempty"`

HasStatus *HasStatus `yaml:"has_status,omitempty"`
// `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,omitempty"`
HasStatusCheck *HasStatusCheck `yaml:"has_status_check,omitempty"`

HasWorkflowResult *HasWorkflowResult `yaml:"has_workflow_result,omitempty"`
HasWorkflow *HasWorkflow `yaml:"has_workflow,omitempty"`

HasLabels *HasLabels `yaml:"has_labels,omitempty"`

Expand All @@ -49,6 +45,12 @@ type Predicates struct {
HasValidSignatures *HasValidSignatures `yaml:"has_valid_signatures,omitempty"`
HasValidSignaturesBy *HasValidSignaturesBy `yaml:"has_valid_signatures_by,omitempty"`
HasValidSignaturesByKeys *HasValidSignaturesByKeys `yaml:"has_valid_signatures_by_keys,omitempty"`

// `has_successful_status`, `has_workflow_result` and `has_status` are deprecated fields that are kept for backwards
// compatibility. `has_status_check` and `has_workflow` replaces it, and can accept any conclusion and status.
HasStatus *HasStatus `yaml:"has_status,omitempty"`
HasSuccessfulStatus *HasSuccessfulStatus `yaml:"has_successful_status,omitempty"`
HasWorkflowResult *HasWorkflowResult `yaml:"has_workflow_result,omitempty"`
}

func (p Predicates) IsZero() bool {
Expand Down Expand Up @@ -108,6 +110,10 @@ func (p *Predicates) Predicates() []Predicate {
ps = append(ps, Predicate(p.HasStatus))
}

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

if p.HasSuccessfulStatus != nil {
ps = append(ps, Predicate(p.HasSuccessfulStatus))
}
Expand All @@ -116,6 +122,10 @@ func (p *Predicates) Predicates() []Predicate {
ps = append(ps, Predicate(p.HasWorkflowResult))
}

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

if p.HasLabels != nil {
ps = append(ps, Predicate(p.HasLabels))
}
Expand Down
89 changes: 13 additions & 76 deletions policy/predicate/status.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,20 +16,17 @@ 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 AllowedConclusions []string

// HasStatus checks that the specified statuses have a completed status with configurable conclusions.
//
// Deprecated: use the more flexible `HasStatusCheck` instead.
type HasStatus struct {
Conclusions AllowedConclusions `yaml:"conclusions,omitempty"`
Statuses []string `yaml:"statuses,omitempty"`
Conclusions []string `yaml:"conclusions,omitempty"`
Statuses []string `yaml:"statuses,omitempty"`
}

func NewHasStatus(statuses []string, conclusions []string) *HasStatus {
Expand All @@ -42,50 +39,15 @@ func NewHasStatus(statuses []string, conclusions []string) *HasStatus {
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
// Convert strings into a regex object to comply with function signature
var checks []common.Regexp
for _, status := range pred.Statuses {
result, ok := statuses[status]
if !ok {
missingResults = append(missingResults, status)
check, err := common.NewRegexp(status)
if err == nil {
checks = append(checks, check)
}
if !slices.Contains(conclusions, result) {
failingStatuses = append(failingStatuses, status)
}
}

if len(missingResults) > 0 {
predicateResult.Values = missingResults
predicateResult.Description = "One or more statuses is missing: " + strings.Join(missingResults, ", ")
predicateResult.Satisfied = false
return &predicateResult, nil
}

if len(failingStatuses) > 0 {
predicateResult.Values = 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.Statuses
predicateResult.Satisfied = true
return &predicateResult, nil
return HasStatusCheck{Checks: checks, Conclusions: pred.Conclusions, noRegex: true}.Evaluate(ctx, prctx)
}

func (pred HasStatus) Trigger() common.Trigger {
Expand All @@ -95,41 +57,16 @@ func (pred HasStatus) Trigger() common.Trigger {
// HasSuccessfulStatus checks that the specified statuses have a successful
// conclusion.
//
// Deprecated: use the more flexible `HasStatus` with `conclusions: ["success"]`
// Deprecated: use the more flexible `HasStatusCheck` 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)
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