From 8aa1ece5ec911c9f4220dc8b7c59268ba9c148e9 Mon Sep 17 00:00:00 2001 From: Anshul Sirur Date: Wed, 14 Oct 2020 20:58:57 +0200 Subject: [PATCH] Allow filtering PRs by state In some situations we might only want to pick up resource versions from PRs that have been merged or closed. For example, to trigger a job based once a PR gets closed. --- README.md | 5 ++-- check.go | 16 +++++++++++++ check_test.go | 66 ++++++++++++++++++++++++++++++++++++++++++++------- github.go | 2 +- in.go | 1 + in_test.go | 26 ++++++++++---------- models.go | 43 ++++++++++++++++++++++----------- out_test.go | 23 +++++++++--------- 8 files changed, 133 insertions(+), 49 deletions(-) diff --git a/README.md b/README.md index 1e1ac086..e2c6e226 100644 --- a/README.md +++ b/README.md @@ -25,8 +25,8 @@ Make sure to check out [#migrating](#migrating) to learn more. | `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. | | `v3_endpoint` | No | `https://api.github.com` | Endpoint to use for the V3 Github API (Restful). | | `v4_endpoint` | No | `https://api.github.com/graphql` | Endpoint to use for the V4 Github API (Graphql). | -| `paths` | No | `["terraform/*/*.tf"]` | Only produce new versions if the PR includes changes to files that match one or more glob patterns or prefixes. | -| `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). | +| `paths` | No | `["terraform/*/*.tf"]` | Only produce new versions if the PR includes changes to files that match one or more glob patterns or prefixes. | +| `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). | | `disable_ci_skip` | No | `true` | Disable ability to skip builds with `[ci skip]` and `[skip ci]` in commit message or pull request title. | | `skip_ssl_verification` | No | `true` | Disable SSL/TLS certificate validation on git and API clients. Use with care! | | `disable_forks` | No | `true` | Disable triggering of the resource if the pull request's fork repository is different to the configured repository. | @@ -35,6 +35,7 @@ Make sure to check out [#migrating](#migrating) to learn more. | `base_branch` | No | `master` | Name of a branch. The pipeline will only trigger on pull requests against the specified branch. | | `labels` | No | `["bug", "enhancement"]` | The labels on the PR. The pipeline will only trigger on pull requests having at least one of the specified labels. | | `disable_git_lfs` | No | `true` | Disable Git LFS, skipping an attempt to convert pointers of files tracked into their corresponding objects when checked out into a working copy. | +| `states` | No | `["OPEN", "MERGED"]` | The PR states to select (`OPEN`, `MERGED` or `CLOSED`). The pipeline will only trigger on pull requests matching one of the specified states. Default is ["OPEN"]. | Notes: - If `v3_endpoint` is set, `v4_endpoint` must also be set (and the other way around). diff --git a/check.go b/check.go index df128717..34fb58a3 100644 --- a/check.go +++ b/check.go @@ -38,6 +38,22 @@ Loop: continue } + // Filter out pull request if it does not have a filtered state + if len(request.Source.States) > 0 { + stateFound := false + + for _, state := range request.Source.States { + if p.State == state { + stateFound = true + break + } + } + + if !stateFound { + continue + } + } + // Filter out pull request if it does not contain at least one of the desired labels if len(request.Source.Labels) > 0 { labelFound := false diff --git a/check_test.go b/check_test.go index 9715394c..86fabf66 100644 --- a/check_test.go +++ b/check_test.go @@ -3,6 +3,7 @@ package resource_test import ( "testing" + "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" resource "github.com/telia-oss/github-pr-resource" "github.com/telia-oss/github-pr-resource/fakes" @@ -10,15 +11,18 @@ import ( var ( testPullRequests = []*resource.PullRequest{ - createTestPR(1, "master", true, false, 0, nil), - createTestPR(2, "master", false, false, 0, nil), - createTestPR(3, "master", false, false, 0, nil), - createTestPR(4, "master", false, false, 0, nil), - createTestPR(5, "master", false, true, 0, nil), - createTestPR(6, "master", false, false, 0, nil), - createTestPR(7, "develop", false, false, 0, []string{"enhancement"}), - createTestPR(8, "master", false, false, 1, []string{"wontfix"}), - createTestPR(9, "master", false, false, 0, nil), + createTestPR(1, "master", true, false, 0, nil, githubv4.PullRequestStateOpen), + createTestPR(2, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), + createTestPR(3, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), + createTestPR(4, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), + createTestPR(5, "master", false, true, 0, nil, githubv4.PullRequestStateOpen), + createTestPR(6, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), + createTestPR(7, "develop", false, false, 0, []string{"enhancement"}, githubv4.PullRequestStateOpen), + createTestPR(8, "master", false, false, 1, []string{"wontfix"}, githubv4.PullRequestStateOpen), + createTestPR(9, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), + createTestPR(10, "master", false, false, 0, nil, githubv4.PullRequestStateClosed), + createTestPR(11, "master", false, false, 0, nil, githubv4.PullRequestStateMerged), + createTestPR(12, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), } ) @@ -185,6 +189,50 @@ func TestCheck(t *testing.T) { resource.NewVersion(testPullRequests[6]), }, }, + + { + description: "check returns latest version from a PR with a single state filter", + source: resource.Source{ + Repository: "itsdalmo/test-repository", + AccessToken: "oauthtoken", + States: []githubv4.PullRequestState{githubv4.PullRequestStateClosed}, + }, + version: resource.Version{}, + pullRequests: testPullRequests, + files: [][]string{}, + expected: resource.CheckResponse{ + resource.NewVersion(testPullRequests[9]), + }, + }, + + { + description: "check filters out versions from a PR which do not match the state filter", + source: resource.Source{ + Repository: "itsdalmo/test-repository", + AccessToken: "oauthtoken", + States: []githubv4.PullRequestState{githubv4.PullRequestStateOpen}, + }, + version: resource.Version{}, + pullRequests: testPullRequests[9:11], + files: [][]string{}, + expected: resource.CheckResponse(nil), + }, + + { + description: "check returns versions from a PR with multiple state filters", + source: resource.Source{ + Repository: "itsdalmo/test-repository", + AccessToken: "oauthtoken", + States: []githubv4.PullRequestState{githubv4.PullRequestStateClosed, githubv4.PullRequestStateMerged}, + }, + version: resource.NewVersion(testPullRequests[11]), + pullRequests: testPullRequests, + files: [][]string{}, + expected: resource.CheckResponse{ + resource.NewVersion(testPullRequests[10]), + resource.NewVersion(testPullRequests[9]), + }, + }, } for _, tc := range tests { diff --git a/github.go b/github.go index fbbe4734..33372ccc 100644 --- a/github.go +++ b/github.go @@ -136,7 +136,7 @@ func (m *GithubClient) ListOpenPullRequests() ([]*PullRequest, error) { "repositoryOwner": githubv4.String(m.Owner), "repositoryName": githubv4.String(m.Repository), "prFirst": githubv4.Int(100), - "prStates": []githubv4.PullRequestState{githubv4.PullRequestStateOpen}, + "prStates": []githubv4.PullRequestState{githubv4.PullRequestStateOpen, githubv4.PullRequestStateClosed, githubv4.PullRequestStateMerged}, "prCursor": (*githubv4.String)(nil), "commitsLast": githubv4.Int(1), "prReviewStates": []githubv4.PullRequestReviewState{githubv4.PullRequestReviewStateApproved}, diff --git a/in.go b/in.go index cfde7e8d..fa0eb242 100644 --- a/in.go +++ b/in.go @@ -51,6 +51,7 @@ func Get(request GetRequest, github Github, git Git, outputDir string) (*GetResp metadata.Add("message", pull.Tip.Message) metadata.Add("author", pull.Tip.Author.User.Login) metadata.Add("author_email", pull.Tip.Author.Email) + metadata.Add("state", string(pull.State)) // Write version and metadata for reuse in PUT path := filepath.Join(outputDir, ".git", "resource") diff --git a/in_test.go b/in_test.go index 3ac8360e..a335421b 100644 --- a/in_test.go +++ b/in_test.go @@ -41,9 +41,9 @@ func TestGet(t *testing.T) { ApprovedReviewCount: "0", }, parameters: resource.GetParameters{}, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0"}`, - metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"}]`, + metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { description: "get supports unlocking with git crypt", @@ -59,9 +59,9 @@ func TestGet(t *testing.T) { ApprovedReviewCount: "0", }, parameters: resource.GetParameters{}, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0"}`, - metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"}]`, + metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { description: "get supports rebasing", @@ -78,9 +78,9 @@ func TestGet(t *testing.T) { parameters: resource.GetParameters{ IntegrationTool: "rebase", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0"}`, - metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"}]`, + metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { description: "get supports checkout", @@ -97,9 +97,9 @@ func TestGet(t *testing.T) { parameters: resource.GetParameters{ IntegrationTool: "checkout", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0"}`, - metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"}]`, + metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { description: "get supports git_depth", @@ -116,9 +116,9 @@ func TestGet(t *testing.T) { parameters: resource.GetParameters{ GitDepth: 2, }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0"}`, - metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"}]`, + metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, }, { description: "get supports list_changed_files", @@ -135,7 +135,7 @@ func TestGet(t *testing.T) { parameters: resource.GetParameters{ ListChangedFiles: true, }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), files: []resource.ChangedFileObject{ { Path: "README.md", @@ -145,7 +145,7 @@ func TestGet(t *testing.T) { }, }, versionString: `{"pr":"pr1","commit":"commit1","committed":"0001-01-01T00:00:00Z","approved_review_count":"0"}`, - metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"}]`, + metadataString: `[{"name":"pr","value":"1"},{"name":"title","value":"pr1 title"},{"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"},{"name":"author_email","value":"user@example.com"},{"name":"state","value":"OPEN"}]`, filesString: "README.md\nOther.md\n", }, } @@ -319,6 +319,7 @@ func createTestPR( isCrossRepo bool, approvedReviews int, labels []string, + state githubv4.PullRequestState, ) *resource.PullRequest { n := strconv.Itoa(count) d := time.Now().AddDate(0, 0, -count) @@ -349,6 +350,7 @@ func createTestPR( URL: fmt.Sprintf("repo%s url", n), }, IsCrossRepository: isCrossRepo, + State: state, }, Tip: resource.CommitObject{ ID: fmt.Sprintf("commit%s", n), diff --git a/models.go b/models.go index a252cd4e..b7ddb1cd 100644 --- a/models.go +++ b/models.go @@ -2,6 +2,7 @@ package resource import ( "errors" + "fmt" "strconv" "time" @@ -10,20 +11,21 @@ import ( // Source represents the configuration for the resource. type Source struct { - Repository string `json:"repository"` - AccessToken string `json:"access_token"` - V3Endpoint string `json:"v3_endpoint"` - V4Endpoint string `json:"v4_endpoint"` - Paths []string `json:"paths"` - IgnorePaths []string `json:"ignore_paths"` - DisableCISkip bool `json:"disable_ci_skip"` - DisableGitLFS bool `json:"disable_git_lfs"` - SkipSSLVerification bool `json:"skip_ssl_verification"` - DisableForks bool `json:"disable_forks"` - GitCryptKey string `json:"git_crypt_key"` - BaseBranch string `json:"base_branch"` - RequiredReviewApprovals int `json:"required_review_approvals"` - Labels []string `json:"labels"` + Repository string `json:"repository"` + AccessToken string `json:"access_token"` + V3Endpoint string `json:"v3_endpoint"` + V4Endpoint string `json:"v4_endpoint"` + Paths []string `json:"paths"` + IgnorePaths []string `json:"ignore_paths"` + DisableCISkip bool `json:"disable_ci_skip"` + DisableGitLFS bool `json:"disable_git_lfs"` + SkipSSLVerification bool `json:"skip_ssl_verification"` + DisableForks bool `json:"disable_forks"` + GitCryptKey string `json:"git_crypt_key"` + BaseBranch string `json:"base_branch"` + RequiredReviewApprovals int `json:"required_review_approvals"` + Labels []string `json:"labels"` + States []githubv4.PullRequestState `json:"states"` } // Validate the source configuration. @@ -40,6 +42,18 @@ func (s *Source) Validate() error { if s.V4Endpoint != "" && s.V3Endpoint == "" { return errors.New("v3_endpoint must be set together with v4_endpoint") } + for _, state := range s.States { + switch state { + case githubv4.PullRequestStateOpen: + case githubv4.PullRequestStateClosed: + case githubv4.PullRequestStateMerged: + default: + return errors.New(fmt.Sprintf("states value \"%s\" must be one of: OPEN, MERGED, CLOSED", state)) + } + } + if len(s.States) == 0 { + s.States = []githubv4.PullRequestState{githubv4.PullRequestStateOpen} + } return nil } @@ -96,6 +110,7 @@ type PullRequestObject struct { URL string } IsCrossRepository bool + State githubv4.PullRequestState } // CommitObject represents the GraphQL commit node. diff --git a/out_test.go b/out_test.go index b1fe56d7..79009881 100644 --- a/out_test.go +++ b/out_test.go @@ -6,6 +6,7 @@ import ( "testing" "time" + "github.com/shurcooL/githubv4" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" resource "github.com/telia-oss/github-pr-resource" @@ -33,7 +34,7 @@ func TestPut(t *testing.T) { CommittedDate: time.Time{}, }, parameters: resource.PutParameters{}, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, { @@ -50,7 +51,7 @@ func TestPut(t *testing.T) { parameters: resource.PutParameters{ Status: "success", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, { @@ -68,7 +69,7 @@ func TestPut(t *testing.T) { Status: "failure", Context: "build", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, { @@ -87,7 +88,7 @@ func TestPut(t *testing.T) { BaseContext: "concourse-ci-custom", Context: "build", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, { @@ -105,7 +106,7 @@ func TestPut(t *testing.T) { Status: "failure", TargetURL: "https://targeturl.com/concourse", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, { @@ -123,7 +124,7 @@ func TestPut(t *testing.T) { Status: "failure", Description: "Concourse CI build", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, { @@ -140,7 +141,7 @@ func TestPut(t *testing.T) { parameters: resource.PutParameters{ Comment: "comment", }, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, { @@ -157,7 +158,7 @@ func TestPut(t *testing.T) { parameters: resource.PutParameters{ DeletePreviousComments: true, }, - pullRequest: createTestPR(1, "master", false, false, 0, []string{}), + pullRequest: createTestPR(1, "master", false, false, 0, []string{}, githubv4.PullRequestStateOpen), }, } @@ -250,7 +251,7 @@ func TestVariableSubstitution(t *testing.T) { Comment: fmt.Sprintf("$%s", variableName), }, expectedComment: variableValue, - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, { @@ -269,7 +270,7 @@ func TestVariableSubstitution(t *testing.T) { TargetURL: fmt.Sprintf("%s$%s", variableURL, variableName), }, expectedTargetURL: fmt.Sprintf("%s%s", variableURL, variableValue), - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, { @@ -287,7 +288,7 @@ func TestVariableSubstitution(t *testing.T) { Comment: "$THIS_IS_NOT_SUBSTITUTED", }, expectedComment: "$THIS_IS_NOT_SUBSTITUTED", - pullRequest: createTestPR(1, "master", false, false, 0, nil), + pullRequest: createTestPR(1, "master", false, false, 0, nil, githubv4.PullRequestStateOpen), }, }