Skip to content

Commit

Permalink
Don't attempt to reopen a PR after updating a branch if it already ex…
Browse files Browse the repository at this point in the history
…ists

The PR remediation was checking if a PR with the same contents already
exist as a fast and cheap check to avoid triggering the remediation
again. But in case the profile changes or the contents change for
another reason, we want to push to the existing branch to update it, but
don't try to open a PR again as that would fail with 422.

To do that, this patch adds a check if a PR is already open from the
branch we just pushed to and if it is, just returns before opening the
PR again.
  • Loading branch information
jhrozek committed Dec 15, 2023
1 parent a03bc3e commit c3a3e17
Showing 1 changed file with 34 additions and 3 deletions.
37 changes: 34 additions & 3 deletions internal/engine/actions/remediate/pull_request/pull_request.go
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ func (r *Remediator) Do(
var remErr error
switch remAction {
case interfaces.ActionOptOn:
alreadyExists, err := prAlreadyExists(ctx, r.ghCli, repo, magicComment)
alreadyExists, err := prWithContentAlreadyExists(ctx, r.ghCli, repo, magicComment)
if err != nil {
return nil, fmt.Errorf("cannot check if PR already exists: %w", err)
}
Expand Down Expand Up @@ -319,6 +319,19 @@ func (r *Remediator) runGit(
}
zerolog.Ctx(ctx).Debug().Msgf("Push output: %s", b.String())

// if a PR from this branch already exists, don't create a new one
// this handles the case where the content changed (e.g. profile changed)
// but the PR was not closed
prAlreadyExists, err := prFromBranchAlreadyExists(ctx, r.ghCli, pbRepo, branchBaseName(title))
if err != nil {
return fmt.Errorf("cannot check if PR from branch already exists: %w", err)
}

if prAlreadyExists {
zerolog.Ctx(ctx).Info().Msg("PR from branch already exists, won't create a new one")
return nil
}

_, err = r.ghCli.CreatePullRequest(
ctx, pbRepo.GetOwner(), pbRepo.GetName(),
title, body,
Expand Down Expand Up @@ -434,13 +447,12 @@ func createReviewBody(prText, magicComment string) (string, error) {
}

// returns true if an open PR with the magic comment already exists
func prAlreadyExists(
func prWithContentAlreadyExists(
ctx context.Context,
cli provifv1.GitHub,
repo *pb.Repository,
magicComment string,
) (bool, error) {
// TODO(jakub): pagination
openPrs, err := cli.ListPullRequests(ctx, repo.GetOwner(), repo.GetName(), &github.PullRequestListOptions{})
if err != nil {
return false, fmt.Errorf("cannot list pull requests: %w", err)
Expand All @@ -453,3 +465,22 @@ func prAlreadyExists(
}
return false, nil
}

func prFromBranchAlreadyExists(
ctx context.Context,
cli provifv1.GitHub,
repo *pb.Repository,
branchName string,
) (bool, error) {
// TODO(jakub): pagination
opts := &github.PullRequestListOptions{
Head: branchName,
}

openPrs, err := cli.ListPullRequests(ctx, repo.GetOwner(), repo.GetName(), opts)
if err != nil {
return false, fmt.Errorf("cannot list pull requests: %w", err)
}

return len(openPrs) > 0, nil
}

0 comments on commit c3a3e17

Please sign in to comment.