Skip to content

Commit 0414f3e

Browse files
author
Kristian
authored
Merge pull request #126 from m17ch/master
Feature request: required_review_approvals parameter
2 parents 94d3f6b + 432c2ea commit 0414f3e

File tree

7 files changed

+82
-51
lines changed

7 files changed

+82
-51
lines changed

‎README.md

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,24 +19,26 @@ Make sure to check out [#migrating](#migrating) to learn more.
1919

2020
## Source Configuration
2121

22-
| Parameter | Required | Example | Description |
23-
|-------------------------|----------|----------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
24-
| `repository` | Yes | `itsdalmo/test-repository` | The repository to target. |
25-
| `access_token` | Yes | | A Github Access Token with repository access (required for setting status on commits). N.B. If you want github-pr-resource to work with a private repository. Set `repo:full` permissions on the access token you create on GitHub. If it is a public repository, `repo:status` is enough. |
26-
| `v3_endpoint` | No | `https://api.github.com` | Endpoint to use for the V3 Github API (Restful). |
27-
| `v4_endpoint` | No | `https://api.github.com/graphql` | Endpoint to use for the V4 Github API (Graphql). |
28-
| `paths` | No | `terraform/*/*.tf` | Only produce new versions if the PR includes changes to files that match one or more glob patterns or prefixes. |
29-
| `ignore_paths` | No | `.ci/` | Inverse of the above. Pattern syntax is documented in [filepath.Match](https://golang.org/pkg/path/filepath/#Match), or a path prefix can be specified (e.g. `.ci/` will match everything in the `.ci` directory). |
30-
| `disable_ci_skip` | No | `true` | Disable ability to skip builds with `[ci skip]` and `[skip ci]` in commit message or pull request title. |
31-
| `skip_ssl_verification` | No | `true` | Disable SSL/TLS certificate validation on git and API clients. Use with care! |
32-
| `disable_forks` | No | `true` | Disable triggering of the resource if the pull request's fork repository is different to the configured repository. |
33-
| `git_crypt_key` | No | `AEdJVENSWVBUS0VZAAAAA...` | Base64 encoded git-crypt key. Setting this will unlock / decrypt the repository with git-crypt. To get the key simply execute `git-crypt export-key -- - | base64` in an encrypted repository. |
34-
| `base_branch` | No | `master` | Name of a branch. The pipeline will only trigger on pull requests against the specified branch. |
22+
| Parameter | Required | Example | Description |
23+
|-----------------------------|----------|----------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
24+
| `repository` | Yes | `itsdalmo/test-repository` | The repository to target. |
25+
| `access_token` | Yes | | A Github Access Token with repository access (required for setting status on commits). N.B. If you want github-pr-resource to work with a private repository. Set `repo:full` permissions on the access token you create on GitHub. If it is a public repository, `repo:status` is enough. |
26+
| `v3_endpoint` | No | `https://api.github.com` | Endpoint to use for the V3 Github API (Restful). |
27+
| `v4_endpoint` | No | `https://api.github.com/graphql` | Endpoint to use for the V4 Github API (Graphql). |
28+
| `paths` | No | `terraform/*/*.tf` | Only produce new versions if the PR includes changes to files that match one or more glob patterns or prefixes. |
29+
| `ignore_paths` | No | `.ci/` | Inverse of the above. Pattern syntax is documented in [filepath.Match](https://golang.org/pkg/path/filepath/#Match), or a path prefix can be specified (e.g. `.ci/` will match everything in the `.ci` directory). |
30+
| `disable_ci_skip` | No | `true` | Disable ability to skip builds with `[ci skip]` and `[skip ci]` in commit message or pull request title. |
31+
| `skip_ssl_verification` | No | `true` | Disable SSL/TLS certificate validation on git and API clients. Use with care! |
32+
| `disable_forks` | No | `true` | Disable triggering of the resource if the pull request's fork repository is different to the configured repository. |
33+
| `required_review_approvals` | No | `2` | Disable triggering of the resource if the pull request does not have at least `X` approved review(s). |
34+
| `git_crypt_key` | No | `AEdJVENSWVBUS0VZAAAAA...` | Base64 encoded git-crypt key. Setting this will unlock / decrypt the repository with git-crypt. To get the key simply execute `git-crypt export-key -- - | base64` in an encrypted repository. |
35+
| `base_branch` | No | `master` | Name of a branch. The pipeline will only trigger on pull requests against the specified branch. |
3536

3637
Notes:
3738
- If `v3_endpoint` is set, `v4_endpoint` must also be set (and the other way around).
3839
- Look at the [Concourse Resources documentation](https://concourse-ci.org/resources.html#resource-webhook-token)
3940
for webhook token configuration.
41+
- When using `required_review_approvals`, you may also want to enable GitHub's branch protection rules to [dismiss stale pull request approvals when new commits are pushed](https://help.github.com/en/articles/enabling-required-reviews-for-pull-requests).
4042

4143
## Behaviour
4244

@@ -205,6 +207,7 @@ If you are coming from [jtarchie/github-pullrequest-resource][original-resource]
205207
- `api_endpoint` -> `v3_endpoint`
206208
- `base` -> `base_branch`
207209
- `base_url` -> `target_url`
210+
- `require_review_approval` -> `required_review_approvals` (`bool` to `int`)
208211
- `get`:
209212
- `git.depth` -> `git_depth`
210213
- `put`:
@@ -222,7 +225,6 @@ If you are coming from [jtarchie/github-pullrequest-resource][original-resource]
222225

223226
#### Parameters that did not make it:
224227
- `src`:
225-
- `require_review_approval`
226228
- `authorship_restriction`
227229
- `label`
228230
- `git_config`: You can now get the pr/author info from .git/resource/metadata.json instead

‎check.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,11 @@ Loop:
4242
continue
4343
}
4444

45+
// Filter pull request if it does not have the required number of approved review(s).
46+
if p.ApprovedReviewCount < request.Source.RequiredReviewApprovals {
47+
continue
48+
}
49+
4550
// Fetch files once if paths/ignore_paths are specified.
4651
var files []string
4752

‎check_test.go

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,15 @@ import (
1010

1111
var (
1212
testPullRequests = []*resource.PullRequest{
13-
createTestPR(1, "master", true, false),
14-
createTestPR(2, "master", false, false),
15-
createTestPR(3, "master", false, false),
16-
createTestPR(4, "master", false, false),
17-
createTestPR(5, "master", false, true),
18-
createTestPR(6, "master", false, false),
19-
createTestPR(7, "develop", false, false),
13+
createTestPR(1, "master", true, false, 0),
14+
createTestPR(2, "master", false, false, 0),
15+
createTestPR(3, "master", false, false, 0),
16+
createTestPR(4, "master", false, false, 0),
17+
createTestPR(5, "master", false, true, 0),
18+
createTestPR(6, "master", false, false, 0),
19+
createTestPR(7, "develop", false, false, 0),
20+
createTestPR(8, "master", false, false, 1),
21+
createTestPR(9, "master", false, false, 0),
2022
}
2123
)
2224

@@ -151,6 +153,19 @@ func TestCheck(t *testing.T) {
151153
resource.NewVersion(testPullRequests[6]),
152154
},
153155
},
156+
{
157+
description: "check correctly ignores PRs with no approved reviews when specified",
158+
source: resource.Source{
159+
Repository: "itsdalmo/test-repository",
160+
AccessToken: "oauthtoken",
161+
RequiredReviewApprovals: 1,
162+
},
163+
version: resource.NewVersion(testPullRequests[8]),
164+
pullRequests: testPullRequests,
165+
expected: resource.CheckResponse{
166+
resource.NewVersion(testPullRequests[7]),
167+
},
168+
},
154169
}
155170

156171
for _, tc := range tests {

‎github.go

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,9 @@ func (m *GithubClient) ListOpenPullRequests() ([]*PullRequest, error) {
104104
Edges []struct {
105105
Node struct {
106106
PullRequestObject
107+
Reviews struct {
108+
TotalCount int
109+
} `graphql:"reviews(states: $prReviewStates)"`
107110
Commits struct {
108111
Edges []struct {
109112
Node struct {
@@ -128,6 +131,7 @@ func (m *GithubClient) ListOpenPullRequests() ([]*PullRequest, error) {
128131
"prStates": []githubv4.PullRequestState{githubv4.PullRequestStateOpen},
129132
"prCursor": (*githubv4.String)(nil),
130133
"commitsLast": githubv4.Int(1),
134+
"prReviewStates": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStateApproved},
131135
}
132136

133137
var response []*PullRequest
@@ -138,8 +142,9 @@ func (m *GithubClient) ListOpenPullRequests() ([]*PullRequest, error) {
138142
for _, p := range query.Repository.PullRequests.Edges {
139143
for _, c := range p.Node.Commits.Edges {
140144
response = append(response, &PullRequest{
141-
PullRequestObject: p.Node.PullRequestObject,
142-
Tip: c.Node.Commit,
145+
PullRequestObject: p.Node.PullRequestObject,
146+
Tip: c.Node.Commit,
147+
ApprovedReviewCount: p.Node.Reviews.TotalCount,
143148
})
144149
}
145150
}

‎in_test.go

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ func TestGet(t *testing.T) {
4040
CommittedDate: time.Time{},
4141
},
4242
parameters: resource.GetParameters{},
43-
pullRequest: createTestPR(1, "master", false, false),
43+
pullRequest: createTestPR(1, "master", false, false, 0),
4444
versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z"}`,
4545
metadataString: `[{"name":"pr","value":"1"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"}]`,
4646
},
@@ -57,7 +57,7 @@ func TestGet(t *testing.T) {
5757
CommittedDate: time.Time{},
5858
},
5959
parameters: resource.GetParameters{},
60-
pullRequest: createTestPR(1, "master", false, false),
60+
pullRequest: createTestPR(1, "master", false, false, 0),
6161
versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z"}`,
6262
metadataString: `[{"name":"pr","value":"1"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"}]`,
6363
},
@@ -75,7 +75,7 @@ func TestGet(t *testing.T) {
7575
parameters: resource.GetParameters{
7676
IntegrationTool: "rebase",
7777
},
78-
pullRequest: createTestPR(1, "master", false, false),
78+
pullRequest: createTestPR(1, "master", false, false, 0),
7979
versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z"}`,
8080
metadataString: `[{"name":"pr","value":"1"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"}]`,
8181
},
@@ -93,7 +93,7 @@ func TestGet(t *testing.T) {
9393
parameters: resource.GetParameters{
9494
IntegrationTool: "checkout",
9595
},
96-
pullRequest: createTestPR(1, "master", false, false),
96+
pullRequest: createTestPR(1, "master", false, false, 0),
9797
versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z"}`,
9898
metadataString: `[{"name":"pr","value":"1"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"}]`,
9999
},
@@ -111,7 +111,7 @@ func TestGet(t *testing.T) {
111111
parameters: resource.GetParameters{
112112
GitDepth: 2,
113113
},
114-
pullRequest: createTestPR(1, "master", false, false),
114+
pullRequest: createTestPR(1, "master", false, false, 0),
115115
versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z"}`,
116116
metadataString: `[{"name":"pr","value":"1"},{"name":"url","value":"pr1 url"},{"name":"head_name","value":"pr1"},{"name":"head_sha","value":"oid1"},{"name":"base_name","value":"master"},{"name":"base_sha","value":"sha"},{"name":"message","value":"commit message1"},{"name":"author","value":"login1"}]`,
117117
},
@@ -129,7 +129,7 @@ func TestGet(t *testing.T) {
129129
parameters: resource.GetParameters{
130130
ListChangedFiles: true,
131131
},
132-
pullRequest: createTestPR(1, "master", false, false),
132+
pullRequest: createTestPR(1, "master", false, false, 0),
133133
files: []resource.ChangedFileObject{
134134
{
135135
Path: "README.md",
@@ -298,13 +298,14 @@ func TestGetSkipDownload(t *testing.T) {
298298
}
299299
}
300300

301-
func createTestPR(count int, baseName string, skipCI bool, isCrossRepo bool) *resource.PullRequest {
301+
func createTestPR(count int, baseName string, skipCI bool, isCrossRepo bool, approvedReviews int) *resource.PullRequest {
302302
n := strconv.Itoa(count)
303303
d := time.Now().AddDate(0, 0, -count)
304304
m := fmt.Sprintf("commit message%s", n)
305305
if skipCI {
306306
m = "[skip ci]" + m
307307
}
308+
approvedCount := approvedReviews
308309

309310
return &resource.PullRequest{
310311
PullRequestObject: resource.PullRequestObject{
@@ -330,6 +331,7 @@ func createTestPR(count int, baseName string, skipCI bool, isCrossRepo bool) *re
330331
},
331332
},
332333
},
334+
ApprovedReviewCount: approvedCount,
333335
}
334336
}
335337

‎models.go

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,18 @@ import (
1010

1111
// Source represents the configuration for the resource.
1212
type Source struct {
13-
Repository string `json:"repository"`
14-
AccessToken string `json:"access_token"`
15-
V3Endpoint string `json:"v3_endpoint"`
16-
V4Endpoint string `json:"v4_endpoint"`
17-
Paths []string `json:"paths"`
18-
IgnorePaths []string `json:"ignore_paths"`
19-
DisableCISkip bool `json:"disable_ci_skip"`
20-
SkipSSLVerification bool `json:"skip_ssl_verification"`
21-
DisableForks bool `json:"disable_forks"`
22-
GitCryptKey string `json:"git_crypt_key"`
23-
BaseBranch string `json:"base_branch"`
13+
Repository string `json:"repository"`
14+
AccessToken string `json:"access_token"`
15+
V3Endpoint string `json:"v3_endpoint"`
16+
V4Endpoint string `json:"v4_endpoint"`
17+
Paths []string `json:"paths"`
18+
IgnorePaths []string `json:"ignore_paths"`
19+
DisableCISkip bool `json:"disable_ci_skip"`
20+
SkipSSLVerification bool `json:"skip_ssl_verification"`
21+
DisableForks bool `json:"disable_forks"`
22+
GitCryptKey string `json:"git_crypt_key"`
23+
BaseBranch string `json:"base_branch"`
24+
RequiredReviewApprovals int `json:"required_review_approvals"`
2425
}
2526

2627
// Validate the source configuration.
@@ -73,7 +74,8 @@ func NewVersion(p *PullRequest) Version {
7374
// PullRequest represents a pull request and includes the tip (commit).
7475
type PullRequest struct {
7576
PullRequestObject
76-
Tip CommitObject
77+
Tip CommitObject
78+
ApprovedReviewCount int
7779
}
7880

7981
// PullRequestObject represents the GraphQL commit node.

0 commit comments

Comments
 (0)