From acaad735537114ea2c1d4290ce0f3135b27f3af7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1mer=20B=C3=A1lint?= Date: Sat, 16 Nov 2019 20:51:47 +0100 Subject: [PATCH 1/3] Added option to reuse GitSCM's excluded users list --- .../cloudbees/jenkins/GitHubPushTrigger.java | 31 ++++ .../jenkins/GitHubPushTrigger/config.groovy | 6 + .../jenkins/GitHubPushTriggerTest.java | 153 ++++++++++++++++-- 3 files changed, 177 insertions(+), 13 deletions(-) diff --git a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java index 62259c733..7e9b924a1 100644 --- a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java +++ b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java @@ -11,6 +11,9 @@ import hudson.model.Item; import hudson.model.Job; import hudson.model.Project; +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; @@ -23,6 +26,7 @@ import jenkins.scm.api.SCMEvent; import jenkins.triggers.SCMTriggerItem; import jenkins.triggers.SCMTriggerItem.SCMTriggerItems; + import org.apache.commons.jelly.XMLOutput; import org.jenkinsci.plugins.github.GitHubPlugin; import org.jenkinsci.plugins.github.admin.GitHubHookRegisterProblemMonitor; @@ -34,6 +38,7 @@ import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; +import org.kohsuke.stapler.DataBoundSetter; import org.kohsuke.stapler.Stapler; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -65,6 +70,7 @@ * @author Kohsuke Kawaguchi */ public class GitHubPushTrigger extends Trigger> implements GitHubTrigger { + private boolean useGitExcludedUsers; @DataBoundConstructor public GitHubPushTrigger() { @@ -98,6 +104,22 @@ public void onPost(final GitHubTriggerEvent event) { if (Objects.isNull(job)) { return; // nothing to do } + if (useGitExcludedUsers) { + Set excludedUsers = null; + if (job instanceof AbstractProject) { + SCM scm = ((AbstractProject) job).getScm(); + if (scm instanceof GitSCM) { + UserExclusion exclusions = ((GitSCM) scm).getExtensions().get(UserExclusion.class); + if (exclusions != null) { + excludedUsers = exclusions.getExcludedUsersNormalized(); + } + } + } + + if (excludedUsers != null && excludedUsers.contains(event.getTriggeredByUser())) { + return; // user is excluded from triggering build + } + } Job currentJob = notNull(job, "Job can't be null"); @@ -241,6 +263,15 @@ public DescriptorImpl getDescriptor() { return (DescriptorImpl) super.getDescriptor(); } + public boolean isUseGitExcludedUsers() { + return useGitExcludedUsers; + } + + @DataBoundSetter + public void setUseGitExcludedUsers(Boolean useGitExcludedUsers) { + this.useGitExcludedUsers = useGitExcludedUsers != null ? useGitExcludedUsers : false; + } + /** * Action object for {@link Project}. Used to display the polling log. */ diff --git a/src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.groovy b/src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.groovy index c9a140f5c..29e0ff909 100644 --- a/src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.groovy +++ b/src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.groovy @@ -2,6 +2,8 @@ package com.cloudbees.jenkins.GitHubPushTrigger import com.cloudbees.jenkins.GitHubPushTrigger +def f = namespace(lib.FormTagLib); + tr { td(colspan: 4) { div(id: 'gh-hooks-warn') @@ -18,3 +20,7 @@ InlineWarning.setup({ }).start(); """) } + +f.entry() { + f.checkbox(title: _("Use Git excluded user list (\"Polling ignores commits from certain users\")"), field: "useGitExcludedUsers") +} diff --git a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java index 00a529c28..236b46206 100644 --- a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java +++ b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java @@ -1,10 +1,17 @@ package com.cloudbees.jenkins; -import hudson.model.FreeStyleProject; -import hudson.plugins.git.GitSCM; -import hudson.plugins.git.util.Build; -import hudson.plugins.git.util.BuildData; -import hudson.util.FormValidation; +import static com.cloudbees.jenkins.GitHubWebHookFullTest.classpath; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; +import static org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventListenerTest.TRIGGERED_BY_USER_FROM_RESOURCE; + +import java.io.IOException; +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.concurrent.TimeUnit; + +import javax.inject.Inject; + import org.eclipse.jgit.lib.ObjectId; import org.jenkinsci.plugins.github.admin.GitHubHookRegisterProblemMonitor; import org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventListenerTest; @@ -16,16 +23,21 @@ import org.junit.Test; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import org.mockito.Mockito; -import javax.inject.Inject; -import java.io.IOException; -import java.util.HashMap; -import java.util.concurrent.TimeUnit; +import com.cloudbees.jenkins.GitHubPushTrigger.DescriptorImpl; -import static com.cloudbees.jenkins.GitHubWebHookFullTest.classpath; -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; -import static org.jenkinsci.plugins.github.webhook.subscriber.DefaultPushGHEventListenerTest.TRIGGERED_BY_USER_FROM_RESOURCE; +import hudson.model.FreeStyleProject; +import hudson.plugins.git.GitSCM; +import hudson.plugins.git.extensions.impl.UserExclusion; +import hudson.plugins.git.util.Build; +import hudson.plugins.git.util.BuildData; +import hudson.util.FormValidation; +import hudson.util.ReflectionUtils; +import hudson.util.SequentialExecutionQueue; + +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.times; /** * @author lanwen (Merkushev Kirill) @@ -96,4 +108,119 @@ public void shouldReturnOkOnNoAnyProblem() throws Exception { FormValidation validation = descriptor.doCheckHookRegistered(job); assertThat("all ok", validation.kind, is(FormValidation.Kind.OK)); } + + private SequentialExecutionQueue addSpyToQueueField() { + Field queueField = ReflectionUtils.findField(DescriptorImpl.class, "queue"); + ReflectionUtils.makeAccessible(queueField); + SequentialExecutionQueue queue = (SequentialExecutionQueue)ReflectionUtils.getField(queueField, descriptor); + SequentialExecutionQueue spiedQueue = Mockito.spy(queue); + ReflectionUtils.setField(queueField, descriptor, spiedQueue); + return spiedQueue; + } + + @Test + public void shouldSkipBuildIfExclusionEnabledWithMatchingUser() throws IOException { + SequentialExecutionQueue spiedQueue = addSpyToQueueField(); + + String matchingUserName = "userName"; + FreeStyleProject project = jRule.createFreeStyleProject(); + GitHubPushTrigger trigger = new GitHubPushTrigger(); + trigger.setUseGitExcludedUsers(true); + trigger.start(project, false); + project.addTrigger(trigger); + GitSCM scm = new GitSCM("https://localhost/dummy.git"); + UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + + matchingUserName + System.lineSeparator() + + "somethingElse" + System.lineSeparator()); + scm.getExtensions().add(userExclusion); + project.setScm(scm); + + GitHubTriggerEvent event = GitHubTriggerEvent.create() + .withTimestamp(System.currentTimeMillis()) + .withOrigin("origin") + .withTriggeredByUser(matchingUserName) + .build(); + trigger.onPost(event); + + verify(spiedQueue, times(0)).execute(Mockito.any(Runnable.class)); + } + + @Test + public void shouldTriggerBuildIfExclusionEnabledWithNonMatchingUser() 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 = new GitSCM("https://localhost/dummy.git"); + UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + + "nonMatchingUserName" + System.lineSeparator() + + "somethingElse" + System.lineSeparator()); + scm.getExtensions().add(userExclusion); + project.setScm(scm); + + GitHubTriggerEvent event = GitHubTriggerEvent.create() + .withTimestamp(System.currentTimeMillis()) + .withOrigin("origin") + .withTriggeredByUser("userName") + .build(); + trigger.onPost(event); + + verify(spiedQueue).execute(Mockito.any(Runnable.class)); + } + + @Test + public void shouldTriggerBuildIfExclusionDisabledWithMatchingUser() throws IOException { + SequentialExecutionQueue spiedQueue = addSpyToQueueField(); + + String matchingUserName = "userName"; + FreeStyleProject project = jRule.createFreeStyleProject(); + GitHubPushTrigger trigger = new GitHubPushTrigger(); + trigger.setUseGitExcludedUsers(false); + trigger.start(project, false); + project.addTrigger(trigger); + GitSCM scm = new GitSCM("https://localhost/dummy.git"); + UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + + matchingUserName + System.lineSeparator() + + "somethingElse" + System.lineSeparator()); + scm.getExtensions().add(userExclusion); + project.setScm(scm); + + GitHubTriggerEvent event = GitHubTriggerEvent.create() + .withTimestamp(System.currentTimeMillis()) + .withOrigin("origin") + .withTriggeredByUser(matchingUserName) + .build(); + trigger.onPost(event); + + verify(spiedQueue).execute(Mockito.any(Runnable.class)); + } + + @Test + public void shouldTriggerBuildIfExclusionDisabledWithNonMatchingUser() throws IOException { + SequentialExecutionQueue spiedQueue = addSpyToQueueField(); + + FreeStyleProject project = jRule.createFreeStyleProject(); + GitHubPushTrigger trigger = new GitHubPushTrigger(); + trigger.setUseGitExcludedUsers(false); + trigger.start(project, false); + project.addTrigger(trigger); + GitSCM scm = new GitSCM("https://localhost/dummy.git"); + UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + + "nonMatchingUserName" + System.lineSeparator() + + "somethingElse" + System.lineSeparator()); + scm.getExtensions().add(userExclusion); + project.setScm(scm); + + GitHubTriggerEvent event = GitHubTriggerEvent.create() + .withTimestamp(System.currentTimeMillis()) + .withOrigin("origin") + .withTriggeredByUser("userName") + .build(); + trigger.onPost(event); + + verify(spiedQueue).execute(Mockito.any(Runnable.class)); + } } From 72c01ddb7c3237c33c54065db1a7e39731bad7f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?P=C3=A1mer=20B=C3=A1lint?= Date: Wed, 18 Dec 2019 08:23:52 +0100 Subject: [PATCH 2/3] Made comparison case insensitive, pointed this out in the help text Added an extra test case for the case insensitive scenario --- .../cloudbees/jenkins/GitHubPushTrigger.java | 13 ++++++--- .../jenkins/GitHubPushTrigger/config.groovy | 2 +- .../jenkins/GitHubPushTriggerTest.java | 27 +++++++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java index 7e9b924a1..7cdd9a5e3 100644 --- a/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java +++ b/src/main/java/com/cloudbees/jenkins/GitHubPushTrigger.java @@ -54,6 +54,7 @@ import java.util.Collection; import java.util.Collections; import java.util.Date; +import java.util.HashSet; import java.util.List; import java.util.Objects; import java.util.Set; @@ -105,18 +106,24 @@ public void onPost(final GitHubTriggerEvent event) { return; // nothing to do } if (useGitExcludedUsers) { - Set excludedUsers = null; + 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 (exclusions != null) { - excludedUsers = exclusions.getExcludedUsersNormalized(); + for (String userName: exclusions.getExcludedUsersNormalized()) { + lowercaseExcludedUsers.add(userName.toLowerCase()); + } } } } - if (excludedUsers != null && excludedUsers.contains(event.getTriggeredByUser())) { + String lowercaseTriggeredByUser = null; + if (event.getTriggeredByUser() != null) { + lowercaseTriggeredByUser = event.getTriggeredByUser().toLowerCase(); + } + if (lowercaseExcludedUsers != null && lowercaseExcludedUsers.contains(lowercaseTriggeredByUser)) { return; // user is excluded from triggering build } } diff --git a/src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.groovy b/src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.groovy index 29e0ff909..caf3c0589 100644 --- a/src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.groovy +++ b/src/main/resources/com/cloudbees/jenkins/GitHubPushTrigger/config.groovy @@ -22,5 +22,5 @@ InlineWarning.setup({ } f.entry() { - f.checkbox(title: _("Use Git excluded user list (\"Polling ignores commits from certain users\")"), field: "useGitExcludedUsers") + f.checkbox(title: _("Use Git excluded user list (\"Polling ignores commits from certain users\", comparison is case insensitive)"), field: "useGitExcludedUsers") } diff --git a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java index 236b46206..c7223d2a1 100644 --- a/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java +++ b/src/test/java/com/cloudbees/jenkins/GitHubPushTriggerTest.java @@ -145,6 +145,33 @@ public void shouldSkipBuildIfExclusionEnabledWithMatchingUser() throws IOExcepti verify(spiedQueue, times(0)).execute(Mockito.any(Runnable.class)); } + @Test + public void shouldSkipBuildIfExclusionEnabledWithMatchingUserCaseInsensitive() throws IOException { + SequentialExecutionQueue spiedQueue = addSpyToQueueField(); + + String matchingUserName = "userName".toLowerCase(); + FreeStyleProject project = jRule.createFreeStyleProject(); + GitHubPushTrigger trigger = new GitHubPushTrigger(); + trigger.setUseGitExcludedUsers(true); + trigger.start(project, false); + project.addTrigger(trigger); + GitSCM scm = new GitSCM("https://localhost/dummy.git"); + UserExclusion userExclusion = new UserExclusion("something" + System.lineSeparator() + + matchingUserName.toUpperCase() + System.lineSeparator() + + "somethingElse" + System.lineSeparator()); + scm.getExtensions().add(userExclusion); + project.setScm(scm); + + GitHubTriggerEvent event = GitHubTriggerEvent.create() + .withTimestamp(System.currentTimeMillis()) + .withOrigin("origin") + .withTriggeredByUser(matchingUserName) + .build(); + trigger.onPost(event); + + verify(spiedQueue, times(0)).execute(Mockito.any(Runnable.class)); + } + @Test public void shouldTriggerBuildIfExclusionEnabledWithNonMatchingUser() throws IOException { SequentialExecutionQueue spiedQueue = addSpyToQueueField(); 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 3/3] 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() )); }