From 64c55d515cfcbe36b5f48f0bee9e2c70b44db014 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1mer=20B=C3=A1lint?= Date: Fri, 5 Jun 2020 16:22:31 +0200 Subject: [PATCH] Rebased to latest remote, added branch checking --- .../cloudbees/jenkins/GitHubPushTrigger.java | 57 +++++++++--- .../cloudbees/jenkins/GitHubTriggerEvent.java | 23 ++++- .../DefaultPushGHEventSubscriber.java | 1 + .../jenkins/GitHubPushTriggerTest.java | 91 +++++++++++++++++-- .../DefaultPushGHEventListenerTest.java | 2 + 5 files changed, 148 insertions(+), 26 deletions(-) diff --git a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java index 7cdd9a5e3..943381679 100644 --- a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java +++ b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java @@ -11,9 +11,9 @@ import hudson.model.Item; import hudson.model.Job; import hudson.model.Project; +import hudson.plugins.git.BranchSpec; import hudson.plugins.git.GitSCM; import hudson.plugins.git.extensions.impl.UserExclusion; -import hudson.scm.SCM; import hudson.triggers.SCMTrigger; import hudson.triggers.Trigger; import hudson.triggers.TriggerDescriptor; @@ -60,6 +60,7 @@ import java.util.Set; import java.util.concurrent.Executors; import java.util.concurrent.ThreadFactory; +import java.util.regex.Matcher; import static org.apache.commons.lang3.StringUtils.isEmpty; import static org.apache.commons.lang3.Validate.notNull; @@ -105,26 +106,30 @@ public void onPost(final GitHubTriggerEvent event) { if (Objects.isNull(job)) { return; // nothing to do } - if (useGitExcludedUsers) { - Set lowercaseExcludedUsers = new HashSet<>(); - if (job instanceof AbstractProject) { - SCM scm = ((AbstractProject) job).getScm(); - if (scm instanceof GitSCM) { - UserExclusion exclusions = ((GitSCM) scm).getExtensions().get(UserExclusion.class); + if (job instanceof AbstractProject && (((AbstractProject) job).getScm()) instanceof GitSCM) { + GitSCM scm = (GitSCM) (((AbstractProject) job).getScm()); + if (!branchMatchesGitBranchToBeBuilt(scm, event.getRef())) { + return; + } + + if (useGitExcludedUsers) { + Set lowercaseExcludedUsers = new HashSet<>(); + if (job instanceof AbstractProject) { + UserExclusion exclusions = scm.getExtensions().get(UserExclusion.class); if (exclusions != null) { for (String userName: exclusions.getExcludedUsersNormalized()) { lowercaseExcludedUsers.add(userName.toLowerCase()); } } } - } - String lowercaseTriggeredByUser = null; - if (event.getTriggeredByUser() != null) { - lowercaseTriggeredByUser = event.getTriggeredByUser().toLowerCase(); - } - if (lowercaseExcludedUsers != null && lowercaseExcludedUsers.contains(lowercaseTriggeredByUser)) { - return; // user is excluded from triggering build + String lowercaseTriggeredByUser = null; + if (event.getTriggeredByUser() != null) { + lowercaseTriggeredByUser = event.getTriggeredByUser().toLowerCase(); + } + if (lowercaseExcludedUsers != null && lowercaseExcludedUsers.contains(lowercaseTriggeredByUser)) { + return; // user is excluded from triggering build + } } } @@ -197,6 +202,30 @@ public void run() { }); } + private boolean branchMatchesGitBranchToBeBuilt(GitSCM scm, String ref) { + List< BranchSpec > branches = scm.getBranches(); + for (BranchSpec branch: branches) { + if (!branch.matches(ref)) { + // code block copied from GitSCM plugin's GitSCM.compareRemoteRevisionWithImpl() + // convert head `refs/(heads|tags|whatever)/branch` into shortcut notation `remote/branch` + String name; + Matcher matcher = GitSCM.GIT_REF.matcher(ref); + if (matcher.matches()) { + name = "origin" + ref.substring(matcher.group(1).length()); + } else { + name = "origin" + "/" + ref; + } + + if (!branch.matches(name)) { + continue; + } + + return true; + } + } + return false; + } + /** * Returns the file that records the last/current polling activity. */ diff --git a/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java b/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java index 364631c9e..2fc6964cc 100644 --- a/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java +++ b/src/main/java/com/cloudbees/jenkins/GitHubTriggerEvent.java @@ -22,10 +22,13 @@ public class GitHubTriggerEvent { */ private final String triggeredByUser; - private GitHubTriggerEvent(long timestamp, String origin, String triggeredByUser) { + private final String ref; + + private GitHubTriggerEvent(long timestamp, String origin, String triggeredByUser, String ref) { this.timestamp = timestamp; this.origin = origin; this.triggeredByUser = triggeredByUser; + this.ref = ref; } public static Builder create() { @@ -44,6 +47,10 @@ public String getTriggeredByUser() { return triggeredByUser; } + public String getRef() { + return ref; + } + @Override public boolean equals(Object o) { if (this == o) { @@ -61,6 +68,9 @@ public boolean equals(Object o) { if (origin != null ? !origin.equals(that.origin) : that.origin != null) { return false; } + if (ref != null ? !ref.equals(that.ref) : that.ref != null) { + return false; + } return triggeredByUser != null ? triggeredByUser.equals(that.triggeredByUser) : that.triggeredByUser == null; } @@ -69,6 +79,7 @@ public int hashCode() { int result = (int) (timestamp ^ (timestamp >>> 32)); result = 31 * result + (origin != null ? origin.hashCode() : 0); result = 31 * result + (triggeredByUser != null ? triggeredByUser.hashCode() : 0); + result = 31 * result + (ref != null ? ref.hashCode() : 0); return result; } @@ -78,6 +89,7 @@ public String toString() { + "timestamp=" + timestamp + ", origin='" + origin + '\'' + ", triggeredByUser='" + triggeredByUser + '\'' + + ", ref='" + ref + '\'' + '}'; } @@ -88,6 +100,7 @@ public static class Builder { private long timestamp; private String origin; private String triggeredByUser; + private String ref; private Builder() { timestamp = System.currentTimeMillis(); @@ -108,8 +121,13 @@ public Builder withTriggeredByUser(String triggeredByUser) { return this; } + public Builder withRef(String ref) { + this.ref = ref; + return this; + } + public GitHubTriggerEvent build() { - return new GitHubTriggerEvent(timestamp, origin, triggeredByUser); + return new GitHubTriggerEvent(timestamp, origin, triggeredByUser, ref); } @Override @@ -118,6 +136,7 @@ public String toString() { + "timestamp=" + timestamp + ", origin='" + origin + '\'' + ", triggeredByUser='" + triggeredByUser + '\'' + + ", ref='" + ref + '\'' + '}'; } } diff --git a/src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java b/src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java index 7568af0e9..74f0dde31 100644 --- a/src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java +++ b/src/main/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventSubscriber.java @@ -112,6 +112,7 @@ public void run() { .withTimestamp(event.getTimestamp()) .withOrigin(event.getOrigin()) .withTriggeredByUser(pusherName) + .withRef(push.getRef()) .build() ); } else { diff --git a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java index c7223d2a1..70cbf3206 100644 --- a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java +++ b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java @@ -7,7 +7,9 @@ import java.io.IOException; import java.lang.reflect.Field; +import java.util.Collections; import java.util.HashMap; +import java.util.List; import java.util.concurrent.TimeUnit; import javax.inject.Inject; @@ -26,9 +28,13 @@ import org.mockito.Mockito; import com.cloudbees.jenkins.GitHubPushTrigger.DescriptorImpl; +import com.google.common.collect.Lists; import hudson.model.FreeStyleProject; +import hudson.plugins.git.BranchSpec; import hudson.plugins.git.GitSCM; +import hudson.plugins.git.SubmoduleConfig; +import hudson.plugins.git.extensions.GitSCMExtension; import hudson.plugins.git.extensions.impl.UserExclusion; import hudson.plugins.git.util.Build; import hudson.plugins.git.util.BuildData; @@ -118,6 +124,16 @@ private SequentialExecutionQueue addSpyToQueueField() { return spiedQueue; } + private GitSCM createGitSCM(String repository) { + return new GitSCM(GitSCM.createRepoList(repository, null), + Lists.newArrayList(new BranchSpec("*/master")), + false, + Collections.emptyList(), + null, + null, + Collections.emptyList()); + } + @Test public void shouldSkipBuildIfExclusionEnabledWithMatchingUser() throws IOException { SequentialExecutionQueue spiedQueue = addSpyToQueueField(); @@ -128,10 +144,10 @@ public void shouldSkipBuildIfExclusionEnabledWithMatchingUser() throws IOExcepti trigger.setUseGitExcludedUsers(true); trigger.start(project, false); project.addTrigger(trigger); - GitSCM scm = new GitSCM("https://localhost/dummy.git"); + GitSCM scm = createGitSCM("https://localhost/dummy.git"); UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + matchingUserName + System.lineSeparator() + - "somethingElse" + System.lineSeparator()); + "somethingElse" + System.lineSeparator()); scm.getExtensions().add(userExclusion); project.setScm(scm); @@ -139,6 +155,7 @@ public void shouldSkipBuildIfExclusionEnabledWithMatchingUser() throws IOExcepti .withTimestamp(System.currentTimeMillis()) .withOrigin("origin") .withTriggeredByUser(matchingUserName) + .withRef("refs/heads/master") .build(); trigger.onPost(event); @@ -155,10 +172,10 @@ public void shouldSkipBuildIfExclusionEnabledWithMatchingUserCaseInsensitive() t trigger.setUseGitExcludedUsers(true); trigger.start(project, false); project.addTrigger(trigger); - GitSCM scm = new GitSCM("https://localhost/dummy.git"); + GitSCM scm = createGitSCM("https://localhost/dummy.git"); UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + matchingUserName.toUpperCase() + System.lineSeparator() + - "somethingElse" + System.lineSeparator()); + "somethingElse" + System.lineSeparator()); scm.getExtensions().add(userExclusion); project.setScm(scm); @@ -166,6 +183,7 @@ public void shouldSkipBuildIfExclusionEnabledWithMatchingUserCaseInsensitive() t .withTimestamp(System.currentTimeMillis()) .withOrigin("origin") .withTriggeredByUser(matchingUserName) + .withRef("refs/heads/master") .build(); trigger.onPost(event); @@ -181,10 +199,10 @@ public void shouldTriggerBuildIfExclusionEnabledWithNonMatchingUser() throws IOE trigger.setUseGitExcludedUsers(true); trigger.start(project, false); project.addTrigger(trigger); - GitSCM scm = new GitSCM("https://localhost/dummy.git"); + GitSCM scm = createGitSCM("https://localhost/dummy.git"); UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + "nonMatchingUserName" + System.lineSeparator() + - "somethingElse" + System.lineSeparator()); + "somethingElse" + System.lineSeparator()); scm.getExtensions().add(userExclusion); project.setScm(scm); @@ -192,6 +210,7 @@ public void shouldTriggerBuildIfExclusionEnabledWithNonMatchingUser() throws IOE .withTimestamp(System.currentTimeMillis()) .withOrigin("origin") .withTriggeredByUser("userName") + .withRef("refs/heads/master") .build(); trigger.onPost(event); @@ -208,10 +227,10 @@ public void shouldTriggerBuildIfExclusionDisabledWithMatchingUser() throws IOExc trigger.setUseGitExcludedUsers(false); trigger.start(project, false); project.addTrigger(trigger); - GitSCM scm = new GitSCM("https://localhost/dummy.git"); + GitSCM scm = createGitSCM("https://localhost/dummy.git"); UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + matchingUserName + System.lineSeparator() + - "somethingElse" + System.lineSeparator()); + "somethingElse" + System.lineSeparator()); scm.getExtensions().add(userExclusion); project.setScm(scm); @@ -219,6 +238,7 @@ public void shouldTriggerBuildIfExclusionDisabledWithMatchingUser() throws IOExc .withTimestamp(System.currentTimeMillis()) .withOrigin("origin") .withTriggeredByUser(matchingUserName) + .withRef("refs/heads/master") .build(); trigger.onPost(event); @@ -234,10 +254,10 @@ public void shouldTriggerBuildIfExclusionDisabledWithNonMatchingUser() throws IO trigger.setUseGitExcludedUsers(false); trigger.start(project, false); project.addTrigger(trigger); - GitSCM scm = new GitSCM("https://localhost/dummy.git"); + GitSCM scm = createGitSCM("https://localhost/dummy.git"); UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + "nonMatchingUserName" + System.lineSeparator() + - "somethingElse" + System.lineSeparator()); + "somethingElse" + System.lineSeparator()); scm.getExtensions().add(userExclusion); project.setScm(scm); @@ -245,9 +265,60 @@ public void shouldTriggerBuildIfExclusionDisabledWithNonMatchingUser() throws IO .withTimestamp(System.currentTimeMillis()) .withOrigin("origin") .withTriggeredByUser("userName") + .withRef("refs/heads/master") + .build(); + trigger.onPost(event); + + verify(spiedQueue).execute(Mockito.any(Runnable.class)); + } + + @Test + public void shouldTriggerBuildIfBranchNameMatches() throws IOException { + SequentialExecutionQueue spiedQueue = addSpyToQueueField(); + + FreeStyleProject project = jRule.createFreeStyleProject(); + GitHubPushTrigger trigger = new GitHubPushTrigger(); + trigger.setUseGitExcludedUsers(true); + trigger.start(project, false); + project.addTrigger(trigger); + GitSCM scm = createGitSCM("https://localhost/dummy.git"); + List< BranchSpec > branches = scm.getBranches(); + branches.clear(); + branches.add(new BranchSpec("*/master")); + branches.add(new BranchSpec("*/develop")); + project.setScm(scm); + + GitHubTriggerEvent event = GitHubTriggerEvent.create() + .withTimestamp(System.currentTimeMillis()) + .withOrigin("origin") + .withTriggeredByUser("userName") + .withRef("refs/heads/master") .build(); trigger.onPost(event); verify(spiedQueue).execute(Mockito.any(Runnable.class)); } + + @Test + public void shouldSkipBuildIfBranchNameDoesntMatch() throws IOException { + SequentialExecutionQueue spiedQueue = addSpyToQueueField(); + + FreeStyleProject project = jRule.createFreeStyleProject(); + GitHubPushTrigger trigger = new GitHubPushTrigger(); + trigger.setUseGitExcludedUsers(true); + trigger.start(project, false); + project.addTrigger(trigger); + GitSCM scm = createGitSCM("https://localhost/dummy.git"); + project.setScm(scm); + + GitHubTriggerEvent event = GitHubTriggerEvent.create() + .withTimestamp(System.currentTimeMillis()) + .withOrigin("origin") + .withTriggeredByUser("userName") + .withRef("refs/heads/featureBranch") + .build(); + trigger.onPost(event); + + verify(spiedQueue, times(0)).execute(Mockito.any(Runnable.class)); + } } diff --git a/src/test/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventListenerTest.java b/src/test/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventListenerTest.java index 78851d578..2e6a2511c 100644 --- a/src/test/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventListenerTest.java +++ b/src/test/java/org/jenkinsci/plugins/github/webhook/subscriber/DefaultPushGHEventListenerTest.java @@ -62,6 +62,7 @@ public void shouldParsePushPayload() throws Exception { .withTimestamp(subscriberEvent.getTimestamp()) .withOrigin("shouldParsePushPayload") .withTriggeredByUser(TRIGGERED_BY_USER_FROM_RESOURCE) + .withRef("refs/heads/master") .build() )); } @@ -85,6 +86,7 @@ public void shouldReceivePushHookOnWorkflow() throws Exception { .withTimestamp(subscriberEvent.getTimestamp()) .withOrigin("shouldReceivePushHookOnWorkflow") .withTriggeredByUser(TRIGGERED_BY_USER_FROM_RESOURCE) + .withRef("refs/heads/master") .build() )); }