Skip to content

Commit

Permalink
Add Option to not post status to github on failures and add equals op…
Browse files Browse the repository at this point in the history
…erator for modifications
  • Loading branch information
RoryDoherty committed Feb 8, 2024
1 parent 16cd898 commit 14959a4
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 26 deletions.
56 changes: 31 additions & 25 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# policy-bot
# policy-bot <!-- omit in toc -->

[![Docker Pulls](https://img.shields.io/docker/pulls/palantirtechnologies/policy-bot.svg)](https://hub.docker.com/r/palantirtechnologies/policy-bot/)

Expand All @@ -20,23 +20,24 @@ UI to view the detailed approval status of any pull request.
[required status check]: https://help.github.com/articles/enabling-required-status-checks/
[required reviews]: https://help.github.com/articles/about-required-reviews-for-pull-requests/

* [Configuration](#configuration)
+ [policy.yml Specification](#policyyml-specification)
+ [Approval Rules](#approval-rules)
+ [Approval Policies](#approval-policies)
+ [Disapproval Policy](#disapproval-policy)
+ [Caveats and Notes](#caveats-and-notes)
- [Configuration](#configuration)
- [policy.yml Specification](#policyyml-specification)
- [Approval Rules](#approval-rules)
- [Approval Policies](#approval-policies)
- [Disapproval Policy](#disapproval-policy)
- [Testing and Debugging Policies](#testing-and-debugging-policies)
- [Caveats and Notes](#caveats-and-notes)
- [Disapproval is Disabled by Default](#disapproval-is-disabled-by-default)
- [Interactions with GitHub Reviews](#interactions-with-github-reviews)
- [`or`, `and`, and `if` (Rule Predicates)](#or-and-and-if-rule-predicates)
- [Cross-organization Membership Tests](#cross-organization-membership-tests)
- [Update Merges](#update-merges)
- [Automatically Requesting Reviewers](#automatically-requesting-reviewers)
* [Security](#security)
* [Deployment](#deployment)
* [Development](#development)
* [Contributing](#contributing)
* [License](#license)
- [Security](#security)
- [Deployment](#deployment)
- [Development](#development)
- [Contributing](#contributing)
- [License](#license)

## Configuration

Expand Down Expand Up @@ -97,9 +98,14 @@ approval_rules:
- "^staging/.*$"
requires:
count: 0

# Settings which affect how policy-bot acts
settings:
# Set the below to true if you only want policy-bot statuses on PRs where the policy has evaluated to true
only_post_success_status: false
```
#### Notes on YAML Syntax
#### Notes on YAML Syntax <!-- omit in toc -->
The YAML language specification supports flow scalars (basic values like strings
and numbers) in three formats:
Expand All @@ -117,7 +123,7 @@ escape characters, which can cause confusion when used for regex strings
- Plain: There are no escape characters. Backslash characters do not need to be escaped.
e.g. `^BREAKING CHANGE: (\w| )+$`

#### Remote Policy Configuration
#### Remote Policy Configuration <!-- omit in toc -->

You can also define a remote policy by specifying a repository, path, and ref
(only repository is required). Instead of defining a `policy` key, you would
Expand Down Expand Up @@ -228,7 +234,7 @@ if:

# "modified_lines" is satisfied if the number of lines added or deleted by
# the pull request matches any of the listed conditions. Each expression is
# an operator (one of '<' or '>'), an optional space, and a number.
# an operator (one of '<', '>' or '='), an optional space, and a number.
modified_lines:
additions: "> 100"
deletions: "> 100"
Expand Down Expand Up @@ -638,7 +644,7 @@ repository `admin` permissions as organization owners are not selected.

The `teams` mode needs the team visibility to be set to `visibile` to enable this functionality for a given team.

##### Example
##### Example <!-- omit in toc -->

Given the following example requirement rule,

Expand All @@ -660,7 +666,7 @@ set of users of in
Where the Pull Request Author and any non direct collaborators have been removed
from the set.

#### Invalidating Approval on Push
#### Invalidating Approval on Push <!-- omit in toc -->

By default, `policy-bot` does not invalidate exisitng approvals when users add
new commits to a pull request. You can control this behavior for each rule in a
Expand Down Expand Up @@ -688,7 +694,7 @@ in mid-2023 because computing it was unreliable and inaccurate (see issue

[#598]: https://github.com/palantir/policy-bot/issues/598

#### Expanding Required Reviewers
#### Expanding Required Reviewers <!-- omit in toc -->

The details view for a pull request shows the users, organizations, teams, and
permission levels that are reqired to approve each rule. When the
Expand All @@ -715,7 +721,7 @@ While `policy-bot` can be used to implement security controls on GitHub
repositories, there are important limitations to be aware of before adopting
this approach.

### Status Checks
### Status Checks <!-- omit in toc -->

`policy-bot` reports approval status to GitHub using [commit statuses][]. While
statuses cannot be deleted, they can be set or overwritten by any user with
Expand All @@ -732,7 +738,7 @@ approve and merge a pull request before `policy-bot` can detect the problem.
Organizations concerned about this case should monitor and alert on the
relevant audit logs or minimize write access to repositories.

### Comment Edits
### Comment Edits <!-- omit in toc -->

GitHub users with sufficient permissions can edit the comments of other users,
possibly changing an unrelated comment into one that enables approval.
Expand All @@ -745,7 +751,7 @@ logs.
This issue can also be minimized by only using GitHub reviews for approval, at
the expense of removing the ability to self-approve pull requests.

### Commit Users
### Commit Users <!-- omit in toc -->

GitHub associates commits with users by mapping the email address in a commit
to email addresses associated with GitHub user accounts. `policy-bot` then uses
Expand All @@ -762,7 +768,7 @@ failure modes in this process:
If using GitHub Enterprise, both of these issues are avoidable by using the
[commit-current-user-check][] pre-receive hook.

### Update Merge Conflicts
### Update Merge Conflicts <!-- omit in toc -->

When using the `ignore_update_merges` option, `policy-bot` cannot tell the
difference between clean merges and merges that contain conflict resolution.
Expand Down Expand Up @@ -796,7 +802,7 @@ variables for server values are prefixed with `POLICYBOT_` (e.g.
`POLICYBOT_PORT`). This prefix can be overridden by setting the
`POLICYBOT_ENV_PREFIX` environment variable.

### GitHub App Configuration
### GitHub App Configuration <!-- omit in toc -->

To configure `policy-bot` as a GitHub App, set these options in GitHub:

Expand Down Expand Up @@ -843,7 +849,7 @@ generated values:
- Client secret (`github.oauth.client_secret`)
- Private key (`github.app.private_key`)

### Operations
### Operations <!-- omit in toc -->

`policy-bot` uses [go-baseapp](https://github.com/palantir/go-baseapp) and
[go-githubapp](https://github.com/palantir/go-githubapp), both of which emit
Expand Down Expand Up @@ -904,7 +910,7 @@ and [Yarn](https://yarnpkg.com/).
modified config file `policy-bot.yml`
- The server is available at `http://localhost:8080/`

### Example Policy Files
### Example Policy Files <!-- omit in toc -->

Example policy files can be found in [`config/policy-examples`](https://github.com/palantir/policy-bot/tree/develop/config/policy-examples)

Expand Down
5 changes: 5 additions & 0 deletions policy/policy.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ type RemoteConfig struct {
type Config struct {
Policy Policy `yaml:"policy"`
ApprovalRules []*approval.Rule `yaml:"approval_rules"`
Settings Settings `yaml:"settings"`
}

type Settings struct {
OnlyPostSuccessStatus bool `yaml:"only_post_success_status"`
}

type Policy struct {
Expand Down
7 changes: 7 additions & 0 deletions policy/predicate/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ const (
OpNone CompareOp = iota
OpLessThan
OpGreaterThan
OpEquals
)

type ComparisonExpr struct {
Expand All @@ -170,6 +171,8 @@ func (exp ComparisonExpr) Evaluate(n int64) bool {
return n < exp.Value
case OpGreaterThan:
return n > exp.Value
case OpEquals:
return n == exp.Value
}
return false
}
Expand All @@ -185,6 +188,8 @@ func (exp ComparisonExpr) MarshalText() ([]byte, error) {
op = "<"
case OpGreaterThan:
op = ">"
case OpEquals:
op = "="
default:
return nil, errors.Errorf("unknown operation: %d", exp.Op)
}
Expand Down Expand Up @@ -213,6 +218,8 @@ func (exp *ComparisonExpr) UnmarshalText(text []byte) error {
op = OpLessThan
case '>':
op = OpGreaterThan
case '=':
op = OpEquals
default:
return errors.Errorf("invalid comparison operator: %c", text[i])
}
Expand Down
70 changes: 69 additions & 1 deletion policy/predicate/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,70 @@ func TestModifiedLines(t *testing.T) {
},
},
})

p = &ModifiedLines{
Total: ComparisonExpr{Op: OpEquals, Value: 100},
}

runFileTests(t, p, []FileTestCase{
{
"total",
[]*pull.File{
{Additions: 20, Deletions: 20},
{Additions: 20},
{Additions: 20, Deletions: 20},
},
&common.PredicateResult{
Satisfied: true,
Values: []string{"total 100"},
ConditionValues: []string{"total modifications = 100"},
},
},
})

p = &ModifiedLines{
Additions: ComparisonExpr{Op: OpEquals, Value: 100},
Deletions: ComparisonExpr{Op: OpEquals, Value: 25},
}

runFileTests(t, p, []FileTestCase{
{
"empty",
[]*pull.File{},
&common.PredicateResult{
Satisfied: false,
Values: []string{"+0", "-0"},
ConditionValues: []string{"added lines = 100", "deleted lines = 25"},
},
},
{
"additions",
[]*pull.File{
{Additions: 55},
{Additions: 45},
},
&common.PredicateResult{
Satisfied: true,
Values: []string{"+100"},
ConditionValues: []string{"added lines = 100"},
},
},
{
"deletions",
[]*pull.File{
{Additions: 5, Deletions: 5},
{Deletions: 10},
{Additions: 5},
{Deletions: 10},
},
&common.PredicateResult{
Satisfied: true,
Values: []string{"-25"},
ConditionValues: []string{"deleted lines = 25"},
},
},
})

}

func TestComparisonExpr(t *testing.T) {
Expand Down Expand Up @@ -352,6 +416,10 @@ func TestComparisonExpr(t *testing.T) {
Input: ">100",
Output: ComparisonExpr{Op: OpGreaterThan, Value: 100},
},
"equals": {
Input: "=100",
Output: ComparisonExpr{Op: OpEquals, Value: 100},
},
"innerSpaces": {
Input: "< 35",
Output: ComparisonExpr{Op: OpLessThan, Value: 35},
Expand All @@ -365,7 +433,7 @@ func TestComparisonExpr(t *testing.T) {
Output: ComparisonExpr{Op: OpLessThan, Value: 35},
},
"invalidOp": {
Input: "=10",
Input: "~10",
Err: true,
},
"invalidValue": {
Expand Down
5 changes: 5 additions & 0 deletions server/handler/eval_context.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,11 @@ func (ec *EvalContext) PostStatus(ctx context.Context, state, message string) {
return
}

if state != "success" && ec.Config.Config.Settings.OnlyPostSuccessStatus {
logger.Info().Msg("Skipping status update as it is not success and the setting is enabled to only post status updates on success")
return
}

if !ec.PullContext.IsOpen() {
logger.Info().Msg("Skipping status update because PR state is not open")
return
Expand Down

0 comments on commit 14959a4

Please sign in to comment.