Skip to content

Commit

Permalink
Merge pull request #1 from digitalocean/lost_commits
Browse files Browse the repository at this point in the history 
Version selection refactor, logging, schema previews, and more
  • Loading branch information
Christopher Mancini authored Aug 27, 2019
2 parents 94d3f6b + 9f1c080 commit 68e1b79
Show file tree
Hide file tree
Showing 31 changed files with 2,236 additions and 1,015 deletions.
2 changes: 1 addition & 1 deletion Dockerfile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ ADD . /go/src/github.com/telia-oss/github-pr-resource
WORKDIR /go/src/github.com/telia-oss/github-pr-resource
RUN go get -u -v github.com/go-task/task/cmd/task && task build

FROM alpine:3.8 as resource
FROM alpine:3.10 as resource
COPY --from=builder /go/src/github.com/telia-oss/github-pr-resource/build /opt/resource
RUN apk add --update --no-cache \
git \
Expand Down
70 changes: 52 additions & 18 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,20 @@ Make sure to check out [#migrating](#migrating) to learn more.

## Source Configuration

| Parameter | Required | Example | Description |
|-------------------------|----------|----------------------------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `repository` | Yes | `itsdalmo/test-repository` | The repository to target. |
| `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). |
| `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. |
| `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. |
| `base_branch` | No | `master` | Name of a branch. The pipeline will only trigger on pull requests against the specified branch. |
| Parameter | Required | Example | Description |
|-------------------------|----------|----------------------------------|--------------|
| `repository` | Yes | `itsdalmo/test-repository` | The repository to target |
| `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) |
| `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 |
| `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. |
| `base_branch` | No | `master` | Name of a branch. The pipeline will only trigger on pull requests against the specified branch |
| `preview_schema` | No | `true` | if enabled, an `Accept: application/vnd.github.starfire-preview+json` header will be appended to each request to enable preview schema's that are hidden behind a feature flag on GitHub |

Notes:
- If `v3_endpoint` is set, `v4_endpoint` must also be set (and the other way around).
Expand All @@ -45,13 +46,46 @@ Notes:
Produces new versions for all commits (after the last version) ordered by the committed date.
A version is represented as follows:

- `pr`: The pull request number.
- `commit`: The commit SHA.
- `committed`: Timestamp of when the commit was committed. Used to filter subsequent checks.
- `pr`: The pull request number
- `commit`: The commit SHA
- `updated`: Timestamp of when the pull request was last updated at the time of the check

If several commits are pushed to a given PR at the same time, the last commit will be the new version.
If several commits are pushed to a given PR at the same time, the PR with the latest updated at will be the newest version.

#### search

The GraphQL search for pull requests uses the `Search` endpoint and follows the following pattern:

`is:pr is:open repo:%s/%s updated:>%s sort:updated`

Which means that we want to search for only OPEN PULL REQUESTS that have been UPDATED since the latest `updated` timestamp of the last check. To test this query, you can simply use the search box in the navigation of github.com.

Then, we use the [PullRequestTimelineItemsConnection](https://developer.github.com/v4/object/pullrequesttimelineitemsconnection/) to fetch all commits / events on the PRs timeline since the latest `updated` timestamp of the last check. This allows us to iterate over the pull requests and filter them as is covered in the next section.

#### filters

There are many ways by which this resource filters pull requests and each filter is a function in the `filter.go` file within the `pullrequest` package. This makes it very easy to test the functionality of each filter. There are two types:

* negative filters which when return true, the pull request should be excluded (skipped) from versions
* positive filters which when return true, the pull request should be included from versions

Current negative filters:

* `pullrequest.SkipCI` which will exclude PRs containing `[skip ci|ci skip]` in the PR Title / Message
* `pullrequest.BaseBranch` which will exclude PRs where the base branch (e.g. `master`) does not match the source configuration
* `pullrequest.Fork` which will exclude PRs from forks when `disable_forks` is configured true

Current positive filters:
* `pullrequest.Created` which will include PRs with `Created == Updated` OR `Created > HeadRef.Commited | Authored | Pushed`
* `pullrequest.BaseRefChanged` which will include PRs where a [BaseRefChanged](https://developer.github.com/v4/object/baserefchangedevent/) occurred
* `pullrequest.BaseRefForcePushed` which will include PRs where a [BaseRefForcePushed](https://developer.github.com/v4/object/baserefforcepushedevent) occurred
* `pullrequest.HeadRefForcePushed` which will include PRs where a [HeadRefForcePushed](https://developer.github.com/v4/object/headrefforcepushedevent) occurred
* `pullrequest.Reopened` which will include PRs where a [BaseRefChanged](https://developer.github.com/v4/object/reopenedevent) occurred
* `pullrequest.BuildCI` which will include PRs with a new comment containing `[build ci|ci build]`
* `pullrequest.NewCommits` which will include PRs with a new commit since the last `updated` timestamp of the last check

**Note on webhooks:**

This resource does not implement any caching, so it should work well with webhooks (should be subscribed to `push` and `pull_request` events).
One thing to keep in mind however, is that pull requests that are opened from a fork and commits to said fork will not
generate notifications over the webhook. So if you have a repository with little traffic and expect pull requests from forks,
Expand All @@ -77,7 +111,7 @@ requested version and the metadata emitted by `get` are available to your tasks
- `.git/resource/changed_files` (if enabled by `list_changed_files`)

The information in `metadata.json` is also available as individual files in the `.git/resource` directory, e.g. the `base_sha`
is available as `.git/resource/base_sha`. For a complete list of available (individual) metadata files, please check the code
is available as `.git/resource/base_sha`. For a complete list of available (individual) metadata files, please check the code
[here](https://github.com/telia-oss/github-pr-resource/blob/master/in.go#L66).

When specifying `skip_download` the pull request volume mounted to subsequent tasks will be empty, which is a problem
Expand Down
9 changes: 6 additions & 3 deletions Taskfile.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,12 @@ tasks:
desc: Run test suite
deps: [generate]
cmds:
- gofmt -s -l -w .
- go vet -v ./...
- go test -race -v ./...
- |
{{ if ne (env "SKIP_TEST") "true" }}
gofmt -s -l -w .
go vet -v ./...
go test -race -v -cover ./...
{{ end }}
e2e:
desc: Run E2E test suite
Expand Down
171 changes: 65 additions & 106 deletions check.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,154 +2,113 @@ package resource

import (
"fmt"
"path/filepath"
"regexp"
"log"
"sort"
"strings"
"time"

"github.com/telia-oss/github-pr-resource/pullrequest"
)

func findPulls(since time.Time, gh Github) ([]pullrequest.PullRequest, error) {
if since.IsZero() {
return gh.GetLatestOpenPullRequest()
}
return gh.ListOpenPullRequests(since)
}

// Check (business logic)
func Check(request CheckRequest, manager Github) (CheckResponse, error) {
var response CheckResponse

pulls, err := manager.ListOpenPullRequests()
pulls, err := findPulls(request.Version.UpdatedDate, manager)
if err != nil {
return nil, fmt.Errorf("failed to get last commits: %s", err)
}

disableSkipCI := request.Source.DisableCISkip
paths := request.Source.Paths
iPaths := request.Source.IgnorePaths

Loop:
for _, p := range pulls {
// [ci skip]/[skip ci] in Pull request title
if !disableSkipCI && ContainsSkipCI(p.Title) {
continue
}
// [ci skip]/[skip ci] in Commit message
if !disableSkipCI && ContainsSkipCI(p.Tip.Message) {
continue
}
// Filter pull request if the BaseBranch does not match the one specified in source
if request.Source.BaseBranch != "" && p.PullRequestObject.BaseRefName != request.Source.BaseBranch {
continue
}
// Filter out commits that are too old.
if !p.Tip.CommittedDate.Time.After(request.Version.CommittedDate) {
continue
}
log.Println("total pulls found:", len(pulls))

if request.Source.DisableForks && p.IsCrossRepository {
for _, p := range pulls {
log.Printf("evaluate pull: %+v\n", p)
if !newVersion(request, p) {
log.Println("no new version found")
continue
}

// Fetch files once if paths/ignore_paths are specified.
var files []string

if len(request.Source.Paths) > 0 || len(request.Source.IgnorePaths) > 0 {
files, err = manager.ListModifiedFiles(p.Number)
if len(paths)+len(iPaths) > 0 {
log.Println("pattern/s configured")
p.Files, err = pullRequestFiles(p.Number, manager)
if err != nil {
return nil, fmt.Errorf("failed to list modified files: %s", err)
return nil, err
}
}

// Skip version if no files match the specified paths.
if len(request.Source.Paths) > 0 {
var wanted []string
for _, pattern := range request.Source.Paths {
w, err := FilterPath(files, pattern)
if err != nil {
return nil, fmt.Errorf("path match failed: %s", err)
}
wanted = append(wanted, w...)
}
if len(wanted) == 0 {
continue Loop
log.Println("paths configured:", paths)
log.Println("ignore paths configured:", iPaths)
log.Println("changed files found:", p.Files)

switch {
case pullrequest.Patterns(paths)(p) && !pullrequest.Files(paths, false)(p):
log.Println("paths excluded pull")
continue
case !pullrequest.Patterns(paths)(p) && pullrequest.Patterns(iPaths)(p) && pullrequest.Files(iPaths, true)(p):
log.Println("ignore paths excluded pull")
continue
}
}

// Skip version if all files are ignored.
if len(request.Source.IgnorePaths) > 0 {
wanted := files
for _, pattern := range request.Source.IgnorePaths {
wanted, err = FilterIgnorePath(wanted, pattern)
if err != nil {
return nil, fmt.Errorf("ignore path match failed: %s", err)
}
}
if len(wanted) == 0 {
continue Loop
}
}
response = append(response, NewVersion(p))
}

// Sort the commits by date
sort.Sort(response)

// If there are no new but an old version = return the old
if len(response) == 0 && request.Version.PR != "" {
if len(response) == 0 && request.Version.PR != 0 {
log.Println("no new versions, use old")
response = append(response, request.Version)
}

// If there are new versions and no previous = return just the latest
if len(response) != 0 && request.Version.PR == "" {
if len(response) != 0 && request.Version.PR == 0 {
response = CheckResponse{response[len(response)-1]}
}
return response, nil
}

// ContainsSkipCI returns true if a string contains [ci skip] or [skip ci].
func ContainsSkipCI(s string) bool {
re := regexp.MustCompile("(?i)\\[(ci skip|skip ci)\\]")
return re.MatchString(s)
}

// FilterIgnorePath ...
func FilterIgnorePath(files []string, pattern string) ([]string, error) {
var out []string
for _, file := range files {
match, err := filepath.Match(pattern, file)
if err != nil {
return nil, err
}
if !match && !IsInsidePath(pattern, file) {
out = append(out, file)
}
}
return out, nil
}
log.Println("version count in response:", len(response))
log.Println("versions:", response)

// FilterPath ...
func FilterPath(files []string, pattern string) ([]string, error) {
var out []string
for _, file := range files {
match, err := filepath.Match(pattern, file)
if err != nil {
return nil, err
}
if match || IsInsidePath(pattern, file) {
out = append(out, file)
}
}
return out, nil
return response, nil
}

// IsInsidePath checks whether the child path is inside the parent path.
//
// /foo/bar is inside /foo, but /foobar is not inside /foo.
// /foo is inside /foo, but /foo is not inside /foo/
func IsInsidePath(parent, child string) bool {
if parent == child {
func newVersion(r CheckRequest, p pullrequest.PullRequest) bool {
switch {
// negative filters
case pullrequest.SkipCI(r.Source.DisableCISkip)(p),
pullrequest.BaseBranch(r.Source.BaseBranch)(p),
pullrequest.Fork(r.Source.DisableForks)(p):
return false
// positive filters
case pullrequest.Created()(p),
pullrequest.BaseRefChanged()(p),
pullrequest.BaseRefForcePushed()(p),
pullrequest.HeadRefForcePushed()(p),
pullrequest.Reopened()(p),
pullrequest.BuildCI()(p),
pullrequest.NewCommits(r.Version.UpdatedDate)(p):
return true
}

// we add a trailing slash so that we only get prefix matches on a
// directory separator
parentWithTrailingSlash := parent
if !strings.HasSuffix(parentWithTrailingSlash, string(filepath.Separator)) {
parentWithTrailingSlash += string(filepath.Separator)
return false
}

func pullRequestFiles(n int, manager Github) ([]string, error) {
files, err := manager.GetChangedFiles(n)
if err != nil {
return nil, fmt.Errorf("failed to list modified files: %s", err)
}

return strings.HasPrefix(child, parentWithTrailingSlash)
return files, nil
}

// CheckRequest ...
Expand All @@ -166,7 +125,7 @@ func (r CheckResponse) Len() int {
}

func (r CheckResponse) Less(i, j int) bool {
return r[j].CommittedDate.After(r[i].CommittedDate)
return r[j].UpdatedDate.After(r[i].UpdatedDate)
}

func (r CheckResponse) Swap(i, j int) {
Expand Down
Loading

0 comments on commit 68e1b79

Please sign in to comment.