From 9951a1d818e27736c0d2026bc165bb7e302ca7ec Mon Sep 17 00:00:00 2001 From: Zaki Shaikh Date: Tue, 16 Jun 2026 13:05:10 +0530 Subject: [PATCH] fix(gitlab): discard label removal events on merge requests Label removal events on GitLab merge requests were incorrectly triggering pipeline runs. The hasOnlyLabelsChanged check used an OR condition that matched both additions and removals. Changed to compare current vs previous label count so only label additions are processed. Signed-off-by: Zaki Shaikh Co-Authored-By: Claude Opus 4.6 (1M context) --- pkg/provider/gitlab/detect.go | 14 +++++++- pkg/provider/gitlab/detect_test.go | 53 ++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/pkg/provider/gitlab/detect.go b/pkg/provider/gitlab/detect.go index 3eed737fb4..557110195f 100644 --- a/pkg/provider/gitlab/detect.go +++ b/pkg/provider/gitlab/detect.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" "net/http" + "slices" "github.com/openshift-pipelines/pipelines-as-code/pkg/provider" gitlab "gitlab.com/gitlab-org/api/client-go" @@ -87,13 +88,16 @@ func (v *Provider) Detect(req *http.Request, payload string, logger *zap.Sugared // hasOnlyLabelsChanged checks if the only change in the merge request is to its labels. // This function ensures that other fields remain unchanged. +// Labels changes are treated when they're done standalone and not done with other changes +// for example, if a merge request is marked as draft and then the labels are changed, this is not a valid merge request update. func hasOnlyLabelsChanged(gitEvent *gitlab.MergeEvent) bool { changes := gitEvent.Changes - labelsChanged := len(changes.Labels.Previous) > 0 || len(changes.Labels.Current) > 0 + labelsChanged := isNewLabelAdded(changes.Labels.Previous, changes.Labels.Current) // Only Labels can change — everything else must be zero or nil onlyUpdatedAtOrLabels := labelsChanged && + !changes.Draft.Previous && !changes.Draft.Current && changes.Assignees.Previous == nil && changes.Assignees.Current == nil && changes.Reviewers.Previous == nil && changes.Reviewers.Current == nil && changes.Description.Previous == "" && changes.Description.Current == "" && @@ -108,3 +112,11 @@ func hasOnlyLabelsChanged(gitEvent *gitlab.MergeEvent) bool { return onlyUpdatedAtOrLabels } + +func isNewLabelAdded(previous, current []*gitlab.EventLabel) bool { + return slices.ContainsFunc(current, func(label *gitlab.EventLabel) bool { + return !slices.ContainsFunc(previous, func(prev *gitlab.EventLabel) bool { + return prev.Title == label.Title + }) + }) +} diff --git a/pkg/provider/gitlab/detect_test.go b/pkg/provider/gitlab/detect_test.go index eae0b1b1ab..82b444c9b5 100644 --- a/pkg/provider/gitlab/detect_test.go +++ b/pkg/provider/gitlab/detect_test.go @@ -13,6 +13,12 @@ import ( const largeComment = "/Lorem Ipsum is simply dummy text of the printing and typesetting industry. Lorem Ipsum has been the industry's standard dummy text ever since the 1500s" +func mrEventWithChanges(sample thelp.TEvent, changesJSON string) string { + base := sample.MREventAsJSON("update", "") + idx := strings.LastIndex(base, "}") + return base[:idx] + `,"changes": {` + changesJSON + `}}` +} + func TestProviderDetect(t *testing.T) { sample := thelp.TEvent{ Username: "foo", @@ -173,6 +179,53 @@ func TestProviderDetect(t *testing.T) { isGL: true, processReq: true, }, + { + name: "good/mergeRequest update Event with label addition", + event: mrEventWithChanges(sample, `"labels": {"previous": [], "current": [{"id": 1, "title": "bug"}]}`), + eventType: gitlab.EventTypeMergeRequest, + isGL: true, + processReq: true, + }, + { + name: "bad/mergeRequest update Event with label removal", + event: mrEventWithChanges(sample, `"labels": {"previous": [{"id": 1, "title": "bug"}], "current": []}`), + eventType: gitlab.EventTypeMergeRequest, + isGL: true, + processReq: false, + wantReason: "this 'Merge Request' update event changes are not supported", + }, + { + name: "bad/mergeRequest update Event with label removal partial", + event: mrEventWithChanges(sample, `"labels": {"previous": [{"id": 1, "title": "bug"}, {"id": 2, "title": "feature"}], "current": [{"id": 1, "title": "bug"}]}`), + eventType: gitlab.EventTypeMergeRequest, + isGL: true, + processReq: false, + wantReason: "this 'Merge Request' update event changes are not supported", + }, + { + name: "bad/mergeRequest update Event with label addition on draft to ready transition", + event: mrEventWithChanges(sample, `"labels": {"previous": [], "current": [{"id": 1, "title": "bug"}]}, "draft": {"previous": true, "current": false}`), + eventType: gitlab.EventTypeMergeRequest, + isGL: true, + processReq: false, + wantReason: "this 'Merge Request' update event changes are not supported", + }, + { + name: "bad/mergeRequest update Event with label addition on ready to draft transition", + event: mrEventWithChanges(sample, `"labels": {"previous": [], "current": [{"id": 1, "title": "bug"}]}, "draft": {"previous": false, "current": true}`), + eventType: gitlab.EventTypeMergeRequest, + isGL: true, + processReq: false, + wantReason: "this 'Merge Request' update event changes are not supported", + }, + { + name: "bad/mergeRequest update Event with label addition and description change", + event: mrEventWithChanges(sample, `"labels": {"previous": [], "current": [{"id": 1, "title": "bug"}]}, "description": {"previous": "old", "current": "new"}`), + eventType: gitlab.EventTypeMergeRequest, + isGL: true, + processReq: false, + wantReason: "this 'Merge Request' update event changes are not supported", + }, { name: "good/commit comment /retest command", event: sample.CommitNoteEventAsJSON("/retest", "create", "null"),