Skip to content

Commit

Permalink
Add equals operator for file modifications checks (#712)
Browse files Browse the repository at this point in the history
  • Loading branch information
RoryDoherty authored Feb 9, 2024
1 parent 63aae55 commit 4a173ef
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 26 deletions.
51 changes: 26 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 @@ -99,7 +100,7 @@ approval_rules:
count: 0
```
#### 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 +118,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 +229,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 +639,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 +661,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 +689,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 +716,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 +733,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 +746,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 +763,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 +797,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 +844,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 +905,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
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

0 comments on commit 4a173ef

Please sign in to comment.