jenkinsci / jenkinsci/gitlab-plugin

Accepted Merge Request Events" trigger also fires on merge request updates when triggerOpenMergeRequestOnPush is unset

Open
#1,872 2 comments 0 reactions 0 assignees View on GitHub
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.