Skip to content

Commit da7f9af

Browse files
committed
feat: introduce deduplicate-pipelineruns setting
Introduce a new 'deduplicate-pipelineruns' setting to prevent duplicate PipelineRuns for the same commit across different event types (e.g., push and pull_request). - Implement fingerprint generation using MD5 hash of commit SHA, PipelineRun name, head branch, and repository name. - Store the fingerprint as a label on the PipelineRun resource. - Check for existing non-completed PipelineRuns with the same fingerprint before creating a new one. - Deprecate the 'skip-push-event-for-pr-commits' setting and add a warning log when it is used. - Update the default ConfigMap and settings documentation. - Add comprehensive unit tests for deduplication logic and deprecation warnings. Signed-off-by: Zaki Shaikh <zashaikh@redhat.com> Assisted-by: Gemini (via Cursor)
1 parent 9d60ff7 commit da7f9af

9 files changed

Lines changed: 145 additions & 5 deletions

File tree

config/302-pac-configmap.yaml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,17 @@ data:
171171
# Default: false
172172
require-ok-to-test-sha: "false"
173173

174+
# Deduplicate PipelineRuns using fingerprinting to prevent duplicate runs for the same commit
175+
# Default: true
176+
deduplicate-pipelineruns: "true"
177+
178+
# DEPRECATED: use deduplicate-pipelineruns instead.
174179
# When enabled, this option prevents duplicate pipeline runs when a commit appears in
175180
# both a push event and a pull request. If a push event comes from a commit that is
176181
# part of an open pull request, the push event will be skipped as it would create
177182
# a duplicate pipeline run.
178-
# Default: true
179-
skip-push-event-for-pr-commits: "true"
183+
# Default: false
184+
skip-push-event-for-pr-commits: "false"
180185

181186
# Configure a custom console here, the driver support custom parameters from
182187
# Repo CR along a few other template variable, see documentation for more

docs/content/docs/install/settings.md

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,23 @@ There are a few things you can configure through the ConfigMap
152152
risk and should be aware of the potential security vulnerabilities.
153153
(only GitHub and Gitea is supported at the moment).
154154

155-
* `skip-push-event-for-pr-commits`
155+
* `deduplicate-pipelineruns`
156+
157+
When enabled, this option uses fingerprinting to prevent duplicate PipelineRuns
158+
when the same commit would trigger multiple events (e.g., both a push event
159+
and a pull request event). Whichever event arrives first will trigger the
160+
PipelineRun, and subsequent events for the same commit and pipeline will be
161+
deduplicated.
162+
163+
The fingerprint is generated from the commit SHA, repository name, and
164+
pipeline name, ensuring that unique pipelines for the same commit are still
165+
allowed to run.
166+
167+
Default: `true`
168+
169+
* `skip-push-event-for-pr-commits` (DEPRECATED)
170+
171+
**DEPRECATED: Use `deduplicate-pipelineruns` instead.**
156172

157173
When enabled, this option prevents duplicate PipelineRuns when a commit appears in
158174
both a push event and a pull request. If a push event comes from a commit that is
@@ -162,7 +178,7 @@ There are a few things you can configure through the ConfigMap
162178
This feature works by checking if a pushed commit SHA exists in any open pull request,
163179
and if so, skipping the push event processing.
164180

165-
Default: `true`
181+
Default: `false`
166182

167183
**Note:** This setting does not apply to git tag push events. Tag push events will always trigger
168184
pipeline runs regardless of whether the tagged commit is part of an open pull request.

pkg/apis/pipelinesascode/keys/keys.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ const (
6060
CancelInProgress = pipelinesascode.GroupName + "/cancel-in-progress"
6161
LogURL = pipelinesascode.GroupName + "/log-url"
6262
ExecutionOrder = pipelinesascode.GroupName + "/execution-order"
63+
PipelineRunFingerprint = pipelinesascode.GroupName + "/fingerprint"
6364
SCMReportingPLRStarted = pipelinesascode.GroupName + "/scm-reporting-plr-started"
6465
// PublicGithubAPIURL default is "https://api.github.com" but it can be overridden by X-GitHub-Enterprise-Host header.
6566
PublicGithubAPIURL = "https://api.github.com"

pkg/params/settings/config.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ type Settings struct {
7373
EnableCancelInProgressOnPullRequests bool `json:"enable-cancel-in-progress-on-pull-requests"`
7474
EnableCancelInProgressOnPush bool `json:"enable-cancel-in-progress-on-push"`
7575

76+
// DeduplicatePipelineRuns uses fingerprinting to prevent duplicate PipelineRuns for the same commit
77+
DeduplicatePipelineRuns bool `json:"deduplicate-pipelineruns" default:"true"` //nolint:tagalign
78+
79+
// SkipPushEventForPRCommits is deprecated, use DeduplicatePipelineRuns instead
7680
SkipPushEventForPRCommits bool `json:"skip-push-event-for-pr-commits" default:"true"` // nolint:tagalign
7781

7882
CustomConsoleName string `json:"custom-console-name"`

pkg/params/settings/config_test.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ func TestSyncConfig(t *testing.T) {
2626
RemoteTasks: true,
2727
MaxKeepRunsUpperLimit: 0,
2828
DefaultMaxKeepRuns: 0,
29+
DeduplicatePipelineRuns: true,
2930
BitbucketCloudCheckSourceIP: true,
3031
BitbucketCloudAdditionalSourceIP: "",
3132
TektonDashboardURL: "",
@@ -59,6 +60,7 @@ func TestSyncConfig(t *testing.T) {
5960
"default-max-keep-runs": "5",
6061
"bitbucket-cloud-check-source-ip": "false",
6162
"bitbucket-cloud-additional-source-ip": "some-ip",
63+
"deduplicate-pipelineruns": "true",
6264
"tekton-dashboard-url": "https://tekton-dashboard",
6365
"auto-configure-new-github-repo": "true",
6466
"auto-configure-repo-namespace-template": "template",
@@ -85,6 +87,7 @@ func TestSyncConfig(t *testing.T) {
8587
RemoteTasks: false,
8688
MaxKeepRunsUpperLimit: 10,
8789
DefaultMaxKeepRuns: 5,
90+
DeduplicatePipelineRuns: true,
8891
BitbucketCloudCheckSourceIP: false,
8992
BitbucketCloudAdditionalSourceIP: "some-ip",
9093
TektonDashboardURL: "https://tekton-dashboard",

pkg/pipelineascode/pipelineascode.go

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@ package pipelineascode
22

33
import (
44
"context"
5+
"crypto/md5" //nolint:gosec
6+
"encoding/hex"
57
"fmt"
8+
"strings"
69
"sync"
710

811
"github.com/openshift-pipelines/pipelines-as-code/pkg/action"
@@ -171,9 +174,48 @@ func (p *PacRun) Run(ctx context.Context) error {
171174
return nil
172175
}
173176

177+
func (p *PacRun) generateFingerprint(pr *tektonv1.PipelineRun, repoName string) string {
178+
prName := pr.GetName()
179+
if prName == "" {
180+
prName = pr.GetGenerateName()
181+
}
182+
183+
headBranch := strings.TrimPrefix(p.event.HeadBranch, "refs/heads/")
184+
headBranch = strings.TrimPrefix(headBranch, "refs/tags/")
185+
186+
data := fmt.Sprintf("%s-%s-%s-%s", p.event.SHA, prName, headBranch, repoName)
187+
hash := md5.Sum([]byte(data)) //nolint:gosec
188+
return hex.EncodeToString(hash[:])[:16]
189+
}
190+
174191
func (p *PacRun) startPR(ctx context.Context, match matcher.Match) (*tektonv1.PipelineRun, error) {
175192
var gitAuthSecretName string
176193

194+
if p.pacInfo.DeduplicatePipelineRuns {
195+
fingerprint := p.generateFingerprint(match.PipelineRun, match.Repo.GetName())
196+
197+
// Check for existing run with same fingerprint
198+
labelSelector := fmt.Sprintf("%s=%s", keys.PipelineRunFingerprint, fingerprint)
199+
existing, err := p.run.Clients.Tekton.TektonV1().PipelineRuns(match.Repo.GetNamespace()).List(ctx, metav1.ListOptions{
200+
LabelSelector: labelSelector,
201+
})
202+
if err == nil {
203+
for i := range existing.Items {
204+
pr := &existing.Items[i]
205+
if !pr.IsDone() {
206+
p.logger.Infof("Skipping duplicate PipelineRun - fingerprint %s already exists: %s",
207+
fingerprint, pr.GetName())
208+
return pr, nil
209+
}
210+
}
211+
}
212+
// Add fingerprint to labels so it gets created with it
213+
if match.PipelineRun.Labels == nil {
214+
match.PipelineRun.Labels = map[string]string{}
215+
}
216+
match.PipelineRun.Labels[keys.PipelineRunFingerprint] = fingerprint
217+
}
218+
177219
// Automatically create a secret with the token to be reused by git-clone task
178220
if p.pacInfo.SecretAutoCreation {
179221
if annotation, ok := match.PipelineRun.GetAnnotations()[keys.GitAuthSecret]; ok {

pkg/pipelineascode/pipelineascode_startpr_test.go

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1069,3 +1069,63 @@ func TestStartPR_ConcurrentWithSameSecret(t *testing.T) {
10691069
assert.Assert(t, attempts >= 1, "Secret creation should have been attempted at least once, got %d attempts", attempts)
10701070
t.Logf("Successfully created %d/%d concurrent PipelineRuns with shared secret (%d creation attempts)", successCount, numConcurrent, attempts)
10711071
}
1072+
1073+
func TestStartPR_Deduplication(t *testing.T) {
1074+
fixture := setupStartPRTestDefault(t)
1075+
defer fixture.teardown()
1076+
1077+
fixture.pacInfo.DeduplicatePipelineRuns = true
1078+
match := createTestMatch(true, nil)
1079+
match.PipelineRun.Annotations[keys.OriginalPRName] = "test-pipeline"
1080+
1081+
// Create an existing PipelineRun with the same fingerprint
1082+
fingerprint := fixture.pac.generateFingerprint(match.PipelineRun, match.Repo.GetName())
1083+
existingPR := &pipelinev1.PipelineRun{
1084+
ObjectMeta: metav1.ObjectMeta{
1085+
Name: "existing-pr",
1086+
Namespace: match.Repo.GetNamespace(),
1087+
Labels: map[string]string{
1088+
keys.PipelineRunFingerprint: fingerprint,
1089+
},
1090+
},
1091+
}
1092+
_, err := fixture.cs.Clients.Tekton.TektonV1().PipelineRuns(match.Repo.GetNamespace()).Create(fixture.ctx, existingPR, metav1.CreateOptions{})
1093+
assert.NilError(t, err)
1094+
1095+
// Attempt to start a new PR with the same fingerprint
1096+
pr, err := fixture.pac.startPR(fixture.ctx, match)
1097+
1098+
assert.NilError(t, err)
1099+
assert.Assert(t, pr != nil)
1100+
assert.Equal(t, pr.GetName(), "existing-pr")
1101+
}
1102+
1103+
func TestStartPR_NoDeduplicationIfDisabled(t *testing.T) {
1104+
fixture := setupStartPRTestDefault(t)
1105+
defer fixture.teardown()
1106+
1107+
fixture.pacInfo.DeduplicatePipelineRuns = false
1108+
match := createTestMatch(true, nil)
1109+
match.PipelineRun.Annotations[keys.OriginalPRName] = "test-pipeline"
1110+
1111+
// Create an existing PipelineRun with the same fingerprint
1112+
fingerprint := fixture.pac.generateFingerprint(match.PipelineRun, match.Repo.GetName())
1113+
existingPR := &pipelinev1.PipelineRun{
1114+
ObjectMeta: metav1.ObjectMeta{
1115+
Name: "existing-pr",
1116+
Namespace: match.Repo.GetNamespace(),
1117+
Labels: map[string]string{
1118+
keys.PipelineRunFingerprint: fingerprint,
1119+
},
1120+
},
1121+
}
1122+
_, err := fixture.cs.Clients.Tekton.TektonV1().PipelineRuns(match.Repo.GetNamespace()).Create(fixture.ctx, existingPR, metav1.CreateOptions{})
1123+
assert.NilError(t, err)
1124+
1125+
// Attempt to start a new PR with the same fingerprint
1126+
pr, err := fixture.pac.startPR(fixture.ctx, match)
1127+
1128+
assert.NilError(t, err)
1129+
assert.Assert(t, pr != nil)
1130+
assert.Assert(t, pr.GetName() != "existing-pr")
1131+
}

pkg/provider/github/parse_payload.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,10 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt
333333
v.Logger.Infof("Processing tag push event for commit %s despite skip-push-events-for-pr-commits being enabled (tag events are excluded from this setting)", sha)
334334
}
335335

336+
if v.pacInfo.SkipPushEventForPRCommits {
337+
v.Logger.Warn("The 'skip-push-event-for-pr-commits' setting is deprecated and will be removed in a future version. Please use 'deduplicate-pipelineruns' instead.")
338+
}
339+
336340
// Only check if the flag is enabled, and there are pull requests associated with this commit, and it's not a tag push event.
337341
if v.pacInfo.SkipPushEventForPRCommits && len(prs) > 0 && !isGitTagEvent {
338342
isPartOfPR, prNumber := v.isCommitPartOfPullRequest(sha, org, repoName, prs)

pkg/provider/github/parse_payload_test.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,7 @@ func TestParsePayLoad(t *testing.T) {
10101010
})
10111011
}
10121012

1013-
logger, _ := logger.GetLogger()
1013+
logger, observer := logger.GetLogger()
10141014
gprovider := Provider{
10151015
ghClient: ghClient,
10161016
Logger: logger,
@@ -1033,6 +1033,11 @@ func TestParsePayLoad(t *testing.T) {
10331033
return
10341034
}
10351035
assert.NilError(t, err)
1036+
1037+
if tt.skipPushEventForPRCommits {
1038+
logMsg := observer.FilterMessageSnippet("The 'skip-push-event-for-pr-commits' setting is deprecated").TakeAll()
1039+
assert.Assert(t, len(logMsg) > 0, "Deprecation warning not found in logs")
1040+
}
10361041
// If shaRet is empty, this is a skip case (push event for PR commit)
10371042
// In this case, ret should be nil
10381043
if tt.shaRet == "" {

0 commit comments

Comments
 (0)