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 Equals operator for file modifications checks #712

Merged
merged 1 commit into from
Feb 9, 2024
Merged
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
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