jenkinsci / jenkinsci/gitlab-plugin
Accepted Merge Request Events" trigger also fires on merge request updates when triggerOpenMergeRequestOnPush is unset
- Dominant language
- Java
- Stars
- 1.4k
- Forks
- 615
- Avg merge
- 4h 32m
- Merged PRs (30d)
- 10
Description
### Jenkins and plugins versions report
## Bug description
When only **"Accepted Merge Request Events"** is checked in the GitLab trigger configuration (and "Trigger open merge requests" is left to its default/unset value), the job incorrectly triggers on **any merge request update** (e.g. pushing new commits, editing the MR description, etc.), not just when the MR is actually merged/accepted.
## Steps to reproduce
1. Create a Jenkins pipeline/freestyle job with the GitLab trigger enabled
2. Uncheck **"Build when a change is pushed to GitLab"**
3. Uncheck **"Opened Merge Request Events"**
4. Check **"Accepted Merge Request Events"**
5. Leave "Trigger open merge requests on push to source branch" at its default (unspecified)
6. Update a merge request (push a commit, edit description, etc.)
7. **Observed:** the job triggers
8. **Expected:** the job should only trigger when the MR is merged (action=`merge`)
## Root cause
In `MergeRequestHookTriggerHandlerFactory.java`, the field `triggerOpenMergeRequestOnPush` in `GitLabPushTrigger` is declared without a default value, so it is `null` at runtime.
The trigger chain conditions compared against `TriggerOpenMergeRequest.never` **without null-checking**:
```java
// Before fix — null != never evaluates to true
chain.rejectUnless(
triggerOpenMergeRequest != TriggerOpenMergeRequest.never, // null != never → true
of(State.opened, State.updated),
of(Action.update))
.acceptIf(
triggerOpenMergeRequest != TriggerOpenMergeRequest.never, // null != never → true
of(State.updated), of(Action.update))
.acceptIf(
triggerOpenMergeRequest != TriggerOpenMergeRequest.never && triggerOpenMergeRequest != null,
of(State.opened),
of(Action.update))
```
When `triggerOpenMergeRequest` is `null`:
- The **reject rule** for `update` events is **not** added (`rejectUnless(true, ...)` is a no-op)
- An **accept rule** for `state=updated, action=update` **is** added
This causes any MR update event to pass the filter and trigger the build.
Ironically, the third condition already had a null check (`&& triggerOpenMergeRequest != null`), but the first two did not.
## Fix
Extract the three conditions into a single boolean that properly null-guards:
```java
boolean triggerOnOpenMergeRequestEnabled =
triggerOpenMergeRequest != null && triggerOpenMergeRequest != TriggerOpenMergeRequest.never;
```
Then use this boolean consistently for all three chain rules.
A regression test has been added for the exact scenario: `triggerOnAcceptedMergeRequest=true` with default (null) `triggerOpenMergeRequestOnPush`, receiving a `state=updated, action=update` event.
## Affected file
- `src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerFactory.java`
## PR
Incoming
### What Operating System are you using (both controller, and any agents involved in the problem)?
Linux
### Reproduction steps
Already in description
### Expected Results
Already in description
### Actual Results
Already in description
### Anything else?
_No response_
### Are you interested in contributing a fix?
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.