Backport merged commit SHA instead of individual commits (#313)

Attempts to simplify backport operations by cherry-picking the
merged commit of a PR on the base branch instead of cherry-picking
each individual commit from the PR.

bot/internal/bot/backport.go

@@ -46,7 +46,7 @@ func (b *Bot) Backport(ctx context.Context) error {
 		return trace.BadParameter("automatic backports are only supported for internal contributors")
 	}
 
-	pull, err := b.c.GitHub.GetPullRequestWithCommits(ctx,
+	pull, err := b.c.GitHub.GetPullRequest(ctx,
 		b.c.Environment.Organization,
 		b.c.Environment.Repository,
 		b.c.Environment.Number)
@@ -155,7 +155,7 @@ func (b *Bot) Backport(ctx context.Context) error {
 // BackportLocal executes dry run backport workflow locally. No git commands
 // are actually executed, just printed in the console.
 func (b *Bot) BackportLocal(ctx context.Context, branch string) error {
-	pull, err := b.c.GitHub.GetPullRequestWithCommits(ctx,
+	pull, err := b.c.GitHub.GetPullRequest(ctx,
 		b.c.Environment.Organization,
 		b.c.Environment.Repository,
 		b.c.Environment.Number)
@@ -236,16 +236,15 @@ func (b *Bot) createBackportBranch(ctx context.Context, organization string, rep
 		return trace.Wrap(err)
 	}
 
-	// Cherry-pick all commits from the PR to the backport branch.
-	for _, commit := range pull.Commits {
-		if err := git("cherry-pick", commit); err != nil {
-			// If cherry-pick fails with conflict, abort it, otherwise we
-			// won't be able to switch branch for the next backport.
-			if errAbrt := git("cherry-pick", "--abort"); errAbrt != nil {
-				return trace.NewAggregate(err, errAbrt)
-			}
-			return trace.Wrap(err)
+	// Cherry-pick the _merged_ commit on the base branch instead of
+	// the individual commits from the PR to the backport branch.
+	if err := git("cherry-pick", pull.MergeCommitSHA); err != nil {
+		// If cherry-pick fails with conflict, abort it, otherwise we
+		// won't be able to switch branch for the next backport.
+		if errAbrt := git("cherry-pick", "--abort"); errAbrt != nil {
+			return trace.NewAggregate(err, errAbrt)
 		}
+		return trace.Wrap(err)
 	}
 
 	// Push the backport branch to Github.

bot/internal/bot/bot.go

@@ -50,9 +50,6 @@ type Client interface {
 	// GetPullRequest returns a specific Pull Request.
 	GetPullRequest(ctx context.Context, organization string, repository string, number int) (github.PullRequest, error)
 
-	// GetPullRequestWithCommits returns a specific Pull Request with commits.
-	GetPullRequestWithCommits(ctx context.Context, organization string, repository string, number int) (github.PullRequest, error)
-
 	// CreatePullRequest will create a Pull Request.
 	CreatePullRequest(ctx context.Context, organization string, repository string, title string, head string, base string, body string, draft bool) (int, error)
 

bot/internal/github/github.go

@@ -196,11 +196,14 @@ type PullRequest struct {
 	UnsafeLabels []string
 	// Fork determines if the pull request is from a fork.
 	Fork bool
-	// Commits is a list of commit SHAs for the pull request.
+	// MergeCommitSHA changes depending on the state of the pull request. Before merging a pull request,
+	// the MergeCommitSHA holds the SHA of the test merge commit. After merging a pull request,
+	// the MergeCommitSHA changes depending on how you merged the pull request:
 	//
-	// It is only populated if the pull request was fetched using
-	// GetPullRequestWithCommits method.
-	Commits []string
+	// If merged as a merge commit, MergeCommitSHA represents the SHA of the merge commit.
+	// If merged via a squash, MergeCommitSHA represents the SHA of the squashed commit on the base branch.
+	// If rebased, MergeCommitSHA represents the commit that the base branch was updated to.
+	MergeCommitSHA string
 }
 
 // Branch is a git Branch.
@@ -280,27 +283,6 @@ func (f PullRequestFiles) SourceFiles() (files PullRequestFiles) {
 	return files
 }
 
-// GetPullRequestWithCommits returns the specified pull request with commits.
-func (c *Client) GetPullRequestWithCommits(ctx context.Context, organization string, repository string, number int) (PullRequest, error) {
-	pull, err := c.GetPullRequest(ctx, organization, repository, number)
-	if err != nil {
-		return PullRequest{}, trace.Wrap(err)
-	}
-
-	commits, _, err := c.client.PullRequests.ListCommits(ctx, organization, repository, number, &go_github.ListOptions{})
-	if err != nil {
-		return PullRequest{}, trace.Wrap(err)
-	}
-
-	for _, commit := range commits {
-		if len(commit.Parents) <= 1 { // Skip merge commits.
-			pull.Commits = append(pull.Commits, *commit.SHA)
-		}
-	}
-
-	return pull, nil
-}
-
 // GetPullRequest returns a specific Pull Request.
 func (c *Client) GetPullRequest(ctx context.Context, organization string, repository string, number int) (PullRequest, error) {
 	pull, _, err := c.client.PullRequests.Get(ctx,
@@ -329,10 +311,11 @@ func (c *Client) GetPullRequest(ctx context.Context, organization string, reposi
 			Ref: pull.GetHead().GetRef(),
 			SHA: pull.GetHead().GetSHA(),
 		},
-		UnsafeTitle:  pull.GetTitle(),
-		UnsafeBody:   pull.GetBody(),
-		UnsafeLabels: labels,
-		Fork:         pull.GetHead().GetRepo().GetFork(),
+		UnsafeTitle:    pull.GetTitle(),
+		UnsafeBody:     pull.GetBody(),
+		UnsafeLabels:   labels,
+		Fork:           pull.GetHead().GetRepo().GetFork(),
+		MergeCommitSHA: pull.GetMergeCommitSHA(),
 	}, nil
 }
 
@@ -375,10 +358,11 @@ func (c *Client) ListPullRequests(ctx context.Context, organization string, repo
 					Ref: pull.GetHead().GetRef(),
 					SHA: pull.GetHead().GetSHA(),
 				},
-				UnsafeTitle:  pull.GetTitle(),
-				UnsafeBody:   pull.GetBody(),
-				UnsafeLabels: labels,
-				Fork:         pull.GetHead().GetRepo().GetFork(),
+				UnsafeTitle:    pull.GetTitle(),
+				UnsafeBody:     pull.GetBody(),
+				UnsafeLabels:   labels,
+				Fork:           pull.GetHead().GetRepo().GetFork(),
+				MergeCommitSHA: pull.GetMergeCommitSHA(),
 			})
 		}
 		if resp.NextPage == 0 {