diff --git a/pkg/notes/notes.go b/pkg/notes/notes.go index b0782563076..fc78defa485 100644 --- a/pkg/notes/notes.go +++ b/pkg/notes/notes.go @@ -28,6 +28,7 @@ import ( "net/http" "net/url" "regexp" + "slices" "sort" "strconv" "strings" @@ -50,6 +51,7 @@ import ( var ( errNoPRIDFoundInCommitMessage = errors.New("no PR IDs found in the commit message") + errNoOriginPRIDFoundInPR = errors.New("no origin PR IDs found in the PR") errNoPRFoundForCommitSHA = errors.New("no PR found for this commit") apiSleepTime int64 = 60 ) @@ -1034,55 +1036,91 @@ func (g *Gatherer) prsForCommitFromMessage(commitMessage string) (prs []*gogithu if err != nil { return nil, err } - var res *gogithub.PullRequest - var resp *gogithub.Response - for _, pr := range prsNum { + for _, prNum := range prsNum { // Given the PR number that we've now converted to an integer, get the PR from // the API - for { - res, resp, err = g.client.GetPullRequest(g.context, g.options.GithubOrg, g.options.GithubRepo, pr) - if err != nil { - if !canWaitAndRetry(resp, err) { - return nil, err - } - } else { - break + pr, err := g.getPr(prNum) + if err != nil { + return prs, err + } + prs = append(prs, pr) + + originPrNum, origPrErr := originPrNumFromPr(pr) + if origPrErr == nil && slices.Index(prsNum, originPrNum) == -1 { + originPr, err := g.getPr(originPrNum) + if err == nil { + prs = append(prs, originPr) } } - prs = append(prs, res) } return prs, err } -func prsNumForCommitFromMessage(commitMessage string) (prs []int, err error) { +func (g *Gatherer) getPr(prNum int) (*gogithub.PullRequest, error) { + for { + res, resp, err := g.client.GetPullRequest(g.context, g.options.GithubOrg, g.options.GithubRepo, prNum) + if err != nil { + if !canWaitAndRetry(resp, err) { + return nil, err + } + } else { + return res, err + } + } +} + +var ( // Thankfully k8s-merge-robot commits the PR number consistently. If this ever // stops being true, this definitely won't work anymore. - regex := regexp.MustCompile(`Merge pull request #(?P\d+)`) - pr := prForRegex(regex, commitMessage) - if pr != 0 { + regexMergedPR = regexp.MustCompile(`Merge pull request #(?P\d+)`) + + // If the PR was squash merged, the regexp is different. + regexSquashPR = regexp.MustCompile(`\(#(?P\d+)\)`) + + // The branch name created by "hack/cherry_pick_pull.sh". + regexHackBranch = regexp.MustCompile(`automated-cherry-pick-of-#(?P\d+)`) + + // The branch name created by k8s-infra-cherrypick-robot. + regexProwBranch = regexp.MustCompile(`cherry-pick-(?P\d+)-to`) +) + +func prsNumForCommitFromMessage(commitMessage string) (prs []int, err error) { + if pr := prForRegex(regexMergedPR, commitMessage); pr != 0 { prs = append(prs, pr) } - regex = regexp.MustCompile(`automated-cherry-pick-of-#(?P\d+)`) - pr = prForRegex(regex, commitMessage) - if pr != 0 { + if pr := prForRegex(regexSquashPR, commitMessage); pr != 0 { prs = append(prs, pr) } - // If the PR was squash merged, the regexp is different - regex = regexp.MustCompile(`\(#(?P\d+)\)`) - pr = prForRegex(regex, commitMessage) - if pr != 0 { + if pr := prForRegex(regexHackBranch, commitMessage); pr != 0 { prs = append(prs, pr) } - if prs == nil { + if len(prs) == 0 { return nil, errNoPRIDFoundInCommitMessage + } else { + return prs, nil } +} - return prs, nil +func originPrNumFromPr(pr *gogithub.PullRequest) (origPRNum int, err error) { + if pr == nil || pr.Head == nil || pr.Head.Label == nil { + return 0, errNoOriginPRIDFoundInPR + } + origPRNum = prForRegex(regexHackBranch, *pr.Head.Label) + if origPRNum != 0 { + return origPRNum, nil + } + + origPRNum = prForRegex(regexProwBranch, *pr.Head.Label) + if origPRNum != 0 { + return origPRNum, nil + } + + return 0, errNoOriginPRIDFoundInPR } func prForRegex(regex *regexp.Regexp, commitMessage string) int { diff --git a/pkg/notes/notes_gatherer_test.go b/pkg/notes/notes_gatherer_test.go index 99fd8486ca4..d06abafdb2a 100644 --- a/pkg/notes/notes_gatherer_test.go +++ b/pkg/notes/notes_gatherer_test.go @@ -311,6 +311,77 @@ func TestGatherNotes(t *testing.T) { }, expectedGetPullRequestCallCount: 6, }, + + "when the PR is a cherry pick": { + commitList: []*github.RepositoryCommit{ + repoCommit("125", "Merge a prow cherry-pick (#125)"), + repoCommit("126", "Merge hack cherry-pick (#126)"), + repoCommit("127", "Merge hack cherry-pick (#127)"), + }, + getPullRequestStubber: func(t *testing.T) getPullRequestStub { + seenPRs := newIntsRecorder(122, 123, 124, 125, 126, 127) + prsMap := map[int]*github.PullRequest{ + 122: { + Number: intPtr(122), + Body: strPtr("122\n```release-note\nfrom 122\n```\n"), + }, + 123: { + Number: intPtr(123), + Body: strPtr("123\n```release-note\nfrom 123\n```\n"), + }, + 124: { + Number: intPtr(124), + Body: strPtr("124\n```release-note\nfrom 124\n```\n"), + }, + 125: { + Number: intPtr(125), + Body: strPtr("No release note"), + Head: &github.PullRequestBranch{ + Label: strPtr("k8s-infra-cherrypick-robot:cherry-pick-122-to-release-0.x"), + }, + }, + 126: { + Number: intPtr(126), + Body: strPtr("Empty release note\n```release-note\n\n```\n"), + Head: &github.PullRequestBranch{ + Label: strPtr("fork:automated-cherry-pick-of-#123-upstream-release-0.x"), + }, + }, + 127: { + Number: intPtr(127), + Body: strPtr("127\n```release-note\nfrom 127\n```\n"), + Head: &github.PullRequestBranch{ + Label: strPtr("fork:automated-cherry-pick-of-#124-upstream-release-0.x"), + }, + }, + } + return func(_ context.Context, org, repo string, prID int) (*github.PullRequest, *github.Response, error) { + checkOrgRepo(t, org, repo) + if err := seenPRs.Mark(prID); err != nil { + t.Errorf("In GetPullRequest: %v", err) + } + return prsMap[prID], nil, nil + } + }, + resultsChecker: func(t *testing.T, results []*Result) { + expectMap := map[string]int{ + "125": 122, + "126": 123, + "127": 127, + } + + for _, result := range results { + expected, found := expectMap[*result.commit.SHA] + if !found { + t.Errorf("Unexpected SHA %s", *result.commit.SHA) + } + if expected != *result.pullRequest.Number { + t.Errorf("Expecting %d got %d for SHA %s", expected, *result.pullRequest.Number, *result.commit.SHA) + } + } + }, + expectedGetPullRequestCallCount: 6, + }, "when GetPullRequest(...) returns an error": { commitList: []*github.RepositoryCommit{repoCommit("some-sha", "some #123 thing")}, listPullRequestsWithCommitStubber: func(t *testing.T) listPullRequestsWithCommitStub {