Skip to content

Commit 3f20335

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 eb39d14 commit 3f20335

File tree

10 files changed

+231
-5
lines changed

10 files changed

+231
-5
lines changed

config/302-pac-configmap.yaml

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -166,12 +166,17 @@ data:
166166
# Default: false
167167
require-ok-to-test-sha: "false"
168168

169+
# Deduplicate PipelineRuns using fingerprinting to prevent duplicate runs for the same commit
170+
# Default: true
171+
deduplicate-pipelineruns: "true"
172+
173+
# DEPRECATED: use deduplicate-pipelineruns instead.
169174
# When enabled, this option prevents duplicate pipeline runs when a commit appears in
170175
# both a push event and a pull request. If a push event comes from a commit that is
171176
# part of an open pull request, the push event will be skipped as it would create
172177
# a duplicate pipeline run.
173-
# Default: true
174-
skip-push-event-for-pr-commits: "true"
178+
# Default: false
179+
skip-push-event-for-pr-commits: "false"
175180

176181
# Configure a custom console here, the driver support custom parameters from
177182
# 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, head branch, 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
@@ -71,6 +71,10 @@ type Settings struct {
7171
EnableCancelInProgressOnPullRequests bool `json:"enable-cancel-in-progress-on-pull-requests"`
7272
EnableCancelInProgressOnPush bool `json:"enable-cancel-in-progress-on-push"`
7373

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

7680
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"
@@ -172,9 +175,48 @@ func (p *PacRun) Run(ctx context.Context) error {
172175
return nil
173176
}
174177

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

195+
if p.pacInfo.DeduplicatePipelineRuns {
196+
fingerprint := p.generateFingerprint(match.PipelineRun, match.Repo.GetName())
197+
198+
// Check for existing run with same fingerprint
199+
labelSelector := fmt.Sprintf("%s=%s", keys.PipelineRunFingerprint, fingerprint)
200+
existing, err := p.run.Clients.Tekton.TektonV1().PipelineRuns(match.Repo.GetNamespace()).List(ctx, metav1.ListOptions{
201+
LabelSelector: labelSelector,
202+
})
203+
if err == nil {
204+
for i := range existing.Items {
205+
pr := &existing.Items[i]
206+
if !pr.IsDone() {
207+
p.logger.Infof("Skipping duplicate PipelineRun - fingerprint %s already exists: %s",
208+
fingerprint, pr.GetName())
209+
return pr, nil
210+
}
211+
}
212+
}
213+
// Add fingerprint to labels so it gets created with it
214+
if match.PipelineRun.Labels == nil {
215+
match.PipelineRun.Labels = map[string]string{}
216+
}
217+
match.PipelineRun.Labels[keys.PipelineRunFingerprint] = fingerprint
218+
}
219+
178220
// Automatically create a secret with the token to be reused by git-clone task
179221
if p.pacInfo.SecretAutoCreation {
180222
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
@@ -378,6 +378,10 @@ func (v *Provider) processEvent(ctx context.Context, event *info.Event, eventInt
378378
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)
379379
}
380380

381+
if v.pacInfo.SkipPushEventForPRCommits {
382+
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.")
383+
}
384+
381385
// Only check if the flag is enabled, and there are pull requests associated with this commit, and it's not a tag push event.
382386
if v.pacInfo.SkipPushEventForPRCommits && len(prs) > 0 && !isGitTagEvent {
383387
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
@@ -1037,7 +1037,7 @@ func TestParsePayLoad(t *testing.T) {
10371037
})
10381038
}
10391039

1040-
logger, _ := logger.GetLogger()
1040+
logger, observer := logger.GetLogger()
10411041
gprovider := Provider{
10421042
ghClient: ghClient,
10431043
Logger: logger,
@@ -1060,6 +1060,11 @@ func TestParsePayLoad(t *testing.T) {
10601060
return
10611061
}
10621062
assert.NilError(t, err)
1063+
1064+
if tt.skipPushEventForPRCommits {
1065+
logMsg := observer.FilterMessageSnippet("The 'skip-push-event-for-pr-commits' setting is deprecated").TakeAll()
1066+
assert.Assert(t, len(logMsg) > 0, "Deprecation warning not found in logs")
1067+
}
10631068
// If shaRet is empty, this is a skip case (push event for PR commit)
10641069
// In this case, ret should be nil
10651070
if tt.shaRet == "" {
Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
//go:build e2e
2+
3+
package test
4+
5+
import (
6+
"context"
7+
"fmt"
8+
"net/http"
9+
"regexp"
10+
"testing"
11+
12+
tgithub "github.com/openshift-pipelines/pipelines-as-code/test/pkg/github"
13+
"github.com/openshift-pipelines/pipelines-as-code/test/pkg/payload"
14+
twait "github.com/openshift-pipelines/pipelines-as-code/test/pkg/wait"
15+
"github.com/tektoncd/pipeline/pkg/names"
16+
"gotest.tools/v3/assert"
17+
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
18+
)
19+
20+
func TestGithubDeduplicatePipelineRuns(t *testing.T) {
21+
targetNS := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-ns")
22+
23+
ctx := context.TODO()
24+
ctx, runcnx, opts, ghcnx, err := tgithub.Setup(ctx, false, false)
25+
assert.NilError(t, err)
26+
27+
yamlEntries := map[string]string{}
28+
yamlEntries[".tekton/pipelinerun-match-all-branch.yaml"] = "testdata/pipelinerun.yaml"
29+
30+
repoinfo, resp, err := ghcnx.Client().Repositories.Get(ctx, opts.Organization, opts.Repo)
31+
assert.NilError(t, err)
32+
if resp != nil && resp.StatusCode == http.StatusNotFound {
33+
t.Errorf("Repository %s not found in %s", opts.Organization, opts.Repo)
34+
}
35+
36+
err = tgithub.CreateCRD(ctx, t, repoinfo, runcnx, opts, targetNS)
37+
assert.NilError(t, err)
38+
39+
entries, err := payload.GetEntries(yamlEntries, targetNS, "*", "pull_request, push", map[string]string{})
40+
assert.NilError(t, err)
41+
42+
targetRefName := fmt.Sprintf("refs/heads/%s",
43+
names.SimpleNameGenerator.RestrictLengthWithRandomSuffix("pac-e2e-test"))
44+
45+
sha, vref, err := tgithub.PushFilesToRef(ctx, ghcnx.Client(), "Test Github Deduplicate PipelineRuns", repoinfo.GetDefaultBranch(), targetRefName,
46+
opts.Organization, opts.Repo, entries)
47+
assert.NilError(t, err)
48+
49+
runcnx.Clients.Log.Infof("Commit %s has been created and pushed to %s", sha, vref.GetURL())
50+
number, err := tgithub.PRCreate(ctx, runcnx, ghcnx, opts.Organization,
51+
opts.Repo, targetRefName, repoinfo.GetDefaultBranch(), "Test Github Deduplicate PipelineRuns")
52+
assert.NilError(t, err)
53+
54+
g := &tgithub.PRTest{
55+
PRNumber: number,
56+
SHA: sha,
57+
TargetNamespace: targetNS,
58+
Logger: runcnx.Clients.Log,
59+
Provider: ghcnx,
60+
Options: opts,
61+
TargetRefName: targetRefName,
62+
Cnx: runcnx,
63+
}
64+
65+
defer g.TearDown(ctx, t)
66+
67+
runcnx.Clients.Log.Infof("Pull request %d has been created", number)
68+
err = twait.UntilPipelineRunCreated(ctx, runcnx.Clients, twait.Opts{
69+
RepoName: targetNS,
70+
Namespace: targetNS,
71+
MinNumberStatus: 1,
72+
PollTimeout: twait.DefaultTimeout,
73+
TargetSHA: sha,
74+
})
75+
assert.NilError(t, err)
76+
77+
// but here we can't guarantee that for which event PipelineRun is created because we don't which
78+
// event is received first on controller either push or pull_request.
79+
prs, err := runcnx.Clients.Tekton.TektonV1().PipelineRuns(targetNS).List(ctx, metav1.ListOptions{})
80+
assert.NilError(t, err)
81+
assert.Assert(t, len(prs.Items) == 1)
82+
83+
maxLines := int64(50)
84+
err = twait.RegexpMatchingInControllerLog(ctx, runcnx, *regexp.MustCompile("Skipping duplicate PipelineRun"), 10, "controller", &maxLines)
85+
assert.NilError(t, err)
86+
}

0 commit comments

Comments
 (0)