Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support user regex filter for note trigger #923

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> implements MergeReques
private TriggerOpenMergeRequest triggerOpenMergeRequestOnPush;
private boolean triggerOnNoteRequest = true;
private String noteRegex = "";
private String userRegex = "";
private boolean ciSkip = true;
private boolean skipWorkInProgressMergeRequest;
private boolean setBuildDescription = true;
Expand Down Expand Up @@ -118,7 +119,7 @@ public class GitLabPushTrigger extends Trigger<Job<?, ?>> implements MergeReques
@Deprecated
@GeneratePojoBuilder(intoPackage = "*.builder.generated", withFactoryMethod = "*")
public GitLabPushTrigger(boolean triggerOnPush, boolean triggerOnMergeRequest, boolean triggerOnAcceptedMergeRequest, boolean triggerOnClosedMergeRequest,
TriggerOpenMergeRequest triggerOpenMergeRequestOnPush, boolean triggerOnNoteRequest, String noteRegex,
TriggerOpenMergeRequest triggerOpenMergeRequestOnPush, boolean triggerOnNoteRequest, String noteRegex, String userRegex,
boolean skipWorkInProgressMergeRequest, boolean ciSkip,
boolean setBuildDescription, boolean addNoteOnMergeRequest, boolean addCiMessage, boolean addVoteOnMergeRequest,
boolean acceptMergeRequestOnSuccess, BranchFilterType branchFilterType,
Expand All @@ -131,6 +132,7 @@ public GitLabPushTrigger(boolean triggerOnPush, boolean triggerOnMergeRequest, b
this.triggerOnClosedMergeRequest = triggerOnClosedMergeRequest;
this.triggerOnNoteRequest = triggerOnNoteRequest;
this.noteRegex = noteRegex;
this.userRegex = userRegex;
this.triggerOpenMergeRequestOnPush = triggerOpenMergeRequestOnPush;
this.triggerOnPipelineEvent = triggerOnPipelineEvent;
this.ciSkip = ciSkip;
Expand Down Expand Up @@ -243,6 +245,10 @@ public String getNoteRegex() {
return this.noteRegex == null ? "" : this.noteRegex;
}

public String getUserRegex() {
return this.userRegex == null ? "" : this.userRegex;
}

@Override
public TriggerOpenMergeRequest getTriggerOpenMergeRequestOnPush() {
return triggerOpenMergeRequestOnPush;
Expand Down Expand Up @@ -338,6 +344,11 @@ public void setNoteRegex(String noteRegex) {
this.noteRegex = noteRegex;
}

@DataBoundSetter
public void setUserRegex(String userRegex) {
this.userRegex = userRegex;
}

@DataBoundSetter
public void setCiSkip(boolean ciSkip) {
this.ciSkip = ciSkip;
Expand Down Expand Up @@ -483,7 +494,7 @@ public void onPost(final PipelineHook hook) {

private void initializeTriggerHandler() {
mergeRequestHookTriggerHandler = newMergeRequestHookTriggerHandler(this);
noteHookTriggerHandler = newNoteHookTriggerHandler(triggerOnNoteRequest, noteRegex);
noteHookTriggerHandler = newNoteHookTriggerHandler(triggerOnNoteRequest, noteRegex, userRegex);
pushHookTriggerHandler = newPushHookTriggerHandler(triggerOnPush, triggerOpenMergeRequestOnPush, skipWorkInProgressMergeRequest);
pipelineTriggerHandler = newPipelineHookTriggerHandler(triggerOnPipelineEvent);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@ public final class NoteHookTriggerHandlerFactory {

private NoteHookTriggerHandlerFactory() {}

public static NoteHookTriggerHandler newNoteHookTriggerHandler(boolean triggerOnNoteRequest, String noteRegex) {
public static NoteHookTriggerHandler newNoteHookTriggerHandler(boolean triggerOnNoteRequest, String noteRegex, String userRegex) {
if (triggerOnNoteRequest) {
return new NoteHookTriggerHandlerImpl(noteRegex);
return new NoteHookTriggerHandlerImpl(noteRegex, userRegex);
} else {
return new NopNoteHookTriggerHandler();
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
package com.dabsquared.gitlabjenkins.trigger.handler.note;

import com.dabsquared.gitlabjenkins.cause.CauseData;
import com.dabsquared.gitlabjenkins.connection.GitLabConnectionProperty;
import com.dabsquared.gitlabjenkins.gitlab.api.GitLabClient;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.NoteHook;
import com.dabsquared.gitlabjenkins.publisher.GitLabMessagePublisher;
import com.dabsquared.gitlabjenkins.trigger.exception.NoRevisionToBuildException;
import com.dabsquared.gitlabjenkins.trigger.filter.BranchFilter;
import com.dabsquared.gitlabjenkins.trigger.filter.MergeRequestLabelFilter;
Expand All @@ -26,13 +29,16 @@ class NoteHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<NoteHook>

private final String noteRegex;

NoteHookTriggerHandlerImpl(String noteRegex) {
private final String userRegex;

NoteHookTriggerHandlerImpl(String noteRegex, String userRegex) {
this.noteRegex = noteRegex;
this.userRegex = userRegex;
}

@Override
public void handle(Job<?, ?> job, NoteHook hook, boolean ciSkip, BranchFilter branchFilter, MergeRequestLabelFilter mergeRequestLabelFilter) {
if (isValidTriggerPhrase(hook.getObjectAttributes().getNote())) {
if (isValidTriggerPhrase(hook.getObjectAttributes().getNote()) && isAllowedUser(hook.getUser().getUsername())) {
super.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
}
}
Expand Down Expand Up @@ -67,8 +73,8 @@ protected CauseData retrieveCauseData(NoteHook hook) {
.withTargetProjectId(hook.getMergeRequest().getTargetProjectId())
.withBranch(hook.getMergeRequest().getSourceBranch())
.withSourceBranch(hook.getMergeRequest().getSourceBranch())
.withUserName(hook.getMergeRequest().getLastCommit().getAuthor().getName())
.withUserEmail(hook.getMergeRequest().getLastCommit().getAuthor().getEmail())
.withUserName(hook.getUser().getUsername())
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not backward compatible. People that are relying on this being the user name and email of committer will see bugs.

Might be useful to create new field that deals with the hook user.

.withUserEmail(hook.getUser().getEmail())
.withSourceRepoHomepage(hook.getMergeRequest().getSource().getHomepage())
.withSourceRepoName(hook.getMergeRequest().getSource().getName())
.withSourceNamespace(hook.getMergeRequest().getSource().getNamespace())
Expand All @@ -85,7 +91,7 @@ protected CauseData retrieveCauseData(NoteHook hook) {
.withTargetNamespace(hook.getMergeRequest().getTarget().getNamespace())
.withTargetRepoSshUrl(hook.getMergeRequest().getTarget().getSshUrl())
.withTargetRepoHttpUrl(hook.getMergeRequest().getTarget().getHttpUrl())
.withTriggeredByUser(hook.getMergeRequest().getLastCommit().getAuthor().getName())
.withTriggeredByUser(hook.getUser().getName())
.withLastCommit(hook.getMergeRequest().getLastCommit().getId())
.withTargetProjectUrl(hook.getMergeRequest().getTarget().getWebUrl())
.withTriggerPhrase(hook.getObjectAttributes().getNote())
Expand Down Expand Up @@ -124,4 +130,13 @@ private boolean isValidTriggerPhrase(String note) {
final Pattern pattern = Pattern.compile(this.noteRegex);
return pattern.matcher(note).matches();
}

private boolean isAllowedUser(String user) {
if (StringUtils.isEmpty(this.userRegex)) {
return true;
}

final Pattern pattern = Pattern.compile(this.userRegex);
return pattern.matcher(user).matches();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,9 @@
<f:entry title="Comment (regex) for triggering a build" help="/plugin/gitlab-plugin/help/help-noteRegex.html">
<f:textbox field="noteRegex" default="Jenkins please retry a build"/>
</f:entry>
<f:entry title="User (regex) for triggering a build" help="/plugin/gitlab-plugin/help/help-userRegex.html">
<f:textbox field="userRegex" default=""/>
</f:entry>
</table>
</f:entry>
<f:advanced>
Expand Down
5 changes: 5 additions & 0 deletions src/main/webapp/help/help-userRegex.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
<div>
<div>
<p>When filled, the comment user in the merge request matching this regular expression will trigger a build.</p>
</div>
</div>
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package com.dabsquared.gitlabjenkins.trigger.handler.note;

import com.dabsquared.gitlabjenkins.gitlab.hook.model.State;
import com.dabsquared.gitlabjenkins.gitlab.hook.model.User;
import com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterFactory;
import com.dabsquared.gitlabjenkins.trigger.filter.BranchFilterType;
import hudson.Launcher;
Expand Down Expand Up @@ -52,7 +53,7 @@ public class NoteHookTriggerHandlerImplTest {

@Before
public void setup() {
noteHookTriggerHandler = new NoteHookTriggerHandlerImpl("ci-run");
noteHookTriggerHandler = new NoteHookTriggerHandlerImpl("ci-run", "test");
}

@Test
Expand All @@ -78,6 +79,7 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
.withUpdatedAt(currentDate)
.withUrl("https://gitlab.org/test/merge_requests/1#note_1")
.build())
.withUser(user().withName("test").withUsername("test").withEmail("[email protected]").build())
.withMergeRequest(mergeRequestObjectAttributes().withDescription("[ci-skip]").build())
.build(), true, BranchFilterFactory.newBranchFilter(branchFilterConfig().build(BranchFilterType.All)),
newMergeRequestLabelFilter(null));
Expand Down Expand Up @@ -109,6 +111,11 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
Date currentDate = new Date();
project.setQuietPeriod(0);
noteHookTriggerHandler.handle(project, noteHook()
.withUser(user()
.withName("test")
.withUsername("test")
.withEmail("[email protected]")
.build())
.withObjectAttributes(noteObjectAttributes()
.withId(1)
.withNote("ci-run")
Expand Down Expand Up @@ -152,4 +159,76 @@ public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListen
buildTriggered.block(10000);
assertThat(buildTriggered.isSignaled(), is(true));
}

@Test
public void note_userNotAllowed() throws IOException, InterruptedException, GitAPIException, ExecutionException {
Git.init().setDirectory(tmp.getRoot()).call();
tmp.newFile("test");
Git git = Git.open(tmp.getRoot());
git.add().addFilepattern("test");
RevCommit commit = git.commit().setMessage("test").call();
ObjectId head = git.getRepository().resolve(Constants.HEAD);
String repositoryUrl = tmp.getRoot().toURI().toString();

final OneShotEvent buildTriggered = new OneShotEvent();
FreeStyleProject project = jenkins.createFreeStyleProject();
project.setScm(new GitSCM(repositoryUrl));
project.getBuildersList().add(new TestBuilder() {
@Override
public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
buildTriggered.signal();
return true;
}
});
Date currentDate = new Date();
project.setQuietPeriod(0);
noteHookTriggerHandler.handle(project, noteHook()
.withUser(user()
.withName("user_not_allow")
.withUsername("user_not_allow")
.withEmail("[email protected]")
.build())
.withObjectAttributes(noteObjectAttributes()
.withId(1)
.withNote("ci-run")
.withAuthorId(1)
.withProjectId(1)
.withCreatedAt(currentDate)
.withUpdatedAt(currentDate)
.withUrl("https://gitlab.org/test/merge_requests/1#note_1")
.build())
.withMergeRequest(mergeRequestObjectAttributes()
.withTargetBranch("refs/heads/" + git.nameRev().add(head).call().get(head))
.withState(State.opened)
.withIid(1)
.withTitle("test")
.withTargetProjectId(1)
.withSourceProjectId(1)
.withSourceBranch("feature")
.withTargetBranch("master")
.withLastCommit(commit().withAuthor(user().withName("test").build()).withId(commit.getName()).build())
.withSource(project()
.withName("test")
.withNamespace("test-namespace")
.withHomepage("https://gitlab.org/test")
.withUrl("[email protected]:test.git")
.withSshUrl("[email protected]:test.git")
.withHttpUrl("https://gitlab.org/test.git")
.build())
.withTarget(project()
.withName("test")
.withNamespace("test-namespace")
.withHomepage("https://gitlab.org/test")
.withUrl("[email protected]:test.git")
.withSshUrl("[email protected]:test.git")
.withHttpUrl("https://gitlab.org/test.git")
.withWebUrl("https://gitlab.org/test.git")
.build())
.build())
.build(), true, BranchFilterFactory.newBranchFilter(branchFilterConfig().build(BranchFilterType.All)),
newMergeRequestLabelFilter(null));

buildTriggered.block(10000);
assertThat(buildTriggered.isSignaled(), is(false));
}
}