diff --git a/README.md b/README.md index e3cfa70..b4496df 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,11 @@ References can use `#123`, `owner/repo#123`, or `https://github.com/owner/repo/i but must identify issues in the PR's repository. Linking works for open and closed issues without requiring backport labels. +### Add release label to issues when a PR is merged + +When a PR is merged into `main` or a `release/*` branch, the issues it closes (from the PR description or the merge commit message) +get the next `release/` label and lose the `backport/` label of the target branch. + ## Testing changes to the application To test changes to the application you need: diff --git a/src/main/java/org/keycloak/gh/bot/AddReleaseLabelOnMerge.java b/src/main/java/org/keycloak/gh/bot/AddReleaseLabelOnMerge.java new file mode 100644 index 0000000..9d38a99 --- /dev/null +++ b/src/main/java/org/keycloak/gh/bot/AddReleaseLabelOnMerge.java @@ -0,0 +1,91 @@ +package org.keycloak.gh.bot; + +import io.quarkiverse.githubapp.event.PullRequest; +import org.jboss.logging.Logger; +import org.keycloak.gh.bot.utils.Labels; +import org.keycloak.gh.bot.utils.ReleaseLabels; +import org.kohsuke.github.GHEventPayload; +import org.kohsuke.github.GHFileNotFoundException; +import org.kohsuke.github.GHIssue; +import org.kohsuke.github.GHPullRequest; +import org.kohsuke.github.GHRepository; +import org.kohsuke.github.GHTag; + +import java.io.IOException; +import java.util.List; +import java.util.Set; + +/** + * Replaces the labeller GitHub workflow of keycloak/keycloak, which relied on 'pull_request_target'. + * When a PR is merged, the issues it closes get the 'release/...' label and lose the 'backport/...' label for the target branch. + */ +public class AddReleaseLabelOnMerge { + + private static final Logger logger = Logger.getLogger(AddReleaseLabelOnMerge.class); + + void onClose(@PullRequest.Closed GHEventPayload.PullRequest payload) throws IOException { + GHPullRequest pullRequest = payload.getPullRequest(); + if (!pullRequest.isMerged()) { + return; + } + + GHRepository repository = payload.getRepository(); + String base = pullRequest.getBase().getRef(); + + // Only fetch what is needed for the target branch + List branches = base.equals("main") ? repository.getBranches().keySet().stream().toList() : List.of(); + List tags = base.startsWith("release/") ? repository.listTags().toList().stream().map(GHTag::getName).toList() : List.of(); + + ReleaseLabels.Result labels = ReleaseLabels.forBaseBranch(base, branches, tags); + if (labels == null) { + logger.infof("PR %s merged into %s: no release label applicable", pullRequest.getNumber(), base); + return; + } + + Set issueNumbers = ReleaseLabels.findIssues(repository.getFullName(), pullRequest.getBody(), null); + String mergeCommitSha = pullRequest.getMergeCommitSha(); + if (issueNumbers.isEmpty() && mergeCommitSha != null) { + String commitMessage = repository.getCommit(mergeCommitSha).getCommitShortInfo().getMessage(); + issueNumbers = ReleaseLabels.findIssues(repository.getFullName(), pullRequest.getBody(), commitMessage); + } + if (issueNumbers.isEmpty()) { + logger.infof("PR %s merged into %s: no linked issues found", pullRequest.getNumber(), base); + return; + } + + createLabelIfMissing(repository, labels.releaseLabel()); + + for (int issueNumber : issueNumbers) { + GHIssue issue; + try { + issue = repository.getIssue(issueNumber); + } catch (GHFileNotFoundException e) { + logger.errorf(e, "PR %s merged into %s: referenced issue %s not found", pullRequest.getNumber(), base, issueNumber); + continue; + } + if (issue.isPullRequest()) { + continue; + } + issue.addLabels(labels.releaseLabel()); + if (issue.getLabels().stream().anyMatch(l -> l.getName().equals(labels.backportLabel()))) { + issue.removeLabel(labels.backportLabel()); + } + logger.infof("PR %s merged into %s: labelled issue %s with %s", pullRequest.getNumber(), base, issueNumber, labels.releaseLabel()); + } + } + + private void createLabelIfMissing(GHRepository repository, String label) throws IOException { + if (Labels.hasLabel(repository, label)) { + return; + } + try { + repository.createLabel(label, "0E8A16"); + } catch (IOException e) { + // A concurrent merge for the same release might have created the label in the meantime + if (!Labels.hasLabel(repository, label)) { + throw e; + } + } + } + +} diff --git a/src/main/java/org/keycloak/gh/bot/utils/Labels.java b/src/main/java/org/keycloak/gh/bot/utils/Labels.java index eb451c7..bf5d40c 100644 --- a/src/main/java/org/keycloak/gh/bot/utils/Labels.java +++ b/src/main/java/org/keycloak/gh/bot/utils/Labels.java @@ -46,7 +46,7 @@ public static void removeLabel(GHIssue issue, String label) throws IOException { } } - private static boolean hasLabel(GHRepository repository, String label) throws IOException { + public static boolean hasLabel(GHRepository repository, String label) throws IOException { PagedIterator itr = repository.listLabels().withPageSize(100).iterator(); while (itr.hasNext()) { if (itr.next().getName().equals(label)) { diff --git a/src/main/java/org/keycloak/gh/bot/utils/ReleaseLabels.java b/src/main/java/org/keycloak/gh/bot/utils/ReleaseLabels.java new file mode 100644 index 0000000..700ea1b --- /dev/null +++ b/src/main/java/org/keycloak/gh/bot/utils/ReleaseLabels.java @@ -0,0 +1,65 @@ +package org.keycloak.gh.bot.utils; + +import java.util.Collection; +import java.util.Comparator; +import java.util.Set; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +public class ReleaseLabels { + + public record Result(String releaseLabel, String backportLabel) { + } + + private static final Pattern RELEASE_BRANCH = Pattern.compile("release/(\\d+)\\.(\\d+)"); + + /** + * @return the labels for a PR merged into the given branch, or null if the branch is neither main nor a release branch + */ + public static Result forBaseBranch(String base, Collection branches, Collection tags) { + if (base.equals("main")) { + int[] last = branches.stream() + .map(RELEASE_BRANCH::matcher) + .filter(Matcher::matches) + .map(m -> new int[]{Integer.parseInt(m.group(1)), Integer.parseInt(m.group(2))}) + .max(Comparator.comparingInt(v -> v[0]).thenComparingInt(v -> v[1])) + .orElse(null); + if (last == null) { + return null; + } + String label = "release/" + last[0] + "." + (last[1] + 1) + ".0"; + // There will be no more minor release on the 26.x line; 26.9.0 is actually released as the major bump 27.0.0. + if (label.equals("release/26.9.0")) { + label = "release/27.0.0"; + } + return new Result(label, "backport/main"); + } + + Matcher m = RELEASE_BRANCH.matcher(base); + if (m.matches()) { + String majorMinor = m.group(1) + "." + m.group(2); + Pattern tagPattern = Pattern.compile(Pattern.quote(majorMinor) + "\\.(\\d+)"); + int lastMicro = tags.stream() + .map(tagPattern::matcher) + .filter(Matcher::matches) + .mapToInt(t -> Integer.parseInt(t.group(1))) + .max() + .orElse(-1); + return new Result("release/" + majorMinor + "." + (lastMicro + 1), "backport/" + majorMinor); + } + + return null; + } + + /** + * Finds issues closed by the PR description, falling back to the merge commit message if the description has none. + */ + public static Set findIssues(String repository, String prBody, String commitMessage) { + Set issues = PullRequestParser.closingIssues(prBody, repository); + if (issues.isEmpty()) { + issues = PullRequestParser.closingIssues(commitMessage, repository); + } + return issues; + } + +} diff --git a/src/test/java/org/keycloak/gh/bot/AddReleaseLabelOnMergeTest.java b/src/test/java/org/keycloak/gh/bot/AddReleaseLabelOnMergeTest.java new file mode 100644 index 0000000..69189da --- /dev/null +++ b/src/test/java/org/keycloak/gh/bot/AddReleaseLabelOnMergeTest.java @@ -0,0 +1,234 @@ +package org.keycloak.gh.bot; + +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.kohsuke.github.GHBranch; +import org.kohsuke.github.GHCommit; +import org.kohsuke.github.GHCommitPointer; +import org.kohsuke.github.GHEventPayload; +import org.kohsuke.github.GHFileNotFoundException; +import org.kohsuke.github.GHIssue; +import org.kohsuke.github.GHLabel; +import org.kohsuke.github.GHPullRequest; +import org.kohsuke.github.GHRepository; +import org.kohsuke.github.GHTag; +import org.kohsuke.github.PagedIterable; +import org.kohsuke.github.PagedIterator; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Iterator; +import java.util.List; +import java.util.Map; + +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.mockito.ArgumentMatchers.anyInt; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; +import static org.mockito.Mockito.when; + +class AddReleaseLabelOnMergeTest { + + private final AddReleaseLabelOnMerge handler = new AddReleaseLabelOnMerge(); + private final GHEventPayload.PullRequest payload = mock(GHEventPayload.PullRequest.class); + private final GHRepository repository = mock(GHRepository.class); + private final GHPullRequest pr = mock(GHPullRequest.class); + private final GHCommitPointer base = mock(GHCommitPointer.class); + private final GHIssue issue = mock(GHIssue.class); + + @BeforeEach + void setUp() throws Exception { + when(payload.getRepository()).thenReturn(repository); + when(repository.getFullName()).thenReturn("keycloak/keycloak"); + when(payload.getPullRequest()).thenReturn(pr); + when(pr.isMerged()).thenReturn(true); + when(pr.getNumber()).thenReturn(1); + when(pr.getBody()).thenReturn("Closes #123"); + when(pr.getBase()).thenReturn(base); + when(base.getRef()).thenReturn("main"); + when(repository.getBranches()).thenReturn(Map.of( + "main", mock(GHBranch.class), "release/26.7", mock(GHBranch.class), "release/26.8", mock(GHBranch.class))); + when(repository.getIssue(123)).thenReturn(issue); + issueLabels("backport/main"); + repositoryLabels("backport/main"); + } + + @Test + void mainCreatesLabelAndMovesIssue() throws Exception { + handler.onClose(payload); + + verify(repository).createLabel("release/27.0.0", "0E8A16"); + verify(issue).addLabels("release/27.0.0"); + verify(issue).removeLabel("backport/main"); + verify(repository, never()).listTags(); + } + + @Test + void releaseBranchUsesNextMicroAndKeepsOtherBackportLabels() throws Exception { + when(base.getRef()).thenReturn("release/26.8"); + repositoryTags("26.8.0", "26.8.2", "26.7.5", "nightly"); + issueLabels("backport/26.8", "backport/main"); + + handler.onClose(payload); + + verify(repository).createLabel("release/26.8.3", "0E8A16"); + verify(issue).addLabels("release/26.8.3"); + verify(issue).removeLabel("backport/26.8"); + verify(issue, never()).removeLabel("backport/main"); + verify(repository, never()).getBranches(); + } + + @Test + void existingLabelIsNotCreatedAgain() throws Exception { + repositoryLabels("release/27.0.0"); + + handler.onClose(payload); + + verify(repository, never()).createLabel(anyString(), anyString()); + verify(issue).addLabels("release/27.0.0"); + } + + @Test + void labelCreatedConcurrentlyIsAccepted() throws Exception { + when(repository.createLabel("release/27.0.0", "0E8A16")).thenAnswer(i -> { + repositoryLabels("release/27.0.0"); + throw new IOException("already exists"); + }); + + handler.onClose(payload); + + verify(issue).addLabels("release/27.0.0"); + } + + @Test + void labelCreationFailureIsPropagated() throws Exception { + when(repository.createLabel("release/27.0.0", "0E8A16")).thenThrow(new IOException("forbidden")); + + assertThrows(IOException.class, () -> handler.onClose(payload)); + + verify(issue, never()).addLabels(anyString()); + } + + @Test + void issueWithoutBackportLabelOnlyGetsReleaseLabel() throws Exception { + issueLabels(); + + handler.onClose(payload); + + verify(issue).addLabels("release/27.0.0"); + verify(issue, never()).removeLabel(anyString()); + } + + @Test + void missingIssuesAndPullRequestsAreSkipped() throws Exception { + GHIssue otherPr = mock(GHIssue.class); + when(otherPr.isPullRequest()).thenReturn(true); + when(repository.getIssue(999)).thenThrow(new GHFileNotFoundException("not found")); + when(repository.getIssue(456)).thenReturn(otherPr); + when(pr.getBody()).thenReturn("Closes #999\nCloses #456\nFixes #123"); + + handler.onClose(payload); + + verify(issue).addLabels("release/27.0.0"); + verify(otherPr, never()).addLabels(anyString()); + verify(otherPr, never()).removeLabel(anyString()); + } + + @Test + void commitMessageIsUsedWhenDescriptionHasNoIssue() throws Exception { + GHCommit commit = mock(GHCommit.class); + GHCommit.ShortInfo info = mock(GHCommit.ShortInfo.class); + when(pr.getBody()).thenReturn("No closing keyword here"); + when(pr.getMergeCommitSha()).thenReturn("abc"); + when(repository.getCommit("abc")).thenReturn(commit); + when(commit.getCommitShortInfo()).thenReturn(info); + when(info.getMessage()).thenReturn("Some change\n\nCloses #123"); + + handler.onClose(payload); + + verify(issue).addLabels("release/27.0.0"); + } + + @Test + void commitIsNotFetchedWhenDescriptionHasIssue() throws Exception { + when(pr.getBody()).thenReturn("Closes #123"); + when(pr.getMergeCommitSha()).thenReturn("abc"); + + handler.onClose(payload); + + verify(issue).addLabels("release/27.0.0"); + verify(repository, never()).getCommit(anyString()); + } + + @Test + void unmergedPRsAreIgnored() throws Exception { + when(pr.isMerged()).thenReturn(false); + + handler.onClose(payload); + + verifyNoInteractions(repository); + } + + @Test + void unrelatedBranchesAreIgnored() throws Exception { + when(base.getRef()).thenReturn("feature/x"); + + handler.onClose(payload); + + verify(repository, never()).getBranches(); + verify(repository, never()).listTags(); + verify(repository, never()).getIssue(123); + } + + @Test + void noLinkedIssuesDoesNothing() throws Exception { + when(pr.getBody()).thenReturn("Just a change"); + + handler.onClose(payload); + + verify(repository, never()).createLabel(anyString(), anyString()); + verify(repository, never()).getIssue(123); + } + + private GHLabel label(String name) { + GHLabel label = mock(GHLabel.class); + when(label.getName()).thenReturn(name); + return label; + } + + @SuppressWarnings("unchecked") + private void repositoryLabels(String... names) throws IOException { + List labels = Arrays.stream(names).map(this::label).toList(); + PagedIterable iterable = mock(PagedIterable.class); + when(iterable.withPageSize(anyInt())).thenReturn(iterable); + when(iterable.iterator()).thenAnswer(i -> { + Iterator delegate = labels.iterator(); + PagedIterator iterator = mock(PagedIterator.class); + when(iterator.hasNext()).thenAnswer(x -> delegate.hasNext()); + when(iterator.next()).thenAnswer(x -> delegate.next()); + return iterator; + }); + when(repository.listLabels()).thenReturn(iterable); + } + + private void issueLabels(String... names) { + List labels = Arrays.stream(names).map(this::label).toList(); + when(issue.getLabels()).thenReturn(labels); + } + + @SuppressWarnings("unchecked") + private void repositoryTags(String... names) throws IOException { + List tags = Arrays.stream(names).map(name -> { + GHTag tag = mock(GHTag.class); + when(tag.getName()).thenReturn(name); + return tag; + }).toList(); + PagedIterable iterable = mock(PagedIterable.class); + when(iterable.toList()).thenReturn(tags); + when(repository.listTags()).thenReturn(iterable); + } + +} diff --git a/src/test/java/org/keycloak/gh/bot/utils/ReleaseLabelsTest.java b/src/test/java/org/keycloak/gh/bot/utils/ReleaseLabelsTest.java new file mode 100644 index 0000000..30013a2 --- /dev/null +++ b/src/test/java/org/keycloak/gh/bot/utils/ReleaseLabelsTest.java @@ -0,0 +1,66 @@ +package org.keycloak.gh.bot.utils; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.List; +import java.util.Set; + +public class ReleaseLabelsTest { + + private static final String REPO = "keycloak/keycloak"; + + @Test + public void mainBranch() { + ReleaseLabels.Result r = ReleaseLabels.forBaseBranch("main", List.of("main", "release/9.9", "release/26.4", "release/26.10", "release/25.0"), List.of()); + Assertions.assertEquals("release/26.11.0", r.releaseLabel()); + Assertions.assertEquals("backport/main", r.backportLabel()); + } + + @Test + public void mainBranchMajorBump() { + ReleaseLabels.Result r = ReleaseLabels.forBaseBranch("main", List.of("main", "release/26.8"), List.of()); + Assertions.assertEquals("release/27.0.0", r.releaseLabel()); + } + + @Test + public void releaseBranch() { + ReleaseLabels.Result r = ReleaseLabels.forBaseBranch("release/26.4", List.of(), List.of("26.4.0", "26.4.9", "26.4.10", "26.40.11", "26.5.3")); + Assertions.assertEquals("release/26.4.11", r.releaseLabel()); + Assertions.assertEquals("backport/26.4", r.backportLabel()); + } + + @Test + public void otherBranch() { + Assertions.assertNull(ReleaseLabels.forBaseBranch("feature/x", List.of("release/26.4"), List.of())); + } + + // Same cases as .github/scripts/pr-find-issues-test.sh in keycloak/keycloak + @Test + public void issueReferenceVariants() { + for (String body : List.of( + "Closes #123", + "Fixes #123", + "Fixes: #123", + "Fixes: keycloak/keycloak#123", + "Fixes https://github.com/keycloak/keycloak/issues/123")) { + Assertions.assertEquals(Set.of(123), ReleaseLabels.findIssues(REPO, body, null), body); + } + } + + @Test + public void issueReferenceEdgeCases() { + Assertions.assertEquals(Set.of(123), ReleaseLabels.findIssues(REPO, "Fix #123", null)); + Assertions.assertEquals(Set.of(), ReleaseLabels.findIssues(REPO, "Discloses #123", null)); + Assertions.assertEquals(Set.of(7), ReleaseLabels.findIssues(REPO, "Closes #2147483648\nCloses #7", null)); + } + + @Test + public void issuesFromBodyOrCommit() { + Assertions.assertEquals(Set.of(12, 34), + ReleaseLabels.findIssues(REPO, "Closes #12\nfixes: https://github.com/keycloak/keycloak/issues/34\nCloses other/repo#5", "Closes #99")); + Assertions.assertEquals(Set.of(99), ReleaseLabels.findIssues(REPO, "nothing", "Closes #99")); + Assertions.assertEquals(Set.of(), ReleaseLabels.findIssues(REPO, null, null)); + } + +}