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.

Sign up for GitHub

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

Version selection refactor, logging, schema previews, and more #1

Merged
merged 1 commit into from
Aug 27, 2019
Merged

Version selection refactor, logging, schema previews, and more #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion Dockerfile
View file
Open in desktop
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
Open in desktop
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
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than include this list, can we just link to some godoc? This type of documentation is a prime example of "a lie waiting to happen" - it's too far from the code to expect anyone to remember to update it when they change the code.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, however, I wanted it documented on the README for now because all of this work is in a feature branch and I am not sure it will ever get merged back upstream. Once we merge it to the master branch, I will replace it to a link to GoDoc.


**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
Open in desktop
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
Open in desktop
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