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 support for pending merge refs #208

Merged
merged 26 commits into from
Mar 17, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
98 changes: 98 additions & 0 deletions gitclone/bitriseapi/pr_merge_ref.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
package bitriseapi

import (
"encoding/json"
"errors"
"fmt"
"net/http"
"time"

"github.com/bitrise-io/go-utils/retry"
"github.com/bitrise-io/go-utils/v2/log"
"github.com/hashicorp/go-retryablehttp"
)

var logger = log.NewLogger()
var ErrServiceAccountAuth = errors.New(`authentication error: Bitrise can't connect to the git server to check the freshness of the merge ref.
Check the Service Credential User in App Settings > Integrations`)

type MergeRefChecker interface {
IsMergeRefUpToDate(ref string) (bool, error)
}

func NewMergeRefChecker(buildURL string, apiToken string, client *retryablehttp.Client) MergeRefChecker {
return apiMergeRefChecker{
buildURL: buildURL,
apiToken: apiToken,
client: client,
}
}

type apiMergeRefChecker struct {
buildURL string
apiToken string
client *retryablehttp.Client
}

type mergeRefResponse struct {
Status string `json:"status"`
}

type mergeRefFetcher func(attempt uint) (mergeRefResponse, error)

func (c apiMergeRefChecker) IsMergeRefUpToDate(ref string) (bool, error) {
return doPoll(c.fetchMergeRef, time.Second*2)
}

func doPoll(fetcher mergeRefFetcher, retryWaitTime time.Duration) (bool, error) {
isUpToDate := false
pollAction := func(attempt uint) (err error, shouldAbort bool) {
resp, err := fetcher(attempt)
if err != nil {
godrei marked this conversation as resolved.
Show resolved Hide resolved
logger.Warnf("Error while checking merge ref: %s", err)
godrei marked this conversation as resolved.
Show resolved Hide resolved
logger.Warnf("Retrying request...")
return err, false
}
switch resp.Status {
case "up-to-date":
isUpToDate = true
return nil, true
case "auth_error": // TODO
return ErrServiceAccountAuth, true
case "pending":
logger.Warnf("Attempt %d: not up-to-date yet", attempt)
return fmt.Errorf("pending"), false
default:
return fmt.Errorf("unknown status: %s", resp.Status), false
}
}

err := retry.Times(5).Wait(retryWaitTime).TryWithAbort(pollAction)
godrei marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return false, err
}
return isUpToDate, nil
}

func (c apiMergeRefChecker) fetchMergeRef(attempt uint) (mergeRefResponse, error) {
url := fmt.Sprintf("%s/pull_request_merge_ref_status", c.buildURL)
req, err := retryablehttp.NewRequest(http.MethodGet, url, nil)
if err != nil {
return mergeRefResponse{}, err
}
req.Header.Set("HTTP_BUILD_API_TOKEN", c.apiToken)

resp, err := c.client.Do(req)
if err != nil {
return mergeRefResponse{}, err
}
defer resp.Body.Close()

var response mergeRefResponse
err = json.NewDecoder(resp.Body).Decode(&response)
if err != nil {
return mergeRefResponse{}, err
}

return mergeRefResponse{}, nil
}
83 changes: 83 additions & 0 deletions gitclone/bitriseapi/pr_merge_ref_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package bitriseapi

import (
"fmt"
"testing"
"time"
)

func Test_isMergeRefUpToDate(t *testing.T) {
tests := []struct {
name string
fetcher mergeRefFetcher
want bool
wantErr bool
}{
{
name: "Up-to-date for the first check",
fetcher: func(uint) (mergeRefResponse, error) {
return mergeRefResponse{Status: "up-to-date"}, nil
},
want: true,
},
{
name: "Pending for first check, up-to-date for second",
fetcher: func(attempt uint) (mergeRefResponse, error) {
if attempt == 0 {
return mergeRefResponse{Status: "pending"}, nil
}
return mergeRefResponse{Status: "up-to-date"}, nil
},
want: true,
},
{
name: "Unrecoverable error for first check",
fetcher: func(attempt uint) (mergeRefResponse, error) {
return mergeRefResponse{Status: "auth_error"}, nil
},
want: false,
wantErr: true,
},
{
name: "Network error for first check, success for second",
fetcher: func(attempt uint) (mergeRefResponse, error) {
if attempt == 0 {
return mergeRefResponse{}, fmt.Errorf("mocked network error")
}
return mergeRefResponse{Status: "up-to-date"}, nil
},
want: true,
},
{
name: "Exceeding retries, result is still pending",
fetcher: func(attempt uint) (mergeRefResponse, error) {
return mergeRefResponse{Status: "pending"}, nil
},
want: false,
wantErr: true,
},
{
name: "Unknown status for first check, success for second",
fetcher: func(attempt uint) (mergeRefResponse, error) {
if attempt == 0 {
return mergeRefResponse{Status: "unknown"}, nil
}
return mergeRefResponse{Status: "up-to-date"}, nil
},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
retryWaitTime := time.Duration(0)
got, err := doPoll(tt.fetcher, retryWaitTime)
if (err != nil) != tt.wantErr {
t.Errorf("doPoll() error = %v, wantErr %v", err, tt.wantErr)
return
}
if got != tt.want {
t.Errorf("doPoll() got = %v, want %v", got, tt.want)
}
})
}
}
44 changes: 44 additions & 0 deletions gitclone/bitriseapi/pr_patch.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
package bitriseapi

import (
"fmt"
"net/http"
"net/url"
"path/filepath"

"github.com/bitrise-io/go-steputils/input"
"github.com/bitrise-io/go-utils/filedownloader"
)

type PatchSource interface {
// GetPRPatch fetches the git patch file of the PR (if available) and returns its local file path
GetPRPatch() (string, error)
}

func NewPatchSource(buildURL, apiToken string) PatchSource {
return apiPatchSource{
buildURL: buildURL,
apiToken: apiToken,
}
}

type apiPatchSource struct {
buildURL string
apiToken string
}

func (s apiPatchSource) GetPRPatch() (string, error) {
u, err := url.Parse(s.buildURL)
if err != nil {
return "", fmt.Errorf("could not parse build URL: %v", err)
}

if u.Scheme == "file" {
return filepath.Join(u.Path, "diff.txt"), nil
}

// TODO
diffURL := fmt.Sprintf("%s/diff.txt?api_token=%s", s.buildURL, s.apiToken)
fileProvider := input.NewFileProvider(filedownloader.New(http.DefaultClient))
return fileProvider.LocalPath(diffURL)
}
65 changes: 35 additions & 30 deletions gitclone/checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

"github.com/bitrise-io/go-utils/command/git"
"github.com/bitrise-io/go-utils/log"
"github.com/bitrise-steplib/steps-git-clone/gitclone/bitriseapi"
)

// CheckoutMethod is the checkout method used
Expand Down Expand Up @@ -88,8 +89,8 @@ type checkoutStrategy interface {
// | headBranch | | | | | X | |
// |=========================================================================|

func selectCheckoutMethod(cfg Config, patch patchSource) (CheckoutMethod, string) {
isPR := cfg.PRSourceRepositoryURL != "" || cfg.PRDestBranch != "" || cfg.PRMergeBranch != ""
func selectCheckoutMethod(cfg Config, patchSource bitriseapi.PatchSource, mergeRefChecker bitriseapi.MergeRefChecker) (CheckoutMethod, string) {
isPR := cfg.PRSourceRepositoryURL != "" || cfg.PRDestBranch != "" || cfg.PRMergeRef != ""
if !isPR {
if cfg.Commit != "" {
return CheckoutCommitMethod, ""
Expand Down Expand Up @@ -130,31 +131,48 @@ func selectCheckoutMethod(cfg Config, patch patchSource) (CheckoutMethod, string

// Fallback (Bitbucket only): it's a PR from a fork we can't access, so we fetch the PR patch file through
// the API and apply the diff manually
if cfg.BuildURL != "" {
patchFile := getPatchFile(patch, cfg.BuildURL, cfg.BuildAPIToken)
if patchFile != "" {
log.Infof("Merging Pull Request despite the option to disable merging, as it is opened from a private fork.")
return CheckoutPRDiffFileMethod, patchFile
}
patchFile, err := patchSource.GetPRPatch()
if err != nil {
log.Warnf("Patch file unavailable for PR: %v", err)
}
if err == nil && patchFile != "" {
log.Infof("Merging Pull Request despite the option to disable merging, as it is opened from a private fork.")
return CheckoutPRDiffFileMethod, patchFile
}

log.Warnf(privateForkAuthWarning)
return CheckoutForkCommitMethod, ""
}

// PR: check out the merge result (merging the PR branch into the destination branch)
if cfg.PRMergeBranch != "" {
// Merge ref (such as refs/pull/2/merge) is available in the destination repo, we can access that
// even if the PR source is a private repo
if cfg.PRMergeRef != "" {
// Merge ref (such as refs/pull/2/merge) is available in the destination repo.
// This field is only defined if the merge ref is known to be up-to-date and is safe to checkout.
// See `PRUnverifiedMergeRef` below for handling a potentially outdated merge ref.
// Note about PRs from private forks: we can access this merge ref of the destination repo even if the source
// repo is private and not accessible to Bitrise
return CheckoutPRMergeBranchMethod, ""
}

// Fallback (Bitbucket only): fetch the PR patch file through the API and apply the diff manually
if cfg.BuildURL != "" {
patchFile := getPatchFile(patch, cfg.BuildURL, cfg.BuildAPIToken)
if patchFile != "" {
return CheckoutPRDiffFileMethod, patchFile
// Merge ref is available, but it might be outdated, we need to check its status and potentially trigger an update
// before we can use it for checkout
if cfg.PRUnverifiedMergeRef != "" {
upToDate, err := mergeRefChecker.IsMergeRefUpToDate(cfg.PRUnverifiedMergeRef)
if err != nil {
log.Warnf("Failed to check PR merge ref freshness: %s", err)
}
if upToDate {
return CheckoutPRMergeBranchMethod, ""
}
}

// Fallback (Bitbucket only): fetch the PR patch file through the API and apply the diff manually
patchFile, err := patchSource.GetPRPatch()
if err != nil {
log.Warnf("Patch file unavailable for PR: %s", err)
}
if err == nil && patchFile != "" {
return CheckoutPRDiffFileMethod, patchFile
}

// As a last resort, fetch target + PR branches and do a manual merge
Expand All @@ -163,19 +181,6 @@ func selectCheckoutMethod(cfg Config, patch patchSource) (CheckoutMethod, string
return CheckoutPRManualMergeMethod, ""
}

func getPatchFile(patch patchSource, buildURL, buildAPIToken string) string {
if patch != nil {
patchFile, err := patch.getDiffPath(buildURL, buildAPIToken)
if err != nil {
log.Warnf("Diff file unavailable: %v", err)
} else {
return patchFile
}
}

return ""
}

func createCheckoutStrategy(checkoutMethod CheckoutMethod, cfg Config, patchFile string) (checkoutStrategy, error) {
switch checkoutMethod {
case CheckoutNoneMethod:
Expand Down Expand Up @@ -222,7 +227,7 @@ func createCheckoutStrategy(checkoutMethod CheckoutMethod, cfg Config, patchFile
}
case CheckoutPRMergeBranchMethod:
{
params, err := NewPRMergeRefParams(cfg.PRMergeBranch, cfg.PRHeadBranch)
params, err := NewPRMergeRefParams(cfg.PRMergeRef, cfg.PRHeadBranch)
if err != nil {
return nil, err
}
Expand Down
26 changes: 0 additions & 26 deletions gitclone/checkout_method_pr_auto_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,9 @@ package gitclone

import (
"fmt"
"net/http"
"net/url"
"path/filepath"
"strings"

"github.com/bitrise-io/go-steputils/input"
"github.com/bitrise-io/go-utils/command/git"
"github.com/bitrise-io/go-utils/filedownloader"
"github.com/bitrise-io/go-utils/log"
)

Expand Down Expand Up @@ -67,24 +62,3 @@ func (c checkoutPRDiffFile) do(gitCmd git.Git, fetchOptions fetchOptions, fallba
func (c checkoutPRDiffFile) getBuildTriggerRef() string {
return ""
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moved to bitriseapi package

type patchSource interface {
getDiffPath(buildURL, apiToken string) (string, error)
}

type defaultPatchSource struct{}

func (defaultPatchSource) getDiffPath(buildURL, apiToken string) (string, error) {
url, err := url.Parse(buildURL)
if err != nil {
return "", fmt.Errorf("could not parse diff file URL: %v", err)
}

if url.Scheme == "file" {
return filepath.Join(url.Path, "diff.txt"), nil
}

diffURL := fmt.Sprintf("%s/diff.txt?api_token=%s", buildURL, apiToken)
fileProvider := input.NewFileProvider(filedownloader.New(http.DefaultClient))
return fileProvider.LocalPath(diffURL)
}
Loading